[dpdk-dev] [PATCH 1/2] Revert "eal: fix default mempool ops"

[dpdk-dev] [PATCH 1/2] Revert "eal: fix default mempool ops"

[Hemant Agrawal]

This reverts commit fe06cb6c54fe5ada299ebba40a382bee37c919f2.
---
 lib/librte_eal/bsdapp/eal/eal.c   | 3 ---
 lib/librte_eal/linuxapp/eal/eal.c | 3 ---
 2 files changed, 6 deletions(-)

diff --git a/lib/librte_eal/bsdapp/eal/eal.c b/lib/librte_eal/bsdapp/eal/eal.c
index 1622a41..b612af0 100644
--- a/lib/librte_eal/bsdapp/eal/eal.c
+++ b/lib/librte_eal/bsdapp/eal/eal.c
@@ -86,9 +86,6 @@ int rte_cycles_vmware_tsc_map;
 const char *
 rte_eal_mbuf_default_mempool_ops(void)
 {
-	if (internal_config.user_mbuf_pool_ops_name == NULL)
-		return RTE_MBUF_DEFAULT_MEMPOOL_OPS;
-
 	return internal_config.user_mbuf_pool_ops_name;
 }
 
diff --git a/lib/librte_eal/linuxapp/eal/eal.c b/lib/librte_eal/linuxapp/eal/eal.c
index 451fdaf..21024b4 100644
--- a/lib/librte_eal/linuxapp/eal/eal.c
+++ b/lib/librte_eal/linuxapp/eal/eal.c
@@ -96,9 +96,6 @@ int rte_cycles_vmware_tsc_map;
 const char *
 rte_eal_mbuf_default_mempool_ops(void)
 {
-	if (internal_config.user_mbuf_pool_ops_name == NULL)
-		return RTE_MBUF_DEFAULT_MEMPOOL_OPS;
-
 	return internal_config.user_mbuf_pool_ops_name;
 }
 
-- 
2.7.4

[dpdk-dev] [PATCH 2/2] test: use the best mempool ops API to get mbuf pool

[Hemant Agrawal]

This patch replaces the uses of default mempool ops API
with newly supported best_mempool_ops API.

Signed-off-by: Hemant Agrawal <hemant.agrawal@nxp.com>
---
 test/test/test_mempool.c      | 3 ++-
 test/test/test_mempool_perf.c | 5 +++--
 2 files changed, 5 insertions(+), 3 deletions(-)

diff --git a/test/test/test_mempool.c b/test/test/test_mempool.c
index f39df5a..63f921e 100644
--- a/test/test/test_mempool.c
+++ b/test/test/test_mempool.c
@@ -25,6 +25,7 @@
 #include <rte_mempool.h>
 #include <rte_spinlock.h>
 #include <rte_malloc.h>
+#include <rte_mbuf_pool_ops.h>
 
 #include "test.h"
 
@@ -485,7 +486,7 @@ test_mempool(void)
 	struct rte_mempool *mp_nocache = NULL;
 	struct rte_mempool *mp_stack = NULL;
 	struct rte_mempool *default_pool = NULL;
-	const char *default_pool_ops = rte_eal_mbuf_default_mempool_ops();
+	const char *default_pool_ops = rte_mbuf_best_mempool_ops();
 
 	rte_atomic32_init(&synchro);
 
diff --git a/test/test/test_mempool_perf.c b/test/test/test_mempool_perf.c
index 70de0b3..dfeefab 100644
--- a/test/test/test_mempool_perf.c
+++ b/test/test/test_mempool_perf.c
@@ -25,7 +25,7 @@
 #include <rte_mempool.h>
 #include <rte_spinlock.h>
 #include <rte_malloc.h>
