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: <3fed59c6-e959-4863-b1c5-1927ef0d61df@kernel.org>
Date: Thu, 8 May 2025 18:59:54 +0200
From: Jesper Dangaard Brouer <hawk@...nel.org>
To: Jon Kohler <jon@...anix.com>
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 08/05/2025 05.18, Jon Kohler wrote:
> 
>> On May 7, 2025, at 4:58 PM, Jesper Dangaard Brouer<hawk@...nel.org>  wrote:
>>
>> On 07/05/2025 21.57, Jon Kohler wrote:
>>>> 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.
>> Good point.
>>
>>>> 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()
>> I like xdp_linear_len() as it is descriptive/clear.
 >
> Ok thanks, I’ll send out a v4 to codify that.

I'll ack a V4 with that change.

I do notice Jakub isn't a fan of the patch in general, but it seems
quite popular given the other high profile kernel developers that acked
in V3.  I think it increase code readability for people that are less
familiar with XDP code and meaning of the pointers (e.g. data_hard_start
vs. data_end vs. data vs. data_meta). (We don't even have some ascii art
showing these pointers).

--Jesper