On Thu, Jun 09, 2016 at 02:18:57PM +0100, Hunt, David wrote: > > > > > > As I mentioned earlier, My take is not to create the separate API's for > > > > external mempool handlers.In my view, It's same, just that sepreate > > > > mempool handler through function pointers. > > > > > > > > To keep the backward compatibility, I think we can extend the flags > > > > in rte_mempool_create and have a single API external/internal pool > > > > creation(makes easy for existing applications too, add a just mempool > > > > flag command line argument to existing applications to choose the > > > > mempool handler) > > > May be I am interpreting it wrong, but, are you suggesting a single > > > mempool handler for all buffer/packet needs of an application (passed as > > > command line argument)? > > > That would be inefficient especially for cases where pool is backed by a > > > hardware. The application wouldn't want its generic buffers to consume > > > hardware resource which would be better used for packets. > > It may vary from platform to platform or particular use case. For instance, > > the HW external pool manager for generic buffers may scale better than SW > > multi > > producers/multi-consumer implementation when the number of cores > N > > (as no locking involved in enqueue/dequeue(again it is depended on > > specific HW implementation)) > > > > I thought their no harm in selecting the external pool handlers > > in root level itself(rte_mempool_create) as by default it is > > SW MP/MC and it just an option to override if the application wants it. > > > > Jerin > > > > > So, how about we go with the following, based on Shreyansh's suggestion: > > 1. Add in #define MEMPOOL_F_EMM_ALLOC 0x0010 (EMM for External Mempool > Manager) > > 2. Check this bit in rte_mempool_create() just before the other bits are > checked (by the way, the flags check has been moved to rte_mempool_create(), > as per an earlier patchset, but was inadvertantly reverted) > > /* > * First check to see if we use the config'd mempool hander. > * Then examine the combinations of SP/SC/MP/MC flags to > * set the correct index into the table of ops structs. > */ > if (flags & MEMPOOL_F_EMM_ALLOC) > rte_mempool_set_ops_byname(mp, RTE_MBUF_DEFAULT_MEMPOOL_OPS) > else (flags & (MEMPOOL_F_SP_PUT | MEMPOOL_F_SC_GET)) > rte_mempool_set_ops_byname(mp, "ring_sp_sc"); > else if (flags & MEMPOOL_F_SP_PUT) > rte_mempool_set_ops_byname(mp, "ring_sp_mc"); > else if (flags & MEMPOOL_F_SC_GET) > rte_mempool_set_ops_byname(mp, "ring_mp_sc"); > else > rte_mempool_set_ops_byname(mp, "ring_mp_mc"); > > 3. Modify rte_pktmbuf_pool_create to pass the bit to rte_mempool_create > > - sizeof(struct rte_pktmbuf_pool_private), socket_id, 0); > + sizeof(struct rte_pktmbuf_pool_private), socket_id, > + MEMPOOL_F_PKT_ALLOC); > > > This will allow legacy apps to use one external handler ( as defined > RTE_MBUF_DEFAULT_MEMPOOL_OPS) by adding the MEMPOOL_F_EMM_ALLOC bit to their > flags in the call to rte_mempool_create(). > > Of course, if an app wants to use more than one external handler, they can > call create_empty and set_ops_byname() for each mempool they create.
+1 Since rte_pktmbuf_pool_create does not take flag, I think this the only option left with for legacy apps. > > Regards, > Dave. > > > > > > > > > >