[PATCH net-next 0/6] selftests: mlxsw: Fixes

[PATCH net-next 0/6] selftests: mlxsw: Fixes

[Petr Machata]

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

[PATCH net-next 1/6] selftests: net: mirror_gre_vlan_bridge_1q: Make an FDB entry static

[Petr Machata]

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.

Fixes: 9c7c8a82442c ("selftests: forwarding: mirror_gre_vlan_bridge_1q: Add more tests")
Signed-off-by: Petr Machata <petrm@nvidia.com>
Reviewed-by: Ido Schimmel <idosch@nvidia.com>
---
 .../selftests/net/forwarding/mirror_gre_vlan_bridge_1q.sh       | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

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 c02291e9841e..880e3ab9d088 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
@@ -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

[PATCH net-next 2/6] selftests: mlxsw: Remove a redundant if statement in port_scale test

[Petr Machata]

From: Danielle Ratson <danieller@nvidia.com>

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.

Fixes: 5154b1b826d9b ("selftests: mlxsw: Add a scale test for physical ports")
Reported-by: Ido Schimmel <idosch@nvidia.com>
Signed-off-by: Danielle Ratson <danieller@nvidia.com>
Reviewed-by: Petr Machata <petrm@nvidia.com>
Signed-off-by: Petr Machata <petrm@nvidia.com>
---
 tools/testing/selftests/drivers/net/mlxsw/port_scale.sh | 6 +-----
 1 file changed, 1 insertion(+), 5 deletions(-)

diff --git a/tools/testing/selftests/drivers/net/mlxsw/port_scale.sh b/tools/testing/selftests/drivers/net/mlxsw/port_scale.sh
index f813ffefc07e..65f43a7ce9c9 100644
--- a/tools/testing/selftests/drivers/net/mlxsw/port_scale.sh
+++ b/tools/testing/selftests/drivers/net/mlxsw/port_scale.sh
@@ -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

[PATCH net-next 3/6] selftests: mlxsw: Remove a redundant if statement in tc_flower_scale test

[Petr Machata]

From: Danielle Ratson <danieller@nvidia.com>

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.

Fixes: abfce9e062021 ("selftests: mlxsw: Reduce running time using offload indication")
Reported-by: Ido Schimmel <idosch@nvidia.com>
Signed-off-by: Danielle Ratson <danieller@nvidia.com>
Reviewed-by: Petr Machata <petrm@nvidia.com>
Signed-off-by: Petr Machata <petrm@nvidia.com>
---
 .../testing/selftests/drivers/net/mlxsw/tc_flower_scale.sh  | 6 +-----
 1 file changed, 1 insertion(+), 5 deletions(-)

diff --git a/tools/testing/selftests/drivers/net/mlxsw/tc_flower_scale.sh b/tools/testing/selftests/drivers/net/mlxsw/tc_flower_scale.sh
index cc0f07e72cf2..aa74be9f47c8 100644
--- a/tools/testing/selftests/drivers/net/mlxsw/tc_flower_scale.sh
+++ b/tools/testing/selftests/drivers/net/mlxsw/tc_flower_scale.sh
@@ -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

[PATCH net-next 4/6] selftests: mlxsw: Return correct error code in resource scale tests

[Petr Machata]

From: Danielle Ratson <danieller@nvidia.com>

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.

Signed-off-by: Danielle Ratson <danieller@nvidia.com>
Reviewed-by: Petr Machata <petrm@nvidia.com>
Signed-off-by: Petr Machata <petrm@nvidia.com>
---
 .../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(-)

