[PATCH mptcp-next v14 3/8] tcp: add recv_should_stop helper

Geliang Tang posted 8 patches 2 months, 2 weeks ago
There is a newer version of this series
[PATCH mptcp-next v14 3/8] tcp: add recv_should_stop helper
Posted by Geliang Tang 2 months, 2 weeks ago
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. It will be used
for MPTCP too.

Suggested-by: Paolo Abeni <pabeni@redhat.com>
Signed-off-by: Geliang Tang <tanggeliang@kylinos.cn>
---
 include/net/tcp.h | 23 +++++++++++++++
 net/ipv4/tcp.c    | 73 ++++++++---------------------------------------
 2 files changed, 35 insertions(+), 61 deletions(-)

diff --git a/include/net/tcp.h b/include/net/tcp.h
index 0deb5e9dd911..94a50a9daf65 100644
--- a/include/net/tcp.h
+++ b/include/net/tcp.h
@@ -2942,4 +2942,27 @@ 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)
+{
+	if (sock_flag(sk, SOCK_DONE))
+		return -ENETDOWN;
+
+	if (sk->sk_err)
+		return sock_error(sk);
+
+	if (sk->sk_shutdown & RCV_SHUTDOWN)
+		return -ESHUTDOWN;
+
+	if (sk->sk_state == TCP_CLOSE)
+		return -ENOTCONN;
+
+	if (!timeo)
+		return -EAGAIN;
+
+	if (signal_pending(current))
+		return sock_intr_errno(timeo);
+
+	return 0;
+}
+
 #endif	/* _TCP_H */
