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

[Panu Matilainen]

On 03/09/2016 01:38 PM, Hunt, David wrote:
> 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.

The problem with such build options is that the feature is for all 
practical purposes unusable in a distro setting where DPDK is just 
another shared library used by multiple applications.

> 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)?

Yes, that's the way to do it. Thanks.

	- Panu -



> Thanks,
> David.
>
>
>
>
>