Re: [dpdk-dev] [PATCH v8 1/3] mempool: support external mempool operations

[Jerin Jacob <jerin.jacob@caviumnetworks.com>]

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.
> 
> 
> 
> 
> 
> 
> 
> 
> 
>