Bug fixes and an enhancement for the recent forwarding tests: - only check tc version on tc tests - only check for mz for tests that will use it - handle multipath tests failing with 0 packet count - fix ping command for IPv6 on Debian jessie - improve summary of multipath tests
BTW: who is the cruel person that made whitespace 7 spaces in these files?
David Ahern (5): selftests: forwarding: Only check tc version for tc tests selftests: forwarding: Only check for mz when it is needed 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
tools/testing/selftests/net/forwarding/lib.sh | 37 ++++++++++------- .../selftests/net/forwarding/router_multipath.sh | 47 +++++++++++++--------- .../testing/selftests/net/forwarding/tc_actions.sh | 2 +- .../testing/selftests/net/forwarding/tc_chains.sh | 2 +- .../testing/selftests/net/forwarding/tc_common.sh | 3 ++ .../testing/selftests/net/forwarding/tc_flower.sh | 2 +- .../selftests/net/forwarding/tc_shblocks.sh | 2 +- 7 files changed, 59 insertions(+), 36 deletions(-)
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@gmail.com --- 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 +- 6 files changed, 24 insertions(+), 15 deletions(-)
diff --git a/tools/testing/selftests/net/forwarding/lib.sh b/tools/testing/selftests/net/forwarding/lib.sh index 23866a685f77..d0af52109360 100644 --- a/tools/testing/selftests/net/forwarding/lib.sh +++ b/tools/testing/selftests/net/forwarding/lib.sh @@ -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 diff --git a/tools/testing/selftests/net/forwarding/tc_actions.sh b/tools/testing/selftests/net/forwarding/tc_actions.sh index 84234317a25d..8ab5cf0a960b 100755 --- a/tools/testing/selftests/net/forwarding/tc_actions.sh +++ b/tools/testing/selftests/net/forwarding/tc_actions.sh @@ -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"
diff --git a/tools/testing/selftests/net/forwarding/tc_chains.sh b/tools/testing/selftests/net/forwarding/tc_chains.sh index 94c114ad8b44..2fd15226974b 100755 --- a/tools/testing/selftests/net/forwarding/tc_chains.sh +++ b/tools/testing/selftests/net/forwarding/tc_chains.sh @@ -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"
diff --git a/tools/testing/selftests/net/forwarding/tc_common.sh b/tools/testing/selftests/net/forwarding/tc_common.sh index acd0b520241c..9d3b64a2a264 100644 --- a/tools/testing/selftests/net/forwarding/tc_common.sh +++ b/tools/testing/selftests/net/forwarding/tc_common.sh @@ -1,6 +1,8 @@ #!/bin/bash # SPDX-License-Identifier: GPL-2.0
+CHECK_TC="yes" + tc_check_packets() { local id=$1 diff --git a/tools/testing/selftests/net/forwarding/tc_flower.sh b/tools/testing/selftests/net/forwarding/tc_flower.sh index 026a4ea4b2fb..032b882adfc0 100755 --- a/tools/testing/selftests/net/forwarding/tc_flower.sh +++ b/tools/testing/selftests/net/forwarding/tc_flower.sh @@ -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"
diff --git a/tools/testing/selftests/net/forwarding/tc_shblocks.sh b/tools/testing/selftests/net/forwarding/tc_shblocks.sh index cfc8a2ace388..077b98048ef4 100755 --- a/tools/testing/selftests/net/forwarding/tc_shblocks.sh +++ b/tools/testing/selftests/net/forwarding/tc_shblocks.sh @@ -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"
On Thu, Mar 01, 2018 at 11:09:43AM -0800, David Ahern wrote:
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@gmail.com
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 +-
bridge_vlan_aware.sh also uses tc to check flooding and learning. Looks good otherwise.
Reviewed-by: Ido Schimmel idosch@mellanox.com
Thanks, David! -- To unsubscribe from this list: send the line "unsubscribe linux-kselftest" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Add a CHECK_MZ flag and only check if mz exists when flag is set. If it does not exist, exit non-0.
Signed-off-by: David Ahern dsahern@gmail.com --- tools/testing/selftests/net/forwarding/lib.sh | 8 +++++--- tools/testing/selftests/net/forwarding/router_multipath.sh | 1 + tools/testing/selftests/net/forwarding/tc_common.sh | 1 + 3 files changed, 7 insertions(+), 3 deletions(-)
diff --git a/tools/testing/selftests/net/forwarding/lib.sh b/tools/testing/selftests/net/forwarding/lib.sh index d0af52109360..f21b3ac11d27 100644 --- a/tools/testing/selftests/net/forwarding/lib.sh +++ b/tools/testing/selftests/net/forwarding/lib.sh @@ -48,9 +48,11 @@ if [[ ! -x "$(command -v jq)" ]]; then exit 1 fi
-if [[ ! -x "$(command -v $MZ)" ]]; then - echo "SKIP: $MZ not installed" - exit 0 +if [[ "$CHECK_MZ" = "yes" ]]; then + if [[ ! -x "$(command -v $MZ)" ]]; then + echo "SKIP: $MZ not installed" + exit 1 + fi fi
if [[ ! -v NUM_NETIFS ]]; then diff --git a/tools/testing/selftests/net/forwarding/router_multipath.sh b/tools/testing/selftests/net/forwarding/router_multipath.sh index d31888e3133e..d40016443272 100755 --- a/tools/testing/selftests/net/forwarding/router_multipath.sh +++ b/tools/testing/selftests/net/forwarding/router_multipath.sh @@ -2,6 +2,7 @@ # SPDX-License-Identifier: GPL-2.0
NUM_NETIFS=8 +CHECK_MZ="yes" source lib.sh
h1_create() diff --git a/tools/testing/selftests/net/forwarding/tc_common.sh b/tools/testing/selftests/net/forwarding/tc_common.sh index 9d3b64a2a264..d93cd7599bc1 100644 --- a/tools/testing/selftests/net/forwarding/tc_common.sh +++ b/tools/testing/selftests/net/forwarding/tc_common.sh @@ -2,6 +2,7 @@ # SPDX-License-Identifier: GPL-2.0
CHECK_TC="yes" +CHECK_MZ="yes"
tc_check_packets() {
On Thu, Mar 01, 2018 at 11:09:44AM -0800, David Ahern wrote:
Add a CHECK_MZ flag and only check if mz exists when flag is set. If it does not exist, exit non-0.
Signed-off-by: David Ahern dsahern@gmail.com
The only test that doesn't require it is router.sh so I'm not sure it's worth the effort. If you still want to keep the patch then bridge_vlan_aware.sh needs to be patched as well. -- To unsubscribe from this list: send the line "unsubscribe linux-kselftest" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 3/1/18 12:36 PM, Ido Schimmel wrote:
On Thu, Mar 01, 2018 at 11:09:44AM -0800, David Ahern wrote:
Add a CHECK_MZ flag and only check if mz exists when flag is set. If it does not exist, exit non-0.
Signed-off-by: David Ahern dsahern@gmail.com
The only test that doesn't require it is router.sh so I'm not sure it's worth the effort. If you still want to keep the patch then bridge_vlan_aware.sh needs to be patched as well.
ok, I'll drop this one -- To unsubscribe from this list: send the line "unsubscribe linux-kselftest" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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@gmail.com --- tools/testing/selftests/net/forwarding/router_multipath.sh | 7 +++++++ 1 file changed, 7 insertions(+)
diff --git a/tools/testing/selftests/net/forwarding/router_multipath.sh b/tools/testing/selftests/net/forwarding/router_multipath.sh index d40016443272..9c66b3fbf994 100755 --- a/tools/testing/selftests/net/forwarding/router_multipath.sh +++ b/tools/testing/selftests/net/forwarding/router_multipath.sh @@ -169,6 +169,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)
On Thu, Mar 01, 2018 at 11:09:45AM -0800, David Ahern wrote:
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@gmail.com
Reviewed-by: Ido Schimmel idosch@mellanox.com
Thanks! -- To unsubscribe from this list: send the line "unsubscribe linux-kselftest" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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@gmail.com --- tools/testing/selftests/net/forwarding/router_multipath.sh | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/tools/testing/selftests/net/forwarding/router_multipath.sh b/tools/testing/selftests/net/forwarding/router_multipath.sh index 9c66b3fbf994..e227ec052804 100755 --- a/tools/testing/selftests/net/forwarding/router_multipath.sh +++ b/tools/testing/selftests/net/forwarding/router_multipath.sh @@ -250,7 +250,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)
On Thu, Mar 01, 2018 at 11:09:46AM -0800, David Ahern wrote:
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@gmail.com
Yep, I forgot to replace that one.
Reviewed-by: Ido Schimmel idosch@mellanox.com
BTW, I have a patch to convert the IPv6 multipath test to use mz. I'm waiting for your L4 hash patches to be accepted :) -- To unsubscribe from this list: send the line "unsubscribe linux-kselftest" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 3/1/18 12:39 PM, Ido Schimmel wrote:
On Thu, Mar 01, 2018 at 11:09:46AM -0800, David Ahern wrote:
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@gmail.com
Yep, I forgot to replace that one.
Reviewed-by: Ido Schimmel idosch@mellanox.com
BTW, I have a patch to convert the IPv6 multipath test to use mz. I'm waiting for your L4 hash patches to be accepted :)
Can you forward it? That is what is driving this series. -- To unsubscribe from this list: send the line "unsubscribe linux-kselftest" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Thu, Mar 01, 2018 at 12:43:42PM -0700, David Ahern wrote:
On 3/1/18 12:39 PM, Ido Schimmel wrote:
On Thu, Mar 01, 2018 at 11:09:46AM -0800, David Ahern wrote:
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@gmail.com
Yep, I forgot to replace that one.
Reviewed-by: Ido Schimmel idosch@mellanox.com
BTW, I have a patch to convert the IPv6 multipath test to use mz. I'm waiting for your L4 hash patches to be accepted :)
Can you forward it? That is what is driving this series.
Feel free to submit with your patches (sorry about the spaces!):
diff --git a/tools/testing/selftests/net/forwarding/router_multipath.sh b/tools/testing/selftests/net/forwarding/router_multipath.sh index d31888e3133e..3ebd9cca06d0 100755 --- a/tools/testing/selftests/net/forwarding/router_multipath.sh +++ b/tools/testing/selftests/net/forwarding/router_multipath.sh @@ -232,7 +232,10 @@ multipath6_test() local weight_rp13=$2 local t0_rp12 t0_rp13 t1_rp12 t1_rp13 local packets_rp12 packets_rp13 + local hash_policy
+ hash_policy=$(sysctl -n net.ipv6.fib_multipath_hash_policy) + sysctl -q -w net.ipv6.fib_multipath_hash_policy=1 ip route replace 2001:db8:2::/64 vrf vrf-r1 \ nexthop via fe80:2::22 dev $rp12 weight $weight_rp12 \ nexthop via fe80:3::23 dev $rp13 weight $weight_rp13 @@ -240,10 +243,8 @@ multipath6_test() t0_rp12=$(link_stats_tx_packets_get $rp12) t0_rp13=$(link_stats_tx_packets_get $rp13)
- # 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 - done + $MZ $h1 -6 -q -p 64 -A 2001:db8:1::2 -B 2001:db8:2::2 -d 1msec \ + -t udp "sp=1024,dp=0-32768"
t1_rp12=$(link_stats_tx_packets_get $rp12) t1_rp13=$(link_stats_tx_packets_get $rp13) @@ -255,6 +256,7 @@ multipath6_test() ip route replace 2001:db8:2::/64 vrf vrf-r1 \ nexthop via fe80:2::22 dev $rp12 \ nexthop via fe80:3::23 dev $rp13 + sysctl -q -w net.ipv6.fib_multipath_hash_policy=$hash_policy }
multipath_test()
-- To unsubscribe from this list: send the line "unsubscribe linux-kselftest" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Add a better description to the summary for multipath tests. e.g.,
INFO: Running IPv6 multipath tests TEST: Multipath: ECMP [PASS] INFO: Expected ratio 1.00 Measured ratio 1.02 TEST: Multipath: Weighted MP 2:1 [PASS] INFO: Expected ratio 2.00 Measured ratio 2.02 TEST: Multipath: Weighted MP 11:45 [PASS] INFO: Expected ratio 4.09 Measured ratio 4.03
Signed-off-by: David Ahern dsahern@gmail.com --- .../selftests/net/forwarding/router_multipath.sh | 37 ++++++++++++---------- 1 file changed, 20 insertions(+), 17 deletions(-)
diff --git a/tools/testing/selftests/net/forwarding/router_multipath.sh b/tools/testing/selftests/net/forwarding/router_multipath.sh index e227ec052804..f03f628b6dd6 100755 --- a/tools/testing/selftests/net/forwarding/router_multipath.sh +++ b/tools/testing/selftests/net/forwarding/router_multipath.sh @@ -161,10 +161,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 @@ -193,14 +194,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 @@ -225,7 +227,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 \ @@ -236,8 +238,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
@@ -258,7 +261,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 \ @@ -268,14 +271,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()
On Thu, Mar 01, 2018 at 11:09:47AM -0800, David Ahern wrote:
Add a better description to the summary for multipath tests. e.g.,
INFO: Running IPv6 multipath tests TEST: Multipath: ECMP [PASS] INFO: Expected ratio 1.00 Measured ratio 1.02 TEST: Multipath: Weighted MP 2:1 [PASS] INFO: Expected ratio 2.00 Measured ratio 2.02 TEST: Multipath: Weighted MP 11:45 [PASS] INFO: Expected ratio 4.09 Measured ratio 4.03
Signed-off-by: David Ahern dsahern@gmail.com
I tried to make the output as nice as the tests you sent me, but I failed. Happy you pitched in :)
Reviewed-by: Ido Schimmel idosch@mellanox.com -- To unsubscribe from this list: send the line "unsubscribe linux-kselftest" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Thu, Mar 01, 2018 at 11:09:42AM -0800, David Ahern wrote:
Bug fixes and an enhancement for the recent forwarding tests:
- only check tc version on tc tests
- only check for mz for tests that will use it
- handle multipath tests failing with 0 packet count
- fix ping command for IPv6 on Debian jessie
- improve summary of multipath tests
BTW: who is the cruel person that made whitespace 7 spaces in these files?
I see this in some functions in router_multipath.sh . Not sure how that happened... Will be more careful in the future. -- To unsubscribe from this list: send the line "unsubscribe linux-kselftest" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
linux-kselftest-mirror@lists.linaro.org
-
David Ahern -
Ido Schimmel