Linux-RDMA Archive on lore.kernel.org
 help / color / Atom feed
* Re: [PATCHv5 for-next 1/1] RDMA/rxe: Disable ipv6 features when ipv6.disable in cmdline
  2021-04-13 23:42 [PATCHv5 for-next 1/1] RDMA/rxe: Disable ipv6 features when ipv6.disable in cmdline Zhu Yanjun
@ 2021-04-13 10:48 ` Leon Romanovsky
  2021-04-14  3:46   ` Zhu Yanjun
  2021-04-13 13:56 ` Kamal Heib
  1 sibling, 1 reply; 7+ messages in thread
From: Leon Romanovsky @ 2021-04-13 10:48 UTC (permalink / raw)
  To: Zhu Yanjun; +Cc: zyjzyj2000, dledford, jgg, linux-rdma, Yi Zhang

On Tue, Apr 13, 2021 at 07:42:52PM -0400, Zhu Yanjun wrote:
> From: Zhu Yanjun <zyjzyj2000@gmail.com>
> 
> When ipv6.disable=1 is set in cmdline, ipv6 is actually disabled
> in the stack. As such, the operations of ipv6 in RXE will fail.
> So ipv6 features in RXE should also be disabled in RXE.
> 
> Link: https://lore.kernel.org/linux-rdma/880d7b59-4b17-a44f-1a91-88257bfc3aaa@redhat.com/T/#t
> Fixes: 8700e3e7c4857 ("Soft RoCE driver")
> Reported-by: Yi Zhang <yi.zhang@redhat.com>
> Signed-off-by: Zhu Yanjun <zyjzyj2000@gmail.com>
> ---
> V4->V5: Clean up signature block and remove error message
> V3->V4: Check the returned value instead of ipv6 module
> V2->V3: Remove print message
> V1->V2: Modify the pr_info messages
> ---
>  drivers/infiniband/sw/rxe/rxe_net.c | 13 ++++++++++++-
>  1 file changed, 12 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/infiniband/sw/rxe/rxe_net.c b/drivers/infiniband/sw/rxe/rxe_net.c
> index 01662727dca0..984c3ac449bd 100644
> --- a/drivers/infiniband/sw/rxe/rxe_net.c
> +++ b/drivers/infiniband/sw/rxe/rxe_net.c
> @@ -208,7 +208,13 @@ static struct socket *rxe_setup_udp_tunnel(struct net *net, __be16 port,
>  	/* Create UDP socket */
>  	err = udp_sock_create(net, &udp_cfg, &sock);
>  	if (err < 0) {
> -		pr_err("failed to create udp socket. err = %d\n", err);
> +		/* If UDP tunnel over ipv6 fails with -EAFNOSUPPORT, the tunnel
> +		 * over ipv4 still works. This error message will not pop out.
> +		 * If UDP tunnle over ipv4 fails or other errors with ipv6
> +		 * tunnel, this error should pop out.
> +		 */
> +		if (!((err == -EAFNOSUPPORT) && (ipv6)))

You have too much brackets.

if (err != -EAFNOSUPPORT || !ipv6)))

> +			pr_err("failed to create udp socket. err = %d\n", err);
>  		return ERR_PTR(err);
>  	}
>  
> @@ -620,6 +626,11 @@ static int rxe_net_ipv6_init(void)
>  	recv_sockets.sk6 = rxe_setup_udp_tunnel(&init_net,
>  						htons(ROCE_V2_UDP_DPORT), true);
>  	if (IS_ERR(recv_sockets.sk6)) {
> +		/* Though IPv6 is not supported, IPv4 still needs to continue
> +		 */
> +		if (PTR_ERR(recv_sockets.sk6) == -EAFNOSUPPORT)
> +			return 0;
> +
>  		recv_sockets.sk6 = NULL;
>  		pr_err("Failed to create IPv6 UDP tunnel\n");
>  		return -1;
> -- 
> 2.27.0
> 

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCHv5 for-next 1/1] RDMA/rxe: Disable ipv6 features when ipv6.disable in cmdline
  2021-04-13 23:42 [PATCHv5 for-next 1/1] RDMA/rxe: Disable ipv6 features when ipv6.disable in cmdline Zhu Yanjun
  2021-04-13 10:48 ` Leon Romanovsky
@ 2021-04-13 13:56 ` Kamal Heib
  2021-04-13 14:39   ` Leon Romanovsky
  1 sibling, 1 reply; 7+ messages in thread
