[dpdk-dev] [PATCH v2 0/4] hash: improve multiple places

[dpdk-dev] [PATCH v2 0/4] hash: improve multiple places

[Yipeng Wang]

This patch set depends on Honnappa's patch set:
http://patchwork.dpdk.org/cover/47268/

This patch set fixes/improves a couple of places mostly
on unit tests:

commit 1: remove unnecessary code in hash library.
commit 2: use jhash in multiwriter unit test.
commit 3: improve readwrite test to consider the extendable table.
commit 4: remove scaling unit test.


V1->V2:
* In commit 2 change use_jhash to a macro instead of a hard coded
local variable (Bruce).
* Add commit 4 to remove scaling unit test (Bruce).

Signed-off-by: Yipeng Wang <yipeng1.wang@intel.com>

Yipeng Wang (4):
  hash: fix unnecessary pause
  test/hash: change multiwriter test to use jhash
  test/hash: add readwrite test for ext table
  test/hash: remove hash scaling unit test

 lib/librte_hash/rte_cuckoo_hash.c |   4 +-
 test/test/Makefile                |   1 -
 test/test/autotest_data.py        |   6 --
 test/test/meson.build             |   2 -
 test/test/test_hash_multiwriter.c |  10 +-
 test/test/test_hash_readwrite.c   |  70 +++++++++++---
 test/test/test_hash_scaling.c     | 191 --------------------------------------
 7 files changed, 69 insertions(+), 215 deletions(-)
 delete mode 100644 test/test/test_hash_scaling.c

-- 
2.7.4

[dpdk-dev] [PATCH v2 1/4] hash: fix unnecessary pause

[Yipeng Wang]

There is a rte_pause in hash table reset function.
Since the loop is not a polling loop on shared
data structure, the rte_pause is not needed.

Fixes: b26473ff8f4a ("hash: add reset function")
Cc: stable@dpdk.org

Signed-off-by: Yipeng Wang <yipeng1.wang@intel.com>
---
 lib/librte_hash/rte_cuckoo_hash.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/lib/librte_hash/rte_cuckoo_hash.c b/lib/librte_hash/rte_cuckoo_hash.c
index 0648a22..4a2647e 100644
--- a/lib/librte_hash/rte_cuckoo_hash.c
+++ b/lib/librte_hash/rte_cuckoo_hash.c
@@ -574,14 +574,14 @@ rte_hash_reset(struct rte_hash *h)
 
 	/* clear the free ring */
 	while (rte_ring_dequeue(h->free_slots, &ptr) == 0)
-		rte_pause();
+		continue;
 
 	/* clear free extendable bucket ring and memory */
 	if (h->ext_table_support) {
 		memset(h->buckets_ext, 0, h->num_buckets *
 						sizeof(struct rte_hash_bucket));
 		while (rte_ring_dequeue(h->free_ext_bkts, &ptr) == 0)
-			rte_pause();
+			continue;
 	}
 
 	/* Repopulate the free slots ring. Entry zero is reserved for key misses */
-- 
2.7.4

[dpdk-dev] [PATCH v2 2/4] test/hash: change multiwriter test to use jhash

[Yipeng Wang]

With sequential key, the test will cover more corner
cases with jhash instead of crc hash, since jhash
generates more random hash pattern on sequential key.
It is useful for functional verification.

Signed-off-by: Yipeng Wang <yipeng1.wang@intel.com>
---
 test/test/test_hash_multiwriter.c | 10 +++++++++-
 1 file changed, 9 insertions(+), 1 deletion(-)

diff --git a/test/test/test_hash_multiwriter.c b/test/test/test_hash_multiwriter.c
index 6a3eb10..456bc5f 100644
--- a/test/test/test_hash_multiwriter.c
+++ b/test/test/test_hash_multiwriter.c
@@ -12,6 +12,7 @@
 #include <rte_malloc.h>
 #include <rte_random.h>
 #include <rte_spinlock.h>
+#include <rte_jhash.h>
 
 #include "test.h"
 
@@ -31,6 +32,9 @@
 
 #define RTE_APP_TEST_HASH_MULTIWRITER_FAILED 0
 
+/* Use jhash or crc hash */
+#define USE_JHASH 1
+
 struct {
 	uint32_t *keys;
 	uint32_t *found;
@@ -108,10 +112,14 @@ test_hash_multiwriter(void)
 	struct rte_hash_parameters hash_params = {
 		.entries = nb_entries,
 		.key_len = sizeof(uint32_t),
-		.hash_func = rte_hash_crc,
 		.hash_func_init_val = 0,
 		.socket_id = rte_socket_id(),
 	};
+	if (USE_JHASH)
+		hash_params.hash_func = rte_jhash;
+	else
+		hash_params.hash_func = rte_hash_crc;
+
 	if (use_htm)
 		hash_params.extra_flag =
 			RTE_HASH_EXTRA_FLAGS_TRANS_MEM_SUPPORT
-- 
2.7.4

[dpdk-dev] [PATCH v2 3/4] test/hash: add readwrite test for ext table

[Yipeng Wang]

This commit improves the readwrite test to consider
extendable table feature and add more functional tests
to cover more corner cases.

Signed-off-by: Yipeng Wang <yipeng1.wang@intel.com>
---
 test/test/test_hash_readwrite.c | 70 ++++++++++++++++++++++++++++++++++-------
 1 file changed, 58 insertions(+), 12 deletions(-)

diff --git a/test/test/test_hash_readwrite.c b/test/test/test_hash_readwrite.c
index a8fadd0..13c28e0 100644
--- a/test/test/test_hash_readwrite.c
+++ b/test/test/test_hash_readwrite.c
@@ -21,6 +21,8 @@
 #define TOTAL_ENTRY (16*1024*1024)
 #define TOTAL_INSERT (15*1024*1024)
 
+#define TOTAL_INSERT_EXT (16*1024*1024)
+
 #define NUM_TEST 3
 unsigned int core_cnt[NUM_TEST] = {2, 4, 8};
 
@@ -59,8 +61,10 @@ test_hash_readwrite_worker(__attribute__((unused)) void *arg)
 	uint64_t i, offset;
 	uint32_t lcore_id = rte_lcore_id();
 	uint64_t begin, cycles;
-	int ret;
+	int *ret;
 
+	ret = rte_malloc(NULL, sizeof(int) *
+				tbl_rw_test_param.num_insert, 0);
 	for (i = 0; i < rte_lcore_count(); i++) {
 		if (slave_core_ids[i] == lcore_id)
 			break;
@@ -79,13 +83,30 @@ test_hash_readwrite_worker(__attribute__((unused)) void *arg)
 				tbl_rw_test_param.keys + i) > 0)
 			break;
 
-		ret = rte_hash_add_key(tbl_rw_test_param.h,
+		ret[i - offset] = rte_hash_add_key(tbl_rw_test_param.h,
 				     tbl_rw_test_param.keys + i);
-		if (ret < 0)
+		if (ret[i - offset] < 0)
+			break;
+
+		/* lookup a random key */
+		uint32_t rand = rte_rand() % (i + 1 - offset);
+
+		if (rte_hash_lookup(tbl_rw_test_param.h,
+				tbl_rw_test_param.keys + rand) != ret[rand])
+			break;
+
+
+		if (rte_hash_del_key(tbl_rw_test_param.h,
+				tbl_rw_test_param.keys + rand) != ret[rand])
+			break;
+
+		ret[rand] = rte_hash_add_key(tbl_rw_test_param.h,
+					tbl_rw_test_param.keys + rand);
+		if (ret[rand] < 0)
 			break;
 
 		if (rte_hash_lookup(tbl_rw_test_param.h,
-				tbl_rw_test_param.keys + i) != ret)
+			tbl_rw_test_param.keys + rand) != ret[rand])
 			break;
 	}
 
@@ -100,7 +121,7 @@ test_hash_readwrite_worker(__attribute__((unused)) void *arg)
 }
 
 static int
-init_params(int use_htm, int use_jhash)
+init_params(int use_ext, int use_htm, int use_jhash)
 {
 	unsigned int i;
 
@@ -129,6 +150,13 @@ init_params(int use_htm, int use_jhash)
 			RTE_HASH_EXTRA_FLAGS_RW_CONCURRENCY |
 			RTE_HASH_EXTRA_FLAGS_MULTI_WRITER_ADD;
 
+	if (use_ext)
+		hash_params.extra_flag |=
+			RTE_HASH_EXTRA_FLAGS_EXT_TABLE;
+	else
+		hash_params.extra_flag &=
+		       ~RTE_HASH_EXTRA_FLAGS_EXT_TABLE;
+
 	hash_params.name = "tests";
 
 	handle = rte_hash_create(&hash_params);
@@ -167,7 +195,7 @@ init_params(int use_htm, int use_jhash)
 }
 
 static int
