Re: [PATCH v2 bpf-next 3/8] bpf: tcp: Introduce bpf_tcp_ops.{enqueue,dequeue}_rcvq().

["Alexei Starovoitov" <alexei.starovoitov@gmail.com>]

On Thu, Sep 24, 2026 at 05:39 PM Kuniyuki Iwashima <kuniyu@google.com> wrote:
>> can we drop the flag ?
>> hdr_opt_len() is called for every tx skb without per-socket opt-in.
>
> Oh, I assumed bpf_tcp_ops keeps the same behaviour.

bpf_tcp_ops were moved out of BPF_SOCK_OPS_TEST_FLAG() guards on
purpose. None of the existing members has per-socket opt-in.
hdr_opt_len() runs for every tx skb, rtt() for every rtt sample.
See the log of commit 3bb54768fe3e
("bpf: tcp: Support parse/len/write header option hooks in bpf_tcp_ops"):
"A per-member/per-cgroup gate could be added later if the extra
fast-path work proves measurable."

> We have a bpf prog to turn the hook on (from cgroup_skb/ingress),
> only when ingress rate exceeds the allocated capacity, to signal that
> via a custom TCP option and turn the hook off from the hook itself.
> (and I planned to post another patch for that)

That should work today without another patch and without the flag.
cgroup_skb/ingress prog does
  bpf_sk_storage_get(&map, sk, 0, BPF_SK_STORAGE_GET_F_CREATE);
when the rate is exceeded. hdr_opt_len() and write_hdr_opt() do
  bpf_sk_storage_get(&map, sk, 0, 0);
and return when it's NULL. write_hdr_opt() calls
bpf_sk_storage_delete() after the option went out.
cg_skb_func_proto() has both helpers and get_func_proto() in
bpf_tcp_ops.c allows them in every member.
egress_read_sock_fields() in test_sock_fields.c does the cgroup_skb
part.

> Anyway, is it because calling bpf prog is not very expensive
> or per-prog switch is preferable to per-socket flag ?

The switch is still per socket. It's sk_storage instead of a bit in
tcp_sock, so every prog has its own.

While such new flag is per socket, but it's one for all progs.

And you want to take the last bit of u8 bpf_sock_ops_cb_flags...

As far as the cost. A socket that didn't opt in pays for an indirect
call into the prog and for bpf_sk_storage_get() that finds nothing,
but only in cgroups where bpf_tcp_ops with enqueue_rcvq is attached.
The members that are not set are NULL and bpf_tcp_ops_call() skips
them. That's what hdr_opt_len() costs on tx.

I don't remember what Amery measured, but worth benchmarking
for your case.

>> The prog in patch 8 already returns when the socket has no
>> sk_storage.
>
> Yes, but this is only for bpf verifier.

tcp_init_autolowat_cb() can be the only place that passes
BPF_SK_STORAGE_GET_F_CREATE.
So for a socket that didn't do setsockopt(BPF_TCP_AUTOLOWAT)
other cbs will return NULL on lookup.

tcp_disable_autolowat() will do:
  bpf_sk_storage_delete(&tcp_autolowat_map, sk);
  bpf_tcp_ops_set_rcvlowat(sk, 1);

> Just an idea, would it make sense to add a kfunc to move bpf_tcp_ops
> callback to a shadow pointer by allocating sizeof(bpf_tcp_ops) * 2 ?

Doesn't look right to me.

Amery,
please share your thoughts here.