Re: [PATCH net-next v2 02/11] net: make netdev_lock() protect netdev->reg_state

[Kuniyuki Iwashima <kuniyu@amazon.com>]

From: Jakub Kicinski <kuba@kernel.org>
Date: Tue, 14 Jan 2025 19:53:10 -0800
> Protect writes to netdev->reg_state with netdev_lock().
> From now on holding netdev_lock() is sufficient to prevent
> the net_device from getting unregistered, so code which
> wants to hold just a single netdev around no longer needs
> to hold rtnl_lock.
> 
> We do not protect the NETREG_UNREGISTERED -> NETREG_RELEASED
> transition. We'd need to move mutex_destroy(netdev->lock)
> to .release, but the real reason is that trying to stop
> the unregistration process mid-way would be unsafe / crazy.
> Taking references on such devices is not safe, either.
> So the intended semantics are to lock REGISTERED devices.
> 
> Reviewed-by: Joe Damato <jdamato@fastly.com>
> Reviewed-by: Eric Dumazet <edumazet@google.com>
> Signed-off-by: Jakub Kicinski <kuba@kernel.org>
> ---
> v2:
>  - reorder with next patch
> v1: https://lore.kernel.org/20250114035118.110297-4-kuba@kernel.org
> ---
>  include/linux/netdevice.h | 2 +-
>  net/core/dev.c            | 6 ++++++
>  2 files changed, 7 insertions(+), 1 deletion(-)
> 
> diff --git a/include/linux/netdevice.h b/include/linux/netdevice.h
> index 891c5bdb894c..30963c5d409b 100644
> --- a/include/linux/netdevice.h
> +++ b/include/linux/netdevice.h
> @@ -2448,7 +2448,7 @@ struct net_device {
>  	 * Should always be taken using netdev_lock() / netdev_unlock() helpers.
>  	 * Drivers are free to use it for other protection.
>  	 *
> -	 * Protects: @net_shaper_hierarchy.
> +	 * Protects: @reg_state, @net_shaper_hierarchy.
>  	 * Ordering: take after rtnl_lock.
>  	 */
>  	struct mutex		lock;
> diff --git a/net/core/dev.c b/net/core/dev.c
> index fda4e1039bf0..6603c08768f6 100644
> --- a/net/core/dev.c
> +++ b/net/core/dev.c
> @@ -10668,7 +10668,9 @@ int register_netdevice(struct net_device *dev)
>  
>  	ret = netdev_register_kobject(dev);
>  
> +	netdev_lock(dev);
>  	WRITE_ONCE(dev->reg_state, ret ? NETREG_UNREGISTERED : NETREG_REGISTERED);
> +	netdev_unlock(dev);

Do we need the lock before list_netdevice() ?

It's not a big deal, so

Reviewed-by: Kuniyuki Iwashima <kuniyu@amazon.com>