diff mbox series

[B,SRU,1/1] selftests/net: bump timeout to 5 minutes

Message ID 20210505062000.21838-2-po-hsu.lin@canonical.com
State New
Headers show
Series selftests/net: bump timeout to 5 minutes | expand

Commit Message

Po-Hsu Lin May 5, 2021, 6:20 a.m. UTC 
BugLink: https://bugs.launchpad.net/bugs/1856010

We found that with the latest mainline kernel (5.12.0-051200rc8) on
some KVM instances / bare-metal systems, the following tests will take
longer than the kselftest framework default timeout (45 seconds) to
run and thus got terminated with TIMEOUT error:
* xfrm_policy.sh - took about 2m20s
* pmtu.sh - took about 3m5s
* udpgso_bench.sh - took about 60s

Bump the timeout setting to 5 minutes to allow them have a chance to
finish.

https://bugs.launchpad.net/bugs/1856010
Signed-off-by: Po-Hsu Lin <po-hsu.lin@canonical.com>
Signed-off-by: David S. Miller <davem@davemloft.net>
(Backported from commit b881d089c7c9c7032da812cda1b4b0818f477780)
[PHLin: context adjustment]
Signed-off-by: Po-Hsu Lin <po-hsu.lin@canonical.com>
---
 tools/testing/selftests/net/Makefile | 2 ++
 tools/testing/selftests/net/settings | 1 +
 2 files changed, 3 insertions(+)
 create mode 100644 tools/testing/selftests/net/settings

Comments

Stefan Bader May 5, 2021, 9 a.m. UTC | #1 
On 05.05.21 08:20, Po-Hsu Lin wrote:
> BugLink: https://bugs.launchpad.net/bugs/1856010
> 
> We found that with the latest mainline kernel (5.12.0-051200rc8) on
> some KVM instances / bare-metal systems, the following tests will take
> longer than the kselftest framework default timeout (45 seconds) to
> run and thus got terminated with TIMEOUT error:
> * xfrm_policy.sh - took about 2m20s
> * pmtu.sh - took about 3m5s
> * udpgso_bench.sh - took about 60s
> 
> Bump the timeout setting to 5 minutes to allow them have a chance to
> finish.

In the cover email you mention this is only needed for Bionic because newer 
series disable the timeout completely. Just even with that info, the commit 
message sounds odd to me. Why would timing of a 5.12 kernel matter for bionic?

-Stefan
> 
> https://bugs.launchpad.net/bugs/1856010
> Signed-off-by: Po-Hsu Lin <po-hsu.lin@canonical.com>
> Signed-off-by: David S. Miller <davem@davemloft.net>
> (Backported from commit b881d089c7c9c7032da812cda1b4b0818f477780)
> [PHLin: context adjustment]
> Signed-off-by: Po-Hsu Lin <po-hsu.lin@canonical.com>
> ---
>   tools/testing/selftests/net/Makefile | 2 ++
>   tools/testing/selftests/net/settings | 1 +
>   2 files changed, 3 insertions(+)
>   create mode 100644 tools/testing/selftests/net/settings
> 
> diff --git a/tools/testing/selftests/net/Makefile b/tools/testing/selftests/net/Makefile
> index 80b03af..337d84a 100644
> --- a/tools/testing/selftests/net/Makefile
> +++ b/tools/testing/selftests/net/Makefile
> @@ -10,6 +10,8 @@ TEST_GEN_FILES += psock_fanout psock_tpacket msg_zerocopy
>   TEST_GEN_PROGS = reuseport_bpf reuseport_bpf_cpu reuseport_bpf_numa
>   TEST_GEN_PROGS += reuseport_dualstack reuseaddr_conflict
>   
> +TEST_FILES := settings
> +
>   KSFT_KHDR_INSTALL := 1
>   include ../lib.mk
>   
> diff --git a/tools/testing/selftests/net/settings b/tools/testing/selftests/net/settings
> new file mode 100644
> index 0000000..694d707
> --- /dev/null
> +++ b/tools/testing/selftests/net/settings
> @@ -0,0 +1 @@
> +timeout=300
>
Po-Hsu Lin May 5, 2021, 10:18 a.m. UTC | #2 
On Wed, May 5, 2021 at 5:00 PM Stefan Bader <stefan.bader@canonical.com> wrote:
>
> On 05.05.21 08:20, Po-Hsu Lin wrote:
> > BugLink: https://bugs.launchpad.net/bugs/1856010
> >
> > We found that with the latest mainline kernel (5.12.0-051200rc8) on
> > some KVM instances / bare-metal systems, the following tests will take
> > longer than the kselftest framework default timeout (45 seconds) to
> > run and thus got terminated with TIMEOUT error:
> > * xfrm_policy.sh - took about 2m20s
> > * pmtu.sh - took about 3m5s
> > * udpgso_bench.sh - took about 60s
> >
> > Bump the timeout setting to 5 minutes to allow them have a chance to
> > finish.
>
> In the cover email you mention this is only needed for Bionic because newer
> series disable the timeout completely. Just even with that info, the commit
> message sounds odd to me. Why would timing of a 5.12 kernel matter for bionic?
>
> -Stefan

