Re: [PATCH net-next v2 6/8] netdev: depend on netdev->lock for xdp features

[Jakub Kicinski <kuba@kernel.org>]

On Fri, 11 Apr 2025 10:41:58 -0700 Stanislav Fomichev wrote:
> > Ugh, REGISTER is ops locked we'd need conditional locking here.
> > 
> > Stanislav, I can make the REGISTERED notifier fully locked, right?
> > I suspect any new object we add that's protected by the instance
> > lock will want to lock the dev.  
> 
> Are you suggesting to do s/netdev_lock_ops/netdev_lock/ around
> call_netdevice_notifiers in register_netdevice?

Aha

> We can try, the biggest concern, as usual, are the stacking devices
> (with an extra lock), but casually grepping for NETDEV_REGISTER
> doesn't bring up anything suspicious.
> 
> But if you're gonna do conditional locking for NETDEV_UNREGISTER, any
> reason not to play it safe and add conditional locking to
> NETDEV_REGISTER in netdev_genl_netdevice_event?

Just trying to think what will lead to fewer problems down the line.

Let's me think it thru. So we have this situation:
 - device A - getting registered
 - device L - listens / has the notifier
with upper devs our concern is usually that taking the A's lock
around the notifier forces A -> L lock ordering (between A's instance
lock and whatever lock of L, can be either instance or some other).

If A is an arbitrary device then L has to already be ready to handle
its REGISTER callback under A's instance lock, because A may be ops
locked. So as you say for generic bonding/team changing lock type
is a noop.

If A is a specific device that L is looking for - changing the lock type
around REGISTER may impact L. /me goes to look at the code
Ugh there is a bunch of drivers that wait for a specific device to
register and then take a lock. Let me play it safe then...