Re: usages issue with external mempool

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

On Thu, Jul 28, 2016 at 10:32:44AM +0200, Olivier MATZ wrote:

Hi Olivier,

> Hi Hemant, Jerin,
> 
> On 07/27/2016 11:51 AM, Jerin Jacob wrote:
> > On Tue, Jul 26, 2016 at 10:11:13AM +0000, Hemant Agrawal wrote:
> 
> > 
> > I agree, To me, this is very bad. I have raised this concern earlier
> > also
> > 
> > Since applications like OVS goes through "rte_mempool_create" for
> > even packet buffer pool creation. IMO it make senses to extend
> > "rte_mempool_create" to take one more argument to provide external pool
> > handler name(NULL for default). I don't see any valid technical reason
> > to treat external pool handler based mempool creation API different
> > from default handler.
> 
> I disagree that changing from one function do_many_stuff(11 args) to several
> do_one_stuff(few args) functions is a regression.
> 
> I don't feel that having a new function with 12 args solves anything.
> What is the problem of having 20 lines of code for initializing a mbuf pool?
> The new API gives more flexibility, and it allow an application to define
> its own function if the default one cannot be used.
>

The problem I have in this scheme is that there is no visibility on converging part of
external vs default based pool manager Creation/usage. Can we deprecate the
original (11 args) API have ONLY ONE WAY to create/use the mempool
irrespective of external or internal ?.So in application perspective it is
converged and its matter selecting the handler name.
I believe deprecation of (11 args) API is the only that can happen ?
Any other thoughts on converging it?

for an example,
look at cryptodev, selection of SW based virtual or HW based physical
device section and usage are identical in APPLICATION perspective.


 

 
> I think that the name of the functions pretty well defines what they do:
> 
>   rte_mempool_create_empty(): create an empty mempool
>   rte_mempool_set_ops_byname(): set the mempool handler from its name
>   rte_pktmbuf_pool_init(): initialize the mempool as a packet pool
>   rte_mempool_populate_default(): populate the pool with objects
>   rte_mempool_obj_iter(): call a function for each object
> 

I agree.New APIs are great. See above


Jerin