XDP hardware hints discussion mail archive
 help / color / mirror / Atom feed
From: "Toke Høiland-Jørgensen" <toke@redhat.com>
To: Alexander Lobakin <alexandr.lobakin@intel.com>
Cc: "John Fastabend" <john.fastabend@gmail.com>,
	"Alexei Starovoitov" <ast@kernel.org>,
	"Daniel Borkmann" <daniel@iogearbox.net>,
	"Andrii Nakryiko" <andrii@kernel.org>,
	"Larysa Zaremba" <larysa.zaremba@intel.com>,
	"Michal Swiatkowski" <michal.swiatkowski@linux.intel.com>,
	"Jesper Dangaard Brouer" <hawk@kernel.org>,
	"Björn Töpel" <bjorn@kernel.org>,
	"Magnus Karlsson" <magnus.karlsson@intel.com>,
	"Maciej Fijalkowski" <maciej.fijalkowski@intel.com>,
	"Jonathan Lemon" <jonathan.lemon@gmail.com>,
	"Lorenzo Bianconi" <lorenzo@kernel.org>,
	"David S. Miller" <davem@davemloft.net>,
	"Eric Dumazet" <edumazet@google.com>,
	"Jakub Kicinski" <kuba@kernel.org>,
	"Paolo Abeni" <pabeni@redhat.com>,
	"Jesse Brandeburg" <jesse.brandeburg@intel.com>,
	"Yajun Deng" <yajun.deng@linux.dev>,
	"Willem de Bruijn" <willemb@google.com>,
	bpf@vger.kernel.org, netdev@vger.kernel.org,
	linux-kernel@vger.kernel.org, xdp-hints@xdp-project.net
Subject: [xdp-hints] Re: [PATCH RFC bpf-next 00/52] bpf, xdp: introduce and use Generic Hints/metadata
Date: Mon, 04 Jul 2022 19:14:04 +0200	[thread overview]
Message-ID: <87a69o94wz.fsf@toke.dk> (raw)
In-Reply-To: <20220704154440.7567-1-alexandr.lobakin@intel.com>

Alexander Lobakin <alexandr.lobakin@intel.com> writes:

