DPDK patches and discussions
 help / color / Atom feed
* [dpdk-dev] [PATCH 0/2] test/meson: fix hash readwrite timeout failure
@ 2019-09-06  5:49 agupta3
  2019-09-06  5:49 ` [dpdk-dev] [PATCH 1/2] test/meson: hash test split into shorter subtests agupta3
                   ` (3 more replies)
  0 siblings, 4 replies; 38+ messages in thread
From: agupta3 @ 2019-09-06  5:49 UTC (permalink / raw)
  To: yipeng1.wang, sameh.gobriel, bruce.richardson, pablo.de.lara.guarch
  Cc: dev, Amit Gupta

From: Amit Gupta <agupta3@marvell.com>

V1 changes:
* hash_readwrite and hash_readwrite lockfree meson test was
taking longer time to complete. The test always get TIMEOUT.
* hash readwrtie test is split into functional and perf tests 
and moved to dpdk fast and perf testsuite accordingly.
* hash readwrite lockfree is moved to dpdk perf testsuite.

Amit Gupta (2):
  test/meson: hash test split into shorter subtests
  test/meson: hash lf test moved to dpdk perf testsuite

 app/test/meson.build           |   5 +-
 app/test/test_hash_readwrite.c | 146 +++++++++++++++++++++++++++++++++++++++++
 2 files changed, 149 insertions(+), 2 deletions(-)

-- 
1.8.3.1


^ permalink raw reply	[flat|nested] 38+ messages in thread

* [dpdk-dev] [PATCH 1/2] test/meson: hash test split into shorter subtests
  2019-09-06  5:49 [dpdk-dev] [PATCH 0/2] test/meson: fix hash readwrite timeout failure agupta3
@ 2019-09-06  5:49 ` agupta3
  2019-09-11 17:05   ` Wang, Yipeng1
  2019-09-06  5:49 ` [dpdk-dev] [PATCH 2/2] test/meson: hash lf test moved to dpdk perf testsuite agupta3
                   ` (2 subsequent siblings)
  3 siblings, 1 reply; 38+ messages in thread
From: agupta3 @ 2019-09-06  5:49 UTC (permalink / raw)
  To: yipeng1.wang, sameh.gobriel, bruce.richardson, pablo.de.lara.guarch
  Cc: dev, Amit Gupta

From: Amit Gupta <agupta3@marvell.com>

hash_readwrite meson test was taking longer time to complete.
The test always get TIMEOUT, hence test is split into
functional and perf test. perf test is being moved under
dpdk perf testsuites in  meson build.

Signed-off-by: Amit Gupta <agupta3@marvell.com>
---
 app/test/meson.build           |   3 +-
 app/test/test_hash_readwrite.c | 146 +++++++++++++++++++++++++++++++++++++++++
 2 files changed, 148 insertions(+), 1 deletion(-)

