[PATCH 1/2] drivers/mempool: add stack mempool handler as driver

[PATCH 1/2] drivers/mempool: add stack mempool handler as driver

[Shreyansh Jain]

CONFIG_RTE_DRIVER_MEMPOOL_STACK option added to common_base.
Stack mempool handler moved from lib/librte_mempool into drivers/mempool.

With this patch, the Stack mempool handler registration is optional and
toggled via the configuration file. In case disabled (N), it would imply
request for creating of mempool would fail.

Signed-off-by: Shreyansh Jain <shreyansh.jain@nxp.com>
---
 config/common_base                                 |   5 +
 drivers/Makefile                                   |   1 +
 drivers/mempool/Makefile                           |  36 +++++
 drivers/mempool/stack/Makefile                     |  55 ++++++++
 drivers/mempool/stack/rte_mempool_stack.c          | 147 +++++++++++++++++++++
 .../mempool/stack/rte_mempool_stack_version.map    |   4 +
 lib/librte_mempool/Makefile                        |   1 -
 lib/librte_mempool/rte_mempool_stack.c             | 147 ---------------------
 8 files changed, 248 insertions(+), 148 deletions(-)
 create mode 100644 drivers/mempool/Makefile
 create mode 100644 drivers/mempool/stack/Makefile
 create mode 100644 drivers/mempool/stack/rte_mempool_stack.c
 create mode 100644 drivers/mempool/stack/rte_mempool_stack_version.map
 delete mode 100644 lib/librte_mempool/rte_mempool_stack.c

diff --git a/config/common_base b/config/common_base
index 37aa1e1..3e70aa0 100644
--- a/config/common_base
+++ b/config/common_base
@@ -464,6 +464,11 @@ CONFIG_RTE_MEMPOOL_CACHE_MAX_SIZE=512
 CONFIG_RTE_LIBRTE_MEMPOOL_DEBUG=n
 
 #
+# Compile Mempool drivers
+#
+CONFIG_RTE_DRIVER_MEMPOOL_STACK=y
+
+#
 # Compile librte_mbuf
 #
 CONFIG_RTE_LIBRTE_MBUF=y
diff --git a/drivers/Makefile b/drivers/Makefile
index 81c03a8..193056b 100644
--- a/drivers/Makefile
+++ b/drivers/Makefile
@@ -31,6 +31,7 @@
 
 include $(RTE_SDK)/mk/rte.vars.mk
 
+DIRS-y += mempool
 DIRS-y += net
 DIRS-$(CONFIG_RTE_LIBRTE_CRYPTODEV) += crypto
 
diff --git a/drivers/mempool/Makefile b/drivers/mempool/Makefile
new file mode 100644
index 0000000..b256a34
--- /dev/null
+++ b/drivers/mempool/Makefile
@@ -0,0 +1,36 @@
+#   BSD LICENSE
+#
+#   Copyright(c) 2017 NXP. All rights reserved.
+#   All rights reserved.
+#
+#   Redistribution and use in source and binary forms, with or without
+#   modification, are permitted provided that the following conditions
+#   are met:
+#
+#     * Redistributions of source code must retain the above copyright
+#       notice, this list of conditions and the following disclaimer.
+#     * Redistributions in binary form must reproduce the above copyright
+#       notice, this list of conditions and the following disclaimer in
+#       the documentation and/or other materials provided with the
+#       distribution.
+#     * Neither the name of NXP nor the names of its
+#       contributors may be used to endorse or promote products derived
+#       from this software without specific prior written permission.
+#
+#   THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS
+#   "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT
+#   LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR
+#   A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT
+#   OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL,
+#   SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT
+#   LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE,
+#   DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY
+#   THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
+#   (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE
+#   OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
+
+include $(RTE_SDK)/mk/rte.vars.mk
+
+DIRS-$(CONFIG_RTE_DRIVER_MEMPOOL_STACK) += stack
+
+include $(RTE_SDK)/mk/rte.subdir.mk
diff --git a/drivers/mempool/stack/Makefile b/drivers/mempool/stack/Makefile
new file mode 100644
index 0000000..f98eb5b
--- /dev/null
+++ b/drivers/mempool/stack/Makefile
@@ -0,0 +1,55 @@
+#   BSD LICENSE
+#
+#   Copyright(c) 2017 NXP.
+#   All rights reserved.
+#
+#   Redistribution and use in source and binary forms, with or without
+#   modification, are permitted provided that the following conditions
+#   are met:
+#
+#     * Redistributions of source code must retain the above copyright
+#       notice, this list of conditions and the following disclaimer.
+#     * Redistributions in binary form must reproduce the above copyright
+#       notice, this list of conditions and the following disclaimer in
+#       the documentation and/or other materials provided with the
+#       distribution.
+#     * Neither the name of NXP nor the names of its
+#       contributors may be used to endorse or promote products derived
+#       from this software without specific prior written permission.
+#
+#   THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS
+#   "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT
+#   LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR
+#   A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT
+#   OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL,
+#   SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT
+#   LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE,
+#   DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY
+#   THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
+#   (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE
+#   OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
+
+include $(RTE_SDK)/mk/rte.vars.mk
+
+#
+# library name
+#
+LIB = librte_mempool_stack.a
+
+CFLAGS += -O3
+CFLAGS += $(WERROR_FLAGS)
+
+# Headers
+CFLAGS += -I$(RTE_SDK)/lib/librte_mempool
+
+EXPORT_MAP := rte_mempool_stack_version.map
+
+LIBABIVER := 1
+
+SRCS-$(CONFIG_RTE_DRIVER_MEMPOOL_STACK) += rte_mempool_stack.c
+
+# this lib depends upon:
+DEPDIRS-$(CONFIG_RTE_DRIVER_MEMPOOL_STACK) += lib/librte_eal
+DEPDIRS-$(CONFIG_RTE_DRIVER_MEMPOOL_STACK) += lib/librte_mempool
+
+include $(RTE_SDK)/mk/rte.lib.mk
diff --git a/drivers/mempool/stack/rte_mempool_stack.c b/drivers/mempool/stack/rte_mempool_stack.c
new file mode 100644
index 0000000..817f77e
--- /dev/null
+++ b/drivers/mempool/stack/rte_mempool_stack.c
@@ -0,0 +1,147 @@
+/*-
+ *   BSD LICENSE
+ *
+ *   Copyright(c) 2016 Intel Corporation. All rights reserved.
+ *   All rights reserved.
+ *
+ *   Redistribution and use in source and binary forms, with or without
+ *   modification, are permitted provided that the following conditions
+ *   are met:
+ *
+ *     * Redistributions of source code must retain the above copyright
+ *       notice, this list of conditions and the following disclaimer.
+ *     * Redistributions in binary form must reproduce the above copyright
+ *       notice, this list of conditions and the following disclaimer in
+ *       the documentation and/or other materials provided with the
+ *       distribution.
+ *     * Neither the name of Intel Corporation nor the names of its
+ *       contributors may be used to endorse or promote products derived
+ *       from this software without specific prior written permission.
+ *
+ *   THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS
+ *   "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT
+ *   LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR
+ *   A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT
+ *   OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL,
+ *   SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT
+ *   LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE,
+ *   DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY
+ *   THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
+ *   (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE
+ *   OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
+ */
+
+#include <stdio.h>
+#include <rte_mempool.h>
+#include <rte_malloc.h>
+
+struct rte_mempool_stack {
+	rte_spinlock_t sl;
+
+	uint32_t size;
+	uint32_t len;
+	void *objs[];
+};
+
+static int
+stack_alloc(struct rte_mempool *mp)
+{
+	struct rte_mempool_stack *s;
+	unsigned n = mp->size;
+	int size = sizeof(*s) + (n+16)*sizeof(void *);
+
+	/* Allocate our local memory structure */
+	s = rte_zmalloc_socket("mempool-stack",
+			size,
+			RTE_CACHE_LINE_SIZE,
+			mp->socket_id);
+	if (s == NULL) {
+		RTE_LOG(ERR, MEMPOOL, "Cannot allocate stack!\n");
+		return -ENOMEM;
+	}
+
+	rte_spinlock_init(&s->sl);
+
+	s->size = n;
+	mp->pool_data = s;
+
+	return 0;
+}
+
+static int
+stack_enqueue(struct rte_mempool *mp, void * const *obj_table,
+		unsigned n)
+{
+	struct rte_mempool_stack *s = mp->pool_data;
+	void **cache_objs;
+	unsigned index;
+
+	rte_spinlock_lock(&s->sl);
+	cache_objs = &s->objs[s->len];
+
+	/* Is there sufficient space in the stack ? */
+	if ((s->len + n) > s->size) {
+		rte_spinlock_unlock(&s->sl);
+		return -ENOBUFS;
+	}
+
+	/* Add elements back into the cache */
+	for (index = 0; index < n; ++index, obj_table++)
+		cache_objs[index] = *obj_table;
+
+	s->len += n;
+
+	rte_spinlock_unlock(&s->sl);
+	return 0;
+}
+
+static int
+stack_dequeue(struct rte_mempool *mp, void **obj_table,
+		unsigned n)
+{
+	struct rte_mempool_stack *s = mp->pool_data;
+	void **cache_objs;
+	unsigned index, len;
+
+	rte_spinlock_lock(&s->sl);
+
+	if (unlikely(n > s->len)) {
+		rte_spinlock_unlock(&s->sl);
+		return -ENOENT;
+	}
+
+	cache_objs = s->objs;
+
+	for (index = 0, len = s->len - 1; index < n;
+			++index, len--, obj_table++)
+		*obj_table = cache_objs[len];
+
+	s->len -= n;
+	rte_spinlock_unlock(&s->sl);
+	return 0;
+}
+
+static unsigned
+stack_get_count(const struct rte_mempool *mp)
+{
+	struct rte_mempool_stack *s = mp->pool_data;
+
+	return s->len;
+}
+
+static void
+stack_free(struct rte_mempool *mp)
+{
+	rte_free((void *)(mp->pool_data));
+}
+
+static struct rte_mempool_ops ops_stack = {
+	.name = "stack",
+	.alloc = stack_alloc,
+	.free = stack_free,
+	.enqueue = stack_enqueue,
+	.dequeue = stack_dequeue,
+	.get_count = stack_get_count
+};
+
+MEMPOOL_REGISTER_OPS(ops_stack);
diff --git a/drivers/mempool/stack/rte_mempool_stack_version.map b/drivers/mempool/stack/rte_mempool_stack_version.map
new file mode 100644
index 0000000..8591cc0
--- /dev/null
+++ b/drivers/mempool/stack/rte_mempool_stack_version.map
@@ -0,0 +1,4 @@
+DPDK_17.05 {
+
+	local: *;
+};
diff --git a/lib/librte_mempool/Makefile b/lib/librte_mempool/Makefile
index 057a6ab..a4c089e 100644
--- a/lib/librte_mempool/Makefile
+++ b/lib/librte_mempool/Makefile
@@ -44,7 +44,6 @@ LIBABIVER := 2
 SRCS-$(CONFIG_RTE_LIBRTE_MEMPOOL) +=  rte_mempool.c
 SRCS-$(CONFIG_RTE_LIBRTE_MEMPOOL) +=  rte_mempool_ops.c
 SRCS-$(CONFIG_RTE_LIBRTE_MEMPOOL) +=  rte_mempool_ring.c
-SRCS-$(CONFIG_RTE_LIBRTE_MEMPOOL) +=  rte_mempool_stack.c
 # install includes
 SYMLINK-$(CONFIG_RTE_LIBRTE_MEMPOOL)-include := rte_mempool.h
 
