diff mbox series

[autotools-client-tests] UBUNTU: SAUCE: ubuntu_kernel_selftests: disable txtimestamp.sh

Message ID 20200812115919.41378-1-paolo.pisati@canonical.com
State New
Headers show
Series [autotools-client-tests] UBUNTU: SAUCE: ubuntu_kernel_selftests: disable txtimestamp.sh | expand

Commit Message

Paolo Pisati Aug. 12, 2020, 11:59 a.m. UTC
The timestamp net selftests exercise the SOF_TIMESTAMPING* code in the network stack
under a variety of conditions (ipv4/ipv6, tcp/udp/raw/raw_ipproto/pf_packet,
data/nodata, setsockopt/cmsg), by sending packets round-trip over loopback (but
through netem to simulate delays), and verifying that timestamps in pkts correspond
to the expected delay (plus a small delta ~500us to accommodate "noise").

Unfortunately, txtimestamp has shown a very erratic behaviour for the entire
Groovy dev cycle.

Originally, the test was merged upstream around v5.0 in
toos/testing/selftest/networking/timestamping:

commit cda261f421ba2682e25f46beb229172706fc7fcd
Author: Willem de Bruijn <willemb@google.com>
Date:   Thu Dec 20 16:22:54 2018 -0500

    selftests: add txtimestamp kselftest

but disconnected from the rest of kernel selftests (and our
autotools-client-tests). We started executing it (and seeing failures) in the
Groovy cycle after it was moved to selftest/net and hooked to kernel selftests
(commit 5ef5c90e3cb3 "selftests: move timestamping selftests to net folder"),
but manually running it in a Focal environment shows the same inconsistencies.

By executing the test in a 10 iteration loop, the chances it doesn't complete
successfully are very high (and running it in VM makes the failure ratio go up
significantly):

$ cd linux/tools/testing/selftests/net
$ make
...
$ cat << EOF > test.sh
#!/bin/bash

THRE=10;
while /bin/true; do
        cnt=0;
        while [ $cnt -lt $THRE ]; do
                cnt=$((cnt+1));
                sudo ./txtimestamp.sh > /dev/null 2> ./timestamp.log;
                [ $? -ne 0 ] && echo "Fail at cycle $cnt" && break;
        done;
        [ $cnt -ge $THRE ] && echo "Successfully completed a $THRE-iteration cycle";
done
EOF

$ chmod +x test.sh
$ ./test.sh > txtimestamp.log

and let it soak for a bit:

...
Fail at cycle 3
Fail at cycle 1
Fail at cycle 5
Successfully completed a 10-iteration cycle
Successfully completed a 10-iteration cycle
Successfully completed a 10-iteration cycle
Fail at cycle 6
Successfully completed a 10-iteration cycle
Fail at cycle 8
Fail at cycle 9
Successfully completed a 10-iteration cycle
Fail at cycle 2
Fail at cycle 10
Successfully completed a 10-iteration cycle
Successfully completed a 10-iteration cycle
Fail at cycle 3
Fail at cycle 9
Fail at cycle 9
Fail at cycle 1
Fail at cycle 6
Successfully completed a 10-iteration cycle
Fail at cycle 7
Fail at cycle 9
Successfully completed a 10-iteration cycle
Successfully completed a 10-iteration cycle
Fail at cycle 6
Fail at cycle 7
Fail at cycle 6
Fail at cycle 5
Successfully completed a 10-iteration cycle
Successfully completed a 10-iteration cycle
Fail at cycle 1
Fail at cycle 4
...

For all these reasons, i don't think we should mark the entire selftests/net as
FAILED based on txtimestamp results and actually disconnect it from autotools.

Signed-off-by: Paolo Pisati <paolo.pisati@canonical.com>
---
 ubuntu_kernel_selftests/ubuntu_kernel_selftests.py | 8 ++++++++
 1 file changed, 8 insertions(+)

Comments

