From mboxrd@z Thu Jan 1 00:00:00 1970 From: Dhana Eadala Subject: [PATCH] hash: fix memcmp function pointer in multi-process environment Date: Sun, 13 Mar 2016 22:16:48 -0400 Message-ID: <1457921808-14261-1-git-send-email-edreddy@gmail.com> Cc: dev@dpdk.org, Dhana Eadala To: bruce.richardson@intel.com, pablo.de.lara.guarch@intel.com, michael.qiu@intel.com Return-path: Received: from mail-qk0-f172.google.com (mail-qk0-f172.google.com [209.85.220.172]) by dpdk.org (Postfix) with ESMTP id EB1FE4A65 for ; Mon, 14 Mar 2016 03:16:52 +0100 (CET) Received: by mail-qk0-f172.google.com with SMTP id s5so69755504qkd.0 for ; Sun, 13 Mar 2016 19:16:52 -0700 (PDT) List-Id: patches and discussions about DPDK List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dev-bounces@dpdk.org Sender: "dev" We found a problem in dpdk-2.2 using under multi-process environment. Here is the brief description how we are using the dpdk: We have two processes proc1, proc2 using dpdk. These proc1 and proc2 are two different compiled binaries. proc1 is started as primary process and proc2 as secondary process. proc1: Calls srcHash = rte_hash_create("src_hash_name") to create rte_hash structure. As part of this, this api initalized the rte_hash structure and set the srcHash->rte_hash_cmp_eq to the address of memcmp() from proc1 address space. proc2: calls srcHash = rte_hash_find_existing("src_hash_name"). This function call returns the rte_hash created by proc1. This srcHash->rte_hash_cmp_eq still points to the address of memcmp() from proc1 address space. Later proc2 calls rte_hash_lookup_with_hash(srcHash, (const void*) &key, key.sig); rte_hash_lookup_with_hash() invokes __rte_hash_lookup_with_hash(), which in turn calls h->rte_hash_cmp_eq(key, k->key, h->key_len). This leads to a crash as h->rte_hash_cmp_eq is an address from proc1 address space and is invalid address in proc2 address space. We found, from dpdk documentation, that " The use of function pointers between multiple processes running based of different compiled binaries is not supported, since the location of a given function in one process may be different to its location in a second. This prevents the librte_hash library from behaving properly as in a multi- threaded instance, since it uses a pointer to the hash function internally. To work around this issue, it is recommended that multi-process applications perform the hash calculations by directly calling the hashing function from the code and then using the rte_hash_add_with_hash()/rte_hash_lookup_with_hash() functions instead of the functions which do the hashing internally, such as rte_hash_add()/rte_hash_lookup(). " We did follow the recommended steps by invoking rte_hash_lookup_with_hash(). It was no issue up to and including dpdk-2.0. In later releases started crashing because rte_hash_cmp_eq is introduced in dpdk-2.1 We fixed it with the following patch and would like to submit the patch to dpdk.org. Patch is created such that, if anyone wanted to use dpdk in multi-process environment with function pointers not shared, they need to define RTE_LIB_MP_NO_FUNC_PTR in their Makefile. Without defining this flag in Makefile, it works as it is now. Signed-off-by: Dhana Eadala --- lib/librte_hash/rte_cuckoo_hash.c | 28 ++++++++++++++++++++++++++++ 1 file changed, 28 insertions(+) diff --git a/lib/librte_hash/rte_cuckoo_hash.c b/lib/librte_hash/rte_cuckoo_hash.c index 3e3167c..0946777 100644 --- a/lib/librte_hash/rte_cuckoo_hash.c +++ b/lib/librte_hash/rte_cuckoo_hash.c @@ -594,7 +594,11 @@ __rte_hash_add_key_with_hash(const struct rte_hash *h, const void *key, prim_bkt->signatures[i].alt == alt_hash) { k = (struct rte_hash_key *) ((char *)keys + prim_bkt->key_idx[i] * h->key_entry_size); +#ifdef RTE_LIB_MP_NO_FUNC_PTR + if (memcmp(key, k->key, h->key_len) == 0) { +#else if (h->rte_hash_cmp_eq(key, k->key, h->key_len) == 0) { +#endif /* Enqueue index of free slot back in the ring. */ enqueue_slot_back(h, cached_free_slots, slot_id); /* Update data */ @@ -614,7 +618,11 @@ __rte_hash_add_key_with_hash(const struct rte_hash *h, const void *key, sec_bkt->signatures[i].current == alt_hash) { k = (struct rte_hash_key *) ((char *)keys + sec_bkt->key_idx[i] * h->key_entry_size); +#ifdef RTE_LIB_MP_NO_FUNC_PTR + if (memcmp(key, k->key, h->key_len) == 0) { +#else if (h->rte_hash_cmp_eq(key, k->key, h->key_len) == 0) { +#endif /* Enqueue index of free slot back in the ring. */ enqueue_slot_back(h, cached_free_slots, slot_id); /* Update data */ @@ -725,7 +733,11 @@ __rte_hash_lookup_with_hash(const struct rte_hash *h, const void *key, bkt->signatures[i].sig != NULL_SIGNATURE) { k = (struct rte_hash_key *) ((char *)keys + bkt->key_idx[i] * h->key_entry_size); +#ifdef RTE_LIB_MP_NO_FUNC_PTR + if (memcmp (key, k->key, h->key_len) == 0) { +#else if (h->rte_hash_cmp_eq(key, k->key, h->key_len) == 0) { +#endif if (data != NULL) *data = k->pdata; /* @@ -748,7 +760,11 @@ __rte_hash_lookup_with_hash(const struct rte_hash *h, const void *key, bkt->signatures[i].alt == sig) { k = (struct rte_hash_key *) ((char *)keys + bkt->key_idx[i] * h->key_entry_size); +#ifdef RTE_LIB_MP_NO_FUNC_PTR + if (memcmp(key, k->key, h->key_len) == 0) { +#else if (h->rte_hash_cmp_eq(key, k->key, h->key_len) == 0) { +#endif if (data != NULL) *data = k->pdata; /* @@ -840,7 +856,11 @@ __rte_hash_del_key_with_hash(const struct rte_hash *h, const void *key, bkt->signatures[i].sig != NULL_SIGNATURE) { k = (struct rte_hash_key *) ((char *)keys + bkt->key_idx[i] * h->key_entry_size); +#ifdef RTE_LIB_MP_NO_FUNC_PTR + if (memcmp(key, k->key, h->key_len) == 0) { +#else if (h->rte_hash_cmp_eq(key, k->key, h->key_len) == 0) { +#endif remove_entry(h, bkt, i); /* @@ -863,7 +883,11 @@ __rte_hash_del_key_with_hash(const struct rte_hash *h, const void *key, bkt->signatures[i].sig != NULL_SIGNATURE) { k = (struct rte_hash_key *) ((char *)keys + bkt->key_idx[i] * h->key_entry_size); +#ifdef RTE_LIB_MP_NO_FUNC_PTR + if (memcmp(key, k->key, h->key_len) == 0) { +#else if (h->rte_hash_cmp_eq(key, k->key, h->key_len) == 0) { +#endif remove_entry(h, bkt, i); /* @@ -980,7 +1004,11 @@ lookup_stage3(unsigned idx, const struct rte_hash_key *key_slot, const void * co unsigned hit; unsigned key_idx; +#ifdef RTE_LIB_MP_NO_FUNC_PTR + hit = !memcmp(key_slot->key, keys[idx], h->key_len); +#else hit = !h->rte_hash_cmp_eq(key_slot->key, keys[idx], h->key_len); +#endif if (data != NULL) data[idx] = key_slot->pdata; -- 2.5.0