On 3/3/2016 11:36 AM, Dhana Eadala wrote:
> 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>
> ---
>
Some comments:
1. your commit log need to refactor, better to limit every line less
than 80 character.
2. I think you could add the ifdef here in
lib/librte_hash/rte_cuckoo_hash.c :
/*
* If x86 architecture is used, select appropriate compare function,
* which may use x86 instrinsics, otherwise use memcmp
*/
#if defined(RTE_ARCH_X86_64) || defined(RTE_ARCH_I686) ||\
defined(RTE_ARCH_X86_X32) || defined(RTE_ARCH_ARM64)
/* Select function to compare keys */
switch (params->key_len) {
case 16:
h->rte_hash_cmp_eq = rte_hash_k16_cmp_eq;
break;
[...]
break;
default:
/* If key is not multiple of 16, use generic memcmp */
h->rte_hash_cmp_eq = memcmp;
}
#else
h->rte_hash_cmp_eq = memcmp;
#endif
So that could remove other #ifdef in those lines.
3. I don't think ask others to write RTE_LIB_MP_NO_FUNC_PTR in makefile
is a good idea, if you really want to do that, please add a doc so that
others could know it.
Thanks,
Michael
