Re: [dpdk-dev] [PATCH v3 1/3] hash: fix position bug in 'free key with position'

["Wang, Yipeng1" <yipeng1.wang@intel.com>]

> -----Original Message-----
> From: Dharmik Thakkar [mailto:dharmik.thakkar@arm.com]
> Sent: Thursday, May 9, 2019 6:39 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 v3 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 | 11 +++++++----
>  1 file changed, 7 insertions(+), 4 deletions(-)
> 
> diff --git a/lib/librte_hash/rte_cuckoo_hash.c
> b/lib/librte_hash/rte_cuckoo_hash.c
> index 261267b7fd3d..8646ca52e60b 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;
> 
>  	/* Out of bounds */
> -	if (position >= total_entries)
> +	if (key_idx >= total_entries)
[Wang, Yipeng] 
Compiling fail after this commit.
/rte_cuckoo_hash.c:1600:14: error: comparison between signed and unsigned integer expressions [-Werror=sign-compare]
  if (key_idx >= total_entries)
please fix.

Please run the test-build and test-meson-build scripts. I currently have some issue with my meson setup on my new system so I will
rely on other reviewers or you guys to pass the meson test.

Logically these two bug fixes are valid and should be upstreamed otherwise this API breaks. But this is the experimental tag for :)

Thanks
Yipeng