Ioana Ciornei <[email protected]> writes:
> Add a new selftest - ethtool_std_stats.sh - which validates the > eth-ctrl, eth-mac and pause standard statistics exported by an > interface. Not all counters can be validated in software, especially > those that are keeping track of errors. Counters such as > SingleCollisionFrames, FrameCheckSequenceErrors etc are not tested nor > included in this new selftest. > > The central part of this patch is the traffic_test() function which > gathers the 'before' counter values, sends a batch of traffic and then > interrogates again the same counters in order to determine if the delta > is on target. The function receives an array through which the caller > can request what counters to be interrogated and, for each of them, what > is their target delta value. > > The output from this selftest looks as follows on a LX2160ARDB board: > > $ ./ethtool_std_stats.sh eth0 eth1 > TEST: eth-ctrl tx on eth0 (MACControlFramesTransmitted on eth0) [ OK ] > TEST: eth-ctrl tx on eth0 (MACControlFramesReceived on eth1) [ OK ] > TEST: eth-ctrl tx on eth1 (MACControlFramesTransmitted on eth1) [ OK ] > TEST: eth-ctrl tx on eth1 (MACControlFramesReceived on eth0) [ OK ] > TEST: eth-mac bcast tx on eth0 (BroadcastFramesXmittedOK on eth0) [ OK ] > TEST: eth-mac bcast tx on eth0 (FramesTransmittedOK on eth0) [ OK ] > TEST: eth-mac bcast tx on eth0 (OctetsTransmittedOK on eth0) [ OK ] > TEST: eth-mac bcast tx on eth0 (MulticastFramesXmittedOK on eth0) [ OK ] > TEST: eth-mac bcast tx on eth0 (BroadcastFramesReceivedOK on eth1) [ OK ] > TEST: eth-mac bcast tx on eth0 (FramesReceivedOK on eth1) [ OK ] > TEST: eth-mac bcast tx on eth0 (OctetsReceivedOK on eth1) [ OK ] > TEST: eth-mac bcast tx on eth0 (MulticastFramesReceivedOK on eth1) [ OK ] > TEST: eth-mac mcast tx on eth0 (BroadcastFramesXmittedOK on eth0) [ OK ] > TEST: eth-mac mcast tx on eth0 (FramesTransmittedOK on eth0) [ OK ] > TEST: eth-mac mcast tx on eth0 (OctetsTransmittedOK on eth0) [ OK ] > TEST: eth-mac mcast tx on eth0 (MulticastFramesXmittedOK on eth0) [ OK ] > TEST: eth-mac mcast tx on eth0 (BroadcastFramesReceivedOK on eth1) [ OK ] > TEST: eth-mac mcast tx on eth0 (FramesReceivedOK on eth1) [ OK ] > TEST: eth-mac mcast tx on eth0 (OctetsReceivedOK on eth1) [ OK ] > TEST: eth-mac mcast tx on eth0 (MulticastFramesReceivedOK on eth1) [ OK ] > TEST: eth-mac ucast tx on eth0 (BroadcastFramesXmittedOK on eth0) [ OK ] > TEST: eth-mac ucast tx on eth0 (FramesTransmittedOK on eth0) [ OK ] > TEST: eth-mac ucast tx on eth0 (OctetsTransmittedOK on eth0) [ OK ] > TEST: eth-mac ucast tx on eth0 (MulticastFramesXmittedOK on eth0) [ OK ] > TEST: eth-mac ucast tx on eth0 (BroadcastFramesReceivedOK on eth1) [ OK ] > TEST: eth-mac ucast tx on eth0 (FramesReceivedOK on eth1) [ OK ] > TEST: eth-mac ucast tx on eth0 (OctetsReceivedOK on eth1) [ OK ] > TEST: eth-mac ucast tx on eth0 (MulticastFramesReceivedOK on eth1) [ OK ] > TEST: pause tx on eth0 (tx_pause_frames on eth0) [ OK ] > TEST: pause tx on eth0 (rx_pause_frames on eth1) [ OK ] > TEST: pause tx on eth1 (tx_pause_frames on eth1) [ OK ] > TEST: pause tx on eth1 (rx_pause_frames on eth0) [ OK ] > > Please note that not all MACs are counting the software injected pause > frames as real Tx pause. For example, on a LS1028ARDB the selftest > output will reflect the fact that neither the ENETC MAC, nor the Felix > switch MAC are able to detect Tx pause frames injected by software. > > $ ./ethtool_std_stats.sh eno0 swp0 > (...) > TEST: Not all MACs detect injected pause frames [XFAIL] > TEST: pause tx on eno0 (rx_pause_frames on swp0) [ OK ] > TEST: Not all MACs detect injected pause frames [XFAIL] > TEST: pause tx on swp0 (rx_pause_frames on eno0) [ OK ] > > Signed-off-by: Ioana Ciornei <[email protected]> > --- > .../testing/selftests/drivers/net/hw/Makefile | 1 + > .../drivers/net/hw/ethtool_std_stats.sh | 192 ++++++++++++++++++ > 2 files changed, 193 insertions(+) > create mode 100755 > tools/testing/selftests/drivers/net/hw/ethtool_std_stats.sh > > diff --git a/tools/testing/selftests/drivers/net/hw/Makefile > b/tools/testing/selftests/drivers/net/hw/Makefile > index a64140333a46..d447813a14b2 100644 > --- a/tools/testing/selftests/drivers/net/hw/Makefile > +++ b/tools/testing/selftests/drivers/net/hw/Makefile > @@ -26,6 +26,7 @@ TEST_PROGS = \ > ethtool_extended_state.sh \ > ethtool_mm.sh \ > ethtool_rmon.sh \ > + ethtool_std_stats.sh \ > hw_stats_l3.sh \ > hw_stats_l3_gre.sh \ > iou-zcrx.py \ > diff --git a/tools/testing/selftests/drivers/net/hw/ethtool_std_stats.sh > b/tools/testing/selftests/drivers/net/hw/ethtool_std_stats.sh > new file mode 100755 > index 000000000000..4ce8f140a18c > --- /dev/null > +++ b/tools/testing/selftests/drivers/net/hw/ethtool_std_stats.sh > @@ -0,0 +1,192 @@ > +#!/bin/bash > +# SPDX-License-Identifier: GPL-2.0 > +#shellcheck disable=SC2034 # SC does not see the global variables > + > +ALL_TESTS=" > + test_eth_ctrl_stats > + test_eth_mac_stats > + test_pause_stats > +" > +STABLE_MAC_ADDRS=yes > +NUM_NETIFS=2 > +lib_dir=$(dirname "$0") > +# shellcheck source=./../../../net/forwarding/lib.sh > +source "$lib_dir"/../../../net/forwarding/lib.sh > + > +traffic_test() > +{ > + local iface=$1; shift > + local neigh=$1; shift > + local num_tx=$1; shift > + local pkt_format="$1"; shift > + local title="$1"; shift > + declare -a counters=( "${@:2:$1}" ); shift "$(( $1 + 1 ))" Please make this local -a. declare inside a function also introduces a local variable, so the effect is the same, but using local is more obvious. BTW, just a suggestion, personally I'd just make the counters a pure "rest" argument: local -a counters=("$@") simply because functions usually don't need more than one, and it's easier to use on call site, and easier to understand what's going on. > + local before after delta target_high extra > + local int grp counter target unit > + local num_rx=$((num_tx * 2)) > + local xfail_message > + local src="aggregate" And local i. > + > + for i in "${!counters[@]}"; do > + read -r int grp counter target unit xfail_message <<< > "${counters[$i]}" > + before[i]=$(ethtool_std_stats_get "$int" "$grp" "$counter" > "$src") > + done > + > + # shellcheck disable=SC2086 # needs split options > + $MZ "$iface" -q -c "$num_tx" $pkt_format > + > + # shellcheck disable=SC2086 # needs split options > + $MZ "$neigh" -q -c "$num_rx" $pkt_format > + > + for i in "${!counters[@]}"; do There should be a RET=0 here, so that the check_err below gets a clean slate. > + read -r int grp counter target unit xfail_message<<< > "${counters[$i]}" > + > + after[i]=$(ethtool_std_stats_get "$int" "$grp" "$counter" > "$src") > + if [[ "${after[$i]}" == "null" ]]; then > + log_test_skip "$int does not support $grp-$counter" > + continue; > + fi > + > + delta=$((after[i] - before[i])) > + > + # Allow an extra 1% tolerance for random packets sent by the > stack > + extra=$((num_pkts * unit / 100)) > + target_high=$((target + extra)) > + > + RET=0 > + [ "$delta" -ge "$target" ] && [ "$delta" -le "$target_high" ] > + err="$?" > + if [[ $err != 0 ]] && [[ -n $xfail_message ]]; then > + log_test_xfail "$xfail_message" I think this should mention the $title as well. I think that the xfail_message could be the log_test's opt_str, so: log_test_xfail "$title" "$xfail_message" A bit annoying that log_test_xfail clears the retmsg. It does so so that messages from previous runs do not show up, which is desirable in general, but here it would be handy. > + continue; > + fi > + check_err "$err" "$grp-$counter is not valid on $int (expected > $target, got $delta)" > + log_test "$title" "$counter on $int" > + done > +} > + > +test_eth_ctrl_stats() > +{ > + local pkt_format="-a own -b bcast 88:08 -p 64" > + local num_pkts=1000 > + local counters local -a (Though yeah, bash doesn't care.) > + > + counters=("$h1 eth-ctrl MACControlFramesTransmitted $num_pkts 1" > + "$h2 eth-ctrl MACControlFramesReceived $num_pkts 1") > + traffic_test "$h1" "$h2" "$num_pkts" "$pkt_format" \ > + "eth-ctrl tx on $h1" \ > + "${#counters[@]}" "${counters[@]}" > + > + counters=("$h2 eth-ctrl MACControlFramesTransmitted $num_pkts 1" > + "$h1 eth-ctrl MACControlFramesReceived $num_pkts 1") > + traffic_test "$h2" "$h1" "$num_pkts" "$pkt_format" \ > + "eth-ctrl tx on $h2" \ > + "${#counters[@]}" "${counters[@]}" > +} > + > +test_eth_mac_stats() > +{ > + local pkt_size=100 > + local pkt_size_fcs=$((pkt_size + 4)) > + local bcast_pkt_format="-a own -b bcast -p $pkt_size" > + local mcast_pkt_format="-a own -b -b 01:00:5E:00:00:01 -p $pkt_size" > + local ucast_pkt_format="-a own -b $h2_mac -p $pkt_size" > + local num_pkts=2000 > + local octets=$((pkt_size_fcs * num_pkts)) > + local counters local -a > + > + counters=("$h1 eth-mac BroadcastFramesXmittedOK $num_pkts 1" > + "$h1 eth-mac FramesTransmittedOK $num_pkts 1" > + "$h1 eth-mac OctetsTransmittedOK $octets $pkt_size_fcs" > + "$h1 eth-mac MulticastFramesXmittedOK 0 1" > + "$h2 eth-mac BroadcastFramesReceivedOK $num_pkts 1" > + "$h2 eth-mac FramesReceivedOK $num_pkts 1" > + "$h2 eth-mac OctetsReceivedOK $octets $pkt_size_fcs" > + "$h2 eth-mac MulticastFramesReceivedOK 0 1") > + traffic_test "$h1" "$h2" "$num_pkts" "$bcast_pkt_format" \ > + "eth-mac bcast tx on $h1" \ > + "${#counters[@]}" "${counters[@]}" > + > + counters=("$h1 eth-mac BroadcastFramesXmittedOK 0 1" > + "$h1 eth-mac FramesTransmittedOK $num_pkts 1" > + "$h1 eth-mac OctetsTransmittedOK $octets $pkt_size_fcs" > + "$h1 eth-mac MulticastFramesXmittedOK $num_pkts 1" > + "$h2 eth-mac BroadcastFramesReceivedOK 0 1" > + "$h2 eth-mac FramesReceivedOK $num_pkts 1" > + "$h2 eth-mac OctetsReceivedOK $octets $pkt_size_fcs" > + "$h2 eth-mac MulticastFramesReceivedOK $num_pkts 1") > + traffic_test "$h1" "$h2" "$num_pkts" "$mcast_pkt_format" \ > + "eth-mac mcast tx on $h1" \ > + "${#counters[@]}" "${counters[@]}" > + > + counters=("$h1 eth-mac BroadcastFramesXmittedOK 0 1" > + "$h1 eth-mac FramesTransmittedOK $num_pkts 1" > + "$h1 eth-mac OctetsTransmittedOK $octets $pkt_size_fcs" > + "$h1 eth-mac MulticastFramesXmittedOK 0 1" > + "$h2 eth-mac BroadcastFramesReceivedOK 0 1" > + "$h2 eth-mac FramesReceivedOK $num_pkts 1" > + "$h2 eth-mac OctetsReceivedOK $octets $pkt_size_fcs" > + "$h2 eth-mac MulticastFramesReceivedOK 0 1") > + traffic_test "$h1" "$h2" "$num_pkts" "$ucast_pkt_format" \ > + "eth-mac ucast tx on $h1" \ > + "${#counters[@]}" "${counters[@]}" > +} > + > +test_pause_stats() > +{ > + local pkt_format="-a own -b 01:80:c2:00:00:01 88:08:00:01:00:01" > + local xfail_message="Not all MACs detect injected pause frames" > + local num_pkts=2000 > + local counters i counters should be declared local -a > + > + # Check that there is pause frame support > + for ((i = 1; i <= NUM_NETIFS; ++i)); do > + if ! ethtool -I --json -a "${NETIFS[p$i]}" > /dev/null 2>&1; > then > + log_test_skip "No support for pause frames, skip tests" > + exit > + fi > + done > + > + counters=("$h1 pause tx_pause_frames $num_pkts 1 $xfail_message" > + "$h2 pause rx_pause_frames $num_pkts 1") > + traffic_test "$h1" "$h2" "$num_pkts" "$pkt_format" \ > + "pause tx on $h1" \ > + "${#counters[@]}" "${counters[@]}" > + > + counters=("$h2 pause tx_pause_frames $num_pkts 1 $xfail_message" > + "$h1 pause rx_pause_frames $num_pkts 1") > + traffic_test "$h2" "$h1" "$num_pkts" "$pkt_format" \ > + "pause tx on $h2" \ > + "${#counters[@]}" "${counters[@]}" > +} > + > +setup_prepare() > +{ > + h1=${NETIFS[p1]} > + h2=${NETIFS[p2]} > + > + h2_mac=$(mac_get "$h2") > + > + for iface in $h1 $h2; do These should be quoted. iface should be local. > + ip link set dev "$iface" up Plesae use defer: ip link set dev "$iface" up defer ip link set dev "$iface" down Then drop the hand-rolled cleanup() altogether. Retain the "trap cleanup EXIT" -- it will pick up forwarding/lib.sh's cleanup(), which calls pre_cleanup and defer_scopes_cleanup(). > + done > +} > + > +cleanup() > +{ > + pre_cleanup > + > + for iface in $h2 $h1; do > + ip link set dev "$iface" down > + done > +} > + > +check_ethtool_counter_group_support > +trap cleanup EXIT > + > +setup_prepare > +setup_wait > + > +tests_run > + > +exit "$EXIT_STATUS"