-
+#include <rte_mbuf_pool_ops.h>
 #include "test.h"
 
 /*
@@ -308,7 +308,8 @@ test_mempool_perf(void)
 	if (mp_cache == NULL)
 		goto err;
 
-	default_pool_ops = rte_eal_mbuf_default_mempool_ops();
+	default_pool_ops = rte_mbuf_best_mempool_ops();
+
 	/* Create a mempool based on Default handler */
 	default_pool = rte_mempool_create_empty("default_pool",
 						MEMPOOL_SIZE,
-- 
2.7.4

Re: [dpdk-dev] [PATCH 1/2] Revert "eal: fix default mempool ops"

[Hemant Agrawal]

Hi Pavan,
	Your patch was breaking the design of the best_mempool_ops and the whole purpose of selection was getting lost. 
I guess you were trying to fix  test_mempool.  I have sent another patch, which fixes that and start using the best mempool ops API
instead of default mempool ops API.

Regards,
Hemant

> -----Original Message-----
> From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Hemant Agrawal
> Sent: Friday, February 02, 2018 1:17 AM
> To: olivier.matz@6wind.com; pbhagavatula@caviumnetworks.com
> Cc: thomas@monjalon.net; dev@dpdk.org
> Subject: [dpdk-dev] [PATCH 1/2] Revert "eal: fix default mempool ops"
> 
> This reverts commit fe06cb6c54fe5ada299ebba40a382bee37c919f2.
> ---
>  lib/librte_eal/bsdapp/eal/eal.c   | 3 ---
>  lib/librte_eal/linuxapp/eal/eal.c | 3 ---
>  2 files changed, 6 deletions(-)
> 
> diff --git a/lib/librte_eal/bsdapp/eal/eal.c b/lib/librte_eal/bsdapp/eal/eal.c
> index 1622a41..b612af0 100644
> --- a/lib/librte_eal/bsdapp/eal/eal.c
> +++ b/lib/librte_eal/bsdapp/eal/eal.c
> @@ -86,9 +86,6 @@ int rte_cycles_vmware_tsc_map;  const char *
>  rte_eal_mbuf_default_mempool_ops(void)
>  {
> -	if (internal_config.user_mbuf_pool_ops_name == NULL)
> -		return RTE_MBUF_DEFAULT_MEMPOOL_OPS;
> -
>  	return internal_config.user_mbuf_pool_ops_name;
>  }
> 
> diff --git a/lib/librte_eal/linuxapp/eal/eal.c b/lib/librte_eal/linuxapp/eal/eal.c
> index 451fdaf..21024b4 100644
> --- a/lib/librte_eal/linuxapp/eal/eal.c
> +++ b/lib/librte_eal/linuxapp/eal/eal.c
> @@ -96,9 +96,6 @@ int rte_cycles_vmware_tsc_map;  const char *
>  rte_eal_mbuf_default_mempool_ops(void)
>  {
> -	if (internal_config.user_mbuf_pool_ops_name == NULL)
> -		return RTE_MBUF_DEFAULT_MEMPOOL_OPS;
> -
>  	return internal_config.user_mbuf_pool_ops_name;
>  }
> 
> --
> 2.7.4

Re: [dpdk-dev] [PATCH 1/2] Revert "eal: fix default mempool ops"

[Pavan Nikhilesh]

Hi Hemanth,

	Currently, best_mempool_ops is broken because when
rte_mbuf_user_mempool_ops is invoked it is expected to returns 'NULL' through
internal_config.user_mbuf_pool_ops_name. IMO it is best to create a named
memzone ('mbuf_user_pool_ops') at the end of eal_init and copy mbuf-pool-ops
passed to eal.

`rte_eal_mbuf_default_mempool_ops` is not expected to return 'NULL' would doing
so break the ABI?.

---
/**
 * Get default pool ops name for mbuf
 *
 * @return
 *   returns default pool ops name.
 */
const char *
rte_eal_mbuf_default_mempool_ops(void);
---

IMO creating named mempool at the end of eal_init and changing
`rte_mbuf_user_mempool_ops` as below would be a better solution.

rte_mbuf_user_mempool_ops(void)
{
...
        mz = rte_memzone_lookup("mbuf_user_pool_ops");
        if (mz == NULL)
                return NULL;
	...
}

Thoughts?

Pavan.

On Thu, Feb 01, 2018 at 07:56:47PM +0000, Hemant Agrawal wrote:
> Hi Pavan,
> 	Your patch was breaking the design of the best_mempool_ops and the whole purpose of selection was getting lost.
> I guess you were trying to fix  test_mempool.  I have sent another patch, which fixes that and start using the best mempool ops API
> instead of default mempool ops API.
>
> Regards,
> Hemant
>
> > -----Original Message-----
> > From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Hemant Agrawal
> > Sent: Friday, February 02, 2018 1:17 AM
> > To: olivier.matz@6wind.com; pbhagavatula@caviumnetworks.com
> > Cc: thomas@monjalon.net; dev@dpdk.org
> > Subject: [dpdk-dev] [PATCH 1/2] Revert "eal: fix default mempool ops"
> >
> > This reverts commit fe06cb6c54fe5ada299ebba40a382bee37c919f2.
> > ---

Re: [dpdk-dev] [PATCH 1/2] Revert "eal: fix default mempool ops"

[Hemant Agrawal]

HI Pavan,
> 	Currently, best_mempool_ops is broken because when
> rte_mbuf_user_mempool_ops is invoked it is expected to returns 'NULL' through
> internal_config.user_mbuf_pool_ops_name. IMO it is best to create a named
> memzone ('mbuf_user_pool_ops') at the end of eal_init and copy mbuf-pool-ops
> passed to eal.
> 
> `rte_eal_mbuf_default_mempool_ops` is not expected to return 'NULL' would
> doing so break the ABI?.
> 
> ---
> /**
>  * Get default pool ops name for mbuf
>  *
>  * @return
>  *   returns default pool ops name.
>  */
> const char *
> rte_eal_mbuf_default_mempool_ops(void);
> ---
> 
> IMO creating named mempool at the end of eal_init and changing
> `rte_mbuf_user_mempool_ops` as below would be a better solution.
> 
> rte_mbuf_user_mempool_ops(void)
> {
> ...
>         mz = rte_memzone_lookup("mbuf_user_pool_ops");
>         if (mz == NULL)
>                 return NULL;
> 	...
> }
> 
> Thoughts?


[Hemant]  It seems reasonable. We can also deprecate the eal default mempool ops API . I will be sending patch shortly.
 
Unfortunately all NXP platforms are broken at the moment, so we need to get it fixed fast.

Hemant

> 
> Pavan.
> 
> On Thu, Feb 01, 2018 at 07:56:47PM +0000, Hemant Agrawal wrote:
> > Hi Pavan,
> > 	Your patch was breaking the design of the best_mempool_ops and the
> whole purpose of selection was getting lost.
> > I guess you were trying to fix  test_mempool.  I have sent another
> > patch, which fixes that and start using the best mempool ops API instead of
> default mempool ops API.
> >
> > Regards,
> > Hemant
> >
> > > -----Original Message-----
> > > From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Hemant Agrawal
> > > Sent: Friday, February 02, 2018 1:17 AM
> > > To: olivier.matz@6wind.com; pbhagavatula@caviumnetworks.com
> > > Cc: thomas@monjalon.net; dev@dpdk.org
> > > Subject: [dpdk-dev] [PATCH 1/2] Revert "eal: fix default mempool ops"
> > >
> > > This reverts commit fe06cb6c54fe5ada299ebba40a382bee37c919f2.
> > > ---

[dpdk-dev] [PATCH FIX-OPTION-2 1/2] eal: use named memzone to store user mempool ops name

[Hemant Agrawal]

The new mbuf pool ops name API uses the named memzone to store different
types of configured mempool ops name. It is better to also save the user
configured mempool ops name in named memzone. This way the best mempool
ops name can easily get the user configured mempool ops name for it's
decisions.

This will also avoid the need to maintain a eal api for default mempool
ops name. 

Signed-off-by: Hemant Agrawal <hemant.agrawal@nxp.com>
---
 lib/librte_eal/bsdapp/eal/Makefile         |  1 +
 lib/librte_eal/bsdapp/eal/eal.c            |  5 +++
 lib/librte_eal/common/eal_common_mempool.c | 50 ++++++++++++++++++++++++++++++
 lib/librte_eal/common/eal_private.h        | 12 +++++++
 lib/librte_eal/common/meson.build          |  1 +
 lib/librte_eal/linuxapp/eal/Makefile       |  1 +
 lib/librte_eal/linuxapp/eal/eal.c          |  5 +++
 7 files changed, 75 insertions(+)
 create mode 100644 lib/librte_eal/common/eal_common_mempool.c

diff --git a/lib/librte_eal/bsdapp/eal/Makefile b/lib/librte_eal/bsdapp/eal/Makefile
index dd455e6..f07dace 100644
--- a/lib/librte_eal/bsdapp/eal/Makefile
+++ b/lib/librte_eal/bsdapp/eal/Makefile
@@ -38,6 +38,7 @@ SRCS-$(CONFIG_RTE_EXEC_ENV_BSDAPP) += eal_alarm.c
 SRCS-$(CONFIG_RTE_EXEC_ENV_BSDAPP) += eal_common_lcore.c
 SRCS-$(CONFIG_RTE_EXEC_ENV_BSDAPP) += eal_common_timer.c
 SRCS-$(CONFIG_RTE_EXEC_ENV_BSDAPP) += eal_common_memzone.c
+SRCS-$(CONFIG_RTE_EXEC_ENV_BSDAPP) += eal_common_mempool.c
 SRCS-$(CONFIG_RTE_EXEC_ENV_BSDAPP) += eal_common_log.c
 SRCS-$(CONFIG_RTE_EXEC_ENV_BSDAPP) += eal_common_launch.c
 SRCS-$(CONFIG_RTE_EXEC_ENV_BSDAPP) += eal_common_memory.c
diff --git a/lib/librte_eal/bsdapp/eal/eal.c b/lib/librte_eal/bsdapp/eal/eal.c
index 1622a41..2c33a60 100644
--- a/lib/librte_eal/bsdapp/eal/eal.c
+++ b/lib/librte_eal/bsdapp/eal/eal.c
@@ -621,6 +621,11 @@ rte_eal_init(int argc, char **argv)
 		return -1;
 	}
 
