[v2,1/1] test/meson: hash lf test moved to dpdk perf testsuite
Checks
Commit Message
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(-)
Comments
<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);
> -----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 :)
<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?
>
> -----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.
> -----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.
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.
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.
@@ -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 = [
@@ -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);