From mboxrd@z Thu Jan  1 00:00:00 1970
Return-Path: <dev-bounces@dpdk.org>
Received: from mails.dpdk.org (mails.dpdk.org [217.70.189.124])
	by inbox.dpdk.org (Postfix) with ESMTP id 346F1A00C2;
	Thu, 13 Oct 2022 10:26:44 +0200 (CEST)
Received: from [217.70.189.124] (localhost [127.0.0.1])
	by mails.dpdk.org (Postfix) with ESMTP id 155A242C95;
	Thu, 13 Oct 2022 10:26:44 +0200 (CEST)
Received: from out1-smtp.messagingengine.com (out1-smtp.messagingengine.com
 [66.111.4.25]) by mails.dpdk.org (Postfix) with ESMTP id D101042C27;
 Thu, 13 Oct 2022 10:26:42 +0200 (CEST)
Received: from compute4.internal (compute4.nyi.internal [10.202.2.44])
 by mailout.nyi.internal (Postfix) with ESMTP id 49F2C5C00E1;
 Thu, 13 Oct 2022 04:26:42 -0400 (EDT)
Received: from mailfrontend1 ([10.202.2.162])
 by compute4.internal (MEProxy); Thu, 13 Oct 2022 04:26:42 -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=1665649602; x=
 1665736002; bh=uN36gr9mn5mJ9VAV8C0NpyuiZsCkelWwbSvuS8ljz8A=; b=A
 esLoD6mXEK41/5AJmP9oWI9bvQ81cMqXziCymBHCRfWO++MYfpYFmCEpeI+nt8u4
 g+0E3YVN6/6pF3CEFWADWTcdbKodYcNiW/jGXCETd3ldyVUkv4UJdNfsOXk+VUkv
 RBQqCRI5+c2Hrhq3mWkM/RNmSv5lSJW8GZKb/wZu6eg7TR8Sf74VELsVGenuEVg5
 MPIUUK7BzcVry8WmksTiiNMQXyLjMPyTD7v/xvjY6Ou60cQUZrtvaBGvmxgZjItn
 qngpPf2x3R0bY+SSvh//FTDw7q5xST6MjwLkeD51zJrZJiwidF4bQ/NQDtiy9m5x
 4pv3vGPUGSiiHNGM+kKog==
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=1665649602; x=
 1665736002; bh=uN36gr9mn5mJ9VAV8C0NpyuiZsCkelWwbSvuS8ljz8A=; b=P
 cpXe5kOIWyzarGnCf87jPBuScOIdzXXD7zsseTWhjHf4zcA5J0wDlpIrh/GzUeqT
 DVQhm2QtJ9CO4Bft3uLG3N2bZNjqtcjuWKYTBFvrB5QNXAylNeUPwBSRTiZI1uAN
 mnSZlub8z/ZJOYIxsKNOjbJN/oMABFdvjUwRiJ8RE6xTeVaI3b0arjK/gQj1tFrp
 Dkvbw8U8cl2OcVCrcuibBnQULCDyBETTmeQNtPSEU2W8TPqAUkgIqEn8F55uqip3
 Ns7/vnzilkQ2Q9L7HXcb43eoSnWCBfiXaFYu0Fs+SG8AGHjLSzkPV05dcDrmQth+
 GtpLBOjV18iJnINqVWOig==
X-ME-Sender: <xms:wctHYwNTBmj_33h9EzUGakoXqMcLtpV5kRBh68VvNrV4bS3le6aBHw>
 <xme:wctHY28mAGLRR-6IJqMaTN5wwFNp1ZCWHN59yLVKPcMI_4QxLwDjgdV72HbTBP1XF
 bNvSvOBK1OH1nz9Uw>
