Re: [PATCH bpf-next V2 1/7] net: xdp: Add xdp_rx_meta structure

[Jakub Sitnicki <jakub@cloudflare.com>]

On Thu, Jul 17, 2025 at 04:40 PM +02, Jesper Dangaard Brouer wrote:
> 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@kernel.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@kernel.org>
>>> Signed-off-by: Jesper Dangaard Brouer <hawk@kernel.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

Any name that doesn't lean on the already overloaded "metadata" term is
a better alternative, in my mind :-)