Linux-kselftest Archive on lore.kernel.org
 help / color / Atom feed
* [PATCH] selftests: txtimestamp: tear down setup() 'tc' and 'ip' env on EXIT
@ 2020-07-21 14:52 Paolo Pisati
  2020-07-21 15:04 ` Willem de Bruijn
  0 siblings, 1 reply; 7+ messages in thread
From: Paolo Pisati @ 2020-07-21 14:52 UTC (permalink / raw)
  To: David S . Miller, Jakub Kicinski, Shuah Khan, Willem de Bruijn,
	Jian Yang
  Cc: netdev, linux-kselftest, linux-kernel

Add a cleanup() path upon exit, making it possible to run the test twice in a
row:

$ sudo bash -x ./txtimestamp.sh
+ set -e
++ ip netns identify
+ [[ '' == \r\o\o\t ]]
+ main
+ [[ 0 -eq 0 ]]
+ run_test_all
+ setup
+ tc qdisc add dev lo root netem delay 1ms
Error: Exclusivity flag on, cannot modify.

Signed-off-by: Paolo Pisati <paolo.pisati@canonical.com>
---
 tools/testing/selftests/net/txtimestamp.sh | 10 ++++++++++
 1 file changed, 10 insertions(+)

diff --git a/tools/testing/selftests/net/txtimestamp.sh b/tools/testing/selftests/net/txtimestamp.sh
index eea6f5193693..77f29cabff87 100755
--- a/tools/testing/selftests/net/txtimestamp.sh
+++ b/tools/testing/selftests/net/txtimestamp.sh
@@ -23,6 +23,14 @@ setup() {
 		action mirred egress redirect dev ifb_netem0
 }
 
+cleanup() {
+	tc filter del dev lo parent ffff:
+	tc qdisc del dev lo handle ffff: ingress
+	tc qdisc del dev ifb_netem0 root
+	ip link del ifb_netem0
+	tc qdisc del dev lo root
+}
+
 run_test_v4v6() {
 	# SND will be delayed 1000us
 	# ACK will be delayed 6000us: 1 + 2 ms round-trip
@@ -75,6 +83,8 @@ main() {
 	fi
 }
 
+trap cleanup EXIT
+
 if [[ "$(ip netns identify)" == "root" ]]; then
 	./in_netns.sh $0 $@
 else
-- 
2.27.0


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

* Re: [PATCH] selftests: txtimestamp: tear down setup() 'tc' and 'ip' env on EXIT
  2020-07-21 14:52 [PATCH] selftests: txtimestamp: tear down setup() 'tc' and 'ip' env on EXIT Paolo Pisati
@ 2020-07-21 15:04 ` Willem de Bruijn
  2020-07-21 16:17   ` [PATCH v2] selftest: txtimestamp: fix net ns entry logic Paolo Pisati
  0 siblings, 1 reply; 7+ messages in thread
From: Willem de Bruijn @ 2020-07-21 15:04 UTC (permalink / raw)
  To: Paolo Pisati
  Cc: David S . Miller, Jakub Kicinski, Shuah Khan, Jian Yang,
	Network Development, open list:KERNEL SELFTEST FRAMEWORK,
	linux-kernel

On Tue, Jul 21, 2020 at 10:52 AM Paolo Pisati
<paolo.pisati@canonical.com> wrote:
>
> Add a cleanup() path upon exit, making it possible to run the test twice in a
> row:
>
> $ sudo bash -x ./txtimestamp.sh
> + set -e
> ++ ip netns identify
> + [[ '' == \r\o\o\t ]]
> + main
> + [[ 0 -eq 0 ]]
> + run_test_all
> + setup
> + tc qdisc add dev lo root netem delay 1ms
> Error: Exclusivity flag on, cannot modify.
>
> Signed-off-by: Paolo Pisati <paolo.pisati@canonical.com>

The test should already clean up after itself, by being run inside a
network namespace. That is a more robust method to ensure that all
state is reset.

The issue here is that the else branch is taken in

  if [[ "$(ip netns identify)" == "root" ]]; then
          ./in_netns.sh $0 $@
  else
          main $@
  fi

because the ip netns identify usually returns an empty string, not
"root". If we fix that, no need to add additional cleanup.

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

* [PATCH v2] selftest: txtimestamp: fix net ns entry logic
  2020-07-21 15:04 ` Willem de Bruijn