+	if (rte_eal_mempool_ops_config() < 0) {
+		rte_eal_init_alert("Cannot config user Mempool Ops\n");
+		return -1;
+	}
+
 	eal_check_mem_on_local_socket();
 
 	eal_thread_init_master(rte_config.master_lcore);
diff --git a/lib/librte_eal/common/eal_common_mempool.c b/lib/librte_eal/common/eal_common_mempool.c
new file mode 100644
index 0000000..1d10a75
--- /dev/null
+++ b/lib/librte_eal/common/eal_common_mempool.c
@@ -0,0 +1,50 @@
+/* SPDX-License-Identifier: BSD-3-Clause
+ * Copyright 2018 NXP
+ */
+
+#include <stdio.h>
+#include <stdint.h>
+#include <stdlib.h>
+#include <stdarg.h>
+#include <unistd.h>
+#include <inttypes.h>
+#include <string.h>
+#include <sys/mman.h>
+#include <sys/queue.h>
+
+#include <rte_memory.h>
+#include <rte_eal.h>
+#include <rte_log.h>
+#include <rte_errno.h>
+#include <rte_memzone.h>
+
+#include "eal_private.h"
+#include "eal_internal_cfg.h"
+
+/* init mempool ops */
+int
+rte_eal_mempool_ops_config(void)
+{
+	RTE_LOG(DEBUG, EAL, "Configuring user mempool ops name...\n");
+
+	/* secondary processes don't need to initialise anything */
+	if (rte_eal_process_type() == RTE_PROC_SECONDARY)
+		return 0;
+
+	if (internal_config.user_mbuf_pool_ops_name) {
+		const struct rte_memzone *mz;
+
+		mz = rte_memzone_lookup("mbuf_user_pool_ops");
+		if (mz == NULL) {
+			mz = rte_memzone_reserve("mbuf_user_pool_ops",
+				32, SOCKET_ID_ANY, 0);
+			if (mz == NULL)
+				return -rte_errno;
+		}
+
+		strncpy(mz->addr, internal_config.user_mbuf_pool_ops_name,
+			strlen(internal_config.user_mbuf_pool_ops_name));
+	}
+
+	return 0;
+}
diff --git a/lib/librte_eal/common/eal_private.h b/lib/librte_eal/common/eal_private.h
index 0b28770..76beaff 100644
--- a/lib/librte_eal/common/eal_private.h
+++ b/lib/librte_eal/common/eal_private.h
@@ -205,4 +205,16 @@ struct rte_bus *rte_bus_find_by_device_name(const char *str);
 
 int rte_mp_channel_init(void);
 
