Re: [RFC PATCH net-next 1/4] txhash: Make rethinking txhash behavior configurable via sysctl

From: Eric Dumazet <eric.dumazet@gmail.com>
To: Akhmat Karakotov <hmukos@yandex-team.ru>, netdev@vger.kernel.org
Cc: tom@herbertland.com, mitradir@yandex-team.ru, zeil@yandex-team.ru
Subject: Re: [RFC PATCH net-next 1/4] txhash: Make rethinking txhash behavior configurable via sysctl
Date: Mon, 25 Oct 2021 13:54:54 -0700	[thread overview]
Message-ID: <af6dcf30-75d5-7b4a-66cd-837a703101b1@gmail.com> (raw)
In-Reply-To: <20211025203521.13507-2-hmukos@yandex-team.ru>

On 10/25/21 1:35 PM, Akhmat Karakotov wrote:
> Add a per ns sysctl that controls the txhash rethink behavior,
> sk_rethink_txhash. When enabled, the same behavior is retained, when
> disabled, rethink is not performed. Sysctl is disabled by default.
> 
> Signed-off-by: Akhmat Karakotov <hmukos@yandex-team.ru>
> ---
>  include/net/netns/core.h    |  2 ++
>  include/net/sock.h          | 34 +++++++++++++++++++++-------------
>  include/uapi/linux/socket.h |  3 +++
>  net/core/net_namespace.c    |  3 +++
>  net/core/sysctl_net_core.c  |  7 +++++++
>  5 files changed, 36 insertions(+), 13 deletions(-)
> 
> diff --git a/include/net/netns/core.h b/include/net/netns/core.h
> index 36c2d998a43c..177980b46ed7 100644
> --- a/include/net/netns/core.h
> +++ b/include/net/netns/core.h
> @@ -11,6 +11,8 @@ struct netns_core {
>  
>  	int	sysctl_somaxconn;
>  
> +	unsigned int sysctl_txrehash;

We have u8 sysctls, to keep this structure small.

> +
>  #ifdef CONFIG_PROC_FS
>  	int __percpu *sock_inuse;
>  	struct prot_inuse __percpu *prot_inuse;
> diff --git a/include/net/sock.h b/include/net/sock.h
> index 66a9a90f9558..d8a73edb1629 100644
> --- a/include/net/sock.h
> +++ b/include/net/sock.h
> @@ -577,6 +577,18 @@ static inline bool sk_user_data_is_nocopy(const struct sock *sk)
>  			   __tmp | SK_USER_DATA_NOCOPY);		\
>  })
>  
> +static inline
> +struct net *sock_net(const struct sock *sk)
> +{
> +	return read_pnet(&sk->sk_net);
> +}
> +
> +static inline
> +void sock_net_set(struct sock *sk, struct net *net)
> +{
> +	write_pnet(&sk->sk_net, net);
> +}
> +
>  /*
>   * SK_CAN_REUSE and SK_NO_REUSE on a socket mean that the socket is OK
>   * or not whether his port will be reused by someone else. SK_FORCE_REUSE
> @@ -1942,10 +1954,18 @@ static inline void sk_set_txhash(struct sock *sk)
>  
>  static inline bool sk_rethink_txhash(struct sock *sk)
>  {
> -	if (sk->sk_txhash) {
> +	unsigned int rehash;
> +
> +	if (!sk->sk_txhash)
> +		return false;
> +
> +	rehash = READ_ONCE(sock_net(sk)->core.sysctl_txrehash);
> +
> +	if (rehash) {
>  		sk_set_txhash(sk);
>  		return true;
>  	}
> +
>  	return false;
>  }
>  
> @@ -2596,18 +2616,6 @@ static inline void sk_eat_skb(struct sock *sk, struct sk_buff *skb)
>  	__kfree_skb(skb);
>  }
>  
> -static inline
> -struct net *sock_net(const struct sock *sk)
> -{
> -	return read_pnet(&sk->sk_net);
> -}
> -
> -static inline
> -void sock_net_set(struct sock *sk, struct net *net)
> -{
> -	write_pnet(&sk->sk_net, net);
> -}
> -
>  static inline bool
>  skb_sk_is_prefetched(struct sk_buff *skb)
>  {
> diff --git a/include/uapi/linux/socket.h b/include/uapi/linux/socket.h
> index eb0a9a5b6e71..0accd6102ece 100644
> --- a/include/uapi/linux/socket.h
> +++ b/include/uapi/linux/socket.h
> @@ -31,4 +31,7 @@ struct __kernel_sockaddr_storage {
>  
>  #define SOCK_BUF_LOCK_MASK (SOCK_SNDBUF_LOCK | SOCK_RCVBUF_LOCK)
>  
> +#define SOCK_TXREHASH_DISABLED	0
> +#define SOCK_TXREHASH_ENABLED	1
> +
>  #endif /* _UAPI_LINUX_SOCKET_H */
> diff --git a/net/core/net_namespace.c b/net/core/net_namespace.c
> index a448a9b5bb2d..0d833b861f00 100644
> --- a/net/core/net_namespace.c
> +++ b/net/core/net_namespace.c
> @@ -359,6 +359,9 @@ static __net_init int setup_net(struct net *net, struct user_namespace *user_ns)
>  static int __net_init net_defaults_init_net(struct net *net)
>  {
>  	net->core.sysctl_somaxconn = SOMAXCONN;
> +
> +	net->core.sysctl_txrehash = SOCK_TXREHASH_DISABLED;
> +
>  	return 0;
>  }
>  
> diff --git a/net/core/sysctl_net_core.c b/net/core/sysctl_net_core.c
> index c8496c1142c9..34144abbb6a0 100644
> --- a/net/core/sysctl_net_core.c
> +++ b/net/core/sysctl_net_core.c
> @@ -592,6 +592,13 @@ static struct ctl_table netns_core_table[] = {
>  		.extra1		= SYSCTL_ZERO,
>  		.proc_handler	= proc_dointvec_minmax
>  	},
> +	{
> +		.procname	= "txrehash",
> +		.data		= &init_net.core.sysctl_txrehash,
> +		.maxlen		= sizeof(unsigned int),
> +		.mode		= 0644,
> +		.proc_handler	= proc_do_intvec

   .maxlen = sizeof(u8),
   extra1 = SYSCTL_ZERO,
   extra2 = SYSCTL_ONE,
   .proc_handler = proc_dou8vec_minmax,
> +	},
>  	{ }
>  };
>  
> 