[PATCH] event/sw: fix selftest xstats reset API usage

[PATCH] event/sw: fix selftest xstats reset API usage

[Harry van Haaren]

The eventdev xstats reset API takes an ID of "uint32_t", while
the rest of the xstats APIs require an "unsigned int". On some
platforms these might not be the same bitwidth, however this was
assumed in the code.

Fix by providing a uint32_t to the xstats_reset() function.
Fixes: e21df4b062b5 ("test/eventdev: add SW xstats tests")

Reported-by: Morten Brørup <mb@smartsharesystems.com>
Signed-off-by: Harry van Haaren <harry.van.haaren@intel.com>

---

Note this is a fix for a potential build issue in 64-bit BE systems.

---
 drivers/event/sw/sw_evdev_selftest.c | 15 +++++++++------
 1 file changed, 9 insertions(+), 6 deletions(-)

diff --git a/drivers/event/sw/sw_evdev_selftest.c b/drivers/event/sw/sw_evdev_selftest.c
index ed7ae6a685..f8496bc44e 100644
--- a/drivers/event/sw/sw_evdev_selftest.c
+++ b/drivers/event/sw/sw_evdev_selftest.c
@@ -1525,10 +1525,11 @@ xstats_id_reset_tests(struct test *t)
 				dev_names[i], dev_expected[i], val);
 			goto fail;
 		}
-		/* reset to zero */
+		/* reset to zero: note API requires uint32_t not unsigned int */
+		uint32_t reset_id = id;
 		int reset_ret = rte_event_dev_xstats_reset(evdev,
 						RTE_EVENT_DEV_XSTATS_DEVICE, 0,
-						&id,
+						&reset_id,
 						1);
 		if (reset_ret) {
 			printf("%d: failed to reset successfully\n", __LINE__);
@@ -1647,10 +1648,11 @@ xstats_id_reset_tests(struct test *t)
 				port_expected[i], id);
 			failed = 1;
 		}
-		/* reset to zero */
+		/* reset to zero: note API requires uint32_t not unsigned int */
+		uint32_t reset_id = id;
 		int reset_ret = rte_event_dev_xstats_reset(evdev,
 						RTE_EVENT_DEV_XSTATS_PORT, PORT,
-						&id,
+						&reset_id,
 						1);
 		if (reset_ret) {
 			printf("%d: failed to reset successfully\n", __LINE__);
@@ -1762,10 +1764,11 @@ xstats_id_reset_tests(struct test *t)
 				queue_names[i], queue_expected[i], val);
 			failed = 1;
 		}
-		/* reset to zero */
+		/* reset to zero: note API requires uint32_t not unsigned int */
+		uint32_t reset_id = id;
 		int reset_ret = rte_event_dev_xstats_reset(evdev,
 						RTE_EVENT_DEV_XSTATS_QUEUE,
-						queue, &id, 1);
+						queue, &reset_id, 1);
 		if (reset_ret) {
 			printf("%d: failed to reset successfully\n", __LINE__);
 			failed = 1;
-- 
2.34.1

Re: [PATCH] event/sw: fix selftest xstats reset API usage

[Thomas Monjalon]

12/10/2022 14:34, Harry van Haaren:
> The eventdev xstats reset API takes an ID of "uint32_t", while
> the rest of the xstats APIs require an "unsigned int". On some
> platforms these might not be the same bitwidth, however this was
> assumed in the code.
> 
> Fix by providing a uint32_t to the xstats_reset() function.

Why adding a comment about the type in the code?
We are not adding /* using the right type */ in each line of code.

[...]
> -		/* reset to zero */
> +		/* reset to zero: note API requires uint32_t not unsigned int */
> +		uint32_t reset_id = id;
>  		int reset_ret = rte_event_dev_xstats_reset(evdev,

RE: [PATCH] event/sw: fix selftest xstats reset API usage

[Van Haaren, Harry]

> -----Original Message-----
> From: Thomas Monjalon <thomas@monjalon.net>
> Sent: Wednesday, October 12, 2022 2:06 PM
> To: Van Haaren, Harry <harry.van.haaren@intel.com>
> Cc: dev@dpdk.org; jerinj@marvell.com; Li, WeiyuanX <weiyuanx.li@intel.com>;
> ferruh.yigit@amd.com; andrew.rybchenko@oktetlabs.ru; Morten Brørup
> <mb@smartsharesystems.com>
> Subject: Re: [PATCH] event/sw: fix selftest xstats reset API usage
> 
> 12/10/2022 14:34, Harry van Haaren:
> > The eventdev xstats reset API takes an ID of "uint32_t", while
> > the rest of the xstats APIs require an "unsigned int". On some
> > platforms these might not be the same bitwidth, however this was
> > assumed in the code.
> >
> > Fix by providing a uint32_t to the xstats_reset() function.
> 
> Why adding a comment about the type in the code?
> We are not adding /* using the right type */ in each line of code.

Correct; but as the SW PMD was the only selftest code that actually
uses xstats_reset() I err-ed on the side of warning devs that there's
some "funky" or inconsistent API requirements here.

I can respin without if that seems better, but my intention was to
help future readers..?

<snip>

Re: [PATCH] event/sw: fix selftest xstats reset API usage

[Thomas Monjalon]

12/10/2022 15:08, Van Haaren, Harry:
> From: Thomas Monjalon <thomas@monjalon.net>
> > 12/10/2022 14:34, Harry van Haaren:
> > > The eventdev xstats reset API takes an ID of "uint32_t", while
> > > the rest of the xstats APIs require an "unsigned int". On some
> > > platforms these might not be the same bitwidth, however this was
> > > assumed in the code.
> > >
> > > Fix by providing a uint32_t to the xstats_reset() function.
> > 
> > Why adding a comment about the type in the code?
> > We are not adding /* using the right type */ in each line of code.
> 
> Correct; but as the SW PMD was the only selftest code that actually
> uses xstats_reset() I err-ed on the side of warning devs that there's
> some "funky" or inconsistent API requirements here.
> 
> I can respin without if that seems better, but my intention was to
> help future readers..?

It does not really help.
If you want to help, and you think the API is wrong,
then please send a patch to propose a change in the API.