Re: [PATCH net] net: dsa: link interfaces with the DSA master to get rid of lockdep warnings

[Vladimir Oltean <olteanv@gmail.com>]

On Mon, Jul 13, 2020 at 08:33:19PM +0300, Vladimir Oltean wrote:
> On Mon, Jul 13, 2020 at 08:30:49PM +0300, Vladimir Oltean wrote:
> > Hi Andrew,
> > 
> > On Mon, Jul 13, 2020 at 06:47:28PM +0200, Andrew Lunn wrote:
> > > > diff --git a/net/dsa/slave.c b/net/dsa/slave.c
> > > > index 743caabeaaa6..a951b2a7d79a 100644
> > > > --- a/net/dsa/slave.c
> > > > +++ b/net/dsa/slave.c
> > > > @@ -1994,6 +1994,13 @@ int dsa_slave_create(struct dsa_port *port)
> > > >  			   ret, slave_dev->name);
> > > >  		goto out_phy;
> > > >  	}
> > > > +	rtnl_lock();
> > > > +	ret = netdev_upper_dev_link(master, slave_dev, NULL);
> > > > +	rtnl_unlock();
> > > > +	if (ret) {
> > > > +		unregister_netdevice(slave_dev);
> > > > +		goto out_phy;
> > > > +	}
> > > 
> > > Hi Vladimir
> > > 
> > > A common pattern we see in bugs is that the driver sets up something
> > > critical after calling register_netdev(), not realising that that call
> > > can go off and really start using the interface before it returns. So
> > > in general, i like to have register_netdev() last, nothing after it.
> > > 
> > > Please could you move this before register_netdev().
> > > 
> > > Thanks
> > > 	Andrew
> > 
> > It doesn't work after register_netdev(). The call to
> 
> I mean it doesn't work when netdev_upper_dev_link() is _before_
> register_netdev().
> 
> > netdev_upper_dev_link() fails and no network interface gets probed. VLAN
> > performs registration and linkage in the same order:
> > 
> > https://git.kernel.org/pub/scm/linux/kernel/git/netdev/net-next.git/tree/net/8021q/vlan.c#n175
> > 
> > So I think this part is fine.
> > 
> > Thanks,
> > -Vladimir

One difference from VLAN is that in that case, the entire
register_vlan_device() function runs under RTNL.
When those bugs that you talk about are found, who starts using the
network interface too early? User space or someone else? Would RTNL be
enough to avoid that?

Thanks,
-Vladimir