netdev.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH net-next 00/11] selftests: Various fixes
@ 2019-01-31 22:35 Petr Machata
  2019-01-31 22:35 ` [PATCH net-next 01/11] selftests: forwarding: Make ping timeout configurable Petr Machata
                   ` (11 more replies)
  0 siblings, 12 replies; 13+ messages in thread
From: Petr Machata @ 2019-01-31 22:35 UTC (permalink / raw)
  To: netdev; +Cc: Ido Schimmel

This patch set contains various fixes whose common denominator is
improving quality of forwarding and mlxsw selftests.

Most of the fixes are improvements in determinism (such that timing and
latency don't impact the test performance). These were prompted by
regular runs of the test suite on a hardware emulator, the performance
of which is necessarily lower than that of the real device.

Patches #1 (from Ido), #2 and #3 make changes to ping limits.

Patches #4 and #5 add more sleep in places where things need more time
to finish.

Patches #6 and #7 fix two tests in the suite of mirror-to-gretap tests
where underlay involves a VLAN device over an 802.1q bridge.

Patches #8, #9 and #10 fix bugs in mirror-to-gretap test where underlay
involves a LAG device.

Patch #11 fixes a missed RET initialization in mirror-to-gretap flower
test.

Ido Schimmel (1):
  selftests: forwarding: Make ping timeout configurable

Petr Machata (10):
  selftests: forwarding: mirror_lib: Update ping limits
  selftests: mlxsw: Update ping limits
  selftests: forwarding: mirror_gre_changes: Fix TTL test
  selftests: forwarding: mirror_lib: Wait for tardy mirrored packets
  selftests: forwarding: mirror_gre_vlan_bridge_1q: Fix untagged test
  selftests: forwarding: mirror_gre_vlan_bridge_1q: Fix roaming test
  selftests: forwarding: mirror_gre_bridge_1q_lag: Flush neighbors
  selftests: forwarding: mirror_gre_bridge_1q_lag: Enable forwarding
  selftests: forwarding: mirror_gre_bridge_1q_lag: Ignore ARP
  selftests: forwarding: mirror_gre_flower: Fix test result handling

 .../selftests/drivers/net/mlxsw/qos_dscp_bridge.sh |  3 +-
 .../selftests/drivers/net/mlxsw/qos_dscp_router.sh |  3 +-
 .../net/forwarding/forwarding.config.sample        |  3 +
 tools/testing/selftests/net/forwarding/lib.sh      |  7 +-
 .../net/forwarding/mirror_gre_bridge_1q_lag.sh     | 11 ++-
 .../selftests/net/forwarding/mirror_gre_changes.sh |  1 +
 .../selftests/net/forwarding/mirror_gre_flower.sh  |  4 +-
 .../net/forwarding/mirror_gre_vlan_bridge_1q.sh    | 88 +++++++++++++++++++---
 .../testing/selftests/net/forwarding/mirror_lib.sh |  5 +-
 .../selftests/net/forwarding/router_broadcast.sh   |  3 +-
 10 files changed, 107 insertions(+), 21 deletions(-)

-- 
2.4.11


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

* [PATCH net-next 01/11] selftests: forwarding: Make ping timeout configurable
  2019-01-31 22:35 [PATCH net-next 00/11] selftests: Various fixes Petr Machata
@ 2019-01-31 22:35 ` Petr Machata
  2019-01-31 22:35 ` [PATCH net-next 02/11] selftests: forwarding: mirror_lib: Update ping limits Petr Machata
                   ` (10 subsequent siblings)
  11 siblings, 0 replies; 13+ messages in thread
From: Petr Machata @ 2019-01-31 22:35 UTC (permalink / raw)
  To: netdev; +Cc: Ido Schimmel

From: Ido Schimmel <idosch@mellanox.com>

The current timeout (2 seconds) proved to be too low for some (emulated)
systems where we run the tests.

Make the timeout configurable and default to 5 seconds.

Signed-off-by: Ido Schimmel <idosch@mellanox.com>
Signed-off-by: Petr Machata <petrm@mellanox.com>
---
 tools/testing/selftests/drivers/net/mlxsw/qos_dscp_bridge.sh    | 2 +-
 tools/testing/selftests/drivers/net/mlxsw/qos_dscp_router.sh    | 2 +-
 tools/testing/selftests/net/forwarding/forwarding.config.sample | 3 +++
 tools/testing/selftests/net/forwarding/lib.sh                   | 7 +++++--
 tools/testing/selftests/net/forwarding/mirror_lib.sh            | 3 ++-
 tools/testing/selftests/net/forwarding/router_broadcast.sh      | 3 ++-
 6 files changed, 14 insertions(+), 6 deletions(-)

diff --git a/tools/testing/selftests/drivers/net/mlxsw/qos_dscp_bridge.sh b/tools/testing/selftests/drivers/net/mlxsw/qos_dscp_bridge.sh
index 1ca631d5aaba..111baedc5742 100755
--- a/tools/testing/selftests/drivers/net/mlxsw/qos_dscp_bridge.sh
+++ b/tools/testing/selftests/drivers/net/mlxsw/qos_dscp_bridge.sh
@@ -150,7 +150,7 @@ dscp_ping_test()
 
 	ip vrf exec $vrf_name \
 	   ${PING} -Q $dscp_10 ${sip:+-I $sip} $dip \
-		   -c 10 -i 0.1 -w 2 &> /dev/null
+		   -c 10 -i 0.1 -w $PING_TIMEOUT &> /dev/null
 
 	local -A t1s
 	eval "t1s=($(dscp_fetch_stats $dev_10 10)
diff --git a/tools/testing/selftests/drivers/net/mlxsw/qos_dscp_router.sh b/tools/testing/selftests/drivers/net/mlxsw/qos_dscp_router.sh
index 281d90766e12..6fa717ef91ef 100755
--- a/tools/testing/selftests/drivers/net/mlxsw/qos_dscp_router.sh
+++ b/tools/testing/selftests/drivers/net/mlxsw/qos_dscp_router.sh
@@ -171,7 +171,7 @@ dscp_ping_test()
 
 	ip vrf exec $vrf_name \
 	   ${PING} -Q $dscp ${sip:+-I $sip} $dip \
-		   -c 10 -i 0.1 -w 2 &> /dev/null
+		   -c 10 -i 0.1 -w $PING_TIMEOUT &> /dev/null
 
 	eval "local -A dev1_t1s=($(dscp_fetch_stats $dev1 0))"
 	eval "local -A dev2_t1s=($(dscp_fetch_stats $dev2 0))"
diff --git a/tools/testing/selftests/net/forwarding/forwarding.config.sample b/tools/testing/selftests/net/forwarding/forwarding.config.sample
index e819d049d9ce..e2adb533c8fc 100644
--- a/tools/testing/selftests/net/forwarding/forwarding.config.sample
+++ b/tools/testing/selftests/net/forwarding/forwarding.config.sample
@@ -33,3 +33,6 @@ PAUSE_ON_CLEANUP=no
 NETIF_TYPE=veth
 # Whether to create virtual interfaces (veth) or not
 NETIF_CREATE=yes
+# Timeout (in seconds) before ping exits regardless of how many packets have
+# been sent or received
+PING_TIMEOUT=5
diff --git a/tools/testing/selftests/net/forwarding/lib.sh b/tools/testing/selftests/net/forwarding/lib.sh
index c1f16bb992dc..9385dc971269 100644
--- a/tools/testing/selftests/net/forwarding/lib.sh
+++ b/tools/testing/selftests/net/forwarding/lib.sh
@@ -17,6 +17,7 @@ NETIF_TYPE=${NETIF_TYPE:=veth}
 NETIF_CREATE=${NETIF_CREATE:=yes}
 MCD=${MCD:=smcrouted}
 MC_CLI=${MC_CLI:=smcroutectl}
+PING_TIMEOUT=${PING_TIMEOUT:=5}
 
 relative_path="${BASH_SOURCE%/*}"
 if [[ "$relative_path" == "${BASH_SOURCE}" ]]; then
@@ -820,7 +821,8 @@ ping_do()
 	local vrf_name
 
 	vrf_name=$(master_name_get $if_name)
-	ip vrf exec $vrf_name $PING $args $dip -c 10 -i 0.1 -w 2 &> /dev/null
+	ip vrf exec $vrf_name \
+		$PING $args $dip -c 10 -i 0.1 -w $PING_TIMEOUT &> /dev/null
 }
 
 ping_test()
@@ -840,7 +842,8 @@ ping6_do()
 	local vrf_name
 
 	vrf_name=$(master_name_get $if_name)
-	ip vrf exec $vrf_name $PING6 $args $dip -c 10 -i 0.1 -w 2 &> /dev/null
+	ip vrf exec $vrf_name \
+		$PING6 $args $dip -c 10 -i 0.1 -w $PING_TIMEOUT &> /dev/null
 }
 
 ping6_test()
diff --git a/tools/testing/selftests/net/forwarding/mirror_lib.sh b/tools/testing/selftests/net/forwarding/mirror_lib.sh
index 07991e1025c7..a1c0389eb049 100644
--- a/tools/testing/selftests/net/forwarding/mirror_lib.sh
+++ b/tools/testing/selftests/net/forwarding/mirror_lib.sh
@@ -31,7 +31,8 @@ mirror_test()
 
 	local t0=$(tc_rule_stats_get $dev $pref)
 	ip vrf exec $vrf_name \
-	   ${PING} ${sip:+-I $sip} $dip -c 10 -i 0.1 -w 2 &> /dev/null
+	   ${PING} ${sip:+-I $sip} $dip -c 10 -i 0.1 -w $PING_TIMEOUT \
+	   &> /dev/null
 	local t1=$(tc_rule_stats_get $dev $pref)
 	local delta=$((t1 - t0))
 	# Tolerate a couple stray extra packets.
diff --git a/tools/testing/selftests/net/forwarding/router_broadcast.sh b/tools/testing/selftests/net/forwarding/router_broadcast.sh
index 7bd2ebb6e9de..9a678ece32b4 100755
--- a/tools/testing/selftests/net/forwarding/router_broadcast.sh
+++ b/tools/testing/selftests/net/forwarding/router_broadcast.sh
@@ -170,7 +170,8 @@ ping_test_from()
 
 	log_info "ping $dip, expected reply from $from"
 	ip vrf exec $(master_name_get $oif) \
-	$PING -I $oif $dip -c 10 -i 0.1 -w 2 -b 2>&1 | grep $from &> /dev/null
+		$PING -I $oif $dip -c 10 -i 0.1 -w $PING_TIMEOUT -b 2>&1 \
+		| grep $from &> /dev/null
 	check_err_fail $fail $?
 }
 
-- 
2.4.11


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

* [PATCH net-next 02/11] selftests: forwarding: mirror_lib: Update ping limits
  2019-01-31 22:35 [PATCH net-next 00/11] selftests: Various fixes Petr Machata
  2019-01-31 22:35 ` [PATCH net-next 01/11] selftests: forwarding: Make ping timeout configurable Petr Machata
@ 2019-01-31 22:35 ` Petr Machata
  2019-01-31 22:35 ` [PATCH net-next 03/11] selftests: mlxsw: " Petr Machata
                   ` (9 subsequent siblings)
  11 siblings, 0 replies; 13+ messages in thread
From: Petr Machata @ 2019-01-31 22:35 UTC (permalink / raw)
  To: netdev; +Cc: Ido Schimmel

The current ping intervals are too short for running mirroring tests in
simulator. This leads to ping sending a follow-up ping before the reply
arrives, thus sending more than the requested 10 ICMP requests. Those
are mirrored, and over a certain threshold the test case run is
considered a failure, because too much traffic is observed.

Bump interval and timeout numbers 5x in mirroring tests to address the
spurious failures.

Signed-off-by: Petr Machata <petrm@mellanox.com>
---
 tools/testing/selftests/net/forwarding/mirror_lib.sh | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/tools/testing/selftests/net/forwarding/mirror_lib.sh b/tools/testing/selftests/net/forwarding/mirror_lib.sh
index a1c0389eb049..878f8d53327a 100644
--- a/tools/testing/selftests/net/forwarding/mirror_lib.sh
+++ b/tools/testing/selftests/net/forwarding/mirror_lib.sh
@@ -29,10 +29,11 @@ mirror_test()
 	local pref=$1; shift
 	local expect=$1; shift
 
+	local ping_timeout=$((PING_TIMEOUT * 5))
 	local t0=$(tc_rule_stats_get $dev $pref)
 	ip vrf exec $vrf_name \
-	   ${PING} ${sip:+-I $sip} $dip -c 10 -i 0.1 -w $PING_TIMEOUT \
-	   &> /dev/null
+	   ${PING} ${sip:+-I $sip} $dip -c 10 -i 0.5 -w $ping_timeout \
+		   &> /dev/null
 	local t1=$(tc_rule_stats_get $dev $pref)
 	local delta=$((t1 - t0))
 	# Tolerate a couple stray extra packets.
-- 
2.4.11


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

* [PATCH net-next 03/11] selftests: mlxsw: Update ping limits
  2019-01-31 22:35 [PATCH net-next 00/11] selftests: Various fixes Petr Machata
  2019-01-31 22:35 ` [PATCH net-next 01/11] selftests: forwarding: Make ping timeout configurable Petr Machata
  2019-01-31 22:35 ` [PATCH net-next 02/11] selftests: forwarding: mirror_lib: Update ping limits Petr Machata
@ 2019-01-31 22:35 ` Petr Machata
  2019-01-31 22:35 ` [PATCH net-next 04/11] selftests: forwarding: mirror_gre_changes: Fix TTL test Petr Machata
                   ` (8 subsequent siblings)
  11 siblings, 0 replies; 13+ messages in thread
From: Petr Machata @ 2019-01-31 22:35 UTC (permalink / raw)
  To: netdev; +Cc: Ido Schimmel

The current ping intervals are too short for running mirroring tests in
simulator. This leads to ping sending a follow-up ping before the reply
arrives, thus sending more than the requested 10 ICMP requests. This
traffic is seen at the counters, and causes spurious failures.

Bump interval and timeout numbers 5x in mirroring tests to address the
spurious failures.

Signed-off-by: Petr Machata <petrm@mellanox.com>
---
 tools/testing/selftests/drivers/net/mlxsw/qos_dscp_bridge.sh | 3 ++-
 tools/testing/selftests/drivers/net/mlxsw/qos_dscp_router.sh | 3 ++-
 2 files changed, 4 insertions(+), 2 deletions(-)

diff --git a/tools/testing/selftests/drivers/net/mlxsw/qos_dscp_bridge.sh b/tools/testing/selftests/drivers/net/mlxsw/qos_dscp_bridge.sh
index 111baedc5742..40f16f2a3afd 100755
--- a/tools/testing/selftests/drivers/net/mlxsw/qos_dscp_bridge.sh
+++ b/tools/testing/selftests/drivers/net/mlxsw/qos_dscp_bridge.sh
@@ -148,9 +148,10 @@ dscp_ping_test()
 	eval "t0s=($(dscp_fetch_stats $dev_10 10)
 		   $(dscp_fetch_stats $dev_20 20))"
 
