Re: [PATCH v3 bpf-next 2/9] bpf: tcp: Add a new per-socket flag and kfunc for bpf_tcp_ops.

[Stanislav Fomichev <sdf.kernel@gmail.com>]

On 10/05, Kuniyuki Iwashima wrote:
> On Mon, Oct 5, 2026 at 11:30 AM Stanislav Fomichev <sdf.kernel@gmail.com> wrote:
> >
> > On 10/05, Kuniyuki Iwashima wrote:
> > > The legacy SOCK_OPS guards some hooks with a per-socket flag,
> > > tp->bpf_sock_ops_cb_flags.
> > >
> > > In contrast, bpf_tcp_ops was initially designed without per-socket
> > > flags so that users can simply define only the callbacks they need.
> > >
> > > However, it turned out that even attaching a bpf_tcp_ops with a NULL
> > > callback incurs measurable overhead in the fast path. [0]
> > >
> > > We could reuse tp->bpf_sock_ops_cb_flags, but it is u8 and has only
> > > 1 bit left for a new opt-in callback, while not all of the 7 existing
> > > opt-in hooks are in the fast path and really need a flag guard.
> > >
> > > Moreover, tp->bpf_sock_ops_cb_flags has two bpf helper functions to
> > > update it, both of which require sock_owned_by_me(sk):
> > >
> > >   * bpf_sock_ops_cb_flags_set()
> > >   * bpf_setsockopt(TCP_BPF_SOCK_OPS_CB_FLAGS)
> >
> > Can we return -EINVAL from these when used with sock_ops?
> 
> bpf_sock_ops_cb_flags_set() is not allowed for bpf_tcp_ops,
> so we could add a check in bpf_setsockopt() for bpf_tcp_ops
> to disallow TCP_BPF_SOCK_OPS_CB_FLAGS.
> 
> But not all of bpf_tcp_ops hooks are opt-in, so we cannot prevent
> attaching bpf_tcp_ops to SOCK_OPS-enabled sockets.
> 
> 
> > If we keep one flag we can presumably put both tcp_call_bpf_Xarg and
> > bpf_tcp_ops_call under the same if conditional?

[..]
 
> I assuemd we will use the new flag only for bpf_tcp_ops and will no
> longer add a new bit to the sock_ops flag and fainlly we can deprecate
> it under a Kconfig like CONFIG_BPF_LEGACY_SOCK_OPS.

Don't think CONFIG_LEGACY is happening :-D I might be overthinking it..
Realistically, maybe something like bubbling up cgroup_bpf_enabled(sock_ops)
is an option as well? So these (old) runtime checks are happening only when
we have sock_ops attached (and/or separate mechanism for struct_ops?). 
But if you don't see anything wrong in your benchmark, let's not overcomplicate.