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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 





On 07/05/2025 19.47, Jon Kohler wrote:


On May 7, 2025, at 1:21 PM, Willem de Bruijn <willemdebruijn.kernel@xxxxxxxxx> 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@xxxxxxxxxx> 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() ?

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

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

--Jesper




[Index of Archives]     [Linux Samsung SoC]     [Linux Rockchip SoC]     [Linux Actions SoC]     [Linux for Synopsys ARC Processors]     [Linux NFS]     [Linux NILFS]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]


  Powered by Linux