Re: [PATCH bpf-next 01/13] bpf: Ignore dynptr offset in skb data access

On Tue, Jul 01, 2025 at 01:55 PM -07, Andrii Nakryiko wrote:

On Mon, Jun 30, 2025 at 8:23 AM Jakub Sitnicki [off-list ref] wrote:

quoted

Prepare to use (struct bpf_dynptr)->offset to distinguish between an skb
dynptr for the payload vs the metadata area.

ptr->offset is always set to zero by bpf_dynptr_from_skb(). We don't need
to account for it on access.

Huh?.. What about bpf_dynptr_adjust()? This is a wrong approach to
have some magical offset values.

Crap. I'm not gonna lie. I totally missed that.

You're right. It completely breaks down.

I was hoping I could piggyback on skb dynptr, but doesn't look like it.

More general question about your patch set: is there ever a need to
work with both metadata and data as one area of memory (i.e., copying
both metadata and data in the same single operation, or setting it as
one thing?). If not, why not have two different dynptrs, one for data
(what we have today) and one exclusively for packet's metadata?

Having two dynptr kinds, one for payload, one for metadata, sounds like
a much better direction. I will pivot to that.

Metadata and payload are logically separate, AFAIK. It just so happens
that the metadata is currently located in front of the payload.

I asked around to find out why is it so - it seems that the decision was
made to place the metadata like that becase it saves you one additional
pointer load. Otherwise you'd need something like
__sk_buff->data_meta_end to marks the end of metadata.