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

[Jiayuan Chen <jiayuan.chen@linux.dev>]

On 5/8/26 7:30 PM, Kuniyuki Iwashima wrote:
> On Fri, May 8, 2026 at 3:37 AM Jiayuan Chen <jiayuan.chen@linux.dev> wrote:
>>
>> On 5/8/26 3:33 PM, Kuniyuki Iwashima wrote:
>>> Now, it is time to add the new hooks for BPF_SOCK_OPS_RCVLOWAT_CB.
>>>
>>> Let's invoke the BPF SOCK_OPS prog when
>>>
>>>     1. TCP stack enqueues skb to sk->sk_receive_queue
>>>        -> tcp_queue_rcv(), tcp_ofo_queue(), and tcp_fastopen_add_skb()
>>>
>>>     2. TCP recvmsg() completes
>>>        -> __tcp_cleanup_rbuf()
>>>
>>> This will allow the BPF prog to parse each skb and dynamically
>>> adjust sk->sk_rcvlowat to suppress unnecessary EPOLLIN wakeups
>>> until sufficient data (e.g., a full RPC frame) is available
>>> in the receive queue.
>>>
>>> Note that the direct access to bpf_sock_ops.data is intentionally
>>> disabled by passing 0 as end_offset.
>>>
>>> Instead, the BPF prog is supposed to use bpf_skb_load_bytes()
>>> with bpf_sock_ops because payload is not in the linear area
>>> with TCP header/data split on and skb may contain a RPC
>>> descriptor in skb frag.  This also simplifies the BPF prog.
>>>
>>> Signed-off-by: Kuniyuki Iwashima <kuniyu@google.com>
>>> ---
>>>    include/net/tcp.h       | 14 ++++++++++++++
>>>    net/ipv4/tcp.c          |  2 ++
>>>    net/ipv4/tcp_fastopen.c |  2 ++
>>>    net/ipv4/tcp_input.c    | 10 ++++++++++
>>>    4 files changed, 28 insertions(+)
>>>
>>> diff --git a/include/net/tcp.h b/include/net/tcp.h
>>> index 4e9e634e276b..003e46c9b500 100644
>>> --- a/include/net/tcp.h
>>> +++ b/include/net/tcp.h
>>> @@ -737,6 +737,20 @@ static inline struct request_sock *cookie_bpf_check(struct net *net, struct sock
>>>    }
>>>    #endif
>>>
>>> +#ifdef CONFIG_CGROUP_BPF
>>> +void bpf_skops_rcvlowat(struct sock *sk, struct sk_buff *skb);
>>> +
>>> +static inline void tcp_bpf_rcvlowat(struct sock *sk, struct sk_buff *skb)
>>> +{
>>> +     if (BPF_SOCK_OPS_TEST_FLAG(tcp_sk(sk), BPF_SOCK_OPS_RCVLOWAT_CB_FLAG))
>>> +             bpf_skops_rcvlowat(sk, skb);
>>> +}
>>> +#else
>>> +static inline void tcp_bpf_rcvlowat(struct sock *sk, struct sk_buff *skb)
>>> +{
>>> +}
>>> +#endif
>>> +
>>>    /* From net/ipv6/syncookies.c */
>>>    int __cookie_v6_check(const struct ipv6hdr *iph, const struct tcphdr *th);
>>>    struct sock *cookie_v6_check(struct sock *sk, struct sk_buff *skb);
>>> diff --git a/net/ipv4/tcp.c b/net/ipv4/tcp.c
>>> index 1d9e52fc454f..80144b97a87a 100644
>>> --- a/net/ipv4/tcp.c
>>> +++ b/net/ipv4/tcp.c
>>> @@ -1602,6 +1602,8 @@ void __tcp_cleanup_rbuf(struct sock *sk, int copied)
>>>                tcp_mstamp_refresh(tp);
>>>                tcp_send_ack(sk);
>>>        }
>>> +
>>> +     tcp_bpf_rcvlowat(sk, NULL);
>>>    }
>>>
>> tcp_read_skb (process frame 1 and __skb_unlink)
>> └─ sk_psock_verdict_recv
>>       └─ sk_psock_verdict_apply
>>           └─ tcp_eat_skb
>>               └─ tcp_cleanup_rbuf
>>                   └─ __tcp_cleanup_rbuf
>>                       └─ BPF RCVLOWAT_CB
>>                           └─ bpf_sock_ops_tcp_set_rcvlowat (wakeup=true)
>>                               └─ tcp_data_ready
>>                                   └─ sk_psock_verdict_data_ready
>>                                       └─ tcp_read_skb (frame 2)
>>                                           └─ ... → tcp_read_skb (frame 3) ...
>>
>> For strparser it use read_sock instead of read_skb and it will become
>> more complicated...
> To be clear, this feature is NOT to use strparser/sockmap.
>> I think this will cause stack overflow with amounts of skbs in receive
>> queue or infinite call(not tested) for sockmap/kTLS/strparser.
>>
> BPF user is responsible for not doing silly things.
>
> tcp_bpf_strp_read_sock() can have loop detection logic,
> but it's only if really needed.


Similar infinite recursion problems for reference:
  https://lore.kernel.org/r/20220929070407.965581-5-martin.lau@linux.dev
  https://lore.kernel.org/bpf/20260421155804.135786-1-kafai.wan@linux.dev/

They were not solved in TCP side but in ops side.


Can we try to handle it on the BPF/OPS side first and only
prevent it elsewhere if it's not feasible there ?


Thanks