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 68926A0540; Tue, 13 Dec 2022 18:14:34 +0100 (CET) Received: from mails.dpdk.org (localhost [127.0.0.1]) by mails.dpdk.org (Postfix) with ESMTP id 55C6E40146; Tue, 13 Dec 2022 18:14:34 +0100 (CET) Received: from mga04.intel.com (mga04.intel.com [192.55.52.120]) by mails.dpdk.org (Postfix) with ESMTP id 337EC400D5 for ; Tue, 13 Dec 2022 18:14:32 +0100 (CET) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=intel.com; i=@intel.com; q=dns/txt; s=Intel; t=1670951673; x=1702487673; h=date:from:to:cc:subject:message-id:references: content-transfer-encoding:in-reply-to:mime-version; bh=wVLQrPmxv2LetlaWFyyQC43bXhSmpDmCPBqaAk1crVg=; b=XhCpXOO6GstOGuU80rjQ33uzvjz7wP6cMyUm5pLoe5cuoQaiGsLDSEKs bYYiuHgV2UPDUYB0oS55Oo5E9NkK3IS+l3STN3NjQCnHY/5bXHOdMLbeg swvzTyXJOI5L6JWLCzQmzl1tHI3KhGGDXBwc3T3CGtcrSpmeGp87+t8rA JTJD9t0a3IiG6qL+8OkGxS4kg0UVdyZJRat8+Cl9QDmtkq1aOzDzq5kuQ kDeUlUCtcyVHneXvuPVKFlCLHuSdLpD5PfoPesoL8CE6yHeLRsU8ct2e3 K2Ndv5VSGJizV7X/x5SkA6OfWfaY/PhPfJHChfnbA6/5Kr/qHRsLeg4RY g==; X-IronPort-AV: E=McAfee;i="6500,9779,10560"; a="316887252" X-IronPort-AV: E=Sophos;i="5.96,242,1665471600"; d="scan'208";a="316887252" Received: from fmsmga007.fm.intel.com ([10.253.24.52]) by fmsmga104.fm.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 13 Dec 2022 09:12:22 -0800 X-ExtLoop1: 1 X-IronPort-AV: E=McAfee;i="6500,9779,10560"; a="650796343" X-IronPort-AV: E=Sophos;i="5.96,242,1665471600"; d="scan'208";a="650796343" Received: from fmsmsx601.amr.corp.intel.com ([10.18.126.81]) by fmsmga007.fm.intel.com with ESMTP; 13 Dec 2022 09:12:21 -0800 Received: from fmsmsx610.amr.corp.intel.com (10.18.126.90) by fmsmsx601.amr.corp.intel.com (10.18.126.81) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256) id 15.1.2507.16; Tue, 13 Dec 2022 09:12:21 -0800 Received: from FMSEDG603.ED.cps.intel.com (10.1.192.133) by fmsmsx610.amr.corp.intel.com (10.18.126.90) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256) id 15.1.2507.16 via Frontend Transport; Tue, 13 Dec 2022 09:12:21 -0800 Received: from NAM04-DM6-obe.outbound.protection.outlook.com (104.47.73.44) by edgegateway.intel.com (192.55.55.68) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.1.2507.16; Tue, 13 Dec 2022 09:12:21 -0800 ARC-Seal: i=1; a=rsa-sha256; s=arcselector9901; d=microsoft.com; cv=none; b=H2fCtreMQDNGWRrKRm3TLkflquRjrfETZxN3tRptKZtxy3lwZgoemX4vsW/C5yBUj8VIzOaE0cGWLgaJdfYu6JrD3J7lS/JNkQgfU09FfjviiEdeulfEpr7XBzJP1pJz0NDjuaJOgh4QHAj662OloU5wXfbWbhUocHhEe/XwTMK7D5Cmp7jHvGNTGXhMZJDRrJveeq3hq8kiLXpeYzwZdDGlXl/y3HHsKCuymlkNJ6iMkpozFsOHf7uEuMpZFeYIvneHUZgiY3U4PD7MknjwrwzJdYoz+ixyWUqoVg4am2052HPcEjEy2SV6DE4bPNW+qvStr4AmOaCj+9fTJpp66A== 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=hUagIS4fspn7bI1RDXOKx78fifyiDAVSO0j9iNPoesY=; b=CM1tF2YhpajDqwshc+nwrnP5QwBP4AnXiPc90+UL63ADxQKwPnhiXISwl/5RJ88iStxKwHbwu7HNuviIImHrWq3v83KB1/pgil0nlywok61wj6nXpHwjEDQpHRQsBpIlP8+t/cl2uMH+UcR3F/H908hS9greeKRTaJmOXFlFx3zw7phl9pOOivCfPckK1ngmFF7n/jBclcqRUmv6AFqKKAgZMtCQ8uS3atmLecTmFS2jgjV6zrNYSUfcXKx6D4f68sc4unp0iDlDFTROvSD7c+axLAocZb4y/XNhDFq4NaK7u5wq+iUpg/lp6BoJ/HevWGShMHNkoZGO9bjhYF5H1A== 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 PH0PR11MB5205.namprd11.prod.outlook.com (2603:10b6:510:3d::24) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.20.5880.19; Tue, 13 Dec 2022 17:12:19 +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; Tue, 13 Dec 2022 17:12:19 +0000 Date: Tue, 13 Dec 2022 17:12:12 +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> <98CBD80474FA8B44BF855DF32C47DC35D87586@smartserver.smartshare.dk> Content-Type: text/plain; charset="iso-8859-1" Content-Disposition: inline Content-Transfer-Encoding: 8bit In-Reply-To: X-ClientProxiedBy: LO4P123CA0414.GBRP123.PROD.OUTLOOK.COM (2603:10a6:600:189::23) To MWHPR11MB1629.namprd11.prod.outlook.com (2603:10b6:301:d::21) MIME-Version: 1.0 X-MS-PublicTrafficType: Email X-MS-TrafficTypeDiagnostic: MWHPR11MB1629:EE_|PH0PR11MB5205:EE_ X-MS-Office365-Filtering-Correlation-Id: 91f2231d-4060-48eb-1ac7-08dadd2d30c9 X-MS-Exchange-SenderADCheck: 1 X-MS-Exchange-AntiSpam-Relay: 0 X-Microsoft-Antispam: BCL:0; X-Microsoft-Antispam-Message-Info: GUdHBY+2BS3vfc/fSE+53UkTd0UNEPmSEnuNDqISZOjhTiqeYRxqWrwLUjX/+Ph8jpyRBIr0ast+XvK7GwzTV8nM8cBQ6xYS0mHRfGAROFBDDqEW1kSM6IzhmUOmOTiWYH0dkT4zuiJceeCEQ/Cr9id+hS3lYFAHo4+lJNELA451g1HTsDzIIlWcilEYoktH4TsK4jiOd6jIjGaOpjo3wsequdcRlxMCBz3qwF9IS8VmjvsCTMzPs+scOR69dJyyMrkjWzOrBv1Y16Zp8JRnCUrknmj0WZAgkkr/2boOo/GRAdwg62AnLX3WDa5DrMKV9OZf2CsiOxkusjKv/cbJ6BZl0OhaIzAlvkeoW2cSjrMYm9vg158ODWCw+056+E2FzuhO30LTZPHgZNfnM6Kp2m/43ubiGFZg1yhhDqjqJH6IaqNOvIo6nTwjrCCQSaa18nwxw0WzFe/sEwaYDsAb1zBxnE5Gw2N65tFG6yHVyIZoAewANleeMZ5trJVtxDyVlCfSVpHBFgiB0sHBWnm6UBmjzFqIBOXMz8CoXOQnJpQVqlZlU2Ka0NeSjSEzMaCeErDndL/KBbSJijoYf6ptAeunIYNehklvyqbn3mmWHekGS0QogrWjOJNU0yH1Z/4jwgVXmOoOajm7sscRj/xX+Q== 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)(136003)(366004)(346002)(39860400002)(376002)(396003)(451199015)(38100700002)(6506007)(44832011)(478600001)(82960400001)(41300700001)(186003)(4326008)(86362001)(66946007)(6512007)(26005)(8676002)(66556008)(66574015)(83380400001)(66476007)(5660300002)(6486002)(8936002)(6916009)(316002)(6666004)(2906002); DIR:OUT; SFP:1102; X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1 X-MS-Exchange-AntiSpam-MessageData-0: =?iso-8859-1?Q?7bOGrBIv739oR8p3TUB5SoIdjTurVzluzIvHEI+/xqO8gKEYYi0XfsOP9D?= =?iso-8859-1?Q?yASlnNQZblBJ4g6NUxRjiv1zRATaokDuPPu8m7qet2oM+8kYPcR0K6njAq?= =?iso-8859-1?Q?tR4tsNHwcnC4S9vZkUJ05J18V4BinhikMV/FnWHhNHxY3ir15UYndPYsz3?= =?iso-8859-1?Q?9+9w6Ur6KClnHQwBkbDTp9R93oOSy2iRgo3jBDS2MRyBVtdb/sE0JQdeK0?= =?iso-8859-1?Q?fnuCqlM8bBDTFHnRzbtimLp01JzORqsYsk4HEwTUUTf7BfI2GwaM/e2hBY?= =?iso-8859-1?Q?lS0oCjcDImnd9b9nYLbUZvur5rs+1U0VCRR+jIeQloFYdmCuG8GjEbJoO7?= =?iso-8859-1?Q?muc2C5BExX4Eox/7Iprx8OcTE+0Du6+W419lcZmL9Uns5cPVfEHOlXH++s?= =?iso-8859-1?Q?NZSx742qcfEWoL0k2DeX5WY3fhTpEKJURGnmuQvYcPTnTUevNz75BqsuxV?= =?iso-8859-1?Q?HDMwY6+HEkfPMnao64cSDebUfEb26/AVGy9Mp2K1sBcKidyu3dI/0Dt361?= =?iso-8859-1?Q?h5fgSJmmQzfo8F3kg9fs7Ak58LufE780TEfXJJawdjfzetbxoVd1XX2qDk?= =?iso-8859-1?Q?KclNgPiWZ2MeoS6jzR1RyO1MakvRKInrJgWbtz+nAq3xIxpX1f+dMYrl2k?= =?iso-8859-1?Q?8kl+lX6HYdL3tnPNmJ4rTGEq5vslfn8CmFY4kJ8a0/apfgEntJrC3kSTCT?= =?iso-8859-1?Q?ea1wbFajWXdAYqRFN0hllvIjwF12CA+Wvi7VbBfw5mbVyK6axQa0XAlspx?= =?iso-8859-1?Q?ScorOCR4pz4bzRnoJQjibo0MEawS0QSlqh9oq1Jh1xsG2l4rjpElw7esLv?= =?iso-8859-1?Q?GLnRi88Ab6Rc+BTg2tmxGwSiyX7BeC2gONP5s80Jf+73nQ3qsv7CH9PH1K?= =?iso-8859-1?Q?M/dj+QZDuq180NLNhMsfzh+8x4gStlfN3uBFS5YnAw3BOTz1j632s7W5no?= =?iso-8859-1?Q?8sXwyb9YTSpKc2nhh+TneskJFSCpK+yZq92ElusyWR+n7o5XJgB+xojqey?= =?iso-8859-1?Q?jNdVR/lK5AuH0KbFLmzxvgnSdghIBocOAPIAPbmSR6IhiE5Pnq5iG1Wflz?= =?iso-8859-1?Q?vp9mvEyAoIikS3/QoTqY0OGPs2Iuamaw/3qkOs01aBiV0PQmzkQMY/mE2V?= =?iso-8859-1?Q?AwwaRXWUFqZbFyGzamtNJWgtt7WCBC8j/KuLKh7bRkqFFbzK0uvIb5vm4V?= =?iso-8859-1?Q?lq9ILNXUlkgWvUwrDTGWPRLOK5aV89Nqvyl4vLPe1QHpTIepZCLJid7cMR?= =?iso-8859-1?Q?wIjvrVVxpGUzrzNGQASxyNaIwxZzzdUNG7QhTSf+RLVTOBdwdcg5dZYVVd?= =?iso-8859-1?Q?CZCvRyipTlK+s2PlvhaMB8JI9tD3H+rGNXCdNu7do7Aihgb2CWiDY6qSGw?= =?iso-8859-1?Q?YNH2TYHxtGBObiitYWvAigXcNi3Qkk4uLapVxuqsphif59QEASNzoOWDsq?= =?iso-8859-1?Q?RHLK7QqYrqvovo57FgD9MdDqBHV53Ait4MdBwh6gqZTVY3EII/c7LCovno?= =?iso-8859-1?Q?4GF0yubPGp6GKkp2DnGz55SeQgTbcdlkygjJTYMb0OwMqUkJ0iiAS7KQp0?= =?iso-8859-1?Q?eFoT/vaylZqrqTC2/Xg+m7kOlq9PTn+FOXGKRNk8CpXRhRsTD/rtAF/7cs?= =?iso-8859-1?Q?ng+7o5718P9xFuWVGN6oWjp5kT517jlomXxMebZ9+mZTSMU0i2XZXuQQ?= =?iso-8859-1?Q?=3D=3D?= X-MS-Exchange-CrossTenant-Network-Message-Id: 91f2231d-4060-48eb-1ac7-08dadd2d30c9 X-MS-Exchange-CrossTenant-AuthSource: MWHPR11MB1629.namprd11.prod.outlook.com X-MS-Exchange-CrossTenant-AuthAs: Internal X-MS-Exchange-CrossTenant-OriginalArrivalTime: 13 Dec 2022 17:12:19.2786 (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: X7Kd4X7HtADRJyeZRotE3WojgYZqRagtVzZ/nuFDHnDyhjkQGvEq6ne2Q8v9P2jTiSSQCcGZLMkeTDgG+gDl+Ij4N7hN53k+v4zlW0eZ3U4= X-MS-Exchange-Transport-CrossTenantHeadersStamped: PH0PR11MB5205 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:16:07PM +0000, Bruce Richardson wrote: > On Mon, Dec 12, 2022 at 01:03:52PM +0100, Morten Brørup wrote: > > > From: Bruce Richardson [mailto:bruce.richardson@intel.com] > > > Sent: Monday, 12 December 2022 12.21 > > > > > > 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. > > > > > > If we wanted to fix this generally, we should rely on type promotion, so the existing _int function should be updated to take an int64_t value, and the _u64 function should be renamed to _uint (and still take an uint64_t value). However, that would break the ABI, and would need to go through some process for that. So let's not change this now. > > > > Yes, not making "int" an "i64" type was a poor design decision on my part > in the earlier versions. Thankfully negative values are rarely needed > beyond the range of 32-bits, but we should probably look to update this as > you suggest at the next ABI break window. > Actually, most of the work for this can be done without affecting ABI, I believe, and for the two functions that would be affected, function versioning could be used to cover those. I think it's better to make the change now using versioning rather than waiting, as it's likely to be forgotten if we wait. I'll work up a patchset for this so we can review and discuss... /Bruce