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

Commit Message

Guangguan Wang Dec. 9, 2024, 1:06 p.m. UTC

AF_INET6 is not supported for smc-r v2 client before, even if the
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 <[e-mail]>
Reviewed-by: Wen Gu <[e-mail]>
Reviewed-by: Dust Li <[e-mail]>
Reviewed-by: D. Wythe <[e-mail]>
Reviewed-by: Wenjia Zhang <[e-mail]>
---
 net/smc/af_smc.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

Comments

Wenjia Zhang Dec. 9, 2024, 2:10 p.m. UTC | #1

On 09.12.24 14:06, Guangguan Wang wrote:
> AF_INET6 is not supported for smc-r v2 client before, even if the
> 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 <[e-mail]>
> Reviewed-by: Wen Gu <[e-mail]>
> Reviewed-by: Dust Li <[e-mail]>
> Reviewed-by: D. Wythe <[e-mail]>
> Reviewed-by: Wenjia Zhang <[e-mail]>
> ---
>   net/smc/af_smc.c | 5 ++++-
>   1 file changed, 4 insertions(+), 1 deletion(-)
> 
> diff --git a/net/smc/af_smc.c b/net/smc/af_smc.c
> index 9d76e902fd77..c3f9c0457418 100644
> --- a/net/smc/af_smc.c
> +++ b/net/smc/af_smc.c
> @@ -1116,7 +1116,10 @@ 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) ||
> -	    smc->clcsock->sk->sk_family != AF_INET ||
> +#if IS_ENABLED(CONFIG_IPV6)
> +	    (smc->clcsock->sk->sk_family == AF_INET6 &&
> +	     !ipv6_addr_v4mapped(&smc->clcsock->sk->sk_v6_rcv_saddr)) ||
> +#endif
>   	    !smc_clc_ueid_count() ||
>   	    smc_find_rdma_device(smc, ini))
>   		ini->smcr_version &= ~SMC_V2;

@Guangguan, I think Halil's point is valid, and we need to verify if 
checking on saddr is sufficient before this patch is applied. i.e. what 
about one peer with ipv4 mapped ipv6 communicates with another peer with 
a real ipv6 address? Is it possible? If yes, would SMCRv2 be used? I 
still haven't thought much on this yet, but it is worth to verify. Maybe 
you already have the answer?

@Jakub, could you please give us some more time to verify the issue 
mentioned above?

Thanks,
Wenjia

Jakub Kicinski Dec. 9, 2024, 11:43 p.m. UTC | #2

On Mon, 9 Dec 2024 15:10:56 +0100 Wenjia Zhang wrote:
> @Jakub, could you please give us some more time to verify the issue 
> mentioned above?

No problem. I'll marked it as Awaiting upstream, please repost when
ready. This is our usual process when maintainers of a subsystem need
more time or need to do validation before we merge.

Guangguan Wang Dec. 10, 2024, 10:57 a.m. UTC | #3

On 2024/12/9 22:10, Wenjia Zhang wrote:
> 
> 
> On 09.12.24 14:06, Guangguan Wang wrote:
>> AF_INET6 is not supported for smc-r v2 client before, even if the
>> 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 <[e-mail]>
>> Reviewed-by: Wen Gu <[e-mail]>
>> Reviewed-by: Dust Li <[e-mail]>
>> Reviewed-by: D. Wythe <[e-mail]>
>> Reviewed-by: Wenjia Zhang <[e-mail]>
>> ---
>>   net/smc/af_smc.c | 5 ++++-
>>   1 file changed, 4 insertions(+), 1 deletion(-)
>>
>> diff --git a/net/smc/af_smc.c b/net/smc/af_smc.c
>> index 9d76e902fd77..c3f9c0457418 100644
>> --- a/net/smc/af_smc.c
>> +++ b/net/smc/af_smc.c
>> @@ -1116,7 +1116,10 @@ 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) ||
>> -        smc->clcsock->sk->sk_family != AF_INET ||
>> +#if IS_ENABLED(CONFIG_IPV6)
>> +        (smc->clcsock->sk->sk_family == AF_INET6 &&
>> +         !ipv6_addr_v4mapped(&smc->clcsock->sk->sk_v6_rcv_saddr)) ||
>> +#endif
>>           !smc_clc_ueid_count() ||
>>           smc_find_rdma_device(smc, ini))
>>           ini->smcr_version &= ~SMC_V2;
> 
> @Guangguan, I think Halil's point is valid, and we need to verify if checking on saddr is sufficient before this patch is applied. i.e. what about one peer with ipv4 mapped ipv6 communicates with another peer with a real ipv6 address? Is it possible? If yes, would SMCRv2 be used? I still haven't thought much on this yet, but it is worth to verify. Maybe you already have the answer?

Hi, Wenjia

I have replied the answer to the thread of v2 patch(https://lore.kernel.org/netdev/[e-mail]/)

If there are still any doubts or any other points to clarification, please let me know.

Thanks,
Guangguan Wang

> 
> @Jakub, could you please give us some more time to verify the issue mentioned above?
> 
> Thanks,
> Wenjia

Halil Pasic Dec. 10, 2024, 11:10 a.m. UTC | #4

On Mon,  9 Dec 2024 21:06:49 +0800
Guangguan Wang <[e-mail]> wrote:

> AF_INET6 is not supported for smc-r v2 client before, even if the
> 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 <[e-mail]>
> Reviewed-by: Wen Gu <[e-mail]>
> Reviewed-by: Dust Li <[e-mail]>
> Reviewed-by: D. Wythe <[e-mail]>
> Reviewed-by: Wenjia Zhang <[e-mail]>

Reviewed-by: Halil Pasic <[e-mail]>

Wenjia Zhang Dec. 10, 2024, 12:50 p.m. UTC | #5

On 10.12.24 00:43, Jakub Kicinski wrote:
> On Mon, 9 Dec 2024 15:10:56 +0100 Wenjia Zhang wrote:
>> @Jakub, could you please give us some more time to verify the issue
>> mentioned above?
> 
> No problem. I'll marked it as Awaiting upstream, please repost when
> ready. This is our usual process when maintainers of a subsystem need
> more time or need to do validation before we merge.
@Guanguan, I saw your answer and the tests result, thank you for the 
effort! It is totally fine to me now to let it go upstream.

@Jakub, thank you for giving us more time! It's verified now. I think 
they can be applied:-)

Thanks,
Wenjia

Patch

diff --git a/net/smc/af_smc.c b/net/smc/af_smc.c
index 9d76e902fd77..c3f9c0457418 100644
--- a/net/smc/af_smc.c
+++ b/net/smc/af_smc.c
@@ -1116,7 +1116,10 @@  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) ||
-	    smc->clcsock->sk->sk_family != AF_INET ||
+#if IS_ENABLED(CONFIG_IPV6)
+	    (smc->clcsock->sk->sk_family == AF_INET6 &&
+	     !ipv6_addr_v4mapped(&smc->clcsock->sk->sk_v6_rcv_saddr)) ||
+#endif
 	    !smc_clc_ueid_count() ||
 	    smc_find_rdma_device(smc, ini))
 		ini->smcr_version &= ~SMC_V2;