From: Kamal Heib @ 2021-04-13 13:56 UTC (permalink / raw)
  To: Zhu Yanjun, zyjzyj2000; +Cc: Yi Zhang, dledford, linux-rdma, jgg



On 4/14/21 2:42 AM, Zhu Yanjun wrote:
> From: Zhu Yanjun <zyjzyj2000@gmail.com>
> 
> When ipv6.disable=1 is set in cmdline, ipv6 is actually disabled
> in the stack. As such, the operations of ipv6 in RXE will fail.
> So ipv6 features in RXE should also be disabled in RXE.
> 
> Link: https://lore.kernel.org/linux-rdma/880d7b59-4b17-a44f-1a91-88257bfc3aaa@redhat.com/T/#t
> Fixes: 8700e3e7c4857 ("Soft RoCE driver")
> Reported-by: Yi Zhang <yi.zhang@redhat.com>
> Signed-off-by: Zhu Yanjun <zyjzyj2000@gmail.com>
> ---
> V4->V5: Clean up signature block and remove error message
> V3->V4: Check the returned value instead of ipv6 module
> V2->V3: Remove print message
> V1->V2: Modify the pr_info messages
> ---
>  drivers/infiniband/sw/rxe/rxe_net.c | 13 ++++++++++++-
>  1 file changed, 12 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/infiniband/sw/rxe/rxe_net.c b/drivers/infiniband/sw/rxe/rxe_net.c
> index 01662727dca0..984c3ac449bd 100644
> --- a/drivers/infiniband/sw/rxe/rxe_net.c
> +++ b/drivers/infiniband/sw/rxe/rxe_net.c
> @@ -208,7 +208,13 @@ static struct socket *rxe_setup_udp_tunnel(struct net *net, __be16 port,
>  	/* Create UDP socket */
>  	err = udp_sock_create(net, &udp_cfg, &sock);
>  	if (err < 0) {
> -		pr_err("failed to create udp socket. err = %d\n", err);
> +		/* If UDP tunnel over ipv6 fails with -EAFNOSUPPORT, the tunnel
> +		 * over ipv4 still works. This error message will not pop out.
> +		 * If UDP tunnle over ipv4 fails or other errors with ipv6
> +		 * tunnel, this error should pop out.
> +		 */
> +		if (!((err == -EAFNOSUPPORT) && (ipv6)))
> +			pr_err("failed to create udp socket. err = %d\n", err);
>  		return ERR_PTR(err);
>  	}
>  
> @@ -620,6 +626,11 @@ static int rxe_net_ipv6_init(void)
>  	recv_sockets.sk6 = rxe_setup_udp_tunnel(&init_net,
>  						htons(ROCE_V2_UDP_DPORT), true);
>  	if (IS_ERR(recv_sockets.sk6)) {
> +		/* Though IPv6 is not supported, IPv4 still needs to continue
> +		 */
> +		if (PTR_ERR(recv_sockets.sk6) == -EAFNOSUPPORT)
> +			return 0;
> +
>  		recv_sockets.sk6 = NULL;
>  		pr_err("Failed to create IPv6 UDP tunnel\n");
>  		return -1;
> 

I think the following change is much simpler than changing the udp_sock_create()
helper function?


diff --git a/drivers/infiniband/sw/rxe/rxe_net.c
b/drivers/infiniband/sw/rxe/rxe_net.c
index 01662727dca0..b56d6f76ab31 100644
--- a/drivers/infiniband/sw/rxe/rxe_net.c
+++ b/drivers/infiniband/sw/rxe/rxe_net.c
@@ -621,6 +621,11 @@ static int rxe_net_ipv6_init(void)
                                                htons(ROCE_V2_UDP_DPORT), true);
        if (IS_ERR(recv_sockets.sk6)) {
                recv_sockets.sk6 = NULL;
+               if (PTR_ERR(recv_sockets.sk6) == -EAFNOSUPPORT) {
+                       pr_warn("Create IPv6 UDP tunnel is not supported\n");
+                       return 0;
+               }
+
                pr_err("Failed to create IPv6 UDP tunnel\n");
                return -1;
        }
-- 
2.26.3


Thanks,
Kamal


^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCHv5 for-next 1/1] RDMA/rxe: Disable ipv6 features when ipv6.disable in cmdline
  2021-04-13 13:56 ` Kamal Heib
@ 2021-04-13 14:39   ` Leon Romanovsky
  2021-04-13 19:34     ` Kamal Heib
  0 siblings, 1 reply; 7+ messages in thread
From: Leon Romanovsky @ 2021-04-13 14:39 UTC (permalink / raw)
  To: Kamal Heib; +Cc: Zhu Yanjun, zyjzyj2000, Yi Zhang, dledford, linux-rdma, jgg

On Tue, Apr 13, 2021 at 04:56:05PM +0300, Kamal Heib wrote:
> 
> 
> On 4/14/21 2:42 AM, Zhu Yanjun wrote:
> > From: Zhu Yanjun <zyjzyj2000@gmail.com>
> > 
> > When ipv6.disable=1 is set in cmdline, ipv6 is actually disabled
> > in the stack. As such, the operations of ipv6 in RXE will fail.
> > So ipv6 features in RXE should also be disabled in RXE.
> > 
> > Link: https://lore.kernel.org/linux-rdma/880d7b59-4b17-a44f-1a91-88257bfc3aaa@redhat.com/T/#t
> > Fixes: 8700e3e7c4857 ("Soft RoCE driver")
> > Reported-by: Yi Zhang <yi.zhang@redhat.com>
> > Signed-off-by: Zhu Yanjun <zyjzyj2000@gmail.com>
> > ---
> > V4->V5: Clean up signature block and remove error message
> > V3->V4: Check the returned value instead of ipv6 module
> > V2->V3: Remove print message
> > V1->V2: Modify the pr_info messages
> > ---
> >  drivers/infiniband/sw/rxe/rxe_net.c | 13 ++++++++++++-
> >  1 file changed, 12 insertions(+), 1 deletion(-)
> > 
> > diff --git a/drivers/infiniband/sw/rxe/rxe_net.c b/drivers/infiniband/sw/rxe/rxe_net.c
> > index 01662727dca0..984c3ac449bd 100644
> > --- a/drivers/infiniband/sw/rxe/rxe_net.c
> > +++ b/drivers/infiniband/sw/rxe/rxe_net.c
> > @@ -208,7 +208,13 @@ static struct socket *rxe_setup_udp_tunnel(struct net *net, __be16 port,
> >  	/* Create UDP socket */
> >  	err = udp_sock_create(net, &udp_cfg, &sock);
> >  	if (err < 0) {
> > -		pr_err("failed to create udp socket. err = %d\n", err);
> > +		/* If UDP tunnel over ipv6 fails with -EAFNOSUPPORT, the tunnel
> > +		 * over ipv4 still works. This error message will not pop out.
> > +		 * If UDP tunnle over ipv4 fails or other errors with ipv6
> > +		 * tunnel, this error should pop out.
> > +		 */
> > +		if (!((err == -EAFNOSUPPORT) && (ipv6)))
> > +			pr_err("failed to create udp socket. err = %d\n", err);
> >  		return ERR_PTR(err);
> >  	}
> >  
> > @@ -620,6 +626,11 @@ static int rxe_net_ipv6_init(void)
> >  	recv_sockets.sk6 = rxe_setup_udp_tunnel(&init_net,
> >  						htons(ROCE_V2_UDP_DPORT), true);
> >  	if (IS_ERR(recv_sockets.sk6)) {
> > +		/* Though IPv6 is not supported, IPv4 still needs to continue
> > +		 */
> > +		if (PTR_ERR(recv_sockets.sk6) == -EAFNOSUPPORT)
> > +			return 0;
> > +
> >  		recv_sockets.sk6 = NULL;
> >  		pr_err("Failed to create IPv6 UDP tunnel\n");
> >  		return -1;
> > 
> 
> I think the following change is much simpler than changing the udp_sock_create()
> helper function?
> 
> 
> diff --git a/drivers/infiniband/sw/rxe/rxe_net.c
> b/drivers/infiniband/sw/rxe/rxe_net.c
> index 01662727dca0..b56d6f76ab31 100644
> --- a/drivers/infiniband/sw/rxe/rxe_net.c
> +++ b/drivers/infiniband/sw/rxe/rxe_net.c
> @@ -621,6 +621,11 @@ static int rxe_net_ipv6_init(void)
>                                                 htons(ROCE_V2_UDP_DPORT), true);
>         if (IS_ERR(recv_sockets.sk6)) {
>                 recv_sockets.sk6 = NULL;
> +               if (PTR_ERR(recv_sockets.sk6) == -EAFNOSUPPORT) {

You have "recv_sockets.sk6 = NULL;" in the line above.

> +                       pr_warn("Create IPv6 UDP tunnel is not supported\n");
> +                       return 0;
> +               }
> +
>                 pr_err("Failed to create IPv6 UDP tunnel\n");
>                 return -1;
>         }
> -- 
> 2.26.3
> 
> 
> Thanks,
> Kamal
> 

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCHv5 for-next 1/1] RDMA/rxe: Disable ipv6 features when ipv6.disable in cmdline
  2021-04-13 14:39   ` Leon Romanovsky
