On Tue, Jul 01, 2025 at 01:55 PM -07, Andrii Nakryiko wrote: > On Mon, Jun 30, 2025 at 8:23 AM Jakub Sitnicki <jakub@xxxxxxxxxxxxxx> wrote: >> >> 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.