+	local ping_timeout=$((PING_TIMEOUT * 5))
 	ip vrf exec $vrf_name \
 	   ${PING} -Q $dscp_10 ${sip:+-I $sip} $dip \
-		   -c 10 -i 0.1 -w $PING_TIMEOUT &> /dev/null
+		   -c 10 -i 0.5 -w $ping_timeout &> /dev/null
 
 	local -A t1s
 	eval "t1s=($(dscp_fetch_stats $dev_10 10)
diff --git a/tools/testing/selftests/drivers/net/mlxsw/qos_dscp_router.sh b/tools/testing/selftests/drivers/net/mlxsw/qos_dscp_router.sh
index 6fa717ef91ef..9faf02e32627 100755
--- a/tools/testing/selftests/drivers/net/mlxsw/qos_dscp_router.sh
+++ b/tools/testing/selftests/drivers/net/mlxsw/qos_dscp_router.sh
@@ -169,9 +169,10 @@ dscp_ping_test()
 	eval "local -A dev1_t0s=($(dscp_fetch_stats $dev1 0))"
 	eval "local -A dev2_t0s=($(dscp_fetch_stats $dev2 0))"
 
+	local ping_timeout=$((PING_TIMEOUT * 5))
 	ip vrf exec $vrf_name \
 	   ${PING} -Q $dscp ${sip:+-I $sip} $dip \