+/**
+ * Mempool ops configuration
+ *
+ * This function is private to EAL.
+ *
+ * Set the user defined mempool ops in the named memzone area.
+ *
+ * @return
+ *   0 on success, negative on error
+ */
+int rte_eal_mempool_ops_config(void);
+
 #endif /* _EAL_PRIVATE_H_ */
diff --git a/lib/librte_eal/common/meson.build b/lib/librte_eal/common/meson.build
index 82b8910..3c6125b 100644
--- a/lib/librte_eal/common/meson.build
+++ b/lib/librte_eal/common/meson.build
@@ -16,6 +16,7 @@ common_sources = files(
 	'eal_common_lcore.c',
 	'eal_common_log.c',
 	'eal_common_memory.c',
+	'eal_common_mempool.c',
 	'eal_common_memzone.c',
 	'eal_common_options.c',
 	'eal_common_proc.c',
diff --git a/lib/librte_eal/linuxapp/eal/Makefile b/lib/librte_eal/linuxapp/eal/Makefile
index 7e5bbe8..83a08b0 100644
--- a/lib/librte_eal/linuxapp/eal/Makefile
+++ b/lib/librte_eal/linuxapp/eal/Makefile
@@ -46,6 +46,7 @@ SRCS-$(CONFIG_RTE_EXEC_ENV_LINUXAPP) += eal_alarm.c
 SRCS-$(CONFIG_RTE_EXEC_ENV_LINUXAPP) += eal_common_lcore.c
 SRCS-$(CONFIG_RTE_EXEC_ENV_LINUXAPP) += eal_common_timer.c
 SRCS-$(CONFIG_RTE_EXEC_ENV_LINUXAPP) += eal_common_memzone.c
+SRCS-$(CONFIG_RTE_EXEC_ENV_LINUXAPP) += eal_common_mempool.c
 SRCS-$(CONFIG_RTE_EXEC_ENV_LINUXAPP) += eal_common_log.c
 SRCS-$(CONFIG_RTE_EXEC_ENV_LINUXAPP) += eal_common_launch.c
 SRCS-$(CONFIG_RTE_EXEC_ENV_LINUXAPP) += eal_common_memory.c
diff --git a/lib/librte_eal/linuxapp/eal/eal.c b/lib/librte_eal/linuxapp/eal/eal.c
index 451fdaf..42a79d1 100644
--- a/lib/librte_eal/linuxapp/eal/eal.c
+++ b/lib/librte_eal/linuxapp/eal/eal.c
@@ -877,6 +877,11 @@ rte_eal_init(int argc, char **argv)
 		return -1;
 	}
 
