[dpdk-dev,FIX-OPTION-1] mbuf: fix the logic of user mempool ops API
Checks
Commit Message
From: Nipun Gupta <nipun.gupta@nxp.com>
The existing rte_eal_mbuf_default mempool ops can return the compile time
default ops name if the user has not provided command line inputs for
mempool ops name. It will break the logic of best mempool ops as it will
never return platform hw mempool ops.
This patch introduces a new API to just return the user mempool ops only.
Fixes: 8b0f7f434132 ("mbuf: maintain user and compile time mempool ops name")
Signed-off-by: Nipun Gupta <nipun.gupta@nxp.com>
---
lib/librte_eal/bsdapp/eal/eal.c | 7 +++++++
lib/librte_eal/common/include/rte_eal.h | 12 ++++++++++++
lib/librte_eal/linuxapp/eal/eal.c | 7 +++++++
lib/librte_eal/rte_eal_version.map | 1 +
lib/librte_mbuf/rte_mbuf_pool_ops.c | 2 +-
5 files changed, 28 insertions(+), 1 deletion(-)
Comments
On Fri, Feb 02, 2018 at 01:33:01PM +0530, Hemant Agrawal wrote:
> From: Nipun Gupta <nipun.gupta@nxp.com>
>
> The existing rte_eal_mbuf_default mempool ops can return the compile time
> default ops name if the user has not provided command line inputs for
> mempool ops name. It will break the logic of best mempool ops as it will
> never return platform hw mempool ops.
>
> This patch introduces a new API to just return the user mempool ops only.
>
> Fixes: 8b0f7f434132 ("mbuf: maintain user and compile time mempool ops name")
>
> Signed-off-by: Nipun Gupta <nipun.gupta@nxp.com>
This option is fine for me. I think we may also consider deprecating
rte_eal_mbuf_default_mempool_ops(), as it is done in option 2.
Acked-by: Olivier Matz <olivier.matz@6wind.com>
Hi Olivier,
> On Fri, Feb 02, 2018 at 01:33:01PM +0530, Hemant Agrawal wrote:
> > From: Nipun Gupta <nipun.gupta@nxp.com>
> >
> > The existing rte_eal_mbuf_default mempool ops can return the compile
> > time default ops name if the user has not provided command line inputs
> > for mempool ops name. It will break the logic of best mempool ops as
> > it will never return platform hw mempool ops.
> >
> > This patch introduces a new API to just return the user mempool ops only.
> >
> > Fixes: 8b0f7f434132 ("mbuf: maintain user and compile time mempool ops
> > name")
> >
> > Signed-off-by: Nipun Gupta <nipun.gupta@nxp.com>
>
> This option is fine for me. I think we may also consider deprecating
> rte_eal_mbuf_default_mempool_ops(), as it is done in option 2.
[Hemant] Ok. Please also ack following. I will update patchwork for remaining patches accordingly.
[PATCH v2] doc: remove eal API for default mempool ops name
> Acked-by: Olivier Matz <olivier.matz@6wind.com>
Thanks
Hemant
Hi Hemant,
On Sunday 04 February 2018 11:52 AM, Shukla, Santosh wrote:
> The existing rte_eal_mbuf_default mempool ops can return the compile time
> default ops name if the user has not provided command line inputs for
> mempool ops name. It will break the logic of best mempool ops as it will
> never return platform hw mempool ops.
>
> This patch introduces a new API to just return the user mempool ops only.
>
> Fixes: 8b0f7f434132 ("mbuf: maintain user and compile time mempool ops name")
>
> Signed-off-by: Nipun Gupta <nipun.gupta@nxp.com>
> ---
This patch introduces regression for octeontx platform.
Fails with:
---------
testpmd: create a new mbuf pool <mbuf_pool_socket_0>: n=163456, size=2176, socket=0
testpmd: preferred mempool ops selected: dpaa
dpaa_mbuf_create_pool(): bman_new_pool() failed
EAL: Error - exiting with code: 1
----------
Command:
./testpmd -c 0xe00000 -n 4 -- --portmask 0x1 --nb-cores=2 --port-topology=loop --rxq=2 --txq=2 --rss-ip --no-flush-rx
-------------
It's because rte_dpaa_bus_probe() sets platform mempool ops
i.e. rte_mbuf_set_platform_mempool_ops(DPAA_MEMPOOL_OPS_NAME);
therefore `rte_mbuf_best_mempool_ops` returns with `dpaa` ops which
is incorrect mempool_ops choice for octeontx platform.
We can fix such regression with following ways:
# option 1) unset LIBRTE_DPAA_MEMPOOL=y from common_armv8a_linuxapp config and
set at dpaa/dpaa2 specific configs
commit: 1ee9569576f6 ("config: enable dpaaX drivers for generic ARMv8")
# option 2) Don;t set platform mempool ops in dpaa bus layer instead defer it
to driver layer.
Thanks.
Thanks we also identified it. The fix is on the way.
Sent from my Android phone using TouchDown (www.symantec.com)
-----Original Message-----
From: santosh [santosh.shukla@caviumnetworks.com]
Received: Sunday, 04 Feb 2018, 12:05PM
To: Hemant Agrawal [hemant.agrawal@nxp.com]; dev [dev@dpdk.org]
CC: Olivier MATZ [olivier.matz@6wind.com]; Jerin Jacob [jerin.jacob@caviumnetworks.com]; Nipun Gupta [nipun.gupta@nxp.com]
Subject: Re: Fw: [PATCH FIX-OPTION-1] mbuf: fix the logic of user mempool ops API
Hi Hemant,
On Sunday 04 February 2018 11:52 AM, Shukla, Santosh wrote:
> The existing rte_eal_mbuf_default mempool ops can return the compile time
> default ops name if the user has not provided command line inputs for
> mempool ops name. It will break the logic of best mempool ops as it will
> never return platform hw mempool ops.
>
> This patch introduces a new API to just return the user mempool ops only.
>
> Fixes: 8b0f7f434132 ("mbuf: maintain user and compile time mempool ops name")
>
> Signed-off-by: Nipun Gupta <nipun.gupta@nxp.com>
> ---
This patch introduces regression for octeontx platform.
Fails with:
---------
testpmd: create a new mbuf pool <mbuf_pool_socket_0>: n=163456, size=2176, socket=0
testpmd: preferred mempool ops selected: dpaa
dpaa_mbuf_create_pool(): bman_new_pool() failed
EAL: Error - exiting with code: 1
----------
Command:
./testpmd -c 0xe00000 -n 4 -- --portmask 0x1 --nb-cores=2 --port-topology=loop --rxq=2 --txq=2 --rss-ip --no-flush-rx
-------------
It's because rte_dpaa_bus_probe() sets platform mempool ops
i.e. rte_mbuf_set_platform_mempool_ops(DPAA_MEMPOOL_OPS_NAME);
therefore `rte_mbuf_best_mempool_ops` returns with `dpaa` ops which
is incorrect mempool_ops choice for octeontx platform.
We can fix such regression with following ways:
# option 1) unset LIBRTE_DPAA_MEMPOOL=y from common_armv8a_linuxapp config and
set at dpaa/dpaa2 specific configs
commit: 1ee9569576f6 ("config: enable dpaaX drivers for generic ARMv8")
# option 2) Don;t set platform mempool ops in dpaa bus layer instead defer it
to driver layer.
Thanks.
02/02/2018 14:40, Olivier Matz:
> On Fri, Feb 02, 2018 at 01:33:01PM +0530, Hemant Agrawal wrote:
> > From: Nipun Gupta <nipun.gupta@nxp.com>
> >
> > The existing rte_eal_mbuf_default mempool ops can return the compile time
> > default ops name if the user has not provided command line inputs for
> > mempool ops name. It will break the logic of best mempool ops as it will
> > never return platform hw mempool ops.
> >
> > This patch introduces a new API to just return the user mempool ops only.
> >
> > Fixes: 8b0f7f434132 ("mbuf: maintain user and compile time mempool ops name")
> >
> > Signed-off-by: Nipun Gupta <nipun.gupta@nxp.com>
>
> This option is fine for me. I think we may also consider deprecating
> rte_eal_mbuf_default_mempool_ops(), as it is done in option 2.
>
> Acked-by: Olivier Matz <olivier.matz@6wind.com>
Applied, thanks
@@ -82,6 +82,13 @@ struct internal_config internal_config;
/* used by rte_rdtsc() */
int rte_cycles_vmware_tsc_map;
+/* Return user provided mbuf pool ops name */
+const char * __rte_experimental
+rte_eal_mbuf_user_pool_ops(void)
+{
+ return internal_config.user_mbuf_pool_ops_name;
+}
+
/* Return mbuf pool ops name */
const char *
rte_eal_mbuf_default_mempool_ops(void)
@@ -452,6 +452,18 @@ static inline int rte_gettid(void)
enum rte_iova_mode rte_eal_iova_mode(void);
/**
+ * @warning
+ * @b EXPERIMENTAL: this API may change without prior notice
+ *
+ * Get user provided pool ops name for mbuf
+ *
+ * @return
+ * returns user provided pool ops name.
+ */
+const char * __rte_experimental
+rte_eal_mbuf_user_pool_ops(void);
+
+/**
* Get default pool ops name for mbuf
*
* @return
@@ -92,6 +92,13 @@ struct internal_config internal_config;
/* used by rte_rdtsc() */
int rte_cycles_vmware_tsc_map;
+/* Return user provided mbuf pool ops name */
+const char * __rte_experimental
+rte_eal_mbuf_user_pool_ops(void)
+{
+ return internal_config.user_mbuf_pool_ops_name;
+}
+
/* Return mbuf pool ops name */
const char *
rte_eal_mbuf_default_mempool_ops(void)
@@ -220,6 +220,7 @@ EXPERIMENTAL {
rte_eal_devargs_remove;
rte_eal_hotplug_add;
rte_eal_hotplug_remove;
+ rte_eal_mbuf_user_pool_ops;
rte_mp_action_register;
rte_mp_action_unregister;
rte_mp_sendmsg;
@@ -74,7 +74,7 @@ rte_mbuf_user_mempool_ops(void)
mz = rte_memzone_lookup("mbuf_user_pool_ops");
if (mz == NULL)
- return rte_eal_mbuf_default_mempool_ops();
+ return rte_eal_mbuf_user_pool_ops();
return mz->addr;
}