> From: Toke H??iland-J??rgensen <toke@redhat.com>
> Date: Wed, 29 Jun 2022 15:43:05 +0200
>
>> John Fastabend <john.fastabend@gmail.com> writes:
>> 
>> > Alexander Lobakin wrote:
>> >> This RFC is to give the whole picture. It will most likely be split
>> >> onto several series, maybe even merge cycles. See the "table of
>> >> contents" below.
>> >
>> > Even for RFC its a bit much. Probably improve the summary
>> > message here as well I'm still not clear on the overall
>> > architecture so not sure I want to dig into patches.
>> 
>> +1 on this, and piggybacking on your comment to chime in on the general
>> architecture.
>> 
>> >> Now, a NIC driver, or even a SmartNIC itself, can put those params
>> >> there in a well-defined format. The format is fixed, but can be of
>> >> several different types represented by structures, which definitions
>> >> are available to the kernel, BPF programs and the userland.
>> >
>> > I don't think in general the format needs to be fixed.
>> 
>> No, that's the whole point of BTF: it's not supposed to be UAPI, we'll
>> use CO-RE to enable dynamic formats...
>> 
>> [...]
>> 
>> >> It is fixed due to it being almost a UAPI, and the exact format can
>> >> be determined by reading the last 10 bytes of metadata. They contain
>> >> a 2-byte magic ID to not confuse it with a non-compatible meta and
>> >> a 8-byte combined BTF ID + type ID: the ID of the BTF where this
>> >> structure is defined and the ID of that definition inside that BTF.
>> >> Users can obtain BTF IDs by structure types using helpers available
>> >> in the kernel, BPF (written by the CO-RE/verifier) and the userland
>> >> (libbpf -> kernel call) and then rely on those ID when reading data
>> >> to make sure whether they support it and what to do with it.
>> >> Why separate magic and ID? The idea is to make different formats
>> >> always contain the basic/"generic" structure embedded at the end.
>> >> This way we can still benefit in purely generic consumers (like
>> >> cpumap) while providing some "extra" data to those who support it.
>> >
>> > I don't follow this. If you have a struct in your driver name it
>> > something obvious, ice_xdp_metadata. If I understand things
>> > correctly just dump the BTF for the driver, extract the
>> > struct and done you can use CO-RE reads. For the 'fixed' case
>> > this looks easy. And I don't think you even need a patch for this.
>> 
>> ...however as we've discussed previously, we do need a bit of
>> infrastructure around this. In particular, we need to embed the embed
>> the BTF ID into the metadata itself so BPF can do runtime disambiguation
>> between different formats (and add the right CO-RE primitives to make
>> this easy). This is for two reasons:
>> 
>> - The metadata might be different per-packet (e.g., PTP packets with
>>   timestamps interleaved with bulk data without them)
>> 
>> - With redirects we may end up processing packets from different devices
>>   in a single XDP program (in devmap or cpumap, or on a veth) so we need
>>   to be able to disambiguate at runtime.
>> 
>> So I think the part of the design that puts the BTF ID into the end of
>> the metadata struct is sound; however, the actual format doesn't have to
>> be fixed, we can use CO-RE to pick out the bits that a given BPF program
>> needs; we just need a convention for how drivers report which format(s)
>> they support. Which we should also agree on (and add core infrastructure
>> around) so each driver doesn't go around inventing their own
>> conventions.
>> 
>> >> The enablement of this feature is controlled on attaching/replacing
>> >> XDP program on an interface with two new parameters: that combined
>> >> BTF+type ID and metadata threshold.
>> >> The threshold specifies the minimum frame size which a driver (or
>> >> NIC) should start composing metadata from. It is introduced instead
>> >> of just false/true flag due to that often it's not worth it to spend
>> >> cycles to fetch all that data for such small frames: let's say, it
>> >> can be even faster to just calculate checksums for them on CPU
>> >> rather than touch non-coherent DMA zone. Simple XDP_DROP case loses
>> >> 15 Mpps on 64 byte frames with enabled metadata, threshold can help
>> >> mitigate that.
>> >
>> > I would put this in the bonus category. Can you do the simple thing
>> > above without these extra bits and then add them later. Just
>> > pick some overly conservative threshold to start with.
>> 
>> Yeah, I'd agree this kind of configuration is something that can be
>> added later, and also it's sort of orthogonal to the consumption of the
>> metadata itself.
>> 
>> Also, tying this configuration into the loading of an XDP program is a
>> terrible interface: these are hardware configuration options, let's just
>> put them into ethtool or 'ip link' like any other piece of device
>> configuration.
>
> I don't believe it fits there, especially Ethtool. Ethtool is for
> hardware configuration, XDP/AF_XDP is 95% software stuff (apart from
> offload bits which is purely NFP's for now).

But XDP-hints is about consuming hardware features. When you're
configuring which metadata items you want, you're saying "please provide
me with these (hardware) features". So ethtool is an excellent place to
do that :)

> I follow that way:
>
> 1) you pick a program you want to attach;
> 2) usually they are written for special needs and usecases;
> 3) so most likely that program will be tied with metadata/driver/etc
>    in some way;
> 4) so you want to enable Hints of a particular format primarily for
>    this program and usecase, same with threshold and everything
>    else.
>
> Pls explain how you see it, I might be wrong for sure.

As above: XDP hints is about giving XDP programs (and AF_XDP consumers)
access to metadata that is not currently available. Tying the lifetime
of that hardware configuration (i.e., which information to provide) to
the lifetime of an XDP program is not a good interface: for one thing,
how will it handle multiple programs? What about when XDP is not used at
all but you still want to configure the same features?

In addition, in every other case where we do dynamic data access (with
CO-RE) the BPF program is a consumer that modifies itself to access the
data provided by the kernel. I get that this is harder to achieve for
AF_XDP, but then let's solve that instead of making a totally
inconsistent interface for XDP.

I'm as excited as you about the prospect of having totally programmable
hardware where you can just specify any arbitrary metadata format and
it'll provide that for you. But that is an orthogonal feature: let's
start with creating a dynamic interface for consuming the (static)
hardware features we already have, and then later we can have a separate
interface for configuring more dynamic hardware features. XDP-hints is
about adding this consumption feature in a way that's sufficiently
dynamic that we can do the other (programmable hardware) thing on top
later...

-Toke


  parent reply	other threads:[~2022-07-04 17:14 UTC|newest]

