* [RFC PATCH mptcp-next] mptcp: annotate data-races around sysctl reads @ 2026-08-14 3:27 Tao Cui 2026-08-14 3:51 ` gang.yan 2026-08-14 4:31 ` MPTCP CI 0 siblings, 2 replies; 4+ messages in thread From: Tao Cui @ 2026-08-14 3:27 UTC (permalink / raw) To: mptcp; +Cc: matttbe, martineau, geliang, Tao Cui From: Tao Cui <cuitao@kylinos.cn> #626 plans a READ_ONCE-over-sysctls series for the string sysctls; this is the numeric side. Asking whether to send it on its own or fold it in. The per-netns MPTCP sysctl values (net.mptcp.enabled, add_addr_timeout, checksum_enabled, allow_join_initial_addr_port, stale_loss_cnt, close_timeout, pm_type) are written from the sysctl handlers and read without locking through the ctrl.c accessors. Add READ_ONCE() on the readers and WRITE_ONCE() on the pm_type store in proc_path_manager(), matching what is already done for other mptcp fields (fully_established, local_id, remote_id, sysctl_tcp_wmem[0]). The string sysctls (path_manager, scheduler) are not covered: they need atomic replacement, see the tracker below. KCSAN reproduces the race on net.mptcp.enabled, and the READ_ONCE makes it go away: BUG: KCSAN: data-race in mptcp_is_enabled / proc_dou8vec_minmax write to 0xffff8f93c18553e9 of 1 bytes by task 214 on cpu 1: proc_dou8vec_minmax+0x1b1/0x200 proc_sys_call_handler+0x268/0x350 vfs_write+0x423/0x710 read to 0xffff8f93c18553e9 of 1 bytes by task 72 on cpu 0: mptcp_is_enabled+0x50/0x60 mptcp_init_sock+0x2a/0x1c0 inet_create+0x3f9/0x5a0 value changed: 0x00 -> 0x01 No functional change. Link: https://github.com/multipath-tcp/mptcp_net-next/issues/626 Signed-off-by: Tao Cui <cuitao@kylinos.cn> --- net/mptcp/ctrl.c | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/net/mptcp/ctrl.c b/net/mptcp/ctrl.c index 63c5747f0f63..b0ef6aea4eba 100644 --- a/net/mptcp/ctrl.c +++ b/net/mptcp/ctrl.c @@ -50,39 +50,39 @@ static struct mptcp_pernet *mptcp_get_pernet(const struct net *net) int mptcp_is_enabled(const struct net *net) { - return mptcp_get_pernet(net)->mptcp_enabled; + return READ_ONCE(mptcp_get_pernet(net)->mptcp_enabled); } unsigned int mptcp_get_add_addr_timeout(const struct net *net) { - return mptcp_get_pernet(net)->add_addr_timeout; + return READ_ONCE(mptcp_get_pernet(net)->add_addr_timeout); } int mptcp_is_checksum_enabled(const struct net *net) { - return mptcp_get_pernet(net)->checksum_enabled; + return READ_ONCE(mptcp_get_pernet(net)->checksum_enabled); } int mptcp_allow_join_id0(const struct net *net) { - return mptcp_get_pernet(net)->allow_join_initial_addr_port; + return READ_ONCE(mptcp_get_pernet(net)->allow_join_initial_addr_port); } unsigned int mptcp_stale_loss_cnt(const struct net *net) { - return mptcp_get_pernet(net)->stale_loss_cnt; + return READ_ONCE(mptcp_get_pernet(net)->stale_loss_cnt); } unsigned int mptcp_close_timeout(const struct sock *sk) { if (sock_flag(sk, SOCK_DEAD)) return TCP_TIMEWAIT_LEN; - return mptcp_get_pernet(sock_net(sk))->close_timeout; + return READ_ONCE(mptcp_get_pernet(sock_net(sk))->close_timeout); } int mptcp_get_pm_type(const struct net *net) { - return mptcp_get_pernet(net)->pm_type; + return READ_ONCE(mptcp_get_pernet(net)->pm_type); } const char *mptcp_get_path_manager(const struct net *net) @@ -230,7 +230,7 @@ static int proc_path_manager(const struct ctl_table *ctl, int write, pm_type = MPTCP_PM_TYPE_KERNEL; else if (strncmp(pm_name, "userspace", MPTCP_PM_NAME_MAX) == 0) pm_type = MPTCP_PM_TYPE_USERSPACE; - pernet->pm_type = pm_type; + WRITE_ONCE(pernet->pm_type, pm_type); } } -- 2.43.0 ^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [RFC PATCH mptcp-next] mptcp: annotate data-races around sysctl reads 2026-08-14 3:27 [RFC PATCH mptcp-next] mptcp: annotate data-races around sysctl reads Tao Cui @ 2026-08-14 3:51 ` gang.yan 2026-08-14 5:26 ` Tao Cui 2026-08-14 4:31 ` MPTCP CI 1 sibling, 1 reply; 4+ messages in thread From: gang.yan @ 2026-08-14 3:51 UTC (permalink / raw) To: Tao Cui, mptcp; +Cc: matttbe, martineau, geliang, Tao Cui August 14, 2026 at 11:27 AM, "Tao Cui" <cui.tao@linux.dev mailto:cui.tao@linux.dev?to=%22Tao%20Cui%22%20%3Ccui.tao%40linux.dev%3E > wrote: Hi, Tao Thanks for your patch, but it has been submitted by Matt before in [1]. I think Matt wanted change the PM and sched sysctl from string to atomically, that may need another patches (READ_ONCE is not enough), right? If no, you can wait for the other maintainers' reply. If yes, I still think this patch should keep author as matt, the rest of others can be yours. Note: Some patches about sched is only in our export branch, not mainlined, so it's better to do your work based on it. [1] https://patchwork.kernel.org/project/mptcp/patch/20260601-mptcp-add-addr6-port-ts-fixes-v2-v1-4-d7c842e80446@kernel.org/ Thanks Gang > > From: Tao Cui <cuitao@kylinos.cn> > > #626 plans a READ_ONCE-over-sysctls series for the string sysctls; this > is the numeric side. Asking whether to send it on its own or fold it in. > > The per-netns MPTCP sysctl values (net.mptcp.enabled, add_addr_timeout, > checksum_enabled, allow_join_initial_addr_port, stale_loss_cnt, > close_timeout, pm_type) are written from the sysctl handlers and read > without locking through the ctrl.c accessors. > > Add READ_ONCE() on the readers and WRITE_ONCE() on the pm_type store in > proc_path_manager(), matching what is already done for other mptcp > fields (fully_established, local_id, remote_id, sysctl_tcp_wmem[0]). > > The string sysctls (path_manager, scheduler) are not covered: they need > atomic replacement, see the tracker below. > > KCSAN reproduces the race on net.mptcp.enabled, and the READ_ONCE makes > it go away: > > BUG: KCSAN: data-race in mptcp_is_enabled / proc_dou8vec_minmax > > write to 0xffff8f93c18553e9 of 1 bytes by task 214 on cpu 1: > proc_dou8vec_minmax+0x1b1/0x200 > proc_sys_call_handler+0x268/0x350 > vfs_write+0x423/0x710 > > read to 0xffff8f93c18553e9 of 1 bytes by task 72 on cpu 0: > mptcp_is_enabled+0x50/0x60 > mptcp_init_sock+0x2a/0x1c0 > inet_create+0x3f9/0x5a0 > > value changed: 0x00 -> 0x01 > > No functional change. > > Link: https://github.com/multipath-tcp/mptcp_net-next/issues/626 > Signed-off-by: Tao Cui <cuitao@kylinos.cn> > --- > net/mptcp/ctrl.c | 16 ++++++++-------- > 1 file changed, 8 insertions(+), 8 deletions(-) > > diff --git a/net/mptcp/ctrl.c b/net/mptcp/ctrl.c > index 63c5747f0f63..b0ef6aea4eba 100644 > --- a/net/mptcp/ctrl.c > +++ b/net/mptcp/ctrl.c > @@ -50,39 +50,39 @@ static struct mptcp_pernet *mptcp_get_pernet(const struct net *net) > > int mptcp_is_enabled(const struct net *net) > { > - return mptcp_get_pernet(net)->mptcp_enabled; > + return READ_ONCE(mptcp_get_pernet(net)->mptcp_enabled); > } > > unsigned int mptcp_get_add_addr_timeout(const struct net *net) > { > - return mptcp_get_pernet(net)->add_addr_timeout; > + return READ_ONCE(mptcp_get_pernet(net)->add_addr_timeout); > } > > int mptcp_is_checksum_enabled(const struct net *net) > { > - return mptcp_get_pernet(net)->checksum_enabled; > + return READ_ONCE(mptcp_get_pernet(net)->checksum_enabled); > } > > int mptcp_allow_join_id0(const struct net *net) > { > - return mptcp_get_pernet(net)->allow_join_initial_addr_port; > + return READ_ONCE(mptcp_get_pernet(net)->allow_join_initial_addr_port); > } > > unsigned int mptcp_stale_loss_cnt(const struct net *net) > { > - return mptcp_get_pernet(net)->stale_loss_cnt; > + return READ_ONCE(mptcp_get_pernet(net)->stale_loss_cnt); > } > > unsigned int mptcp_close_timeout(const struct sock *sk) > { > if (sock_flag(sk, SOCK_DEAD)) > return TCP_TIMEWAIT_LEN; > - return mptcp_get_pernet(sock_net(sk))->close_timeout; > + return READ_ONCE(mptcp_get_pernet(sock_net(sk))->close_timeout); > } > > int mptcp_get_pm_type(const struct net *net) > { > - return mptcp_get_pernet(net)->pm_type; > + return READ_ONCE(mptcp_get_pernet(net)->pm_type); > } > > const char *mptcp_get_path_manager(const struct net *net) > @@ -230,7 +230,7 @@ static int proc_path_manager(const struct ctl_table *ctl, int write, > pm_type = MPTCP_PM_TYPE_KERNEL; > else if (strncmp(pm_name, "userspace", MPTCP_PM_NAME_MAX) == 0) > pm_type = MPTCP_PM_TYPE_USERSPACE; > - pernet->pm_type = pm_type; > + WRITE_ONCE(pernet->pm_type, pm_type); > } > } > > -- > 2.43.0 > ^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [RFC PATCH mptcp-next] mptcp: annotate data-races around sysctl reads 2026-08-14 3:51 ` gang.yan @ 2026-08-14 5:26 ` Tao Cui 0 siblings, 0 replies; 4+ messages in thread From: Tao Cui @ 2026-08-14 5:26 UTC (permalink / raw) To: gang.yan, mptcp; +Cc: cui.tao, matttbe, martineau, geliang, Tao Cui Hi Gang, 在 2026/8/14 11:51, gang.yan@linux.dev 写道: > August 14, 2026 at 11:27 AM, "Tao Cui" <cui.tao@linux.dev mailto:cui.tao@linux.dev?to=%22Tao%20Cui%22%20%3Ccui.tao%40linux.dev%3E > wrote: > > Hi, Tao > > Thanks for your patch, but it has been submitted by Matt before in [1]. > > I think Matt wanted change the PM and sched sysctl from string to atomically, > that may need another patches (READ_ONCE is not enough), right? If no, you can > wait for the other maintainers' reply. > > If yes, I still think this patch should keep author as matt, the rest of > others can be yours. > > Note: Some patches about sched is only in our export branch, not mainlined, > so it's better to do your work based on it. > Thanks for the review. I did see #626 before sending and referenced it, I just read its scope wrong: I assumed the patch it links was only about the string sysctls and missed that the numeric readers were already covered there. And thanks for the export branch tip, I'd only been looking at mainline and didn't know some of the sched patches are only in export. The only bit mine adds is the WRITE_ONCE() on the pm_type store in proc_path_manager(), to pair with the mptcp_get_pm_type() read. Matt, if you want that too just grab it, otherwise I can send it as a small follow-up. Thanks, Tao > [1] https://patchwork.kernel.org/project/mptcp/patch/20260601-mptcp-add-addr6-port-ts-fixes-v2-v1-4-d7c842e80446@kernel.org/ > > Thanks > Gang > >> >> From: Tao Cui <cuitao@kylinos.cn> >> >> #626 plans a READ_ONCE-over-sysctls series for the string sysctls; this >> is the numeric side. Asking whether to send it on its own or fold it in. >> >> The per-netns MPTCP sysctl values (net.mptcp.enabled, add_addr_timeout, >> checksum_enabled, allow_join_initial_addr_port, stale_loss_cnt, >> close_timeout, pm_type) are written from the sysctl handlers and read >> without locking through the ctrl.c accessors. >> >> Add READ_ONCE() on the readers and WRITE_ONCE() on the pm_type store in >> proc_path_manager(), matching what is already done for other mptcp >> fields (fully_established, local_id, remote_id, sysctl_tcp_wmem[0]). >> >> The string sysctls (path_manager, scheduler) are not covered: they need >> atomic replacement, see the tracker below. >> >> KCSAN reproduces the race on net.mptcp.enabled, and the READ_ONCE makes >> it go away: >> >> BUG: KCSAN: data-race in mptcp_is_enabled / proc_dou8vec_minmax >> >> write to 0xffff8f93c18553e9 of 1 bytes by task 214 on cpu 1: >> proc_dou8vec_minmax+0x1b1/0x200 >> proc_sys_call_handler+0x268/0x350 >> vfs_write+0x423/0x710 >> >> read to 0xffff8f93c18553e9 of 1 bytes by task 72 on cpu 0: >> mptcp_is_enabled+0x50/0x60 >> mptcp_init_sock+0x2a/0x1c0 >> inet_create+0x3f9/0x5a0 >> >> value changed: 0x00 -> 0x01 >> >> No functional change. >> >> Link: https://github.com/multipath-tcp/mptcp_net-next/issues/626 >> Signed-off-by: Tao Cui <cuitao@kylinos.cn> >> --- >> net/mptcp/ctrl.c | 16 ++++++++-------- >> 1 file changed, 8 insertions(+), 8 deletions(-) >> >> diff --git a/net/mptcp/ctrl.c b/net/mptcp/ctrl.c >> index 63c5747f0f63..b0ef6aea4eba 100644 >> --- a/net/mptcp/ctrl.c >> +++ b/net/mptcp/ctrl.c >> @@ -50,39 +50,39 @@ static struct mptcp_pernet *mptcp_get_pernet(const struct net *net) >> >> int mptcp_is_enabled(const struct net *net) >> { >> - return mptcp_get_pernet(net)->mptcp_enabled; >> + return READ_ONCE(mptcp_get_pernet(net)->mptcp_enabled); >> } >> >> unsigned int mptcp_get_add_addr_timeout(const struct net *net) >> { >> - return mptcp_get_pernet(net)->add_addr_timeout; >> + return READ_ONCE(mptcp_get_pernet(net)->add_addr_timeout); >> } >> >> int mptcp_is_checksum_enabled(const struct net *net) >> { >> - return mptcp_get_pernet(net)->checksum_enabled; >> + return READ_ONCE(mptcp_get_pernet(net)->checksum_enabled); >> } >> >> int mptcp_allow_join_id0(const struct net *net) >> { >> - return mptcp_get_pernet(net)->allow_join_initial_addr_port; >> + return READ_ONCE(mptcp_get_pernet(net)->allow_join_initial_addr_port); >> } >> >> unsigned int mptcp_stale_loss_cnt(const struct net *net) >> { >> - return mptcp_get_pernet(net)->stale_loss_cnt; >> + return READ_ONCE(mptcp_get_pernet(net)->stale_loss_cnt); >> } >> >> unsigned int mptcp_close_timeout(const struct sock *sk) >> { >> if (sock_flag(sk, SOCK_DEAD)) >> return TCP_TIMEWAIT_LEN; >> - return mptcp_get_pernet(sock_net(sk))->close_timeout; >> + return READ_ONCE(mptcp_get_pernet(sock_net(sk))->close_timeout); >> } >> >> int mptcp_get_pm_type(const struct net *net) >> { >> - return mptcp_get_pernet(net)->pm_type; >> + return READ_ONCE(mptcp_get_pernet(net)->pm_type); >> } >> >> const char *mptcp_get_path_manager(const struct net *net) >> @@ -230,7 +230,7 @@ static int proc_path_manager(const struct ctl_table *ctl, int write, >> pm_type = MPTCP_PM_TYPE_KERNEL; >> else if (strncmp(pm_name, "userspace", MPTCP_PM_NAME_MAX) == 0) >> pm_type = MPTCP_PM_TYPE_USERSPACE; >> - pernet->pm_type = pm_type; >> + WRITE_ONCE(pernet->pm_type, pm_type); >> } >> } >> >> -- >> 2.43.0 >> ^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [RFC PATCH mptcp-next] mptcp: annotate data-races around sysctl reads 2026-08-14 3:27 [RFC PATCH mptcp-next] mptcp: annotate data-races around sysctl reads Tao Cui 2026-08-14 3:51 ` gang.yan @ 2026-08-14 4:31 ` MPTCP CI 1 sibling, 0 replies; 4+ messages in thread From: MPTCP CI @ 2026-08-14 4:31 UTC (permalink / raw) To: Tao Cui; +Cc: mptcp Hi Tao, 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/31767814900 Initiator: Patchew Applier Commits: https://github.com/multipath-tcp/mptcp_net-next/commits/fa3baeab64f6 Patchwork: https://patchwork.kernel.org/project/mptcp/list/?series=1145806 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] 4+ messages in thread
end of thread, other threads:[~2026-08-14 5:26 UTC | newest] Thread overview: 4+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2026-08-14 3:27 [RFC PATCH mptcp-next] mptcp: annotate data-races around sysctl reads Tao Cui 2026-08-14 3:51 ` gang.yan 2026-08-14 5:26 ` Tao Cui 2026-08-14 4:31 ` MPTCP CI
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.