Received-SPF: pass (google.com: domain of linux-kernel+bounces-173040-linux.lists.archive=gmail.com@vger.kernel.org designates 2604:1380:45e3:2400::1 as permitted sender) client-ip=2604:1380:45e3:2400::1;
Message-ID: <e3993bb2-3aac-4b07-8f8a-e537fa902af4@intel.com>
Date: Wed, 8 May 2024 12:07:48 +0200
User-Agent: Mozilla Thunderbird
Subject: Re: [PATCH net-next 01/14] net: qede: use extack in
 qede_flow_parse_ports()
Content-Language: en-US
To: =?UTF-8?Q?Asbj=C3=B8rn_Sloth_T=C3=B8nnesen?= <ast@fiberby.net>,
	<netdev@vger.kernel.org>
CC: <linux-kernel@vger.kernel.org>, "David S. Miller" <davem@davemloft.net>,
	Eric Dumazet <edumazet@google.com>, Jakub Kicinski <kuba@kernel.org>, "Paolo
 Abeni" <pabeni@redhat.com>, Manish Chopra <manishc@marvell.com>
References: <20240507104421.1628139-1-ast@fiberby.net>
 <20240507104421.1628139-2-ast@fiberby.net>
From: Przemek Kitszel <przemyslaw.kitszel@intel.com>
In-Reply-To: <20240507104421.1628139-2-ast@fiberby.net>
Content-Type: text/plain; charset="UTF-8"; format=flowed
Content-Transfer-Encoding: 8bit
Precedence: bulk
MIME-Version: 1.0
X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-MessageData-0: =?utf-8?B?RFBmUTYyTnJlcXFGUE5lWUVuWkdoY0dYcU0razVxd3hseGtobzFWSWF0SkRB?=
 =?utf-8?B?QWZmeXo2YW9HVjlSbVdZa0xWWnY1ZEFuMDh2aW9LR3J3N3lscGw0ZFhCY0pQ?=
 =?utf-8?B?VmZ6aXI0UnBzUzRiR0dlVFQ0WTFwd09rRjBXcHVSSjBHd1NqZ3NDNTZyeklm?=
 =?utf-8?B?ZGsrMUEzYkVkNGlrbzgvWktvWjRqYWIxRU9sajllL2IvWjNFOEpPZTJhRXZS?=
 =?utf-8?B?QUdZeFBxK0lYS3EvcHFadDBKeWRnZGJrMTBRWU9jZWV6aUxJWUd5YWMxd0dP?=
 =?utf-8?B?cjFrcHFXcHF0QlNLU2FuRkdBbGhuRWcvZERiSHRMenl2WEhCdHR0M0RGZUsr?=
 =?utf-8?B?NzZvQzd3cEFST2lrWlk2Nmpmb1REVEZmVFZXZ00ybkthTXVRZmRjdmVJYmtu?=
 =?utf-8?B?S09iYjFPZTByTUlFazRSODhoZWovZXBxOENERk9lOXZnRFFpb2VzMTlEbjkw?=
 =?utf-8?B?eGZsS3d6WUdhSHNjU2NodjVvcGxMVVd0SnpyM2p3K0Q1R2NPQ2twZnNrWDFa?=
 =?utf-8?B?SUJTVDFLTWt6dnlScVhTYnFaZFNSNWg0Y1hrL3JDZzRlOFU5MW9kRk8vSDl3?=
 =?utf-8?B?d3JLOUJwUEZwTXgxSU02RHZ0UUFJR0FWRVlDS3V2M0hrcmFQUCthL2ZURDVU?=
 =?utf-8?B?VWZGb1VUZHdicEJRdmJhVW5sZHNzZERwYTNrTm9adHBEbTIvN2tEOURMMHN4?=
 =?utf-8?B?QjhwaE9RalN0cWxLQzJFT2ZBMFNZK0hXYnQ2WFV1VlVzNXRxZFdNZXZVWWhj?=
 =?utf-8?B?SU11Y2dsRiszN3R2ZjRWejcwYmZlNDRrd2ovVGVrS01LcDJLcXhOdkZSZkdY?=
 =?utf-8?B?UmdFTGJDaXNhdGVpeGJka0h3ZWkvY1k3d1U3QXVMRSs0Nm1oMzErRDhsR0I3?=
 =?utf-8?B?QW9keGl1ZE1tYmJXa01uOHNyYmdqRFE1SnpSSzliWUd4WmxWK21FWkxGTFdE?=
 =?utf-8?B?OEdTaXZscm1lNi94OU9jbnFvN1lic0svY1ZKelVFTjBqcUQxbnI4bnRWUDMw?=
 =?utf-8?B?V0o4c1YyUEJGMVFBNUNTVnpORHRpMTFVMWZWaDVGL0xGNC9FSkJlZDQ4bTFW?=
 =?utf-8?B?ZEdZd2NFaHFwREsrbDhVYjE3aWsvZGQva3JSbXNaRHh1TEx0SjhtU3dzZEF5?=
 =?utf-8?B?KzVGTlpXWStWakwyOHZObWlJV0dJOWlwS3FSVHNrYysxU2UvbzY3SVN5S1Fo?=
 =?utf-8?B?RHNYbm1IamErMFMyNjVXSzVycHcyTWpWRDN4TkcwUHljOFRkL0lCbUlKMWNR?=
 =?utf-8?B?WHhHMXNiTlN4NzlpckdTWUp6aDJKaUN1WkpKVEY0N0dBMGI4L2pQSTBHM0hM?=
 =?utf-8?B?RWZyam42VUNaRlBKYzYySEpJMVFUVnowRXU5eE5seHB1elM5RVNzSWpqdVAw?=
 =?utf-8?B?eE16RDYrMFYxUU9IamxHZHVha0x4eW1taHdCeVZmRW1UelhmemZGdW81citH?=
 =?utf-8?B?ZWxhREhlaGpaSHlVTnJ0L05FSGxBREFGUXhsTnhWRVRmbS9talhPQkVNc2d1?=
 =?utf-8?B?WVdlNklMM3d6UHJyUmFPdm8zV2lMN0xURStGQk82RnhSTVR4bVBrSGZkSlk3?=
 =?utf-8?B?dS9hOGNVSStZNlE4YklMejN4SEhpN2NDMldudWwxaGxKNUZGLzNRUVBCeDNF?=
 =?utf-8?B?cDd2ZzlVSWp3dGk0UkRreDFSR2h4M0trNlBHRGtVWVVQTkFTcjMrQzd0QWRa?=
 =?utf-8?B?SjlSaVA4dU5idC96NWVmcU9yYldCbXBWMEhDbElkenJMTmprTmxNUEVzTm8y?=
 =?utf-8?B?MGlrRUJhUm5yTUJVNTc1WExGRW12d2VCYTJLNU5kbG11Mzk2TTBpMytSd0dD?=
 =?utf-8?B?QUMxVG8zeVExd2dmT1JUZnVJa0Z5dUl6STVQZEdQVlF3VXhHYTcrWlo2bFFK?=
 =?utf-8?B?bjU3RlBIK3ZqS1ZPVFF0ZHNML0F5alZQVlIyVHMrbmJPMVhVcnIrWFJtNFA2?=
 =?utf-8?B?WXovUUYxdTVGUExJeUF2aWdON01iVFkvdTRtNTI0ZEpmZU1ZVHVMeloxQU9S?=
 =?utf-8?B?dlIxYWFKYzlabC9KKy93OHN3WTVBV0J2SGxyVkMxdXJmTjFLNzV5RUpXT0Nj?=
 =?utf-8?B?SVI1Z0lJblk3NjdtZExzN2NQWVZaYjJkUVoza2Jrby9SNnRkR2hyYWFCK0hR?=
 =?utf-8?B?SWUwVEQ3dFNtVHNYQjhDWkZaRFJkVGsvOEFVMFhDbEtsa1MvdGFGVWpqQlpY?=
 =?utf-8?B?VFE9PQ==?=
X-MS-Exchange-CrossTenant-Network-Message-Id: e88756ae-4ae3-471b-ad43-08dc6f46b93e
X-MS-Exchange-CrossTenant-AuthSource: MN6PR11MB8102.namprd11.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 08 May 2024 10:07:52.6752
 (UTC)
X-MS-Exchange-CrossTenant-FromEntityHeader: Hosted
X-MS-Exchange-CrossTenant-Id: 46c98d88-e344-4ed4-8496-4ed7712e255d
X-MS-Exchange-CrossTenant-MailboxType: HOSTED
X-MS-Exchange-CrossTenant-UserPrincipalName: 21v5rcIZ/iVlaS91acsKbY++DermTvKgsDprjipC6v58g3Ai7nCGrOBL7lCAnmd8UY5esJgUUPiH3TWHJmyfi03EN+nsPhreiN0HxXwvoI0=
X-MS-Exchange-Transport-CrossTenantHeadersStamped: IA0PR11MB7211
X-OriginatorOrg: intel.com

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