[dpdk-dev] [PATCH v3 3/4] mempool: allow rte_pktmbuf_pool_create switch between memool handlers

[Hunt, David]

Hi Panu,

On 3/9/2016 10:54 AM, Panu Matilainen wrote:
> On 03/09/2016 11:50 AM, David Hunt wrote:
>> If the user wants to have rte_pktmbuf_pool_create() use an external 
>> mempool
>> handler, they define RTE_MEMPOOL_HANDLER_NAME to be the name of the
>> mempool handler they wish to use, and change RTE_MEMPOOL_HANDLER_EXT 
>> to 'y'
>>
>> Signed-off-by: David Hunt <david.hunt at intel.com>
>> ---
>>   config/common_base         | 2 ++
>>   lib/librte_mbuf/rte_mbuf.c | 8 ++++++++
>>   2 files changed, 10 insertions(+)
>>
>> diff --git a/config/common_base b/config/common_base
>> index 1af28c8..9d70cf4 100644
>> --- a/config/common_base
>> +++ b/config/common_base
>> @@ -350,6 +350,8 @@ CONFIG_RTE_RING_PAUSE_REP_COUNT=0
>>   CONFIG_RTE_LIBRTE_MEMPOOL=y
>>   CONFIG_RTE_MEMPOOL_CACHE_MAX_SIZE=512
>>   CONFIG_RTE_LIBRTE_MEMPOOL_DEBUG=n
>> +CONFIG_RTE_MEMPOOL_HANDLER_EXT=n
>> +CONFIG_RTE_MEMPOOL_HANDLER_NAME="custom_handler"
>>
>>   #
>>   # Compile librte_mbuf
>> diff --git a/lib/librte_mbuf/rte_mbuf.c b/lib/librte_mbuf/rte_mbuf.c
>> index c18b438..42b0cd1 100644
>> --- a/lib/librte_mbuf/rte_mbuf.c
>> +++ b/lib/librte_mbuf/rte_mbuf.c
>> @@ -167,10 +167,18 @@ rte_pktmbuf_pool_create(const char *name, 
>> unsigned n,
>>       mbp_priv.mbuf_data_room_size = data_room_size;
>>       mbp_priv.mbuf_priv_size = priv_size;
>>
>> +#ifdef RTE_MEMPOOL_HANDLER_EXT
>> +    return rte_mempool_create_ext(name, n, elt_size,
>> +        cache_size, sizeof(struct rte_pktmbuf_pool_private),
>> +        rte_pktmbuf_pool_init, &mbp_priv, rte_pktmbuf_init, NULL,
>> +        socket_id, 0,
>> +        RTE_MEMPOOL_HANDLER_NAME);
>> +#else
>>       return rte_mempool_create(name, n, elt_size,
>>           cache_size, sizeof(struct rte_pktmbuf_pool_private),
>>           rte_pktmbuf_pool_init, &mbp_priv, rte_pktmbuf_init, NULL,
>>           socket_id, 0);
>> +#endif
>>   }
>>
>>   /* do some sanity checks on a mbuf: panic if it fails */
>>
>
> This kind of thing really has to be run-time configurable, not a 
> library build-time option.
>
>     - Panu -

Interesting point. I was attempting to minimise the amount of 
application code changes.
Would you prefer if I took out that change, and added a new
rte_pktmbuf_pool_create_ext() function which tool an extra parameter as 
the mempool handler name to use?

/* helper to create a mbuf pool using external mempool handler */
struct rte_mempool *
rte_pktmbuf_pool_create_ext(const char *name, unsigned n,
     unsigned cache_size, uint16_t priv_size, uint16_t data_room_size,
     int socket_id,  const char *handler_name)

That way we could leave the old rte_pktmbuf_pool_create() exactly as it 
is, and any apps that wanted to use an
external handler could call rte_pktmbuf_pool_create_ext()
I could do this easily enough for v4 (which I hope to get out later today)?

Thanks,
David.