Re: [PATCH net-next v2 2/2] net: Add dev_getbyhwaddr_rtnl() helper

[Kuniyuki Iwashima <kuniyu@amazon.com>]

From: Breno Leitao <leitao@debian.org>
Date: Mon, 10 Feb 2025 03:56:14 -0800
> Add dedicated helper for finding devices by hardware address when
> holding rtnl_lock, similar to existing dev_getbyhwaddr_rcu(). This prevents
> PROVE_LOCKING warnings when rtnl_lock is held but RCU read lock is not.

No one uses dev_getbyhwaddr() yet, so this patch itself doens't fix
the warninig.

You are missing patch 3 to convert arp_req_set_public().  Other call
sites are under RCU.


> 
> Extract common address comparison logic into dev_comp_addr().
> 
> The context about this change could be found in the following
> discussion:
> 
> Link: https://lore.kernel.org/all/20250206-scarlet-ermine-of-improvement-1fcac5@leitao/
> 
> Cc: kuniyu@amazon.com
> Cc: ushankar@purestorage.com
> Suggested-by: Eric Dumazet <edumazet@google.com>
> Signed-off-by: Breno Leitao <leitao@debian.org>
> ---
>  include/linux/netdevice.h |  2 ++
>  net/core/dev.c            | 36 +++++++++++++++++++++++++++++++++---
>  2 files changed, 35 insertions(+), 3 deletions(-)
> 
> diff --git a/include/linux/netdevice.h b/include/linux/netdevice.h
> index 0deee1313f23a625242678c8e571533e69a05263..6f0f5d327b41bfd5e0ccf9a3e63d6082bdf45d14 100644
> --- a/include/linux/netdevice.h
> +++ b/include/linux/netdevice.h
> @@ -3271,6 +3271,8 @@ static inline struct net_device *first_net_device_rcu(struct net *net)
>  }
>  
>  int netdev_boot_setup_check(struct net_device *dev);
> +struct net_device *dev_getbyhwaddr(struct net *net, unsigned short type,
> +				   const char *hwaddr);
>  struct net_device *dev_getbyhwaddr_rcu(struct net *net, unsigned short type,
>  				       const char *hwaddr);
>  struct net_device *dev_getfirstbyhwtype(struct net *net, unsigned short type);
> diff --git a/net/core/dev.c b/net/core/dev.c
> index c7e726f81406ece98801441dce3d683c8e0c9d99..2a0fbb319b2ad1b2aae908bc87ef19504cc42909 100644
> --- a/net/core/dev.c
> +++ b/net/core/dev.c
> @@ -1121,6 +1121,12 @@ int netdev_get_name(struct net *net, char *name, int ifindex)
>  	return ret;
>  }
>  
> +static bool dev_comp_addr(struct net_device *dev, unsigned short type,
> +			  const char *ha)
> +{
> +	return dev->type == type && !memcmp(dev->dev_addr, ha, dev->addr_len);
> +}
> +
>  /**
>   *	dev_getbyhwaddr_rcu - find a device by its hardware address
>   *	@net: the applicable net namespace
> @@ -1129,7 +1135,7 @@ int netdev_get_name(struct net *net, char *name, int ifindex)
>   *
>   *	Search for an interface by MAC address. Returns NULL if the device
>   *	is not found or a pointer to the device.
> - *	The caller must hold RCU or RTNL.
> + *	The caller must hold RCU.
>   *	The returned device has not had its ref count increased
>   *	and the caller must therefore be careful about locking
>   *
> @@ -1141,14 +1147,38 @@ struct net_device *dev_getbyhwaddr_rcu(struct net *net, unsigned short type,
>  	struct net_device *dev;
>  
>  	for_each_netdev_rcu(net, dev)
> -		if (dev->type == type &&
> -		    !memcmp(dev->dev_addr, ha, dev->addr_len))
> +		if (dev_comp_addr(dev, type, ha))
>  			return dev;
>  
>  	return NULL;
>  }
>  EXPORT_SYMBOL(dev_getbyhwaddr_rcu);
>  
> +/**
> + *	dev_getbyhwaddr - find a device by its hardware address
> + *	@net: the applicable net namespace
> + *	@type: media type of device
> + *	@ha: hardware address
> + *
> + *	Similar to dev_getbyhwaddr_rcu(), but the owner needs to hold
> + *	rtnl_lock.
> + *
> + *	Return: pointer to the net_device, or NULL if not found
> + */
> +struct net_device *dev_getbyhwaddr(struct net *net, unsigned short type,
> +				   const char *ha)
> +{
> +	struct net_device *dev;
> +
> +	ASSERT_RTNL();
> +	for_each_netdev(net, dev)
> +		if (dev_comp_addr(dev, type, ha))
> +			return dev;
> +
> +	return NULL;
> +}
> +EXPORT_SYMBOL(dev_getbyhwaddr);
> +
>  struct net_device *dev_getfirstbyhwtype(struct net *net, unsigned short type)
>  {
>  	struct net_device *dev, *ret = NULL;
> 
> -- 
> 2.43.5