[RESEND mptcp-next v2 0/2] add tcp_recv_should_stop helper

[RESEND mptcp-next v2 0/2] add tcp_recv_should_stop helper

[Geliang Tang]

From: Geliang Tang <tanggeliang@kylinos.cn>

RESEND:
 - to trigger ai review.

Two cleanups for "implement read_sock" series.

v2:
 - v1 changed the original behavior when replacing tcp_recv_should_stop.
   This version preserves the behavior unchanged.
 - add a cleanup patch for read_sock into this set.

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

Gang Yan (1):
  mptcp: check desc->count in read_sock

Geliang Tang (1):
  tcp: add recv_should_stop helper

 include/net/tcp.h    |  9 +++++++++
 net/ipv4/tcp.c       | 10 ++--------
 net/mptcp/protocol.c | 12 ++++--------
 3 files changed, 15 insertions(+), 16 deletions(-)

-- 
2.53.0

[RESEND mptcp-next v2 1/2] mptcp: check desc->count in read_sock

[Geliang Tang]

From: Gang Yan <yangang@kylinos.cn>

In __tcp_read_sock(), it checks whether desc->count equals 0 within the
loop and breaks the loop if true. This check should also be synchronized
to __mptcp_read_sock().

Co-developed-by: Geliang Tang <geliang@kernel.org>
Signed-off-by: Geliang Tang <geliang@kernel.org>
Signed-off-by: Gang Yan <yangang@kylinos.cn>
---
 net/mptcp/protocol.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/net/mptcp/protocol.c b/net/mptcp/protocol.c
index b5676b37f8f4..dc9fdd664871 100644
--- a/net/mptcp/protocol.c
+++ b/net/mptcp/protocol.c
@@ -4384,6 +4384,8 @@ static int __mptcp_read_sock(struct sock *sk, read_descriptor_t *desc,
 		}
 
 		mptcp_eat_recv_skb(sk, skb);
+		if (!desc->count)
+			break;
 	}
 
 	if (noack)
-- 
2.53.0

[RESEND mptcp-next v2 2/2] tcp: add recv_should_stop helper

[Geliang Tang]

From: Geliang Tang <tanggeliang@kylinos.cn>

Factor out a new helper tcp_recv_should_stop() from tcp_recvmsg_locked()
and tcp_splice_read() to check whether to stop receiving. And use this
helper in mptcp_recvmsg() and mptcp_splice_read() to reduce redundant code.

Suggested-by: Paolo Abeni <pabeni@redhat.com>
Acked-by: Mat Martineau <martineau@kernel.org>
Signed-off-by: Geliang Tang <tanggeliang@kylinos.cn>
---
 include/net/tcp.h    |  9 +++++++++
 net/ipv4/tcp.c       | 10 ++--------
 net/mptcp/protocol.c | 10 ++--------
 3 files changed, 13 insertions(+), 16 deletions(-)

diff --git a/include/net/tcp.h b/include/net/tcp.h
index f87bdacb5a69..229613b56f73 100644
--- a/include/net/tcp.h
+++ b/include/net/tcp.h
@@ -3069,4 +3069,13 @@ enum skb_drop_reason tcp_inbound_hash(struct sock *sk,
 		const void *saddr, const void *daddr,
 		int family, int dif, int sdif);
 
+static inline int tcp_recv_should_stop(struct sock *sk, long timeo)
+{
+	return sk->sk_err ||
+	       sk->sk_state == TCP_CLOSE ||
+	       (sk->sk_shutdown & RCV_SHUTDOWN) ||
+	       signal_pending(current) ||
+	       !timeo;
+}
+
 #endif	/* _TCP_H */