diff --git a/app/test/meson.build b/app/test/meson.build
index ec40943..94fd9f8 100644
--- a/app/test/meson.build
+++ b/app/test/meson.build
@@ -219,7 +219,7 @@ fast_test_names = [
         'distributor_autotest',
         'eventdev_common_autotest',
         'fbarray_autotest',
-        'hash_readwrite_autotest',
+        'hash_readwrite_func_autotest',
         'hash_readwrite_lf_autotest',
         'ipsec_autotest',
         'kni_autotest',
@@ -262,6 +262,7 @@ perf_test_names = [
         'stack_perf_autotest',
         'stack_lf_perf_autotest',
         'rand_perf_autotest',
+        'hash_readwrite_perf_autotest',
 ]
 
 driver_test_names = [
diff --git a/app/test/test_hash_readwrite.c b/app/test/test_hash_readwrite.c
index 4376b09..c25e904 100644
--- a/app/test/test_hash_readwrite.c
+++ b/app/test/test_hash_readwrite.c
@@ -606,6 +606,150 @@ struct {
 }
 
 static int
+test_hash_rw_perf_main(void)
+{
+	/*
+	 * Variables used to choose different tests.
+	 * use_htm indicates if hardware transactional memory should be used.
+	 * reader_faster indicates if the reader threads should finish earlier
+	 * than writer threads. This is to timing either reader threads or
+	 * writer threads for performance numbers.
+	 */
+	int use_htm, reader_faster;
+	unsigned int i = 0, core_id = 0;
+
+	if (rte_lcore_count() < 3) {
+		printf("Not enough cores for hash_readwrite_autotest, expecting at least 3\n");
+		return TEST_SKIPPED;
+	}
+
+	RTE_LCORE_FOREACH_SLAVE(core_id) {
+		slave_core_ids[i] = core_id;
+		i++;
+	}
+
+	setlocale(LC_NUMERIC, "");
+
+	if (rte_tm_supported()) {
+		printf("Hardware transactional memory (lock elision) "
+			"is supported\n");
+
+		printf("Test read-write with Hardware transactional memory\n");
+
+		use_htm = 1;
+
+		reader_faster = 1;
+		if (test_hash_readwrite_perf(&htm_results, use_htm,
+							reader_faster) < 0)
+			return -1;
+
+		reader_faster = 0;
+		if (test_hash_readwrite_perf(&htm_results, use_htm,
+							reader_faster) < 0)
+			return -1;
+	} else {
+		printf("Hardware transactional memory (lock elision) "
+			"is NOT supported\n");
+	}
+
+	printf("Test read-write without Hardware transactional memory\n");
+	use_htm = 0;
+
+	reader_faster = 1;
+	if (test_hash_readwrite_perf(&non_htm_results, use_htm,
+							reader_faster) < 0)
+		return -1;
+	reader_faster = 0;
+	if (test_hash_readwrite_perf(&non_htm_results, use_htm,
+							reader_faster) < 0)
+		return -1;
+
+	printf("================\n");
+	printf("Results summary:\n");
+	printf("================\n");
+
+	printf("single read: %u\n", htm_results.single_read);
+	printf("single write: %u\n", htm_results.single_write);
+	for (i = 0; i < NUM_TEST; i++) {
+		printf("+++ core_cnt: %u +++\n", core_cnt[i]);
+		printf("HTM:\n");
+		printf("  read only: %u\n", htm_results.read_only[i]);
+		printf("  write only: %u\n", htm_results.write_only[i]);
+		printf("  read-write read: %u\n", htm_results.read_write_r[i]);
+		printf("  read-write write: %u\n", htm_results.read_write_w[i]);
+
+		printf("non HTM:\n");
+		printf("  read only: %u\n", non_htm_results.read_only[i]);
+		printf("  write only: %u\n", non_htm_results.write_only[i]);
+		printf("  read-write read: %u\n",
+			non_htm_results.read_write_r[i]);
+		printf("  read-write write: %u\n",
+			non_htm_results.read_write_w[i]);
+	}
+
+	return 0;
+}
+
+static int
+test_hash_rw_func_main(void)
+{
+	/*
+	 * Variables used to choose different tests.
+	 * use_htm indicates if hardware transactional memory should be used.
+	 * reader_faster indicates if the reader threads should finish earlier
+	 * than writer threads. This is to timing either reader threads or
+	 * writer threads for performance numbers.
+	 */
+	int use_htm, use_ext;
+	unsigned int i = 0, core_id = 0;
+
+	if (rte_lcore_count() < 3) {
+		printf("Not enough cores for hash_readwrite_autotest, expecting at least 3\n");
+		return TEST_SKIPPED;
+	}
+
+	RTE_LCORE_FOREACH_SLAVE(core_id) {
+		slave_core_ids[i] = core_id;
+		i++;
+	}
+
+	setlocale(LC_NUMERIC, "");
+
+	if (rte_tm_supported()) {
+		printf("Hardware transactional memory (lock elision) "
+			"is supported\n");
+
+		printf("Test read-write with Hardware transactional memory\n");
+
+		use_htm = 1;
+		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;
+
+	} else {
+		printf("Hardware transactional memory (lock elision) "
+			"is NOT supported\n");
+	}
+
+	printf("Test read-write without Hardware transactional memory\n");
+	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;
+
+	return 0;
+}
+
+static int
 test_hash_readwrite_main(void)
 {
 	/*
@@ -706,3 +850,5 @@ struct {
 }
 
 REGISTER_TEST_COMMAND(hash_readwrite_autotest, test_hash_readwrite_main);
+REGISTER_TEST_COMMAND(hash_readwrite_func_autotest, test_hash_rw_func_main);
+REGISTER_TEST_COMMAND(hash_readwrite_perf_autotest, test_hash_rw_perf_main);
-- 
1.8.3.1


^ permalink raw reply	[flat|nested] 38+ messages in thread

* [dpdk-dev] [PATCH 2/2] test/meson: hash lf test moved to dpdk perf testsuite
  2019-09-06  5:49 [dpdk-dev] [PATCH 0/2] test/meson: fix hash readwrite timeout failure agupta3
  2019-09-06  5:49 ` [dpdk-dev] [PATCH 1/2] test/meson: hash test split into shorter subtests agupta3
@ 2019-09-06  5:49 ` agupta3
  2019-09-11 17:13   ` Wang, Yipeng1
                     ` (2 more replies)
  2019-09-11  5:55 ` [dpdk-dev] [PATCH 0/2] test/meson: fix hash readwrite timeout failure Amit Gupta
  2020-02-03 19:49 ` [dpdk-dev] [PATCH v2 0/5] " Honnappa Nagarahalli
  3 siblings, 3 replies; 38+ messages in thread
From: agupta3 @ 2019-09-06  5:49 UTC (permalink / raw)
  To: yipeng1.wang, sameh.gobriel, bruce.richardson, pablo.de.lara.guarch
  Cc: dev, Amit Gupta

From: Amit Gupta <agupta3@marvell.com>

hash_readwrite_lf test always getting TIMEOUT as required
time to finish this test was much longer compare to time
required for fast tests(10s). Hence, the test is being
moved to perf test category for its execution to complete.

Signed-off-by: Amit Gupta <agupta3@marvell.com>
---
 app/test/meson.build | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/app/test/meson.build b/app/test/meson.build
index 94fd9f8..34141c5 100644
--- a/app/test/meson.build
+++ b/app/test/meson.build
@@ -220,7 +220,6 @@ fast_test_names = [
         'eventdev_common_autotest',
         'fbarray_autotest',
         'hash_readwrite_func_autotest',
-        'hash_readwrite_lf_autotest',
         'ipsec_autotest',
         'kni_autotest',
         'kvargs_autotest',
@@ -263,6 +262,7 @@ perf_test_names = [
         'stack_lf_perf_autotest',
         'rand_perf_autotest',
         'hash_readwrite_perf_autotest',
+        'hash_readwrite_lf_autotest',
 ]
 
 driver_test_names = [
-- 
1.8.3.1


^ permalink raw reply	[flat|nested] 38+ messages in thread

* Re: [dpdk-dev] [PATCH 0/2] test/meson: fix hash readwrite timeout failure
  2019-09-06  5:49 [dpdk-dev] [PATCH 0/2] test/meson: fix hash readwrite timeout failure agupta3
  2019-09-06  5:49 ` [dpdk-dev] [PATCH 1/2] test/meson: hash test split into shorter subtests agupta3
  2019-09-06  5:49 ` [dpdk-dev] [PATCH 2/2] test/meson: hash lf test moved to dpdk perf testsuite agupta3
@ 2019-09-11  5:55 ` Amit Gupta
  2020-02-03 19:49 ` [dpdk-dev] [PATCH v2 0/5] " Honnappa Nagarahalli
  3 siblings, 0 replies; 38+ messages in thread
From: Amit Gupta @ 2019-09-11  5:55 UTC (permalink / raw)
  To: yipeng1.wang, sameh.gobriel, bruce.richardson, pablo.de.lara.guarch; +Cc: dev

Ping..

-----Original Message-----
From: agupta3@marvell.com <agupta3@marvell.com> 
Sent: Friday, September 6, 2019 11:20 AM
To: yipeng1.wang@intel.com; sameh.gobriel@intel.com; bruce.richardson@intel.com; pablo.de.lara.guarch@intel.com
Cc: dev@dpdk.org; Amit Gupta <agupta3@marvell.com>
Subject: [PATCH 0/2] test/meson: fix hash readwrite timeout failure

From: Amit Gupta <agupta3@marvell.com>

V1 changes:
* hash_readwrite and hash_readwrite lockfree meson test was taking longer time to complete. The test always get TIMEOUT.
* hash readwrtie test is split into functional and perf tests and moved to dpdk fast and perf testsuite accordingly.
* hash readwrite lockfree is moved to dpdk perf testsuite.

Amit Gupta (2):
  test/meson: hash test split into shorter subtests
  test/meson: hash lf test moved to dpdk perf testsuite

 app/test/meson.build           |   5 +-
 app/test/test_hash_readwrite.c | 146 +++++++++++++++++++++++++++++++++++++++++
 2 files changed, 149 insertions(+), 2 deletions(-)

--
1.8.3.1


^ permalink raw reply	[flat|nested] 38+ messages in thread

* Re: [dpdk-dev] [PATCH 1/2] test/meson: hash test split into shorter subtests
  2019-09-06  5:49 ` [dpdk-dev] [PATCH 1/2] test/meson: hash test split into shorter subtests agupta3
@ 2019-09-11 17:05   ` Wang, Yipeng1
  2019-10-17  5:02     ` Amit Gupta
  0 siblings, 1 reply; 38+ messages in thread
From: Wang, Yipeng1 @ 2019-09-11 17:05 UTC (permalink / raw)
  To: agupta3, Gobriel, Sameh, Richardson, Bruce, De Lara Guarch, Pablo; +Cc: dev

>-----Original Message-----
>From: agupta3@marvell.com [mailto:agupta3@marvell.com]
>Sent: Thursday, September 5, 2019 10:50 PM
>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; Amit Gupta <agupta3@marvell.com>
>Subject: [PATCH 1/2] test/meson: hash test split into shorter subtests
>
>From: Amit Gupta <agupta3@marvell.com>
>
>hash_readwrite meson test was taking longer time to complete.
>The test always get TIMEOUT, hence test is split into
>functional and perf test. perf test is being moved under
>dpdk perf testsuites in  meson build.
>
>Signed-off-by: Amit Gupta <agupta3@marvell.com>
[Wang, Yipeng] 
Acked-by: Yipeng Wang <yipeng1.wang@intel.com>


Thanks for the patch!

^ permalink raw reply	[flat|nested] 38+ messages in thread

* Re: [dpdk-dev] [PATCH 2/2] test/meson: hash lf test moved to dpdk perf testsuite
  2019-09-06  5:49 ` [dpdk-dev] [PATCH 2/2] test/meson: hash lf test moved to dpdk perf testsuite agupta3
@ 2019-09-11 17:13   ` Wang, Yipeng1
  2019-09-12 15:00     ` Honnappa Nagarahalli
  2019-09-13  8:12   ` [dpdk-dev] [PATCH v2 1/1] " agupta3
  2019-09-13  8:15   ` agupta3
  2 siblings, 1 reply; 38+ messages in thread
From: Wang, Yipeng1 @ 2019-09-11 17:13 UTC (permalink / raw)
  To: agupta3, Gobriel, Sameh, Dharmik Thakkar, ruifeng.wang,
	honnappa.nagarahalli
  Cc: dev

>-----Original Message-----
>From: agupta3@marvell.com [mailto:agupta3@marvell.com]
>Sent: Thursday, September 5, 2019 10:50 PM
>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; Amit Gupta <agupta3@marvell.com>
>Subject: [PATCH 2/2] test/meson: hash lf test moved to dpdk perf testsuite
>
>From: Amit Gupta <agupta3@marvell.com>
>
>hash_readwrite_lf test always getting TIMEOUT as required
>time to finish this test was much longer compare to time
>required for fast tests(10s). Hence, the test is being
>moved to perf test category for its execution to complete.
>
>Signed-off-by: Amit Gupta <agupta3@marvell.com>
>---
> app/test/meson.build | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
>diff --git a/app/test/meson.build b/app/test/meson.build
>index 94fd9f8..34141c5 100644
>--- a/app/test/meson.build
>+++ b/app/test/meson.build
>@@ -220,7 +220,6 @@ fast_test_names = [
>         'eventdev_common_autotest',
>         'fbarray_autotest',
>         'hash_readwrite_func_autotest',
>-        'hash_readwrite_lf_autotest',
>         'ipsec_autotest',
>         'kni_autotest',
>         'kvargs_autotest',
>@@ -263,6 +262,7 @@ perf_test_names = [
>         'stack_lf_perf_autotest',
>         'rand_perf_autotest',
>         'hash_readwrite_perf_autotest',
>+        'hash_readwrite_lf_autotest',
> ]
>
> driver_test_names = [
>--
>1.8.3.1
 [Wang, Yipeng] 
I believe the lf_autotest includes functional test as well which is critical for testing the lock free implementation on non-TSO machine.
Do you think it is possible to also separate this test?
I also include the ARM folks for opinions.

Thanks!
Yipeng


^ permalink raw reply	[flat|nested] 38+ messages in thread

* Re: [dpdk-dev] [PATCH 2/2] test/meson: hash lf test moved to dpdk perf testsuite
  2019-09-11 17:13   ` Wang, Yipeng1
@ 2019-09-12 15:00     ` Honnappa Nagarahalli
  2019-09-13  8:24       ` Amit Gupta
  0 siblings, 1 reply; 38+ messages in thread
From: Honnappa Nagarahalli @ 2019-09-12 15:00 UTC (permalink / raw)
  To: Wang, Yipeng1, agupta3, Gobriel, Sameh, Dharmik Thakkar,
	Ruifeng Wang (Arm Technology China)
  Cc: dev, nd, Honnappa Nagarahalli, nd

<snip>

> >Subject: [PATCH 2/2] test/meson: hash lf test moved to dpdk perf
> >testsuite
> >
> >From: Amit Gupta <agupta3@marvell.com>
> >
> >hash_readwrite_lf test always getting TIMEOUT as required time to
> >finish this test was much longer compare to time required for fast
> >tests(10s). Hence, the test is being moved to perf test category for
> >its execution to complete.
> >
> >Signed-off-by: Amit Gupta <agupta3@marvell.com>
> >---
> > app/test/meson.build | 2 +-
> > 1 file changed, 1 insertion(+), 1 deletion(-)
> >
> >diff --git a/app/test/meson.build b/app/test/meson.build index
> >94fd9f8..34141c5 100644
> >--- a/app/test/meson.build
> >+++ b/app/test/meson.build
> >@@ -220,7 +220,6 @@ fast_test_names = [
> >         'eventdev_common_autotest',
> >         'fbarray_autotest',
> >         'hash_readwrite_func_autotest',
> >-        'hash_readwrite_lf_autotest',
> >         'ipsec_autotest',
> >         'kni_autotest',
> >         'kvargs_autotest',
> >@@ -263,6 +262,7 @@ perf_test_names = [
> >         'stack_lf_perf_autotest',
> >         'rand_perf_autotest',
> >         'hash_readwrite_perf_autotest',
> >+        'hash_readwrite_lf_autotest',
> > ]
> >
> > driver_test_names = [
> >--
> >1.8.3.1
>  [Wang, Yipeng]
> I believe the lf_autotest includes functional test as well which is critical for
> testing the lock free implementation on non-TSO machine.
> Do you think it is possible to also separate this test?
> I also include the ARM folks for opinions.
Thanks Yipeng. We combined the functional testing with performance testing. So, it is all performance tests. From that perspective, I suggest changing 'hash_readwrite_lf_autotest' to 'hash_readwrite_lf_perf_autotest'.

> 
> Thanks!
> Yipeng


^ permalink raw reply	[flat|nested] 38+ messages in thread

* [dpdk-dev] [PATCH v2 1/1] test/meson: hash lf test moved to dpdk perf testsuite
  2019-09-06  5:49 ` [dpdk-dev] [PATCH 2/2] test/meson: hash lf test moved to dpdk perf testsuite agupta3
  2019-09-11 17:13   ` Wang, Yipeng1
@ 2019-09-13  8:12   ` " agupta3
  2019-09-13 14:40     ` Aaron Conole
  2019-09-13  8:15   ` agupta3
  2 siblings, 1 reply; 38+ messages in thread
From: agupta3 @ 2019-09-13  8:12 UTC (permalink / raw)
  To: Yipeng Wang, Sameh Gobriel, Bruce Richardson, Pablo de Lara
  Cc: dev, Amit Gupta

From: Amit Gupta <agupta3@marvell.com>

hash_readwrite_lf test always getting TIMEOUT as required
time to finish this test was much longer compare to time
required for fast tests(10s). Hence, the test is being renamed
moved to perf test category for its execution to complete.

Signed-off-by: Amit Gupta <agupta3@marvell.com>
---
 app/test/meson.build              | 2 +-
 app/test/test_hash_readwrite_lf.c | 3 ++-
 2 files changed, 3 insertions(+), 2 deletions(-)

diff --git a/app/test/meson.build b/app/test/meson.build
index 94fd9f8..57d5316 100644
--- a/app/test/meson.build
+++ b/app/test/meson.build
@@ -220,7 +220,6 @@ fast_test_names = [
         'eventdev_common_autotest',
         'fbarray_autotest',
         'hash_readwrite_func_autotest',
-        'hash_readwrite_lf_autotest',
         'ipsec_autotest',
         'kni_autotest',
         'kvargs_autotest',
@@ -263,6 +262,7 @@ perf_test_names = [
         'stack_lf_perf_autotest',
         'rand_perf_autotest',
         'hash_readwrite_perf_autotest',
+        'hash_readwrite_lf_perf_autotest',
 ]
 
 driver_test_names = [
diff --git a/app/test/test_hash_readwrite_lf.c b/app/test/test_hash_readwrite_lf.c
index 1f2fba4..33d63fa 100644
--- a/app/test/test_hash_readwrite_lf.c
+++ b/app/test/test_hash_readwrite_lf.c
@@ -1431,4 +1431,5 @@ struct {
 	return 0;
 }
 
-REGISTER_TEST_COMMAND(hash_readwrite_lf_autotest, test_hash_readwrite_lf_main);
+REGISTER_TEST_COMMAND(hash_readwrite_lf_perf_autotest,
+		      test_hash_readwrite_lf_main);
-- 
1.8.3.1


^ permalink raw reply	[flat|nested] 38+ messages in thread

* [dpdk-dev] [PATCH v2 1/1] test/meson: hash lf test moved to dpdk perf testsuite
  2019-09-06  5:49 ` [dpdk-dev] [PATCH 2/2] test/meson: hash lf test moved to dpdk perf testsuite agupta3
  2019-09-11 17:13   ` Wang, Yipeng1
  2019-09-13  8:12   ` [dpdk-dev] [PATCH v2 1/1] " agupta3
@ 2019-09-13  8:15   ` agupta3
  2 siblings, 0 replies; 38+ messages in thread
From: agupta3 @ 2019-09-13  8:15 UTC (permalink / raw)
  To: Yipeng Wang, Sameh Gobriel, Bruce Richardson, Pablo de Lara
  Cc: dev, Amit Gupta

From: Amit Gupta <agupta3@marvell.com>

hash_readwrite_lf test always getting TIMEOUT as required
time to finish this test was much longer compare to time
required for fast tests(10s). Hence, the test is being renamed
moved to perf test category for its execution to complete.

Signed-off-by: Amit Gupta <agupta3@marvell.com>
---
v2 Changes:
*hash_readwrite_lf_autotest renamed to hash_readwrite_lf_pref_autotest

 app/test/meson.build              | 2 +-
 app/test/test_hash_readwrite_lf.c | 3 ++-
 2 files changed, 3 insertions(+), 2 deletions(-)

diff --git a/app/test/meson.build b/app/test/meson.build
index 94fd9f8..57d5316 100644
--- a/app/test/meson.build
+++ b/app/test/meson.build
@@ -220,7 +220,6 @@ fast_test_names = [
         'eventdev_common_autotest',
         'fbarray_autotest',
         'hash_readwrite_func_autotest',
-        'hash_readwrite_lf_autotest',
         'ipsec_autotest',
         'kni_autotest',
         'kvargs_autotest',
@@ -263,6 +262,7 @@ perf_test_names = [
         'stack_lf_perf_autotest',
         'rand_perf_autotest',
         'hash_readwrite_perf_autotest',
+        'hash_readwrite_lf_perf_autotest',
 ]
 
 driver_test_names = [
diff --git a/app/test/test_hash_readwrite_lf.c b/app/test/test_hash_readwrite_lf.c
index 1f2fba4..33d63fa 100644
--- a/app/test/test_hash_readwrite_lf.c
+++ b/app/test/test_hash_readwrite_lf.c
@@ -1431,4 +1431,5 @@ struct {
 	return 0;
 }
 
-REGISTER_TEST_COMMAND(hash_readwrite_lf_autotest, test_hash_readwrite_lf_main);
+REGISTER_TEST_COMMAND(hash_readwrite_lf_perf_autotest,
+		      test_hash_readwrite_lf_main);
-- 
1.8.3.1


^ permalink raw reply	[flat|nested] 38+ messages in thread

* Re: [dpdk-dev] [PATCH 2/2] test/meson: hash lf test moved to dpdk perf testsuite
  2019-09-12 15:00     ` Honnappa Nagarahalli
@ 2019-09-13  8:24       ` Amit Gupta
  0 siblings, 0 replies; 38+ messages in thread
From: Amit Gupta @ 2019-09-13  8:24 UTC (permalink / raw)
  To: Honnappa Nagarahalli, Wang, Yipeng1, Gobriel, Sameh,
	Dharmik Thakkar, Ruifeng Wang (Arm Technology China)
  Cc: dev, nd, nd



> -----Original Message-----
> From: Honnappa Nagarahalli <Honnappa.Nagarahalli@arm.com>
> Sent: Thursday, September 12, 2019 8:31 PM
> To: Wang, Yipeng1 <yipeng1.wang@intel.com>; Amit Gupta
> <agupta3@marvell.com>; Gobriel, Sameh <sameh.gobriel@intel.com>;
> Dharmik Thakkar <Dharmik.Thakkar@arm.com>; Ruifeng Wang (Arm
> Technology China) <Ruifeng.Wang@arm.com>
> Cc: dev@dpdk.org; nd <nd@arm.com>; Honnappa Nagarahalli
> <Honnappa.Nagarahalli@arm.com>; nd <nd@arm.com>
> Subject: [EXT] RE: [PATCH 2/2] test/meson: hash lf test moved to dpdk perf
> testsuite
> 
> External Email
> 
> ----------------------------------------------------------------------
> <snip>
> 
> > >Subject: [PATCH 2/2] test/meson: hash lf test moved to dpdk perf
> > >testsuite
> > >
> > >From: Amit Gupta <agupta3@marvell.com>
> > >
> > >hash_readwrite_lf test always getting TIMEOUT as required time to
> > >finish this test was much longer compare to time required for fast
> > >tests(10s). Hence, the test is being moved to perf test category for
> > >its execution to complete.
> > >
> > >Signed-off-by: Amit Gupta <agupta3@marvell.com>
> > >---
> > > app/test/meson.build | 2 +-
> > > 1 file changed, 1 insertion(+), 1 deletion(-)
> > >
> > >diff --git a/app/test/meson.build b/app/test/meson.build index
> > >94fd9f8..34141c5 100644
> > >--- a/app/test/meson.build
> > >+++ b/app/test/meson.build
> > >@@ -220,7 +220,6 @@ fast_test_names = [
> > >         'eventdev_common_autotest',
> > >         'fbarray_autotest',
> > >         'hash_readwrite_func_autotest',
> > >-        'hash_readwrite_lf_autotest',
> > >         'ipsec_autotest',
> > >         'kni_autotest',
> > >         'kvargs_autotest',
> > >@@ -263,6 +262,7 @@ perf_test_names = [
> > >         'stack_lf_perf_autotest',
> > >         'rand_perf_autotest',
> > >         'hash_readwrite_perf_autotest',
> > >+        'hash_readwrite_lf_autotest',
> > > ]
> > >
> > > driver_test_names = [
> > >--
> > >1.8.3.1
> >  [Wang, Yipeng]
> > I believe the lf_autotest includes functional test as well which is
> > critical for testing the lock free implementation on non-TSO machine.
> > Do you think it is possible to also separate this test?
> > I also include the ARM folks for opinions.
> Thanks Yipeng. We combined the functional testing with performance
> testing. So, it is all performance tests. From that perspective, I suggest
> changing 'hash_readwrite_lf_autotest' to
> 'hash_readwrite_lf_perf_autotest'.
Agreed, will update in v2!
> 
> >
> > Thanks!
> > Yipeng


^ permalink raw reply	[flat|nested] 38+ messages in thread

* Re: [dpdk-dev] [PATCH v2 1/1] test/meson: hash lf test moved to dpdk perf testsuite
  2019-09-13  8:12   ` [dpdk-dev] [PATCH v2 1/1] " agupta3
@ 2019-09-13 14:40     ` Aaron Conole
  2019-09-13 15:09       ` Wang, Yipeng1
  0 siblings, 1 reply; 38+ messages in thread
From: Aaron Conole @ 2019-09-13 14:40 UTC (permalink / raw)
  To: agupta3
  Cc: Yipeng Wang, Sameh Gobriel, Bruce Richardson, Pablo de Lara, dev,
	Honnappa Nagarahalli

<agupta3@marvell.com> writes:

> From: Amit Gupta <agupta3@marvell.com>
>
> hash_readwrite_lf test always getting TIMEOUT as required
> time to finish this test was much longer compare to time
> required for fast tests(10s). Hence, the test is being renamed
> moved to perf test category for its execution to complete.
>
> Signed-off-by: Amit Gupta <agupta3@marvell.com>
> ---

Okay.  I'll note that we pass the '-t 3' flag, so it is actually timing
out with 30s instead of the default 10.  We do this because occasionally
the lpm6 and table tests would also exceed the 10s timeout in the travis
environment.  I agree, it's better to pull the perf part of tests out.

I think there isn't any additional functional test in this readwrite -
is that so?  If it is, then we need to also prioritize adding back in
some of the functional testing.  Maybe I misread the lf_autotest,
though.

>  app/test/meson.build              | 2 +-
>  app/test/test_hash_readwrite_lf.c | 3 ++-
>  2 files changed, 3 insertions(+), 2 deletions(-)
>
> diff --git a/app/test/meson.build b/app/test/meson.build
> index 94fd9f8..57d5316 100644
> --- a/app/test/meson.build
> +++ b/app/test/meson.build
> @@ -220,7 +220,6 @@ fast_test_names = [
>          'eventdev_common_autotest',
>          'fbarray_autotest',
>          'hash_readwrite_func_autotest',
> -        'hash_readwrite_lf_autotest',
>          'ipsec_autotest',
>          'kni_autotest',
>          'kvargs_autotest',
> @@ -263,6 +262,7 @@ perf_test_names = [
>          'stack_lf_perf_autotest',
>          'rand_perf_autotest',
>          'hash_readwrite_perf_autotest',
> +        'hash_readwrite_lf_perf_autotest',
>  ]
>  
>  driver_test_names = [
> diff --git a/app/test/test_hash_readwrite_lf.c b/app/test/test_hash_readwrite_lf.c
> index 1f2fba4..33d63fa 100644
> --- a/app/test/test_hash_readwrite_lf.c
> +++ b/app/test/test_hash_readwrite_lf.c
> @@ -1431,4 +1431,5 @@ struct {
>  	return 0;
>  }
>  
> -REGISTER_TEST_COMMAND(hash_readwrite_lf_autotest, test_hash_readwrite_lf_main);
> +REGISTER_TEST_COMMAND(hash_readwrite_lf_perf_autotest,
> +		      test_hash_readwrite_lf_main);

^ permalink raw reply	[flat|nested] 38+ messages in thread

* Re: [dpdk-dev] [PATCH v2 1/1] test/meson: hash lf test moved to dpdk perf testsuite
  2019-09-13 14:40     ` Aaron Conole
@ 2019-09-13 15:09       ` Wang, Yipeng1
  2019-09-13 15:46         ` Honnappa Nagarahalli
  0 siblings, 1 reply; 38+ messages in thread
From: Wang, Yipeng1 @ 2019-09-13 15:09 UTC (permalink / raw)
  To: Aaron Conole, agupta3
  Cc: Gobriel, Sameh, Richardson, Bruce, De Lara Guarch, Pablo, dev,
	Honnappa Nagarahalli

> -----Original Message-----
> From: Aaron Conole [mailto:aconole@redhat.com]
> Sent: Friday, September 13, 2019 7:41 AM
> To: agupta3@marvell.com
> Cc: 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>; dev@dpdk.org; Honnappa Nagarahalli
> <Honnappa.Nagarahalli@arm.com>
> Subject: Re: [dpdk-dev] [PATCH v2 1/1] test/meson: hash lf test moved to
> dpdk perf testsuite
> 
> <agupta3@marvell.com> writes:
> 
> > From: Amit Gupta <agupta3@marvell.com>
> >
> > hash_readwrite_lf test always getting TIMEOUT as required time to
> > finish this test was much longer compare to time required for fast
> > tests(10s). Hence, the test is being renamed moved to perf test
> > category for its execution to complete.
> >
> > Signed-off-by: Amit Gupta <agupta3@marvell.com>
> > ---
> 
> Okay.  I'll note that we pass the '-t 3' flag, so it is actually timing out with 30s
> instead of the default 10.  We do this because occasionally the lpm6 and table
> tests would also exceed the 10s timeout in the travis environment.  I agree,
> it's better to pull the perf part of tests out.
> 
> I think there isn't any additional functional test in this readwrite - is that so?
> If it is, then we need to also prioritize adding back in some of the functional
> testing.  Maybe I misread the lf_autotest, though.
> 
[Wang, Yipeng] 
Yes that is my concern too, if we just move all the lock-free test into perf test then we miss
the functional test.
Would any of you like to consider adding a small functional test into the readwrite or readwrite_lf_functional?
That would be great :)



^ permalink raw reply	[flat|nested] 38+ messages in thread

* Re: [dpdk-dev] [PATCH v2 1/1] test/meson: hash lf test moved to dpdk perf testsuite
  2019-09-13 15:09       ` Wang, Yipeng1
@ 2019-09-13 15:46         ` Honnappa Nagarahalli
  2019-09-16  4:39           ` Amit Gupta
  2019-10-17  4:57           ` Amit Gupta
  0 siblings, 2 replies; 38+ messages in thread
From: Honnappa Nagarahalli @ 2019-09-13 15:46 UTC (permalink / raw)
  To: Wang, Yipeng1, Aaron Conole, agupta3
  Cc: Gobriel, Sameh, Richardson, Bruce, De Lara Guarch, Pablo, dev,
	Honnappa Nagarahalli, nd, nd

<snip>

> >
> > <agupta3@marvell.com> writes:
> >
> > > From: Amit Gupta <agupta3@marvell.com>
> > >
> > > hash_readwrite_lf test always getting TIMEOUT as required time to
> > > finish this test was much longer compare to time required for fast
> > > tests(10s). Hence, the test is being renamed moved to perf test
> > > category for its execution to complete.
> > >
> > > Signed-off-by: Amit Gupta <agupta3@marvell.com>
> > > ---
> >
> > Okay.  I'll note that we pass the '-t 3' flag, so it is actually
> > timing out with 30s instead of the default 10.  We do this because
> > occasionally the lpm6 and table tests would also exceed the 10s
> > timeout in the travis environment.  I agree, it's better to pull the perf part
> of tests out.
> >
> > I think there isn't any additional functional test in this readwrite - is that so?
> > If it is, then we need to also prioritize adding back in some of the
> > functional testing.  Maybe I misread the lf_autotest, though.
> >
> [Wang, Yipeng]
> Yes that is my concern too, if we just move all the lock-free test into perf test
> then we miss the functional test.
> Would any of you like to consider adding a small functional test into the
> readwrite or readwrite_lf_functional?
> That would be great :)
Yes, I will take up for readwrite_lf_functional. But, I do not have much bandwidth for 19.11. I suggest we move only part of the tests to perf tests instead for 19.11, this would serve both the purposes.

Amit, would it be possible to check what tests will run within the timeout period?
> 


^ permalink raw reply	[flat|nested] 38+ messages in thread

* Re: [dpdk-dev] [PATCH v2 1/1] test/meson: hash lf test moved to dpdk perf testsuite
  2019-09-13 15:46         ` Honnappa Nagarahalli
@ 2019-09-16  4:39           ` Amit Gupta
  2019-10-17  4:57           ` Amit Gupta
  1 sibling, 0 replies; 38+ messages in thread
From: Amit Gupta @ 2019-09-16  4:39 UTC (permalink / raw)
  To: Honnappa Nagarahalli, Wang, Yipeng1, Aaron Conole
  Cc: Gobriel, Sameh, Richardson, Bruce, De Lara Guarch, Pablo, dev, nd, nd



> -----Original Message-----
> From: Honnappa Nagarahalli <Honnappa.Nagarahalli@arm.com>
> Sent: Friday, September 13, 2019 9:17 PM
> To: Wang, Yipeng1 <yipeng1.wang@intel.com>; Aaron Conole
> <aconole@redhat.com>; Amit Gupta <agupta3@marvell.com>
> Cc: Gobriel, Sameh <sameh.gobriel@intel.com>; Richardson, Bruce
> <bruce.richardson@intel.com>; De Lara Guarch, Pablo
> <pablo.de.lara.guarch@intel.com>; dev@dpdk.org; Honnappa Nagarahalli
> <Honnappa.Nagarahalli@arm.com>; nd <nd@arm.com>; nd <nd@arm.com>
> Subject: [EXT] RE: [dpdk-dev] [PATCH v2 1/1] test/meson: hash lf test moved
> to dpdk perf testsuite
> 
> External Email
> 
> ----------------------------------------------------------------------
> <snip>
> 
> > >
> > > <agupta3@marvell.com> writes:
> > >
> > > > From: Amit Gupta <agupta3@marvell.com>
> > > >
> > > > hash_readwrite_lf test always getting TIMEOUT as required time to
> > > > finish this test was much longer compare to time required for fast
> > > > tests(10s). Hence, the test is being renamed moved to perf test
> > > > category for its execution to complete.
> > > >
> > > > Signed-off-by: Amit Gupta <agupta3@marvell.com>
> > > > ---
> > >
> > > Okay.  I'll note that we pass the '-t 3' flag, so it is actually
> > > timing out with 30s instead of the default 10.  We do this because
> > > occasionally the lpm6 and table tests would also exceed the 10s
> > > timeout in the travis environment.  I agree, it's better to pull the
> > > perf part
> > of tests out.
> > >
> > > I think there isn't any additional functional test in this readwrite - is that
> so?
> > > If it is, then we need to also prioritize adding back in some of the
> > > functional testing.  Maybe I misread the lf_autotest, though.
> > >
> > [Wang, Yipeng]
> > Yes that is my concern too, if we just move all the lock-free test
> > into perf test then we miss the functional test.
> > Would any of you like to consider adding a small functional test into
> > the readwrite or readwrite_lf_functional?
> > That would be great :)
> Yes, I will take up for readwrite_lf_functional. But, I do not have much
> bandwidth for 19.11. I suggest we move only part of the tests to perf tests
> instead for 19.11, this would serve both the purposes.
> 
> Amit, would it be possible to check what tests will run within the timeout
> period?
> >
Not even a single subtest of hash_readwrite_lf runs within the timeout period of 10s. 
generate_key logic itself is taking ~8s to generate all required keys.

^ permalink raw reply	[flat|nested] 38+ messages in thread

* Re: [dpdk-dev] [PATCH v2 1/1] test/meson: hash lf test moved to dpdk perf testsuite
  2019-09-13 15:46         ` Honnappa Nagarahalli
  2019-09-16  4:39           ` Amit Gupta
@ 2019-10-17  4:57           ` Amit Gupta
  2019-10-17 13:16             ` Aaron Conole
  1 sibling, 1 reply; 38+ messages in thread
From: Amit Gupta @ 2019-10-17  4:57 UTC (permalink / raw)
  To: Honnappa Nagarahalli, Wang, Yipeng1, Aaron Conole
  Cc: Gobriel, Sameh, Richardson, Bruce, De Lara Guarch, Pablo, dev, nd, nd



> -----Original Message-----
> From: Honnappa Nagarahalli <Honnappa.Nagarahalli@arm.com>
> Sent: Friday, September 13, 2019 9:17 PM
> To: Wang, Yipeng1 <yipeng1.wang@intel.com>; Aaron Conole
> <aconole@redhat.com>; Amit Gupta <agupta3@marvell.com>
> Cc: Gobriel, Sameh <sameh.gobriel@intel.com>; Richardson, Bruce
> <bruce.richardson@intel.com>; De Lara Guarch, Pablo
> <pablo.de.lara.guarch@intel.com>; dev@dpdk.org; Honnappa Nagarahalli
> <Honnappa.Nagarahalli@arm.com>; nd <nd@arm.com>; nd <nd@arm.com>
> Subject: [EXT] RE: [dpdk-dev] [PATCH v2 1/1] test/meson: hash lf test moved
> to dpdk perf testsuite
> 
> External Email
> 
> ----------------------------------------------------------------------
> <snip>
> 
> > >
> > > <agupta3@marvell.com> writes:
> > >
> > > > From: Amit Gupta <agupta3@marvell.com>
> > > >
> > > > hash_readwrite_lf test always getting TIMEOUT as required time to
> > > > finish this test was much longer compare to time required for fast
> > > > tests(10s). Hence, the test is being renamed moved to perf test
> > > > category for its execution to complete.
> > > >
> > > > Signed-off-by: Amit Gupta <agupta3@marvell.com>
> > > > ---
> > >
> > > Okay.  I'll note that we pass the '-t 3' flag, so it is actually
> > > timing out with 30s instead of the default 10.  We do this because
> > > occasionally the lpm6 and table tests would also exceed the 10s
> > > timeout in the travis environment.  I agree, it's better to pull the
> > > perf part
> > of tests out.
> > >
> > > I think there isn't any additional functional test in this readwrite - is that
> so?
> > > If it is, then we need to also prioritize adding back in some of the
> > > functional testing.  Maybe I misread the lf_autotest, though.
> > >
> > [Wang, Yipeng]
> > Yes that is my concern too, if we just move all the lock-free test
> > into perf test then we miss the functional test.
> > Would any of you like to consider adding a small functional test into
> > the readwrite or readwrite_lf_functional?
> > That would be great :)
> Yes, I will take up for readwrite_lf_functional. But, I do not have much
> bandwidth for 19.11. I suggest we move only part of the tests to perf tests
> instead for 19.11, this would serve both the purposes.
> 
> Amit, would it be possible to check what tests will run within the timeout
> period?
> >
@Wang, Yipeng1, is it good if we do the change as @Honnappa Nagarahalli suggestion of changing 'hash_readwrite_lf_autotest' to 'hash_readwrite_lf_perf_autotest' for the time being and later once have sufficient bandwidth we can move only perf part of the test to perf tests.





^ permalink raw reply	[flat|nested] 38+ messages in thread

* Re: [dpdk-dev] [PATCH 1/2] test/meson: hash test split into shorter subtests
  2019-09-11 17:05   ` Wang, Yipeng1
@ 2019-10-17  5:02     ` Amit Gupta
  2019-11-01  4:54       ` Amit Gupta
  0 siblings, 1 reply; 38+ messages in thread
From: Amit Gupta @ 2019-10-17  5:02 UTC (permalink / raw)
  To: Wang, Yipeng1, Gobriel, Sameh, Richardson, Bruce, De Lara Guarch, Pablo
  Cc: dev



> -----Original Message-----
> From: Wang, Yipeng1 <yipeng1.wang@intel.com>
> Sent: Wednesday, September 11, 2019 10:35 PM
> To: Amit Gupta <agupta3@marvell.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
> Subject: [EXT] RE: [PATCH 1/2] test/meson: hash test split into shorter
> subtests
> 
> External Email
> 
> ----------------------------------------------------------------------
> >-----Original Message-----
> >From: agupta3@marvell.com [mailto:agupta3@marvell.com]
> >Sent: Thursday, September 5, 2019 10:50 PM
> >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; Amit Gupta <agupta3@marvell.com>
> >Subject: [PATCH 1/2] test/meson: hash test split into shorter subtests
> >
> >From: Amit Gupta <agupta3@marvell.com>
> >
> >hash_readwrite meson test was taking longer time to complete.
> >The test always get TIMEOUT, hence test is split into functional and
> >perf test. perf test is being moved under dpdk perf testsuites in
> >meson build.
> >
> >Signed-off-by: Amit Gupta <agupta3@marvell.com>
> [Wang, Yipeng]
> Acked-by: Yipeng Wang <yipeng1.wang@intel.com>
> 
> 
> Thanks for the patch!

@Wang, Yipeng1, any plan on taking this patch ?


Regards,
Amit

^ permalink raw reply	[flat|nested] 38+ messages in thread

* Re: [dpdk-dev] [PATCH v2 1/1] test/meson: hash lf test moved to dpdk perf testsuite
  2019-10-17  4:57           ` Amit Gupta
@ 2019-10-17 13:16             ` Aaron Conole
  2019-10-24  7:22               ` David Marchand
  0 siblings, 1 reply; 38+ messages in thread
From: Aaron Conole @ 2019-10-17 13:16 UTC (permalink / raw)
  To: Amit Gupta
  Cc: Honnappa Nagarahalli, Wang\, Yipeng1, Gobriel\,
	Sameh, Richardson\, Bruce, De Lara Guarch\, Pablo, dev\,
	nd

Amit Gupta <agupta3@marvell.com> writes:

>> -----Original Message-----
>> From: Honnappa Nagarahalli <Honnappa.Nagarahalli@arm.com>
>> Sent: Friday, September 13, 2019 9:17 PM
>> To: Wang, Yipeng1 <yipeng1.wang@intel.com>; Aaron Conole
>> <aconole@redhat.com>; Amit Gupta <agupta3@marvell.com>
>> Cc: Gobriel, Sameh <sameh.gobriel@intel.com>; Richardson, Bruce
>> <bruce.richardson@intel.com>; De Lara Guarch, Pablo
>> <pablo.de.lara.guarch@intel.com>; dev@dpdk.org; Honnappa Nagarahalli
>> <Honnappa.Nagarahalli@arm.com>; nd <nd@arm.com>; nd <nd@arm.com>
>> Subject: [EXT] RE: [dpdk-dev] [PATCH v2 1/1] test/meson: hash lf test moved
>> to dpdk perf testsuite
>> 
>> External Email
>> 
>> ----------------------------------------------------------------------
>> <snip>
>> 
>> > >
>> > > <agupta3@marvell.com> writes:
>> > >
>> > > > From: Amit Gupta <agupta3@marvell.com>
>> > > >
>> > > > hash_readwrite_lf test always getting TIMEOUT as required time to
>> > > > finish this test was much longer compare to time required for fast
>> > > > tests(10s). Hence, the test is being renamed moved to perf test
>> > > > category for its execution to complete.
>> > > >
>> > > > Signed-off-by: Amit Gupta <agupta3@marvell.com>
>> > > > ---
>> > >
>> > > Okay.  I'll note that we pass the '-t 3' flag, so it is actually
>> > > timing out with 30s instead of the default 10.  We do this because
>> > > occasionally the lpm6 and table tests would also exceed the 10s
>> > > timeout in the travis environment.  I agree, it's better to pull the
>> > > perf part
>> > of tests out.
>> > >
>> > > I think there isn't any additional functional test in this readwrite - is that
>> so?
>> > > If it is, then we need to also prioritize adding back in some of the
>> > > functional testing.  Maybe I misread the lf_autotest, though.
>> > >
>> > [Wang, Yipeng]
>> > Yes that is my concern too, if we just move all the lock-free test
>> > into perf test then we miss the functional test.
>> > Would any of you like to consider adding a small functional test into
>> > the readwrite or readwrite_lf_functional?
>> > That would be great :)
>> Yes, I will take up for readwrite_lf_functional. But, I do not have much
>> bandwidth for 19.11. I suggest we move only part of the tests to perf tests
>> instead for 19.11, this would serve both the purposes.
>> 
>> Amit, would it be possible to check what tests will run within the timeout
>> period?
>> >
> @Wang, Yipeng1, is it good if we do the change as @Honnappa
> Nagarahalli suggestion of changing 'hash_readwrite_lf_autotest' to
> 'hash_readwrite_lf_perf_autotest' for the time being and later once
> have sufficient bandwidth we can move only perf part of the test to
> perf tests.

NAK.

I don't like that proposal.  While it's true that there are occasional
TIMEOUT failures with the current setup, I'd much prefer these timeouts
(which we can easily distinguish) to removing the test from the travis
chain.  My understanding is that there *are* some functionality being
exercised by this test that isn't exercised elsewhere.  I'd prefer we
don't sacrifice the coverage.

^ permalink raw reply	[flat|nested] 38+ messages in thread

* Re: [dpdk-dev] [PATCH v2 1/1] test/meson: hash lf test moved to dpdk perf testsuite
  2019-10-17 13:16             ` Aaron Conole
@ 2019-10-24  7:22               ` David Marchand
  0 siblings, 0 replies; 38+ messages in thread
From: David Marchand @ 2019-10-24  7:22 UTC (permalink / raw)
  To: Amit Gupta
  Cc: Honnappa Nagarahalli, Wang, Yipeng1, Gobriel, Sameh, Richardson,
	Bruce, De Lara Guarch, Pablo, dev, nd, Aaron Conole

On Thu, Oct 17, 2019 at 3:16 PM Aaron Conole <aconole@redhat.com> wrote:
>
> Amit Gupta <agupta3@marvell.com> writes:
>
> >> -----Original Message-----
> >> From: Honnappa Nagarahalli <Honnappa.Nagarahalli@arm.com>
> >> Sent: Friday, September 13, 2019 9:17 PM
> >> To: Wang, Yipeng1 <yipeng1.wang@intel.com>; Aaron Conole
> >> <aconole@redhat.com>; Amit Gupta <agupta3@marvell.com>
> >> Cc: Gobriel, Sameh <sameh.gobriel@intel.com>; Richardson, Bruce
> >> <bruce.richardson@intel.com>; De Lara Guarch, Pablo
> >> <pablo.de.lara.guarch@intel.com>; dev@dpdk.org; Honnappa Nagarahalli
> >> <Honnappa.Nagarahalli@arm.com>; nd <nd@arm.com>; nd <nd@arm.com>
> >> Subject: [EXT] RE: [dpdk-dev] [PATCH v2 1/1] test/meson: hash lf test moved
> >> to dpdk perf testsuite
> >>
> >> External Email
> >>
> >> ----------------------------------------------------------------------
> >> <snip>
> >>
> >> > >
> >> > > <agupta3@marvell.com> writes:
> >> > >
> >> > > > From: Amit Gupta <agupta3@marvell.com>
> >> > > >
> >> > > > hash_readwrite_lf test always getting TIMEOUT as required time to
> >> > > > finish this test was much longer compare to time required for fast
> >> > > > tests(10s). Hence, the test is being renamed moved to perf test
> >> > > > category for its execution to complete.
> >> > > >
> >> > > > Signed-off-by: Amit Gupta <agupta3@marvell.com>
> >> > > > ---
> >> > >
> >> > > Okay.  I'll note that we pass the '-t 3' flag, so it is actually
> >> > > timing out with 30s instead of the default 10.  We do this because
> >> > > occasionally the lpm6 and table tests would also exceed the 10s
> >> > > timeout in the travis environment.  I agree, it's better to pull the
> >> > > perf part
> >> > of tests out.
> >> > >
> >> > > I think there isn't any additional functional test in this readwrite - is that
> >> so?
> >> > > If it is, then we need to also prioritize adding back in some of the
> >> > > functional testing.  Maybe I misread the lf_autotest, though.
> >> > >
> >> > [Wang, Yipeng]
> >> > Yes that is my concern too, if we just move all the lock-free test
> >> > into perf test then we miss the functional test.
> >> > Would any of you like to consider adding a small functional test into
> >> > the readwrite or readwrite_lf_functional?
> >> > That would be great :)
> >> Yes, I will take up for readwrite_lf_functional. But, I do not have much
> >> bandwidth for 19.11. I suggest we move only part of the tests to perf tests
> >> instead for 19.11, this would serve both the purposes.
> >>
> >> Amit, would it be possible to check what tests will run within the timeout
> >> period?
> >> >
> > @Wang, Yipeng1, is it good if we do the change as @Honnappa
> > Nagarahalli suggestion of changing 'hash_readwrite_lf_autotest' to
> > 'hash_readwrite_lf_perf_autotest' for the time being and later once
> > have sufficient bandwidth we can move only perf part of the test to
> > perf tests.
>
> NAK.
>
> I don't like that proposal.  While it's true that there are occasional
> TIMEOUT failures with the current setup, I'd much prefer these timeouts
> (which we can easily distinguish) to removing the test from the travis
> chain.  My understanding is that there *are* some functionality being
> exercised by this test that isn't exercised elsewhere.  I'd prefer we
> don't sacrifice the coverage.

+1 and marking this patch as rejected.

On a sidenote, Amit, please be careful about the versioning of your
patches and update their status in patchwork.
I had two patches named the same with one marked as NEW (but no
comment on it) and this current thread patch marked as SUPERSEDED.

Thanks.

-- 
David Marchand

^ permalink raw reply	[flat|nested] 38+ messages in thread

* Re: [dpdk-dev] [PATCH 1/2] test/meson: hash test split into shorter subtests
  2019-10-17  5:02     ` Amit Gupta
@ 2019-11-01  4:54       ` Amit Gupta
  2019-11-01 17:04         ` Wang, Yipeng1
  0 siblings, 1 reply; 38+ messages in thread
From: Amit Gupta @ 2019-11-01  4:54 UTC (permalink / raw)
  To: Wang, Yipeng1, Gobriel, Sameh, Richardson, Bruce, De Lara Guarch,
	Pablo, Jerin Jacob Kollanukkaran
  Cc: dev

Ping!

> -----Original Message-----
> From: Amit Gupta
> Sent: Thursday, October 17, 2019 10:33 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
> Subject: RE: [PATCH 1/2] test/meson: hash test split into shorter subtests
> 
> 
> 
> > -----Original Message-----
> > From: Wang, Yipeng1 <yipeng1.wang@intel.com>
> > Sent: Wednesday, September 11, 2019 10:35 PM
> > To: Amit Gupta <agupta3@marvell.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
> > Subject: [EXT] RE: [PATCH 1/2] test/meson: hash test split into
> > shorter subtests
> >
> > External Email
> >
> > ----------------------------------------------------------------------
> > >-----Original Message-----
> > >From: agupta3@marvell.com [mailto:agupta3@marvell.com]
> > >Sent: Thursday, September 5, 2019 10:50 PM
> > >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; Amit Gupta <agupta3@marvell.com>
> > >Subject: [PATCH 1/2] test/meson: hash test split into shorter
> > >subtests
> > >
> > >From: Amit Gupta <agupta3@marvell.com>
> > >
> > >hash_readwrite meson test was taking longer time to complete.
> > >The test always get TIMEOUT, hence test is split into functional and
> > >perf test. perf test is being moved under dpdk perf testsuites in
> > >meson build.
> > >
> > >Signed-off-by: Amit Gupta <agupta3@marvell.com>
> > [Wang, Yipeng]
> > Acked-by: Yipeng Wang <yipeng1.wang@intel.com>
> >
> >
> > Thanks for the patch!
> 
> @Wang, Yipeng1, any plan on taking this patch ?
> 
> 
> Regards,
> Amit

^ permalink raw reply	[flat|nested] 38+ messages in thread

* Re: [dpdk-dev] [PATCH 1/2] test/meson: hash test split into shorter subtests
  2019-11-01  4:54       ` Amit Gupta
@ 2019-11-01 17:04         ` Wang, Yipeng1
  2019-11-05 16:37           ` Ferruh Yigit
  0 siblings, 1 reply; 38+ messages in thread
From: Wang, Yipeng1 @ 2019-11-01 17:04 UTC (permalink / raw)
  To: Amit Gupta, Gobriel, Sameh, Thomas Monjalon; +Cc: dev

Hi, Amit,

I think I acked this patch. But from patchwork seems you superseded this patch set accidentally. So Thomas might have missed it. 
To make his life easier, you may submit a newer version with my acked. I believe Thomas will see it. 

Thanks
Yipeng

>-----Original Message-----
>From: Amit Gupta [mailto:agupta3@marvell.com]
>Sent: Thursday, October 31, 2019 9:54 PM
>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>; Jerin Jacob Kollanukkaran
><jerinj@marvell.com>
>Cc: dev@dpdk.org
>Subject: RE: [PATCH 1/2] test/meson: hash test split into shorter subtests
>
>Ping!
>
>> -----Original Message-----
>> From: Amit Gupta
>> Sent: Thursday, October 17, 2019 10:33 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
>> Subject: RE: [PATCH 1/2] test/meson: hash test split into shorter subtests
>>
>>
>>
>> > -----Original Message-----
>> > From: Wang, Yipeng1 <yipeng1.wang@intel.com>
>> > Sent: Wednesday, September 11, 2019 10:35 PM
>> > To: Amit Gupta <agupta3@marvell.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
>> > Subject: [EXT] RE: [PATCH 1/2] test/meson: hash test split into
>> > shorter subtests
>> >
>> > External Email
>> >
>> > ----------------------------------------------------------------------
>> > >-----Original Message-----
>> > >From: agupta3@marvell.com [mailto:agupta3@marvell.com]
>> > >Sent: Thursday, September 5, 2019 10:50 PM
>> > >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; Amit Gupta <agupta3@marvell.com>
>> > >Subject: [PATCH 1/2] test/meson: hash test split into shorter
>> > >subtests
>> > >
>> > >From: Amit Gupta <agupta3@marvell.com>
>> > >
>> > >hash_readwrite meson test was taking longer time to complete.
>> > >The test always get TIMEOUT, hence test is split into functional and
>> > >perf test. perf test is being moved under dpdk perf testsuites in
>> > >meson build.
>> > >
>> > >Signed-off-by: Amit Gupta <agupta3@marvell.com>
>> > [Wang, Yipeng]
>> > Acked-by: Yipeng Wang <yipeng1.wang@intel.com>
>> >
>> >
>> > Thanks for the patch!
>>
>> @Wang, Yipeng1, any plan on taking this patch ?
>>
>>
>> Regards,
>> Amit

^ permalink raw reply	[flat|nested] 38+ messages in thread

* Re: [dpdk-dev] [PATCH 1/2] test/meson: hash test split into shorter subtests
  2019-11-01 17:04         ` Wang, Yipeng1
@ 2019-11-05 16:37           ` Ferruh Yigit
  2019-11-07  3:32             ` [dpdk-dev] [EXT] " Amit Gupta
  0 siblings, 1 reply; 38+ messages in thread
From: Ferruh Yigit @ 2019-11-05 16:37 UTC (permalink / raw)
  To: Wang, Yipeng1, Amit Gupta, Gobriel, Sameh, Thomas Monjalon
  Cc: dev, David Marchand

On 11/1/2019 5:04 PM, Wang, Yipeng1 wrote:
> Hi, Amit,
> 
> I think I acked this patch. But from patchwork seems you superseded this patch set accidentally. So Thomas might have missed it. 
> To make his life easier, you may submit a newer version with my acked. I believe Thomas will see it. 

cc'ed David.

Hi Amit,

The patch is in "Superseded" state as Yipeng said, as far as I understand that
is not the case, I am updating their status as "New". Can you please confirm
this is the correct state?

> 
> Thanks
> Yipeng
> 
>> -----Original Message-----
>> From: Amit Gupta [mailto:agupta3@marvell.com]
>> Sent: Thursday, October 31, 2019 9:54 PM
>> 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>; Jerin Jacob Kollanukkaran
>> <jerinj@marvell.com>
>> Cc: dev@dpdk.org
>> Subject: RE: [PATCH 1/2] test/meson: hash test split into shorter subtests
>>
>> Ping!
>>
>>> -----Original Message-----
>>> From: Amit Gupta
>>> Sent: Thursday, October 17, 2019 10:33 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
>>> Subject: RE: [PATCH 1/2] test/meson: hash test split into shorter subtests
>>>
>>>
>>>
>>>> -----Original Message-----
>>>> From: Wang, Yipeng1 <yipeng1.wang@intel.com>
>>>> Sent: Wednesday, September 11, 2019 10:35 PM
>>>> To: Amit Gupta <agupta3@marvell.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
>>>> Subject: [EXT] RE: [PATCH 1/2] test/meson: hash test split into
>>>> shorter subtests
>>>>
>>>> External Email
>>>>
>>>> ----------------------------------------------------------------------
>>>>> -----Original Message-----
>>>>> From: agupta3@marvell.com [mailto:agupta3@marvell.com]
>>>>> Sent: Thursday, September 5, 2019 10:50 PM
>>>>> 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; Amit Gupta <agupta3@marvell.com>
>>>>> Subject: [PATCH 1/2] test/meson: hash test split into shorter
>>>>> subtests
>>>>>
>>>>> From: Amit Gupta <agupta3@marvell.com>
>>>>>
>>>>> hash_readwrite meson test was taking longer time to complete.
>>>>> The test always get TIMEOUT, hence test is split into functional and
>>>>> perf test. perf test is being moved under dpdk perf testsuites in
>>>>> meson build.
>>>>>
>>>>> Signed-off-by: Amit Gupta <agupta3@marvell.com>
>>>> [Wang, Yipeng]
>>>> Acked-by: Yipeng Wang <yipeng1.wang@intel.com>
>>>>
>>>>
>>>> Thanks for the patch!
>>>
>>> @Wang, Yipeng1, any plan on taking this patch ?
>>>
>>>
>>> Regards,
>>> Amit


^ permalink raw reply	[flat|nested] 38+ messages in thread

* Re: [dpdk-dev] [EXT] Re: [PATCH 1/2] test/meson: hash test split into shorter subtests
  2019-11-05 16:37           ` Ferruh Yigit
@ 2019-11-07  3:32             ` " Amit Gupta
  2019-12-31  4:56               ` Amit Gupta
  0 siblings, 1 reply; 38+ messages in thread
From: Amit Gupta @ 2019-11-07  3:32 UTC (permalink / raw)
  To: Ferruh Yigit, Wang, Yipeng1, Gobriel, Sameh, Thomas Monjalon
  Cc: dev, David Marchand

Hi Yipeng,

Thanks for changing the state to 'New'. Yes this patch was wrongly Superseded.


Regards,
Amit

> -----Original Message-----
> From: Ferruh Yigit <ferruh.yigit@intel.com>
> Sent: Tuesday, November 5, 2019 10:07 PM
> To: Wang, Yipeng1 <yipeng1.wang@intel.com>; Amit Gupta
> <agupta3@marvell.com>; Gobriel, Sameh <sameh.gobriel@intel.com>;
> Thomas Monjalon <thomas@monjalon.net>
> Cc: dev@dpdk.org; David Marchand <david.marchand@redhat.com>
> Subject: [EXT] Re: [dpdk-dev] [PATCH 1/2] test/meson: hash test split into
> shorter subtests
> 
> External Email
> 
> ----------------------------------------------------------------------
> On 11/1/2019 5:04 PM, Wang, Yipeng1 wrote:
> > Hi, Amit,
> >
> > I think I acked this patch. But from patchwork seems you superseded this
> patch set accidentally. So Thomas might have missed it.
> > To make his life easier, you may submit a newer version with my acked. I
> believe Thomas will see it.
> 
> cc'ed David.
> 
> Hi Amit,
> 
> The patch is in "Superseded" state as Yipeng said, as far as I understand that
> is not the case, I am updating their status as "New". Can you please confirm
> this is the correct state?
> 
> >
> > Thanks
> > Yipeng
> >
> >> -----Original Message-----
> >> From: Amit Gupta [mailto:agupta3@marvell.com]
> >> Sent: Thursday, October 31, 2019 9:54 PM
> >> 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>; Jerin Jacob Kollanukkaran
> >> <jerinj@marvell.com>
> >> Cc: dev@dpdk.org
> >> Subject: RE: [PATCH 1/2] test/meson: hash test split into shorter
> >> subtests
> >>
> >> Ping!
> >>
> >>> -----Original Message-----
> >>> From: Amit Gupta
> >>> Sent: Thursday, October 17, 2019 10:33 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
> >>> Subject: RE: [PATCH 1/2] test/meson: hash test split into shorter
> >>> subtests
> >>>
> >>>
> >>>
> >>>> -----Original Message-----
> >>>> From: Wang, Yipeng1 <yipeng1.wang@intel.com>
> >>>> Sent: Wednesday, September 11, 2019 10:35 PM
> >>>> To: Amit Gupta <agupta3@marvell.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
> >>>> Subject: [EXT] RE: [PATCH 1/2] test/meson: hash test split into
> >>>> shorter subtests
> >>>>
> >>>> External Email
> >>>>
> >>>> -------------------------------------------------------------------
> >>>> ---
> >>>>> -----Original Message-----
> >>>>> From: agupta3@marvell.com [mailto:agupta3@marvell.com]
> >>>>> Sent: Thursday, September 5, 2019 10:50 PM
> >>>>> 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; Amit Gupta <agupta3@marvell.com>
> >>>>> Subject: [PATCH 1/2] test/meson: hash test split into shorter
> >>>>> subtests
> >>>>>
> >>>>> From: Amit Gupta <agupta3@marvell.com>
> >>>>>
> >>>>> hash_readwrite meson test was taking longer time to complete.
> >>>>> The test always get TIMEOUT, hence test is split into functional
> >>>>> and perf test. perf test is being moved under dpdk perf testsuites
> >>>>> in meson build.
> >>>>>
> >>>>> Signed-off-by: Amit Gupta <agupta3@marvell.com>
> >>>> [Wang, Yipeng]
> >>>> Acked-by: Yipeng Wang <yipeng1.wang@intel.com>
> >>>>
> >>>>
> >>>> Thanks for the patch!
> >>>
> >>> @Wang, Yipeng1, any plan on taking this patch ?
> >>>
> >>>
> >>> Regards,
> >>> Amit


^ permalink raw reply	[flat|nested] 38+ messages in thread

* Re: [dpdk-dev] [EXT] Re: [PATCH 1/2] test/meson: hash test split into shorter subtests
  2019-11-07  3:32             ` [dpdk-dev] [EXT] " Amit Gupta
@ 2019-12-31  4:56               ` Amit Gupta
  0 siblings, 0 replies; 38+ messages in thread
From: Amit Gupta @ 2019-12-31  4:56 UTC (permalink / raw)
  To: Ferruh Yigit, Wang, Yipeng1, Gobriel, Sameh, Thomas Monjalon
  Cc: dev, David Marchand

Ping!

Any plan to merge this change ?


Regards,
Amit

> -----Original Message-----
> From: Amit Gupta
> Sent: Thursday, November 7, 2019 9:03 AM
> To: Ferruh Yigit <ferruh.yigit@intel.com>; Wang, Yipeng1
> <yipeng1.wang@intel.com>; Gobriel, Sameh <sameh.gobriel@intel.com>;
> Thomas Monjalon <thomas@monjalon.net>
> Cc: dev@dpdk.org; David Marchand <david.marchand@redhat.com>
> Subject: RE: [EXT] Re: [dpdk-dev] [PATCH 1/2] test/meson: hash test split
> into shorter subtests
> 
> Hi Yipeng,
> 
> Thanks for changing the state to 'New'. Yes this patch was wrongly
> Superseded.
> 
> 
> Regards,
> Amit
> 
> > -----Original Message-----
> > From: Ferruh Yigit <ferruh.yigit@intel.com>
> > Sent: Tuesday, November 5, 2019 10:07 PM
> > To: Wang, Yipeng1 <yipeng1.wang@intel.com>; Amit Gupta
> > <agupta3@marvell.com>; Gobriel, Sameh <sameh.gobriel@intel.com>;
> > Thomas Monjalon <thomas@monjalon.net>
> > Cc: dev@dpdk.org; David Marchand <david.marchand@redhat.com>
> > Subject: [EXT] Re: [dpdk-dev] [PATCH 1/2] test/meson: hash test split
> > into shorter subtests
> >
> > External Email
> >
> > ----------------------------------------------------------------------
> > On 11/1/2019 5:04 PM, Wang, Yipeng1 wrote:
> > > Hi, Amit,
> > >
> > > I think I acked this patch. But from patchwork seems you superseded
> > > this
> > patch set accidentally. So Thomas might have missed it.
> > > To make his life easier, you may submit a newer version with my
> > > acked. I
> > believe Thomas will see it.
> >
> > cc'ed David.
> >
> > Hi Amit,
> >
> > The patch is in "Superseded" state as Yipeng said, as far as I
> > understand that is not the case, I am updating their status as "New".
> > Can you please confirm this is the correct state?
> >
> > >
> > > Thanks
> > > Yipeng
> > >
> > >> -----Original Message-----
> > >> From: Amit Gupta [mailto:agupta3@marvell.com]
> > >> Sent: Thursday, October 31, 2019 9:54 PM
> > >> 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>; Jerin Jacob Kollanukkaran
> > >> <jerinj@marvell.com>
> > >> Cc: dev@dpdk.org
> > >> Subject: RE: [PATCH 1/2] test/meson: hash test split into shorter
> > >> subtests
> > >>
> > >> Ping!
> > >>
> > >>> -----Original Message-----
> > >>> From: Amit Gupta
> > >>> Sent: Thursday, October 17, 2019 10:33 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
> > >>> Subject: RE: [PATCH 1/2] test/meson: hash test split into shorter
> > >>> subtests
> > >>>
> > >>>
> > >>>
> > >>>> -----Original Message-----
> > >>>> From: Wang, Yipeng1 <yipeng1.wang@intel.com>
> > >>>> Sent: Wednesday, September 11, 2019 10:35 PM
> > >>>> To: Amit Gupta <agupta3@marvell.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
> > >>>> Subject: [EXT] RE: [PATCH 1/2] test/meson: hash test split into
> > >>>> shorter subtests
> > >>>>
> > >>>> External Email
> > >>>>
> > >>>> -----------------------------------------------------------------
> > >>>> --
> > >>>> ---
> > >>>>> -----Original Message-----
> > >>>>> From: agupta3@marvell.com [mailto:agupta3@marvell.com]
> > >>>>> Sent: Thursday, September 5, 2019 10:50 PM
> > >>>>> 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; Amit Gupta <agupta3@marvell.com>
> > >>>>> Subject: [PATCH 1/2] test/meson: hash test split into shorter
> > >>>>> subtests
> > >>>>>
> > >>>>> From: Amit Gupta <agupta3@marvell.com>
> > >>>>>
> > >>>>> hash_readwrite meson test was taking longer time to complete.
> > >>>>> The test always get TIMEOUT, hence test is split into functional
> > >>>>> and perf test. perf test is being moved under dpdk perf
> > >>>>> testsuites in meson build.
> > >>>>>
> > >>>>> Signed-off-by: Amit Gupta <agupta3@marvell.com>
> > >>>> [Wang, Yipeng]
> > >>>> Acked-by: Yipeng Wang <yipeng1.wang@intel.com>
> > >>>>
> > >>>>
> > >>>> Thanks for the patch!
> > >>>
> > >>> @Wang, Yipeng1, any plan on taking this patch ?
> > >>>
> > >>>
> > >>> Regards,
> > >>> Amit


^ permalink raw reply	[flat|nested] 38+ messages in thread

* [dpdk-dev] [PATCH v2 0/5] test/meson: fix hash readwrite timeout failure
  2019-09-06  5:49 [dpdk-dev] [PATCH 0/2] test/meson: fix hash readwrite timeout failure agupta3
                   ` (2 preceding siblings ...)
  2019-09-11  5:55 ` [dpdk-dev] [PATCH 0/2] test/meson: fix hash readwrite timeout failure Amit Gupta
@ 2020-02-03 19:49 ` " Honnappa Nagarahalli
  2020-02-03 19:49   ` [dpdk-dev] [PATCH v2 1/5] test/meson: hash test split into shorter subtests Honnappa Nagarahalli
                     ` (5 more replies)
  3 siblings, 6 replies; 38+ messages in thread
From: Honnappa Nagarahalli @ 2020-02-03 19:49 UTC (permalink / raw)
  To: agupta3, yipeng1.wang, sameh.gobriel, honnappa.nagarahalli
  Cc: thomas, david.marchand, dev, nd

V2 changes:
- Remove duplicated code in the existing patch
- Add lock-free option to hash read-write functional test cases
- Move existing loack-free test cases to performance tests
- A typo fix in the comments for lock-free extendable table feature

V1 changes:
- hash_readwrite and hash_readwrite lockfree meson test was
  taking longer time to complete. The test always get TIMEOUT.
- hash readwrtie test is split into functional and perf tests 
  and moved to dpdk fast and perf testsuite accordingly.
- hash readwrite lockfree is moved to dpdk perf testsuite.

Amit Gupta (1):
  test/meson: hash test split into shorter subtests

Honnappa Nagarahalli (4):
  test/hash: remove duplicated test code
  test/hash: add lock free reader writer functional tests
  test/hash: move reader writer lock free tests to perf tests
  hash: correct lock free extendable table support

 app/test/Makefile                             |   2 +-
 app/test/autotest_data.py                     |  14 ++-
 app/test/meson.build                          |   7 +-
 app/test/test_hash_readwrite.c                | 117 +++++++++++++-----
 ...ite_lf.c => test_hash_readwrite_lf_perf.c} |   7 +-
 lib/librte_hash/rte_hash.h                    |   2 -
 6 files changed, 107 insertions(+), 42 deletions(-)
 rename app/test/{test_hash_readwrite_lf.c => test_hash_readwrite_lf_perf.c} (99%)

-- 
2.17.1


^ permalink raw reply	[flat|nested] 38+ messages in thread

* [dpdk-dev] [PATCH v2 1/5] test/meson: hash test split into shorter subtests
  2020-02-03 19:49 ` [dpdk-dev] [PATCH v2 0/5] " Honnappa Nagarahalli
@ 2020-02-03 19:49   ` Honnappa Nagarahalli
  2020-02-03 19:49   ` [dpdk-dev] [PATCH v2 2/5] test/hash: remove duplicated test code Honnappa Nagarahalli
                     ` (4 subsequent siblings)
  5 siblings, 0 replies; 38+ messages in thread
From: Honnappa Nagarahalli @ 2020-02-03 19:49 UTC (permalink / raw)
  To: agupta3, yipeng1.wang, sameh.gobriel, honnappa.nagarahalli
  Cc: thomas, david.marchand, dev, nd

From: Amit Gupta <agupta3@marvell.com>

hash_readwrite meson test was taking longer time to complete.
The test always get TIMEOUT, hence test is split into
functional and perf test. perf test is being moved under
dpdk perf testsuites in  meson build.

Signed-off-by: Amit Gupta <agupta3@marvell.com>
Acked-by: Yipeng Wang <yipeng1.wang@intel.com>
---
 app/test/meson.build           |   3 +-
 app/test/test_hash_readwrite.c | 146 +++++++++++++++++++++++++++++++++
 2 files changed, 148 insertions(+), 1 deletion(-)

diff --git a/app/test/meson.build b/app/test/meson.build
index 22b0cefaa..08c0ecb3f 100644
--- a/app/test/meson.build
+++ b/app/test/meson.build
@@ -233,7 +233,7 @@ fast_test_names = [
         'distributor_autotest',
         'eventdev_common_autotest',
         'fbarray_autotest',
-        'hash_readwrite_autotest',
+        'hash_readwrite_func_autotest',
         'hash_readwrite_lf_autotest',
         'ipsec_autotest',
         'kni_autotest',
@@ -282,6 +282,7 @@ perf_test_names = [
         'stack_perf_autotest',
         'stack_lf_perf_autotest',
         'rand_perf_autotest',
+        'hash_readwrite_perf_autotest',
 ]
 
 driver_test_names = [
diff --git a/app/test/test_hash_readwrite.c b/app/test/test_hash_readwrite.c
index 615767fb6..aa55db7fe 100644
--- a/app/test/test_hash_readwrite.c
+++ b/app/test/test_hash_readwrite.c
@@ -605,6 +605,150 @@ test_hash_readwrite_perf(struct perf *perf_results, int use_htm,
 	return -1;
 }
 
+static int
+test_hash_rw_perf_main(void)
+{
+	/*
+	 * Variables used to choose different tests.
+	 * use_htm indicates if hardware transactional memory should be used.
+	 * reader_faster indicates if the reader threads should finish earlier
+	 * than writer threads. This is to timing either reader threads or
+	 * writer threads for performance numbers.
+	 */
+	int use_htm, reader_faster;
+	unsigned int i = 0, core_id = 0;
+
+	if (rte_lcore_count() < 3) {
+		printf("Not enough cores for hash_readwrite_autotest, expecting at least 3\n");
+		return TEST_SKIPPED;
+	}
+
+	RTE_LCORE_FOREACH_SLAVE(core_id) {
+		slave_core_ids[i] = core_id;
+		i++;
+	}
+
+	setlocale(LC_NUMERIC, "");
+
+	if (rte_tm_supported()) {
+		printf("Hardware transactional memory (lock elision) "
+			"is supported\n");
+
+		printf("Test read-write with Hardware transactional memory\n");
+
+		use_htm = 1;
+
+		reader_faster = 1;
+		if (test_hash_readwrite_perf(&htm_results, use_htm,
+							reader_faster) < 0)
+			return -1;
+
+		reader_faster = 0;
+		if (test_hash_readwrite_perf(&htm_results, use_htm,
+							reader_faster) < 0)
+			return -1;
+	} else {
+		printf("Hardware transactional memory (lock elision) "
+			"is NOT supported\n");
+	}
+
+	printf("Test read-write without Hardware transactional memory\n");
+	use_htm = 0;
+
+	reader_faster = 1;
+	if (test_hash_readwrite_perf(&non_htm_results, use_htm,
+							reader_faster) < 0)
+		return -1;
+	reader_faster = 0;
+	if (test_hash_readwrite_perf(&non_htm_results, use_htm,
+							reader_faster) < 0)
+		return -1;
+
+	printf("================\n");
+	printf("Results summary:\n");
+	printf("================\n");
+
+	printf("single read: %u\n", htm_results.single_read);
+	printf("single write: %u\n", htm_results.single_write);
+	for (i = 0; i < NUM_TEST; i++) {
+		printf("+++ core_cnt: %u +++\n", core_cnt[i]);
+		printf("HTM:\n");
+		printf("  read only: %u\n", htm_results.read_only[i]);
+		printf("  write only: %u\n", htm_results.write_only[i]);
+		printf("  read-write read: %u\n", htm_results.read_write_r[i]);
+		printf("  read-write write: %u\n", htm_results.read_write_w[i]);
+
+		printf("non HTM:\n");
+		printf("  read only: %u\n", non_htm_results.read_only[i]);
+		printf("  write only: %u\n", non_htm_results.write_only[i]);
+		printf("  read-write read: %u\n",
+			non_htm_results.read_write_r[i]);
+		printf("  read-write write: %u\n",
+			non_htm_results.read_write_w[i]);
+	}
+
+	return 0;
+}
+
+static int
+test_hash_rw_func_main(void)
+{
+	/*
+	 * Variables used to choose different tests.
+	 * use_htm indicates if hardware transactional memory should be used.
+	 * reader_faster indicates if the reader threads should finish earlier
+	 * than writer threads. This is to timing either reader threads or
+	 * writer threads for performance numbers.
+	 */
+	int use_htm, use_ext;
+	unsigned int i = 0, core_id = 0;
+
+	if (rte_lcore_count() < 3) {
+		printf("Not enough cores for hash_readwrite_autotest, expecting at least 3\n");
+		return TEST_SKIPPED;
+	}
+
+	RTE_LCORE_FOREACH_SLAVE(core_id) {
+		slave_core_ids[i] = core_id;
+		i++;
+	}
+
+	setlocale(LC_NUMERIC, "");
+
+	if (rte_tm_supported()) {
+		printf("Hardware transactional memory (lock elision) "
+			"is supported\n");
+
+		printf("Test read-write with Hardware transactional memory\n");
+
+		use_htm = 1;
+		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;
+
+	} else {
+		printf("Hardware transactional memory (lock elision) "
+			"is NOT supported\n");
+	}
+
+	printf("Test read-write without Hardware transactional memory\n");
+	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;
+
+	return 0;
+}
+
 static int
 test_hash_readwrite_main(void)
 {
@@ -706,3 +850,5 @@ test_hash_readwrite_main(void)
 }
 
 REGISTER_TEST_COMMAND(hash_readwrite_autotest, test_hash_readwrite_main);
+REGISTER_TEST_COMMAND(hash_readwrite_func_autotest, test_hash_rw_func_main);
+REGISTER_TEST_COMMAND(hash_readwrite_perf_autotest, test_hash_rw_perf_main);
-- 
2.17.1


^ permalink raw reply	[flat|nested] 38+ messages in thread

* [dpdk-dev] [PATCH v2 2/5] test/hash: remove duplicated test code
  2020-02-03 19:49 ` [dpdk-dev] [PATCH v2 0/5] " Honnappa Nagarahalli
  2020-02-03 19:49   ` [dpdk-dev] [PATCH v2 1/5] test/meson: hash test split into shorter subtests Honnappa Nagarahalli
@ 2020-02-03 19:49   ` Honnappa Nagarahalli
  2020-02-05  8:48     ` David Marchand
  2020-02-03 19:49   ` [dpdk-dev] [PATCH v2 3/5] test/hash: add lock free reader writer functional tests Honnappa Nagarahalli
                     ` (3 subsequent siblings)
  5 siblings, 1 reply; 38+ messages in thread
From: Honnappa Nagarahalli @ 2020-02-03 19:49 UTC (permalink / raw)
  To: agupta3, yipeng1.wang, sameh.gobriel, honnappa.nagarahalli
  Cc: thomas, david.marchand, dev, nd

The test case target 'hash_readwrite_autotest' is covered by
'hash_readwrite_func_autotest' and 'hash_readwrite_perf_autotest'.
Hence, it is removed along with its test code.

Signed-off-by: Honnappa Nagarahalli <honnappa.nagarahalli@arm.com>
---
 app/test/autotest_data.py      |  10 +++-
 app/test/test_hash_readwrite.c | 101 ---------------------------------
 2 files changed, 8 insertions(+), 103 deletions(-)

diff --git a/app/test/autotest_data.py b/app/test/autotest_data.py
index 6deb97bcc..71db4b3f6 100644
--- a/app/test/autotest_data.py
+++ b/app/test/autotest_data.py
@@ -664,8 +664,14 @@
         "Report":  None,
     },
     {
-        "Name":    "Hash read-write concurrency autotest",
-        "Command": "hash_readwrite_autotest",
+        "Name":    "Hash read-write concurrency functional autotest",
+        "Command": "hash_readwrite_func_autotest",
+        "Func":    default_autotest,
+        "Report":  None,
+    },
+    {
+        "Name":    "Hash read-write concurrency perf autotest",
+        "Command": "hash_readwrite_perf_autotest",
         "Func":    default_autotest,
         "Report":  None,
     },
diff --git a/app/test/test_hash_readwrite.c b/app/test/test_hash_readwrite.c
index aa55db7fe..635ed5a9f 100644
--- a/app/test/test_hash_readwrite.c
+++ b/app/test/test_hash_readwrite.c
@@ -749,106 +749,5 @@ test_hash_rw_func_main(void)
 	return 0;
 }
 
