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] [day] [month] [year] [list]

Message-ID: <20251020012947.2033-1-gongfan1@huawei.com>
Date: Mon, 20 Oct 2025 09:29:44 +0800
From: Fan Gong <gongfan1@...wei.com>
To: <markus.elfring@....de>
CC: <andrew+netdev@...n.ch>, <davem@...emloft.net>, <edumazet@...gle.com>,
	<gongfan1@...wei.com>, <guoxin09@...wei.com>, <gur.stavi@...wei.com>,
	<horms@...nel.org>, <kuba@...nel.org>, <linux-doc@...r.kernel.org>,
	<linux-kernel@...r.kernel.org>, <luosifu@...wei.com>,
	<luoyang82@...artners.com>, <meny.yossefi@...wei.com>,
	<netdev@...r.kernel.org>, <pabeni@...hat.com>, <pavan.chebbi@...adcom.com>,
	<shenchenyang1@...ilicon.com>, <shijing34@...wei.com>, <wulike1@...wei.com>,
	<zhoushuai28@...wei.com>, <zhuyikai1@...artners.com>
Subject: Re: [PATCH net-next v02 4/6] hinic3: Add mac filter ops

On 10/17/2025 7:51 PM, Markus Elfring wrote:
> …> +++ b/drivers/net/ethernet/huawei/hinic3/hinic3_filter.c
>> @@ -0,0 +1,413 @@
> …> +static int hinic3_mac_filter_sync(struct net_device *netdev,
>> +				  struct list_head *mac_filter_list, bool uc)
>> +{
> …
>> +		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;
>> +	}
>> +
>> +	return add_count;
>> +
>> +err_out:
>> +	return err;
>> +}
>
> Is there a need to move any resource cleanup actions behind a more appropriate label?
> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/Documentation/process/coding-style.rst?h=v6.17#n532
>
>
> Regards,
> Markus

Hi, Markus. Thanks for your comment.

Your suggestion is great. In "hinic3_mac_filter_sync", there are two places in
the code that return error values.
Actually the error handling code you quoted should be refined as a new function
because its error handling is special and cannot be normalized to function error
path.

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

And anther place(only cleanup del_list & add_list) should be moved behind a
error label in function error path according to linux doc.