@ 2020-07-21 16:17   ` Paolo Pisati
  2020-07-21 16:25     ` Willem de Bruijn
  2020-07-21 23:11     ` David Miller
  0 siblings, 2 replies; 7+ messages in thread
From: Paolo Pisati @ 2020-07-21 16:17 UTC (permalink / raw)
  To: David S . Miller, Jakub Kicinski, Shuah Khan, Willem de Bruijn,
	Jian Yang
  Cc: netdev, linux-kselftest, linux-kernel

According to 'man 8 ip-netns', if `ip netns identify` returns an empty string,
there's no net namespace associated with current PID: fix the net ns entrance
logic.

Signed-off-by: Paolo Pisati <paolo.pisati@canonical.com>
---
 tools/testing/selftests/net/txtimestamp.sh | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tools/testing/selftests/net/txtimestamp.sh b/tools/testing/selftests/net/txtimestamp.sh
index eea6f5193693..31637769f59f 100755
--- a/tools/testing/selftests/net/txtimestamp.sh
+++ b/tools/testing/selftests/net/txtimestamp.sh
@@ -75,7 +75,7 @@ main() {
 	fi
 }
 
-if [[ "$(ip netns identify)" == "root" ]]; then
+if [[ -z "$(ip netns identify)" ]]; then
 	./in_netns.sh $0 $@
 else
 	main $@
-- 
2.27.0


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

* Re: [PATCH v2] selftest: txtimestamp: fix net ns entry logic
  2020-07-21 16:17   ` [PATCH v2] selftest: txtimestamp: fix net ns entry logic Paolo Pisati
@ 2020-07-21 16:25     ` Willem de Bruijn
  2020-07-22  8:37       ` Paolo Pisati
  2020-07-21 23:11     ` David Miller
  1 sibling, 1 reply; 7+ messages in thread
From: Willem de Bruijn @ 2020-07-21 16:25 UTC (permalink / raw)
  To: Paolo Pisati
  Cc: David S . Miller, Jakub Kicinski, Shuah Khan, Jian Yang,
	Network Development, open list:KERNEL SELFTEST FRAMEWORK,
	linux-kernel

On Tue, Jul 21, 2020 at 12:17 PM Paolo Pisati
<paolo.pisati@canonical.com> wrote:
>
> According to 'man 8 ip-netns', if `ip netns identify` returns an empty string,
> there's no net namespace associated with current PID: fix the net ns entrance
> logic.
>
> Signed-off-by: Paolo Pisati <paolo.pisati@canonical.com>

Fixes: cda261f421ba ("selftests: add txtimestamp kselftest")

Acked-by: Willem de Bruijn <willemb@google.com>

Thanks for the fix.

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

* Re: [PATCH v2] selftest: txtimestamp: fix net ns entry logic
  2020-07-21 16:17   ` [PATCH v2] selftest: txtimestamp: fix net ns entry logic Paolo Pisati
  2020-07-21 16:25     ` Willem de Bruijn
@ 2020-07-21 23:11     ` David Miller
  1 sibling, 0 replies; 7+ messages in thread
From: David Miller @ 2020-07-21 23:11 UTC (permalink / raw)
  To: paolo.pisati
  Cc: kuba, shuah, willemb, jianyang, netdev, linux-kselftest, linux-kernel

From: Paolo Pisati <paolo.pisati@canonical.com>
Date: Tue, 21 Jul 2020 18:17:10 +0200

> According to 'man 8 ip-netns', if `ip netns identify` returns an empty string,
> there's no net namespace associated with current PID: fix the net ns entrance
> logic.
> 
> Signed-off-by: Paolo Pisati <paolo.pisati@canonical.com>

Applied, thanks.

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

* Re: [PATCH v2] selftest: txtimestamp: fix net ns entry logic
  2020-07-21 16:25     ` Willem de Bruijn
@ 2020-07-22  8:37       ` Paolo Pisati
  2020-07-22 12:49         ` Willem de Bruijn
  0 siblings, 1 reply; 7+ messages in thread
From: Paolo Pisati @ 2020-07-22  8:37 UTC (permalink / raw)
  To: Willem de Bruijn
  Cc: David S . Miller, Jakub Kicinski, Shuah Khan, Jian Yang,
	Network Development, open list:KERNEL SELFTEST FRAMEWORK,
	linux-kernel

On Tue, Jul 21, 2020 at 6:26 PM Willem de Bruijn
<willemdebruijn.kernel@gmail.com> wrote:
>
> Fixes: cda261f421ba ("selftests: add txtimestamp kselftest")
>
> Acked-by: Willem de Bruijn <willemb@google.com>

Besides, is it just me or this test fails frequently? I've been
running it on 5.4.x, 5.7.x and 5.8-rcX and it often fails:

...
    USR: 1595405084 s 947366 us (seq=0, len=0)
    SND: 1595405084 s 948686 us (seq=9, len=10)  (USR +1319 us)
ERROR: 6542 us expected between 6000 and 6500
    ACK: 1595405084 s 953908 us (seq=9, len=10)  (USR +6541 us)
    USR: 1595405084 s 997979 us (seq=0, len=0)
    SND: 1595405084 s 999101 us (seq=19, len=10)  (USR +1121 us)
    ACK: 1595405085 s 4438 us (seq=19, len=10)  (USR +6458 us)
    USR: 1595405085 s 49317 us (seq=0, len=0)
    SND: 1595405085 s 50680 us (seq=29, len=10)  (USR +1363 us)
ERROR: 6661 us expected between 6000 and 6500
    ACK: 1595405085 s 55978 us (seq=29, len=10)  (USR +6661 us)
    USR: 1595405085 s 101049 us (seq=0, len=0)
    SND: 1595405085 s 102342 us (seq=39, len=10)  (USR +1293 us)
ERROR: 6578 us expected between 6000 and 6500
    ACK: 1595405085 s 107627 us (seq=39, len=10)  (USR +6577 us)
    USR-SND: count=4, avg=1274 us, min=1121 us, max=1363 us
    USR-ACK: count=4, avg=6559 us, min=6458 us, max=6661 us


In particular, "run_test_v4v6 ${args}       # tcp" is the most
susceptible to failures (though i've seen the udp variant fail too).
-- 
bye,
p.

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

* Re: [PATCH v2] selftest: txtimestamp: fix net ns entry logic
  2020-07-22  8:37       ` Paolo Pisati
