> -----Original Message----- > From: Shijith Thotton <sthot...@marvell.com> > Sent: Monday, April 4, 2022 10:36 AM > To: Van Haaren, Harry <harry.van.haa...@intel.com>; dev@dpdk.org; Jerin Jacob > Kollanukkaran <jer...@marvell.com> > Cc: Pavan Nikhilesh Bhagavatula <pbhagavat...@marvell.com>; Ray Kinsella > <m...@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?
