selftests: mlxsw: Fixes

This patch set carries fixes to selftest issues that we have hit in our nightly regression run. Almost all are in mlxsw selftests, though one is in a generic forwarding selftest.

- In patch #1, in an ERSPAN test, install an FDB entry as static instead of (implicitly) as local.

- In the mlxsw resource-scale test, an if statement overrides the value of $?, which is supposed to contain the result of the test. As a result, the resource scale test can spuriously pass.

In patches #2 and #3, remove the if statements to fix the issue in, respectively, port_scale test and tc_flower_scale tests.

- Again in the mlxsw resource-scale test, when more then one sub-test is run, a successful sub-test overrides any previous failures. This causes a spurious pass of the overall test. This is fixed in patch #4.

- In patch #5, increase a tolerance in a mlxsw-specific RED backlog test. This test is very noisy, due to rounding errors and the unpredictability of software traffic generation. By bumping the tolerance from 5 % to 10, get the failure rate to zero. This shouldn't impact the accuracy, mistakes in backlog configuration (e.g. due to wrong cell size) are likely to cause a much larger discrepancy.

- In patch #6, fix mausezahn invocation in the mlxsw ERSPAN scale test. The test failed because of the wrong invocation.

Danielle Ratson (3):
selftests: mlxsw: Remove a redundant if statement in port_scale test
selftests: mlxsw: Remove a redundant if statement in tc_flower_scale
test
selftests: mlxsw: Return correct error code in resource scale tests

Petr Machata (3):
selftests: net: mirror_gre_vlan_bridge_1q: Make an FDB entry static
selftests: mlxsw: Increase the tolerance of backlog buildup
selftests: mlxsw: Fix mausezahn invocation in ERSPAN scale test

 .../drivers/net/mlxsw/mirror_gre_scale.sh     |  3 ++-
 .../selftests/drivers/net/mlxsw/port_scale.sh |  6 +-----
 .../drivers/net/mlxsw/sch_red_core.sh         |  4 ++--
 .../net/mlxsw/spectrum-2/resource_scale.sh    |  4 +++-
 .../net/mlxsw/spectrum/resource_scale.sh      |  4 +++-
 .../drivers/net/mlxsw/tc_flower_scale.sh      |  6 +-----
 .../forwarding/mirror_gre_vlan_bridge_1q.sh   |  2 +-
 .../selftests/net/forwarding/mirror_lib.sh    | 19 +++++++++++++++++--
 8 files changed, 30 insertions(+), 18 deletions(-)

2.26.2

The FDB roaming test installs a destination MAC address on the wrong interface of an FDB database and tests whether the mirroring fails, because packets are sent to the wrong port. The test by mistake installs the FDB entry as local. This worked previously, because drivers were notified of local FDB entries in the same way as of static entries. However that has been fixed in the commit 6ab4c3117aec ("net: bridge: don't notify switchdev for local FDB addresses"), and local entries are not notified anymore. As a result, the HW is not reconfigured for the FDB roam, and mirroring keeps working, failing the test.

To fix the issue, mark the FDB entry as static.

---
 .../selftests/net/forwarding/mirror_gre_vlan_bridge_1q.sh       | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

index c02291e9841e..880e3ab9d088 100755
@@ -271,7 +271,7 @@ test_span_gre_fdb_roaming()
 
 	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
+		bridge fdb add dev $swp2 $h3mac vlan 555 master static
 		sleep 1
 		fail_test_span_gre_dir $tundev ingress
 

2.26.2

From: Danielle Ratson <[email protected]>

Currently, the error return code of the failure condition is lost after using an if statement, so the test doesn't fail when it should.

Remove the if statement that separates the condition and the error code check, so the test won't always pass.

---
 tools/testing/selftests/drivers/net/mlxsw/port_scale.sh | 6 +-----
 1 file changed, 1 insertion(+), 5 deletions(-)