X-ME-Received: <xmr:wctHY3RvTISxtT797ggCGif1hzoFr_R-f4WY6FYIFgBBxZ8yhC8p_qsuhtowsOlGP2xYli8zyHTEAhNocME4ylVtIg>
X-ME-Proxy-Cause: gggruggvucftvghtrhhoucdtuddrgedvfedrfeektddgtdegucetufdoteggodetrfdotf
 fvucfrrhhofhhilhgvmecuhfgrshhtofgrihhlpdfqfgfvpdfurfetoffkrfgpnffqhgen
 uceurghilhhouhhtmecufedttdenucesvcftvggtihhpihgvnhhtshculddquddttddmne
 cujfgurhephffvvefufffkjghfggfgtgesthhqredttddtudenucfhrhhomhepvfhhohhm
 rghsucfoohhnjhgrlhhonhcuoehthhhomhgrshesmhhonhhjrghlohhnrdhnvghtqeenuc
 ggtffrrghtthgvrhhnpeevueegudejtdfhuddvhfdtveetleetkeevhfeuueegfeffffff
 leeluefhhefgveenucffohhmrghinhepphhrohhofhhpohhinhhtrdgtohhmpdguphgukh
 drohhrghenucevlhhushhtvghrufhiiigvpedtnecurfgrrhgrmhepmhgrihhlfhhrohhm
 pehthhhomhgrshesmhhonhhjrghlohhnrdhnvght
X-ME-Proxy: <xmx:wctHY4sYgZzstfomE8DkFEvK-GDP_6A-y4mtCz4AXFKga4QEv1tUGw>
 <xmx:wctHY4cZbAXldliLiZYGutYuPanMdZpUleF6ZSiZ7tNb0J5_cRm2Gg>
 <xmx:wctHY83EUIllGRkHzOe8ibpbo8qZ_M6NUceGZeNXkhmdn1tMrdqGfw>
 <xmx:wstHY396MUhxRTmsnh51gm01NCHld4nzJsisO0L1XJiFw5bfKhqLqA>
Feedback-ID: i47234305:Fastmail
Received: by mail.messagingengine.com (Postfix) with ESMTPA; Thu,
 13 Oct 2022 04:26:39 -0400 (EDT)
From: Thomas Monjalon <thomas@monjalon.net>
To: Morten =?ISO-8859-1?Q?Br=F8rup?= <mb@smartsharesystems.com>,
 Jerin Jacob <jerinjacobk@gmail.com>, Sachin Saxena <sachin.saxena@oss.nxp.com>,
 Hemant Agrawal <hemant.agrawal@nxp.com>, Ori Kam <orika@nvidia.com>,
 Liron Himi <lironh@marvell.com>,
 Pavan Nikhilesh Bhagavatula <pbhagavatula@marvell.com>
Cc: "Van Haaren, Harry" <harry.van.haaren@intel.com>,
 Jerin Jacob Kollanukkaran <jerinj@marvell.com>, "dev@dpdk.org" <dev@dpdk.org>,
 "Li, WeiyuanX" <weiyuanx.li@intel.com>, Ferruh Yigit <ferruh.yigit@amd.com>,
 Andrew Rybchenko <andrew.rybchenko@oktetlabs.ru>,
 "david.marchand@redhat.com" <david.marchand@redhat.com>, techboard@dpdk.org
Subject: Re: xstats id type
Date: Thu, 13 Oct 2022 10:26:37 +0200
Message-ID: <4855955.TLkxdtWsSY@thomas>
In-Reply-To: <PH0PR18MB4086BE83D1C9F8311402A621DE259@PH0PR18MB4086.namprd18.prod.outlook.com>
References: <98CBD80474FA8B44BF855DF32C47DC35D873D4@smartserver.smartshare.dk>
 <98CBD80474FA8B44BF855DF32C47DC35D873DA@smartserver.smartshare.dk>
 <PH0PR18MB4086BE83D1C9F8311402A621DE259@PH0PR18MB4086.namprd18.prod.outlook.com>
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 <dev.dpdk.org>
List-Unsubscribe: <https://mails.dpdk.org/options/dev>,
 <mailto:dev-request@dpdk.org?subject=unsubscribe>