diff --git a/lib/librte_mempool/rte_mempool_stack.c b/lib/librte_mempool/rte_mempool_stack.c
deleted file mode 100644
index 817f77e..0000000
--- a/lib/librte_mempool/rte_mempool_stack.c
+++ /dev/null
@@ -1,147 +0,0 @@
-/*-
- *   BSD LICENSE
- *
- *   Copyright(c) 2016 Intel Corporation. All rights reserved.
- *   All rights reserved.
- *
- *   Redistribution and use in source and binary forms, with or without
- *   modification, are permitted provided that the following conditions
- *   are met:
- *
- *     * Redistributions of source code must retain the above copyright
- *       notice, this list of conditions and the following disclaimer.
- *     * Redistributions in binary form must reproduce the above copyright
- *       notice, this list of conditions and the following disclaimer in
- *       the documentation and/or other materials provided with the
- *       distribution.
- *     * Neither the name of Intel Corporation nor the names of its
- *       contributors may be used to endorse or promote products derived
- *       from this software without specific prior written permission.
- *
- *   THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS
- *   "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT
- *   LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR
- *   A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT
- *   OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL,
- *   SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT
- *   LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE,
- *   DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY
- *   THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
- *   (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE
- *   OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
- */
-
-#include <stdio.h>
-#include <rte_mempool.h>
-#include <rte_malloc.h>
-
-struct rte_mempool_stack {
-	rte_spinlock_t sl;
-
-	uint32_t size;
-	uint32_t len;
-	void *objs[];
-};
-
-static int
-stack_alloc(struct rte_mempool *mp)
-{
-	struct rte_mempool_stack *s;
-	unsigned n = mp->size;
-	int size = sizeof(*s) + (n+16)*sizeof(void *);
-
-	/* Allocate our local memory structure */
-	s = rte_zmalloc_socket("mempool-stack",
-			size,
-			RTE_CACHE_LINE_SIZE,
-			mp->socket_id);
-	if (s == NULL) {
-		RTE_LOG(ERR, MEMPOOL, "Cannot allocate stack!\n");
-		return -ENOMEM;
-	}
-
-	rte_spinlock_init(&s->sl);
-
-	s->size = n;
-	mp->pool_data = s;
-
-	return 0;
-}
-
-static int
-stack_enqueue(struct rte_mempool *mp, void * const *obj_table,
-		unsigned n)
-{
-	struct rte_mempool_stack *s = mp->pool_data;
-	void **cache_objs;
-	unsigned index;
-
-	rte_spinlock_lock(&s->sl);
-	cache_objs = &s->objs[s->len];
-
-	/* Is there sufficient space in the stack ? */
-	if ((s->len + n) > s->size) {
-		rte_spinlock_unlock(&s->sl);
-		return -ENOBUFS;
-	}
-
-	/* Add elements back into the cache */
-	for (index = 0; index < n; ++index, obj_table++)
-		cache_objs[index] = *obj_table;
-
-	s->len += n;
-
-	rte_spinlock_unlock(&s->sl);
-	return 0;
-}
-
-static int
-stack_dequeue(struct rte_mempool *mp, void **obj_table,
-		unsigned n)
-{
-	struct rte_mempool_stack *s = mp->pool_data;
-	void **cache_objs;
-	unsigned index, len;
-
-	rte_spinlock_lock(&s->sl);
-
-	if (unlikely(n > s->len)) {
-		rte_spinlock_unlock(&s->sl);
-		return -ENOENT;
-	}
-
-	cache_objs = s->objs;
-
-	for (index = 0, len = s->len - 1; index < n;
-			++index, len--, obj_table++)
-		*obj_table = cache_objs[len];
-
-	s->len -= n;
-	rte_spinlock_unlock(&s->sl);
-	return 0;
-}
-
-static unsigned
-stack_get_count(const struct rte_mempool *mp)
-{
-	struct rte_mempool_stack *s = mp->pool_data;
-
-	return s->len;
-}
-
-static void
-stack_free(struct rte_mempool *mp)
-{
-	rte_free((void *)(mp->pool_data));
-}
-
-static struct rte_mempool_ops ops_stack = {
-	.name = "stack",
-	.alloc = stack_alloc,
-	.free = stack_free,
-	.enqueue = stack_enqueue,
-	.dequeue = stack_dequeue,
-	.get_count = stack_get_count
-};
-
-MEMPOOL_REGISTER_OPS(ops_stack);
-- 
2.7.4

[PATCH 2/2] drivers/mempool: add ring mempool handler as driver

[Shreyansh Jain]

CONFIG_RTE_DRIVER_MEMPOOL_RING option is introduced.
Ring Mempool handler moved from lib/librte_mempool into drivers/mempool.

With this patch, the Ring mempool handler registration is optional and
toggled via the configuration file. In case disabled (N), it would imply
request for creating of mempool would fail.

Signed-off-by: Shreyansh Jain <shreyansh.jain@nxp.com>
---
 config/common_base                                |   1 +
 drivers/mempool/Makefile                          |   1 +
 drivers/mempool/ring/Makefile                     |  55 ++++++++
 drivers/mempool/ring/rte_mempool_ring.c           | 161 ++++++++++++++++++++++
 drivers/mempool/ring/rte_mempool_ring_version.map |   4 +
 lib/librte_mempool/Makefile                       |   1 -
 lib/librte_mempool/rte_mempool_ring.c             | 161 ----------------------
 7 files changed, 222 insertions(+), 162 deletions(-)
 create mode 100644 drivers/mempool/ring/Makefile
 create mode 100644 drivers/mempool/ring/rte_mempool_ring.c
 create mode 100644 drivers/mempool/ring/rte_mempool_ring_version.map
 delete mode 100644 lib/librte_mempool/rte_mempool_ring.c

diff --git a/config/common_base b/config/common_base
index 3e70aa0..97e4bba 100644
--- a/config/common_base
+++ b/config/common_base
@@ -466,6 +466,7 @@ CONFIG_RTE_LIBRTE_MEMPOOL_DEBUG=n
 #
 # Compile Mempool drivers
 #
+CONFIG_RTE_DRIVER_MEMPOOL_RING=y
 CONFIG_RTE_DRIVER_MEMPOOL_STACK=y
 
 #
diff --git a/drivers/mempool/Makefile b/drivers/mempool/Makefile
index b256a34..d840022 100644
--- a/drivers/mempool/Makefile
+++ b/drivers/mempool/Makefile
@@ -31,6 +31,7 @@
 
 include $(RTE_SDK)/mk/rte.vars.mk
 
+DIRS-$(CONFIG_RTE_DRIVER_MEMPOOL_RING) += ring
 DIRS-$(CONFIG_RTE_DRIVER_MEMPOOL_STACK) += stack
 
 include $(RTE_SDK)/mk/rte.subdir.mk
diff --git a/drivers/mempool/ring/Makefile b/drivers/mempool/ring/Makefile
new file mode 100644
index 0000000..21315c2
--- /dev/null
+++ b/drivers/mempool/ring/Makefile
@@ -0,0 +1,55 @@
+#   BSD LICENSE
+#
+#   Copyright(c) 2017 NXP.
+#   All rights reserved.
+#
+#   Redistribution and use in source and binary forms, with or without
+#   modification, are permitted provided that the following conditions
+#   are met:
+#
+#     * Redistributions of source code must retain the above copyright
+#       notice, this list of conditions and the following disclaimer.
+#     * Redistributions in binary form must reproduce the above copyright
+#       notice, this list of conditions and the following disclaimer in
+#       the documentation and/or other materials provided with the
+#       distribution.
+#     * Neither the name of NXP nor the names of its
+#       contributors may be used to endorse or promote products derived
+#       from this software without specific prior written permission.
+#
+#   THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS
+#   "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT
+#   LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR
+#   A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT
+#   OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL,
+#   SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT
+#   LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE,
+#   DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY
+#   THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
+#   (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE
+#   OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
+
+include $(RTE_SDK)/mk/rte.vars.mk
+
+#
+# library name
+#
+LIB = librte_mempool_ring.a
+
+CFLAGS += -O3
+CFLAGS += $(WERROR_FLAGS)
+
+# Headers
+CFLAGS += -I$(RTE_SDK)/lib/librte_mempool
+
+EXPORT_MAP := rte_mempool_ring_version.map
+
+LIBABIVER := 1
+
+SRCS-$(CONFIG_RTE_DRIVER_MEMPOOL_RING) += rte_mempool_ring.c
+
+# this lib depends upon:
+DEPDIRS-$(CONFIG_RTE_DRIVER_MEMPOOL_RING) += lib/librte_eal
+DEPDIRS-$(CONFIG_RTE_DRIVER_MEMPOOL_RING) += lib/librte_mempool lib/librte_ring
+
+include $(RTE_SDK)/mk/rte.lib.mk
diff --git a/drivers/mempool/ring/rte_mempool_ring.c b/drivers/mempool/ring/rte_mempool_ring.c
new file mode 100644
index 0000000..b9aa64d
--- /dev/null
+++ b/drivers/mempool/ring/rte_mempool_ring.c
@@ -0,0 +1,161 @@
+/*-
+ *   BSD LICENSE
+ *
+ *   Copyright(c) 2010-2016 Intel Corporation. All rights reserved.
+ *   All rights reserved.
+ *
+ *   Redistribution and use in source and binary forms, with or without
+ *   modification, are permitted provided that the following conditions
+ *   are met:
+ *
+ *     * Redistributions of source code must retain the above copyright
+ *       notice, this list of conditions and the following disclaimer.
+ *     * Redistributions in binary form must reproduce the above copyright
+ *       notice, this list of conditions and the following disclaimer in
+ *       the documentation and/or other materials provided with the
+ *       distribution.
+ *     * Neither the name of Intel Corporation nor the names of its
+ *       contributors may be used to endorse or promote products derived
+ *       from this software without specific prior written permission.
+ *
+ *   THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS
+ *   "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT
+ *   LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR
+ *   A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT
+ *   OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL,
+ *   SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT
+ *   LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE,
+ *   DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY
+ *   THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
+ *   (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE
+ *   OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
+ */
+
+#include <stdio.h>
+#include <string.h>
+
+#include <rte_errno.h>
+#include <rte_ring.h>
+#include <rte_mempool.h>
+
+static int
+common_ring_mp_enqueue(struct rte_mempool *mp, void * const *obj_table,
+		unsigned n)
+{
+	return rte_ring_mp_enqueue_bulk(mp->pool_data, obj_table, n);
+}
+
+static int
+common_ring_sp_enqueue(struct rte_mempool *mp, void * const *obj_table,
+		unsigned n)
+{
+	return rte_ring_sp_enqueue_bulk(mp->pool_data, obj_table, n);
+}
+
+static int
+common_ring_mc_dequeue(struct rte_mempool *mp, void **obj_table, unsigned n)
+{
+	return rte_ring_mc_dequeue_bulk(mp->pool_data, obj_table, n);
+}
+
+static int
+common_ring_sc_dequeue(struct rte_mempool *mp, void **obj_table, unsigned n)
+{
+	return rte_ring_sc_dequeue_bulk(mp->pool_data, obj_table, n);
+}
+
+static unsigned
+common_ring_get_count(const struct rte_mempool *mp)
+{
+	return rte_ring_count(mp->pool_data);
+}
+
+
+static int
+common_ring_alloc(struct rte_mempool *mp)
+{
+	int rg_flags = 0, ret;
+	char rg_name[RTE_RING_NAMESIZE];
+	struct rte_ring *r;
+
+	ret = snprintf(rg_name, sizeof(rg_name),
+		RTE_MEMPOOL_MZ_FORMAT, mp->name);
+	if (ret < 0 || ret >= (int)sizeof(rg_name)) {
+		rte_errno = ENAMETOOLONG;
+		return -rte_errno;
+	}
+
+	/* ring flags */
+	if (mp->flags & MEMPOOL_F_SP_PUT)
+		rg_flags |= RING_F_SP_ENQ;
+	if (mp->flags & MEMPOOL_F_SC_GET)
+		rg_flags |= RING_F_SC_DEQ;
+
+	/*
+	 * Allocate the ring that will be used to store objects.
+	 * Ring functions will return appropriate errors if we are
+	 * running as a secondary process etc., so no checks made
+	 * in this function for that condition.
+	 */
+	r = rte_ring_create(rg_name, rte_align32pow2(mp->size + 1),
+		mp->socket_id, rg_flags);
+	if (r == NULL)
+		return -rte_errno;
+
+	mp->pool_data = r;
+
+	return 0;
+}
+
+static void
+common_ring_free(struct rte_mempool *mp)
+{
+	rte_ring_free(mp->pool_data);
+}
+
+/*
+ * The following 4 declarations of mempool ops structs address
+ * the need for the backward compatible mempool handlers for
+ * single/multi producers and single/multi consumers as dictated by the
+ * flags provided to the rte_mempool_create function
+ */
+static const struct rte_mempool_ops ops_mp_mc = {
+	.name = "ring_mp_mc",
+	.alloc = common_ring_alloc,
+	.free = common_ring_free,
+	.enqueue = common_ring_mp_enqueue,
+	.dequeue = common_ring_mc_dequeue,
+	.get_count = common_ring_get_count,
+};
+
+static const struct rte_mempool_ops ops_sp_sc = {
+	.name = "ring_sp_sc",
+	.alloc = common_ring_alloc,
+	.free = common_ring_free,
+	.enqueue = common_ring_sp_enqueue,
+	.dequeue = common_ring_sc_dequeue,
+	.get_count = common_ring_get_count,
+};
+
+static const struct rte_mempool_ops ops_mp_sc = {
+	.name = "ring_mp_sc",
+	.alloc = common_ring_alloc,
+	.free = common_ring_free,
+	.enqueue = common_ring_mp_enqueue,
+	.dequeue = common_ring_sc_dequeue,
+	.get_count = common_ring_get_count,
+};
+
+static const struct rte_mempool_ops ops_sp_mc = {
+	.name = "ring_sp_mc",
+	.alloc = common_ring_alloc,
+	.free = common_ring_free,
+	.enqueue = common_ring_sp_enqueue,
+	.dequeue = common_ring_mc_dequeue,
+	.get_count = common_ring_get_count,
+};
+
+MEMPOOL_REGISTER_OPS(ops_mp_mc);
+MEMPOOL_REGISTER_OPS(ops_sp_sc);
+MEMPOOL_REGISTER_OPS(ops_mp_sc);
+MEMPOOL_REGISTER_OPS(ops_sp_mc);
diff --git a/drivers/mempool/ring/rte_mempool_ring_version.map b/drivers/mempool/ring/rte_mempool_ring_version.map
new file mode 100644
index 0000000..8591cc0
--- /dev/null
+++ b/drivers/mempool/ring/rte_mempool_ring_version.map
@@ -0,0 +1,4 @@
+DPDK_17.05 {
+
+	local: *;
+};
diff --git a/lib/librte_mempool/Makefile b/lib/librte_mempool/Makefile
index a4c089e..4cbf772 100644
--- a/lib/librte_mempool/Makefile
+++ b/lib/librte_mempool/Makefile
@@ -43,7 +43,6 @@ LIBABIVER := 2
 # all source are stored in SRCS-y
 SRCS-$(CONFIG_RTE_LIBRTE_MEMPOOL) +=  rte_mempool.c
 SRCS-$(CONFIG_RTE_LIBRTE_MEMPOOL) +=  rte_mempool_ops.c
-SRCS-$(CONFIG_RTE_LIBRTE_MEMPOOL) +=  rte_mempool_ring.c
 # install includes
 SYMLINK-$(CONFIG_RTE_LIBRTE_MEMPOOL)-include := rte_mempool.h
 
