diff mbox series

[v4,1/3] hash: fix position bug in 'free key with position'

Message ID 20190509171907.14693-2-dharmik.thakkar@arm.com (mailing list archive)
State Accepted, archived
Headers
Series hash: fix bugs in 'free key with position' |

Checks

Context Check Description
ci/checkpatch success coding style OK
ci/Intel-compilation success Compilation OK
ci/mellanox-Performance-Testing success Performance Testing PASS
ci/intel-Performance-Testing success Performance Testing PASS

Commit Message

Dharmik Thakkar May 9, 2019, 5:19 p.m. UTC 
  Currently, in rte_hash_free_key_with_position(), the position returned
to the ring of free_slots leads to an unexpected conflict with a key
already in use.

This patch fixes incorrect position returned to the ring of free_slots.

Bugzilla ID: 261
Fixes: 9d033dac7d7c ("hash: support no free on delete")
Cc: honnappa.nagarahalli@arm.com
Cc: stable@dpdk.org

Reported-by: Linfan <zhongdahulinfan@163.com>
Suggested-by: Linfan <zhongdahulinfan@163.com>
Signed-off-by: Dharmik Thakkar <dharmik.thakkar@arm.com>
---
 lib/librte_hash/rte_cuckoo_hash.c | 13 ++++++++-----
 1 file changed, 8 insertions(+), 5 deletions(-)

Comments

