Re: [PATCH mptcp-next 4/4] mptcp: use READ_ONCE() over sysctls

[Geliang Tang <geliang@kernel.org>]

Hi Matt,

Thanks for this patch.

On Mon, 2026-06-01 at 18:27 +1000, Matthieu Baerts (NGI0) wrote:
> To avoid KCSAN issues.
> 
> This patch is in theory for -net, and will need to be split in
> multiple
> patches, with different Fixes tags. But I prefer to wait for Eric's
> patches, as I noticed he already started to modify mptcp_is_enabled:
> 
>  
> https://lore.kernel.org/CANn89iLdwhhwLyO6zRjWMEY3t9g60ZE8ZhOVx33ucg_uRETbmQ@mail.gmail.com
> 
> Still, keeping this patch in this series, not to forget about it.
> 
> Reported-by: Eric Dumazet <edumazet@google.com>
> Closes:
> https://lore.kernel.org/CANn89iL=os-60kDKqMDdyiXuPF5CG=eejS0vmthwpDGXz_Bp8A@mail.gmail.com
> Signed-off-by: Matthieu Baerts (NGI0) <matttbe@kernel.org>
> ---
>  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..1dee42601700 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)
> @@ -551,7 +551,7 @@ void mptcp_active_detect_blackhole(struct sock
> *ssk, bool expired)
>  
>  	net = sock_net(ssk);
>  	timeouts = inet_csk(ssk)->icsk_retransmits;
> -	to_max = mptcp_get_pernet(net)-
> >syn_retrans_before_tcp_fallback;
> +	to_max = READ_ONCE(mptcp_get_pernet(net)-
> >syn_retrans_before_tcp_fallback);
>  
>  	if (timeouts == to_max || (timeouts < to_max && expired)) {
>  		subflow->mpc_drop = 1;
> 

I hesitated for a moment about why READ_ONCE() was not added in
mptcp_get_path_manager() and mptcp_get_scheduler(), and the result is
that they are defined as arrays rather than pointers, so READ_ONCE() is
not applicable.

So this patch looks good to me.

    Reviewed-by: Geliang Tang <geliang@kernel.org>

Thanks,
-Geliang