[dpdk-dev] [PATCH 1/2] mbuf: update default Mempool ops with HW active pool
Olivier MATZ
olivier.matz at 6wind.com
Fri Dec 22 15:41:51 CET 2017
Hi,
On Fri, Dec 15, 2017 at 03:54:42PM +0530, Hemant Agrawal wrote:
> 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.
>
> Signed-off-by: Hemant Agrawal <hemant.agrawal at nxp.com>
> ---
> lib/librte_mbuf/rte_mbuf.c | 33 ++++++++++++++++++++++++++++++++-
> lib/librte_mbuf/rte_mbuf.h | 13 +++++++++++++
> 2 files changed, 45 insertions(+), 1 deletion(-)
>
> diff --git a/lib/librte_mbuf/rte_mbuf.c b/lib/librte_mbuf/rte_mbuf.c
> index 7543662..e074afa 100644
> --- a/lib/librte_mbuf/rte_mbuf.c
> +++ b/lib/librte_mbuf/rte_mbuf.c
> @@ -148,6 +148,37 @@ rte_pktmbuf_init(struct rte_mempool *mp,
> m->next = NULL;
> }
>
> +static const char *active_mbuf_pool_ops_name;
> +
> +int
> +rte_pktmbuf_reg_active_mempool_ops(const char *ops_name)
I think active_mempool is not the best name: it is not always active
if the user forces another one.
Since there is only one pool like this, would "platform_mempool" be a
better name?
For naming, I suggest "pktmbuf" can be "mbuf", it's shorter and there is
no need anymore to differentiate with ctrlmbuf, because ctrlmbuf will be
removed soon. I also think "register" is clearer than "reg". So, what
about rte_mbuf_register_platform_mempool_ops()?
> +{
> + if (active_mbuf_pool_ops_name == NULL) {
> + active_mbuf_pool_ops_name = ops_name;
> + return 0;
> + }
> + RTE_LOG(ERR, MBUF,
> + "%s is already registered as active pktmbuf pool ops\n",
> + active_mbuf_pool_ops_name);
> + return -EACCES;
> +}
> +
> +/* Return mbuf pool ops name */
> +static const char *
> +rte_pktmbuf_active_mempool_ops(void)
> +{
> + const char *default_ops = rte_eal_mbuf_default_mempool_ops();
> +
> + /* If mbuf default ops is same as compile time default
> + * Just to be sure that no one has updated it by other means.
> + */
> + if ((strcmp(default_ops, RTE_MBUF_DEFAULT_MEMPOOL_OPS) == 0) &&
> + (active_mbuf_pool_ops_name != NULL))
> + return active_mbuf_pool_ops_name;
> + else
> + return default_ops;
> +}
The name of this function is confusing because it does not really return
the active mempool. If the user selected a pool with
--mbuf-pool-ops-name, it is returned...
...except if --mbuf-pool-ops-name=<name of default ops> was passed,
which I think is also very confusing.
More information about the dev
mailing list