[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <CAMB2axPLuQ75_JSqkR43-UVBUi9Yj7juHFLCkDvSLPL445SZew@mail.gmail.com>
Date: Wed, 10 Sep 2025 11:17:52 -0400
From: Amery Hung <ameryhung@...il.com>
To: Jakub Kicinski <kuba@...nel.org>
Cc: bpf@...r.kernel.org, netdev@...r.kernel.org, alexei.starovoitov@...il.com,
andrii@...nel.org, daniel@...earbox.net, stfomichev@...il.com,
martin.lau@...nel.org, mohsin.bashr@...il.com, noren@...dia.com,
dtatulea@...dia.com, saeedm@...dia.com, tariqt@...dia.com, mbloch@...dia.com,
maciej.fijalkowski@...el.com, kernel-team@...a.com
Subject: Re: [PATCH bpf-next v2 3/7] bpf: Support pulling non-linear xdp data
On Mon, Sep 8, 2025 at 9:54 PM Jakub Kicinski <kuba@...nel.org> wrote:
>
> On Fri, 5 Sep 2025 10:33:47 -0700 Amery Hung wrote:
> > + * Direct packet access allows reading and writing linear XDP data through
> > + * packet pointers (i.e., &xdp_md->data + offsets).
>
> Add:
> The amount of data which ends up in the linear part of the xdp_buf
> depends on the NIC and its configuration.
[...]
>
> > When an eBPF program wants
> > + * to directly access data that may be in the non-linear area, call this kfunc
> ^^^^
> maybe s/data/headers
>
> > + * to make sure the data is available in the linear area.
>
> Should we add a mention here of the copy helpers and dynptr for
> accessing data without pulling?
[...]
>
> > + * This kfunc can also be used with bpf_xdp_adjust_head() to decapsulate
> > + * headers in the non-linear data area.
> > + *
> > + * A call to this kfunc is susceptible to change the underlying packet buffer.
>
> Maybe:
> A call to this kfunc will modify the buffer geometry.
Will improve the comment based on the suggestions. Thanks!
>
> > + * Therefore, at load time, all checks on pointers previously done by the
> > + * verifier are invalidated and must be performed again, if the kfunc is used
> > + * in combination with direct packet access.
>
> > void *data_end = xdp->data + len;
>
> nit: I think the code would be easier to follow if we renamed this
> to "new_end"?
I was following the common pattern in other XDP kfuncs, but can change
it for readability.
>
>
> Larger note: I wonder if we should support "shifting the buffer down"
> if there's insufficient tailroom. XDP has rather copious headroom,
> but tailroom may be pretty tight, and it may depend on the length of
> the headers. So if there's not enough tailroom but there's enough
> headroom -- should we try to memmove the existing headers?
I think it should. If users want to reserve space for metadata, they
can check the headroom before pulling data.
If the kfunc does not do memmove(), users are still able to do so in
XDP programs through bpf_xdp_adjust_head() and memmove(), but it feels
less easy to use IMO.
Powered by blists - more mailing lists