Re: [dpdk-dev] [PATCH 1/2] mbuf: update default Mempool ops with HW active pool

[Hemant Agrawal <hemant.agrawal@nxp.com>]

Hi Olivier,

On 12/22/2017 8:29 PM, Olivier MATZ wrote:
> On Mon, Dec 18, 2017 at 03:06:21PM +0530, Hemant Agrawal wrote:
>> On 12/18/2017 2:25 PM, Jerin Jacob wrote:
>>> -----Original Message-----
>>>> Date: Fri, 15 Dec 2017 15:54:42 +0530
>>>> From: Hemant Agrawal <hemant.agrawal@nxp.com>
>>>> To: olivier.matz@6wind.com, santosh.shukla@caviumnetworks.com
>>>> CC: dev@dpdk.org
>>>> Subject: [dpdk-dev] [PATCH 1/2] mbuf: update default Mempool ops with HW
>>>>  active pool
>>>> X-Mailer: git-send-email 2.7.4
>>>>
>>>> With this patch the specific HW mempool are no longer required to be
>>>> specified in the config file at compile. A default active hw mempool
>>>> can be detected dynamically and published to default mempools ops
>>>> config at run time. Only one type of HW mempool can be active default.
>>>
>>> For me, it looks very reasonable approach as it caters the basic use
>>> case without any change in the application nor the additional(--mbuf-pool-ops-name)
>>> EAL command line scheme to select different mempool ops.
>>> Though, this option will not enough cater all the use case. I think, we can have
>>> three options and the following order of precedence to select the mempool ops
>>>
>>> 1) This patch(update active mempool based on the device probe())
>>> 2) Selection of mempool ops though --mbuf-pool-ops-name= EAL commandline argument.
>>> Which can overridden the scheme(1)
>>> 3) More sophisticated mempool section based on
>>> a) The ethdev PMD capability exposed through existing rte_eth_dev_pool_ops_supported()
>>> b) Add mempool ops option in rte_pktmbuf_pool_create()
>>> http://dpdk.org/ml/archives/dev/2017-December/083985.html
>>> c) Use (a) and (b) to select the update the mempool ops with
>>> some "weight" based algorithm like
>>> http://dpdk.org/dev/patchwork/patch/32245/
>>>
>>
>> Yes! We need more options to fine tune control over the mempool uses,
>> specially when dealing with HW mempools.
>>
>> Once the above mentioned mechanisms will be in place, it will be much easier
>> and flexible.
>
> I'm inline with this description. It would be great if the same binary can work
> on different platforms without configuration.
>
> I just feel it's a bit messy to have:
>
> - rte_eal_mbuf_default_mempool_ops() in eal API
>   return user-selected ops if any, or compile-time default
>
> - rte_pktmbuf_active_mempool_ops() in mbuf API
>   return platform ops except if a selected user ops != compile default
>
> Thomas suggested somewhere (but I don't remember in which thread) to have
> rte_eal_mbuf_default_mempool_ops() in mbuf code, and I think he was right.
>

The idea is good. It will break ABI, but we can move around in 
systematic way.

> I think the whole mbuf pool ops selection mechanism should be at the
> same place. I could be in a specific file of librte_mbuf.
>
> The API could be:
> - get compile time default ops
We can get them from "RTE_MBUF_DEFAULT_MEMPOOL_OPS"
or "
const char *rte_get_mbuf_config_mempool_ops(void)

> - get/set platform ops (NULL if none)

const char *rte_get_mbuf_platform_mempool_ops(void);
int rte_set_mbuf_platform_mempool_ops(const char *ops_name);

> - get/set user ops (NULL if none)
internal_config will only store the command line argument or NULL.

rename : rte_eal_mbuf_default_mempool_ops(void)
to  const char *rte_get_mbuf_user_mempool_ops(void)

> - get preferred ops from currently configured PMD
>

The following proposal is updating the mempool_ops name in 
internal_config, I thing that is not the best solution. We shall not 
update the internal config.

eal: add API to set default mbuf mempool ops
http://dpdk.org/ml/archives/dev/2017-December/083849.html
rte_eth_dev_get_preferred_pool_name()

> - get best ops: return user, or pmd-prefered, or platform, or default.
>

pktmbuf_pool_create is currently calling, *rte_eal_mbuf_default_mempool_ops*

This should be changed to call *rte_get_mbuf_best_mempool_ops* in the 
following order
	1. rte_get_mbuf_user_mempool_ops
         2. rte_eth_dev_get_preferred_pool_name (whenever this API comes)
         3. rte_get_mbuf_platform_mempool_ops
         4. rte_get_mbuf_config_mempool_ops

> rte_pktmbuf_pool_create() will use "get best ops" if no ops (NULL) is
> passed as argument.
>

However, we shall still provide a additional API, 
*rte_pktmbuf_pool_create_specific* if user still wants fine control or 
don't want to use the default best logic.