[PATCH bpf-next v3] bpf: drop duplicate check_app_limited in tcp_bpf_push

[PATCH bpf-next v3] bpf: drop duplicate check_app_limited in tcp_bpf_push

[Geliang Tang]

From: Geliang Tang <tanggeliang@kylinos.cn>

Before commit c5c37af6ecad9 ("tcp: Convert do_tcp_sendpages() to use
MSG_SPLICE_PAGES"), do_tcp_sendpages() did not call
tcp_rate_check_app_limited() internally, so callers needed an explicit
tcp_rate_check_app_limited() to cover it. That commit replaced
do_tcp_sendpages() with direct tcp_sendmsg_locked() calls, which perform
the check on every path that queues data. The outer call became redundant
but was left in place.

The site changed here, tcp_bpf_push(), holds the socket lock and invokes
tcp_sendmsg_locked() on every iteration. The early-return paths in
tcp_sendmsg_locked() that skip tcp_rate_check_app_limited() -- the
MSG_ZEROCOPY allocation failure and MSG_FASTOPEN branches -- return
without queueing any MSG_SPLICE_PAGES data, so there is no functional
consequence from omitting the outer check.

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.

Signed-off-by: Geliang Tang <tanggeliang@kylinos.cn>
---
Note:

This patch was originally part of my ongoing "MPTCP sockmap support"
series [1] (patch 5). Matthieu suggested converting it to a fix and sending
it directly to netdev. Removing the redundant tcp_rate_check_app_limited()
call benefits my subsequent MPTCP work: in patch 3 of that series, I
implement an MPTCP-specific mptcp_rate_check_app_limited() function and
call it in mptcp_sendmsg_locked(). This allows me to reuse tcp_bpf_push()
by simply replacing tcp_sendmsg_locked() with

		sk->sk_socket->ops->sendmsg_locked(),

without carrying protocol-specific assumptions.

v3:
 - send this patch separately to bpf as Jakub suggested.
 - update the commit log as bot+bpf-ci suggested.

v2:
 - drop the "Fixes" tags and update the prefixes and commit logs as Jiayuan
   suggested.
 - https://patchwork.kernel.org/project/netdevbpf/cover/cover.1789469930.git.tanggeliang@kylinos.cn/

v1:
 - https://patchwork.kernel.org/project/netdevbpf/cover/cover.1789368148.git.tanggeliang@kylinos.cn/

[1]
MPTCP sockmap support
https://lore.kernel.org/mptcp/b5f9e8d7-b738-1df6-3b5e-1d54cbbc663c@gmail.com/T/#t
---
 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);
-- 
2.53.0

Re: [PATCH bpf-next v3] bpf: drop duplicate check_app_limited in tcp_bpf_push

[bot+bpf-ci]

