ring: make tailq fully local
authorAnatoly Burakov <anatoly.burakov@intel.com>
Fri, 20 Jun 2014 15:42:19 +0000 (16:42 +0100)
committerThomas Monjalon <thomas.monjalon@6wind.com>
Tue, 22 Jul 2014 17:42:18 +0000 (19:42 +0200)
Since the data structures such as rings are shared in their entirety,
those TAILQ pointers are shared as well. Meaning that, after a
successful rte_ring creation, the tailq_next pointer of the last
ring in the TAILQ will be updated with a pointer to a ring which may
not be present in the address space of another process (i.e. a ring
that may be host-local or guest-local, and not shared over IVSHMEM).
Any successive ring create/lookup on the other side of IVSHMEM will
result in trying to dereference an invalid pointer.

This patchset fixes this problem by creating a default tailq entry
that may be used by any data structure that chooses to use TAILQs.
This default TAILQ entry will consist of a tailq_next/tailq_prev
pointers, and an opaque pointer to arbitrary data. All TAILQ
pointers from data structures themselves will be removed and
replaced by those generic TAILQ entries, thus fixing the problem
of potentially exposing local address space to shared structures.

Signed-off-by: Anatoly Burakov <anatoly.burakov@intel.com>
Acked-by: Konstantin Ananyev <konstantin.ananyev@intel.com>
lib/librte_eal/linuxapp/eal/eal_ivshmem.c
lib/librte_ring/Makefile
lib/librte_ring/rte_ring.c
lib/librte_ring/rte_ring.h

index 4e59df5..16d407e 100644 (file)
@@ -50,6 +50,7 @@
 #include <rte_errno.h>
 #include <rte_ring.h>
 #include <rte_mempool.h>
+#include <rte_malloc.h>
 #include <rte_common.h>
 #include <rte_ivshmem.h>
 #include <rte_tailq_elem.h>
@@ -101,7 +102,7 @@ static int memseg_idx;
 static int pagesz;
 
 /* Tailq heads to add rings to */
-TAILQ_HEAD(rte_ring_list, rte_ring);
+TAILQ_HEAD(rte_ring_list, rte_tailq_entry);
 
 /*
  * Utility functions
@@ -754,6 +755,7 @@ rte_eal_ivshmem_obj_init(void)
        struct ivshmem_segment * seg;
        struct rte_memzone * mz;
        struct rte_ring * r;
+       struct rte_tailq_entry *te;
        unsigned i, ms, idx;
        uint64_t offset;
 
@@ -808,6 +810,8 @@ rte_eal_ivshmem_obj_init(void)
                mcfg->memzone_idx++;
        }
 
+       rte_rwlock_write_lock(RTE_EAL_TAILQ_RWLOCK);
+
        /* find rings */
        for (i = 0; i < mcfg->memzone_idx; i++) {
                mz = &mcfg->memzone[i];
@@ -819,10 +823,19 @@ rte_eal_ivshmem_obj_init(void)
 
                r = (struct rte_ring*) (mz->addr_64);
 
-               TAILQ_INSERT_TAIL(ring_list, r, next);
+               te = rte_zmalloc("RING_TAILQ_ENTRY", sizeof(*te), 0);
+               if (te == NULL) {
+                       RTE_LOG(ERR, EAL, "Cannot allocate ring tailq entry!\n");
+                       return -1;
+               }
+
+               te->data = (void *) r;
+
+               TAILQ_INSERT_TAIL(ring_list, te, next);
 
                RTE_LOG(DEBUG, EAL, "Found ring: '%s' at %p\n", r->name, mz->addr);
        }
+       rte_rwlock_write_unlock(RTE_EAL_TAILQ_RWLOCK);
 
 #ifdef RTE_LIBRTE_IVSHMEM_DEBUG
        rte_memzone_dump(stdout);
index 550507d..2380a43 100644 (file)
@@ -42,7 +42,7 @@ SRCS-$(CONFIG_RTE_LIBRTE_RING) := rte_ring.c
 # install includes
 SYMLINK-$(CONFIG_RTE_LIBRTE_RING)-include := rte_ring.h
 
-# this lib needs eal
-DEPDIRS-$(CONFIG_RTE_LIBRTE_RING) += lib/librte_eal
+# this lib needs eal and rte_malloc
+DEPDIRS-$(CONFIG_RTE_LIBRTE_RING) += lib/librte_eal lib/librte_malloc
 
 include $(RTE_SDK)/mk/rte.lib.mk
index c62065d..b9ddccc 100644 (file)
@@ -75,6 +75,7 @@
 #include <rte_log.h>
 #include <rte_memory.h>
 #include <rte_memzone.h>