diff --git a/lib/librte_mempool/rte_mempool_ring.c b/lib/librte_mempool/rte_mempool_ring.c
deleted file mode 100644
index b9aa64d..0000000
--- a/lib/librte_mempool/rte_mempool_ring.c
+++ /dev/null
@@ -1,161 +0,0 @@
-/*-
- *   BSD LICENSE
- *
- *   Copyright(c) 2010-2016 Intel Corporation. All rights reserved.
- *   All rights reserved.
- *
- *   Redistribution and use in source and binary forms, with or without
- *   modification, are permitted provided that the following conditions
- *   are met:
- *
- *     * Redistributions of source code must retain the above copyright
- *       notice, this list of conditions and the following disclaimer.
- *     * Redistributions in binary form must reproduce the above copyright
- *       notice, this list of conditions and the following disclaimer in
- *       the documentation and/or other materials provided with the
- *       distribution.
- *     * Neither the name of Intel Corporation nor the names of its
- *       contributors may be used to endorse or promote products derived
- *       from this software without specific prior written permission.
- *
- *   THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS
- *   "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT
- *   LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR
- *   A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT
- *   OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL,
- *   SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT
- *   LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE,
- *   DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY
- *   THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
- *   (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE
- *   OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
- */
-
-#include <stdio.h>
-#include <string.h>
-
-#include <rte_errno.h>
-#include <rte_ring.h>
-#include <rte_mempool.h>
-
-static int
-common_ring_mp_enqueue(struct rte_mempool *mp, void * const *obj_table,
-		unsigned n)
-{
-	return rte_ring_mp_enqueue_bulk(mp->pool_data, obj_table, n);
-}
-
-static int
-common_ring_sp_enqueue(struct rte_mempool *mp, void * const *obj_table,
-		unsigned n)
-{
-	return rte_ring_sp_enqueue_bulk(mp->pool_data, obj_table, n);
-}
-
-static int
-common_ring_mc_dequeue(struct rte_mempool *mp, void **obj_table, unsigned n)
-{
-	return rte_ring_mc_dequeue_bulk(mp->pool_data, obj_table, n);
-}
-
-static int
-common_ring_sc_dequeue(struct rte_mempool *mp, void **obj_table, unsigned n)
-{
-	return rte_ring_sc_dequeue_bulk(mp->pool_data, obj_table, n);
-}
-
-static unsigned
-common_ring_get_count(const struct rte_mempool *mp)
-{
-	return rte_ring_count(mp->pool_data);
-}
-
-
-static int
-common_ring_alloc(struct rte_mempool *mp)
-{
-	int rg_flags = 0, ret;
-	char rg_name[RTE_RING_NAMESIZE];
-	struct rte_ring *r;
-
-	ret = snprintf(rg_name, sizeof(rg_name),
-		RTE_MEMPOOL_MZ_FORMAT, mp->name);
-	if (ret < 0 || ret >= (int)sizeof(rg_name)) {
-		rte_errno = ENAMETOOLONG;
-		return -rte_errno;
-	}
-
-	/* ring flags */
-	if (mp->flags & MEMPOOL_F_SP_PUT)
-		rg_flags |= RING_F_SP_ENQ;
-	if (mp->flags & MEMPOOL_F_SC_GET)
-		rg_flags |= RING_F_SC_DEQ;
-
-	/*
-	 * Allocate the ring that will be used to store objects.
-	 * Ring functions will return appropriate errors if we are
-	 * running as a secondary process etc., so no checks made
-	 * in this function for that condition.
-	 */
-	r = rte_ring_create(rg_name, rte_align32pow2(mp->size + 1),
-		mp->socket_id, rg_flags);
-	if (r == NULL)
-		return -rte_errno;
-
-	mp->pool_data = r;
-
-	return 0;
-}
-
-static void
-common_ring_free(struct rte_mempool *mp)
-{
-	rte_ring_free(mp->pool_data);
-}
-
-/*
- * The following 4 declarations of mempool ops structs address
- * the need for the backward compatible mempool handlers for
- * single/multi producers and single/multi consumers as dictated by the
- * flags provided to the rte_mempool_create function
- */
-static const struct rte_mempool_ops ops_mp_mc = {
-	.name = "ring_mp_mc",
-	.alloc = common_ring_alloc,
-	.free = common_ring_free,
-	.enqueue = common_ring_mp_enqueue,
-	.dequeue = common_ring_mc_dequeue,
-	.get_count = common_ring_get_count,
-};
-
-static const struct rte_mempool_ops ops_sp_sc = {
-	.name = "ring_sp_sc",
-	.alloc = common_ring_alloc,
-	.free = common_ring_free,
-	.enqueue = common_ring_sp_enqueue,
-	.dequeue = common_ring_sc_dequeue,
-	.get_count = common_ring_get_count,
-};
-
-static const struct rte_mempool_ops ops_mp_sc = {
-	.name = "ring_mp_sc",
-	.alloc = common_ring_alloc,
-	.free = common_ring_free,
-	.enqueue = common_ring_mp_enqueue,
-	.dequeue = common_ring_sc_dequeue,
-	.get_count = common_ring_get_count,
-};
-
-static const struct rte_mempool_ops ops_sp_mc = {
-	.name = "ring_sp_mc",
-	.alloc = common_ring_alloc,
-	.free = common_ring_free,
-	.enqueue = common_ring_sp_enqueue,
-	.dequeue = common_ring_mc_dequeue,
-	.get_count = common_ring_get_count,
-};
-
-MEMPOOL_REGISTER_OPS(ops_mp_mc);
-MEMPOOL_REGISTER_OPS(ops_sp_sc);
-MEMPOOL_REGISTER_OPS(ops_mp_sc);
-MEMPOOL_REGISTER_OPS(ops_sp_mc);
-- 
2.7.4

Re: [PATCH 1/2] drivers/mempool: add stack mempool handler as driver

[Hunt, David]