-		   -c 10 -i 0.1 -w $PING_TIMEOUT &> /dev/null
+		   -c 10 -i 0.5 -w $ping_timeout &> /dev/null
 
 	eval "local -A dev1_t1s=($(dscp_fetch_stats $dev1 0))"
 	eval "local -A dev2_t1s=($(dscp_fetch_stats $dev2 0))"
-- 
2.4.11


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

* [PATCH net-next 04/11] selftests: forwarding: mirror_gre_changes: Fix TTL test
  2019-01-31 22:35 [PATCH net-next 00/11] selftests: Various fixes Petr Machata
                   ` (2 preceding siblings ...)
  2019-01-31 22:35 ` [PATCH net-next 03/11] selftests: mlxsw: " Petr Machata
@ 2019-01-31 22:35 ` Petr Machata
  2019-01-31 22:35 ` [PATCH net-next 05/11] selftests: forwarding: mirror_lib: Wait for tardy mirrored packets Petr Machata
                   ` (7 subsequent siblings)
  11 siblings, 0 replies; 13+ messages in thread
From: Petr Machata @ 2019-01-31 22:35 UTC (permalink / raw)
  To: netdev; +Cc: Ido Schimmel

When running in a simulator, the TTL change takes a while to settle and
during this time the performance of the packet processing is lowered.
The resulting instability leads to ping sending more packets as it
assumes some have been dropped. This then leads to regular spurious
failures as more packets than expected are observed.

Sleep a bit to give the system time to stabilize.

Signed-off-by: Petr Machata <petrm@mellanox.com>
---
 tools/testing/selftests/net/forwarding/mirror_gre_changes.sh | 1 +
 1 file changed, 1 insertion(+)

diff --git a/tools/testing/selftests/net/forwarding/mirror_gre_changes.sh b/tools/testing/selftests/net/forwarding/mirror_gre_changes.sh
index 135902aa8b11..472bd023e2a5 100755
--- a/tools/testing/selftests/net/forwarding/mirror_gre_changes.sh
+++ b/tools/testing/selftests/net/forwarding/mirror_gre_changes.sh
@@ -79,6 +79,7 @@ test_span_gre_ttl()
 	mirror_test v$h1 192.0.2.1 192.0.2.2 $h3 77 0
 
 	ip link set dev $tundev type $type ttl 50
+	sleep 2
 	mirror_test v$h1 192.0.2.1 192.0.2.2 $h3 77 10
 
 	ip link set dev $tundev type $type ttl 100
-- 
2.4.11


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

* [PATCH net-next 05/11] selftests: forwarding: mirror_lib: Wait for tardy mirrored packets
  2019-01-31 22:35 [PATCH net-next 00/11] selftests: Various fixes Petr Machata
                   ` (3 preceding siblings ...)
  2019-01-31 22:35 ` [PATCH net-next 04/11] selftests: forwarding: mirror_gre_changes: Fix TTL test Petr Machata
@ 2019-01-31 22:35 ` Petr Machata
  2019-01-31 22:35 ` [PATCH net-next 06/11] selftests: forwarding: mirror_gre_vlan_bridge_1q: Fix untagged test Petr Machata
                   ` (6 subsequent siblings)
  11 siblings, 0 replies; 13+ messages in thread
