DPDK patches and discussions
 help / color / mirror / Atom feed
* [PATCH] reorder: fix registration of dynamic field in mbuf
@ 2023-03-13  9:34 Volodymyr Fialko
  2023-03-13 10:19 ` David Marchand
                   ` (2 more replies)
  0 siblings, 3 replies; 12+ messages in thread
From: Volodymyr Fialko @ 2023-03-13  9:34 UTC (permalink / raw)
  To: dev, Reshma Pattan, David Marchand, Andrew Rybchenko
  Cc: jerinj, anoobj, Volodymyr Fialko

It's possible to initialize reorder buffer with user allocated memory via
rte_reorder_init() function. In such case rte_reorder_create() not required
and reorder dynamic field in rte_mbuf will be not registered.

Fixes: 01f3496695b5 ("reorder: switch sequence number to dynamic mbuf field")

Signed-off-by: Volodymyr Fialko <vfialko@marvell.com>
---
 lib/reorder/rte_reorder.c | 40 ++++++++++++++++++++++++++++++---------
 1 file changed, 31 insertions(+), 9 deletions(-)

diff --git a/lib/reorder/rte_reorder.c b/lib/reorder/rte_reorder.c
index 6e029c9e02..a759a9c434 100644
--- a/lib/reorder/rte_reorder.c
+++ b/lib/reorder/rte_reorder.c
@@ -54,6 +54,28 @@ struct rte_reorder_buffer {
 static void
 rte_reorder_free_mbufs(struct rte_reorder_buffer *b);
 
+static int
+rte_reorder_dynf_register(void)
+{
+	int ret;
+
+	static const struct rte_mbuf_dynfield reorder_seqn_dynfield_desc = {
+		.name = RTE_REORDER_SEQN_DYNFIELD_NAME,
+		.size = sizeof(rte_reorder_seqn_t),
+		.align = __alignof__(rte_reorder_seqn_t),
+	};
+
+	if (rte_reorder_seqn_dynfield_offset > 0)
+		return 0;
+
+	ret = rte_mbuf_dynfield_register(&reorder_seqn_dynfield_desc);
+	if (ret < 0)
+		return ret;
+	rte_reorder_seqn_dynfield_offset = ret;
+
+	return 0;
+}
+
 struct rte_reorder_buffer *
 rte_reorder_init(struct rte_reorder_buffer *b, unsigned int bufsize,
 		const char *name, unsigned int size)
@@ -85,6 +107,12 @@ rte_reorder_init(struct rte_reorder_buffer *b, unsigned int bufsize,
 		rte_errno = EINVAL;
 		return NULL;
 	}
+	if (rte_reorder_dynf_register()) {
+		RTE_LOG(ERR, REORDER, "Failed to register mbuf field for reorder sequence"
+				      " number\n");
+		rte_errno = ENOMEM;
+		return NULL;
+	}
 
 	memset(b, 0, bufsize);
 	strlcpy(b->name, name, sizeof(b->name));
@@ -106,11 +134,6 @@ rte_reorder_create(const char *name, unsigned socket_id, unsigned int size)
 	struct rte_reorder_list *reorder_list;
 	const unsigned int bufsize = sizeof(struct rte_reorder_buffer) +
 					(2 * size * sizeof(struct rte_mbuf *));
-	static const struct rte_mbuf_dynfield reorder_seqn_dynfield_desc = {
-		.name = RTE_REORDER_SEQN_DYNFIELD_NAME,
-		.size = sizeof(rte_reorder_seqn_t),
-		.align = __alignof__(rte_reorder_seqn_t),
-	};
 
 	reorder_list = RTE_TAILQ_CAST(rte_reorder_tailq.head, rte_reorder_list);
 
@@ -128,10 +151,9 @@ rte_reorder_create(const char *name, unsigned socket_id, unsigned int size)
 		return NULL;
 	}
 
