* [PATCH net-next 0/2] annotate data-races around sysctl_tcp_wmem[0]
@ 2024-03-08 11:25 Jason Xing
2024-03-08 11:25 ` [PATCH net-next 1/2] mptcp: annotate a data-race " Jason Xing
` (2 more replies)
0 siblings, 3 replies; 9+ messages in thread
From: Jason Xing @ 2024-03-08 11:25 UTC (permalink / raw)
To: edumazet, dsahern, matttbe, martineau, geliang, kuba, pabeni, davem
Cc: mptcp, netdev, kerneljasonxing, Jason Xing
From: Jason Xing <kernelxing@tencent.com>
Adding simple READ_ONCE() can avoid reading the sysctl knob meanwhile
someone is trying to change it.
Jason Xing (2):
mptcp: annotate a data-race around sysctl_tcp_wmem[0]
tcp: annotate a data-race around sysctl_tcp_wmem[0]
net/ipv4/tcp.c | 2 +-
net/mptcp/protocol.h | 2 +-
2 files changed, 2 insertions(+), 2 deletions(-)
--
2.37.3
^ permalink raw reply [flat|nested] 9+ messages in thread
* [PATCH net-next 1/2] mptcp: annotate a data-race around sysctl_tcp_wmem[0]
2024-03-08 11:25 [PATCH net-next 0/2] annotate data-races around sysctl_tcp_wmem[0] Jason Xing
@ 2024-03-08 11:25 ` Jason Xing
2024-03-08 12:54 ` Eric Dumazet
2024-03-08 20:26 ` Mat Martineau
2024-03-08 11:25 ` [PATCH net-next 2/2] tcp: " Jason Xing
2024-03-11 19:39 ` [PATCH net-next 0/2] annotate data-races around sysctl_tcp_wmem[0] patchwork-bot+netdevbpf
2 siblings, 2 replies; 9+ messages in thread
From: Jason Xing @ 2024-03-08 11:25 UTC (permalink / raw)
To: edumazet, dsahern, matttbe, martineau, geliang, kuba, pabeni, davem
Cc: mptcp, netdev, kerneljasonxing, Jason Xing
From: Jason Xing <kernelxing@tencent.com>
It's possible that writer and the reader can manipulate the same
sysctl knob concurrently. Using READ_ONCE() to prevent reading
an old value.
Signed-off-by: Jason Xing <kernelxing@tencent.com>
---
net/mptcp/protocol.h | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/net/mptcp/protocol.h b/net/mptcp/protocol.h
index f16edef6026a..a10ebf3ee10a 100644
--- a/net/mptcp/protocol.h
+++ b/net/mptcp/protocol.h
@@ -850,7 +850,7 @@ static inline void __mptcp_sync_sndbuf(struct sock *sk)
if (sk->sk_userlocks & SOCK_SNDBUF_LOCK)
return;
- new_sndbuf = sock_net(sk)->ipv4.sysctl_tcp_wmem[0];
+ new_sndbuf = READ_ONCE(sock_net(sk)->ipv4.sysctl_tcp_wmem[0]);
mptcp_for_each_subflow(mptcp_sk(sk), subflow) {
ssk_sndbuf = READ_ONCE(mptcp_subflow_tcp_sock(subflow)->sk_sndbuf);
--
2.37.3
^ permalink raw reply related [flat|nested] 9+ messages in thread
* [PATCH net-next 2/2] tcp: annotate a data-race around sysctl_tcp_wmem[0]
2024-03-08 11:25 [PATCH net-next 0/2] annotate data-races around sysctl_tcp_wmem[0] Jason Xing
2024-03-08 11:25 ` [PATCH net-next 1/2] mptcp: annotate a data-race " Jason Xing
@ 2024-03-08 11:25 ` Jason Xing
2024-03-08 12:21 ` tcp: annotate a data-race around sysctl_tcp_wmem[0]: Tests Results MPTCP CI
` (2 more replies)
2024-03-11 19:39 ` [PATCH net-next 0/2] annotate data-races around sysctl_tcp_wmem[0] patchwork-bot+netdevbpf
2 siblings, 3 replies; 9+ messages in thread
From: Jason Xing @ 2024-03-08 11:25 UTC (permalink / raw)
To: edumazet, dsahern, matttbe, martineau, geliang, kuba, pabeni, davem
Cc: mptcp, netdev, kerneljasonxing, Jason Xing
From: Jason Xing <kernelxing@tencent.com>
When reading wmem[0], it could be changed concurrently without
READ_ONCE() protection. So add one annotation here.
Signed-off-by: Jason Xing <kernelxing@tencent.com>
---
net/ipv4/tcp.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/net/ipv4/tcp.c b/net/ipv4/tcp.c
index c5b83875411a..e3904c006e63 100644
--- a/net/ipv4/tcp.c
+++ b/net/ipv4/tcp.c
@@ -975,7 +975,7 @@ int tcp_wmem_schedule(struct sock *sk, int copy)
* Use whatever is left in sk->sk_forward_alloc and tcp_wmem[0]
* to guarantee some progress.
*/
- left = sock_net(sk)->ipv4.sysctl_tcp_wmem[0] - sk->sk_wmem_queued;
+ left = READ_ONCE(sock_net(sk)->ipv4.sysctl_tcp_wmem[0]) - sk->sk_wmem_queued;
if (left > 0)
sk_forced_mem_schedule(sk, min(left, copy));
return min(copy, sk->sk_forward_alloc);
--
2.37.3
^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: tcp: annotate a data-race around sysctl_tcp_wmem[0]: Tests Results
2024-03-08 11:25 ` [PATCH net-next 2/2] tcp: " Jason Xing
@ 2024-03-08 12:21 ` MPTCP CI
2024-03-08 12:51 ` [PATCH net-next 2/2] tcp: annotate a data-race around sysctl_tcp_wmem[0] Eric Dumazet
2024-03-08 13:45 ` tcp: annotate a data-race around sysctl_tcp_wmem[0]: Tests Results MPTCP CI
2 siblings, 0 replies; 9+ messages in thread
From: MPTCP CI @ 2024-03-08 12:21 UTC (permalink / raw)
To: Jason Xing; +Cc: mptcp
Hi Jason,
Thank you for your modifications, that's great!
Our CI (GitHub Action) did some validations and here is its report:
- KVM Validation: normal: Success! ✅
- KVM Validation: btf (only bpftest_all): Success! ✅
- Task: https://github.com/multipath-tcp/mptcp_net-next/actions/runs/8202785775
Initiator: Patchew Applier
Commits: https://github.com/multipath-tcp/mptcp_net-next/commits/e55bd7ed14f5
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] 9+ messages in thread
* Re: [PATCH net-next 2/2] tcp: annotate a data-race around sysctl_tcp_wmem[0]
2024-03-08 11:25 ` [PATCH net-next 2/2] tcp: " Jason Xing
2024-03-08 12:21 ` tcp: annotate a data-race around sysctl_tcp_wmem[0]: Tests Results MPTCP CI
@ 2024-03-08 12:51 ` Eric Dumazet
2024-03-08 13:45 ` tcp: annotate a data-race around sysctl_tcp_wmem[0]: Tests Results MPTCP CI
2 siblings, 0 replies; 9+ messages in thread
From: Eric Dumazet @ 2024-03-08 12:51 UTC (permalink / raw)
To: Jason Xing
Cc: dsahern, matttbe, martineau, geliang, kuba, pabeni, davem, mptcp,
netdev, Jason Xing
On Fri, Mar 8, 2024 at 12:25 PM Jason Xing <kerneljasonxing@gmail.com> wrote:
>
> From: Jason Xing <kernelxing@tencent.com>
>
> When reading wmem[0], it could be changed concurrently without
> READ_ONCE() protection. So add one annotation here.
>
> Signed-off-by: Jason Xing <kernelxing@tencent.com>
> ---
> net/ipv4/tcp.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/net/ipv4/tcp.c b/net/ipv4/tcp.c
> index c5b83875411a..e3904c006e63 100644
> --- a/net/ipv4/tcp.c
> +++ b/net/ipv4/tcp.c
> @@ -975,7 +975,7 @@ int tcp_wmem_schedule(struct sock *sk, int copy)
> * Use whatever is left in sk->sk_forward_alloc and tcp_wmem[0]
> * to guarantee some progress.
> */
> - left = sock_net(sk)->ipv4.sysctl_tcp_wmem[0] - sk->sk_wmem_queued;
> + left = READ_ONCE(sock_net(sk)->ipv4.sysctl_tcp_wmem[0]) - sk->sk_wmem_queued;
SGTM, you could have split the long line...
Reviewed-by: Eric Dumazet <edumazet@google.com>
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH net-next 1/2] mptcp: annotate a data-race around sysctl_tcp_wmem[0]
2024-03-08 11:25 ` [PATCH net-next 1/2] mptcp: annotate a data-race " Jason Xing
@ 2024-03-08 12:54 ` Eric Dumazet
2024-03-08 20:26 ` Mat Martineau
1 sibling, 0 replies; 9+ messages in thread
From: Eric Dumazet @ 2024-03-08 12:54 UTC (permalink / raw)
To: Jason Xing
Cc: dsahern, matttbe, martineau, geliang, kuba, pabeni, davem, mptcp,
netdev, Jason Xing
On Fri, Mar 8, 2024 at 12:25 PM Jason Xing <kerneljasonxing@gmail.com> wrote:
>
> From: Jason Xing <kernelxing@tencent.com>
>
> It's possible that writer and the reader can manipulate the same
> sysctl knob concurrently. Using READ_ONCE() to prevent reading
> an old value.
>
> Signed-off-by: Jason Xing <kernelxing@tencent.com>
Reviewed-by: Eric Dumazet <edumazet@google.com>
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: tcp: annotate a data-race around sysctl_tcp_wmem[0]: Tests Results
2024-03-08 11:25 ` [PATCH net-next 2/2] tcp: " Jason Xing
2024-03-08 12:21 ` tcp: annotate a data-race around sysctl_tcp_wmem[0]: Tests Results MPTCP CI
2024-03-08 12:51 ` [PATCH net-next 2/2] tcp: annotate a data-race around sysctl_tcp_wmem[0] Eric Dumazet
@ 2024-03-08 13:45 ` MPTCP CI
2 siblings, 0 replies; 9+ messages in thread
From: MPTCP CI @ 2024-03-08 13:45 UTC (permalink / raw)
To: Jason Xing; +Cc: mptcp
Hi Jason,
Thank you for your modifications, that's great!
Our CI (Cirrus) did some validations with a debug kernel and here is its report:
- KVM Validation: debug (except selftest_mptcp_join):
- Success! ✅:
- Task: https://cirrus-ci.com/task/5015312026828800
- Summary: https://api.cirrus-ci.com/v1/artifact/task/5015312026828800/summary/summary.txt
- KVM Validation: debug (only selftest_mptcp_join):
- Unstable: 1 failed test(s): selftest_mptcp_join 🔴:
- Task: https://cirrus-ci.com/task/6141211933671424
- Summary: https://api.cirrus-ci.com/v1/artifact/task/6141211933671424/summary/summary.txt
Initiator: Patchew Applier
Commits: https://github.com/multipath-tcp/mptcp_net-next/commits/e55bd7ed14f5
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-debug
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] 9+ messages in thread
* Re: [PATCH net-next 1/2] mptcp: annotate a data-race around sysctl_tcp_wmem[0]
2024-03-08 11:25 ` [PATCH net-next 1/2] mptcp: annotate a data-race " Jason Xing
2024-03-08 12:54 ` Eric Dumazet
@ 2024-03-08 20:26 ` Mat Martineau
1 sibling, 0 replies; 9+ messages in thread
From: Mat Martineau @ 2024-03-08 20:26 UTC (permalink / raw)
To: Jason Xing
Cc: edumazet, dsahern, Matthieu Baerts, geliang, kuba, Paolo Abeni,
davem, mptcp, netdev, Jason Xing
On Fri, 8 Mar 2024, Jason Xing wrote:
> From: Jason Xing <kernelxing@tencent.com>
>
> It's possible that writer and the reader can manipulate the same
> sysctl knob concurrently. Using READ_ONCE() to prevent reading
> an old value.
>
> Signed-off-by: Jason Xing <kernelxing@tencent.com>
> ---
> net/mptcp/protocol.h | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/net/mptcp/protocol.h b/net/mptcp/protocol.h
> index f16edef6026a..a10ebf3ee10a 100644
> --- a/net/mptcp/protocol.h
> +++ b/net/mptcp/protocol.h
> @@ -850,7 +850,7 @@ static inline void __mptcp_sync_sndbuf(struct sock *sk)
> if (sk->sk_userlocks & SOCK_SNDBUF_LOCK)
> return;
>
> - new_sndbuf = sock_net(sk)->ipv4.sysctl_tcp_wmem[0];
> + new_sndbuf = READ_ONCE(sock_net(sk)->ipv4.sysctl_tcp_wmem[0]);
> mptcp_for_each_subflow(mptcp_sk(sk), subflow) {
> ssk_sndbuf = READ_ONCE(mptcp_subflow_tcp_sock(subflow)->sk_sndbuf);
>
Looks good to me, thanks Jason.
Reviewed-by: Mat Martineau <martineau@kernel.org>
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH net-next 0/2] annotate data-races around sysctl_tcp_wmem[0]
2024-03-08 11:25 [PATCH net-next 0/2] annotate data-races around sysctl_tcp_wmem[0] Jason Xing
2024-03-08 11:25 ` [PATCH net-next 1/2] mptcp: annotate a data-race " Jason Xing
2024-03-08 11:25 ` [PATCH net-next 2/2] tcp: " Jason Xing
@ 2024-03-11 19:39 ` patchwork-bot+netdevbpf
2 siblings, 0 replies; 9+ messages in thread
From: patchwork-bot+netdevbpf @ 2024-03-11 19:39 UTC (permalink / raw)
To: Jason Xing
Cc: edumazet, dsahern, matttbe, martineau, geliang, kuba, pabeni,
davem, mptcp, netdev, kernelxing
Hello:
This series was applied to netdev/net-next.git (main)
by David S. Miller <davem@davemloft.net>:
On Fri, 8 Mar 2024 19:25:02 +0800 you wrote:
> From: Jason Xing <kernelxing@tencent.com>
>
> Adding simple READ_ONCE() can avoid reading the sysctl knob meanwhile
> someone is trying to change it.
>
> Jason Xing (2):
> mptcp: annotate a data-race around sysctl_tcp_wmem[0]
> tcp: annotate a data-race around sysctl_tcp_wmem[0]
>
> [...]
Here is the summary with links:
- [net-next,1/2] mptcp: annotate a data-race around sysctl_tcp_wmem[0]
https://git.kernel.org/netdev/net-next/c/9eb430d40e44
- [net-next,2/2] tcp: annotate a data-race around sysctl_tcp_wmem[0]
https://git.kernel.org/netdev/net-next/c/683a67da9561
You are awesome, thank you!
--
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html
^ permalink raw reply [flat|nested] 9+ messages in thread
end of thread, other threads:[~2024-03-11 19:39 UTC | newest]
Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2024-03-08 11:25 [PATCH net-next 0/2] annotate data-races around sysctl_tcp_wmem[0] Jason Xing
2024-03-08 11:25 ` [PATCH net-next 1/2] mptcp: annotate a data-race " Jason Xing
2024-03-08 12:54 ` Eric Dumazet
2024-03-08 20:26 ` Mat Martineau
2024-03-08 11:25 ` [PATCH net-next 2/2] tcp: " Jason Xing
2024-03-08 12:21 ` tcp: annotate a data-race around sysctl_tcp_wmem[0]: Tests Results MPTCP CI
2024-03-08 12:51 ` [PATCH net-next 2/2] tcp: annotate a data-race around sysctl_tcp_wmem[0] Eric Dumazet
2024-03-08 13:45 ` tcp: annotate a data-race around sysctl_tcp_wmem[0]: Tests Results MPTCP CI
2024-03-11 19:39 ` [PATCH net-next 0/2] annotate data-races around sysctl_tcp_wmem[0] patchwork-bot+netdevbpf
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.