Date:   Wed, 17 Nov 2021 16:15:49 +0200
From:   Leon Romanovsky <leon@kernel.org>
To:     Jason Gunthorpe <jgg@nvidia.com>
Cc:     Jiri Pirko <jiri@resnulli.us>, Jakub Kicinski <kuba@kernel.org>,
        Jiri Pirko <jiri@nvidia.com>, Ido Schimmel <idosch@idosch.org>,
        "David S . Miller" <davem@davemloft.net>,
        linux-kernel@vger.kernel.org, netdev@vger.kernel.org,
        edwin.peer@broadcom.com
Subject: Re: [PATCH net-next] devlink: Require devlink lock during device
 reload
Message-ID: <YZUOlQnQ0NVxRaO/@unreal>
References: <YY0G90fJpu/OtF8L@nanopsycho>
 <YY0J8IOLQBBhok2M@unreal>
 <YY4aEFkVuqR+vauw@nanopsycho>
 <YZCqVig9GQi/o1iz@unreal>
 <YZJCdSy+wzqlwrE2@nanopsycho>
 <20211115125359.GM2105516@nvidia.com>
 <YZJx8raQt+FkKaeY@nanopsycho>
 <20211115150931.GA2386342@nvidia.com>
 <YZNWRXzzRYMNhUEO@nanopsycho>
 <20211116124442.GX2105516@nvidia.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <20211116124442.GX2105516@nvidia.com>
Precedence: bulk

On Tue, Nov 16, 2021 at 08:44:42AM -0400, Jason Gunthorpe wrote:
> On Tue, Nov 16, 2021 at 07:57:09AM +0100, Jiri Pirko wrote:
> 
> > >There is only one place in the entire kernel calling the per-ns
> > >register_netdevice_notifier_dev_net() and it is burred inside another
> > >part of mlx5 for some reason..
> > 
> > Yep. I added it there to solve this deadlock.
> 
> I wonder how it can work safely inside a driver, since when are
> drivers NS aware?
> 
>         uplink_priv->bond->nb.notifier_call = mlx5e_rep_esw_bond_netevent;
>         ret = register_netdevice_notifier_dev_net(netdev,
>                                                   &uplink_priv->bond->nb,
>                                                   &uplink_priv->bond->nn);
> 
> Doesn't that just loose events when the user moves netdev to another
> namespace?

I don't think so, it looks like holding rtnl_lock is enough.
However, we need all events and not NS-specific ones and it maybe solves
the deadlock, but doesn't solve our issue.

BTW, this makes me wonder how commit 554873e51711 ("net: Do not take net_rwsem in __rtnl_link_unregister()")
aligns with the comment near pernet_ops_rwsem and explode usage in other
places.

    57 /*
    58  * pernet_ops_rwsem: protects: pernet_list, net_generic_ids,
    59  * init_net_initialized and first_device pointer.
    60  * This is internal net namespace object. Please, don't use it
    61  * outside.
    62  */

Thanks