On Fri, Feb 27, 2026 at 04:45:47PM +0100, Petr Machata wrote:

Ioana Ciornei [off-list ref] writes:

quoted

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 <ioana.ciornei@nxp.com>
---
 .../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".

quoted

+	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.

quoted

+
+	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?

quoted

+		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.

quoted

+			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.

quoted

+
+	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.

quoted

+
+	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.

quoted

+
+	# 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.

quoted

+		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.