[PATCH v2] RDMA: Fix netdev tracker in ib_device_set_netdev

[PATCH v2] RDMA: Fix netdev tracker in ib_device_set_netdev

[David Ahern]

If a netdev has already been assigned, ib_device_set_netdev needs to
release the reference on the older netdev but it is mistakenly being
called for the new netdev. Fix it and in the process use netdev_put
to be symmetrical with the netdev_hold.

Fixes: 09f530f0c6d6 ("RDMA: Add netdevice_tracker to ib_device_set_netdev()")
Signed-off-by: David Ahern <dsahern@kernel.org>
---
v2
- remove __dev_put now that netdev_put is used

 drivers/infiniband/core/device.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/drivers/infiniband/core/device.c b/drivers/infiniband/core/device.c
index 55aa7aa32d4a..9b99112baf47 100644
--- a/drivers/infiniband/core/device.c
+++ b/drivers/infiniband/core/device.c
@@ -2167,14 +2167,13 @@ int ib_device_set_netdev(struct ib_device *ib_dev, struct net_device *ndev,
 	}
 
 	if (old_ndev)
-		netdev_tracker_free(ndev, &pdata->netdev_tracker);
+		netdev_put(old_ndev, &pdata->netdev_tracker);
 	if (ndev)
 		netdev_hold(ndev, &pdata->netdev_tracker, GFP_ATOMIC);
 	rcu_assign_pointer(pdata->netdev, ndev);
 	spin_unlock_irqrestore(&pdata->netdev_lock, flags);
 
 	add_ndev_hash(pdata);
-	__dev_put(old_ndev);
 
 	return 0;
 }
-- 
2.39.3 (Apple Git-146)

Re: [PATCH v2] RDMA: Fix netdev tracker in ib_device_set_netdev

[Leon Romanovsky]

On Wed, Jul 10, 2024 at 01:33:10PM -0700, David Ahern wrote:
> If a netdev has already been assigned, ib_device_set_netdev needs to
> release the reference on the older netdev but it is mistakenly being
> called for the new netdev. Fix it and in the process use netdev_put
> to be symmetrical with the netdev_hold.
> 
> Fixes: 09f530f0c6d6 ("RDMA: Add netdevice_tracker to ib_device_set_netdev()")
> Signed-off-by: David Ahern <dsahern@kernel.org>
> ---
> v2
> - remove __dev_put now that netdev_put is used
> 
>  drivers/infiniband/core/device.c | 3 +--
>  1 file changed, 1 insertion(+), 2 deletions(-)
> 
> diff --git a/drivers/infiniband/core/device.c b/drivers/infiniband/core/device.c
> index 55aa7aa32d4a..9b99112baf47 100644
> --- a/drivers/infiniband/core/device.c
> +++ b/drivers/infiniband/core/device.c
> @@ -2167,14 +2167,13 @@ int ib_device_set_netdev(struct ib_device *ib_dev, struct net_device *ndev,
>  	}
>  
>  	if (old_ndev)
> -		netdev_tracker_free(ndev, &pdata->netdev_tracker);
> +		netdev_put(old_ndev, &pdata->netdev_tracker);
>  	if (ndev)
>  		netdev_hold(ndev, &pdata->netdev_tracker, GFP_ATOMIC);
>  	rcu_assign_pointer(pdata->netdev, ndev);
>  	spin_unlock_irqrestore(&pdata->netdev_lock, flags);
>  
>  	add_ndev_hash(pdata);
> -	__dev_put(old_ndev);

I applied with the following change:
diff --git a/drivers/infiniband/core/device.c b/drivers/infiniband/core/device.c
index 49a2ddaf5a38..1f867b7f7020 100644
--- a/drivers/infiniband/core/device.c
+++ b/drivers/infiniband/core/device.c
@@ -2184,14 +2184,12 @@ int ib_device_set_netdev(struct ib_device *ib_dev, struct net_device *ndev,
                return 0;
        }

-       if (old_ndev)
-               netdev_put(old_ndev, &pdata->netdev_tracker);
-       if (ndev)
-               netdev_hold(ndev, &pdata->netdev_tracker, GFP_ATOMIC);
+       netdev_hold(ndev, &pdata->netdev_tracker, GFP_ATOMIC);
        rcu_assign_pointer(pdata->netdev, ndev);
        spin_unlock_irqrestore(&pdata->netdev_lock, flags);

        add_ndev_hash(pdata);
+       netdev_put(old_ndev, &pdata->netdev_tracker);

        return 0;
 }


>  
>  	return 0;
>  }
> -- 
> 2.39.3 (Apple Git-146)
> 
> 

Re: [PATCH v2] RDMA: Fix netdev tracker in ib_device_set_netdev

[Leon Romanovsky]

