Re: [PATCH net-next v2 2/2] net/smc: support ipv4 mapped ipv6 addr client for smc-r v2

[Guangguan Wang <guangguan.wang@xxxxxxxxxxxxxxxxx>]

On 2024/12/5 18:16, Wenjia Zhang wrote:
> 
> 
> On 02.12.24 13:52, Guangguan Wang wrote:
>> AF_INET6 is not supported for smc-r v2 client before, event if the
> %s/event/even/g
> 

I will fix it in the next version.

>> ipv6 addr is ipv4 mapped. Thus, when using AF_INET6, smc-r connection
>> will fallback to tcp, especially for java applications running smc-r.
>> This patch support ipv4 mapped ipv6 addr client for smc-r v2. Clients
>> using real global ipv6 addr is still not supported yet.
>>
>> Signed-off-by: Guangguan Wang <guangguan.wang@xxxxxxxxxxxxxxxxx>
>> Reviewed-by: Wen Gu <guwen@xxxxxxxxxxxxxxxxx>
>> Reviewed-by: Dust Li <dust.li@xxxxxxxxxxxxxxxxx>
>> Reviewed-by: D. Wythe <alibuda@xxxxxxxxxxxxxxxxx>
>> ---
>>   net/smc/af_smc.c | 5 +++++
>>   1 file changed, 5 insertions(+)
>>
>> diff --git a/net/smc/af_smc.c b/net/smc/af_smc.c
>> index 9d76e902fd77..5b13dd759766 100644
>> --- a/net/smc/af_smc.c
>> +++ b/net/smc/af_smc.c
>> @@ -1116,7 +1116,12 @@ static int smc_find_proposal_devices(struct smc_sock *smc,
>>       ini->check_smcrv2 = true;
>>       ini->smcrv2.saddr = smc->clcsock->sk->sk_rcv_saddr;
>>       if (!(ini->smcr_version & SMC_V2) ||
>> +#if IS_ENABLED(CONFIG_IPV6)
>> +        (smc->clcsock->sk->sk_family != AF_INET &&
>> +         !ipv6_addr_v4mapped(&smc->clcsock->sk->sk_v6_rcv_saddr)) ||
> I think here you want to say !(smc->clcsock->sk->sk_family == AF_INET && ipv6_addr_v4mapped(&smc->clcsock->sk->sk_v6_rcv_saddr)), right? If it is, the negativ form of the logical operation (a&&b) is (!a)||(!b), i.e. here should be:
> （smc->clcsock->sk->sk_family != AF_INET）|| （!ipv6_addr_v4mapped(&smc->clcsock->sk->sk_v6_rcv_saddr)）
> 

(smc->clcsock->sk->sk_family != AF_INET && !ipv6_addr_v4mapped(&smc->clcsock->sk->sk_v6_rcv_saddr)) not equls to !(smc->clcsock->sk->sk_family == AF_INET && ipv6_addr_v4mapped(&smc->clcsock->sk->sk_v6_rcv_saddr))

Following your logic, here also can be: 
!(smc->clcsock->sk->sk_family == AF_INET || ipv6_addr_v4mapped(&smc->clcsock->sk->sk_v6_rcv_saddr))

I think both is acceptable. But in order to keep consistent with the code when IPV6 is not enabled, I prefer (smc->clcsock->sk->sk_family != AF_INET && !ipv6_addr_v4mapped(&smc->clcsock->sk->sk_v6_rcv_saddr)).

>> +#else
>>           smc->clcsock->sk->sk_family != AF_INET ||
>> +#endif
>>           !smc_clc_ueid_count() ||
>>           smc_find_rdma_device(smc, ini))
>>           ini->smcr_version &= ~SMC_V2;