[-- Attachment #1: Type: text/plain, Size: 1369 bytes --]

> bpf: drop duplicate check_app_limited in tcp_bpf_push
>
> Before commit c5c37af6ecad9 ("tcp: Convert do_tcp_sendpages() to use
> MSG_SPLICE_PAGES"), do_tcp_sendpages() did not call
> tcp_rate_check_app_limited() internally, so callers needed an explicit
> tcp_rate_check_app_limited() to cover it. That commit replaced
> do_tcp_sendpages() with direct tcp_sendmsg_locked() calls, which perform
> the check on every path that queues data. The outer call became redundant
> but was left in place.

This isn't a bug, but would it be clearer to cite ebf2e8860eea ("tcp_bpf:
Inline do_tcp_sendpages as it's now a wrapper around tcp_sendmsg") as the
commit that left the outer tcp_rate_check_app_limited() in tcp_bpf_push(),
with c5c37af6ecad9 mentioned as the commit that moved the check inside
do_tcp_sendpages()?

Looking at the git history, c5c37af6ecad9 only touched net/ipv4/tcp.c and
turned do_tcp_sendpages() into a wrapper that calls tcp_sendmsg_locked().
The direct tcp_sendmsg_locked() call in tcp_bpf_push() was introduced by
ebf2e8860eea, which kept the outer tcp_rate_check_app_limited(sk) while
inlining the wrapper.


---
AI reviewed your patch. Please fix the bug or email reply why it's not a bug.
See: https://github.com/kernel-patches/vmtest/blob/master/ci/claude/README.md

CI run summary: https://github.com/kernel-patches/bpf/actions/runs/35203307255

Re: [PATCH bpf-next v3] bpf: drop duplicate check_app_limited in tcp_bpf_push

[Jiayuan Chen]

On 9/17/26 5:45 PM, bot+bpf-ci@kernel.org wrote:
>> bpf: drop duplicate check_app_limited in tcp_bpf_push
>>
>> Before commit c5c37af6ecad9 ("tcp: Convert do_tcp_sendpages() to use
>> MSG_SPLICE_PAGES"), do_tcp_sendpages() did not call
>> tcp_rate_check_app_limited() internally, so callers needed an explicit
>> tcp_rate_check_app_limited() to cover it. That commit replaced
>> do_tcp_sendpages() with direct tcp_sendmsg_locked() calls, which perform
>> the check on every path that queues data. The outer call became redundant
>> but was left in place.
> This isn't a bug, but would it be clearer to cite ebf2e8860eea ("tcp_bpf:
> Inline do_tcp_sendpages as it's now a wrapper around tcp_sendmsg") as the
> commit that left the outer tcp_rate_check_app_limited() in tcp_bpf_push(),
> with c5c37af6ecad9 mentioned as the commit that moved the check inside
> do_tcp_sendpages()?
>
> Looking at the git history, c5c37af6ecad9 only touched net/ipv4/tcp.c and
> turned do_tcp_sendpages() into a wrapper that calls tcp_sendmsg_locked().
> The direct tcp_sendmsg_locked() call in tcp_bpf_push() was introduced by
> ebf2e8860eea, which kept the outer tcp_rate_check_app_limited(sk) while
> inlining the wrapper.
>

CI is right. ebf2e8860eea should be accurate.


> ---
> AI reviewed your patch. Please fix the bug or email reply why it's not a bug.
> See: https://github.com/kernel-patches/vmtest/blob/master/ci/claude/README.md
>
> CI run summary: https://github.com/kernel-patches/bpf/actions/runs/35203307255

Re: [PATCH bpf-next v3] bpf: drop duplicate check_app_limited in tcp_bpf_push

[Geliang Tang]

Hi Jiayuan,

On Mon, 2026-09-21 at 19:46 +0800, Jiayuan Chen wrote:
> 
> On 9/17/26 5:45 PM, bot+bpf-ci@kernel.org wrote:
> > > bpf: drop duplicate check_app_limited in tcp_bpf_push
> > > 
> > > Before commit c5c37af6ecad9 ("tcp: Convert do_tcp_sendpages() to
> > > use
> > > MSG_SPLICE_PAGES"), do_tcp_sendpages() did not call
> > > tcp_rate_check_app_limited() internally, so callers needed an
> > > explicit
> > > tcp_rate_check_app_limited() to cover it. That commit replaced
> > > do_tcp_sendpages() with direct tcp_sendmsg_locked() calls, which
> > > perform
> > > the check on every path that queues data. The outer call became
> > > redundant
> > > but was left in place.
> > This isn't a bug, but would it be clearer to cite ebf2e8860eea
> > ("tcp_bpf:
> > Inline do_tcp_sendpages as it's now a wrapper around tcp_sendmsg")
> > as the
> > commit that left the outer tcp_rate_check_app_limited() in
> > tcp_bpf_push(),
> > with c5c37af6ecad9 mentioned as the commit that moved the check
> > inside
> > do_tcp_sendpages()?
> > 
> > Looking at the git history, c5c37af6ecad9 only touched
> > net/ipv4/tcp.c and
> > turned do_tcp_sendpages() into a wrapper that calls
> > tcp_sendmsg_locked().
> > The direct tcp_sendmsg_locked() call in tcp_bpf_push() was
> > introduced by
> > ebf2e8860eea, which kept the outer tcp_rate_check_app_limited(sk)
> > while
> > inlining the wrapper.
> > 
> 
> CI is right. ebf2e8860eea should be accurate.

You're right, thanks for the careful review. ebf2e8860eea is indeed the
commit that inlined the wrapper and left the outer check in place.
Updated in v4.

Thanks,
-Geliang

> 
> 
> > ---
> > AI reviewed your patch. Please fix the bug or email reply why it's
> > not a bug.
> > See:
> > https://github.com/kernel-patches/vmtest/blob/master/ci/claude/README.md
> > 
> > CI run summary:
> > https://github.com/kernel-patches/bpf/actions/runs/35203307255