Skip to contentSkip to conversation

selftests: forwarding: misc bug fixes and enhancements

PATCH In review David Ahern 2 participants 6 messages Mar 1, 2018 – Mar 2, 2018

Subsystems (4)
lore
Share & tools
lore
#0 lore raw

v2 cover selftests: forwarding: misc bug fixes and enhancements

Bug fixes and an enhancement for the recent forwarding tests:
- only check tc version on tc tests
- handle multipath tests failing with 0 packet count
- fix ping command for IPv6 on Debian jessie
- improve summary of multipath tests

v2
- add CHECK_TC to bridge_vlan_aware.sh (Ido)
- dropped patch 2; always check for mz given its use
- fixed commit message for the last patch (Multipath: was dropped)

David Ahern (4):
selftests: forwarding: Only check tc version for tc tests
selftests: forwarding: Handle 0 for packet difference in multipath
tests
selftests: forwarding: Use PING6 instead of ping for ipv6 multipath
test
selftests: forwarding: Add description to the multipath tests

 .../selftests/net/forwarding/bridge_vlan_aware.sh  |  1 +
 tools/testing/selftests/net/forwarding/lib.sh      | 29 ++++++++------
 .../selftests/net/forwarding/router_multipath.sh   | 46 +++++++++++++---------
 .../testing/selftests/net/forwarding/tc_actions.sh |  2 +-
 .../testing/selftests/net/forwarding/tc_chains.sh  |  2 +-
 .../testing/selftests/net/forwarding/tc_common.sh  |  2 +
 .../testing/selftests/net/forwarding/tc_flower.sh  |  2 +-
 .../selftests/net/forwarding/tc_shblocks.sh        |  2 +-
 8 files changed, 53 insertions(+), 33 deletions(-)
Signature · 6 lines
2.11.0

--
To unsubscribe from this list: send the line "unsubscribe linux-kselftest" in
the body of a message to majordomo at vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
#1 lore raw

PATCH v2 1/4 selftests: forwarding: Only check tc version for tc tests

Capabilities of tc command are irrelevant for router tests:

    $ ./router.sh
    SKIP: iproute2 too old, missing shared block support

Add a CHECK_TC flag and only check tc capabilities if set. Add flag to tc_common.sh and have it sourced before lib.sh

Also, if the command lacks some feature the test should exit non-0.

Signed-off-by David Ahern <dsahern at gmail.com>
---
 .../selftests/net/forwarding/bridge_vlan_aware.sh  |  1 +
 tools/testing/selftests/net/forwarding/lib.sh      | 29 ++++++++++++++--------
 .../testing/selftests/net/forwarding/tc_actions.sh |  2 +-
 .../testing/selftests/net/forwarding/tc_chains.sh  |  2 +-
 .../testing/selftests/net/forwarding/tc_common.sh  |  2 ++
 .../testing/selftests/net/forwarding/tc_flower.sh  |  2 +-
 .../selftests/net/forwarding/tc_shblocks.sh        |  2 +-
 7 files changed, 25 insertions(+), 15 deletions(-)
tools/testing/selftests/net/forwarding/bridge_vlan_aware.sh +1 -0
index 651998e70557..75d922438bc9 100755
@@ -2,6 +2,7 @@
 # SPDX-License-Identifier: GPL-2.0
 
 NUM_NETIFS=4
+CHECK_TC="yes"
 source lib.sh
 
 h1_create()
tools/testing/selftests/net/forwarding/lib.sh +18 -11
index 23866a685f77..d0af52109360 100644
@@ -19,26 +19,33 @@ fi
 ##############################################################################
 # Sanity checks
 
+check_tc_version()
+{
+	tc -j &> /dev/null
+	if [[ $? -ne 0 ]]; then
+		echo "SKIP: iproute2 too old; tc is missing JSON support"
+		exit 1
+	fi
+
+	tc filter help 2>&1 | grep block &> /dev/null
+	if [[ $? -ne 0 ]]; then
+		echo "SKIP: iproute2 too old; tc is missing shared block support"
+		exit 1
+	fi
+}
+
 if [[ "$(id -u)" -ne 0 ]]; then
 	echo "SKIP: need root privileges"
 	exit 0
 fi
 
-tc -j &> /dev/null
-if [[ $? -ne 0 ]]; then
-	echo "SKIP: iproute2 too old, missing JSON support"
-	exit 0
-fi
-
-tc filter help 2>&1 | grep block &> /dev/null
-if [[ $? -ne 0 ]]; then
-	echo "SKIP: iproute2 too old, missing shared block support"
-	exit 0
+if [[ "$CHECK_TC" = "yes" ]]; then
+	check_tc_version
 fi
 
 if [[ ! -x "$(command -v jq)" ]]; then
 	echo "SKIP: jq not installed"
-	exit 0
+	exit 1
 fi
 
 if [[ ! -x "$(command -v $MZ)" ]]; then
tools/testing/selftests/net/forwarding/tc_actions.sh +1 -1
index 84234317a25d..8ab5cf0a960b 100755
@@ -2,8 +2,8 @@
 # SPDX-License-Identifier: GPL-2.0
 
 NUM_NETIFS=4