@ 2020-07-22 12:49         ` Willem de Bruijn
  0 siblings, 0 replies; 7+ messages in thread
From: Willem de Bruijn @ 2020-07-22 12:49 UTC (permalink / raw)
  To: Paolo Pisati
  Cc: Willem de Bruijn, David S . Miller, Jakub Kicinski, Shuah Khan,
	Jian Yang, Network Development,
	open list:KERNEL SELFTEST FRAMEWORK, linux-kernel

On Wed, Jul 22, 2020 at 4:37 AM Paolo Pisati <paolo.pisati@canonical.com> wrote:
>
> On Tue, Jul 21, 2020 at 6:26 PM Willem de Bruijn
> <willemdebruijn.kernel@gmail.com> wrote:
> >
> > Fixes: cda261f421ba ("selftests: add txtimestamp kselftest")
> >
> > Acked-by: Willem de Bruijn <willemb@google.com>
>
> Besides, is it just me or this test fails frequently? I've been
> running it on 5.4.x, 5.7.x and 5.8-rcX and it often fails:
>
> ...
>     USR: 1595405084 s 947366 us (seq=0, len=0)
>     SND: 1595405084 s 948686 us (seq=9, len=10)  (USR +1319 us)
> ERROR: 6542 us expected between 6000 and 6500
>     ACK: 1595405084 s 953908 us (seq=9, len=10)  (USR +6541 us)
>     USR: 1595405084 s 997979 us (seq=0, len=0)
>     SND: 1595405084 s 999101 us (seq=19, len=10)  (USR +1121 us)
>     ACK: 1595405085 s 4438 us (seq=19, len=10)  (USR +6458 us)
>     USR: 1595405085 s 49317 us (seq=0, len=0)
>     SND: 1595405085 s 50680 us (seq=29, len=10)  (USR +1363 us)
> ERROR: 6661 us expected between 6000 and 6500
>     ACK: 1595405085 s 55978 us (seq=29, len=10)  (USR +6661 us)
>     USR: 1595405085 s 101049 us (seq=0, len=0)
>     SND: 1595405085 s 102342 us (seq=39, len=10)  (USR +1293 us)
> ERROR: 6578 us expected between 6000 and 6500
>     ACK: 1595405085 s 107627 us (seq=39, len=10)  (USR +6577 us)
>     USR-SND: count=4, avg=1274 us, min=1121 us, max=1363 us
>     USR-ACK: count=4, avg=6559 us, min=6458 us, max=6661 us
>
>
> In particular, "run_test_v4v6 ${args}       # tcp" is the most
> susceptible to failures (though i've seen the udp variant fail too).

Not for me. The interval bounds have been set as is based on previous
experience.

Are you running it inside a VM? Especially qemu without kvm
acceleration could increase jitter.

The reports are not far outside the bounds. They can be extended a bit
if that considerably reduces flakiness.

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

end of thread, back to index

Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-07-21 14:52 [PATCH] selftests: txtimestamp: tear down setup() 'tc' and 'ip' env on EXIT Paolo Pisati
2020-07-21 15:04 ` Willem de Bruijn
2020-07-21 16:17   ` [PATCH v2] selftest: txtimestamp: fix net ns entry logic Paolo Pisati
2020-07-21 16:25     ` Willem de Bruijn
2020-07-22  8:37       ` Paolo Pisati
2020-07-22 12:49         ` Willem de Bruijn
2020-07-21 23:11     ` David Miller

Linux-kselftest Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/linux-kselftest/0 linux-kselftest/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 linux-kselftest linux-kselftest/ https://lore.kernel.org/linux-kselftest \
		linux-kselftest@vger.kernel.org
	public-inbox-index linux-kselftest

Example config snippet for mirrors

Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.kernel.vger.linux-kselftest


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