-test_hash_readwrite_functional(int use_htm)
+test_hash_readwrite_functional(int use_ext, int use_htm)
 {
 	unsigned int i;
 	const void *next_key;
@@ -178,6 +206,7 @@ test_hash_readwrite_functional(int use_htm)
 	uint32_t lost_keys = 0;
 	int use_jhash = 1;
 	int slave_cnt = rte_lcore_count() - 1;
+	uint32_t tot_insert = 0;
 
 	rte_atomic64_init(&gcycles);
 	rte_atomic64_clear(&gcycles);
@@ -185,11 +214,16 @@ test_hash_readwrite_functional(int use_htm)
 	rte_atomic64_init(&ginsertions);
 	rte_atomic64_clear(&ginsertions);
 
-	if (init_params(use_htm, use_jhash) != 0)
+	if (init_params(use_ext, use_htm, use_jhash) != 0)
 		goto err;
 
+	if (use_ext)
+		tot_insert = TOTAL_INSERT_EXT;
+	else
+		tot_insert = TOTAL_INSERT;
+
 	tbl_rw_test_param.num_insert =
-		TOTAL_INSERT / slave_cnt;
+		tot_insert / slave_cnt;
 
 	tbl_rw_test_param.rounded_tot_insert =
 		tbl_rw_test_param.num_insert
@@ -345,7 +379,7 @@ test_hash_readwrite_perf(struct perf *perf_results, int use_htm,
 	rte_atomic64_init(&gwrite_cycles);
 	rte_atomic64_clear(&gwrite_cycles);
 
-	if (init_params(use_htm, use_jhash) != 0)
+	if (init_params(0, use_htm, use_jhash) != 0)
 		goto err;
 
 	/*
@@ -579,7 +613,7 @@ test_hash_readwrite_main(void)
 	 * than writer threads. This is to timing either reader threads or
 	 * writer threads for performance numbers.
 	 */
-	int use_htm, reader_faster;
+	int use_htm, use_ext,  reader_faster;
 	unsigned int i = 0, core_id = 0;
 
 	if (rte_lcore_count() <= 2) {
@@ -602,7 +636,13 @@ test_hash_readwrite_main(void)
 		printf("Test read-write with Hardware transactional memory\n");
 
 		use_htm = 1;
-		if (test_hash_readwrite_functional(use_htm) < 0)
+		use_ext = 0;
+
+		if (test_hash_readwrite_functional(use_ext, use_htm) < 0)
+			return -1;
+
+		use_ext = 1;
+		if (test_hash_readwrite_functional(use_ext, use_htm) < 0)
 			return -1;
 
 		reader_faster = 1;
@@ -621,8 +661,14 @@ test_hash_readwrite_main(void)
 
 	printf("Test read-write without Hardware transactional memory\n");
 	use_htm = 0;
-	if (test_hash_readwrite_functional(use_htm) < 0)
+	use_ext = 0;
+	if (test_hash_readwrite_functional(use_ext, use_htm) < 0)
 		return -1;
+
+	use_ext = 1;
+	if (test_hash_readwrite_functional(use_ext, use_htm) < 0)
+		return -1;
+
 	reader_faster = 1;
 	if (test_hash_readwrite_perf(&non_htm_results, use_htm,
 							reader_faster) < 0)
-- 
2.7.4

[dpdk-dev] [PATCH v2 4/4] test/hash: remove hash scaling unit test

[Yipeng Wang]

The hash scaling unit test is not really needed
any more since the multi-writer is supported now
inside the library and it is tested by multi-writer
unit test.

Signed-off-by: Yipeng Wang <yipeng1.wang@intel.com>
---
 test/test/Makefile            |   1 -
 test/test/autotest_data.py    |   6 --
 test/test/meson.build         |   2 -
 test/test/test_hash_scaling.c | 191 ------------------------------------------
 4 files changed, 200 deletions(-)
 delete mode 100644 test/test/test_hash_scaling.c

diff --git a/test/test/Makefile b/test/test/Makefile
index 41028cc..276e7cd 100644
--- a/test/test/Makefile
+++ b/test/test/Makefile
@@ -112,7 +112,6 @@ SRCS-$(CONFIG_RTE_LIBRTE_HASH) += test_hash.c
 SRCS-$(CONFIG_RTE_LIBRTE_HASH) += test_thash.c
 SRCS-$(CONFIG_RTE_LIBRTE_HASH) += test_hash_perf.c
 SRCS-$(CONFIG_RTE_LIBRTE_HASH) += test_hash_functions.c
-SRCS-$(CONFIG_RTE_LIBRTE_HASH) += test_hash_scaling.c
 SRCS-$(CONFIG_RTE_LIBRTE_HASH) += test_hash_multiwriter.c
 SRCS-$(CONFIG_RTE_LIBRTE_HASH) += test_hash_readwrite.c
 SRCS-$(CONFIG_RTE_LIBRTE_HASH) += test_hash_readwrite_lf.c
diff --git a/test/test/autotest_data.py b/test/test/autotest_data.py
index f68d9b1..31020f1 100644
--- a/test/test/autotest_data.py
+++ b/test/test/autotest_data.py
@@ -345,12 +345,6 @@
         "Report":  None,
     },
     {
-        "Name":    "Hash scaling autotest",
-        "Command": "hash_scaling_autotest",
-        "Func":    default_autotest,
-        "Report":  None,
-    },
-    {
         "Name":    "Hash multiwriter autotest",
         "Command": "hash_multiwriter_autotest",
         "Func":    default_autotest,
diff --git a/test/test/meson.build b/test/test/meson.build
index e403bd5..c0b1928 100644
--- a/test/test/meson.build
+++ b/test/test/meson.build
@@ -47,7 +47,6 @@ test_sources = files('commands.c',
 	'test_hash_readwrite.c',
 	'test_hash_perf.c',
 	'test_hash_readwrite_lf.c',
-	'test_hash_scaling.c',
 	'test_interrupts.c',
 	'test_kni.c',
 	'test_kvargs.c',
@@ -168,7 +167,6 @@ test_names = [
 	'eventdev_sw_autotest',
 	'func_reentrancy_autotest',
 	'flow_classify_autotest',
-	'hash_scaling_autotest',
 	'hash_autotest',
 	'hash_functions_autotest',
 	'hash_multiwriter_autotest',
diff --git a/test/test/test_hash_scaling.c b/test/test/test_hash_scaling.c
deleted file mode 100644
index 07765a7..0000000
--- a/test/test/test_hash_scaling.c
+++ /dev/null
@@ -1,191 +0,0 @@
-/* SPDX-License-Identifier: BSD-3-Clause
- * Copyright(c) 2015 Intel Corporation
- */
-
-#include <stdio.h>
-
-#include <rte_cycles.h>
-#include <rte_hash.h>
-#include <rte_hash_crc.h>
-#include <rte_spinlock.h>
-#include <rte_launch.h>
-
-#include "test.h"
-
-/*
- * Check condition and return an error if true. Assumes that "handle" is the
- * name of the hash structure pointer to be freed.
- */
-#define RETURN_IF_ERROR(cond, str, ...) do {				\
-	if (cond) {							\
-		printf("ERROR line %d: " str "\n", __LINE__,		\
-							##__VA_ARGS__);	\
-		if (handle)						\
-			rte_hash_free(handle);				\
-		return -1;						\
-	}								\
-} while (0)
-
-enum locking_mode_t {
-	NORMAL_LOCK,
-	LOCK_ELISION,
-	NULL_LOCK
-};
-
-struct {
-	uint32_t num_iterations;
-	struct rte_hash *h;
-	rte_spinlock_t *lock;
-	int locking_mode;
-} tbl_scaling_test_params;
-
-static rte_atomic64_t gcycles;
-
-static int test_hash_scaling_worker(__attribute__((unused)) void *arg)
-{
-	uint64_t i, key;
-	uint32_t thr_id = rte_sys_gettid();
-	uint64_t begin, cycles = 0;
-
-	switch (tbl_scaling_test_params.locking_mode) {
-
-	case NORMAL_LOCK:
-
-		for (i = 0; i < tbl_scaling_test_params.num_iterations; i++) {
-			/*	different threads get different keys because
-				we use the thread-id in the key computation
-			 */
-			key = rte_hash_crc(&i, sizeof(i), thr_id);
-			begin = rte_rdtsc_precise();
-			rte_spinlock_lock(tbl_scaling_test_params.lock);
-			rte_hash_add_key(tbl_scaling_test_params.h, &key);
-			rte_spinlock_unlock(tbl_scaling_test_params.lock);
-			cycles += rte_rdtsc_precise() - begin;
-		}
-		break;
-
-	case LOCK_ELISION:
-
-		for (i = 0; i < tbl_scaling_test_params.num_iterations; i++) {
-			key = rte_hash_crc(&i, sizeof(i), thr_id);
-			begin = rte_rdtsc_precise();
-			rte_spinlock_lock_tm(tbl_scaling_test_params.lock);
-			rte_hash_add_key(tbl_scaling_test_params.h, &key);
-			rte_spinlock_unlock_tm(tbl_scaling_test_params.lock);
-			cycles += rte_rdtsc_precise() - begin;
-		}
-		break;
-
-	default:
-
-		for (i = 0; i < tbl_scaling_test_params.num_iterations; i++) {
-			key = rte_hash_crc(&i, sizeof(i), thr_id);
-			begin = rte_rdtsc_precise();
-			rte_hash_add_key(tbl_scaling_test_params.h, &key);
-			cycles += rte_rdtsc_precise() - begin;
-		}
-	}
-
-	rte_atomic64_add(&gcycles, cycles);
-
-	return 0;
-}
-
-/*
- * Do scalability perf tests.
- */
-static int
-test_hash_scaling(int locking_mode)
-{
-	static unsigned calledCount =    1;
-	uint32_t num_iterations = 1024*1024;
-	uint64_t i, key;
-	struct rte_hash_parameters hash_params = {
-		.entries = num_iterations*2,
-		.key_len = sizeof(key),
-		.hash_func = rte_hash_crc,
-		.hash_func_init_val = 0,
-		.socket_id = rte_socket_id(),
-		.extra_flag = RTE_HASH_EXTRA_FLAGS_TRANS_MEM_SUPPORT
-	};
-	struct rte_hash *handle;
-	char name[RTE_HASH_NAMESIZE];
-	rte_spinlock_t lock;
-
-	rte_spinlock_init(&lock);
-
-	snprintf(name, 32, "test%u", calledCount++);
-	hash_params.name = name;
-
-	handle = rte_hash_create(&hash_params);
-	RETURN_IF_ERROR(handle == NULL, "hash creation failed");
-
-	tbl_scaling_test_params.num_iterations =
-		num_iterations/rte_lcore_count();
-	tbl_scaling_test_params.h = handle;
-	tbl_scaling_test_params.lock = &lock;
-	tbl_scaling_test_params.locking_mode = locking_mode;
-
-	rte_atomic64_init(&gcycles);
-	rte_atomic64_clear(&gcycles);
-
-	/* fill up to initial size */
-	for (i = 0; i < num_iterations; i++) {
-		key = rte_hash_crc(&i, sizeof(i), 0xabcdabcd);
-		rte_hash_add_key(tbl_scaling_test_params.h, &key);
-	}
-
-	rte_eal_mp_remote_launch(test_hash_scaling_worker, NULL, CALL_MASTER);
-	rte_eal_mp_wait_lcore();
-
-	unsigned long long int cycles_per_operation =
-		rte_atomic64_read(&gcycles)/
-		(tbl_scaling_test_params.num_iterations*rte_lcore_count());
-	const char *lock_name;
-
-	switch (locking_mode) {
-	case NORMAL_LOCK:
-		lock_name = "normal spinlock";
-		break;
-	case LOCK_ELISION:
-		lock_name = "lock elision";
-		break;
-	default:
-		lock_name = "null lock";
-	}
-	printf("--------------------------------------------------------\n");
-	printf("Cores: %d; %s mode ->  cycles per operation: %llu\n",
-		rte_lcore_count(), lock_name, cycles_per_operation);
-	printf("--------------------------------------------------------\n");
-	/* CSV output */
-	printf(">>>%d,%s,%llu\n", rte_lcore_count(), lock_name,
-		cycles_per_operation);
-
-	rte_hash_free(handle);
-	return 0;
-}
-
-static int
-test_hash_scaling_main(void)
-{
-	int r = 0;
-
-	if (rte_lcore_count() == 1)
-		r = test_hash_scaling(NULL_LOCK);
-
-	if (r == 0)
-		r = test_hash_scaling(NORMAL_LOCK);
-
-	if (!rte_tm_supported()) {
-		printf("Hardware transactional memory (lock elision) is NOT supported\n");
-		return r;
-	}
-	printf("Hardware transactional memory (lock elision) is supported\n");
-
-	if (r == 0)
-		r = test_hash_scaling(LOCK_ELISION);
-
-	return r;
-}
-
-REGISTER_TEST_COMMAND(hash_scaling_autotest, test_hash_scaling_main);
-- 
2.7.4

Re: [dpdk-dev] [PATCH v2 1/4] hash: fix unnecessary pause

[Mattias Rönnblom]

On 2018-10-24 20:09, Yipeng Wang wrote:
> There is a rte_pause in hash table reset function.
> Since the loop is not a polling loop on shared
> data structure, the rte_pause is not needed.
> 

I'm guessing the <rte_pause.h> include is longer needed.

Re: [dpdk-dev] [PATCH v2 1/4] hash: fix unnecessary pause

[Bruce Richardson]

On Thu, Oct 25, 2018 at 08:31:16AM +0200, Mattias Rönnblom wrote:
> On 2018-10-24 20:09, Yipeng Wang wrote:
> > There is a rte_pause in hash table reset function.
> > Since the loop is not a polling loop on shared
> > data structure, the rte_pause is not needed.
> > 
> 
> I'm guessing the <rte_pause.h> include is longer needed.

Good point, otherwise:

Acked-by: Bruce Richardson <bruce.richardson@intel.com>

Re: [dpdk-dev] [PATCH v2 2/4] test/hash: change multiwriter test to use jhash

[Bruce Richardson]

On Wed, Oct 24, 2018 at 11:09:28AM -0700, Yipeng Wang wrote:
> With sequential key, the test will cover more corner
> cases with jhash instead of crc hash, since jhash
> generates more random hash pattern on sequential key.
> It is useful for functional verification.
> 
> Signed-off-by: Yipeng Wang <yipeng1.wang@intel.com>
> ---
>  test/test/test_hash_multiwriter.c | 10 +++++++++-
>  1 file changed, 9 insertions(+), 1 deletion(-)
> 
> diff --git a/test/test/test_hash_multiwriter.c b/test/test/test_hash_multiwriter.c
> index 6a3eb10..456bc5f 100644
> --- a/test/test/test_hash_multiwriter.c
> +++ b/test/test/test_hash_multiwriter.c
> @@ -12,6 +12,7 @@
>  #include <rte_malloc.h>
>  #include <rte_random.h>
>  #include <rte_spinlock.h>
> +#include <rte_jhash.h>
>  
>  #include "test.h"
>  
> @@ -31,6 +32,9 @@
>  
>  #define RTE_APP_TEST_HASH_MULTIWRITER_FAILED 0
>  
> +/* Use jhash or crc hash */
> +#define USE_JHASH 1
> +
>  struct {
>  	uint32_t *keys;
>  	uint32_t *found;
> @@ -108,10 +112,14 @@ test_hash_multiwriter(void)
>  	struct rte_hash_parameters hash_params = {
>  		.entries = nb_entries,
>  		.key_len = sizeof(uint32_t),
> -		.hash_func = rte_hash_crc,
>  		.hash_func_init_val = 0,
>  		.socket_id = rte_socket_id(),
>  	};
> +	if (USE_JHASH)
> +		hash_params.hash_func = rte_jhash;
> +	else
> +		hash_params.hash_func = rte_hash_crc;
> +
>  	if (use_htm)
>  		hash_params.extra_flag =
>  			RTE_HASH_EXTRA_FLAGS_TRANS_MEM_SUPPORT
> -- 
As I commented on v1, rather than having a macro at the top hard-coded to
jhash, why not just do a straight replacement of crc to jhash in the
structure definition. Since changing the hash function will involve editing
the source code anyway, I see little point in using the macro at the top -
especially since there is no indication to the user what effect removing it
or changing it to zero has.

/Bruce

Re: [dpdk-dev] [PATCH v2 3/4] test/hash: add readwrite test for ext table

[Bruce Richardson]

On Wed, Oct 24, 2018 at 11:09:29AM -0700, Yipeng Wang wrote:
> This commit improves the readwrite test to consider
> extendable table feature and add more functional tests
> to cover more corner cases.
> 
> Signed-off-by: Yipeng Wang <yipeng1.wang@intel.com>
> ---

Acked-by: Bruce Richardson <bruce.richardson@intel.com>

Re: [dpdk-dev] [PATCH v2 4/4] test/hash: remove hash scaling unit test

[Bruce Richardson]

On Wed, Oct 24, 2018 at 11:09:30AM -0700, Yipeng Wang wrote:
> The hash scaling unit test is not really needed
> any more since the multi-writer is supported now
> inside the library and it is tested by multi-writer
> unit test.
> 
> Signed-off-by: Yipeng Wang <yipeng1.wang@intel.com>
> ---
>  test/test/Makefile            |   1 -
>  test/test/autotest_data.py    |   6 --
>  test/test/meson.build         |   2 -
>  test/test/test_hash_scaling.c | 191 ------------------------------------------
>  4 files changed, 200 deletions(-)
>  delete mode 100644 test/test/test_hash_scaling.c
> 
I always like having less code that needs maintenance! :-)

Acked-by: Bruce Richardson <bruce.richardson@intel.com>

[dpdk-dev] [PATCH v3 0/6] hash: improve multiple places

[Yipeng Wang]

This patch set depends on Honnappa's patch set:
http://patchwork.dpdk.org/cover/47268/

This patch set fixes/improves a couple of places mostly
on unit tests:

commit 1: remove unnecessary code in hash library.
commit 2: use jhash in multiwriter unit test.
commit 3: cover more test case in unit test.
commit 4: improve readwrite test to consider the extendable table.
commit 5: remove scaling unit test.
commit 6: add readwrite test to meson and autotest file.

v2->v3:
* Split commit 4 to commit 3 and 4 (Honnappa)
* Remove hard coded macro (Bruce)
* Add commit 6 fix to add readwrite test to autotest files.
* Remove unnecessary header in commit 1 (Mattias)

V1->V2:
* In commit 2 change use_jhash to a macro instead of a hard coded
local variable (Bruce).
* Add commit 4 to remove scaling unit test (Bruce).

Signed-off-by: Yipeng Wang <yipeng1.wang@intel.com>

Yipeng Wang (6):
  hash: fix unnecessary pause
  test/hash: change multiwriter test to use jhash
  test/hash: test more corner cases in unit test
  test/hash: add readwrite test for ext table
  test/hash: remove hash scaling unit test
  test/hash: fix to add read-write test to autotest

 lib/librte_hash/rte_cuckoo_hash.c |   5 +-
 test/test/Makefile                |   1 -
 test/test/autotest_data.py        |  12 +--
 test/test/meson.build             |   3 +-
 test/test/test_hash_multiwriter.c |   3 +-
 test/test/test_hash_readwrite.c   |  73 ++++++++++++---
 test/test/test_hash_scaling.c     | 191 --------------------------------------
 7 files changed, 70 insertions(+), 218 deletions(-)
 delete mode 100644 test/test/test_hash_scaling.c

-- 
2.7.4

[dpdk-dev] [PATCH v3 1/6] hash: fix unnecessary pause

[Yipeng Wang]

There is a rte_pause in hash table reset function.
Since the loop is not a polling loop on shared
data structure, the rte_pause is not needed.

Fixes: b26473ff8f4a ("hash: add reset function")
Cc: stable@dpdk.org

Signed-off-by: Yipeng Wang <yipeng1.wang@intel.com>
Acked-by: Bruce Richardson <bruce.richardson@intel.com>
---
 lib/librte_hash/rte_cuckoo_hash.c | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/lib/librte_hash/rte_cuckoo_hash.c b/lib/librte_hash/rte_cuckoo_hash.c
index 0648a22..5ddcccd 100644
--- a/lib/librte_hash/rte_cuckoo_hash.c
+++ b/lib/librte_hash/rte_cuckoo_hash.c
@@ -27,7 +27,6 @@
 #include <rte_spinlock.h>
 #include <rte_ring.h>
 #include <rte_compat.h>
-#include <rte_pause.h>
 
 #include "rte_hash.h"
 #include "rte_cuckoo_hash.h"
@@ -574,14 +573,14 @@ rte_hash_reset(struct rte_hash *h)
 
 	/* clear the free ring */
 	while (rte_ring_dequeue(h->free_slots, &ptr) == 0)
-		rte_pause();
+		continue;
 
 	/* clear free extendable bucket ring and memory */
 	if (h->ext_table_support) {
 		memset(h->buckets_ext, 0, h->num_buckets *
 						sizeof(struct rte_hash_bucket));
 		while (rte_ring_dequeue(h->free_ext_bkts, &ptr) == 0)
-			rte_pause();
+			continue;
 	}
 
 	/* Repopulate the free slots ring. Entry zero is reserved for key misses */
-- 
2.7.4

[dpdk-dev] [PATCH v3 2/6] test/hash: change multiwriter test to use jhash

[Yipeng Wang]

With sequential key, the test will cover more corner
cases with jhash instead of crc hash, since jhash
generates more random hash pattern on sequential key.
It is useful for functional verification.

Signed-off-by: Yipeng Wang <yipeng1.wang@intel.com>
---
 test/test/test_hash_multiwriter.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/test/test/test_hash_multiwriter.c b/test/test/test_hash_multiwriter.c
index 6a3eb10..d447f6d 100644
--- a/test/test/test_hash_multiwriter.c
+++ b/test/test/test_hash_multiwriter.c
@@ -12,6 +12,7 @@
 #include <rte_malloc.h>
 #include <rte_random.h>
 #include <rte_spinlock.h>
+#include <rte_jhash.h>
 
 #include "test.h"
 
@@ -108,7 +109,7 @@ test_hash_multiwriter(void)
 	struct rte_hash_parameters hash_params = {
 		.entries = nb_entries,
 		.key_len = sizeof(uint32_t),
-		.hash_func = rte_hash_crc,
+		.hash_func = rte_jhash,
 		.hash_func_init_val = 0,
 		.socket_id = rte_socket_id(),
 	};
-- 
2.7.4

[dpdk-dev] [PATCH v3 3/6] test/hash: test more corner cases in unit test

[Yipeng Wang]

This commit improves the readwrite unit test to cover
more corner cases and reduces the testing time by
reducing the total key count.

Signed-off-by: Yipeng Wang <yipeng1.wang@intel.com>
Acked-by: Bruce Richardson <bruce.richardson@intel.com>
---
 test/test/test_hash_readwrite.c | 31 +++++++++++++++++++++++++------
 1 file changed, 25 insertions(+), 6 deletions(-)

diff --git a/test/test/test_hash_readwrite.c b/test/test/test_hash_readwrite.c
index a8fadd0..a45c669 100644
--- a/test/test/test_hash_readwrite.c
+++ b/test/test/test_hash_readwrite.c
@@ -18,8 +18,8 @@
 
 #define RTE_RWTEST_FAIL 0
 
-#define TOTAL_ENTRY (16*1024*1024)
-#define TOTAL_INSERT (15*1024*1024)
+#define TOTAL_ENTRY (5*1024*1024)
+#define TOTAL_INSERT (4.5*1024*1024)
 
 #define NUM_TEST 3
 unsigned int core_cnt[NUM_TEST] = {2, 4, 8};
@@ -59,8 +59,10 @@ test_hash_readwrite_worker(__attribute__((unused)) void *arg)
 	uint64_t i, offset;
 	uint32_t lcore_id = rte_lcore_id();
 	uint64_t begin, cycles;
-	int ret;
+	int *ret;
 
+	ret = rte_malloc(NULL, sizeof(int) *
+				tbl_rw_test_param.num_insert, 0);
 	for (i = 0; i < rte_lcore_count(); i++) {
 		if (slave_core_ids[i] == lcore_id)
 			break;
@@ -79,13 +81,30 @@ test_hash_readwrite_worker(__attribute__((unused)) void *arg)
 				tbl_rw_test_param.keys + i) > 0)
 			break;
 
-		ret = rte_hash_add_key(tbl_rw_test_param.h,
+		ret[i - offset] = rte_hash_add_key(tbl_rw_test_param.h,
 				     tbl_rw_test_param.keys + i);
-		if (ret < 0)
+		if (ret[i - offset] < 0)
+			break;
+
+		/* lookup a random key */
+		uint32_t rand = rte_rand() % (i + 1 - offset);
+
+		if (rte_hash_lookup(tbl_rw_test_param.h,
+				tbl_rw_test_param.keys + rand) != ret[rand])
+			break;
+
+
+		if (rte_hash_del_key(tbl_rw_test_param.h,
+				tbl_rw_test_param.keys + rand) != ret[rand])
+			break;
+
+		ret[rand] = rte_hash_add_key(tbl_rw_test_param.h,
+					tbl_rw_test_param.keys + rand);
+		if (ret[rand] < 0)
 			break;
 
 		if (rte_hash_lookup(tbl_rw_test_param.h,
-				tbl_rw_test_param.keys + i) != ret)
+			tbl_rw_test_param.keys + rand) != ret[rand])
 			break;
 	}
 
-- 
2.7.4

[dpdk-dev] [PATCH v3 4/6] test/hash: add readwrite test for ext table

[Yipeng Wang]

This commit improves the readwrite test to consider
extendable table feature.

Signed-off-by: Yipeng Wang <yipeng1.wang@intel.com>
Acked-by: Bruce Richardson <bruce.richardson@intel.com>
---
 test/test/test_hash_readwrite.c | 42 +++++++++++++++++++++++++++++++++--------
 1 file changed, 34 insertions(+), 8 deletions(-)

diff --git a/test/test/test_hash_readwrite.c b/test/test/test_hash_readwrite.c
index a45c669..8cbe85bd 100644
--- a/test/test/test_hash_readwrite.c
+++ b/test/test/test_hash_readwrite.c
@@ -20,6 +20,7 @@
 
 #define TOTAL_ENTRY (5*1024*1024)
 #define TOTAL_INSERT (4.5*1024*1024)
+#define TOTAL_INSERT_EXT (5*1024*1024)
 
 #define NUM_TEST 3
 unsigned int core_cnt[NUM_TEST] = {2, 4, 8};
@@ -119,7 +120,7 @@ test_hash_readwrite_worker(__attribute__((unused)) void *arg)
 }
 
 static int
-init_params(int use_htm, int use_jhash)
+init_params(int use_ext, int use_htm, int use_jhash)
 {
 	unsigned int i;
 
@@ -148,6 +149,13 @@ init_params(int use_htm, int use_jhash)
 			RTE_HASH_EXTRA_FLAGS_RW_CONCURRENCY |
 			RTE_HASH_EXTRA_FLAGS_MULTI_WRITER_ADD;
 
+	if (use_ext)
+		hash_params.extra_flag |=
+			RTE_HASH_EXTRA_FLAGS_EXT_TABLE;
+	else
+		hash_params.extra_flag &=
+		       ~RTE_HASH_EXTRA_FLAGS_EXT_TABLE;
+
 	hash_params.name = "tests";
 
 	handle = rte_hash_create(&hash_params);
@@ -186,7 +194,7 @@ init_params(int use_htm, int use_jhash)
 }
 
 static int
-test_hash_readwrite_functional(int use_htm)
+test_hash_readwrite_functional(int use_ext, int use_htm)
 {
 	unsigned int i;
 	const void *next_key;
@@ -197,6 +205,7 @@ test_hash_readwrite_functional(int use_htm)
 	uint32_t lost_keys = 0;
 	int use_jhash = 1;
 	int slave_cnt = rte_lcore_count() - 1;
+	uint32_t tot_insert = 0;
 
 	rte_atomic64_init(&gcycles);
 	rte_atomic64_clear(&gcycles);
@@ -204,11 +213,16 @@ test_hash_readwrite_functional(int use_htm)
 	rte_atomic64_init(&ginsertions);
 	rte_atomic64_clear(&ginsertions);
 
-	if (init_params(use_htm, use_jhash) != 0)
+	if (init_params(use_ext, use_htm, use_jhash) != 0)
 		goto err;
 
+	if (use_ext)
+		tot_insert = TOTAL_INSERT_EXT;
+	else
+		tot_insert = TOTAL_INSERT;
+
 	tbl_rw_test_param.num_insert =
-		TOTAL_INSERT / slave_cnt;
+		tot_insert / slave_cnt;
 
 	tbl_rw_test_param.rounded_tot_insert =
 		tbl_rw_test_param.num_insert
@@ -364,7 +378,7 @@ test_hash_readwrite_perf(struct perf *perf_results, int use_htm,
 	rte_atomic64_init(&gwrite_cycles);
 	rte_atomic64_clear(&gwrite_cycles);
 
-	if (init_params(use_htm, use_jhash) != 0)
+	if (init_params(0, use_htm, use_jhash) != 0)
 		goto err;
 
 	/*
@@ -598,7 +612,7 @@ test_hash_readwrite_main(void)
 	 * than writer threads. This is to timing either reader threads or
 	 * writer threads for performance numbers.
 	 */
-	int use_htm, reader_faster;
+	int use_htm, use_ext,  reader_faster;
 	unsigned int i = 0, core_id = 0;
 
 	if (rte_lcore_count() <= 2) {
@@ -621,7 +635,13 @@ test_hash_readwrite_main(void)
 		printf("Test read-write with Hardware transactional memory\n");
 
 		use_htm = 1;
-		if (test_hash_readwrite_functional(use_htm) < 0)
+		use_ext = 0;
+
+		if (test_hash_readwrite_functional(use_ext, use_htm) < 0)
+			return -1;
+
+		use_ext = 1;
+		if (test_hash_readwrite_functional(use_ext, use_htm) < 0)
 			return -1;
 
 		reader_faster = 1;
@@ -640,8 +660,14 @@ test_hash_readwrite_main(void)
 
 	printf("Test read-write without Hardware transactional memory\n");
 	use_htm = 0;
-	if (test_hash_readwrite_functional(use_htm) < 0)
+	use_ext = 0;
+	if (test_hash_readwrite_functional(use_ext, use_htm) < 0)
 		return -1;
+
+	use_ext = 1;
+	if (test_hash_readwrite_functional(use_ext, use_htm) < 0)
+		return -1;
+
 	reader_faster = 1;
 	if (test_hash_readwrite_perf(&non_htm_results, use_htm,
 							reader_faster) < 0)
-- 
2.7.4

[dpdk-dev] [PATCH v3 5/6] test/hash: remove hash scaling unit test

[Yipeng Wang]

The hash scaling unit test is not really needed
any more since the multi-writer is supported now
inside the library and it is tested by multi-writer
unit test.

Signed-off-by: Yipeng Wang <yipeng1.wang@intel.com>
Acked-by: Bruce Richardson <bruce.richardson@intel.com>
---
 test/test/Makefile            |   1 -
 test/test/autotest_data.py    |   6 --
 test/test/meson.build         |   2 -
 test/test/test_hash_scaling.c | 191 ------------------------------------------
 4 files changed, 200 deletions(-)
 delete mode 100644 test/test/test_hash_scaling.c

diff --git a/test/test/Makefile b/test/test/Makefile
index 1b5c070..6b77e15 100644
--- a/test/test/Makefile
+++ b/test/test/Makefile
@@ -113,7 +113,6 @@ SRCS-$(CONFIG_RTE_LIBRTE_HASH) += test_hash.c
 SRCS-$(CONFIG_RTE_LIBRTE_HASH) += test_thash.c
 SRCS-$(CONFIG_RTE_LIBRTE_HASH) += test_hash_perf.c
 SRCS-$(CONFIG_RTE_LIBRTE_HASH) += test_hash_functions.c
-SRCS-$(CONFIG_RTE_LIBRTE_HASH) += test_hash_scaling.c
 SRCS-$(CONFIG_RTE_LIBRTE_HASH) += test_hash_multiwriter.c
 SRCS-$(CONFIG_RTE_LIBRTE_HASH) += test_hash_readwrite.c
 SRCS-$(CONFIG_RTE_LIBRTE_HASH) += test_hash_readwrite_lf.c
diff --git a/test/test/autotest_data.py b/test/test/autotest_data.py
index 4eae588..9265617 100644
--- a/test/test/autotest_data.py
+++ b/test/test/autotest_data.py
@@ -345,12 +345,6 @@
         "Report":  None,
     },
     {
-        "Name":    "Hash scaling autotest",
-        "Command": "hash_scaling_autotest",
-        "Func":    default_autotest,
-        "Report":  None,
-    },
-    {
         "Name":    "Hash multiwriter autotest",
         "Command": "hash_multiwriter_autotest",
         "Func":    default_autotest,
diff --git a/test/test/meson.build b/test/test/meson.build
index faef5a4..7ebe9ab 100644
--- a/test/test/meson.build
+++ b/test/test/meson.build
@@ -47,7 +47,6 @@ test_sources = files('commands.c',
 	'test_hash_readwrite.c',
 	'test_hash_perf.c',
 	'test_hash_readwrite_lf.c',
-	'test_hash_scaling.c',
 	'test_interrupts.c',
 	'test_kni.c',
 	'test_kvargs.c',
@@ -170,7 +169,6 @@ test_names = [
 	'external_mem_autotest',
 	'func_reentrancy_autotest',
 	'flow_classify_autotest',
-	'hash_scaling_autotest',
 	'hash_autotest',
 	'hash_functions_autotest',
 	'hash_multiwriter_autotest',
diff --git a/test/test/test_hash_scaling.c b/test/test/test_hash_scaling.c
deleted file mode 100644
index 07765a7..0000000
--- a/test/test/test_hash_scaling.c
+++ /dev/null
@@ -1,191 +0,0 @@
-/* SPDX-License-Identifier: BSD-3-Clause
- * Copyright(c) 2015 Intel Corporation
- */
-
-#include <stdio.h>
-
-#include <rte_cycles.h>
-#include <rte_hash.h>
-#include <rte_hash_crc.h>
-#include <rte_spinlock.h>
-#include <rte_launch.h>
-
-#include "test.h"
-
-/*
- * Check condition and return an error if true. Assumes that "handle" is the
- * name of the hash structure pointer to be freed.
- */
-#define RETURN_IF_ERROR(cond, str, ...) do {				\
-	if (cond) {							\
-		printf("ERROR line %d: " str "\n", __LINE__,		\
-							##__VA_ARGS__);	\
-		if (handle)						\
-			rte_hash_free(handle);				\
-		return -1;						\
-	}								\
-} while (0)
-
-enum locking_mode_t {
-	NORMAL_LOCK,
-	LOCK_ELISION,
-	NULL_LOCK
-};
-
-struct {
-	uint32_t num_iterations;
-	struct rte_hash *h;
-	rte_spinlock_t *lock;
-	int locking_mode;
-} tbl_scaling_test_params;
-
-static rte_atomic64_t gcycles;
-
-static int test_hash_scaling_worker(__attribute__((unused)) void *arg)
-{
-	uint64_t i, key;
-	uint32_t thr_id = rte_sys_gettid();
-	uint64_t begin, cycles = 0;
-
-	switch (tbl_scaling_test_params.locking_mode) {
-
-	case NORMAL_LOCK:
-
-		for (i = 0; i < tbl_scaling_test_params.num_iterations; i++) {
-			/*	different threads get different keys because
-				we use the thread-id in the key computation
-			 */
-			key = rte_hash_crc(&i, sizeof(i), thr_id);
-			begin = rte_rdtsc_precise();
-			rte_spinlock_lock(tbl_scaling_test_params.lock);
-			rte_hash_add_key(tbl_scaling_test_params.h, &key);
-			rte_spinlock_unlock(tbl_scaling_test_params.lock);
-			cycles += rte_rdtsc_precise() - begin;
-		}
-		break;
-
-	case LOCK_ELISION:
-
-		for (i = 0; i < tbl_scaling_test_params.num_iterations; i++) {
-			key = rte_hash_crc(&i, sizeof(i), thr_id);
-			begin = rte_rdtsc_precise();
-			rte_spinlock_lock_tm(tbl_scaling_test_params.lock);
-			rte_hash_add_key(tbl_scaling_test_params.h, &key);
-			rte_spinlock_unlock_tm(tbl_scaling_test_params.lock);
-			cycles += rte_rdtsc_precise() - begin;
-		}
-		break;
-
-	default:
-
-		for (i = 0; i < tbl_scaling_test_params.num_iterations; i++) {
-			key = rte_hash_crc(&i, sizeof(i), thr_id);
-			begin = rte_rdtsc_precise();
-			rte_hash_add_key(tbl_scaling_test_params.h, &key);
-			cycles += rte_rdtsc_precise() - begin;
-		}
-	}
-
-	rte_atomic64_add(&gcycles, cycles);
-
-	return 0;
-}
-
-/*
- * Do scalability perf tests.
- */
-static int
-test_hash_scaling(int locking_mode)
-{
-	static unsigned calledCount =    1;
-	uint32_t num_iterations = 1024*1024;
-	uint64_t i, key;
-	struct rte_hash_parameters hash_params = {
-		.entries = num_iterations*2,
-		.key_len = sizeof(key),
-		.hash_func = rte_hash_crc,
-		.hash_func_init_val = 0,
-		.socket_id = rte_socket_id(),
-		.extra_flag = RTE_HASH_EXTRA_FLAGS_TRANS_MEM_SUPPORT
-	};
-	struct rte_hash *handle;
-	char name[RTE_HASH_NAMESIZE];
-	rte_spinlock_t lock;
-
-	rte_spinlock_init(&lock);
-
-	snprintf(name, 32, "test%u", calledCount++);
-	hash_params.name = name;
-
-	handle = rte_hash_create(&hash_params);
-	RETURN_IF_ERROR(handle == NULL, "hash creation failed");
-
-	tbl_scaling_test_params.num_iterations =
-		num_iterations/rte_lcore_count();
-	tbl_scaling_test_params.h = handle;
-	tbl_scaling_test_params.lock = &lock;
-	tbl_scaling_test_params.locking_mode = locking_mode;
-
-	rte_atomic64_init(&gcycles);
-	rte_atomic64_clear(&gcycles);
-
-	/* fill up to initial size */
-	for (i = 0; i < num_iterations; i++) {
-		key = rte_hash_crc(&i, sizeof(i), 0xabcdabcd);
-		rte_hash_add_key(tbl_scaling_test_params.h, &key);
-	}
-
-	rte_eal_mp_remote_launch(test_hash_scaling_worker, NULL, CALL_MASTER);
-	rte_eal_mp_wait_lcore();
-
-	unsigned long long int cycles_per_operation =
-		rte_atomic64_read(&gcycles)/
-		(tbl_scaling_test_params.num_iterations*rte_lcore_count());
-	const char *lock_name;
-
-	switch (locking_mode) {
-	case NORMAL_LOCK:
-		lock_name = "normal spinlock";
-		break;
-	case LOCK_ELISION:
-		lock_name = "lock elision";
-		break;
-	default:
-		lock_name = "null lock";
-	}
-	printf("--------------------------------------------------------\n");
-	printf("Cores: %d; %s mode ->  cycles per operation: %llu\n",
-		rte_lcore_count(), lock_name, cycles_per_operation);
-	printf("--------------------------------------------------------\n");
-	/* CSV output */
-	printf(">>>%d,%s,%llu\n", rte_lcore_count(), lock_name,
-		cycles_per_operation);
-
-	rte_hash_free(handle);
-	return 0;
-}
-
-static int
-test_hash_scaling_main(void)
-{
-	int r = 0;
-
-	if (rte_lcore_count() == 1)
-		r = test_hash_scaling(NULL_LOCK);
-
-	if (r == 0)
-		r = test_hash_scaling(NORMAL_LOCK);
-
-	if (!rte_tm_supported()) {
-		printf("Hardware transactional memory (lock elision) is NOT supported\n");
-		return r;
-	}
-	printf("Hardware transactional memory (lock elision) is supported\n");
-
-	if (r == 0)
-		r = test_hash_scaling(LOCK_ELISION);
-
-	return r;
-}
-
-REGISTER_TEST_COMMAND(hash_scaling_autotest, test_hash_scaling_main);
-- 
2.7.4

[dpdk-dev] [PATCH v3 6/6] test/hash: fix to add read-write test to autotest

[Yipeng Wang]

Add read-write concurrency test to meson and autotest
file.

Fixes: 0eb3726ebcf1 ("test/hash: add test for read/write concurrency")
Cc: stable@dpdk.org

Signed-off-by: Yipeng Wang <yipeng1.wang@intel.com>
---
 test/test/autotest_data.py | 6 ++++++
 test/test/meson.build      | 1 +
 2 files changed, 7 insertions(+)

diff --git a/test/test/autotest_data.py b/test/test/autotest_data.py
index 9265617..0df686a 100644
--- a/test/test/autotest_data.py
+++ b/test/test/autotest_data.py
@@ -574,6 +574,12 @@
         "Report":  None,
     },
     {
+        "Name":    "Hash read-write concurrency autotest",
+        "Command": "hash_readwrite_autotest",
+        "Func":    default_autotest,
+        "Report":  None,
+    },
+    {
         "Name":    "Hash read-write lock-free concurrency autotest",
         "Command": "hash_readwrite_lf_autotest",
         "Func":    default_autotest,
diff --git a/test/test/meson.build b/test/test/meson.build
index 7ebe9ab..1c1fcd7 100644
--- a/test/test/meson.build
+++ b/test/test/meson.build
@@ -173,6 +173,7 @@ test_names = [
 	'hash_functions_autotest',
 	'hash_multiwriter_autotest',
 	'hash_perf_autotest',
+	'hash_readwrite_autotest',
 	'hash_readwrite_lf_autotest',
 	'interrupt_autotest',
 	'kni_autotest',
-- 
2.7.4

Re: [dpdk-dev] [PATCH v2 1/4] hash: fix unnecessary pause

[Honnappa Nagarahalli]

> 
> There is a rte_pause in hash table reset function.
> Since the loop is not a polling loop on shared data structure, the rte_pause
> is not needed.
> 
> Fixes: b26473ff8f4a ("hash: add reset function")
> Cc: stable@dpdk.org
> 
> Signed-off-by: Yipeng Wang <yipeng1.wang@intel.com>
> ---
>  lib/librte_hash/rte_cuckoo_hash.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/lib/librte_hash/rte_cuckoo_hash.c
> b/lib/librte_hash/rte_cuckoo_hash.c
> index 0648a22..4a2647e 100644
> --- a/lib/librte_hash/rte_cuckoo_hash.c
> +++ b/lib/librte_hash/rte_cuckoo_hash.c
> @@ -574,14 +574,14 @@ rte_hash_reset(struct rte_hash *h)
> 
>  	/* clear the free ring */
>  	while (rte_ring_dequeue(h->free_slots, &ptr) == 0)
> -		rte_pause();
> +		continue;
Minor comment: 'continue' can be removed.

> 
>  	/* clear free extendable bucket ring and memory */
>  	if (h->ext_table_support) {
>  		memset(h->buckets_ext, 0, h->num_buckets *
>  						sizeof(struct
> rte_hash_bucket));
>  		while (rte_ring_dequeue(h->free_ext_bkts, &ptr) == 0)
> -			rte_pause();
> +			continue;
>  	}
> 
>  	/* Repopulate the free slots ring. Entry zero is reserved for key
> misses */
> --
> 2.7.4

Re: [dpdk-dev] [PATCH v2 3/4] test/hash: add readwrite test for ext table

[Honnappa Nagarahalli]

> Subject: [PATCH v2 3/4] test/hash: add readwrite test for ext table
> 
> This commit improves the readwrite test to consider extendable table
> feature and add more functional tests to cover more corner cases.
> 
I think the part covering the more corner cases should go into a separate commit.

> Signed-off-by: Yipeng Wang <yipeng1.wang@intel.com>
> ---
>  test/test/test_hash_readwrite.c | 70
> ++++++++++++++++++++++++++++++++++-------
>  1 file changed, 58 insertions(+), 12 deletions(-)
> 
> diff --git a/test/test/test_hash_readwrite.c
> b/test/test/test_hash_readwrite.c index a8fadd0..13c28e0 100644
> --- a/test/test/test_hash_readwrite.c
> +++ b/test/test/test_hash_readwrite.c
> @@ -21,6 +21,8 @@
>  #define TOTAL_ENTRY (16*1024*1024)
>  #define TOTAL_INSERT (15*1024*1024)
> 
> +#define TOTAL_INSERT_EXT (16*1024*1024)
> +
>  #define NUM_TEST 3
>  unsigned int core_cnt[NUM_TEST] = {2, 4, 8};
> 
> @@ -59,8 +61,10 @@ test_hash_readwrite_worker(__attribute__((unused))
> void *arg)
>  	uint64_t i, offset;
>  	uint32_t lcore_id = rte_lcore_id();
>  	uint64_t begin, cycles;
> -	int ret;
> +	int *ret;
> 
> +	ret = rte_malloc(NULL, sizeof(int) *
> +				tbl_rw_test_param.num_insert, 0);
This memory needs to be freed.

>  	for (i = 0; i < rte_lcore_count(); i++) {
>  		if (slave_core_ids[i] == lcore_id)
>  			break;
> @@ -79,13 +83,30 @@ test_hash_readwrite_worker(__attribute__((unused))
> void *arg)
>  				tbl_rw_test_param.keys + i) > 0)
>  			break;
> 
> -		ret = rte_hash_add_key(tbl_rw_test_param.h,
> +		ret[i - offset] = rte_hash_add_key(tbl_rw_test_param.h,
>  				     tbl_rw_test_param.keys + i);
> -		if (ret < 0)
> +		if (ret[i - offset] < 0)
> +			break;
> +
> +		/* lookup a random key */
> +		uint32_t rand = rte_rand() % (i + 1 - offset);
> +
> +		if (rte_hash_lookup(tbl_rw_test_param.h,
> +				tbl_rw_test_param.keys + rand) != ret[rand])
> +			break;
> +
> +
> +		if (rte_hash_del_key(tbl_rw_test_param.h,
> +				tbl_rw_test_param.keys + rand) != ret[rand])
> +			break;
> +
> +		ret[rand] = rte_hash_add_key(tbl_rw_test_param.h,
> +					tbl_rw_test_param.keys + rand);
> +		if (ret[rand] < 0)
>  			break;
> 
>  		if (rte_hash_lookup(tbl_rw_test_param.h,
> -				tbl_rw_test_param.keys + i) != ret)
> +			tbl_rw_test_param.keys + rand) != ret[rand])
>  			break;
>  	}
> 
> @@ -100,7 +121,7 @@ test_hash_readwrite_worker(__attribute__((unused))
> void *arg)  }
> 
>  static int
> -init_params(int use_htm, int use_jhash)
> +init_params(int use_ext, int use_htm, int use_jhash)
>  {
>  	unsigned int i;
> 
> @@ -129,6 +150,13 @@ init_params(int use_htm, int use_jhash)
>  			RTE_HASH_EXTRA_FLAGS_RW_CONCURRENCY |
>  			RTE_HASH_EXTRA_FLAGS_MULTI_WRITER_ADD;
> 
> +	if (use_ext)
> +		hash_params.extra_flag |=
> +			RTE_HASH_EXTRA_FLAGS_EXT_TABLE;
> +	else
> +		hash_params.extra_flag &=
> +		       ~RTE_HASH_EXTRA_FLAGS_EXT_TABLE;
> +
>  	hash_params.name = "tests";
> 
>  	handle = rte_hash_create(&hash_params); @@ -167,7 +195,7 @@
> init_params(int use_htm, int use_jhash)  }
> 
>  static int
> -test_hash_readwrite_functional(int use_htm)
> +test_hash_readwrite_functional(int use_ext, int use_htm)
>  {
>  	unsigned int i;
>  	const void *next_key;
> @@ -178,6 +206,7 @@ test_hash_readwrite_functional(int use_htm)
>  	uint32_t lost_keys = 0;
>  	int use_jhash = 1;
>  	int slave_cnt = rte_lcore_count() - 1;
> +	uint32_t tot_insert = 0;
> 
>  	rte_atomic64_init(&gcycles);
>  	rte_atomic64_clear(&gcycles);
> @@ -185,11 +214,16 @@ test_hash_readwrite_functional(int use_htm)
>  	rte_atomic64_init(&ginsertions);
>  	rte_atomic64_clear(&ginsertions);
> 
> -	if (init_params(use_htm, use_jhash) != 0)
> +	if (init_params(use_ext, use_htm, use_jhash) != 0)
>  		goto err;
> 
> +	if (use_ext)
> +		tot_insert = TOTAL_INSERT_EXT;
> +	else
> +		tot_insert = TOTAL_INSERT;
> +
>  	tbl_rw_test_param.num_insert =
> -		TOTAL_INSERT / slave_cnt;
> +		tot_insert / slave_cnt;
> 
>  	tbl_rw_test_param.rounded_tot_insert =
>  		tbl_rw_test_param.num_insert
> @@ -345,7 +379,7 @@ test_hash_readwrite_perf(struct perf *perf_results,
> int use_htm,
>  	rte_atomic64_init(&gwrite_cycles);
>  	rte_atomic64_clear(&gwrite_cycles);
> 
> -	if (init_params(use_htm, use_jhash) != 0)
> +	if (init_params(0, use_htm, use_jhash) != 0)
>  		goto err;
> 
>  	/*
> @@ -579,7 +613,7 @@ test_hash_readwrite_main(void)
>  	 * than writer threads. This is to timing either reader threads or
>  	 * writer threads for performance numbers.
>  	 */
> -	int use_htm, reader_faster;
> +	int use_htm, use_ext,  reader_faster;
>  	unsigned int i = 0, core_id = 0;
> 
>  	if (rte_lcore_count() <= 2) {
> @@ -602,7 +636,13 @@ test_hash_readwrite_main(void)
>  		printf("Test read-write with Hardware transactional
> memory\n");
> 
>  		use_htm = 1;
> -		if (test_hash_readwrite_functional(use_htm) < 0)
> +		use_ext = 0;
> +
> +		if (test_hash_readwrite_functional(use_ext, use_htm) < 0)
> +			return -1;
> +
> +		use_ext = 1;
> +		if (test_hash_readwrite_functional(use_ext, use_htm) < 0)
>  			return -1;
> 
>  		reader_faster = 1;
> @@ -621,8 +661,14 @@ test_hash_readwrite_main(void)
> 
>  	printf("Test read-write without Hardware transactional memory\n");
>  	use_htm = 0;
> -	if (test_hash_readwrite_functional(use_htm) < 0)
> +	use_ext = 0;
> +	if (test_hash_readwrite_functional(use_ext, use_htm) < 0)
>  		return -1;
> +
> +	use_ext = 1;
> +	if (test_hash_readwrite_functional(use_ext, use_htm) < 0)
> +		return -1;
> +
>  	reader_faster = 1;
>  	if (test_hash_readwrite_perf(&non_htm_results, use_htm,
>  							reader_faster) < 0)
> --
> 2.7.4

Re: [dpdk-dev] [PATCH v2 1/4] hash: fix unnecessary pause

[Wang, Yipeng1]

>-----Original Message-----
>From: Honnappa Nagarahalli [mailto:Honnappa.Nagarahalli@arm.com]
>> ---
>>  lib/librte_hash/rte_cuckoo_hash.c | 4 ++--
>>  1 file changed, 2 insertions(+), 2 deletions(-)
>>
>> diff --git a/lib/librte_hash/rte_cuckoo_hash.c
>> b/lib/librte_hash/rte_cuckoo_hash.c
>> index 0648a22..4a2647e 100644
>> --- a/lib/librte_hash/rte_cuckoo_hash.c
>> +++ b/lib/librte_hash/rte_cuckoo_hash.c
>> @@ -574,14 +574,14 @@ rte_hash_reset(struct rte_hash *h)
>>
>>  	/* clear the free ring */
>>  	while (rte_ring_dequeue(h->free_slots, &ptr) == 0)
>> -		rte_pause();
>> +		continue;
>Minor comment: 'continue' can be removed.
>
[Wang, Yipeng]  I did not find a pretty way to do it without coding style warning, e.g. add semicolon in same line...
Anyone know a common way to do it in DPDK?

Re: [dpdk-dev] [PATCH v3 3/6] test/hash: test more corner cases in unit test

[Honnappa Nagarahalli]

>
> This commit improves the readwrite unit test to cover more corner cases and
> reduces the testing time by reducing the total key count.
>
> Signed-off-by: Yipeng Wang <yipeng1.wang@intel.com>
> Acked-by: Bruce Richardson <bruce.richardson@intel.com>
> ---
>  test/test/test_hash_readwrite.c | 31 +++++++++++++++++++++++++------
>  1 file changed, 25 insertions(+), 6 deletions(-)
>
> diff --git a/test/test/test_hash_readwrite.c b/test/test/test_hash_readwrite.c
> index a8fadd0..a45c669 100644
> --- a/test/test/test_hash_readwrite.c
> +++ b/test/test/test_hash_readwrite.c
> @@ -18,8 +18,8 @@
>
>  #define RTE_RWTEST_FAIL 0
>
> -#define TOTAL_ENTRY (16*1024*1024)
> -#define TOTAL_INSERT (15*1024*1024)
> +#define TOTAL_ENTRY (5*1024*1024)
> +#define TOTAL_INSERT (4.5*1024*1024)
>
>  #define NUM_TEST 3
>  unsigned int core_cnt[NUM_TEST] = {2, 4, 8}; @@ -59,8 +59,10 @@
> test_hash_readwrite_worker(__attribute__((unused)) void *arg)
>  uint64_t i, offset;
>  uint32_t lcore_id = rte_lcore_id();
>  uint64_t begin, cycles;
> -int ret;
> +int *ret;
>
> +ret = rte_malloc(NULL, sizeof(int) *
> +tbl_rw_test_param.num_insert, 0);
This memory needs to be freed at the end of this function

>  for (i = 0; i < rte_lcore_count(); i++) {
>  if (slave_core_ids[i] == lcore_id)
>  break;
> @@ -79,13 +81,30 @@ test_hash_readwrite_worker(__attribute__((unused))
> void *arg)
>  tbl_rw_test_param.keys + i) > 0)
>  break;
>
> -ret = rte_hash_add_key(tbl_rw_test_param.h,
> +ret[i - offset] = rte_hash_add_key(tbl_rw_test_param.h,
>       tbl_rw_test_param.keys + i);
> -if (ret < 0)
> +if (ret[i - offset] < 0)
> +break;
> +
> +/* lookup a random key */
> +uint32_t rand = rte_rand() % (i + 1 - offset);
> +
> +if (rte_hash_lookup(tbl_rw_test_param.h,
> +tbl_rw_test_param.keys + rand) != ret[rand])
> +break;
> +
> +
> +if (rte_hash_del_key(tbl_rw_test_param.h,
> +tbl_rw_test_param.keys + rand) != ret[rand])
> +break;
> +
> +ret[rand] = rte_hash_add_key(tbl_rw_test_param.h,
> +tbl_rw_test_param.keys + rand);
> +if (ret[rand] < 0)
>  break;
>
>  if (rte_hash_lookup(tbl_rw_test_param.h,
> -tbl_rw_test_param.keys + i) != ret)
> +tbl_rw_test_param.keys + rand) != ret[rand])
>  break;
>  }
>
> --
> 2.7.4

IMPORTANT NOTICE: The contents of this email and any attachments are confidential and may also be privileged. If you are not the intended recipient, please notify the sender immediately and do not disclose the contents to any other person, use it for any purpose, or store or copy the information in any medium. Thank you.

[dpdk-dev] [PATCH v4 0/6] hash: improve multiple places

[Yipeng Wang]

This patch set depends on Honnappa's patch set:
http://patchwork.dpdk.org/cover/47268/

This patch set fixes/improves a couple of places mostly
on unit tests:

commit 1: remove unnecessary code in hash library.
commit 2: use jhash in multiwriter unit test.
commit 3: cover more test case in unit test.
commit 4: improve readwrite test to consider the extendable table.
commit 5: remove scaling unit test.
commit 6: add readwrite test to meson and autotest file.

v3->v4:
* Fix memory leak in commit 3 (Honnappa)

v2->v3:
* Split commit 4 to commit 3 and 4 (Honnappa)
* Remove hard coded macro (Bruce)
* Add commit 6 fix to add readwrite test to autotest files.
* Remove unnecessary header in commit 1 (Mattias)

V1->V2:
* In commit 2 change use_jhash to a macro instead of a hard coded
local variable (Bruce).
* Add commit 4 to remove scaling unit test (Bruce).

Signed-off-by: Yipeng Wang <yipeng1.wang@intel.com>

Yipeng Wang (6):
  hash: fix unnecessary pause
  test/hash: change multiwriter test to use jhash
  test/hash: test more corner cases in unit test
  test/hash: add readwrite test for ext table
  test/hash: remove hash scaling unit test
  test/hash: fix to add read-write test to autotest

 lib/librte_hash/rte_cuckoo_hash.c |   5 +-
 test/test/Makefile                |   1 -
 test/test/autotest_data.py        |  12 +--
 test/test/meson.build             |   3 +-
 test/test/test_hash_multiwriter.c |   3 +-
 test/test/test_hash_readwrite.c   |  74 ++++++++++++---
 test/test/test_hash_scaling.c     | 191 --------------------------------------
 7 files changed, 71 insertions(+), 218 deletions(-)
 delete mode 100644 test/test/test_hash_scaling.c

-- 
2.7.4

[dpdk-dev] [PATCH v4 1/6] hash: fix unnecessary pause

[Yipeng Wang]

There is a rte_pause in hash table reset function.
Since the loop is not a polling loop on shared
data structure, the rte_pause is not needed.

Fixes: b26473ff8f4a ("hash: add reset function")
Cc: stable@dpdk.org

Signed-off-by: Yipeng Wang <yipeng1.wang@intel.com>
Acked-by: Bruce Richardson <bruce.richardson@intel.com>
---
 lib/librte_hash/rte_cuckoo_hash.c | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/lib/librte_hash/rte_cuckoo_hash.c b/lib/librte_hash/rte_cuckoo_hash.c
index 0648a22..5ddcccd 100644
--- a/lib/librte_hash/rte_cuckoo_hash.c
+++ b/lib/librte_hash/rte_cuckoo_hash.c
@@ -27,7 +27,6 @@
 #include <rte_spinlock.h>
 #include <rte_ring.h>
 #include <rte_compat.h>
-#include <rte_pause.h>
 
 #include "rte_hash.h"
 #include "rte_cuckoo_hash.h"
@@ -574,14 +573,14 @@ rte_hash_reset(struct rte_hash *h)
 
 	/* clear the free ring */
 	while (rte_ring_dequeue(h->free_slots, &ptr) == 0)
-		rte_pause();
+		continue;
 
 	/* clear free extendable bucket ring and memory */
 	if (h->ext_table_support) {
 		memset(h->buckets_ext, 0, h->num_buckets *
 						sizeof(struct rte_hash_bucket));
 		while (rte_ring_dequeue(h->free_ext_bkts, &ptr) == 0)
-			rte_pause();
+			continue;
 	}
 
 	/* Repopulate the free slots ring. Entry zero is reserved for key misses */
-- 
2.7.4

[dpdk-dev] [PATCH v4 2/6] test/hash: change multiwriter test to use jhash

[Yipeng Wang]

With sequential key, the test will cover more corner
cases with jhash instead of crc hash, since jhash
generates more random hash pattern on sequential key.
It is useful for functional verification.

Signed-off-by: Yipeng Wang <yipeng1.wang@intel.com>
---
 test/test/test_hash_multiwriter.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/test/test/test_hash_multiwriter.c b/test/test/test_hash_multiwriter.c
index 6a3eb10..d447f6d 100644
--- a/test/test/test_hash_multiwriter.c
+++ b/test/test/test_hash_multiwriter.c
@@ -12,6 +12,7 @@
 #include <rte_malloc.h>
 #include <rte_random.h>
 #include <rte_spinlock.h>
+#include <rte_jhash.h>
 
 #include "test.h"
 
@@ -108,7 +109,7 @@ test_hash_multiwriter(void)
 	struct rte_hash_parameters hash_params = {
 		.entries = nb_entries,
 		.key_len = sizeof(uint32_t),
-		.hash_func = rte_hash_crc,
+		.hash_func = rte_jhash,
 		.hash_func_init_val = 0,
 		.socket_id = rte_socket_id(),
 	};
-- 
2.7.4

[dpdk-dev] [PATCH v4 3/6] test/hash: test more corner cases in unit test

[Yipeng Wang]

This commit improves the readwrite unit test to cover
more corner cases and reduces the testing time by
reducing the total key count.

Signed-off-by: Yipeng Wang <yipeng1.wang@intel.com>
Acked-by: Bruce Richardson <bruce.richardson@intel.com>
---
 test/test/test_hash_readwrite.c | 32 ++++++++++++++++++++++++++------
 1 file changed, 26 insertions(+), 6 deletions(-)

diff --git a/test/test/test_hash_readwrite.c b/test/test/test_hash_readwrite.c
index a8fadd0..26e802c 100644
--- a/test/test/test_hash_readwrite.c
+++ b/test/test/test_hash_readwrite.c
@@ -18,8 +18,8 @@
 
 #define RTE_RWTEST_FAIL 0
 
-#define TOTAL_ENTRY (16*1024*1024)
-#define TOTAL_INSERT (15*1024*1024)
+#define TOTAL_ENTRY (5*1024*1024)
+#define TOTAL_INSERT (4.5*1024*1024)
 
 #define NUM_TEST 3
 unsigned int core_cnt[NUM_TEST] = {2, 4, 8};
@@ -59,8 +59,10 @@ test_hash_readwrite_worker(__attribute__((unused)) void *arg)
 	uint64_t i, offset;
 	uint32_t lcore_id = rte_lcore_id();
 	uint64_t begin, cycles;
-	int ret;
+	int *ret;
 
+	ret = rte_malloc(NULL, sizeof(int) *
+				tbl_rw_test_param.num_insert, 0);
 	for (i = 0; i < rte_lcore_count(); i++) {
 		if (slave_core_ids[i] == lcore_id)
 			break;
@@ -79,13 +81,30 @@ test_hash_readwrite_worker(__attribute__((unused)) void *arg)
 				tbl_rw_test_param.keys + i) > 0)
 			break;
 
-		ret = rte_hash_add_key(tbl_rw_test_param.h,
+		ret[i - offset] = rte_hash_add_key(tbl_rw_test_param.h,
 				     tbl_rw_test_param.keys + i);
-		if (ret < 0)
+		if (ret[i - offset] < 0)
+			break;
+
+		/* lookup a random key */
+		uint32_t rand = rte_rand() % (i + 1 - offset);
+
+		if (rte_hash_lookup(tbl_rw_test_param.h,
+				tbl_rw_test_param.keys + rand) != ret[rand])
+			break;
+
+
+		if (rte_hash_del_key(tbl_rw_test_param.h,
+				tbl_rw_test_param.keys + rand) != ret[rand])
+			break;
+
+		ret[rand] = rte_hash_add_key(tbl_rw_test_param.h,
+					tbl_rw_test_param.keys + rand);
+		if (ret[rand] < 0)
 			break;
 
 		if (rte_hash_lookup(tbl_rw_test_param.h,
-				tbl_rw_test_param.keys + i) != ret)
+			tbl_rw_test_param.keys + rand) != ret[rand])
 			break;
 	}
 
@@ -96,6 +115,7 @@ test_hash_readwrite_worker(__attribute__((unused)) void *arg)
 	for (; i < offset + tbl_rw_test_param.num_insert; i++)
 		tbl_rw_test_param.keys[i] = RTE_RWTEST_FAIL;
 
+	rte_free(ret);
 	return 0;
 }
 