-source lib.sh
 source tc_common.sh
+source lib.sh
 
 tcflags="skip_hw"
 
tools/testing/selftests/net/forwarding/tc_chains.sh +1 -1
index 94c114ad8b44..2fd15226974b 100755
@@ -2,8 +2,8 @@
 # SPDX-License-Identifier: GPL-2.0
 
 NUM_NETIFS=2
-source lib.sh
 source tc_common.sh
+source lib.sh
 
 tcflags="skip_hw"
 
tools/testing/selftests/net/forwarding/tc_common.sh +2 -0
index acd0b520241c..9d3b64a2a264 100644
@@ -1,6 +1,8 @@
 #!/bin/bash
 # SPDX-License-Identifier: GPL-2.0
 
+CHECK_TC="yes"
+
 tc_check_packets()
 {
 	local id=$1
tools/testing/selftests/net/forwarding/tc_flower.sh +1 -1
index 026a4ea4b2fb..032b882adfc0 100755
@@ -2,8 +2,8 @@
 # SPDX-License-Identifier: GPL-2.0
 
 NUM_NETIFS=2
-source lib.sh
 source tc_common.sh
+source lib.sh
 
 tcflags="skip_hw"
 
tools/testing/selftests/net/forwarding/tc_shblocks.sh +1 -1
index cfc8a2ace388..077b98048ef4 100755
@@ -2,8 +2,8 @@
 # SPDX-License-Identifier: GPL-2.0
 
 NUM_NETIFS=4
-source lib.sh
 source tc_common.sh
+source lib.sh
 
 tcflags="skip_hw"
 
Signature · 6 lines
2.11.0

--
To unsubscribe from this list: send the line "unsubscribe linux-kselftest" in
the body of a message to majordomo at vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
#2 lore raw

PATCH v2 2/4 selftests: forwarding: Handle 0 for packet difference in multipath tests

In reply to David Ahern

If the packet stats have a difference of 0, the test output shows:
INFO: Expected ratio 2.00 Measured ratio
Runtime error (func=(main), adr=9): Divide by zero (standard_in) 2: syntax error
(standard_in) 1: syntax error
./router_multipath.sh: line 187: test: : integer expression expected
TEST: Multipath [FAIL]
Too large discrepancy between expected and measured ratios

Handle the 0 and display a cleaner message:
INFO: Running IPv6 multipath tests
TEST: Multipath [FAIL]
Packet difference is 0

Signed-off-by David Ahern <dsahern at gmail.com>
Reviewed-by Ido Schimmel <idosch at mellanox.com>
---
 tools/testing/selftests/net/forwarding/router_multipath.sh | 7 +++++++
 1 file changed, 7 insertions(+)
tools/testing/selftests/net/forwarding/router_multipath.sh +7 -0
index d31888e3133e..c1a437c5b15f 100755
@@ -168,6 +168,13 @@ multipath_eval()
 
        RET=0
 
+       if [[ "$packets_rp12" -eq "0" || "$packets_rp13" -eq "0" ]]; then
+              check_err 1 "Packet difference is 0"
+              log_test "Multipath"
+              log_info "Expected ratio $weights_ratio"
+              return
+       fi
+
        if [[ "$weight_rp12" -gt "$weight_rp13" ]]; then
                weights_ratio=$(echo "scale=2; $weight_rp12 / $weight_rp13" \
 		       | bc -l)
Signature · 6 lines
2.11.0

--
To unsubscribe from this list: send the line "unsubscribe linux-kselftest" in
the body of a message to majordomo at vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
#3 lore raw

PATCH v2 3/4 selftests: forwarding: Use PING6 instead of ping for ipv6 multipath test

In reply to David Ahern

On Debian jessie ping can not handle IPv6 addresses so the command fails. Use PING6 which is set to ping6.

Signed-off-by David Ahern <dsahern at gmail.com>
Reviewed-by Ido Schimmel <idosch at mellanox.com>
---
 tools/testing/selftests/net/forwarding/router_multipath.sh | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
tools/testing/selftests/net/forwarding/router_multipath.sh +1 -1
index c1a437c5b15f..745fb40bfa44 100755
@@ -249,7 +249,7 @@ multipath6_test()
 
        # Generate 16384 echo requests, each with a random flow label.
        for _ in $(seq 1 16384); do
-	       ip vrf exec vrf-h1 ping 2001:db8:2::2 -F 0 -c 1 -q &> /dev/null
+	       ip vrf exec vrf-h1 $PING6 2001:db8:2::2 -F 0 -c 1 -q &> /dev/null
        done
 
        t1_rp12=$(link_stats_tx_packets_get $rp12)
Signature · 6 lines
2.11.0

--
To unsubscribe from this list: send the line "unsubscribe linux-kselftest" in
the body of a message to majordomo at vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
#4 lore raw

PATCH v2 4/4 selftests: forwarding: Add description to the multipath tests

In reply to David Ahern

Add a better description to the summary for multipath tests. e.g.,

INFO: Running IPv6 multipath tests
TEST: ECMP [PASS]
INFO: Expected ratio 1.00 Measured ratio 1.02
TEST: Weighted MP 2:1 [PASS]
INFO: Expected ratio 2.00 Measured ratio 2.02
TEST: Weighted MP 11:45 [PASS]
INFO: Expected ratio 4.09 Measured ratio 4.03

Signed-off-by David Ahern <dsahern at gmail.com>
Reviewed-by Ido Schimmel <idosch at mellanox.com>
---
 .../selftests/net/forwarding/router_multipath.sh   | 37 ++++++++++++----------
 1 file changed, 20 insertions(+), 17 deletions(-)
tools/testing/selftests/net/forwarding/router_multipath.sh +20 -17
index 745fb40bfa44..55595305a604 100755
@@ -160,10 +160,11 @@ router2_destroy()
 
 multipath_eval()
 {
-       local weight_rp12=$1
-       local weight_rp13=$2
-       local packets_rp12=$3
-       local packets_rp13=$4
+       local desc="$1"
+       local weight_rp12=$2
+       local weight_rp13=$3
+       local packets_rp12=$4
+       local packets_rp13=$5
        local weights_ratio packets_ratio diff
 
        RET=0
@@ -192,14 +193,15 @@ multipath_eval()
 
        test "$(echo "$diff / $weights_ratio > 0.1" | bc -l)" -eq 0
        check_err $? "Too large discrepancy between expected and measured ratios"
-       log_test "Multipath"
+       log_test "$desc"
        log_info "Expected ratio $weights_ratio Measured ratio $packets_ratio"
 }
 
 multipath4_test()
 {
-       local weight_rp12=$1
-       local weight_rp13=$2
+       local desc="$1"
+       local weight_rp12=$2
+       local weight_rp13=$3
        local t0_rp12 t0_rp13 t1_rp12 t1_rp13
        local packets_rp12 packets_rp13
        local hash_policy
@@ -224,7 +226,7 @@ multipath4_test()
 
        let "packets_rp12 = $t1_rp12 - $t0_rp12"
        let "packets_rp13 = $t1_rp13 - $t0_rp13"
-       multipath_eval $weight_rp12 $weight_rp13 $packets_rp12 $packets_rp13
+       multipath_eval "$desc" $weight_rp12 $weight_rp13 $packets_rp12 $packets_rp13
 
        # Restore settings.
        ip route replace 198.51.100.0/24 vrf vrf-r1 \
@@ -235,8 +237,9 @@ multipath4_test()
 
 multipath6_test()
 {
-       local weight_rp12=$1
-       local weight_rp13=$2
+       local desc="$1"
+       local weight_rp12=$2
+       local weight_rp13=$3
        local t0_rp12 t0_rp13 t1_rp12 t1_rp13
        local packets_rp12 packets_rp13
 
@@ -257,7 +260,7 @@ multipath6_test()
 
        let "packets_rp12 = $t1_rp12 - $t0_rp12"
        let "packets_rp13 = $t1_rp13 - $t0_rp13"
-       multipath_eval $weight_rp12 $weight_rp13 $packets_rp12 $packets_rp13
+       multipath_eval "$desc" $weight_rp12 $weight_rp13 $packets_rp12 $packets_rp13
 
        ip route replace 2001:db8:2::/64 vrf vrf-r1 \
 	       nexthop via fe80:2::22 dev $rp12 \
@@ -267,14 +270,14 @@ multipath6_test()
 multipath_test()
 {
 	log_info "Running IPv4 multipath tests"
-	multipath4_test 1 1
-	multipath4_test 2 1
-	multipath4_test 11 45
+	multipath4_test "ECMP" 1 1
+	multipath4_test "Weighted MP 2:1" 2 1
+	multipath4_test "Weighted MP 11:45" 11 45
 
 	log_info "Running IPv6 multipath tests"
-	multipath6_test 1 1
-	multipath6_test 2 1
-	multipath6_test 11 45
+	multipath6_test "ECMP" 1 1
+	multipath6_test "Weighted MP 2:1" 2 1
+	multipath6_test "Weighted MP 11:45" 11 45
 }
 
 setup_prepare()
Signature · 6 lines
2.11.0

--
To unsubscribe from this list: send the line "unsubscribe linux-kselftest" in
the body of a message to majordomo at vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
maintainer
#5 lore raw
In reply to David Ahern

From: David Ahern <dsahern at gmail.com>
Date: Thu, 1 Mar 2018 13:49:29 -0800

Quoted 10 lines

Bug fixes and an enhancement for the recent forwarding tests:
- only check tc version on tc tests
- handle multipath tests failing with 0 packet count
- fix ping command for IPv6 on Debian jessie
- improve summary of multipath tests

v2
- add CHECK_TC to bridge_vlan_aware.sh (Ido)
- dropped patch 2; always check for mz given its use
- fixed commit message for the last patch (Multipath: was dropped)

Series applied, thanks David.
--
To unsubscribe from this list: send the line "unsubscribe linux-kselftest" in the body of a message to majordomo at vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html