From: Petr Machata @ 2019-01-31 22:35 UTC (permalink / raw)
  To: netdev; +Cc: Ido Schimmel

When running in an environment with poor performance (such as a
simulator), processing mirrored packets can take a while. Evaluating the
condition too soon leads to spurious "seen 9, expected 10" failures as
the last packet doesn't have enough time to get mirrored and the mirror
to arrive and bump the observed counters.

Wait for one ping interval before evaluating the test.

Signed-off-by: Petr Machata <petrm@mellanox.com>
---
 tools/testing/selftests/net/forwarding/mirror_lib.sh | 1 +
 1 file changed, 1 insertion(+)

diff --git a/tools/testing/selftests/net/forwarding/mirror_lib.sh b/tools/testing/selftests/net/forwarding/mirror_lib.sh
index 878f8d53327a..00797597fcf5 100644
--- a/tools/testing/selftests/net/forwarding/mirror_lib.sh
+++ b/tools/testing/selftests/net/forwarding/mirror_lib.sh
@@ -34,6 +34,7 @@ mirror_test()
 	ip vrf exec $vrf_name \
 	   ${PING} ${sip:+-I $sip} $dip -c 10 -i 0.5 -w $ping_timeout \
 		   &> /dev/null
+	sleep 0.5
 	local t1=$(tc_rule_stats_get $dev $pref)
 	local delta=$((t1 - t0))
 	# Tolerate a couple stray extra packets.
-- 
2.4.11


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

* [PATCH net-next 06/11] selftests: forwarding: mirror_gre_vlan_bridge_1q: Fix untagged test
  2019-01-31 22:35 [PATCH net-next 00/11] selftests: Various fixes Petr Machata
                   ` (4 preceding siblings ...)
  2019-01-31 22:35 ` [PATCH net-next 05/11] selftests: forwarding: mirror_lib: Wait for tardy mirrored packets Petr Machata
