> From: Van Haaren, Harry [mailto:harry.van.haa...@intel.com] > Sent: Wednesday, 12 October 2022 12.30 > > > -----Original Message----- > > From: Jerin Jacob <jerinjac...@gmail.com> > > Sent: Wednesday, October 12, 2022 10:45 AM > > > > On Wed, Oct 12, 2022 at 1:40 PM Morten Brørup > <m...@smartsharesystems.com> > > wrote: > > > > > > Hi Jerin (eventdev maintainer), > > > > + harry.van.haa...@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
