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 80AB6A054A; Tue, 25 Oct 2022 12:38:50 +0200 (CEST) Received: from [217.70.189.124] (localhost [127.0.0.1]) by mails.dpdk.org (Postfix) with ESMTP id 256E7427FF; Tue, 25 Oct 2022 12:38:50 +0200 (CEST) Received: from mga01.intel.com (mga01.intel.com [192.55.52.88]) by mails.dpdk.org (Postfix) with ESMTP id B193641143 for ; Tue, 25 Oct 2022 12:38: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=1666694327; x=1698230327; h=date:from:to:cc:subject:message-id:references: content-transfer-encoding:in-reply-to:mime-version; bh=RJiHrOWfNeR75odTvKwnnVX7cdSWI9wKSf+KujIDJNI=; b=N1yG9/BdaZxCxgM3Q0iXQR9MnJ4umEJxpPnUVG7+tWhS9mYkOEesXGH2 L0rmaD7T+PZNvJ0uI+O2uNINvZ1F5bvJAFGWMCGARAPlBFb8kmvRDazh/ 02Ar9an0KHcD4TfTweO+Stsed31K+xfDl+0hRkLtIw/n9Z7jYaYr+oSAn dKAj5wIHg2CUvlGA1F2MXN3LhJLr5aTT3bMYeiZ36TgtUIy/5Y+ueSkK+ zT/jSs3L6oxB9VQ2RmCF4WrXRIBxfnlTDWfaZXzPkBKY+c0HBHwzBxqog CU/ExP1MCYVI0P6GQVO6my9UGxO/DrtGUr7po/20Nzx8wB+/BEh1MTAjV g==; X-IronPort-AV: E=McAfee;i="6500,9779,10510"; a="334237637" X-IronPort-AV: E=Sophos;i="5.95,211,1661842800"; d="scan'208";a="334237637" Received: from fmsmga003.fm.intel.com ([10.253.24.29]) by fmsmga101.fm.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 25 Oct 2022 03:38:46 -0700 X-ExtLoop1: 1 X-IronPort-AV: E=McAfee;i="6500,9779,10510"; a="720809379" X-IronPort-AV: E=Sophos;i="5.95,211,1661842800"; d="scan'208";a="720809379" Received: from orsmsx603.amr.corp.intel.com ([10.22.229.16]) by FMSMGA003.fm.intel.com with ESMTP; 25 Oct 2022 03:38:46 -0700 Received: from orsmsx601.amr.corp.intel.com (10.22.229.14) by ORSMSX603.amr.corp.intel.com (10.22.229.16) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256) id 15.1.2375.31; Tue, 25 Oct 2022 03:38:45 -0700 Received: from orsedg603.ED.cps.intel.com (10.7.248.4) 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.2375.31 via Frontend Transport; Tue, 25 Oct 2022 03:38:45 -0700 Received: from NAM12-DM6-obe.outbound.protection.outlook.com (104.47.59.174) by edgegateway.intel.com (134.134.137.100) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.1.2375.31; Tue, 25 Oct 2022 03:38:45 -0700 ARC-Seal: i=1; a=rsa-sha256; s=arcselector9901; d=microsoft.com; cv=none; b=LcqBV2zgyRXrOjB134yDkPUmpLlNzHwivW3Fu0MfjVcFOic0G5rSqOeEs5cCB9aXthV9S1No+qtwtcsJyFK1GUwxkyicqrBVT6iRtWkAH9sk7xCy0fRxKSyFsL3J6fzmEuq1SC13mugn3Wn3eovsFz/htngHzrDGQ0hGV5ibVG7klasYLgh2liodin96RaP2wtLszdAsNeIdlzrnIhtaHKcjv5zOT8+bdP4ThGcyJ/j3QwWQ83dgv7Kph4yvnR9fq5uvTvGgMZ8TrjUHWQxH4Zt9ay3pZYnqjcDySjNG1q1R55YOGYiAlirqThFzC7Z9YkJIAmReOpi4WkRqikVVGQ== 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=pP8Sh4+aCS6b/98aq/KxFjUxjfSBvnFNzQbVMCXCrVw=; b=Bs2TQfDFBFEOz4xd/oSMubWowtvFri+bygMMnXF7+QTlIAgi7UtPo0pg0HzXpuEyyVzHyNI2V44ClLDtPTH07QZejuachOcdm5mUAe9ehpauMc03Q5o5elJT9AJoNapFfryH53/Bz3tikaLMyCxC3s/TgQOmBg5QLjSV0lVazE5lFcEXIhfEkgoNtqkiQQAZdPrT7yVCrhYZPuhjPlGo+ujs2UDWRAEtSPJfAb2ejd6+gPZIIgA1xMA/E3ajySydPIrCJsHa6v4bkMa7D/PNNL9gh5S4DuPBboguTmRkxgf5yo1U2CYdJk0WGjEQ3vq7qklBWhmBCszCJo2Fqm9uBw== 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 MWHPR11MB1629.namprd11.prod.outlook.com (2603:10b6:301:d::21) by SN7PR11MB7466.namprd11.prod.outlook.com (2603:10b6:806:34c::6) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.20.5746.25; Tue, 25 Oct 2022 10:38:43 +0000 Received: from MWHPR11MB1629.namprd11.prod.outlook.com ([fe80::b849:4a51:a25f:ddd6]) by MWHPR11MB1629.namprd11.prod.outlook.com ([fe80::b849:4a51:a25f:ddd6%10]) with mapi id 15.20.5746.028; Tue, 25 Oct 2022 10:38:42 +0000 Date: Tue, 25 Oct 2022 11:38:36 +0100 From: Bruce Richardson To: David Marchand CC: Mattias =?iso-8859-1?Q?R=F6nnblom?= , , Morten =?iso-8859-1?Q?Br=F8rup?= , Ciara Power Subject: Re: [PATCH v3 1/4] telemetry: support boolean type Message-ID: References: <20221013074928.3062458-1-david.marchand@redhat.com> <20221025090052.429232-1-david.marchand@redhat.com> <20221025090052.429232-2-david.marchand@redhat.com> <6de84928-8f39-7e82-44ec-da40eb992380@lysator.liu.se> Content-Type: text/plain; charset="iso-8859-1" Content-Disposition: inline Content-Transfer-Encoding: 8bit In-Reply-To: X-ClientProxiedBy: LO4P123CA0285.GBRP123.PROD.OUTLOOK.COM (2603:10a6:600:195::20) To MWHPR11MB1629.namprd11.prod.outlook.com (2603:10b6:301:d::21) MIME-Version: 1.0 X-MS-PublicTrafficType: Email X-MS-TrafficTypeDiagnostic: MWHPR11MB1629:EE_|SN7PR11MB7466:EE_ X-MS-Office365-Filtering-Correlation-Id: 387f3c45-5fb2-4408-9223-08dab6751619 X-MS-Exchange-SenderADCheck: 1 X-MS-Exchange-AntiSpam-Relay: 0 X-Microsoft-Antispam: BCL:0; X-Microsoft-Antispam-Message-Info: sE5UDmgocEIyKhrnOlUVXrkimD+mRVhxYzDnxMPb69W28VDc+p8IqeUYuGMscMTrt1vybwnozg7pRT5GDzPJ5MDOEXTHFDKVdxiQq2Ga7d6B/SMa21GOwDleSY02RaUA0xz4VZ3MARa5IgpAktJ05nRKAG+tlqIJ5QRfK3LzqNRnEITHX9N8taUn1YVkrolyyPBW8U9EQ6RxfBXd1DVeDBuaWhNDlO/amGNaLjLqHclIun4Ft9mV/oyKwTZuHwrPu0IsDJ1ZQRzoRLgAD3FXi6G4CNriXykcpz8/MTtpo6cXmUQgsry9/dnwQJ2knU1mh+tYpZB5/ZXl43I55QGAsTWlq0nhoQiaYTelKrc7lLpmmEQIYky3JUOCXOGaB4yB8IFxozmp+tu5KabmnaS4cWdoz2B1eXan2nJFd2EGLys+wCTyq5HDVmUHbfyQfCyNWirDa7PVtksYcpDFe4Si835Lu6MQybAGnrqK1Jx5I8XVwXfGeSXlEyMscalvXefY7525wMPJKfb01L4FltO/GYmMX/8h7PTfFUt6eP7hA4C1/Y1vlDSp8RNeSc9nrsl5ps7wJCdtumbItoFnaqfvWs7cyx1o2qWIvN8Mm70QmhG8FVcC5RkX9VXBqE9cjW4/4imx+kfaR2luCi+XX52hqSAg0KFBTxwIitltuC9QRagqUuW6vwlXVH/frMjQ5VtYwtBKeqYFUgF6KlwCMn1+7Q== X-Forefront-Antispam-Report: CIP:255.255.255.255; CTRY:; LANG:en; SCL:1; SRV:; IPV:NLI; SFV:NSPM; H:MWHPR11MB1629.namprd11.prod.outlook.com; PTR:; CAT:NONE; SFS:(13230022)(376002)(136003)(346002)(39860400002)(396003)(366004)(451199015)(41300700001)(6916009)(2906002)(30864003)(4326008)(4001150100001)(54906003)(44832011)(66946007)(5660300002)(66476007)(86362001)(8676002)(66556008)(26005)(316002)(6512007)(53546011)(6506007)(66574015)(186003)(82960400001)(83380400001)(38100700002)(107886003)(6486002)(478600001)(6666004)(8936002); DIR:OUT; SFP:1102; X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1 X-MS-Exchange-AntiSpam-MessageData-0: =?iso-8859-1?Q?qoFWAEk6qDUQ/HQgGhd0GM1rtm8d81f3ZL2DRImxsXKMcH6MnAO65etL2D?= =?iso-8859-1?Q?KsuZ8h7qY4tLmdv2MREr+zrrnbH9nzh/kaIoyfqDSOGXgpxo/hffvdfLI0?= =?iso-8859-1?Q?1Tl9pKcum2CrNzEOfROJpdn3s9jIf5XWJYv/Or+6wEwVOuO/eJQLL3bUhc?= =?iso-8859-1?Q?M1CgXt/sJ27VU9X57oQ4KDW6YrmL8yZhh+K+Wx3jc4XXqN85gbglYVorU9?= =?iso-8859-1?Q?a+EyNgS/3i2/9h6c2G6XGpyalIi/z7O5lLiPjnKSEaXKHcbNp630q/qu56?= =?iso-8859-1?Q?kqzZNbqdF0GE501dVQebVtRgqV5hC7/WeElt7jDOjYwoaBOvH582HbtQlI?= =?iso-8859-1?Q?Ro+VdfU3EMv5o8grvZbgFflyFU6cIQ/K2DQuxU45pAJlFNa1A5h1EUcSTT?= =?iso-8859-1?Q?EqPjbkL3cFPSdoFqs7VTKw0Kuu4k+hjCzS5pXdlV+qXoZUOjTPdhffIXBH?= =?iso-8859-1?Q?VxylQXvMGKf4vTgrIINFrU7v0WJ1wqzSgf1JrKWtlY2LAswbc1Tx0onyrv?= =?iso-8859-1?Q?BIWnLIfxWAIXqE1IaLZZrOoJPo1+RkwPuJIHeDMYo/0Gw0CIDIF/5cgaTd?= =?iso-8859-1?Q?EMTyU/YRAqAiv/wbqj3laekuqTRbL1BkyqoZjOnsJoE2lhovoKJzjm4bXq?= =?iso-8859-1?Q?7sJs3ChM/E3lVVf51f5uVlpfOiSqF4THoCoB/nZcC5tWlMvUULdY2FaxFL?= =?iso-8859-1?Q?ia8/M0BUV/QWBoUucZOfJu2+1SvgyTfOPAJgNPMEQ+b57QQrXeh0aAAbTb?= =?iso-8859-1?Q?0jkNCm/P/MRDXEziJZVz1BQuqTZc4nnsrjc7oraHYUrgKQU2dIt5TGrlOo?= =?iso-8859-1?Q?QWjxRX5Hf7AnATNklEGJcqhXQtyuAlxTHDG1a6J8PVIjuoIPo0mHVduoOn?= =?iso-8859-1?Q?7DdlPIIGYAwSGJv69eS1REHPMSC8ywacuTpW+Tbr6L33fXishzR7t0k2Vn?= =?iso-8859-1?Q?Zsy7Mg37GWM6I5Ka/zrw1jYGCNn9sbfZZEKKh7X9oSqMEfpg3xsTcbv6CU?= =?iso-8859-1?Q?FT3KtWDyI77E3bmu5oSdvYqkRRqvL9y1wWB6liMY3KVyh02sIdIpbPH118?= =?iso-8859-1?Q?kYKxnYLJaoVePMT1fu6VIy1JvWd2Etq824SbEr2KV5u31yrJe/V6n8fZ3t?= =?iso-8859-1?Q?STG22pQp+MIWIrKUB+pwTjXRTs7FXlXIjR4q8qgRYFnCwz6xkfUB36XS9V?= =?iso-8859-1?Q?gmZF5bU+MePvV42KOCeWUzExko11YBIz2hIRMbXNVf/J5ugKV4Np/X00hA?= =?iso-8859-1?Q?PaFhm8dX8MvKPjf54Ek6ezxoEFbttFxBLzpPZna2oLsdKkpjdezx+1SHc6?= =?iso-8859-1?Q?pSIa1tT4W4HNdgYUjTAoucy9u6sZzcPVyWIKUEOMk3wHGikrwnMw9sPywb?= =?iso-8859-1?Q?2Tf6ZiapeWuGXp4IJoeeniP3gAMoy83kR5W2ht6rJLa1en1+e4ZSyZXLRR?= =?iso-8859-1?Q?7WH4OwGHOOQvN3JMIJJsw5y+CjKnDJra80Q4xp/vVrZIn9LQBG9y77yMWt?= =?iso-8859-1?Q?M667e3tf3aOQA9L9Kig4kN4ddnKqsEha43JycehXvrluhKdzttKQe+BE32?= =?iso-8859-1?Q?oGV/FiYwb+95R6EsyicvYTX2tPasITS1zdzXwdUjCtXzR+mQ23OjfF2g58?= =?iso-8859-1?Q?b3UDVUtQ92CxUiQUrOrpUU+Xx8g7/d/9vIU9UdLd3ikxm1dr6VGZcjMw?= =?iso-8859-1?Q?=3D=3D?= X-MS-Exchange-CrossTenant-Network-Message-Id: 387f3c45-5fb2-4408-9223-08dab6751619 X-MS-Exchange-CrossTenant-AuthSource: MWHPR11MB1629.namprd11.prod.outlook.com X-MS-Exchange-CrossTenant-AuthAs: Internal X-MS-Exchange-CrossTenant-OriginalArrivalTime: 25 Oct 2022 10:38:42.7362 (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: 4irH/+Mjn3xgXQrQpZTmhjMb51cwkRuAyqBFlvkZ9IvJ/8Q3oky/gZOzCFf1IHb4ZKnbgfviFOnlmYuS7pjSugeznqMUTJzWatQBFiwXuTA= X-MS-Exchange-Transport-CrossTenantHeadersStamped: SN7PR11MB7466 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 Tue, Oct 25, 2022 at 11:43:27AM +0200, David Marchand wrote: > On Tue, Oct 25, 2022 at 11:34 AM Mattias Rönnblom wrote: > > > > On 2022-10-25 11:00, David Marchand wrote: > > > Add the boolean type RTE_TEL_BOOL_VAL for values in arrays and dicts. > > > > > > Signed-off-by: David Marchand > > > Acked-by: Morten Brørup > > > Acked-by: Bruce Richardson > > > Acked-by: Ciara Power > > > --- > > > Changes since v1: > > > - fixed doxygen description, > > > > > > --- > > > app/test/test_telemetry_data.c | 88 +++++++++++++++++++++++++++++++++- > > > lib/telemetry/rte_telemetry.h | 36 ++++++++++++++ > > > lib/telemetry/telemetry.c | 24 +++++++++- > > > lib/telemetry/telemetry_data.c | 44 +++++++++++++++-- > > > lib/telemetry/telemetry_data.h | 5 ++ > > > lib/telemetry/telemetry_json.h | 34 +++++++++++++ > > > lib/telemetry/version.map | 10 ++++ > > > 7 files changed, 233 insertions(+), 8 deletions(-) > > > > > > diff --git a/app/test/test_telemetry_data.c b/app/test/test_telemetry_data.c > > > index d92667a527..134e018fde 100644 > > > --- a/app/test/test_telemetry_data.c > > > +++ b/app/test/test_telemetry_data.c > > > @@ -353,6 +353,84 @@ test_array_with_array_u64_values(void) > > > return CHECK_OUTPUT("[[0,1,2,3,4],[0,1,2,3,4]]"); > > > } > > > > > > +static int > > > +test_case_array_bool(void) > > > +{ > > > + int i; > > > + > > > + rte_tel_data_start_array(&response_data, RTE_TEL_BOOL_VAL); > > > + for (i = 0; i < 5; i++) > > > + rte_tel_data_add_array_bool(&response_data, (i % 2) == 0); > > > + return CHECK_OUTPUT("[true,false,true,false,true]"); > > > +} > > > + > > > +static int > > > +test_case_add_dict_bool(void) > > > +{ > > > + int i = 0; > > > + char name_of_value[8]; > > > + > > > + rte_tel_data_start_dict(&response_data); > > > + > > > + for (i = 0; i < 5; i++) { > > > + sprintf(name_of_value, "dict_%d", i); > > > + rte_tel_data_add_dict_bool(&response_data, name_of_value, > > > + (i % 2) == 0); > > > + } > > > + return CHECK_OUTPUT("{\"dict_0\":true,\"dict_1\":false,\"dict_2\":true," > > > + "\"dict_3\":false,\"dict_4\":true}"); > > > +} > > > + > > > +static int > > > +test_dict_with_array_bool_values(void) > > > +{ > > > + int i; > > > + > > > + struct rte_tel_data *child_data = rte_tel_data_alloc(); > > > + rte_tel_data_start_array(child_data, RTE_TEL_BOOL_VAL); > > > + > > > + struct rte_tel_data *child_data2 = rte_tel_data_alloc(); > > > + rte_tel_data_start_array(child_data2, RTE_TEL_BOOL_VAL); > > > + > > > + rte_tel_data_start_dict(&response_data); > > > + > > > + for (i = 0; i < 10; i++) { > > > + rte_tel_data_add_array_bool(child_data, (i % 2) == 0); > > > + rte_tel_data_add_array_bool(child_data2, (i % 2) == 1); > > > + } > > > + > > > + rte_tel_data_add_dict_container(&response_data, "dict_0", > > > + child_data, 0); > > > + rte_tel_data_add_dict_container(&response_data, "dict_1", > > > + child_data2, 0); > > > + > > > + return CHECK_OUTPUT("{\"dict_0\":[true,false,true,false,true,false,true,false,true,false]," > > > + "\"dict_1\":[false,true,false,true,false,true,false,true,false,true]}"); > > > +} > > > + > > > +static int > > > +test_array_with_array_bool_values(void) > > > +{ > > > + int i; > > > + > > > + struct rte_tel_data *child_data = rte_tel_data_alloc(); > > > + rte_tel_data_start_array(child_data, RTE_TEL_BOOL_VAL); > > > + > > > + struct rte_tel_data *child_data2 = rte_tel_data_alloc(); > > > + rte_tel_data_start_array(child_data2, RTE_TEL_BOOL_VAL); > > > + > > > + rte_tel_data_start_array(&response_data, RTE_TEL_CONTAINER); > > > + > > > + for (i = 0; i < 5; i++) { > > > + rte_tel_data_add_array_bool(child_data, (i % 2) == 0); > > > + rte_tel_data_add_array_bool(child_data2, (i % 2) == 1); > > > + } > > > + rte_tel_data_add_array_container(&response_data, child_data, 0); > > > + rte_tel_data_add_array_container(&response_data, child_data2, 0); > > > + > > > + return CHECK_OUTPUT("[[true,false,true,false,true],[false,true,false,true,false]]"); > > > +} > > > + > > > static int > > > test_string_char_escaping(void) > > > { > > > @@ -428,15 +506,21 @@ telemetry_data_autotest(void) > > > test_null_return, > > > test_simple_string, > > > test_case_array_string, > > > - test_case_array_int, test_case_array_u64, > > > - test_case_add_dict_int, test_case_add_dict_u64, > > > + test_case_array_int, > > > + test_case_array_u64, > > > + test_case_array_bool, > > > + test_case_add_dict_int, > > > + test_case_add_dict_u64, > > > + test_case_add_dict_bool, > > > test_case_add_dict_string, > > > test_dict_with_array_int_values, > > > test_dict_with_array_u64_values, > > > + test_dict_with_array_bool_values, > > > test_dict_with_array_string_values, > > > test_dict_with_dict_values, > > > test_array_with_array_int_values, > > > test_array_with_array_u64_values, > > > + test_array_with_array_bool_values, > > > test_array_with_array_string_values, > > > test_string_char_escaping, > > > test_array_char_escaping, > > > diff --git a/lib/telemetry/rte_telemetry.h b/lib/telemetry/rte_telemetry.h > > > index a0d21d6b7f..e7f6c2ae43 100644 > > > --- a/lib/telemetry/rte_telemetry.h > > > +++ b/lib/telemetry/rte_telemetry.h > > > @@ -2,6 +2,7 @@ > > > * Copyright(c) 2018 Intel Corporation > > > */ > > > > > > +#include > > > #include > > > > > > #include > > > @@ -46,6 +47,7 @@ enum rte_tel_value_type { > > > RTE_TEL_INT_VAL, /** a signed 32-bit int value */ > > > RTE_TEL_U64_VAL, /** an unsigned 64-bit int value */ > > > RTE_TEL_CONTAINER, /** a container struct */ > > > + RTE_TEL_BOOL_VAL, /** a boolean value */ > > > }; > > > > > > /** > > > @@ -155,6 +157,22 @@ int > > > rte_tel_data_add_array_container(struct rte_tel_data *d, > > > struct rte_tel_data *val, int keep); > > > > > > +/** > > > + * Add a boolean to an array. > > > + * The array must have been started by rte_tel_data_start_array() with > > > + * RTE_TEL_BOOL_VAL as the type parameter. > > > + * > > > + * @param d > > > + * The data structure passed to the callback > > > + * @param x > > > + * The boolean value to be returned in the array > > > + * @return > > > + * 0 on success, negative errno on error > > > + */ > > > +__rte_experimental > > > +int > > > +rte_tel_data_add_array_bool(struct rte_tel_data *d, bool x); > > > + > > > /** > > > * Add a string value to a dictionary. > > > * The dict must have been started by rte_tel_data_start_dict(). > > > @@ -233,6 +251,24 @@ int > > > rte_tel_data_add_dict_container(struct rte_tel_data *d, const char *name, > > > struct rte_tel_data *val, int keep); > > > > > > +/** > > > + * Add a boolean value to a dictionary. > > > + * The dict must have been started by rte_tel_data_start_dict(). > > > > ...as RTE_TEL_BOOL_VAL as the type parameter? > > > > > + * > > > + * @param d > > > + * The data structure passed to the callback > > > + * @param name > > > + * The name the value is to be stored under in the dict > > > + * Must contain only alphanumeric characters or the symbols: '_' or '/' > > > + * @param val > > > + * The boolean value to be stored in the dict > > > + * @return > > > + * 0 on success, negative errno on error, E2BIG on string truncation of name. > > > + */ > > > +__rte_experimental > > > +int > > > +rte_tel_data_add_dict_bool(struct rte_tel_data *d, const char *name, bool val); > > > + > > > /** > > > * This telemetry callback is used when registering a telemetry command. > > > * It handles getting and formatting information to be returned to telemetry > > > diff --git a/lib/telemetry/telemetry.c b/lib/telemetry/telemetry.c > > > index 8fbb4f3060..276d0f337d 100644 > > > --- a/lib/telemetry/telemetry.c > > > +++ b/lib/telemetry/telemetry.c > > > @@ -168,7 +168,9 @@ container_to_json(const struct rte_tel_data *d, char *out_buf, size_t buf_len) > > > unsigned int i; > > > > > > if (d->type != RTE_TEL_DICT && d->type != RTE_TEL_ARRAY_U64 && > > > - d->type != RTE_TEL_ARRAY_INT && d->type != RTE_TEL_ARRAY_STRING) > > > + d->type != RTE_TEL_ARRAY_INT && > > > + d->type != RTE_TEL_ARRAY_STRING && > > > + d->type != RTE_TEL_ARRAY_BOOL) > > > > Use a switch for improved readability. > > > > > return snprintf(out_buf, buf_len, "null"); > > > > > > used = rte_tel_json_empty_array(out_buf, buf_len, 0); > > > @@ -187,6 +189,11 @@ container_to_json(const struct rte_tel_data *d, char *out_buf, size_t buf_len) > > > used = rte_tel_json_add_array_string(out_buf, > > > buf_len, used, > > > d->data.array[i].sval); > > > + if (d->type == RTE_TEL_ARRAY_BOOL) > > > > This whole section should be refactor to use a switch statement, imo. > > > > > + for (i = 0; i < d->data_len; i++) > > > + used = rte_tel_json_add_array_bool(out_buf, > > > + buf_len, used, > > > + d->data.array[i].boolval); > > > if (d->type == RTE_TEL_DICT) > > > for (i = 0; i < d->data_len; i++) { > > > const struct tel_dict_entry *v = &d->data.dict[i]; > > > @@ -206,6 +213,11 @@ container_to_json(const struct rte_tel_data *d, char *out_buf, size_t buf_len) > > > buf_len, used, > > > v->name, v->value.u64val); > > > break; > > > + case RTE_TEL_BOOL_VAL: > > > + used = rte_tel_json_add_obj_bool(out_buf, > > > + buf_len, used, > > > + v->name, v->value.boolval); > > > + break; > > > case RTE_TEL_CONTAINER: > > > { > > > char temp[buf_len]; > > > @@ -273,6 +285,11 @@ output_json(const char *cmd, const struct rte_tel_data *d, int s) > > > buf_len, used, > > > v->name, v->value.u64val); > > > break; > > > + case RTE_TEL_BOOL_VAL: > > > + used = rte_tel_json_add_obj_bool(cb_data_buf, > > > + buf_len, used, > > > + v->name, v->value.boolval); > > > + break; > > > case RTE_TEL_CONTAINER: > > > { > > > char temp[buf_len]; > > > @@ -294,6 +311,7 @@ output_json(const char *cmd, const struct rte_tel_data *d, int s) > > > case RTE_TEL_ARRAY_STRING: > > > case RTE_TEL_ARRAY_INT: > > > case RTE_TEL_ARRAY_U64: > > > + case RTE_TEL_ARRAY_BOOL: > > > case RTE_TEL_ARRAY_CONTAINER: > > > used = rte_tel_json_empty_array(cb_data_buf, buf_len, 0); > > > for (i = 0; i < d->data_len; i++) > > > @@ -310,6 +328,10 @@ output_json(const char *cmd, const struct rte_tel_data *d, int s) > > > used = rte_tel_json_add_array_u64(cb_data_buf, > > > buf_len, used, > > > d->data.array[i].u64val); > > > + else if (d->type == RTE_TEL_ARRAY_BOOL) > > > + used = rte_tel_json_add_array_bool(cb_data_buf, > > > + buf_len, used, > > > + d->data.array[i].boolval); > > > else if (d->type == RTE_TEL_ARRAY_CONTAINER) { > > > char temp[buf_len]; > > > const struct container *rec_data = > > > diff --git a/lib/telemetry/telemetry_data.c b/lib/telemetry/telemetry_data.c > > > index 34366ecee3..13a7ce7034 100644 > > > --- a/lib/telemetry/telemetry_data.c > > > +++ b/lib/telemetry/telemetry_data.c > > > @@ -16,10 +16,11 @@ int > > > rte_tel_data_start_array(struct rte_tel_data *d, enum rte_tel_value_type type) > > > { > > > enum tel_container_types array_types[] = { > > > - RTE_TEL_ARRAY_STRING, /* RTE_TEL_STRING_VAL = 0 */ > > > - RTE_TEL_ARRAY_INT, /* RTE_TEL_INT_VAL = 1 */ > > > - RTE_TEL_ARRAY_U64, /* RTE_TEL_u64_VAL = 2 */ > > > - RTE_TEL_ARRAY_CONTAINER, /* RTE_TEL_CONTAINER = 3 */ > > > + [RTE_TEL_STRING_VAL] = RTE_TEL_ARRAY_STRING, > > > + [RTE_TEL_INT_VAL] = RTE_TEL_ARRAY_INT, > > > + [RTE_TEL_U64_VAL] = RTE_TEL_ARRAY_U64, > > > + [RTE_TEL_CONTAINER] = RTE_TEL_ARRAY_CONTAINER, > > > + [RTE_TEL_BOOL_VAL] = RTE_TEL_ARRAY_BOOL, > > > > Seems like a brittle construct to me. Replace this with a switch statement. > > > > > }; > > > d->type = array_types[type]; > > > d->data_len = 0; > > > @@ -80,6 +81,17 @@ rte_tel_data_add_array_u64(struct rte_tel_data *d, uint64_t x) > > > return 0; > > > } > > > > > > +int > > > +rte_tel_data_add_array_bool(struct rte_tel_data *d, bool x) > > > +{ > > > + if (d->type != RTE_TEL_ARRAY_BOOL) > > > + return -EINVAL; > > > + if (d->data_len >= RTE_TEL_MAX_ARRAY_ENTRIES) > > > + return -ENOSPC; > > > + d->data.array[d->data_len++].boolval = x; > > > + return 0; > > > +} > > > + > > > int > > > rte_tel_data_add_array_container(struct rte_tel_data *d, > > > struct rte_tel_data *val, int keep) > > > @@ -87,7 +99,8 @@ rte_tel_data_add_array_container(struct rte_tel_data *d, > > > if (d->type != RTE_TEL_ARRAY_CONTAINER || > > > (val->type != RTE_TEL_ARRAY_U64 > > > && val->type != RTE_TEL_ARRAY_INT > > > - && val->type != RTE_TEL_ARRAY_STRING)) > > > + && val->type != RTE_TEL_ARRAY_STRING > > > + && val->type != RTE_TEL_ARRAY_BOOL)) > > > > Maybe the introduction of a valid_type() static function would be in > > order? Implemented by means of a (surprise!) switch statement. :) > > > > Could be is_valid_type() and rename valid_name() us_valid_name() in the > > process. > > > > Use in rte_tel_data_add_dict_container() as well. > > > > > return -EINVAL; > > > if (d->data_len >= RTE_TEL_MAX_ARRAY_ENTRIES) > > > return -ENOSPC; > > > @@ -179,6 +192,26 @@ rte_tel_data_add_dict_u64(struct rte_tel_data *d, > > > return bytes < RTE_TEL_MAX_STRING_LEN ? 0 : E2BIG; > > > } > > > > > > +int > > > +rte_tel_data_add_dict_bool(struct rte_tel_data *d, > > > + const char *name, bool val) > > > +{ > > > + struct tel_dict_entry *e = &d->data.dict[d->data_len]; > > > + if (d->type != RTE_TEL_DICT) > > > + return -EINVAL; > > > + if (d->data_len >= RTE_TEL_MAX_DICT_ENTRIES) > > > + return -ENOSPC; > > > + > > > + if (!valid_name(name)) > > > + return -EINVAL; > > > + > > > + d->data_len++; > > > + e->type = RTE_TEL_BOOL_VAL; > > > + e->value.boolval = val; > > > + const size_t bytes = strlcpy(e->name, name, RTE_TEL_MAX_STRING_LEN); > > > + return bytes < RTE_TEL_MAX_STRING_LEN ? 0 : E2BIG; > > > +} > > > + > > > int > > > rte_tel_data_add_dict_container(struct rte_tel_data *d, const char *name, > > > struct rte_tel_data *val, int keep) > > > @@ -188,6 +221,7 @@ rte_tel_data_add_dict_container(struct rte_tel_data *d, const char *name, > > > if (d->type != RTE_TEL_DICT || (val->type != RTE_TEL_ARRAY_U64 > > > && val->type != RTE_TEL_ARRAY_INT > > > && val->type != RTE_TEL_ARRAY_STRING > > > + && val->type != RTE_TEL_ARRAY_BOOL > > > && val->type != RTE_TEL_DICT)) > > > return -EINVAL; > > > if (d->data_len >= RTE_TEL_MAX_DICT_ENTRIES) > > > diff --git a/lib/telemetry/telemetry_data.h b/lib/telemetry/telemetry_data.h > > > index 26aa28e72c..c840486b18 100644 > > > --- a/lib/telemetry/telemetry_data.h > > > +++ b/lib/telemetry/telemetry_data.h > > > @@ -5,6 +5,9 @@ > > > #ifndef _TELEMETRY_DATA_H_ > > > #define _TELEMETRY_DATA_H_ > > > > > > +#include > > > +#include > > > + > > > #include "rte_telemetry.h" > > > > > > enum tel_container_types { > > > @@ -15,6 +18,7 @@ enum tel_container_types { > > > RTE_TEL_ARRAY_INT, /** array of signed, 32-bit int values */ > > > RTE_TEL_ARRAY_U64, /** array of unsigned 64-bit int values */ > > > RTE_TEL_ARRAY_CONTAINER, /** array of container structs */ > > > + RTE_TEL_ARRAY_BOOL, /** array of boolean values */ > > > }; > > > > > > struct container { > > > @@ -30,6 +34,7 @@ union tel_value { > > > char sval[RTE_TEL_MAX_STRING_LEN]; > > > int ival; > > > uint64_t u64val; > > > + bool boolval; > > > struct container container; > > > }; > > > > > > diff --git a/lib/telemetry/telemetry_json.h b/lib/telemetry/telemetry_json.h > > > index e3fae7c30d..c97da97366 100644 > > > --- a/lib/telemetry/telemetry_json.h > > > +++ b/lib/telemetry/telemetry_json.h > > > @@ -7,6 +7,7 @@ > > > > > > #include > > > #include > > > +#include > > > #include > > > #include > > > #include > > > @@ -159,6 +160,21 @@ rte_tel_json_add_array_u64(char *buf, const int len, const int used, > > > return ret == 0 ? used : end + ret; > > > } > > > > > > +/* Appends a boolean into the JSON array in the provided buffer. */ > > > +static inline int > > > +rte_tel_json_add_array_bool(char *buf, const int len, const int used, > > > + bool val) > > > +{ > > > + int ret, end = used - 1; /* strip off final delimiter */ > > > + if (used <= 2) /* assume empty, since minimum is '[]' */ > > > + return __json_snprintf(buf, len, "[%s]", > > > + val ? "true" : "false"); > > > > Have "true" and "false" as #defines, and make a bool_str() helper, which > > returns the (constant) string form. > > > > > + > > > + ret = __json_snprintf(buf + end, len - end, ",%s]", > > > + val ? "true" : "false"); > > > + return ret == 0 ? used : end + ret; > > > +} > > > + > > > /* > > > * Add a new element with raw JSON value to the JSON array stored in the > > > * provided buffer. > > > @@ -193,6 +209,24 @@ rte_tel_json_add_obj_u64(char *buf, const int len, const int used, > > > return ret == 0 ? used : end + ret; > > > } > > > > > > +/** > > > + * Add a new element with boolean value to the JSON object stored in the > > > + * provided buffer. > > > + */ > > > +static inline int > > > > I know this pattern isn't introduced in this patch, but why is this > > function in the header file? > > > > > +rte_tel_json_add_obj_bool(char *buf, const int len, const int used, > > > + const char *name, bool val) > > > +{ > > > + int ret, end = used - 1; > > > + if (used <= 2) /* assume empty, since minimum is '{}' */ > > > > RTE_ASSERT() wouldn't be more helpful? The input buffer is malformed, > > correct? > > > > Either way, the magic '2' should be a #define. > > > > > + return __json_snprintf(buf, len, "{\"%s\":%s}", name, > > > + val ? "true" : "false"); > > > + > > > + ret = __json_snprintf(buf + end, len - end, ",\"%s\":%s}", > > > + name, val ? "true" : "false"); > > > + return ret == 0 ? used : end + ret; > > > +} > > > + > > > > This looks like cut-and-paste. Merge into one function, parameterized by > > the start and end delimiter. > > > > > /** > > > * Add a new element with int value to the JSON object stored in the > > > * provided buffer. > > > diff --git a/lib/telemetry/version.map b/lib/telemetry/version.map > > > index 9794f9ea20..88f58d4d89 100644 > > > --- a/lib/telemetry/version.map > > > +++ b/lib/telemetry/version.map > > > @@ -19,7 +19,17 @@ DPDK_23 { > > > local: *; > > > }; > > > > > > +EXPERIMENTAL { > > > + global: > > > + > > > + # added in 22.11 > > > + rte_tel_data_add_array_bool; > > > + rte_tel_data_add_dict_bool; > > > +}; > > > + > > > INTERNAL { > > > + global: > > > + > > > rte_telemetry_legacy_register; > > > rte_telemetry_init; > > > }; > > > > I agree with most of the comments. > I can follow up on those cleanups, but I don't have the time now. > > I'll let the telemetry maintainers decide on the fate of those patches. > Thanks. > Well, given the lateness in the release process, if it's a choice between getting the patches in as-is, vs not getting them in at all, I think having them in 22.11 is the better choice. While there is cleanup that can be done, I don't see this patch as making the overall code any worse. My 2c. /Bruce