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 4F81845A9F; Thu, 3 Oct 2024 13:39:27 +0200 (CEST) Received: from mails.dpdk.org (localhost [127.0.0.1]) by mails.dpdk.org (Postfix) with ESMTP id 1F39240299; Thu, 3 Oct 2024 13:39:27 +0200 (CEST) Received: from mgamail.intel.com (mgamail.intel.com [192.198.163.10]) by mails.dpdk.org (Postfix) with ESMTP id 34FB940298 for ; Thu, 3 Oct 2024 13:39:24 +0200 (CEST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=intel.com; i=@intel.com; q=dns/txt; s=Intel; t=1727955565; x=1759491565; h=date:from:to:cc:subject:message-id:references: in-reply-to:mime-version; bh=L2nvAOnLuPEJGhBxxL7z3qk7ToLsBdij1v7FHUyha98=; b=QhC+44SVSz5E/16ccyRJue30K3g3Ob5Bx7yYM8l4EE3jmrFnimAj12ED FHNmjozXRYEKPhcfAgP0Hz3YMhPXmUDFwapJppFloeLvEiYhXT6ZviVva 6QVUO3xNqn5jT9XH0yM8xU3UNjPbtTVsjFaTMmFj1W+RJhpG7tiuvb+3r uO1TrA0MB4Vz1fsLPUWUfVsefBFm2sVx+PEu7YCFtQbqOFw8Xm42wjHqH V7ImjMFQqJY5SWiJfapuuLJ7HnJe3/WfAqewJVuavPOMcNdIYb/R2XWmV cZ86tev7cEp0QAzhpfytq9p//FYfHeoZqouaRz1czxH8ANVL07SgUZ0OP w==; X-CSE-ConnectionGUID: fhomWMI3TxmoKC9KMAB0rQ== X-CSE-MsgGUID: +Z9+vBNGS/C3SFAHP5K+bw== X-IronPort-AV: E=McAfee;i="6700,10204,11214"; a="38503348" X-IronPort-AV: E=Sophos;i="6.11,174,1725346800"; d="scan'208";a="38503348" Received: from fmviesa008.fm.intel.com ([10.60.135.148]) by fmvoesa104.fm.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 03 Oct 2024 04:39:22 -0700 X-CSE-ConnectionGUID: kP86TLvCRsKvw42cpjUfvA== X-CSE-MsgGUID: WRhViYrcQCm+76Yzw/QUbA== X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="6.11,174,1725346800"; d="scan'208";a="74427674" Received: from orsmsx601.amr.corp.intel.com ([10.22.229.14]) by fmviesa008.fm.intel.com with ESMTP/TLS/AES256-GCM-SHA384; 03 Oct 2024 04:39:22 -0700 Received: from orsmsx610.amr.corp.intel.com (10.22.229.23) by ORSMSX601.amr.corp.intel.com (10.22.229.14) 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 04:39:21 -0700 Received: from ORSEDG601.ED.cps.intel.com (10.7.248.6) by orsmsx610.amr.corp.intel.com (10.22.229.23) 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 04:39:21 -0700 Received: from NAM04-DM6-obe.outbound.protection.outlook.com (104.47.73.49) by edgegateway.intel.com (134.134.137.102) 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 04:39:20 -0700 ARC-Seal: i=1; a=rsa-sha256; s=arcselector10001; d=microsoft.com; cv=none; b=lqg0YKcJA9+sph6MpdWkyvRErX+YXuZdY2G7NkKE4LHlG9jqoNYT9m1B8DzSUOdLBibb8r9Dq2xF1Qd03DmAbk/wqGrlNFcZkAWE3HIUj1fBXMRvqTAnOPilrtU4FWQ04ODaMmyLEKJ4zOOjwtfxoqLu1KNuY95rUVwXyQgxvLzkG7hr5xwxviB+M5bASdbJoxvFmlU/h5+3ncqTzeGoPaVa3wSLLQH/uFq8YgSYpMQ2pPzZUx+IeInmUD4YUxSLKh2hYnPQOYNpAN611MACEmHrt+6VQ3cg73IVpIQlTP+QvmYxaEicHca/o/SuZmCqyV1CrM4n7JcbYJtZo58n/w== 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=rBqpUTovthtfk5ui8zLwbjlvgajuZIYvMo4pp8u/TQg=; b=P7OpxphnX7d4sY7VoQK0eipJInCJhZL7ieiu6eEdpnxZkGURAPJ7RWKfKgYwlFp7pol8COMcJXRPreHlnhapSDo+y78ctG9M3vzFtHWOLixKIZ7jY6psC7vD24NZXelgGMASfmi92tzKzMB8nqKr6wKbe38WwTVh7tC5wRpfBeaMvDIlUykWDWMXwHZPqGgNv3kKoPZ/wYcNlSIdKxNC+4MWNDnE6lfqTR51+6GmuW/iP4gfn2uDbsy665KZSctpk1osEd2S0CPe/JbXpag+PzpucmXSVsaHb5cIqOUWQpOUfNi1g4QkNzd+uueH6Z8jBwoG23sj8kRRhQ6W1y+p2g== 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 DM4PR11MB7374.namprd11.prod.outlook.com (2603:10b6:8:102::11) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.20.8026.18; Thu, 3 Oct 2024 11:39:18 +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 11:39:18 +0000 Date: Thu, 3 Oct 2024 12:39:11 +0100 From: Bruce Richardson To: Robin Jarry CC: , , Subject: Re: [PATCH dpdk v2 1/2] telemetry: add api to register command with private argument Message-ID: References: <20241002155709.2522273-1-david.marchand@redhat.com> <20241003112438.902397-6-rjarry@redhat.com> Content-Type: text/plain; charset="us-ascii" Content-Disposition: inline In-Reply-To: <20241003112438.902397-6-rjarry@redhat.com> X-ClientProxiedBy: DB7PR05CA0015.eurprd05.prod.outlook.com (2603:10a6:10:36::28) To DS0PR11MB7309.namprd11.prod.outlook.com (2603:10b6:8:13e::17) MIME-Version: 1.0 X-MS-PublicTrafficType: Email X-MS-TrafficTypeDiagnostic: DS0PR11MB7309:EE_|DM4PR11MB7374:EE_ X-MS-Office365-Filtering-Correlation-Id: d1910a2d-0807-4839-0449-08dce3a003ff 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|1800799024|366016|376014; X-Microsoft-Antispam-Message-Info: =?us-ascii?Q?49H9wNrDGovd/c/a/uHeOKZ7F+unOTyts6+P1BLtAoKarFAHibYgUESTChKC?= =?us-ascii?Q?0LJPIy7+AYPF3pdjbWdaYYWhWsTShDvRGAsNnydjUglzsWecnGsVmiIUTvn/?= =?us-ascii?Q?xBbEYQ6oSx4i2EKnK3mfMrUr8Q9H1jBkaI6FqYe4ZozsH3sew55Kn3dZdZ+L?= =?us-ascii?Q?ltqnC+XN23cwl1YOuD/RUptEoNo/odgBlFjT74gRDJkwqvzhWFNM8k6vN8mc?= =?us-ascii?Q?yqWYPmH2Td8YbOZI8K4Auzli3OK6ur9PRzuZdXGC7PJYMBZ7/peQ+TZPVtYy?= =?us-ascii?Q?fjw7wgKoBsaYg/P3DcB2406P9dwFc6iuS38x5WbqMJFvmklRDBHBWyOy2glZ?= =?us-ascii?Q?OE892/mvNpdgdqrtUujLtykrT9K01LTOagjYWXAeWiopbX1W8FWRqESuFjrP?= =?us-ascii?Q?lso7jxVL7YWjEEhwztZzBUB6uqoqYcHMCoNQRyGJ9z5SKu9ek5TMoTU/0KOA?= =?us-ascii?Q?tA0lwyiNql06e6ymiabYbt5n/0f5wlRHHs5nl8+V1V2316g5XUsfaZcUzoec?= =?us-ascii?Q?zTtKk8zU5RbZT1322b1UD7x/zNWHBhPPZuh7fIiLVyIJ5IdhOFiu43nVNVea?= =?us-ascii?Q?SgPkQQJDzDlOZjQnh0TGbdd0zKBJuicVeNPCNbaG5Dq6fd1Megf3q+DfnSzk?= =?us-ascii?Q?LEuvo809CeDpXki8xpZ0SJAvHrm9wpkX5O6T2YDrNiHqort5JR3UsOdmfcf9?= =?us-ascii?Q?z5b62Ns4FIzU+spHeWRHLAqRQCmKI1129YWUOL9Xk5R3Sm8TP59yMlmN+QGx?= =?us-ascii?Q?G+awTBeJlPs5CpXRAQY0+7+P8Dw95gRsiEiL6EpyoTmZOueRyT0HzpPH6ZsQ?= =?us-ascii?Q?rampJZvljj/imYYQ/iVymbiO18fR/8cyGPAasZl2mbdfFhGSEjxBY1wFi4JJ?= =?us-ascii?Q?7eHDix4Xa9/11kvWEX22kXIDilhFqMbuySLFZ/lvMdkFyb1REofiGu7lnx6x?= =?us-ascii?Q?fy6xwyIBX/EHOtPDPzqgviLkBfyoyyd7CvTFoaw3lvyiex4sTl8rXNqe9/q4?= =?us-ascii?Q?MPTAC2DnK4EuIWlY9eoYykA4dYUFlYHcENxYi/VVOvCa5yVVZLPtEJgYUaPl?= =?us-ascii?Q?nihYhv5039NHlyfgAauzVCZV/o7CUliO4xExfuz8YBlooClDLObGER4vYgJi?= =?us-ascii?Q?wAmI9tZu7w+ZqSlW9QIzFhiAVYrPrCS85nSZr9jcCWshWQSvnl4vwWxYPOMo?= =?us-ascii?Q?Eom9fMksc8bxnVOCWdaMSiINKnMnvIC1/RKT6J/X+gzBaZO1SlSL3rf2NtIJ?= =?us-ascii?Q?Y9pCB0gz0FyAyYb6HF7TgaFDvvdnWnYthMTkNh0P/A=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)(1800799024)(366016)(376014); DIR:OUT; SFP:1101; X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1 X-MS-Exchange-AntiSpam-MessageData-0: =?us-ascii?Q?nDvNS5OKiNZHzj02gwq02fmbXiz//sd7CiwSQNE5jfGl+oGgnb1SAuABXbZB?= =?us-ascii?Q?QoaWyCWWhcN6WX49VCPKuhFNG7Ca/OK5GQ4Un/keprGmvfyypGEchCud+roy?= =?us-ascii?Q?1e18UTm6NkFEjsLKaQ5vLw0q9OcFv4QO5i1CTHIwVinbiycM0VEBtyYjJcAy?= =?us-ascii?Q?6g4tqC4f3PBk0s/zb4+DD06bK1DSB4o7qvT5o0ny6QjvNccHtctg2eqwfw+k?= =?us-ascii?Q?mQeMA1f0woiHNgiXxEM1KeUPNnK8+iRIG2lv6hvVVulnxkjhSn9TafPl6gx4?= =?us-ascii?Q?nVrZYoYp7D0m/KIKiDlAIUigsBLOV37pNSU+iq7Be0vnlQRs8ND5HVWUYnZz?= =?us-ascii?Q?C1KeK/HfgMYeg+EflY4G01VzUBkXKedR1ihEDCK+nlBvN35I1lM5k8KUFFm4?= =?us-ascii?Q?uKaBetGoexXpZgzvKUL/3AKfXrVFrLG7bg3sdyjXQPA0YMBLca1ZcjdfDTmE?= =?us-ascii?Q?0y32NFHD/z30LsTYmz4kNI4AtdbFc9+maXfJg5QhbYKxOnYe0s5IhhY7rO7k?= =?us-ascii?Q?QqL/ryfGZTyP7Z3/nRzbBdVsBS+cPIKYU1IR+ig+c6y75KReKJCznru26RfI?= =?us-ascii?Q?CvBn1W7E1emH7g8HDv+V1+LWZoer5Ochk+dCzOVCEqu+WEtUwotRD1M7vBjE?= =?us-ascii?Q?+mwJGlB2MPA9TJWQne155gt7rvIdAuV8pbdqhkAhWGlROQ3yOxy8qEkNJL98?= =?us-ascii?Q?Ey95Rid6vw28JoEyQQiu3ycMGlbTlnBqLqEZPFdcqubp4niL/2UCS+WYX+b/?= =?us-ascii?Q?HlPPOH7XOT1XzCFClQJNk6qQJDnvzdRCZMRazQHOzTX4bsvQ/BuyVnHKOw9+?= =?us-ascii?Q?Sd1moe7juDglzo/hXBDQafa4SbICjC1dXirnTD+TOi9NDyiWVnaygsD6CB6O?= =?us-ascii?Q?RwCW2rNbvFyDsLabAkiXH1xY6A1IvXQQ1gyqjkZeeOk/Cj3pd39+9FEnEMJX?= =?us-ascii?Q?Ho4CscAdKOUxjzBJ6IBHF1ESdM5l4xzSbjXZ+rNdOMwdlNK08dCoc0CVIcrq?= =?us-ascii?Q?PR79rBtMKBVpYCj4Ssy1ZD3YjNQDmYutQ0ND93mh8V1Fq18SX2M+6+tXlMAg?= =?us-ascii?Q?+MEagELLKgrGOVx+sW8Wt+ZfBpyHHWEz+OWDi9VipZj9PMtiy/w2YxhOx20+?= =?us-ascii?Q?LBuBQ0T9uaJSkOJ9SQv6O3+2A2RV/RxCG5ozpiiVLhK0kqVSrqHVBmMQSiEa?= =?us-ascii?Q?04kSkF65f3L7K0P8QC54gWu/oool5cm/1hfpgJ31RVlqzuYUDlfMykl6l39D?= =?us-ascii?Q?Xd9yFGJLrU57DqQSkYb97JIkIKFx/mSNZHLHyXQeNhKZ+gk1EtJl1IO/ZXOs?= =?us-ascii?Q?lbQdpj7DdrOS0+3Ozb1JckcffKBwPIA8SwmmWgLyXjARtJch2S6TJVkQyvb4?= =?us-ascii?Q?J8SBa0rmXRwbXNWuoq32OBj7KbZ2xMbKfCjVG9+O3z+7KKEMORhxIM8gJP8B?= =?us-ascii?Q?BMEd+HoStMnl8WaCe0WC7Lra3mItL2u+AgNXcUm8QJA9TQ/un+mHrvZEa51R?= =?us-ascii?Q?FbKTvT8b05nl6pSINplE9CE67MKIgWFWV9CBKnWCWUcATVGuUn1Llgi9wv3d?= =?us-ascii?Q?3N+n9GRRxGqRjWLbkTg5LZCBHWL1cPpXBaqJgiOJ+2df0aOKmvAoxH7Xg9QH?= =?us-ascii?Q?7g=3D=3D?= X-MS-Exchange-CrossTenant-Network-Message-Id: d1910a2d-0807-4839-0449-08dce3a003ff X-MS-Exchange-CrossTenant-AuthSource: DS0PR11MB7309.namprd11.prod.outlook.com X-MS-Exchange-CrossTenant-AuthAs: Internal X-MS-Exchange-CrossTenant-OriginalArrivalTime: 03 Oct 2024 11:39:18.2199 (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: VrWtch5KmU0d8tlegrPacm3NtE6grOLMoelWO3wXppI9VqgFag+BTcE7cwLZj4biXpxZHZNKmiYGPr35GUyOBMbiXzPBfM7q/t/pNHQgH+k= X-MS-Exchange-Transport-CrossTenantHeadersStamped: DM4PR11MB7374 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 Thu, Oct 03, 2024 at 01:24:41PM +0200, Robin Jarry wrote: > Add a new rte_telemetry_register_cmd_arg public function to register > a telemetry endpoint with a callback that takes an additional private > argument. > > This will be used in the next commit to protect ethdev endpoints with > a lock. > > Update perform_command() to take a struct callback object copied from > the list of callbacks and invoke the correct function pointer. > > Signed-off-by: Robin Jarry > --- I like this solution, and seems a good general addition to telemetry also. Couple of minor comments inline below. Acked-by: Bruce Richardson > lib/telemetry/rte_telemetry.h | 46 +++++++++++++++++++++++++++++++++++ > lib/telemetry/telemetry.c | 38 +++++++++++++++++++++++------ > lib/telemetry/version.map | 3 +++ > 3 files changed, 79 insertions(+), 8 deletions(-) > > diff --git a/lib/telemetry/rte_telemetry.h b/lib/telemetry/rte_telemetry.h > index cab9daa6fed6..3fbfda138b16 100644 > --- a/lib/telemetry/rte_telemetry.h > +++ b/lib/telemetry/rte_telemetry.h > @@ -336,6 +336,30 @@ rte_tel_data_add_dict_uint_hex(struct rte_tel_data *d, const char *name, > typedef int (*telemetry_cb)(const char *cmd, const char *params, > struct rte_tel_data *info); > > +/** > + * This telemetry callback is used when registering a telemetry command with > + * rte_telemetry_register_cmd_arg(). > + * > + * It handles getting and formatting information to be returned to telemetry > + * when requested. > + * > + * @param cmd > + * The cmd that was requested by the client. > + * @param params > + * Contains data required by the callback function. > + * @param info > + * The information to be returned to the caller. > + * @param arg > + * The opaque value that was passed to rte_telemetry_register_cmd_arg(). > + * I think we tend to slightly indent the text on the line after the @param, as in: * @param arg * The opaque value... > + * @return > + * Length of buffer used on success. > + * @return > + * Negative integer on error. > + */ > +typedef int (*telemetry_arg_cb)(const char *cmd, const char *params, > + struct rte_tel_data *info, void *arg); > + Not sure about this, but I'd tend to have the arg parameter as second parameter, to keep the "info" as the final parameter. My suggested order would be: (cmd, arg, params, info) > /** > * Used for handling data received over a telemetry socket. > * > @@ -367,6 +391,28 @@ typedef void * (*handler)(void *sock_id); > int > rte_telemetry_register_cmd(const char *cmd, telemetry_cb fn, const char *help); >