-static int
-test_hash_readwrite_main(void)
-{
-	/*
-	 * Variables used to choose different tests.
-	 * use_htm indicates if hardware transactional memory should be used.
-	 * reader_faster indicates if the reader threads should finish earlier
-	 * than writer threads. This is to timing either reader threads or
-	 * writer threads for performance numbers.
-	 */
-	int use_htm, use_ext,  reader_faster;
-	unsigned int i = 0, core_id = 0;
-
-	if (rte_lcore_count() < 3) {
-		printf("Not enough cores for hash_readwrite_autotest, expecting at least 3\n");
-		return TEST_SKIPPED;
-	}
-
-	RTE_LCORE_FOREACH_SLAVE(core_id) {
-		slave_core_ids[i] = core_id;
-		i++;
-	}
-
-	setlocale(LC_NUMERIC, "");
-
-	if (rte_tm_supported()) {
-		printf("Hardware transactional memory (lock elision) "
-			"is supported\n");
-
-		printf("Test read-write with Hardware transactional memory\n");
-
-		use_htm = 1;
-		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(&htm_results, use_htm,
-							reader_faster) < 0)
-			return -1;
-
-		reader_faster = 0;
-		if (test_hash_readwrite_perf(&htm_results, use_htm,
-							reader_faster) < 0)
-			return -1;
-	} else {
-		printf("Hardware transactional memory (lock elision) "
-			"is NOT supported\n");
-	}
-
-	printf("Test read-write without Hardware transactional memory\n");
-	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)
-		return -1;
-	reader_faster = 0;
-	if (test_hash_readwrite_perf(&non_htm_results, use_htm,
-							reader_faster) < 0)
-		return -1;
-
-	printf("================\n");
-	printf("Results summary:\n");
-	printf("================\n");
-
-	printf("single read: %u\n", htm_results.single_read);
-	printf("single write: %u\n", htm_results.single_write);
-	for (i = 0; i < NUM_TEST; i++) {
-		printf("+++ core_cnt: %u +++\n", core_cnt[i]);
-		printf("HTM:\n");
-		printf("  read only: %u\n", htm_results.read_only[i]);
-		printf("  write only: %u\n", htm_results.write_only[i]);
-		printf("  read-write read: %u\n", htm_results.read_write_r[i]);
-		printf("  read-write write: %u\n", htm_results.read_write_w[i]);
-
-		printf("non HTM:\n");
-		printf("  read only: %u\n", non_htm_results.read_only[i]);
-		printf("  write only: %u\n", non_htm_results.write_only[i]);
-		printf("  read-write read: %u\n",
-			non_htm_results.read_write_r[i]);
-		printf("  read-write write: %u\n",
-			non_htm_results.read_write_w[i]);
-	}
-
-	return 0;
-}
-
-REGISTER_TEST_COMMAND(hash_readwrite_autotest, test_hash_readwrite_main);
 REGISTER_TEST_COMMAND(hash_readwrite_func_autotest, test_hash_rw_func_main);
 REGISTER_TEST_COMMAND(hash_readwrite_perf_autotest, test_hash_rw_perf_main);
