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 64604A0543; Wed, 12 Oct 2022 18:28:27 +0200 (CEST) Received: from [217.70.189.124] (localhost [127.0.0.1]) by mails.dpdk.org (Postfix) with ESMTP id 0C72240156; Wed, 12 Oct 2022 18:28:27 +0200 (CEST) Received: from wout5-smtp.messagingengine.com (wout5-smtp.messagingengine.com [64.147.123.21]) by mails.dpdk.org (Postfix) with ESMTP id 1700740041 for ; Wed, 12 Oct 2022 18:28:26 +0200 (CEST) Received: from compute2.internal (compute2.nyi.internal [10.202.2.46]) by mailout.west.internal (Postfix) with ESMTP id 5C3A5320092E; Wed, 12 Oct 2022 12:28:24 -0400 (EDT) Received: from mailfrontend2 ([10.202.2.163]) by compute2.internal (MEProxy); Wed, 12 Oct 2022 12:28:25 -0400 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=monjalon.net; h= cc:cc:content-transfer-encoding:content-type:date:date:from:from :in-reply-to:in-reply-to:message-id:mime-version:references :reply-to:sender:subject:subject:to:to; s=fm3; t=1665592103; x= 1665678503; bh=JUX+zg/FCDYUoD6mUsWg1sbinAWiRW/qyu6vE/aDlBY=; b=z aL4w6DXhGxstUDvXB0x2w5fCeledeP7/Ih1FFRVev1FRDi8X7GJDH13swYJd7m8n GGEmyJceXBnbun9HzUl0uleoccaTMKwuCxN3TwqQrpP/eBwgiS+7aZvqTxcOmjY/ /KPZObqhcGaFqN3UyXNVWee5nA7ia2TFwxoEdqKpnkibCo6pf7h3xjBbulJB0EKf uBi2m/E9HUJLDD5Nu9FNjv6rHEBAf49jN49i1ZutGBU9KGINNxoZ/ZaydKQPEcg1 CqHNtN93IJ3y/ucVk/r5/5Dq7dJZ5o7ueur36L/hTx0LmI743r45IwL0EeEJx+32 Kq7+AS77fFjChvqCBMUjA== DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d= messagingengine.com; h=cc:cc:content-transfer-encoding :content-type:date:date:feedback-id:feedback-id:from:from :in-reply-to:in-reply-to:message-id:mime-version:references :reply-to:sender:subject:subject:to:to:x-me-proxy:x-me-proxy :x-me-sender:x-me-sender:x-sasl-enc; s=fm3; t=1665592103; x= 1665678503; bh=JUX+zg/FCDYUoD6mUsWg1sbinAWiRW/qyu6vE/aDlBY=; b=I 7fJuxfaMQ/3Dr2FvZ/ClzUYY8Sr+3oDMnOMB1JgrT0EXbhzKUN1SZjZcoLO8BVJs LIul31HeYDJSYOlPPj1JIaTP5/GSzXnK/+CCf1azVdmX2PSPdogG6epqVueX3Flr iOVmpYXL5qRe73QJ8GLdhxw83YdjHHuQ5BocLdQl3daUt5QZvlwutAj+RFPRjbKB 2CpaPD+pB2VPQoND9hhttZNVYb+K619kce8Z4CY3r6p4VNdsGuhxw9Z4yo+2/JBZ RH09G8IyeIc+y9uXMVhRH16GbQcVDha7lebQ4VhrzidFwT1kc8HfIqKXzuCSdBZL irDzc2SfvVptZo+K70LeA== X-ME-Sender: X-ME-Received: X-ME-Proxy-Cause: gggruggvucftvghtrhhoucdtuddrgedvfedrfeejkedguddtvdcutefuodetggdotefrod ftvfcurfhrohhfihhlvgemucfhrghsthforghilhdpqfgfvfdpuffrtefokffrpgfnqfgh necuuegrihhlohhuthemuceftddtnecusecvtfgvtghiphhivghnthhsucdlqddutddtmd enucfjughrpefhvfevufffkfgjfhgggfgtsehtqhertddttddunecuhfhrohhmpefvhhho mhgrshcuofhonhhjrghlohhnuceothhhohhmrghssehmohhnjhgrlhhonhdrnhgvtheqne cuggftrfgrthhtvghrnhepfefhjeeluedvvedtuddtuedtvefhieejtefhffeujefhtedu udevtdektdeikeffnecuvehluhhsthgvrhfuihiivgeptdenucfrrghrrghmpehmrghilh hfrhhomhepthhhohhmrghssehmohhnjhgrlhhonhdrnhgvth X-ME-Proxy: Feedback-ID: i47234305:Fastmail Received: by mail.messagingengine.com (Postfix) with ESMTPA; Wed, 12 Oct 2022 12:28:22 -0400 (EDT) From: Thomas Monjalon To: Morten =?ISO-8859-1?Q?Br=F8rup?= , Jerin Jacob Cc: "Van Haaren, Harry" , Jerin Jacob , dev@dpdk.org, "Li, WeiyuanX" , Ferruh Yigit , Andrew Rybchenko , david.marchand@redhat.com Subject: Re: rte_event_dev_xstats_reset id type Date: Wed, 12 Oct 2022 18:28:20 +0200 Message-ID: <6614700.R56niFO833@thomas> In-Reply-To: References: <98CBD80474FA8B44BF855DF32C47DC35D873D4@smartserver.smartshare.dk> <98CBD80474FA8B44BF855DF32C47DC35D873D8@smartserver.smartshare.dk> MIME-Version: 1.0 Content-Transfer-Encoding: quoted-printable Content-Type: text/plain; charset="iso-8859-1" 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 12/10/2022 18:16, Jerin Jacob: > On Wed, Oct 12, 2022 at 9:05 PM Morten Br=F8rup wrote: > > > > > From: Thomas Monjalon [mailto:thomas@monjalon.net] > > > Sent: Wednesday, 12 October 2022 17.13 > > > > > > 12/10/2022 14:14, Van Haaren, Harry: > > > > From: Morten Br=F8rup > > > > > From: Van Haaren, Harry [mailto:harry.van.haaren@intel.com] > > > > > > From: Jerin Jacob > > > > > > > On Wed, Oct 12, 2022 at 1:40 PM Morten Br=F8rup wrote: > > > > > > > > > > > > > > > > Hi Jerin (eventdev maintainer), > > > > > > > > > > > > > > + harry.van.haaren@intel.com as the changes in > > > drivers/event/sw. > > > > > > > > > > > > Thanks Jerin. > > > > > > > > > > > > > > > > > > > > While looking into bug #1101 [1], I noticed a mix of unsign= ed > > > int > > > > > > and uint32_t in > > > > > > > the test code, which will fail on 64-bit big endian CPUs. > > > > > > > > > > > > Aha; that we can fix. I am curious why this isn't found in > > > CI/reported > > > > > > before. > > > > > > > > > > We probably don't test any 64-bit *big endian* architectures. Just > > > a guess. > > > > > > > > Seems so yes. > > > > > > > > > > > > Specifically, rte_event_dev_xstats_reset() is called with t= he > > > "ids" > > > > > > parameter > > > > > > > pointing to an unsigned int [2], but that parameter is a > > > pointer to > > > > > > an uint32_t. > > > > > > > > > > > > > > > > I think the type of the ids array parameter to > > > > > > rte_event_dev_xstats_reset() should > > > > > > > be changed to unsigned int array, like in the other > > > > > > rte_event_dev_xxx() functions. > > > > > > > > > > > > In this case, we have the option to change the type of a variab= le > > > in a > > > > > > test-case, or change API and cause API/ABI breakage. > > > > > > > > > > Well.. yes, but I would phrase that last option: Change the > > > API/ABI, so related > > > > > functions consistently use the same type for the same variable, > > > instead of randomly > > > > > mixing uint64_t, uint32_t and unsigned int, depending on function. > > > > > > > > Aah ok; I see your point now; there is inconsistent usage of > > > uint32_t/unsigned int > > > > between the Eventdev APIs itself. Agree this is sub-optimal, and > > > would have been > > > > nice to have spotted before the Eventdev API was stabilized. > > > > > > > > > > > > > Unfortunately, these functions are not marked experimental, so > > > breaking API/ABI is > > > > > hard to do. :-( > > > > > > > > Agreed again. > > > > > > 22.11 is a breaking release, > > > and changing type in the API is not much impactful, > > > so that's something you can change now, > > > or be quiet forever :) > > > > Question: > > 1. Only change the "xstats id" type in the one eventdev function, which= deviates from other eventdev functions, or > > 2. Change the "xstats id" type for all xstats functions across all devi= ce types, for consistency across device types? > > > > If 2, then what would be a good type? >=20 > +1 for second option and the type as uint32_t >=20 > > > > Ethdev uses uint64_t for xstats id, and (speaking without knowledge abo= ut its internals) that seems like overkill to me. Arrays of these are being= used, so size does matter. uint64_t is not overkill if you consider having stats per queue with a pred= ictable scheme. That's an improvement I would like to work on, so I would like to keep uint64_t please.