Re: [PATCH v1 bpf-next 7/8] bpf: tcp: Add SOCK_OPS rcvlowat hook.

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] The new BPF_SOCK_OPS_RCVLOWAT_CB hook is incorrectly rejected by is_locked_tcp_sock_ops(), preventing the BPF program from modifying callback flags.
--

commit ef96e613bd920ae4e567781955c5e3d4f11f7f57
Author: Kuniyuki Iwashima <kuniyu@google.com>

bpf: tcp: Add SOCK_OPS rcvlowat hook.

This commit introduces a new BPF_SOCK_OPS_RCVLOWAT_CB hook that is invoked
when TCP enqueues an skb or completes recvmsg(). The hook allows BPF
programs to dynamically adjust sk->sk_rcvlowat to suppress unnecessary
wakeups.

[ ... ]

> diff --git a/net/ipv4/tcp_input.c b/net/ipv4/tcp_input.c
> index 7e26503fd96d4..a70a8f5830258 100644
> --- a/net/ipv4/tcp_input.c
> +++ b/net/ipv4/tcp_input.c
> @@ -204,6 +204,12 @@ static void bpf_skops_established(struct sock *sk, int bpf_op,
>  	/* sk with TCP_REPAIR_ON does not have skb in tcp_finish_connect */
>  	bpf_skops_common_locked(sk, bpf_op, skb, skb ? tcp_hdrlen(skb) : 0);
>  }
> +
> +void bpf_skops_rcvlowat(struct sock *sk, struct sk_buff *skb)
> +{
> +	/* skb is NULL when called from __tcp_cleanup_rbuf(). */
> +	bpf_skops_common_locked(sk, BPF_SOCK_OPS_RCVLOWAT_CB, skb, 0);
> +}

Does this regression prevent the BPF program from dynamically disabling the
hook once the RPC boundaries are known?

Since BPF_SOCK_OPS_RCVLOWAT_CB is greater than BPF_SOCK_OPS_WRITE_HDR_OPT_CB,
is_locked_tcp_sock_ops() will return false for this hook:

net/core/filter.c:is_locked_tcp_sock_ops() {
	return bpf_sock->op <= BPF_SOCK_OPS_WRITE_HDR_OPT_CB;
}

While this intentionally prevents bpf_setsockopt() from being used
unnecessarily, bpf_sock_ops_cb_flags_set() also relies on this check:

net/core/filter.c:bpf_sock_ops_cb_flags_set() {
	if (!is_locked_tcp_sock_ops(bpf_sock))
		return -EOPNOTSUPP;
	...
}

This means any attempt to clear the callback flag using
bpf_sock_ops_cb_flags_set() within the hook will fail with -EOPNOTSUPP.
This could force the callback to run on every single SKB enqueue for the
lifetime of the connection.

Should is_locked_tcp_sock_ops() or bpf_sock_ops_cb_flags_set() be updated
to allow disabling the callback flag for this new hook?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260508073355.3916746-1-kuniyu@google.com?part=7