+	if (rte_eal_mempool_ops_config() < 0) {
+		rte_eal_init_alert("Cannot config user Mempool Ops\n");
+		return -1;
+	}
+
 	eal_check_mem_on_local_socket();
 
 	eal_thread_init_master(rte_config.master_lcore);
-- 
2.7.4

[dpdk-dev] [PATCH FIX-OPTION-1] mbuf: fix the logic of user mempool ops API

[Hemant Agrawal]

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(-)

diff --git a/lib/librte_eal/bsdapp/eal/eal.c b/lib/librte_eal/bsdapp/eal/eal.c
index 1622a41..4eafcb5 100644
--- a/lib/librte_eal/bsdapp/eal/eal.c
+++ b/lib/librte_eal/bsdapp/eal/eal.c
@@ -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)
diff --git a/lib/librte_eal/common/include/rte_eal.h b/lib/librte_eal/common/include/rte_eal.h
index 08c6637..044474e 100644
--- a/lib/librte_eal/common/include/rte_eal.h
+++ b/lib/librte_eal/common/include/rte_eal.h
@@ -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
diff --git a/lib/librte_eal/linuxapp/eal/eal.c b/lib/librte_eal/linuxapp/eal/eal.c
index 451fdaf..38306bf 100644
--- a/lib/librte_eal/linuxapp/eal/eal.c
+++ b/lib/librte_eal/linuxapp/eal/eal.c
@@ -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)
diff --git a/lib/librte_eal/rte_eal_version.map b/lib/librte_eal/rte_eal_version.map
index 4146907..2e6cbe9 100644
--- a/lib/librte_eal/rte_eal_version.map
+++ b/lib/librte_eal/rte_eal_version.map
@@ -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;
diff --git a/lib/librte_mbuf/rte_mbuf_pool_ops.c b/lib/librte_mbuf/rte_mbuf_pool_ops.c
index 385fc43..48cc342 100644
--- a/lib/librte_mbuf/rte_mbuf_pool_ops.c
+++ b/lib/librte_mbuf/rte_mbuf_pool_ops.c
@@ -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;
 }
 
