From: Jakub Sitnicki <jakub@cloudflare.com>
To: Martin KaFai Lau <martin.lau@linux.dev>
Cc: "Jakub Kicinski" <kuba@kernel.org>,
bpf@vger.kernel.org, "Alexei Starovoitov" <ast@kernel.org>,
"Andrii Nakryiko" <andrii@kernel.org>,
"Arthur Fabre" <arthur@arthurfabre.com>,
"Daniel Borkmann" <daniel@iogearbox.net>,
"Eduard Zingerman" <eddyz87@gmail.com>,
"Eric Dumazet" <edumazet@google.com>,
"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>,
kernel-team@cloudflare.com, netdev@vger.kernel.org,
"Stanislav Fomichev" <sdf@fomichev.me>
Subject: Re: [PATCH bpf-next v4 2/8] bpf: Enable read/write access to skb metadata through a dynptr
Date: Thu, 24 Jul 2025 21:43:22 +0200 [thread overview]
Message-ID: <87jz3xwf1h.fsf@cloudflare.com> (raw)
In-Reply-To: <0190e181-c592-454a-a99b-5ec361ce84e9@linux.dev> (Martin KaFai Lau's message of "Thu, 24 Jul 2025 08:52:04 -0700")
On Thu, Jul 24, 2025 at 08:52 AM -07, Martin KaFai Lau wrote:
> On 7/24/25 4:53 AM, Jakub Sitnicki wrote:
>> In this series we maintain the status quo. Access metadata dynptr is
>> limited to TC BPF hook only, so we provide the same guarntees as the
>> existing __sk_buff->data_meta.
>
> The verifier tracks if the __sk_buff->data_meta is written in
> "seen_direct_write". tc_cls_act_prologue is called and that should have
> triggered skb_metadata_clear for a clone skb. Meaning, for a clone skb, I think
> __sk_buff->data_meta is read-only.
>
> bpf_dynptr_from_skb_meta can set the DYNPTR_RDONLY_BIT if the skb is a clone.
Oh that it clever. TIL. So if we end up calling:
tc_cls_act_prologue
bpf_skb_pull_data
bpf_try_make_writable(skb, skb_headlen(skb))
__bpf_try_make_writable
skb_ensure_writable
pskb_expand_head
skb_metadata_clear
skb_metadata_set(skb, 0)
skb_shinfo(skb)->meta_len = 0;
... then the metadata is not so much read-only but inaccessible for the
clone. The dynptr will reflect this state, so all seems right.
Let me see if I can capture that in a test, though.
BTW. I've wondered why pskb_expand_head doesn't just copy the metadata,
but left dealing with it till the next step. If it did, we could just
operate on a copy.
next prev parent reply other threads:[~2025-07-24 19:43 UTC|newest]
Thread overview: 21+ messages / expand[flat|nested] mbox.gz Atom feed top
2025-07-23 17:36 [PATCH bpf-next v4 0/8] Add a dynptr type for skb metadata for TC BPF Jakub Sitnicki
2025-07-23 17:36 ` [PATCH bpf-next v4 1/8] bpf: Add dynptr type for skb metadata Jakub Sitnicki
2025-07-24 1:54 ` Martin KaFai Lau
2025-07-24 11:56 ` Jakub Sitnicki
2025-07-23 17:36 ` [PATCH bpf-next v4 2/8] bpf: Enable read/write access to skb metadata through a dynptr Jakub Sitnicki
2025-07-23 21:58 ` Eduard Zingerman
2025-07-24 0:02 ` Martin KaFai Lau
2025-07-24 9:44 ` Jakub Sitnicki
2025-07-24 0:30 ` Jakub Kicinski
2025-07-24 11:53 ` Jakub Sitnicki
2025-07-24 15:52 ` Martin KaFai Lau
2025-07-24 19:43 ` Jakub Sitnicki [this message]
2025-07-25 9:43 ` Jakub Sitnicki
2025-07-25 14:34 ` Jakub Kicinski
2025-07-23 17:36 ` [PATCH bpf-next v4 3/8] selftests/bpf: Cover verifier checks for skb_meta dynptr type Jakub Sitnicki
2025-07-23 17:36 ` [PATCH bpf-next v4 4/8] selftests/bpf: Pass just bpf_map to xdp_context_test helper Jakub Sitnicki
2025-07-23 17:36 ` [PATCH bpf-next v4 5/8] selftests/bpf: Parametrize test_xdp_context_tuntap Jakub Sitnicki
2025-07-23 17:36 ` [PATCH bpf-next v4 6/8] selftests/bpf: Cover read access to skb metadata via dynptr Jakub Sitnicki
2025-07-23 17:36 ` [PATCH bpf-next v4 7/8] selftests/bpf: Cover write " Jakub Sitnicki
2025-07-23 17:36 ` [PATCH bpf-next v4 8/8] selftests/bpf: Cover read/write to skb metadata at an offset Jakub Sitnicki
2025-07-23 22:02 ` Eduard Zingerman
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=87jz3xwf1h.fsf@cloudflare.com \
--to=jakub@cloudflare.com \
--cc=andrii@kernel.org \
--cc=arthur@arthurfabre.com \
--cc=ast@kernel.org \
--cc=bpf@vger.kernel.org \
--cc=daniel@iogearbox.net \
--cc=eddyz87@gmail.com \
--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=martin.lau@linux.dev \
--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.