Hi Stefan,

In order to submit this patch to upstream, I have it tested with 5.12
as a justification
Theoretically any kernel that has the commit which introduced the
default 45 seconds timeout can be affected by this issue.

I took a closer look in the net test directory again, it looks like
bionic does not have these three test cases.
Maybe that's why we didn't add our sauce patch to disable the timeout on Bionic.
Base on this fact I am ok with the decision either to drop this or
keep SRU it to make it align with upstream code.
Thanks
Sam

> >
> > https://bugs.launchpad.net/bugs/1856010
> > Signed-off-by: Po-Hsu Lin <po-hsu.lin@canonical.com>
> > Signed-off-by: David S. Miller <davem@davemloft.net>
> > (Backported from commit b881d089c7c9c7032da812cda1b4b0818f477780)
> > [PHLin: context adjustment]
> > Signed-off-by: Po-Hsu Lin <po-hsu.lin@canonical.com>
> > ---
> >   tools/testing/selftests/net/Makefile | 2 ++
> >   tools/testing/selftests/net/settings | 1 +
> >   2 files changed, 3 insertions(+)
> >   create mode 100644 tools/testing/selftests/net/settings
> >
> > diff --git a/tools/testing/selftests/net/Makefile b/tools/testing/selftests/net/Makefile
> > index 80b03af..337d84a 100644
> > --- a/tools/testing/selftests/net/Makefile
> > +++ b/tools/testing/selftests/net/Makefile
> > @@ -10,6 +10,8 @@ TEST_GEN_FILES += psock_fanout psock_tpacket msg_zerocopy
> >   TEST_GEN_PROGS = reuseport_bpf reuseport_bpf_cpu reuseport_bpf_numa
> >   TEST_GEN_PROGS += reuseport_dualstack reuseaddr_conflict
> >
> > +TEST_FILES := settings
> > +
> >   KSFT_KHDR_INSTALL := 1
> >   include ../lib.mk
> >
> > diff --git a/tools/testing/selftests/net/settings b/tools/testing/selftests/net/settings
> > new file mode 100644
> > index 0000000..694d707
> > --- /dev/null
> > +++ b/tools/testing/selftests/net/settings
> > @@ -0,0 +1 @@
> > +timeout=300
> >
>
>
Stefan Bader May 5, 2021, 1:16 p.m. UTC | #3 
On 05.05.21 12:18, Po-Hsu Lin wrote:
> On Wed, May 5, 2021 at 5:00 PM Stefan Bader <stefan.bader@canonical.com> wrote:
>>
>> On 05.05.21 08:20, Po-Hsu Lin wrote:
>>> BugLink: https://bugs.launchpad.net/bugs/1856010
>>>
>>> We found that with the latest mainline kernel (5.12.0-051200rc8) on
>>> some KVM instances / bare-metal systems, the following tests will take
>>> longer than the kselftest framework default timeout (45 seconds) to
>>> run and thus got terminated with TIMEOUT error:
>>> * xfrm_policy.sh - took about 2m20s
>>> * pmtu.sh - took about 3m5s
>>> * udpgso_bench.sh - took about 60s
>>>
>>> Bump the timeout setting to 5 minutes to allow them have a chance to
>>> finish.
>>
>> In the cover email you mention this is only needed for Bionic because newer
>> series disable the timeout completely. Just even with that info, the commit
>> message sounds odd to me. Why would timing of a 5.12 kernel matter for bionic?
>>
>> -Stefan
> 
> Hi Stefan,
> 
> In order to submit this patch to upstream, I have it tested with 5.12
> as a justification
> Theoretically any kernel that has the commit which introduced the
> default 45 seconds timeout can be affected by this issue.
> 
> I took a closer look in the net test directory again, it looks like
> bionic does not have these three test cases.
> Maybe that's why we didn't add our sauce patch to disable the timeout on Bionic.
> Base on this fact I am ok with the decision either to drop this or
> keep SRU it to make it align with upstream code.

Oh, ok, so the whole message is what went upstream that way. Hm, maybe in this 
case a short explanation why backporting to bionic in the s-o-b area would be 
helpful. Just looking at git log could otherwise be a bit confusing. Maybe like 
below?

-Stefan