-	rte_reorder_seqn_dynfield_offset =
-		rte_mbuf_dynfield_register(&reorder_seqn_dynfield_desc);
-	if (rte_reorder_seqn_dynfield_offset < 0) {
-		RTE_LOG(ERR, REORDER, "Failed to register mbuf field for reorder sequence number\n");
+	if (rte_reorder_dynf_register()) {
+		RTE_LOG(ERR, REORDER, "Failed to register mbuf field for reorder sequence"
+				      " number\n");
 		rte_errno = ENOMEM;
 		return NULL;
 	}
-- 
2.34.1


^ permalink raw reply	[flat|nested] 12+ messages in thread
* [PATCH v2] reorder: fix registration of dynamic field in mbuf
@ 2023-03-13 12:59 Volodymyr Fialko
  0 siblings, 0 replies; 12+ messages in thread
From: Volodymyr Fialko @ 2023-03-13 12:59 UTC (permalink / raw)
  To: dev, Reshma Pattan, David Marchand, Andrew Rybchenko
  Cc: jerinj, anoobj, Volodymyr Fialko, stable

It's possible to initialize reorder buffer with user allocated memory via
rte_reorder_init() function. In such case rte_reorder_create() is not
required and reorder dynamic field in rte_mbuf will not be registered.

Both reorder lib and mbuf dynamic field are using `rte_mcfg_tailq`
read/write lock for synchronization, to avoid deadlocking move reorder
buffer initialization before queue insertion.

Fixes: 01f3496695b5 ("reorder: switch sequence number to dynamic mbuf field")
Cc: stable@dpdk.org

Signed-off-by: Volodymyr Fialko <vfialko@marvell.com>
---
 lib/reorder/rte_reorder.c | 94 +++++++++++++++++++++++++--------------
 lib/reorder/rte_reorder.h |  1 +
 2 files changed, 61 insertions(+), 34 deletions(-)

diff --git a/lib/reorder/rte_reorder.c b/lib/reorder/rte_reorder.c
index 6e029c9e02..66d2cc07b7 100644
--- a/lib/reorder/rte_reorder.c
+++ b/lib/reorder/rte_reorder.c
@@ -60,6 +60,11 @@ rte_reorder_init(struct rte_reorder_buffer *b, unsigned int bufsize,
 {
 	const unsigned int min_bufsize = sizeof(*b) +
 					(2 * size * sizeof(struct rte_mbuf *));
+	static const struct rte_mbuf_dynfield reorder_seqn_dynfield_desc = {
+		.name = RTE_REORDER_SEQN_DYNFIELD_NAME,
+		.size = sizeof(rte_reorder_seqn_t),
+		.align = __alignof__(rte_reorder_seqn_t),
+	};
 
 	if (b == NULL) {
 		RTE_LOG(ERR, REORDER, "Invalid reorder buffer parameter:"
@@ -86,6 +91,14 @@ rte_reorder_init(struct rte_reorder_buffer *b, unsigned int bufsize,
 		return NULL;
 	}
 
+	rte_reorder_seqn_dynfield_offset = rte_mbuf_dynfield_register(&reorder_seqn_dynfield_desc);
+	if (rte_reorder_seqn_dynfield_offset < 0) {
+		RTE_LOG(ERR, REORDER, "Failed to register mbuf field for reorder sequence"
+				      " number, rte_errno: %i\n", rte_errno);
+		rte_errno = ENOMEM;
+		return NULL;
+	}
+
 	memset(b, 0, bufsize);
 	strlcpy(b->name, name, sizeof(b->name));
 	b->memsize = bufsize;
@@ -98,21 +111,45 @@ rte_reorder_init(struct rte_reorder_buffer *b, unsigned int bufsize,
 	return b;
 }
 
+/*
+ * Insert new entry into global list.
+ * Returns pointer to already inserted entry if such exists, or to newly inserted one.
+ */
+static struct rte_tailq_entry*
+rte_reorder_entry_insert(struct rte_tailq_entry *new_te)
+{
+	struct rte_reorder_list *reorder_list;
+	struct rte_reorder_buffer *b, *nb;
+	struct rte_tailq_entry *te;
+
+	rte_mcfg_tailq_write_lock();
+
+	reorder_list = RTE_TAILQ_CAST(rte_reorder_tailq.head, rte_reorder_list);
+	/* guarantee there's no existing */
+	TAILQ_FOREACH(te, reorder_list, next) {
+		b = (struct rte_reorder_buffer *) te->data;
+		nb = (struct rte_reorder_buffer *) new_te->data;
+		if (strncmp(nb->name, b->name, RTE_REORDER_NAMESIZE) == 0)
+			break;
+	}
+
+	if (te == NULL) {
+		TAILQ_INSERT_TAIL(reorder_list, new_te, next);
+		te = new_te;
+	}
+
+	rte_mcfg_tailq_write_unlock();
+
+	return te;
+}
+
 struct rte_reorder_buffer*
 rte_reorder_create(const char *name, unsigned socket_id, unsigned int size)
 {
 	struct rte_reorder_buffer *b = NULL;
-	struct rte_tailq_entry *te;
-	struct rte_reorder_list *reorder_list;
+	struct rte_tailq_entry *te, *te_inserted;
 	const unsigned int bufsize = sizeof(struct rte_reorder_buffer) +
 					(2 * size * sizeof(struct rte_mbuf *));
-	static const struct rte_mbuf_dynfield reorder_seqn_dynfield_desc = {
-		.name = RTE_REORDER_SEQN_DYNFIELD_NAME,
-		.size = sizeof(rte_reorder_seqn_t),
-		.align = __alignof__(rte_reorder_seqn_t),
-	};
-
-	reorder_list = RTE_TAILQ_CAST(rte_reorder_tailq.head, rte_reorder_list);
 
 	/* Check user arguments. */
 	if (!rte_is_power_of_2(size)) {
@@ -128,32 +165,12 @@ rte_reorder_create(const char *name, unsigned socket_id, unsigned int size)
 		return NULL;
 	}
 
-	rte_reorder_seqn_dynfield_offset =
-		rte_mbuf_dynfield_register(&reorder_seqn_dynfield_desc);
-	if (rte_reorder_seqn_dynfield_offset < 0) {
-		RTE_LOG(ERR, REORDER, "Failed to register mbuf field for reorder sequence number\n");
-		rte_errno = ENOMEM;
-		return NULL;
-	}
-
-	rte_mcfg_tailq_write_lock();
-
-	/* guarantee there's no existing */
-	TAILQ_FOREACH(te, reorder_list, next) {
-		b = (struct rte_reorder_buffer *) te->data;
-		if (strncmp(name, b->name, RTE_REORDER_NAMESIZE) == 0)
-			break;
-	}
-	if (te != NULL)
-		goto exit;
-
 	/* allocate tailq entry */
 	te = rte_zmalloc("REORDER_TAILQ_ENTRY", sizeof(*te), 0);
 	if (te == NULL) {
 		RTE_LOG(ERR, REORDER, "Failed to allocate tailq entry\n");
 		rte_errno = ENOMEM;
-		b = NULL;
-		goto exit;
+		return NULL;
 	}
 
 	/* Allocate memory to store the reorder buffer structure. */
@@ -162,14 +179,23 @@ rte_reorder_create(const char *name, unsigned socket_id, unsigned int size)
 		RTE_LOG(ERR, REORDER, "Memzone allocation failed\n");
 		rte_errno = ENOMEM;
 		rte_free(te);
+		return NULL;
 	} else {
-		rte_reorder_init(b, bufsize, name, size);
+		if (rte_reorder_init(b, bufsize, name, size) == NULL) {
+			rte_free(b);
+			rte_free(te);
+			return NULL;
+		}
 		te->data = (void *)b;
-		TAILQ_INSERT_TAIL(reorder_list, te, next);
 	}
 
-exit:
-	rte_mcfg_tailq_write_unlock();
+	te_inserted = rte_reorder_entry_insert(te);
+	if (te_inserted != te) {
+		rte_free(b);
+		rte_free(te);
+		return te_inserted->data;
+	}
+
 	return b;
 }
 
diff --git a/lib/reorder/rte_reorder.h b/lib/reorder/rte_reorder.h
index cc95015fa6..9a5998f2f6 100644
--- a/lib/reorder/rte_reorder.h
+++ b/lib/reorder/rte_reorder.h
@@ -82,6 +82,7 @@ rte_reorder_create(const char *name, unsigned socket_id, unsigned int size);
  *   The initialized reorder buffer instance, or NULL on error
  *   On error case, rte_errno will be set appropriately:
  *    - EINVAL - invalid parameters
+ *    - ENOMEM - not enough memory to register dynamic field
  */
 struct rte_reorder_buffer *
 rte_reorder_init(struct rte_reorder_buffer *b, unsigned int bufsize,
-- 
2.34.1


^ permalink raw reply	[flat|nested] 12+ messages in thread

end of thread, other threads:[~2023-03-16 15:55 UTC | newest]

Thread overview: 12+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-03-13  9:34 [PATCH] reorder: fix registration of dynamic field in mbuf Volodymyr Fialko
2023-03-13 10:19 ` David Marchand
2023-03-13 13:08   ` [EXT] " Volodymyr Fialko
2023-03-13 13:04 ` [PATCH v2] " Volodymyr Fialko
2023-03-15 15:29   ` Volodymyr Fialko
2023-03-15 15:43     ` David Marchand
2023-03-15 15:46       ` Pattan, Reshma
2023-03-16 15:36         ` David Marchand
2023-03-16 15:55   ` David Marchand
2023-03-13 15:51 ` [PATCH] " Stephen Hemminger
2023-03-13 17:29   ` [EXT] " Volodymyr Fialko
2023-03-13 12:59 [PATCH v2] " Volodymyr Fialko

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).