Re: [PATCH v1 bpf-next 3/8] bpf: tcp: Support bpf_skb_load_bytes() 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:15 AM Stanislav Fomichev <sdf.kernel@gmail.com> wrote:
> >
> > On 05/08, Kuniyuki Iwashima wrote:
> > > When a TCP skb is queued to sk->sk_receive_queue, BPF SOCK_OPS
> > > prog can be called with BPF_SOCK_OPS_RCVLOWAT_CB.
> > >
> > > In this hook, we want to parse the RPC descriptor in the skb
> > > and adjust sk->sk_rcvlowat based on the RPC frame size.
> > >
> > > However, we cannot access payload via bpf_sock_ops.data on
> > > modern NICs with TCP header/data split on as the payload is
> > > not placed in the linear area.
> > >
> > > Let's support bpf_skb_load_bytes() for BPF_SOCK_OPS_RCVLOWAT_CB.
> > >
> > > Two notes:
> > >
> > >   1) bpf_sock_ops_kern.skb will be NULL when the BPF prog is
> > >       invoked from recvmsg().
> > >
> > >   2) Access to bpf_sock_ops.data will be disabled by passing
> > >       0 end_offset to bpf_skops_init_skb().
> > >
> > > Signed-off-by: Kuniyuki Iwashima <kuniyu@google.com>
> > > ---
> > >  net/core/filter.c | 34 ++++++++++++++++++++++++++++++++++
> > >  1 file changed, 34 insertions(+)
> > >
> > > diff --git a/net/core/filter.c b/net/core/filter.c
> > > index 5fa9189eb772..94d07a15b2ab 100644
> > > --- a/net/core/filter.c
> > > +++ b/net/core/filter.c
> > > @@ -7718,6 +7718,38 @@ static const struct bpf_func_proto bpf_sk_assign_proto = {
> > >       .arg3_type      = ARG_ANYTHING,
> > >  };
> > >
> > > +BPF_CALL_4(bpf_sock_ops_skb_load_bytes, struct bpf_sock_ops_kern *, bpf_sock,
> > > +        u32, offset, void *, to, u32, len)
> > > +{
> > > +     int err;
> > > +
> > > +     if (bpf_sock->op != BPF_SOCK_OPS_RCVLOWAT_CB) {
> > > +             err = -EOPNOTSUPP;
> > > +             goto err_clear;
> > > +     }
> > > +
> > > +     if (!bpf_sock->skb) {
> > > +             err = -EPERM;
> > > +             goto err_clear;
> > > +     }
> > > +
> > > +     return ____bpf_skb_load_bytes(bpf_sock->skb, offset, to, len);
> >
> > nit: wondering whether it's better to use __bpf_skb_load_bytes (which
> > wraps BPF_CALL_4's ____bpf_skb_load_bytes) here? Seems a bit less
> > magic and used already by dynptr?
> 
> I guess dynptr used it because dynptr is in a different file under kernel/.
> 
> Users in the same file (sk_reuseport_load_bytes() and
> sk_reuseport_load_bytes_relative()) call ____bpf_skb_load_bytes()
> directly and clang inlined it there (including __bpf_skb_load_bytes())
> while dynptr tail-called __bpf_skb_load_bytes().
> 
> I see the point (I had to read BPF_CALL_4() macro for ____), but
> ____bpf_skb_load_bytes() looks better.

SG!