> Thanks
> Sam
> 
>>>
>>> https://bugs.launchpad.net/bugs/1856010
>>> Signed-off-by: Po-Hsu Lin <po-hsu.lin@canonical.com>
>>> Signed-off-by: David S. Miller <davem@davemloft.net>
>>> (Backported from commit b881d089c7c9c7032da812cda1b4b0818f477780)
[not strictly needed, but to have this aligned across series]
>>> [PHLin: context adjustment]
>>> Signed-off-by: Po-Hsu Lin <po-hsu.lin@canonical.com>
>>> ---
>>>    tools/testing/selftests/net/Makefile | 2 ++
>>>    tools/testing/selftests/net/settings | 1 +
>>>    2 files changed, 3 insertions(+)
>>>    create mode 100644 tools/testing/selftests/net/settings
>>>
>>> diff --git a/tools/testing/selftests/net/Makefile b/tools/testing/selftests/net/Makefile
>>> index 80b03af..337d84a 100644
>>> --- a/tools/testing/selftests/net/Makefile
>>> +++ b/tools/testing/selftests/net/Makefile
>>> @@ -10,6 +10,8 @@ TEST_GEN_FILES += psock_fanout psock_tpacket msg_zerocopy
>>>    TEST_GEN_PROGS = reuseport_bpf reuseport_bpf_cpu reuseport_bpf_numa
>>>    TEST_GEN_PROGS += reuseport_dualstack reuseaddr_conflict
>>>
>>> +TEST_FILES := settings
>>> +
>>>    KSFT_KHDR_INSTALL := 1
>>>    include ../lib.mk
>>>
>>> diff --git a/tools/testing/selftests/net/settings b/tools/testing/selftests/net/settings
>>> new file mode 100644
>>> index 0000000..694d707
>>> --- /dev/null
>>> +++ b/tools/testing/selftests/net/settings
>>> @@ -0,0 +1 @@
>>> +timeout=300
>>>
>>
>>
Po-Hsu Lin May 6, 2021, 10:03 a.m. UTC | #4 
On Wed, May 5, 2021 at 9:16 PM Stefan Bader <stefan.bader@canonical.com> wrote:
>
> On 05.05.21 12:18, Po-Hsu Lin wrote:
> > On Wed, May 5, 2021 at 5:00 PM Stefan Bader <stefan.bader@canonical.com> wrote:
> >>
> >> On 05.05.21 08:20, Po-Hsu Lin wrote:
> >>> BugLink: https://bugs.launchpad.net/bugs/1856010
> >>>
> >>> We found that with the latest mainline kernel (5.12.0-051200rc8) on
> >>> some KVM instances / bare-metal systems, the following tests will take
> >>> longer than the kselftest framework default timeout (45 seconds) to
> >>> run and thus got terminated with TIMEOUT error:
> >>> * xfrm_policy.sh - took about 2m20s
> >>> * pmtu.sh - took about 3m5s
> >>> * udpgso_bench.sh - took about 60s
> >>>
> >>> Bump the timeout setting to 5 minutes to allow them have a chance to
> >>> finish.
> >>
> >> In the cover email you mention this is only needed for Bionic because newer
> >> series disable the timeout completely. Just even with that info, the commit
> >> message sounds odd to me. Why would timing of a 5.12 kernel matter for bionic?
> >>
> >> -Stefan
> >
> > Hi Stefan,
> >
> > In order to submit this patch to upstream, I have it tested with 5.12
> > as a justification
> > Theoretically any kernel that has the commit which introduced the
> > default 45 seconds timeout can be affected by this issue.
> >
> > I took a closer look in the net test directory again, it looks like
> > bionic does not have these three test cases.
> > Maybe that's why we didn't add our sauce patch to disable the timeout on Bionic.
> > Base on this fact I am ok with the decision either to drop this or
> > keep SRU it to make it align with upstream code.
>
> Oh, ok, so the whole message is what went upstream that way. Hm, maybe in this
> case a short explanation why backporting to bionic in the s-o-b area would be
> helpful. Just looking at git log could otherwise be a bit confusing. Maybe like
> below?
>
> -Stefan
Yeah you're right, by just looking the commit message itself is a bit
confusing. I think your suggestion is looking good.
Thanks.
Sam