@ 2019-01-31 22:35 ` Petr Machata
  2019-01-31 22:35 ` [PATCH net-next 07/11] selftests: forwarding: mirror_gre_vlan_bridge_1q: Fix roaming test Petr Machata
                   ` (5 subsequent siblings)
  11 siblings, 0 replies; 13+ messages in thread
From: Petr Machata @ 2019-01-31 22:35 UTC (permalink / raw)
  To: netdev; +Cc: Ido Schimmel

The untagged egress test sets up mirroring to {,ip6}gretap such that the
underlay goes through a bridge. Then VLAN flags are manipulated to test
that the traffic leaves the bridge 802.1q-tagged or not, as appropriate.

However, when a neighbor expires at the time that the bridge VLAN is
configured as PVID and egress untagged, the following discovery process
can't finish, because the IP address on H3 is still at the VLAN-tagged
netdevice. This manifests by occasional failures where only several of
the 10 required packets get through.

Therefore, when reconfiguring the VLAN flags, move the IP address to the
appropriate device in the H3 VRF.

In addition to that, take this opportunity to embed an ASCII art diagram
to make the topology move obvious.

Signed-off-by: Petr Machata <petrm@mellanox.com>
---
 .../net/forwarding/mirror_gre_vlan_bridge_1q.sh    | 63 +++++++++++++++++++---
 1 file changed, 57 insertions(+), 6 deletions(-)

diff --git a/tools/testing/selftests/net/forwarding/mirror_gre_vlan_bridge_1q.sh b/tools/testing/selftests/net/forwarding/mirror_gre_vlan_bridge_1q.sh
index 204b25f13934..43ee505f9f71 100755
--- a/tools/testing/selftests/net/forwarding/mirror_gre_vlan_bridge_1q.sh
+++ b/tools/testing/selftests/net/forwarding/mirror_gre_vlan_bridge_1q.sh
@@ -1,11 +1,44 @@
 #!/bin/bash
 # SPDX-License-Identifier: GPL-2.0
 
-# This test uses standard topology for testing gretap. See
-# mirror_gre_topo_lib.sh for more details.
-#
 # Test for "tc action mirred egress mirror" when the underlay route points at a
 # vlan device on top of a bridge device with vlan filtering (802.1q).
+#
+#   +---------------------+                             +---------------------+
+#   | H1                  |                             |                  H2 |
+#   |     + $h1           |                             |           $h2 +     |
+#   |     | 192.0.2.1/28  |                             |  192.0.2.2/28 |     |
+#   +-----|---------------+                             +---------------|-----+
+#         |                                                             |
+#   +-----|-------------------------------------------------------------|-----+
+#   | SW  o--> mirred egress mirror dev {gt4,gt6}                       |     |
+#   |     |                                                             |     |
+#   | +---|-------------------------------------------------------------|---+ |
+#   | |   + $swp1                    br1                          $swp2 +   | |
+#   | |                                                                     | |
+#   | |   + $swp3                                                           | |
+#   | +---|-----------------------------------------------------------------+ |
+#   |     |                        |                                          |
+#   |     |                        + br1.555                                  |
+#   |     |                          192.0.2.130/28                           |
+#   |     |                          2001:db8:2::2/64                         |
+#   |     |                                                                   |
+#   |     |                     + gt6 (ip6gretap)      + gt4 (gretap)         |
+#   |     |                     : loc=2001:db8:2::1    : loc=192.0.2.129      |
+#   |     |                     : rem=2001:db8:2::2    : rem=192.0.2.130      |
+#   |     |                     : ttl=100              : ttl=100              |
+#   |     |                     : tos=inherit          : tos=inherit          |
+#   |     |                     :                      :                      |
+#   +-----|---------------------:----------------------:----------------------+
+#         |                     :                      :
+#   +-----|---------------------:----------------------:----------------------+
+#   | H3  + $h3                 + h3-gt6 (ip6gretap)   + h3-gt4 (gretap)      |
+#   |     |                       loc=2001:db8:2::2      loc=192.0.2.130      |
+#   |     + $h3.555               rem=2001:db8:2::1      rem=192.0.2.129      |
+#   |       192.0.2.130/28        ttl=100                ttl=100              |
+#   |       2001:db8:2::2/64      tos=inherit            tos=inherit          |
+#   |                                                                         |
+#   +-------------------------------------------------------------------------+
 
 ALL_TESTS="
 	test_gretap
@@ -30,6 +63,15 @@ source mirror_gre_topo_lib.sh
 
 require_command $ARPING
 
+h3_addr_add_del()
+{
+	local add_del=$1; shift
+	local dev=$1; shift
+
+	ip addr $add_del dev $dev 192.0.2.130/28
+	ip addr $add_del dev $dev 2001:db8:2::2/64
+}
+
 setup_prepare()
 {
 	h1=${NETIFS[p1]}
@@ -55,7 +97,8 @@ setup_prepare()
 	ip route rep 192.0.2.130/32 dev br1.555
 	ip -6 route rep 2001:db8:2::2/128 dev br1.555
 
-	vlan_create $h3 555 v$h3 192.0.2.130/28 2001:db8:2::2/64
+	vlan_create $h3 555 v$h3
+	h3_addr_add_del add $h3.555
 
 	ip link set dev $swp3 master br1
 	bridge vlan add dev $swp3 vid 555
@@ -68,6 +111,8 @@ cleanup()
 
 	ip link set dev $swp2 nomaster
 	ip link set dev $swp3 nomaster
+
+	h3_addr_add_del del $h3.555
 	vlan_destroy $h3 555
 	vlan_destroy br1 555
 
@@ -182,13 +227,19 @@ test_span_gre_untagged_egress()
 	quick_test_span_gre_dir $tundev ingress
 	quick_test_span_vlan_dir $h3 555 ingress
 
+	h3_addr_add_del del $h3.555
 	bridge vlan add dev $swp3 vid 555 pvid untagged
-	sleep 1
+	h3_addr_add_del add $h3
+	sleep 5
+
 	quick_test_span_gre_dir $tundev ingress
 	fail_test_span_vlan_dir $h3 555 ingress
 
+	h3_addr_add_del del $h3
 	bridge vlan add dev $swp3 vid 555
-	sleep 1
+	h3_addr_add_del add $h3.555
+	sleep 5
+
 	quick_test_span_gre_dir $tundev ingress
 	quick_test_span_vlan_dir $h3 555 ingress
 
-- 
2.4.11


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

* [PATCH net-next 07/11] selftests: forwarding: mirror_gre_vlan_bridge_1q: Fix roaming test
  2019-01-31 22:35 [PATCH net-next 00/11] selftests: Various fixes Petr Machata
                   ` (5 preceding siblings ...)
  2019-01-31 22:35 ` [PATCH net-next 06/11] selftests: forwarding: mirror_gre_vlan_bridge_1q: Fix untagged test Petr Machata
@ 2019-01-31 22:35 ` Petr Machata
  2019-01-31 22:35 ` [PATCH net-next 08/11] selftests: forwarding: mirror_gre_bridge_1q_lag: Flush neighbors Petr Machata
                   ` (4 subsequent siblings)
  11 siblings, 0 replies; 13+ messages in thread
