RE: [PATCH 1/6] eventdev: support to set queue attributes at runtime

["Van Haaren, Harry" <harry.van.haaren@intel.com>]

> -----Original Message-----
> From: Shijith Thotton <sthotton@marvell.com>
> Sent: Monday, April 4, 2022 10:36 AM
> To: Van Haaren, Harry <harry.van.haaren@intel.com>; dev@dpdk.org; Jerin Jacob
> Kollanukkaran <jerinj@marvell.com>
> Cc: Pavan Nikhilesh Bhagavatula <pbhagavatula@marvell.com>; Ray Kinsella
> <mdr@ashroe.eu>
> Subject: RE: [PATCH 1/6] eventdev: support to set queue attributes at runtime
> 
> ><snip>
> >
> >> +/**
> >> + * Set an event queue attribute at runtime.
> >> + *
> >> + * @param dev
> >> + *   Event device pointer
> >> + * @param queue_id
> >> + *   Event queue index
> >> + * @param attr_id
> >> + *   Event queue attribute id
> >> + * @param attr_value
> >> + *   Event queue attribute value
> >> + *
> >> + * @return
> >> + *  - 0: Success.
> >> + *  - <0: Error code on failure.
> >> + */
> >> +typedef int (*eventdev_queue_attr_set_t)(struct rte_eventdev *dev,
> >> +					 uint8_t queue_id, uint32_t attr_id,
> >> +					 uint32_t attr_value);
> >
> >Is using a uint64_t a better type for attr_value? Given there might be more in
> >future,
> >limiting to 32-bits now may cause headaches later, and uint64_t doesn't cost
> >extra?
> >
> >I think 32-bits of attr_id is enough :)
> >
> >Same comment on the _get() API in patch 2/6, a uint64_t * would be a better fit
> >there in my opinion.
> >
> ><snip>
> 
> Changing size of attr_value will an ABI break. Can we wait till a need arises ?

Ah, I forgot that the _get() function is already upstream in DPDK today.

Its actually an API *and* ABI break, which is worse, as user code would have to
change (not just a re-compile against the newer DPDK version...). Any application
attempting source-compatibility with 21.11 and 22.11 would have to #ifdef the
parameter, switching uint32_t* and uint64_t*... or use some magic void* hacks.

Yes I suppose that waiting until a u64 is required for a real-world use-case is probably
better than breaking existing users code today (or in next ABI breaking release) with the
intent of getting to "perfect" API/ABIs...

Suggest to use a u64 for _set() to avoid getting into this same situation again,
but leave _get() as is, until it is required to change for a real use-case?