netdev - Re: [PATCH bpf-next V2 1/7] net: xdp: Add xdp_rx

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <d0561121-3d36-4c55-8dbb-fc6b802a0f68@kernel.org>
Date: Thu, 17 Jul 2025 16:40:47 +0200
From: Jesper Dangaard Brouer <hawk@...nel.org>
To: Jakub Sitnicki <jakub@...udflare.com>
Cc: bpf@...r.kernel.org, netdev@...r.kernel.org,
 Jakub Kicinski <kuba@...nel.org>, lorenzo@...nel.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
Subject: Re: [PATCH bpf-next V2 1/7] net: xdp: Add xdp_rx_meta structure



On 17/07/2025 11.19, Jakub Sitnicki wrote:
> On Wed, Jul 02, 2025 at 04:58 PM +02, Jesper Dangaard Brouer wrote:
>> From: Lorenzo Bianconi <lorenzo@...nel.org>
>>
>> Introduce the `xdp_rx_meta` structure to serve as a container for XDP RX
>> hardware hints within XDP packet buffers. Initially, this structure will
>> accommodate `rx_hash` and `rx_vlan` metadata. (The `rx_timestamp` hint will
>> get stored in `skb_shared_info`).
>>
>> A key design aspect is making this metadata accessible both during BPF
>> program execution (via `struct xdp_buff`) and later if an `struct
>> xdp_frame` is materialized (e.g., for XDP_REDIRECT).
>> To achieve this:
>>    - The `struct xdp_frame` embeds an `xdp_rx_meta` field directly for
>>      storage.
>>    - The `struct xdp_buff` includes an `xdp_rx_meta` pointer. This pointer
>>      is initialized (in `xdp_prepare_buff`) to point to the memory location
>>      within the packet buffer's headroom where the `xdp_frame`'s embedded
>>      `rx_meta` field would reside.
>>
>> This setup allows BPF kfuncs, operating on `xdp_buff`, to populate the
>> metadata in the precise location where it will be found if an `xdp_frame`
>> is subsequently created.
>>
>> The availability of this metadata storage area within the buffer is
>> indicated by the `XDP_FLAGS_META_AREA` flag in `xdp_buff->flags` (and
>> propagated to `xdp_frame->flags`). This flag is only set if sufficient
>> headroom (at least `XDP_MIN_HEADROOM`, currently 192 bytes) is present.
>> Specific hints like `XDP_FLAGS_META_RX_HASH` and `XDP_FLAGS_META_RX_VLAN`
>> will then denote which types of metadata have been populated into the
>> `xdp_rx_meta` structure.
>>
>> This patch is a step for enabling the preservation and use of XDP RX
>> hints across operations like XDP_REDIRECT.
>>
>> Signed-off-by: Lorenzo Bianconi <lorenzo@...nel.org>
>> Signed-off-by: Jesper Dangaard Brouer <hawk@...nel.org>
>> ---
>>   include/net/xdp.h       |   57 +++++++++++++++++++++++++++++++++++------------
>>   net/core/xdp.c          |    1 +
>>   net/xdp/xsk_buff_pool.c |    4 ++-
>>   3 files changed, 47 insertions(+), 15 deletions(-)
>>
>> diff --git a/include/net/xdp.h b/include/net/xdp.h
>> index b40f1f96cb11..f52742a25212 100644
>> --- a/include/net/xdp.h
>> +++ b/include/net/xdp.h
>> @@ -71,11 +71,31 @@ struct xdp_txq_info {
>>   	struct net_device *dev;
>>   };
>>   
>> +struct xdp_rx_meta {
>> +	struct xdp_rx_meta_hash {
>> +		u32 val;
>> +		u32 type; /* enum xdp_rss_hash_type */
>> +	} hash;
>> +	struct xdp_rx_meta_vlan {
>> +		__be16 proto;
>> +		u16 tci;
>> +	} vlan;
>> +};
>> +
>> +/* Storage area for HW RX metadata only available with reasonable headroom
>> + * available. Less than XDP_PACKET_HEADROOM due to Intel drivers.
>> + */
>> +#define XDP_MIN_HEADROOM	192
>> +
>>   enum xdp_buff_flags {
>>   	XDP_FLAGS_HAS_FRAGS		= BIT(0), /* non-linear xdp buff */
>>   	XDP_FLAGS_FRAGS_PF_MEMALLOC	= BIT(1), /* xdp paged memory is under
>>   						   * pressure
>>   						   */
>> +	XDP_FLAGS_META_AREA		= BIT(2), /* storage area available */
> 
> Idea: Perhaps this could be called *HW*_META_AREA to differentiate from
> the existing custom metadata area:
> 

I agree, that calling it META_AREA can easily be misunderstood and 
confused with metadata or data_meta.

What do you think about renaming this to "hints" ?
  E.g. XDP_FLAGS_HINTS_AREA
  or   XDP_FLAGS_HINTS_AVAIL

And also renaming XDP_FLAGS_META_RX_* to
  e.g XDP_FLAGS_META_RX_HASH -> XDP_FLAGS_HINT_RX_HASH
                            or  XDP_FLAGS_HW_HINT_RX_HASH

> https://docs.kernel.org/networking/xdp-rx-metadata.html#af-xdp
> 
>> +	XDP_FLAGS_META_RX_HASH		= BIT(3), /* hw rx hash */
>> +	XDP_FLAGS_META_RX_VLAN		= BIT(4), /* hw rx vlan */
>> +	XDP_FLAGS_META_RX_TS		= BIT(5), /* hw rx timestamp */
>>   };
>>   
> 
> [...]