From: Petr Machata @ 2019-01-31 22:35 UTC (permalink / raw)
  To: netdev; +Cc: Ido Schimmel

ARP or ND traffic can cause spurious migration of FDB back to $swp3.
Mirroring is then updated in accordance with the change, and mirrored
packets are seen at h3, causing a failure.

Detect the case of this spurious roaming, and retry the test.

Signed-off-by: Petr Machata <petrm@mellanox.com>
---
 .../net/forwarding/mirror_gre_vlan_bridge_1q.sh    | 25 ++++++++++++++++------
 1 file changed, 19 insertions(+), 6 deletions(-)

diff --git a/tools/testing/selftests/net/forwarding/mirror_gre_vlan_bridge_1q.sh b/tools/testing/selftests/net/forwarding/mirror_gre_vlan_bridge_1q.sh
index 43ee505f9f71..c02291e9841e 100755
--- a/tools/testing/selftests/net/forwarding/mirror_gre_vlan_bridge_1q.sh
+++ b/tools/testing/selftests/net/forwarding/mirror_gre_vlan_bridge_1q.sh
@@ -269,12 +269,25 @@ test_span_gre_fdb_roaming()
 	mirror_install $swp1 ingress $tundev "matchall $tcflags"
 	quick_test_span_gre_dir $tundev ingress
 
-	bridge fdb del dev $swp3 $h3mac vlan 555 master
-	bridge fdb add dev $swp2 $h3mac vlan 555 master
-	sleep 1
-	fail_test_span_gre_dir $tundev ingress
-
-	bridge fdb del dev $swp2 $h3mac vlan 555 master
+	while ((RET == 0)); do
+		bridge fdb del dev $swp3 $h3mac vlan 555 master 2>/dev/null
+		bridge fdb add dev $swp2 $h3mac vlan 555 master
+		sleep 1
+		fail_test_span_gre_dir $tundev ingress
+
+		if ! bridge fdb sh dev $swp2 vlan 555 master \
+		    | grep -q $h3mac; then
+			printf "TEST: %-60s  [RETRY]\n" \
+				"$what: MAC roaming ($tcflags)"
+			# ARP or ND probably reprimed the FDB while the test
+			# was running. We would get a spurious failure.
+			RET=0
+			continue
+		fi
+		break
+	done
+
+	bridge fdb del dev $swp2 $h3mac vlan 555 master 2>/dev/null
 	# Re-prime FDB
 	$ARPING -I br1.555 192.0.2.130 -fqc 1
 	sleep 1
-- 
2.4.11


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

* [PATCH net-next 08/11] selftests: forwarding: mirror_gre_bridge_1q_lag: Flush neighbors
  2019-01-31 22:35 [PATCH net-next 00/11] selftests: Various fixes Petr Machata
                   ` (6 preceding siblings ...)
  2019-01-31 22:35 ` [PATCH net-next 07/11] selftests: forwarding: mirror_gre_vlan_bridge_1q: Fix roaming test Petr Machata
