From: Jakub Sitnicki <jakub@cloudflare.com>
To: Andrii Nakryiko <andrii.nakryiko@gmail.com>
Cc: bpf@vger.kernel.org, "Alexei Starovoitov" <ast@kernel.org>,
"Arthur Fabre" <arthur@arthurfabre.com>,
"Eric Dumazet" <edumazet@google.com>,
"Jakub Kicinski" <kuba@kernel.org>,
"Jesper Dangaard Brouer" <hawk@kernel.org>,
"Jesse Brandeburg" <jbrandeburg@cloudflare.com>,
"Joanne Koong" <joannelkoong@gmail.com>,
"Lorenzo Bianconi" <lorenzo@kernel.org>,
"Toke Høiland-Jørgensen" <thoiland@redhat.com>,
"Yan Zhai" <yan@cloudflare.com>,
netdev@vger.kernel.org, kernel-team@cloudflare.com,
"Stanislav Fomichev" <sdf@fomichev.me>
Subject: Re: [PATCH bpf-next 01/13] bpf: Ignore dynptr offset in skb data access
Date: Wed, 02 Jul 2025 10:20:56 +0200 [thread overview]
Message-ID: <87bjq3nguv.fsf@cloudflare.com> (raw)
In-Reply-To: <CAEf4Bza_=HUKT6_sxrO=xC37DGyTgnvKtqd9Zdmq-crbtdTYSA@mail.gmail.com> (Andrii Nakryiko's message of "Tue, 1 Jul 2025 13:55:01 -0700")
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@cloudflare.com> 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.
next prev parent reply other threads:[~2025-07-02 8:20 UTC|newest]
Thread overview: 25+ messages / expand[flat|nested] mbox.gz Atom feed top
2025-06-30 14:55 [PATCH bpf-next 00/13] Extend skb dynptr for metadata access from TC Jakub Sitnicki
2025-06-30 14:55 ` [PATCH bpf-next 01/13] bpf: Ignore dynptr offset in skb data access Jakub Sitnicki
2025-07-01 20:55 ` Andrii Nakryiko
2025-07-02 8:20 ` Jakub Sitnicki [this message]
2025-06-30 14:55 ` [PATCH bpf-next 02/13] bpf: Helpers for skb dynptr read/write/slice Jakub Sitnicki
2025-07-01 2:03 ` kernel test robot
2025-07-01 11:13 ` Jakub Sitnicki
2025-07-01 3:06 ` kernel test robot
2025-06-30 14:55 ` [PATCH bpf-next 03/13] bpf: Add new variant of skb dynptr for the metadata area Jakub Sitnicki
2025-06-30 16:27 ` Stanislav Fomichev
2025-06-30 20:34 ` Jakub Sitnicki
2025-07-01 20:59 ` Andrii Nakryiko
2025-07-02 8:22 ` Jakub Sitnicki
2025-06-30 14:55 ` [PATCH bpf-next 04/13] bpf: Enable read access to skb metadata with bpf_dynptr_read Jakub Sitnicki
2025-06-30 14:55 ` [PATCH bpf-next 05/13] bpf: Enable write access to skb metadata with bpf_dynptr_write Jakub Sitnicki
2025-06-30 14:55 ` [PATCH bpf-next 06/13] bpf: Enable read-write access to skb metadata with dynptr slice Jakub Sitnicki
2025-06-30 14:55 ` [PATCH bpf-next 07/13] net: Clear skb metadata on handover from device to protocol Jakub Sitnicki
2025-06-30 16:25 ` Stanislav Fomichev
2025-06-30 20:30 ` Jakub Sitnicki
2025-06-30 14:55 ` [PATCH bpf-next 08/13] selftests/bpf: Pass just bpf_map to xdp_context_test helper Jakub Sitnicki
2025-06-30 14:55 ` [PATCH bpf-next 09/13] selftests/bpf: Parametrize test_xdp_context_tuntap Jakub Sitnicki
2025-06-30 14:55 ` [PATCH bpf-next 10/13] selftests/bpf: Cover read access to skb metadata via dynptr Jakub Sitnicki
2025-06-30 14:55 ` [PATCH bpf-next 11/13] selftests/bpf: Cover write " Jakub Sitnicki
2025-06-30 14:55 ` [PATCH bpf-next 12/13] selftests/bpf: Cover lack of access to skb metadata at ip layer Jakub Sitnicki
2025-06-30 14:55 ` [PATCH bpf-next 13/13] selftests/bpf: Count successful bpf program runs Jakub Sitnicki
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=87bjq3nguv.fsf@cloudflare.com \
--to=jakub@cloudflare.com \
--cc=andrii.nakryiko@gmail.com \
--cc=arthur@arthurfabre.com \
--cc=ast@kernel.org \
--cc=bpf@vger.kernel.org \
--cc=edumazet@google.com \
--cc=hawk@kernel.org \
--cc=jbrandeburg@cloudflare.com \
--cc=joannelkoong@gmail.com \
--cc=kernel-team@cloudflare.com \
--cc=kuba@kernel.org \
--cc=lorenzo@kernel.org \
--cc=netdev@vger.kernel.org \
--cc=sdf@fomichev.me \
--cc=thoiland@redhat.com \
--cc=yan@cloudflare.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.