* [RESEND mptcp-next v2 0/2] add tcp_recv_should_stop helper
@ 2026-03-17 9:44 Geliang Tang
2026-03-17 9:44 ` [RESEND mptcp-next v2 1/2] mptcp: check desc->count in read_sock Geliang Tang
` (2 more replies)
0 siblings, 3 replies; 6+ messages in thread
From: Geliang Tang @ 2026-03-17 9:44 UTC (permalink / raw)
To: mptcp; +Cc: 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
^ permalink raw reply [flat|nested] 6+ messages in thread* [RESEND mptcp-next v2 1/2] mptcp: check desc->count in read_sock 2026-03-17 9:44 [RESEND mptcp-next v2 0/2] add tcp_recv_should_stop helper Geliang Tang @ 2026-03-17 9:44 ` Geliang Tang 2026-03-26 18:21 ` Matthieu Baerts 2026-03-17 9:44 ` [RESEND mptcp-next v2 2/2] tcp: add recv_should_stop helper Geliang Tang 2026-03-17 11:13 ` [RESEND mptcp-next v2 0/2] add tcp_recv_should_stop helper MPTCP CI 2 siblings, 1 reply; 6+ messages in thread From: Geliang Tang @ 2026-03-17 9:44 UTC (permalink / raw) To: mptcp; +Cc: Gang Yan, 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 ^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [RESEND mptcp-next v2 1/2] mptcp: check desc->count in read_sock 2026-03-17 9:44 ` [RESEND mptcp-next v2 1/2] mptcp: check desc->count in read_sock Geliang Tang @ 2026-03-26 18:21 ` Matthieu Baerts 0 siblings, 0 replies; 6+ messages in thread From: Matthieu Baerts @ 2026-03-26 18:21 UTC (permalink / raw) To: Geliang Tang, mptcp; +Cc: Gang Yan 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. ^ permalink raw reply [flat|nested] 6+ messages in thread
* [RESEND mptcp-next v2 2/2] tcp: add recv_should_stop helper 2026-03-17 9:44 [RESEND mptcp-next v2 0/2] add tcp_recv_should_stop helper Geliang Tang 2026-03-17 9:44 ` [RESEND mptcp-next v2 1/2] mptcp: check desc->count in read_sock Geliang Tang @ 2026-03-17 9:44 ` Geliang Tang 2026-03-26 18:29 ` Matthieu Baerts 2026-03-17 11:13 ` [RESEND mptcp-next v2 0/2] add tcp_recv_should_stop helper MPTCP CI 2 siblings, 1 reply; 6+ messages in thread From: Geliang Tang @ 2026-03-17 9:44 UTC (permalink / raw) To: mptcp; +Cc: Geliang Tang, Paolo Abeni, Mat Martineau 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 ^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [RESEND mptcp-next v2 2/2] tcp: add recv_should_stop helper 2026-03-17 9:44 ` [RESEND mptcp-next v2 2/2] tcp: add recv_should_stop helper Geliang Tang @ 2026-03-26 18:29 ` Matthieu Baerts 0 siblings, 0 replies; 6+ messages in thread From: Matthieu Baerts @ 2026-03-26 18:29 UTC (permalink / raw) To: Geliang Tang, mptcp; +Cc: Geliang Tang, Paolo Abeni, Mat Martineau 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. ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [RESEND mptcp-next v2 0/2] add tcp_recv_should_stop helper 2026-03-17 9:44 [RESEND mptcp-next v2 0/2] add tcp_recv_should_stop helper Geliang Tang 2026-03-17 9:44 ` [RESEND mptcp-next v2 1/2] mptcp: check desc->count in read_sock Geliang Tang 2026-03-17 9:44 ` [RESEND mptcp-next v2 2/2] tcp: add recv_should_stop helper Geliang Tang @ 2026-03-17 11:13 ` MPTCP CI 2 siblings, 0 replies; 6+ messages in thread From: MPTCP CI @ 2026-03-17 11:13 UTC (permalink / raw) To: Geliang Tang; +Cc: mptcp 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) ^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2026-03-26 18:29 UTC | newest] Thread overview: 6+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2026-03-17 9:44 [RESEND mptcp-next v2 0/2] add tcp_recv_should_stop helper Geliang Tang 2026-03-17 9:44 ` [RESEND mptcp-next v2 1/2] mptcp: check desc->count in read_sock Geliang Tang 2026-03-26 18:21 ` Matthieu Baerts 2026-03-17 9:44 ` [RESEND mptcp-next v2 2/2] tcp: add recv_should_stop helper Geliang Tang 2026-03-26 18:29 ` Matthieu Baerts 2026-03-17 11:13 ` [RESEND mptcp-next v2 0/2] add tcp_recv_should_stop helper MPTCP CI
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox