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: sta...@dpdk.org

Signed-off-by: Volodymyr Fialko <vfia...@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

Reply via email to