linux-kernel - Re: [PATCH net-next v3] xdp: Add helpers for head length, headroom, and metadata length

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: <6FF98F38-2AE5-4000-8827-81369C3FB429@nutanix.com>
Date: Wed, 7 May 2025 19:57:00 +0000
From: Jon Kohler <jon@...anix.com>
To: Jesper Dangaard Brouer <hawk@...nel.org>
CC: Willem de Bruijn <willemdebruijn.kernel@...il.com>,
        Zvi Effron
	<zeffron@...tgames.com>,
        Stanislav Fomichev <stfomichev@...il.com>,
        Jason
 Wang <jasowang@...hat.com>, Andrew Lunn <andrew+netdev@...n.ch>,
        "David S.
 Miller" <davem@...emloft.net>,
        Eric Dumazet <edumazet@...gle.com>, Jakub
 Kicinski <kuba@...nel.org>,
        Paolo Abeni <pabeni@...hat.com>, Alexei
 Starovoitov <ast@...nel.org>,
        Daniel Borkmann <daniel@...earbox.net>,
        John
 Fastabend <john.fastabend@...il.com>,
        Simon Horman <horms@...nel.org>,
        "netdev@...r.kernel.org" <netdev@...r.kernel.org>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
        "bpf@...r.kernel.org" <bpf@...r.kernel.org>,
        Jacob Keller
	<jacob.e.keller@...el.com>
Subject: Re: [PATCH net-next v3] xdp: Add helpers for head length, headroom,
 and metadata length



> On May 7, 2025, at 3:04 PM, Jesper Dangaard Brouer <hawk@...nel.org> wrote:
> 
> 
> 
> On 07/05/2025 19.47, Jon Kohler wrote:
>>> On May 7, 2025, at 1:21 PM, Willem de Bruijn <willemdebruijn.kernel@...il.com> wrote:
>>> 
>>> 
>>> Jesper Dangaard Brouer wrote:
>>>> 
>>>> 
>>>> On 07/05/2025 19.02, Zvi Effron wrote:
>>>>> On Wed, May 7, 2025 at 9:37 AM Jesper Dangaard Brouer <hawk@...nel.org> wrote:
>>>>>> 
>>>>>> 
>>>>>> 
>>>>>> On 07/05/2025 15.29, Willem de Bruijn wrote:
>>>>>>> Stanislav Fomichev wrote:
>>>>>>>> On 05/06, Jon Kohler wrote:
>>>>>>>>> Introduce new XDP helpers:
>>>>>>>>> - xdp_headlen: Similar to skb_headlen
>>>>>> 
>>>>>> I really dislike xdp_headlen(). This "headlen" originates from an SKB
>>>>>> implementation detail, that I don't think we should carry over into XDP
>>>>>> land.
>>>>>> We need to come up with something that isn't easily mis-read as the
>>>>>> header-length.
>>>>> 
>>>>> ... snip ...
>>>>> 
>>>>>>>> + * xdp_headlen - Calculate the length of the data in an XDP buffer
>>>>> 
>>>>> How about xdp_datalen()?
>>>> 
>>>> Yes, I like xdp_datalen() :-)
>>> 
>>> This is confusing in that it is the inverse of skb->data_len:
>>> which is exactly the part of the data not in the skb head.
>>> 
>>> There is value in consistent naming. I've never confused headlen
>>> with header len.
>>> 
>>> But if diverging, at least let's choose something not
>>> associated with skbs with a different meaning.
>> Brainstorming a few options:
>> - xdp_head_datalen() ?
>> - xdp_base_datalen() ?
>> - xdp_base_headlen() ?
>> - xdp_buff_datalen() ?
>> - xdp_buff_headlen() ?
>> - xdp_datalen() ? (ZivE, JesperB)
>> - xdp_headlen() ? (WillemB, JonK, StanislavF, JacobK, DanielB)
> 
> What about keeping it really simple: xdp_buff_len() ?

This is suspiciously close to xdp_get_buff_len(), so there could be some
confusion there, since that takes paged/frags into account transparently.

> 
> Or even simpler: xdp_len() as the function documentation already
> describe this doesn't include frags.

There is a neat hint from Lorenzo’s change in bpf.h for bpf_xdp_get_buff_len()
that talks about both linear and paged length. Also, xdp_buff_flags’s 
XDP_FLAGS_HAS_FRAGS says non-linear xdp buff.

Taking those hints, what about:
xdp_linear_len() == xdp->data_end - xdp->data
xdp_paged_len() == sinfo->xdp_frags_size
xdp_get_buff_len() == xdp_linear_len() + xdp_paged_len()

Just a thought. If not, that’s ok. I’m happy to do xdp_len, but do you then have a
suggestion about getting the non-linear size only?

> 
> To Jon, you seems to be on a cleanup spree:
> For SKBs netstack have this diagram documented [1].  Which also explains
> the concept of a "head" buffer, which isn't a concept for XDP.  I would
> really like to see a diagram documenting both xdp_buff and xdp_frame
> data structures via ascii art, like the one for SKBs. (Hint, this is
> actually defined in the header file include/linux/skbuff.h, but
> converted to RST/HTML format.)
> 
> [1] https://docs.kernel.org/networking/skbuff.html

I certainly am in a cleanup sort of mood, happy to help here. I see what
you're talking about, I’ll take a stab at this in a separate patch. Thanks
for the push and tip!