On Fri, May 27, 2016 at 10:52:42AM +0100, Hunt, David wrote: > > > On 5/24/2016 4:35 PM, Jerin Jacob wrote: > > On Thu, May 19, 2016 at 02:44:59PM +0100, David Hunt wrote: > > > + /* > > > + * Since we have 4 combinations of the SP/SC/MP/MC examine the flags to > > > + * set the correct index into the handler table. > > > + */ > > > + if (flags & (MEMPOOL_F_SP_PUT | MEMPOOL_F_SC_GET)) > > > + rte_mempool_set_handler(mp, "ring_sp_sc"); > > > + else if (flags & MEMPOOL_F_SP_PUT) > > > + rte_mempool_set_handler(mp, "ring_sp_mc"); > > > + else if (flags & MEMPOOL_F_SC_GET) > > > + rte_mempool_set_handler(mp, "ring_mp_sc"); > > > + else > > > + rte_mempool_set_handler(mp, "ring_mp_mc"); > > IMO, We should decouple the implementation specific flags of _a_ > > external pool manager implementation from the generic > > rte_mempool_create_empty > > function as going further when we introduce new flags for custom HW > > accelerated > > external pool manager then this common code will be bloated. > > These flags are only there to maintain backward compatibility for the > default handlers. I would not > envisage adding more flags to this, I would suggest just adding a new > handler using the new API calls. > So I would not see this code growing much in the future.
IMHO, For _each_ HW accelerated external pool manager we may need to introduce specific flag to tune to specific use cases.i.e MEMPOOL_F_* flags for this exiting pool manager implemented in SW. For instance, when we add a new HW external pool manager we may need to add MEMPOOL_MYHW_DONT_FREE_ON_SEND (just a random name) to achieve certain functionally. So I propose let "unsigned flags" in mempool create to be the opaque type and each external pool manager can define what it makes sense to that specific pool manager as there is NO other means to configure the pool manager. For instance, on HW accelerated pool manager, the flag MEMPOOL_F_SP_PUT may not make much sense as it can work with MP without any additional settings in HW. So instead of adding these checks in common code, IMO, lets move this to a pool manager specific "function pointer" function and invoke the function pointer from generic mempool create function. What do you think? Jerin