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 87F73A0586; Wed, 19 Oct 2022 15:48:05 +0200 (CEST) Received: from [217.70.189.124] (localhost [127.0.0.1]) by mails.dpdk.org (Postfix) with ESMTP id 79F9842BC7; Wed, 19 Oct 2022 15:48:05 +0200 (CEST) Received: from mga18.intel.com (mga18.intel.com [134.134.136.126]) by mails.dpdk.org (Postfix) with ESMTP id B03CC410D1 for ; Wed, 19 Oct 2022 15:48:03 +0200 (CEST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=intel.com; i=@intel.com; q=dns/txt; s=Intel; t=1666187283; x=1697723283; h=date:from:to:cc:subject:message-id:references: in-reply-to:mime-version; bh=BJ+HTpWMI1ZumwKXCjavTuLOE4JyLaPKrcwXTOKH/co=; b=kZfVmlL8SKJHWNNzTH34m8H+lxTlevYhwn/XLte3+Bv2ciGHTBOy7Y/U Rzf66s81K3G0tXntSMKL4kbnYV6SftF/ulb0hO4sLBmTTdtI/SvYQqvJZ LjDyzIeO0OxQB+NRIRUcOmsLUg2wOPNBn/z/eDqyPFOl6d45lBUuJK0q7 mioQcVLLguy4it+WJlpEM1hYEt7id4DFldJ9/JymjKBBgcCXx8kZICNsx n5WPHZ8UJhfR5WEckm6P1O60dzh9hrn0UGbuL0wiMVvnhg+0l76uSAdso 3AZ7KcMrSbASdBgOilhvmbrDRMsi+p1UPTO0lx0qceGNLL+3rowCep/+8 A==; X-IronPort-AV: E=McAfee;i="6500,9779,10505"; a="289729765" X-IronPort-AV: E=Sophos;i="5.95,196,1661842800"; d="scan'208";a="289729765" Received: from orsmga005.jf.intel.com ([10.7.209.41]) by orsmga106.jf.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 19 Oct 2022 06:48:02 -0700 X-ExtLoop1: 1 X-IronPort-AV: E=McAfee;i="6500,9779,10505"; a="804307702" X-IronPort-AV: E=Sophos;i="5.95,196,1661842800"; d="scan'208";a="804307702" Received: from orsmsx602.amr.corp.intel.com ([10.22.229.15]) by orsmga005.jf.intel.com with ESMTP; 19 Oct 2022 06:48:02 -0700 Received: from orsmsx612.amr.corp.intel.com (10.22.229.25) by ORSMSX602.amr.corp.intel.com (10.22.229.15) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256) id 15.1.2375.31; Wed, 19 Oct 2022 06:48:02 -0700 Received: from orsmsx610.amr.corp.intel.com (10.22.229.23) by ORSMSX612.amr.corp.intel.com (10.22.229.25) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256) id 15.1.2375.31; Wed, 19 Oct 2022 06:48:01 -0700 Received: from orsedg603.ED.cps.intel.com (10.7.248.4) 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.2375.31 via Frontend Transport; Wed, 19 Oct 2022 06:48:01 -0700 Received: from NAM12-MW2-obe.outbound.protection.outlook.com (104.47.66.47) 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; Wed, 19 Oct 2022 06:47:59 -0700 ARC-Seal: i=1; a=rsa-sha256; s=arcselector9901; d=microsoft.com; cv=none; b=mfyl283yeMX7+jLmt5bxm6kdzkiHWtccp4Ys6sazZI2gFsxzDomJw4eCtPaTSa1t4ekRKrN4SkDBDvSS+Mot56INcnwVytfBYbL5O7yg9et1NN+rAq0+ttBYhusO5vgYkHli6Hp87gxxoBFn7VDRam4OB31o9k5VR7DwWVmCxlJ3DLndiHFo084akv/45foVkibORvUEF/L9vUDKoC3vRf0E9vOg9kzWneA6ZmG02CAAXbmVsh3o5ILDsEnQAQWZKuTva6NSKpRmAuayRtQV6Uun5zqeIvObVFfTY8ou+Sz1QzU8DoPp5XhclY+/UspagWmXZu5XvXgqWwtpvADKeg== 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=AT/i+kux/greJtcyg1eI72GQgXBdUVI+c38IKc9mI9o=; b=Px20EFN0FRDe14+jcSRJuWZNhfP146QUQzj11V1WnJbVix9HA3/iTX8fcaTZYjveFXijAkW2EEMGMwRKDih7ZQg3RgARRpGtPU7Ljp2fOdwD8WIF7AhYAdNIBs4r6XuHilK3XzLMGLdWShClqKjvzq7kIeOsBcshQFRrPx5WvMigB+E0XvA17LgiFFKV4fjZ2pcJU2OWdnFTDZVBg2DqltZ4LB3adt+fcyI8VKIRwambPnqBGzBcpiq4VWb5Yom2LLpRRFtCpobmF4EP05znawZC530HBIJLexnbGM53qEq8Rshb8mkHLQ4Z23QvdRyqEdomkv8rYkgqYDnahIyMVg== 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 DM4PR11MB5406.namprd11.prod.outlook.com (2603:10b6:5:395::16) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.20.5723.34; Wed, 19 Oct 2022 13:47:57 +0000 Received: from MWHPR11MB1629.namprd11.prod.outlook.com ([fe80::5582:9796:3aaa:aa1]) by MWHPR11MB1629.namprd11.prod.outlook.com ([fe80::5582:9796:3aaa:aa1%12]) with mapi id 15.20.5723.034; Wed, 19 Oct 2022 13:47:57 +0000 Date: Wed, 19 Oct 2022 14:47:50 +0100 From: Bruce Richardson To: David Marchand CC: , , Ciara Power Subject: Re: [PATCH] telemetry: support boolean type Message-ID: References: <20221019073702.3948624-1-david.marchand@redhat.com> Content-Type: text/plain; charset="us-ascii" Content-Disposition: inline In-Reply-To: <20221019073702.3948624-1-david.marchand@redhat.com> X-ClientProxiedBy: LO4P123CA0661.GBRP123.PROD.OUTLOOK.COM (2603:10a6:600:316::15) To MWHPR11MB1629.namprd11.prod.outlook.com (2603:10b6:301:d::21) MIME-Version: 1.0 X-MS-PublicTrafficType: Email X-MS-TrafficTypeDiagnostic: MWHPR11MB1629:EE_|DM4PR11MB5406:EE_ X-MS-Office365-Filtering-Correlation-Id: f105fbfb-4cad-40cf-afdb-08dab1d88744 X-MS-Exchange-SenderADCheck: 1 X-MS-Exchange-AntiSpam-Relay: 0 X-Microsoft-Antispam: BCL:0; X-Microsoft-Antispam-Message-Info: SClAIP27f07hf6ZqHJYB9qlet/ZrF+lYGIse2wHuusgtcK6Q3UrWMOVtTdkyZBnYe9kJXXjnbhrN0P5hdUdM+ls5g+HbqjeZH7ZhGneGPQmY4iO5exeR2/2KG8AOVIjhWBhhgR7kwj+zN5OkAoPirM0IeaRJ8G1uJ3sbntMWeHl64+SBYUmGt21OcjPmn+p8cjTApltMaaRCUYlXOYtmJEZD20q9EqGONUPowXBwotQ6Knt8FGiOFxxDWkNZ3o1UQnwPoV4bGJ0Ay4GHFp/5fdaiPSCmuYG9gvDarxr+fnWZQA7jfI7lQke30A/iRmzylEI3y0khL4KaaLZ8izZxH8q3WiLTyOu9i2H8EUhFQTRN4aDlhNDMauCD8F7iaGVBSf0819lJ3pWYlJgjZEEfXAcPyv7GUYE/NzOtw0OgXBPupzgIep2pCqKWZrDjlDOsc/S4q7dLJCchI9uK28PLXNw4d1iaNT64BdTXvoDRc7rJwcOazSHOD6op3Y4HaKmaTe57cUlseryNC+Z01lXHvTUNVWpfRp/YWqUujn0FOE4TLN1vGTJzs2IJpM2ccsoc4tNVEfrsv91xrf0xL0YTYqPr+NbJvmFBX0E5FD0nJswUor/Mp0rv+5879DxBlzd8bmaSZejgLHbkd324CG8wSnLlsdYZTKzscYLk4hKKP9dwQNuTv1A2j73YPowagjk2iii/v0FvGTs/Eb0jkbSBzw== 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)(396003)(376002)(39860400002)(136003)(366004)(346002)(451199015)(6916009)(8936002)(6512007)(26005)(6506007)(66946007)(6666004)(8676002)(66556008)(66476007)(107886003)(41300700001)(4326008)(82960400001)(2906002)(186003)(316002)(5660300002)(44832011)(38100700002)(83380400001)(478600001)(6486002)(86362001); DIR:OUT; SFP:1102; X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1 X-MS-Exchange-AntiSpam-MessageData-0: =?us-ascii?Q?p5ZSO4zHepQPv5iA080YxvEyZrUYM9qyQXnIrXo9bCEyYda+EIIh+piILXqj?= =?us-ascii?Q?W5Ps8QhsnkQXozCRjYhd4JCjUXfOrm9NHOzZhXnrUxyN9nYUyL9XFvldW/pG?= =?us-ascii?Q?cOWW3sjK5c8pBD9r7RRda1T92ZgHpcq/Ie4NjVJQsDOzug3/AAAsJOG4K1qA?= =?us-ascii?Q?02flFhbmZyqqxNtSPz2C1sRg8Tlhn1cZwHDoR1Zh4EO2KVArQMQnFTG3FlFD?= =?us-ascii?Q?utz0rfugEd8b2x7Td21K+ZpX5feekMkRUOHcesyeKRTy881jsUv4XboX1syv?= =?us-ascii?Q?GgyIqGdQzcube4HaY4N57RgtVY2gi3cy4WlEd2xGDoQIKiRbPyGuVBsoyM0A?= =?us-ascii?Q?PHk5pXZAm2GIc563Oy9jJNPikECK3X3rRmu5TPbLuO2KvJQh+r9Loe95vJu0?= =?us-ascii?Q?ofnk3MPJsq8MZRA6Cf1zEUlbcvAJ7zbCZbF6FmcBjqAQZDce9e89OwN6bBJw?= =?us-ascii?Q?nXpGrsMFaCHrZYd80r6547/UIU5NDTjsdYbh6WpHRLpYSJChozpVYGPS0wF3?= =?us-ascii?Q?gtJozDvchG4lTxMbbIq9IeIb7QqMipnhS5TiOKk8eIQFMDw0f7nvmVl0ztkX?= =?us-ascii?Q?6MTYxeLBlCcTO0dzO4FMYiB1prLxcmsrkmwRbPusIEuervA7B7YwgAwS57bw?= =?us-ascii?Q?juHEaxZma/HskB5VhhK8C6Dr4F9Wo3zGfUyyh1YpQWPt8Ca0lEOrS0dk/x3m?= =?us-ascii?Q?DwHrifUyt4U0d2af4Q7qoZGyn3u7DuyW2i4fSGuz1MMOU9DsH/B/d8yALOm/?= =?us-ascii?Q?W2XADZUXGQu1A5QEp2cIm2dhcAM3rxF+7gGxXJVhvjNIL6Hl27B+fz1XFaGf?= =?us-ascii?Q?JEedcJPZMSF1pWJeglm+jiT1t5iYd0GrM0bcBrMp7/w9p7QAUZTxL0RJ1ugW?= =?us-ascii?Q?QnWgGfhlTIfnErXDofEfOlP2leO0Ki5JEvwQeNcG4G9xjTEP78aLkx0a31/P?= =?us-ascii?Q?10INe3qlPO9bdSksdfXUGgnSRpI26Al5rG+paZTAJSjDlnqQCt57LH/g+4Ot?= =?us-ascii?Q?2eh7LjgtFY2CvBH0crf1tpLWOTcIrC/deFCXJtonxsFa20+Er718HtY9qvkG?= =?us-ascii?Q?kDWZBp1NbErtFXeE4I0U2qZtbnyynnIDbPEIjqjcNVl2OOux8RHYJb9BFWxz?= =?us-ascii?Q?NVmIJj1dkH5Qqw4IRa9spVagwJInnzdikz5c6jEslzMgT7gRmALd2P5kuOVt?= =?us-ascii?Q?+cqe7rcGkc6Iasnfpf5Xwv3v3fyrSJgV3h25C1mSenYb+Vl4jM2+Tpl+PvTW?= =?us-ascii?Q?q+xH8kmbxHA+llhsxGgrkvOfLlVQZUqqE+YthVSoDBR/T4p87xlz2Mp+HaP1?= =?us-ascii?Q?TX77mXTel7AbvbMRb6q42XxJ+il1gtGpCh49TxDPZVi1mhOyfkZ5xVg1L+pS?= =?us-ascii?Q?u7tY/DGLSt6nP3kveQkHF+Czp9Fe02/7W0kc18PjKa7USl35+kOreaeCcaBv?= =?us-ascii?Q?EVWfp9Rmc9GgNLDTiGR55A+CA6vyYUCFnrGJrPnetNoc6eUT+F5HPbPBDXRI?= =?us-ascii?Q?vixLqvhEaIZ/tIGd5CAR5+H2JCLIO2CjykfBGKRHBY1ND/3sCVpjuTybcb0u?= =?us-ascii?Q?7s2YXAeM8SclewZoTlzT2ttYwz7x20/HKXMOOolZJ+Q+kFeA2RP02kgFpHdQ?= =?us-ascii?Q?V4W2PF7IMnZJx5eOxdaKPs4=3D?= X-MS-Exchange-CrossTenant-Network-Message-Id: f105fbfb-4cad-40cf-afdb-08dab1d88744 X-MS-Exchange-CrossTenant-AuthSource: MWHPR11MB1629.namprd11.prod.outlook.com X-MS-Exchange-CrossTenant-AuthAs: Internal X-MS-Exchange-CrossTenant-OriginalArrivalTime: 19 Oct 2022 13:47:56.9636 (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: 6iMrVo3QmU/8Bf9zYfob4N5WUJ8EH+U5imkxFKWDg55gpcg1lO4n1WvLeTgyOzqkFpwRZmd4kUas6C6VnYV5LPPSRLrcrGDCtQbjnkH7+Ss= X-MS-Exchange-Transport-CrossTenantHeadersStamped: DM4PR11MB5406 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 19, 2022 at 09:37:02AM +0200, David Marchand wrote: > Add the boolean type RTE_TEL_BOOL_VAL for values in arrays and dicts. > > Signed-off-by: David Marchand > --- This patch looks pretty good to me. Some very small comments inline below. One thing I notice is that we are not supporting booleans except as part of an array or dictionary. Is it likely that we will ever want to have a telemetry command that just returns true/false alone? Don't see that being necessary just yet, so: Reviewed-by: Bruce Richardson Acked-by: Bruce Richardson > 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 | 5 ++ > 7 files changed, 228 insertions(+), 8 deletions(-) > > diff --git a/lib/telemetry/rte_telemetry.h b/lib/telemetry/rte_telemetry.h > index a0d21d6b7f..5d74212f17 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 number to be returned in the array number -> boolean value > + * @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(). > + * > + * @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 number to be stored in the dict number -> boolean value > + * @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 > diff --git a/lib/telemetry/telemetry_data.c b/lib/telemetry/telemetry_data.c > index 5b319c18fb..4f81f71e03 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, > }; Really like this change! > 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) > 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"); > + > + ret = __json_snprintf(buf + end, len - end, ",%s]", > + val ? "true" : "false"); Wonder if it's worthwhile doing a macro for this conditional, since the same ternary-operator snippet appears 4 times in this code. > + 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; > } >