List-Archive: <http://mails.dpdk.org/archives/dev/>
List-Post: <mailto:dev@dpdk.org>
List-Help: <mailto:dev-request@dpdk.org?subject=help>
List-Subscribe: <https://mails.dpdk.org/listinfo/dev>,
 <mailto:dev-request@dpdk.org?subject=subscribe>
Errors-To: dev-bounces@dpdk.org

13/10/2022 09:12, Pavan Nikhilesh Bhagavatula:
> From: Morten Br=F8rup <mb@smartsharesystems.com>
> > +TO: rawdev maintainers, regexdev maintainers
> >=20
> > > From: Thomas Monjalon [mailto:thomas@monjalon.net]
> > > Sent: Wednesday, 12 October 2022 22.44
> > >
> > > 12/10/2022 18:47, Jerin Jacob:
> > > > On Wed, Oct 12, 2022 at 9:58 PM Thomas Monjalon
> > <thomas@monjalon.net>
> > > wrote:
> > > > >
> > > > > 12/10/2022 18:16, Jerin Jacob:
> > > > > > On Wed, Oct 12, 2022 at 9:05 PM Morten Br=F8rup
> > > <mb@smartsharesystems.com> 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 <mb@smartsharesystems.com>
> > > > > > > > > > From: Van Haaren, Harry
> > > [mailto:harry.van.haaren@intel.com]
> > > > > > > > > > > From: Jerin Jacob <jerinjacobk@gmail.com>
> > > > > > > > > > > > On Wed, Oct 12, 2022 at 1:40 PM Morten Br=F8rup wro=
te:
> > > > > > > > > > > > >
> > > > > > > > > > > > > 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 fou=
nd
> > > 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.
> > > > > > > >
> > > > > > > > 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?
> > > > > >
> > > > > > +1 for second option and the type as uint32_t
> > > > > >
> > > > > > >
> > > > > > > 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.
> > > > >
> > > > > uint64_t is not overkill if you consider having stats per queue
> > > with a predictable scheme.
> > > > > That's an improvement I would like to work on,
> > > >
> > > > You mean to use a bitmask hence uint64_t.
> > > > Currently it is mapped as arrays so 2^64 stats may not be needed.
> > > >
> > > > No strong opinion, I was just curious to understand "stats per queue
> > > > with a predictable scheme" and how uint64_t helps with  that.
> > >
> > > Yes I mean some bits are used for the queue number.
> > > Something like in slide 11 of this presentation:
> > > https://urldefense.proofpoint.com/v2/url?u=3Dhttp-
> > 3A__fast.dpdk.org_events_slides_DPDK-2D2019-2D09-2DEthernet-
> > 5FStatistics.pdf&d=3DDwIFAw&c=3DnKjWec2b6R0mOyPaz7xtfQ&r=3D1cjuAHrGh74
> > 5jHNmj2fD85sUMIJ2IPIDsIJzo6FN6Z0&m=3DApdcbroZzSNlcY1t4c8iv9HZk6YSJOA
> > Hpg93zuyIEEWa6xkViBTdoCA3iir_FCtW&s=3DwEMA0lnyrTmxmmDINhzOagGvV
> > Z3TcIrzfK5NbJHafdM&e=3D
> >=20
> > With this presentation in mind, I strongly agree with Thomas that uint6=
4_t is
> > the best choice of type for xstats id.
> >=20
> > A quick search shows that both eventdev and rawdev use "unsigned int",
> > except rte_event_dev_xstats_reset() and rte_rawdev_xstats_reset(), which
> > both use uint32_t. And regexdev uses uint16_t. Other device APIs don't =
have
> > xstats.
>=20
> Harry,=20
> Are you working on a patch for this change? If not I will do it.
>=20
> Thomas,
> Are you ok to break the ABI without deprication notice i.e. make ID as u6=
4 for eventdev?

Yes, it is only increasing size of function parameters, right?
The only problematic part is that the application must pass
a pointer of the right size, meaning some application code change.

It would be an exception in the process,
so I am Cc'ing the techboard for more opinions.