RE: rte_event_dev_xstats_reset id type

["Morten Brørup" <mb@smartsharesystems.com>]

> From: Van Haaren, Harry [mailto:harry.van.haaren@intel.com]
> Sent: Wednesday, 12 October 2022 12.30
> 
> > -----Original Message-----
> > From: Jerin Jacob <jerinjacobk@gmail.com>
> > Sent: Wednesday, October 12, 2022 10:45 AM
> >
> > On Wed, Oct 12, 2022 at 1:40 PM Morten Brørup
> <mb@smartsharesystems.com>
> > 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.

> 
> 
> > > 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.

Unfortunately, these functions are not marked experimental, so breaking API/ABI is hard to do. :-(

> Lets change the unit test code from "unsigned int" to uint32_t, and
> that will fix the issue?
> 
> From a quick review in the test code, there are 3x occurrences of
> "unsigned int id" being used.
> I will send a patch to change them later today.

A simple change to uint32_t would be incorrect.

rte_event_dev_xstats_by_name_get() uses unsigned int, not uint32_t.

Only rte_event_dev_xstats_reset() uses uint32_t.

> 
> 
> > > Or even better, use the same type for an "xstats id" across all
> device types. For
> > ethdev devices, they are uint64_t, but I don't know why. (They are
> passed around as
> > arrays, so they could be 32 bit. I guess that they were originally
> not used in arrays, so
> > unsigned int seemed the logical choice.)
> > >
> > >
> > > [1]: https://bugs.dpdk.org/show_bug.cgi?id=1101
> > > [2]:
> https://git.dpdk.org/dpdk/tree/drivers/event/sw/sw_evdev_selftest.c#n17
> 66
> > >
> > >
> > > Med venlig hilsen / Kind regards,
> > > -Morten Brørup