-- 
2.7.4

Re: [dpdk-dev] [PATCH FIX-OPTION-1] mbuf: fix the logic of user mempool ops API

[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>

Re: [dpdk-dev] [PATCH FIX-OPTION-1] mbuf: fix the logic of user mempool ops API

[Hemant Agrawal]

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

Re: [dpdk-dev] [PATCH FIX-OPTION-1] mbuf: fix the logic of user mempool ops API

[Thomas Monjalon]

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

Re: [dpdk-dev] Fw: [PATCH FIX-OPTION-1] mbuf: fix the logic of user mempool ops API

[santosh]

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.

Re: [dpdk-dev] [PATCH FIX-OPTION-1] mbuf: fix the logic of user mempool ops API

[Hemant Agrawal]

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.

[dpdk-dev] [PATCH FIX-OPTION-2 2/2] mbuf: fix user mempool ops get to use only named memzone

[Hemant Agrawal]

The eal default mempool ops API can also return the compile
time default mempool ops name, which can break the best mempool
ops name logic.

Fixes: a3acc3144a76 ("mbuf: add pool ops selection functions")

Signed-off-by: Hemant Agrawal <hemant.agrawal@nxp.com>
---
 lib/librte_mbuf/rte_mbuf_pool_ops.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/lib/librte_mbuf/rte_mbuf_pool_ops.c b/lib/librte_mbuf/rte_mbuf_pool_ops.c
index 385fc43..a636c28 100644
--- a/lib/librte_mbuf/rte_mbuf_pool_ops.c
+++ b/lib/librte_mbuf/rte_mbuf_pool_ops.c
@@ -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 NULL;
 	return mz->addr;
 }
 
-- 
2.7.4

[dpdk-dev] [PATCH] doc: remove eal API for default mempool ops name

[Hemant Agrawal]

Signed-off-by: Hemant Agrawal <hemant.agrawal@nxp.com>
---
 doc/guides/rel_notes/deprecation.rst | 9 +++++++++
 1 file changed, 9 insertions(+)

diff --git a/doc/guides/rel_notes/deprecation.rst b/doc/guides/rel_notes/deprecation.rst
index d59ad59..a2b391c 100644
--- a/doc/guides/rel_notes/deprecation.rst
+++ b/doc/guides/rel_notes/deprecation.rst
@@ -8,6 +8,15 @@ API and ABI deprecation notices are to be posted here.
 Deprecation Notices
 -------------------
 
