[PATCH net-next 0/6] net/smc: several features's implementation for smc v2.1

[PATCH net-next 0/6] net/smc: several features's implementation for smc v2.1

[Guangguan Wang]

This patch set implement several new features in SMC v2.1(https://
www.ibm.com/support/pages/node/7009315), including vendor unique
experimental options, max connections per lgr negotiation, max links
per lgr negotiation.

I have removed the RFC tag and changed the patch series to formal
patch series from this version.

RFC v2 - v1:
 - more description in commit message
 - modify SMC_CONN_PER_LGR_xxx and SMC_LINKS_ADD_LNK_xxx
   macro defination and usage
 - rename variable release_ver to release_nr
 - remove redundant release version check in client
 - explicitly set the rc value in smc_llc_cli/srv_add_link

RFC v1 - RFC v2:
 - Remove ini pointer NULL check and fix code style in
   smc_clc_send_confirm_accept.
 - Optimize the max_conns check in smc_clc_xxx_v2x_features_validate.

Guangguan Wang (6):
  net/smc: support smc release version negotiation in clc handshake
  net/smc: add vendor unique experimental options area in clc handshake
  net/smc: support smc v2.x features validate
  net/smc: support max connections per lgr negotiation
  net/smc: support max links per lgr negotiation in clc handshake
  net/smc: Extend SMCR v2 linkgroup netlink attribute

 include/uapi/linux/smc.h |   2 +
 net/smc/af_smc.c         |  83 ++++++++++++++++------
 net/smc/smc.h            |   5 +-
 net/smc/smc_clc.c        | 150 ++++++++++++++++++++++++++++++++-------
 net/smc/smc_clc.h        |  53 ++++++++++++--
 net/smc/smc_core.c       |  13 +++-
 net/smc/smc_core.h       |  25 +++++++
 net/smc/smc_llc.c        |  25 +++++--
 8 files changed, 301 insertions(+), 55 deletions(-)

-- 
2.24.3 (Apple Git-128)

[PATCH net-next 1/6] net/smc: support smc release version negotiation in clc handshake

[Guangguan Wang]

Support smc release version negotiation in clc handshake based on
SMC v2, where no negotiation process for different releases, but
for different versions. The latest smc release version was updated
to v2.1. And currently there are two release versions of SMCv2, v2.0
and v2.1. In the release version negotiation, client sends the preferred
release version by CLC Proposal Message, server makes decision for which
release version to use based on the client preferred release version and
self-supported release version (here choose the minimum release version
of the client preferred and server latest supported), then the decision
returns to client by CLC Accept Message. Client confirms the decision by
CLC Confirm Message.

Client                                    Server
      Proposal(preferred release version)
     ------------------------------------>

      Accept(accpeted release version)
 min(client preferred, server latest supported)
     <------------------------------------

      Confirm(accpeted release version)
     ------------------------------------>

Signed-off-by: Guangguan Wang <guangguan.wang@linux.alibaba.com>
Reviewed-by: Tony Lu <tonylu@linux.alibaba.com>
---
 net/smc/af_smc.c   | 18 ++++++++++++++++--
 net/smc/smc.h      |  5 ++++-
 net/smc/smc_clc.c  | 14 +++++++-------
 net/smc/smc_clc.h  | 23 ++++++++++++++++++++++-
 net/smc/smc_core.h |  1 +
 5 files changed, 50 insertions(+), 11 deletions(-)

diff --git a/net/smc/af_smc.c b/net/smc/af_smc.c
index a7f887d91d89..97265691bc95 100644
--- a/net/smc/af_smc.c
+++ b/net/smc/af_smc.c
@@ -1187,6 +1187,9 @@ static int smc_connect_rdma_v2_prepare(struct smc_sock *smc,
 			return SMC_CLC_DECL_NOINDIRECT;
 		}
 	}
+
+	ini->release_nr = fce->release;
+
 	return 0;
 }
 
@@ -1355,6 +1358,13 @@ static int smc_connect_ism(struct smc_sock *smc,
 		struct smc_clc_msg_accept_confirm_v2 *aclc_v2 =
 			(struct smc_clc_msg_accept_confirm_v2 *)aclc;
 
+		if (ini->first_contact_peer) {
+			struct smc_clc_first_contact_ext *fce =
+				smc_get_clc_first_contact_ext(aclc_v2, true);
+
+			ini->release_nr = fce->release;
+		}
+
 		rc = smc_v2_determine_accepted_chid(aclc_v2, ini);
 		if (rc)
 			return rc;
@@ -1389,7 +1399,7 @@ static int smc_connect_ism(struct smc_sock *smc,
 	}
 
 	rc = smc_clc_send_confirm(smc, ini->first_contact_local,
-				  aclc->hdr.version, eid, NULL);
+				  aclc->hdr.version, eid, ini);
 	if (rc)
 		goto connect_abort;
 	mutex_unlock(&smc_server_lgr_pending);
@@ -1965,6 +1975,10 @@ static int smc_listen_v2_check(struct smc_sock *new_smc,
 		}
 	}
 
+	ini->release_nr = pclc_v2_ext->hdr.flag.release;
+	if (pclc_v2_ext->hdr.flag.release > SMC_RELEASE)
+		ini->release_nr = SMC_RELEASE;
+
 out:
 	if (!ini->smcd_version && !ini->smcr_version)
 		return rc;
@@ -2412,7 +2426,7 @@ static void smc_listen_work(struct work_struct *work)
 	/* send SMC Accept CLC message */
 	accept_version = ini->is_smcd ? ini->smcd_version : ini->smcr_version;
 	rc = smc_clc_send_accept(new_smc, ini->first_contact_local,
-				 accept_version, ini->negotiated_eid);
+				 accept_version, ini->negotiated_eid, ini);
 	if (rc)
 		goto out_unlock;
 
diff --git a/net/smc/smc.h b/net/smc/smc.h
index 2eeea4cdc718..9cce1a41e011 100644
--- a/net/smc/smc.h
+++ b/net/smc/smc.h
@@ -21,7 +21,10 @@
 
 #define SMC_V1		1		/* SMC version V1 */
 #define SMC_V2		2		/* SMC version V2 */
-#define SMC_RELEASE	0
+
+#define SMC_RELEASE_0 0
+#define SMC_RELEASE_1 1
+#define SMC_RELEASE	SMC_RELEASE_1 /* the latest release version */
 
 #define SMCPROTO_SMC		0	/* SMC protocol, IPv4 */
 #define SMCPROTO_SMC6		1	/* SMC protocol, IPv6 */
diff --git a/net/smc/smc_clc.c b/net/smc/smc_clc.c
index b9b8b07aa702..7c5627c6abcc 100644
--- a/net/smc/smc_clc.c
+++ b/net/smc/smc_clc.c
@@ -420,11 +420,11 @@ smc_clc_msg_decl_valid(struct smc_clc_msg_decline *dclc)
 	return true;
 }
 
-static void smc_clc_fill_fce(struct smc_clc_first_contact_ext *fce, int *len)
+static void smc_clc_fill_fce(struct smc_clc_first_contact_ext *fce, int *len, int release_nr)
 {
 	memset(fce, 0, sizeof(*fce));
 	fce->os_type = SMC_CLC_OS_LINUX;
-	fce->release = SMC_RELEASE;
+	fce->release = release_nr;
 	memcpy(fce->hostname, smc_hostname, sizeof(smc_hostname));
 	(*len) += sizeof(*fce);
 }