On Thu, Jul 11, 2024 at 01:12:37PM +0300, Leon Romanovsky wrote:
> On Wed, Jul 10, 2024 at 01:33:10PM -0700, David Ahern wrote:
> > If a netdev has already been assigned, ib_device_set_netdev needs to
> > release the reference on the older netdev but it is mistakenly being
> > called for the new netdev. Fix it and in the process use netdev_put
> > to be symmetrical with the netdev_hold.
> > 
> > Fixes: 09f530f0c6d6 ("RDMA: Add netdevice_tracker to ib_device_set_netdev()")
> > Signed-off-by: David Ahern <dsahern@kernel.org>
> > ---
> > v2
> > - remove __dev_put now that netdev_put is used
> > 
> >  drivers/infiniband/core/device.c | 3 +--
> >  1 file changed, 1 insertion(+), 2 deletions(-)
> > 
> > diff --git a/drivers/infiniband/core/device.c b/drivers/infiniband/core/device.c
> > index 55aa7aa32d4a..9b99112baf47 100644
> > --- a/drivers/infiniband/core/device.c
> > +++ b/drivers/infiniband/core/device.c
> > @@ -2167,14 +2167,13 @@ int ib_device_set_netdev(struct ib_device *ib_dev, struct net_device *ndev,
> >  	}
> >  
> >  	if (old_ndev)
> > -		netdev_tracker_free(ndev, &pdata->netdev_tracker);
> > +		netdev_put(old_ndev, &pdata->netdev_tracker);
> >  	if (ndev)
> >  		netdev_hold(ndev, &pdata->netdev_tracker, GFP_ATOMIC);
> >  	rcu_assign_pointer(pdata->netdev, ndev);
> >  	spin_unlock_irqrestore(&pdata->netdev_lock, flags);
> >  
> >  	add_ndev_hash(pdata);
> > -	__dev_put(old_ndev);
> 
> I applied with the following change:
> diff --git a/drivers/infiniband/core/device.c b/drivers/infiniband/core/device.c
> index 49a2ddaf5a38..1f867b7f7020 100644
> --- a/drivers/infiniband/core/device.c
> +++ b/drivers/infiniband/core/device.c
> @@ -2184,14 +2184,12 @@ int ib_device_set_netdev(struct ib_device *ib_dev, struct net_device *ndev,
>                 return 0;
>         }
> 
> -       if (old_ndev)
> -               netdev_put(old_ndev, &pdata->netdev_tracker);
> -       if (ndev)
> -               netdev_hold(ndev, &pdata->netdev_tracker, GFP_ATOMIC);
> +       netdev_hold(ndev, &pdata->netdev_tracker, GFP_ATOMIC);
>         rcu_assign_pointer(pdata->netdev, ndev);
>         spin_unlock_irqrestore(&pdata->netdev_lock, flags);
> 
>         add_ndev_hash(pdata);
> +       netdev_put(old_ndev, &pdata->netdev_tracker);
> 
>         return 0;
>  }

The final version is:
diff --git a/drivers/infiniband/core/device.c b/drivers/infiniband/core/device.c
index 49a2ddaf5a38..9c31eef9f590 100644
--- a/drivers/infiniband/core/device.c
+++ b/drivers/infiniband/core/device.c
@@ -2184,15 +2184,12 @@ int ib_device_set_netdev(struct ib_device *ib_dev, struct net_device *ndev,
                return 0;
        }

-       if (old_ndev)
-               netdev_put(old_ndev, &pdata->netdev_tracker);
-       if (ndev)
-               netdev_hold(ndev, &pdata->netdev_tracker, GFP_ATOMIC);
+       netdev_put(old_ndev, &pdata->netdev_tracker);
+       netdev_hold(ndev, &pdata->netdev_tracker, GFP_ATOMIC);
        rcu_assign_pointer(pdata->netdev, ndev);
        spin_unlock_irqrestore(&pdata->netdev_lock, flags);

        add_ndev_hash(pdata);
-
        return 0;
 }
 EXPORT_SYMBOL(ib_device_set_netdev);


> 
> 
> >  
> >  	return 0;
> >  }
> > -- 
> > 2.39.3 (Apple Git-146)
> > 
> > 
> 

Re: [PATCH v2] RDMA: Fix netdev tracker in ib_device_set_netdev

[Leon Romanovsky]

On Wed, 10 Jul 2024 13:33:10 -0700, David Ahern wrote:
> If a netdev has already been assigned, ib_device_set_netdev needs to
> release the reference on the older netdev but it is mistakenly being
> called for the new netdev. Fix it and in the process use netdev_put
> to be symmetrical with the netdev_hold.
> 
> 

Applied, thanks!

[1/1] RDMA: Fix netdev tracker in ib_device_set_netdev
      https://git.kernel.org/rdma/rdma/c/bbff20fb3e6a00

Best regards,
-- 
Leon Romanovsky <leon@kernel.org>