linux-kernel - Re: [PATCH net-next 01/14] net: qede: use extack in qede_flow_parse

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]

Date: Wed, 8 May 2024 12:07:48 +0200
From: Przemek Kitszel <przemyslaw.kitszel@...el.com>
To: Asbjørn Sloth Tønnesen <ast@...erby.net>,
	<netdev@...r.kernel.org>
CC: <linux-kernel@...r.kernel.org>, "David S. Miller" <davem@...emloft.net>,
	Eric Dumazet <edumazet@...gle.com>, Jakub Kicinski <kuba@...nel.org>, "Paolo
 Abeni" <pabeni@...hat.com>, Manish Chopra <manishc@...vell.com>
Subject: Re: [PATCH net-next 01/14] net: qede: use extack in
 qede_flow_parse_ports()

On 5/7/24 12:44, Asbjørn Sloth Tønnesen wrote:
> Convert qede_flow_parse_ports to use extack,
> and drop the edev argument.
> 
> Convert DP_NOTICE call to use NL_SET_ERR_MSG_MOD instead.
> 
> In calls to qede_flow_parse_ports(), use NULL as extack
> for now, until a subsequent patch makes extack available.
> 
> Only compile tested.
> 
> Signed-off-by: Asbjørn Sloth Tønnesen <ast@...erby.net>
> ---
>   drivers/net/ethernet/qlogic/qede/qede_filter.c | 9 +++++----
>   1 file changed, 5 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/net/ethernet/qlogic/qede/qede_filter.c b/drivers/net/ethernet/qlogic/qede/qede_filter.c
> index ded48523c383..3995baa2daa6 100644
> --- a/drivers/net/ethernet/qlogic/qede/qede_filter.c
> +++ b/drivers/net/ethernet/qlogic/qede/qede_filter.c
> @@ -1700,7 +1700,7 @@ static int qede_parse_actions(struct qede_dev *edev,
>   }
>   
>   static int
> -qede_flow_parse_ports(struct qede_dev *edev, struct flow_rule *rule,
> +qede_flow_parse_ports(struct netlink_ext_ack *extack, struct flow_rule *rule,
>   		      struct qede_arfs_tuple *t)

there are ~40 cases in drivers/net/ethernet that have an extack param as
not the last one, and over 1250 that have an extack as the last param.
My grepping was very naive, and counted both forward declarations and
implementations, but it's clear what is the preference.

Could you please convert the series to be that way?

>   {
>   	if (flow_rule_match_key(rule, FLOW_DISSECTOR_KEY_PORTS)) {
> @@ -1709,7 +1709,8 @@ qede_flow_parse_ports(struct qede_dev *edev, struct flow_rule *rule,
>   		flow_rule_match_ports(rule, &match);
>   		if ((match.key->src && match.mask->src != htons(U16_MAX)) ||
>   		    (match.key->dst && match.mask->dst != htons(U16_MAX))) {
> -			DP_NOTICE(edev, "Do not support ports masks\n");
> +			NL_SET_ERR_MSG_MOD(extack,
> +					   "Do not support ports masks");
>   			return -EINVAL;
>   		}
>   
> @@ -1747,7 +1748,7 @@ qede_flow_parse_v6_common(struct qede_dev *edev, struct flow_rule *rule,
>   		memcpy(&t->dst_ipv6, &match.key->dst, sizeof(addr));
>   	}
>   
> -	err = qede_flow_parse_ports(edev, rule, t);
> +	err = qede_flow_parse_ports(NULL, rule, t);
>   	if (err)
>   		return err;
>   
> @@ -1774,7 +1775,7 @@ qede_flow_parse_v4_common(struct qede_dev *edev, struct flow_rule *rule,
>   		t->dst_ipv4 = match.key->dst;
>   	}
>   
> -	err = qede_flow_parse_ports(edev, rule, t);
> +	err = qede_flow_parse_ports(NULL, rule, t);
>   	if (err)
>   		return err;
>