From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mails.dpdk.org (mails.dpdk.org [217.70.189.124]) by inbox.dpdk.org (Postfix) with ESMTP id 4090245594; Fri, 5 Jul 2024 11:50:01 +0200 (CEST) Received: from mails.dpdk.org (localhost [127.0.0.1]) by mails.dpdk.org (Postfix) with ESMTP id 3350442EA3; Fri, 5 Jul 2024 11:50:01 +0200 (CEST) Received: from mgamail.intel.com (mgamail.intel.com [198.175.65.14]) by mails.dpdk.org (Postfix) with ESMTP id D8BFF402BE; Fri, 5 Jul 2024 11:49:58 +0200 (CEST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=intel.com; i=@intel.com; q=dns/txt; s=Intel; t=1720173000; x=1751709000; h=date:from:to:cc:subject:message-id:references: in-reply-to:mime-version; bh=54qC75BAhEKYt8+TIv8q1qLsqv+GIMxrlG0SpkNsPPs=; b=L8A8a3XfMc3P9V1Eb33Z2KRs+nJk4x7NRE7x1TtgRz2WxFfOve/B4T40 zGSG47hDx4Y20+N82zaBCO3+O0ytjZMIZ53tuRceF8Cy6tc0PpLewYFRJ fF+HDpYS2qOI4S5M7Ns+JHhYQ3tBJAfk4MnNrbgRZgary9uVwS0Y+8p0I 0RGswxqISXZ7sbh0I2X0SapiKHgZWME5qQpWn53pV3Vt9kb6VAgpvSeH1 7luXbrr+vsGs37XZjBTqMxsWnoWBycmWRnCW579vONkmv63mfSZRSc7Z5 wH78zjSaqqU/DbFmbx6eIxsLoNcoU/3Am6XWii3kEs7X/QtD/x30882GO w==; X-CSE-ConnectionGUID: YsMZJ34aQLu2Vo0QwhaVZg== X-CSE-MsgGUID: vHyxakg0TzegWGezWya2YQ== X-IronPort-AV: E=McAfee;i="6700,10204,11123"; a="21273041" X-IronPort-AV: E=Sophos;i="6.09,184,1716274800"; d="scan'208";a="21273041" Received: from fmviesa003.fm.intel.com ([10.60.135.143]) by orvoesa106.jf.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 05 Jul 2024 02:49:58 -0700 X-CSE-ConnectionGUID: TBRqssStTBqPFsByzmBO2Q== X-CSE-MsgGUID: 8wtv6W7mTR+IVfL5d0x/Vg== X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="6.09,184,1716274800"; d="scan'208";a="51165257" Received: from fmsmsx603.amr.corp.intel.com ([10.18.126.83]) by fmviesa003.fm.intel.com with ESMTP/TLS/AES256-GCM-SHA384; 05 Jul 2024 02:49:57 -0700 Received: from fmsmsx610.amr.corp.intel.com (10.18.126.90) by fmsmsx603.amr.corp.intel.com (10.18.126.83) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256) id 15.1.2507.39; Fri, 5 Jul 2024 02:49:57 -0700 Received: from fmsedg601.ED.cps.intel.com (10.1.192.135) by fmsmsx610.amr.corp.intel.com (10.18.126.90) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256) id 15.1.2507.39 via Frontend Transport; Fri, 5 Jul 2024 02:49:57 -0700 Received: from NAM04-DM6-obe.outbound.protection.outlook.com (104.47.73.41) by edgegateway.intel.com (192.55.55.70) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.1.2507.39; Fri, 5 Jul 2024 02:49:56 -0700 ARC-Seal: i=1; a=rsa-sha256; s=arcselector9901; d=microsoft.com; cv=none; b=T7GqSfG1IUvqvCIVNkykil1szgzrQcfnEp2KneR6SmTglbeV8M8t58L5zrEV/IGrNPY8TUzKfYkq4im/wrowz07G5D4E/H2I6kv9DwxD7jxyq8xFTAQjnSzV8gwskh7k7sLOtatsZ+gBXg+RK97OD79/xfiwBzRqOu6LdvksnKjvga0jZWeY8H+DfurPHdlkILPl1r9XIPuFBCrVeDAOh8Y0k2VXTpqixawPqlL6JIheA4654qCQClnmf184GmYSawqoHmfPTrOS+2LBhuo0SlZh7Ms6NFbK9PN6y6W/i7/1UXxrrX40JbqQ8Ikb8CL/bdEIA2GWWRhLwIHwjcHa5A== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=microsoft.com; s=arcselector9901; h=From:Date:Subject:Message-ID:Content-Type:MIME-Version:X-MS-Exchange-AntiSpam-MessageData-ChunkCount:X-MS-Exchange-AntiSpam-MessageData-0:X-MS-Exchange-AntiSpam-MessageData-1; bh=4jiajnBtY+Mdh5TWjO1ISWkRwOZ15xYQoO8pWTKPwcg=; b=CqnMBnv2gHOgphZZsJqrM1jcFxm3JFgZ1K6wHxiEd/q0J9D9JPwS2aTM2BhNF2H86tezKFiCYB8JkLPyBGsloh7t3Bga2axi1IpqAGgBy74HYaJL7uxZvvtsm2hqXcVBC83QPlj0omkamt+c8lU8Kx32usGlNL2tosZhf+BBvaOOSh8m7BPwHM2l9iUYVO8vaokIXlfXcIhWu9rzxCHwhVyoPIFo9sdQo214L64pegPmnaT9/rkyMj1LuNXHmUNk0JjAX7an+4Ccgtsal17uauSAYvusWlXuH7FL3patVXV4Bu/vzGSlhnvRURqsp0K8d7SP8eR6BnkxS9cVNCoVqw== ARC-Authentication-Results: i=1; mx.microsoft.com 1; spf=pass smtp.mailfrom=intel.com; dmarc=pass action=none header.from=intel.com; dkim=pass header.d=intel.com; arc=none Authentication-Results: dkim=none (message not signed) header.d=none;dmarc=none action=none header.from=intel.com; Received: from DS0PR11MB7309.namprd11.prod.outlook.com (2603:10b6:8:13e::17) by PH7PR11MB7596.namprd11.prod.outlook.com (2603:10b6:510:27e::12) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.20.7741.25; Fri, 5 Jul 2024 09:49:54 +0000 Received: from DS0PR11MB7309.namprd11.prod.outlook.com ([fe80::f120:cc1f:d78d:ae9b]) by DS0PR11MB7309.namprd11.prod.outlook.com ([fe80::f120:cc1f:d78d:ae9b%7]) with mapi id 15.20.7741.029; Fri, 5 Jul 2024 09:49:54 +0000 Date: Fri, 5 Jul 2024 10:49:49 +0100 From: Bruce Richardson To: Soumyadeep Hore CC: , , Subject: Re: [PATCH v3 1/2] net/cpfl: fix check for opcodes of received ctlq messages Message-ID: References: <20240705050831.2639342-3-soumyadeep.hore@intel.com> <20240705083032.2756071-1-soumyadeep.hore@intel.com> <20240705083032.2756071-2-soumyadeep.hore@intel.com> Content-Type: text/plain; charset="us-ascii" Content-Disposition: inline In-Reply-To: <20240705083032.2756071-2-soumyadeep.hore@intel.com> X-ClientProxiedBy: DU7P189CA0011.EURP189.PROD.OUTLOOK.COM (2603:10a6:10:552::25) To DS0PR11MB7309.namprd11.prod.outlook.com (2603:10b6:8:13e::17) MIME-Version: 1.0 X-MS-PublicTrafficType: Email X-MS-TrafficTypeDiagnostic: DS0PR11MB7309:EE_|PH7PR11MB7596:EE_ X-MS-Office365-Filtering-Correlation-Id: d7ac2f2e-1afd-45f0-0e7b-08dc9cd7d280 X-MS-Exchange-SenderADCheck: 1 X-MS-Exchange-AntiSpam-Relay: 0 X-Microsoft-Antispam: BCL:0;ARA:13230040|366016|1800799024|376014; X-Microsoft-Antispam-Message-Info: =?us-ascii?Q?/7MD6Mu1OSGnJM6b7OwmqeknyobJBuUXDx2YOZo73uJzco2gcrc5zNf+BT1W?= =?us-ascii?Q?WzR6JKGH8zhNyLNpim5b+dbofJtpIbnIQns6MMG0B4Kjuiyktiosa3pR6Ty8?= =?us-ascii?Q?B+jWlQacMd84qEKsvLMz6LJXrsqdr8wMnF51YJt3ykxqpaDki0uu7uFSyJtl?= =?us-ascii?Q?vz28KIGUaDN9QMbhgpbLgtbDWcGM4gMgTD4myXAS37rxr45sYi1Rj9rr1+we?= =?us-ascii?Q?dhxEzBQqlsBP/P1LkV6rcKh7UWePEbMU6CY+ct4K25WrtaLCyDvUTGScEQ8z?= =?us-ascii?Q?bjVV2O2wfwgRFre220K8I+6gflvOiwXY6wOzytkB6ho3k8fdPXFxBhoAjnG/?= =?us-ascii?Q?d+TtdMmZtYZ2qpbZziZ20/OTAmhK216qfV0QN7TmNy857peosjdf6zvmQ+C3?= =?us-ascii?Q?6f42P6NXe93AZ9ZDaA2xa4I4tB7ta07S3lwHXeUxdIEStpf4MK/rqMBy1sy/?= =?us-ascii?Q?g/Zw4ZXCN0ZmeTfqp0Idx8EKD58d04z8nkk/VaH6tuoNAQxN0u4vVTbIpRJk?= =?us-ascii?Q?6cg3xI3iMo11h4a3/MoD6HkWugrHl5CzGBUWhwL/4Ho+vngEJ+StjS+c3d/1?= =?us-ascii?Q?dHOAtUf/OC5pG7spks0ge3jEsCMdXZgR2sqyXzmPOqYv6HEUhSLkjDQlffRn?= =?us-ascii?Q?e+2TvxsN1ky6ona6GDrvoT5H528WqK1eS9xz6PNowFQ3/HI/ucvtIhdPZnF2?= =?us-ascii?Q?Ji+xb9Fal735+1yZfrR/evcliaQjPvqDNcwhT89SWnDNxnmC8eFDPE+mwiwI?= =?us-ascii?Q?o59LyjbmqvKkTZmRHewp2ABZH/hAj2UHLvCKxkRvQioZBwvMJcvawkwXtQWb?= =?us-ascii?Q?VlMwONxfG4lV0sYN0V9XF4cB63ZBr/FEPau5GBW2d4HTC+iF4EkBBIh6gQWs?= =?us-ascii?Q?RpHpHu8UhxMKkhSDEQMWDyIj1UC1hXF3q/0g/8IXR2ZgAImp+xiR/uSOa/RI?= =?us-ascii?Q?3BLlQsUeH0YIa18B1wuqISznc7YdekKx1m7ZQhM3jD+T5hEELkD7JV4ZpY8c?= =?us-ascii?Q?ShQMLWkf57qLZFXIAVBmwjrbWKXSkSz8RHOY9ojqP5g84aOx+PEoj8EGLCcf?= =?us-ascii?Q?Ivt28Uduo6ekpo0yAhCh1YJg7lbbeWuJKqw0F7uQgo3Z8SY0weKcSpqIGjxQ?= =?us-ascii?Q?mqrxt5exniP0HbriZtKe2jvWIMDUI/qPp7BrYY7RB4oulXSNLrChBoxSr8y0?= =?us-ascii?Q?3JGDk6GtinwbENpIomatTxFlvlVzWU35c6f50x28hhXfU+yTOrOSi6mLk6Wx?= =?us-ascii?Q?/zy0iUYTV07A3a/bKlsuLKracim9bOWAfrLw7FzWvEx+KeHquJlKdaU0ohQn?= =?us-ascii?Q?FB4tUe6UdhIGno/v2S1xj/gVTjc9pdU+tGhHZXv75e8kkQ=3D=3D?= X-Forefront-Antispam-Report: CIP:255.255.255.255; CTRY:; LANG:en; SCL:1; SRV:; IPV:NLI; SFV:NSPM; H:DS0PR11MB7309.namprd11.prod.outlook.com; PTR:; CAT:NONE; SFS:(13230040)(366016)(1800799024)(376014); DIR:OUT; SFP:1101; X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1 X-MS-Exchange-AntiSpam-MessageData-0: =?us-ascii?Q?najRgJLohas4f2FI4BXsRbygLml+3Y/LxT/cVSgE23YPlUa8OgPxz/g7Ojvv?= =?us-ascii?Q?iW8Sh++GTqgtZ3sKvnGNdHKp/MSCTJUUUZK/7jrDwxYP6kYkZKoadLqEiG8J?= =?us-ascii?Q?+zHd04Jx5t0xPQWM/J/IiVJk6cwMP6zXdBydG6yaZmRNo0f0fAh/ZjW/IFPG?= =?us-ascii?Q?qJITJBeEmcRBAlwDl4Vl31G6/bc0SvQzWvmG0c/qyKgl0jDsZgY5rxmEFxln?= =?us-ascii?Q?a1FE+coYjFUJb0GrBOUmxNSiZUKt6q/caSjgsTz1VHn6pY8M68TA27Rxwkxf?= =?us-ascii?Q?vZrQm/qUMwpmqZf9SpSUsZbOXYedANKoFnFDoX7HYsF/jYkcIlStRR336ESa?= =?us-ascii?Q?wxNkh+VKV1/e/8TK6jN+t/M3DPvC74tu3Gm3CyLweVf5JEVK0YeSXCOIQELl?= =?us-ascii?Q?tgHXaI0ob1QUXUBw4icnsGEGuQDohlK26vY5ZgmOkf6xiJDOjfIiTbRot/wB?= =?us-ascii?Q?8NmU3lvIHwTmxafXMEGQo48xTgvub6TO1msQB3/g0d4VBhXYRguDUpAS3Z/2?= =?us-ascii?Q?5IJCPgcEtmzSjhzsSUaBfU6FqsYpJvBNFGIQsi8DTRnyybxPz+L+557lKU4n?= =?us-ascii?Q?8Y0Go/A136797tWKriCddRquQWC1t74jMdjSIMsHrQFO4MnBCB6IubLav2sI?= =?us-ascii?Q?u83CdovFhmxmw3TvsR8dbtBJxPCQJAKyB0hF61HvD7zqs1JbzT/fSi7OmdRp?= =?us-ascii?Q?lFpoArzyz5VRNSmbUJFW11hhV9s8lvjx0lCjpxD3lQG9plNPsrZsvJw9ZCdq?= =?us-ascii?Q?3dhLxqEB6ayHyssYs9AbIHiIhAQIoWP+RscP+pKXoOEj9/+qef0OOVZDiNxo?= =?us-ascii?Q?cxwrh6sfJ1WG1NYKkRKNZXrD7VQWmd+lPa9eSgSjTLjTzsSJbaJ+bjjijyuj?= =?us-ascii?Q?z10mLqXHxcoSsDNqyPKFmcQ+pVBxV27qhPKyVdIC6lBE433JZM/LWrlcznDr?= =?us-ascii?Q?fYj8eFTtDB+gQpKJnb2vZIVxmdm5OYCvh0Xs2wIcXvZ/oA8qcmL11QGFviYt?= =?us-ascii?Q?dGbqpckN9rGySZWu5eTkr2HUPCk/5Zavi6MT+LcSo1fYx0pUOKeQFT/peI45?= =?us-ascii?Q?+90KUmqFyoPFYOgC8626QUZ0wM7Z4DOpoXFVYFDIfubNNCGr0sffBTFWMPVO?= =?us-ascii?Q?WsOr9XShIdwSpbA3n5rhgWRcJMnsulNgdgRA1Ky+spsTTkOyT08RlzTvESEC?= =?us-ascii?Q?gptTgJQbJAqLe41eh7xCFvfBrXQPvuRkeG5FlEMrHxRVKTHp8G2q6hHMjfZt?= =?us-ascii?Q?voO3Xd6aonnD9vGzCSisDCndfJJOJGcOj6ygEkwdaeFdXyy3a33FrSluKeGX?= =?us-ascii?Q?J7E3SxEaubfBJgsCqW7nkDYq3L/kGwVuiSGfJjastRCTr4pLkEtmPfLSBJ9i?= =?us-ascii?Q?Wc+IRWbjdLm4VEIzjeo7o+TuMg4/IvS8FQtKIVja3ZoUZHvZDdw+IlFv/dAH?= =?us-ascii?Q?xJopEg1AvJAO2B+tLCjYfJCYDTAZ+VUe0WIV3VarJWNF/XiYs7vvjV7lGgWO?= =?us-ascii?Q?VjP+KuZoK7FR3cERhOZ64v1fsxXXIRA79IdA2c4t+HMoPwmndzVC6Yz8Cb29?= =?us-ascii?Q?mPE+6F2NpLg9FdTF1+fDf9DMRTD6k6HM2b15bvUo7IfpmWWMpoC7xuNR9W6q?= =?us-ascii?Q?/w=3D=3D?= X-MS-Exchange-CrossTenant-Network-Message-Id: d7ac2f2e-1afd-45f0-0e7b-08dc9cd7d280 X-MS-Exchange-CrossTenant-AuthSource: DS0PR11MB7309.namprd11.prod.outlook.com X-MS-Exchange-CrossTenant-AuthAs: Internal X-MS-Exchange-CrossTenant-OriginalArrivalTime: 05 Jul 2024 09:49:54.5324 (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: tPYI89kE5EGdAzy3NXoAuho3mNsFWuyk6zR/n4xPG+G61Wlbjdm43X9ErRMOc5MaLzLbjRBU6wwjhBsai/NiY2IatQ2nElC98Nr+I+4l6QE= X-MS-Exchange-Transport-CrossTenantHeadersStamped: PH7PR11MB7596 X-OriginatorOrg: intel.com X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: DPDK patches and discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dev-bounces@dpdk.org On Fri, Jul 05, 2024 at 08:30:31AM +0000, Soumyadeep Hore wrote: > cpfl_process_rx_ctlq_msg() is used to check error status > returned for specific opcodes and return error messages > accordingly. > > Fixes: db042ef09d26 ("net/cpfl: implement FXP rule creation and destroying") > Cc: stable@dpdk.org Thanks for splitting the patches. While the other patch definitely looks like a fix, is this an enhancement or a fix? More comments inline below. > > Signed-off-by: Soumyadeep Hore > --- > drivers/net/cpfl/cpfl_fxp_rule.c | 52 ++++++++++++++++++++++++++++++++ > 1 file changed, 52 insertions(+) > > diff --git a/drivers/net/cpfl/cpfl_fxp_rule.c b/drivers/net/cpfl/cpfl_fxp_rule.c > index 0e710a007b..4232b192ed 100644 > --- a/drivers/net/cpfl/cpfl_fxp_rule.c > +++ b/drivers/net/cpfl/cpfl_fxp_rule.c > @@ -60,6 +60,52 @@ cpfl_send_ctlq_msg(struct idpf_hw *hw, struct idpf_ctlq_info *cq, u16 num_q_msg, > return ret; > } > > +static int > +cpfl_process_rx_ctlq_msg(u16 msg_opcode, u16 msg_status) > +{ > + int ret = CPFL_CFG_PKT_ERR_OK; > + > + if (msg_status && > + msg_opcode == cpfl_ctlq_sem_query_rule_hash_addr) > + return ret; > + > + switch (msg_status) { > + case CPFL_CFG_PKT_ERR_EEXIST: > + PMD_INIT_LOG(ERR, "The rule has confliction with already existed one"); "The rule conflicts with an existing one" > + ret = CPFL_CFG_PKT_ERR_EEXIST; > + break; > + case CPFL_CFG_PKT_ERR_ENOSPC: > + PMD_INIT_LOG(ERR, "No space left in the table"); > + ret = CPFL_CFG_PKT_ERR_ENOSPC; > + break; > + case CPFL_CFG_PKT_ERR_ESRCH: > + PMD_INIT_LOG(ERR, "Bad opcode"); > + ret = CPFL_CFG_PKT_ERR_ESRCH; > + break; > + case CPFL_CFG_PKT_ERR_ERANGE: > + PMD_INIT_LOG(ERR, "Parameter are out of"); > + ret = CPFL_CFG_PKT_ERR_ERANGE; > + break; > + case CPFL_CFG_PKT_ERR_ESBCOMP: > + PMD_INIT_LOG(ERR, "Completion error"); > + ret = CPFL_CFG_PKT_ERR_ESBCOMP; > + break; > + case CPFL_CFG_PKT_ERR_ENOPIN: > + PMD_INIT_LOG(ERR, "Entry cannot be pinned in the cache"); > + ret = CPFL_CFG_PKT_ERR_ENOPIN; > + break; > + case CPFL_CFG_PKT_ERR_ENOTFND: > + PMD_INIT_LOG(ERR, "Entry does not exists"); > + ret = CPFL_CFG_PKT_ERR_ENOTFND; > + break; > + case CPFL_CFG_PKT_ERR_EMAXCOL: > + PMD_INIT_LOG(ERR, "Maximum number of hash collisions reached"); > + ret = CPFL_CFG_PKT_ERR_EMAXCOL; > + break; > + } > + return ret; > +} This function seems overly long, and doesn't need any branching statements. Would be shorter rewritten as (not tested, just to give the idea): { const char *errmsgs[] = { [CPFL_CFG_PKT_ERR_ESRCH] = "Bad opcode", [CPFL_CFG_PKT_ERR_EEXIST] = "The rule conflicts with an existing one" .... }; if (msg_status == CPFL_CFG_PKT_ERR_OK || msg_opcode = cpfl_ctlq_sem_query_rule_hash_addr) return 0; PMD_INIT_LOG(ERR, "%s", errmsgs[msg_status]); return msg_status; } While something like above should work, if you don't want to use the designated initializers for the error messages you can just define them in a regular array, since the error code are sequential from 1-9. Also, another suggestion: since we can shorten the code this much, do we actually need a separate function just to print the error messages. Maybe define the descriptive text for each error message in cpfl_rules.h beside the enum and put the error printing directly in cpfl_receive_ctlq_msg() function. > + > int > cpfl_receive_ctlq_msg(struct idpf_hw *hw, struct idpf_ctlq_info *cq, u16 num_q_msg, > struct idpf_ctlq_msg q_msg[]) > @@ -92,6 +138,12 @@ cpfl_receive_ctlq_msg(struct idpf_hw *hw, struct idpf_ctlq_info *cq, u16 num_q_m > > /* TODO - process rx controlq message */ > for (i = 0; i < num_q_msg; i++) { > + ret = cpfl_process_rx_ctlq_msg(q_msg[i].opcode, q_msg[i].status); > + if (ret) { > + PMD_INIT_LOG(ERR, "failed to process rx_ctrlq msg"); > + return ret; > + } > + Another argument in favour of handling the error message here directly is that we can avoid having two separate PMD_INIT_LOG error lines. Using a global array of error messages we can collapse the two into one, which would read better: PMD_INIT_LOG(ERR, "Failed to process rx_ctrlq_msg, %s", errmsg(q_msg[i].status)); giving the nice single-line output e.g. : Failed to process rx_ctrlq_msg, bad opcode NOTE: watch the capitalization for the error message strings in this case. > if (q_msg[i].data_len > 0) > dma = q_msg[i].ctx.indirect.payload; > else > -- > Regards, /Bruce