On 20/3/2017 10:03 AM, Shreyansh Jain wrote:
> CONFIG_RTE_DRIVER_MEMPOOL_STACK option added to common_base.
> Stack mempool handler moved from lib/librte_mempool into drivers/mempool.
>
> With this patch, the Stack mempool handler registration is optional and
> toggled via the configuration file. In case disabled (N), it would imply
> request for creating of mempool would fail.
>
> Signed-off-by: Shreyansh Jain <shreyansh.jain@nxp.com>
> ---
>   config/common_base                                 |   5 +
>   drivers/Makefile                                   |   1 +
>   drivers/mempool/Makefile                           |  36 +++++
>   drivers/mempool/stack/Makefile                     |  55 ++++++++
>   drivers/mempool/stack/rte_mempool_stack.c          | 147 +++++++++++++++++++++
>   .../mempool/stack/rte_mempool_stack_version.map    |   4 +
>   lib/librte_mempool/Makefile                        |   1 -
>   lib/librte_mempool/rte_mempool_stack.c             | 147 ---------------------
>   8 files changed, 248 insertions(+), 148 deletions(-)
>   create mode 100644 drivers/mempool/Makefile
>   create mode 100644 drivers/mempool/stack/Makefile
>   create mode 100644 drivers/mempool/stack/rte_mempool_stack.c
>   create mode 100644 drivers/mempool/stack/rte_mempool_stack_version.map
>   delete mode 100644 lib/librte_mempool/rte_mempool_stack.c
>
> diff --git a/config/common_base b/config/common_base
> index 37aa1e1..3e70aa0 100644
> --- a/config/common_base
> +++ b/config/common_base
> @@ -464,6 +464,11 @@ CONFIG_RTE_MEMPOOL_CACHE_MAX_SIZE=512
>   CONFIG_RTE_LIBRTE_MEMPOOL_DEBUG=n
>   
>   #
> +# Compile Mempool drivers
> +#
> +CONFIG_RTE_DRIVER_MEMPOOL_STACK=y
> +
> +#
>   # Compile librte_mbuf
>   #
>   CONFIG_RTE_LIBRTE_MBUF=y
> diff --git a/drivers/Makefile b/drivers/Makefile
> index 81c03a8..193056b 100644
> --- a/drivers/Makefile
> +++ b/drivers/Makefile
> @@ -31,6 +31,7 @@
>   
>   include $(RTE_SDK)/mk/rte.vars.mk
>   
> +DIRS-y += mempool
>   DIRS-y += net
>   DIRS-$(CONFIG_RTE_LIBRTE_CRYPTODEV) += crypto
>   
> diff --git a/drivers/mempool/Makefile b/drivers/mempool/Makefile
> new file mode 100644
> index 0000000..b256a34
> --- /dev/null
> +++ b/drivers/mempool/Makefile
> @@ -0,0 +1,36 @@
> +#   BSD LICENSE
> +#
> +#   Copyright(c) 2017 NXP. All rights reserved.
> +#   All rights reserved.
> +#
> +#   Redistribution and use in source and binary forms, with or without
> +#   modification, are permitted provided that the following conditions
> +#   are met:
> +#
> +#     * Redistributions of source code must retain the above copyright
> +#       notice, this list of conditions and the following disclaimer.
> +#     * Redistributions in binary form must reproduce the above copyright
> +#       notice, this list of conditions and the following disclaimer in
> +#       the documentation and/or other materials provided with the
> +#       distribution.
> +#     * Neither the name of NXP nor the names of its
> +#       contributors may be used to endorse or promote products derived
> +#       from this software without specific prior written permission.
> +#
> +#   THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS
> +#   "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT
> +#   LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR
> +#   A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT
> +#   OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL,
> +#   SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT
> +#   LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE,
> +#   DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY
> +#   THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
> +#   (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE
> +#   OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
> +
> +include $(RTE_SDK)/mk/rte.vars.mk
> +
> +DIRS-$(CONFIG_RTE_DRIVER_MEMPOOL_STACK) += stack
> +
> +include $(RTE_SDK)/mk/rte.subdir.mk
> diff --git a/drivers/mempool/stack/Makefile b/drivers/mempool/stack/Makefile
> new file mode 100644
> index 0000000..f98eb5b
> --- /dev/null
> +++ b/drivers/mempool/stack/Makefile
> @@ -0,0 +1,55 @@
> +#   BSD LICENSE
> +#
> +#   Copyright(c) 2017 NXP.
> +#   All rights reserved.
> +#
> +#   Redistribution and use in source and binary forms, with or without
> +#   modification, are permitted provided that the following conditions
> +#   are met:
> +#
> +#     * Redistributions of source code must retain the above copyright
> +#       notice, this list of conditions and the following disclaimer.
> +#     * Redistributions in binary form must reproduce the above copyright
> +#       notice, this list of conditions and the following disclaimer in
> +#       the documentation and/or other materials provided with the
> +#       distribution.
> +#     * Neither the name of NXP nor the names of its
> +#       contributors may be used to endorse or promote products derived
> +#       from this software without specific prior written permission.
> +#
> +#   THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS
> +#   "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT
> +#   LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR
> +#   A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT
> +#   OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL,
> +#   SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT
> +#   LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE,
> +#   DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY
> +#   THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
> +#   (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE
> +#   OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
> +
> +include $(RTE_SDK)/mk/rte.vars.mk
> +
> +#
> +# library name
> +#
> +LIB = librte_mempool_stack.a
> +
> +CFLAGS += -O3
> +CFLAGS += $(WERROR_FLAGS)
> +
> +# Headers
> +CFLAGS += -I$(RTE_SDK)/lib/librte_mempool
> +
> +EXPORT_MAP := rte_mempool_stack_version.map
> +
> +LIBABIVER := 1
> +
> +SRCS-$(CONFIG_RTE_DRIVER_MEMPOOL_STACK) += rte_mempool_stack.c
> +
> +# this lib depends upon:
> +DEPDIRS-$(CONFIG_RTE_DRIVER_MEMPOOL_STACK) += lib/librte_eal
> +DEPDIRS-$(CONFIG_RTE_DRIVER_MEMPOOL_STACK) += lib/librte_mempool
> +
> +include $(RTE_SDK)/mk/rte.lib.mk
> diff --git a/drivers/mempool/stack/rte_mempool_stack.c b/drivers/mempool/stack/rte_mempool_stack.c
> new file mode 100644
> index 0000000..817f77e
> --- /dev/null
> +++ b/drivers/mempool/stack/rte_mempool_stack.c
> @@ -0,0 +1,147 @@
> +/*-
> + *   BSD LICENSE
> + *
> + *   Copyright(c) 2016 Intel Corporation. All rights reserved.
> + *   All rights reserved.
> + *
> + *   Redistribution and use in source and binary forms, with or without
> + *   modification, are permitted provided that the following conditions
> + *   are met:
> + *
> + *     * Redistributions of source code must retain the above copyright
> + *       notice, this list of conditions and the following disclaimer.
> + *     * Redistributions in binary form must reproduce the above copyright
> + *       notice, this list of conditions and the following disclaimer in
> + *       the documentation and/or other materials provided with the
> + *       distribution.
> + *     * Neither the name of Intel Corporation nor the names of its
> + *       contributors may be used to endorse or promote products derived
> + *       from this software without specific prior written permission.
> + *
> + *   THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS
> + *   "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT
> + *   LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR
> + *   A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT
> + *   OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL,
> + *   SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT
> + *   LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE,
> + *   DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY
> + *   THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
> + *   (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE
> + *   OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
> + */
> +
> +#include <stdio.h>
> +#include <rte_mempool.h>
> +#include <rte_malloc.h>
> +
> +struct rte_mempool_stack {
> +	rte_spinlock_t sl;
> +
> +	uint32_t size;
> +	uint32_t len;
> +	void *objs[];
> +};
> +
> +static int
> +stack_alloc(struct rte_mempool *mp)
> +{
> +	struct rte_mempool_stack *s;
> +	unsigned n = mp->size;
> +	int size = sizeof(*s) + (n+16)*sizeof(void *);
> +
> +	/* Allocate our local memory structure */
> +	s = rte_zmalloc_socket("mempool-stack",
> +			size,
> +			RTE_CACHE_LINE_SIZE,
> +			mp->socket_id);
> +	if (s == NULL) {
> +		RTE_LOG(ERR, MEMPOOL, "Cannot allocate stack!\n");
> +		return -ENOMEM;
> +	}
> +
> +	rte_spinlock_init(&s->sl);
> +
> +	s->size = n;
> +	mp->pool_data = s;
> +
> +	return 0;
> +}
> +
> +static int
> +stack_enqueue(struct rte_mempool *mp, void * const *obj_table,
> +		unsigned n)
> +{
> +	struct rte_mempool_stack *s = mp->pool_data;
> +	void **cache_objs;
> +	unsigned index;
> +
> +	rte_spinlock_lock(&s->sl);
> +	cache_objs = &s->objs[s->len];
> +
> +	/* Is there sufficient space in the stack ? */
> +	if ((s->len + n) > s->size) {
> +		rte_spinlock_unlock(&s->sl);
> +		return -ENOBUFS;
> +	}
> +
> +	/* Add elements back into the cache */
> +	for (index = 0; index < n; ++index, obj_table++)
> +		cache_objs[index] = *obj_table;
> +
> +	s->len += n;
> +
> +	rte_spinlock_unlock(&s->sl);
> +	return 0;
> +}
> +
> +static int
> +stack_dequeue(struct rte_mempool *mp, void **obj_table,
> +		unsigned n)
> +{
> +	struct rte_mempool_stack *s = mp->pool_data;
> +	void **cache_objs;
> +	unsigned index, len;
> +
> +	rte_spinlock_lock(&s->sl);
> +
> +	if (unlikely(n > s->len)) {
> +		rte_spinlock_unlock(&s->sl);
> +		return -ENOENT;
> +	}
> +
> +	cache_objs = s->objs;
> +
> +	for (index = 0, len = s->len - 1; index < n;
> +			++index, len--, obj_table++)
> +		*obj_table = cache_objs[len];
> +
> +	s->len -= n;
> +	rte_spinlock_unlock(&s->sl);
> +	return 0;
> +}
> +
> +static unsigned
> +stack_get_count(const struct rte_mempool *mp)
> +{
> +	struct rte_mempool_stack *s = mp->pool_data;
> +
> +	return s->len;
> +}
> +
> +static void
> +stack_free(struct rte_mempool *mp)
> +{
> +	rte_free((void *)(mp->pool_data));
> +}
> +
> +static struct rte_mempool_ops ops_stack = {
> +	.name = "stack",
> +	.alloc = stack_alloc,
> +	.free = stack_free,
> +	.enqueue = stack_enqueue,
> +	.dequeue = stack_dequeue,
> +	.get_count = stack_get_count
> +};
> +
> +MEMPOOL_REGISTER_OPS(ops_stack);
> diff --git a/drivers/mempool/stack/rte_mempool_stack_version.map b/drivers/mempool/stack/rte_mempool_stack_version.map
> new file mode 100644
> index 0000000..8591cc0
> --- /dev/null
> +++ b/drivers/mempool/stack/rte_mempool_stack_version.map
> @@ -0,0 +1,4 @@
> +DPDK_17.05 {
> +
> +	local: *;
> +};
> diff --git a/lib/librte_mempool/Makefile b/lib/librte_mempool/Makefile
> index 057a6ab..a4c089e 100644
> --- a/lib/librte_mempool/Makefile
> +++ b/lib/librte_mempool/Makefile
> @@ -44,7 +44,6 @@ LIBABIVER := 2
>   SRCS-$(CONFIG_RTE_LIBRTE_MEMPOOL) +=  rte_mempool.c
>   SRCS-$(CONFIG_RTE_LIBRTE_MEMPOOL) +=  rte_mempool_ops.c
>   SRCS-$(CONFIG_RTE_LIBRTE_MEMPOOL) +=  rte_mempool_ring.c
> -SRCS-$(CONFIG_RTE_LIBRTE_MEMPOOL) +=  rte_mempool_stack.c
>   # install includes
>   SYMLINK-$(CONFIG_RTE_LIBRTE_MEMPOOL)-include := rte_mempool.h
>   
> diff --git a/lib/librte_mempool/rte_mempool_stack.c b/lib/librte_mempool/rte_mempool_stack.c
> deleted file mode 100644
> index 817f77e..0000000
> --- a/lib/librte_mempool/rte_mempool_stack.c
> +++ /dev/null
> @@ -1,147 +0,0 @@
> -/*-
> - *   BSD LICENSE
> - *
> - *   Copyright(c) 2016 Intel Corporation. All rights reserved.
> - *   All rights reserved.
> - *
> - *   Redistribution and use in source and binary forms, with or without
> - *   modification, are permitted provided that the following conditions
> - *   are met:
> - *
> - *     * Redistributions of source code must retain the above copyright
> - *       notice, this list of conditions and the following disclaimer.
> - *     * Redistributions in binary form must reproduce the above copyright
> - *       notice, this list of conditions and the following disclaimer in
> - *       the documentation and/or other materials provided with the
> - *       distribution.
> - *     * Neither the name of Intel Corporation nor the names of its
> - *       contributors may be used to endorse or promote products derived
> - *       from this software without specific prior written permission.
> - *
> - *   THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS
> - *   "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT
> - *   LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR
> - *   A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT
> - *   OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL,
> - *   SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT
> - *   LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE,
> - *   DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY
> - *   THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
> - *   (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE
> - *   OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
> - */
> -
> -#include <stdio.h>
> -#include <rte_mempool.h>
> -#include <rte_malloc.h>
> -
> -struct rte_mempool_stack {
> -	rte_spinlock_t sl;
> -
> -	uint32_t size;
> -	uint32_t len;
> -	void *objs[];
> -};
> -
> -static int
> -stack_alloc(struct rte_mempool *mp)
> -{
> -	struct rte_mempool_stack *s;
> -	unsigned n = mp->size;
> -	int size = sizeof(*s) + (n+16)*sizeof(void *);
> -
> -	/* Allocate our local memory structure */
> -	s = rte_zmalloc_socket("mempool-stack",
> -			size,
> -			RTE_CACHE_LINE_SIZE,
> -			mp->socket_id);
> -	if (s == NULL) {
> -		RTE_LOG(ERR, MEMPOOL, "Cannot allocate stack!\n");
> -		return -ENOMEM;
> -	}
> -
> -	rte_spinlock_init(&s->sl);
> -
> -	s->size = n;
> -	mp->pool_data = s;
> -
> -	return 0;
> -}
> -
> -static int
> -stack_enqueue(struct rte_mempool *mp, void * const *obj_table,
> -		unsigned n)
> -{
> -	struct rte_mempool_stack *s = mp->pool_data;
> -	void **cache_objs;
> -	unsigned index;
> -
> -	rte_spinlock_lock(&s->sl);
> -	cache_objs = &s->objs[s->len];
> -
> -	/* Is there sufficient space in the stack ? */
> -	if ((s->len + n) > s->size) {
> -		rte_spinlock_unlock(&s->sl);
> -		return -ENOBUFS;
> -	}
> -
> -	/* Add elements back into the cache */
> -	for (index = 0; index < n; ++index, obj_table++)
> -		cache_objs[index] = *obj_table;
> -
> -	s->len += n;
> -
> -	rte_spinlock_unlock(&s->sl);
> -	return 0;
> -}
> -
> -static int
> -stack_dequeue(struct rte_mempool *mp, void **obj_table,
> -		unsigned n)
> -{
> -	struct rte_mempool_stack *s = mp->pool_data;
> -	void **cache_objs;
> -	unsigned index, len;
> -
> -	rte_spinlock_lock(&s->sl);
> -
> -	if (unlikely(n > s->len)) {
> -		rte_spinlock_unlock(&s->sl);
> -		return -ENOENT;
> -	}
> -
> -	cache_objs = s->objs;
> -
> -	for (index = 0, len = s->len - 1; index < n;
> -			++index, len--, obj_table++)
> -		*obj_table = cache_objs[len];
> -
> -	s->len -= n;
> -	rte_spinlock_unlock(&s->sl);
> -	return 0;
> -}
> -
> -static unsigned
> -stack_get_count(const struct rte_mempool *mp)
> -{
> -	struct rte_mempool_stack *s = mp->pool_data;
> -
> -	return s->len;
> -}
> -
> -static void
> -stack_free(struct rte_mempool *mp)
> -{
> -	rte_free((void *)(mp->pool_data));
> -}
> -
> -static struct rte_mempool_ops ops_stack = {
> -	.name = "stack",
> -	.alloc = stack_alloc,
> -	.free = stack_free,
> -	.enqueue = stack_enqueue,
> -	.dequeue = stack_dequeue,
> -	.get_count = stack_get_count
> -};
> -
> -MEMPOOL_REGISTER_OPS(ops_stack);

Shreyansh,
     Could I suggest you add the parameter "--find-renames" when 
generating the patch files, as this will reduce the size of the patches 
significantly, making for easier review. The patch line count in this 
particular case would be reduced by approx 75%.
Regards,
Dave.

Re: [PATCH 1/2] drivers/mempool: add stack mempool handler as driver

[Shreyansh Jain]

Hello David,

On Monday 20 March 2017 08:20 PM, Hunt, David wrote:
>
> On 20/3/2017 10:03 AM, Shreyansh Jain wrote:
>> CONFIG_RTE_DRIVER_MEMPOOL_STACK option added to common_base.
>> Stack mempool handler moved from lib/librte_mempool into drivers/mempool.
>>

<...>

>> -}
>> -
>> -static struct rte_mempool_ops ops_stack = {
>> -    .name = "stack",
>> -    .alloc = stack_alloc,
>> -    .free = stack_free,
>> -    .enqueue = stack_enqueue,
>> -    .dequeue = stack_dequeue,
>> -    .get_count = stack_get_count
>> -};
>> -
>> -MEMPOOL_REGISTER_OPS(ops_stack);
>
> Shreyansh,
>     Could I suggest you add the parameter "--find-renames" when
> generating the patch files, as this will reduce the size of the patches
> significantly, making for easier review. The patch line count in this
> particular case would be reduced by approx 75%.

Thanks for suggestion.
Yes, I forgot to use this option while creating this patch. If there
are comments and v2 needs to be created, I will keep this in mind.

> Regards,
> Dave.
>
>

Re: [PATCH 1/2] drivers/mempool: add stack mempool handler as driver

[Wiles, Keith]

> On Mar 20, 2017, at 11:55 PM, Shreyansh Jain <shreyansh.jain@nxp.com> wrote:
> 
> Hello David,
> 
> On Monday 20 March 2017 08:20 PM, Hunt, David wrote:
>> 
>> On 20/3/2017 10:03 AM, Shreyansh Jain wrote:
>>> CONFIG_RTE_DRIVER_MEMPOOL_STACK option added to common_base.
>>> Stack mempool handler moved from lib/librte_mempool into drivers/mempool.
>>> 
> 
> <...>
> 
>>> -}
>>> -
>>> -static struct rte_mempool_ops ops_stack = {
>>> -    .name = "stack",
>>> -    .alloc = stack_alloc,
>>> -    .free = stack_free,
>>> -    .enqueue = stack_enqueue,
>>> -    .dequeue = stack_dequeue,
>>> -    .get_count = stack_get_count
>>> -};
>>> -
>>> -MEMPOOL_REGISTER_OPS(ops_stack);
>> 
>> Shreyansh,
>>    Could I suggest you add the parameter "--find-renames" when
>> generating the patch files, as this will reduce the size of the patches
>> significantly, making for easier review. The patch line count in this
>> particular case would be reduced by approx 75%.
> 
> Thanks for suggestion.
> Yes, I forgot to use this option while creating this patch. If there
> are comments and v2 needs to be created, I will keep this in mind.
> 
>> Regards,
>> Dave.

I guess I missed an email, but what is the advantage of moving the ring/stack files to the drivers directory as they are not drivers in the sense of a NIC PMD or any other driver. You can still enable/disable them in the config files right?

Regards,
Keith

Re: [PATCH 1/2] drivers/mempool: add stack mempool handler as driver

[Shreyansh Jain]

Hello Keith,

On Tuesday 21 March 2017 11:32 AM, Wiles, Keith wrote:
>
>> On Mar 20, 2017, at 11:55 PM, Shreyansh Jain <shreyansh.jain@nxp.com> wrote:
>>
>> Hello David,
>>
>> On Monday 20 March 2017 08:20 PM, Hunt, David wrote:
>>>
>>> On 20/3/2017 10:03 AM, Shreyansh Jain wrote:
>>>> CONFIG_RTE_DRIVER_MEMPOOL_STACK option added to common_base.
>>>> Stack mempool handler moved from lib/librte_mempool into drivers/mempool.
>>>>
>>
>> <...>
>>
>>>> -}
>>>> -
>>>> -static struct rte_mempool_ops ops_stack = {
>>>> -    .name = "stack",
>>>> -    .alloc = stack_alloc,
>>>> -    .free = stack_free,
>>>> -    .enqueue = stack_enqueue,
>>>> -    .dequeue = stack_dequeue,
>>>> -    .get_count = stack_get_count
>>>> -};
>>>> -
>>>> -MEMPOOL_REGISTER_OPS(ops_stack);
>>>
>>> Shreyansh,
>>>    Could I suggest you add the parameter "--find-renames" when
>>> generating the patch files, as this will reduce the size of the patches
>>> significantly, making for easier review. The patch line count in this
>>> particular case would be reduced by approx 75%.
>>
>> Thanks for suggestion.
>> Yes, I forgot to use this option while creating this patch. If there
>> are comments and v2 needs to be created, I will keep this in mind.
>>
>>> Regards,
>>> Dave.
>
> I guess I missed an email, but what is the advantage of moving the ring/stack files to the drivers directory as they are not drivers in the sense of a NIC PMD or any other driver. You can still enable/disable them in the config files right?
>