+* eal: a new set of mbuf mempool ops name APIs for user, platform and best
+  mempool names have been defined in ``rte_mbuf`` in v18.02. The uses of
+  ``rte_eal_mbuf_default_mempool_ops`` shall be replaced by
+  ``rte_mbuf_best_mempool_ops``.
+  The following function is now redundant and it is target to be deprecated in 
+  18.05:
+
+  - ``rte_eal_mbuf_default_mempool_ops``
+  
 * eal: several API and ABI changes are planned for ``rte_devargs`` in v18.02.
   The format of device command line parameters will change. The bus will need
   to be explicitly stated in the device declaration. The enum ``rte_devtype``
-- 
2.7.4

[dpdk-dev] [PATCH v2] doc: remove eal API for default mempool ops name

[Hemant Agrawal]

Signed-off-by: Hemant Agrawal <hemant.agrawal@nxp.com>
---
v2: fix checkpatch errors

 doc/guides/rel_notes/deprecation.rst | 9 +++++++++
 1 file changed, 9 insertions(+)

diff --git a/doc/guides/rel_notes/deprecation.rst b/doc/guides/rel_notes/deprecation.rst
index d59ad59..c7d8f25 100644
--- a/doc/guides/rel_notes/deprecation.rst
+++ b/doc/guides/rel_notes/deprecation.rst
@@ -8,6 +8,15 @@ API and ABI deprecation notices are to be posted here.
 Deprecation Notices
 -------------------
 
+* eal: a new set of mbuf mempool ops name APIs for user, platform and best
+  mempool names have been defined in ``rte_mbuf`` in v18.02. The uses of
+  ``rte_eal_mbuf_default_mempool_ops`` shall be replaced by
+  ``rte_mbuf_best_mempool_ops``.
+  The following function is now redundant and it is target to be deprecated in
+  18.05:
+
+  - ``rte_eal_mbuf_default_mempool_ops``
+
 * eal: several API and ABI changes are planned for ``rte_devargs`` in v18.02.
   The format of device command line parameters will change. The bus will need
   to be explicitly stated in the device declaration. The enum ``rte_devtype``
-- 
2.7.4

Re: [dpdk-dev] [PATCH v2] doc: remove eal API for default mempool ops name

[Olivier Matz]

On Fri, Feb 02, 2018 at 02:01:42PM +0530, Hemant Agrawal wrote:
> Signed-off-by: Hemant Agrawal <hemant.agrawal@nxp.com>
> ---
> v2: fix checkpatch errors
> 
>  doc/guides/rel_notes/deprecation.rst | 9 +++++++++
>  1 file changed, 9 insertions(+)
> 
> diff --git a/doc/guides/rel_notes/deprecation.rst b/doc/guides/rel_notes/deprecation.rst
> index d59ad59..c7d8f25 100644
> --- a/doc/guides/rel_notes/deprecation.rst
> +++ b/doc/guides/rel_notes/deprecation.rst
> @@ -8,6 +8,15 @@ API and ABI deprecation notices are to be posted here.
>  Deprecation Notices
>  -------------------
>  
> +* eal: a new set of mbuf mempool ops name APIs for user, platform and best
> +  mempool names have been defined in ``rte_mbuf`` in v18.02. The uses of
> +  ``rte_eal_mbuf_default_mempool_ops`` shall be replaced by
> +  ``rte_mbuf_best_mempool_ops``.
> +  The following function is now redundant and it is target to be deprecated in
> +  18.05:
> +
> +  - ``rte_eal_mbuf_default_mempool_ops``
> +
>  * eal: several API and ABI changes are planned for ``rte_devargs`` in v18.02.
>    The format of device command line parameters will change. The bus will need
>    to be explicitly stated in the device declaration. The enum ``rte_devtype``

Acked-by: Olivier Matz <olivier.matz@6wind.com>

Re: [dpdk-dev] [PATCH v2] doc: remove eal API for default mempool ops name

