Re: [PATCH bpf 1/2] bpf: drop duplicate check_app_limited in tcp_bpf_push

[Geliang Tang <geliang@kernel.org>]

Hi Jiayuan,

On Mon, 2026-09-14 at 16:24 +0800, Jiayuan Chen wrote:
> 
> On 9/14/26 3:14 PM, Geliang Tang wrote:
> > From: Geliang Tang <tanggeliang@kylinos.cn>
> > 
> > When the sendpage->MSG_SPLICE_PAGES migration series replaced
> > do_tcp_sendpages() with direct tcp_sendmsg_locked() calls, callers
> > that
> > had used do_tcp_sendpages() kept an explicit
> > tcp_rate_check_app_limited(sk)
> > that was originally needed to cover do_tcp_sendpages() (which did
> > not call
> > tcp_rate_check_app_limited()). After the inlining,
> > tcp_sendmsg_locked()
> > always provides the check, and the outer call became redundant.
> > 
> > The site changed here, tcp_bpf_push(), is a MSG_SPLICE_PAGES loop
> > that
> > holds the socket lock and only iterates when size > 0;
> > tcp_sendmsg_locked()
> > is invoked on every iteration with state identical to what the
> > outer call
> > sees, so dropping the outer call is safe and behavior-preserving.
> > 
> > A potential benefit of this change is that it facilitates future
> > reuse of
> > tcp_bpf_push() for sockmap support in protocols beyond TCP, such as
> > MPTCP.
> > Since tcp_rate_check_app_limited() is TCP-specific while
> > sendmsg_locked()
> > is a generic interface in struct proto_ops, this change allows us
> > to switch
> > to different protocols via sk->sk_socket->ops->sendmsg_locked()
> > without
> > carrying protocol-specific assumptions.
> > 
> > Fixes: ebf2e8860eea ("tcp_bpf: Inline do_tcp_sendpages as it's now
> > a wrapper around tcp_sendmsg")
> 
> 
> Same as the tls one: this looks like a cleanup to me. Is there a real
> regression that affects kernel or user behavior?
> 
> Do we really need a Fixes tag?

Thanks for the review. I fully agree - I'll drop the "Fixes" tags and
update the commit logs in v2.

Thanks,
-Geliang

> 
> 
> > Signed-off-by: Geliang Tang <tanggeliang@kylinos.cn>
> > ---
> >   net/ipv4/tcp_bpf.c | 1 -
> >   1 file changed, 1 deletion(-)
> > 
> > diff --git a/net/ipv4/tcp_bpf.c b/net/ipv4/tcp_bpf.c
> > index 2e234d155b5e..d5fcf3ce4861 100644
> > --- a/net/ipv4/tcp_bpf.c
> > +++ b/net/ipv4/tcp_bpf.c
> > @@ -108,7 +108,6 @@ static int tcp_bpf_push(struct sock *sk, struct
> > sk_msg *msg, u32 apply_bytes,
> >   		off  = sge->offset;
> >   		page = sg_page(sge);
> >   
> > -		tcp_rate_check_app_limited(sk);
> >   retry:
> >   		msghdr.msg_flags = flags | MSG_SPLICE_PAGES;
> >   		has_tx_ulp = tls_sw_has_ctx_tx(sk);