+#include <rte_malloc.h>
 #include <rte_launch.h>
 #include <rte_tailq.h>
 #include <rte_eal.h>
@@ -89,7 +90,7 @@
 
 #include "rte_ring.h"
 
-TAILQ_HEAD(rte_ring_list, rte_ring);
+TAILQ_HEAD(rte_ring_list, rte_tailq_entry);
 
 /* true if x is a power of 2 */
 #define POWEROF2(x) ((((x)-1) & (x)) == 0)
@@ -155,6 +156,7 @@ rte_ring_create(const char *name, unsigned count, int socket_id,
 {
        char mz_name[RTE_MEMZONE_NAMESIZE];
        struct rte_ring *r;
+       struct rte_tailq_entry *te;
        const struct rte_memzone *mz;
        ssize_t ring_size;
        int mz_flags = 0;
@@ -173,6 +175,13 @@ rte_ring_create(const char *name, unsigned count, int socket_id,
                return NULL;
        }
 
+       te = rte_zmalloc("RING_TAILQ_ENTRY", sizeof(*te), 0);
+       if (te == NULL) {
+               RTE_LOG(ERR, RING, "Cannot reserve memory for tailq\n");
+               rte_errno = ENOMEM;
+               return NULL;
+       }
+
        snprintf(mz_name, sizeof(mz_name), "%s%s", RTE_RING_MZ_PREFIX, name);
 
        rte_rwlock_write_lock(RTE_EAL_TAILQ_RWLOCK);
@@ -186,10 +195,14 @@ rte_ring_create(const char *name, unsigned count, int socket_id,
                /* no need to check return value here, we already checked the
                 * arguments above */
                rte_ring_init(r, name, count, flags);
-               TAILQ_INSERT_TAIL(ring_list, r, next);
+
+               te->data = (void *) r;
+
+               TAILQ_INSERT_TAIL(ring_list, te, next);
        } else {
                r = NULL;
                RTE_LOG(ERR, RING, "Cannot reserve memory\n");
+               rte_free(te);
        }
        rte_rwlock_write_unlock(RTE_EAL_TAILQ_RWLOCK);
 
@@ -272,7 +285,7 @@ rte_ring_dump(FILE *f, const struct rte_ring *r)
 void
 rte_ring_list_dump(FILE *f)
 {
-       const struct rte_ring *mp;
+       const struct rte_tailq_entry *te;
        struct rte_ring_list *ring_list;
 
        /* check that we have an initialised tail queue */
@@ -284,8 +297,8 @@ rte_ring_list_dump(FILE *f)
 
        rte_rwlock_read_lock(RTE_EAL_TAILQ_RWLOCK);
 
-       TAILQ_FOREACH(mp, ring_list, next) {
-               rte_ring_dump(f, mp);
+       TAILQ_FOREACH(te, ring_list, next) {
+               rte_ring_dump(f, (struct rte_ring *) te->data);
        }
 
        rte_rwlock_read_unlock(RTE_EAL_TAILQ_RWLOCK);
@@ -295,7 +308,8 @@ rte_ring_list_dump(FILE *f)
 struct rte_ring *
 rte_ring_lookup(const char *name)
 {
-       struct rte_ring *r;
+       struct rte_tailq_entry *te;
+       struct rte_ring *r = NULL;
        struct rte_ring_list *ring_list;
 
        /* check that we have an initialized tail queue */
@@ -307,15 +321,18 @@ rte_ring_lookup(const char *name)
 
        rte_rwlock_read_lock(RTE_EAL_TAILQ_RWLOCK);
 
-       TAILQ_FOREACH(r, ring_list, next) {
+       TAILQ_FOREACH(te, ring_list, next) {
+               r = (struct rte_ring *) te->data;
                if (strncmp(name, r->name, RTE_RING_NAMESIZE) == 0)
                        break;
        }
 
        rte_rwlock_read_unlock(RTE_EAL_TAILQ_RWLOCK);
 
-       if (r == NULL)
+       if (te == NULL) {
                rte_errno = ENOENT;
+               return NULL;
+       }
 
        return r;
 }
index 4f3e20f..3920830 100644 (file)
@@ -138,8 +138,6 @@ struct rte_ring_debug_stats {
  * a problem.
  */
 struct rte_ring {
-       TAILQ_ENTRY(rte_ring) next;      /**< Next in list. */
-
        char name[RTE_RING_NAMESIZE];    /**< Name of the ring. */
        int flags;                       /**< Flags supplied at creation. */