netdev - Re: [PATCH net-next v02 4/6] hinic3: Add mac filter ops

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: <1e779b80-4645-420d-8cae-36c36c3575e3@redhat.com>
Date: Tue, 21 Oct 2025 12:16:25 +0200
From: Paolo Abeni <pabeni@...hat.com>
To: Fan Gong <gongfan1@...wei.com>, Zhu Yikai <zhuyikai1@...artners.com>,
 netdev@...r.kernel.org, "David S. Miller" <davem@...emloft.net>,
 Eric Dumazet <edumazet@...gle.com>, Jakub Kicinski <kuba@...nel.org>,
 Simon Horman <horms@...nel.org>, Andrew Lunn <andrew+netdev@...n.ch>,
 Markus.Elfring@....de, pavan.chebbi@...adcom.com
Cc: linux-kernel@...r.kernel.org, linux-doc@...r.kernel.org,
 luosifu <luosifu@...wei.com>, Xin Guo <guoxin09@...wei.com>,
 Shen Chenyang <shenchenyang1@...ilicon.com>,
 Zhou Shuai <zhoushuai28@...wei.com>, Wu Like <wulike1@...wei.com>,
 Shi Jing <shijing34@...wei.com>, Luo Yang <luoyang82@...artners.com>,
 Meny Yossefi <meny.yossefi@...wei.com>, Gur Stavi <gur.stavi@...wei.com>
Subject: Re: [PATCH net-next v02 4/6] hinic3: Add mac filter ops

On 10/17/25 10:30 AM, Fan Gong wrote:
> +static int hinic3_mac_filter_sync(struct net_device *netdev,
> +				  struct list_head *mac_filter_list, bool uc)
> +{
> +	struct hinic3_nic_dev *nic_dev = netdev_priv(netdev);
> +	struct list_head tmp_del_list, tmp_add_list;
> +	struct hinic3_mac_filter *fclone;
> +	struct hinic3_mac_filter *ftmp;
> +	struct hinic3_mac_filter *f;
> +	int err = 0, add_count;
> +
> +	INIT_LIST_HEAD(&tmp_del_list);
> +	INIT_LIST_HEAD(&tmp_add_list);
> +
> +	list_for_each_entry_safe(f, ftmp, mac_filter_list, list) {
> +		if (f->state != HINIC3_MAC_WAIT_HW_UNSYNC)
> +			continue;
> +
> +		f->state = HINIC3_MAC_HW_UNSYNCED;
> +		list_move_tail(&f->list, &tmp_del_list);
> +	}
> +
> +	list_for_each_entry_safe(f, ftmp, mac_filter_list, list) {
> +		if (f->state != HINIC3_MAC_WAIT_HW_SYNC)
> +			continue;
> +
> +		fclone = hinic3_mac_filter_entry_clone(f);
> +		if (!fclone) {
> +			err = -ENOMEM;
> +			break;
> +		}
> +
> +		f->state = HINIC3_MAC_HW_SYNCED;
> +		list_add_tail(&fclone->list, &tmp_add_list);
> +	}
> +
> +	if (err) {
> +		hinic3_undo_del_filter_entries(mac_filter_list, &tmp_del_list);
> +		hinic3_undo_add_filter_entries(mac_filter_list, &tmp_add_list);
> +		netdev_err(netdev, "Failed to clone mac_filter_entry\n");
> +
> +		hinic3_cleanup_filter_list(&tmp_del_list);
> +		hinic3_cleanup_filter_list(&tmp_add_list);
> +		goto err_out;
> +	}
> +
> +	add_count = hinic3_mac_filter_sync_hw(netdev, &tmp_del_list,
> +					      &tmp_add_list);
> +	if (!list_empty(&tmp_add_list)) {
> +		/* there were errors, delete all mac in hw */
> +		hinic3_undo_add_filter_entries(mac_filter_list, &tmp_add_list);
> +		/* VF does not support promiscuous mode, don't delete any other uc mac */
> +		if (!HINIC3_IS_VF(nic_dev->hwdev) || !uc) {
> +			list_for_each_entry_safe(f, ftmp, mac_filter_list,
> +						 list) {
> +				if (f->state != HINIC3_MAC_HW_SYNCED)
> +					continue;
> +
> +				fclone = hinic3_mac_filter_entry_clone(f);
> +				if (!fclone)
> +					break;
> +
> +				f->state = HINIC3_MAC_WAIT_HW_SYNC;
> +				list_add_tail(&fclone->list, &tmp_del_list);
> +			}
> +		}
> +
> +		hinic3_cleanup_filter_list(&tmp_add_list);
> +		hinic3_mac_filter_sync_hw(netdev, &tmp_del_list, &tmp_add_list);
> +
> +		/* need to enter promiscuous/allmulti mode */
> +		err = -ENOMEM;
> +		goto err_out;
> +	}

I'm under the impression that this code could be simpler if
hinic3_mac_filter_sync_hw() don't modifiy the argment lists, but set
some flag on the successfully added entries.

In case of failure, you traverse the tmp_add_list and delete all the
unflagged entries.

Both lists are always cleaned-up at function exit.

/P