@ 2021-04-13 19:34     ` Kamal Heib
  2021-04-15 14:53       ` Zhu Yanjun
  0 siblings, 1 reply; 7+ messages in thread
From: Kamal Heib @ 2021-04-13 19:34 UTC (permalink / raw)
  To: Leon Romanovsky
  Cc: Zhu Yanjun, zyjzyj2000, Yi Zhang, dledford, linux-rdma, jgg



On 4/13/21 5:39 PM, Leon Romanovsky wrote:
> On Tue, Apr 13, 2021 at 04:56:05PM +0300, Kamal Heib wrote:
>>
>>
>> On 4/14/21 2:42 AM, Zhu Yanjun wrote:
>>> From: Zhu Yanjun <zyjzyj2000@gmail.com>
>>>
>>> When ipv6.disable=1 is set in cmdline, ipv6 is actually disabled
>>> in the stack. As such, the operations of ipv6 in RXE will fail.
>>> So ipv6 features in RXE should also be disabled in RXE.
>>>
>>> Link: https://lore.kernel.org/linux-rdma/880d7b59-4b17-a44f-1a91-88257bfc3aaa@redhat.com/T/#t
>>> Fixes: 8700e3e7c4857 ("Soft RoCE driver")
>>> Reported-by: Yi Zhang <yi.zhang@redhat.com>
>>> Signed-off-by: Zhu Yanjun <zyjzyj2000@gmail.com>
>>> ---
>>> V4->V5: Clean up signature block and remove error message
>>> V3->V4: Check the returned value instead of ipv6 module
>>> V2->V3: Remove print message
>>> V1->V2: Modify the pr_info messages
>>> ---
>>>  drivers/infiniband/sw/rxe/rxe_net.c | 13 ++++++++++++-
>>>  1 file changed, 12 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/drivers/infiniband/sw/rxe/rxe_net.c b/drivers/infiniband/sw/rxe/rxe_net.c
>>> index 01662727dca0..984c3ac449bd 100644
>>> --- a/drivers/infiniband/sw/rxe/rxe_net.c
>>> +++ b/drivers/infiniband/sw/rxe/rxe_net.c
>>> @@ -208,7 +208,13 @@ static struct socket *rxe_setup_udp_tunnel(struct net *net, __be16 port,
>>>  	/* Create UDP socket */
>>>  	err = udp_sock_create(net, &udp_cfg, &sock);
>>>  	if (err < 0) {
>>> -		pr_err("failed to create udp socket. err = %d\n", err);
>>> +		/* If UDP tunnel over ipv6 fails with -EAFNOSUPPORT, the tunnel
>>> +		 * over ipv4 still works. This error message will not pop out.
>>> +		 * If UDP tunnle over ipv4 fails or other errors with ipv6
>>> +		 * tunnel, this error should pop out.
>>> +		 */
>>> +		if (!((err == -EAFNOSUPPORT) && (ipv6)))
>>> +			pr_err("failed to create udp socket. err = %d\n", err);
>>>  		return ERR_PTR(err);
>>>  	}
>>>  
>>> @@ -620,6 +626,11 @@ static int rxe_net_ipv6_init(void)
>>>  	recv_sockets.sk6 = rxe_setup_udp_tunnel(&init_net,
>>>  						htons(ROCE_V2_UDP_DPORT), true);
>>>  	if (IS_ERR(recv_sockets.sk6)) {
>>> +		/* Though IPv6 is not supported, IPv4 still needs to continue
>>> +		 */
>>> +		if (PTR_ERR(recv_sockets.sk6) == -EAFNOSUPPORT)
>>> +			return 0;
>>> +
>>>  		recv_sockets.sk6 = NULL;
>>>  		pr_err("Failed to create IPv6 UDP tunnel\n");
>>>  		return -1;
>>>
>>
>> I think the following change is much simpler than changing the udp_sock_create()
>> helper function?
>>
>>
>> diff --git a/drivers/infiniband/sw/rxe/rxe_net.c
>> b/drivers/infiniband/sw/rxe/rxe_net.c
>> index 01662727dca0..b56d6f76ab31 100644
>> --- a/drivers/infiniband/sw/rxe/rxe_net.c
>> +++ b/drivers/infiniband/sw/rxe/rxe_net.c
>> @@ -621,6 +621,11 @@ static int rxe_net_ipv6_init(void)
>>                                                 htons(ROCE_V2_UDP_DPORT), true);
>>         if (IS_ERR(recv_sockets.sk6)) {
>>                 recv_sockets.sk6 = NULL;
>> +               if (PTR_ERR(recv_sockets.sk6) == -EAFNOSUPPORT) {
> 
> You have "recv_sockets.sk6 = NULL;" in the line above.
> 

Sorry, my bad...

The idea is to handle this issue in the error path of rxe_net_ipv6_init()
instead of changing the udp_sock_create(), also to make sure that
"recv_sockets.sk6" is set to NULL.

diff --git a/drivers/infiniband/sw/rxe/rxe_net.c
b/drivers/infiniband/sw/rxe/rxe_net.c
index 01662727dca0..445a47f82f42 100644
--- a/drivers/infiniband/sw/rxe/rxe_net.c
+++ b/drivers/infiniband/sw/rxe/rxe_net.c
@@ -615,17 +615,25 @@ static int rxe_net_ipv4_init(void)

 static int rxe_net_ipv6_init(void)
 {
+       int err = 0;
 #if IS_ENABLED(CONFIG_IPV6)

        recv_sockets.sk6 = rxe_setup_udp_tunnel(&init_net,
                                                htons(ROCE_V2_UDP_DPORT), true);
        if (IS_ERR(recv_sockets.sk6)) {
+
+               if (PTR_ERR(recv_sockets.sk6) == -EAFNOSUPPORT) {
+                       pr_warn("Create IPv6 UDP tunnel is not supported\n");
+                       err = 0;
+               } else {
+                       pr_err("Failed to create IPv6 UDP tunnel\n");
+                       err = -1;
+               }
+
                recv_sockets.sk6 = NULL;
-               pr_err("Failed to create IPv6 UDP tunnel\n");
-               return -1;
        }
 #endif
-       return 0;
+       return err;
 }

>> +                       pr_warn("Create IPv6 UDP tunnel is not supported\n");
>> +                       return 0;
>> +               }
>> +
>>                 pr_err("Failed to create IPv6 UDP tunnel\n");
>>                 return -1;
>>         }
>> -- 
>> 2.26.3
>>
>>
>> Thanks,
>> Kamal
>>
> 


^ permalink raw reply	[flat|nested] 7+ messages in thread

* [PATCHv5 for-next 1/1] RDMA/rxe: Disable ipv6 features when ipv6.disable in cmdline
@ 2021-04-13 23:42 Zhu Yanjun
  2021-04-13 10:48 ` Leon Romanovsky
  2021-04-13 13:56 ` Kamal Heib
  0 siblings, 2 replies; 7+ messages in thread
From: Zhu Yanjun @ 2021-04-13 23:42 UTC (permalink / raw)
  To: zyjzyj2000, dledford, jgg, linux-rdma; +Cc: Yi Zhang

From: Zhu Yanjun <zyjzyj2000@gmail.com>

When ipv6.disable=1 is set in cmdline, ipv6 is actually disabled
in the stack. As such, the operations of ipv6 in RXE will fail.
So ipv6 features in RXE should also be disabled in RXE.

Link: https://lore.kernel.org/linux-rdma/880d7b59-4b17-a44f-1a91-88257bfc3aaa@redhat.com/T/#t
Fixes: 8700e3e7c4857 ("Soft RoCE driver")
Reported-by: Yi Zhang <yi.zhang@redhat.com>
Signed-off-by: Zhu Yanjun <zyjzyj2000@gmail.com>
---
V4->V5: Clean up signature block and remove error message
V3->V4: Check the returned value instead of ipv6 module
V2->V3: Remove print message
V1->V2: Modify the pr_info messages
---
 drivers/infiniband/sw/rxe/rxe_net.c | 13 ++++++++++++-
 1 file changed, 12 insertions(+), 1 deletion(-)

diff --git a/drivers/infiniband/sw/rxe/rxe_net.c b/drivers/infiniband/sw/rxe/rxe_net.c
index 01662727dca0..984c3ac449bd 100644
--- a/drivers/infiniband/sw/rxe/rxe_net.c
+++ b/drivers/infiniband/sw/rxe/rxe_net.c
@@ -208,7 +208,13 @@ static struct socket *rxe_setup_udp_tunnel(struct net *net, __be16 port,
 	/* Create UDP socket */
 	err = udp_sock_create(net, &udp_cfg, &sock);
 	if (err < 0) {
-		pr_err("failed to create udp socket. err = %d\n", err);
+		/* If UDP tunnel over ipv6 fails with -EAFNOSUPPORT, the tunnel
+		 * over ipv4 still works. This error message will not pop out.
+		 * If UDP tunnle over ipv4 fails or other errors with ipv6
+		 * tunnel, this error should pop out.
+		 */
+		if (!((err == -EAFNOSUPPORT) && (ipv6)))
+			pr_err("failed to create udp socket. err = %d\n", err);
 		return ERR_PTR(err);
 	}
 
@@ -620,6 +626,11 @@ static int rxe_net_ipv6_init(void)
 	recv_sockets.sk6 = rxe_setup_udp_tunnel(&init_net,
 						htons(ROCE_V2_UDP_DPORT), true);
 	if (IS_ERR(recv_sockets.sk6)) {
+		/* Though IPv6 is not supported, IPv4 still needs to continue
+		 */
+		if (PTR_ERR(recv_sockets.sk6) == -EAFNOSUPPORT)
+			return 0;
+
 		recv_sockets.sk6 = NULL;
 		pr_err("Failed to create IPv6 UDP tunnel\n");
 		return -1;
-- 
2.27.0


^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCHv5 for-next 1/1] RDMA/rxe: Disable ipv6 features when ipv6.disable in cmdline
  2021-04-13 10:48 ` Leon Romanovsky
@ 2021-04-14  3:46   ` Zhu Yanjun
  0 siblings, 0 replies; 7+ messages in thread
From: Zhu Yanjun @ 2021-04-14  3:46 UTC (permalink / raw)
  To: Leon Romanovsky
  Cc: Zhu Yanjun, Doug Ledford, Jason Gunthorpe, RDMA mailing list, Yi Zhang

On Tue, Apr 13, 2021 at 6:48 PM Leon Romanovsky <leon@kernel.org> wrote:
>
> On Tue, Apr 13, 2021 at 07:42:52PM -0400, Zhu Yanjun wrote:
> > From: Zhu Yanjun <zyjzyj2000@gmail.com>
> >
> > When ipv6.disable=1 is set in cmdline, ipv6 is actually disabled
> > in the stack. As such, the operations of ipv6 in RXE will fail.
> > So ipv6 features in RXE should also be disabled in RXE.
> >
> > Link: https://lore.kernel.org/linux-rdma/880d7b59-4b17-a44f-1a91-88257bfc3aaa@redhat.com/T/#t
> > Fixes: 8700e3e7c4857 ("Soft RoCE driver")
> > Reported-by: Yi Zhang <yi.zhang@redhat.com>
> > Signed-off-by: Zhu Yanjun <zyjzyj2000@gmail.com>
> > ---
> > V4->V5: Clean up signature block and remove error message
> > V3->V4: Check the returned value instead of ipv6 module
> > V2->V3: Remove print message
> > V1->V2: Modify the pr_info messages
> > ---
> >  drivers/infiniband/sw/rxe/rxe_net.c | 13 ++++++++++++-
> >  1 file changed, 12 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/infiniband/sw/rxe/rxe_net.c b/drivers/infiniband/sw/rxe/rxe_net.c
> > index 01662727dca0..984c3ac449bd 100644
> > --- a/drivers/infiniband/sw/rxe/rxe_net.c
> > +++ b/drivers/infiniband/sw/rxe/rxe_net.c
> > @@ -208,7 +208,13 @@ static struct socket *rxe_setup_udp_tunnel(struct net *net, __be16 port,
> >       /* Create UDP socket */
> >       err = udp_sock_create(net, &udp_cfg, &sock);
> >       if (err < 0) {
> > -             pr_err("failed to create udp socket. err = %d\n", err);
> > +             /* If UDP tunnel over ipv6 fails with -EAFNOSUPPORT, the tunnel
> > +              * over ipv4 still works. This error message will not pop out.
> > +              * If UDP tunnle over ipv4 fails or other errors with ipv6
> > +              * tunnel, this error should pop out.
> > +              */
> > +             if (!((err == -EAFNOSUPPORT) && (ipv6)))
>
> You have too much brackets.

My code corresponds with the comments.

Zhu Yanjun

>
> if (err != -EAFNOSUPPORT || !ipv6)))
>
> > +                     pr_err("failed to create udp socket. err = %d\n", err);
> >               return ERR_PTR(err);
> >       }
> >
> > @@ -620,6 +626,11 @@ static int rxe_net_ipv6_init(void)
> >       recv_sockets.sk6 = rxe_setup_udp_tunnel(&init_net,
> >                                               htons(ROCE_V2_UDP_DPORT), true);
> >       if (IS_ERR(recv_sockets.sk6)) {
> > +             /* Though IPv6 is not supported, IPv4 still needs to continue
> > +              */
> > +             if (PTR_ERR(recv_sockets.sk6) == -EAFNOSUPPORT)
> > +                     return 0;
> > +
> >               recv_sockets.sk6 = NULL;
> >               pr_err("Failed to create IPv6 UDP tunnel\n");
> >               return -1;
> > --
> > 2.27.0
> >

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCHv5 for-next 1/1] RDMA/rxe: Disable ipv6 features when ipv6.disable in cmdline
  2021-04-13 19:34     ` Kamal Heib
@ 2021-04-15 14:53       ` Zhu Yanjun
  0 siblings, 0 replies; 7+ messages in thread
From: Zhu Yanjun @ 2021-04-15 14:53 UTC (permalink / raw)
  To: Kamal Heib
  Cc: Leon Romanovsky, Zhu Yanjun, Yi Zhang, Doug Ledford,
	RDMA mailing list, Jason Gunthorpe

On Wed, Apr 14, 2021 at 3:34 AM Kamal Heib <kheib@redhat.com> wrote:
>
>
>
> On 4/13/21 5:39 PM, Leon Romanovsky wrote:
> > On Tue, Apr 13, 2021 at 04:56:05PM +0300, Kamal Heib wrote:
> >>
> >>
> >> On 4/14/21 2:42 AM, Zhu Yanjun wrote:
> >>> From: Zhu Yanjun <zyjzyj2000@gmail.com>
> >>>
> >>> When ipv6.disable=1 is set in cmdline, ipv6 is actually disabled
> >>> in the stack. As such, the operations of ipv6 in RXE will fail.
> >>> So ipv6 features in RXE should also be disabled in RXE.
> >>>
> >>> Link: https://lore.kernel.org/linux-rdma/880d7b59-4b17-a44f-1a91-88257bfc3aaa@redhat.com/T/#t
> >>> Fixes: 8700e3e7c4857 ("Soft RoCE driver")
> >>> Reported-by: Yi Zhang <yi.zhang@redhat.com>
> >>> Signed-off-by: Zhu Yanjun <zyjzyj2000@gmail.com>
> >>> ---
> >>> V4->V5: Clean up signature block and remove error message
> >>> V3->V4: Check the returned value instead of ipv6 module
> >>> V2->V3: Remove print message
> >>> V1->V2: Modify the pr_info messages
> >>> ---
> >>>  drivers/infiniband/sw/rxe/rxe_net.c | 13 ++++++++++++-
> >>>  1 file changed, 12 insertions(+), 1 deletion(-)
> >>>
> >>> diff --git a/drivers/infiniband/sw/rxe/rxe_net.c b/drivers/infiniband/sw/rxe/rxe_net.c
> >>> index 01662727dca0..984c3ac449bd 100644
> >>> --- a/drivers/infiniband/sw/rxe/rxe_net.c
> >>> +++ b/drivers/infiniband/sw/rxe/rxe_net.c
> >>> @@ -208,7 +208,13 @@ static struct socket *rxe_setup_udp_tunnel(struct net *net, __be16 port,
> >>>     /* Create UDP socket */
> >>>     err = udp_sock_create(net, &udp_cfg, &sock);
> >>>     if (err < 0) {
> >>> -           pr_err("failed to create udp socket. err = %d\n", err);
> >>> +           /* If UDP tunnel over ipv6 fails with -EAFNOSUPPORT, the tunnel
> >>> +            * over ipv4 still works. This error message will not pop out.
> >>> +            * If UDP tunnle over ipv4 fails or other errors with ipv6
> >>> +            * tunnel, this error should pop out.
> >>> +            */
> >>> +           if (!((err == -EAFNOSUPPORT) && (ipv6)))
> >>> +                   pr_err("failed to create udp socket. err = %d\n", err);
> >>>             return ERR_PTR(err);
> >>>     }
> >>>
> >>> @@ -620,6 +626,11 @@ static int rxe_net_ipv6_init(void)
> >>>     recv_sockets.sk6 = rxe_setup_udp_tunnel(&init_net,
> >>>                                             htons(ROCE_V2_UDP_DPORT), true);
> >>>     if (IS_ERR(recv_sockets.sk6)) {
> >>> +           /* Though IPv6 is not supported, IPv4 still needs to continue
> >>> +            */
> >>> +           if (PTR_ERR(recv_sockets.sk6) == -EAFNOSUPPORT)
> >>> +                   return 0;
> >>> +
> >>>             recv_sockets.sk6 = NULL;
> >>>             pr_err("Failed to create IPv6 UDP tunnel\n");
> >>>             return -1;
> >>>
> >>
> >> I think the following change is much simpler than changing the udp_sock_create()
> >> helper function?
> >>
> >>
> >> diff --git a/drivers/infiniband/sw/rxe/rxe_net.c
> >> b/drivers/infiniband/sw/rxe/rxe_net.c
> >> index 01662727dca0..b56d6f76ab31 100644
> >> --- a/drivers/infiniband/sw/rxe/rxe_net.c
> >> +++ b/drivers/infiniband/sw/rxe/rxe_net.c
> >> @@ -621,6 +621,11 @@ static int rxe_net_ipv6_init(void)
> >>                                                 htons(ROCE_V2_UDP_DPORT), true);
> >>         if (IS_ERR(recv_sockets.sk6)) {
> >>                 recv_sockets.sk6 = NULL;
> >> +               if (PTR_ERR(recv_sockets.sk6) == -EAFNOSUPPORT) {
> >
> > You have "recv_sockets.sk6 = NULL;" in the line above.
> >
>
> Sorry, my bad...
>
> The idea is to handle this issue in the error path of rxe_net_ipv6_init()
> instead of changing the udp_sock_create(), also to make sure that

We do not change udp_sock_create.

> "recv_sockets.sk6" is set to NULL.

Is it necessary to set recv_sockets_sk6 to NULL?

Zhu Yanjun
>
> diff --git a/drivers/infiniband/sw/rxe/rxe_net.c
> b/drivers/infiniband/sw/rxe/rxe_net.c
> index 01662727dca0..445a47f82f42 100644
> --- a/drivers/infiniband/sw/rxe/rxe_net.c
> +++ b/drivers/infiniband/sw/rxe/rxe_net.c
> @@ -615,17 +615,25 @@ static int rxe_net_ipv4_init(void)
>
>  static int rxe_net_ipv6_init(void)
>  {
> +       int err = 0;
>  #if IS_ENABLED(CONFIG_IPV6)
>
>         recv_sockets.sk6 = rxe_setup_udp_tunnel(&init_net,
>                                                 htons(ROCE_V2_UDP_DPORT), true);
>         if (IS_ERR(recv_sockets.sk6)) {
> +
> +               if (PTR_ERR(recv_sockets.sk6) == -EAFNOSUPPORT) {
> +                       pr_warn("Create IPv6 UDP tunnel is not supported\n");
> +                       err = 0;
> +               } else {
> +                       pr_err("Failed to create IPv6 UDP tunnel\n");
> +                       err = -1;
> +               }
> +
>                 recv_sockets.sk6 = NULL;
> -               pr_err("Failed to create IPv6 UDP tunnel\n");
> -               return -1;
>         }
>  #endif
> -       return 0;
> +       return err;
>  }
>
> >> +                       pr_warn("Create IPv6 UDP tunnel is not supported\n");
> >> +                       return 0;
> >> +               }
> >> +
> >>                 pr_err("Failed to create IPv6 UDP tunnel\n");
> >>                 return -1;
> >>         }
> >> --
> >> 2.26.3
> >>
> >>
> >> Thanks,
> >> Kamal
> >>
> >
>

^ permalink raw reply	[flat|nested] 7+ messages in thread

end of thread, back to index

Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-04-13 23:42 [PATCHv5 for-next 1/1] RDMA/rxe: Disable ipv6 features when ipv6.disable in cmdline Zhu Yanjun
2021-04-13 10:48 ` Leon Romanovsky
2021-04-14  3:46   ` Zhu Yanjun
2021-04-13 13:56 ` Kamal Heib
2021-04-13 14:39   ` Leon Romanovsky
2021-04-13 19:34     ` Kamal Heib
2021-04-15 14:53       ` Zhu Yanjun

Linux-RDMA Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/linux-rdma/0 linux-rdma/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 linux-rdma linux-rdma/ https://lore.kernel.org/linux-rdma \
		linux-rdma@vger.kernel.org
	public-inbox-index linux-rdma

Example config snippet for mirrors

Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.kernel.vger.linux-rdma


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git