-- 
2.17.1


^ permalink raw reply	[flat|nested] 38+ messages in thread

* [dpdk-dev] [PATCH v2 3/5] test/hash: add lock free reader writer functional tests
  2020-02-03 19:49 ` [dpdk-dev] [PATCH v2 0/5] " Honnappa Nagarahalli
  2020-02-03 19:49   ` [dpdk-dev] [PATCH v2 1/5] test/meson: hash test split into shorter subtests Honnappa Nagarahalli
  2020-02-03 19:49   ` [dpdk-dev] [PATCH v2 2/5] test/hash: remove duplicated test code Honnappa Nagarahalli
@ 2020-02-03 19:49   ` Honnappa Nagarahalli
  2020-02-05  9:07     ` David Marchand
  2020-02-03 19:49   ` [dpdk-dev] [PATCH v2 4/5] test/hash: move reader writer lock free tests to perf tests Honnappa Nagarahalli
                     ` (2 subsequent siblings)
  5 siblings, 1 reply; 38+ messages in thread
From: Honnappa Nagarahalli @ 2020-02-03 19:49 UTC (permalink / raw)
  To: agupta3, yipeng1.wang, sameh.gobriel, honnappa.nagarahalli
  Cc: thomas, david.marchand, dev, nd

Add lock-free reader writer concurrency functional tests.
These tests will provide the same coverage that non lock-free
APIs have.

Signed-off-by: Honnappa Nagarahalli <honnappa.nagarahalli@arm.com>
---
 app/test/test_hash_readwrite.c | 58 +++++++++++++++++++++-------------
 1 file changed, 36 insertions(+), 22 deletions(-)

diff --git a/app/test/test_hash_readwrite.c b/app/test/test_hash_readwrite.c
index 635ed5a9f..a9429091c 100644
--- a/app/test/test_hash_readwrite.c
+++ b/app/test/test_hash_readwrite.c
@@ -121,7 +121,7 @@ test_hash_readwrite_worker(__attribute__((unused)) void *arg)
 }
 
 static int
-init_params(int use_ext, int use_htm, int use_jhash)
+init_params(int use_ext, int use_htm, int rw_lf, int use_jhash)
 {
 	unsigned int i;
 
@@ -140,15 +140,16 @@ init_params(int use_ext, int use_htm, int use_jhash)
 	else
 		hash_params.hash_func = rte_hash_crc;
 
+	hash_params.extra_flag = RTE_HASH_EXTRA_FLAGS_MULTI_WRITER_ADD;
 	if (use_htm)
-		hash_params.extra_flag =
-			RTE_HASH_EXTRA_FLAGS_TRANS_MEM_SUPPORT |
-			RTE_HASH_EXTRA_FLAGS_RW_CONCURRENCY |
-			RTE_HASH_EXTRA_FLAGS_MULTI_WRITER_ADD;
+		hash_params.extra_flag |=
+			RTE_HASH_EXTRA_FLAGS_TRANS_MEM_SUPPORT;
+	if (rw_lf)
+		hash_params.extra_flag |=
+			RTE_HASH_EXTRA_FLAGS_RW_CONCURRENCY_LF;
 	else
-		hash_params.extra_flag =
-			RTE_HASH_EXTRA_FLAGS_RW_CONCURRENCY |
-			RTE_HASH_EXTRA_FLAGS_MULTI_WRITER_ADD;
+		hash_params.extra_flag |=
+			RTE_HASH_EXTRA_FLAGS_RW_CONCURRENCY;
 
 	if (use_ext)
 		hash_params.extra_flag |=
@@ -195,7 +196,7 @@ init_params(int use_ext, int use_htm, int use_jhash)
 }
 
 static int
-test_hash_readwrite_functional(int use_ext, int use_htm)
+test_hash_readwrite_functional(int use_htm, int use_rw_lf, int use_ext)
 {
 	unsigned int i;
 	const void *next_key;
@@ -214,7 +215,7 @@ test_hash_readwrite_functional(int use_ext, int use_htm)
 	rte_atomic64_init(&ginsertions);
 	rte_atomic64_clear(&ginsertions);
 
-	if (init_params(use_ext, use_htm, use_jhash) != 0)
+	if (init_params(use_ext, use_htm, use_rw_lf, use_jhash) != 0)
 		goto err;
 
 	if (use_ext)
@@ -229,6 +230,8 @@ test_hash_readwrite_functional(int use_ext, int use_htm)
 		tbl_rw_test_param.num_insert
 		* slave_cnt;
 
+	printf("\nHTM = %d, RW-LF = %d, EXT-Table = %d\n",
+		use_htm, use_rw_lf, use_ext);
 	printf("++++++++Start function tests:+++++++++\n");
 
 	/* Fire all threads. */
@@ -379,7 +382,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(0, use_htm, use_jhash) != 0)
+	if (init_params(0, use_htm, 0, use_jhash) != 0)
 		goto err;
 
 	/*
@@ -700,7 +703,6 @@ test_hash_rw_func_main(void)
 	 * than writer threads. This is to timing either reader threads or
 	 * writer threads for performance numbers.
 	 */
-	int use_htm, use_ext;
 	unsigned int i = 0, core_id = 0;
 
 	if (rte_lcore_count() < 3) {
@@ -721,29 +723,41 @@ test_hash_rw_func_main(void)
 
 		printf("Test read-write with Hardware transactional memory\n");
 
-		use_htm = 1;
-		use_ext = 0;
+		/* htm = 1, rw_lf = 0, ext = 0 */
+		if (test_hash_readwrite_functional(1, 0, 0) < 0)
+			return -1;
 
-		if (test_hash_readwrite_functional(use_ext, use_htm) < 0)
+		/* htm = 1, rw_lf = 1, ext = 0 */
+		if (test_hash_readwrite_functional(1, 1, 0) < 0)
 			return -1;
 
-		use_ext = 1;
-		if (test_hash_readwrite_functional(use_ext, use_htm) < 0)
+		/* htm = 1, rw_lf = 0, ext = 1 */
+		if (test_hash_readwrite_functional(1, 0, 1) < 0)
 			return -1;
 
+		/* htm = 1, rw_lf = 1, ext = 1 */
+		if (test_hash_readwrite_functional(1, 1, 1) < 0)
+			return -1;
 	} else {
 		printf("Hardware transactional memory (lock elision) "
 			"is NOT supported\n");
 	}
 
 	printf("Test read-write without Hardware transactional memory\n");
-	use_htm = 0;
-	use_ext = 0;
-	if (test_hash_readwrite_functional(use_ext, use_htm) < 0)
+	/* htm = 0, rw_lf = 0, ext = 0 */
+	if (test_hash_readwrite_functional(0, 0, 0) < 0)
+		return -1;
+
+	/* htm = 0, rw_lf = 1, ext = 0 */
+	if (test_hash_readwrite_functional(0, 1, 0) < 0)
+		return -1;
+
+	/* htm = 0, rw_lf = 0, ext = 1 */
+	if (test_hash_readwrite_functional(0, 0, 1) < 0)
 		return -1;
 
-	use_ext = 1;
-	if (test_hash_readwrite_functional(use_ext, use_htm) < 0)
+	/* htm = 0, rw_lf = 1, ext = 1 */
+	if (test_hash_readwrite_functional(0, 1, 1) < 0)
 		return -1;
 
 	return 0;
-- 
2.17.1


^ permalink raw reply	[flat|nested] 38+ messages in thread

* [dpdk-dev] [PATCH v2 4/5] test/hash: move reader writer lock free tests to perf tests
  2020-02-03 19:49 ` [dpdk-dev] [PATCH v2 0/5] " Honnappa Nagarahalli
                     ` (2 preceding siblings ...)
  2020-02-03 19:49   ` [dpdk-dev] [PATCH v2 3/5] test/hash: add lock free reader writer functional tests Honnappa Nagarahalli
