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.
