| Message ID | 20230816083328.95746-4-guangguan.wang@linux.alibaba.com (mailing list archive) |
|---|---|
| State | Superseded |
| Delegated to: | Netdev Maintainers |
| Headers | show |
| Series | net/smc: several features's implementation for smc v2.1 | expand |
| Context | Check | Description |
|---|---|---|
| netdev/series_format | success | Posting correctly formatted |
| netdev/tree_selection | success | Clearly marked for net-next |
| netdev/fixes_present | success | Fixes tag not required for -next series |
| netdev/header_inline | success | No static functions without inline keyword in header files |
| netdev/build_32bit | success | Errors and warnings before: 1330 this patch: 1330 |
| netdev/cc_maintainers | success | CCed 12 of 12 maintainers |
| netdev/build_clang | success | Errors and warnings before: 1353 this patch: 1353 |
| netdev/verify_signedoff | success | Signed-off-by tag matches author and committer |
| netdev/deprecated_api | success | None detected |
| netdev/check_selftest | success | No net selftest shell script |
| netdev/verify_fixes | success | No Fixes tag |
| netdev/build_allmodconfig_warn | success | Errors and warnings before: 1353 this patch: 1353 |
| netdev/checkpatch | warning | WARNING: line length of 82 exceeds 80 columns |
| netdev/kdoc | success | Errors and warnings before: 0 this patch: 0 |
| netdev/source_inline | success | Was 0 now: 0 |
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);
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
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);
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.
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);