[dpdk-dev] [RFC 0/7] changing mbuf pool handler

Hunt, David david.hunt at intel.com
Wed Oct 5 15:15:38 CEST 2016



On 5/10/2016 12:49 PM, Hemant Agrawal wrote:
> Hi Olivier,
>
>> -----Original Message-----
>> From: Hunt, David [mailto:david.hunt at intel.com]
>> Hi Olivier,
>>
>>
>> On 3/10/2016 4:49 PM, Olivier Matz wrote:
>>> Hi Hemant,
>>>
>>> Thank you for your feedback.
>>>
>>> On 09/22/2016 01:52 PM, Hemant Agrawal wrote:
>>>> Hi Olivier
>>>>
>>>> On 9/19/2016 7:12 PM, Olivier Matz wrote:
>>>>> Hello,
>>>>>
>>>>> Following discussion from [1] ("usages issue with external mempool").
>>>>>
>>>>> This is a tentative to make the mempool_ops feature introduced by
>>>>> David Hunt [2] more widely used by applications.
>>>>>
>>>>> It applies on top of a minor fix in mbuf lib [3].
>>>>>
>>>>> To sumarize the needs (please comment if I did not got it properly):
>>>>>
>>>>> - new hw-assisted mempool handlers will soon be introduced
>>>>> - to make use of it, the new mempool API [4]
>> (rte_mempool_create_empty,
>>>>>     rte_mempool_populate, ...) has to be used
>>>>> - the legacy mempool API (rte_mempool_create) does not allow to
>> change
>>>>>     the mempool ops. The default is "ring_<s|m>p_<s|m>c" depending on
>>>>>     flags.
>>>>> - the mbuf helper (rte_pktmbuf_pool_create) does not allow to change
>>>>>     them either, and the default is RTE_MBUF_DEFAULT_MEMPOOL_OPS
>>>>>     ("ring_mp_mc")
>>>>> - today, most (if not all) applications and examples use either
>>>>>     rte_pktmbuf_pool_create or rte_mempool_create to create the mbuf
>>>>>     pool, making it difficult to take advantage of this feature with
>>>>>     existing apps.
>>>>>
>>>>> My initial idea was to deprecate both rte_pktmbuf_pool_create() and
>>>>> rte_mempool_create(), forcing the applications to use the new API,
>>>>> which is more flexible. But after digging a bit, it appeared that
>>>>> rte_mempool_create() is widely used, and not only for mbufs.
>>>>> Deprecating it would have a big impact on applications, and
>>>>> replacing it with the new API would be overkill in many use-cases.
>>>> I agree with the proposal.
>>>>
>>>>> So I finally tried the following approach (inspired from a
>>>>> suggestion Jerin [5]):
>>>>>
>>>>> - add a new mempool_ops parameter to rte_pktmbuf_pool_create().
>> This
>>>>>     unfortunatelly breaks the API, but I implemented an ABI compat layer.
>>>>>     If the patch is accepted, we could discuss how to announce/schedule
>>>>>     the API change.
>>>>> - update the applications and documentation to prefer
>>>>>     rte_pktmbuf_pool_create() as much as possible
>>>>> - update most used examples (testpmd, l2fwd, l3fwd) to add a new
>> command
>>>>>     line argument to select the mempool handler
>>>>>
>>>>> I hope the external applications would then switch to
>>>>> rte_pktmbuf_pool_create(), since it supports most of the use-cases
>>>>> (even priv_size != 0, since we can call rte_mempool_obj_iter() after) .
>>>>>
>>>> I will still prefer if you can add the "rte_mempool_obj_cb_t *obj_cb,
>>>> void *obj_cb_arg" into "rte_pktmbuf_pool_create". This single
>>>> consolidated wrapper will almost make it certain that applications
>>>> will not try to use rte_mempool_create for packet buffers.
>>> The patch changes the example applications. I'm not sure I understand
>>> why adding these arguments would force application to not use
>>> rte_mempool_create() for packet buffers. Do you have a application in
>> mind?
>>> For the mempool_ops parameter, we must pass it at init because we need
>>> to know the mempool handler before populating the pool. For object
>>> initialization, it can be done after, so I thought it was better to
>>> reduce the number of arguments to avoid to fall in the
>>> mempool_create() syndrom :)
>> I also agree with the proposal. Looks cleaner.
>>
>> I would lean to the side of keeping the parameters to the minimum, i.e.
>> not adding *obj_cb and *obj_cb_arg into rte_pktmbuf_pool_create.
>> Developers always have the option of going with rte_mempool_create if they
>> need more fine-grained control.
> [Hemant] The implementations with hw offloaded mempools don't want developer using *rte_mempool_create* for packet buffer pools.
> This API does not work for hw offloaded mempool.
>
> Also, *rte_mempool_create_empty* - may not be convenient for many application, as it requires calling  4+ APIs.
>
> Olivier is not in favor of deprecating the *rte_mempool_create*.   I agree with concerns raised by him.
>
> Essentially, I was suggesting to upgrade * rte_pktmbuf_pool_create* to be like *rte_mempool_create*  for packet buffers exclusively.
>
> This will provide a clear segregation for API usages w.r.t the packet buffer pool vs all other type of mempools.

Yes, it does sound like we need those extra parameters on 
rte_pktmbuf_pool_create.

Regards,
Dave.


More information about the dev mailing list