Just as reference, following is where this was being discussed:

http://dpdk.org/ml/archives/dev/2017-March/059690.html
http://dpdk.org/ml/archives/dev/2017-March/059753.html
and
http://dpdk.org/ml/archives/dev/2017-March/060501.html

Also, a while back (I can't trace that mailing list exchange), it was
decided that all mempool drivers (stack, ring, others...) would be
moved to drivers/mempool.

For NXP's DPAA2 PMD, we use an offloaded mempool for which there was a
patchset by Hemant [1] which adds that driver to drivers/mempool. In
the same breadth, ring and stack are also being moved to
drivers/mempool as independent drivers (non-offloaded category).

[1] http://dpdk.org/ml/archives/dev/2017-March/060476.html

In my opinion, this would make the lib/* area free of handler/drivers
(almost) and it is a good change. Also, ring and stack use a 
'registration' mechanism - just like PMD and are good candidate to be 
treated as 'drivers' now even though not entirely like a PMD.

You see any downside of this?

> Regards,
> Keith
>
>

-
Shreyansh

Re: [PATCH 1/2] drivers/mempool: add stack mempool handler as driver

[Shreyansh Jain]

On Tuesday 21 March 2017 11:55 AM, Shreyansh Jain wrote:
> Hello Keith,
>
> On Tuesday 21 March 2017 11:32 AM, Wiles, Keith wrote:
>>
>>> On Mar 20, 2017, at 11:55 PM, Shreyansh Jain <shreyansh.jain@nxp.com>
>>> wrote:
>>>
>>> Hello David,
>>>
>>> On Monday 20 March 2017 08:20 PM, Hunt, David wrote:
>>>>
>>>> On 20/3/2017 10:03 AM, Shreyansh Jain wrote:
>>>>> CONFIG_RTE_DRIVER_MEMPOOL_STACK option added to common_base.
>>>>> Stack mempool handler moved from lib/librte_mempool into
>>>>> drivers/mempool.
>>>>>
>>>
>>> <...>
>>>
>>>>> -}
>>>>> -
>>>>> -static struct rte_mempool_ops ops_stack = {
>>>>> -    .name = "stack",
>>>>> -    .alloc = stack_alloc,
>>>>> -    .free = stack_free,
>>>>> -    .enqueue = stack_enqueue,
>>>>> -    .dequeue = stack_dequeue,
>>>>> -    .get_count = stack_get_count
>>>>> -};
>>>>> -
>>>>> -MEMPOOL_REGISTER_OPS(ops_stack);
>>>>
>>>> Shreyansh,
>>>>    Could I suggest you add the parameter "--find-renames" when
>>>> generating the patch files, as this will reduce the size of the patches
>>>> significantly, making for easier review. The patch line count in this
>>>> particular case would be reduced by approx 75%.
>>>
>>> Thanks for suggestion.
>>> Yes, I forgot to use this option while creating this patch. If there
>>> are comments and v2 needs to be created, I will keep this in mind.
>>>
>>>> Regards,
>>>> Dave.
>>
>> I guess I missed an email, but what is the advantage of moving the
>> ring/stack files to the drivers directory as they are not drivers in
>> the sense of a NIC PMD or any other driver. You can still
>> enable/disable them in the config files right?
>>
>
> Just as reference, following is where this was being discussed:
>
> http://dpdk.org/ml/archives/dev/2017-March/059690.html
> http://dpdk.org/ml/archives/dev/2017-March/059753.html
> and
> http://dpdk.org/ml/archives/dev/2017-March/060501.html
>
> Also, a while back (I can't trace that mailing list exchange), it was
> decided that all mempool drivers (stack, ring, others...) would be
> moved to drivers/mempool.

Just to add, discussion at that time was to have:

drivers/bus/<buses using rte_bus>
drivers/mempool/<Offloaded, or not-offloaded memory pool handlers>
drivers/net/<usual PMDs>

There was a drivers/common as well, but somehow it didn't go down well 
in discussions.

>
> For NXP's DPAA2 PMD, we use an offloaded mempool for which there was a
> patchset by Hemant [1] which adds that driver to drivers/mempool. In
> the same breadth, ring and stack are also being moved to
> drivers/mempool as independent drivers (non-offloaded category).
>
> [1] http://dpdk.org/ml/archives/dev/2017-March/060476.html
>
> In my opinion, this would make the lib/* area free of handler/drivers
> (almost) and it is a good change. Also, ring and stack use a
> 'registration' mechanism - just like PMD and are good candidate to be
> treated as 'drivers' now even though not entirely like a PMD.
>
> You see any downside of this?
>
>> Regards,
>> Keith
>>
>>
>
> -
> Shreyansh
>
>

Re: [PATCH 1/2] drivers/mempool: add stack mempool handler as driver

[Wiles, Keith]

> On Mar 21, 2017, at 1:25 AM, Shreyansh Jain <shreyansh.jain@nxp.com> wrote:
> 
> Hello Keith,
> 
> On Tuesday 21 March 2017 11:32 AM, Wiles, Keith wrote:
>> 
>>> On Mar 20, 2017, at 11:55 PM, Shreyansh Jain <shreyansh.jain@nxp.com> wrote:
>>> 
>>> Hello David,
>>> 
>>> On Monday 20 March 2017 08:20 PM, Hunt, David wrote:
>>>> 
>>>> On 20/3/2017 10:03 AM, Shreyansh Jain wrote:
>>>>> CONFIG_RTE_DRIVER_MEMPOOL_STACK option added to common_base.
>>>>> Stack mempool handler moved from lib/librte_mempool into drivers/mempool.
>>>>> 
>>> 
>>> <...>
>>> 
>>>>> -}
>>>>> -
>>>>> -static struct rte_mempool_ops ops_stack = {
>>>>> -    .name = "stack",
>>>>> -    .alloc = stack_alloc,
>>>>> -    .free = stack_free,
>>>>> -    .enqueue = stack_enqueue,
>>>>> -    .dequeue = stack_dequeue,
>>>>> -    .get_count = stack_get_count
>>>>> -};
>>>>> -
>>>>> -MEMPOOL_REGISTER_OPS(ops_stack);
>>>> 
>>>> Shreyansh,
>>>>   Could I suggest you add the parameter "--find-renames" when
>>>> generating the patch files, as this will reduce the size of the patches
>>>> significantly, making for easier review. The patch line count in this
>>>> particular case would be reduced by approx 75%.
>>> 
>>> Thanks for suggestion.
>>> Yes, I forgot to use this option while creating this patch. If there
>>> are comments and v2 needs to be created, I will keep this in mind.
>>> 
>>>> Regards,
>>>> Dave.
>> 
>> I guess I missed an email, but what is the advantage of moving the ring/stack files to the drivers directory as they are not drivers in the sense of a NIC PMD or any other driver. You can still enable/disable them in the config files right?
>> 
> 
> Just as reference, following is where this was being discussed:
> 
> http://dpdk.org/ml/archives/dev/2017-March/059690.html
> http://dpdk.org/ml/archives/dev/2017-March/059753.html
> and
> http://dpdk.org/ml/archives/dev/2017-March/060501.html
> 
> Also, a while back (I can't trace that mailing list exchange), it was
> decided that all mempool drivers (stack, ring, others...) would be
> moved to drivers/mempool.
> 
> For NXP's DPAA2 PMD, we use an offloaded mempool for which there was a
> patchset by Hemant [1] which adds that driver to drivers/mempool. In
> the same breadth, ring and stack are also being moved to
> drivers/mempool as independent drivers (non-offloaded category).
> 
> [1] http://dpdk.org/ml/archives/dev/2017-March/060476.html
> 
> In my opinion, this would make the lib/* area free of handler/drivers
> (almost) and it is a good change. Also, ring and stack use a 'registration' mechanism - just like PMD and are good candidate to be treated as 'drivers' now even though not entirely like a PMD.
> 
> You see any downside of this?

OK, I am up to date now. The only downside is splitting the mempool code up and making it a bit harder to manage, but it is a minor point. I do not see any stopper to prevent the change. Thanks.

> 
>> Regards,
>> Keith
>> 
>> 
> 
> -
> Shreyansh

Regards,
Keith

Re: [PATCH 1/2] drivers/mempool: add stack mempool handler as driver

[Olivier Matz]

Hi Shreyansh,

On Mon, 20 Mar 2017 15:33:09 +0530, Shreyansh Jain <shreyansh.jain@nxp.com> wrote:
> CONFIG_RTE_DRIVER_MEMPOOL_STACK option added to common_base.
> Stack mempool handler moved from lib/librte_mempool into drivers/mempool.
> 
> With this patch, the Stack mempool handler registration is optional and
> toggled via the configuration file. In case disabled (N), it would imply
> request for creating of mempool would fail.
> 
> Signed-off-by: Shreyansh Jain <shreyansh.jain@nxp.com>
> ---
>  config/common_base                                 |   5 +
>  drivers/Makefile                                   |   1 +
>  drivers/mempool/Makefile                           |  36 +++++
>  drivers/mempool/stack/Makefile                     |  55 ++++++++
>  drivers/mempool/stack/rte_mempool_stack.c          | 147 +++++++++++++++++++++
>  .../mempool/stack/rte_mempool_stack_version.map    |   4 +
>  lib/librte_mempool/Makefile                        |   1 -
>  lib/librte_mempool/rte_mempool_stack.c             | 147 ---------------------
>  8 files changed, 248 insertions(+), 148 deletions(-)
>  create mode 100644 drivers/mempool/Makefile
>  create mode 100644 drivers/mempool/stack/Makefile
>  create mode 100644 drivers/mempool/stack/rte_mempool_stack.c
>  create mode 100644 drivers/mempool/stack/rte_mempool_stack_version.map
>  delete mode 100644 lib/librte_mempool/rte_mempool_stack.c



I tried to pass the mempool autotest, and it issues a segfault.
I think the libraries are missing in rte.app.mk, so no handler is
registered.

Adding the following code in lib/librte_mempool/rte_mempool_ops.c
fixes the crash.

        ops = rte_mempool_get_ops(mp->ops_index);
+       if (ops == NULL || ops->alloc == NULL)
+               return -ENOTSUP;
        return ops->alloc(mp);

Now that drivers are not linked to the mempool library, it can
happen that there is no handler. Could you please add this patch in your
patchset?

I also checked that compilation works in shared lib mode. It looks good,
but I think there will be a behavior change if CONFIG_RTE_EAL_PMD_PATH
is empty (default). This option is probably used by distros to indicate
where dpdk plugins are installed, and when it is set, all drivers of
this directory are loaded, so in that case it won't change.

But when unset, the drivers have to be loaded manually, and with this
change, the mempool driver will have to be loaded with the -d eal option.
Could you please check what occurs in that case? At least see if it
displays a nice error or if it crashes. I suspect it will crash

Also, the MAINTAINERS file should be updated.


Thanks,
Olivier

Re: [PATCH 1/2] drivers/mempool: add stack mempool handler as driver

[Shreyansh Jain]

Hello Olivier,

On Friday 24 March 2017 09:52 PM, Olivier Matz wrote:
> Hi Shreyansh,
>
> On Mon, 20 Mar 2017 15:33:09 +0530, Shreyansh Jain <shreyansh.jain@nxp.com> wrote:
>> CONFIG_RTE_DRIVER_MEMPOOL_STACK option added to common_base.
>> Stack mempool handler moved from lib/librte_mempool into drivers/mempool.
>>
>> With this patch, the Stack mempool handler registration is optional and
>> toggled via the configuration file. In case disabled (N), it would imply
>> request for creating of mempool would fail.
>>
>> Signed-off-by: Shreyansh Jain <shreyansh.jain@nxp.com>
>> ---
>>  config/common_base                                 |   5 +
>>  drivers/Makefile                                   |   1 +
>>  drivers/mempool/Makefile                           |  36 +++++
>>  drivers/mempool/stack/Makefile                     |  55 ++++++++
>>  drivers/mempool/stack/rte_mempool_stack.c          | 147 +++++++++++++++++++++
>>  .../mempool/stack/rte_mempool_stack_version.map    |   4 +
>>  lib/librte_mempool/Makefile                        |   1 -
>>  lib/librte_mempool/rte_mempool_stack.c             | 147 ---------------------
>>  8 files changed, 248 insertions(+), 148 deletions(-)
>>  create mode 100644 drivers/mempool/Makefile
>>  create mode 100644 drivers/mempool/stack/Makefile
>>  create mode 100644 drivers/mempool/stack/rte_mempool_stack.c
>>  create mode 100644 drivers/mempool/stack/rte_mempool_stack_version.map
>>  delete mode 100644 lib/librte_mempool/rte_mempool_stack.c
>
>
>
> I tried to pass the mempool autotest, and it issues a segfault.
> I think the libraries are missing in rte.app.mk, so no handler is
> registered.
>
> Adding the following code in lib/librte_mempool/rte_mempool_ops.c
> fixes the crash.
>
>         ops = rte_mempool_get_ops(mp->ops_index);
> +       if (ops == NULL || ops->alloc == NULL)
> +               return -ENOTSUP;
>         return ops->alloc(mp);
>
> Now that drivers are not linked to the mempool library, it can
> happen that there is no handler. Could you please add this patch in your
> patchset?

Indeed. I will add this code set as first patch. Thanks for suggestion/fix.

>
> I also checked that compilation works in shared lib mode. It looks good,
> but I think there will be a behavior change if CONFIG_RTE_EAL_PMD_PATH
> is empty (default). This option is probably used by distros to indicate
> where dpdk plugins are installed, and when it is set, all drivers of
> this directory are loaded, so in that case it won't change.
>
> But when unset, the drivers have to be loaded manually, and with this
> change, the mempool driver will have to be loaded with the -d eal option.
> Could you please check what occurs in that case? At least see if it
> displays a nice error or if it crashes. I suspect it will crash

Ok. I will try this and if there is any issue, fix it.

>
> Also, the MAINTAINERS file should be updated.

Yes, that is something I thought of updating but left it out before 
sending the patch.
One confirmation: I assume that maintainers need to be added with
"drivers/mempool/ring" and "drivers/mempool/stack" folders.
Who should be marked as maintainer - You (because of existing
lib/librte_mempool ownership) or I (because I have moved the code from
lib/librte_mempool)?

I think it would continue to be you but wanted to take your
confirmation before adding the lines.

>
>
> Thanks,
> Olivier
>
>

-
Shreyansh

Re: [PATCH 1/2] drivers/mempool: add stack mempool handler as driver

[Olivier Matz]

Hi Shreyansh,

[...]

> > Also, the MAINTAINERS file should be updated.  
> 
> Yes, that is something I thought of updating but left it out before 
> sending the patch.
> One confirmation: I assume that maintainers need to be added with
> "drivers/mempool/ring" and "drivers/mempool/stack" folders.
> Who should be marked as maintainer - You (because of existing
> lib/librte_mempool ownership) or I (because I have moved the code from
> lib/librte_mempool)?
> 
> I think it would continue to be you but wanted to take your
> confirmation before adding the lines.

Yes, I'll continue to maintain ring and stack handlers.

Thanks
Olivier

Re: [PATCH 1/2] drivers/mempool: add stack mempool handler as driver

[Shreyansh Jain]

Hello Olivier,

On Friday 24 March 2017 09:52 PM, Olivier Matz wrote:
[..]

> I tried to pass the mempool autotest, and it issues a segfault.
> I think the libraries are missing in rte.app.mk, so no handler is
> registered.

I have been trying to simulate the segfault that you are referring to 
above. But, I think it should not be the case. If a mempool handler is 
not registered (as librte_mempool_ring was not included in 
mk/rte.app.mk, so, no "ring_mp_mc"), the caller would get error.

The mempool_autotest is reporting:

--->8--
RTE>>mempool_autotest
cannot allocate mp_nocache mempool
Test Failed
--->8--

>
> Adding the following code in lib/librte_mempool/rte_mempool_ops.c
> fixes the crash.
>
>         ops = rte_mempool_get_ops(mp->ops_index);
> +       if (ops == NULL || ops->alloc == NULL)
> +               return -ENOTSUP;
>         return ops->alloc(mp);

Can you tell me for which case did your code reach 
rte_mempool_ops_alloc() and segfault?

In my case, librte_mempool_ring and librte_mempool_stack are not added 
to mk/rte.app.mk and it is static compilation.

>
> Now that drivers are not linked to the mempool library, it can
> happen that there is no handler. Could you please add this patch in your
> patchset?

Yes, once I can get this issue reproduced. Because I think there is one 
more place similar code should go (rte_mempool_ops_getcount).
As per what I can see, this would only happen if rte_mempool_xmem_create 
is called and then directly alloc is called. That is not happening for 
mempool_autotest.

-
Shreyansh

Re: [PATCH 1/2] drivers/mempool: add stack mempool handler as driver

[Olivier Matz]

On Tue, 28 Mar 2017 17:12:47 +0530, Shreyansh Jain <shreyansh.jain@nxp.com> wrote:
> Hello Olivier,
> 
> On Friday 24 March 2017 09:52 PM, Olivier Matz wrote:
> [..]
> 
> > I tried to pass the mempool autotest, and it issues a segfault.
> > I think the libraries are missing in rte.app.mk, so no handler is
> > registered.  
> 
> I have been trying to simulate the segfault that you are referring to 
> above. But, I think it should not be the case. If a mempool handler is 
> not registered (as librte_mempool_ring was not included in 
> mk/rte.app.mk, so, no "ring_mp_mc"), the caller would get error.
> 
> The mempool_autotest is reporting:
> 
> --->8--  
> RTE>>mempool_autotest  
> cannot allocate mp_nocache mempool
> Test Failed
> --->8--  

Here are the reproduction steps:


git clone http://dpdk.org/git/dpdk
cd dpdk/
wget -O - http://dpdk.org/dev/patchwork/patch/21986/mbox | git am -
wget -O - http://dpdk.org/dev/patchwork/patch/21985/mbox | git am -
make config T=x86_64-native-linuxapp-gcc
make -j32 test-build
echo 128 > /sys/devices/system/node/node0/hugepages/hugepages-2048kB/nr_hugepages
mkdir -p /mnt/huge
mount -t hugetlbfs none /mnt/huge
echo mempool_autotest | ./build/app/test --
# segfault

# replay with debug
make -j32 test-build
make -j32 EXTRA_CFLAGS="-O0 -g" test-build
ulimit -c unlimited
echo mempool_autotest | ./build/app/test --
# segfault + core dump
gdb -c core ./build/app/test

(gdb) bt
#1  0x000000000064dead in rte_mempool_ops_alloc (mp=0x7f8816abdb40)
    at /root/dpdk/lib/librte_mempool/rte_mempool_ops.c:101
#2  0x000000000064c1e7 in rte_mempool_populate_phys (mp=0x7f8816abdb40, 
    vaddr=0x7f880987a800 <error: Cannot access memory at address 0x7f880987a800>, 
    paddr=6958852096, len=26761152, free_cb=0x64c032 <rte_mempool_memchunk_mz_free>, 
    opaque=0x7f8822334d4c) at /root/dpdk/lib/librte_mempool/rte_mempool.c:359
#3  0x000000000064c9db in rte_mempool_populate_default (mp=0x7f8816abdb40)
    at /root/dpdk/lib/librte_mempool/rte_mempool.c:572
#4  0x000000000064d3d4 in rte_mempool_create (name=0x9b1ff0 "test_nocache", n=12671, 
    elt_size=2048, cache_size=0, private_data_size=0, mp_init=0x0, mp_init_arg=0x0, 
    obj_init=0x49f309 <my_obj_init>, obj_init_arg=0x0, socket_id=-1, flags=0)
    at /root/dpdk/lib/librte_mempool/rte_mempool.c:895
#5  0x00000000004a20ed in test_mempool () at /root/dpdk/test/test/test_mempool.c:519
#6  0x0000000000435189 in cmd_autotest_parsed (parsed_result=0x7ffe55006420, 
    cl=0x7c87090, data=0x0) at /root/dpdk/test/test/commands.c:103
#7  0x00000000006749df in cmdline_parse (cl=0x7c87090, 
    buf=0x7c870d8 "mempool_autotest\n")
    at /root/dpdk/lib/librte_cmdline/cmdline_parse.c:359
(gdb) up
#1  0x000000000064dead in rte_mempool_ops_alloc (mp=0x7f8816abdb40)
    at /root/dpdk/lib/librte_mempool/rte_mempool_ops.c:101
101             return ops->alloc(mp);
(gdb) print ops
$1 = (struct rte_mempool_ops *) 0x4e69c00 <rte_mempool_ops_table+64>
(gdb) print *ops
$2 = {name = '\000' <repeats 31 times>, alloc = 0x0, free = 0x0, enqueue = 0x0, 
  dequeue = 0x0, get_count = 0x0}


Regards,
Olivier


> 
> >
> > Adding the following code in lib/librte_mempool/rte_mempool_ops.c
> > fixes the crash.
> >
> >         ops = rte_mempool_get_ops(mp->ops_index);
> > +       if (ops == NULL || ops->alloc == NULL)
> > +               return -ENOTSUP;
> >         return ops->alloc(mp);  
> 
> Can you tell me for which case did your code reach 
> rte_mempool_ops_alloc() and segfault?
> 
> In my case, librte_mempool_ring and librte_mempool_stack are not added 
> to mk/rte.app.mk and it is static compilation.
> 
> >
> > Now that drivers are not linked to the mempool library, it can
> > happen that there is no handler. Could you please add this patch in your
> > patchset?  
> 
> Yes, once I can get this issue reproduced. Because I think there is one 
> more place similar code should go (rte_mempool_ops_getcount).
> As per what I can see, this would only happen if rte_mempool_xmem_create 
> is called and then directly alloc is called. That is not happening for 
> mempool_autotest.
> 
> -
> Shreyansh
> 

Re: [PATCH 1/2] drivers/mempool: add stack mempool handler as driver

[Shreyansh Jain]

Hello Olivier,

On Wednesday 29 March 2017 01:48 PM, Olivier Matz wrote:
> On Tue, 28 Mar 2017 17:12:47 +0530, Shreyansh Jain <shreyansh.jain@nxp.com> wrote:
>> Hello Olivier,
>>
>> On Friday 24 March 2017 09:52 PM, Olivier Matz wrote:
>> [..]
>>
>>> I tried to pass the mempool autotest, and it issues a segfault.
>>> I think the libraries are missing in rte.app.mk, so no handler is
>>> registered.
>>
>> I have been trying to simulate the segfault that you are referring to
>> above. But, I think it should not be the case. If a mempool handler is
>> not registered (as librte_mempool_ring was not included in
>> mk/rte.app.mk, so, no "ring_mp_mc"), the caller would get error.
>>
>> The mempool_autotest is reporting:
>>
>> --->8--
>> RTE>>mempool_autotest
>> cannot allocate mp_nocache mempool
>> Test Failed
>> --->8--
>
> Here are the reproduction steps:
>
>
> git clone http://dpdk.org/git/dpdk
> cd dpdk/
> wget -O - http://dpdk.org/dev/patchwork/patch/21986/mbox | git am -
> wget -O - http://dpdk.org/dev/patchwork/patch/21985/mbox | git am -
> make config T=x86_64-native-linuxapp-gcc
> make -j32 test-build
> echo 128 > /sys/devices/system/node/node0/hugepages/hugepages-2048kB/nr_hugepages
> mkdir -p /mnt/huge
> mount -t hugetlbfs none /mnt/huge
> echo mempool_autotest | ./build/app/test --
> # segfault

Thanks for the steps. I was able to reproduce this.
Don't know why it didn't work earlier.
one more comment below...

>
> # replay with debug
> make -j32 test-build
> make -j32 EXTRA_CFLAGS="-O0 -g" test-build
> ulimit -c unlimited
> echo mempool_autotest | ./build/app/test --
> # segfault + core dump
> gdb -c core ./build/app/test
>
> (gdb) bt
> #1  0x000000000064dead in rte_mempool_ops_alloc (mp=0x7f8816abdb40)
>     at /root/dpdk/lib/librte_mempool/rte_mempool_ops.c:101
> #2  0x000000000064c1e7 in rte_mempool_populate_phys (mp=0x7f8816abdb40,
>     vaddr=0x7f880987a800 <error: Cannot access memory at address 0x7f880987a800>,
>     paddr=6958852096, len=26761152, free_cb=0x64c032 <rte_mempool_memchunk_mz_free>,
>     opaque=0x7f8822334d4c) at /root/dpdk/lib/librte_mempool/rte_mempool.c:359
> #3  0x000000000064c9db in rte_mempool_populate_default (mp=0x7f8816abdb40)
>     at /root/dpdk/lib/librte_mempool/rte_mempool.c:572

I think adding the code that you suggested is not the right place. The 
problem is not in rte_mempool_ops_alloc, where you had suggested the 
check for NULL.

The problem is in rte_mempool_create where return value for 
rte_mempool_set_ops_byname is not being checked.

When the libraries are not statically compiled in, 
rte_mempool_set_ops_byname is returning NULL, which rte_mempool_create 
doesn't handle and goes on to call rte_mempool_ops_alloc - eventually 
segfaulting.


> #4  0x000000000064d3d4 in rte_mempool_create (name=0x9b1ff0 "test_nocache", n=12671,
>     elt_size=2048, cache_size=0, private_data_size=0, mp_init=0x0, mp_init_arg=0x0,
>     obj_init=0x49f309 <my_obj_init>, obj_init_arg=0x0, socket_id=-1, flags=0)
>     at /root/dpdk/lib/librte_mempool/rte_mempool.c:895
> #5  0x00000000004a20ed in test_mempool () at /root/dpdk/test/test/test_mempool.c:519
> #6  0x0000000000435189 in cmd_autotest_parsed (parsed_result=0x7ffe55006420,
>     cl=0x7c87090, data=0x0) at /root/dpdk/test/test/commands.c:103
> #7  0x00000000006749df in cmdline_parse (cl=0x7c87090,
>     buf=0x7c870d8 "mempool_autotest\n")
>     at /root/dpdk/lib/librte_cmdline/cmdline_parse.c:359
> (gdb) up
> #1  0x000000000064dead in rte_mempool_ops_alloc (mp=0x7f8816abdb40)
>     at /root/dpdk/lib/librte_mempool/rte_mempool_ops.c:101
> 101             return ops->alloc(mp);
> (gdb) print ops
> $1 = (struct rte_mempool_ops *) 0x4e69c00 <rte_mempool_ops_table+64>
> (gdb) print *ops
> $2 = {name = '\000' <repeats 31 times>, alloc = 0x0, free = 0x0, enqueue = 0x0,
>   dequeue = 0x0, get_count = 0x0}
>
>
> Regards,
> Olivier
>
>
>>
>>>
>>> Adding the following code in lib/librte_mempool/rte_mempool_ops.c
>>> fixes the crash.
>>>
>>>         ops = rte_mempool_get_ops(mp->ops_index);
>>> +       if (ops == NULL || ops->alloc == NULL)
>>> +               return -ENOTSUP;
>>>         return ops->alloc(mp);

If you think above explanation suffices, I will push a patch for error 
handling in rte_mempool_create returned by rte_mempool_set_ops_byname 
rather than above change originally suggested by you.

>>
>> Can you tell me for which case did your code reach
>> rte_mempool_ops_alloc() and segfault?
>>
>> In my case, librte_mempool_ring and librte_mempool_stack are not added
>> to mk/rte.app.mk and it is static compilation.
>>
>>>
>>> Now that drivers are not linked to the mempool library, it can
>>> happen that there is no handler. Could you please add this patch in your
>>> patchset?
>>
>> Yes, once I can get this issue reproduced. Because I think there is one
>> more place similar code should go (rte_mempool_ops_getcount).
>> As per what I can see, this would only happen if rte_mempool_xmem_create
>> is called and then directly alloc is called. That is not happening for
>> mempool_autotest.
>>
>> -
>> Shreyansh
>>
>
>

Re: [PATCH 1/2] drivers/mempool: add stack mempool handler as driver

[Olivier Matz]

On Wed, 29 Mar 2017 18:25:31 +0530, Shreyansh Jain <shreyansh.jain@nxp.com> wrote:
> Hello Olivier,
> 
> On Wednesday 29 March 2017 01:48 PM, Olivier Matz wrote:
> > On Tue, 28 Mar 2017 17:12:47 +0530, Shreyansh Jain <shreyansh.jain@nxp.com> wrote:  
> >> Hello Olivier,
> >>
> >> On Friday 24 March 2017 09:52 PM, Olivier Matz wrote:
> >> [..]
> >>  
> >>> I tried to pass the mempool autotest, and it issues a segfault.
> >>> I think the libraries are missing in rte.app.mk, so no handler is
> >>> registered.  
> >>
> >> I have been trying to simulate the segfault that you are referring to
> >> above. But, I think it should not be the case. If a mempool handler is
> >> not registered (as librte_mempool_ring was not included in
> >> mk/rte.app.mk, so, no "ring_mp_mc"), the caller would get error.
> >>
> >> The mempool_autotest is reporting:
> >>  
> >> --->8--  
> >> RTE>>mempool_autotest  
> >> cannot allocate mp_nocache mempool
> >> Test Failed  
> >> --->8--  
> >
> > Here are the reproduction steps:
> >
> >
> > git clone http://dpdk.org/git/dpdk
> > cd dpdk/
> > wget -O - http://dpdk.org/dev/patchwork/patch/21986/mbox | git am -
> > wget -O - http://dpdk.org/dev/patchwork/patch/21985/mbox | git am -
> > make config T=x86_64-native-linuxapp-gcc
> > make -j32 test-build
> > echo 128 > /sys/devices/system/node/node0/hugepages/hugepages-2048kB/nr_hugepages
> > mkdir -p /mnt/huge
> > mount -t hugetlbfs none /mnt/huge
> > echo mempool_autotest | ./build/app/test --
> > # segfault  
> 
> Thanks for the steps. I was able to reproduce this.
> Don't know why it didn't work earlier.
> one more comment below...
> 
> >
> > # replay with debug
> > make -j32 test-build
> > make -j32 EXTRA_CFLAGS="-O0 -g" test-build
> > ulimit -c unlimited
> > echo mempool_autotest | ./build/app/test --
> > # segfault + core dump
> > gdb -c core ./build/app/test
> >
> > (gdb) bt
> > #1  0x000000000064dead in rte_mempool_ops_alloc (mp=0x7f8816abdb40)
> >     at /root/dpdk/lib/librte_mempool/rte_mempool_ops.c:101
> > #2  0x000000000064c1e7 in rte_mempool_populate_phys (mp=0x7f8816abdb40,
> >     vaddr=0x7f880987a800 <error: Cannot access memory at address 0x7f880987a800>,
> >     paddr=6958852096, len=26761152, free_cb=0x64c032 <rte_mempool_memchunk_mz_free>,
> >     opaque=0x7f8822334d4c) at /root/dpdk/lib/librte_mempool/rte_mempool.c:359
> > #3  0x000000000064c9db in rte_mempool_populate_default (mp=0x7f8816abdb40)
> >     at /root/dpdk/lib/librte_mempool/rte_mempool.c:572  
> 
> I think adding the code that you suggested is not the right place. The 
> problem is not in rte_mempool_ops_alloc, where you had suggested the 
> check for NULL.
> 
> The problem is in rte_mempool_create where return value for 
> rte_mempool_set_ops_byname is not being checked.
> 
> When the libraries are not statically compiled in, 
> rte_mempool_set_ops_byname is returning NULL, which rte_mempool_create 
> doesn't handle and goes on to call rte_mempool_ops_alloc - eventually 
> segfaulting.

Yes, you're right, it's better to check the return value of
rte_mempool_set_ops_byname() in rte_mempool_create().



> > #4  0x000000000064d3d4 in rte_mempool_create (name=0x9b1ff0 "test_nocache", n=12671,
> >     elt_size=2048, cache_size=0, private_data_size=0, mp_init=0x0, mp_init_arg=0x0,
> >     obj_init=0x49f309 <my_obj_init>, obj_init_arg=0x0, socket_id=-1, flags=0)
> >     at /root/dpdk/lib/librte_mempool/rte_mempool.c:895
> > #5  0x00000000004a20ed in test_mempool () at /root/dpdk/test/test/test_mempool.c:519
> > #6  0x0000000000435189 in cmd_autotest_parsed (parsed_result=0x7ffe55006420,
> >     cl=0x7c87090, data=0x0) at /root/dpdk/test/test/commands.c:103
> > #7  0x00000000006749df in cmdline_parse (cl=0x7c87090,
> >     buf=0x7c870d8 "mempool_autotest\n")
> >     at /root/dpdk/lib/librte_cmdline/cmdline_parse.c:359
> > (gdb) up
> > #1  0x000000000064dead in rte_mempool_ops_alloc (mp=0x7f8816abdb40)
> >     at /root/dpdk/lib/librte_mempool/rte_mempool_ops.c:101
> > 101             return ops->alloc(mp);
> > (gdb) print ops
> > $1 = (struct rte_mempool_ops *) 0x4e69c00 <rte_mempool_ops_table+64>
> > (gdb) print *ops
> > $2 = {name = '\000' <repeats 31 times>, alloc = 0x0, free = 0x0, enqueue = 0x0,
> >   dequeue = 0x0, get_count = 0x0}
> >
> >
> > Regards,
> > Olivier
> >
> >  
> >>  
> >>>
> >>> Adding the following code in lib/librte_mempool/rte_mempool_ops.c
> >>> fixes the crash.
> >>>
> >>>         ops = rte_mempool_get_ops(mp->ops_index);
> >>> +       if (ops == NULL || ops->alloc == NULL)
> >>> +               return -ENOTSUP;
> >>>         return ops->alloc(mp);  
> 
> If you think above explanation suffices, I will push a patch for error 
> handling in rte_mempool_create returned by rte_mempool_set_ops_byname 
> rather than above change originally suggested by you.

Yes, please. Thanks!

Olivier


> 
> >>
> >> Can you tell me for which case did your code reach
> >> rte_mempool_ops_alloc() and segfault?
> >>
> >> In my case, librte_mempool_ring and librte_mempool_stack are not added
> >> to mk/rte.app.mk and it is static compilation.
> >>  
> >>>
> >>> Now that drivers are not linked to the mempool library, it can
> >>> happen that there is no handler. Could you please add this patch in your
> >>> patchset?  
> >>
> >> Yes, once I can get this issue reproduced. Because I think there is one
> >> more place similar code should go (rte_mempool_ops_getcount).
> >> As per what I can see, this would only happen if rte_mempool_xmem_create
> >> is called and then directly alloc is called. That is not happening for
> >> mempool_autotest.
> >>
> >> -
> >> Shreyansh
> >>  
> >
> >  
> 

[PATCH 1/3] mempool: fix segfault for unlinked mempool handler

[Shreyansh Jain]

Fixes: 449c49b93a6b ("mempool: support handler operations")

In case the stack or ring mempool handler are compiled as shared
library and not linked in with test binary, segfault is reported.
This is because return value of rte_mempool_set_ops_byname is not
being checked in rte_mempool_ops_alloc.

This patch handles error returned from rte_mempool_set_ops_byname
when a mempool is not found.

Signed-off-by: Shreyansh Jain <shreyansh.jain@nxp.com>
---
 lib/librte_mempool/rte_mempool.c | 12 ++++++++----
 1 file changed, 8 insertions(+), 4 deletions(-)

diff --git a/lib/librte_mempool/rte_mempool.c b/lib/librte_mempool/rte_mempool.c
index 40d3afd..ef7d3d1 100644
--- a/lib/librte_mempool/rte_mempool.c
+++ b/lib/librte_mempool/rte_mempool.c
@@ -868,6 +868,7 @@ rte_mempool_create(const char *name, unsigned n, unsigned elt_size,
 	rte_mempool_obj_cb_t *obj_init, void *obj_init_arg,
 	int socket_id, unsigned flags)
 {
+	int ret;
 	struct rte_mempool *mp;
 
 	mp = rte_mempool_create_empty(name, n, elt_size, cache_size,
@@ -880,13 +881,16 @@ rte_mempool_create(const char *name, unsigned n, unsigned elt_size,
 	 * set the correct index into the table of ops structs.
 	 */
 	if ((flags & MEMPOOL_F_SP_PUT) && (flags & MEMPOOL_F_SC_GET))
-		rte_mempool_set_ops_byname(mp, "ring_sp_sc", NULL);
+		ret = rte_mempool_set_ops_byname(mp, "ring_sp_sc", NULL);
 	else if (flags & MEMPOOL_F_SP_PUT)
-		rte_mempool_set_ops_byname(mp, "ring_sp_mc", NULL);
+		ret = rte_mempool_set_ops_byname(mp, "ring_sp_mc", NULL);
 	else if (flags & MEMPOOL_F_SC_GET)
-		rte_mempool_set_ops_byname(mp, "ring_mp_sc", NULL);
+		ret = rte_mempool_set_ops_byname(mp, "ring_mp_sc", NULL);
 	else
-		rte_mempool_set_ops_byname(mp, "ring_mp_mc", NULL);
+		ret = rte_mempool_set_ops_byname(mp, "ring_mp_mc", NULL);
+
+	if (ret)
+		goto fail;
 
 	/* call the mempool priv initializer */
 	if (mp_init)
-- 
2.7.4

[PATCH 2/3] mempool: introduce ring mempool driver

[Shreyansh Jain]

Moved from lib/librte_mempool, ring mempool is now an independent
driver.
Shared builds would now need to add librte_mempool_ring for:
* ring_mp_mc
* ring_sp_sc
* ring_sp_mc
* ring_mp_sc

Signed-off-by: Shreyansh Jain <shreyansh.jain@nxp.com>
---
 MAINTAINERS                                        |  1 +
 config/common_base                                 |  5 +++
 drivers/Makefile                                   |  1 +
 drivers/mempool/Makefile                           | 39 ++++++++++++++++++
 drivers/mempool/ring/Makefile                      | 48 ++++++++++++++++++++++
 .../mempool/ring}/rte_mempool_ring.c               |  0
 drivers/mempool/ring/rte_mempool_ring_version.map  |  4 ++
 lib/librte_mempool/Makefile                        |  1 -
 mk/rte.app.mk                                      |  1 +
 9 files changed, 99 insertions(+), 1 deletion(-)
 create mode 100644 drivers/mempool/Makefile
 create mode 100644 drivers/mempool/ring/Makefile
 rename {lib/librte_mempool => drivers/mempool/ring}/rte_mempool_ring.c (100%)
 create mode 100644 drivers/mempool/ring/rte_mempool_ring_version.map

