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

[Guangguan Wang]

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