Re: [RFC PATCH net-next v5 4/9] net/smc: Introduce SMC-D loopback device

[Simon Horman <simon.horman@corigine.com>]

On Sun, Apr 23, 2023 at 08:17:46PM +0800, Wen Gu wrote:
> This patch introduces a kind of loopback device for SMC-D, thus
> enabling the SMC communication between two local sockets within
> one OS instance.
> 
> The loopback device supports basic capabilities defined by SMC-D
> options, and exposed as an SMC-D v2 device.
> 
> The GID of loopback device is random generated, CHID is 0xFFFF
> and SEID is SMCD_DEFAULT_V2_SEID.
> 
> TODO:
> - The hierarchy preference of coexistent SMC-D devices.
> 
> Signed-off-by: Wen Gu <guwen@linux.alibaba.com>
> ---
>  include/net/smc.h      |   6 +
>  net/smc/Makefile       |   2 +-
>  net/smc/af_smc.c       |  12 +-
>  net/smc/smc_cdc.c      |   9 +-
>  net/smc/smc_cdc.h      |   1 +
>  net/smc/smc_ism.h      |   2 +
>  net/smc/smc_loopback.c | 406 +++++++++++++++++++++++++++++++++++++++++++++++++
>  net/smc/smc_loopback.h |  51 +++++++
>  8 files changed, 486 insertions(+), 3 deletions(-)
>  create mode 100644 net/smc/smc_loopback.c
>  create mode 100644 net/smc/smc_loopback.h
> 
> diff --git a/include/net/smc.h b/include/net/smc.h
> index 26206d2..021ca42 100644
> --- a/include/net/smc.h
> +++ b/include/net/smc.h
> @@ -41,6 +41,12 @@ struct smcd_dmb {
>  	dma_addr_t dma_addr;
>  };
>  
> +struct smcd_seid {
> +	u8 seid_string[24];
> +	u8 serial_number[4];
> +	u8 type[4];
> +};
> +
>  #define ISM_EVENT_DMB	0
>  #define ISM_EVENT_GID	1
>  #define ISM_EVENT_SWR	2
> diff --git a/net/smc/Makefile b/net/smc/Makefile
> index 875efcd..a8c3711 100644
> --- a/net/smc/Makefile
> +++ b/net/smc/Makefile
> @@ -4,5 +4,5 @@ obj-$(CONFIG_SMC)	+= smc.o
>  obj-$(CONFIG_SMC_DIAG)	+= smc_diag.o
>  smc-y := af_smc.o smc_pnet.o smc_ib.o smc_clc.o smc_core.o smc_wr.o smc_llc.o
>  smc-y += smc_cdc.o smc_tx.o smc_rx.o smc_close.o smc_ism.o smc_netlink.o smc_stats.o
> -smc-y += smc_tracepoint.o
> +smc-y += smc_tracepoint.o smc_loopback.o
>  smc-$(CONFIG_SYSCTL) += smc_sysctl.o
> diff --git a/net/smc/af_smc.c b/net/smc/af_smc.c
> index 50c38b6..3230309 100644
> --- a/net/smc/af_smc.c
> +++ b/net/smc/af_smc.c
> @@ -53,6 +53,7 @@
>  #include "smc_stats.h"
>  #include "smc_tracepoint.h"
>  #include "smc_sysctl.h"
> +#include "smc_loopback.h"
>  
>  static DEFINE_MUTEX(smc_server_lgr_pending);	/* serialize link group
>  						 * creation on server
> @@ -3482,15 +3483,23 @@ static int __init smc_init(void)
>  		goto out_sock;
>  	}
>  
> +	rc = smc_loopback_init();
> +	if (rc) {
> +		pr_err("%s: smc_loopback_init fails with %d\n", __func__, rc);
> +		goto out_ib;
> +	}
> +
>  	rc = tcp_register_ulp(&smc_ulp_ops);
>  	if (rc) {
>  		pr_err("%s: tcp_ulp_register fails with %d\n", __func__, rc);
> -		goto out_ib;
> +		goto out_lo;
>  	}
>  
>  	static_branch_enable(&tcp_have_smc);
>  	return 0;
>  
> +out_lo:
> +	smc_loopback_exit();
>  out_ib:
>  	smc_ib_unregister_client();
>  out_sock:
> @@ -3528,6 +3537,7 @@ static void __exit smc_exit(void)
>  	tcp_unregister_ulp(&smc_ulp_ops);
>  	sock_unregister(PF_SMC);
>  	smc_core_exit();
> +	smc_loopback_exit();
>  	smc_ib_unregister_client();
>  	smc_ism_exit();
>  	destroy_workqueue(smc_close_wq);
> diff --git a/net/smc/smc_cdc.c b/net/smc/smc_cdc.c
> index 89105e9..2f79bac 100644
> --- a/net/smc/smc_cdc.c
> +++ b/net/smc/smc_cdc.c
> @@ -410,7 +410,14 @@ static void smc_cdc_msg_recv(struct smc_sock *smc, struct smc_cdc_msg *cdc)
>   */
>  static void smcd_cdc_rx_tsklet(struct tasklet_struct *t)
>  {
> -	struct smc_connection *conn = from_tasklet(conn, t, rx_tsklet);
> +	struct smc_connection *conn =
> +		from_tasklet(conn, t, rx_tsklet);

nit: I think the above can be on one line, as it was before this patch.

> +
> +	smcd_cdc_rx_handler(conn);
> +}
> +
> +void smcd_cdc_rx_handler(struct smc_connection *conn)
> +{
>  	struct smcd_cdc_msg *data_cdc;
>  	struct smcd_cdc_msg cdc;
>  	struct smc_sock *smc;
> diff --git a/net/smc/smc_cdc.h b/net/smc/smc_cdc.h
> index 696cc11..11559d4 100644
> --- a/net/smc/smc_cdc.h
> +++ b/net/smc/smc_cdc.h
> @@ -301,5 +301,6 @@ int smcr_cdc_msg_send_validation(struct smc_connection *conn,
>  				 struct smc_wr_buf *wr_buf);
>  int smc_cdc_init(void) __init;
>  void smcd_cdc_rx_init(struct smc_connection *conn);
> +void smcd_cdc_rx_handler(struct smc_connection *conn);
>  
>  #endif /* SMC_CDC_H */
> diff --git a/net/smc/smc_ism.h b/net/smc/smc_ism.h
> index 14d2e77..d18c50a 100644
> --- a/net/smc/smc_ism.h
> +++ b/net/smc/smc_ism.h
> @@ -15,6 +15,8 @@
>  
>  #include "smc.h"
>  
> +#define S390_ISM_IDENT_MASK 0x00FFFF

nit: I think GENMASK is appropriate here.

> +
>  struct smcd_dev_list {	/* List of SMCD devices */
>  	struct list_head list;
>  	struct mutex mutex;	/* Protects list of devices */

...