From 4542f8939779d7acbbdebbf45e32e5df291eec6d Mon Sep 17 00:00:00 2001 From: Anatoly Burakov Date: Fri, 20 Jun 2014 16:42:20 +0100 Subject: [PATCH] hash: make tailq fully local 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. Technically, only rte_ring structure require modification, because IVSHMEM is only using memzones (which aren't in TAILQs) and rings, but for consistency's sake other TAILQ-based data structures were adapted as well. Signed-off-by: Anatoly Burakov Acked-by: Konstantin Ananyev --- lib/librte_hash/rte_fbk_hash.c | 73 +++++++++++++++++++++++++++------- lib/librte_hash/rte_fbk_hash.h | 3 -- lib/librte_hash/rte_hash.c | 61 +++++++++++++++++++++++----- lib/librte_hash/rte_hash.h | 2 - 4 files changed, 111 insertions(+), 28 deletions(-) diff --git a/lib/librte_hash/rte_fbk_hash.c b/lib/librte_hash/rte_fbk_hash.c index d1caabce62..421e1cdd21 100644 --- a/lib/librte_hash/rte_fbk_hash.c +++ b/lib/librte_hash/rte_fbk_hash.c @@ -54,7 +54,7 @@ #include "rte_fbk_hash.h" -TAILQ_HEAD(rte_fbk_hash_list, rte_fbk_hash_table); +TAILQ_HEAD(rte_fbk_hash_list, rte_tailq_entry); /** * Performs a lookup for an existing hash table, and returns a pointer to @@ -69,24 +69,29 @@ TAILQ_HEAD(rte_fbk_hash_list, rte_fbk_hash_table); struct rte_fbk_hash_table * rte_fbk_hash_find_existing(const char *name) { - struct rte_fbk_hash_table *h; + struct rte_fbk_hash_table *h = NULL; + struct rte_tailq_entry *te; struct rte_fbk_hash_list *fbk_hash_list; /* check that we have an initialised tail queue */ if ((fbk_hash_list = - RTE_TAILQ_LOOKUP_BY_IDX(RTE_TAILQ_FBK_HASH, rte_fbk_hash_list)) == NULL) { + RTE_TAILQ_LOOKUP_BY_IDX(RTE_TAILQ_FBK_HASH, + rte_fbk_hash_list)) == NULL) { rte_errno = E_RTE_NO_TAILQ; return NULL; } rte_rwlock_read_lock(RTE_EAL_TAILQ_RWLOCK); - TAILQ_FOREACH(h, fbk_hash_list, next) { + TAILQ_FOREACH(te, fbk_hash_list, next) { + h = (struct rte_fbk_hash_table *) te->data; if (strncmp(name, h->name, RTE_FBK_HASH_NAMESIZE) == 0) break; } rte_rwlock_read_unlock(RTE_EAL_TAILQ_RWLOCK); - if (h == NULL) + if (te == NULL) { rte_errno = ENOENT; + return NULL; + } return h; } @@ -104,6 +109,7 @@ struct rte_fbk_hash_table * rte_fbk_hash_create(const struct rte_fbk_hash_params *params) { struct rte_fbk_hash_table *ht = NULL; + struct rte_tailq_entry *te; char hash_name[RTE_FBK_HASH_NAMESIZE]; const uint32_t mem_size = sizeof(*ht) + (sizeof(ht->t[0]) * params->entries); @@ -112,7 +118,8 @@ rte_fbk_hash_create(const struct rte_fbk_hash_params *params) /* check that we have an initialised tail queue */ if ((fbk_hash_list = - RTE_TAILQ_LOOKUP_BY_IDX(RTE_TAILQ_FBK_HASH, rte_fbk_hash_list)) == NULL) { + RTE_TAILQ_LOOKUP_BY_IDX(RTE_TAILQ_FBK_HASH, + rte_fbk_hash_list)) == NULL) { rte_errno = E_RTE_NO_TAILQ; return NULL; } @@ -134,20 +141,28 @@ rte_fbk_hash_create(const struct rte_fbk_hash_params *params) rte_rwlock_write_lock(RTE_EAL_TAILQ_RWLOCK); /* guarantee there's no existing */ - TAILQ_FOREACH(ht, fbk_hash_list, next) { + TAILQ_FOREACH(te, fbk_hash_list, next) { + ht = (struct rte_fbk_hash_table *) te->data; if (strncmp(params->name, ht->name, RTE_FBK_HASH_NAMESIZE) == 0) break; } - if (ht != NULL) + if (te != NULL) goto exit; + te = rte_zmalloc("FBK_HASH_TAILQ_ENTRY", sizeof(*te), 0); + if (te == NULL) { + RTE_LOG(ERR, HASH, "Failed to allocate tailq entry\n"); + goto exit; + } + /* Allocate memory for table. */ - ht = (struct rte_fbk_hash_table *)rte_malloc_socket(hash_name, mem_size, + ht = (struct rte_fbk_hash_table *)rte_zmalloc_socket(hash_name, mem_size, 0, params->socket_id); - if (ht == NULL) + if (ht == NULL) { + RTE_LOG(ERR, HASH, "Failed to allocate fbk hash table\n"); + rte_free(te); goto exit; - - memset(ht, 0, mem_size); + } /* Set up hash table context. */ snprintf(ht->name, sizeof(ht->name), "%s", params->name); @@ -169,7 +184,9 @@ rte_fbk_hash_create(const struct rte_fbk_hash_params *params) ht->init_val = RTE_FBK_HASH_INIT_VAL_DEFAULT; } - TAILQ_INSERT_TAIL(fbk_hash_list, ht, next); + te->data = (void *) ht; + + TAILQ_INSERT_TAIL(fbk_hash_list, te, next); exit: rte_rwlock_write_unlock(RTE_EAL_TAILQ_RWLOCK); @@ -186,10 +203,38 @@ exit: void rte_fbk_hash_free(struct rte_fbk_hash_table *ht) { + struct rte_tailq_entry *te; + struct rte_fbk_hash_list *fbk_hash_list; + if (ht == NULL) return; - RTE_EAL_TAILQ_REMOVE(RTE_TAILQ_FBK_HASH, rte_fbk_hash_list, ht); + /* check that we have an initialised tail queue */ + if ((fbk_hash_list = + RTE_TAILQ_LOOKUP_BY_IDX(RTE_TAILQ_FBK_HASH, + rte_fbk_hash_list)) == NULL) { + rte_errno = E_RTE_NO_TAILQ; + return; + } + + rte_rwlock_write_lock(RTE_EAL_TAILQ_RWLOCK); + + /* find out tailq entry */ + TAILQ_FOREACH(te, fbk_hash_list, next) { + if (te->data == (void *) ht) + break; + } + + if (te == NULL) { + rte_rwlock_write_unlock(RTE_EAL_TAILQ_RWLOCK); + return; + } + + TAILQ_REMOVE(fbk_hash_list, te, next); + + rte_rwlock_write_unlock(RTE_EAL_TAILQ_RWLOCK); + rte_free(ht); + rte_free(te); } diff --git a/lib/librte_hash/rte_fbk_hash.h b/lib/librte_hash/rte_fbk_hash.h index 4d1a316795..3d229bf9a7 100644 --- a/lib/librte_hash/rte_fbk_hash.h +++ b/lib/librte_hash/rte_fbk_hash.h @@ -103,11 +103,8 @@ union rte_fbk_hash_entry { }; - /** The four-byte key hash table structure. */ struct rte_fbk_hash_table { - TAILQ_ENTRY(rte_fbk_hash_table) next; /**< Linked list. */ - char name[RTE_FBK_HASH_NAMESIZE]; /**< Name of the hash. */ uint32_t entries; /**< Total number of entries. */ uint32_t entries_per_bucket; /**< Number of entries in a bucket. */ diff --git a/lib/librte_hash/rte_hash.c b/lib/librte_hash/rte_hash.c index b7d882b411..d02b6b4faf 100644 --- a/lib/librte_hash/rte_hash.c +++ b/lib/librte_hash/rte_hash.c @@ -60,7 +60,7 @@ #include "rte_hash.h" -TAILQ_HEAD(rte_hash_list, rte_hash); +TAILQ_HEAD(rte_hash_list, rte_tailq_entry); /* Macro to enable/disable run-time checking of function parameters */ #if defined(RTE_LIBRTE_HASH_DEBUG) @@ -141,24 +141,29 @@ find_first(uint32_t sig, const uint32_t *sig_bucket, uint32_t num_sigs) struct rte_hash * rte_hash_find_existing(const char *name) { - struct rte_hash *h; + struct rte_hash *h = NULL; + struct rte_tailq_entry *te; struct rte_hash_list *hash_list; /* check that we have an initialised tail queue */ - if ((hash_list = RTE_TAILQ_LOOKUP_BY_IDX(RTE_TAILQ_HASH, rte_hash_list)) == NULL) { + if ((hash_list = + RTE_TAILQ_LOOKUP_BY_IDX(RTE_TAILQ_HASH, rte_hash_list)) == NULL) { rte_errno = E_RTE_NO_TAILQ; return NULL; } rte_rwlock_read_lock(RTE_EAL_TAILQ_RWLOCK); - TAILQ_FOREACH(h, hash_list, next) { + TAILQ_FOREACH(te, hash_list, next) { + h = (struct rte_hash *) te->data; if (strncmp(name, h->name, RTE_HASH_NAMESIZE) == 0) break; } rte_rwlock_read_unlock(RTE_EAL_TAILQ_RWLOCK); - if (h == NULL) + if (te == NULL) { rte_errno = ENOENT; + return NULL; + } return h; } @@ -166,6 +171,7 @@ struct rte_hash * rte_hash_create(const struct rte_hash_parameters *params) { struct rte_hash *h = NULL; + struct rte_tailq_entry *te; uint32_t num_buckets, sig_bucket_size, key_size, hash_tbl_size, sig_tbl_size, key_tbl_size, mem_size; char hash_name[RTE_HASH_NAMESIZE]; @@ -212,17 +218,25 @@ rte_hash_create(const struct rte_hash_parameters *params) rte_rwlock_write_lock(RTE_EAL_TAILQ_RWLOCK); /* guarantee there's no existing */ - TAILQ_FOREACH(h, hash_list, next) { + TAILQ_FOREACH(te, hash_list, next) { + h = (struct rte_hash *) te->data; if (strncmp(params->name, h->name, RTE_HASH_NAMESIZE) == 0) break; } - if (h != NULL) + if (te != NULL) + goto exit; + + te = rte_zmalloc("HASH_TAILQ_ENTRY", sizeof(*te), 0); + if (te == NULL) { + RTE_LOG(ERR, HASH, "tailq entry allocation failed\n"); goto exit; + } h = (struct rte_hash *)rte_zmalloc_socket(hash_name, mem_size, CACHE_LINE_SIZE, params->socket_id); if (h == NULL) { RTE_LOG(ERR, HASH, "memory allocation failed\n"); + rte_free(te); goto exit; } @@ -242,7 +256,9 @@ rte_hash_create(const struct rte_hash_parameters *params) h->hash_func = (params->hash_func == NULL) ? DEFAULT_HASH_FUNC : params->hash_func; - TAILQ_INSERT_TAIL(hash_list, h, next); + te->data = (void *) h; + + TAILQ_INSERT_TAIL(hash_list, te, next); exit: rte_rwlock_write_unlock(RTE_EAL_TAILQ_RWLOCK); @@ -253,11 +269,38 @@ exit: void rte_hash_free(struct rte_hash *h) { + struct rte_tailq_entry *te; + struct rte_hash_list *hash_list; + if (h == NULL) return; - RTE_EAL_TAILQ_REMOVE(RTE_TAILQ_HASH, rte_hash_list, h); + /* check that we have an initialised tail queue */ + if ((hash_list = + RTE_TAILQ_LOOKUP_BY_IDX(RTE_TAILQ_HASH, rte_hash_list)) == NULL) { + rte_errno = E_RTE_NO_TAILQ; + return; + } + + rte_rwlock_write_lock(RTE_EAL_TAILQ_RWLOCK); + + /* find out tailq entry */ + TAILQ_FOREACH(te, hash_list, next) { + if (te->data == (void *) h) + break; + } + + if (te == NULL) { + rte_rwlock_write_unlock(RTE_EAL_TAILQ_RWLOCK); + return; + } + + TAILQ_REMOVE(hash_list, te, next); + + rte_rwlock_write_unlock(RTE_EAL_TAILQ_RWLOCK); + rte_free(h); + rte_free(te); } static inline int32_t diff --git a/lib/librte_hash/rte_hash.h b/lib/librte_hash/rte_hash.h index 5228e3a05d..2ecaf1ad19 100644 --- a/lib/librte_hash/rte_hash.h +++ b/lib/librte_hash/rte_hash.h @@ -86,8 +86,6 @@ struct rte_hash_parameters { /** A hash table structure. */ struct rte_hash { - TAILQ_ENTRY(rte_hash) next;/**< Next in list. */ - char name[RTE_HASH_NAMESIZE]; /**< Name of the hash. */ uint32_t entries; /**< Total table entries. */ uint32_t bucket_entries; /**< Bucket entries. */ -- 2.20.1