Re: [PATCH net 2/2] tls: drop duplicate check_app_limited in tls_push_sg

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.

Fixes: e117dcfd646e ("tls: Inline do_tcp_sendpages()")

Does the Fixes: tag identify the correct commit? The changelog states the
outer call "was originally needed to cover do_tcp_sendpages() (which did
not call tcp_rate_check_app_limited())" and that it "became redundant"
only "after the inlining". But e117dcfd646e did not introduce the
redundancy.

At e117dcfd646e's parent, do_tcp_sendpages() was already a thin wrapper
that funnels into tcp_sendmsg_locked():

  ssize_t do_tcp_sendpages(struct sock *sk, struct page *page, int offset,
                           size_t size, int flags)
  {
          struct bio_vec bvec;
          struct msghdr msg = { .msg_flags = flags | MSG_SPLICE_PAGES, };
          bvec_set_page(&bvec, page, size, offset);
          iov_iter_bvec(&msg.msg_iter, ITER_SOURCE, &bvec, 1, size);
          if (flags & MSG_SENDPAGE_NOTLAST)
                  msg.msg_flags |= MSG_MORE;
          return tcp_sendmsg_locked(sk, &msg, size);
  }

The conversion that made this true - and therefore the commit that made
the tls_push_sg() call redundant - is c5c37af6ecad9 ("tcp: Convert
do_tcp_sendpages() to use MSG_SPLICE_PAGES"), three commits earlier in
the same series. Before c5c37af6ecad9, do_tcp_sendpages() open-coded the
send loop and indeed had no rate check, and tcp_sendmsg_locked() already
carried tcp_rate_check_app_limited() at that point. e117dcfd646e only
replaced the wrapper call with its body; it left the
tcp_rate_check_app_limited(sk) line untouched as diff context.

If a Fixes: tag is kept, should it be:

  Fixes: c5c37af6ecad9 ("tcp: Convert do_tcp_sendpages() to use MSG_SPLICE_PAGES")

Both commits landed in the same merge window, so the practical backport
range is unchanged.

so dropping the outer call is safe and behavior-preserving.

A potential benefit of this change is that it facilitates future reuse of
tls_push_sg() for KTLS support in protocols beyond TCP, such as MPTCP.

Fixes: e117dcfd646e ("tls: Inline do_tcp_sendpages()")

Should this patch carry a Fixes: tag? The changelog describes the change
as "safe and behavior-preserving" and motivates it as cleanup plus
preparation for future MPTCP reuse. No user-visible bug, regression, crash
or misbehaviour is described or fixed: the removed
tcp_rate_check_app_limited(sk) is duplicated by the identical call inside
tcp_sendmsg_locked(), so tp->app_limited ends up with the same value
either way.

A Fixes: tag on a no-op cleanup mainly serves to draw the patch into
automated stable selection (AUTOSEL) for no benefit. Either drop the tag,
or state concretely what observable behaviour was wrong before so the tag
is justified.


---
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/34817950722