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 8D2BEA034C; Mon, 12 Dec 2022 12:21:12 +0100 (CET) Received: from mails.dpdk.org (localhost [127.0.0.1]) by mails.dpdk.org (Postfix) with ESMTP id 2521F4021D; Mon, 12 Dec 2022 12:21:12 +0100 (CET) Received: from mga18.intel.com (mga18.intel.com [134.134.136.126]) by mails.dpdk.org (Postfix) with ESMTP id 6E8A640151 for ; Mon, 12 Dec 2022 12:21:10 +0100 (CET) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=intel.com; i=@intel.com; q=dns/txt; s=Intel; t=1670844070; x=1702380070; h=date:from:to:cc:subject:message-id:references: content-transfer-encoding:in-reply-to:mime-version; bh=Vjz/p7tRPR1d3LGqio4H1xvGqrl+ifO1B7TX9X2YeWk=; b=EBYcrDMbuP3/tJQbfgX6GE6UyUwqdCyCikEoMB/rnlAjH2gIMOigUgjE LAqPc1Jjr2HtV6bNEQkm0lV9fE1vAoZeBmU3tQ2Kzr4kEFcPuFDI7S2Gw Q3zShJGsIXpN2xVPXT+tpp9yep+3t9f+HLCn1+e405eBfCIGDN7KFmyZD cOc1fZJ6wHUvyq9NgBkdzSB2W93mrGnFKniJNpL19ke1INAgkOtEXoTvU nrnyaj7F5vlwaErLq+QtyHLKsS/hOAI26WFcp6DFx9dTw5mDxHDo/+7/N DyCJf8F7GVcyaSY6zZF/uFrBkfO7iu8GnRcWeZ3NTIElUrvgep0+37Ivd Q==; X-IronPort-AV: E=McAfee;i="6500,9779,10558"; a="301251293" X-IronPort-AV: E=Sophos;i="5.96,238,1665471600"; d="scan'208";a="301251293" Received: from orsmga008.jf.intel.com ([10.7.209.65]) by orsmga106.jf.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 12 Dec 2022 03:21:08 -0800 X-ExtLoop1: 1 X-IronPort-AV: E=McAfee;i="6500,9779,10558"; a="678883895" X-IronPort-AV: E=Sophos;i="5.96,238,1665471600"; d="scan'208";a="678883895" Received: from orsmsx602.amr.corp.intel.com ([10.22.229.15]) by orsmga008.jf.intel.com with ESMTP; 12 Dec 2022 03:21:08 -0800 Received: from orsmsx603.amr.corp.intel.com (10.22.229.16) 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.2507.16; Mon, 12 Dec 2022 03:21:07 -0800 Received: from ORSEDG602.ED.cps.intel.com (10.7.248.7) 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.2507.16 via Frontend Transport; Mon, 12 Dec 2022 03:21:07 -0800 Received: from NAM10-DM6-obe.outbound.protection.outlook.com (104.47.58.103) by edgegateway.intel.com (134.134.137.103) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.1.2507.16; Mon, 12 Dec 2022 03:21:07 -0800 ARC-Seal: i=1; a=rsa-sha256; s=arcselector9901; d=microsoft.com; cv=none; b=Qkf4wdC6nbe8zHCOYvJ9a0wOQteNBLMNIE+SkXvX6tOeRCs53mThZavWfOfElRP5W3QrIim/i8eMB1wmqE4FvGCrT7/tZARMwO9TrNKqb6K2qpYkyvS+oo0wC+2k4WYEVr3EKFhOtfkVnonoYWpBvFZX6Co6XSA9OBdnzStwsCWrhcWnP9PZU1duJTOUtaMPv5PMVruHSAYUSIiSRLxgdRTZUmeIqpa1xC2v2IVyKPw3qaWCdWDXBN52/0XTIf+n1uymVvPfzrE6SAFtCMRca7wsRH4Oc7KLi+JLBtmYSlH0rAKN0OJu3re7M7HOYMwmKmCyB+q1UBAHCL+SMFxITg== 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=1c1Glh6p3Ovg7OAWgjuFbFq0v6eLwO3CAJ/7Y6YMS+o=; b=TeahBF0N8PSjtzhSeHLnrO5NVv6RIJXHIUYQesBUezc2k+4IVF7XrAlVeIWtIR+i4ElOLFXKxYXzBoKFPjVtByRo1KKeQYj1pgrTTDKlVd7hidfNs3Mt2QhEik5A+03IAtJNwC1RX5aZJQFK5YNejQ9wzfOxxCrOA4DgwU6dNm1kNxSKj1oCPXOCZDgjSjva729psFDUyfsRehHg2Xr4OqoAkXpg3bGwZOfcjlzmjAbaEsm9iBeH9jCnl/sxkqAkm6yEfwB3pQuo2q0LXlkOv3xaBvvb8MCwlovr3sGgvog+Vx9XbWyZ+/fIq0gNeARGwom8dPT/9949kzSCtXWjoQ== 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 SA1PR11MB6918.namprd11.prod.outlook.com (2603:10b6:806:2bf::8) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.20.5880.14; Mon, 12 Dec 2022 11:21:06 +0000 Received: from MWHPR11MB1629.namprd11.prod.outlook.com ([fe80::18bd:edae:ad31:a228]) by MWHPR11MB1629.namprd11.prod.outlook.com ([fe80::18bd:edae:ad31:a228%3]) with mapi id 15.20.5880.019; Mon, 12 Dec 2022 11:21:05 +0000 Date: Mon, 12 Dec 2022 11:20:57 +0000 From: Bruce Richardson To: Morten =?iso-8859-1?Q?Br=F8rup?= CC: Huisong Li , , , , , Subject: Re: [PATCH V3 00/11] telemetry: add u32 value type and hex integer string API Message-ID: References: <20221208080540.62913-1-lihuisong@huawei.com> <20221212064306.39232-1-lihuisong@huawei.com> <98CBD80474FA8B44BF855DF32C47DC35D87585@smartserver.smartshare.dk> Content-Type: text/plain; charset="iso-8859-1" Content-Disposition: inline Content-Transfer-Encoding: 8bit In-Reply-To: <98CBD80474FA8B44BF855DF32C47DC35D87585@smartserver.smartshare.dk> X-ClientProxiedBy: LO4P123CA0004.GBRP123.PROD.OUTLOOK.COM (2603:10a6:600:150::9) To MWHPR11MB1629.namprd11.prod.outlook.com (2603:10b6:301:d::21) MIME-Version: 1.0 X-MS-PublicTrafficType: Email X-MS-TrafficTypeDiagnostic: MWHPR11MB1629:EE_|SA1PR11MB6918:EE_ X-MS-Office365-Filtering-Correlation-Id: 54478bbe-1006-4036-ab4c-08dadc32f4fb X-MS-Exchange-SenderADCheck: 1 X-MS-Exchange-AntiSpam-Relay: 0 X-Microsoft-Antispam: BCL:0; X-Microsoft-Antispam-Message-Info: P21rMUQLR3AgR1iPr9iODjjd9qLE97fnhcQpckisPxvDscxMK8cgPFHlzOibaRC+7zTqfnlBX4uzMY4yQBqpXbRf75WTcvbsL7d6oIqRL32b/NPS5PAKFwOigbAieGbcNX3+SElpu4XCCyC/DPRvc2Ryhp6DoR6aIG4G0Y/B2Q10uBTVM0pw6znHUDF+pERidzgyZorwqBv4lgy9C6scwLCETScuk4SnhzHL49eraMRJyv77gR+LekGB2lq+HdDzm5DyXotso0bE0DnAH8z4lSYN9Pb4hovaIbHJtU5VhQFFbHN4UHV2lqvFRAUeR3P3VJbH5FbXSfF+riwpGGBekbHZJL9GuiaT2Vo0S01frGAXVCS2XfGFPh1WPIVF3UtF1fWg4iLWlhA9kg1Vd/EonmA9Q9lUfS6HqfaL57gdWS1vav9jE9YNl6p3+M4ctBI0M37IdxX3EjKAgLjl67ohc4+r3Lxjk0x0X6fylmxEhBO59eu83puWppSv4mpySfpeOiaoZkLl9JANm76zhJ/mTgRV6P2xS+6OM1rkx+GTVfJapT6diaV+AhEb+Cfon51f96j4RoZ1niSN9pFHz6gdFjeJZhLwPe5JGxWNYvpQLXSR+LUR6fGtu4oWospE6rYimH7TPPKjXqASlLLF295Y0A== 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)(136003)(39860400002)(366004)(346002)(376002)(451199015)(8936002)(83380400001)(86362001)(44832011)(6486002)(6666004)(5660300002)(2906002)(186003)(6506007)(66574015)(6512007)(26005)(41300700001)(66946007)(82960400001)(38100700002)(478600001)(8676002)(6916009)(66556008)(4326008)(316002)(66476007); DIR:OUT; SFP:1102; X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1 X-MS-Exchange-AntiSpam-MessageData-0: =?iso-8859-1?Q?KTv0yL18x6mbKpARIcde4eDYa2HGr4HjbO6GjlG/s/HJYjihsW6oTZPh4v?= =?iso-8859-1?Q?q4xSDAsWA8ZFl2VERn02gtj4+AGGWCjDdy7A3742cKQVLEKFt5zLn69QTo?= =?iso-8859-1?Q?DOdaeFd/DWA8xX7vAtdvwx77MDP8d5TwwPxiuveETFg+3BnqTU5LWhLEI8?= =?iso-8859-1?Q?OZZ4I+hCnrE8x6tMWE/CoD05jpET8Kq5IgNvlNZtANu6fx+Ruy5PEJNSeP?= =?iso-8859-1?Q?Hm9FbZj6QWrGcHcP+LRzELLnLLnEN31eDBEEXhNGLtZHUdQEycCatXn+Vs?= =?iso-8859-1?Q?HmCBO80QADKbZBIuOpA+JEW1EaKCeY7uweWgrd85URzS4/UQjp79WFr9sk?= =?iso-8859-1?Q?FpPyrO0m+SxvkpA04ulBFdpQqdA2gIFprACld7g65IlH98Tv8fzac5vxiz?= =?iso-8859-1?Q?qnS6pEHfiAl/wkrie22BKH9n6cVffoJDfsILuAMaEHnXEVpbrz8hGW4XOr?= =?iso-8859-1?Q?tS+GNT0ZUZd8URNvLf2yibuuitwGihnvOwna7z47fncggryY+isJYJ2OBs?= =?iso-8859-1?Q?jxwv4VZlFwen/W/zExnwMeSDGLxpj342HWsjnhuJB0qWmYp+siU+dWYAFC?= =?iso-8859-1?Q?N+bmWOrnZIYZsQd9psJf7ikIC+pX0V5ihRrXAcTEp+bUleYntgSgm1FMjK?= =?iso-8859-1?Q?POp0mM9lDd1B0RS6y0ruVRZtp5d17gT6t+SWxpZ/ITFMnxBJ/kBPyGjLyu?= =?iso-8859-1?Q?PZyEMyhnTZ/+bY8qupXRVon6bmpYe7/FgrLGH0j2Vm9wRlBWBRlqmDbJgF?= =?iso-8859-1?Q?ORJBw5vu11CpJrQMYwx4aoj/HOdrhE60nEQ1yFhDjJGnF4VtARKQccuZz7?= =?iso-8859-1?Q?JhN5Kmz8h7N0ZWy1OladcBHB2N9K30xwvQ5tvQ8ipc6Q5QOnThGf4VpFuY?= =?iso-8859-1?Q?LXUdOjI+HNDLsRtYh4bU8OS9bqKvQ9qC0SwiS73I7AB7vIUnAaN/h4YexU?= =?iso-8859-1?Q?bfZUe/TK5x4apThNm3bgxTx3aNI4i7o+BJ3LjGvGBaEPXLN9gnTv3gn0K4?= =?iso-8859-1?Q?xYFFKLi7Ll5XYXjYlPMZ2PEA1XYuAS++EEo+mXFbm1KJ51haJ4Al4s488D?= =?iso-8859-1?Q?CT1Ifprlw+R20p+IWuiEhZUrjl4u0jYroQXuOQOitYsEZZTsIyRLUUk+vw?= =?iso-8859-1?Q?FA2awyXBx1/3Xkz09th/roICZrqLD7ibNv7CfaBFciCAnCejbgSfwfunK7?= =?iso-8859-1?Q?HQp666CVyU6b5xDelTvJEMepz/7sXnH16KAxs9XNj8Z2MhBs4jjZyHGYrd?= =?iso-8859-1?Q?r2wJlDUq9oYwgCayEnoZLHtELfqZRLIrVlnfKORKoRs7Xw0iy8SfT9nC8x?= =?iso-8859-1?Q?FlW6xi5iCXMnxmekhRRo4mNkosc/irG5YIyrtvP2WzSOFiFZjeSm0PdcQR?= =?iso-8859-1?Q?+0kStsEQWGF3J70yC8ZHC9u+npQysF35sjIfsKHKFnqk1xMyyE4DpatXCF?= =?iso-8859-1?Q?t2Nth5q19DSqvO17hHuueOCta6k0Yx+pIz9tpnfLBhM8+Q50vVDLyrhStH?= =?iso-8859-1?Q?xdnOVUOvvqW75fvVJLgzPhS8vc4sLpZeFy5ANnOHNpft2f4ZCGuD1H5rr+?= =?iso-8859-1?Q?0z3nz1UkGSM1WH7//Z0pIAqkrfbpbBQkbwvQTaKojOiwXK6K//UdpqaP7X?= =?iso-8859-1?Q?2miZPYpwnE4q0wgZscBNo2iENS8GzANio31TFPX4Pjuh+by+Q8acoFFg?= =?iso-8859-1?Q?=3D=3D?= X-MS-Exchange-CrossTenant-Network-Message-Id: 54478bbe-1006-4036-ab4c-08dadc32f4fb X-MS-Exchange-CrossTenant-AuthSource: MWHPR11MB1629.namprd11.prod.outlook.com X-MS-Exchange-CrossTenant-AuthAs: Internal X-MS-Exchange-CrossTenant-OriginalArrivalTime: 12 Dec 2022 11:21:05.8736 (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: 5s2GVh1dUrE5AQ/v/YC0FW24rM02CufCbNAvOYViS2cvJ77/ibyjcMWvYsUGf9ExKJRF94y/7/warfM424494uJkJMiwpFj+QUBwmotyHIo= X-MS-Exchange-Transport-CrossTenantHeadersStamped: SA1PR11MB6918 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 Mon, Dec 12, 2022 at 12:02:32PM +0100, Morten Brørup wrote: > > From: Bruce Richardson [mailto:bruce.richardson@intel.com] > > Sent: Monday, 12 December 2022 11.32 > > > > On Mon, Dec 12, 2022 at 02:42:55PM +0800, Huisong Li wrote: > > > Some lib telemetry interfaces add the 'u32' and 'u64' data by the > > > rte_tel_data_add_dict/array_int API. This may cause data conversion > > error > > > or data truncation. > > > > > > The 'u32' data can not be assigned to signed 32-bit integer. However, > > > assigning to u64 is very wasteful, after all, the buffer capacity of > > each > > > transfer is limited. So it is necessary for 'u32' data to add usigned > > > 32-bit integer type and a series of 'u32' operation APIs. > > > > > > This patchset uses the new 'u32' API to resolve the problem of data > > > conversion error, and use the 'u64' API to add 'u64' data. > > > > > > In addition, this patchset introduces two APIs to store u32 and u64 > > > values as hexadecimal encoded strings in telemetry library. > > > > > > --- -v3: fix a misspelling mistake in commit log. -v2: - fix ABI > > break > > > warning. - introduce two APIs to store u32 and u64 values as > > hexadecimal > > > encoded strings. > > > > > I'm not convinced about adding the u32 value generically to the > > telemetry > > lib - except in the case of having explicit function calls for u32 vs > > u64 > > hex strings. Having a u32 type doesn't gain us any space internally > > over a > > u64 value, since all values are in a union type. Also, for output as > > json, > > the numeric values are all output as decimal values, meaning that the > > value > > 1 appears as the same size in the output string whether it is a u32 or > > u64 > > type. Now, it may save space in a future binary output format, but even > > then it still may not do so. > > I agree that a u32 doesn't gain any space internally. > > However, many SNMP counters are unsigned 32 bit, and expected to wrap around as such. > > So I suppose the u32 type might be useful for SNMP, if obtained through the telemetry library. > > Alternatively, we could somehow reuse the u64 type and require the application to pass (value & UINT32_MAX) to the u64 functions. To make this easy to use, we should add some wrappers to do it for the application. And eventually we would probably end up with something very similar to this patch. > I think just using the u64 functions is probably simplest and best right now. If we add support for something like snmp then yes, it would make sense to explicitly add it, but it seems like a lot of extra code for little or no benefit until we support something like that. > > > > Therefore, I'd tend to keep the existing u64 type as-is, and instead > > only > > add the functions for outputting hex values. Those hex output functions > > could take an additional parameter indicating the desired hex output > > length, as there could well be cases where we want just 16-bit hex > > value > > too. > > The way I read the patch series, the hex output length is not fixed, but an u64 value of 5 will be output as 0x5, not 0x0000000000000005. > > So the only benefit of having both u32_hex and u64_hex functions is to avoid type promotion from uint32_t to uint64_t on input for 32 bit values. > > Instead of passing a 3rd parameter or adding u16_hex functions, we could consider just having one set of hex functions using uint64_t for the value, and rely on type promotion for 32 and 16 bit values. > +1 to having only a single hex function, and I think type promotion should work fine. However, I still think it might be worthwhile allowing the user to pass in a min output length parameter too. I find for many hex strings having the leading zeros to explicitly show the length can be useful. The value "0" could cover the current behaviour of no-padding, otherwise the parameter should indicate the number of bits to be displayed. (If we want to lock it down we can use an enum parameter rather than int to limit it to 0, 8, 16, 32 or 64 bit displayed values). All that said, I'm not massively concerned if we want to just keep the current approach of always just printing without any leading zeros. It's a nice-to-have only for me. /Bruce