index f813ffefc07e..65f43a7ce9c9 100644
@@ -55,10 +55,6 @@ port_test()
 	      | jq '.[][][] | select(.name=="physical_ports") |.["occ"]')
 
 	[[ $occ -eq $max_ports ]]
-	if [[ $should_fail -eq 0 ]]; then
-		check_err $? "Mismatch ports number: Expected $max_ports, got $occ."
-	else
-		check_err_fail $should_fail $? "Reached more ports than expected"
-	fi
+	check_err_fail $should_fail $? "Attempt to create $max_ports ports (actual result $occ)"
 
 }

2.26.2

From: Danielle Ratson <[email protected]>

Currently, the error return code of the failure condition is lost after using an if statement, so the test doesn't fail when it should.

Remove the if statement that separates the condition and the error code check, so the test won't always pass.

---
 .../testing/selftests/drivers/net/mlxsw/tc_flower_scale.sh  | 6 +-----
 1 file changed, 1 insertion(+), 5 deletions(-)

index cc0f07e72cf2..aa74be9f47c8 100644
@@ -98,11 +98,7 @@ __tc_flower_test()
 			jq -r '[ .[] | select(.kind == "flower") |
 			.options | .in_hw ]' | jq .[] | wc -l)
 	[[ $((offload_count - 1)) -eq $count ]]
-	if [[ $should_fail -eq 0 ]]; then
-		check_err $? "Offload mismatch"
-	else
-		check_err_fail $should_fail $? "Offload more than expacted"
-	fi
+	check_err_fail $should_fail $? "Attempt to offload $count rules (actual result $((offload_count - 1)))"
 }
 
 tc_flower_test()

2.26.2

From: Danielle Ratson <[email protected]>

Currently, the resource scale test checks a few cases, when the error code resets between the cases. So for example, if one case fails and the consecutive case passes, the error code eventually will fit the last test and will be 0.

Save a new return code that will hold the 'or' return codes of all the cases, so the final return code will consider all the cases.

---
 .../selftests/drivers/net/mlxsw/spectrum-2/resource_scale.sh  | 4 +++-
 .../selftests/drivers/net/mlxsw/spectrum/resource_scale.sh    | 4 +++-
 2 files changed, 6 insertions(+), 2 deletions(-)

index 4a1c9328555f..50654f8a8c37 100755
@@ -30,6 +30,7 @@ trap cleanup EXIT
 
 ALL_TESTS="router tc_flower mirror_gre tc_police port"
 for current_test in ${TESTS:-$ALL_TESTS}; do
+	RET_FIN=0
 	source ${current_test}_scale.sh
 
 	num_netifs_var=${current_test^^}_NUM_NETIFS

@@ -48,8 +49,9 @@ for current_test in ${TESTS:-$ALL_TESTS}; do
 		else
 			log_test "'$current_test' overflow $target"
 		fi
+		RET_FIN=$(( RET_FIN || RET ))
 	done
 done
 current_test=""
 
-exit "$RET"
+exit "$RET_FIN"

index 087a884f66cd..685dfb3478b3 100755
@@ -24,6 +24,7 @@ trap cleanup EXIT
 
 ALL_TESTS="router tc_flower mirror_gre tc_police port"
 for current_test in ${TESTS:-$ALL_TESTS}; do
+	RET_FIN=0
 	source ${current_test}_scale.sh
 
 	num_netifs_var=${current_test^^}_NUM_NETIFS

@@ -50,8 +51,9 @@ for current_test in ${TESTS:-$ALL_TESTS}; do
 				log_test "'$current_test' [$profile] overflow $target"
 			fi
 		done
+		RET_FIN=$(( RET_FIN || RET ))
 	done
 done
 current_test=""
 
-exit "$RET"
+exit "$RET_FIN"

2.26.2

