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

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

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <2dbf25a0-05a6-d899-3351-598e952a927d@linux.ibm.com>
Date:   Mon, 28 Aug 2023 14:54:57 +0200
From:   Wenjia Zhang <wenjia@...ux.ibm.com>
To:     Guangguan Wang <guangguan.wang@...ux.alibaba.com>,
        jaka@...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: [RFC PATCH v2 net-next 4/6] net/smc: support max connections per
 lgr negotiation



On 15.08.23 08:31, Guangguan Wang wrote:
> 
> 
> On 2023/8/10 00:04, Wenjia Zhang wrote:
>>
>>
>> On 07.08.23 08:27, Guangguan Wang wrote:
>>> Support max connections per lgr negotiation for SMCR v2.1,
>>> which is one of smc v2.1 features.
> ...
>>> diff --git a/net/smc/smc_core.c b/net/smc/smc_core.c
>>> index 6aa3db47a956..5de1fbaa6e28 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_RMBS_PER_LGR_MAX;
>>
>>
>> It is kind of confused sometimes SMC_RMBS_PER_LGR_MAX is used and sometimes SMC_CONN_PER_LGR_MAX. IMO, you can use SMC_CONN_PER_LGR_MAX in the patches series for the new feature, because they are the same value and the name is more suiable.
> 
> OK, I will re-define the macros like this:
> #define SMC_CONN_PER_LGR_MAX 255
> #define SMC_CONN_PER_LGR_MIN 16
> #define SMC_CONN_PER_LGR_PREFER 255 //vendors or distrubutions can modify this to a value between 16-255 as needed.
> 
> ...
>>> @@ -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:
>>
>> Did I miss the negotiation process somewhere for the following scenario?
>> (Example 4 in the document)
>> Client                 Server
>>      Proposal(max conns(16))
>>      ----------------------->
>>
>>      Accept(max conns(32))
>>      <-----------------------
>>
>>      Confirm(max conns(32))
>>      ----------------------->
> 
> Did you mean the accepted max conns is different(not 32) from the Example 4 when the proposal max conns is 16?
> 
> As described in (https://www.ibm.com/support/pages/node/7009315) page 41:
> ...
> 2. Max conns and max links values sent in the CLC Proposal are the client preferred values.
> 3. The v2.1 values sent in the Accept message are the final values. The client must accept the values or
> DECLINE the connection.
> 4. Max conns and links values sent in the CLC Accept are the final values (server dictates). The server can
> either honor the client’s preferred values or return different (negotiated but final) values.
> ...
> 
> If I understand correctly, the server dictates the final value of max conns, but how the server dictates the final
> value of max conns is not defined in SMC v2.1. In this patch, the server use the minimum value of client preferred
> value and server preferred value as the final value of max conns. The max links is negotiated with the same logic.
> 
> Client                 Server
>       Proposal(max conns(client preferred))
>       ----------------------->
>   
>       Accept(max conns(accepted value)) accepted value=min(client preferred, server preferred)
>       <-----------------------
>   
>       Confirm(max conns(accepted value))
>       ----------------------->
> 
> I also will add this description into commit message for better understanding.
> 
> Thanks,
> Guangguan Wang
> 
> 
> 

Sorry for the late answer, I'm just back from vacation.

That's true that the protocol does not define how the server decides the 
final value(s). I'm wondering if there is some reason for you to use the 
minimum value instead of maximum (corresponding to the examples in the 
document). If the both prefered values (client's and server's) are in 
the range of the acceptable value, why not the maximum? Is there any 
consideration on that?

Best,
Wenjia