-- 
2.7.4

[dpdk-dev] [PATCH v4 4/6] test/hash: add readwrite test for ext table

[Yipeng Wang]

This commit improves the readwrite test to consider
extendable table feature.

Signed-off-by: Yipeng Wang <yipeng1.wang@intel.com>
Acked-by: Bruce Richardson <bruce.richardson@intel.com>
---
 test/test/test_hash_readwrite.c | 42 +++++++++++++++++++++++++++++++++--------
 1 file changed, 34 insertions(+), 8 deletions(-)

diff --git a/test/test/test_hash_readwrite.c b/test/test/test_hash_readwrite.c
index 26e802c..01f986c 100644
--- a/test/test/test_hash_readwrite.c
+++ b/test/test/test_hash_readwrite.c
@@ -20,6 +20,7 @@
 
 #define TOTAL_ENTRY (5*1024*1024)
 #define TOTAL_INSERT (4.5*1024*1024)
+#define TOTAL_INSERT_EXT (5*1024*1024)
 
 #define NUM_TEST 3
 unsigned int core_cnt[NUM_TEST] = {2, 4, 8};
@@ -120,7 +121,7 @@ test_hash_readwrite_worker(__attribute__((unused)) void *arg)
 }
 
 static int
-init_params(int use_htm, int use_jhash)
+init_params(int use_ext, int use_htm, int use_jhash)
 {
 	unsigned int i;
 
@@ -149,6 +150,13 @@ init_params(int use_htm, int use_jhash)
 			RTE_HASH_EXTRA_FLAGS_RW_CONCURRENCY |
 			RTE_HASH_EXTRA_FLAGS_MULTI_WRITER_ADD;
 
+	if (use_ext)
+		hash_params.extra_flag |=
+			RTE_HASH_EXTRA_FLAGS_EXT_TABLE;
+	else
+		hash_params.extra_flag &=
+		       ~RTE_HASH_EXTRA_FLAGS_EXT_TABLE;
+
 	hash_params.name = "tests";
 
 	handle = rte_hash_create(&hash_params);
@@ -187,7 +195,7 @@ init_params(int use_htm, int use_jhash)
 }
 
 static int
