| Message ID | 2a424c54062a5f1efd13b9ec5b2b0e29c6af2574.1709901020.git.petrm@nvidia.com (mailing list archive) |
|---|---|
| State | Accepted |
| Commit | a22b042660ca1d695cc225401c72b3fb393acd49 |
| Delegated to: | Netdev Maintainers |
| Headers | show |
| Series | mlxsw: Support for nexthop group statistics | expand |
On Fri, 8 Mar 2024 13:59:55 +0100 Petr Machata wrote: > Add to lib.sh support for fetching NH stats, and a new library, > router_mpath_nh_lib.sh, with the common code for testing NH stats. > Use the latter from router_mpath_nh.sh and router_mpath_nh_res.sh. > > The test works by sending traffic through a NH group, and checking that the > reported values correspond to what the link that ultimately receives the > traffic reports having seen. Are the iproute2 patches still on their way? I can't find them and the test is getting skipped.
Jakub Kicinski <kuba@kernel.org> writes: > On Fri, 8 Mar 2024 13:59:55 +0100 Petr Machata wrote: >> Add to lib.sh support for fetching NH stats, and a new library, >> router_mpath_nh_lib.sh, with the common code for testing NH stats. >> Use the latter from router_mpath_nh.sh and router_mpath_nh_res.sh. >> >> The test works by sending traffic through a NH group, and checking that the >> reported values correspond to what the link that ultimately receives the >> traffic reports having seen. > > Are the iproute2 patches still on their way? > I can't find them and the test is getting skipped. I only just sent it. The code is here FWIW: https://github.com/pmachata/iproute2/commits/nh_stats
On Fri, 8 Mar 2024 23:31:57 +0100 Petr Machata wrote: > > Are the iproute2 patches still on their way? > > I can't find them and the test is getting skipped. > > I only just sent it. The code is here FWIW: > > https://github.com/pmachata/iproute2/commits/nh_stats I tried but I'll need to also sync the kernel headers. Maybe I'll wait until David applies :S Will the test still skip, tho, when running on veth?
On 3/8/24 8:48 PM, Jakub Kicinski wrote: > On Fri, 8 Mar 2024 23:31:57 +0100 Petr Machata wrote: >>> Are the iproute2 patches still on their way? >>> I can't find them and the test is getting skipped. >> >> I only just sent it. The code is here FWIW: >> >> https://github.com/pmachata/iproute2/commits/nh_stats > > I tried but I'll need to also sync the kernel headers. > Maybe I'll wait until David applies :S > Will the test still skip, tho, when running on veth? marked the set; will try to get to it tomorrow.
Jakub Kicinski <kuba@kernel.org> writes: > On Fri, 8 Mar 2024 23:31:57 +0100 Petr Machata wrote: >> > Are the iproute2 patches still on their way? >> > I can't find them and the test is getting skipped. >> >> I only just sent it. The code is here FWIW: >> >> https://github.com/pmachata/iproute2/commits/nh_stats > > I tried but I'll need to also sync the kernel headers. > Maybe I'll wait until David applies :S > Will the test still skip, tho, when running on veth? It should run the SW parts and skip the HW ones.
Petr Machata <petrm@nvidia.com> writes: > Jakub Kicinski <kuba@kernel.org> writes: > >> On Fri, 8 Mar 2024 23:31:57 +0100 Petr Machata wrote: >>> > Are the iproute2 patches still on their way? >>> > I can't find them and the test is getting skipped. >>> >>> I only just sent it. The code is here FWIW: >>> >>> https://github.com/pmachata/iproute2/commits/nh_stats >> >> I tried but I'll need to also sync the kernel headers. >> Maybe I'll wait until David applies :S >> Will the test still skip, tho, when running on veth? > > It should run the SW parts and skip the HW ones. In fact why don't I paste the run I still have in the terminal from last week: [root@virtme-ng forwarding]# TESTS=" nh_stats_test_v4 nh_stats_test_v6 " ./router_mpath_nh_res.sh TEST: NH stats test IPv4 [ OK ] TEST: HW stats not offloaded on veth topology [SKIP] TEST: NH stats test IPv6 [ OK ] TEST: HW stats not offloaded on veth topology [SKIP]
On Mon, 11 Mar 2024 12:00:59 +0100 Petr Machata wrote: > > It should run the SW parts and skip the HW ones. > > In fact why don't I paste the run I still have in the terminal from last > week: > > [root@virtme-ng forwarding]# TESTS=" nh_stats_test_v4 nh_stats_test_v6 " ./router_mpath_nh_res.sh > TEST: NH stats test IPv4 [ OK ] > TEST: HW stats not offloaded on veth topology [SKIP] > TEST: NH stats test IPv6 [ OK ] > TEST: HW stats not offloaded on veth topology [SKIP] SKIP beats PASS so the result for the entire test (according to our local result parser) will be SKIP. Can we switch to XFAIL?
Jakub Kicinski <kuba@kernel.org> writes: > On Mon, 11 Mar 2024 12:00:59 +0100 Petr Machata wrote: >> > It should run the SW parts and skip the HW ones. >> >> In fact why don't I paste the run I still have in the terminal from last >> week: >> >> [root@virtme-ng forwarding]# TESTS=" nh_stats_test_v4 nh_stats_test_v6 " ./router_mpath_nh_res.sh >> TEST: NH stats test IPv4 [ OK ] >> TEST: HW stats not offloaded on veth topology [SKIP] >> TEST: NH stats test IPv6 [ OK ] >> TEST: HW stats not offloaded on veth topology [SKIP] > > SKIP beats PASS so the result for the entire test (according to our > local result parser) will be SKIP. Can we switch to XFAIL? Sure. But I don't think SKIP should trump PASS. IMHO SKIP is the weakest status, soon as you have one PASS, that's what the overall outcome should be. I guess the logic behind it was that it's useful to see the tests that skip?
On Mon, 11 Mar 2024 17:30:54 +0100 Petr Machata wrote: > But I don't think SKIP should trump PASS. IMHO SKIP is the weakest > status, soon as you have one PASS, that's what the overall outcome > should be. I guess the logic behind it was that it's useful to see the > tests that skip? SKIP is a signal that something could not be run. People maintaining CIs will try to investigate why. The signal for "everything is working as expected" is PASS, possibly XFAIL if you want to signal that the test didn't run/pass but that's expected.
diff --git a/tools/testing/selftests/net/forwarding/Makefile b/tools/testing/selftests/net/forwarding/Makefile index cdefc9a5ec34..535865b3d1d6 100644 --- a/tools/testing/selftests/net/forwarding/Makefile +++ b/tools/testing/selftests/net/forwarding/Makefile @@ -123,6 +123,7 @@ TEST_FILES := devlink_lib.sh \ mirror_gre_topo_lib.sh \ mirror_lib.sh \ mirror_topo_lib.sh \ + router_mpath_nh_lib.sh \ sch_ets_core.sh \ sch_ets_tests.sh \ sch_tbf_core.sh \ diff --git a/tools/testing/selftests/net/forwarding/lib.sh b/tools/testing/selftests/net/forwarding/lib.sh index d1bf39eaf2b3..e579c2e0c462 100644 --- a/tools/testing/selftests/net/forwarding/lib.sh +++ b/tools/testing/selftests/net/forwarding/lib.sh @@ -900,6 +900,33 @@ hw_stats_get() jq ".[0].stats64.$dir.$stat" } +__nh_stats_get() +{ + local key=$1; shift + local group_id=$1; shift + local member_id=$1; shift + + ip -j -s -s nexthop show id $group_id | + jq --argjson member_id "$member_id" --arg key "$key" \ + '.[].group_stats[] | select(.id == $member_id) | .[$key]' +} + +nh_stats_get() +{ + local group_id=$1; shift + local member_id=$1; shift + + __nh_stats_get packets "$group_id" "$member_id" +} + +nh_stats_get_hw() +{ + local group_id=$1; shift + local member_id=$1; shift + + __nh_stats_get packets_hw "$group_id" "$member_id" +} + humanize() { local speed=$1; shift @@ -2010,3 +2037,10 @@ bail_on_lldpad() fi fi } + +absval() +{ + local v=$1; shift + + echo $((v > 0 ? v : -v)) +} diff --git a/tools/testing/selftests/net/forwarding/router_mpath_nh.sh b/tools/testing/selftests/net/forwarding/router_mpath_nh.sh index 982e0d098ea9..3f0f5dc95542 100755 --- a/tools/testing/selftests/net/forwarding/router_mpath_nh.sh +++ b/tools/testing/selftests/net/forwarding/router_mpath_nh.sh @@ -7,9 +7,12 @@ ALL_TESTS=" multipath_test ping_ipv4_blackhole ping_ipv6_blackhole + nh_stats_test_v4 + nh_stats_test_v6 " NUM_NETIFS=8 source lib.sh +source router_mpath_nh_lib.sh h1_create() { @@ -325,6 +328,16 @@ ping_ipv6_blackhole() ip -6 nexthop del id 1001 } +nh_stats_test_v4() +{ + __nh_stats_test_v4 mpath +} + +nh_stats_test_v6() +{ + __nh_stats_test_v6 mpath +} + setup_prepare() { h1=${NETIFS[p1]} diff --git a/tools/testing/selftests/net/forwarding/router_mpath_nh_lib.sh b/tools/testing/selftests/net/forwarding/router_mpath_nh_lib.sh new file mode 100644 index 000000000000..7e7d62161c34 --- /dev/null +++ b/tools/testing/selftests/net/forwarding/router_mpath_nh_lib.sh @@ -0,0 +1,129 @@ +# SPDX-License-Identifier: GPL-2.0 + +nh_stats_do_test() +{ + local what=$1; shift + local nh1_id=$1; shift + local nh2_id=$1; shift + local group_id=$1; shift + local stats_get=$1; shift + local mz="$@" + + local dp + + RET=0 + + sleep 2 + for ((dp=0; dp < 60000; dp += 10000)); do + local dd + local t0_rp12=$(link_stats_tx_packets_get $rp12) + local t0_rp13=$(link_stats_tx_packets_get $rp13) + local t0_nh1=$($stats_get $group_id $nh1_id) + local t0_nh2=$($stats_get $group_id $nh2_id) + + ip vrf exec vrf-h1 \ + $mz -q -p 64 -d 0 -t udp \ + "sp=1024,dp=$((dp))-$((dp + 10000))" + sleep 2 + + local t1_rp12=$(link_stats_tx_packets_get $rp12) + local t1_rp13=$(link_stats_tx_packets_get $rp13) + local t1_nh1=$($stats_get $group_id $nh1_id) + local t1_nh2=$($stats_get $group_id $nh2_id) + + local d_rp12=$((t1_rp12 - t0_rp12)) + local d_rp13=$((t1_rp13 - t0_rp13)) + local d_nh1=$((t1_nh1 - t0_nh1)) + local d_nh2=$((t1_nh2 - t0_nh2)) + + dd=$(absval $((d_rp12 - d_nh1))) + ((dd < 10)) + check_err $? "Discrepancy between link and $stats_get: d_rp12=$d_rp12 d_nh1=$d_nh1" + + dd=$(absval $((d_rp13 - d_nh2))) + ((dd < 10)) + check_err $? "Discrepancy between link and $stats_get: d_rp13=$d_rp13 d_nh2=$d_nh2" + done + + log_test "NH stats test $what" +} + +nh_stats_test_dispatch_swhw() +{ + local what=$1; shift + local nh1_id=$1; shift + local nh2_id=$1; shift + local group_id=$1; shift + local mz="$@" + + local used + + nh_stats_do_test "$what" "$nh1_id" "$nh2_id" "$group_id" \ + nh_stats_get "${mz[@]}" + + used=$(ip -s -j -d nexthop show id $group_id | + jq '.[].hw_stats.used') + kind=$(ip -j -d link show dev $rp11 | + jq -r '.[].linkinfo.info_kind') + if [[ $used == true ]]; then + nh_stats_do_test "HW $what" "$nh1_id" "$nh2_id" "$group_id" \ + nh_stats_get_hw "${mz[@]}" + elif [[ $kind == veth ]]; then + log_test_skip "HW stats not offloaded on veth topology" + fi +} + +nh_stats_test_dispatch() +{ + local nhgtype=$1; shift + local what=$1; shift + local nh1_id=$1; shift + local nh2_id=$1; shift + local group_id=$1; shift + local mz="$@" + + local enabled + local kind + + if ! ip nexthop help 2>&1 | grep -q hw_stats; then + log_test_skip "NH stats test: ip doesn't support HW stats" + return + fi + + ip nexthop replace id $group_id group $nh1_id/$nh2_id \ + hw_stats on type $nhgtype + enabled=$(ip -s -j -d nexthop show id $group_id | + jq '.[].hw_stats.enabled') + if [[ $enabled == true ]]; then + nh_stats_test_dispatch_swhw "$what" "$nh1_id" "$nh2_id" \ + "$group_id" "${mz[@]}" + elif [[ $enabled == false ]]; then + check_err 1 "HW stats still disabled after enabling" + log_test "NH stats test" + else + log_test_skip "NH stats test: ip doesn't report hw_stats info" + fi + + ip nexthop replace id $group_id group $nh1_id/$nh2_id \ + hw_stats off type $nhgtype +} + +__nh_stats_test_v4() +{ + local nhgtype=$1; shift + + sysctl_set net.ipv4.fib_multipath_hash_policy 1 + nh_stats_test_dispatch $nhgtype "IPv4" 101 102 103 \ + $MZ $h1 -A 192.0.2.2 -B 198.51.100.2 + sysctl_restore net.ipv4.fib_multipath_hash_policy +} + +__nh_stats_test_v6() +{ + local nhgtype=$1; shift + + sysctl_set net.ipv6.fib_multipath_hash_policy 1 + nh_stats_test_dispatch $nhgtype "IPv6" 104 105 106 \ + $MZ -6 $h1 -A 2001:db8:1::2 -B 2001:db8:2::2 + sysctl_restore net.ipv6.fib_multipath_hash_policy +} diff --git a/tools/testing/selftests/net/forwarding/router_mpath_nh_res.sh b/tools/testing/selftests/net/forwarding/router_mpath_nh_res.sh index a60ff54723b7..4b483d24ad00 100755 --- a/tools/testing/selftests/net/forwarding/router_mpath_nh_res.sh +++ b/tools/testing/selftests/net/forwarding/router_mpath_nh_res.sh @@ -5,9 +5,12 @@ ALL_TESTS=" ping_ipv4 ping_ipv6 multipath_test + nh_stats_test_v4 + nh_stats_test_v6 " NUM_NETIFS=8 source lib.sh +source router_mpath_nh_lib.sh h1_create() { @@ -333,6 +336,16 @@ multipath_test() ip nexthop replace id 106 group 104,1/105,1 type resilient } +nh_stats_test_v4() +{ + __nh_stats_test_v4 resilient +} + +nh_stats_test_v6() +{ + __nh_stats_test_v6 resilient +} + setup_prepare() { h1=${NETIFS[p1]}
Add to lib.sh support for fetching NH stats, and a new library, router_mpath_nh_lib.sh, with the common code for testing NH stats. Use the latter from router_mpath_nh.sh and router_mpath_nh_res.sh. The test works by sending traffic through a NH group, and checking that the reported values correspond to what the link that ultimately receives the traffic reports having seen. Signed-off-by: Petr Machata <petrm@nvidia.com> --- .../testing/selftests/net/forwarding/Makefile | 1 + tools/testing/selftests/net/forwarding/lib.sh | 34 +++++ .../net/forwarding/router_mpath_nh.sh | 13 ++ .../net/forwarding/router_mpath_nh_lib.sh | 129 ++++++++++++++++++ .../net/forwarding/router_mpath_nh_res.sh | 13 ++ 5 files changed, 190 insertions(+) create mode 100644 tools/testing/selftests/net/forwarding/router_mpath_nh_lib.sh