Re: [PATCH rdma-rc] RDMA/mlx5: Fix devlink deadlock on net namespace deletion

[Jason Gunthorpe <jgg@nvidia.com>]

On Mon, Oct 19, 2020 at 01:23:23PM +0000, Parav Pandit wrote:
> > > -	err = register_netdevice_notifier(&dev->port[port_num].roce.nb);
> > > +	err = register_netdevice_notifier_net(mlx5_core_net(dev->mdev),
> > > +					      &dev->port[port_num].roce.nb);
> > 
> > This looks racy, what lock needs to be held to keep *mlx5_core_net() stable?
> 
> mlx5_core_net() cannot be accessed outside of mlx5 driver's load, unload, reload path.
> 
> When this is getting executed, devlink cannot be executing reload.
> This is guarded by devlink_reload_enable/disable calls done by mlx5 core.

A comment that devlink_reload_enable/disable() must be held would be
helpful
 
> > 
> > >  	if (err) {
> > >  		dev->port[port_num].roce.nb.notifier_call = NULL;
> > >  		return err;
> > > @@ -3335,7 +3336,8 @@ static int mlx5_add_netdev_notifier(struct
> > > mlx5_ib_dev *dev, u8 port_num)  static void
> > > mlx5_remove_netdev_notifier(struct mlx5_ib_dev *dev, u8 port_num)  {
> > >  	if (dev->port[port_num].roce.nb.notifier_call) {
> > > -		unregister_netdevice_notifier(&dev-
> > >port[port_num].roce.nb);
> > > +		unregister_netdevice_notifier_net(mlx5_core_net(dev-
> > >mdev),
> > > +						  &dev-
> > >port[port_num].roce.nb);
> > 
> > This seems dangerous too, what if the mlx5_core_net changed before we
> > get here?
>
> When I inspected driver, code, I am not aware of any code flow where
> this can change before reaching here, because registration and
> unregistration is done only in driver load, unload and reload path.
> Reload can happen only after devlink_reload_enable() is done.

But we enable reload right after init_one

> > What are the rules for when devlink_net() changes?
> > 
> devlink_net() changes only after unload() callback is completed in driver.

You mean mlx5_devlink_reload_down ?

That seems OK then

Jason