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

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: <6711d96c-6c8c-4b9d-be07-759bfdab7875@linux.alibaba.com>
Date: Thu, 5 Dec 2024 20:02:39 +0800
From: Guangguan Wang <guangguan.wang@...ux.alibaba.com>
To: Wenjia Zhang <wenjia@...ux.ibm.com>, jaka@...ux.ibm.com,
 alibuda@...ux.alibaba.com, tonylu@...ux.alibaba.com,
 guwen@...ux.alibaba.com, davem@...emloft.net, edumazet@...gle.com,
 kuba@...nel.org, pabeni@...hat.com, horms@...nel.org
Cc: linux-rdma@...r.kernel.org, linux-s390@...r.kernel.org,
 netdev@...r.kernel.org, linux-kernel@...r.kernel.org,
 Dust Li <dust.li@...ux.alibaba.com>
Subject: Re: [PATCH net-next v2 2/2] net/smc: support ipv4 mapped ipv6 addr
 client for smc-r v2



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@...ux.alibaba.com>
>> Reviewed-by: Wen Gu <guwen@...ux.alibaba.com>
>> Reviewed-by: Dust Li <dust.li@...ux.alibaba.com>
>> Reviewed-by: D. Wythe <alibuda@...ux.alibaba.com>
>> ---
>>   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;