netdev - Re: [PATCH net-next] net/smc: Unbind smc control from tcp control

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <Y34Aa3MXGqyd+nlQ@TonyMac-Alibaba>
Date:   Wed, 23 Nov 2022 19:13:47 +0800
From:   Tony Lu <tonylu@...ux.alibaba.com>
To:     Jan Karcher <jaka@...ux.ibm.com>
Cc:     David Miller <davem@...emloft.net>,
        Jakub Kicinski <kuba@...nel.org>, netdev@...r.kernel.org,
        linux-s390@...r.kernel.org, Heiko Carstens <hca@...ux.ibm.com>,
        Alexandra Winter <wintera@...ux.ibm.com>,
        Wenjia Zhang <wenjia@...ux.ibm.com>,
        Thorsten Winkler <twinkler@...ux.ibm.com>,
        Stefan Raspl <raspl@...ux.ibm.com>,
        Karsten Graul <kgraul@...ux.ibm.com>
Subject: Re: [PATCH net-next] net/smc: Unbind smc control from tcp control

On Wed, Nov 23, 2022 at 11:58:30AM +0100, Jan Karcher wrote:
> In the past SMC used the values of tcp_{w|r}mem to create the send
> buffer and RMB. We now have our own sysctl knobs to tune them without
> influencing the TCP default.
> 
> This patch removes the dependency on the TCP control by providing our
> own initial values which aim for a low memory footprint.

+1, before introducing sysctl knobs of SMC, we were going to get rid of
TCP and have SMC own values. Now this does it, So I very much agree with
this.

> 
> Signed-off-by: Jan Karcher <jaka@...ux.ibm.com>
> Reviewed-by: Wenjia Zhang <wenjia@...ux.ibm.com>
> ---
>  Documentation/networking/smc-sysctl.rst |  4 ++--
>  net/smc/smc_core.h                      |  6 ++++--
>  net/smc/smc_sysctl.c                    | 10 ++++++----
>  3 files changed, 12 insertions(+), 8 deletions(-)
> 
> diff --git a/Documentation/networking/smc-sysctl.rst b/Documentation/networking/smc-sysctl.rst
> index 6d8acdbe9be1..a1c634d3690a 100644
> --- a/Documentation/networking/smc-sysctl.rst
> +++ b/Documentation/networking/smc-sysctl.rst
> @@ -44,7 +44,7 @@ smcr_testlink_time - INTEGER
>  
>  wmem - INTEGER
>  	Initial size of send buffer used by SMC sockets.
> -	The default value inherits from net.ipv4.tcp_wmem[1].
> +	The default value aims for a small memory footprint and is set to 16KiB.
>  
>  	The minimum value is 16KiB and there is no hard limit for max value, but
>  	only allowed 512KiB for SMC-R and 1MiB for SMC-D.
> @@ -53,7 +53,7 @@ wmem - INTEGER
>  
>  rmem - INTEGER
>  	Initial size of receive buffer (RMB) used by SMC sockets.
> -	The default value inherits from net.ipv4.tcp_rmem[1].
> +	The default value aims for a small memory footprint and is set to 64KiB.
>  
>  	The minimum value is 16KiB and there is no hard limit for max value, but
>  	only allowed 512KiB for SMC-R and 1MiB for SMC-D.
> diff --git a/net/smc/smc_core.h b/net/smc/smc_core.h
> index 285f9bd8e232..67c3937f341d 100644
> --- a/net/smc/smc_core.h
> +++ b/net/smc/smc_core.h
> @@ -206,8 +206,10 @@ struct smc_rtoken {				/* address/key of remote RMB */
>  	u32			rkey;
>  };
>  
> -#define SMC_BUF_MIN_SIZE	16384	/* minimum size of an RMB */
> -#define SMC_RMBE_SIZES		16	/* number of distinct RMBE sizes */
> +#define SMC_SNDBUF_INIT_SIZE 16384 /* initial size of send buffer */
> +#define SMC_RCVBUF_INIT_SIZE 65536 /* initial size of receive buffer */
> +#define SMC_BUF_MIN_SIZE	 16384	/* minimum size of an RMB */
> +#define SMC_RMBE_SIZES		 16	/* number of distinct RMBE sizes */
>  /* theoretically, the RFC states that largest size would be 512K,
>   * i.e. compressed 5 and thus 6 sizes (0..5), despite
>   * struct smc_clc_msg_accept_confirm.rmbe_size being a 4 bit value (0..15)
> diff --git a/net/smc/smc_sysctl.c b/net/smc/smc_sysctl.c
> index b6f79fabb9d3..a63aa79d4856 100644
> --- a/net/smc/smc_sysctl.c
> +++ b/net/smc/smc_sysctl.c
> @@ -19,8 +19,10 @@
>  #include "smc_llc.h"
>  #include "smc_sysctl.h"
>  
> -static int min_sndbuf = SMC_BUF_MIN_SIZE;
> -static int min_rcvbuf = SMC_BUF_MIN_SIZE;
> +static int initial_sndbuf	= SMC_SNDBUF_INIT_SIZE;
> +static int initial_rcvbuf	= SMC_RCVBUF_INIT_SIZE;
> +static int min_sndbuf		= SMC_BUF_MIN_SIZE;
> +static int min_rcvbuf		= SMC_BUF_MIN_SIZE;
>  
>  static struct ctl_table smc_table[] = {
>  	{
> @@ -88,8 +90,8 @@ int __net_init smc_sysctl_net_init(struct net *net)
>  	net->smc.sysctl_autocorking_size = SMC_AUTOCORKING_DEFAULT_SIZE;
>  	net->smc.sysctl_smcr_buf_type = SMCR_PHYS_CONT_BUFS;
>  	net->smc.sysctl_smcr_testlink_time = SMC_LLC_TESTLINK_DEFAULT_TIME;
> -	WRITE_ONCE(net->smc.sysctl_wmem, READ_ONCE(net->ipv4.sysctl_tcp_wmem[1]));
> -	WRITE_ONCE(net->smc.sysctl_rmem, READ_ONCE(net->ipv4.sysctl_tcp_rmem[1]));
> +	WRITE_ONCE(net->smc.sysctl_wmem, initial_sndbuf);
> +	WRITE_ONCE(net->smc.sysctl_rmem, initial_rcvbuf);

Maybe we can use SMC_{SND|RCV}BUF_INIT_SIZE macro directly, instead of
new variables.

Cheers,
Tony Lu

>  
>  	return 0;
>  
> -- 
> 2.34.1