Colin King Aug. 12, 2020, 12:15 p.m. UTC | #1
On 12/08/2020 12:59, Paolo Pisati wrote:
> The timestamp net selftests exercise the SOF_TIMESTAMPING* code in the network stack
> under a variety of conditions (ipv4/ipv6, tcp/udp/raw/raw_ipproto/pf_packet,
> data/nodata, setsockopt/cmsg), by sending packets round-trip over loopback (but
> through netem to simulate delays), and verifying that timestamps in pkts correspond
> to the expected delay (plus a small delta ~500us to accommodate "noise").
> 
> Unfortunately, txtimestamp has shown a very erratic behaviour for the entire
> Groovy dev cycle.
> 
> Originally, the test was merged upstream around v5.0 in
> toos/testing/selftest/networking/timestamping:
> 
> commit cda261f421ba2682e25f46beb229172706fc7fcd
> Author: Willem de Bruijn <willemb@google.com>
> Date:   Thu Dec 20 16:22:54 2018 -0500
> 
>     selftests: add txtimestamp kselftest
> 
> but disconnected from the rest of kernel selftests (and our
> autotools-client-tests). We started executing it (and seeing failures) in the
> Groovy cycle after it was moved to selftest/net and hooked to kernel selftests
> (commit 5ef5c90e3cb3 "selftests: move timestamping selftests to net folder"),
> but manually running it in a Focal environment shows the same inconsistencies.
> 
> By executing the test in a 10 iteration loop, the chances it doesn't complete
> successfully are very high (and running it in VM makes the failure ratio go up
> significantly):
> 
> $ cd linux/tools/testing/selftests/net
> $ make
> ...
> $ cat << EOF > test.sh
> #!/bin/bash
> 
> THRE=10;
> while /bin/true; do
>         cnt=0;
>         while [ $cnt -lt $THRE ]; do
>                 cnt=$((cnt+1));
>                 sudo ./txtimestamp.sh > /dev/null 2> ./timestamp.log;
>                 [ $? -ne 0 ] && echo "Fail at cycle $cnt" && break;
>         done;
>         [ $cnt -ge $THRE ] && echo "Successfully completed a $THRE-iteration cycle";
> done
> EOF
> 
> $ chmod +x test.sh
> $ ./test.sh > txtimestamp.log
> 
> and let it soak for a bit:
> 
> ...
> Fail at cycle 3
> Fail at cycle 1
> Fail at cycle 5
> Successfully completed a 10-iteration cycle
> Successfully completed a 10-iteration cycle
> Successfully completed a 10-iteration cycle
> Fail at cycle 6
> Successfully completed a 10-iteration cycle
> Fail at cycle 8
> Fail at cycle 9
> Successfully completed a 10-iteration cycle
> Fail at cycle 2
> Fail at cycle 10
> Successfully completed a 10-iteration cycle
> Successfully completed a 10-iteration cycle
> Fail at cycle 3
> Fail at cycle 9
> Fail at cycle 9
> Fail at cycle 1
> Fail at cycle 6
> Successfully completed a 10-iteration cycle
> Fail at cycle 7
> Fail at cycle 9
> Successfully completed a 10-iteration cycle
> Successfully completed a 10-iteration cycle
> Fail at cycle 6
> Fail at cycle 7
> Fail at cycle 6
> Fail at cycle 5
> Successfully completed a 10-iteration cycle
> Successfully completed a 10-iteration cycle
> Fail at cycle 1
> Fail at cycle 4
> ...
> 
> For all these reasons, i don't think we should mark the entire selftests/net as
> FAILED based on txtimestamp results and actually disconnect it from autotools.
> 
> Signed-off-by: Paolo Pisati <paolo.pisati@canonical.com>
> ---
>  ubuntu_kernel_selftests/ubuntu_kernel_selftests.py | 8 ++++++++
>  1 file changed, 8 insertions(+)
> 
> diff --git a/ubuntu_kernel_selftests/ubuntu_kernel_selftests.py b/ubuntu_kernel_selftests/ubuntu_kernel_selftests.py
> index e958e537..dc75aec2 100644
> --- a/ubuntu_kernel_selftests/ubuntu_kernel_selftests.py
> +++ b/ubuntu_kernel_selftests/ubuntu_kernel_selftests.py
> @@ -195,6 +195,14 @@ class ubuntu_kernel_selftests(test.test):
>                  cmd = 'sed -i "s/ vmaccess//" ' + mk
>                  utils.system(cmd)
>  
> +            #
> +            # net/txtimestamp.sh is very fragile, disable it
> +            #
> +            fn = 'linux/tools/testing/selftests/net/Makefile'
> +            if os.path.exists(fn):
> +                cmd = 'sed -i "/^TEST_PROGS += txtimestamp.sh$/d" ' + fn
> +                utils.system(cmd)
> +
>      def run_once(self, test_name):
>          if test_name == 'setup':
>              return
> 

