On Wed, Sep 27, 2023 at 1:43 PM Bruce Richardson <bruce.richard...@intel.com> wrote: > > On Tue, Sep 26, 2023 at 11:58:37PM +0530, Jerin Jacob wrote: > > On Mon, Sep 25, 2023 at 12:41 PM Mattias Rönnblom <hof...@lysator.liu.se> > > wrote: > > > > > > On 2023-09-22 09:38, Mattias Rönnblom wrote: > > > > > > <snip> > > > > > > > +int > > > > +rte_dispatcher_create(uint8_t id, uint8_t event_dev_id) > > > > +{ > > > > > > > > > There are two changes I'm considering: > > > > > > 1) Removing the "id" to identify the dispatcher, replacing it with an > > > forward-declared rte_dispatcher struct pointer. > > > > > > struct rte_dispatcher; > > > > > > struct rte_dispatcher * > > > rte_dispatcher_create(uint8_t event_dev_id); > > > > > > > > > The original reason for using an integer id to identify a dispatcher is > > > to make it look like everything else in Eventdev. I find this pattern a > > > little awkward to use - in particular the fact the id is > > > application-allocated (and thus require coordination between different > > > part of the application in case multiple instances are used). > > > > > > 2) Adding a flags field to the create function "for future use". But > > > since the API is experimental, there may not be that much need to > > > attempt to be future-proof? > > > > > > Any thoughts are appreciated. > > > > IMO, better to have rte_dispatcher_create(struct > > rte_dispatch_create_params *params) > > for better future proofing with specific > > rte_dispatch_crearte_params_init() API(No need to add reserved fields > > in rte_dispatch_create_params now, may need only for before removing > > experimental status) > > > > Just 2c. > > > > I don't like using structs in those cases, I'd much rather have a flags > parameter, as flags can be checked for explicit zeros for future proofing, > while a struct cannot be checked for extra space on the end for future > fields added.
For lib/dispatcher library, I have don't have specific preference. So anything is fine for me. However, I thought of understanding your rationale for arguments vs structure(Looks like more of vi vs emac discussion) for _my understanding_. In my view, # Use flags for setting up to express specific behavior, not as inputting a lot of input parameters. # Do we need to check extra space if struct have reserved fields and having init() functions for filling default > > Furthermore, if we need to add new parameters to the create function, I > actually believe it is better to add them as explicit parameters rather > than new fields to the struct. Struct fields can be missed by a user just > recompiling, while new function parameters will be flagged by the compiler I would see this as on the positive side, when - Same code base needs to support multiple DPDK versions. - A lot of times, API consumer may need only _default_ values. Like local_cache value in mempool_create API. So struct with _init() get required values in easy way. My views are based mostly used existing rte_mempool_create() APIs. For some reason, I don't like this scheme. struct rte_mempool * rte_mempool_create(const char *name, unsigned n, unsigned elt_size, unsigned cache_size, unsigned private_data_size, rte_mempool_ctor_t *mp_init, void *mp_init_arg, rte_mempool_obj_cb_t *obj_init, void *obj_init_arg, int socket_id, unsigned flags); > to make the user aware of the change. [There would be no change for ABI > compatibility as function versioning would be usable in both cases] Yes. But need to too much template code via VERSION_SYMBOL where structure scheme does not need. > > /Bruce