diff --git a/MAINTAINERS b/MAINTAINERS
index 4cb6e49..403d0dd 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -211,6 +211,7 @@ Core Libraries
 Memory pool
 M: Olivier Matz <olivier.matz@6wind.com>
 F: lib/librte_mempool/
+F: drivers/mempool/ring
 F: doc/guides/prog_guide/mempool_lib.rst
 F: test/test/test_mempool*
 F: test/test/test_func_reentrancy.c
diff --git a/config/common_base b/config/common_base
index 2d54ddf..9acd557 100644
--- a/config/common_base
+++ b/config/common_base
@@ -461,6 +461,11 @@ CONFIG_RTE_MEMPOOL_CACHE_MAX_SIZE=512
 CONFIG_RTE_LIBRTE_MEMPOOL_DEBUG=n
 
 #
+# Compile Mempool drivers
+#
+CONFIG_RTE_DRIVER_MEMPOOL_RING=y
+
+#
 # Compile librte_mbuf
 #
 CONFIG_RTE_LIBRTE_MBUF=y
diff --git a/drivers/Makefile b/drivers/Makefile
index 81c03a8..193056b 100644
--- a/drivers/Makefile
+++ b/drivers/Makefile
@@ -31,6 +31,7 @@
 
 include $(RTE_SDK)/mk/rte.vars.mk
 