Yep, false positives because of fragile tests is a pain. It may be worth
reporting this upstream to see if they can get this working more
reliably in the future.

Acked-by: Colin Ian King <colin.king@canonical.com>
Marcelo Henrique Cerri Aug. 12, 2020, 12:43 p.m. UTC | #2
Acked-by: Marcelo Henrique Cerri <marcelo.cerri@canonical.com>

On Wed, Aug 12, 2020 at 01:59:19PM +0200, Paolo Pisati wrote:
> The timestamp net selftests exercise the SOF_TIMESTAMPING* code in the network stack
> under a variety of conditions (ipv4/ipv6, tcp/udp/raw/raw_ipproto/pf_packet,
> data/nodata, setsockopt/cmsg), by sending packets round-trip over loopback (but
> through netem to simulate delays), and verifying that timestamps in pkts correspond
> to the expected delay (plus a small delta ~500us to accommodate "noise").
> 
> Unfortunately, txtimestamp has shown a very erratic behaviour for the entire
> Groovy dev cycle.
> 
> Originally, the test was merged upstream around v5.0 in
> toos/testing/selftest/networking/timestamping:
> 
> commit cda261f421ba2682e25f46beb229172706fc7fcd
> Author: Willem de Bruijn <willemb@google.com>
> Date:   Thu Dec 20 16:22:54 2018 -0500
> 
>     selftests: add txtimestamp kselftest
> 
> but disconnected from the rest of kernel selftests (and our
> autotools-client-tests). We started executing it (and seeing failures) in the
> Groovy cycle after it was moved to selftest/net and hooked to kernel selftests
> (commit 5ef5c90e3cb3 "selftests: move timestamping selftests to net folder"),
> but manually running it in a Focal environment shows the same inconsistencies.
> 
> By executing the test in a 10 iteration loop, the chances it doesn't complete
> successfully are very high (and running it in VM makes the failure ratio go up
> significantly):
> 
> $ cd linux/tools/testing/selftests/net
> $ make
> ...
> $ cat << EOF > test.sh
> #!/bin/bash
> 
> THRE=10;
> while /bin/true; do
>         cnt=0;
>         while [ $cnt -lt $THRE ]; do
>                 cnt=$((cnt+1));
>                 sudo ./txtimestamp.sh > /dev/null 2> ./timestamp.log;
>                 [ $? -ne 0 ] && echo "Fail at cycle $cnt" && break;
>         done;
>         [ $cnt -ge $THRE ] && echo "Successfully completed a $THRE-iteration cycle";
> done
> EOF
> 
> $ chmod +x test.sh
> $ ./test.sh > txtimestamp.log
> 
> and let it soak for a bit:
> 
> ...
> Fail at cycle 3
> Fail at cycle 1
> Fail at cycle 5
> Successfully completed a 10-iteration cycle
> Successfully completed a 10-iteration cycle
> Successfully completed a 10-iteration cycle
> Fail at cycle 6
> Successfully completed a 10-iteration cycle
> Fail at cycle 8
> Fail at cycle 9
> Successfully completed a 10-iteration cycle
> Fail at cycle 2
> Fail at cycle 10
> Successfully completed a 10-iteration cycle
> Successfully completed a 10-iteration cycle
> Fail at cycle 3
> Fail at cycle 9
> Fail at cycle 9
> Fail at cycle 1
> Fail at cycle 6
> Successfully completed a 10-iteration cycle
> Fail at cycle 7
> Fail at cycle 9
> Successfully completed a 10-iteration cycle
> Successfully completed a 10-iteration cycle
> Fail at cycle 6
> Fail at cycle 7
> Fail at cycle 6
> Fail at cycle 5
> Successfully completed a 10-iteration cycle
> Successfully completed a 10-iteration cycle
> Fail at cycle 1
> Fail at cycle 4
> ...
> 
> For all these reasons, i don't think we should mark the entire selftests/net as
> FAILED based on txtimestamp results and actually disconnect it from autotools.
> 
> Signed-off-by: Paolo Pisati <paolo.pisati@canonical.com>
> ---
>  ubuntu_kernel_selftests/ubuntu_kernel_selftests.py | 8 ++++++++
>  1 file changed, 8 insertions(+)
> 
> diff --git a/ubuntu_kernel_selftests/ubuntu_kernel_selftests.py b/ubuntu_kernel_selftests/ubuntu_kernel_selftests.py
> index e958e537..dc75aec2 100644
> --- a/ubuntu_kernel_selftests/ubuntu_kernel_selftests.py
> +++ b/ubuntu_kernel_selftests/ubuntu_kernel_selftests.py
> @@ -195,6 +195,14 @@ class ubuntu_kernel_selftests(test.test):
>                  cmd = 'sed -i "s/ vmaccess//" ' + mk
>                  utils.system(cmd)
>  
> +            #
> +            # net/txtimestamp.sh is very fragile, disable it
> +            #
> +            fn = 'linux/tools/testing/selftests/net/Makefile'
> +            if os.path.exists(fn):
> +                cmd = 'sed -i "/^TEST_PROGS += txtimestamp.sh$/d" ' + fn
> +                utils.system(cmd)
> +
>      def run_once(self, test_name):
>          if test_name == 'setup':
>              return
> -- 
> 2.27.0
> 
> 
> -- 
> kernel-team mailing list
> kernel-team@lists.ubuntu.com
> https://lists.ubuntu.com/mailman/listinfo/kernel-team
Po-Hsu Lin Aug. 13, 2020, 2:07 a.m. UTC | #3
Your branch is not up-to-date, this patch cannot be applied
directly as it's missing commit e24a4b63b ("UBUNTU: SAUCE:
ubuntu_kernel_selftests: disable memory hotplug for ARM")

Applied with conflict resolved.

Please work on the up-to-date master branch if possible.
Thanks
Sam
diff mbox series

Patch

diff --git a/ubuntu_kernel_selftests/ubuntu_kernel_selftests.py b/ubuntu_kernel_selftests/ubuntu_kernel_selftests.py
index e958e537..dc75aec2 100644
--- a/ubuntu_kernel_selftests/ubuntu_kernel_selftests.py
+++ b/ubuntu_kernel_selftests/ubuntu_kernel_selftests.py
@@ -195,6 +195,14 @@  class ubuntu_kernel_selftests(test.test):
                 cmd = 'sed -i "s/ vmaccess//" ' + mk
                 utils.system(cmd)
 
+            #
+            # net/txtimestamp.sh is very fragile, disable it
+            #
+            fn = 'linux/tools/testing/selftests/net/Makefile'
+            if os.path.exists(fn):
+                cmd = 'sed -i "/^TEST_PROGS += txtimestamp.sh$/d" ' + fn
+                utils.system(cmd)
+
     def run_once(self, test_name):
         if test_name == 'setup':
             return