>
> > Thanks
> > Sam
> >
> >>>
> >>> https://bugs.launchpad.net/bugs/1856010
> >>> Signed-off-by: Po-Hsu Lin <po-hsu.lin@canonical.com>
> >>> Signed-off-by: David S. Miller <davem@davemloft.net>
> >>> (Backported from commit b881d089c7c9c7032da812cda1b4b0818f477780)
> [not strictly needed, but to have this aligned across series]
> >>> [PHLin: context adjustment]
> >>> Signed-off-by: Po-Hsu Lin <po-hsu.lin@canonical.com>
> >>> ---
> >>>    tools/testing/selftests/net/Makefile | 2 ++
> >>>    tools/testing/selftests/net/settings | 1 +
> >>>    2 files changed, 3 insertions(+)
> >>>    create mode 100644 tools/testing/selftests/net/settings
> >>>
> >>> diff --git a/tools/testing/selftests/net/Makefile b/tools/testing/selftests/net/Makefile
> >>> index 80b03af..337d84a 100644
> >>> --- a/tools/testing/selftests/net/Makefile
> >>> +++ b/tools/testing/selftests/net/Makefile
> >>> @@ -10,6 +10,8 @@ TEST_GEN_FILES += psock_fanout psock_tpacket msg_zerocopy
> >>>    TEST_GEN_PROGS = reuseport_bpf reuseport_bpf_cpu reuseport_bpf_numa
> >>>    TEST_GEN_PROGS += reuseport_dualstack reuseaddr_conflict
> >>>
> >>> +TEST_FILES := settings
> >>> +
> >>>    KSFT_KHDR_INSTALL := 1
> >>>    include ../lib.mk
> >>>
> >>> diff --git a/tools/testing/selftests/net/settings b/tools/testing/selftests/net/settings
> >>> new file mode 100644
> >>> index 0000000..694d707
> >>> --- /dev/null
> >>> +++ b/tools/testing/selftests/net/settings
> >>> @@ -0,0 +1 @@
> >>> +timeout=300
> >>>
> >>
> >>
>
>
Stefan Bader May 7, 2021, 7:33 a.m. UTC | #5 
On 05.05.21 08:20, Po-Hsu Lin wrote:
> BugLink: https://bugs.launchpad.net/bugs/1856010
> 
> We found that with the latest mainline kernel (5.12.0-051200rc8) on
> some KVM instances / bare-metal systems, the following tests will take
> longer than the kselftest framework default timeout (45 seconds) to
> run and thus got terminated with TIMEOUT error:
> * xfrm_policy.sh - took about 2m20s
> * pmtu.sh - took about 3m5s
> * udpgso_bench.sh - took about 60s
> 
> Bump the timeout setting to 5 minutes to allow them have a chance to
> finish.
> 
> https://bugs.launchpad.net/bugs/1856010
> Signed-off-by: Po-Hsu Lin <po-hsu.lin@canonical.com>
> Signed-off-by: David S. Miller <davem@davemloft.net>
> (Backported from commit b881d089c7c9c7032da812cda1b4b0818f477780)
[not strictly needed, but to have this aligned across series]
> [PHLin: context adjustment]
> Signed-off-by: Po-Hsu Lin <po-hsu.lin@canonical.com>
Acked-by: Stefan Bader <stefan.bader@canonical.com>
> ---

With clarification added.

>   tools/testing/selftests/net/Makefile | 2 ++
>   tools/testing/selftests/net/settings | 1 +
>   2 files changed, 3 insertions(+)
>   create mode 100644 tools/testing/selftests/net/settings
> 
> diff --git a/tools/testing/selftests/net/Makefile b/tools/testing/selftests/net/Makefile
> index 80b03af..337d84a 100644
> --- a/tools/testing/selftests/net/Makefile
> +++ b/tools/testing/selftests/net/Makefile
> @@ -10,6 +10,8 @@ TEST_GEN_FILES += psock_fanout psock_tpacket msg_zerocopy
>   TEST_GEN_PROGS = reuseport_bpf reuseport_bpf_cpu reuseport_bpf_numa
>   TEST_GEN_PROGS += reuseport_dualstack reuseaddr_conflict
>   
> +TEST_FILES := settings
> +
>   KSFT_KHDR_INSTALL := 1
>   include ../lib.mk
>   
> diff --git a/tools/testing/selftests/net/settings b/tools/testing/selftests/net/settings
> new file mode 100644
> index 0000000..694d707
> --- /dev/null
> +++ b/tools/testing/selftests/net/settings
> @@ -0,0 +1 @@
> +timeout=300
>
diff mbox series

Patch

diff --git a/tools/testing/selftests/net/Makefile b/tools/testing/selftests/net/Makefile
index 80b03af..337d84a 100644
--- a/tools/testing/selftests/net/Makefile
+++ b/tools/testing/selftests/net/Makefile
@@ -10,6 +10,8 @@  TEST_GEN_FILES += psock_fanout psock_tpacket msg_zerocopy
 TEST_GEN_PROGS = reuseport_bpf reuseport_bpf_cpu reuseport_bpf_numa
 TEST_GEN_PROGS += reuseport_dualstack reuseaddr_conflict
 
+TEST_FILES := settings
+
 KSFT_KHDR_INSTALL := 1
 include ../lib.mk
 
diff --git a/tools/testing/selftests/net/settings b/tools/testing/selftests/net/settings
new file mode 100644
index 0000000..694d707
--- /dev/null
+++ b/tools/testing/selftests/net/settings
@@ -0,0 +1 @@ 
+timeout=300