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: Thu, 07 Jul 2022 01:22:55 +0200 [thread overview]
Message-ID: <87edyxaks0.fsf@toke.dk> (raw)
In-Reply-To: <20220706135023.1464979-1-alexandr.lobakin@intel.com>
Alexander Lobakin <alexandr.lobakin@intel.com> writes:
> From: Toke H??iland-J??rgensen <toke@redhat.com>
> Date: Tue, 05 Jul 2022 20:51:14 +0200
>
>> Alexander Lobakin <alexandr.lobakin@intel.com> writes:
>>
>> [... snipping a bit of context here ...]
>>
>> >> >> 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 :)
>> >
>> > With Ethtool you configure the hardware, e.g. it won't strip VLAN
>> > tags if you disable rx-cvlan-stripping. With configuring metadata
>> > you only tell what you want to see there, don't you?
>>
>> Ah, I think we may be getting closer to identifying the disconnect
>> between our way of thinking about this!
>>
>> In my mind, there's no separate "configuration of the metadata" step.
>> You simply tell the hardware what features you want (say, "enable
>> timestamps and VLAN offload"), and the driver will then provide the
>> information related to these features in the metadata area
>> unconditionally. All XDP hints is about, then, is a way for the driver
>> to inform the rest of the system how that information is actually laid
>> out in the metadata area.
>>
>> Having a separate configuration knob to tell the driver "please lay out
>> these particular bits of metadata this way" seems like a totally
>> unnecessary (and quite complicated) feature to have when we can just let
>> the driver decide and use CO-RE to consume it?
>
> Magnus (he's currently on vacation) told me it would be useful for
> AF_XDP to enable/disable particular metadata, at least from perf
> perspective. Let's say, just fetching of one "checksum ok" bit in
> the driver is faster than walking through all the descriptor words
> and driver logics (i.e. there's several hundred locs in ice which
> just parse descriptor data and build an skb or metadata from it).
> But if we would just enable/disable corresponding features through
> Ethtool, that would hurt XDP_PASS. Maybe it's a bad example, but
> what if I want to have only RSS hash in the metadata (and don't
> want to spend cycles on parsing the rest), but at the same time
> still want skb path to have checksum status to not die at CPU
> checksum calculation?
Hmm, so this feels a little like a driver-specific optimisation? I.e.,
my guess is that not all drivers have a measurable overhead for pulling
out the metadata. Also, once the XDP metadata bits are in place, we can
move in the direction of building SKBs from the same source, so I'm not
sure it's a good idea to assume that the XDP metadata is separate from
what the stack consumes...
In any case, if such an optimisation does turn out to be useful, we can
add it later (backed by rigorous benchmarks, of course), so I think we
can still start with the simple case and iterate from there?
>> >> > 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
>> >
>> > Multiple progs is stuff I didn't cover, but will do later (as you
>> > all say to me, "let's start with something simple" :)). Aaaand
>> > multiple XDP progs (I'm not talking about attaching progs in
>> > differeng modes) is not a kernel feature, rather a libpf feature,
>> > so I believe it should be handled there later...
>>
>> Right, but even if we don't *implement* it straight away we still need
>> to take it into consideration in the design. And expecting libxdp to
>> arbitrate between different XDP programs' metadata formats sounds like a
>> royal PITA :)
>>
>> >> all but you still want to configure the same features?
>> >
>> > What's the point of configuring metadata when there are no progs
>> > attached? To configure it once and not on every prog attach? I'm
>> > not saying I don't like it, just want to clarify.
>>
>> See above: you turn on the features because you want the stack to
>> consume them.
>>
>> > Maybe I need opinions from some more people, just to have an
>> > overview of how most of folks see it and would like to configure
>> > it. 'Cause I heard from at least one of the consumers that
>> > libpf API is a perfect place for Hints to him :)
>>
>> Well, as a program author who wants to consume hints, you'd use
>> lib{bpf,xdp} APIs to do so (probably in the form of suitable CO-RE
>> macros)...
>>
>> >> 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 also see CO-RE more fitting and convenient way to use them, but
>> > didn't manage to solve two things:
>> >
>> > 1) AF_XDP programs, so what to do with them? Prepare patches for
>> > LLVM to make it able to do CO-RE on AF_XDP program load? Or
>> > just hardcode them for particular usecases and NICs? What about
>> > "general-purpose" programs?
>>
>> You provide a library to read the fields. Jesper actually already
>> implemented this, did you look at his code?
>>
>> https://github.com/xdp-project/bpf-examples/tree/master/AF_XDP-interaction
>>
>> It basically builds a lookup table at load-time using BTF information
>> from the kernel, keyed on BTF ID and field name, resolving them into
>> offsets. It's not quite the zero-overhead of CO-RE, but it's fairly
>> close and can be improved upon (CO-RE for userspace being one way of
>> doing that).
>
> Aaaah, sorry, I completely missed that. I thought of something
> similar as well, but then thought "variable field offsets, that
> would annihilate optimization and performance", and our Xsk team
> is super concerned about performance hits when using Hints.
>
>>
>> > And if hardcode, what's the point then to do Generic Hints at
>> > all? Then all it needs is making driver building some meta in
>> > front of frames via on-off button and that's it? Why BTF ID in
>> > the meta then if consumers will access meta hardcoded (via CO-RE
>> > or literally hardcoded, doesn't matter)?
>>
>> You're quite right, we could probably implement all the access to
>> existing (fixed) metadata without using any BTF at all - just define a
>> common struct and some flags to designate which fields are set. In my
>> mind, there are a couple of reasons for going the BTF route instead:
>>
>> - We can leverage CO-RE to get close to optimal efficiency in field
>> access.
>>
>> and, more importantly:
>>
>> - It's infinitely extensible. With the infrastructure in place to make
>> it really easy to consume metadata described by BTF, we lower the bar
>> for future innovation in hardware offloads. Both for just adding new
>> fixed-function stuff to hardware, but especially for fully
>> programmable hardware.
>
> Agree :) That libxdp lookup translator fixed lots of stuff in my
> mind.
Great! Looks like we're slowly converging towards a shared
understanding, then! :)
>> > 2) In-kernel metadata consumers? Also do CO-RE? Otherwise, with no
>> > generic metadata structure they won't be able to benefit from
>> > Hints. But I guess we still need to provide kernel with meta?
>> > Or no?
>>
>> In the short term, I think the "generic structure" approach is fine for
>> leveraging this in the stack. Both your and Jesper's series include
>> this, and I think that's totally fine. Longer term, if it turns out to
>> be useful to have something more dynamic for the stack consumption as
>> well, we could extend it to be CO-RE based as well (most likely by
>> having the stack load a "translator" BPF program or something along
>> those lines).
>
> Oh, that translator prog sounds nice BTW!
Yeah, it's only a rough idea Jesper and I discussed at some point, but I
think it could have potential (see also point above re: making XDP hints
*the* source of metadata for the whole stack; wouldn't it be nice if
drivers didn't have to deal with the intricacies of assembling SKBs?).
-Toke
next prev parent reply other threads:[~2022-07-06 23:23 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
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 [this message]
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=87edyxaks0.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