lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
Open Source and information security mailing list archives
| ||
|
Message-ID: <af4eaa36-75ec-10f2-3a41-81895730f435@linux.ibm.com> Date: Thu, 17 Aug 2023 08:42:19 +0200 From: Jan Karcher <jaka@...ux.ibm.com> To: Guangguan Wang <guangguan.wang@...ux.alibaba.com>, wenjia@...ux.ibm.com, kgraul@...ux.ibm.com, tonylu@...ux.alibaba.com, davem@...emloft.net, edumazet@...gle.com, kuba@...nel.org, pabeni@...hat.com Cc: horms@...nel.org, alibuda@...ux.alibaba.com, guwen@...ux.alibaba.com, linux-s390@...r.kernel.org, netdev@...r.kernel.org, linux-kernel@...r.kernel.org Subject: Re: [PATCH net-next 1/6] net/smc: support smc release version negotiation in clc handshake 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@...ux.alibaba.com> >>> Reviewed-by: Tony Lu <tonylu@...ux.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 >
Powered by blists - more mailing lists