[PATCH net-next 01/14] net: qede: use extack in qede_flow_parse_ports()

Asbjørn Sloth Tønnesen posted 14 patches 1 year, 7 months ago
There is a newer version of this series
[PATCH net-next 01/14] net: qede: use extack in qede_flow_parse_ports()
Posted by Asbjørn Sloth Tønnesen 1 year, 7 months ago
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@fiberby.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)
 {
 	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;
 
-- 
2.43.0

Re: [PATCH net-next 01/14] net: qede: use extack in qede_flow_parse_ports()
Posted by Przemek Kitszel 1 year, 7 months ago
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@fiberby.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;
>   

Re: [PATCH net-next 01/14] net: qede: use extack in qede_flow_parse_ports()
Posted by Asbjørn Sloth Tønnesen 1 year, 7 months ago
Hi Przemek,

Thank you for the review!

On 5/8/24 10:07 AM, Przemek Kitszel wrote:
> 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@fiberby.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?

Sure, will do that in v2.

-- 
Best regards
Asbjørn Sloth Tønnesen
Network Engineer
Fiberby - AS42541