@@ -1019,7 +1019,7 @@ static int smc_clc_send_confirm_accept(struct smc_sock *smc,
 				memcpy(clc_v2->d1.eid, eid, SMC_MAX_EID_LEN);
 			len = SMCD_CLC_ACCEPT_CONFIRM_LEN_V2;
 			if (first_contact)
-				smc_clc_fill_fce(&fce, &len);
+				smc_clc_fill_fce(&fce, &len, ini->release_nr);
 			clc_v2->hdr.length = htons(len);
 		}
 		memcpy(trl.eyecatcher, SMCD_EYECATCHER,
@@ -1063,10 +1063,10 @@ static int smc_clc_send_confirm_accept(struct smc_sock *smc,
 				memcpy(clc_v2->r1.eid, eid, SMC_MAX_EID_LEN);
 			len = SMCR_CLC_ACCEPT_CONFIRM_LEN_V2;
 			if (first_contact) {
-				smc_clc_fill_fce(&fce, &len);
+				smc_clc_fill_fce(&fce, &len, ini->release_nr);
 				fce.v2_direct = !link->lgr->uses_gateway;
 				memset(&gle, 0, sizeof(gle));
-				if (ini && clc->hdr.type == SMC_CLC_CONFIRM) {
+				if (clc->hdr.type == SMC_CLC_CONFIRM) {
 					gle.gid_cnt = ini->smcrv2.gidlist.len;
 					len += sizeof(gle);
 					len += gle.gid_cnt * sizeof(gle.gid[0]);
@@ -1141,7 +1141,7 @@ int smc_clc_send_confirm(struct smc_sock *smc, bool clnt_first_contact,
 
 /* send CLC ACCEPT message across internal TCP socket */
 int smc_clc_send_accept(struct smc_sock *new_smc, bool srv_first_contact,
-			u8 version, u8 *negotiated_eid)
+			u8 version, u8 *negotiated_eid, struct smc_init_info *ini)
 {
 	struct smc_clc_msg_accept_confirm_v2 aclc_v2;
 	int len;
@@ -1149,7 +1149,7 @@ int smc_clc_send_accept(struct smc_sock *new_smc, bool srv_first_contact,
 	memset(&aclc_v2, 0, sizeof(aclc_v2));
 	aclc_v2.hdr.type = SMC_CLC_ACCEPT;
 	len = smc_clc_send_confirm_accept(new_smc, &aclc_v2, srv_first_contact,
-					  version, negotiated_eid, NULL);
+					  version, negotiated_eid, ini);
 	if (len < ntohs(aclc_v2.hdr.length))
 		len = len >= 0 ? -EPROTO : -new_smc->clcsock->sk->sk_err;
 
diff --git a/net/smc/smc_clc.h b/net/smc/smc_clc.h
index 5fee545c9a10..b923e89acafb 100644
--- a/net/smc/smc_clc.h
+++ b/net/smc/smc_clc.h
@@ -370,6 +370,27 @@ smc_get_clc_smcd_v2_ext(struct smc_clc_v2_extension *prop_v2ext)
 		 ntohs(prop_v2ext->hdr.smcd_v2_ext_offset));
 }
 
+static inline struct smc_clc_first_contact_ext *
+smc_get_clc_first_contact_ext(struct smc_clc_msg_accept_confirm_v2 *clc_v2,
+			      bool is_smcd)
+{
+	int clc_v2_len;
+
+	if (clc_v2->hdr.version == SMC_V1 ||
+	    !(clc_v2->hdr.typev2 & SMC_FIRST_CONTACT_MASK))
+		return NULL;
+
+	if (is_smcd)
+		clc_v2_len =
+			offsetofend(struct smc_clc_msg_accept_confirm_v2, d1);
+	else
+		clc_v2_len =
+			offsetofend(struct smc_clc_msg_accept_confirm_v2, r1);
+
+	return (struct smc_clc_first_contact_ext *)(((u8 *)clc_v2) +
+						    clc_v2_len);
+}
+
 struct smcd_dev;
 struct smc_init_info;
 
@@ -382,7 +403,7 @@ int smc_clc_send_proposal(struct smc_sock *smc, struct smc_init_info *ini);
 int smc_clc_send_confirm(struct smc_sock *smc, bool clnt_first_contact,
 			 u8 version, u8 *eid, struct smc_init_info *ini);
 int smc_clc_send_accept(struct smc_sock *smc, bool srv_first_contact,
-			u8 version, u8 *negotiated_eid);
+			u8 version, u8 *negotiated_eid, struct smc_init_info *ini);
 void smc_clc_init(void) __init;
 void smc_clc_exit(void);
 void smc_clc_get_hostname(u8 **host);
diff --git a/net/smc/smc_core.h b/net/smc/smc_core.h
index 3c1b31bfa1cf..b6c68db61f23 100644
--- a/net/smc/smc_core.h
+++ b/net/smc/smc_core.h
@@ -374,6 +374,7 @@ struct smc_init_info {
 	u8			is_smcd;
 	u8			smc_type_v1;
 	u8			smc_type_v2;
+	u8			release_nr;
 	u8			first_contact_peer;
 	u8			first_contact_local;
 	unsigned short		vlan_id;
-- 
2.24.3 (Apple Git-128)

Re: [PATCH net-next 1/6] net/smc: support smc release version negotiation in clc handshake

[Jan Karcher]

On 16/08/2023 10:33, Guangguan Wang wrote:
> Support smc release version negotiation in clc handshake based on
> SMC v2, where no negotiation process for different releases, but
> for different versions. The latest smc release version was updated
> to v2.1. And currently there are two release versions of SMCv2, v2.0
> and v2.1. In the release version negotiation, client sends the preferred
> release version by CLC Proposal Message, server makes decision for which
> release version to use based on the client preferred release version and
> self-supported release version (here choose the minimum release version
> of the client preferred and server latest supported), then the decision
> returns to client by CLC Accept Message. Client confirms the decision by
> CLC Confirm Message.
> 
> Client                                    Server
>        Proposal(preferred release version)
>       ------------------------------------>
> 
>        Accept(accpeted release version)
>   min(client preferred, server latest supported)
>       <------------------------------------
> 
>        Confirm(accpeted release version)
>       ------------------------------------>
> 
> Signed-off-by: Guangguan Wang <guangguan.wang@linux.alibaba.com>
> Reviewed-by: Tony Lu <tonylu@linux.alibaba.com>
> ---
>   net/smc/af_smc.c   | 18 ++++++++++++++++--
>   net/smc/smc.h      |  5 ++++-
>   net/smc/smc_clc.c  | 14 +++++++-------
>   net/smc/smc_clc.h  | 23 ++++++++++++++++++++++-
>   net/smc/smc_core.h |  1 +
>   5 files changed, 50 insertions(+), 11 deletions(-)
> 
> diff --git a/net/smc/af_smc.c b/net/smc/af_smc.c
> index a7f887d91d89..97265691bc95 100644
> --- a/net/smc/af_smc.c
> +++ b/net/smc/af_smc.c
> @@ -1187,6 +1187,9 @@ static int smc_connect_rdma_v2_prepare(struct smc_sock *smc,
>   			return SMC_CLC_DECL_NOINDIRECT;
>   		}
>   	}
> +
> +	ini->release_nr = fce->release;
> +

why would we do this and vvvvv
>   	return 0;
>   }
>   
> @@ -1355,6 +1358,13 @@ static int smc_connect_ism(struct smc_sock *smc,
>   		struct smc_clc_msg_accept_confirm_v2 *aclc_v2 =
>   			(struct smc_clc_msg_accept_confirm_v2 *)aclc;
>   
> +		if (ini->first_contact_peer) {
> +			struct smc_clc_first_contact_ext *fce =
> +				smc_get_clc_first_contact_ext(aclc_v2, true);
> +
> +			ini->release_nr = fce->release;
> +		}
> +

this two times?
Can't we put this together into __smc_connect where those functions get 
called (via smc_connect_rdma and smc_connect_ism)?

Please provide reasoning, it might be that i oversaw the reasoning 
behind this duplication.

Also note: Even if there is a reason to set this information seperate 
for SMC-D and SMC-R think about using your very neat helper function 
(smc_get_clc_first_contact_ext) in smc_connect_rdma_v2_prepare as well.

>   		rc = smc_v2_determine_accepted_chid(aclc_v2, ini);
>   		if (rc)
>   			return rc;
> @@ -1389,7 +1399,7 @@ static int smc_connect_ism(struct smc_sock *smc,
>   	}
>   
>   	rc = smc_clc_send_confirm(smc, ini->first_contact_local,
> -				  aclc->hdr.version, eid, NULL);
> +				  aclc->hdr.version, eid, ini);
>   	if (rc)
>   		goto connect_abort;
>   	mutex_unlock(&smc_server_lgr_pending);
> @@ -1965,6 +1975,10 @@ static int smc_listen_v2_check(struct smc_sock *new_smc,
>   		}
>   	}
>   
> +	ini->release_nr = pclc_v2_ext->hdr.flag.release;
> +	if (pclc_v2_ext->hdr.flag.release > SMC_RELEASE)
> +		ini->release_nr = SMC_RELEASE;
> +
>   out:
>   	if (!ini->smcd_version && !ini->smcr_version)
>   		return rc;
> @@ -2412,7 +2426,7 @@ static void smc_listen_work(struct work_struct *work)
>   	/* send SMC Accept CLC message */
>   	accept_version = ini->is_smcd ? ini->smcd_version : ini->smcr_version;
>   	rc = smc_clc_send_accept(new_smc, ini->first_contact_local,
> -				 accept_version, ini->negotiated_eid);
> +				 accept_version, ini->negotiated_eid, ini);
>   	if (rc)
>   		goto out_unlock;
>   
> diff --git a/net/smc/smc.h b/net/smc/smc.h
> index 2eeea4cdc718..9cce1a41e011 100644
> --- a/net/smc/smc.h
> +++ b/net/smc/smc.h
> @@ -21,7 +21,10 @@
>   
>   #define SMC_V1		1		/* SMC version V1 */
>   #define SMC_V2		2		/* SMC version V2 */
> -#define SMC_RELEASE	0
> +
> +#define SMC_RELEASE_0 0
> +#define SMC_RELEASE_1 1
> +#define SMC_RELEASE	SMC_RELEASE_1 /* the latest release version */
>   
>   #define SMCPROTO_SMC		0	/* SMC protocol, IPv4 */
>   #define SMCPROTO_SMC6		1	/* SMC protocol, IPv6 */
> diff --git a/net/smc/smc_clc.c b/net/smc/smc_clc.c
> index b9b8b07aa702..7c5627c6abcc 100644
> --- a/net/smc/smc_clc.c
> +++ b/net/smc/smc_clc.c
> @@ -420,11 +420,11 @@ smc_clc_msg_decl_valid(struct smc_clc_msg_decline *dclc)
>   	return true;
>   }
>   
> -static void smc_clc_fill_fce(struct smc_clc_first_contact_ext *fce, int *len)
> +static void smc_clc_fill_fce(struct smc_clc_first_contact_ext *fce, int *len, int release_nr)
>   {
>   	memset(fce, 0, sizeof(*fce));
>   	fce->os_type = SMC_CLC_OS_LINUX;
> -	fce->release = SMC_RELEASE;
> +	fce->release = release_nr;
>   	memcpy(fce->hostname, smc_hostname, sizeof(smc_hostname));
>   	(*len) += sizeof(*fce);
>   }
> @@ -1019,7 +1019,7 @@ static int smc_clc_send_confirm_accept(struct smc_sock *smc,
>   				memcpy(clc_v2->d1.eid, eid, SMC_MAX_EID_LEN);
>   			len = SMCD_CLC_ACCEPT_CONFIRM_LEN_V2;
>   			if (first_contact)
> -				smc_clc_fill_fce(&fce, &len);
> +				smc_clc_fill_fce(&fce, &len, ini->release_nr);
>   			clc_v2->hdr.length = htons(len);
>   		}
>   		memcpy(trl.eyecatcher, SMCD_EYECATCHER,
> @@ -1063,10 +1063,10 @@ static int smc_clc_send_confirm_accept(struct smc_sock *smc,
>   				memcpy(clc_v2->r1.eid, eid, SMC_MAX_EID_LEN);
>   			len = SMCR_CLC_ACCEPT_CONFIRM_LEN_V2;
>   			if (first_contact) {
> -				smc_clc_fill_fce(&fce, &len);
> +				smc_clc_fill_fce(&fce, &len, ini->release_nr);
>   				fce.v2_direct = !link->lgr->uses_gateway;
>   				memset(&gle, 0, sizeof(gle));
> -				if (ini && clc->hdr.type == SMC_CLC_CONFIRM) {
> +				if (clc->hdr.type == SMC_CLC_CONFIRM) {
>   					gle.gid_cnt = ini->smcrv2.gidlist.len;
>   					len += sizeof(gle);
>   					len += gle.gid_cnt * sizeof(gle.gid[0]);
> @@ -1141,7 +1141,7 @@ int smc_clc_send_confirm(struct smc_sock *smc, bool clnt_first_contact,
>   
>   /* send CLC ACCEPT message across internal TCP socket */
>   int smc_clc_send_accept(struct smc_sock *new_smc, bool srv_first_contact,
> -			u8 version, u8 *negotiated_eid)
> +			u8 version, u8 *negotiated_eid, struct smc_init_info *ini)
>   {
>   	struct smc_clc_msg_accept_confirm_v2 aclc_v2;
>   	int len;
> @@ -1149,7 +1149,7 @@ int smc_clc_send_accept(struct smc_sock *new_smc, bool srv_first_contact,
>   	memset(&aclc_v2, 0, sizeof(aclc_v2));
>   	aclc_v2.hdr.type = SMC_CLC_ACCEPT;
>   	len = smc_clc_send_confirm_accept(new_smc, &aclc_v2, srv_first_contact,
> -					  version, negotiated_eid, NULL);
> +					  version, negotiated_eid, ini);
>   	if (len < ntohs(aclc_v2.hdr.length))
>   		len = len >= 0 ? -EPROTO : -new_smc->clcsock->sk->sk_err;
>   
> diff --git a/net/smc/smc_clc.h b/net/smc/smc_clc.h
> index 5fee545c9a10..b923e89acafb 100644
> --- a/net/smc/smc_clc.h
> +++ b/net/smc/smc_clc.h
> @@ -370,6 +370,27 @@ smc_get_clc_smcd_v2_ext(struct smc_clc_v2_extension *prop_v2ext)
>   		 ntohs(prop_v2ext->hdr.smcd_v2_ext_offset));
>   }
>   
> +static inline struct smc_clc_first_contact_ext *
> +smc_get_clc_first_contact_ext(struct smc_clc_msg_accept_confirm_v2 *clc_v2,
> +			      bool is_smcd)
> +{
> +	int clc_v2_len;
> +
> +	if (clc_v2->hdr.version == SMC_V1 ||
> +	    !(clc_v2->hdr.typev2 & SMC_FIRST_CONTACT_MASK))
> +		return NULL;
> +
> +	if (is_smcd)
> +		clc_v2_len =
> +			offsetofend(struct smc_clc_msg_accept_confirm_v2, d1);
> +	else
> +		clc_v2_len =
> +			offsetofend(struct smc_clc_msg_accept_confirm_v2, r1);
> +
> +	return (struct smc_clc_first_contact_ext *)(((u8 *)clc_v2) +
> +						    clc_v2_len);
> +}
> +
>   struct smcd_dev;
>   struct smc_init_info;
>   
> @@ -382,7 +403,7 @@ int smc_clc_send_proposal(struct smc_sock *smc, struct smc_init_info *ini);
>   int smc_clc_send_confirm(struct smc_sock *smc, bool clnt_first_contact,
>   			 u8 version, u8 *eid, struct smc_init_info *ini);
>   int smc_clc_send_accept(struct smc_sock *smc, bool srv_first_contact,
> -			u8 version, u8 *negotiated_eid);
> +			u8 version, u8 *negotiated_eid, struct smc_init_info *ini);
>   void smc_clc_init(void) __init;
>   void smc_clc_exit(void);
>   void smc_clc_get_hostname(u8 **host);
> diff --git a/net/smc/smc_core.h b/net/smc/smc_core.h
> index 3c1b31bfa1cf..b6c68db61f23 100644
> --- a/net/smc/smc_core.h
> +++ b/net/smc/smc_core.h
> @@ -374,6 +374,7 @@ struct smc_init_info {
>   	u8			is_smcd;
>   	u8			smc_type_v1;
>   	u8			smc_type_v2;
> +	u8			release_nr;
>   	u8			first_contact_peer;
>   	u8			first_contact_local;
>   	unsigned short		vlan_id;

Re: [PATCH net-next 1/6] net/smc: support smc release version negotiation in clc handshake

[Guangguan Wang]

On 2023/8/16 22:14, Jan Karcher wrote:
> 
> 
> On 16/08/2023 10:33, Guangguan Wang wrote:
>> Support smc release version negotiation in clc handshake based on
>> SMC v2, where no negotiation process for different releases, but
>> for different versions. The latest smc release version was updated
>> to v2.1. And currently there are two release versions of SMCv2, v2.0
>> and v2.1. In the release version negotiation, client sends the preferred
>> release version by CLC Proposal Message, server makes decision for which
>> release version to use based on the client preferred release version and
>> self-supported release version (here choose the minimum release version
>> of the client preferred and server latest supported), then the decision
>> returns to client by CLC Accept Message. Client confirms the decision by
>> CLC Confirm Message.
>>
>> Client                                    Server
>>        Proposal(preferred release version)
>>       ------------------------------------>
>>
>>        Accept(accpeted release version)
>>   min(client preferred, server latest supported)
>>       <------------------------------------
>>
>>        Confirm(accpeted release version)
>>       ------------------------------------>
>>
>> Signed-off-by: Guangguan Wang <guangguan.wang@linux.alibaba.com>
>> Reviewed-by: Tony Lu <tonylu@linux.alibaba.com>
>> ---
>>   net/smc/af_smc.c   | 18 ++++++++++++++++--
>>   net/smc/smc.h      |  5 ++++-
>>   net/smc/smc_clc.c  | 14 +++++++-------
>>   net/smc/smc_clc.h  | 23 ++++++++++++++++++++++-
>>   net/smc/smc_core.h |  1 +
>>   5 files changed, 50 insertions(+), 11 deletions(-)
>>
>> diff --git a/net/smc/af_smc.c b/net/smc/af_smc.c
>> index a7f887d91d89..97265691bc95 100644
>> --- a/net/smc/af_smc.c
>> +++ b/net/smc/af_smc.c
>> @@ -1187,6 +1187,9 @@ static int smc_connect_rdma_v2_prepare(struct smc_sock *smc,
>>               return SMC_CLC_DECL_NOINDIRECT;
>>           }
>>       }
>> +
>> +    ini->release_nr = fce->release;
>> +
> 
> why would we do this and vvvvv
>>       return 0;
>>   }
>>   @@ -1355,6 +1358,13 @@ static int smc_connect_ism(struct smc_sock *smc,
>>           struct smc_clc_msg_accept_confirm_v2 *aclc_v2 =
>>               (struct smc_clc_msg_accept_confirm_v2 *)aclc;
>>   +        if (ini->first_contact_peer) {
>> +            struct smc_clc_first_contact_ext *fce =
>> +                smc_get_clc_first_contact_ext(aclc_v2, true);
>> +
>> +            ini->release_nr = fce->release;
>> +        }
>> +
> 
> this two times?
> Can't we put this together into __smc_connect where those functions get called (via smc_connect_rdma and smc_connect_ism)?
> 
> Please provide reasoning, it might be that i oversaw the reasoning behind this duplication.
> 
ini->release_nr is assigned only when doing first connect, thus this depends on the value test of 
ini->first_contact_peer. I have to follow the ini->first_contact_peer code logic, which may also
make us wonder that why not put ini->first_contact_peer together into __smc_connect.

Indeed, both of ini->first_contact_peer and ini->release_nr can put together into __smc_connect.
But I think it is better to start a new patch series to refactor those code, not in v2.1 features.


> Also note: Even if there is a reason to set this information seperate for SMC-D and SMC-R think about using your very neat helper function (smc_get_clc_first_contact_ext) in smc_connect_rdma_v2_prepare as well.
> 

OK, I will replace the code to smc_get_clc_first_contact_ext.

Thanks,
Guangguan Wang

Re: [PATCH net-next 1/6] net/smc: support smc release version negotiation in clc handshake

[Jan Karcher]

On 17/08/2023 05:18, Guangguan Wang wrote:
> 
> 
> On 2023/8/16 22:14, Jan Karcher wrote:
>>
>>
>> On 16/08/2023 10:33, Guangguan Wang wrote:
>>> Support smc release version negotiation in clc handshake based on
>>> SMC v2, where no negotiation process for different releases, but
>>> for different versions. The latest smc release version was updated
>>> to v2.1. And currently there are two release versions of SMCv2, v2.0
>>> and v2.1. In the release version negotiation, client sends the preferred
>>> release version by CLC Proposal Message, server makes decision for which
>>> release version to use based on the client preferred release version and
>>> self-supported release version (here choose the minimum release version
>>> of the client preferred and server latest supported), then the decision
>>> returns to client by CLC Accept Message. Client confirms the decision by
>>> CLC Confirm Message.
>>>
>>> Client                                    Server
>>>         Proposal(preferred release version)
>>>        ------------------------------------>
>>>
>>>         Accept(accpeted release version)
>>>    min(client preferred, server latest supported)
>>>        <------------------------------------
>>>
>>>         Confirm(accpeted release version)
>>>        ------------------------------------>
>>>
>>> Signed-off-by: Guangguan Wang <guangguan.wang@linux.alibaba.com>
>>> Reviewed-by: Tony Lu <tonylu@linux.alibaba.com>
>>> ---
>>>    net/smc/af_smc.c   | 18 ++++++++++++++++--
>>>    net/smc/smc.h      |  5 ++++-
>>>    net/smc/smc_clc.c  | 14 +++++++-------
>>>    net/smc/smc_clc.h  | 23 ++++++++++++++++++++++-
>>>    net/smc/smc_core.h |  1 +
>>>    5 files changed, 50 insertions(+), 11 deletions(-)
>>>
>>> diff --git a/net/smc/af_smc.c b/net/smc/af_smc.c
>>> index a7f887d91d89..97265691bc95 100644
>>> --- a/net/smc/af_smc.c
>>> +++ b/net/smc/af_smc.c
>>> @@ -1187,6 +1187,9 @@ static int smc_connect_rdma_v2_prepare(struct smc_sock *smc,
>>>                return SMC_CLC_DECL_NOINDIRECT;
>>>            }
>>>        }
>>> +
>>> +    ini->release_nr = fce->release;
>>> +
>>
>> why would we do this and vvvvv
>>>        return 0;
>>>    }
>>>    @@ -1355,6 +1358,13 @@ static int smc_connect_ism(struct smc_sock *smc,
>>>            struct smc_clc_msg_accept_confirm_v2 *aclc_v2 =
>>>                (struct smc_clc_msg_accept_confirm_v2 *)aclc;
>>>    +        if (ini->first_contact_peer) {
>>> +            struct smc_clc_first_contact_ext *fce =
>>> +                smc_get_clc_first_contact_ext(aclc_v2, true);
>>> +
>>> +            ini->release_nr = fce->release;
>>> +        }
>>> +
>>
>> this two times?
>> Can't we put this together into __smc_connect where those functions get called (via smc_connect_rdma and smc_connect_ism)?
>>
>> Please provide reasoning, it might be that i oversaw the reasoning behind this duplication.
>>
> ini->release_nr is assigned only when doing first connect, thus this depends on the value test of
> ini->first_contact_peer. I have to follow the ini->first_contact_peer code logic, which may also
> make us wonder that why not put ini->first_contact_peer together into __smc_connect.
> 
> Indeed, both of ini->first_contact_peer and ini->release_nr can put together into __smc_connect.
> But I think it is better to start a new patch series to refactor those code, not in v2.1 features.

True. It would only be clean if move both. Works for me.

> 
> 
>> Also note: Even if there is a reason to set this information seperate for SMC-D and SMC-R think about using your very neat helper function (smc_get_clc_first_contact_ext) in smc_connect_rdma_v2_prepare as well.
>>
> 
> OK, I will replace the code to smc_get_clc_first_contact_ext.
> 
> Thanks,
> Guangguan Wang
> 

[PATCH net-next 2/6] net/smc: add vendor unique experimental options area in clc handshake

[Guangguan Wang]

Add vendor unique experimental options area in clc handshake. In clc
accept and confirm msg, vendor unique experimental options use the
16-Bytes reserved field, which defined in struct smc_clc_fce_gid_ext
in previous version. Because of the struct smc_clc_first_contact_ext
is widely used and limit the scope of modification, this patch moves
the 16-Bytes reserved field out of struct smc_clc_fce_gid_ext, and
followed with the struct smc_clc_first_contact_ext in a new struct
names struct smc_clc_first_contact_ext_v2x.

For SMC-R first connection, in previous version, the struct smc_clc_
first_contact_ext and the 16-Bytes reserved field has already been
included in clc accept and confirm msg. Thus, this patch use struct
smc_clc_first_contact_ext_v2x instead of the struct smc_clc_first_
contact_ext and the 16-Bytes reserved field in SMC-R clc accept and
confirm msg is compatible with previous version.

For SMC-D first connection, in previous version, only the struct smc_
clc_first_contact_ext is included in clc accept and confirm msg, and
the 16-Bytes reserved field is not included. Thus, when the negotiated
smc release version is the version before v2.1, we still use struct
smc_clc_first_contact_ext for compatible consideration. If the negotiated
smc release version is v2.1 or later, use struct smc_clc_first_contact_
ext_v2x instead.

Signed-off-by: Guangguan Wang <guangguan.wang@linux.alibaba.com>
Reviewed-by: Tony Lu <tonylu@linux.alibaba.com>
---
 net/smc/af_smc.c  |  2 +-
 net/smc/smc_clc.c | 44 +++++++++++++++++++++++---------------------
 net/smc/smc_clc.h | 15 +++++++++++++--
 3 files changed, 37 insertions(+), 24 deletions(-)

diff --git a/net/smc/af_smc.c b/net/smc/af_smc.c
index 97265691bc95..7b54c153bd0d 100644
--- a/net/smc/af_smc.c
+++ b/net/smc/af_smc.c
@@ -1113,7 +1113,7 @@ static int smc_connect_ism_vlan_cleanup(struct smc_sock *smc,
 
 #define SMC_CLC_MAX_ACCEPT_LEN \
 	(sizeof(struct smc_clc_msg_accept_confirm_v2) + \
-	 sizeof(struct smc_clc_first_contact_ext) + \
+	 sizeof(struct smc_clc_first_contact_ext_v2x) + \
 	 sizeof(struct smc_clc_msg_trail))
 
 /* CLC handshake during connect */
diff --git a/net/smc/smc_clc.c b/net/smc/smc_clc.c
index 7c5627c6abcc..624dc970d187 100644
--- a/net/smc/smc_clc.c
+++ b/net/smc/smc_clc.c
@@ -391,9 +391,7 @@ smc_clc_msg_acc_conf_valid(struct smc_clc_msg_accept_confirm_v2 *clc_v2)
 			return false;
 	} else {
 		if (hdr->typev1 == SMC_TYPE_D &&
-		    ntohs(hdr->length) != SMCD_CLC_ACCEPT_CONFIRM_LEN_V2 &&
-		    (ntohs(hdr->length) != SMCD_CLC_ACCEPT_CONFIRM_LEN_V2 +
-				sizeof(struct smc_clc_first_contact_ext)))
+		    ntohs(hdr->length) < SMCD_CLC_ACCEPT_CONFIRM_LEN_V2)
 			return false;
 		if (hdr->typev1 == SMC_TYPE_R &&
 		    ntohs(hdr->length) < SMCR_CLC_ACCEPT_CONFIRM_LEN_V2)
@@ -420,13 +418,19 @@ smc_clc_msg_decl_valid(struct smc_clc_msg_decline *dclc)
 	return true;
 }
 
-static void smc_clc_fill_fce(struct smc_clc_first_contact_ext *fce, int *len, int release_nr)
+static int smc_clc_fill_fce(struct smc_clc_first_contact_ext_v2x *fce,
+			    struct smc_init_info *ini)
 {
+	int ret = sizeof(*fce);
+
 	memset(fce, 0, sizeof(*fce));
-	fce->os_type = SMC_CLC_OS_LINUX;
-	fce->release = release_nr;
-	memcpy(fce->hostname, smc_hostname, sizeof(smc_hostname));
-	(*len) += sizeof(*fce);
+	fce->fce_v20.os_type = SMC_CLC_OS_LINUX;
+	fce->fce_v20.release = ini->release_nr;
+	memcpy(fce->fce_v20.hostname, smc_hostname, sizeof(smc_hostname));
+	if (ini->is_smcd && ini->release_nr < SMC_RELEASE_1)
+		ret = sizeof(struct smc_clc_first_contact_ext);
+
+	return ret;
 }
 
 /* check if received message has a correct header length and contains valid
@@ -986,13 +990,13 @@ static int smc_clc_send_confirm_accept(struct smc_sock *smc,
 				       u8 *eid, struct smc_init_info *ini)
 {
 	struct smc_connection *conn = &smc->conn;
+	struct smc_clc_first_contact_ext_v2x fce;
 	struct smc_clc_msg_accept_confirm *clc;
-	struct smc_clc_first_contact_ext fce;
 	struct smc_clc_fce_gid_ext gle;
 	struct smc_clc_msg_trail trl;
+	int i, len, fce_len;
 	struct kvec vec[5];
 	struct msghdr msg;
-	int i, len;
 
 	/* send SMC Confirm CLC msg */
 	clc = (struct smc_clc_msg_accept_confirm *)clc_v2;
@@ -1018,8 +1022,10 @@ static int smc_clc_send_confirm_accept(struct smc_sock *smc,
 			if (eid && eid[0])
 				memcpy(clc_v2->d1.eid, eid, SMC_MAX_EID_LEN);
 			len = SMCD_CLC_ACCEPT_CONFIRM_LEN_V2;
-			if (first_contact)
-				smc_clc_fill_fce(&fce, &len, ini->release_nr);
+			if (first_contact) {
+				fce_len = smc_clc_fill_fce(&fce, ini);
+				len += fce_len;
+			}
 			clc_v2->hdr.length = htons(len);
 		}
 		memcpy(trl.eyecatcher, SMCD_EYECATCHER,
@@ -1063,15 +1069,14 @@ static int smc_clc_send_confirm_accept(struct smc_sock *smc,
 				memcpy(clc_v2->r1.eid, eid, SMC_MAX_EID_LEN);
 			len = SMCR_CLC_ACCEPT_CONFIRM_LEN_V2;
 			if (first_contact) {
-				smc_clc_fill_fce(&fce, &len, ini->release_nr);
-				fce.v2_direct = !link->lgr->uses_gateway;
-				memset(&gle, 0, sizeof(gle));
+				fce_len = smc_clc_fill_fce(&fce, ini);
+				len += fce_len;
+				fce.fce_v20.v2_direct = !link->lgr->uses_gateway;
 				if (clc->hdr.type == SMC_CLC_CONFIRM) {
+					memset(&gle, 0, sizeof(gle));
 					gle.gid_cnt = ini->smcrv2.gidlist.len;
 					len += sizeof(gle);
 					len += gle.gid_cnt * sizeof(gle.gid[0]);
-				} else {
-					len += sizeof(gle.reserved);
 				}
 			}
 			clc_v2->hdr.length = htons(len);
@@ -1094,7 +1099,7 @@ static int smc_clc_send_confirm_accept(struct smc_sock *smc,
 				   sizeof(trl);
 	if (version > SMC_V1 && first_contact) {
 		vec[i].iov_base = &fce;
-		vec[i++].iov_len = sizeof(fce);
+		vec[i++].iov_len = fce_len;
 		if (!conn->lgr->is_smcd) {
 			if (clc->hdr.type == SMC_CLC_CONFIRM) {
 				vec[i].iov_base = &gle;
@@ -1102,9 +1107,6 @@ static int smc_clc_send_confirm_accept(struct smc_sock *smc,
 				vec[i].iov_base = &ini->smcrv2.gidlist.list;
 				vec[i++].iov_len = gle.gid_cnt *
 						   sizeof(gle.gid[0]);
-			} else {
-				vec[i].iov_base = &gle.reserved;
-				vec[i++].iov_len = sizeof(gle.reserved);
 			}
 		}
 	}
diff --git a/net/smc/smc_clc.h b/net/smc/smc_clc.h
index b923e89acafb..6133276a8839 100644
--- a/net/smc/smc_clc.h
+++ b/net/smc/smc_clc.h
@@ -147,7 +147,9 @@ struct smc_clc_msg_proposal_prefix {	/* prefix part of clc proposal message*/
 struct smc_clc_msg_smcd {	/* SMC-D GID information */
 	struct smc_clc_smcd_gid_chid ism; /* ISM native GID+CHID of requestor */
 	__be16 v2_ext_offset;	/* SMC Version 2 Extension Offset */
-	u8 reserved[28];
+	u8 vendor_oui[3];
+	u8 vendor_exp_options[5];
+	u8 reserved[20];
 };
 
 struct smc_clc_smcd_v2_extension {
@@ -231,8 +233,17 @@ struct smc_clc_first_contact_ext {
 	u8 hostname[SMC_MAX_HOSTNAME_LEN];
 };
 
+struct smc_clc_first_contact_ext_v2x {
+	struct smc_clc_first_contact_ext fce_v20;
+	u8 reserved3[4];
+	__be32 vendor_exp_options;
+	u8 reserved4[8];
+} __packed;		/* format defined in
+			 * IBM Shared Memory Communications Version 2 (Third Edition)
+			 * (https://www.ibm.com/support/pages/node/7009315)
+			 */
+
 struct smc_clc_fce_gid_ext {
-	u8 reserved[16];
 	u8 gid_cnt;
 	u8 reserved2[3];
 	u8 gid[][SMC_GID_SIZE];
-- 
2.24.3 (Apple Git-128)

Re: [PATCH net-next 2/6] net/smc: add vendor unique experimental options area in clc handshake

[Jan Karcher]

Hi Guangguan Wang,

thank you, some minor thoughts on this one.

On 16/08/2023 10:33, Guangguan Wang wrote:
> Add vendor unique experimental options area in clc handshake. In clc
> accept and confirm msg, vendor unique experimental options use the
> 16-Bytes reserved field, which defined in struct smc_clc_fce_gid_ext
> in previous version. Because of the struct smc_clc_first_contact_ext
> is widely used and limit the scope of modification, this patch moves
> the 16-Bytes reserved field out of struct smc_clc_fce_gid_ext, and
> followed with the struct smc_clc_first_contact_ext in a new struct
> names struct smc_clc_first_contact_ext_v2x.
> 
> For SMC-R first connection, in previous version, the struct smc_clc_
> first_contact_ext and the 16-Bytes reserved field has already been
> included in clc accept and confirm msg. Thus, this patch use struct
> smc_clc_first_contact_ext_v2x instead of the struct smc_clc_first_
> contact_ext and the 16-Bytes reserved field in SMC-R clc accept and
> confirm msg is compatible with previous version.
> 
> For SMC-D first connection, in previous version, only the struct smc_
> clc_first_contact_ext is included in clc accept and confirm msg, and
> the 16-Bytes reserved field is not included. Thus, when the negotiated
> smc release version is the version before v2.1, we still use struct
> smc_clc_first_contact_ext for compatible consideration. If the negotiated
> smc release version is v2.1 or later, use struct smc_clc_first_contact_
> ext_v2x instead.
> 
> Signed-off-by: Guangguan Wang <guangguan.wang@linux.alibaba.com>
> Reviewed-by: Tony Lu <tonylu@linux.alibaba.com>
> ---
>   net/smc/af_smc.c  |  2 +-
>   net/smc/smc_clc.c | 44 +++++++++++++++++++++++---------------------
>   net/smc/smc_clc.h | 15 +++++++++++++--
>   3 files changed, 37 insertions(+), 24 deletions(-)
> 
> diff --git a/net/smc/af_smc.c b/net/smc/af_smc.c
> index 97265691bc95..7b54c153bd0d 100644
> --- a/net/smc/af_smc.c
> +++ b/net/smc/af_smc.c
> @@ -1113,7 +1113,7 @@ static int smc_connect_ism_vlan_cleanup(struct smc_sock *smc,
>   
>   #define SMC_CLC_MAX_ACCEPT_LEN \
>   	(sizeof(struct smc_clc_msg_accept_confirm_v2) + \
> -	 sizeof(struct smc_clc_first_contact_ext) + \
> +	 sizeof(struct smc_clc_first_contact_ext_v2x) + \
>   	 sizeof(struct smc_clc_msg_trail))
>   
>   /* CLC handshake during connect */
> diff --git a/net/smc/smc_clc.c b/net/smc/smc_clc.c
> index 7c5627c6abcc..624dc970d187 100644
> --- a/net/smc/smc_clc.c
> +++ b/net/smc/smc_clc.c
> @@ -391,9 +391,7 @@ smc_clc_msg_acc_conf_valid(struct smc_clc_msg_accept_confirm_v2 *clc_v2)
>   			return false;
>   	} else {
>   		if (hdr->typev1 == SMC_TYPE_D &&
> -		    ntohs(hdr->length) != SMCD_CLC_ACCEPT_CONFIRM_LEN_V2 &&
> -		    (ntohs(hdr->length) != SMCD_CLC_ACCEPT_CONFIRM_LEN_V2 +
> -				sizeof(struct smc_clc_first_contact_ext)))
> +		    ntohs(hdr->length) < SMCD_CLC_ACCEPT_CONFIRM_LEN_V2)
>   			return false;
>   		if (hdr->typev1 == SMC_TYPE_R &&
>   		    ntohs(hdr->length) < SMCR_CLC_ACCEPT_CONFIRM_LEN_V2)
> @@ -420,13 +418,19 @@ smc_clc_msg_decl_valid(struct smc_clc_msg_decline *dclc)
>   	return true;
>   }
>   
> -static void smc_clc_fill_fce(struct smc_clc_first_contact_ext *fce, int *len, int release_nr)
> +static int smc_clc_fill_fce(struct smc_clc_first_contact_ext_v2x *fce,
> +			    struct smc_init_info *ini)
>   {
> +	int ret = sizeof(*fce);
> +
>   	memset(fce, 0, sizeof(*fce));
> -	fce->os_type = SMC_CLC_OS_LINUX;
> -	fce->release = release_nr;
> -	memcpy(fce->hostname, smc_hostname, sizeof(smc_hostname));
> -	(*len) += sizeof(*fce);
> +	fce->fce_v20.os_type = SMC_CLC_OS_LINUX;
> +	fce->fce_v20.release = ini->release_nr;

I don't like that this is called fce_v20.release which can be set to 
v2.1 here although the struct is named v20. Maybe let us call the struct 
something like fce_v2_base or fce_base_v2.

> +	memcpy(fce->fce_v20.hostname, smc_hostname, sizeof(smc_hostname));
> +	if (ini->is_smcd && ini->release_nr < SMC_RELEASE_1)
> +		ret = sizeof(struct smc_clc_first_contact_ext);
> +
> +	return ret;
>   }
>   
>   /* check if received message has a correct header length and contains valid
> @@ -986,13 +990,13 @@ static int smc_clc_send_confirm_accept(struct smc_sock *smc,
>   				       u8 *eid, struct smc_init_info *ini)
>   {
>   	struct smc_connection *conn = &smc->conn;
> +	struct smc_clc_first_contact_ext_v2x fce;
>   	struct smc_clc_msg_accept_confirm *clc;
> -	struct smc_clc_first_contact_ext fce;
>   	struct smc_clc_fce_gid_ext gle;
>   	struct smc_clc_msg_trail trl;
> +	int i, len, fce_len;
>   	struct kvec vec[5];
>   	struct msghdr msg;
> -	int i, len;
>   
>   	/* send SMC Confirm CLC msg */
>   	clc = (struct smc_clc_msg_accept_confirm *)clc_v2;
> @@ -1018,8 +1022,10 @@ static int smc_clc_send_confirm_accept(struct smc_sock *smc,
>   			if (eid && eid[0])
>   				memcpy(clc_v2->d1.eid, eid, SMC_MAX_EID_LEN);
>   			len = SMCD_CLC_ACCEPT_CONFIRM_LEN_V2;
> -			if (first_contact)
> -				smc_clc_fill_fce(&fce, &len, ini->release_nr);
> +			if (first_contact) {
> +				fce_len = smc_clc_fill_fce(&fce, ini);
> +				len += fce_len;
> +			}
>   			clc_v2->hdr.length = htons(len);
>   		}
>   		memcpy(trl.eyecatcher, SMCD_EYECATCHER,
> @@ -1063,15 +1069,14 @@ static int smc_clc_send_confirm_accept(struct smc_sock *smc,
>   				memcpy(clc_v2->r1.eid, eid, SMC_MAX_EID_LEN);
>   			len = SMCR_CLC_ACCEPT_CONFIRM_LEN_V2;
>   			if (first_contact) {
> -				smc_clc_fill_fce(&fce, &len, ini->release_nr);
> -				fce.v2_direct = !link->lgr->uses_gateway;
> -				memset(&gle, 0, sizeof(gle));
> +				fce_len = smc_clc_fill_fce(&fce, ini);
> +				len += fce_len;
> +				fce.fce_v20.v2_direct = !link->lgr->uses_gateway;
>   				if (clc->hdr.type == SMC_CLC_CONFIRM) {
> +					memset(&gle, 0, sizeof(gle));
>   					gle.gid_cnt = ini->smcrv2.gidlist.len;
>   					len += sizeof(gle);
>   					len += gle.gid_cnt * sizeof(gle.gid[0]);
> -				} else {
> -					len += sizeof(gle.reserved);
>   				}
>   			}
>   			clc_v2->hdr.length = htons(len);
> @@ -1094,7 +1099,7 @@ static int smc_clc_send_confirm_accept(struct smc_sock *smc,
>   				   sizeof(trl);
>   	if (version > SMC_V1 && first_contact) {
>   		vec[i].iov_base = &fce;
> -		vec[i++].iov_len = sizeof(fce);
> +		vec[i++].iov_len = fce_len;
>   		if (!conn->lgr->is_smcd) {
>   			if (clc->hdr.type == SMC_CLC_CONFIRM) {
>   				vec[i].iov_base = &gle;
> @@ -1102,9 +1107,6 @@ static int smc_clc_send_confirm_accept(struct smc_sock *smc,
>   				vec[i].iov_base = &ini->smcrv2.gidlist.list;
>   				vec[i++].iov_len = gle.gid_cnt *
>   						   sizeof(gle.gid[0]);
> -			} else {
> -				vec[i].iov_base = &gle.reserved;
> -				vec[i++].iov_len = sizeof(gle.reserved);
>   			}
>   		}
>   	}
> diff --git a/net/smc/smc_clc.h b/net/smc/smc_clc.h
> index b923e89acafb..6133276a8839 100644
> --- a/net/smc/smc_clc.h
> +++ b/net/smc/smc_clc.h
> @@ -147,7 +147,9 @@ struct smc_clc_msg_proposal_prefix {	/* prefix part of clc proposal message*/
>   struct smc_clc_msg_smcd {	/* SMC-D GID information */
>   	struct smc_clc_smcd_gid_chid ism; /* ISM native GID+CHID of requestor */
>   	__be16 v2_ext_offset;	/* SMC Version 2 Extension Offset */
> -	u8 reserved[28];
> +	u8 vendor_oui[3];
> +	u8 vendor_exp_options[5];
> +	u8 reserved[20];

Could we either make those variables a bit more self explaining via 
their name (e.g. vendor_organization_uid) or adding a comment /* vendor 
organizationally unique identifier */

>   };
>   
>   struct smc_clc_smcd_v2_extension {
> @@ -231,8 +233,17 @@ struct smc_clc_first_contact_ext {
>   	u8 hostname[SMC_MAX_HOSTNAME_LEN];
>   };
>   
> +struct smc_clc_first_contact_ext_v2x {
> +	struct smc_clc_first_contact_ext fce_v20;

as stated at the top where the release is assigned i'm not completly 
happy with the naming.

> +	u8 reserved3[4];
> +	__be32 vendor_exp_options;
> +	u8 reserved4[8];
> +} __packed;		/* format defined in
> +			 * IBM Shared Memory Communications Version 2 (Third Edition)
> +			 * (https://www.ibm.com/support/pages/node/7009315)
> +			 */
> +
>   struct smc_clc_fce_gid_ext {
> -	u8 reserved[16];
>   	u8 gid_cnt;
>   	u8 reserved2[3];
>   	u8 gid[][SMC_GID_SIZE];

Re: [PATCH net-next 2/6] net/smc: add vendor unique experimental options area in clc handshake

[Guangguan Wang]

On 2023/8/17 05:49, Jan Karcher wrote:
> Hi Guangguan Wang,
> 
> thank you, some minor thoughts on this one.
> 
> On 16/08/2023 10:33, Guangguan Wang wrote:
...
>>   -static void smc_clc_fill_fce(struct smc_clc_first_contact_ext *fce, int *len, int release_nr)
>> +static int smc_clc_fill_fce(struct smc_clc_first_contact_ext_v2x *fce,
>> +                struct smc_init_info *ini)
>>   {
>> +    int ret = sizeof(*fce);
>> +
>>       memset(fce, 0, sizeof(*fce));
>> -    fce->os_type = SMC_CLC_OS_LINUX;
>> -    fce->release = release_nr;
>> -    memcpy(fce->hostname, smc_hostname, sizeof(smc_hostname));
>> -    (*len) += sizeof(*fce);
>> +    fce->fce_v20.os_type = SMC_CLC_OS_LINUX;
>> +    fce->fce_v20.release = ini->release_nr;
> 
> I don't like that this is called fce_v20.release which can be set to v2.1 here although the struct is named v20. Maybe let us call the struct something like fce_v2_base or fce_base_v2.
> 

fce_v2_base sounds better.


>> diff --git a/net/smc/smc_clc.h b/net/smc/smc_clc.h
>> index b923e89acafb..6133276a8839 100644
>> --- a/net/smc/smc_clc.h
>> +++ b/net/smc/smc_clc.h
>> @@ -147,7 +147,9 @@ struct smc_clc_msg_proposal_prefix {    /* prefix part of clc proposal message*/
>>   struct smc_clc_msg_smcd {    /* SMC-D GID information */
>>       struct smc_clc_smcd_gid_chid ism; /* ISM native GID+CHID of requestor */
>>       __be16 v2_ext_offset;    /* SMC Version 2 Extension Offset */
>> -    u8 reserved[28];
>> +    u8 vendor_oui[3];
>> +    u8 vendor_exp_options[5];
>> +    u8 reserved[20];
> 
> Could we either make those variables a bit more self explaining via their name (e.g. vendor_organization_uid) or adding a comment /* vendor organizationally unique identifier */
> 

I will fix it in the next version.

>>   };
>>     struct smc_clc_smcd_v2_extension {
>> @@ -231,8 +233,17 @@ struct smc_clc_first_contact_ext {
>>       u8 hostname[SMC_MAX_HOSTNAME_LEN];
>>   };
>>   +struct smc_clc_first_contact_ext_v2x {
>> +    struct smc_clc_first_contact_ext fce_v20;
> 
> as stated at the top where the release is assigned i'm not completly happy with the naming.
> 

Thanks,
Guangguan Wang

[PATCH net-next 3/6] net/smc: support smc v2.x features validate

[Guangguan Wang]

Support SMC v2.x features validate for SMC v2.1. This is the frame
code for SMC v2.x features validate, and will take effects only when
the negotiated release version is v2.1 or later.

For Server, v2.x features' validation should be done in smc_clc_srv_
v2x_features_validate when receiving v2.1 or later CLC Proposal Message,
such as max conns, max links negotiation, the decision of the final
value of max conns and max links should be made in this function.
And final check for server when receiving v2.1 or later CLC Confirm
Message should be done in smc_clc_v2x_features_confirm_check.

For client, v2.x features' validation should be done in smc_clc_cli_
v2x_features_validate when receiving v2.1 or later CLC Accept Message,
for example, the decision to accpt the accepted value or to decline
should be made in this function.

Signed-off-by: Guangguan Wang <guangguan.wang@linux.alibaba.com>
Reviewed-by: Tony Lu <tonylu@linux.alibaba.com>
---
 net/smc/af_smc.c  | 18 ++++++++++++++++++
 net/smc/smc_clc.c | 46 ++++++++++++++++++++++++++++++++++++++++++++++
 net/smc/smc_clc.h |  7 +++++++
 3 files changed, 71 insertions(+)

diff --git a/net/smc/af_smc.c b/net/smc/af_smc.c
index 7b54c153bd0d..65c02b48331f 100644
--- a/net/smc/af_smc.c
+++ b/net/smc/af_smc.c
@@ -1169,6 +1169,7 @@ static int smc_connect_rdma_v2_prepare(struct smc_sock *smc,
 	struct smc_clc_first_contact_ext *fce =
 		(struct smc_clc_first_contact_ext *)
 			(((u8 *)clc_v2) + sizeof(*clc_v2));
+	int rc;
 
 	if (!ini->first_contact_peer || aclc->hdr.version == SMC_V1)
 		return 0;
@@ -1189,6 +1190,9 @@ static int smc_connect_rdma_v2_prepare(struct smc_sock *smc,
 	}
 
 	ini->release_nr = fce->release;
+	rc = smc_clc_cli_v2x_features_validate(fce, ini);
+	if (rc)
+		return rc;
 
 	return 0;
 }
@@ -1363,6 +1367,9 @@ static int smc_connect_ism(struct smc_sock *smc,
 				smc_get_clc_first_contact_ext(aclc_v2, true);
 
 			ini->release_nr = fce->release;
+			rc = smc_clc_cli_v2x_features_validate(fce, ini);
+			if (rc)
+				return rc;
 		}
 
 		rc = smc_v2_determine_accepted_chid(aclc_v2, ini);
@@ -2413,6 +2420,10 @@ static void smc_listen_work(struct work_struct *work)
 	if (rc)
 		goto out_decl;
 
+	rc = smc_clc_srv_v2x_features_validate(pclc, ini);
+	if (rc)
+		goto out_decl;
+
 	mutex_lock(&smc_server_lgr_pending);
 	smc_close_init(new_smc);
 	smc_rx_init(new_smc);
@@ -2445,6 +2456,13 @@ static void smc_listen_work(struct work_struct *work)
 		goto out_decl;
 	}
 
+	rc = smc_clc_v2x_features_confirm_check(cclc, ini);
+	if (rc) {
+		if (!ini->is_smcd)
+			goto out_unlock;
+		goto out_decl;
+	}
+
 	/* finish worker */
 	if (!ini->is_smcd) {
 		rc = smc_listen_rdma_finish(new_smc, cclc,
diff --git a/net/smc/smc_clc.c b/net/smc/smc_clc.c
index 624dc970d187..f71b22e50be5 100644
--- a/net/smc/smc_clc.c
+++ b/net/smc/smc_clc.c
@@ -1158,6 +1158,52 @@ int smc_clc_send_accept(struct smc_sock *new_smc, bool srv_first_contact,
 	return len > 0 ? 0 : len;
 }
 
+int smc_clc_srv_v2x_features_validate(struct smc_clc_msg_proposal *pclc,
+				      struct smc_init_info *ini)
+{
+	struct smc_clc_v2_extension *pclc_v2_ext;
+
+	if ((!(ini->smcd_version & SMC_V2) && !(ini->smcr_version & SMC_V2)) ||
+	    ini->release_nr < SMC_RELEASE_1)
+		return 0;
+
+	pclc_v2_ext = smc_get_clc_v2_ext(pclc);
+	if (!pclc_v2_ext)
+		return SMC_CLC_DECL_NOV2EXT;
+
+	return 0;
+}
+
+int smc_clc_cli_v2x_features_validate(struct smc_clc_first_contact_ext *fce,
+				      struct smc_init_info *ini)
+{
+	if (ini->release_nr < SMC_RELEASE_1)
+		return 0;
+
+	return 0;
+}
+
+int smc_clc_v2x_features_confirm_check(struct smc_clc_msg_accept_confirm *cclc,
+				       struct smc_init_info *ini)
+{
+	struct smc_clc_msg_accept_confirm_v2 *clc_v2 =
+		(struct smc_clc_msg_accept_confirm_v2 *)cclc;
+	struct smc_clc_first_contact_ext *fce =
+		smc_get_clc_first_contact_ext(clc_v2, ini->is_smcd);
+
+	if (cclc->hdr.version == SMC_V1 ||
+	    !(cclc->hdr.typev2 & SMC_FIRST_CONTACT_MASK))
+		return 0;
+
+	if (ini->release_nr != fce->release)
+		return SMC_CLC_DECL_RELEASEERR;
+
+	if (fce->release < SMC_RELEASE_1)
+		return 0;
+
+	return 0;
+}
+
 void smc_clc_get_hostname(u8 **host)
 {
 	*host = &smc_hostname[0];
diff --git a/net/smc/smc_clc.h b/net/smc/smc_clc.h
index 6133276a8839..66932bfdc6d0 100644
--- a/net/smc/smc_clc.h
+++ b/net/smc/smc_clc.h
@@ -45,6 +45,7 @@
 #define SMC_CLC_DECL_NOSEID	0x03030006  /* peer sent no SEID	      */
 #define SMC_CLC_DECL_NOSMCD2DEV	0x03030007  /* no SMC-Dv2 device found	      */
 #define SMC_CLC_DECL_NOUEID	0x03030008  /* peer sent no UEID	      */
+#define SMC_CLC_DECL_RELEASEERR	0x03030009  /* release version negotiate failed */
 #define SMC_CLC_DECL_MODEUNSUPP	0x03040000  /* smc modes do not match (R or D)*/
 #define SMC_CLC_DECL_RMBE_EC	0x03050000  /* peer has eyecatcher in RMBE    */
 #define SMC_CLC_DECL_OPTUNSUPP	0x03060000  /* fastopen sockopt not supported */
@@ -415,6 +416,12 @@ int smc_clc_send_confirm(struct smc_sock *smc, bool clnt_first_contact,
 			 u8 version, u8 *eid, struct smc_init_info *ini);
 int smc_clc_send_accept(struct smc_sock *smc, bool srv_first_contact,
 			u8 version, u8 *negotiated_eid, struct smc_init_info *ini);
+int smc_clc_srv_v2x_features_validate(struct smc_clc_msg_proposal *pclc,
+				      struct smc_init_info *ini);
+int smc_clc_cli_v2x_features_validate(struct smc_clc_first_contact_ext *fce,
+				      struct smc_init_info *ini);
+int smc_clc_v2x_features_confirm_check(struct smc_clc_msg_accept_confirm *cclc,
+				       struct smc_init_info *ini);
 void smc_clc_init(void) __init;
 void smc_clc_exit(void);
 void smc_clc_get_hostname(u8 **host);
-- 
2.24.3 (Apple Git-128)

Re: [PATCH net-next 3/6] net/smc: support smc v2.x features validate

[Vadim Fedorenko]

On 16/08/2023 09:33, Guangguan Wang wrote:
> Support SMC v2.x features validate for SMC v2.1. This is the frame
> code for SMC v2.x features validate, and will take effects only when
> the negotiated release version is v2.1 or later.
> 
> For Server, v2.x features' validation should be done in smc_clc_srv_
> v2x_features_validate when receiving v2.1 or later CLC Proposal Message,
> such as max conns, max links negotiation, the decision of the final
> value of max conns and max links should be made in this function.
> And final check for server when receiving v2.1 or later CLC Confirm
> Message should be done in smc_clc_v2x_features_confirm_check.
> 
> For client, v2.x features' validation should be done in smc_clc_cli_
> v2x_features_validate when receiving v2.1 or later CLC Accept Message,
> for example, the decision to accpt the accepted value or to decline
> should be made in this function.
> 
> Signed-off-by: Guangguan Wang <guangguan.wang@linux.alibaba.com>
> Reviewed-by: Tony Lu <tonylu@linux.alibaba.com>
> ---
>   net/smc/af_smc.c  | 18 ++++++++++++++++++
>   net/smc/smc_clc.c | 46 ++++++++++++++++++++++++++++++++++++++++++++++
>   net/smc/smc_clc.h |  7 +++++++
>   3 files changed, 71 insertions(+)
> 
> diff --git a/net/smc/af_smc.c b/net/smc/af_smc.c
> index 7b54c153bd0d..65c02b48331f 100644
> --- a/net/smc/af_smc.c
> +++ b/net/smc/af_smc.c
> @@ -1169,6 +1169,7 @@ static int smc_connect_rdma_v2_prepare(struct smc_sock *smc,
>   	struct smc_clc_first_contact_ext *fce =
>   		(struct smc_clc_first_contact_ext *)
>   			(((u8 *)clc_v2) + sizeof(*clc_v2));
> +	int rc;
>   
>   	if (!ini->first_contact_peer || aclc->hdr.version == SMC_V1)
>   		return 0;
> @@ -1189,6 +1190,9 @@ static int smc_connect_rdma_v2_prepare(struct smc_sock *smc,
>   	}
>   
>   	ini->release_nr = fce->release;
> +	rc = smc_clc_cli_v2x_features_validate(fce, ini);
> +	if (rc)
> +		return rc;
>   
>   	return 0;
>   }
> @@ -1363,6 +1367,9 @@ static int smc_connect_ism(struct smc_sock *smc,
>   				smc_get_clc_first_contact_ext(aclc_v2, true);
>   
>   			ini->release_nr = fce->release;
> +			rc = smc_clc_cli_v2x_features_validate(fce, ini);
> +			if (rc)
> +				return rc;
>   		}
>   
>   		rc = smc_v2_determine_accepted_chid(aclc_v2, ini);
> @@ -2413,6 +2420,10 @@ static void smc_listen_work(struct work_struct *work)
>   	if (rc)
>   		goto out_decl;
>   
> +	rc = smc_clc_srv_v2x_features_validate(pclc, ini);
> +	if (rc)
> +		goto out_decl;
> +
>   	mutex_lock(&smc_server_lgr_pending);
>   	smc_close_init(new_smc);
>   	smc_rx_init(new_smc);
> @@ -2445,6 +2456,13 @@ static void smc_listen_work(struct work_struct *work)
>   		goto out_decl;
>   	}
>   
> +	rc = smc_clc_v2x_features_confirm_check(cclc, ini);
> +	if (rc) {
> +		if (!ini->is_smcd)
> +			goto out_unlock;
> +		goto out_decl;
> +	}
> +
>   	/* finish worker */
>   	if (!ini->is_smcd) {
>   		rc = smc_listen_rdma_finish(new_smc, cclc,
> diff --git a/net/smc/smc_clc.c b/net/smc/smc_clc.c
> index 624dc970d187..f71b22e50be5 100644
> --- a/net/smc/smc_clc.c
> +++ b/net/smc/smc_clc.c
> @@ -1158,6 +1158,52 @@ int smc_clc_send_accept(struct smc_sock *new_smc, bool srv_first_contact,
>   	return len > 0 ? 0 : len;
>   }
>   
> +int smc_clc_srv_v2x_features_validate(struct smc_clc_msg_proposal *pclc,
> +				      struct smc_init_info *ini)
> +{
> +	struct smc_clc_v2_extension *pclc_v2_ext;
> +
> +	if ((!(ini->smcd_version & SMC_V2) && !(ini->smcr_version & SMC_V2)) ||
> +	    ini->release_nr < SMC_RELEASE_1)
> +		return 0;
> +
> +	pclc_v2_ext = smc_get_clc_v2_ext(pclc);
> +	if (!pclc_v2_ext)
> +		return SMC_CLC_DECL_NOV2EXT;
> +
> +	return 0;
> +}
> +
> +int smc_clc_cli_v2x_features_validate(struct smc_clc_first_contact_ext *fce,
> +				      struct smc_init_info *ini)
> +{
> +	if (ini->release_nr < SMC_RELEASE_1)
> +		return 0;
> +
> +	return 0;
> +}

This function always returns 0. Is it really what expected?

> +
> +int smc_clc_v2x_features_confirm_check(struct smc_clc_msg_accept_confirm *cclc,
> +				       struct smc_init_info *ini)
> +{
> +	struct smc_clc_msg_accept_confirm_v2 *clc_v2 =
> +		(struct smc_clc_msg_accept_confirm_v2 *)cclc;
> +	struct smc_clc_first_contact_ext *fce =
> +		smc_get_clc_first_contact_ext(clc_v2, ini->is_smcd);
> +
> +	if (cclc->hdr.version == SMC_V1 ||
> +	    !(cclc->hdr.typev2 & SMC_FIRST_CONTACT_MASK))
> +		return 0;
> +
> +	if (ini->release_nr != fce->release)
> +		return SMC_CLC_DECL_RELEASEERR;
> +
> +	if (fce->release < SMC_RELEASE_1)
> +		return 0;
> +
> +	return 0;
> +}
> +
>   void smc_clc_get_hostname(u8 **host)
>   {
>   	*host = &smc_hostname[0];
> diff --git a/net/smc/smc_clc.h b/net/smc/smc_clc.h
> index 6133276a8839..66932bfdc6d0 100644
> --- a/net/smc/smc_clc.h
> +++ b/net/smc/smc_clc.h
> @@ -45,6 +45,7 @@
>   #define SMC_CLC_DECL_NOSEID	0x03030006  /* peer sent no SEID	      */
>   #define SMC_CLC_DECL_NOSMCD2DEV	0x03030007  /* no SMC-Dv2 device found	      */
>   #define SMC_CLC_DECL_NOUEID	0x03030008  /* peer sent no UEID	      */
> +#define SMC_CLC_DECL_RELEASEERR	0x03030009  /* release version negotiate failed */
>   #define SMC_CLC_DECL_MODEUNSUPP	0x03040000  /* smc modes do not match (R or D)*/
>   #define SMC_CLC_DECL_RMBE_EC	0x03050000  /* peer has eyecatcher in RMBE    */
>   #define SMC_CLC_DECL_OPTUNSUPP	0x03060000  /* fastopen sockopt not supported */
> @@ -415,6 +416,12 @@ int smc_clc_send_confirm(struct smc_sock *smc, bool clnt_first_contact,
>   			 u8 version, u8 *eid, struct smc_init_info *ini);
>   int smc_clc_send_accept(struct smc_sock *smc, bool srv_first_contact,
>   			u8 version, u8 *negotiated_eid, struct smc_init_info *ini);
> +int smc_clc_srv_v2x_features_validate(struct smc_clc_msg_proposal *pclc,
> +				      struct smc_init_info *ini);
> +int smc_clc_cli_v2x_features_validate(struct smc_clc_first_contact_ext *fce,
> +				      struct smc_init_info *ini);
> +int smc_clc_v2x_features_confirm_check(struct smc_clc_msg_accept_confirm *cclc,
> +				       struct smc_init_info *ini);
>   void smc_clc_init(void) __init;
>   void smc_clc_exit(void);
>   void smc_clc_get_hostname(u8 **host);

Re: [PATCH net-next 3/6] net/smc: support smc v2.x features validate

[Guangguan Wang]

On 2023/8/16 20:49, Vadim Fedorenko wrote:
> On 16/08/2023 09:33, Guangguan Wang wrote:

>> +
>> +int smc_clc_cli_v2x_features_validate(struct smc_clc_first_contact_ext *fce,
>> +                      struct smc_init_info *ini)
>> +{
>> +    if (ini->release_nr < SMC_RELEASE_1)
>> +        return 0;
>> +
>> +    return 0;
>> +}
> 
> This function always returns 0. Is it really what expected?
> 

This patch is a frame code of v2x features validate.
Please read the next 2 patches, where will fill more code logic in this function.


[PATCH net-next 4/6] net/smc: support max connections per lgr negotiation
 int smc_clc_cli_v2x_features_validate(struct smc_clc_first_contact_ext *fce,
 				      struct smc_init_info *ini)
 {
+	struct smc_clc_first_contact_ext_v2x *fce_v2x =
+		(struct smc_clc_first_contact_ext_v2x *)fce;
+
 	if (ini->release_nr < SMC_RELEASE_1)
 		return 0;
 
+	if (!ini->is_smcd) {
+		if (fce_v2x->max_conns < SMC_CONN_PER_LGR_MIN)
+			return SMC_CLC_DECL_MAXCONNERR;
+		ini->max_conns = fce_v2x->max_conns;
+	}
+
 	return 0;
 }


[PATCH net-next 5/6] net/smc: support max links per lgr negotiation in clc handshake
@@ -1208,6 +1216,11 @@ int smc_clc_cli_v2x_features_validate(struct smc_clc_first_contact_ext *fce,
 		if (fce_v2x->max_conns < SMC_CONN_PER_LGR_MIN)
 			return SMC_CLC_DECL_MAXCONNERR;
 		ini->max_conns = fce_v2x->max_conns;
+
+		if (fce_v2x->max_links > SMC_LINKS_ADD_LNK_MAX ||
+		    fce_v2x->max_links < SMC_LINKS_ADD_LNK_MIN)
+			return SMC_CLC_DECL_MAXLINKERR;
+		ini->max_links = fce_v2x->max_links;
 	}
 
 	return 0;


Thanks,
Guangguan Wang

Re: [PATCH net-next 3/6] net/smc: support smc v2.x features validate

[Jan Karcher]

Hi Guangguan Wang,

minor renaming.

On 16/08/2023 10:33, Guangguan Wang wrote:
> Support SMC v2.x features validate for SMC v2.1. This is the frame
> code for SMC v2.x features validate, and will take effects only when
> the negotiated release version is v2.1 or later.
> 
> For Server, v2.x features' validation should be done in smc_clc_srv_
> v2x_features_validate when receiving v2.1 or later CLC Proposal Message,
> such as max conns, max links negotiation, the decision of the final
> value of max conns and max links should be made in this function.
> And final check for server when receiving v2.1 or later CLC Confirm
> Message should be done in smc_clc_v2x_features_confirm_check.
> 
> For client, v2.x features' validation should be done in smc_clc_cli_
> v2x_features_validate when receiving v2.1 or later CLC Accept Message,

please use either clnt or client for the function. I know we have some 
functions with cli in them but they need to be cleaned up down the road.

Thank you.
- Jan

> for example, the decision to accpt the accepted value or to decline
> should be made in this function.
> 
> Signed-off-by: Guangguan Wang <guangguan.wang@linux.alibaba.com>
> Reviewed-by: Tony Lu <tonylu@linux.alibaba.com>
> ---
>   net/smc/af_smc.c  | 18 ++++++++++++++++++
>   net/smc/smc_clc.c | 46 ++++++++++++++++++++++++++++++++++++++++++++++
>   net/smc/smc_clc.h |  7 +++++++
>   3 files changed, 71 insertions(+)
> 
> diff --git a/net/smc/af_smc.c b/net/smc/af_smc.c
> index 7b54c153bd0d..65c02b48331f 100644
> --- a/net/smc/af_smc.c
> +++ b/net/smc/af_smc.c
> @@ -1169,6 +1169,7 @@ static int smc_connect_rdma_v2_prepare(struct smc_sock *smc,
>   	struct smc_clc_first_contact_ext *fce =
>   		(struct smc_clc_first_contact_ext *)
>   			(((u8 *)clc_v2) + sizeof(*clc_v2));
> +	int rc;
>   
>   	if (!ini->first_contact_peer || aclc->hdr.version == SMC_V1)
>   		return 0;
> @@ -1189,6 +1190,9 @@ static int smc_connect_rdma_v2_prepare(struct smc_sock *smc,
>   	}
>   
>   	ini->release_nr = fce->release;
> +	rc = smc_clc_cli_v2x_features_validate(fce, ini);
> +	if (rc)
> +		return rc;
>   
>   	return 0;
>   }
> @@ -1363,6 +1367,9 @@ static int smc_connect_ism(struct smc_sock *smc,
>   				smc_get_clc_first_contact_ext(aclc_v2, true);
>   
>   			ini->release_nr = fce->release;
> +			rc = smc_clc_cli_v2x_features_validate(fce, ini);
> +			if (rc)
> +				return rc;
>   		}
>   
>   		rc = smc_v2_determine_accepted_chid(aclc_v2, ini);
> @@ -2413,6 +2420,10 @@ static void smc_listen_work(struct work_struct *work)
>   	if (rc)
>   		goto out_decl;
>   
> +	rc = smc_clc_srv_v2x_features_validate(pclc, ini);
> +	if (rc)
> +		goto out_decl;
> +
>   	mutex_lock(&smc_server_lgr_pending);
>   	smc_close_init(new_smc);
>   	smc_rx_init(new_smc);
> @@ -2445,6 +2456,13 @@ static void smc_listen_work(struct work_struct *work)
>   		goto out_decl;
>   	}
>   
> +	rc = smc_clc_v2x_features_confirm_check(cclc, ini);
> +	if (rc) {
> +		if (!ini->is_smcd)
> +			goto out_unlock;
> +		goto out_decl;
> +	}
> +
>   	/* finish worker */
>   	if (!ini->is_smcd) {
>   		rc = smc_listen_rdma_finish(new_smc, cclc,
> diff --git a/net/smc/smc_clc.c b/net/smc/smc_clc.c
> index 624dc970d187..f71b22e50be5 100644
> --- a/net/smc/smc_clc.c
> +++ b/net/smc/smc_clc.c
> @@ -1158,6 +1158,52 @@ int smc_clc_send_accept(struct smc_sock *new_smc, bool srv_first_contact,
>   	return len > 0 ? 0 : len;
>   }
>   
> +int smc_clc_srv_v2x_features_validate(struct smc_clc_msg_proposal *pclc,
> +				      struct smc_init_info *ini)
> +{
> +	struct smc_clc_v2_extension *pclc_v2_ext;
> +
> +	if ((!(ini->smcd_version & SMC_V2) && !(ini->smcr_version & SMC_V2)) ||
> +	    ini->release_nr < SMC_RELEASE_1)
> +		return 0;
> +
> +	pclc_v2_ext = smc_get_clc_v2_ext(pclc);
> +	if (!pclc_v2_ext)
> +		return SMC_CLC_DECL_NOV2EXT;
> +
> +	return 0;
> +}
> +
> +int smc_clc_cli_v2x_features_validate(struct smc_clc_first_contact_ext *fce,
> +				      struct smc_init_info *ini)
> +{
> +	if (ini->release_nr < SMC_RELEASE_1)
> +		return 0;
> +
> +	return 0;
> +}
> +
> +int smc_clc_v2x_features_confirm_check(struct smc_clc_msg_accept_confirm *cclc,
> +				       struct smc_init_info *ini)
> +{
> +	struct smc_clc_msg_accept_confirm_v2 *clc_v2 =
> +		(struct smc_clc_msg_accept_confirm_v2 *)cclc;
> +	struct smc_clc_first_contact_ext *fce =
> +		smc_get_clc_first_contact_ext(clc_v2, ini->is_smcd);
> +
> +	if (cclc->hdr.version == SMC_V1 ||
> +	    !(cclc->hdr.typev2 & SMC_FIRST_CONTACT_MASK))
> +		return 0;
> +
> +	if (ini->release_nr != fce->release)
> +		return SMC_CLC_DECL_RELEASEERR;
> +
> +	if (fce->release < SMC_RELEASE_1)
> +		return 0;
> +
> +	return 0;
> +}
> +
>   void smc_clc_get_hostname(u8 **host)
>   {
>   	*host = &smc_hostname[0];
> diff --git a/net/smc/smc_clc.h b/net/smc/smc_clc.h
> index 6133276a8839..66932bfdc6d0 100644
> --- a/net/smc/smc_clc.h
> +++ b/net/smc/smc_clc.h
> @@ -45,6 +45,7 @@
>   #define SMC_CLC_DECL_NOSEID	0x03030006  /* peer sent no SEID	      */
>   #define SMC_CLC_DECL_NOSMCD2DEV	0x03030007  /* no SMC-Dv2 device found	      */
>   #define SMC_CLC_DECL_NOUEID	0x03030008  /* peer sent no UEID	      */
> +#define SMC_CLC_DECL_RELEASEERR	0x03030009  /* release version negotiate failed */
>   #define SMC_CLC_DECL_MODEUNSUPP	0x03040000  /* smc modes do not match (R or D)*/
>   #define SMC_CLC_DECL_RMBE_EC	0x03050000  /* peer has eyecatcher in RMBE    */
>   #define SMC_CLC_DECL_OPTUNSUPP	0x03060000  /* fastopen sockopt not supported */
> @@ -415,6 +416,12 @@ int smc_clc_send_confirm(struct smc_sock *smc, bool clnt_first_contact,
>   			 u8 version, u8 *eid, struct smc_init_info *ini);
>   int smc_clc_send_accept(struct smc_sock *smc, bool srv_first_contact,
>   			u8 version, u8 *negotiated_eid, struct smc_init_info *ini);
> +int smc_clc_srv_v2x_features_validate(struct smc_clc_msg_proposal *pclc,
> +				      struct smc_init_info *ini);
> +int smc_clc_cli_v2x_features_validate(struct smc_clc_first_contact_ext *fce,
> +				      struct smc_init_info *ini);
> +int smc_clc_v2x_features_confirm_check(struct smc_clc_msg_accept_confirm *cclc,
> +				       struct smc_init_info *ini);
>   void smc_clc_init(void) __init;
>   void smc_clc_exit(void);
>   void smc_clc_get_hostname(u8 **host);

Re: [PATCH net-next 3/6] net/smc: support smc v2.x features validate

[Guangguan Wang]

On 2023/8/17 14:42, Jan Karcher wrote:
> Hi Guangguan Wang,
> 
> minor renaming.
> 
> On 16/08/2023 10:33, Guangguan Wang wrote:
>> Support SMC v2.x features validate for SMC v2.1. This is the frame
>> code for SMC v2.x features validate, and will take effects only when
>> the negotiated release version is v2.1 or later.
>>
>> For Server, v2.x features' validation should be done in smc_clc_srv_
>> v2x_features_validate when receiving v2.1 or later CLC Proposal Message,
>> such as max conns, max links negotiation, the decision of the final
>> value of max conns and max links should be made in this function.
>> And final check for server when receiving v2.1 or later CLC Confirm
>> Message should be done in smc_clc_v2x_features_confirm_check.
>>
>> For client, v2.x features' validation should be done in smc_clc_cli_
>> v2x_features_validate when receiving v2.1 or later CLC Accept Message,
> 
> please use either clnt or client for the function. I know we have some functions with cli in them but they need to be cleaned up down the road.
> 
> Thank you.
> - Jan
> 

Get it, Thanks.

[PATCH net-next 4/6] net/smc: support max connections per lgr negotiation

[Guangguan Wang]

Support max connections per lgr negotiation for SMCR v2.1,
which is one of smc v2.1 features. Server makes decision for
the final value of max conns based on the client preferred
max conns and self-preferred max conns. Here use the minimum
value of client preferred max conns and server preferred max
conns.

Client                                     Server
     Proposal(max conns(client preferred))
     ------------------------------------>

     Accept(max conns(accepted value))
accepted value=min(client preferred, server preferred)
     <-----------------------------------

     Confirm(max conns(accepted value))
     ----------------------------------->

Signed-off-by: Guangguan Wang <guangguan.wang@linux.alibaba.com>
Reviewed-by: Tony Lu <tonylu@linux.alibaba.com>
---
 net/smc/af_smc.c   |  1 +
 net/smc/smc_clc.c  | 41 +++++++++++++++++++++++++++++++++++++++--
 net/smc/smc_clc.h  |  7 +++++--
 net/smc/smc_core.c |  4 +++-
 net/smc/smc_core.h | 12 ++++++++++++
 net/smc/smc_llc.c  |  6 +++++-
 6 files changed, 65 insertions(+), 6 deletions(-)

diff --git a/net/smc/af_smc.c b/net/smc/af_smc.c
index 65c02b48331f..f7174135c74a 100644
--- a/net/smc/af_smc.c
+++ b/net/smc/af_smc.c
@@ -1212,6 +1212,7 @@ static int smc_connect_rdma(struct smc_sock *smc,
 	memcpy(ini->peer_systemid, aclc->r0.lcl.id_for_peer, SMC_SYSTEMID_LEN);
 	memcpy(ini->peer_gid, aclc->r0.lcl.gid, SMC_GID_SIZE);
 	memcpy(ini->peer_mac, aclc->r0.lcl.mac, ETH_ALEN);
+	ini->max_conns = SMC_CONN_PER_LGR_MAX;
 
 	reason_code = smc_connect_rdma_v2_prepare(smc, aclc, ini);
 	if (reason_code)
diff --git a/net/smc/smc_clc.c b/net/smc/smc_clc.c
index f71b22e50be5..6609b06f000f 100644
--- a/net/smc/smc_clc.c
+++ b/net/smc/smc_clc.c
@@ -427,9 +427,17 @@ static int smc_clc_fill_fce(struct smc_clc_first_contact_ext_v2x *fce,
 	fce->fce_v20.os_type = SMC_CLC_OS_LINUX;
 	fce->fce_v20.release = ini->release_nr;
 	memcpy(fce->fce_v20.hostname, smc_hostname, sizeof(smc_hostname));
-	if (ini->is_smcd && ini->release_nr < SMC_RELEASE_1)
+	if (ini->is_smcd && ini->release_nr < SMC_RELEASE_1) {
 		ret = sizeof(struct smc_clc_first_contact_ext);
+		goto out;
+	}
+
+	if (ini->release_nr >= SMC_RELEASE_1) {
+		if (!ini->is_smcd)
+			fce->max_conns = ini->max_conns;
+	}
 
+out:
 	return ret;
 }
 
@@ -931,8 +939,10 @@ int smc_clc_send_proposal(struct smc_sock *smc, struct smc_init_info *ini)
 				sizeof(struct smc_clc_smcd_gid_chid);
 		}
 	}
-	if (smcr_indicated(ini->smc_type_v2))
+	if (smcr_indicated(ini->smc_type_v2)) {
 		memcpy(v2_ext->roce, ini->smcrv2.ib_gid_v2, SMC_GID_SIZE);
+		v2_ext->max_conns = SMC_CONN_PER_LGR_PREFER;
+	}
 
 	pclc_base->hdr.length = htons(plen);
 	memcpy(trl->eyecatcher, SMC_EYECATCHER, sizeof(SMC_EYECATCHER));
@@ -1163,6 +1173,11 @@ int smc_clc_srv_v2x_features_validate(struct smc_clc_msg_proposal *pclc,
 {
 	struct smc_clc_v2_extension *pclc_v2_ext;
 
+	/* default max conn is SMC_CONN_PER_LGR_MAX(255),
+	 * which is the default value in smc v1 and v2.0.
+	 */
+	ini->max_conns = SMC_CONN_PER_LGR_MAX;
+
 	if ((!(ini->smcd_version & SMC_V2) && !(ini->smcr_version & SMC_V2)) ||
 	    ini->release_nr < SMC_RELEASE_1)
 		return 0;
@@ -1171,15 +1186,30 @@ int smc_clc_srv_v2x_features_validate(struct smc_clc_msg_proposal *pclc,
 	if (!pclc_v2_ext)
 		return SMC_CLC_DECL_NOV2EXT;
 
+	if (ini->smcr_version & SMC_V2) {
+		ini->max_conns = min_t(u8, pclc_v2_ext->max_conns, SMC_CONN_PER_LGR_PREFER);
+		if (ini->max_conns < SMC_CONN_PER_LGR_MIN)
+			return SMC_CLC_DECL_MAXCONNERR;
+	}
+
 	return 0;
 }
 
 int smc_clc_cli_v2x_features_validate(struct smc_clc_first_contact_ext *fce,
 				      struct smc_init_info *ini)
 {
+	struct smc_clc_first_contact_ext_v2x *fce_v2x =
+		(struct smc_clc_first_contact_ext_v2x *)fce;
+
 	if (ini->release_nr < SMC_RELEASE_1)
 		return 0;
 
+	if (!ini->is_smcd) {
+		if (fce_v2x->max_conns < SMC_CONN_PER_LGR_MIN)
+			return SMC_CLC_DECL_MAXCONNERR;
+		ini->max_conns = fce_v2x->max_conns;
+	}
+
 	return 0;
 }
 
@@ -1190,6 +1220,8 @@ int smc_clc_v2x_features_confirm_check(struct smc_clc_msg_accept_confirm *cclc,
 		(struct smc_clc_msg_accept_confirm_v2 *)cclc;
 	struct smc_clc_first_contact_ext *fce =
 		smc_get_clc_first_contact_ext(clc_v2, ini->is_smcd);
+	struct smc_clc_first_contact_ext_v2x *fce_v2x =
+		(struct smc_clc_first_contact_ext_v2x *)fce;
 
 	if (cclc->hdr.version == SMC_V1 ||
 	    !(cclc->hdr.typev2 & SMC_FIRST_CONTACT_MASK))
@@ -1201,6 +1233,11 @@ int smc_clc_v2x_features_confirm_check(struct smc_clc_msg_accept_confirm *cclc,
 	if (fce->release < SMC_RELEASE_1)
 		return 0;
 
+	if (!ini->is_smcd) {
+		if (fce_v2x->max_conns != ini->max_conns)
+			return SMC_CLC_DECL_MAXCONNERR;
+	}
+
 	return 0;
 }
 
diff --git a/net/smc/smc_clc.h b/net/smc/smc_clc.h
index 66932bfdc6d0..54077e50c368 100644
--- a/net/smc/smc_clc.h
+++ b/net/smc/smc_clc.h
@@ -46,6 +46,7 @@
 #define SMC_CLC_DECL_NOSMCD2DEV	0x03030007  /* no SMC-Dv2 device found	      */
 #define SMC_CLC_DECL_NOUEID	0x03030008  /* peer sent no UEID	      */
 #define SMC_CLC_DECL_RELEASEERR	0x03030009  /* release version negotiate failed */
+#define SMC_CLC_DECL_MAXCONNERR	0x0303000a  /* max connections negotiate failed */
 #define SMC_CLC_DECL_MODEUNSUPP	0x03040000  /* smc modes do not match (R or D)*/
 #define SMC_CLC_DECL_RMBE_EC	0x03050000  /* peer has eyecatcher in RMBE    */
 #define SMC_CLC_DECL_OPTUNSUPP	0x03060000  /* fastopen sockopt not supported */
@@ -134,7 +135,8 @@ struct smc_clc_smcd_gid_chid {
 struct smc_clc_v2_extension {
 	struct smc_clnt_opts_area_hdr hdr;
 	u8 roce[16];		/* RoCEv2 GID */
-	u8 reserved[16];
+	u8 max_conns;
+	u8 reserved[15];
 	u8 user_eids[][SMC_MAX_EID_LEN];
 };
 
@@ -236,7 +238,8 @@ struct smc_clc_first_contact_ext {
 
 struct smc_clc_first_contact_ext_v2x {
 	struct smc_clc_first_contact_ext fce_v20;
-	u8 reserved3[4];
+	u8 max_conns; /* for SMC-R only */
+	u8 reserved3[3];
 	__be32 vendor_exp_options;
 	u8 reserved4[8];
 } __packed;		/* format defined in
diff --git a/net/smc/smc_core.c b/net/smc/smc_core.c
index 6aa3db47a956..a8091a3e6cdd 100644
--- a/net/smc/smc_core.c
+++ b/net/smc/smc_core.c
@@ -895,9 +895,11 @@ static int smc_lgr_create(struct smc_sock *smc, struct smc_init_info *ini)
 			lgr->uses_gateway = ini->smcrv2.uses_gateway;
 			memcpy(lgr->nexthop_mac, ini->smcrv2.nexthop_mac,
 			       ETH_ALEN);
+			lgr->max_conns = ini->max_conns;
 		} else {
 			ibdev = ini->ib_dev;
 			ibport = ini->ib_port;
+			lgr->max_conns = SMC_CONN_PER_LGR_MAX;
 		}
 		memcpy(lgr->pnet_id, ibdev->pnetid[ibport - 1],
 		       SMC_MAX_PNETID_LEN);
@@ -1890,7 +1892,7 @@ int smc_conn_create(struct smc_sock *smc, struct smc_init_info *ini)
 		    (ini->smcd_version == SMC_V2 ||
 		     lgr->vlan_id == ini->vlan_id) &&
 		    (role == SMC_CLNT || ini->is_smcd ||
-		    (lgr->conns_num < SMC_RMBS_PER_LGR_MAX &&
+		    (lgr->conns_num < lgr->max_conns &&
 		      !bitmap_full(lgr->rtokens_used_mask, SMC_RMBS_PER_LGR_MAX)))) {
 			/* link group found */
 			ini->first_contact_local = 0;
diff --git a/net/smc/smc_core.h b/net/smc/smc_core.h
index b6c68db61f23..32b199477ef3 100644
--- a/net/smc/smc_core.h
+++ b/net/smc/smc_core.h
@@ -22,6 +22,15 @@
 #include "smc_ib.h"
 
 #define SMC_RMBS_PER_LGR_MAX	255	/* max. # of RMBs per link group */
+#define SMC_CONN_PER_LGR_MIN	16	/* min. # of connections per link group */
+#define SMC_CONN_PER_LGR_MAX	255	/* max. # of connections per link group,
+					 * also is the default value for SMC-R v1 and v2.0
+					 */
+#define SMC_CONN_PER_LGR_PREFER	255	/* Preferred connections per link group used for
+					 * SMC-R v2.1 and later negotiation, vendors or
+					 * distrubutions may modify it to a value between
+					 * 16-255 as needed.
+					 */
 
 struct smc_lgr_list {			/* list of link group definition */
 	struct list_head	list;
@@ -331,6 +340,8 @@ struct smc_link_group {
 			__be32			saddr;
 						/* net namespace */
 			struct net		*net;
+			u8			max_conns;
+						/* max conn can be assigned to lgr */
 		};
 		struct { /* SMC-D */
 			u64			peer_gid;
@@ -375,6 +386,7 @@ struct smc_init_info {
 	u8			smc_type_v1;
 	u8			smc_type_v2;
 	u8			release_nr;
+	u8			max_conns;
 	u8			first_contact_peer;
 	u8			first_contact_local;
 	unsigned short		vlan_id;
diff --git a/net/smc/smc_llc.c b/net/smc/smc_llc.c
index 90f0b60b196a..5347b62f1518 100644
--- a/net/smc/smc_llc.c
+++ b/net/smc/smc_llc.c
@@ -52,7 +52,8 @@ struct smc_llc_msg_confirm_link {	/* type 0x01 */
 	u8 link_num;
 	u8 link_uid[SMC_LGR_ID_SIZE];
 	u8 max_links;
-	u8 reserved[9];
+	u8 max_conns;
+	u8 reserved[8];
 };
 
 #define SMC_LLC_FLAG_ADD_LNK_REJ	0x40
@@ -472,6 +473,9 @@ int smc_llc_send_confirm_link(struct smc_link *link,
 	confllc->link_num = link->link_id;
 	memcpy(confllc->link_uid, link->link_uid, SMC_LGR_ID_SIZE);
 	confllc->max_links = SMC_LLC_ADD_LNK_MAX_LINKS;
+	if (link->lgr->smc_version == SMC_V2 &&
+	    link->lgr->peer_smc_release >= SMC_RELEASE_1)
+		confllc->max_conns = link->lgr->max_conns;
 	/* send llc message */
 	rc = smc_wr_tx_send(link, pend);
 put_out:
-- 
2.24.3 (Apple Git-128)

Re: [PATCH net-next 4/6] net/smc: support max connections per lgr negotiation

[Jan Karcher]

On 16/08/2023 10:33, Guangguan Wang wrote:
> Support max connections per lgr negotiation for SMCR v2.1,
> which is one of smc v2.1 features. Server makes decision for
> the final value of max conns based on the client preferred
> max conns and self-preferred max conns. Here use the minimum
> value of client preferred max conns and server preferred max
> conns.
> 
> Client                                     Server
>       Proposal(max conns(client preferred))
>       ------------------------------------>
> 
>       Accept(max conns(accepted value))
> accepted value=min(client preferred, server preferred)
>       <-----------------------------------
> 
>       Confirm(max conns(accepted value))
>       ----------------------------------->
> 
> Signed-off-by: Guangguan Wang <guangguan.wang@linux.alibaba.com>
> Reviewed-by: Tony Lu <tonylu@linux.alibaba.com>
> ---
>   net/smc/af_smc.c   |  1 +
>   net/smc/smc_clc.c  | 41 +++++++++++++++++++++++++++++++++++++++--
>   net/smc/smc_clc.h  |  7 +++++--
>   net/smc/smc_core.c |  4 +++-
>   net/smc/smc_core.h | 12 ++++++++++++
>   net/smc/smc_llc.c  |  6 +++++-
>   6 files changed, 65 insertions(+), 6 deletions(-)
> 
> diff --git a/net/smc/af_smc.c b/net/smc/af_smc.c
> index 65c02b48331f..f7174135c74a 100644
> --- a/net/smc/af_smc.c
> +++ b/net/smc/af_smc.c
> @@ -1212,6 +1212,7 @@ static int smc_connect_rdma(struct smc_sock *smc,
>   	memcpy(ini->peer_systemid, aclc->r0.lcl.id_for_peer, SMC_SYSTEMID_LEN);
>   	memcpy(ini->peer_gid, aclc->r0.lcl.gid, SMC_GID_SIZE);
>   	memcpy(ini->peer_mac, aclc->r0.lcl.mac, ETH_ALEN);
> +	ini->max_conns = SMC_CONN_PER_LGR_MAX;
>   
>   	reason_code = smc_connect_rdma_v2_prepare(smc, aclc, ini);
>   	if (reason_code)
> diff --git a/net/smc/smc_clc.c b/net/smc/smc_clc.c
> index f71b22e50be5..6609b06f000f 100644
> --- a/net/smc/smc_clc.c
> +++ b/net/smc/smc_clc.c
> @@ -427,9 +427,17 @@ static int smc_clc_fill_fce(struct smc_clc_first_contact_ext_v2x *fce,
>   	fce->fce_v20.os_type = SMC_CLC_OS_LINUX;
>   	fce->fce_v20.release = ini->release_nr;
>   	memcpy(fce->fce_v20.hostname, smc_hostname, sizeof(smc_hostname));
> -	if (ini->is_smcd && ini->release_nr < SMC_RELEASE_1)
> +	if (ini->is_smcd && ini->release_nr < SMC_RELEASE_1) {
>   		ret = sizeof(struct smc_clc_first_contact_ext);
> +		goto out;
> +	}
> +
> +	if (ini->release_nr >= SMC_RELEASE_1) {
> +		if (!ini->is_smcd)
> +			fce->max_conns = ini->max_conns;
> +	}
>   
> +out:
>   	return ret;
>   }
>   
> @@ -931,8 +939,10 @@ int smc_clc_send_proposal(struct smc_sock *smc, struct smc_init_info *ini)
>   				sizeof(struct smc_clc_smcd_gid_chid);
>   		}
>   	}
> -	if (smcr_indicated(ini->smc_type_v2))
> +	if (smcr_indicated(ini->smc_type_v2)) {
>   		memcpy(v2_ext->roce, ini->smcrv2.ib_gid_v2, SMC_GID_SIZE);
> +		v2_ext->max_conns = SMC_CONN_PER_LGR_PREFER;
> +	}
>   
>   	pclc_base->hdr.length = htons(plen);
>   	memcpy(trl->eyecatcher, SMC_EYECATCHER, sizeof(SMC_EYECATCHER));
> @@ -1163,6 +1173,11 @@ int smc_clc_srv_v2x_features_validate(struct smc_clc_msg_proposal *pclc,
>   {
>   	struct smc_clc_v2_extension *pclc_v2_ext;
>   
> +	/* default max conn is SMC_CONN_PER_LGR_MAX(255),
> +	 * which is the default value in smc v1 and v2.0.
> +	 */

I'm afraid this comment is not going to age well. You already put this 
to the define itself so i plea to remove it here.

Thank you.
- Jan

> +	ini->max_conns = SMC_CONN_PER_LGR_MAX;
> +
>   	if ((!(ini->smcd_version & SMC_V2) && !(ini->smcr_version & SMC_V2)) ||
>   	    ini->release_nr < SMC_RELEASE_1)
>   		return 0;
> @@ -1171,15 +1186,30 @@ int smc_clc_srv_v2x_features_validate(struct smc_clc_msg_proposal *pclc,
>   	if (!pclc_v2_ext)
>   		return SMC_CLC_DECL_NOV2EXT;
>   
> +	if (ini->smcr_version & SMC_V2) {
> +		ini->max_conns = min_t(u8, pclc_v2_ext->max_conns, SMC_CONN_PER_LGR_PREFER);
> +		if (ini->max_conns < SMC_CONN_PER_LGR_MIN)
> +			return SMC_CLC_DECL_MAXCONNERR;
> +	}
> +
>   	return 0;
>   }
>   
>   int smc_clc_cli_v2x_features_validate(struct smc_clc_first_contact_ext *fce,
>   				      struct smc_init_info *ini)
>   {
> +	struct smc_clc_first_contact_ext_v2x *fce_v2x =
> +		(struct smc_clc_first_contact_ext_v2x *)fce;
> +
>   	if (ini->release_nr < SMC_RELEASE_1)
>   		return 0;
>   
> +	if (!ini->is_smcd) {
> +		if (fce_v2x->max_conns < SMC_CONN_PER_LGR_MIN)
> +			return SMC_CLC_DECL_MAXCONNERR;
> +		ini->max_conns = fce_v2x->max_conns;
> +	}
> +
>   	return 0;
>   }
>   
> @@ -1190,6 +1220,8 @@ int smc_clc_v2x_features_confirm_check(struct smc_clc_msg_accept_confirm *cclc,
>   		(struct smc_clc_msg_accept_confirm_v2 *)cclc;
>   	struct smc_clc_first_contact_ext *fce =
>   		smc_get_clc_first_contact_ext(clc_v2, ini->is_smcd);
> +	struct smc_clc_first_contact_ext_v2x *fce_v2x =
> +		(struct smc_clc_first_contact_ext_v2x *)fce;
>   
>   	if (cclc->hdr.version == SMC_V1 ||
>   	    !(cclc->hdr.typev2 & SMC_FIRST_CONTACT_MASK))
> @@ -1201,6 +1233,11 @@ int smc_clc_v2x_features_confirm_check(struct smc_clc_msg_accept_confirm *cclc,
>   	if (fce->release < SMC_RELEASE_1)
>   		return 0;
>   
> +	if (!ini->is_smcd) {
> +		if (fce_v2x->max_conns != ini->max_conns)
> +			return SMC_CLC_DECL_MAXCONNERR;
> +	}
> +
>   	return 0;
>   }
>   
> diff --git a/net/smc/smc_clc.h b/net/smc/smc_clc.h
> index 66932bfdc6d0..54077e50c368 100644
> --- a/net/smc/smc_clc.h
> +++ b/net/smc/smc_clc.h
> @@ -46,6 +46,7 @@
>   #define SMC_CLC_DECL_NOSMCD2DEV	0x03030007  /* no SMC-Dv2 device found	      */
>   #define SMC_CLC_DECL_NOUEID	0x03030008  /* peer sent no UEID	      */
>   #define SMC_CLC_DECL_RELEASEERR	0x03030009  /* release version negotiate failed */
> +#define SMC_CLC_DECL_MAXCONNERR	0x0303000a  /* max connections negotiate failed */
>   #define SMC_CLC_DECL_MODEUNSUPP	0x03040000  /* smc modes do not match (R or D)*/
>   #define SMC_CLC_DECL_RMBE_EC	0x03050000  /* peer has eyecatcher in RMBE    */
>   #define SMC_CLC_DECL_OPTUNSUPP	0x03060000  /* fastopen sockopt not supported */
> @@ -134,7 +135,8 @@ struct smc_clc_smcd_gid_chid {
>   struct smc_clc_v2_extension {
>   	struct smc_clnt_opts_area_hdr hdr;
>   	u8 roce[16];		/* RoCEv2 GID */
> -	u8 reserved[16];
> +	u8 max_conns;
> +	u8 reserved[15];
>   	u8 user_eids[][SMC_MAX_EID_LEN];
>   };
>   
> @@ -236,7 +238,8 @@ struct smc_clc_first_contact_ext {
>   
>   struct smc_clc_first_contact_ext_v2x {
>   	struct smc_clc_first_contact_ext fce_v20;
> -	u8 reserved3[4];
> +	u8 max_conns; /* for SMC-R only */
> +	u8 reserved3[3];
>   	__be32 vendor_exp_options;
>   	u8 reserved4[8];
>   } __packed;		/* format defined in
> diff --git a/net/smc/smc_core.c b/net/smc/smc_core.c
> index 6aa3db47a956..a8091a3e6cdd 100644
> --- a/net/smc/smc_core.c
> +++ b/net/smc/smc_core.c
> @@ -895,9 +895,11 @@ static int smc_lgr_create(struct smc_sock *smc, struct smc_init_info *ini)
>   			lgr->uses_gateway = ini->smcrv2.uses_gateway;
>   			memcpy(lgr->nexthop_mac, ini->smcrv2.nexthop_mac,
>   			       ETH_ALEN);
> +			lgr->max_conns = ini->max_conns;
>   		} else {
>   			ibdev = ini->ib_dev;
>   			ibport = ini->ib_port;
> +			lgr->max_conns = SMC_CONN_PER_LGR_MAX;
>   		}
>   		memcpy(lgr->pnet_id, ibdev->pnetid[ibport - 1],
>   		       SMC_MAX_PNETID_LEN);
> @@ -1890,7 +1892,7 @@ int smc_conn_create(struct smc_sock *smc, struct smc_init_info *ini)
>   		    (ini->smcd_version == SMC_V2 ||
>   		     lgr->vlan_id == ini->vlan_id) &&
>   		    (role == SMC_CLNT || ini->is_smcd ||
> -		    (lgr->conns_num < SMC_RMBS_PER_LGR_MAX &&
> +		    (lgr->conns_num < lgr->max_conns &&
>   		      !bitmap_full(lgr->rtokens_used_mask, SMC_RMBS_PER_LGR_MAX)))) {
>   			/* link group found */
>   			ini->first_contact_local = 0;
> diff --git a/net/smc/smc_core.h b/net/smc/smc_core.h
> index b6c68db61f23..32b199477ef3 100644
> --- a/net/smc/smc_core.h
> +++ b/net/smc/smc_core.h
> @@ -22,6 +22,15 @@
>   #include "smc_ib.h"
>   
>   #define SMC_RMBS_PER_LGR_MAX	255	/* max. # of RMBs per link group */
> +#define SMC_CONN_PER_LGR_MIN	16	/* min. # of connections per link group */
> +#define SMC_CONN_PER_LGR_MAX	255	/* max. # of connections per link group,
> +					 * also is the default value for SMC-R v1 and v2.0
> +					 */
> +#define SMC_CONN_PER_LGR_PREFER	255	/* Preferred connections per link group used for
> +					 * SMC-R v2.1 and later negotiation, vendors or
> +					 * distrubutions may modify it to a value between
> +					 * 16-255 as needed.
> +					 */
>   
>   struct smc_lgr_list {			/* list of link group definition */
>   	struct list_head	list;
> @@ -331,6 +340,8 @@ struct smc_link_group {
>   			__be32			saddr;
>   						/* net namespace */
>   			struct net		*net;
> +			u8			max_conns;
> +						/* max conn can be assigned to lgr */
>   		};
>   		struct { /* SMC-D */
>   			u64			peer_gid;
> @@ -375,6 +386,7 @@ struct smc_init_info {
>   	u8			smc_type_v1;
>   	u8			smc_type_v2;
>   	u8			release_nr;
> +	u8			max_conns;
>   	u8			first_contact_peer;
>   	u8			first_contact_local;
>   	unsigned short		vlan_id;
> diff --git a/net/smc/smc_llc.c b/net/smc/smc_llc.c
> index 90f0b60b196a..5347b62f1518 100644
> --- a/net/smc/smc_llc.c
> +++ b/net/smc/smc_llc.c
> @@ -52,7 +52,8 @@ struct smc_llc_msg_confirm_link {	/* type 0x01 */
>   	u8 link_num;
>   	u8 link_uid[SMC_LGR_ID_SIZE];
>   	u8 max_links;
> -	u8 reserved[9];
> +	u8 max_conns;
> +	u8 reserved[8];
>   };
>   
>   #define SMC_LLC_FLAG_ADD_LNK_REJ	0x40
> @@ -472,6 +473,9 @@ int smc_llc_send_confirm_link(struct smc_link *link,
>   	confllc->link_num = link->link_id;
>   	memcpy(confllc->link_uid, link->link_uid, SMC_LGR_ID_SIZE);
>   	confllc->max_links = SMC_LLC_ADD_LNK_MAX_LINKS;
> +	if (link->lgr->smc_version == SMC_V2 &&
> +	    link->lgr->peer_smc_release >= SMC_RELEASE_1)
> +		confllc->max_conns = link->lgr->max_conns;
>   	/* send llc message */
>   	rc = smc_wr_tx_send(link, pend);
>   put_out:

Re: [PATCH net-next 4/6] net/smc: support max connections per lgr negotiation

[Guangguan Wang]

On 2023/8/17 14:42, Jan Karcher wrote:
> 
> 
> On 16/08/2023 10:33, Guangguan Wang wrote:
...
>> @@ -1163,6 +1173,11 @@ int smc_clc_srv_v2x_features_validate(struct smc_clc_msg_proposal *pclc,
>>   {
>>       struct smc_clc_v2_extension *pclc_v2_ext;
>>   +    /* default max conn is SMC_CONN_PER_LGR_MAX(255),
>> +     * which is the default value in smc v1 and v2.0.
>> +     */
> 
> I'm afraid this comment is not going to age well. You already put this to the define itself so i plea to remove it here.
> 
> Thank you.
> - Jan
> 

Get it, Thanks.

[PATCH net-next 5/6] net/smc: support max links per lgr negotiation in clc handshake

[Guangguan Wang]

Support max links per lgr negotiation in clc handshake for SMCR v2.1,
which is one of smc v2.1 features. Server makes decision for the final
value of max links based on the client preferred max links and
self-preferred max links. Here use the minimum value of the client
preferred max links and server preferred max links.

Client                                       Server
     Proposal(max links(client preferred))
     -------------------------------------->

     Accept(max links(accepted value))
accepted value=min(client preferred, server preferred)
     <-------------------------------------

      Confirm(max links(accepted value))
     ------------------------------------->

Signed-off-by: Guangguan Wang <guangguan.wang@linux.alibaba.com>
Reviewed-by: Tony Lu <tonylu@linux.alibaba.com>
---
 net/smc/af_smc.c   | 44 +++++++++++++++++++++++++++-----------------
 net/smc/smc_clc.c  | 17 ++++++++++++++++-
 net/smc/smc_clc.h  |  7 +++++--
 net/smc/smc_core.c |  5 +++++
 net/smc/smc_core.h | 12 ++++++++++++
 net/smc/smc_llc.c  | 21 +++++++++++++++++----
 6 files changed, 82 insertions(+), 24 deletions(-)

diff --git a/net/smc/af_smc.c b/net/smc/af_smc.c
index f7174135c74a..86d4a916450a 100644
--- a/net/smc/af_smc.c
+++ b/net/smc/af_smc.c
@@ -610,20 +610,22 @@ static int smcr_clnt_conf_first_link(struct smc_sock *smc)
 	smc_llc_link_active(link);
 	smcr_lgr_set_type(link->lgr, SMC_LGR_SINGLE);
 
-	/* optional 2nd link, receive ADD LINK request from server */
-	qentry = smc_llc_wait(link->lgr, NULL, SMC_LLC_WAIT_TIME,
-			      SMC_LLC_ADD_LINK);
-	if (!qentry) {
-		struct smc_clc_msg_decline dclc;
-
-		rc = smc_clc_wait_msg(smc, &dclc, sizeof(dclc),
-				      SMC_CLC_DECLINE, CLC_WAIT_TIME_SHORT);
-		if (rc == -EAGAIN)
-			rc = 0; /* no DECLINE received, go with one link */
-		return rc;
+	if (link->lgr->max_links > 1) {
+		/* optional 2nd link, receive ADD LINK request from server */
+		qentry = smc_llc_wait(link->lgr, NULL, SMC_LLC_WAIT_TIME,
+				      SMC_LLC_ADD_LINK);
+		if (!qentry) {
+			struct smc_clc_msg_decline dclc;
+
+			rc = smc_clc_wait_msg(smc, &dclc, sizeof(dclc),
+					      SMC_CLC_DECLINE, CLC_WAIT_TIME_SHORT);
+			if (rc == -EAGAIN)
+				rc = 0; /* no DECLINE received, go with one link */
+			return rc;
+		}
+		smc_llc_flow_qentry_clr(&link->lgr->llc_flow_lcl);
+		smc_llc_cli_add_link(link, qentry);
 	}
-	smc_llc_flow_qentry_clr(&link->lgr->llc_flow_lcl);
-	smc_llc_cli_add_link(link, qentry);
 	return 0;
 }
 
@@ -1213,6 +1215,7 @@ static int smc_connect_rdma(struct smc_sock *smc,
 	memcpy(ini->peer_gid, aclc->r0.lcl.gid, SMC_GID_SIZE);
 	memcpy(ini->peer_mac, aclc->r0.lcl.mac, ETH_ALEN);
 	ini->max_conns = SMC_CONN_PER_LGR_MAX;
+	ini->max_links = SMC_LINKS_ADD_LNK_MAX;
 
 	reason_code = smc_connect_rdma_v2_prepare(smc, aclc, ini);
 	if (reason_code)
@@ -1857,10 +1860,12 @@ static int smcr_serv_conf_first_link(struct smc_sock *smc)
 	smc_llc_link_active(link);
 	smcr_lgr_set_type(link->lgr, SMC_LGR_SINGLE);
 
-	down_write(&link->lgr->llc_conf_mutex);
-	/* initial contact - try to establish second link */
-	smc_llc_srv_add_link(link, NULL);
-	up_write(&link->lgr->llc_conf_mutex);
+	if (link->lgr->max_links > 1) {
+		down_write(&link->lgr->llc_conf_mutex);
+		/* initial contact - try to establish second link */
+		smc_llc_srv_add_link(link, NULL);
+		up_write(&link->lgr->llc_conf_mutex);
+	}
 	return 0;
 }
 
@@ -2464,6 +2469,11 @@ static void smc_listen_work(struct work_struct *work)
 		goto out_decl;
 	}
 
+	/* fce smc release version is needed in smc_listen_rdma_finish,
+	 * so save fce info here.
+	 */
+	smc_conn_save_peer_info_fce(new_smc, cclc);
+
 	/* finish worker */
 	if (!ini->is_smcd) {
 		rc = smc_listen_rdma_finish(new_smc, cclc,
diff --git a/net/smc/smc_clc.c b/net/smc/smc_clc.c
index 6609b06f000f..4924c3e9a36a 100644
--- a/net/smc/smc_clc.c
+++ b/net/smc/smc_clc.c
@@ -433,8 +433,10 @@ static int smc_clc_fill_fce(struct smc_clc_first_contact_ext_v2x *fce,
 	}
 
 	if (ini->release_nr >= SMC_RELEASE_1) {
-		if (!ini->is_smcd)
+		if (!ini->is_smcd) {
 			fce->max_conns = ini->max_conns;
+			fce->max_links = ini->max_links;
+		}
 	}
 
 out:
@@ -942,6 +944,7 @@ int smc_clc_send_proposal(struct smc_sock *smc, struct smc_init_info *ini)
 	if (smcr_indicated(ini->smc_type_v2)) {
 		memcpy(v2_ext->roce, ini->smcrv2.ib_gid_v2, SMC_GID_SIZE);
 		v2_ext->max_conns = SMC_CONN_PER_LGR_PREFER;
+		v2_ext->max_links = SMC_LINKS_PER_LGR_MAX_PREFER;
 	}
 
 	pclc_base->hdr.length = htons(plen);
@@ -1177,6 +1180,7 @@ int smc_clc_srv_v2x_features_validate(struct smc_clc_msg_proposal *pclc,
 	 * which is the default value in smc v1 and v2.0.
 	 */
 	ini->max_conns = SMC_CONN_PER_LGR_MAX;
+	ini->max_links = SMC_LINKS_ADD_LNK_MAX;
 
 	if ((!(ini->smcd_version & SMC_V2) && !(ini->smcr_version & SMC_V2)) ||
 	    ini->release_nr < SMC_RELEASE_1)
@@ -1190,6 +1194,10 @@ int smc_clc_srv_v2x_features_validate(struct smc_clc_msg_proposal *pclc,
 		ini->max_conns = min_t(u8, pclc_v2_ext->max_conns, SMC_CONN_PER_LGR_PREFER);
 		if (ini->max_conns < SMC_CONN_PER_LGR_MIN)
 			return SMC_CLC_DECL_MAXCONNERR;
+
+		ini->max_links = min_t(u8, pclc_v2_ext->max_links, SMC_LINKS_PER_LGR_MAX_PREFER);
+		if (ini->max_links < SMC_LINKS_ADD_LNK_MIN)
+			return SMC_CLC_DECL_MAXLINKERR;
 	}
 
 	return 0;
@@ -1208,6 +1216,11 @@ int smc_clc_cli_v2x_features_validate(struct smc_clc_first_contact_ext *fce,
 		if (fce_v2x->max_conns < SMC_CONN_PER_LGR_MIN)
 			return SMC_CLC_DECL_MAXCONNERR;
 		ini->max_conns = fce_v2x->max_conns;
+
+		if (fce_v2x->max_links > SMC_LINKS_ADD_LNK_MAX ||
+		    fce_v2x->max_links < SMC_LINKS_ADD_LNK_MIN)
+			return SMC_CLC_DECL_MAXLINKERR;
+		ini->max_links = fce_v2x->max_links;
 	}
 
 	return 0;
@@ -1236,6 +1249,8 @@ int smc_clc_v2x_features_confirm_check(struct smc_clc_msg_accept_confirm *cclc,
 	if (!ini->is_smcd) {
 		if (fce_v2x->max_conns != ini->max_conns)
 			return SMC_CLC_DECL_MAXCONNERR;
+		if (fce_v2x->max_links != ini->max_links)
+			return SMC_CLC_DECL_MAXLINKERR;
 	}
 
 	return 0;
diff --git a/net/smc/smc_clc.h b/net/smc/smc_clc.h
index 54077e50c368..53fb20d7c14d 100644
--- a/net/smc/smc_clc.h
+++ b/net/smc/smc_clc.h
@@ -47,6 +47,7 @@
 #define SMC_CLC_DECL_NOUEID	0x03030008  /* peer sent no UEID	      */
 #define SMC_CLC_DECL_RELEASEERR	0x03030009  /* release version negotiate failed */
 #define SMC_CLC_DECL_MAXCONNERR	0x0303000a  /* max connections negotiate failed */
+#define SMC_CLC_DECL_MAXLINKERR	0x0303000b  /* max links negotiate failed */
 #define SMC_CLC_DECL_MODEUNSUPP	0x03040000  /* smc modes do not match (R or D)*/
 #define SMC_CLC_DECL_RMBE_EC	0x03050000  /* peer has eyecatcher in RMBE    */
 #define SMC_CLC_DECL_OPTUNSUPP	0x03060000  /* fastopen sockopt not supported */
@@ -136,7 +137,8 @@ struct smc_clc_v2_extension {
 	struct smc_clnt_opts_area_hdr hdr;
 	u8 roce[16];		/* RoCEv2 GID */
 	u8 max_conns;
-	u8 reserved[15];
+	u8 max_links;
+	u8 reserved[14];
 	u8 user_eids[][SMC_MAX_EID_LEN];
 };
 
@@ -239,7 +241,8 @@ struct smc_clc_first_contact_ext {
 struct smc_clc_first_contact_ext_v2x {
 	struct smc_clc_first_contact_ext fce_v20;
 	u8 max_conns; /* for SMC-R only */
-	u8 reserved3[3];
+	u8 max_links; /* for SMC-R only */
+	u8 reserved3[2];
 	__be32 vendor_exp_options;
 	u8 reserved4[8];
 } __packed;		/* format defined in
diff --git a/net/smc/smc_core.c b/net/smc/smc_core.c
index a8091a3e6cdd..1e1475084bb4 100644
--- a/net/smc/smc_core.c
+++ b/net/smc/smc_core.c
@@ -896,10 +896,12 @@ static int smc_lgr_create(struct smc_sock *smc, struct smc_init_info *ini)
 			memcpy(lgr->nexthop_mac, ini->smcrv2.nexthop_mac,
 			       ETH_ALEN);
 			lgr->max_conns = ini->max_conns;
+			lgr->max_links = ini->max_links;
 		} else {
 			ibdev = ini->ib_dev;
 			ibport = ini->ib_port;
 			lgr->max_conns = SMC_CONN_PER_LGR_MAX;
+			lgr->max_links = SMC_LINKS_ADD_LNK_MAX;
 		}
 		memcpy(lgr->pnet_id, ibdev->pnetid[ibport - 1],
 		       SMC_MAX_PNETID_LEN);
@@ -1667,6 +1669,9 @@ void smcr_port_add(struct smc_ib_device *smcibdev, u8 ibport)
 		    !rdma_dev_access_netns(smcibdev->ibdev, lgr->net))
 			continue;
 
+		if (lgr->type == SMC_LGR_SINGLE && lgr->max_links <= 1)
+			continue;
+
 		/* trigger local add link processing */
 		link = smc_llc_usable_link(lgr);
 		if (link)
diff --git a/net/smc/smc_core.h b/net/smc/smc_core.h
index 32b199477ef3..120027d40469 100644
--- a/net/smc/smc_core.h
+++ b/net/smc/smc_core.h
@@ -173,6 +173,15 @@ struct smc_link {
  */
 #define SMC_LINKS_PER_LGR_MAX	3
 #define SMC_SINGLE_LINK		0
+#define SMC_LINKS_ADD_LNK_MIN	1	/* min. # of links per link group */
+#define SMC_LINKS_ADD_LNK_MAX	2	/* max. # of links per link group, also is the
+					 * default value for smc-r v1.0 and v2.0
+					 */
+#define SMC_LINKS_PER_LGR_MAX_PREFER	2	/* Preferred max links per link group used for
+						 * SMC-R v2.1 and later negotiation, vendors or
+						 * distrubutions may modify it to a value between
+						 * 1-2 as needed.
+						 */
 
 /* tx/rx buffer list element for sndbufs list and rmbs list of a lgr */
 struct smc_buf_desc {
@@ -342,6 +351,8 @@ struct smc_link_group {
 			struct net		*net;
 			u8			max_conns;
 						/* max conn can be assigned to lgr */
+			u8			max_links;
+						/* max links can be added in lgr */
 		};
 		struct { /* SMC-D */
 			u64			peer_gid;
@@ -387,6 +398,7 @@ struct smc_init_info {
 	u8			smc_type_v2;
 	u8			release_nr;
 	u8			max_conns;
+	u8			max_links;
 	u8			first_contact_peer;
 	u8			first_contact_local;
 	unsigned short		vlan_id;
diff --git a/net/smc/smc_llc.c b/net/smc/smc_llc.c
index 5347b62f1518..018ce8133b02 100644
--- a/net/smc/smc_llc.c
+++ b/net/smc/smc_llc.c
@@ -59,8 +59,6 @@ struct smc_llc_msg_confirm_link {	/* type 0x01 */
 #define SMC_LLC_FLAG_ADD_LNK_REJ	0x40
 #define SMC_LLC_REJ_RSN_NO_ALT_PATH	1
 
-#define SMC_LLC_ADD_LNK_MAX_LINKS	2
-
 struct smc_llc_msg_add_link {		/* type 0x02 */
 	struct smc_llc_hdr hd;
 	u8 sender_mac[ETH_ALEN];
@@ -472,10 +470,12 @@ int smc_llc_send_confirm_link(struct smc_link *link,
 	hton24(confllc->sender_qp_num, link->roce_qp->qp_num);
 	confllc->link_num = link->link_id;
 	memcpy(confllc->link_uid, link->link_uid, SMC_LGR_ID_SIZE);
-	confllc->max_links = SMC_LLC_ADD_LNK_MAX_LINKS;
+	confllc->max_links = SMC_LINKS_ADD_LNK_MAX;
 	if (link->lgr->smc_version == SMC_V2 &&
-	    link->lgr->peer_smc_release >= SMC_RELEASE_1)
+	    link->lgr->peer_smc_release >= SMC_RELEASE_1) {
 		confllc->max_conns = link->lgr->max_conns;
+		confllc->max_links = link->lgr->max_links;
+	}
 	/* send llc message */
 	rc = smc_wr_tx_send(link, pend);
 put_out:
@@ -1045,6 +1045,11 @@ int smc_llc_cli_add_link(struct smc_link *link, struct smc_llc_qentry *qentry)
 		goto out_reject;
 	}
 
+	if (lgr->type == SMC_LGR_SINGLE && lgr->max_links <= 1) {
+		rc = 0;
+		goto out_reject;
+	}
+
 	ini->vlan_id = lgr->vlan_id;
 	if (lgr->smc_version == SMC_V2) {
 		ini->check_smcrv2 = true;
@@ -1169,6 +1174,9 @@ static void smc_llc_cli_add_link_invite(struct smc_link *link,
 	    lgr->type == SMC_LGR_ASYMMETRIC_PEER)
 		goto out;
 
+	if (lgr->type == SMC_LGR_SINGLE && lgr->max_links <= 1)
+		goto out;
+
 	ini = kzalloc(sizeof(*ini), GFP_KERNEL);
 	if (!ini)
 		goto out;
@@ -1414,6 +1422,11 @@ int smc_llc_srv_add_link(struct smc_link *link,
 		goto out;
 	}
 
+	if (lgr->type == SMC_LGR_SINGLE && lgr->max_links <= 1) {
+		rc = 0;
+		goto out;
+	}
+
 	/* ignore client add link recommendation, start new flow */
 	ini->vlan_id = lgr->vlan_id;
 	if (lgr->smc_version == SMC_V2) {
-- 
2.24.3 (Apple Git-128)

[PATCH net-next 6/6] net/smc: Extend SMCR v2 linkgroup netlink attribute

[Guangguan Wang]

Add SMC_NLA_LGR_R_V2_MAX_CONNS and SMC_NLA_LGR_R_V2_MAX_LINKS
to SMCR v2 linkgroup netlink attribute SMC_NLA_LGR_R_V2 for
linkgroup's detail info showing.

Signed-off-by: Guangguan Wang <guangguan.wang@linux.alibaba.com>
---
 include/uapi/linux/smc.h | 2 ++
 net/smc/smc_core.c       | 4 ++++
 2 files changed, 6 insertions(+)

diff --git a/include/uapi/linux/smc.h b/include/uapi/linux/smc.h
index bb4dacca31e7..837fcd4b0abc 100644
--- a/include/uapi/linux/smc.h
+++ b/include/uapi/linux/smc.h
@@ -107,6 +107,8 @@ enum {
 enum {
 	SMC_NLA_LGR_R_V2_UNSPEC,
 	SMC_NLA_LGR_R_V2_DIRECT,	/* u8 */
+	SMC_NLA_LGR_R_V2_MAX_CONNS,	/* u8 */
+	SMC_NLA_LGR_R_V2_MAX_LINKS,	/* u8 */
 	__SMC_NLA_LGR_R_V2_MAX,
 	SMC_NLA_LGR_R_V2_MAX = __SMC_NLA_LGR_R_V2_MAX - 1
 };
diff --git a/net/smc/smc_core.c b/net/smc/smc_core.c
index 1e1475084bb4..ef5336df3b09 100644
--- a/net/smc/smc_core.c
+++ b/net/smc/smc_core.c
@@ -319,6 +319,10 @@ static int smc_nl_fill_smcr_lgr_v2(struct smc_link_group *lgr,
 		goto errattr;
 	if (nla_put_u8(skb, SMC_NLA_LGR_R_V2_DIRECT, !lgr->uses_gateway))
 		goto errv2attr;
+	if (nla_put_u8(skb, SMC_NLA_LGR_R_V2_MAX_CONNS, lgr->max_conns))
+		goto errv2attr;
+	if (nla_put_u8(skb, SMC_NLA_LGR_R_V2_MAX_LINKS, lgr->max_links))
+		goto errv2attr;
 
 	nla_nest_end(skb, v2_attrs);
 	return 0;
-- 
2.24.3 (Apple Git-128)

Re: [PATCH net-next 0/6] net/smc: several features's implementation for smc v2.1

[Jan Karcher]

On 16/08/2023 10:33, Guangguan Wang wrote:
> This patch set implement several new features in SMC v2.1(https://
> www.ibm.com/support/pages/node/7009315), including vendor unique
> experimental options, max connections per lgr negotiation, max links
> per lgr negotiation.
> 
> I have removed the RFC tag and changed the patch series to formal
> patch series from this version.

Thank you Guangguan for your effort!

I'm done with the first iteration of review.
Please see the comments.

- Jan


> 
> RFC v2 - v1:
>   - more description in commit message
>   - modify SMC_CONN_PER_LGR_xxx and SMC_LINKS_ADD_LNK_xxx
>     macro defination and usage
>   - rename variable release_ver to release_nr
>   - remove redundant release version check in client
>   - explicitly set the rc value in smc_llc_cli/srv_add_link
> 
> RFC v1 - RFC v2:
>   - Remove ini pointer NULL check and fix code style in
>     smc_clc_send_confirm_accept.
>   - Optimize the max_conns check in smc_clc_xxx_v2x_features_validate.
> 
> Guangguan Wang (6):
>    net/smc: support smc release version negotiation in clc handshake
>    net/smc: add vendor unique experimental options area in clc handshake
>    net/smc: support smc v2.x features validate
>    net/smc: support max connections per lgr negotiation
>    net/smc: support max links per lgr negotiation in clc handshake
>    net/smc: Extend SMCR v2 linkgroup netlink attribute
> 
>   include/uapi/linux/smc.h |   2 +
>   net/smc/af_smc.c         |  83 ++++++++++++++++------
>   net/smc/smc.h            |   5 +-
>   net/smc/smc_clc.c        | 150 ++++++++++++++++++++++++++++++++-------
>   net/smc/smc_clc.h        |  53 ++++++++++++--
>   net/smc/smc_core.c       |  13 +++-
>   net/smc/smc_core.h       |  25 +++++++
>   net/smc/smc_llc.c        |  25 +++++--
>   8 files changed, 301 insertions(+), 55 deletions(-)
>