Re: [PATCH net-next v6 4/5] net/smc: Dynamic control auto fallback by socket options

[Karsten Graul <kgraul@linux.ibm.com>]

On 09/02/2022 15:11, D. Wythe wrote:
> From: "D. Wythe" <alibuda@linux.alibaba.com>
> 
> This patch aims to add dynamic control for SMC auto fallback, since we

Same here, we need a different wording. Maybe something like 
"SMC handshake limitation".

> don't have socket option level for SMC yet, which requires we need to
> implement it at the same time.
> 
> This patch does the following:
> 
> - add new socket option level: SOL_SMC.
> - add new SMC socket option: SMC_AUTO_FALLBACK.
> - provide getter/setter for SMC socket options.
> 
> Signed-off-by: D. Wythe <alibuda@linux.alibaba.com>
> ---
>  include/linux/socket.h   |  1 +
>  include/uapi/linux/smc.h |  4 +++
>  net/smc/af_smc.c         | 69 +++++++++++++++++++++++++++++++++++++++++++++++-
>  net/smc/smc.h            |  1 +
>  4 files changed, 74 insertions(+), 1 deletion(-)
> 
> diff --git a/include/linux/socket.h b/include/linux/socket.h
> index 8ef26d8..6f85f5d 100644
> --- a/include/linux/socket.h
> +++ b/include/linux/socket.h
> @@ -366,6 +366,7 @@ struct ucred {
>  #define SOL_XDP		283
>  #define SOL_MPTCP	284
>  #define SOL_MCTP	285
> +#define SOL_SMC		286
>  
>  /* IPX options */
>  #define IPX_TYPE	1
> diff --git a/include/uapi/linux/smc.h b/include/uapi/linux/smc.h
> index 6c2874f..9f2cbf8 100644
> --- a/include/uapi/linux/smc.h
> +++ b/include/uapi/linux/smc.h
> @@ -284,4 +284,8 @@ enum {
>  	__SMC_NLA_SEID_TABLE_MAX,
>  	SMC_NLA_SEID_TABLE_MAX = __SMC_NLA_SEID_TABLE_MAX - 1
>  };
> +
> +/* SMC socket options */
> +#define SMC_AUTO_FALLBACK 1	/* allow auto fallback to TCP */

One idea: SMC_LIMIT_HS ?

> +
>  #endif /* _UAPI_LINUX_SMC_H */
> diff --git a/net/smc/af_smc.c b/net/smc/af_smc.c
> index 8175f60..c313561 100644
> --- a/net/smc/af_smc.c
> +++ b/net/smc/af_smc.c
> @@ -2325,7 +2325,8 @@ static int smc_listen(struct socket *sock, int backlog)
>  
>  	inet_csk(smc->clcsock->sk)->icsk_af_ops = &smc->af_ops;
>  
> -	tcp_sk(smc->clcsock->sk)->smc_in_limited = smc_is_in_limited;
> +	if (smc->auto_fallback)
> +		tcp_sk(smc->clcsock->sk)->smc_in_limited = smc_is_in_limited;
>  
>  	rc = kernel_listen(smc->clcsock, backlog);
>  	if (rc) {
> @@ -2620,6 +2621,67 @@ static int smc_shutdown(struct socket *sock, int how)
>  	return rc ? rc : rc1;
>  }
>  
> +static int __smc_getsockopt(struct socket *sock, int level, int optname,
> +			    char __user *optval, int __user *optlen)
> +{
> +	struct smc_sock *smc;
> +	int val, len;
> +
> +	smc = smc_sk(sock->sk);
> +
> +	if (get_user(len, optlen))
> +		return -EFAULT;
> +
> +	len = min_t(int, len, sizeof(int));
> +
> +	if (len < 0)
> +		return -EINVAL;
> +
> +	switch (optname) {
> +	case SMC_AUTO_FALLBACK:
> +		val = smc->auto_fallback;
> +		break;
> +	default:
> +		return -EOPNOTSUPP;
> +	}
> +
> +	if (put_user(len, optlen))
> +		return -EFAULT;
> +	if (copy_to_user(optval, &val, len))
> +		return -EFAULT;
> +
> +	return 0;
> +}
> +
> +static int __smc_setsockopt(struct socket *sock, int level, int optname,
> +			    sockptr_t optval, unsigned int optlen)
> +{
> +	struct sock *sk = sock->sk;
> +	struct smc_sock *smc;
> +	int val, rc;
> +
> +	smc = smc_sk(sk);
> +
> +	lock_sock(sk);
> +	switch (optname) {
> +	case SMC_AUTO_FALLBACK:
> +		if (optlen < sizeof(int))
> +			return -EINVAL;
> +		if (copy_from_sockptr(&val, optval, sizeof(int)))
> +			return -EFAULT;
> +
> +		smc->auto_fallback = !!val;
> +		rc = 0;
> +		break;
> +	default:
> +		rc = -EOPNOTSUPP;
> +		break;
> +	}
> +	release_sock(sk);
> +
> +	return rc;
> +}
> +
>  static int smc_setsockopt(struct socket *sock, int level, int optname,
>  			  sockptr_t optval, unsigned int optlen)
>  {
> @@ -2629,6 +2691,8 @@ static int smc_setsockopt(struct socket *sock, int level, int optname,
>  
>  	if (level == SOL_TCP && optname == TCP_ULP)
>  		return -EOPNOTSUPP;
> +	else if (level == SOL_SMC)
> +		return __smc_setsockopt(sock, level, optname, optval, optlen);
>  
>  	smc = smc_sk(sk);
>  
> @@ -2711,6 +2775,9 @@ static int smc_getsockopt(struct socket *sock, int level, int optname,
>  	struct smc_sock *smc;
>  	int rc;
>  
> +	if (level == SOL_SMC)
> +		return __smc_getsockopt(sock, level, optname, optval, optlen);
> +
>  	smc = smc_sk(sock->sk);
>  	mutex_lock(&smc->clcsock_release_lock);
>  	if (!smc->clcsock) {
> diff --git a/net/smc/smc.h b/net/smc/smc.h
> index 5e5e38d..a0bdf75 100644
> --- a/net/smc/smc.h
> +++ b/net/smc/smc.h
> @@ -249,6 +249,7 @@ struct smc_sock {				/* smc sock container */
>  	struct work_struct	smc_listen_work;/* prepare new accept socket */
>  	struct list_head	accept_q;	/* sockets to be accepted */
>  	spinlock_t		accept_q_lock;	/* protects accept_q */
> +	bool			auto_fallback;	/* auto fallabck to tcp */

New variable name: limit_smc_hs ?

>  	bool			use_fallback;	/* fallback to tcp */
>  	int			fallback_rsn;	/* reason for fallback */
>  	u32			peer_diagnosis; /* decline reason from peer */

-- 
Karsten