@ 2020-02-03 19:49   ` Honnappa Nagarahalli
  2020-02-03 19:49   ` [dpdk-dev] [PATCH v2 5/5] hash: correct lock free extendable table support Honnappa Nagarahalli
  2020-02-05 18:41   ` [dpdk-dev] [PATCH v2 0/5] test/meson: fix hash readwrite timeout failure David Marchand
  5 siblings, 0 replies; 38+ messages in thread
From: Honnappa Nagarahalli @ 2020-02-03 19:49 UTC (permalink / raw)
  To: agupta3, yipeng1.wang, sameh.gobriel, honnappa.nagarahalli
  Cc: thomas, david.marchand, dev, nd

Move reader writer lock free tests to performance tests.

Signed-off-by: Honnappa Nagarahalli <honnappa.nagarahalli@arm.com>
---
 app/test/Makefile                                          | 2 +-
 app/test/autotest_data.py                                  | 4 ++--
 app/test/meson.build                                       | 4 ++--
 ...t_hash_readwrite_lf.c => test_hash_readwrite_lf_perf.c} | 7 ++++---
 4 files changed, 9 insertions(+), 8 deletions(-)
 rename app/test/{test_hash_readwrite_lf.c => test_hash_readwrite_lf_perf.c} (99%)

diff --git a/app/test/Makefile b/app/test/Makefile
index 57930c00b..d955dbb03 100644
--- a/app/test/Makefile
+++ b/app/test/Makefile
@@ -122,7 +122,7 @@ 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_multiwriter.c
 SRCS-$(CONFIG_RTE_LIBRTE_HASH) += test_hash_readwrite.c
-SRCS-$(CONFIG_RTE_LIBRTE_HASH) += test_hash_readwrite_lf.c
+SRCS-$(CONFIG_RTE_LIBRTE_HASH) += test_hash_readwrite_lf_perf.c
 
 SRCS-$(CONFIG_RTE_LIBRTE_RIB) += test_rib.c
 SRCS-$(CONFIG_RTE_LIBRTE_RIB) += test_rib6.c
diff --git a/app/test/autotest_data.py b/app/test/autotest_data.py
index 71db4b3f6..7b1d01389 100644
--- a/app/test/autotest_data.py
+++ b/app/test/autotest_data.py
@@ -676,8 +676,8 @@
         "Report":  None,
     },
     {
-        "Name":    "Hash read-write lock-free concurrency autotest",
-        "Command": "hash_readwrite_lf_autotest",
+        "Name":    "Hash read-write lock-free concurrency perf autotest",
+        "Command": "hash_readwrite_lf_perf_autotest",
         "Func":    default_autotest,
         "Report":  None,
     },
diff --git a/app/test/meson.build b/app/test/meson.build
index 08c0ecb3f..07cf675a4 100644
--- a/app/test/meson.build
+++ b/app/test/meson.build
@@ -59,7 +59,7 @@ test_sources = files('commands.c',
 	'test_hash_multiwriter.c',
 	'test_hash_readwrite.c',
 	'test_hash_perf.c',
-	'test_hash_readwrite_lf.c',
+	'test_hash_readwrite_lf_perf.c',
 	'test_interrupts.c',
 	'test_ipsec.c',
 	'test_ipsec_sad.c',
@@ -234,7 +234,6 @@ fast_test_names = [
         'eventdev_common_autotest',
         'fbarray_autotest',
         'hash_readwrite_func_autotest',
-        'hash_readwrite_lf_autotest',
         'ipsec_autotest',
         'kni_autotest',
         'kvargs_autotest',
@@ -283,6 +282,7 @@ perf_test_names = [
         'stack_lf_perf_autotest',
         'rand_perf_autotest',
         'hash_readwrite_perf_autotest',
+        'hash_readwrite_lf_perf_autotest'
 ]
 
 driver_test_names = [
diff --git a/app/test/test_hash_readwrite_lf.c b/app/test/test_hash_readwrite_lf_perf.c
similarity index 99%
rename from app/test/test_hash_readwrite_lf.c
rename to app/test/test_hash_readwrite_lf_perf.c
index 97c304054..a7547bea0 100644
--- a/app/test/test_hash_readwrite_lf.c
+++ b/app/test/test_hash_readwrite_lf_perf.c
@@ -1241,7 +1241,7 @@ test_hash_add_ks_lookup_hit_extbkt(struct rwc_perf *rwc_perf_results,
 }
 
 static int
-test_hash_readwrite_lf_main(void)
+test_hash_readwrite_lf_perf_main(void)
 {
 	/*
 	 * Variables used to choose different tests.
@@ -1254,7 +1254,7 @@ test_hash_readwrite_lf_main(void)
 	int ext_bkt = 0;
 
 	if (rte_lcore_count() < 2) {
-		printf("Not enough cores for hash_readwrite_lf_autotest, expecting at least 2\n");
+		printf("Not enough cores for hash_readwrite_lf_perf_autotest, expecting at least 2\n");
 		return TEST_SKIPPED;
 	}
 
@@ -1431,4 +1431,5 @@ test_hash_readwrite_lf_main(void)
 	return 0;
 }
 
-REGISTER_TEST_COMMAND(hash_readwrite_lf_autotest, test_hash_readwrite_lf_main);
+REGISTER_TEST_COMMAND(hash_readwrite_lf_perf_autotest,
+				test_hash_readwrite_lf_perf_main);
-- 
2.17.1


^ permalink raw reply	[flat|nested] 38+ messages in thread

* [dpdk-dev] [PATCH v2 5/5] hash: correct lock free extendable table support
  2020-02-03 19:49 ` [dpdk-dev] [PATCH v2 0/5] " Honnappa Nagarahalli
                     ` (3 preceding siblings ...)
  2020-02-03 19:49   ` [dpdk-dev] [PATCH v2 4/5] test/hash: move reader writer lock free tests to perf tests Honnappa Nagarahalli
