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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20241113133301.GZ4507@kernel.org>
Date: Wed, 13 Nov 2024 13:33:01 +0000
From: Simon Horman <horms@...nel.org>
To: Jijie Shao <shaojijie@...wei.com>
Cc: davem@...emloft.net, edumazet@...gle.com, kuba@...nel.org,
	pabeni@...hat.com, andrew+netdev@...n.ch, shenjian15@...wei.com,
	wangpeiyang1@...wei.com, liuyonglong@...wei.com,
	chenhao418@...wei.com, sudongming1@...wei.com,
	xujunsheng@...wei.com, shiyongbang@...wei.com, libaihan@...wei.com,
	jonathan.cameron@...wei.com, shameerali.kolothum.thodi@...wei.com,
	salil.mehta@...wei.com, netdev@...r.kernel.org,
	linux-kernel@...r.kernel.org
Subject: Re: [PATCH V3 net-next 3/7] net: hibmcge: Add unicast frame filter
 supported in this module

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 <shaojijie@...wei.com>

...

> 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;
>  }
>  

...