From: "Toke Høiland-Jørgensen" <toke@redhat•com>
To: Stanislav Fomichev <stfomichev@gmail•com>,
Arthur Fabre <arthur@arthurfabre•com>
Cc: netdev@vger•kernel.org, bpf@vger•kernel.org,
jakub@cloudflare•com, hawk@kernel•org, yan@cloudflare•com,
jbrandeburg@cloudflare•com, lbiancon@redhat•com, ast@kernel•org,
kuba@kernel•org, edumazet@google•com
Subject: Re: [PATCH RFC bpf-next v2 10/17] bnxt: Propagate trait presence to skb
Date: Thu, 24 Apr 2025 11:49:38 +0200 [thread overview]
Message-ID: <87tt6d7utp.fsf@toke.dk> (raw)
In-Reply-To: <aAl7lz88_8QohyxK@mini-arch>
Stanislav Fomichev <stfomichev@gmail•com> writes:
> On 04/23, Arthur Fabre wrote:
>> On Wed Apr 23, 2025 at 6:36 PM CEST, Stanislav Fomichev wrote:
>> > On 04/22, Arthur Fabre wrote:
>> > > Call the common xdp_buff_update_skb() helper.
>> > >
>> > > Signed-off-by: Arthur Fabre <arthur@arthurfabre•com>
>> > > ---
>> > > drivers/net/ethernet/broadcom/bnxt/bnxt.c | 4 ++++
>> > > 1 file changed, 4 insertions(+)
>> > >
>> > > diff --git a/drivers/net/ethernet/broadcom/bnxt/bnxt.c b/drivers/net/ethernet/broadcom/bnxt/bnxt.c
>> > > index c8e3468eee612ad622bfbecfd7cc1ae3396061fd..0eba3e307a3edbc5fe1abf2fa45e6256d98574c2 100644
>> > > --- a/drivers/net/ethernet/broadcom/bnxt/bnxt.c
>> > > +++ b/drivers/net/ethernet/broadcom/bnxt/bnxt.c
>> > > @@ -2297,6 +2297,10 @@ static int bnxt_rx_pkt(struct bnxt *bp, struct bnxt_cp_ring_info *cpr,
>> > > }
>> > > }
>> > > }
>> > > +
>> > > + if (xdp_active)
>> > > + xdp_buff_update_skb(&xdp, skb);
>> >
>> > For me, the preference for reusing existing metadata area was
>> > because of the patches 10-16: we now need to care about two types of
>> > metadata explicitly.
>>
>> Having to update all the drivers is definitely not ideal. Motivation is:
>>
>> 1. Avoid trait_set() and xdp_adjust_meta() from corrupting each other's
>> data.
>> But that's not a problem if we disallow trait_set() and
>> xdp_adjust_meta() to be used at the same time, so maybe not a good
>> reason anymore (except for maybe 3.)
>>
>> 2. Not have the traits at the "end" of the headroom (ie right next to
>> actual packet data).
>> If it's at the "end", we need to move all of it to make room for
>> every xdp_adjust_head() call.
>> It seems more intrusive to the current SKB API: several funcs assume
>> that there is headroom directly before the packet.
>
> [..]
>
>> 3. I'm not sure how this should be exposed with AF_XDP yet. Either:
>> * Expose raw trait storage, and having it at the "end" of the
>> headroom is nice. But userspace would need to know how to parse the
>> header.
>> * Require the XDP program to copy the traits it wants into the XDP
>> metadata area, which is already exposed to userspace. That would
>> need traits and XDP metadata to coexist.
>
> By keeping the traits at the tail we can just expose raw trait storage
> and let userspace deal with it. But anyway, my main point is to avoid
> having drivers to deal with two separate cases. As long as we can hide
> everything behind a common call, we can change the placement later.
Being able to change the placement (and format) of the data store is the
reason why we should absolutely *not* expose the internal trait storage
to AF_XDP. Once we do that, we effectively make the whole thing UAPI.
The trait get/set API very deliberately does not expose any details
about the underlying storage for exactly this reason :)
-Toke
next prev parent reply other threads:[~2025-04-24 9:49 UTC|newest]
Thread overview: 36+ messages / expand[flat|nested] mbox.gz Atom feed top
2025-04-22 13:23 [PATCH RFC bpf-next v2 00/17] traits: Per packet metadata KV store Arthur Fabre
2025-04-22 13:23 ` [PATCH RFC bpf-next v2 01/17] trait: limited KV store for packet metadata Arthur Fabre
2025-04-24 16:22 ` Alexei Starovoitov
2025-04-25 19:26 ` Arthur Fabre
2025-04-29 23:36 ` Alexei Starovoitov
2025-04-30 9:19 ` Toke Høiland-Jørgensen
2025-04-30 16:29 ` Alexei Starovoitov
2025-05-01 7:30 ` Arthur Fabre
2025-04-30 19:19 ` Jakub Sitnicki
2025-05-01 10:43 ` Toke Høiland-Jørgensen
2025-05-01 14:03 ` Jesper Dangaard Brouer
2025-05-05 10:18 ` Jakub Sitnicki
2025-05-05 12:35 ` Toke Høiland-Jørgensen
2025-04-22 13:23 ` [PATCH RFC bpf-next v2 02/17] xdp: Track if metadata is supported in xdp_frame <> xdp_buff conversions Arthur Fabre
2025-04-22 13:23 ` [PATCH RFC bpf-next v2 03/17] trait: XDP support Arthur Fabre
2025-04-22 13:23 ` [PATCH RFC bpf-next v2 04/17] trait: XDP selftest Arthur Fabre
2025-04-22 13:23 ` [PATCH RFC bpf-next v2 05/17] trait: XDP benchmark Arthur Fabre
2025-04-22 13:23 ` [PATCH RFC bpf-next v2 06/17] trait: Replace memcpy calls with inline copies Arthur Fabre
2025-04-22 13:23 ` [PATCH RFC bpf-next v2 07/17] trait: Replace memmove calls with inline move Arthur Fabre
2025-04-22 13:23 ` [PATCH RFC bpf-next v2 08/17] skb: Extension header in packet headroom Arthur Fabre
2025-04-22 13:23 ` [PATCH RFC bpf-next v2 09/17] trait: Store traits in sk_buff extension Arthur Fabre
2025-04-22 13:23 ` [PATCH RFC bpf-next v2 10/17] bnxt: Propagate trait presence to skb Arthur Fabre
2025-04-23 16:36 ` Stanislav Fomichev
2025-04-23 20:54 ` Arthur Fabre
2025-04-23 23:45 ` Stanislav Fomichev
2025-04-24 9:49 ` Toke Høiland-Jørgensen [this message]
2025-04-24 15:39 ` Stanislav Fomichev
2025-04-24 18:59 ` Jakub Sitnicki
2025-04-25 8:06 ` Toke Høiland-Jørgensen
2025-04-22 13:23 ` [PATCH RFC bpf-next v2 11/17] ice: " Arthur Fabre
2025-04-22 13:23 ` [PATCH RFC bpf-next v2 12/17] veth: " Arthur Fabre
2025-04-22 13:23 ` [PATCH RFC bpf-next v2 13/17] virtio_net: " Arthur Fabre
2025-04-22 13:23 ` [PATCH RFC bpf-next v2 14/17] mlx5: move xdp_buff scope one level up Arthur Fabre
2025-04-22 13:23 ` [PATCH RFC bpf-next v2 15/17] mlx5: Propagate trait presence to skb Arthur Fabre
2025-04-22 13:23 ` [PATCH RFC bpf-next v2 16/17] xdp generic: " Arthur Fabre
2025-04-22 13:23 ` [PATCH RFC bpf-next v2 17/17] trait: Allow socket filters to access traits Arthur Fabre
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
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=87tt6d7utp.fsf@toke.dk \
--to=toke@redhat$(echo .)com \
--cc=arthur@arthurfabre$(echo .)com \
--cc=ast@kernel$(echo .)org \
--cc=bpf@vger$(echo .)kernel.org \
--cc=edumazet@google$(echo .)com \
--cc=hawk@kernel$(echo .)org \
--cc=jakub@cloudflare$(echo .)com \
--cc=jbrandeburg@cloudflare$(echo .)com \
--cc=kuba@kernel$(echo .)org \
--cc=lbiancon@redhat$(echo .)com \
--cc=netdev@vger$(echo .)kernel.org \
--cc=stfomichev@gmail$(echo .)com \
--cc=yan@cloudflare$(echo .)com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
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