Wang, Yipeng1 May 9, 2019, 7:27 p.m. UTC | #1 
>-----Original Message-----
>From: Dharmik Thakkar [mailto:dharmik.thakkar@arm.com]
>Sent: Thursday, May 9, 2019 10:19 AM
>To: Wang, Yipeng1 <yipeng1.wang@intel.com>; Gobriel, Sameh <sameh.gobriel@intel.com>; Richardson, Bruce
><bruce.richardson@intel.com>; De Lara Guarch, Pablo <pablo.de.lara.guarch@intel.com>
>Cc: dev@dpdk.org; honnappa.nagarahalli@arm.com; zhongdahulinfan@163.com; Dharmik Thakkar <dharmik.thakkar@arm.com>;
>stable@dpdk.org
>Subject: [PATCH v4 1/3] hash: fix position bug in 'free key with position'
>
>Currently, in rte_hash_free_key_with_position(), the position returned
>to the ring of free_slots leads to an unexpected conflict with a key
>already in use.
>
>This patch fixes incorrect position returned to the ring of free_slots.
>
>Bugzilla ID: 261
>Fixes: 9d033dac7d7c ("hash: support no free on delete")
>Cc: honnappa.nagarahalli@arm.com
>Cc: stable@dpdk.org
>
>Reported-by: Linfan <zhongdahulinfan@163.com>
>Suggested-by: Linfan <zhongdahulinfan@163.com>
>Signed-off-by: Dharmik Thakkar <dharmik.thakkar@arm.com>
>---
> lib/librte_hash/rte_cuckoo_hash.c | 13 ++++++++-----
> 1 file changed, 8 insertions(+), 5 deletions(-)
>
>diff --git a/lib/librte_hash/rte_cuckoo_hash.c b/lib/librte_hash/rte_cuckoo_hash.c
>index 261267b7fd3d..5029f9f61fae 100644
>--- a/lib/librte_hash/rte_cuckoo_hash.c
>+++ b/lib/librte_hash/rte_cuckoo_hash.c
>@@ -1587,14 +1587,17 @@ int __rte_experimental
> rte_hash_free_key_with_position(const struct rte_hash *h,
> 				const int32_t position)
> {
>-	RETURN_IF_TRUE(((h == NULL) || (position == EMPTY_SLOT)), -EINVAL);
>+	/*  Key index where key is stored, adding the first dummy index*/
[Wang, Yipeng] Minor issue, missing a space at the end.

Acked-by: Yipeng Wang <yipeng1.wang@intel.com>
Dharmik Thakkar May 9, 2019, 8:14 p.m. UTC | #2 
> On May 9, 2019, at 2:27 PM, Wang, Yipeng1 <yipeng1.wang@intel.com> wrote:
> 
> 
> 
>> -----Original Message-----
>> From: Dharmik Thakkar [mailto:dharmik.thakkar@arm.com]
>> Sent: Thursday, May 9, 2019 10:19 AM
>> To: Wang, Yipeng1 <yipeng1.wang@intel.com>; Gobriel, Sameh <sameh.gobriel@intel.com>; Richardson, Bruce
>> <bruce.richardson@intel.com>; De Lara Guarch, Pablo <pablo.de.lara.guarch@intel.com>
>> Cc: dev@dpdk.org; honnappa.nagarahalli@arm.com; zhongdahulinfan@163.com; Dharmik Thakkar <dharmik.thakkar@arm.com>;
>> stable@dpdk.org
>> Subject: [PATCH v4 1/3] hash: fix position bug in 'free key with position'
>> 
>> Currently, in rte_hash_free_key_with_position(), the position returned
>> to the ring of free_slots leads to an unexpected conflict with a key
>> already in use.
>> 
>> This patch fixes incorrect position returned to the ring of free_slots.
>> 
>> Bugzilla ID: 261
>> Fixes: 9d033dac7d7c ("hash: support no free on delete")
>> Cc: honnappa.nagarahalli@arm.com
>> Cc: stable@dpdk.org
>> 
>> Reported-by: Linfan <zhongdahulinfan@163.com>
>> Suggested-by: Linfan <zhongdahulinfan@163.com>
>> Signed-off-by: Dharmik Thakkar <dharmik.thakkar@arm.com>
>> ---
>> lib/librte_hash/rte_cuckoo_hash.c | 13 ++++++++-----
>> 1 file changed, 8 insertions(+), 5 deletions(-)
>> 
>> diff --git a/lib/librte_hash/rte_cuckoo_hash.c b/lib/librte_hash/rte_cuckoo_hash.c
>> index 261267b7fd3d..5029f9f61fae 100644
>> --- a/lib/librte_hash/rte_cuckoo_hash.c
>> +++ b/lib/librte_hash/rte_cuckoo_hash.c
>> @@ -1587,14 +1587,17 @@ int __rte_experimental
>> rte_hash_free_key_with_position(const struct rte_hash *h,
>> 				const int32_t position)
>> {
>> -	RETURN_IF_TRUE(((h == NULL) || (position == EMPTY_SLOT)), -EINVAL);
>> +	/*  Key index where key is stored, adding the first dummy index*/
> [Wang, Yipeng] Minor issue, missing a space at the end.
Is there a need to update the version?
> 
> Acked-by: Yipeng Wang <yipeng1.wang@intel.com>
Thank you!
Thomas Monjalon May 9, 2019, 8:23 p.m. UTC | #3 
09/05/2019 22:14, Dharmik Thakkar:
> > On May 9, 2019, at 2:27 PM, Wang, Yipeng1 <yipeng1.wang@intel.com> wrote:
> >> -	RETURN_IF_TRUE(((h == NULL) || (position == EMPTY_SLOT)), -EINVAL);
> >> +	/*  Key index where key is stored, adding the first dummy index*/
> > [Wang, Yipeng] Minor issue, missing a space at the end.
> Is there a need to update the version?

No, I am applying.
Dharmik Thakkar May 9, 2019, 8:30 p.m. UTC | #4 
> On May 9, 2019, at 3:23 PM, Thomas Monjalon <thomas@monjalon.net> wrote:
> 
> 09/05/2019 22:14, Dharmik Thakkar:
>>> On May 9, 2019, at 2:27 PM, Wang, Yipeng1 <yipeng1.wang@intel.com> wrote:
>>>> -	RETURN_IF_TRUE(((h == NULL) || (position == EMPTY_SLOT)), -EINVAL);
>>>> +	/*  Key index where key is stored, adding the first dummy index*/
>>> [Wang, Yipeng] Minor issue, missing a space at the end.
>> Is there a need to update the version?
> 
> No, I am applying.
Sounds good. Thank you, Thomas!
> 
> 
>
diff mbox series

Patch

diff --git a/lib/librte_hash/rte_cuckoo_hash.c b/lib/librte_hash/rte_cuckoo_hash.c
index 261267b7fd3d..5029f9f61fae 100644
--- a/lib/librte_hash/rte_cuckoo_hash.c
+++ b/lib/librte_hash/rte_cuckoo_hash.c
@@ -1587,14 +1587,17 @@  int __rte_experimental
 rte_hash_free_key_with_position(const struct rte_hash *h,
 				const int32_t position)
 {
-	RETURN_IF_TRUE(((h == NULL) || (position == EMPTY_SLOT)), -EINVAL);
+	/*  Key index where key is stored, adding the first dummy index*/
+	uint32_t key_idx = position + 1;
+
+	RETURN_IF_TRUE(((h == NULL) || (key_idx == EMPTY_SLOT)), -EINVAL);
 
 	unsigned int lcore_id, n_slots;
 	struct lcore_cache *cached_free_slots;
-	const int32_t total_entries = h->num_buckets * RTE_HASH_BUCKET_ENTRIES;
+	const uint32_t total_entries = h->num_buckets * RTE_HASH_BUCKET_ENTRIES;
 
 	/* Out of bounds */
-	if (position >= total_entries)
+	if (key_idx >= total_entries)
 		return -EINVAL;
 	if (h->ext_table_support && h->readwrite_concur_lf_support) {
 		uint32_t index = h->ext_bkt_to_free[position];
@@ -1619,11 +1622,11 @@  rte_hash_free_key_with_position(const struct rte_hash *h,
 		}
 		/* Put index of new free slot in cache. */
 		cached_free_slots->objs[cached_free_slots->len] =
-					(void *)((uintptr_t)position);
+					(void *)((uintptr_t)key_idx);
 		cached_free_slots->len++;
 	} else {
 		rte_ring_sp_enqueue(h->free_slots,
-				(void *)((uintptr_t)position));
+				(void *)((uintptr_t)key_idx));
 	}
 
 	return 0;