diff mbox series

[v6.4-rc1,v5,1/8] RDMA/rxe: Creating listening sock in newlink function

Message ID 20230508075636.352138-2-yanjun.zhu@intel.com (mailing list archive)
State Superseded
Headers show
Series Fix the problem that rxe can not work in net namespace | expand

Commit Message

Zhu Yanjun May 8, 2023, 7:56 a.m. UTC 
From: Zhu Yanjun <yanjun.zhu@linux.dev>

Originally when the module rdma_rxe is loaded, the sock listening on udp
port 4791 is created. Currently moving the creating listening port to
newlink function.

So when running "rdma link add" command, the sock listening on udp port
4791 is created.

Tested-by: Rain River <rain.1986.08.12@gmail.com>
Signed-off-by: Zhu Yanjun <yanjun.zhu@linux.dev>
---
 drivers/infiniband/sw/rxe/rxe.c | 10 ++++------
 1 file changed, 4 insertions(+), 6 deletions(-)

Comments

Bob Pearson June 20, 2023, 5:16 p.m. UTC | #1 
On 5/8/23 02:56, Zhu Yanjun wrote:
> From: Zhu Yanjun <yanjun.zhu@linux.dev>
> 
> Originally when the module rdma_rxe is loaded, the sock listening on udp
> port 4791 is created. Currently moving the creating listening port to
> newlink function.
> 
> So when running "rdma link add" command, the sock listening on udp port
> 4791 is created.
> 
> Tested-by: Rain River <rain.1986.08.12@gmail.com>
> Signed-off-by: Zhu Yanjun <yanjun.zhu@linux.dev>
> ---
>  drivers/infiniband/sw/rxe/rxe.c | 10 ++++------
>  1 file changed, 4 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/infiniband/sw/rxe/rxe.c b/drivers/infiniband/sw/rxe/rxe.c
> index 7a7e713de52d..89b24bc34299 100644
> --- a/drivers/infiniband/sw/rxe/rxe.c
> +++ b/drivers/infiniband/sw/rxe/rxe.c
> @@ -194,6 +194,10 @@ static int rxe_newlink(const char *ibdev_name, struct net_device *ndev)
>  		goto err;
>  	}
>  
> +	err = rxe_net_init();
> +	if (err)
> +		return err;
> +
If you put this here you cannot create more than one rxe device.
E.g. if you type

sudo rdma link add rxe0 type rxe netdev enp6s0
sudo rdma link add rxe1 type rxe netdev lo

the second call will fail. This worked before this patch. Maybe you will fix later but
by itself this patch breaks the driver.

Bob
>  	err = rxe_net_add(ibdev_name, ndev);
>  	if (err) {
>  		rxe_err("failed to add %s\n", ndev->name);
> @@ -210,12 +214,6 @@ static struct rdma_link_ops rxe_link_ops = {
>  
>  static int __init rxe_module_init(void)
>  {
> -	int err;
> -
> -	err = rxe_net_init();
> -	if (err)
> -		return err;
> -
>  	rdma_link_register(&rxe_link_ops);
>  	pr_info("loaded\n");
>  	return 0;
Zhu Yanjun June 20, 2023, 11:40 p.m. UTC | #2 
在 2023/6/21 1:16, Bob Pearson 写道:
> On 5/8/23 02:56, Zhu Yanjun wrote:
>> From: Zhu Yanjun <yanjun.zhu@linux.dev>
>>
>> Originally when the module rdma_rxe is loaded, the sock listening on udp
>> port 4791 is created. Currently moving the creating listening port to
>> newlink function.
>>
>> So when running "rdma link add" command, the sock listening on udp port
>> 4791 is created.
>>
>> Tested-by: Rain River <rain.1986.08.12@gmail.com>
>> Signed-off-by: Zhu Yanjun <yanjun.zhu@linux.dev>
>> ---
>>   drivers/infiniband/sw/rxe/rxe.c | 10 ++++------
>>   1 file changed, 4 insertions(+), 6 deletions(-)
>>
>> diff --git a/drivers/infiniband/sw/rxe/rxe.c b/drivers/infiniband/sw/rxe/rxe.c
>> index 7a7e713de52d..89b24bc34299 100644
>> --- a/drivers/infiniband/sw/rxe/rxe.c
>> +++ b/drivers/infiniband/sw/rxe/rxe.c
>> @@ -194,6 +194,10 @@ static int rxe_newlink(const char *ibdev_name, struct net_device *ndev)
>>   		goto err;
>>   	}
>>   
>> +	err = rxe_net_init();
>> +	if (err)
>> +		return err;
>> +
> If you put this here you cannot create more than one rxe device.
> E.g. if you type
>
> sudo rdma link add rxe0 type rxe netdev enp6s0
> sudo rdma link add rxe1 type rxe netdev lo
>
> the second call will fail. This worked before this patch. Maybe you will fix later but
> by itself this patch breaks the driver.

Hi, Bob

Thanks a lot for your code review.

I made tests. The followings are results. If we add the secode rxe1, the 
second rxe can be created.

# rdma link add rxe0 type rxe netdev eno12399np0
# rdma link add rxe1 type rxe netdev ens7f1np1
# rdma link

link rxe0/1 state ACTIVE physical_state LINK_UP netdev eno12399np0
link rxe1/1 state ACTIVE physical_state LINK_UP netdev ens7f1np1

And the followings are the port 4791 after rxe devices are created.

# ss -lun
State              Recv-Q Send-Q                            Local 
Address:Port                            Peer Address:Port             
Process
...
UNCONN             0 0 0.0.0.0:4791                                 
0.0.0.0:*
...
UNCONN             0 0 [::]:4791                                    [::]:*
..

# rdma link del rxe0
# rdma link del rxe1

After the rxe devices are removed, the port 4791 is removed.

# ss -lun | grep 4791
State              Recv-Q Send-Q                            Local 
Address:Port                            Peer Address:Port             
Process

Zhu Yanjun

>
> Bob
>>   	err = rxe_net_add(ibdev_name, ndev);
>>   	if (err) {
>>   		rxe_err("failed to add %s\n", ndev->name);
>> @@ -210,12 +214,6 @@ static struct rdma_link_ops rxe_link_ops = {
>>   
>>   static int __init rxe_module_init(void)
>>   {
>> -	int err;
>> -
>> -	err = rxe_net_init();
>> -	if (err)
>> -		return err;
>> -
>>   	rdma_link_register(&rxe_link_ops);
>>   	pr_info("loaded\n");
>>   	return 0;
diff mbox series

Patch

diff --git a/drivers/infiniband/sw/rxe/rxe.c b/drivers/infiniband/sw/rxe/rxe.c
index 7a7e713de52d..89b24bc34299 100644
--- a/drivers/infiniband/sw/rxe/rxe.c
+++ b/drivers/infiniband/sw/rxe/rxe.c
@@ -194,6 +194,10 @@  static int rxe_newlink(const char *ibdev_name, struct net_device *ndev)
 		goto err;
 	}
 
+	err = rxe_net_init();
+	if (err)
+		return err;
+
 	err = rxe_net_add(ibdev_name, ndev);
 	if (err) {
 		rxe_err("failed to add %s\n", ndev->name);
@@ -210,12 +214,6 @@  static struct rdma_link_ops rxe_link_ops = {
 
 static int __init rxe_module_init(void)
 {
-	int err;
-
-	err = rxe_net_init();
-	if (err)
-		return err;
-
 	rdma_link_register(&rxe_link_ops);
 	pr_info("loaded\n");
 	return 0;