Thread overview: 98+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-06-28 19:47 [xdp-hints] " Alexander Lobakin
2022-06-28 19:47 ` [xdp-hints] [PATCH RFC bpf-next 01/52] libbpf: factor out BTF loading from load_module_btfs() Alexander Lobakin
2022-06-28 19:47 ` [xdp-hints] [PATCH RFC bpf-next 02/52] libbpf: try to load vmlinux BTF from the kernel first Alexander Lobakin
2022-06-28 19:47 ` [xdp-hints] [PATCH RFC bpf-next 03/52] libbpf: add function to get the pair BTF ID + type ID for a given type Alexander Lobakin
2022-06-28 19:47 ` [xdp-hints] [PATCH RFC bpf-next 04/52] libbpf: patch module BTF ID into BPF insns Alexander Lobakin
2022-06-28 19:47 ` [xdp-hints] [PATCH RFC bpf-next 05/52] net, xdp: decouple XDP code from the core networking code Alexander Lobakin
2022-06-28 19:47 ` [xdp-hints] [PATCH RFC bpf-next 06/52] bpf: pass a pointer to union bpf_attr to bpf_link_ops::update_prog() Alexander Lobakin
2022-06-28 19:47 ` [xdp-hints] [PATCH RFC bpf-next 07/52] net, xdp: remove redundant arguments from dev_xdp_{at,de}tach_link() Alexander Lobakin
2022-06-28 19:47 ` [xdp-hints] [PATCH RFC bpf-next 08/52] net, xdp: factor out XDP install arguments to a separate structure Alexander Lobakin
2022-06-28 19:47 ` [xdp-hints] [PATCH RFC bpf-next 09/52] net, xdp: add ability to specify BTF ID for XDP metadata Alexander Lobakin
2022-06-28 19:47 ` [xdp-hints] [PATCH RFC bpf-next 10/52] net, xdp: add ability to specify frame size threshold " Alexander Lobakin
2022-06-28 19:47 ` [xdp-hints] [PATCH RFC bpf-next 11/52] libbpf: factor out __bpf_set_link_xdp_fd_replace() args into a struct Alexander Lobakin
2022-06-28 19:47 ` [xdp-hints] [PATCH RFC bpf-next 12/52] libbpf: add ability to set the BTF/type ID on setting XDP prog Alexander Lobakin
2022-06-28 19:47 ` [xdp-hints] [PATCH RFC bpf-next 13/52] libbpf: add ability to set the meta threshold " Alexander Lobakin
2022-06-28 19:47 ` [xdp-hints] [PATCH RFC bpf-next 14/52] libbpf: pass &bpf_link_create_opts directly to bpf_program__attach_fd() Alexander Lobakin
2022-06-28 19:47 ` [xdp-hints] [PATCH RFC bpf-next 15/52] libbpf: add bpf_program__attach_xdp_opts() Alexander Lobakin
2022-06-28 19:47 ` [xdp-hints] [PATCH RFC bpf-next 16/52] selftests/bpf: expand xdp_link to check that setting meta opts works Alexander Lobakin
2022-06-28 19:47 ` [xdp-hints] [PATCH RFC bpf-next 17/52] samples/bpf: pass a struct to sample_install_xdp() Alexander Lobakin
2022-06-28 19:47 ` [xdp-hints] [PATCH RFC bpf-next 18/52] samples/bpf: add ability to specify metadata threshold Alexander Lobakin
2022-06-28 19:47 ` [xdp-hints] [PATCH RFC bpf-next 19/52] stddef: make __struct_group() UAPI C++-friendly Alexander Lobakin
2022-06-28 19:47 ` [xdp-hints] [PATCH RFC bpf-next 20/52] net, xdp: move XDP metadata helpers into new xdp_meta.h Alexander Lobakin
2022-06-28 19:47 ` [xdp-hints] [PATCH RFC bpf-next 21/52] net, xdp: allow metadata > 32 Alexander Lobakin
2022-06-28 19:47 ` [xdp-hints] [PATCH RFC bpf-next 22/52] net, skbuff: add ability to skip skb metadata comparison Alexander Lobakin
2022-06-28 19:47 ` [xdp-hints] [PATCH RFC bpf-next 23/52] net, skbuff: constify the @skb argument of skb_hwtstamps() Alexander Lobakin
2022-06-28 19:47 ` [xdp-hints] [PATCH RFC bpf-next 24/52] bpf, xdp: declare generic XDP metadata structure Alexander Lobakin
2022-06-28 19:47 ` [xdp-hints] [PATCH RFC bpf-next 25/52] net, xdp: add basic generic metadata accessors Alexander Lobakin
2022-06-28 19:47 ` [xdp-hints] [PATCH RFC bpf-next 26/52] bpf, btf: add a pair of function to work with the BTF ID + type ID pair Alexander Lobakin
2022-06-28 19:47 ` [xdp-hints] [PATCH RFC bpf-next 27/52] net, xdp: add &sk_buff <-> &xdp_meta_generic converters Alexander Lobakin
2022-06-28 19:47 ` [xdp-hints] [PATCH RFC bpf-next 28/52] net, xdp: prefetch data a bit when building an skb from an &xdp_frame Alexander Lobakin
2022-06-28 19:47 ` [xdp-hints] [PATCH RFC bpf-next 29/52] net, xdp: try to fill skb fields when converting " Alexander Lobakin
2022-06-28 19:47 ` [xdp-hints] [PATCH RFC bpf-next 30/52] net, gro: decouple GRO from the NAPI layer Alexander Lobakin
2022-06-28 19:47 ` [xdp-hints] [PATCH RFC bpf-next 31/52] net, gro: expose some GRO API to use outside of NAPI Alexander Lobakin
2022-06-28 19:47 ` [xdp-hints] [PATCH RFC bpf-next 32/52] bpf, cpumap: switch to GRO from netif_receive_skb_list() Alexander Lobakin
2024-08-07 20:38   ` [xdp-hints] " Daniel Xu
2024-08-08  4:54     ` Lorenzo Bianconi
2024-08-08 11:57       ` Alexander Lobakin
2024-08-08 17:22         ` Lorenzo Bianconi
2024-08-08 20:52         ` Daniel Xu
2024-08-09 10:02           ` Jesper Dangaard Brouer
2024-08-09 12:20           ` Alexander Lobakin
2024-08-09 12:45             ` Toke Høiland-Jørgensen
2024-08-09 12:56               ` Alexander Lobakin
2024-08-09 13:42                 ` Toke Høiland-Jørgensen
2024-08-10  0:54                   ` Martin KaFai Lau
2024-08-10  8:02                   ` Lorenzo Bianconi
2024-08-13  1:33             ` Jakub Kicinski
2024-08-13  9:51               ` Jesper Dangaard Brouer
2024-08-10  8:00         ` Lorenzo Bianconi
2024-08-13 14:09         ` Alexander Lobakin
2024-08-13 14:54           ` Toke Høiland-Jørgensen
2024-08-13 15:57             ` Jesper Dangaard Brouer
2024-08-19 14:50               ` Alexander Lobakin
2024-08-21  0:29                 ` Daniel Xu
2024-08-21 13:16                   ` Alexander Lobakin
2024-08-21 16:36                     ` Daniel Xu
2024-08-13 16:14           ` Lorenzo Bianconi
2024-08-13 16:27           ` Lorenzo Bianconi
2024-08-13 16:31             ` Alexander Lobakin
2024-08-08 20:44       ` Daniel Xu
2024-08-09  9:32         ` Jesper Dangaard Brouer
2022-06-28 19:47 ` [xdp-hints] [PATCH RFC bpf-next 33/52] bpf, cpumap: add option to set a timeout for deferred flush Alexander Lobakin
2022-06-28 19:47 ` [xdp-hints] [PATCH RFC bpf-next 34/52] samples/bpf: add 'timeout' option to xdp_redirect_cpu Alexander Lobakin
2022-06-28 19:47 ` [xdp-hints] [PATCH RFC bpf-next 35/52] net, skbuff: introduce napi_skb_cache_get_bulk() Alexander Lobakin
2022-06-28 19:47 ` [xdp-hints] [PATCH RFC bpf-next 36/52] bpf, cpumap: switch to napi_skb_cache_get_bulk() Alexander Lobakin
2022-06-28 19:47 ` [xdp-hints] [PATCH RFC bpf-next 37/52] rcupdate: fix access helpers for incomplete struct pointers on GCC < 10 Alexander Lobakin
2022-06-28 19:47 ` [xdp-hints] [PATCH RFC bpf-next 38/52] net, xdp: remove unused xdp_attachment_info::flags Alexander Lobakin
2022-06-28 19:47 ` [xdp-hints] [PATCH RFC bpf-next 39/52] net, xdp: make &xdp_attachment_info a bit more useful in drivers Alexander Lobakin
2022-06-28 19:48 ` [xdp-hints] [PATCH RFC bpf-next 40/52] net, xdp: add an RCU version of xdp_attachment_setup() Alexander Lobakin
2022-06-28 19:48 ` [xdp-hints] [PATCH RFC bpf-next 41/52] net, xdp: replace net_device::xdp_prog pointer with &xdp_attachment_info Alexander Lobakin
2022-06-28 19:48 ` [xdp-hints] [PATCH RFC bpf-next 42/52] net, xdp: shortcut skb->dev in bpf_prog_run_generic_xdp() Alexander Lobakin
2022-06-28 19:48 ` [xdp-hints] [PATCH RFC bpf-next 43/52] net, xdp: build XDP generic metadata on Generic (skb) XDP path Alexander Lobakin
2022-06-28 19:48 ` [xdp-hints] [PATCH RFC bpf-next 44/52] net, ice: allow XDP prog hot-swapping Alexander Lobakin
2022-06-28 19:48 ` [xdp-hints] [PATCH RFC bpf-next 45/52] net, ice: consolidate all skb fields processing Alexander Lobakin
2022-06-28 19:48 ` [xdp-hints] [PATCH RFC bpf-next 46/52] net, ice: use an onstack &xdp_meta_generic_rx to store HW frame info Alexander Lobakin
2022-06-28 19:48 ` [xdp-hints] [PATCH RFC bpf-next 47/52] net, ice: build XDP generic metadata Alexander Lobakin
2022-06-28 19:48 ` [xdp-hints] [PATCH RFC bpf-next 48/52] libbpf: compress Endianness ops with a macro Alexander Lobakin
2022-06-28 19:48 ` [xdp-hints] [PATCH RFC bpf-next 49/52] libbpf: add LE <--> CPU conversion helpers Alexander Lobakin
2022-06-28 19:48 ` [xdp-hints] [PATCH RFC bpf-next 50/52] libbpf: introduce a couple memory access helpers Alexander Lobakin
2022-06-28 19:48 ` [xdp-hints] [PATCH RFC bpf-next 51/52] selftests/bpf: fix using test_xdp_meta BPF prog via skeleton infra Alexander Lobakin
2022-06-28 19:48 ` [xdp-hints] [PATCH RFC bpf-next 52/52] selftests/bpf: add XDP Generic Hints selftest Alexander Lobakin
2022-06-29  6:15 ` [xdp-hints] Re: [PATCH RFC bpf-next 00/52] bpf, xdp: introduce and use Generic Hints/metadata John Fastabend
2022-06-29 13:43   ` Toke Høiland-Jørgensen
2022-07-04 15:44     ` Alexander Lobakin
2022-07-04 17:13       ` Jesper Dangaard Brouer
2022-07-05 14:38         ` Alexander Lobakin
2022-07-05 19:08           ` Daniel Borkmann
2022-07-04 17:14       ` Toke Høiland-Jørgensen [this message]
2022-07-05 15:41         ` Alexander Lobakin
2022-07-05 18:51           ` Toke Høiland-Jørgensen
2022-07-06 13:50             ` Alexander Lobakin
2022-07-06 23:22               ` Toke Høiland-Jørgensen
2022-07-07 11:41                 ` Jesper Dangaard Brouer
2022-07-12 10:33                 ` Magnus Karlsson
2022-07-12 14:14                   ` Jesper Dangaard Brouer
2022-07-15 11:11                     ` Magnus Karlsson
2022-06-29 17:56   ` Zvi Effron
2022-06-30  7:39     ` Magnus Karlsson
2022-07-04 15:31   ` Alexander Lobakin

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

  List information: https://lists.xdp-project.net/postorius/lists/xdp-hints.xdp-project.net/

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=87a69o94wz.fsf@toke.dk \
    --to=toke@redhat.com \
    --cc=alexandr.lobakin@intel.com \
    --cc=andrii@kernel.org \
    --cc=ast@kernel.org \
    --cc=bjorn@kernel.org \
    --cc=bpf@vger.kernel.org \
    --cc=daniel@iogearbox.net \
    --cc=davem@davemloft.net \
    --cc=edumazet@google.com \
    --cc=hawk@kernel.org \
    --cc=jesse.brandeburg@intel.com \
    --cc=john.fastabend@gmail.com \
    --cc=jonathan.lemon@gmail.com \
    --cc=kuba@kernel.org \
    --cc=larysa.zaremba@intel.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=lorenzo@kernel.org \
    --cc=maciej.fijalkowski@intel.com \
    --cc=magnus.karlsson@intel.com \
    --cc=michal.swiatkowski@linux.intel.com \
    --cc=netdev@vger.kernel.org \
    --cc=pabeni@redhat.com \
    --cc=willemb@google.com \
    --cc=xdp-hints@xdp-project.net \
    --cc=yajun.deng@linux.dev \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox