[dpdk-dev] [PATCH 0/2] test/meson: fix hash readwrite timeout failure

[dpdk-dev] [PATCH 0/2] test/meson: fix hash readwrite timeout failure

[agupta3]

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

[dpdk-dev] [PATCH 1/2] test/meson: hash test split into shorter subtests

[agupta3]

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

Re: [dpdk-dev] [PATCH 1/2] test/meson: hash test split into shorter subtests

[Wang, Yipeng1]

>-----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!

Re: [dpdk-dev] [PATCH 1/2] test/meson: hash test split into shorter subtests

[Amit Gupta]

> -----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

Re: [dpdk-dev] [PATCH 1/2] test/meson: hash test split into shorter subtests

[Amit Gupta]

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

Re: [dpdk-dev] [PATCH 1/2] test/meson: hash test split into shorter subtests

[Wang, Yipeng1]

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

Re: [dpdk-dev] [PATCH 1/2] test/meson: hash test split into shorter subtests

[Ferruh Yigit]

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

Re: [dpdk-dev] [EXT] Re: [PATCH 1/2] test/meson: hash test split into shorter subtests

[Amit Gupta]

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

Re: [dpdk-dev] [EXT] Re: [PATCH 1/2] test/meson: hash test split into shorter subtests

[Amit Gupta]

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

[dpdk-dev] [PATCH 2/2] test/meson: hash lf test moved to dpdk perf testsuite

[agupta3]

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

Re: [dpdk-dev] [PATCH 2/2] test/meson: hash lf test moved to dpdk perf testsuite

[Wang, Yipeng1]

>-----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

Re: [dpdk-dev] [PATCH 2/2] test/meson: hash lf test moved to dpdk perf testsuite

[Honnappa Nagarahalli]

<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

Re: [dpdk-dev] [PATCH 2/2] test/meson: hash lf test moved to dpdk perf testsuite

[Amit Gupta]

> -----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

[dpdk-dev] [PATCH v2 1/1] test/meson: hash lf test moved to dpdk perf testsuite

[agupta3]

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

Re: [dpdk-dev] [PATCH v2 1/1] test/meson: hash lf test moved to dpdk perf testsuite

[Aaron Conole]

<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);

Re: [dpdk-dev] [PATCH v2 1/1] test/meson: hash lf test moved to dpdk perf testsuite

[Wang, Yipeng1]

> -----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 :)

Re: [dpdk-dev] [PATCH v2 1/1] test/meson: hash lf test moved to dpdk perf testsuite

[Honnappa Nagarahalli]

<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?
> 

Re: [dpdk-dev] [PATCH v2 1/1] test/meson: hash lf test moved to dpdk perf testsuite

[Amit Gupta]

> -----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.

Re: [dpdk-dev] [PATCH v2 1/1] test/meson: hash lf test moved to dpdk perf testsuite

[Amit Gupta]

> -----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.

Re: [dpdk-dev] [PATCH v2 1/1] test/meson: hash lf test moved to dpdk perf testsuite

[Aaron Conole]

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.

Re: [dpdk-dev] [PATCH v2 1/1] test/meson: hash lf test moved to dpdk perf testsuite

[David Marchand]

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

[dpdk-dev] [PATCH v2 1/1] test/meson: hash lf test moved to dpdk perf testsuite

[agupta3]

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

Re: [dpdk-dev] [PATCH 0/2] test/meson: fix hash readwrite timeout failure

[Amit Gupta]

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

[dpdk-dev] [PATCH v2 0/5] test/meson: fix hash readwrite timeout failure

[Honnappa Nagarahalli]

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

[dpdk-dev] [PATCH v2 1/5] test/meson: hash test split into shorter subtests

[Honnappa Nagarahalli]

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

[dpdk-dev] [PATCH v2 2/5] test/hash: remove duplicated test code

[Honnappa Nagarahalli]

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

Re: [dpdk-dev] [PATCH v2 2/5] test/hash: remove duplicated test code

[David Marchand]

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

Re: [dpdk-dev] [PATCH v2 2/5] test/hash: remove duplicated test code

[David Marchand]

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

[dpdk-dev] [PATCH v2 3/5] test/hash: add lock free reader writer functional tests

[Honnappa Nagarahalli]

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

Re: [dpdk-dev] [PATCH v2 3/5] test/hash: add lock free reader writer functional tests

[David Marchand]

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

Re: [dpdk-dev] [PATCH v2 3/5] test/hash: add lock free reader writer functional tests

[Honnappa Nagarahalli]

> 
> 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

Re: [dpdk-dev] [PATCH v2 3/5] test/hash: add lock free reader writer functional tests

[David Marchand]

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

Re: [dpdk-dev] [PATCH v2 3/5] test/hash: add lock free reader writer functional tests

[Wang, Yipeng1]

>-----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!

Re: [dpdk-dev] [PATCH v2 3/5] test/hash: add lock free reader writer functional tests

[Honnappa Nagarahalli]

<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!

Re: [dpdk-dev] [PATCH v2 3/5] test/hash: add lock free reader writer functional tests

[Wang, Yipeng1]

>-----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!

[dpdk-dev] [PATCH v2 4/5] test/hash: move reader writer lock free tests to perf tests

[Honnappa Nagarahalli]

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

[dpdk-dev] [PATCH v2 5/5] hash: correct lock free extendable table support

[Honnappa Nagarahalli]

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

Re: [dpdk-dev] [PATCH v2 0/5] test/meson: fix hash readwrite timeout failure

[David Marchand]

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