Re: [dpdk-dev] [PATCH v2] doc: announce ABI change for pktmbuf pool create API

["Wiles, Keith" <keith.wiles@intel.com>]

> On Dec 18, 2017, at 11:40 PM, Hemant Agrawal <hemant.agrawal@nxp.com> wrote:
> 
> On 12/18/2017 7:21 PM, Wiles, Keith wrote:
>> 
>> 
>>> On Dec 15, 2017, at 4:41 AM, Hemant Agrawal <hemant.agrawal@nxp.com> wrote:
>>> 
>>> Introduce a new argument ops_name in rte_mempool_set_ops_byname
>>> for allowing the application to optionally specify the mempool ops.
>>> 
>>> Signed-off-by: Hemant Agrawal <hemant.agrawal@nxp.com>
>>> ---
>>> v2: fix checkpatch error
>>> 
>>> doc/guides/rel_notes/deprecation.rst | 3 +++
>>> 1 file changed, 3 insertions(+)
>>> 
>>> diff --git a/doc/guides/rel_notes/deprecation.rst b/doc/guides/rel_notes/deprecation.rst
>>> index 13e8543..968ca14 100644
>>> --- a/doc/guides/rel_notes/deprecation.rst
>>> +++ b/doc/guides/rel_notes/deprecation.rst
>>> @@ -53,3 +53,6 @@ Deprecation Notices
>>> 
>>> * librte_meter: The API will change to accommodate configuration profiles.
>>>  Most of the API functions will have an additional opaque parameter.
>>> +
>>> +* librte_mbuf: a new optional parameter for representing name of mempool_ops
>>> +  will be added to the API ``rte_pktmbuf_pool_create``.
>> 
>> 
>> Sorry, for the late response I was on vacation.
>> 
>> My question is why do we need to change rte_pktmbuf_pool_create ABI yet again, why could we not add a new API to just set the name of the pool after it is created. This would allow all current applications to work without any ABI breakage and only require adding a new API call for anyone that wants the name. The rte_pktmbuf_pool_create() routine could assign a default name or some incrementing style name as the default. e.g. ‘pktmbuf_%d’ with a static incrementing variable or whatever you like.
>> 
>> Sorry if this was asked and answered before.
>> 
> 
> I understand the concerns.
> 
> However, the new API to just set the name will not work post create.
> rte_pktmbuf_pool_create is a wrapper API, which complete the mempool configuration on the basis default mempool_ops.

Really can not add the name after the fact, I have not looked, but it seem very odd we can not use the mempool pointer and update the ops_name. What is stopping this from working?

> 
> The idea proposed is to create pktmbuf pool from a specific mempool (ops_name).
> 
> We can leave "rte_pktmbuf_pool_create" as it is.
> and create another similar API with e.g. "rte_pktmbuf_pool_create_specific", which will also take ops_name as argument.  (We can combine the internal implementation with NULL ops_name for rte_pktmbuf_pool_create.)

I would accept this approach over the original patch to change the name of a commonly used API.
> 
> This way we will have flexibility for the applications looking for pktmbufs from a specific mempool.
> 
> any thoughts?
> 
> Hemant
> 
>>> --
>>> 2.7.4
>>> 
>> 
>> Regards,
>> Keith

Regards,
Keith