+DIRS-y += mempool
 DIRS-y += net
 DIRS-$(CONFIG_RTE_LIBRTE_CRYPTODEV) += crypto
 
diff --git a/drivers/mempool/Makefile b/drivers/mempool/Makefile
new file mode 100644
index 0000000..6a8a1da
--- /dev/null
+++ b/drivers/mempool/Makefile
@@ -0,0 +1,39 @@
+#   BSD LICENSE
+#
+#   Copyright(c) 2017 NXP. All rights reserved.
+#   All rights reserved.
+#
+#   Redistribution and use in source and binary forms, with or without
+#   modification, are permitted provided that the following conditions
+#   are met:
+#
+#     * Redistributions of source code must retain the above copyright
+#       notice, this list of conditions and the following disclaimer.
+#     * Redistributions in binary form must reproduce the above copyright
+#       notice, this list of conditions and the following disclaimer in
+#       the documentation and/or other materials provided with the
+#       distribution.
+#     * Neither the name of NXP nor the names of its
+#       contributors may be used to endorse or promote products derived
+#       from this software without specific prior written permission.
+#
+#   THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS
+#   "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT
+#   LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR
+#   A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT
+#   OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL,
+#   SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT
+#   LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE,
+#   DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY
+#   THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
+#   (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE
+#   OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
+
+include $(RTE_SDK)/mk/rte.vars.mk
+
+core-libs := librte_eal librte_mempool librte_ring
+
+DIRS-$(CONFIG_RTE_DRIVER_MEMPOOL_RING) += ring
+DEPDIRS-ring = $(core-libs)
+
+include $(RTE_SDK)/mk/rte.subdir.mk
diff --git a/drivers/mempool/ring/Makefile b/drivers/mempool/ring/Makefile
new file mode 100644
index 0000000..54630d9
--- /dev/null
+++ b/drivers/mempool/ring/Makefile
@@ -0,0 +1,48 @@
+#   BSD LICENSE
+#
+#   Copyright(c) 2017 NXP.
+#   All rights reserved.
+#
+#   Redistribution and use in source and binary forms, with or without
+#   modification, are permitted provided that the following conditions
+#   are met:
+#
+#     * Redistributions of source code must retain the above copyright
+#       notice, this list of conditions and the following disclaimer.
+#     * Redistributions in binary form must reproduce the above copyright
+#       notice, this list of conditions and the following disclaimer in
+#       the documentation and/or other materials provided with the
+#       distribution.
+#     * Neither the name of NXP nor the names of its
+#       contributors may be used to endorse or promote products derived
+#       from this software without specific prior written permission.
+#
+#   THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS
+#   "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT
+#   LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR
+#   A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT
+#   OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL,
+#   SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT
+#   LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE,
+#   DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY
+#   THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
+#   (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE
+#   OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
+
+include $(RTE_SDK)/mk/rte.vars.mk
+
+#
+# library name
+#
+LIB = librte_mempool_ring.a
+
+CFLAGS += -O3
+CFLAGS += $(WERROR_FLAGS)
+
+EXPORT_MAP := rte_mempool_ring_version.map
+
+LIBABIVER := 1
+
+SRCS-$(CONFIG_RTE_DRIVER_MEMPOOL_RING) += rte_mempool_ring.c
+
+include $(RTE_SDK)/mk/rte.lib.mk
diff --git a/lib/librte_mempool/rte_mempool_ring.c b/drivers/mempool/ring/rte_mempool_ring.c
similarity index 100%
rename from lib/librte_mempool/rte_mempool_ring.c
rename to drivers/mempool/ring/rte_mempool_ring.c
diff --git a/drivers/mempool/ring/rte_mempool_ring_version.map b/drivers/mempool/ring/rte_mempool_ring_version.map
new file mode 100644
index 0000000..8591cc0
--- /dev/null
+++ b/drivers/mempool/ring/rte_mempool_ring_version.map
@@ -0,0 +1,4 @@
+DPDK_17.05 {
+
+	local: *;
+};
diff --git a/lib/librte_mempool/Makefile b/lib/librte_mempool/Makefile
index 96b6ca2..efd4383 100644
--- a/lib/librte_mempool/Makefile
+++ b/lib/librte_mempool/Makefile
@@ -43,7 +43,6 @@ LIBABIVER := 2
 # all source are stored in SRCS-y
 SRCS-$(CONFIG_RTE_LIBRTE_MEMPOOL) +=  rte_mempool.c
 SRCS-$(CONFIG_RTE_LIBRTE_MEMPOOL) +=  rte_mempool_ops.c
