Re: [PATCH v1 bpf-next 8/8] selftest: bpf: Add test for BPF_SOCK_OPS_RCVLOWAT_CB.

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

On 05/08, Kuniyuki Iwashima wrote:
> On Fri, May 8, 2026 at 8:35 AM Stanislav Fomichev <sdf.kernel@gmail.com> wrote:
> >
> > On 05/08, Kuniyuki Iwashima wrote:
> > > The test is roughly divided into two stages, and the sequence
> > > is as follows:
> > >
> > >   I) Setup
> > >
> > >     1. Attach two BPF programs to a cgroup
> > >     2. Establish a TCP connection (@client <-> @child) within the cgroup
> > >     3. Enable BPF_SOCK_OPS_RCVLOWAT_CB on @child
> > >
> > >  II) RPC frame exchange in various patterns
> > >
> > >     4. Send a partial RPC descriptor from @client to @child
> > >     5. Verify that epoll does NOT wake up @child
> > >     6. Send the remaining data of the RPC frame
> > >     7. Verify that epoll finally wakes up @child
> > >
> > > During setup, two BPF programs are attached to simulate
> > > a real-world scenario; one is SOCK_OPS and the other is
> > > CGROUP_SOCKOPT.
> > >
> > > While the SOCK_OPS prog handles the dynamic adjustment of
> > > sk->sk_rcvlowat, the CGROUP_SOCKOPT prog is used to enable
> > > BPF_SOCK_OPS_RCVLOWAT_CB via userspace setsockopt() using
> > > pseudo options:
> > >
> > >   #define SOL_BPF               0xdeadbeef
> > >   #define BPF_TCP_AUTOLOWAT     0x8badf00d
> > >
> > >   setsockopt(fd, SOL_BPF, BPF_TCP_AUTOLOWAT, &(int){1}, sizeof(int));
> >
> > Hmm, so you do want to have this enable-on-a-per-socket use case.. Then what
> > happens to the skbs already sitting in the rx queue by the time
> > we enable BPF_SOCK_OPS_RCVLOWAT_CB? Isn't there a race? Or am I missing
> > something?
> 
> If the upper protocol is designed that way, it could be racy.
> 
> For example, HTTP2 could have multiple streams within the
> same TCP connection while only one stream can be upgraded.
> But this use case simply does not work with a single
> sk->sk_rcvlowat in the first place.
> 
> This feature implicitly assumes a single stream per connection.
> 
> In our original implementation, it traversed all (technically
> up to 100) skbs in the queue and parse them like psock.
> 
> But this will be unnecessary if the server drains the queue
> properly before enabling the feature.
> 
> If needed, we could return -EBUSY when the queue is not
> empty.  It's always under lock_sock() or bh_lock_sock().

But even for a single stream it's racy, no? Suppose I do recvmsg()
and then setsockopt(BPF_TCP_AUTOLOWAT, 1). There is a window of time
between recvmsg() and setsockopt() where we can receive a packet?
Or you're saying that if it is an-RPC type protocol, the receiver
explicitly signals to the sender that it's ready to receive (by sending
it an RPC request). If that's the case, let's explicitly explain this
somewhere?