diff --git a/tools/testing/selftests/drivers/net/mlxsw/spectrum-2/resource_scale.sh b/tools/testing/selftests/drivers/net/mlxsw/spectrum-2/resource_scale.sh
index 4a1c9328555f..50654f8a8c37 100755
--- a/tools/testing/selftests/drivers/net/mlxsw/spectrum-2/resource_scale.sh
+++ b/tools/testing/selftests/drivers/net/mlxsw/spectrum-2/resource_scale.sh
@@ -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"
diff --git a/tools/testing/selftests/drivers/net/mlxsw/spectrum/resource_scale.sh b/tools/testing/selftests/drivers/net/mlxsw/spectrum/resource_scale.sh
index 087a884f66cd..685dfb3478b3 100755
--- a/tools/testing/selftests/drivers/net/mlxsw/spectrum/resource_scale.sh
+++ b/tools/testing/selftests/drivers/net/mlxsw/spectrum/resource_scale.sh
@@ -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

[PATCH net-next 5/6] selftests: mlxsw: Increase the tolerance of backlog buildup

[Petr Machata]

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.

Signed-off-by: Petr Machata <petrm@nvidia.com>
Acked-by: Jiri Pirko <jiri@nvidia.com>
---
 tools/testing/selftests/drivers/net/mlxsw/sch_red_core.sh | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/tools/testing/selftests/drivers/net/mlxsw/sch_red_core.sh b/tools/testing/selftests/drivers/net/mlxsw/sch_red_core.sh
index b0cb1aaffdda..33ddd01689be 100644
--- a/tools/testing/selftests/drivers/net/mlxsw/sch_red_core.sh
+++ b/tools/testing/selftests/drivers/net/mlxsw/sch_red_core.sh
@@ -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

[PATCH net-next 6/6] selftests: mlxsw: Fix mausezahn invocation in ERSPAN scale test

[Petr Machata]

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.

Signed-off-by: Petr Machata <petrm@nvidia.com>
---
 .../drivers/net/mlxsw/mirror_gre_scale.sh     |  3 ++-
 .../selftests/net/forwarding/mirror_lib.sh    | 19 +++++++++++++++++--
 2 files changed, 19 insertions(+), 3 deletions(-)

diff --git a/tools/testing/selftests/drivers/net/mlxsw/mirror_gre_scale.sh b/tools/testing/selftests/drivers/net/mlxsw/mirror_gre_scale.sh
index 6f3a70df63bc..e00435753008 100644
--- a/tools/testing/selftests/drivers/net/mlxsw/mirror_gre_scale.sh
+++ b/tools/testing/selftests/drivers/net/mlxsw/mirror_gre_scale.sh
@@ -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
 }
diff --git a/tools/testing/selftests/net/forwarding/mirror_lib.sh b/tools/testing/selftests/net/forwarding/mirror_lib.sh
index 13db1cb50e57..6406cd76a19d 100644
--- a/tools/testing/selftests/net/forwarding/mirror_lib.sh
+++ b/tools/testing/selftests/net/forwarding/mirror_lib.sh
@@ -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

Re: [PATCH net-next 0/6] selftests: mlxsw: Fixes

[patchwork-bot+netdevbpf]

Hello:

This series was applied to netdev/net-next.git (refs/heads/master):

On Fri, 23 Apr 2021 14:19:42 +0200 you wrote:
> 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.
> 
> [...]

Here is the summary with links:
  - [net-next,1/6] selftests: net: mirror_gre_vlan_bridge_1q: Make an FDB entry static
    https://git.kernel.org/netdev/net-next/c/c8d0260cdd96
  - [net-next,2/6] selftests: mlxsw: Remove a redundant if statement in port_scale test
    https://git.kernel.org/netdev/net-next/c/b6fc2f212108
  - [net-next,3/6] selftests: mlxsw: Remove a redundant if statement in tc_flower_scale test
    https://git.kernel.org/netdev/net-next/c/1f1c92139e36
  - [net-next,4/6] selftests: mlxsw: Return correct error code in resource scale tests
    https://git.kernel.org/netdev/net-next/c/059b18e21c63
  - [net-next,5/6] selftests: mlxsw: Increase the tolerance of backlog buildup
    https://git.kernel.org/netdev/net-next/c/dda7f4fa5583
  - [net-next,6/6] selftests: mlxsw: Fix mausezahn invocation in ERSPAN scale test
    https://git.kernel.org/netdev/net-next/c/1233898ab758

You are awesome, thank you!
--
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html