The intention behind this test is to make sure that qdisc limit is correctly projected to the HW. However, first, due to rounding in the qdisc, and then in the driver, the number cannot actually be accurate. And second, the approach to testing this is to oversubscribe the port with traffic generated on the same switch. The actual backlog size therefore fluctuates.

In practice, this test proved to be noisier than the rest, and spuriously fails every now and then. Increase the tolerance to 10 % to avoid these issues.

---
 tools/testing/selftests/drivers/net/mlxsw/sch_red_core.sh | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

index b0cb1aaffdda..33ddd01689be 100644
@@ -507,8 +507,8 @@ do_red_test()
 	check_err $? "backlog $backlog / $limit Got $pct% marked packets, expected == 0."
 	local diff=$((limit - backlog))
 	pct=$((100 * diff / limit))
-	((0 <= pct && pct <= 5))
-	check_err $? "backlog $backlog / $limit expected <= 5% distance"
+	((0 <= pct && pct <= 10))
+	check_err $? "backlog $backlog / $limit expected <= 10% distance"
 	log_test "TC $((vlan - 10)): RED backlog > limit"
 
 	stop_traffic

2.26.2

The mirror_gre_scale test creates as many ERSPAN sessions as the underlying chip supports, and tests that they all work. In order to determine that it issues a stream of ICMP packets and checks if they are mirrored as expected.

However, the mausezahn invocation missed the -6 flag to identify the use of IPv6 protocol, and was sending ICMP messages over IPv6, as opposed to ICMP6. It also didn't pass an explicit source IP address, which apparently worked at some point in the past, but does not anymore.

To fix these issues, extend the function mirror_test() in mirror_lib by detecting the IPv6 protocol addresses, and using a different ICMP scheme. Fix __mirror_gre_test() in the selftest itself to pass a source IP address.

---
 .../drivers/net/mlxsw/mirror_gre_scale.sh     |  3 ++-
 .../selftests/net/forwarding/mirror_lib.sh    | 19 +++++++++++++++++--
 2 files changed, 19 insertions(+), 3 deletions(-)

index 6f3a70df63bc..e00435753008 100644
@@ -120,12 +120,13 @@ __mirror_gre_test()
 	sleep 5
 
 	for ((i = 0; i < count; ++i)); do
+		local sip=$(mirror_gre_ipv6_addr 1 $i)::1
 		local dip=$(mirror_gre_ipv6_addr 1 $i)::2
 		local htun=h3-gt6-$i
 		local message
 
 		icmp6_capture_install $htun
-		mirror_test v$h1 "" $dip $htun 100 10
+		mirror_test v$h1 $sip $dip $htun 100 10
 		icmp6_capture_uninstall $htun
 	done
 }

index 13db1cb50e57..6406cd76a19d 100644
@@ -20,6 +20,13 @@ mirror_uninstall()
 	tc filter del dev $swp1 $direction pref 1000
 }
 
+is_ipv6()
+{
+	local addr=$1; shift
+
+	[[ -z ${addr//[0-9a-fA-F:]/} ]]
+}
+
 mirror_test()
 {
 	local vrf_name=$1; shift

@@ -29,9 +36,17 @@ mirror_test()
 	local pref=$1; shift
 	local expect=$1; shift
 
+	if is_ipv6 $dip; then
+		local proto=-6
+		local type="icmp6 type=128" # Echo request.
+	else
+		local proto=
+		local type="icmp echoreq"
+	fi
+
 	local t0=$(tc_rule_stats_get $dev $pref)
-	$MZ $vrf_name ${sip:+-A $sip} -B $dip -a own -b bc -q \
-	    -c 10 -d 100msec -t icmp type=8
+	$MZ $proto $vrf_name ${sip:+-A $sip} -B $dip -a own -b bc -q \
+	    -c 10 -d 100msec -t $type
 	sleep 0.5
 	local t1=$(tc_rule_stats_get $dev $pref)
 	local delta=$((t1 - t0))

2.26.2