diff --git a/net/ipv4/tcp.c b/net/ipv4/tcp.c
index dfd677c689ef..b0df7c4a0e20 100644
--- a/net/ipv4/tcp.c
+++ b/net/ipv4/tcp.c
@@ -895,9 +895,7 @@ ssize_t tcp_splice_read(struct socket *sock, loff_t *ppos,
 		release_sock(sk);
 		lock_sock(sk);
 
-		if (sk->sk_err || sk->sk_state == TCP_CLOSE ||
-		    (sk->sk_shutdown & RCV_SHUTDOWN) ||
-		    signal_pending(current))
+		if (tcp_recv_should_stop(sk, timeo))
 			break;
 	}
 
@@ -2767,11 +2765,7 @@ static int tcp_recvmsg_locked(struct sock *sk, struct msghdr *msg, size_t len,
 			break;
 
 		if (copied) {
-			if (!timeo ||
-			    sk->sk_err ||
-			    sk->sk_state == TCP_CLOSE ||
-			    (sk->sk_shutdown & RCV_SHUTDOWN) ||
-			    signal_pending(current))
+			if (tcp_recv_should_stop(sk, timeo))
 				break;
 		} else {
 			if (sock_flag(sk, SOCK_DONE))
diff --git a/net/mptcp/protocol.c b/net/mptcp/protocol.c
index dc9fdd664871..b26b1098751a 100644
--- a/net/mptcp/protocol.c
+++ b/net/mptcp/protocol.c
@@ -2317,11 +2317,7 @@ static int mptcp_recvmsg(struct sock *sk, struct msghdr *msg, size_t len,
 			break;
 
 		if (copied) {
-			if (sk->sk_err ||
-			    sk->sk_state == TCP_CLOSE ||
-			    (sk->sk_shutdown & RCV_SHUTDOWN) ||
-			    !timeo ||
-			    signal_pending(current))
+			if (tcp_recv_should_stop(sk, timeo))
 				break;
 		} else {
 			if (sk->sk_err) {
@@ -4508,9 +4504,7 @@ static ssize_t mptcp_splice_read(struct socket *sock, loff_t *ppos,
 		release_sock(sk);
 		lock_sock(sk);
 
-		if (sk->sk_err || sk->sk_state == TCP_CLOSE ||
-		    (sk->sk_shutdown & RCV_SHUTDOWN) ||
-		    signal_pending(current))
+		if (tcp_recv_should_stop(sk, timeo))
 			break;
 	}
 
-- 
2.53.0

Re: [RESEND mptcp-next v2 0/2] add tcp_recv_should_stop helper

[MPTCP CI]

Hi Geliang,

Thank you for your modifications, that's great!

Our CI did some validations and here is its report:

- KVM Validation: normal (except selftest_mptcp_join): Success! ✅
- KVM Validation: normal (only selftest_mptcp_join): Success! ✅
- KVM Validation: debug (except selftest_mptcp_join): Success! ✅
- KVM Validation: debug (only selftest_mptcp_join): Success! ✅
- KVM Validation: btf-normal (only bpftest_all): Success! ✅
- KVM Validation: btf-debug (only bpftest_all): Success! ✅
- Task: https://github.com/multipath-tcp/mptcp_net-next/actions/runs/23189152154

Initiator: Patchew Applier
Commits: https://github.com/multipath-tcp/mptcp_net-next/commits/10891a8a1061
Patchwork: https://patchwork.kernel.org/project/mptcp/list/?series=1067876


If there are some issues, you can reproduce them using the same environment as
the one used by the CI thanks to a docker image, e.g.:

    $ cd [kernel source code]
    $ docker run -v "${PWD}:${PWD}:rw" -w "${PWD}" --privileged --rm -it \
        --pull always mptcp/mptcp-upstream-virtme-docker:latest \
        auto-normal

For more details:

    https://github.com/multipath-tcp/mptcp-upstream-virtme-docker


Please note that despite all the efforts that have been already done to have a
stable tests suite when executed on a public CI like here, it is possible some
reported issues are not due to your modifications. Still, do not hesitate to
help us improve that ;-)

Cheers,
MPTCP GH Action bot
Bot operated by Matthieu Baerts (NGI0 Core)

Re: [RESEND mptcp-next v2 1/2] mptcp: check desc->count in read_sock

[Matthieu Baerts]

Hi Gang,

On 17/03/2026 10:44, Geliang Tang wrote:
> From: Gang Yan <yangang@kylinos.cn>
> 
> In __tcp_read_sock(), it checks whether desc->count equals 0 within the
> loop and breaks the loop if true. This check should also be synchronized
> to __mptcp_read_sock().

This description sounds "strange":

- Is this a fix or an optimisation? i.e. is this check needed for some
  reason?
  → If yes, please explain why in the commit message, and eventually add
    a Fixes tag.

- Or is it just some extra code that is not needed for MPTCP, but
  doesn't hurt
  → If yes, do we really need it?

Cheers,
Matt
-- 
Sponsored by the NGI0 Core fund.

Re: [RESEND mptcp-next v2 2/2] tcp: add recv_should_stop helper

[Matthieu Baerts]

Hi Geliang,

On 17/03/2026 10:44, Geliang Tang wrote:
> From: Geliang Tang <tanggeliang@kylinos.cn>
> 
> Factor out a new helper tcp_recv_should_stop() from tcp_recvmsg_locked()
> and tcp_splice_read() to check whether to stop receiving. And use this
> helper in mptcp_recvmsg() and mptcp_splice_read() to reduce redundant code.
> 
> Suggested-by: Paolo Abeni <pabeni@redhat.com>
> Acked-by: Mat Martineau <martineau@kernel.org>
> Signed-off-by: Geliang Tang <tanggeliang@kylinos.cn>
> ---
>  include/net/tcp.h    |  9 +++++++++
>  net/ipv4/tcp.c       | 10 ++--------
>  net/mptcp/protocol.c | 10 ++--------
>  3 files changed, 13 insertions(+), 16 deletions(-)
> 
> diff --git a/include/net/tcp.h b/include/net/tcp.h
> index f87bdacb5a69..229613b56f73 100644
> --- a/include/net/tcp.h
> +++ b/include/net/tcp.h
> @@ -3069,4 +3069,13 @@ enum skb_drop_reason tcp_inbound_hash(struct sock *sk,
>  		const void *saddr, const void *daddr,
>  		int family, int dif, int sdif);
>  
> +static inline int tcp_recv_should_stop(struct sock *sk, long timeo)
> +{
> +	return sk->sk_err ||
> +	       sk->sk_state == TCP_CLOSE ||
> +	       (sk->sk_shutdown & RCV_SHUTDOWN) ||
> +	       signal_pending(current) ||
> +	       !timeo;
> +}
> +
>  #endif	/* _TCP_H */
> diff --git a/net/ipv4/tcp.c b/net/ipv4/tcp.c
> index dfd677c689ef..b0df7c4a0e20 100644
> --- a/net/ipv4/tcp.c
> +++ b/net/ipv4/tcp.c
> @@ -895,9 +895,7 @@ ssize_t tcp_splice_read(struct socket *sock, loff_t *ppos,
>  		release_sock(sk);
>  		lock_sock(sk);
>  
> -		if (sk->sk_err || sk->sk_state == TCP_CLOSE ||
> -		    (sk->sk_shutdown & RCV_SHUTDOWN) ||
> -		    signal_pending(current))
> +		if (tcp_recv_should_stop(sk, timeo))

Here, you are changing the behaviour, that's then not a simple
refactoring like what you are describing in the commit message. Same in
mptcp_splice_read(). If checking 'timeo' here is better for some reason,
please add these two checks in a dedicated commit clearly explaining
this reason.

The alternative is not to include 'timeo' in the new helper, so here you
would have:

  if (tcp_recv_should_stop(sk))

and in tcp_recvmsg_locked():

  if (!timeo || tcp_recv_should_stop(sk))

Cheers,
Matt
-- 
Sponsored by the NGI0 Core fund.