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 622FF45A9A; Thu, 3 Oct 2024 11:46:50 +0200 (CEST) Received: from mails.dpdk.org (localhost [127.0.0.1]) by mails.dpdk.org (Postfix) with ESMTP id F10B64025E; Thu, 3 Oct 2024 11:46:49 +0200 (CEST) Received: from mgamail.intel.com (mgamail.intel.com [198.175.65.20]) by mails.dpdk.org (Postfix) with ESMTP id DA445400D7; Thu, 3 Oct 2024 11:46:47 +0200 (CEST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=intel.com; i=@intel.com; q=dns/txt; s=Intel; t=1727948809; x=1759484809; h=date:from:to:cc:subject:message-id:references: content-transfer-encoding:in-reply-to:mime-version; bh=LjmTz2JIyO68EL+XJhbUywwhhWdcvvMcqRsO5y8Oimw=; b=CVRt3tsyPhN0QA+Y9MzolVx6uG7rPwygndv9xtEEXlwjsG8mNROGfNiN csAKPU9UuZiL4IRsVKkkfdYhamHmwm2ZooPHH/dgHImUFC9JTZcH8o98J nvWXPYxYxZcMeiZK6E2wDc94zqUKa9fI974A9xCRuk3q7NLtjiOI2EFg+ +mEgAnYZCfEl62zD2Szra8V+Exmf1J71hfRF+qaQW9z6Cb6lHBsYFvlbY GQ3lH1DLWMbOwfi3vEjDrVY2ngtV/oN2Xe/HbvA0Z5rdhoQNzbslQJKbq ha/PjWM7a1ymwjs8TeyKkC6oaR5CHCK1qaN5AgLkYCvEOS0dEu66mfPe1 Q==; X-CSE-ConnectionGUID: WmGYiEOsQ/O9YYQKGrr8EQ== X-CSE-MsgGUID: tpR5aSjWTNeZLauK7a/DFg== X-IronPort-AV: E=McAfee;i="6700,10204,11213"; a="26942740" X-IronPort-AV: E=Sophos;i="6.11,174,1725346800"; d="scan'208";a="26942740" Received: from orviesa003.jf.intel.com ([10.64.159.143]) by orvoesa112.jf.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 03 Oct 2024 02:46:46 -0700 X-CSE-ConnectionGUID: RBsERqErQQaGV7R50tqaHQ== X-CSE-MsgGUID: 3bHbOz3qRc+fm2eTmtlnkg== X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="6.11,174,1725346800"; d="scan'208";a="79144882" Received: from fmsmsx603.amr.corp.intel.com ([10.18.126.83]) by orviesa003.jf.intel.com with ESMTP/TLS/AES256-GCM-SHA384; 03 Oct 2024 02:46:46 -0700 Received: from fmsmsx603.amr.corp.intel.com (10.18.126.83) 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; Thu, 3 Oct 2024 02:46:45 -0700 Received: from fmsedg602.ED.cps.intel.com (10.1.192.136) 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 via Frontend Transport; Thu, 3 Oct 2024 02:46:45 -0700 Received: from NAM12-DM6-obe.outbound.protection.outlook.com (104.47.59.170) by edgegateway.intel.com (192.55.55.71) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.1.2507.39; Thu, 3 Oct 2024 02:46:45 -0700 ARC-Seal: i=1; a=rsa-sha256; s=arcselector10001; d=microsoft.com; cv=none; b=GzAena9EwE0UYT/UKoOLMHlbrJ4AGMoR2NNAdiVFtnpe8AJ6QpERpKZt4gbwh8lGDJBaLJc3OVDJ9N/FwRecsOpyMNvA1swvEE57Qk6ls8Y8zKYw9ewYQKT4zK0/u5mx1kZhRHag/wvYDD8JQxjbrZmTbOvAzElQ0P8SyCMzYvz8W3wJD7ZGXNraeYDHZXZ39pSRinheFGaWwGSy13+ke99b1gTLBnX7UjazX96INUCYcfCcc9BHXkRz4DrO0S7dHP7Vk80wGZA68DVKTDV0ojmYNkGCbn+rHieT/vYlmWrY3Lz17KfqcrddWqwokUMrYnN7+jIfL6RatKXtBtnGxQ== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=microsoft.com; s=arcselector10001; 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=7hTTk7F+kCnZ8XLd0PpJtLb0sTEs+msAS+Lj72qXD6o=; b=erasXobVoVtQA2wus5aLgOkI1NSWlQ3qNPU3dkm0D2NP52axbu59P8mYx2w6feERdrfFsZEVLnZnWPhG0Ri4A/D86zf2PJ0ev3ZIn+0LBWrtGotgdK4+QPTb+JI2oCCHbhT2KUkdcq/ru8dlERgjnt4UaLWhql3649XdK0WvktRlWbWBJppqpsjtuW4q35lSVcjmPLNGUbRuIjJM2Tyw184cnVHy6CPBQ0EKuiU6oiKxk38CTlVzEoolEHmnUG68Y1SNFx2I0Mpiknr3Gr8yhEm/aE6pf4zNqfpLtHSGO6IMJzwJElQ4UQ9JVTZd9LZTcSpB6Q4K3MMVWcNPRjXkPg== 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 MN2PR11MB4533.namprd11.prod.outlook.com (2603:10b6:208:264::19) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.20.7982.28; Thu, 3 Oct 2024 09:46:43 +0000 Received: from DS0PR11MB7309.namprd11.prod.outlook.com ([fe80::f120:cc1f:d78d:ae9b]) by DS0PR11MB7309.namprd11.prod.outlook.com ([fe80::f120:cc1f:d78d:ae9b%4]) with mapi id 15.20.8026.014; Thu, 3 Oct 2024 09:46:43 +0000 Date: Thu, 3 Oct 2024 10:46:37 +0100 From: Bruce Richardson To: Robin Jarry CC: David Marchand , , , , Thomas Monjalon , Ferruh Yigit , Andrew Rybchenko , Keith Wiles , "Ciara Power" Subject: Re: [PATCH 2/2] ethdev: fix race on ports for telemetry commands Message-ID: References: <20241002155709.2522273-1-david.marchand@redhat.com> <20241002155709.2522273-3-david.marchand@redhat.com> Content-Type: text/plain; charset="utf-8" Content-Disposition: inline Content-Transfer-Encoding: 8bit In-Reply-To: X-ClientProxiedBy: DUZPR01CA0271.eurprd01.prod.exchangelabs.com (2603:10a6:10:4b9::27) To DS0PR11MB7309.namprd11.prod.outlook.com (2603:10b6:8:13e::17) MIME-Version: 1.0 X-MS-PublicTrafficType: Email X-MS-TrafficTypeDiagnostic: DS0PR11MB7309:EE_|MN2PR11MB4533:EE_ X-MS-Office365-Filtering-Correlation-Id: 13506ff7-afc3-4c17-ce4b-08dce3904989 X-LD-Processed: 46c98d88-e344-4ed4-8496-4ed7712e255d,ExtAddr X-MS-Exchange-SenderADCheck: 1 X-MS-Exchange-AntiSpam-Relay: 0 X-Microsoft-Antispam: BCL:0;ARA:13230040|376014|1800799024|366016; X-Microsoft-Antispam-Message-Info: =?utf-8?B?SUNKaWJONkY4TFQwNnp3TFVQLzVNTnVCcFpnRElmbmY3N2xmY29YamxHZEhR?= =?utf-8?B?alNRa3hia216SUwzaEg2VnFJaFRueFZzOWQzY2Y0c1I0VWUvSU1tdldJbGlC?= =?utf-8?B?bWg0amUzREZWQ2lYOGNjSm9waVpLS0krTmdGc2phejJWcjNrUjh1VVVVUExq?= =?utf-8?B?MUhScnR1SWpXRE5OQ2xFNjM4V092MjdLSnFLdDkxby9CV2wreUZVanl0N09x?= =?utf-8?B?WExSdUhnMG1iYy9iMGswanpUSU5hc3RnNG1SUWd3ejlCRUNNMlVoMmJ6ckdY?= =?utf-8?B?R01WaXFVbkpwOWVESGROUG5veVJVb2d0bVR1akxGVWtDa2oydjBMZGhzaktN?= =?utf-8?B?Z3lValQ0cGFjdGRpb21PSlNsYVR5c0FVdXR5MlNSNi8wQVI0ZTBsd2FTdC9J?= =?utf-8?B?S0tZTW9WT1NaMmZRcWd1L3VzdU5VM3ZzTmJYU0ljZjVXUzZYc0F1OUM1NzFv?= =?utf-8?B?NGwySm9GbnVEOGJkZFQrZ3dHNkZhZ1pSWUVIMTRsd1B2amVUZllZbzFkckVj?= =?utf-8?B?MDZXUzBXOXpqR25lTkk1SWFFeWswRkhpRGFCZTkxUytiR3NHSXhVZlI1RFpJ?= =?utf-8?B?MmoyQ1pyN2ZmTitYQTIveGhTaVRPbWhRaGNKUThIU1ZnTldicVRnVDlaWkcz?= =?utf-8?B?djVLT3RMOWU5K0VZamZDQWRheFowc3BGWTdLRlNURDNsbnp3eG01d0htVmdw?= =?utf-8?B?aGdPdWZxRXY0VTVEamNnSlFWYllSaUQzMzNkWVBScUY5RGVSNCthd0ZBMS9m?= =?utf-8?B?TVVoVVlmRnlKckVielRrTTU4V3QxV0p3S3hTTUUyTFg4aUliOEdRYXBJNWFV?= =?utf-8?B?TXgzbmcyejd6WVJMSFpvL0xqZzRUMTEvN2ltZEZrZEtUdDE2d3ZRdnRwYkxS?= =?utf-8?B?SDlaYkJ6ZXd1WjZ6UjNIenFKdUwzcCtienlpbjBZTnZBT2tPaDkvOEtvcndv?= =?utf-8?B?RzFXazhsVXg1MDhpcEN5OFp4SGg0NTk3U0szbWJpYkM3YmNRM0RWaUVUZ3gy?= =?utf-8?B?NDJjLzN5QVZCd1BKQVVORkRzTDVLR0dZeTloK0Y3SXFVTnlpVnhsRzFSdTBS?= =?utf-8?B?NHRVMTAyenlSUGNsNXBpL0F4K1c1aFlpbEgzcHc2WVJPQVYzUDdMU0x3akhu?= =?utf-8?B?d3NCaktmc29CbGdUL20rYnlwTEFrZzljMVpDMmlDdE9KRnNqN0t4NisrV21V?= =?utf-8?B?Mm9rNnJ1ejM1YklGS0xVeExldEprdWJ6T2R5Z0I1Y3V2Q2liUXZzTjJkVHl6?= =?utf-8?B?WUdIMXdCWHRGZHNGWThhUWxMSDdtV3JJMUZBejlYR1BNb0N3czU2T2ZsemZS?= =?utf-8?B?L3F6dlBCVXBmZk8zajBEUm1xTExnSVFrK0FPTkR2U3hCeDlNdFRpby9pcExU?= =?utf-8?B?WUlWdzJOTVRuejFiaEZMWmV3b3RrYTVFRksrZXRnUWxUUGhvcld4cHgrTEZW?= =?utf-8?B?aUxWUnRXcXJ5Mlk1cFNiUmFkaFh6VTExNFlYb2hGTWZHT3VTbk9vSEo5NDdE?= =?utf-8?B?eUZSOTlpUWZrNnQ1YnRhMTh4WStmSGF1cGxmQUlCUXRHSk5wZ2FESnVOOVN6?= =?utf-8?B?OWJMSGRHb0N4ekYvQnorODhrU2RiMzREdkh4UUVuOTAvY2xEa1JZMmoyU0ZF?= =?utf-8?B?ZVgydTU1azl5d21uWjl5dUFLVnBqb3NDaURWaUFVOVZYTFBTTVd6NmZNRkU5?= =?utf-8?B?b2YybFRJd0lQSS94USt0eWZRTjlzUDBUVGk4Q3l5RVdDSTFzUGkvYUVBPT0=?= 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)(376014)(1800799024)(366016); DIR:OUT; SFP:1101; X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1 X-MS-Exchange-AntiSpam-MessageData-0: =?utf-8?B?WUxWLzlBck1SQUF2b05DM3NuUTN3MWtzUFNnZm9COUg2a2hBUHV4bG9iUm9x?= =?utf-8?B?cjdOQW54NUNCNVozb0JzdnB6eEw5OU1wWnpUalhHYko0UDVHRElTNFZzZDBG?= =?utf-8?B?Q3pybUN0ZGRWeVBuUS9nWnpEcEZpWE1EZTZ3dTdRNzJYZXkxNWdzWnlpMTZu?= =?utf-8?B?bDhpaEswTnpSTVNZYmVzc0licG9lU0FmdWNhcWM5Z2FyVVRvWXpxYTZkRW03?= =?utf-8?B?RUFYTkYrOUlOQjZSZFRsVGdLdEFBOXkraXhnSGpUZGlKcXBiVkxhZnJ6TWQ5?= =?utf-8?B?WEFWbkZnc2diK1VHNkRuQlNlVEx3T1pqS0pZck5IaFoyMFFlVlV0N1J4LzZs?= =?utf-8?B?WmZnVUlWd2ZjUFdqWHFPRXl2em1QQTQ5aWNCdmx3VnFQSlJlUlpoUWdmWk5B?= =?utf-8?B?VjJGREl4QlZPUG9TOXJZSEJ0cG1Ydmg4bERhMWtHYS9uaXVsVDFuWm9Dd2NP?= =?utf-8?B?U09oWGkvVWZlejljZW1TeExIK1JqV2VCVit5VDI2NjNXMkdQbkZSSWdyMkg2?= =?utf-8?B?Mmd1ZnA4TUk2clBPY05MSjE5ZWxVcStQbHQ0bmFORXdCWFRaNmUwNE9EYUVU?= =?utf-8?B?Y0JpVFk0MEpNUVAyb0w3LzlrSGlINER2U21HUXk2d0t5ZFU3OWlXWXpLTDFU?= =?utf-8?B?dFU1WnJ3SWVRVW5IKy9oL2ZqNUhDWHFTclVEYVlSWm9jQWZMdnhGbm9pU2Vx?= =?utf-8?B?Q0JiYmpBUkllL3piM0MvOWpydHA4MFJrSmloTmZqSUxBN3VobGIwYyt5MUxC?= =?utf-8?B?cW5FOVNFMDQ4cVB0MHRhbU5KeW5yRmhlNktvL1hOQ2JIeUN4Y1U0VEpPNEVi?= =?utf-8?B?Rk94eEl6R0NYUC9aTVdSQ1BRSVlDWk5DbSt1dEliN09LYUJycW90b1ExSGZq?= =?utf-8?B?eVQ0RXhuSExMbEJPSHNZTSs5RXBLL0cxTmJlMGs1MnNJdVNZNEQwdm0zMkZy?= =?utf-8?B?bDRlZjM4YVZGWm5TblMyVnpGSWFJSmxkclFvdTdBVnhveExIY0JIWWpnQlRk?= =?utf-8?B?bkcza1dDRC9rN3E0S21lMDhpYWtwRVl3eWRraklOK2xERGoyRDJ1MWZxTGhP?= =?utf-8?B?d1dPY1dLM3dUKzM1MGJ2M1BtSE9kbHNZMnlsNlBmVFlEb1RycjJwOVdPUU1V?= =?utf-8?B?alh1N2YvcTNFK1ZqcytaUEtDbkllc0tESHlhdFg1N3VjaVRSaTJraHZMN2Vw?= =?utf-8?B?ZWVkQVhaQ21UVDNOSXhJWWJKNU81UVZ3YjVkZmJwRzFlM3JyUCtnbVAvTmkr?= =?utf-8?B?OUxORUZkTkYxbkRKMkxOYjBNaUUrMElzYS9ocllOMjlaUDB6Zi9zM3psYUNj?= =?utf-8?B?WDlvUVZKQkpaRStvOFI0NGV3dHZyZld1Tk0wUWZNNmV3eWE4ZXFhWkhZS1Jz?= =?utf-8?B?WDdJeE95RlJJV2k0OG12WmN5eEZWZkpTSmcvQlNwWkN5dkNWOWwxbTMyVmhI?= =?utf-8?B?UlRNMEFKS1gzdndoL1FxOEdLUzBIUmk0TW5qZTFqOWZyd0xPMVhDdXBBU1p5?= =?utf-8?B?WlZYVjc0SDhwT1BMMVhXdHlUNDJsK3U0bFc3ZkJQNmVzcEIwT1hyUzMvUDE2?= =?utf-8?B?WHZ0Qm5HSENZQ0JJT0hZV2RWNHZ2YXBHemhDcnlxWTFyQTlhZW40WENKcjc1?= =?utf-8?B?RXVoMXJidkJqMzFadHF0UEVnK3o4TmhpOHBFanBFZ0tBbGNPSEFFWU5oaXVz?= =?utf-8?B?dTZNOS9mV0dXM2ZLS3I4UzI3dGQxY3BTZnEyQnVXTndFejBBU3htWnEwUVg1?= =?utf-8?B?WVNPVXJwZXBUOWVUR3BXY2RqM21pTmhMUXF1QkplRURUYlR6UHN2ZloveXJH?= =?utf-8?B?QUNJRytwbVhnWHNWRk1LVVpGekhaaHVkVEdvQjExcVNXd3B6MEt2eG4wNUF4?= =?utf-8?B?Q1ZRSE1kdXdWRXk5OXZxS0JxdG1wQmZMUG5NRHF1enhQaSt4U2lESEJCeFdV?= =?utf-8?B?K3hPRDBMUXNTQzY2M1lSL2JkVmlJeHM5QXlVM2FKR2taYzFYLytvMWFwU1lO?= =?utf-8?B?cEZhc2xSM1RHY2xzRUZBc2ZoMVRtZFlHdWxncmxMQjFYYytubExqeXQ4amtz?= =?utf-8?B?bE9KaUNjaUI4aVVFb05PdlUzWXI3d1NkQ2x2S0RxcUF6N3BONTVXcjhDbVRD?= =?utf-8?B?dkZFRUJJeGN4c3d4TUtMRlZwa0hTNzI0OHRodXFkY0o5ckVOdmhlcytRV1Rp?= =?utf-8?B?Rnc9PQ==?= X-MS-Exchange-CrossTenant-Network-Message-Id: 13506ff7-afc3-4c17-ce4b-08dce3904989 X-MS-Exchange-CrossTenant-AuthSource: DS0PR11MB7309.namprd11.prod.outlook.com X-MS-Exchange-CrossTenant-AuthAs: Internal X-MS-Exchange-CrossTenant-OriginalArrivalTime: 03 Oct 2024 09:46:42.9632 (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: FJTy4Mmljwesd6fe3CldqEufOd1d4rL0udddsT90u0ejwxHkN5zI9sB/4B6qzfeVtlz5xpcGz/p5/QkQRuhyVw5ofyDkG9ep+r6ysHVi07Y= X-MS-Exchange-Transport-CrossTenantHeadersStamped: MN2PR11MB4533 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 Wed, Oct 02, 2024 at 09:26:10PM +0200, Robin Jarry wrote: > David Marchand, Oct 02, 2024 at 21:18: > > On Wed, Oct 2, 2024 at 9:09 PM Robin Jarry wrote: > > > I was going to suggest adding a rte_spinlock_t* parameter to a new > > > telemetry register function that would need to be held while the > > > callback is invoked. Or if we want to keep doors open to other kinds of > > > lock, a wrapper callback. > > > > Well, as you had experimented this approach, we know this does not > > work: the ethdev lock is in dpdk shared memory which is not available > > yet at the time RTE_INIT() is called. > > > > A single callback is strange, I guess you mean pre/post callbacks then. > > It could be a single function that will wrap the callbacks. E.g.: > > static int > eth_dev_telemetry_with_lock( > telemetry_cb fn, const char *cmd, const char *params, struct rte_tel_data *d) > { > int ret; > rte_spinlock_lock(rte_mcfg_ethdev_get_lock()); > ret = fn(cmd, params, d); > rte_spinlock_unlock(rte_mcfg_ethdev_get_lock()); > return ret; > } > > RTE_INIT(ethdev_init_telemetry) > { > .... > rte_telemetry_register_cmd("/ethdev/stats", eth_dev_handle_port_stats, > "Returns the common stats for a port. Parameters: int port_id", > eth_dev_telemetry_with_lock); > .... > } > > I'm not sure which solution is the uglier :D > I don't actually mind this latter solution, except that the order of the parameters should be reversed (and it breaks the ABI, unless we add a special new function for it) For me, the wrapper function should be the main callback, and the real (unwrapped) function the extra parameter to be called. That extra parameter to callbacks should just be a generic pointer, so it can be data or function that is passed around. rte_telemetry_register_param_cmd(const char *cmd, telemetry_cb fn, void *param, const char *help) Or more specifically: rte_telemetry_register_param_cmd("/ethdev/stats", eth_dev_telemetry_with_lock, /* callback */ eth_dev_handle_port_stats, /* parameter */ "Returns the common stats for a port. Parameters: int port_id"); /Bruce PS: Yes, I realise that using void * as function pointers is not always recommended, but we already use dlsym which does so!