Re: [PATCH net-next v2 01/11] net: add netdev_lock() / netdev_unlock() helpers

[Ido Schimmel <idosch@idosch.org>]

On Wed, Jan 15, 2025 at 09:36:11AM +0100, Przemek Kitszel wrote:
> On 1/15/25 04:53, Jakub Kicinski wrote:
> > Add helpers for locking the netdev instance, use it in drivers
> > and the shaper code. This will make grepping for the lock usage
> > much easier, as we extend the lock to cover more fields.
> > 
> > Reviewed-by: Joe Damato <jdamato@fastly.com>
> > Signed-off-by: Jakub Kicinski <kuba@kernel.org>
> > Reviewed-by: Eric Dumazet <edumazet@google.com>
> > ---
> > CC: anthony.l.nguyen@intel.com
> > CC: przemyslaw.kitszel@intel.com
> > CC: jiri@resnulli.us
> > ---
> >   include/linux/netdevice.h                   | 23 ++++++-
> >   drivers/net/ethernet/intel/iavf/iavf_main.c | 74 ++++++++++-----------
> >   drivers/net/netdevsim/ethtool.c             |  4 +-
> >   net/shaper/shaper.c                         |  6 +-
> >   4 files changed, 63 insertions(+), 44 deletions(-)
> 
> Thank you,
> Reviewed-by: Przemek Kitszel <przemyslaw.kitszel@intel.com>
> 
> and Ack for iavf too
> 
> > 
> > diff --git a/include/linux/netdevice.h b/include/linux/netdevice.h
> > index bced03fb349e..891c5bdb894c 100644
> > --- a/include/linux/netdevice.h
> > +++ b/include/linux/netdevice.h
> > @@ -2444,8 +2444,12 @@ struct net_device {
> >   	u32			napi_defer_hard_irqs;
> >   	/**
> > -	 * @lock: protects @net_shaper_hierarchy, feel free to use for other
> > -	 * netdev-scope protection. Ordering: take after rtnl_lock.
> > +	 * @lock: netdev-scope lock, protects a small selection of fields.
> > +	 * Should always be taken using netdev_lock() / netdev_unlock() helpers.
> > +	 * Drivers are free to use it for other protection.
> 
> As with devl_lock(), would be good to specify the ordering for those who
> happen to take both. My guess is that devl_lock() is after netdev_lock()

devl_lock() protects the entire devlink instance and net devices are
registered under the instance lock, so I expect the order to be:

devl_lock() -> rtnl_lock() -> netdev_lock()

> 
> > +	 *
> > +	 * Protects: @net_shaper_hierarchy.
> > +	 * Ordering: take after rtnl_lock.
> >   	 */
> >   	struct mutex		lock;
> 
>