Subject: Re: [PATCH V3 net-next 3/7] net: hibmcge: Add unicast frame filter supported in this module

From: Simon Horman <[email-masked]>
Date: Wed, 13 Nov 2024 13:33:01 +0000

On Mon, Nov 11, 2024 at 10:55:54PM +0800, Jijie Shao wrote:
> MAC supports filtering unmatched unicast packets according to
> the MAC address table. This patch adds the support for
> unicast frame filtering.
> 
> To support automatic restoration of MAC entries
> after reset, the driver saves a copy of MAC entries in the driver.
> 
> Signed-off-by: Jijie Shao <[email-masked]>

...

> diff --git a/drivers/net/ethernet/hisilicon/hibmcge/hbg_main.c b/drivers/net/ethernet/hisilicon/hibmcge/hbg_main.c

...

>  static int hbg_net_set_mac_address(struct net_device *netdev, void *addr)
>  {
>  	struct hbg_priv *priv = netdev_priv(netdev);
>  	u8 *mac_addr;
> +	bool is_exists;
> +	u32 index;

nit: If you have to respin for some other reason,
     please arrange these local variables in reverse
     xmas tree order - longest line to shortest.

     Also, from an English language PoV, is_exists is a bit tautological.
     Not that it really matters, but maybe addr_exists would work?

>  
>  	mac_addr = ((struct sockaddr *)addr)->sa_data;
>  
>  	if (!is_valid_ether_addr(mac_addr))
>  		return -EADDRNOTAVAIL;
>  
> -	hbg_hw_set_uc_addr(priv, ether_addr_to_u64(mac_addr));
> -	dev_addr_set(netdev, mac_addr);
> +	/* The index of host mac is always 0.
> +	 * If new mac address already exists,
> +	 * delete the existing mac address and
> +	 * add it to the position with index 0.
> +	 */
> +	is_exists = !hbg_get_index_from_mac_table(priv, mac_addr, &index);
> +	hbg_set_mac_to_mac_table(priv, 0, mac_addr);
> +	if (is_exists)
> +		hbg_set_mac_to_mac_table(priv, index, NULL);
>  
> +	dev_addr_set(netdev, mac_addr);
>  	return 0;
>  }
>  

...