[Ferruh Yigit]

On 2/2/2018 2:01 PM, Olivier Matz wrote:
> On Fri, Feb 02, 2018 at 02:01:42PM +0530, Hemant Agrawal wrote:
>> Signed-off-by: Hemant Agrawal <hemant.agrawal@nxp.com>
>> ---
>> v2: fix checkpatch errors
>>
>>  doc/guides/rel_notes/deprecation.rst | 9 +++++++++
>>  1 file changed, 9 insertions(+)
>>
>> diff --git a/doc/guides/rel_notes/deprecation.rst b/doc/guides/rel_notes/deprecation.rst
>> index d59ad59..c7d8f25 100644
>> --- a/doc/guides/rel_notes/deprecation.rst
>> +++ b/doc/guides/rel_notes/deprecation.rst
>> @@ -8,6 +8,15 @@ API and ABI deprecation notices are to be posted here.
>>  Deprecation Notices
>>  -------------------
>>  
>> +* eal: a new set of mbuf mempool ops name APIs for user, platform and best
>> +  mempool names have been defined in ``rte_mbuf`` in v18.02. The uses of
>> +  ``rte_eal_mbuf_default_mempool_ops`` shall be replaced by
>> +  ``rte_mbuf_best_mempool_ops``.
>> +  The following function is now redundant and it is target to be deprecated in
>> +  18.05:
>> +
>> +  - ``rte_eal_mbuf_default_mempool_ops``
>> +
>>  * eal: several API and ABI changes are planned for ``rte_devargs`` in v18.02.
>>    The format of device command line parameters will change. The bus will need
>>    to be explicitly stated in the device declaration. The enum ``rte_devtype``
> 
> Acked-by: Olivier Matz <olivier.matz@6wind.com>

Acked-by: Ferruh Yigit <ferruh.yigit@intel.com>

Re: [dpdk-dev] [PATCH v2] doc: remove eal API for default mempool ops name

[Thomas Monjalon]

13/02/2018 12:28, Ferruh Yigit:
> On 2/2/2018 2:01 PM, Olivier Matz wrote:
> > On Fri, Feb 02, 2018 at 02:01:42PM +0530, Hemant Agrawal wrote:
> >> Signed-off-by: Hemant Agrawal <hemant.agrawal@nxp.com>
> >> ---
> >> +* eal: a new set of mbuf mempool ops name APIs for user, platform and best
> >> +  mempool names have been defined in ``rte_mbuf`` in v18.02. The uses of
> >> +  ``rte_eal_mbuf_default_mempool_ops`` shall be replaced by
> >> +  ``rte_mbuf_best_mempool_ops``.
> >> +  The following function is now redundant and it is target to be deprecated in
> >> +  18.05:
> >> +
> >> +  - ``rte_eal_mbuf_default_mempool_ops``
> > 
> > Acked-by: Olivier Matz <olivier.matz@6wind.com>
> 
> Acked-by: Ferruh Yigit <ferruh.yigit@intel.com>

Applied

Re: [dpdk-dev] [PATCH v2] doc: remove eal API for default mempool ops name

[Jerin Jacob]

-----Original Message-----
> Date: Fri,  2 Feb 2018 14:01:42 +0530
> From: Hemant Agrawal <hemant.agrawal@nxp.com>
> To: olivier.matz@6wind.com, thomas@monjalon.net,
>  pbhagavatula@caviumnetworks.com
> CC: nipun.gupta@nxp.com, jerin.jacob@caviumnetworks.com,
>  santosh.shukla@caviumnetworks.com, dev@dpdk.org, Hemant Agrawal
>  <hemant.agrawal@nxp.com>
> Subject: [PATCH v2] doc: remove eal API for default mempool ops name
> X-Mailer: git-send-email 2.7.4
> 
> Signed-off-by: Hemant Agrawal <hemant.agrawal@nxp.com>

Acked-by: Jerin Jacob <jerin.jacob@caviumnetworks.com>