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 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);
Under the hood, 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 <edreddy at gmail.com>
---
 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

Reply via email to