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

[Paolo Abeni <pabeni@redhat.com>]

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