diff --git a/net/ipv4/tcp.c b/net/ipv4/tcp.c
index 5265a9453814..8b4917a7f262 100644
--- a/net/ipv4/tcp.c
+++ b/net/ipv4/tcp.c
@@ -842,26 +842,14 @@ ssize_t tcp_splice_read(struct socket *sock, loff_t *ppos,
 		if (ret < 0)
 			break;
 		else if (!ret) {
+			int err;
+
 			if (spliced)
 				break;
-			if (sock_flag(sk, SOCK_DONE))
-				break;
-			if (sk->sk_err) {
-				ret = sock_error(sk);
-				break;
-			}
-			if (sk->sk_shutdown & RCV_SHUTDOWN)
-				break;
-			if (sk->sk_state == TCP_CLOSE) {
-				/*
-				 * This occurs when user tries to read
-				 * from never connected socket.
-				 */
-				ret = -ENOTCONN;
-				break;
-			}
-			if (!timeo) {
-				ret = -EAGAIN;
+			err = tcp_recv_should_stop(sk, timeo);
+			if (err < 0) {
+				if (err != -ENETDOWN && err != -ESHUTDOWN)
+					ret = err;
 				break;
 			}
 			/* if __tcp_splice_read() got nothing while we have
@@ -873,10 +861,6 @@ ssize_t tcp_splice_read(struct socket *sock, loff_t *ppos,
 			ret = sk_wait_data(sk, &timeo, NULL);
 			if (ret < 0)
 				break;
-			if (signal_pending(current)) {
-				ret = sock_intr_errno(timeo);
-				break;
-			}
 			continue;
 		}
 		tss.len -= ret;
@@ -887,9 +871,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;
 	}
 
@@ -2732,42 +2714,11 @@ static int tcp_recvmsg_locked(struct sock *sk, struct msghdr *msg, size_t len,
 		if (copied >= target && !READ_ONCE(sk->sk_backlog.tail))
 			break;
 
-		if (copied) {
-			if (!timeo ||
-			    sk->sk_err ||
-			    sk->sk_state == TCP_CLOSE ||
-			    (sk->sk_shutdown & RCV_SHUTDOWN) ||
-			    signal_pending(current))
-				break;
-		} else {
-			if (sock_flag(sk, SOCK_DONE))
-				break;
-
-			if (sk->sk_err) {
-				copied = sock_error(sk);
-				break;
-			}
-
-			if (sk->sk_shutdown & RCV_SHUTDOWN)
-				break;
-
-			if (sk->sk_state == TCP_CLOSE) {
-				/* This occurs when user tries to read
-				 * from never connected socket.
-				 */
-				copied = -ENOTCONN;
-				break;
-			}
-
-			if (!timeo) {
-				copied = -EAGAIN;
-				break;
-			}
-
-			if (signal_pending(current)) {
-				copied = sock_intr_errno(timeo);
-				break;
-			}
+		err = tcp_recv_should_stop(sk, timeo);
+		if (err < 0) {
+			if (!copied && err != -ENETDOWN && err != -ESHUTDOWN)
+				copied = err;
+			break;
 		}
 
 		if (copied >= target) {
-- 
2.43.0
Re: [PATCH mptcp-next v14 3/8] tcp: add recv_should_stop helper
Posted by Mat Martineau 2 months ago
On Mon, 24 Nov 2025, 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. It will be used
> for MPTCP too.
>
> Suggested-by: Paolo Abeni <pabeni@redhat.com>
> Signed-off-by: Geliang Tang <tanggeliang@kylinos.cn>
> ---
> include/net/tcp.h | 23 +++++++++++++++
> net/ipv4/tcp.c    | 73 ++++++++---------------------------------------
> 2 files changed, 35 insertions(+), 61 deletions(-)
>

Acked-by: Mat Martineau <martineau@kernel.org>

> diff --git a/include/net/tcp.h b/include/net/tcp.h
> index 0deb5e9dd911..94a50a9daf65 100644
> --- a/include/net/tcp.h
> +++ b/include/net/tcp.h
> @@ -2942,4 +2942,27 @@ 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)
> +{
> +	if (sock_flag(sk, SOCK_DONE))
> +		return -ENETDOWN;
> +
> +	if (sk->sk_err)
> +		return sock_error(sk);
> +
> +	if (sk->sk_shutdown & RCV_SHUTDOWN)
> +		return -ESHUTDOWN;
> +
> +	if (sk->sk_state == TCP_CLOSE)
> +		return -ENOTCONN;
> +
> +	if (!timeo)
> +		return -EAGAIN;
> +
> +	if (signal_pending(current))
> +		return sock_intr_errno(timeo);
> +
> +	return 0;
> +}
> +
> #endif	/* _TCP_H */
> diff --git a/net/ipv4/tcp.c b/net/ipv4/tcp.c
> index 5265a9453814..8b4917a7f262 100644
> --- a/net/ipv4/tcp.c
> +++ b/net/ipv4/tcp.c
> @@ -842,26 +842,14 @@ ssize_t tcp_splice_read(struct socket *sock, loff_t *ppos,
> 		if (ret < 0)
> 			break;
> 		else if (!ret) {
> +			int err;
> +
> 			if (spliced)
> 				break;
> -			if (sock_flag(sk, SOCK_DONE))
> -				break;
> -			if (sk->sk_err) {
> -				ret = sock_error(sk);
> -				break;
> -			}
> -			if (sk->sk_shutdown & RCV_SHUTDOWN)
> -				break;
> -			if (sk->sk_state == TCP_CLOSE) {
> -				/*
> -				 * This occurs when user tries to read
> -				 * from never connected socket.
> -				 */
> -				ret = -ENOTCONN;
> -				break;
> -			}
> -			if (!timeo) {
> -				ret = -EAGAIN;
> +			err = tcp_recv_should_stop(sk, timeo);
> +			if (err < 0) {
> +				if (err != -ENETDOWN && err != -ESHUTDOWN)
> +					ret = err;
> 				break;
> 			}
> 			/* if __tcp_splice_read() got nothing while we have
> @@ -873,10 +861,6 @@ ssize_t tcp_splice_read(struct socket *sock, loff_t *ppos,
> 			ret = sk_wait_data(sk, &timeo, NULL);
> 			if (ret < 0)
> 				break;
> -			if (signal_pending(current)) {
> -				ret = sock_intr_errno(timeo);
> -				break;
> -			}
> 			continue;
> 		}
> 		tss.len -= ret;
> @@ -887,9 +871,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;
> 	}
>
> @@ -2732,42 +2714,11 @@ static int tcp_recvmsg_locked(struct sock *sk, struct msghdr *msg, size_t len,
> 		if (copied >= target && !READ_ONCE(sk->sk_backlog.tail))
> 			break;
>
> -		if (copied) {
> -			if (!timeo ||
> -			    sk->sk_err ||
> -			    sk->sk_state == TCP_CLOSE ||
> -			    (sk->sk_shutdown & RCV_SHUTDOWN) ||
> -			    signal_pending(current))
> -				break;
> -		} else {
> -			if (sock_flag(sk, SOCK_DONE))
> -				break;
> -
> -			if (sk->sk_err) {
> -				copied = sock_error(sk);
> -				break;
> -			}
> -
> -			if (sk->sk_shutdown & RCV_SHUTDOWN)
> -				break;
> -
> -			if (sk->sk_state == TCP_CLOSE) {
> -				/* This occurs when user tries to read
> -				 * from never connected socket.
> -				 */
> -				copied = -ENOTCONN;
> -				break;
> -			}
> -
> -			if (!timeo) {
> -				copied = -EAGAIN;
> -				break;
> -			}
> -
> -			if (signal_pending(current)) {
> -				copied = sock_intr_errno(timeo);
> -				break;
> -			}
> +		err = tcp_recv_should_stop(sk, timeo);
> +		if (err < 0) {
> +			if (!copied && err != -ENETDOWN && err != -ESHUTDOWN)
> +				copied = err;
> +			break;
> 		}
>
> 		if (copied >= target) {
> -- 
> 2.43.0
>
>
>
Re: [PATCH mptcp-next v14 3/8] tcp: add recv_should_stop helper
Posted by Mat Martineau 2 months ago
On Mon, 24 Nov 2025, 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. It will be used
> for MPTCP too.
>
> Suggested-by: Paolo Abeni <pabeni@redhat.com>
> Signed-off-by: Geliang Tang <tanggeliang@kylinos.cn>
> ---
> include/net/tcp.h | 23 +++++++++++++++
> net/ipv4/tcp.c    | 73 ++++++++---------------------------------------
> 2 files changed, 35 insertions(+), 61 deletions(-)
>
> diff --git a/include/net/tcp.h b/include/net/tcp.h
> index 0deb5e9dd911..94a50a9daf65 100644
> --- a/include/net/tcp.h
> +++ b/include/net/tcp.h
> @@ -2942,4 +2942,27 @@ 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)
> +{
> +	if (sock_flag(sk, SOCK_DONE))
> +		return -ENETDOWN;
> +
> +	if (sk->sk_err)
> +		return sock_error(sk);

Hi Geliang -

sock_error() will clear sk_err, and this introduces some changes in 
certain code paths that are refactored below (especially where data was 
read successfully before the error was encountered).

I suggest either handling sk_err outside of this helper so the exact 
behavior of the existing code can be maintained, or maybe defer this 
helper for a separate patch series.

- Mat


> +
> +	if (sk->sk_shutdown & RCV_SHUTDOWN)
> +		return -ESHUTDOWN;
> +
> +	if (sk->sk_state == TCP_CLOSE)
> +		return -ENOTCONN;
> +
> +	if (!timeo)
> +		return -EAGAIN;
> +
> +	if (signal_pending(current))
> +		return sock_intr_errno(timeo);
> +
> +	return 0;
> +}
> +
> #endif	/* _TCP_H */
> diff --git a/net/ipv4/tcp.c b/net/ipv4/tcp.c
> index 5265a9453814..8b4917a7f262 100644
> --- a/net/ipv4/tcp.c
> +++ b/net/ipv4/tcp.c
> @@ -842,26 +842,14 @@ ssize_t tcp_splice_read(struct socket *sock, loff_t *ppos,
> 		if (ret < 0)
> 			break;
> 		else if (!ret) {
> +			int err;
> +
> 			if (spliced)
> 				break;
> -			if (sock_flag(sk, SOCK_DONE))
> -				break;
> -			if (sk->sk_err) {
> -				ret = sock_error(sk);
> -				break;
> -			}
> -			if (sk->sk_shutdown & RCV_SHUTDOWN)
> -				break;
> -			if (sk->sk_state == TCP_CLOSE) {
> -				/*
> -				 * This occurs when user tries to read
> -				 * from never connected socket.
> -				 */
> -				ret = -ENOTCONN;
> -				break;
> -			}
> -			if (!timeo) {
> -				ret = -EAGAIN;
> +			err = tcp_recv_should_stop(sk, timeo);
> +			if (err < 0) {
> +				if (err != -ENETDOWN && err != -ESHUTDOWN)
> +					ret = err;
> 				break;
> 			}
> 			/* if __tcp_splice_read() got nothing while we have
> @@ -873,10 +861,6 @@ ssize_t tcp_splice_read(struct socket *sock, loff_t *ppos,
> 			ret = sk_wait_data(sk, &timeo, NULL);
> 			if (ret < 0)
> 				break;
> -			if (signal_pending(current)) {
> -				ret = sock_intr_errno(timeo);
> -				break;
> -			}
> 			continue;
> 		}
> 		tss.len -= ret;
> @@ -887,9 +871,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;
> 	}
>
> @@ -2732,42 +2714,11 @@ static int tcp_recvmsg_locked(struct sock *sk, struct msghdr *msg, size_t len,
> 		if (copied >= target && !READ_ONCE(sk->sk_backlog.tail))
> 			break;
>
> -		if (copied) {
> -			if (!timeo ||
> -			    sk->sk_err ||
> -			    sk->sk_state == TCP_CLOSE ||
> -			    (sk->sk_shutdown & RCV_SHUTDOWN) ||
> -			    signal_pending(current))
> -				break;
> -		} else {
> -			if (sock_flag(sk, SOCK_DONE))
> -				break;
> -
> -			if (sk->sk_err) {
> -				copied = sock_error(sk);
> -				break;
> -			}
> -
> -			if (sk->sk_shutdown & RCV_SHUTDOWN)
> -				break;
> -
> -			if (sk->sk_state == TCP_CLOSE) {
> -				/* This occurs when user tries to read
> -				 * from never connected socket.
> -				 */
> -				copied = -ENOTCONN;
> -				break;
> -			}
> -
> -			if (!timeo) {
> -				copied = -EAGAIN;
> -				break;
> -			}
> -
> -			if (signal_pending(current)) {
> -				copied = sock_intr_errno(timeo);
> -				break;
> -			}
> +		err = tcp_recv_should_stop(sk, timeo);
> +		if (err < 0) {
> +			if (!copied && err != -ENETDOWN && err != -ESHUTDOWN)
> +				copied = err;
> +			break;
> 		}
>
> 		if (copied >= target) {
> -- 
> 2.43.0
>
>
>