-SRCS-$(CONFIG_RTE_LIBRTE_MEMPOOL) +=  rte_mempool_ring.c
 SRCS-$(CONFIG_RTE_LIBRTE_MEMPOOL) +=  rte_mempool_stack.c
 # install includes
 SYMLINK-$(CONFIG_RTE_LIBRTE_MEMPOOL)-include := rte_mempool.h
diff --git a/mk/rte.app.mk b/mk/rte.app.mk
index 62a2a1a..5ebb6ec 100644
--- a/mk/rte.app.mk
+++ b/mk/rte.app.mk
@@ -101,6 +101,7 @@ _LDLIBS-$(CONFIG_RTE_LIBRTE_REORDER)        += -lrte_reorder
 
 ifeq ($(CONFIG_RTE_BUILD_SHARED_LIB),n)
 # plugins (link only if static libraries)
+_LDLIBS-$(CONFIG_RTE_DRIVER_MEMPOOL_RING)   += -lrte_mempool_ring
 
 _LDLIBS-$(CONFIG_RTE_LIBRTE_PMD_AF_PACKET)  += -lrte_pmd_af_packet
 _LDLIBS-$(CONFIG_RTE_LIBRTE_BNX2X_PMD)      += -lrte_pmd_bnx2x -lz
-- 
2.7.4

[PATCH 3/3] mempool: introduce stack mempool driver

[Shreyansh Jain]

Moved from lib/librte_mempool, stack mempool handler is an independent
driver.
Shared builds would now require to link in librte_mempool_stack for
"stack" mempool handler.

Signed-off-by: Shreyansh Jain <shreyansh.jain@nxp.com>
---
 MAINTAINERS                                        |  1 +
 config/common_base                                 |  1 +
 drivers/mempool/Makefile                           |  2 +
 drivers/mempool/stack/Makefile                     | 51 ++++++++++++++++++++++
 .../mempool/stack}/rte_mempool_stack.c             |  0
 .../mempool/stack/rte_mempool_stack_version.map    |  4 ++
 lib/librte_mempool/Makefile                        |  1 -
 mk/rte.app.mk                                      |  1 +
 8 files changed, 60 insertions(+), 1 deletion(-)
 create mode 100644 drivers/mempool/stack/Makefile
 rename {lib/librte_mempool => drivers/mempool/stack}/rte_mempool_stack.c (100%)
 create mode 100644 drivers/mempool/stack/rte_mempool_stack_version.map

diff --git a/MAINTAINERS b/MAINTAINERS
index 403d0dd..bd402d1 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -211,6 +211,7 @@ Core Libraries
 Memory pool
 M: Olivier Matz <olivier.matz@6wind.com>
 F: lib/librte_mempool/
+F: drivers/mempool/stack
 F: drivers/mempool/ring
 F: doc/guides/prog_guide/mempool_lib.rst
 F: test/test/test_mempool*
diff --git a/config/common_base b/config/common_base
index 9acd557..41191c8 100644
--- a/config/common_base
+++ b/config/common_base
@@ -464,6 +464,7 @@ CONFIG_RTE_LIBRTE_MEMPOOL_DEBUG=n
 # Compile Mempool drivers
 #
 CONFIG_RTE_DRIVER_MEMPOOL_RING=y
+CONFIG_RTE_DRIVER_MEMPOOL_STACK=y
 
 #
 # Compile librte_mbuf
diff --git a/drivers/mempool/Makefile b/drivers/mempool/Makefile
index 6a8a1da..0c6c45c 100644
--- a/drivers/mempool/Makefile
+++ b/drivers/mempool/Makefile
@@ -35,5 +35,7 @@ core-libs := librte_eal librte_mempool librte_ring
 
 DIRS-$(CONFIG_RTE_DRIVER_MEMPOOL_RING) += ring
 DEPDIRS-ring = $(core-libs)
+DIRS-$(CONFIG_RTE_DRIVER_MEMPOOL_STACK) += stack
+DEPDIRS-stack = $(core-libs)
 
 include $(RTE_SDK)/mk/rte.subdir.mk
diff --git a/drivers/mempool/stack/Makefile b/drivers/mempool/stack/Makefile
new file mode 100644
index 0000000..8f3125c
--- /dev/null
+++ b/drivers/mempool/stack/Makefile
@@ -0,0 +1,51 @@
+#   BSD LICENSE
+#
+#   Copyright(c) 2017 NXP.
+#   All rights reserved.
+#
+#   Redistribution and use in source and binary forms, with or without
+#   modification, are permitted provided that the following conditions
+#   are met:
+#
+#     * Redistributions of source code must retain the above copyright
+#       notice, this list of conditions and the following disclaimer.
+#     * Redistributions in binary form must reproduce the above copyright
+#       notice, this list of conditions and the following disclaimer in
+#       the documentation and/or other materials provided with the
+#       distribution.
+#     * Neither the name of NXP nor the names of its
+#       contributors may be used to endorse or promote products derived
+#       from this software without specific prior written permission.
+#
+#   THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS
+#   "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT
+#   LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR
+#   A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT
+#   OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL,
+#   SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT
+#   LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE,
+#   DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY
+#   THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
+#   (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE
+#   OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
+
+include $(RTE_SDK)/mk/rte.vars.mk
+
+#
+# library name
+#
+LIB = librte_mempool_stack.a
+
+CFLAGS += -O3
+CFLAGS += $(WERROR_FLAGS)
+
+# Headers
+CFLAGS += -I$(RTE_SDK)/lib/librte_mempool
+
+EXPORT_MAP := rte_mempool_stack_version.map
+
+LIBABIVER := 1
+
+SRCS-$(CONFIG_RTE_DRIVER_MEMPOOL_STACK) += rte_mempool_stack.c
+
+include $(RTE_SDK)/mk/rte.lib.mk
diff --git a/lib/librte_mempool/rte_mempool_stack.c b/drivers/mempool/stack/rte_mempool_stack.c
similarity index 100%
rename from lib/librte_mempool/rte_mempool_stack.c
rename to drivers/mempool/stack/rte_mempool_stack.c
diff --git a/drivers/mempool/stack/rte_mempool_stack_version.map b/drivers/mempool/stack/rte_mempool_stack_version.map
new file mode 100644
index 0000000..8591cc0
--- /dev/null
+++ b/drivers/mempool/stack/rte_mempool_stack_version.map
@@ -0,0 +1,4 @@
+DPDK_17.05 {
+
+	local: *;
+};
diff --git a/lib/librte_mempool/Makefile b/lib/librte_mempool/Makefile
index efd4383..7b5bdfe 100644
--- a/lib/librte_mempool/Makefile
+++ b/lib/librte_mempool/Makefile
@@ -43,7 +43,6 @@ LIBABIVER := 2
 # all source are stored in SRCS-y
 SRCS-$(CONFIG_RTE_LIBRTE_MEMPOOL) +=  rte_mempool.c
 SRCS-$(CONFIG_RTE_LIBRTE_MEMPOOL) +=  rte_mempool_ops.c
-SRCS-$(CONFIG_RTE_LIBRTE_MEMPOOL) +=  rte_mempool_stack.c
 # install includes
 SYMLINK-$(CONFIG_RTE_LIBRTE_MEMPOOL)-include := rte_mempool.h
 
diff --git a/mk/rte.app.mk b/mk/rte.app.mk
index 5ebb6ec..336e448 100644
--- a/mk/rte.app.mk
+++ b/mk/rte.app.mk
@@ -102,6 +102,7 @@ _LDLIBS-$(CONFIG_RTE_LIBRTE_REORDER)        += -lrte_reorder
 ifeq ($(CONFIG_RTE_BUILD_SHARED_LIB),n)
 # plugins (link only if static libraries)
 _LDLIBS-$(CONFIG_RTE_DRIVER_MEMPOOL_RING)   += -lrte_mempool_ring
+_LDLIBS-$(CONFIG_RTE_DRIVER_MEMPOOL_STACK)  += -lrte_mempool_stack
 
 _LDLIBS-$(CONFIG_RTE_LIBRTE_PMD_AF_PACKET)  += -lrte_pmd_af_packet
 _LDLIBS-$(CONFIG_RTE_LIBRTE_BNX2X_PMD)      += -lrte_pmd_bnx2x -lz
-- 
2.7.4

Re: [PATCH 1/3] mempool: fix segfault for unlinked mempool handler

[Shreyansh Jain]

On Friday 31 March 2017 10:59 AM, Shreyansh Jain wrote:
> Fixes: 449c49b93a6b ("mempool: support handler operations")
>
> In case the stack or ring mempool handler are compiled as shared
> library and not linked in with test binary, segfault is reported.
> This is because return value of rte_mempool_set_ops_byname is not
> being checked in rte_mempool_ops_alloc.
>
> This patch handles error returned from rte_mempool_set_ops_byname
> when a mempool is not found.
>
> Signed-off-by: Shreyansh Jain <shreyansh.jain@nxp.com>
> ---
>  lib/librte_mempool/rte_mempool.c | 12 ++++++++----
>  1 file changed, 8 insertions(+), 4 deletions(-)
>

Self NACK
Forgot to set v2 and version history