@ 2020-02-03 19:49   ` Honnappa Nagarahalli
  2020-02-05 18:41   ` [dpdk-dev] [PATCH v2 0/5] test/meson: fix hash readwrite timeout failure David Marchand
  5 siblings, 0 replies; 38+ messages in thread
From: Honnappa Nagarahalli @ 2020-02-03 19:49 UTC (permalink / raw)
  To: agupta3, yipeng1.wang, sameh.gobriel, honnappa.nagarahalli
  Cc: thomas, david.marchand, dev, nd, stable

Lock-free extendable table is supported. Correct the comments.

Fixes: f401363d984a ("hash: support lock-free extendable bucket")
Cc: stable@dpdk.org

Signed-off-by: Honnappa Nagarahalli <honnappa.nagarahalli@arm.com>
---
 lib/librte_hash/rte_hash.h | 2 --
 1 file changed, 2 deletions(-)

diff --git a/lib/librte_hash/rte_hash.h b/lib/librte_hash/rte_hash.h
index 063701173..ed0673b73 100644
--- a/lib/librte_hash/rte_hash.h
+++ b/lib/librte_hash/rte_hash.h
@@ -51,8 +51,6 @@ extern "C" {
 
 /** Flag to support lock free reader writer concurrency. Both single writer
  * and multi writer use cases are supported.
- * Currently, extendable bucket table feature is not supported with
- * this feature.
  */
 #define RTE_HASH_EXTRA_FLAGS_RW_CONCURRENCY_LF 0x20
 
-- 
2.17.1


^ permalink raw reply	[flat|nested] 38+ messages in thread

* Re: [dpdk-dev] [PATCH v2 2/5] test/hash: remove duplicated test code
  2020-02-03 19:49   ` [dpdk-dev] [PATCH v2 2/5] test/hash: remove duplicated test code Honnappa Nagarahalli
@ 2020-02-05  8:48     ` David Marchand
  2020-02-05 16:42       ` David Marchand
  0 siblings, 1 reply; 38+ messages in thread
From: David Marchand @ 2020-02-05  8:48 UTC (permalink / raw)
  To: ci, Thomas Monjalon
  Cc: Amit Gupta, Wang, Yipeng1, Gobriel, Sameh, dev, nd, Mcnamara,
	John, Yigit, Ferruh, Honnappa Nagarahalli

On Mon, Feb 3, 2020 at 8:49 PM Honnappa Nagarahalli
<honnappa.nagarahalli@arm.com> wrote:
>
> The test case target 'hash_readwrite_autotest' is covered by
> 'hash_readwrite_func_autotest' and 'hash_readwrite_perf_autotest'.
> Hence, it is removed along with its test code.
>
> Signed-off-by: Honnappa Nagarahalli <honnappa.nagarahalli@arm.com>
> ---
>  app/test/autotest_data.py      |  10 +++-
>  app/test/test_hash_readwrite.c | 101 ---------------------------------
>  2 files changed, 8 insertions(+), 103 deletions(-)
>
> diff --git a/app/test/autotest_data.py b/app/test/autotest_data.py
> index 6deb97bcc..71db4b3f6 100644
> --- a/app/test/autotest_data.py
> +++ b/app/test/autotest_data.py
> @@ -664,8 +664,14 @@
>          "Report":  None,
>      },
>      {
> -        "Name":    "Hash read-write concurrency autotest",
> -        "Command": "hash_readwrite_autotest",
> +        "Name":    "Hash read-write concurrency functional autotest",
> +        "Command": "hash_readwrite_func_autotest",
> +        "Func":    default_autotest,
> +        "Report":  None,
> +    },
> +    {
> +        "Name":    "Hash read-write concurrency perf autotest",
> +        "Command": "hash_readwrite_perf_autotest",
>          "Func":    default_autotest,
>          "Report":  None,
>      },
> diff --git a/app/test/test_hash_readwrite.c b/app/test/test_hash_readwrite.c
> index aa55db7fe..635ed5a9f 100644
> --- a/app/test/test_hash_readwrite.c
> +++ b/app/test/test_hash_readwrite.c
> @@ -749,106 +749,5 @@ test_hash_rw_func_main(void)
>         return 0;
>  }
>
> -static int
> -test_hash_readwrite_main(void)
> -{
> -       /*
> -        * Variables used to choose different tests.
> -        * use_htm indicates if hardware transactional memory should be used.
> -        * reader_faster indicates if the reader threads should finish earlier
> -        * than writer threads. This is to timing either reader threads or
> -        * writer threads for performance numbers.
> -        */
> -       int use_htm, use_ext,  reader_faster;
> -       unsigned int i = 0, core_id = 0;
> -
> -       if (rte_lcore_count() < 3) {
> -               printf("Not enough cores for hash_readwrite_autotest, expecting at least 3\n");
> -               return TEST_SKIPPED;
> -       }
> -
> -       RTE_LCORE_FOREACH_SLAVE(core_id) {
> -               slave_core_ids[i] = core_id;
> -               i++;
> -       }
> -
> -       setlocale(LC_NUMERIC, "");
> -
> -       if (rte_tm_supported()) {
> -               printf("Hardware transactional memory (lock elision) "
> -                       "is supported\n");
> -
> -               printf("Test read-write with Hardware transactional memory\n");
> -
> -               use_htm = 1;
> -               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(&htm_results, use_htm,
> -                                                       reader_faster) < 0)
> -                       return -1;
> -
> -               reader_faster = 0;
> -               if (test_hash_readwrite_perf(&htm_results, use_htm,
> -                                                       reader_faster) < 0)
> -                       return -1;
> -       } else {
> -               printf("Hardware transactional memory (lock elision) "
> -                       "is NOT supported\n");
> -       }
> -
> -       printf("Test read-write without Hardware transactional memory\n");
> -       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)
> -               return -1;
> -       reader_faster = 0;
> -       if (test_hash_readwrite_perf(&non_htm_results, use_htm,
> -                                                       reader_faster) < 0)
> -               return -1;
> -
> -       printf("================\n");
> -       printf("Results summary:\n");
> -       printf("================\n");
> -
> -       printf("single read: %u\n", htm_results.single_read);
> -       printf("single write: %u\n", htm_results.single_write);
> -       for (i = 0; i < NUM_TEST; i++) {
> -               printf("+++ core_cnt: %u +++\n", core_cnt[i]);
> -               printf("HTM:\n");
> -               printf("  read only: %u\n", htm_results.read_only[i]);
> -               printf("  write only: %u\n", htm_results.write_only[i]);
> -               printf("  read-write read: %u\n", htm_results.read_write_r[i]);
> -               printf("  read-write write: %u\n", htm_results.read_write_w[i]);
> -
> -               printf("non HTM:\n");
> -               printf("  read only: %u\n", non_htm_results.read_only[i]);
> -               printf("  write only: %u\n", non_htm_results.write_only[i]);
> -               printf("  read-write read: %u\n",
> -                       non_htm_results.read_write_r[i]);
> -               printf("  read-write write: %u\n",
> -                       non_htm_results.read_write_w[i]);
> -       }
> -
> -       return 0;
> -}
> -
> -REGISTER_TEST_COMMAND(hash_readwrite_autotest, test_hash_readwrite_main);
>  REGISTER_TEST_COMMAND(hash_readwrite_func_autotest, test_hash_rw_func_main);
>  REGISTER_TEST_COMMAND(hash_readwrite_perf_autotest, test_hash_rw_perf_main);
> --
> 2.17.1
>

I can see that DTS references this test:
https://git.dpdk.org/tools/dts/tree/tests/TestSuite_unit_tests_eal.py#n164

Travis (and UNH) also runs those unit tests (via the list included in meson).

Is there a reason to keep those in DTS?


Thanks.

--
David Marchand


^ permalink raw reply	[flat|nested] 38+ messages in thread

* Re: [dpdk-dev] [PATCH v2 3/5] test/hash: add lock free reader writer functional tests
  2020-02-03 19:49   ` [dpdk-dev] [PATCH v2 3/5] test/hash: add lock free reader writer functional tests Honnappa Nagarahalli
@ 2020-02-05  9:07     ` David Marchand
  2020-02-05 16:22       ` Honnappa Nagarahalli
  0 siblings, 1 reply; 38+ messages in thread
From: David Marchand @ 2020-02-05  9:07 UTC (permalink / raw)
  To: Honnappa Nagarahalli
  Cc: Amit Gupta, Wang, Yipeng1, Gobriel, Sameh, Thomas Monjalon, dev, nd

On Mon, Feb 3, 2020 at 8:49 PM Honnappa Nagarahalli
<honnappa.nagarahalli@arm.com> wrote:
>
> Add lock-free reader writer concurrency functional tests.
> These tests will provide the same coverage that non lock-free
> APIs have.
>
> Signed-off-by: Honnappa Nagarahalli <honnappa.nagarahalli@arm.com>
> ---
>  app/test/test_hash_readwrite.c | 58 +++++++++++++++++++++-------------
>  1 file changed, 36 insertions(+), 22 deletions(-)
>
> diff --git a/app/test/test_hash_readwrite.c b/app/test/test_hash_readwrite.c
> index 635ed5a9f..a9429091c 100644
> --- a/app/test/test_hash_readwrite.c
> +++ b/app/test/test_hash_readwrite.c
> @@ -121,7 +121,7 @@ test_hash_readwrite_worker(__attribute__((unused)) void *arg)
>  }
>
>  static int
> -init_params(int use_ext, int use_htm, int use_jhash)
> +init_params(int use_ext, int use_htm, int rw_lf, int use_jhash)
>  {
>         unsigned int i;
>
> @@ -140,15 +140,16 @@ init_params(int use_ext, int use_htm, int use_jhash)
>         else
>                 hash_params.hash_func = rte_hash_crc;
>
> +       hash_params.extra_flag = RTE_HASH_EXTRA_FLAGS_MULTI_WRITER_ADD;
>         if (use_htm)
> -               hash_params.extra_flag =
> -                       RTE_HASH_EXTRA_FLAGS_TRANS_MEM_SUPPORT |
> -                       RTE_HASH_EXTRA_FLAGS_RW_CONCURRENCY |
> -                       RTE_HASH_EXTRA_FLAGS_MULTI_WRITER_ADD;
> +               hash_params.extra_flag |=
> +                       RTE_HASH_EXTRA_FLAGS_TRANS_MEM_SUPPORT;
> +       if (rw_lf)
> +               hash_params.extra_flag |=
> +                       RTE_HASH_EXTRA_FLAGS_RW_CONCURRENCY_LF;
>         else
> -               hash_params.extra_flag =
> -                       RTE_HASH_EXTRA_FLAGS_RW_CONCURRENCY |
> -                       RTE_HASH_EXTRA_FLAGS_MULTI_WRITER_ADD;
> +               hash_params.extra_flag |=
> +                       RTE_HASH_EXTRA_FLAGS_RW_CONCURRENCY;
>
>         if (use_ext)
>                 hash_params.extra_flag |=
> @@ -195,7 +196,7 @@ init_params(int use_ext, int use_htm, int use_jhash)
>  }
>
>  static int
> -test_hash_readwrite_functional(int use_ext, int use_htm)
> +test_hash_readwrite_functional(int use_htm, int use_rw_lf, int use_ext)

This is a bit hard to read, please keep the same order than init_params.


>  {
>         unsigned int i;
>         const void *next_key;
> @@ -214,7 +215,7 @@ test_hash_readwrite_functional(int use_ext, int use_htm)
>         rte_atomic64_init(&ginsertions);
>         rte_atomic64_clear(&ginsertions);
>
> -       if (init_params(use_ext, use_htm, use_jhash) != 0)
> +       if (init_params(use_ext, use_htm, use_rw_lf, use_jhash) != 0)
>                 goto err;
>
>         if (use_ext)
> @@ -229,6 +230,8 @@ test_hash_readwrite_functional(int use_ext, int use_htm)
>                 tbl_rw_test_param.num_insert
>                 * slave_cnt;
>
> +       printf("\nHTM = %d, RW-LF = %d, EXT-Table = %d\n",
> +               use_htm, use_rw_lf, use_ext);
>         printf("++++++++Start function tests:+++++++++\n");
>
>         /* Fire all threads. */
> @@ -379,7 +382,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(0, use_htm, use_jhash) != 0)
> +       if (init_params(0, use_htm, 0, use_jhash) != 0)
>                 goto err;
>
>         /*
> @@ -700,7 +703,6 @@ test_hash_rw_func_main(void)
>          * than writer threads. This is to timing either reader threads or
>          * writer threads for performance numbers.
>          */
> -       int use_htm, use_ext;

The comments block just before is out of sync.


>         unsigned int i = 0, core_id = 0;
>
>         if (rte_lcore_count() < 3) {
> @@ -721,29 +723,41 @@ test_hash_rw_func_main(void)
>
>                 printf("Test read-write with Hardware transactional memory\n");
>
> -               use_htm = 1;
> -               use_ext = 0;
> +               /* htm = 1, rw_lf = 0, ext = 0 */

I didn't like those local variables.
But comments tend to get out of sync fairly easily, please remove too.


> +               if (test_hash_readwrite_functional(1, 0, 0) < 0)
> +                       return -1;
>
> -               if (test_hash_readwrite_functional(use_ext, use_htm) < 0)
> +               /* htm = 1, rw_lf = 1, ext = 0 */
> +               if (test_hash_readwrite_functional(1, 1, 0) < 0)
>                         return -1;
>
> -               use_ext = 1;
> -               if (test_hash_readwrite_functional(use_ext, use_htm) < 0)
> +               /* htm = 1, rw_lf = 0, ext = 1 */
> +               if (test_hash_readwrite_functional(1, 0, 1) < 0)
>                         return -1;
>
> +               /* htm = 1, rw_lf = 1, ext = 1 */
> +               if (test_hash_readwrite_functional(1, 1, 1) < 0)
> +                       return -1;
>         } else {
>                 printf("Hardware transactional memory (lock elision) "
>                         "is NOT supported\n");
>         }
>
>         printf("Test read-write without Hardware transactional memory\n");
> -       use_htm = 0;
> -       use_ext = 0;
> -       if (test_hash_readwrite_functional(use_ext, use_htm) < 0)
> +       /* htm = 0, rw_lf = 0, ext = 0 */
> +       if (test_hash_readwrite_functional(0, 0, 0) < 0)
> +               return -1;
> +
> +       /* htm = 0, rw_lf = 1, ext = 0 */
> +       if (test_hash_readwrite_functional(0, 1, 0) < 0)
> +               return -1;
> +
> +       /* htm = 0, rw_lf = 0, ext = 1 */
> +       if (test_hash_readwrite_functional(0, 0, 1) < 0)
>                 return -1;
>
> -       use_ext = 1;
> -       if (test_hash_readwrite_functional(use_ext, use_htm) < 0)
> +       /* htm = 0, rw_lf = 1, ext = 1 */
> +       if (test_hash_readwrite_functional(0, 1, 1) < 0)
>                 return -1;
>
>         return 0;
> --
> 2.17.1
>


-- 
David Marchand


^ permalink raw reply	[flat|nested] 38+ messages in thread

