[dpdk-dev] rte_table_hash_ext.c: rte_table_hash_ext_lookup is non-reentrant

[Alipour, Mehrdad]

Hi,


We have tested lib/librte_table/rte_table_hash_ext.c: rte_table_hash_ext_lookup 
using dpdk 19.11

and it appears that the function is not reentrant safe causing incorrect hash 
lookup misses at very small rate (< 0.005%)

when invoked by multiple threads in parallel.



The reason for reentrancy issue is as follows:

    Embedding the "grinders" array in the rte_table_hash

   structure results in a hash lookup that is non-reentrant

   because temporary values are stored in the grinders array

   during the hash lookup.  The fix is to put the grinders

   array on the stack.



We propose the following change to fix the reentrancy issue (courtesy of Brian 
Hiltscher,  bhilt...@ciena.com<mailto:bhilt...@ciena.com>):



diff --git a/lib/librte_table/rte_table_hash_ext.c 
b/lib/librte_table/rte_table_hash_ext.c

index 802a24fe0..4a1504c54 100644

--- a/lib/librte_table/rte_table_hash_ext.c

+++ b/lib/librte_table/rte_table_hash_ext.c

@@ -66,6 +66,15 @@ struct grinder {

            uint32_t key_index;

};



+/*

+ * Embedding the "grinders" array in the rte_table_hash

+ * structure results in a hash lookup that is non-reentrant

+ * because temporary values are stored in the grinders array

+ * during the hash lookup.  The fix is to put the grinders

+ * array on the stack.

+ */

+#define FIX_HASH_BUG 1

+

struct rte_table_hash {

             struct rte_table_stats stats;



@@ -85,10 +94,10 @@ struct rte_table_hash {

             uint32_t data_size_shl;

             uint32_t key_stack_tos;

             uint32_t bkt_ext_stack_tos;

-

+#ifndef FIX_HASH_BUG

             /* Grinder */

             struct grinder grinders[RTE_PORT_IN_BURST_SIZE_MAX];

-

+#endif

             /* Tables */

             uint64_t *key_mask;

             struct bucket *buckets;

@@ -856,7 +865,12 @@ static int rte_table_hash_ext_lookup(

             void **entries)

{

             struct rte_table_hash *t = (struct rte_table_hash *) table;

-             struct grinder *g = t->grinders;

+#ifndef FIX_HASH_BUG

+        struct grinder *g = t->grinders;

+#else

+            struct grinder grinders[RTE_PORT_IN_BURST_SIZE_MAX];

+            struct grinder *g = grinders; // make grinders structure per thread

+#endif



Thanks,

Mehrdad Alipour

malip...@ciena.com