Message-ID: <20a3558f-43c5-46a2-8395-c6d966ea5caf@kernel.org>
Date: Thu, 7 Aug 2025 21:07:58 +0200
From: Jesper Dangaard Brouer <hawk@...nel.org>
To: Martin KaFai Lau <martin.lau@...ux.dev>
Cc: Jakub Kicinski <kuba@...nel.org>, Lorenzo Bianconi <lorenzo@...nel.org>,
 Stanislav Fomichev <stfomichev@...il.com>, bpf@...r.kernel.org,
 netdev@...r.kernel.org, Alexei Starovoitov <ast@...nel.org>,
 Daniel Borkmann <borkmann@...earbox.net>,
 Eric Dumazet <eric.dumazet@...il.com>, "David S. Miller"
 <davem@...emloft.net>, Paolo Abeni <pabeni@...hat.com>, sdf@...ichev.me,
 kernel-team@...udflare.com, arthur@...hurfabre.com, jakub@...udflare.com,
 Jesse Brandeburg <jbrandeburg@...udflare.com>,
 Andrew Rzeznik <arzeznik@...udflare.com>
Subject: Re: [PATCH bpf-next V2 0/7] xdp: Allow BPF to set RX hints for
 XDP_REDIRECTed packets



On 06/08/2025 03.24, Martin KaFai Lau wrote:
> On 8/4/25 6:18 AM, Jesper Dangaard Brouer wrote:
>> Do keep-in-mind that "moving skb allocation out of the driver" is not
>> part of this patchset and a moonshot goal that will take a long time
>> (but we are already "simulation" this via XDP-redirect for years now).
> 
> The XDP_PASS was first done in the very early days of BPF in 2016. The 
> XDP-redirect then followed a similar setup. A lot has improved since 
> then. A moonshot in 2016 does not necessarily mean it is still hard to 
> do now. e.g. Loop is feasible. Directly reading/writing skb is also easier.
> 

Please enlighten me. How can easily give XDP-progs access to the SKB
data-structure.  What changes do we need?
You mentioned XDP returning an SKB pointer, but that will need changes
to every XDP driver, right?


> Let’s first quantify what the performance loss would be if the skb is 
> allocated and field-set by the xdp prog (for the general XDP_PASS case 
> and the redirect+cpumap case). If it’s really worth it, let’s see what 
> it would take for the XDP program to achieve similar optimizations.
> 
>> Drivers should obviously not unconditionally populate the xdp_frame's
>> rx_meta area.  It is first time to populate rx_meta, once driver reach
> 
> afaict, the rx_meta is reserved regardless though. The xdp prog cannot 
> use that space for data_meta. The rx_meta will grow in time.
> 

My view is that we a memory area of minimum 192 bytes available as
headroom, that we are currently not using, that seems a waste.  The
data_meta was limited to 32 bytes for a long time without complaints, so
I don't think that is a concern.  If rx_meta grows, we propose changing
to the traits implementation, which gives us a dynamic compressed struct.


> My preference is to allow xdp prog to decide what needs to write in 
> data_meta and decides what needs to set in the skb directly. This is the 
> general case it should support first and then optimize.
> 

Yan and I have previously[1] (Oct 2024) explored adding a common
callback to XDP drivers, which have access to both the xdp_buff and SKB
in the function call. (Ignore the GRO disable bit, focus on callback)

We named the functions: xdp_buff_fixup_skb_offloading() and
  xdp_frame_fixup_skb_offloading()
We implemented the driver changes for [bnxt], [mlx5], [ice] and [veth].

What do you think of the idea of adding a BPF-hook, at this callback,
which have access to both the XDP and SKB pointer.
That would allow us to implement your idea, right?

--Jesper


[1] https://lore.kernel.org/all/cover.1718919473.git.yan@cloudflare.com/#r

[bnxt] 
https://lore.kernel.org/all/f804c22ca168ec3aedb0ee754bfbee71764eb894.1718919473.git.yan@cloudflare.com/

[mlx5] 
https://lore.kernel.org/all/17595a278ee72964b83c0bd0b502152aa025f600.1718919473.git.yan@cloudflare.com/

[ice] 
https://lore.kernel.org/all/a9eba425bfd3bfac7e7be38fe86ad5dbff3ae01f.1718919473.git.yan@cloudflare.com/

[veth] 
https://lore.kernel.org/all/b7c75daecca9c4e36ef79af683d288653a9b5b82.1718919473.git.yan@cloudflare.com/

Powered by blists - more mailing lists