* Re: [dpdk-dev] [PATCH v2 3/5] test/hash: add lock free reader writer functional tests
  2020-02-05  9:07     ` David Marchand
@ 2020-02-05 16:22       ` Honnappa Nagarahalli
  2020-02-05 16:41         ` David Marchand
  0 siblings, 1 reply; 38+ messages in thread
From: Honnappa Nagarahalli @ 2020-02-05 16:22 UTC (permalink / raw)
  To: David Marchand
  Cc: Amit Gupta, Wang, Yipeng1, Gobriel, Sameh, thomas, dev, nd,
	Honnappa Nagarahalli, nd

> 
> On Mon, Feb 3, 2020 at 8:49 PM Honnappa Nagarahalli
> <honnappa.nagarahalli@arm.com> wrote:
> >
> > Add lock-free reader writer concurrency functional tests.
> > These tests will provide the same coverage that non lock-free APIs
> > have.
> >
> > Signed-off-by: Honnappa Nagarahalli <honnappa.nagarahalli@arm.com>
> > ---
> >  app/test/test_hash_readwrite.c | 58
> > +++++++++++++++++++++-------------
> >  1 file changed, 36 insertions(+), 22 deletions(-)
> >
> > diff --git a/app/test/test_hash_readwrite.c
> > b/app/test/test_hash_readwrite.c index 635ed5a9f..a9429091c 100644
> > --- a/app/test/test_hash_readwrite.c
> > +++ b/app/test/test_hash_readwrite.c
> > @@ -121,7 +121,7 @@
> test_hash_readwrite_worker(__attribute__((unused))
> > void *arg)  }
> >
> >  static int
> > -init_params(int use_ext, int use_htm, int use_jhash)
> > +init_params(int use_ext, int use_htm, int rw_lf, int use_jhash)
> >  {
> >         unsigned int i;
> >
> > @@ -140,15 +140,16 @@ init_params(int use_ext, int use_htm, int
> use_jhash)
> >         else
> >                 hash_params.hash_func = rte_hash_crc;
> >
> > +       hash_params.extra_flag =
> > + RTE_HASH_EXTRA_FLAGS_MULTI_WRITER_ADD;
> >         if (use_htm)
> > -               hash_params.extra_flag =
> > -                       RTE_HASH_EXTRA_FLAGS_TRANS_MEM_SUPPORT |
> > -                       RTE_HASH_EXTRA_FLAGS_RW_CONCURRENCY |
> > -                       RTE_HASH_EXTRA_FLAGS_MULTI_WRITER_ADD;
> > +               hash_params.extra_flag |=
> > +                       RTE_HASH_EXTRA_FLAGS_TRANS_MEM_SUPPORT;
> > +       if (rw_lf)
> > +               hash_params.extra_flag |=
> > +                       RTE_HASH_EXTRA_FLAGS_RW_CONCURRENCY_LF;
> >         else
> > -               hash_params.extra_flag =
> > -                       RTE_HASH_EXTRA_FLAGS_RW_CONCURRENCY |
> > -                       RTE_HASH_EXTRA_FLAGS_MULTI_WRITER_ADD;
> > +               hash_params.extra_flag |=
> > +                       RTE_HASH_EXTRA_FLAGS_RW_CONCURRENCY;
> >
> >         if (use_ext)
> >                 hash_params.extra_flag |= @@ -195,7 +196,7 @@
> > init_params(int use_ext, int use_htm, int use_jhash)  }
> >
> >  static int
> > -test_hash_readwrite_functional(int use_ext, int use_htm)
> > +test_hash_readwrite_functional(int use_htm, int use_rw_lf, int
> > +use_ext)
> 
> This is a bit hard to read, please keep the same order than init_params.
It looks like it is better to change the init_params. Otherwise, the code in test_hash_rw_func_main becomes hard to read. See the comment below.

> 
> 
> >  {
> >         unsigned int i;
> >         const void *next_key;
> > @@ -214,7 +215,7 @@ test_hash_readwrite_functional(int use_ext, int
> use_htm)
> >         rte_atomic64_init(&ginsertions);
> >         rte_atomic64_clear(&ginsertions);
> >
> > -       if (init_params(use_ext, use_htm, use_jhash) != 0)
> > +       if (init_params(use_ext, use_htm, use_rw_lf, use_jhash) != 0)
> >                 goto err;
> >
> >         if (use_ext)
> > @@ -229,6 +230,8 @@ test_hash_readwrite_functional(int use_ext, int
> use_htm)
> >                 tbl_rw_test_param.num_insert
> >                 * slave_cnt;
> >
> > +       printf("\nHTM = %d, RW-LF = %d, EXT-Table = %d\n",
> > +               use_htm, use_rw_lf, use_ext);
> >         printf("++++++++Start function tests:+++++++++\n");
> >
> >         /* Fire all threads. */
> > @@ -379,7 +382,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(0, use_htm, use_jhash) != 0)
> > +       if (init_params(0, use_htm, 0, use_jhash) != 0)
> >                 goto err;
> >
> >         /*
> > @@ -700,7 +703,6 @@ test_hash_rw_func_main(void)
> >          * than writer threads. This is to timing either reader threads or
> >          * writer threads for performance numbers.
> >          */
> > -       int use_htm, use_ext;
> 
> The comments block just before is out of sync.
> 
> 
> >         unsigned int i = 0, core_id = 0;
> >
> >         if (rte_lcore_count() < 3) {
> > @@ -721,29 +723,41 @@ test_hash_rw_func_main(void)
> >
> >                 printf("Test read-write with Hardware transactional
> > memory\n");
> >
> > -               use_htm = 1;
> > -               use_ext = 0;
> > +               /* htm = 1, rw_lf = 0, ext = 0 */
> 
> I didn't like those local variables.
> But comments tend to get out of sync fairly easily, please remove too.
> 
> 
> > +               if (test_hash_readwrite_functional(1, 0, 0) < 0)
> > +                       return -1;
> >
> > -               if (test_hash_readwrite_functional(use_ext, use_htm) < 0)
> > +               /* htm = 1, rw_lf = 1, ext = 0 */
> > +               if (test_hash_readwrite_functional(1, 1, 0) < 0)
> >                         return -1;
> >
> > -               use_ext = 1;
> > -               if (test_hash_readwrite_functional(use_ext, use_htm) < 0)
> > +               /* htm = 1, rw_lf = 0, ext = 1 */
> > +               if (test_hash_readwrite_functional(1, 0, 1) < 0)
> >                         return -1;
> >
> > +               /* htm = 1, rw_lf = 1, ext = 1 */
> > +               if (test_hash_readwrite_functional(1, 1, 1) < 0)
> > +                       return -1;
> >         } else {
> >                 printf("Hardware transactional memory (lock elision) "
> >                         "is NOT supported\n");
> >         }
> >
> >         printf("Test read-write without Hardware transactional memory\n");
> > -       use_htm = 0;
> > -       use_ext = 0;
> > -       if (test_hash_readwrite_functional(use_ext, use_htm) < 0)
> > +       /* htm = 0, rw_lf = 0, ext = 0 */
> > +       if (test_hash_readwrite_functional(0, 0, 0) < 0)
> > +               return -1;
> > +
> > +       /* htm = 0, rw_lf = 1, ext = 0 */
> > +       if (test_hash_readwrite_functional(0, 1, 0) < 0)
> > +               return -1;
> > +
> > +       /* htm = 0, rw_lf = 0, ext = 1 */
> > +       if (test_hash_readwrite_functional(0, 0, 1) < 0)
> >                 return -1;
> >
> > -       use_ext = 1;
> > -       if (test_hash_readwrite_functional(use_ext, use_htm) < 0)
> > +       /* htm = 0, rw_lf = 1, ext = 1 */
> > +       if (test_hash_readwrite_functional(0, 1, 1) < 0)
> >                 return -1;
The ordering of bits (0-0-0, 0-1-0, 0-0-1, 0-1-1) looks better here.

> >
> >         return 0;
> > --
> > 2.17.1
> >
> 
> 
> --
> David Marchand


^ permalink raw reply	[flat|nested] 38+ messages in thread

* Re: [dpdk-dev] [PATCH v2 3/5] test/hash: add lock free reader writer functional tests
  2020-02-05 16:22       ` Honnappa Nagarahalli
@ 2020-02-05 16:41         ` David Marchand
  2020-02-05 19:34           ` Wang, Yipeng1
  0 siblings, 1 reply; 38+ messages in thread
From: David Marchand @ 2020-02-05 16:41 UTC (permalink / raw)
  To: Honnappa Nagarahalli
  Cc: Amit Gupta, Wang, Yipeng1, Gobriel, Sameh, thomas, dev, nd

On Wed, Feb 5, 2020 at 5:22 PM Honnappa Nagarahalli
<Honnappa.Nagarahalli@arm.com> wrote:
>
> >
> > On Mon, Feb 3, 2020 at 8:49 PM Honnappa Nagarahalli
> > <honnappa.nagarahalli@arm.com> wrote:
> > >
> > > Add lock-free reader writer concurrency functional tests.
> > > These tests will provide the same coverage that non lock-free APIs
> > > have.
> > >
> > > Signed-off-by: Honnappa Nagarahalli <honnappa.nagarahalli@arm.com>
> > > ---
> > >  app/test/test_hash_readwrite.c | 58
> > > +++++++++++++++++++++-------------
> > >  1 file changed, 36 insertions(+), 22 deletions(-)
> > >
> > > diff --git a/app/test/test_hash_readwrite.c
> > > b/app/test/test_hash_readwrite.c index 635ed5a9f..a9429091c 100644
> > > --- a/app/test/test_hash_readwrite.c
> > > +++ b/app/test/test_hash_readwrite.c
> > > @@ -121,7 +121,7 @@
> > test_hash_readwrite_worker(__attribute__((unused))
> > > void *arg)  }
> > >
> > >  static int
> > > -init_params(int use_ext, int use_htm, int use_jhash)
> > > +init_params(int use_ext, int use_htm, int rw_lf, int use_jhash)
> > >  {
> > >         unsigned int i;
> > >
> > > @@ -140,15 +140,16 @@ init_params(int use_ext, int use_htm, int
> > use_jhash)
> > >         else
> > >                 hash_params.hash_func = rte_hash_crc;
> > >
> > > +       hash_params.extra_flag =
> > > + RTE_HASH_EXTRA_FLAGS_MULTI_WRITER_ADD;
> > >         if (use_htm)
> > > -               hash_params.extra_flag =
> > > -                       RTE_HASH_EXTRA_FLAGS_TRANS_MEM_SUPPORT |
> > > -                       RTE_HASH_EXTRA_FLAGS_RW_CONCURRENCY |
> > > -                       RTE_HASH_EXTRA_FLAGS_MULTI_WRITER_ADD;
> > > +               hash_params.extra_flag |=
> > > +                       RTE_HASH_EXTRA_FLAGS_TRANS_MEM_SUPPORT;
> > > +       if (rw_lf)
> > > +               hash_params.extra_flag |=
> > > +                       RTE_HASH_EXTRA_FLAGS_RW_CONCURRENCY_LF;
> > >         else
> > > -               hash_params.extra_flag =
> > > -                       RTE_HASH_EXTRA_FLAGS_RW_CONCURRENCY |
> > > -                       RTE_HASH_EXTRA_FLAGS_MULTI_WRITER_ADD;
> > > +               hash_params.extra_flag |=
> > > +                       RTE_HASH_EXTRA_FLAGS_RW_CONCURRENCY;
> > >
> > >         if (use_ext)
> > >                 hash_params.extra_flag |= @@ -195,7 +196,7 @@
> > > init_params(int use_ext, int use_htm, int use_jhash)  }
> > >
> > >  static int
> > > -test_hash_readwrite_functional(int use_ext, int use_htm)
> > > +test_hash_readwrite_functional(int use_htm, int use_rw_lf, int
> > > +use_ext)
> >
> > This is a bit hard to read, please keep the same order than init_params.
> It looks like it is better to change the init_params. Otherwise, the code in test_hash_rw_func_main becomes hard to read. See the comment below.
>
> >
> >
> > >  {
> > >         unsigned int i;
> > >         const void *next_key;
> > > @@ -214,7 +215,7 @@ test_hash_readwrite_functional(int use_ext, int
> > use_htm)
> > >         rte_atomic64_init(&ginsertions);
> > >         rte_atomic64_clear(&ginsertions);
> > >
> > > -       if (init_params(use_ext, use_htm, use_jhash) != 0)
> > > +       if (init_params(use_ext, use_htm, use_rw_lf, use_jhash) != 0)
> > >                 goto err;
> > >
> > >         if (use_ext)
> > > @@ -229,6 +230,8 @@ test_hash_readwrite_functional(int use_ext, int
> > use_htm)
> > >                 tbl_rw_test_param.num_insert
> > >                 * slave_cnt;
> > >
> > > +       printf("\nHTM = %d, RW-LF = %d, EXT-Table = %d\n",
> > > +               use_htm, use_rw_lf, use_ext);
> > >         printf("++++++++Start function tests:+++++++++\n");
> > >
> > >         /* Fire all threads. */
> > > @@ -379,7 +382,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(0, use_htm, use_jhash) != 0)
> > > +       if (init_params(0, use_htm, 0, use_jhash) != 0)
> > >                 goto err;
> > >
> > >         /*
> > > @@ -700,7 +703,6 @@ test_hash_rw_func_main(void)
> > >          * than writer threads. This is to timing either reader threads or
> > >          * writer threads for performance numbers.
> > >          */
> > > -       int use_htm, use_ext;
> >
> > The comments block just before is out of sync.
> >
> >
> > >         unsigned int i = 0, core_id = 0;
> > >
> > >         if (rte_lcore_count() < 3) {
> > > @@ -721,29 +723,41 @@ test_hash_rw_func_main(void)
> > >
> > >                 printf("Test read-write with Hardware transactional
> > > memory\n");
> > >
> > > -               use_htm = 1;
> > > -               use_ext = 0;
> > > +               /* htm = 1, rw_lf = 0, ext = 0 */
> >
> > I didn't like those local variables.
> > But comments tend to get out of sync fairly easily, please remove too.
> >
> >
> > > +               if (test_hash_readwrite_functional(1, 0, 0) < 0)
> > > +                       return -1;
> > >
> > > -               if (test_hash_readwrite_functional(use_ext, use_htm) < 0)
> > > +               /* htm = 1, rw_lf = 1, ext = 0 */
> > > +               if (test_hash_readwrite_functional(1, 1, 0) < 0)
> > >                         return -1;
> > >
> > > -               use_ext = 1;
> > > -               if (test_hash_readwrite_functional(use_ext, use_htm) < 0)
> > > +               /* htm = 1, rw_lf = 0, ext = 1 */
> > > +               if (test_hash_readwrite_functional(1, 0, 1) < 0)
> > >                         return -1;
> > >
> > > +               /* htm = 1, rw_lf = 1, ext = 1 */
> > > +               if (test_hash_readwrite_functional(1, 1, 1) < 0)
> > > +                       return -1;
> > >         } else {
> > >                 printf("Hardware transactional memory (lock elision) "
> > >                         "is NOT supported\n");
> > >         }
> > >
> > >         printf("Test read-write without Hardware transactional memory\n");
> > > -       use_htm = 0;
> > > -       use_ext = 0;
> > > -       if (test_hash_readwrite_functional(use_ext, use_htm) < 0)
> > > +       /* htm = 0, rw_lf = 0, ext = 0 */
> > > +       if (test_hash_readwrite_functional(0, 0, 0) < 0)
> > > +               return -1;
> > > +
> > > +       /* htm = 0, rw_lf = 1, ext = 0 */
> > > +       if (test_hash_readwrite_functional(0, 1, 0) < 0)
> > > +               return -1;
> > > +
> > > +       /* htm = 0, rw_lf = 0, ext = 1 */
> > > +       if (test_hash_readwrite_functional(0, 0, 1) < 0)
> > >                 return -1;
> > >
> > > -       use_ext = 1;
> > > -       if (test_hash_readwrite_functional(use_ext, use_htm) < 0)
> > > +       /* htm = 0, rw_lf = 1, ext = 1 */
> > > +       if (test_hash_readwrite_functional(0, 1, 1) < 0)
> > >                 return -1;
> The ordering of bits (0-0-0, 0-1-0, 0-0-1, 0-1-1) looks better here.

Ok, forget my comment.
I just want to get rid of this series and we stop getting random
timeout in the CI.
I will take it as is and cleanup if I find some time later.


-- 
David Marchand


^ permalink raw reply	[flat|nested] 38+ messages in thread

* Re: [dpdk-dev] [PATCH v2 2/5] test/hash: remove duplicated test code
  2020-02-05  8:48     ` David Marchand
@ 2020-02-05 16:42       ` David Marchand
  0 siblings, 0 replies; 38+ messages in thread
From: David Marchand @ 2020-02-05 16:42 UTC (permalink / raw)
  To: ci, Thomas Monjalon
  Cc: Amit Gupta, Wang, Yipeng1, Gobriel, Sameh, dev, nd, Mcnamara,
	John, Yigit, Ferruh, Honnappa Nagarahalli

On Wed, Feb 5, 2020 at 9:48 AM David Marchand <david.marchand@redhat.com> wrote:
>
> On Mon, Feb 3, 2020 at 8:49 PM Honnappa Nagarahalli
> <honnappa.nagarahalli@arm.com> wrote:
> >
> > The test case target 'hash_readwrite_autotest' is covered by
> > 'hash_readwrite_func_autotest' and 'hash_readwrite_perf_autotest'.
> > Hence, it is removed along with its test code.
> >
> > Signed-off-by: Honnappa Nagarahalli <honnappa.nagarahalli@arm.com>
> > ---
> >  app/test/autotest_data.py      |  10 +++-
> >  app/test/test_hash_readwrite.c | 101 ---------------------------------
> >  2 files changed, 8 insertions(+), 103 deletions(-)
> >
> > diff --git a/app/test/autotest_data.py b/app/test/autotest_data.py
> > index 6deb97bcc..71db4b3f6 100644
> > --- a/app/test/autotest_data.py
> > +++ b/app/test/autotest_data.py
> > @@ -664,8 +664,14 @@
> >          "Report":  None,
> >      },
> >      {
> > -        "Name":    "Hash read-write concurrency autotest",
> > -        "Command": "hash_readwrite_autotest",
> > +        "Name":    "Hash read-write concurrency functional autotest",
> > +        "Command": "hash_readwrite_func_autotest",
> > +        "Func":    default_autotest,
> > +        "Report":  None,
> > +    },
> > +    {
> > +        "Name":    "Hash read-write concurrency perf autotest",
> > +        "Command": "hash_readwrite_perf_autotest",
> >          "Func":    default_autotest,
> >          "Report":  None,
> >      },
> > diff --git a/app/test/test_hash_readwrite.c b/app/test/test_hash_readwrite.c
> > index aa55db7fe..635ed5a9f 100644
> > --- a/app/test/test_hash_readwrite.c
> > +++ b/app/test/test_hash_readwrite.c
> > @@ -749,106 +749,5 @@ test_hash_rw_func_main(void)
> >         return 0;
> >  }
> >
> > -static int
> > -test_hash_readwrite_main(void)
> > -{
> > -       /*
> > -        * Variables used to choose different tests.
> > -        * use_htm indicates if hardware transactional memory should be used.
> > -        * reader_faster indicates if the reader threads should finish earlier
> > -        * than writer threads. This is to timing either reader threads or
> > -        * writer threads for performance numbers.
> > -        */
> > -       int use_htm, use_ext,  reader_faster;
> > -       unsigned int i = 0, core_id = 0;
> > -
> > -       if (rte_lcore_count() < 3) {
> > -               printf("Not enough cores for hash_readwrite_autotest, expecting at least 3\n");
> > -               return TEST_SKIPPED;
> > -       }
> > -
> > -       RTE_LCORE_FOREACH_SLAVE(core_id) {
> > -               slave_core_ids[i] = core_id;
> > -               i++;
> > -       }
> > -
> > -       setlocale(LC_NUMERIC, "");
> > -
> > -       if (rte_tm_supported()) {
> > -               printf("Hardware transactional memory (lock elision) "
> > -                       "is supported\n");
> > -
> > -               printf("Test read-write with Hardware transactional memory\n");
> > -
> > -               use_htm = 1;
> > -               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(&htm_results, use_htm,
> > -                                                       reader_faster) < 0)
> > -                       return -1;
> > -
> > -               reader_faster = 0;
> > -               if (test_hash_readwrite_perf(&htm_results, use_htm,
> > -                                                       reader_faster) < 0)
> > -                       return -1;
> > -       } else {
> > -               printf("Hardware transactional memory (lock elision) "
> > -                       "is NOT supported\n");
> > -       }
> > -
> > -       printf("Test read-write without Hardware transactional memory\n");
> > -       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)
> > -               return -1;
> > -       reader_faster = 0;
> > -       if (test_hash_readwrite_perf(&non_htm_results, use_htm,
> > -                                                       reader_faster) < 0)
> > -               return -1;
> > -
> > -       printf("================\n");
> > -       printf("Results summary:\n");
> > -       printf("================\n");
> > -
> > -       printf("single read: %u\n", htm_results.single_read);
> > -       printf("single write: %u\n", htm_results.single_write);
> > -       for (i = 0; i < NUM_TEST; i++) {
> > -               printf("+++ core_cnt: %u +++\n", core_cnt[i]);
> > -               printf("HTM:\n");
> > -               printf("  read only: %u\n", htm_results.read_only[i]);
> > -               printf("  write only: %u\n", htm_results.write_only[i]);
> > -               printf("  read-write read: %u\n", htm_results.read_write_r[i]);
> > -               printf("  read-write write: %u\n", htm_results.read_write_w[i]);
> > -
> > -               printf("non HTM:\n");
> > -               printf("  read only: %u\n", non_htm_results.read_only[i]);
> > -               printf("  write only: %u\n", non_htm_results.write_only[i]);
> > -               printf("  read-write read: %u\n",
> > -                       non_htm_results.read_write_r[i]);
> > -               printf("  read-write write: %u\n",
> > -                       non_htm_results.read_write_w[i]);
> > -       }
> > -
> > -       return 0;
> > -}
> > -
> > -REGISTER_TEST_COMMAND(hash_readwrite_autotest, test_hash_readwrite_main);
> >  REGISTER_TEST_COMMAND(hash_readwrite_func_autotest, test_hash_rw_func_main);
> >  REGISTER_TEST_COMMAND(hash_readwrite_perf_autotest, test_hash_rw_perf_main);
> > --
> > 2.17.1
> >
>
> I can see that DTS references this test:
> https://git.dpdk.org/tools/dts/tree/tests/TestSuite_unit_tests_eal.py#n164
>
> Travis (and UNH) also runs those unit tests (via the list included in meson).
>
> Is there a reason to keep those in DTS?


