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 531F5A0543; Wed, 12 Oct 2022 17:35:51 +0200 (CEST) Received: from [217.70.189.124] (localhost [127.0.0.1]) by mails.dpdk.org (Postfix) with ESMTP id 0AE284309C; Wed, 12 Oct 2022 17:35:51 +0200 (CEST) Received: from smartserver.smartsharesystems.com (smartserver.smartsharesystems.com [77.243.40.215]) by mails.dpdk.org (Postfix) with ESMTP id 2CBCC43027 for ; Wed, 12 Oct 2022 17:35:50 +0200 (CEST) X-MimeOLE: Produced By Microsoft Exchange V6.5 Content-class: urn:content-classes:message MIME-Version: 1.0 Content-Type: text/plain; charset="iso-8859-1" Content-Transfer-Encoding: quoted-printable Subject: RE: rte_event_dev_xstats_reset id type Date: Wed, 12 Oct 2022 17:35:45 +0200 Message-ID: <98CBD80474FA8B44BF855DF32C47DC35D873D8@smartserver.smartshare.dk> In-Reply-To: <8864071.MhkbZ0Pkbq@thomas> X-MS-Has-Attach: X-MS-TNEF-Correlator: Thread-Topic: rte_event_dev_xstats_reset id type Thread-Index: AdjeTSRe+0dbgVkZS/6ROjAfPcxZbQAAQPhg References: <98CBD80474FA8B44BF855DF32C47DC35D873D4@smartserver.smartshare.dk> <98CBD80474FA8B44BF855DF32C47DC35D873D6@smartserver.smartshare.dk> <8864071.MhkbZ0Pkbq@thomas> From: =?iso-8859-1?Q?Morten_Br=F8rup?= To: "Thomas Monjalon" , "Jerin Jacob" , "Van Haaren, Harry" Cc: "Jerin Jacob" , , "Li, WeiyuanX" , "Ferruh Yigit" , "Andrew Rybchenko" , 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 > From: Thomas Monjalon [mailto:thomas@monjalon.net] > Sent: Wednesday, 12 October 2022 17.13 >=20 > 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 = unsigned > 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 = the > "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 = variable > 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. >=20 > 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 = device types, for consistency across device types? If 2, then what would be a good type? Ethdev uses uint64_t for xstats id, and (speaking without knowledge = about its internals) that seems like overkill to me. Arrays of these are = being used, so size does matter.