-test_hash_readwrite_functional(int use_htm)
+test_hash_readwrite_functional(int use_ext, int use_htm)
 {
 	unsigned int i;
 	const void *next_key;
@@ -198,6 +206,7 @@ test_hash_readwrite_functional(int use_htm)
 	uint32_t lost_keys = 0;
 	int use_jhash = 1;
 	int slave_cnt = rte_lcore_count() - 1;
+	uint32_t tot_insert = 0;
 
 	rte_atomic64_init(&gcycles);
 	rte_atomic64_clear(&gcycles);
@@ -205,11 +214,16 @@ test_hash_readwrite_functional(int use_htm)
 	rte_atomic64_init(&ginsertions);
 	rte_atomic64_clear(&ginsertions);
 
-	if (init_params(use_htm, use_jhash) != 0)
+	if (init_params(use_ext, use_htm, use_jhash) != 0)
 		goto err;
 
+	if (use_ext)
+		tot_insert = TOTAL_INSERT_EXT;
+	else
+		tot_insert = TOTAL_INSERT;
+
 	tbl_rw_test_param.num_insert =
-		TOTAL_INSERT / slave_cnt;
+		tot_insert / slave_cnt;
 
 	tbl_rw_test_param.rounded_tot_insert =
 		tbl_rw_test_param.num_insert
@@ -365,7 +379,7 @@ test_hash_readwrite_perf(struct perf *perf_results, int use_htm,
 	rte_atomic64_init(&gwrite_cycles);
 	rte_atomic64_clear(&gwrite_cycles);
 
-	if (init_params(use_htm, use_jhash) != 0)
+	if (init_params(0, use_htm, use_jhash) != 0)
 		goto err;
 
 	/*
@@ -599,7 +613,7 @@ test_hash_readwrite_main(void)
 	 * than writer threads. This is to timing either reader threads or
 	 * writer threads for performance numbers.
 	 */
-	int use_htm, reader_faster;
+	int use_htm, use_ext,  reader_faster;
 	unsigned int i = 0, core_id = 0;
 
 	if (rte_lcore_count() <= 2) {
@@ -622,7 +636,13 @@ test_hash_readwrite_main(void)
 		printf("Test read-write with Hardware transactional memory\n");
 
 		use_htm = 1;
-		if (test_hash_readwrite_functional(use_htm) < 0)
+		use_ext = 0;
+
+		if (test_hash_readwrite_functional(use_ext, use_htm) < 0)
+			return -1;
+
+		use_ext = 1;
+		if (test_hash_readwrite_functional(use_ext, use_htm) < 0)
 			return -1;
 
 		reader_faster = 1;
@@ -641,8 +661,14 @@ test_hash_readwrite_main(void)
 
 	printf("Test read-write without Hardware transactional memory\n");
 	use_htm = 0;
-	if (test_hash_readwrite_functional(use_htm) < 0)
+	use_ext = 0;
+	if (test_hash_readwrite_functional(use_ext, use_htm) < 0)
 		return -1;
+
+	use_ext = 1;
+	if (test_hash_readwrite_functional(use_ext, use_htm) < 0)
+		return -1;
+
 	reader_faster = 1;
 	if (test_hash_readwrite_perf(&non_htm_results, use_htm,
 							reader_faster) < 0)
-- 
2.7.4

[dpdk-dev] [PATCH v4 5/6] test/hash: remove hash scaling unit test

[Yipeng Wang]

The hash scaling unit test is not really needed
any more since the multi-writer is supported now
inside the library and it is tested by multi-writer
unit test.

Signed-off-by: Yipeng Wang <yipeng1.wang@intel.com>
Acked-by: Bruce Richardson <bruce.richardson@intel.com>
---
 test/test/Makefile            |   1 -
 test/test/autotest_data.py    |   6 --
 test/test/meson.build         |   2 -
 test/test/test_hash_scaling.c | 191 ------------------------------------------
 4 files changed, 200 deletions(-)
 delete mode 100644 test/test/test_hash_scaling.c

diff --git a/test/test/Makefile b/test/test/Makefile
index 1b5c070..6b77e15 100644
--- a/test/test/Makefile
+++ b/test/test/Makefile
@@ -113,7 +113,6 @@ SRCS-$(CONFIG_RTE_LIBRTE_HASH) += test_hash.c
 SRCS-$(CONFIG_RTE_LIBRTE_HASH) += test_thash.c
 SRCS-$(CONFIG_RTE_LIBRTE_HASH) += test_hash_perf.c
 SRCS-$(CONFIG_RTE_LIBRTE_HASH) += test_hash_functions.c
-SRCS-$(CONFIG_RTE_LIBRTE_HASH) += test_hash_scaling.c
 SRCS-$(CONFIG_RTE_LIBRTE_HASH) += test_hash_multiwriter.c
 SRCS-$(CONFIG_RTE_LIBRTE_HASH) += test_hash_readwrite.c
 SRCS-$(CONFIG_RTE_LIBRTE_HASH) += test_hash_readwrite_lf.c
diff --git a/test/test/autotest_data.py b/test/test/autotest_data.py
index 4eae588..9265617 100644
--- a/test/test/autotest_data.py
+++ b/test/test/autotest_data.py
@@ -345,12 +345,6 @@
         "Report":  None,
     },
     {
-        "Name":    "Hash scaling autotest",
-        "Command": "hash_scaling_autotest",
-        "Func":    default_autotest,
-        "Report":  None,
-    },
-    {
         "Name":    "Hash multiwriter autotest",
         "Command": "hash_multiwriter_autotest",
         "Func":    default_autotest,
diff --git a/test/test/meson.build b/test/test/meson.build
index faef5a4..7ebe9ab 100644
--- a/test/test/meson.build
+++ b/test/test/meson.build
@@ -47,7 +47,6 @@ test_sources = files('commands.c',
 	'test_hash_readwrite.c',
 	'test_hash_perf.c',
 	'test_hash_readwrite_lf.c',
-	'test_hash_scaling.c',
 	'test_interrupts.c',
 	'test_kni.c',
 	'test_kvargs.c',
@@ -170,7 +169,6 @@ test_names = [
 	'external_mem_autotest',
 	'func_reentrancy_autotest',
 	'flow_classify_autotest',
-	'hash_scaling_autotest',
 	'hash_autotest',
 	'hash_functions_autotest',
 	'hash_multiwriter_autotest',
diff --git a/test/test/test_hash_scaling.c b/test/test/test_hash_scaling.c
deleted file mode 100644
index 07765a7..0000000
--- a/test/test/test_hash_scaling.c
+++ /dev/null
@@ -1,191 +0,0 @@
-/* SPDX-License-Identifier: BSD-3-Clause
- * Copyright(c) 2015 Intel Corporation
- */
-
-#include <stdio.h>
-
-#include <rte_cycles.h>
-#include <rte_hash.h>
-#include <rte_hash_crc.h>
-#include <rte_spinlock.h>
-#include <rte_launch.h>
-
-#include "test.h"
-
-/*
- * Check condition and return an error if true. Assumes that "handle" is the
- * name of the hash structure pointer to be freed.
- */
-#define RETURN_IF_ERROR(cond, str, ...) do {				\
-	if (cond) {							\
-		printf("ERROR line %d: " str "\n", __LINE__,		\
-							##__VA_ARGS__);	\
-		if (handle)						\
-			rte_hash_free(handle);				\
-		return -1;						\
-	}								\
-} while (0)
-
-enum locking_mode_t {
-	NORMAL_LOCK,
-	LOCK_ELISION,
-	NULL_LOCK
-};
-
-struct {
-	uint32_t num_iterations;
-	struct rte_hash *h;
-	rte_spinlock_t *lock;
-	int locking_mode;
-} tbl_scaling_test_params;
-
-static rte_atomic64_t gcycles;
-
-static int test_hash_scaling_worker(__attribute__((unused)) void *arg)
-{
-	uint64_t i, key;
-	uint32_t thr_id = rte_sys_gettid();
-	uint64_t begin, cycles = 0;
-
-	switch (tbl_scaling_test_params.locking_mode) {
-
-	case NORMAL_LOCK:
-
-		for (i = 0; i < tbl_scaling_test_params.num_iterations; i++) {
-			/*	different threads get different keys because
-				we use the thread-id in the key computation
-			 */
-			key = rte_hash_crc(&i, sizeof(i), thr_id);
-			begin = rte_rdtsc_precise();
-			rte_spinlock_lock(tbl_scaling_test_params.lock);
-			rte_hash_add_key(tbl_scaling_test_params.h, &key);
-			rte_spinlock_unlock(tbl_scaling_test_params.lock);
-			cycles += rte_rdtsc_precise() - begin;
-		}
-		break;
-
-	case LOCK_ELISION:
-
-		for (i = 0; i < tbl_scaling_test_params.num_iterations; i++) {
-			key = rte_hash_crc(&i, sizeof(i), thr_id);
-			begin = rte_rdtsc_precise();
-			rte_spinlock_lock_tm(tbl_scaling_test_params.lock);
-			rte_hash_add_key(tbl_scaling_test_params.h, &key);
-			rte_spinlock_unlock_tm(tbl_scaling_test_params.lock);
-			cycles += rte_rdtsc_precise() - begin;
-		}
-		break;
-
-	default:
-
-		for (i = 0; i < tbl_scaling_test_params.num_iterations; i++) {
-			key = rte_hash_crc(&i, sizeof(i), thr_id);
-			begin = rte_rdtsc_precise();
-			rte_hash_add_key(tbl_scaling_test_params.h, &key);
-			cycles += rte_rdtsc_precise() - begin;
-		}
-	}
-
-	rte_atomic64_add(&gcycles, cycles);
-
-	return 0;
-}
-
-/*
- * Do scalability perf tests.
- */
-static int
-test_hash_scaling(int locking_mode)
-{
-	static unsigned calledCount =    1;
-	uint32_t num_iterations = 1024*1024;
-	uint64_t i, key;
-	struct rte_hash_parameters hash_params = {
-		.entries = num_iterations*2,
-		.key_len = sizeof(key),
-		.hash_func = rte_hash_crc,
-		.hash_func_init_val = 0,
-		.socket_id = rte_socket_id(),
-		.extra_flag = RTE_HASH_EXTRA_FLAGS_TRANS_MEM_SUPPORT
-	};
-	struct rte_hash *handle;
-	char name[RTE_HASH_NAMESIZE];
-	rte_spinlock_t lock;
-
-	rte_spinlock_init(&lock);
-
-	snprintf(name, 32, "test%u", calledCount++);
-	hash_params.name = name;
-
-	handle = rte_hash_create(&hash_params);
-	RETURN_IF_ERROR(handle == NULL, "hash creation failed");
-
-	tbl_scaling_test_params.num_iterations =
-		num_iterations/rte_lcore_count();
-	tbl_scaling_test_params.h = handle;
-	tbl_scaling_test_params.lock = &lock;
-	tbl_scaling_test_params.locking_mode = locking_mode;
-
-	rte_atomic64_init(&gcycles);
-	rte_atomic64_clear(&gcycles);
-
-	/* fill up to initial size */
-	for (i = 0; i < num_iterations; i++) {
-		key = rte_hash_crc(&i, sizeof(i), 0xabcdabcd);
-		rte_hash_add_key(tbl_scaling_test_params.h, &key);
-	}
-
-	rte_eal_mp_remote_launch(test_hash_scaling_worker, NULL, CALL_MASTER);
-	rte_eal_mp_wait_lcore();
-
-	unsigned long long int cycles_per_operation =
-		rte_atomic64_read(&gcycles)/
-		(tbl_scaling_test_params.num_iterations*rte_lcore_count());
-	const char *lock_name;
-
-	switch (locking_mode) {
-	case NORMAL_LOCK:
-		lock_name = "normal spinlock";
-		break;
-	case LOCK_ELISION:
-		lock_name = "lock elision";
-		break;
-	default:
-		lock_name = "null lock";
-	}
-	printf("--------------------------------------------------------\n");
-	printf("Cores: %d; %s mode ->  cycles per operation: %llu\n",
-		rte_lcore_count(), lock_name, cycles_per_operation);
-	printf("--------------------------------------------------------\n");
-	/* CSV output */
-	printf(">>>%d,%s,%llu\n", rte_lcore_count(), lock_name,
-		cycles_per_operation);
-
-	rte_hash_free(handle);
-	return 0;
-}
-
-static int
-test_hash_scaling_main(void)
-{
-	int r = 0;
-
-	if (rte_lcore_count() == 1)
-		r = test_hash_scaling(NULL_LOCK);
-
-	if (r == 0)
-		r = test_hash_scaling(NORMAL_LOCK);
-
-	if (!rte_tm_supported()) {
-		printf("Hardware transactional memory (lock elision) is NOT supported\n");
-		return r;
-	}
-	printf("Hardware transactional memory (lock elision) is supported\n");
-
-	if (r == 0)
-		r = test_hash_scaling(LOCK_ELISION);
-
-	return r;
-}
-
-REGISTER_TEST_COMMAND(hash_scaling_autotest, test_hash_scaling_main);
-- 
2.7.4

[dpdk-dev] [PATCH v4 6/6] test/hash: fix to add read-write test to autotest

[Yipeng Wang]

Add read-write concurrency test to meson and autotest
file.

Fixes: 0eb3726ebcf1 ("test/hash: add test for read/write concurrency")
Cc: stable@dpdk.org

Signed-off-by: Yipeng Wang <yipeng1.wang@intel.com>
---
 test/test/autotest_data.py | 6 ++++++
 test/test/meson.build      | 1 +
 2 files changed, 7 insertions(+)

diff --git a/test/test/autotest_data.py b/test/test/autotest_data.py
index 9265617..0df686a 100644
--- a/test/test/autotest_data.py
+++ b/test/test/autotest_data.py
@@ -574,6 +574,12 @@
         "Report":  None,
     },
     {
+        "Name":    "Hash read-write concurrency autotest",
+        "Command": "hash_readwrite_autotest",
+        "Func":    default_autotest,
+        "Report":  None,
+    },
+    {
         "Name":    "Hash read-write lock-free concurrency autotest",
         "Command": "hash_readwrite_lf_autotest",
         "Func":    default_autotest,
diff --git a/test/test/meson.build b/test/test/meson.build
index 7ebe9ab..1c1fcd7 100644
--- a/test/test/meson.build
+++ b/test/test/meson.build
@@ -173,6 +173,7 @@ test_names = [
 	'hash_functions_autotest',
 	'hash_multiwriter_autotest',
 	'hash_perf_autotest',
+	'hash_readwrite_autotest',
 	'hash_readwrite_lf_autotest',
 	'interrupt_autotest',
 	'kni_autotest',
-- 
2.7.4

Re: [dpdk-dev] [PATCH v3 3/6] test/hash: test more corner cases in unit test

[Thomas Monjalon]

> > --- a/test/test/test_hash_readwrite.c
> > +++ b/test/test/test_hash_readwrite.c
> > test_hash_readwrite_worker(__attribute__((unused)) void *arg)
> >  uint64_t i, offset;
> >  uint32_t lcore_id = rte_lcore_id();
> >  uint64_t begin, cycles;
> > -int ret;
> > +int *ret;
> >
> > +ret = rte_malloc(NULL, sizeof(int) *
> > +tbl_rw_test_param.num_insert, 0);
> This memory needs to be freed at the end of this function

I am waiting a v4 for this change, please.

Re: [dpdk-dev] [PATCH v2 1/4] hash: fix unnecessary pause

[Bruce Richardson]

On Fri, Oct 26, 2018 at 03:04:12AM +0100, Wang, Yipeng1 wrote:
> >-----Original Message-----
> >From: Honnappa Nagarahalli [mailto:Honnappa.Nagarahalli@arm.com]
> >> ---
> >>  lib/librte_hash/rte_cuckoo_hash.c | 4 ++--
> >>  1 file changed, 2 insertions(+), 2 deletions(-)
> >>
> >> diff --git a/lib/librte_hash/rte_cuckoo_hash.c
> >> b/lib/librte_hash/rte_cuckoo_hash.c
> >> index 0648a22..4a2647e 100644
> >> --- a/lib/librte_hash/rte_cuckoo_hash.c
> >> +++ b/lib/librte_hash/rte_cuckoo_hash.c
> >> @@ -574,14 +574,14 @@ rte_hash_reset(struct rte_hash *h)
> >>
> >>  	/* clear the free ring */
> >>  	while (rte_ring_dequeue(h->free_slots, &ptr) == 0)
> >> -		rte_pause();
> >> +		continue;
> >Minor comment: 'continue' can be removed.
> >
> [Wang, Yipeng]  I did not find a pretty way to do it without coding style warning, e.g. add semicolon in same line...
> Anyone know a common way to do it in DPDK?
> 
Semi-colon on a new line, possibly with comment should work ok. However,
the continue is harmless, so I'm ok with it as-is.

Re: [dpdk-dev] [PATCH v4 0/6] hash: improve multiple places

[Thomas Monjalon]

> Yipeng Wang (6):
>   hash: fix unnecessary pause
>   test/hash: change multiwriter test to use jhash
>   test/hash: test more corner cases in unit test
>   test/hash: add readwrite test for ext table
>   test/hash: remove hash scaling unit test
>   test/hash: fix to add read-write test to autotest

Applied, thanks