I will take this as a no and go ahead with this patch.


-- 
David Marchand


^ permalink raw reply	[flat|nested] 38+ messages in thread

* Re: [dpdk-dev] [PATCH v2 0/5] test/meson: fix hash readwrite timeout failure
  2020-02-03 19:49 ` [dpdk-dev] [PATCH v2 0/5] " Honnappa Nagarahalli
                     ` (4 preceding siblings ...)
  2020-02-03 19:49   ` [dpdk-dev] [PATCH v2 5/5] hash: correct lock free extendable table support Honnappa Nagarahalli
@ 2020-02-05 18:41   ` David Marchand
  5 siblings, 0 replies; 38+ messages in thread
From: David Marchand @ 2020-02-05 18:41 UTC (permalink / raw)
  To: Honnappa Nagarahalli
  Cc: Amit Gupta, Wang, Yipeng1, Gobriel, Sameh, Thomas Monjalon, dev, nd

On Mon, Feb 3, 2020 at 8:49 PM Honnappa Nagarahalli
<honnappa.nagarahalli@arm.com> wrote:
>
> V2 changes:
> - Remove duplicated code in the existing patch
> - Add lock-free option to hash read-write functional test cases
> - Move existing loack-free test cases to performance tests
> - A typo fix in the comments for lock-free extendable table feature
>
> V1 changes:
> - hash_readwrite and hash_readwrite lockfree meson test was
>   taking longer time to complete. The test always get TIMEOUT.
> - hash readwrtie test is split into functional and perf tests
>   and moved to dpdk fast and perf testsuite accordingly.
> - hash readwrite lockfree is moved to dpdk perf testsuite.
>
> Amit Gupta (1):
>   test/meson: hash test split into shorter subtests
>
> Honnappa Nagarahalli (4):
>   test/hash: remove duplicated test code
>   test/hash: add lock free reader writer functional tests
>   test/hash: move reader writer lock free tests to perf tests
>   hash: correct lock free extendable table support
>
>  app/test/Makefile                             |   2 +-
>  app/test/autotest_data.py                     |  14 ++-
>  app/test/meson.build                          |   7 +-
>  app/test/test_hash_readwrite.c                | 117 +++++++++++++-----
>  ...ite_lf.c => test_hash_readwrite_lf_perf.c} |   7 +-
>  lib/librte_hash/rte_hash.h                    |   2 -
>  6 files changed, 107 insertions(+), 42 deletions(-)
>  rename app/test/{test_hash_readwrite_lf.c => test_hash_readwrite_lf_perf.c} (99%)

Squashed patch 2 in patch 1.
Series applied, thanks.


-- 
David Marchand


^ permalink raw reply	[flat|nested] 38+ messages in thread

* Re: [dpdk-dev] [PATCH v2 3/5] test/hash: add lock free reader writer functional tests
  2020-02-05 16:41         ` David Marchand
@ 2020-02-05 19:34           ` Wang, Yipeng1
  2020-02-05 19:52             ` Honnappa Nagarahalli
  0 siblings, 1 reply; 38+ messages in thread
From: Wang, Yipeng1 @ 2020-02-05 19:34 UTC (permalink / raw)
  To: David Marchand, Honnappa Nagarahalli
  Cc: Amit Gupta, Gobriel, Sameh, thomas, dev, nd

>-----Original Message-----
>From: David Marchand [mailto:david.marchand@redhat.com]
>Sent: Wednesday, February 5, 2020 8:42 AM
>To: Honnappa Nagarahalli <Honnappa.Nagarahalli@arm.com>
>Cc: Amit Gupta <agupta3@marvell.com>; Wang, Yipeng1 <yipeng1.wang@intel.com>; Gobriel, Sameh <sameh.gobriel@intel.com>;
>thomas@monjalon.net; dev <dev@dpdk.org>; nd <nd@arm.com>
>Subject: Re: [PATCH v2 3/5] test/hash: add lock free reader writer functional tests
>
>On Wed, Feb 5, 2020 at 5:22 PM Honnappa Nagarahalli
><Honnappa.Nagarahalli@arm.com> wrote:
>>
>> >
>> > On Mon, Feb 3, 2020 at 8:49 PM Honnappa Nagarahalli
>> > <honnappa.nagarahalli@arm.com> wrote:
>> > >
>> > > Add lock-free reader writer concurrency functional tests.
>> > > These tests will provide the same coverage that non lock-free APIs
>> > > have.
>> > >
>> > > Signed-off-by: Honnappa Nagarahalli <honnappa.nagarahalli@arm.com>
>> > > ---
>> > >  app/test/test_hash_readwrite.c | 58
>> > > +++++++++++++++++++++-------------
>> > >  1 file changed, 36 insertions(+), 22 deletions(-)
>> > >
>> > > diff --git a/app/test/test_hash_readwrite.c
>> > > b/app/test/test_hash_readwrite.c index 635ed5a9f..a9429091c 100644
>> > > --- a/app/test/test_hash_readwrite.c
>> > > +++ b/app/test/test_hash_readwrite.c
>> > > @@ -121,7 +121,7 @@
>> > test_hash_readwrite_worker(__attribute__((unused))
>> > > void *arg)  }
>> > >
>> > >  static int
>> > > -init_params(int use_ext, int use_htm, int use_jhash)
>> > > +init_params(int use_ext, int use_htm, int rw_lf, int use_jhash)
>> > >  {
>> > >         unsigned int i;
>> > >
>> > > @@ -140,15 +140,16 @@ init_params(int use_ext, int use_htm, int
>> > use_jhash)
>> > >         else
>> > >                 hash_params.hash_func = rte_hash_crc;
>> > >
>> > > +       hash_params.extra_flag =
>> > > + RTE_HASH_EXTRA_FLAGS_MULTI_WRITER_ADD;
>> > >         if (use_htm)
>> > > -               hash_params.extra_flag =
>> > > -                       RTE_HASH_EXTRA_FLAGS_TRANS_MEM_SUPPORT |
>> > > -                       RTE_HASH_EXTRA_FLAGS_RW_CONCURRENCY |
>> > > -                       RTE_HASH_EXTRA_FLAGS_MULTI_WRITER_ADD;
>> > > +               hash_params.extra_flag |=
>> > > +                       RTE_HASH_EXTRA_FLAGS_TRANS_MEM_SUPPORT;

[Wang, Yipeng] Thanks for the patch Honnappa. Here I think we still need the RTE_HASH_EXTRA_FLAGS_RW_CONCURRENCY
Flag even with HTM.

Other commits in this series look good to me and seems David already applied.

Thanks!

^ permalink raw reply	[flat|nested] 38+ messages in thread

* Re: [dpdk-dev] [PATCH v2 3/5] test/hash: add lock free reader writer functional tests
  2020-02-05 19:34           ` Wang, Yipeng1
@ 2020-02-05 19:52             ` Honnappa Nagarahalli
  2020-02-05 19:57               ` Wang, Yipeng1
  0 siblings, 1 reply; 38+ messages in thread
From: Honnappa Nagarahalli @ 2020-02-05 19:52 UTC (permalink / raw)
  To: Wang, Yipeng1, David Marchand
  Cc: Amit Gupta, Gobriel, Sameh, thomas, dev, nd, Honnappa Nagarahalli, nd

<snip>

> >> > >
> >> > > Add lock-free reader writer concurrency functional tests.
> >> > > These tests will provide the same coverage that non lock-free
> >> > > APIs have.
> >> > >
> >> > > Signed-off-by: Honnappa Nagarahalli
> >> > > <honnappa.nagarahalli@arm.com>
> >> > > ---
> >> > >  app/test/test_hash_readwrite.c | 58
> >> > > +++++++++++++++++++++-------------
> >> > >  1 file changed, 36 insertions(+), 22 deletions(-)
> >> > >
> >> > > diff --git a/app/test/test_hash_readwrite.c
> >> > > b/app/test/test_hash_readwrite.c index 635ed5a9f..a9429091c
> >> > > 100644
> >> > > --- a/app/test/test_hash_readwrite.c
> >> > > +++ b/app/test/test_hash_readwrite.c
> >> > > @@ -121,7 +121,7 @@
> >> > test_hash_readwrite_worker(__attribute__((unused))
> >> > > void *arg)  }
> >> > >
> >> > >  static int
> >> > > -init_params(int use_ext, int use_htm, int use_jhash)
> >> > > +init_params(int use_ext, int use_htm, int rw_lf, int use_jhash)
> >> > >  {
> >> > >         unsigned int i;
> >> > >
> >> > > @@ -140,15 +140,16 @@ init_params(int use_ext, int use_htm, int
> >> > use_jhash)
> >> > >         else
> >> > >                 hash_params.hash_func = rte_hash_crc;
> >> > >
> >> > > +       hash_params.extra_flag =
> >> > > + RTE_HASH_EXTRA_FLAGS_MULTI_WRITER_ADD;
> >> > >         if (use_htm)
> >> > > -               hash_params.extra_flag =
> >> > > -                       RTE_HASH_EXTRA_FLAGS_TRANS_MEM_SUPPORT |
> >> > > -                       RTE_HASH_EXTRA_FLAGS_RW_CONCURRENCY |
> >> > > -                       RTE_HASH_EXTRA_FLAGS_MULTI_WRITER_ADD;
> >> > > +               hash_params.extra_flag |=
> >> > > +                       RTE_HASH_EXTRA_FLAGS_TRANS_MEM_SUPPORT;
> 
> [Wang, Yipeng] Thanks for the patch Honnappa. Here I think we still need the
> RTE_HASH_EXTRA_FLAGS_RW_CONCURRENCY
> Flag even with HTM.
I have made RTE_HASH_EXTRA_FLAGS_RW_CONCURRENCY depend on 'rw_lf' flag. The test case HTM + RTE_HASH_EXTRA_FLAGS_RW_CONCURRENCY will still run when 'rw_lf' is set to 0.

> 
> Other commits in this series look good to me and seems David already
> applied.
> 
> Thanks!

^ permalink raw reply	[flat|nested] 38+ messages in thread

* Re: [dpdk-dev] [PATCH v2 3/5] test/hash: add lock free reader writer functional tests
  2020-02-05 19:52             ` Honnappa Nagarahalli
@ 2020-02-05 19:57               ` Wang, Yipeng1
  0 siblings, 0 replies; 38+ messages in thread
From: Wang, Yipeng1 @ 2020-02-05 19:57 UTC (permalink / raw)
  To: Honnappa Nagarahalli, David Marchand
  Cc: Amit Gupta, Gobriel, Sameh, thomas, dev, nd, nd

>-----Original Message-----
>From: Honnappa Nagarahalli [mailto:Honnappa.Nagarahalli@arm.com]
>Sent: Wednesday, February 5, 2020 11:52 AM
>To: Wang, Yipeng1 <yipeng1.wang@intel.com>; David Marchand <david.marchand@redhat.com>
>Cc: Amit Gupta <agupta3@marvell.com>; Gobriel, Sameh <sameh.gobriel@intel.com>; thomas@monjalon.net; dev <dev@dpdk.org>;
>nd <nd@arm.com>; Honnappa Nagarahalli <Honnappa.Nagarahalli@arm.com>; nd <nd@arm.com>
>Subject: RE: [PATCH v2 3/5] test/hash: add lock free reader writer functional tests
>
><snip>
>
>> >> > >
>> >> > > Add lock-free reader writer concurrency functional tests.
>> >> > > These tests will provide the same coverage that non lock-free
>> >> > > APIs have.
>> >> > >
>> >> > > Signed-off-by: Honnappa Nagarahalli
>> >> > > <honnappa.nagarahalli@arm.com>
>> >> > > ---
>> >> > >  app/test/test_hash_readwrite.c | 58
>> >> > > +++++++++++++++++++++-------------
>> >> > >  1 file changed, 36 insertions(+), 22 deletions(-)
>> >> > >
>> >> > > diff --git a/app/test/test_hash_readwrite.c
>> >> > > b/app/test/test_hash_readwrite.c index 635ed5a9f..a9429091c
>> >> > > 100644
>> >> > > --- a/app/test/test_hash_readwrite.c
>> >> > > +++ b/app/test/test_hash_readwrite.c
>> >> > > @@ -121,7 +121,7 @@
>> >> > test_hash_readwrite_worker(__attribute__((unused))
>> >> > > void *arg)  }
>> >> > >
>> >> > >  static int
>> >> > > -init_params(int use_ext, int use_htm, int use_jhash)
>> >> > > +init_params(int use_ext, int use_htm, int rw_lf, int use_jhash)
>> >> > >  {
>> >> > >         unsigned int i;
>> >> > >
>> >> > > @@ -140,15 +140,16 @@ init_params(int use_ext, int use_htm, int
>> >> > use_jhash)
>> >> > >         else
>> >> > >                 hash_params.hash_func = rte_hash_crc;
>> >> > >
>> >> > > +       hash_params.extra_flag =
>> >> > > + RTE_HASH_EXTRA_FLAGS_MULTI_WRITER_ADD;
>> >> > >         if (use_htm)
>> >> > > -               hash_params.extra_flag =
>> >> > > -                       RTE_HASH_EXTRA_FLAGS_TRANS_MEM_SUPPORT |
>> >> > > -                       RTE_HASH_EXTRA_FLAGS_RW_CONCURRENCY |
>> >> > > -                       RTE_HASH_EXTRA_FLAGS_MULTI_WRITER_ADD;
>> >> > > +               hash_params.extra_flag |=
>> >> > > +                       RTE_HASH_EXTRA_FLAGS_TRANS_MEM_SUPPORT;
>>
>> [Wang, Yipeng] Thanks for the patch Honnappa. Here I think we still need the
>> RTE_HASH_EXTRA_FLAGS_RW_CONCURRENCY
>> Flag even with HTM.
>I have made RTE_HASH_EXTRA_FLAGS_RW_CONCURRENCY depend on 'rw_lf' flag. The test case HTM +
>RTE_HASH_EXTRA_FLAGS_RW_CONCURRENCY will still run when 'rw_lf' is set to 0.
>
[Wang, Yipeng] 
I see, thought was an "else if". It is correct then,
Thanks!

^ permalink raw reply	[flat|nested] 38+ messages in thread

end of thread, back to index

Thread overview: 38+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-09-06  5:49 [dpdk-dev] [PATCH 0/2] test/meson: fix hash readwrite timeout failure agupta3
2019-09-06  5:49 ` [dpdk-dev] [PATCH 1/2] test/meson: hash test split into shorter subtests agupta3
2019-09-11 17:05   ` Wang, Yipeng1
2019-10-17  5:02     ` Amit Gupta
2019-11-01  4:54       ` Amit Gupta
2019-11-01 17:04         ` Wang, Yipeng1
2019-11-05 16:37           ` Ferruh Yigit
2019-11-07  3:32             ` [dpdk-dev] [EXT] " Amit Gupta
2019-12-31  4:56               ` Amit Gupta
2019-09-06  5:49 ` [dpdk-dev] [PATCH 2/2] test/meson: hash lf test moved to dpdk perf testsuite agupta3
2019-09-11 17:13   ` Wang, Yipeng1
2019-09-12 15:00     ` Honnappa Nagarahalli
2019-09-13  8:24       ` Amit Gupta
2019-09-13  8:12   ` [dpdk-dev] [PATCH v2 1/1] " agupta3
2019-09-13 14:40     ` Aaron Conole
2019-09-13 15:09       ` Wang, Yipeng1
2019-09-13 15:46         ` Honnappa Nagarahalli
2019-09-16  4:39           ` Amit Gupta
2019-10-17  4:57           ` Amit Gupta
2019-10-17 13:16             ` Aaron Conole
2019-10-24  7:22               ` David Marchand
2019-09-13  8:15   ` agupta3
2019-09-11  5:55 ` [dpdk-dev] [PATCH 0/2] test/meson: fix hash readwrite timeout failure Amit Gupta
2020-02-03 19:49 ` [dpdk-dev] [PATCH v2 0/5] " Honnappa Nagarahalli
2020-02-03 19:49   ` [dpdk-dev] [PATCH v2 1/5] test/meson: hash test split into shorter subtests Honnappa Nagarahalli
2020-02-03 19:49   ` [dpdk-dev] [PATCH v2 2/5] test/hash: remove duplicated test code Honnappa Nagarahalli
2020-02-05  8:48     ` David Marchand
2020-02-05 16:42       ` David Marchand
2020-02-03 19:49   ` [dpdk-dev] [PATCH v2 3/5] test/hash: add lock free reader writer functional tests Honnappa Nagarahalli
2020-02-05  9:07     ` David Marchand
2020-02-05 16:22       ` Honnappa Nagarahalli
2020-02-05 16:41         ` David Marchand
2020-02-05 19:34           ` Wang, Yipeng1
2020-02-05 19:52             ` Honnappa Nagarahalli
2020-02-05 19:57               ` Wang, Yipeng1
2020-02-03 19:49   ` [dpdk-dev] [PATCH v2 4/5] test/hash: move reader writer lock free tests to perf tests Honnappa Nagarahalli
2020-02-03 19:49   ` [dpdk-dev] [PATCH v2 5/5] hash: correct lock free extendable table support Honnappa Nagarahalli
2020-02-05 18:41   ` [dpdk-dev] [PATCH v2 0/5] test/meson: fix hash readwrite timeout failure David Marchand

DPDK patches and discussions

Archives are clonable:
	git clone --mirror http://inbox.dpdk.org/dev/0 dev/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 dev dev/ http://inbox.dpdk.org/dev \
		dev@dpdk.org
	public-inbox-index dev


Newsgroup available over NNTP:
	nntp://inbox.dpdk.org/inbox.dpdk.dev


AGPL code for this site: git clone https://public-inbox.org/ public-inbox