On Fri, Feb 27, 2026 at 04:45:47PM +0100, Petr Machata wrote:
>
> 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.
Ok.
>
> 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.
Why I had this overly complicated retrieval of the array is because I
started out with multiple arrays and then restructured but forgot about
this. Will change to just retrieve the "rest".
> > + 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.
Sure.
>
> > +
> > + 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.
You want me to move the RET=0 from some lines below to here, right?
>
> > + 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"
Good point.
>
> 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.)
Ok.
>
> > +
> > + 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
Ok.
>
> > +
> > + 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
Ok.
>
> > +
> > + # 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.
Ok.
> > + 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().
Sure! Thanks.