@ 2019-01-31 22:35 ` Petr Machata
  2019-01-31 22:35 ` [PATCH net-next 09/11] selftests: forwarding: mirror_gre_bridge_1q_lag: Enable forwarding Petr Machata
                   ` (3 subsequent siblings)
  11 siblings, 0 replies; 13+ messages in thread
From: Petr Machata @ 2019-01-31 22:35 UTC (permalink / raw)
  To: netdev; +Cc: Ido Schimmel

After one LAG slave is downed and another upped, it takes a while for
the neighbor on a bridge to time out and get renegotiated. The test does
prompt update of FDB entries by arpinging. But because the neighbor
still references another address, offloading is not possible, and some
packets may end up not being mirrored.

To force the neighbor renegotiation, simply flush the neighbor table at
the bridge.

Signed-off-by: Petr Machata <petrm@mellanox.com>
---
 tools/testing/selftests/net/forwarding/mirror_gre_bridge_1q_lag.sh | 1 +
 1 file changed, 1 insertion(+)

diff --git a/tools/testing/selftests/net/forwarding/mirror_gre_bridge_1q_lag.sh b/tools/testing/selftests/net/forwarding/mirror_gre_bridge_1q_lag.sh
index 61844caf671e..ce60e3050811 100755
--- a/tools/testing/selftests/net/forwarding/mirror_gre_bridge_1q_lag.sh
+++ b/tools/testing/selftests/net/forwarding/mirror_gre_bridge_1q_lag.sh
@@ -225,6 +225,7 @@ test_lag_slave()
 
 	# Test connectivity through $up_dev when $down_dev is set down.
 	ip link set dev $down_dev down
+	ip neigh flush dev br1
 	setup_wait_dev $up_dev
 	setup_wait_dev $host_dev
 	$ARPING -I br1 192.0.2.130 -qfc 1
-- 
2.4.11


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

* [PATCH net-next 09/11] selftests: forwarding: mirror_gre_bridge_1q_lag: Enable forwarding
  2019-01-31 22:35 [PATCH net-next 00/11] selftests: Various fixes Petr Machata
                   ` (7 preceding siblings ...)
  2019-01-31 22:35 ` [PATCH net-next 08/11] selftests: forwarding: mirror_gre_bridge_1q_lag: Flush neighbors Petr Machata
@ 2019-01-31 22:35 ` Petr Machata
  2019-01-31 22:35 ` [PATCH net-next 10/11] selftests: forwarding: mirror_gre_bridge_1q_lag: Ignore ARP Petr Machata
                   ` (2 subsequent siblings)
  11 siblings, 0 replies; 13+ messages in thread
From: Petr Machata @ 2019-01-31 22:35 UTC (permalink / raw)
  To: netdev; +Cc: Ido Schimmel

This test relies on routing in the primary traffic path, but neglects to
enable forwarding. Do so.

Signed-off-by: Petr Machata <petrm@mellanox.com>
---
 tools/testing/selftests/net/forwarding/mirror_gre_bridge_1q_lag.sh | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/tools/testing/selftests/net/forwarding/mirror_gre_bridge_1q_lag.sh b/tools/testing/selftests/net/forwarding/mirror_gre_bridge_1q_lag.sh
index ce60e3050811..1900c31e938c 100755
--- a/tools/testing/selftests/net/forwarding/mirror_gre_bridge_1q_lag.sh
+++ b/tools/testing/selftests/net/forwarding/mirror_gre_bridge_1q_lag.sh
@@ -190,6 +190,8 @@ setup_prepare()
 	h4_create
 	switch_create
 
+	forwarding_enable
+
 	trap_install $h3 ingress
 	trap_install $h4 ingress
 }
@@ -201,6 +203,8 @@ cleanup()
 	trap_uninstall $h4 ingress
 	trap_uninstall $h3 ingress
 
+	forwarding_restore
+
 	switch_destroy
 	h4_destroy
 	h3_destroy
-- 
2.4.11


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

* [PATCH net-next 10/11] selftests: forwarding: mirror_gre_bridge_1q_lag: Ignore ARP
  2019-01-31 22:35 [PATCH net-next 00/11] selftests: Various fixes Petr Machata
                   ` (8 preceding siblings ...)
  2019-01-31 22:35 ` [PATCH net-next 09/11] selftests: forwarding: mirror_gre_bridge_1q_lag: Enable forwarding Petr Machata
