From: "Alipour, Mehrdad" <malipour@ciena.com>
To: "dev@dpdk.org" <dev@dpdk.org>
Subject: [dpdk-dev] rte_table_hash_ext.c: rte_table_hash_ext_lookup is non-reentrant
Date: Tue, 6 Jul 2021 14:27:18 +0000 [thread overview]
Message-ID: <DM5PR0401MB3590D330FEAE33A46D287577C41B9@DM5PR0401MB3590.namprd04.prod.outlook.com> (raw)
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, bhiltsch@ciena.com<mailto:bhiltsch@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
malipour@ciena.com
reply other threads:[~2021-07-06 14:27 UTC|newest]
Thread overview: [no followups] expand[flat|nested] mbox.gz Atom feed
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=DM5PR0401MB3590D330FEAE33A46D287577C41B9@DM5PR0401MB3590.namprd04.prod.outlook.com \
--to=malipour@ciena.com \
--cc=dev@dpdk.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).