@ 2019-01-31 22:35 ` Petr Machata
  2019-01-31 22:35 ` [PATCH net-next 11/11] selftests: forwarding: mirror_gre_flower: Fix test result handling Petr Machata
  2019-02-01 23:26 ` [PATCH net-next 00/11] selftests: Various fixes David Miller
  11 siblings, 0 replies; 13+ messages in thread
From: Petr Machata @ 2019-01-31 22:35 UTC (permalink / raw)
  To: netdev; +Cc: Ido Schimmel

This test sets up mirroring such that it mirrors all overlay traffic.
That includes ARP, which causes occasional miscounts and spurious
failures. Ignore ARP explicitly to avoid these problems.

Signed-off-by: Petr Machata <petrm@mellanox.com>
---
 tools/testing/selftests/net/forwarding/mirror_gre_bridge_1q_lag.sh | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/tools/testing/selftests/net/forwarding/mirror_gre_bridge_1q_lag.sh b/tools/testing/selftests/net/forwarding/mirror_gre_bridge_1q_lag.sh
index 1900c31e938c..28d568c48a73 100755
--- a/tools/testing/selftests/net/forwarding/mirror_gre_bridge_1q_lag.sh
+++ b/tools/testing/selftests/net/forwarding/mirror_gre_bridge_1q_lag.sh
@@ -224,8 +224,11 @@ test_lag_slave()
 
 	RET=0
 
+	tc filter add dev $swp1 ingress pref 999 \
+		proto 802.1q flower vlan_ethtype arp $tcflags \
+		action pass
 	mirror_install $swp1 ingress gt4 \
-		       "proto 802.1q flower vlan_id 333 $tcflags"
+		"proto 802.1q flower vlan_id 333 $tcflags"
 
 	# Test connectivity through $up_dev when $down_dev is set down.
 	ip link set dev $down_dev down
@@ -245,6 +248,7 @@ test_lag_slave()
 	ip link set dev $up_dev up
 	ip link set dev $down_dev up
 	mirror_uninstall $swp1 ingress
+	tc filter del dev $swp1 ingress pref 999
 
 	log_test "$what ($tcflags)"
 }
-- 
2.4.11


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

* [PATCH net-next 11/11] selftests: forwarding: mirror_gre_flower: Fix test result handling
  2019-01-31 22:35 [PATCH net-next 00/11] selftests: Various fixes Petr Machata
                   ` (9 preceding siblings ...)
  2019-01-31 22:35 ` [PATCH net-next 10/11] selftests: forwarding: mirror_gre_bridge_1q_lag: Ignore ARP Petr Machata
@ 2019-01-31 22:35 ` Petr Machata
  2019-02-01 23:26 ` [PATCH net-next 00/11] selftests: Various fixes David Miller
  11 siblings, 0 replies; 13+ messages in thread
From: Petr Machata @ 2019-01-31 22:35 UTC (permalink / raw)
  To: netdev; +Cc: Ido Schimmel

The global variable RET needs to be initialized before each call to
log_test. This test case sets it once before running the tests, but then
calls log_tests for every individual test. Thus a failure in one of the
tests causes spurious failures in follow-up tests as well.

Fix by moving the initialization of RET from test_all() to
full_test_span_gre_dir_acl(), a function that implements the test.

Signed-off-by: Petr Machata <petrm@mellanox.com>
---
 tools/testing/selftests/net/forwarding/mirror_gre_flower.sh | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/tools/testing/selftests/net/forwarding/mirror_gre_flower.sh b/tools/testing/selftests/net/forwarding/mirror_gre_flower.sh
index 12914f40612d..09389f3b9369 100755
--- a/tools/testing/selftests/net/forwarding/mirror_gre_flower.sh
+++ b/tools/testing/selftests/net/forwarding/mirror_gre_flower.sh
@@ -81,6 +81,8 @@ full_test_span_gre_dir_acl()
 	local match_dip=$1; shift
 	local what=$1; shift
 
+	RET=0
+
 	mirror_install $swp1 $direction $tundev \
 		       "protocol ip flower $tcflags dst_ip $match_dip"
 	fail_test_span_gre_dir $tundev $direction
@@ -108,8 +110,6 @@ test_ip6gretap()
 
 test_all()
 {
-	RET=0
-
 	slow_path_trap_install $swp1 ingress
 	slow_path_trap_install $swp1 egress
 
-- 
2.4.11


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

* Re: [PATCH net-next 00/11] selftests: Various fixes
  2019-01-31 22:35 [PATCH net-next 00/11] selftests: Various fixes Petr Machata
                   ` (10 preceding siblings ...)
  2019-01-31 22:35 ` [PATCH net-next 11/11] selftests: forwarding: mirror_gre_flower: Fix test result handling Petr Machata
@ 2019-02-01 23:26 ` David Miller
  11 siblings, 0 replies; 13+ messages in thread
From: David Miller @ 2019-02-01 23:26 UTC (permalink / raw)
  To: petrm; +Cc: netdev, idosch

From: Petr Machata <petrm@mellanox.com>
Date: Thu, 31 Jan 2019 22:35:08 +0000

> This patch set contains various fixes whose common denominator is
> improving quality of forwarding and mlxsw selftests.
> 
> Most of the fixes are improvements in determinism (such that timing and
> latency don't impact the test performance). These were prompted by
> regular runs of the test suite on a hardware emulator, the performance
> of which is necessarily lower than that of the real device.
> 
> Patches #1 (from Ido), #2 and #3 make changes to ping limits.
> 
> Patches #4 and #5 add more sleep in places where things need more time
> to finish.
> 
> Patches #6 and #7 fix two tests in the suite of mirror-to-gretap tests
> where underlay involves a VLAN device over an 802.1q bridge.
> 
> Patches #8, #9 and #10 fix bugs in mirror-to-gretap test where underlay
> involves a LAG device.
> 
> Patch #11 fixes a missed RET initialization in mirror-to-gretap flower
> test.

Series applied, thanks.

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

end of thread, other threads:[~2019-02-01 23:26 UTC | newest]

Thread overview: 13+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-01-31 22:35 [PATCH net-next 00/11] selftests: Various fixes Petr Machata
2019-01-31 22:35 ` [PATCH net-next 01/11] selftests: forwarding: Make ping timeout configurable Petr Machata
2019-01-31 22:35 ` [PATCH net-next 02/11] selftests: forwarding: mirror_lib: Update ping limits Petr Machata
2019-01-31 22:35 ` [PATCH net-next 03/11] selftests: mlxsw: " Petr Machata
2019-01-31 22:35 ` [PATCH net-next 04/11] selftests: forwarding: mirror_gre_changes: Fix TTL test Petr Machata
2019-01-31 22:35 ` [PATCH net-next 05/11] selftests: forwarding: mirror_lib: Wait for tardy mirrored packets Petr Machata
2019-01-31 22:35 ` [PATCH net-next 06/11] selftests: forwarding: mirror_gre_vlan_bridge_1q: Fix untagged test Petr Machata
2019-01-31 22:35 ` [PATCH net-next 07/11] selftests: forwarding: mirror_gre_vlan_bridge_1q: Fix roaming test Petr Machata
2019-01-31 22:35 ` [PATCH net-next 08/11] selftests: forwarding: mirror_gre_bridge_1q_lag: Flush neighbors Petr Machata
2019-01-31 22:35 ` [PATCH net-next 09/11] selftests: forwarding: mirror_gre_bridge_1q_lag: Enable forwarding Petr Machata
2019-01-31 22:35 ` [PATCH net-next 10/11] selftests: forwarding: mirror_gre_bridge_1q_lag: Ignore ARP Petr Machata
2019-01-31 22:35 ` [PATCH net-next 11/11] selftests: forwarding: mirror_gre_flower: Fix test result handling Petr Machata
2019-02-01 23:26 ` [PATCH net-next 00/11] selftests: Various fixes David Miller

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).