[PATCH net-next v3 0/9] selftests: drivers: bash support for remote traffic generators

[PATCH net-next v3 0/9] selftests: drivers: bash support for remote traffic generators

[Ioana Ciornei]

This patch set aims to add the necessary support so that bash written
selftests are also able to easily run with a remote traffic generator
system, either be it in another netns or one accessible through ssh.

This patch set is a result of the discussion from v1:
https://lore.kernel.org/all/20260303084330.340b6459@kernel.org/
Even though the python infrastructure is already established, some
things are easier in bash and it would be a shame to leave behind the
bash tests that we already have. Version 1 of this patch set also had
patches exporting standard statistics in the dpaa2 drivers. Those
patches will be submitted separately so that they do not depend on this
set.

This support is based on the requirements described in the
tools/testing/selftests/drivers/net/README.rst file.

Mainly, the drivers/net selftests should be able to run on a interface
specified through the NETIF env variable. On top of that, variables such
as REMOTE_TYPE and REMOTE_ARGS define how the remote traffic generator
can be accessed. Patch 2/9 parses these env variables and constructs the
NETIFS array that bash tests are accustomed to. This is with the
intention of enabling already written tests to incur minimal changes.

The second patch also defines the TARGETS array which will hold the
necessary information about the target on which a specific interface
is located.

For example, a net.config which looks like below:
	NETIF=eth0
	LOCAL_V4=192.168.1.1
	REMOTE_V4=192.168.1.2
	REMOTE_TYPE=ssh
	REMOTE_ARGS=root@192.168.1.2

will generate the NETIFS and TARGETS arrays with the following data.

	NETIFS[p1]="eth0"
	NETIFS[p2]="eth2"

	TARGETS[eth0]="local:"
	TARGETS[eth2]="ssh:root@192.168.1.2"

The above will be true if on the remote target, the interface which has
the 192.168.1.2 address is named eth2.

The values held in the TARGETS array will be used by the new 'run_on'
helper added in patch 3/9 to know how to run a specific command, on the
local system, on another netns or by using ssh. Patch 4/9 updates some
helpers to use run_on so that, for example, lib.sh is able to ensure
stable MAC addresses even with the remote interface located in another
netns.

The next 4 patches, 4/9-8/9 update the ethtool_rmon.sh script so that it
can work with the kselftest infrastructure and the new
NETIF/REMOTE_TYPE etc way of working. Beside updating each ip link or
ethtool command to use the run_on helper, the patches also remove any
testing done on the remote interface.

The last patch adds a new test which checks the standard counters -
eth-ctrl, eth-mac and pause - and uses the new infrastructure put in
place by the first patches.

With this patch set, both tests can be run using a net.config file and
run_kselftest.sh as shown below.

 $ make -C tools/testing/selftests/ TARGETS="drivers/net drivers/net/hw" \
 install INSTALL_PATH=/tmp/ksft-net-drv
 $ cd /tmp/ksft-net-drv/
 $ cat > ./drivers/net/net.config <<EOF
 NETIF=endpmac17
 LOCAL_V4=17.0.0.1
 REMOTE_V4=17.0.0.2
 REMOTE_TYPE=ssh
 REMOTE_ARGS=root@192.168.5.200
 EOF

 $ ./run_kselftest.sh -t drivers/net/hw:ethtool_rmon.sh
 TAP version 13
 1..1
 # timeout set to 0
 # selftests: drivers/net/hw: ethtool_rmon.sh
 # TEST: rx-pkts64to64 on endpmac17                                    [ OK ]
 # TEST: rx-pkts65to127 on endpmac17                                   [ OK ]
 # TEST: rx-pkts128to255 on endpmac17                                  [ OK ]
 # TEST: rx-pkts256to511 on endpmac17                                  [ OK ]
 # TEST: rx-pkts512to1023 on endpmac17                                 [ OK ]
 # TEST: rx-pkts1024to1518 on endpmac17                                [ OK ]
 # TEST: rx-pkts1519to10240 on endpmac17                               [ OK ]
 # TEST: tx-pkts64to64 on endpmac17                                    [ OK ]
 # TEST: tx-pkts65to127 on endpmac17                                   [ OK ]
 # TEST: tx-pkts128to255 on endpmac17                                  [ OK ]
 # TEST: tx-pkts256to511 on endpmac17                                  [ OK ]
 # TEST: tx-pkts512to1023 on endpmac17                                 [ OK ]
 # TEST: tx-pkts1024to1518 on endpmac17                                [ OK ]
 # TEST: tx-pkts1519to10240 on endpmac17                               [ OK ]
 ok 1 selftests: drivers/net/hw: ethtool_rmon.sh

 $ ./run_kselftest.sh -t drivers/net/hw:ethtool_std_stats.sh
 TAP version 13
 1..1
 # timeout set to 0
 # selftests: drivers/net/hw: ethtool_std_stats.sh
 # TEST: eth-ctrl-MACControlFramesTransmitted on endpmac17             [ OK ]
 # TEST: eth-ctrl-MACControlFramesReceived on endpmac17                [ OK ]
 # TEST: eth-mac-FrameCheckSequenceErrors on endpmac17                 [ OK ]
 # TEST: eth-mac-AlignmentErrors on endpmac17                          [ OK ]
 # TEST: eth-mac-FramesLostDueToIntMACXmitError on endpmac17           [ OK ]
 # TEST: eth-mac-CarrierSenseErrors on endpmac17 (not supported)       [SKIP]
 # TEST: eth-mac-FramesLostDueToIntMACRcvError on endpmac17            [ OK ]
 # TEST: eth-mac-InRangeLengthErrors on endpmac17 (not supported)      [SKIP]
 # TEST: eth-mac-OutOfRangeLengthField on endpmac17 (not supported)    [SKIP]
 # TEST: eth-mac-FrameTooLongErrors on endpmac17 (not supported)       [SKIP]
 # TEST: eth-mac-FramesAbortedDueToXSColls on endpmac17 (not supported)  [SKIP]
 # TEST: eth-mac-SingleCollisionFrames on endpmac17 (not supported)    [SKIP]
 # TEST: eth-mac-MultipleCollisionFrames on endpmac17 (not supported)  [SKIP]
 # TEST: eth-mac-FramesWithDeferredXmissions on endpmac17 (not supported)  [SKIP]
 # TEST: eth-mac-LateCollisions on endpmac17 (not supported)           [SKIP]
 # TEST: eth-mac-FramesWithExcessiveDeferral on endpmac17 (not supported)  [SKIP]
 # TEST: eth-mac-BroadcastFramesXmittedOK on endpmac17                 [ OK ]
 # TEST: eth-mac-OctetsTransmittedOK on endpmac17                      [ OK ]
 # TEST: eth-mac-BroadcastFramesReceivedOK on endpmac17                [ OK ]
 # TEST: eth-mac-OctetsReceivedOK on endpmac17                         [ OK ]
 # TEST: eth-mac-FramesTransmittedOK on endpmac17                      [ OK ]
 # TEST: eth-mac-MulticastFramesXmittedOK on endpmac17                 [ OK ]
 # TEST: eth-mac-FramesReceivedOK on endpmac17                         [ OK ]
 # TEST: eth-mac-MulticastFramesReceivedOK on endpmac17                [ OK ]
 # TEST: pause-tx_pause_frames on endpmac17                            [ OK ]
 # TEST: pause-rx_pause_frames on endpmac17                            [ OK ]
 ok 1 selftests: drivers/net/hw: ethtool_std_stats.sh # SKIP

Changes in v3:
- Change the TARGET variable into CUR_TARGET
- Always fallback on running a command locally when either TARGETS is
  not declared or there is no entry for a specific interface
- Swap patches 2 & 3 between them so that the run_cmd used in now patch
  #2 is defined in patch #2
- Add some more double quoting

Changes in v2:
- 1/9: Convert jq to the --arg usage form
- Patches 2/9-8/9 are new in this version
- 9/9: Use the new run_on helper
- 9/9: No longer checking that each counter has a 1% tolerance against the
  target. The only upper limit is UINT32_MAX.
- 9/9: Check that both the error counters and the collision related ones are
  zero since this is the most probable scenario (we don't expect errors
  with the traffic that we are sending).
- 9/9: Removed any checks performed on the remot interface counters since
  that is being used only as a traffic generator.

Ioana Ciornei (9):
  selftests: forwarding: extend ethtool_std_stats_get with pause
    statistics
  selftests: net: add helpers for running a command on other targets
  selftests: net: extend lib.sh to parse drivers/net/net.config
  selftests: net: update some helpers to use run_on
  selftests: drivers: hw: cleanup shellcheck warnings in the rmon test
  selftests: drivers: hw: test rmon counters only on first interface
  selftests: drivers: hw: replace counter upper limit with UINT32_MAX in
    rmon test
  selftests: drivers: hw: update ethtool_rmon to work with a single
    local interface
  selftests: drivers: hw: add test for the ethtool standard counters

 .../testing/selftests/drivers/net/README.rst  |   3 +
 .../testing/selftests/drivers/net/hw/Makefile |   1 +
 .../selftests/drivers/net/hw/ethtool_rmon.sh  |  55 +++---
 .../drivers/net/hw/ethtool_std_stats.sh       | 187 ++++++++++++++++++
 tools/testing/selftests/net/forwarding/lib.sh | 151 ++++++++++++--
 tools/testing/selftests/net/lib.sh            |  41 +++-
 6 files changed, 394 insertions(+), 44 deletions(-)
 create mode 100755 tools/testing/selftests/drivers/net/hw/ethtool_std_stats.sh

-- 
2.25.1

[PATCH net-next v3 1/9] selftests: forwarding: extend ethtool_std_stats_get with pause statistics

[Ioana Ciornei]

Even though pause frame statistics are not exported through the same
ethtool command, there is no point in adding another helper just for
them. Extent the ethtool_std_stats_get() function so that we are able to
interrogate using the same helper all the standard statistics.

And since we are touching the else case, convert that as well to the jq
--arg form in order to be easier to read.

Signed-off-by: Ioana Ciornei <ioana.ciornei@nxp.com>
---
Changes in v3:
- none
Changes in v2:
- convert jq to the --arg usage form

 tools/testing/selftests/net/forwarding/lib.sh | 8 ++++++--
 1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/tools/testing/selftests/net/forwarding/lib.sh b/tools/testing/selftests/net/forwarding/lib.sh
index a9034f0bb58b..cf40cb766c68 100644
--- a/tools/testing/selftests/net/forwarding/lib.sh
+++ b/tools/testing/selftests/net/forwarding/lib.sh
@@ -831,8 +831,12 @@ ethtool_std_stats_get()
 	local name=$1; shift
 	local src=$1; shift
 
-	ethtool --json -S $dev --groups $grp -- --src $src | \
-		jq '.[]."'"$grp"'"."'$name'"'
+	if [[ "$grp" == "pause" ]]; then
+		ethtool -I --json -a "$dev" | jq --arg name "$name" '.[].statistics[$name]'
+	else
+		ethtool --json -S "$dev" --groups "$grp" -- --src "$src" | \
+			jq --arg grp "$grp" --arg name "$name" '.[][$grp][$name]'
+	fi
 }
 
 qdisc_stats_get()
-- 
2.25.1

[PATCH net-next v3 2/9] selftests: net: add helpers for running a command on other targets

[Ioana Ciornei]

Add a couple of helpers which can be used by tests which need to run a
specific bash command on a different target than the local system, be it
either another netns or a remote system accessible through ssh.

The run_cmd() function decides where to execute the command passed
through $@ based on the env variable TARGET value while run_on() will
receive the target through its first argument.

Signed-off-by: Ioana Ciornei <ioana.ciornei@nxp.com>
---
Changes in v3:
- s/TARGET/CUR_TARGET
- always fallback on running a command locally when either TARGETS is
  not declared or there is no entry for a specific interface
Changes in v2:
- patch is new

 tools/testing/selftests/net/lib.sh | 39 ++++++++++++++++++++++++++++++
 1 file changed, 39 insertions(+)

diff --git a/tools/testing/selftests/net/lib.sh b/tools/testing/selftests/net/lib.sh
index b40694573f4c..f7c54d05758e 100644
--- a/tools/testing/selftests/net/lib.sh
+++ b/tools/testing/selftests/net/lib.sh
@@ -28,6 +28,10 @@ EXIT_STATUS=0
 # Per-test return value. Clear at the beginning of each test.
 RET=0
 
+# If a specific command needs to be executed on another target than local, set
+# this appropriately before calling run_cmd
+CUR_TARGET="local:"
+
 ##############################################################################
 # Helpers
 
@@ -670,3 +674,38 @@ cmd_jq()
 	# return success only in case of non-empty output
 	[ ! -z "$output" ]
 }
+
+run_cmd()
+{
+	IFS=':' read -r type args <<< "$CUR_TARGET"
+
+	case "$type" in
+		netns)
+			# Execute command in network namespace
+			# args contains the namespace name
+			ip netns exec "$args" "$@"
+			;;
+		ssh)
+			# Execute command via SSH args contains user@host
+			ssh -n "$args" "$@"
+			;;
+		local|*)
+			# Execute command locally. This is also the fallback
+			# case in case the CUR_TARGET is not set.
+			"$@"
+			;;
+	esac
+}
+
+run_on()
+{
+	local iface=$1; shift
+
+	if declare -p TARGETS &>/dev/null; then
+		CUR_TARGET="${TARGETS[$iface]}"
+	else
+		CUR_TARGET="local:"
+	fi
+
+	run_cmd "$@"
+}
-- 
2.25.1

[PATCH net-next v3 3/9] selftests: net: extend lib.sh to parse drivers/net/net.config

[Ioana Ciornei]

Extend lib.sh so that it's able to parse driver/net/net.config and
environment variables such as NETIF, REMOTE_TYPE, LOCAL_V4 etc described
in drivers/net/README.rst.

In order to make the transition towards running with a single local
interface smoother for the bash networking driver tests, beside sourcing
the net.config file also translate the new env variables into the old
style based on the NETIFS array. Since the NETIFS array only holds the
network interface names, also add a new array - TARGETS - which keeps
track of the target on which a specific interfaces resides - local,
netns or accesible through an ssh command.

For example, a net.config which looks like below:

	NETIF=eth0
	LOCAL_V4=192.168.1.1
	REMOTE_V4=192.168.1.2
	REMOTE_TYPE=ssh
	REMOTE_ARGS=root@192.168.1.2

will generate the NETIFS and TARGETS arrays with the following data.

	NETIFS[p1]="eth0"
	NETIFS[p2]="eth2"

	TARGETS[eth0]="local:"
	TARGETS[eth2]="ssh:root@192.168.1.2"

The above will be true if on the remote target, the interface which has
the 192.168.1.2 address is named eth2.

Since the TARGETS array is indexed by the network interface name,
document a new restriction README.rst which states that the remote
interface cannot have the same name as the local one.

Also keep the old way of populating the NETIFS variable based on the
command line arguments. This will be invoked in case NETIF is not
defined.

Signed-off-by: Ioana Ciornei <ioana.ciornei@nxp.com>
---
Changes in v3:
- s/TARGET/CUR_TARGET
- this used to be patch #2/9 in v2. Swapped the two patches so that the
  run_cmd used in this patch is defined earlier, not later.
Changes in v2:
- patch is new

 .../testing/selftests/drivers/net/README.rst  |   3 +
 tools/testing/selftests/net/forwarding/lib.sh | 130 ++++++++++++++++--
 2 files changed, 124 insertions(+), 9 deletions(-)

diff --git a/tools/testing/selftests/drivers/net/README.rst b/tools/testing/selftests/drivers/net/README.rst
index c94992acf10b..8d8d9d62e763 100644
--- a/tools/testing/selftests/drivers/net/README.rst
+++ b/tools/testing/selftests/drivers/net/README.rst
@@ -26,6 +26,9 @@ The netdevice against which tests will be run must exist, be running
 Refer to list of :ref:`Variables` later in this file to set up running
 the tests against a real device.
 
+Also, make sure that if you are using a remote machine for traffic injection,
+the local and remote interfaces have different names.
+
 Both modes required
 ~~~~~~~~~~~~~~~~~~~
 
diff --git a/tools/testing/selftests/net/forwarding/lib.sh b/tools/testing/selftests/net/forwarding/lib.sh
index cf40cb766c68..fb5aa56343e1 100644
--- a/tools/testing/selftests/net/forwarding/lib.sh
+++ b/tools/testing/selftests/net/forwarding/lib.sh
@@ -3,6 +3,7 @@
 
 ##############################################################################
 # Topology description. p1 looped back to p2, p3 to p4 and so on.
+#shellcheck disable=SC2034 # SC doesn't see our uses of global variables
 
 declare -A NETIFS=(
     [p1]=veth0
@@ -17,6 +18,26 @@ declare -A NETIFS=(
     [p10]=veth9
 )
 
+# Array indexed by the network interface name keeping track of the target on
+# which the interface resides. Values will be strings of the following format -
+# <type>:<args>.
+# TARGETS[eth0]="local:" - meaning that the eth0 interface is accessible locally
+# TARGETS[eth1]="netns:foo" - eth1 is in the foo netns
+# TARGETS[eth2]="ssh:root@10.0.0.2" - eth2 is accessible through running the
+# 'ssh root@10.0.0.2' command.
+declare -A TARGETS=(
+    [veth0]="local:"
+    [veth1]="local:"
+    [veth2]="local:"
+    [veth3]="local:"
+    [veth4]="local:"
+    [veth5]="local:"
+    [veth6]="local:"
+    [veth7]="local:"
+    [veth8]="local:"
+    [veth9]="local:"
+)
+
 # Port that does not have a cable connected.
 : "${NETIF_NO_CABLE:=eth8}"
 
@@ -340,17 +361,108 @@ fi
 ##############################################################################
 # Command line options handling
 
-count=0
+check_env() {
+	local vars_needed=("LOCAL_V4,LOCAL_V6"
+			   "REMOTE_V4,REMOTE_V6"
+			   "REMOTE_TYPE"
+			   "REMOTE_ARGS")
+	local missing=()
+	local choice
+
+	# If a choice has multiple comma separated options, at least one must
+	# exist
+	for choice in "${vars_needed[@]}"; do
+		IFS=',' read -ra entries <<< "$choice"
+
+		local found=0
+		for entry in "${entries[@]}"; do
+			if [[ -n "${!entry}" ]]; then
+				found=1
+				break
+			fi
+		done
 
-while [[ $# -gt 0 ]]; do
-	if [[ "$count" -eq "0" ]]; then
-		unset NETIFS
-		declare -A NETIFS
+		if [[ $found -eq 0 ]]; then
+			missing+=("$choice")
+		fi
+	done
+
+	# Make sure v4 / v6 configs are symmetric
+	if [[ (-n "${LOCAL_V6}" && -z "${REMOTE_V6}") || \
+	      (-z "${LOCAL_V6}" && -n "${REMOTE_V6}") ]]; then
+		missing+=("LOCAL_V6,REMOTE_V6")
 	fi
-	count=$((count + 1))
-	NETIFS[p$count]="$1"
-	shift
-done
+
+	if [[ (-n "${LOCAL_V4}" && -z "${REMOTE_V4}") || \
+	      (-z "${LOCAL_V4}" && -n "${REMOTE_V4}") ]]; then
+		missing+=("LOCAL_V4,REMOTE_V4")
+	fi
+
+	if [[ ${#missing[@]} -gt 0 ]]; then
+		echo "SKIP: Invalid environment, missing configuration: ${missing[*]}"
+		echo "Please see tools/testing/selftests/drivers/net/README.rst"
+		exit "$ksft_skip"
+	fi
+}
+
+get_ifname_by_ip()
+{
+	local ip_addr=$1; shift
+
+	run_cmd ip -j addr show to "$ip_addr" | jq -r '.[].ifname'
+}
+
+# If there is a configuration file, source it
+if [[ -f $net_forwarding_dir/../../drivers/net/net.config ]]; then
+	source "$net_forwarding_dir/../../drivers/net/net.config"
+fi
+
+# In case NETIF is specified, then the test expects to pass the arguments
+# through the variables specified in drivers/net/README.rst file. If not,
+# fallback on parsing the script arguments for interface names.
+if [[ -v NETIF ]]; then
+	if (( NUM_NETIFS > 2)); then
+		echo "SKIP: NETIF defined and NUM_NETIFS is bigger than 2"
+		exit "$ksft_skip"
+	fi
+
+	check_env
+
+	# Populate the NETIF and TARGETS arrays automatically based on the
+	# environment variables
+	unset NETIFS
+	declare -A NETIFS
+
+	NETIFS[p1]="$NETIF"
+	TARGETS[$NETIF]="local:"
+
+	# Locate the name of the remote interface
+	if [[ -v REMOTE_V4 ]]; then
+		remote_netif=$(CUR_TARGET="$REMOTE_TYPE:$REMOTE_ARGS" get_ifname_by_ip "$REMOTE_V4")
+	else
+		remote_netif=$(CUR_TARGET="$REMOTE_TYPE:$REMOTE_ARGS" get_ifname_by_ip "$REMOTE_V6")
+	fi
+	if [[ ! -n "$remote_netif" ]]; then
+		echo "SKIP: cannot find remote interface"
+		exit "$ksft_skip"
+	fi
+
+	NETIFS[p2]="$remote_netif"
+	TARGETS[$remote_netif]="$REMOTE_TYPE:$REMOTE_ARGS"
+else
+	count=0
+
+	while [[ $# -gt 0 ]]; do
+		if [[ "$count" -eq "0" ]]; then
+			unset NETIFS
+			declare -A NETIFS
+		fi
+		count=$((count + 1))
+		NETIFS[p$count]="$1"
+		TARGETS[$1]="local:"
+		shift
+	done
+fi
 
 ##############################################################################
 # Network interfaces configuration
-- 
2.25.1

[PATCH net-next v3 4/9] selftests: net: update some helpers to use run_on

[Ioana Ciornei]

Update some helpers so that they are capable to run commands on
different targets than the local one. This patch makes the necesasy
modification for those helpers / sections of code which are needed for
the ethtool_rmon.sh test that will be converted in the next patches.

For example, mac_addr_prepare() and mac_addr_restore() used when
STABLE_MAC_ADDRS=yes need to ensure stable MAC addresses on interfaces
located even in other namespaces. In order to do that, append the 'ip
link' commands with a 'run_on $dev' tag.

The same run_on is necessary also when verifying if all the interfaces
listed in NETIFS are indeed available.

Signed-off-by: Ioana Ciornei <ioana.ciornei@nxp.com>
---
Changes in v3:
- added some more double quotes
Changes in v2:
- patch is new

 tools/testing/selftests/net/forwarding/lib.sh | 17 ++++++++++-------
 tools/testing/selftests/net/lib.sh            |  2 +-
 2 files changed, 11 insertions(+), 8 deletions(-)

diff --git a/tools/testing/selftests/net/forwarding/lib.sh b/tools/testing/selftests/net/forwarding/lib.sh
index fb5aa56343e1..f76de37d0caf 100644
--- a/tools/testing/selftests/net/forwarding/lib.sh
+++ b/tools/testing/selftests/net/forwarding/lib.sh
@@ -530,10 +530,10 @@ mac_addr_prepare()
 		dev=${NETIFS[p$i]}
 		new_addr=$(printf "00:01:02:03:04:%02x" $i)
 
-		MAC_ADDR_ORIG["$dev"]=$(ip -j link show dev $dev | jq -e '.[].address')
+		MAC_ADDR_ORIG["$dev"]=$(run_on "$dev" ip -j link show dev "$dev" | jq -e '.[].address')
 		# Strip quotes
 		MAC_ADDR_ORIG["$dev"]=${MAC_ADDR_ORIG["$dev"]//\"/}
-		ip link set dev $dev address $new_addr
+		run_on "$dev" ip link set dev "$dev" address $new_addr
 	done
 }
 
@@ -543,7 +543,7 @@ mac_addr_restore()
 
 	for ((i = 1; i <= NUM_NETIFS; ++i)); do
 		dev=${NETIFS[p$i]}
-		ip link set dev $dev address ${MAC_ADDR_ORIG["$dev"]}
+		run_on "$dev" ip link set dev "$dev" address ${MAC_ADDR_ORIG["$dev"]}
 	done
 }
 
@@ -556,7 +556,9 @@ if [[ "$STABLE_MAC_ADDRS" = "yes" ]]; then
 fi
 
 for ((i = 1; i <= NUM_NETIFS; ++i)); do
-	ip link show dev ${NETIFS[p$i]} &> /dev/null
+	int="${NETIFS[p$i]}"
+
+	run_on "$int" ip link show dev "$int" &> /dev/null
 	if [[ $? -ne 0 ]]; then
 		echo "SKIP: could not find all required interfaces"
 		exit $ksft_skip
@@ -639,7 +641,7 @@ setup_wait_dev_with_timeout()
 	local i
 
 	for ((i = 1; i <= $max_iterations; ++i)); do
-		ip link show dev $dev up \
+		run_on "$dev" ip link show dev "$dev" up \
 			| grep 'state UP' &> /dev/null
 		if [[ $? -ne 0 ]]; then
 			sleep 1
@@ -944,9 +946,10 @@ ethtool_std_stats_get()
 	local src=$1; shift
 
 	if [[ "$grp" == "pause" ]]; then
-		ethtool -I --json -a "$dev" | jq --arg name "$name" '.[].statistics[$name]'
+		run_on "$dev" ethtool -I --json -a "$dev" | \
+			jq --arg name "$name" '.[].statistics[$name]'
 	else
-		ethtool --json -S "$dev" --groups "$grp" -- --src "$src" | \
+		run_on "$dev" ethtool --json -S "$dev" --groups "$grp" -- --src "$src" | \
 			jq --arg grp "$grp" --arg name "$name" '.[][$grp][$name]'
 	fi
 }
diff --git a/tools/testing/selftests/net/lib.sh b/tools/testing/selftests/net/lib.sh
index f7c54d05758e..14f262a593c9 100644
--- a/tools/testing/selftests/net/lib.sh
+++ b/tools/testing/selftests/net/lib.sh
@@ -518,7 +518,7 @@ mac_get()
 {
 	local if_name=$1
 
-	ip -j link show dev $if_name | jq -r '.[]["address"]'
+	run_on "$if_name" ip -j link show dev "$if_name" | jq -r '.[]["address"]'
 }
 
 kill_process()
-- 
2.25.1

[PATCH net-next v3 5/9] selftests: drivers: hw: cleanup shellcheck warnings in the rmon test

[Ioana Ciornei]

If run on the ethtool_rmon.sh script, shellcheck generates a bunch of
false positive errors. Suppress those checks that generate them.

Also cleanup the remaining warnings by using double quoting around the
used variables.

Signed-off-by: Ioana Ciornei <ioana.ciornei@nxp.com>
---
Changes in v3:
- none
Changes in v2:
- patch is new

 .../selftests/drivers/net/hw/ethtool_rmon.sh  | 53 ++++++++++---------
 1 file changed, 28 insertions(+), 25 deletions(-)

diff --git a/tools/testing/selftests/drivers/net/hw/ethtool_rmon.sh b/tools/testing/selftests/drivers/net/hw/ethtool_rmon.sh
index 8f60c1685ad4..417627e7a592 100755
--- a/tools/testing/selftests/drivers/net/hw/ethtool_rmon.sh
+++ b/tools/testing/selftests/drivers/net/hw/ethtool_rmon.sh
@@ -1,5 +1,7 @@
 #!/bin/bash
 # SPDX-License-Identifier: GPL-2.0
+#shellcheck disable=SC2034 # SC does not see the global variables
+#shellcheck disable=SC2317,SC2329 # unused functions
 
 ALL_TESTS="
 	rmon_rx_histogram
@@ -19,11 +21,12 @@ ensure_mtu()
 {
 	local iface=$1; shift
 	local len=$1; shift
-	local current=$(ip -j link show dev $iface | jq -r '.[0].mtu')
 	local required=$((len - ETH_HLEN - ETH_FCS_LEN))
+	local current
 
-	if [ $current -lt $required ]; then
-		ip link set dev $iface mtu $required || return 1
+	current=$(ip -j link show dev "$iface" | jq -r '.[0].mtu')
+	if [ "$current" -lt "$required" ]; then
+		ip link set dev "$iface" mtu "$required" || return 1
 	fi
 }
 
@@ -46,23 +49,23 @@ bucket_test()
 	len=$((len - ETH_FCS_LEN))
 	len=$((len > 0 ? len : 0))
 
-	before=$(ethtool --json -S $iface --groups rmon | \
+	before=$(ethtool --json -S "$iface" --groups rmon | \
 		jq -r ".[0].rmon[\"${set}-pktsNtoM\"][$bucket].val")
 
 	# Send 10k one way and 20k in the other, to detect counters
 	# mapped to the wrong direction
-	$MZ $neigh -q -c $num_rx -p $len -a own -b bcast -d 10us
-	$MZ $iface -q -c $num_tx -p $len -a own -b bcast -d 10us
+	"$MZ" "$neigh" -q -c "$num_rx" -p "$len" -a own -b bcast -d 10us
+	"$MZ" "$iface" -q -c "$num_tx" -p "$len" -a own -b bcast -d 10us
 
-	after=$(ethtool --json -S $iface --groups rmon | \
+	after=$(ethtool --json -S "$iface" --groups rmon | \
 		jq -r ".[0].rmon[\"${set}-pktsNtoM\"][$bucket].val")
 
 	delta=$((after - before))
 
-	expected=$([ $set = rx ] && echo $num_rx || echo $num_tx)
+	expected=$([ "$set" = rx ] && echo "$num_rx" || echo "$num_tx")
 
 	# Allow some extra tolerance for other packets sent by the stack
-	[ $delta -ge $expected ] && [ $delta -le $((expected + 100)) ]
+	[ "$delta" -ge "$expected" ] && [ "$delta" -le $((expected + 100)) ]
 }
 
 rmon_histogram()
@@ -78,23 +81,23 @@ rmon_histogram()
 	while read -r -a bucket; do
 		step="$set-pkts${bucket[0]}to${bucket[1]} on $iface"
 
-		for if in $iface $neigh; do
-			if ! ensure_mtu $if ${bucket[0]}; then
+		for if in "$iface" "$neigh"; do
+			if ! ensure_mtu "$if" "${bucket[0]}"; then
 				log_test_xfail "$if does not support the required MTU for $step"
 				return
 			fi
 		done
 
-		if ! bucket_test $iface $neigh $set $nbuckets ${bucket[0]}; then
+		if ! bucket_test "$iface" "$neigh" "$set" "$nbuckets" "${bucket[0]}"; then
 			check_err 1 "$step failed"
 			return 1
 		fi
 		log_test "$step"
 		nbuckets=$((nbuckets + 1))
-	done < <(ethtool --json -S $iface --groups rmon | \
+	done < <(ethtool --json -S "$iface" --groups rmon | \
 		jq -r ".[0].rmon[\"${set}-pktsNtoM\"][]|[.low, .high]|@tsv" 2>/dev/null)
 
-	if [ $nbuckets -eq 0 ]; then
+	if [ "$nbuckets" -eq 0 ]; then
 		log_test_xfail "$iface does not support $set histogram counters"
 		return
 	fi
@@ -102,14 +105,14 @@ rmon_histogram()
 
 rmon_rx_histogram()
 {
-	rmon_histogram $h1 $h2 rx
-	rmon_histogram $h2 $h1 rx
+	rmon_histogram "$h1" "$h2" rx
+	rmon_histogram "$h2" "$h1" rx
 }
 
 rmon_tx_histogram()
 {
-	rmon_histogram $h1 $h2 tx
-	rmon_histogram $h2 $h1 tx
+	rmon_histogram "$h1" "$h2" tx
+	rmon_histogram "$h2" "$h1" tx
 }
 
 setup_prepare()
@@ -117,9 +120,9 @@ setup_prepare()
 	h1=${NETIFS[p1]}
 	h2=${NETIFS[p2]}
 
-	for iface in $h1 $h2; do
-		netif_mtu[$iface]=$(ip -j link show dev $iface | jq -r '.[0].mtu')
-		ip link set dev $iface up
+	for iface in "$h1" "$h2"; do
+		netif_mtu["$iface"]=$(ip -j link show dev "$iface" | jq -r '.[0].mtu')
+		ip link set dev "$iface" up
 	done
 }
 
@@ -127,9 +130,9 @@ cleanup()
 {
 	pre_cleanup
 
-	for iface in $h2 $h1; do
-		ip link set dev $iface \
-			mtu ${netif_mtu[$iface]} \
+	for iface in "$h2" "$h1"; do
+		ip link set dev "$iface" \
+			mtu "${netif_mtu[$iface]}" \
 			down
 	done
 }
@@ -142,4 +145,4 @@ setup_wait
 
 tests_run
 
-exit $EXIT_STATUS
+exit "$EXIT_STATUS"
-- 
2.25.1

[PATCH net-next v3 6/9] selftests: drivers: hw: test rmon counters only on first interface

[Ioana Ciornei]

The selftests in drivers/net are slowly transitioning to being able to
be used on systems with a single network interface. The first step for the
ethtool_rmon.sh test is to only validate that the rmon counters are
properly exported on the first interface supplied as an argument.

Remove the rmon_histogram calls which intend to test also the rmon
counters on the 2nd interface. This also removes the need for the remote
system, which should be used only to inject traffic, to also support
rmon counters.

Signed-off-by: Ioana Ciornei <ioana.ciornei@nxp.com>
---
Changes in v3:
- none
Changes in v2:
- patch is new

 tools/testing/selftests/drivers/net/hw/ethtool_rmon.sh | 2 --
 1 file changed, 2 deletions(-)

diff --git a/tools/testing/selftests/drivers/net/hw/ethtool_rmon.sh b/tools/testing/selftests/drivers/net/hw/ethtool_rmon.sh
index 417627e7a592..636429018b6b 100755
--- a/tools/testing/selftests/drivers/net/hw/ethtool_rmon.sh
+++ b/tools/testing/selftests/drivers/net/hw/ethtool_rmon.sh
@@ -106,13 +106,11 @@ rmon_histogram()
 rmon_rx_histogram()
 {
 	rmon_histogram "$h1" "$h2" rx
-	rmon_histogram "$h2" "$h1" rx
 }
 
 rmon_tx_histogram()
 {
 	rmon_histogram "$h1" "$h2" tx
-	rmon_histogram "$h2" "$h1" tx
 }
 
 setup_prepare()
-- 
2.25.1

[PATCH net-next v3 7/9] selftests: drivers: hw: replace counter upper limit with UINT32_MAX in rmon test

[Ioana Ciornei]

The ethtool_rmon.sh script checks that the number of packets sent /
received during a test matches the expected value with a 1% tolerance.

Since in the next patches this test will gain the capability to also be
run on systems with a single interface where the traffic generator is
accesible through ssh, use the UINT32_MAX as the upper limit. This is
necessary since the same interface will be used also for control traffic
(the ssh commands) as well as the mausezahn generated one.

Signed-off-by: Ioana Ciornei <ioana.ciornei@nxp.com>
---
Changes in v3:
- none
Changes in v2:
- patch is new

 tools/testing/selftests/drivers/net/hw/ethtool_rmon.sh | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/tools/testing/selftests/drivers/net/hw/ethtool_rmon.sh b/tools/testing/selftests/drivers/net/hw/ethtool_rmon.sh
index 636429018b6b..1baaaab3c3a4 100755
--- a/tools/testing/selftests/drivers/net/hw/ethtool_rmon.sh
+++ b/tools/testing/selftests/drivers/net/hw/ethtool_rmon.sh
@@ -12,6 +12,7 @@ NUM_NETIFS=2
 lib_dir=$(dirname "$0")
 source "$lib_dir"/../../../net/forwarding/lib.sh
 
+UINT32_MAX=$((2**32 - 1))
 ETH_FCS_LEN=4
 ETH_HLEN=$((6+6+2))
 
@@ -64,8 +65,7 @@ bucket_test()
 
 	expected=$([ "$set" = rx ] && echo "$num_rx" || echo "$num_tx")
 
-	# Allow some extra tolerance for other packets sent by the stack
-	[ "$delta" -ge "$expected" ] && [ "$delta" -le $((expected + 100)) ]
+	[ "$delta" -ge "$expected" ] && [ "$delta" -le "$UINT32_MAX" ]
 }
 
 rmon_histogram()
-- 
2.25.1

[PATCH net-next v3 8/9] selftests: drivers: hw: update ethtool_rmon to work with a single local interface

[Ioana Ciornei]

This patch finalizes the transition to work with a single local
interface for the ethtool_rmon.sh test. Each 'ip link' and 'ethtool'
command used by the test is annotated with the necessary run_on in
order to be executed on the necessary target system, be it local, in
another network namespace or through ssh.

Since we need NETIF up and running also for control traffic, we now
expect that the interfaces are up and running and do not touch bring
them up or down at the end of the test. This is also documented in the
drivers/net/README.rst.

The ethtool_rmon.sh script can still be used in the older fashion by
passing two interfaces as command line arguments, the only restriction
is that those interfaces need to be already up.

As part of the kselftest infrastructure, this test can be run in the
following manner:

 $ make -C tools/testing/selftests/ TARGETS="drivers/net drivers/net/hw" \
 install INSTALL_PATH=/tmp/ksft-net-drv
 $ cd /tmp/ksft-net-drv/
 $ cat > ./drivers/net/net.config <<EOF
 NETIF=endpmac17
 LOCAL_V4=17.0.0.1
 REMOTE_V4=17.0.0.2
 REMOTE_TYPE=ssh
 REMOTE_ARGS=root@192.168.5.200
 EOF

 $ ./run_kselftest.sh -t drivers/net/hw:ethtool_rmon.sh
 TAP version 13
 1..1
 # timeout set to 0
 # selftests: drivers/net/hw: ethtool_rmon.sh
 # TEST: rx-pkts64to64 on endpmac17                                    [ OK ]
 # TEST: rx-pkts65to127 on endpmac17                                   [ OK ]
 # TEST: rx-pkts128to255 on endpmac17                                  [ OK ]
 # TEST: rx-pkts256to511 on endpmac17                                  [ OK ]
 # TEST: rx-pkts512to1023 on endpmac17                                 [ OK ]
 # TEST: rx-pkts1024to1518 on endpmac17                                [ OK ]
 # TEST: rx-pkts1519to10240 on endpmac17                               [ OK ]
 # TEST: tx-pkts64to64 on endpmac17                                    [ OK ]
 # TEST: tx-pkts65to127 on endpmac17                                   [ OK ]
 # TEST: tx-pkts128to255 on endpmac17                                  [ OK ]
 # TEST: tx-pkts256to511 on endpmac17                                  [ OK ]
 # TEST: tx-pkts512to1023 on endpmac17                                 [ OK ]
 # TEST: tx-pkts1024to1518 on endpmac17                                [ OK ]
 # TEST: tx-pkts1519to10240 on endpmac17                               [ OK ]
 ok 1 selftests: drivers/net/hw: ethtool_rmon.sh

Signed-off-by: Ioana Ciornei <ioana.ciornei@nxp.com>
---
Changes in v3:
- none
Changes in v2:
- patch is new


 .../selftests/drivers/net/hw/ethtool_rmon.sh  | 22 +++++++++----------
 1 file changed, 11 insertions(+), 11 deletions(-)

diff --git a/tools/testing/selftests/drivers/net/hw/ethtool_rmon.sh b/tools/testing/selftests/drivers/net/hw/ethtool_rmon.sh
index 1baaaab3c3a4..5e5fc80867a0 100755
--- a/tools/testing/selftests/drivers/net/hw/ethtool_rmon.sh
+++ b/tools/testing/selftests/drivers/net/hw/ethtool_rmon.sh
@@ -25,9 +25,9 @@ ensure_mtu()
 	local required=$((len - ETH_HLEN - ETH_FCS_LEN))
 	local current
 
-	current=$(ip -j link show dev "$iface" | jq -r '.[0].mtu')
+	current=$(run_on "$iface" ip -j link show dev "$iface" | jq -r '.[0].mtu')
 	if [ "$current" -lt "$required" ]; then
-		ip link set dev "$iface" mtu "$required" || return 1
+		run_on "$iface" ip link set dev "$iface" mtu "$required" || return 1
 	fi
 }
 
@@ -50,13 +50,13 @@ bucket_test()
 	len=$((len - ETH_FCS_LEN))
 	len=$((len > 0 ? len : 0))
 
-	before=$(ethtool --json -S "$iface" --groups rmon | \
+	before=$(run_on "$iface" ethtool --json -S "$iface" --groups rmon | \
 		jq -r ".[0].rmon[\"${set}-pktsNtoM\"][$bucket].val")
 
 	# Send 10k one way and 20k in the other, to detect counters
 	# mapped to the wrong direction
-	"$MZ" "$neigh" -q -c "$num_rx" -p "$len" -a own -b bcast -d 10us
-	"$MZ" "$iface" -q -c "$num_tx" -p "$len" -a own -b bcast -d 10us
+	run_on "$neigh" "$MZ" "$neigh" -q -c "$num_rx" -p "$len" -a own -b bcast -d 10us
+	run_on "$iface" "$MZ" "$iface" -q -c "$num_tx" -p "$len" -a own -b bcast -d 10us
 
 	after=$(ethtool --json -S "$iface" --groups rmon | \
 		jq -r ".[0].rmon[\"${set}-pktsNtoM\"][$bucket].val")
@@ -94,7 +94,7 @@ rmon_histogram()
 		fi
 		log_test "$step"
 		nbuckets=$((nbuckets + 1))
-	done < <(ethtool --json -S "$iface" --groups rmon | \
+	done < <(run_on "$iface" ethtool --json -S "$iface" --groups rmon | \
 		jq -r ".[0].rmon[\"${set}-pktsNtoM\"][]|[.low, .high]|@tsv" 2>/dev/null)
 
 	if [ "$nbuckets" -eq 0 ]; then
@@ -119,8 +119,8 @@ setup_prepare()
 	h2=${NETIFS[p2]}
 
 	for iface in "$h1" "$h2"; do
-		netif_mtu["$iface"]=$(ip -j link show dev "$iface" | jq -r '.[0].mtu')
-		ip link set dev "$iface" up
+		netif_mtu["$iface"]=$(run_on "$iface" ip -j link show dev "$iface" | \
+			jq -r '.[0].mtu')
 	done
 }
 
@@ -128,10 +128,10 @@ cleanup()
 {
 	pre_cleanup
 
+	# Do not bring down the interfaces, just configure the initial MTU
 	for iface in "$h2" "$h1"; do
-		ip link set dev "$iface" \
-			mtu "${netif_mtu[$iface]}" \
-			down
+		run_on "$iface" ip link set dev "$iface" \
+			mtu "${netif_mtu[$iface]}"
 	done
 }
 
-- 
2.25.1

[PATCH net-next v3 9/9] selftests: drivers: hw: add test for the ethtool standard counters

[Ioana Ciornei]

Add a new selftest - ethtool_std_stats.sh - which validates the
eth-ctrl, eth-mac and pause standard statistics exported by an
interface. Collision related eth-mac counters as well as the error ones
will be checked against zero since that is the most likely correct
scenario.

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:

 $ ./run_kselftest.sh -t drivers/net/hw:ethtool_std_stats.sh
 TAP version 13
 1..1
 # timeout set to 0
 # selftests: drivers/net/hw: ethtool_std_stats.sh
 # TEST: eth-ctrl-MACControlFramesTransmitted on endpmac17             [ OK ]
 # TEST: eth-ctrl-MACControlFramesReceived on endpmac17                [ OK ]
 # TEST: eth-mac-FrameCheckSequenceErrors on endpmac17                 [ OK ]
 # TEST: eth-mac-AlignmentErrors on endpmac17                          [ OK ]
 # TEST: eth-mac-FramesLostDueToIntMACXmitError on endpmac17           [ OK ]
 # TEST: eth-mac-CarrierSenseErrors on endpmac17 (not supported)       [SKIP]
 # TEST: eth-mac-FramesLostDueToIntMACRcvError on endpmac17            [ OK ]
 # TEST: eth-mac-InRangeLengthErrors on endpmac17 (not supported)      [SKIP]
 # TEST: eth-mac-OutOfRangeLengthField on endpmac17 (not supported)    [SKIP]
 # TEST: eth-mac-FrameTooLongErrors on endpmac17 (not supported)       [SKIP]
 # TEST: eth-mac-FramesAbortedDueToXSColls on endpmac17 (not supported)  [SKIP]
 # TEST: eth-mac-SingleCollisionFrames on endpmac17 (not supported)    [SKIP]
 # TEST: eth-mac-MultipleCollisionFrames on endpmac17 (not supported)  [SKIP]
 # TEST: eth-mac-FramesWithDeferredXmissions on endpmac17 (not supported)  [SKIP]
 # TEST: eth-mac-LateCollisions on endpmac17 (not supported)           [SKIP]
 # TEST: eth-mac-FramesWithExcessiveDeferral on endpmac17 (not supported)  [SKIP]
 # TEST: eth-mac-BroadcastFramesXmittedOK on endpmac17                 [ OK ]
 # TEST: eth-mac-OctetsTransmittedOK on endpmac17                      [ OK ]
 # TEST: eth-mac-BroadcastFramesReceivedOK on endpmac17                [ OK ]
 # TEST: eth-mac-OctetsReceivedOK on endpmac17                         [ OK ]
 # TEST: eth-mac-FramesTransmittedOK on endpmac17                      [ OK ]
 # TEST: eth-mac-MulticastFramesXmittedOK on endpmac17                 [ OK ]
 # TEST: eth-mac-FramesReceivedOK on endpmac17                         [ OK ]
 # TEST: eth-mac-MulticastFramesReceivedOK on endpmac17                [ OK ]
 # TEST: pause-tx_pause_frames on endpmac17                            [ OK ]
 # TEST: pause-rx_pause_frames on endpmac17                            [ OK ]
 ok 1 selftests: drivers/net/hw: ethtool_std_stats.sh # SKIP

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.

 $ ./run_kselftest.sh -t drivers/net/hw:ethtool_std_stats.sh
 (...)
 # TEST: pause-tx_pause_frames on eno0 (software sent pause frames not detected)  [XFAIL]
 # TEST: pause-rx_pause_frames on eno0                                 [ OK ]

Signed-off-by: Ioana Ciornei <ioana.ciornei@nxp.com>
---
Changes in v3:
- none
Changes in v2:
- Use the new run_on helper
- No longer checking that each counter has a 1% tolerance against the
  target. The only upper limit is UINT32_MAX.
- Check that both the error counters and the collision related ones are
  zero since this is the most probable scenario (we don't expect errors
  with the traffic that we are sending).
- Removed any checks performed on the remot interface counters since
  that is being used only as a traffic generator.

 .../testing/selftests/drivers/net/hw/Makefile |   1 +
 .../drivers/net/hw/ethtool_std_stats.sh       | 187 ++++++++++++++++++
 2 files changed, 188 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 91df028abfc0..2b185d2fa32e 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..b411a0c80bde
--- /dev/null
+++ b/tools/testing/selftests/drivers/net/hw/ethtool_std_stats.sh
@@ -0,0 +1,187 @@
+#!/bin/bash
+# SPDX-License-Identifier: GPL-2.0
+#shellcheck disable=SC2034 # SC does not see the global variables
+#shellcheck disable=SC2317,SC2329 # unused functions
+
+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
+
+UINT32_MAX=$((2**32 - 1))
+
+traffic_test()
+{
+	local iface=$1; shift
+	local neigh=$1; shift
+	local num_tx=$1; shift
+	local pkt_format="$1"; shift
+	local -a counters=("$@")
+	local int grp counter target exact_check
+	local before after delta
+	local num_rx=$((num_tx * 2))
+	local xfail_message
+	local src="aggregate"
+	local i
+
+	for i in "${!counters[@]}"; do
+		read -r int grp counter target exact_check xfail_message <<< "${counters[$i]}"
+
+		before[i]=$(ethtool_std_stats_get "$int" "$grp" "$counter" "$src")
+	done
+
+	# shellcheck disable=SC2086 # needs split options
+	run_on "$iface" "$MZ" "$iface" -q -c "$num_tx" $pkt_format
+
+	# shellcheck disable=SC2086 # needs split options
+	run_on "$neigh" "$MZ" "$neigh" -q -c "$num_rx" $pkt_format
+
+	for i in "${!counters[@]}"; do
+		read -r int grp counter target exact_check xfail_message <<< "${counters[$i]}"
+
+		after[i]=$(ethtool_std_stats_get "$int" "$grp" "$counter" "$src")
+		if [[ "${after[$i]}" == "null" ]]; then
+			log_test_skip "$grp-$counter on $int" "not supported"
+			continue;
+		fi
+
+		delta=$((after[i] - before[i]))
+
+		RET=0
+		if [ "$exact_check" -ne 0 ]; then
+			[ "$delta" -eq "$target" ]
+		else
+			[ "$delta" -ge "$target" ] && [ "$delta" -le "$UINT32_MAX" ]
+		fi
+		err="$?"
+
+		if [[ $err != 0  ]] && [[ -n $xfail_message ]]; then
+			log_test_xfail "$grp-$counter on $int" "$xfail_message"
+			continue;
+		fi
+		check_err "$err" "$grp-$counter is not valid on $int (expected $target, got $delta)"
+		log_test "$grp-$counter on $int"
+	done
+}
+
+test_eth_ctrl_stats()
+{
+	local pkt_format="-a own -b bcast 88:08 -p 64"
+	local num_pkts=1000
+	local -a counters
+
+	counters=("$h1 eth-ctrl MACControlFramesTransmitted $num_pkts 0")
+	traffic_test "$h1" "$h2" "$num_pkts" "$pkt_format" \
+		"${counters[@]}"
+
+	counters=("$h1 eth-ctrl MACControlFramesReceived $num_pkts 0")
+	traffic_test "$h2" "$h1" "$num_pkts" "$pkt_format" \
+		"${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 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 -a counters error_cnt collision_cnt
+
+	# Error counters should be exactly zero
+	counters=("$h1 eth-mac FrameCheckSequenceErrors 0 1"
+		  "$h1 eth-mac AlignmentErrors 0 1"
+		  "$h1 eth-mac FramesLostDueToIntMACXmitError 0 1"
+		  "$h1 eth-mac CarrierSenseErrors 0 1"
+		  "$h1 eth-mac FramesLostDueToIntMACRcvError 0 1"
+		  "$h1 eth-mac InRangeLengthErrors 0 1"
+		  "$h1 eth-mac OutOfRangeLengthField 0 1"
+		  "$h1 eth-mac FrameTooLongErrors 0 1"
+		  "$h1 eth-mac FramesAbortedDueToXSColls 0 1")
+	traffic_test "$h1" "$h2" "$num_pkts" "$bcast_pkt_format" \
+		"${counters[@]}"
+
+	# Collision related counters should also be zero
+	counters=("$h1 eth-mac SingleCollisionFrames 0 1"
+		  "$h1 eth-mac MultipleCollisionFrames 0 1"
+		  "$h1 eth-mac FramesWithDeferredXmissions 0 1"
+		  "$h1 eth-mac LateCollisions 0 1"
+		  "$h1 eth-mac FramesWithExcessiveDeferral 0 1")
+	traffic_test "$h1" "$h2" "$num_pkts" "$bcast_pkt_format" \
+		"${counters[@]}"
+
+	counters=("$h1 eth-mac BroadcastFramesXmittedOK $num_pkts 0"
+		  "$h1 eth-mac OctetsTransmittedOK $octets 0")
+	traffic_test "$h1" "$h2" "$num_pkts" "$bcast_pkt_format" \
+		"${counters[@]}"
+
+	counters=("$h1 eth-mac BroadcastFramesReceivedOK $num_pkts 0"
+		  "$h1 eth-mac OctetsReceivedOK $octets 0")
+	traffic_test "$h2" "$h1" "$num_pkts" "$bcast_pkt_format" \
+		"${counters[@]}"
+
+	counters=("$h1 eth-mac FramesTransmittedOK $num_pkts 0"
+		  "$h1 eth-mac MulticastFramesXmittedOK $num_pkts 0")
+	traffic_test "$h1" "$h2" "$num_pkts" "$mcast_pkt_format" \
+		"${counters[@]}"
+
+	counters=("$h1 eth-mac FramesReceivedOK $num_pkts 0"
+		  "$h1 eth-mac MulticastFramesReceivedOK $num_pkts 0")
+	traffic_test "$h2" "$h1" "$num_pkts" "$mcast_pkt_format" \
+		"${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="software sent pause frames not detected"
+	local num_pkts=2000
+	local -a counters
+	local int
+	local i
+
+	# Check that there is pause frame support
+	for ((i = 1; i <= NUM_NETIFS; ++i)); do
+		int="${NETIFS[p$i]}"
+		if ! run_on "$int" ethtool -I --json -a "$int" > /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 0 $xfail_message")
+	traffic_test "$h1" "$h2" "$num_pkts" "$pkt_format" \
+		"${counters[@]}"
+
+	counters=("$h1 pause rx_pause_frames $num_pkts 0")
+	traffic_test "$h2" "$h1" "$num_pkts" "$pkt_format" \
+		"${counters[@]}"
+}
+
+setup_prepare()
+{
+	local iface
+
+	h1=${NETIFS[p1]}
+	h2=${NETIFS[p2]}
+
+	h2_mac=$(mac_get "$h2")
+}
+
+check_ethtool_counter_group_support
+trap cleanup EXIT
+
+setup_prepare
+setup_wait
+
+tests_run
+
+exit "$EXIT_STATUS"
-- 
2.25.1

Re: [PATCH net-next v3 3/9] selftests: net: extend lib.sh to parse drivers/net/net.config

[Petr Machata]

Ioana Ciornei <ioana.ciornei@nxp.com> writes:

> Extend lib.sh so that it's able to parse driver/net/net.config and
> environment variables such as NETIF, REMOTE_TYPE, LOCAL_V4 etc described
> in drivers/net/README.rst.
>
> In order to make the transition towards running with a single local
> interface smoother for the bash networking driver tests, beside sourcing
> the net.config file also translate the new env variables into the old
> style based on the NETIFS array. Since the NETIFS array only holds the
> network interface names, also add a new array - TARGETS - which keeps
> track of the target on which a specific interfaces resides - local,
> netns or accesible through an ssh command.
>
> For example, a net.config which looks like below:
>
> 	NETIF=eth0
> 	LOCAL_V4=192.168.1.1
> 	REMOTE_V4=192.168.1.2
> 	REMOTE_TYPE=ssh
> 	REMOTE_ARGS=root@192.168.1.2
>
> will generate the NETIFS and TARGETS arrays with the following data.
>
> 	NETIFS[p1]="eth0"
> 	NETIFS[p2]="eth2"
>
> 	TARGETS[eth0]="local:"
> 	TARGETS[eth2]="ssh:root@192.168.1.2"
>
> The above will be true if on the remote target, the interface which has
> the 192.168.1.2 address is named eth2.
>
> Since the TARGETS array is indexed by the network interface name,
> document a new restriction README.rst which states that the remote
> interface cannot have the same name as the local one.

Isn't this going to be a somewhat common scenario though?

> Also keep the old way of populating the NETIFS variable based on the
> command line arguments. This will be invoked in case NETIF is not
> defined.
>
> Signed-off-by: Ioana Ciornei <ioana.ciornei@nxp.com>
> ---
> Changes in v3:
> - s/TARGET/CUR_TARGET
> - this used to be patch #2/9 in v2. Swapped the two patches so that the
>   run_cmd used in this patch is defined earlier, not later.
> Changes in v2:
> - patch is new
>
>  .../testing/selftests/drivers/net/README.rst  |   3 +
>  tools/testing/selftests/net/forwarding/lib.sh | 130 ++++++++++++++++--
>  2 files changed, 124 insertions(+), 9 deletions(-)
>
> diff --git a/tools/testing/selftests/drivers/net/README.rst b/tools/testing/selftests/drivers/net/README.rst
> index c94992acf10b..8d8d9d62e763 100644
> --- a/tools/testing/selftests/drivers/net/README.rst
> +++ b/tools/testing/selftests/drivers/net/README.rst
> @@ -26,6 +26,9 @@ The netdevice against which tests will be run must exist, be running
>  Refer to list of :ref:`Variables` later in this file to set up running
>  the tests against a real device.
>  
> +Also, make sure that if you are using a remote machine for traffic injection,
> +the local and remote interfaces have different names.
> +
>  Both modes required
>  ~~~~~~~~~~~~~~~~~~~
>  
> diff --git a/tools/testing/selftests/net/forwarding/lib.sh b/tools/testing/selftests/net/forwarding/lib.sh
> index cf40cb766c68..fb5aa56343e1 100644
> --- a/tools/testing/selftests/net/forwarding/lib.sh
> +++ b/tools/testing/selftests/net/forwarding/lib.sh
> @@ -3,6 +3,7 @@
>  
>  ##############################################################################
>  # Topology description. p1 looped back to p2, p3 to p4 and so on.
> +#shellcheck disable=SC2034 # SC doesn't see our uses of global variables
>  
>  declare -A NETIFS=(
>      [p1]=veth0
> @@ -17,6 +18,26 @@ declare -A NETIFS=(
>      [p10]=veth9
>  )
>  
> +# Array indexed by the network interface name keeping track of the target on
> +# which the interface resides. Values will be strings of the following format -
> +# <type>:<args>.
> +# TARGETS[eth0]="local:" - meaning that the eth0 interface is accessible locally
> +# TARGETS[eth1]="netns:foo" - eth1 is in the foo netns
> +# TARGETS[eth2]="ssh:root@10.0.0.2" - eth2 is accessible through running the
> +# 'ssh root@10.0.0.2' command.
> +declare -A TARGETS=(

This is a helper array for internal use only, not a user interface. It
should just be declared suitably close to where it's actually
initialized. It doesn't even need its own initialization, there's code
down there to initialize it accordingly.

> +    [veth0]="local:"
> +    [veth1]="local:"
> +    [veth2]="local:"
> +    [veth3]="local:"
> +    [veth4]="local:"
> +    [veth5]="local:"
> +    [veth6]="local:"
> +    [veth7]="local:"
> +    [veth8]="local:"
> +    [veth9]="local:"
> +)
> +
>  # Port that does not have a cable connected.
>  : "${NETIF_NO_CABLE:=eth8}"
>  
> @@ -340,17 +361,108 @@ fi
>  ##############################################################################
>  # Command line options handling
>  
> -count=0
> +check_env() {
> +	local vars_needed=("LOCAL_V4,LOCAL_V6"
> +			   "REMOTE_V4,REMOTE_V6"
> +			   "REMOTE_TYPE"
> +			   "REMOTE_ARGS")

Given how much hand-rolled code is needed for cross-checking local x
remote config symmetry anyway, I wonder if this whole thing should just
be very simply:

	if [[ ! (( -n "$LOCAL_V4" && -n "$REMOTE_V4") ||
		 ( -n "$LOCAL_V6" && -n "$REMOTE_V6" )) ]]; then
		echo "SKIP: Invalid environment, missing or inconsistent LOCAL_V4/REMOTE_V4/LOCAL_V6/REMOTE_V6"
                exit "$ksft_skip"
        fi

Then you still need a manual check for REMOTE_TYPE and REMOTE_ARGS, but
that's still just three checks all told like the current code, and we
get rid of the double nested loop that's kinda tricky to understand. It
doesn't look like the generalization really pays off here.

> +	local missing=()
> +	local choice
> +
> +	# If a choice has multiple comma separated options, at least one must
> +	# exist
> +	for choice in "${vars_needed[@]}"; do
> +		IFS=',' read -ra entries <<< "$choice"

Entries should be declared as local.

> +
> +		local found=0
> +		for entry in "${entries[@]}"; do
> +			if [[ -n "${!entry}" ]]; then
> +				found=1
> +				break
> +			fi
> +		done
>  
> -while [[ $# -gt 0 ]]; do
> -	if [[ "$count" -eq "0" ]]; then
> -		unset NETIFS
> -		declare -A NETIFS
> +		if [[ $found -eq 0 ]]; then
> +			missing+=("$choice")
> +		fi
> +	done
> +
> +	# Make sure v4 / v6 configs are symmetric
> +	if [[ (-n "${LOCAL_V6}" && -z "${REMOTE_V6}") || \
> +	      (-z "${LOCAL_V6}" && -n "${REMOTE_V6}") ]]; then
> +		missing+=("LOCAL_V6,REMOTE_V6")
>  	fi
> -	count=$((count + 1))
> -	NETIFS[p$count]="$1"
> -	shift
> -done
> +
> +	if [[ (-n "${LOCAL_V4}" && -z "${REMOTE_V4}") || \
> +	      (-z "${LOCAL_V4}" && -n "${REMOTE_V4}") ]]; then
> +		missing+=("LOCAL_V4,REMOTE_V4")
> +	fi
> +
> +	if [[ ${#missing[@]} -gt 0 ]]; then
> +		echo "SKIP: Invalid environment, missing configuration: ${missing[*]}"
> +		echo "Please see tools/testing/selftests/drivers/net/README.rst"
> +		exit "$ksft_skip"
> +	fi
> +}
> +
> +get_ifname_by_ip()
> +{
> +	local ip_addr=$1; shift
> +
> +	run_cmd ip -j addr show to "$ip_addr" | jq -r '.[].ifname'
> +}
> +
> +# If there is a configuration file, source it
> +if [[ -f $net_forwarding_dir/../../drivers/net/net.config ]]; then
> +	source "$net_forwarding_dir/../../drivers/net/net.config"
> +fi

This is going to impact all tests though, not just those that are meant
to work with this. If I have this file, it sets NETIF, and all my
forwarding tests (presumably) break, because NETIFS doesn't get
configured correctly.

The requirements for driver tests are different from forwarding tests.
In particular, interfaces do not belong to driver tests, the test can't
bring them up or down, reassign addresses etc., merely to use them for
traffic. (Which you know, I'm just stating it for context.) So I think
there needs to be an opt-in variable for this stuff. Like setting
DRIVER_TEST_CONFORMANT=yes before sourcing the library or whatever.

Then NETIF ceases to be a tell and becomes an environment requirement
for check_env to validate like the others.

(Another alternative is to extract a common core from lib.sh and have
both lib.sh and net/driver/lib.sh use that. But that's more of a change
and I'm not even sure it's the right way to slice it.)

> +# In case NETIF is specified, then the test expects to pass the arguments
> +# through the variables specified in drivers/net/README.rst file. If not,
> +# fallback on parsing the script arguments for interface names.
> +if [[ -v NETIF ]]; then
> +	if (( NUM_NETIFS > 2)); then
> +		echo "SKIP: NETIF defined and NUM_NETIFS is bigger than 2"
> +		exit "$ksft_skip"
> +	fi
> +
> +	check_env
> +
> +	# Populate the NETIF and TARGETS arrays automatically based on the
> +	# environment variables
> +	unset NETIFS
> +	declare -A NETIFS
> +
> +	NETIFS[p1]="$NETIF"
> +	TARGETS[$NETIF]="local:"
> +
> +	# Locate the name of the remote interface
> +	if [[ -v REMOTE_V4 ]]; then
> +		remote_netif=$(CUR_TARGET="$REMOTE_TYPE:$REMOTE_ARGS" get_ifname_by_ip "$REMOTE_V4")
> +	else
> +		remote_netif=$(CUR_TARGET="$REMOTE_TYPE:$REMOTE_ARGS" get_ifname_by_ip "$REMOTE_V6")
> +	fi
> +	if [[ ! -n "$remote_netif" ]]; then
> +		echo "SKIP: cannot find remote interface"
> +		exit "$ksft_skip"
> +	fi

This should check that $NETIF != $remote_netif.

> +	NETIFS[p2]="$remote_netif"
> +	TARGETS[$remote_netif]="$REMOTE_TYPE:$REMOTE_ARGS"
> +else
> +	count=0
> +
> +	while [[ $# -gt 0 ]]; do
> +		if [[ "$count" -eq "0" ]]; then
> +			unset NETIFS
> +			declare -A NETIFS
> +		fi
> +		count=$((count + 1))
> +		NETIFS[p$count]="$1"
> +		TARGETS[$1]="local:"
> +		shift
> +	done
> +fi
>  
>  ##############################################################################
>  # Network interfaces configuration

Re: [PATCH net-next v3 2/9] selftests: net: add helpers for running a command on other targets

[Petr Machata]

Ioana Ciornei <ioana.ciornei@nxp.com> writes:

> Add a couple of helpers which can be used by tests which need to run a
> specific bash command on a different target than the local system, be it
> either another netns or a remote system accessible through ssh.
>
> The run_cmd() function decides where to execute the command passed
> through $@ based on the env variable TARGET value while run_on() will
> receive the target through its first argument.
>
> Signed-off-by: Ioana Ciornei <ioana.ciornei@nxp.com>
> ---
> Changes in v3:
> - s/TARGET/CUR_TARGET
> - always fallback on running a command locally when either TARGETS is
>   not declared or there is no entry for a specific interface
> Changes in v2:
> - patch is new
>
>  tools/testing/selftests/net/lib.sh | 39 ++++++++++++++++++++++++++++++
>  1 file changed, 39 insertions(+)
>
> diff --git a/tools/testing/selftests/net/lib.sh b/tools/testing/selftests/net/lib.sh
> index b40694573f4c..f7c54d05758e 100644
> --- a/tools/testing/selftests/net/lib.sh
> +++ b/tools/testing/selftests/net/lib.sh
> @@ -28,6 +28,10 @@ EXIT_STATUS=0
>  # Per-test return value. Clear at the beginning of each test.
>  RET=0
>  
> +# If a specific command needs to be executed on another target than local, set
> +# this appropriately before calling run_cmd
> +CUR_TARGET="local:"
> +

These are new functions, they should just expose these parameters as
actual function parameters, not as hidden environment. If it is
desirable that run_cmd() can run local commands, maybe it makes sense to
make the current run_cmd() a helper like __run_cmd(), which takes the
remote-config parameter, and have run_cmd() be a wrapper:

run_cmd()
{
	__run_cmd local: "$@"
}

>  ##############################################################################
>  # Helpers
>  
> @@ -670,3 +674,38 @@ cmd_jq()
>  	# return success only in case of non-empty output
>  	[ ! -z "$output" ]
>  }
> +
> +run_cmd()
> +{
> +	IFS=':' read -r type args <<< "$CUR_TARGET"
> +
> +	case "$type" in
> +		netns)
> +			# Execute command in network namespace
> +			# args contains the namespace name
> +			ip netns exec "$args" "$@"
> +			;;
> +		ssh)
> +			# Execute command via SSH args contains user@host
> +			ssh -n "$args" "$@"
> +			;;
> +		local|*)
> +			# Execute command locally. This is also the fallback
> +			# case in case the CUR_TARGET is not set.
> +			"$@"
> +			;;
> +	esac
> +}
> +
> +run_on()
> +{
> +	local iface=$1; shift
> +
> +	if declare -p TARGETS &>/dev/null; then
> +		CUR_TARGET="${TARGETS[$iface]}"
> +	else
> +		CUR_TARGET="local:"
> +	fi
> +
> +	run_cmd "$@"
> +}

Re: [PATCH net-next v3 1/9] selftests: forwarding: extend ethtool_std_stats_get with pause statistics

[Petr Machata]

Ioana Ciornei <ioana.ciornei@nxp.com> writes:

> Even though pause frame statistics are not exported through the same
> ethtool command, there is no point in adding another helper just for
> them. Extent the ethtool_std_stats_get() function so that we are able to
> interrogate using the same helper all the standard statistics.
>
> And since we are touching the else case, convert that as well to the jq
> --arg form in order to be easier to read.
>
> Signed-off-by: Ioana Ciornei <ioana.ciornei@nxp.com>

Reviewed-by: Petr Machata <petrm@nvidia.com>

Re: [PATCH net-next v3 1/9] selftests: forwarding: extend ethtool_std_stats_get with pause statistics

[Petr Machata]

Ioana Ciornei <ioana.ciornei@nxp.com> writes:

> Even though pause frame statistics are not exported through the same
> ethtool command, there is no point in adding another helper just for
> them. Extent the ethtool_std_stats_get() function so that we are able to
> interrogate using the same helper all the standard statistics.
>
> And since we are touching the else case, convert that as well to the jq
> --arg form in order to be easier to read.
>
> Signed-off-by: Ioana Ciornei <ioana.ciornei@nxp.com>
> ---
> Changes in v3:
> - none
> Changes in v2:
> - convert jq to the --arg usage form
>
>  tools/testing/selftests/net/forwarding/lib.sh | 8 ++++++--
>  1 file changed, 6 insertions(+), 2 deletions(-)
>
> diff --git a/tools/testing/selftests/net/forwarding/lib.sh b/tools/testing/selftests/net/forwarding/lib.sh
> index a9034f0bb58b..cf40cb766c68 100644
> --- a/tools/testing/selftests/net/forwarding/lib.sh
> +++ b/tools/testing/selftests/net/forwarding/lib.sh
> @@ -831,8 +831,12 @@ ethtool_std_stats_get()
>  	local name=$1; shift
>  	local src=$1; shift
>  
> -	ethtool --json -S $dev --groups $grp -- --src $src | \
> -		jq '.[]."'"$grp"'"."'$name'"'
> +	if [[ "$grp" == "pause" ]]; then
> +		ethtool -I --json -a "$dev" | jq --arg name "$name" '.[].statistics[$name]'

I missed this overlong line. Fold it, please. The R-b applies with this change.

> +	else
> +		ethtool --json -S "$dev" --groups "$grp" -- --src "$src" | \
> +			jq --arg grp "$grp" --arg name "$name" '.[][$grp][$name]'
> +	fi
>  }
>  
>  qdisc_stats_get()

Re: [PATCH net-next v3 4/9] selftests: net: update some helpers to use run_on

[Petr Machata]

Ioana Ciornei <ioana.ciornei@nxp.com> writes:

> Update some helpers so that they are capable to run commands on
> different targets than the local one. This patch makes the necesasy
> modification for those helpers / sections of code which are needed for
> the ethtool_rmon.sh test that will be converted in the next patches.
>
> For example, mac_addr_prepare() and mac_addr_restore() used when
> STABLE_MAC_ADDRS=yes need to ensure stable MAC addresses on interfaces
> located even in other namespaces. In order to do that, append the 'ip
> link' commands with a 'run_on $dev' tag.
>
> The same run_on is necessary also when verifying if all the interfaces
> listed in NETIFS are indeed available.
>
> Signed-off-by: Ioana Ciornei <ioana.ciornei@nxp.com>

This looks good, but I'm unhappy with some overlong lines again :)

> ---
> Changes in v3:
> - added some more double quotes
> Changes in v2:
> - patch is new
>
>  tools/testing/selftests/net/forwarding/lib.sh | 17 ++++++++++-------
>  tools/testing/selftests/net/lib.sh            |  2 +-
>  2 files changed, 11 insertions(+), 8 deletions(-)
>
> diff --git a/tools/testing/selftests/net/forwarding/lib.sh b/tools/testing/selftests/net/forwarding/lib.sh
> index fb5aa56343e1..f76de37d0caf 100644
> --- a/tools/testing/selftests/net/forwarding/lib.sh
> +++ b/tools/testing/selftests/net/forwarding/lib.sh
> @@ -530,10 +530,10 @@ mac_addr_prepare()
>  		dev=${NETIFS[p$i]}
>  		new_addr=$(printf "00:01:02:03:04:%02x" $i)
>  
> -		MAC_ADDR_ORIG["$dev"]=$(ip -j link show dev $dev | jq -e '.[].address')
> +		MAC_ADDR_ORIG["$dev"]=$(run_on "$dev" ip -j link show dev "$dev" | jq -e '.[].address')

Here.

>  		# Strip quotes
>  		MAC_ADDR_ORIG["$dev"]=${MAC_ADDR_ORIG["$dev"]//\"/}
> -		ip link set dev $dev address $new_addr
> +		run_on "$dev" ip link set dev "$dev" address $new_addr
>  	done
>  }
>  
> @@ -543,7 +543,7 @@ mac_addr_restore()
>  
>  	for ((i = 1; i <= NUM_NETIFS; ++i)); do
>  		dev=${NETIFS[p$i]}
> -		ip link set dev $dev address ${MAC_ADDR_ORIG["$dev"]}
> +		run_on "$dev" ip link set dev "$dev" address ${MAC_ADDR_ORIG["$dev"]}

And here.

>  	done
>  }
>  
> @@ -556,7 +556,9 @@ if [[ "$STABLE_MAC_ADDRS" = "yes" ]]; then
>  fi
>  
>  for ((i = 1; i <= NUM_NETIFS; ++i)); do
> -	ip link show dev ${NETIFS[p$i]} &> /dev/null
> +	int="${NETIFS[p$i]}"
> +
> +	run_on "$int" ip link show dev "$int" &> /dev/null
>  	if [[ $? -ne 0 ]]; then
>  		echo "SKIP: could not find all required interfaces"
>  		exit $ksft_skip
> @@ -639,7 +641,7 @@ setup_wait_dev_with_timeout()
>  	local i
>  
>  	for ((i = 1; i <= $max_iterations; ++i)); do
> -		ip link show dev $dev up \
> +		run_on "$dev" ip link show dev "$dev" up \
>  			| grep 'state UP' &> /dev/null
>  		if [[ $? -ne 0 ]]; then
>  			sleep 1
> @@ -944,9 +946,10 @@ ethtool_std_stats_get()
>  	local src=$1; shift
>  
>  	if [[ "$grp" == "pause" ]]; then
> -		ethtool -I --json -a "$dev" | jq --arg name "$name" '.[].statistics[$name]'
> +		run_on "$dev" ethtool -I --json -a "$dev" | \
> +			jq --arg name "$name" '.[].statistics[$name]'

Oh, I see, you fold it here. Um, would be better in 1/9 though :)

>  	else
> -		ethtool --json -S "$dev" --groups "$grp" -- --src "$src" | \
> +		run_on "$dev" ethtool --json -S "$dev" --groups "$grp" -- --src "$src" | \
>  			jq --arg grp "$grp" --arg name "$name" '.[][$grp][$name]'

This needs to be folded as well.

>  	fi
>  }
> diff --git a/tools/testing/selftests/net/lib.sh b/tools/testing/selftests/net/lib.sh
> index f7c54d05758e..14f262a593c9 100644
> --- a/tools/testing/selftests/net/lib.sh
> +++ b/tools/testing/selftests/net/lib.sh
> @@ -518,7 +518,7 @@ mac_get()
>  {
>  	local if_name=$1
>  
> -	ip -j link show dev $if_name | jq -r '.[]["address"]'
> +	run_on "$if_name" ip -j link show dev "$if_name" | jq -r '.[]["address"]'
>  }
>  
>  kill_process()

Re: [PATCH net-next v3 1/9] selftests: forwarding: extend ethtool_std_stats_get with pause statistics

[Ioana Ciornei]

On Fri, Mar 20, 2026 at 12:20:01PM +0100, Petr Machata wrote:
> 
> Ioana Ciornei <ioana.ciornei@nxp.com> writes:
> 
> > Even though pause frame statistics are not exported through the same
> > ethtool command, there is no point in adding another helper just for
> > them. Extent the ethtool_std_stats_get() function so that we are able to
> > interrogate using the same helper all the standard statistics.
> >
> > And since we are touching the else case, convert that as well to the jq
> > --arg form in order to be easier to read.
> >
> > Signed-off-by: Ioana Ciornei <ioana.ciornei@nxp.com>
> > ---
> > Changes in v3:
> > - none
> > Changes in v2:
> > - convert jq to the --arg usage form
> >
> >  tools/testing/selftests/net/forwarding/lib.sh | 8 ++++++--
> >  1 file changed, 6 insertions(+), 2 deletions(-)
> >
> > diff --git a/tools/testing/selftests/net/forwarding/lib.sh b/tools/testing/selftests/net/forwarding/lib.sh
> > index a9034f0bb58b..cf40cb766c68 100644
> > --- a/tools/testing/selftests/net/forwarding/lib.sh
> > +++ b/tools/testing/selftests/net/forwarding/lib.sh
> > @@ -831,8 +831,12 @@ ethtool_std_stats_get()
> >  	local name=$1; shift
> >  	local src=$1; shift
> >  
> > -	ethtool --json -S $dev --groups $grp -- --src $src | \
> > -		jq '.[]."'"$grp"'"."'$name'"'
> > +	if [[ "$grp" == "pause" ]]; then
> > +		ethtool -I --json -a "$dev" | jq --arg name "$name" '.[].statistics[$name]'
> 
> I missed this overlong line. Fold it, please. The R-b applies with this change.

Ok. Will fold it.
I also realised that I should have passed --src "$src" to this ethtool
call.

Re: [PATCH net-next v3 5/9] selftests: drivers: hw: cleanup shellcheck warnings in the rmon test

[Petr Machata]

Ioana Ciornei <ioana.ciornei@nxp.com> writes:

> If run on the ethtool_rmon.sh script, shellcheck generates a bunch of
> false positive errors. Suppress those checks that generate them.
>
> Also cleanup the remaining warnings by using double quoting around the
> used variables.
>
> Signed-off-by: Ioana Ciornei <ioana.ciornei@nxp.com>

Reviewed-by: Petr Machata <petrm@nvidia.com>

Re: [PATCH net-next v3 6/9] selftests: drivers: hw: test rmon counters only on first interface

[Petr Machata]

Ioana Ciornei <ioana.ciornei@nxp.com> writes:

> The selftests in drivers/net are slowly transitioning to being able to
> be used on systems with a single network interface. The first step for the
> ethtool_rmon.sh test is to only validate that the rmon counters are
> properly exported on the first interface supplied as an argument.
>
> Remove the rmon_histogram calls which intend to test also the rmon
> counters on the 2nd interface. This also removes the need for the remote
> system, which should be used only to inject traffic, to also support
> rmon counters.
>
> Signed-off-by: Ioana Ciornei <ioana.ciornei@nxp.com>

Reviewed-by: Petr Machata <petrm@nvidia.com>

Re: [PATCH net-next v3 7/9] selftests: drivers: hw: replace counter upper limit with UINT32_MAX in rmon test

[Petr Machata]

Ioana Ciornei <ioana.ciornei@nxp.com> writes:

> The ethtool_rmon.sh script checks that the number of packets sent /
> received during a test matches the expected value with a 1% tolerance.
>
> Since in the next patches this test will gain the capability to also be
> run on systems with a single interface where the traffic generator is
> accesible through ssh, use the UINT32_MAX as the upper limit. This is
> necessary since the same interface will be used also for control traffic
> (the ssh commands) as well as the mausezahn generated one.
>
> Signed-off-by: Ioana Ciornei <ioana.ciornei@nxp.com>

Reviewed-by: Petr Machata <petrm@nvidia.com>

Re: [PATCH net-next v3 8/9] selftests: drivers: hw: update ethtool_rmon to work with a single local interface

[Petr Machata]

Ioana Ciornei <ioana.ciornei@nxp.com> writes:

> This patch finalizes the transition to work with a single local
> interface for the ethtool_rmon.sh test. Each 'ip link' and 'ethtool'
> command used by the test is annotated with the necessary run_on in
> order to be executed on the necessary target system, be it local, in
> another network namespace or through ssh.
>
> Since we need NETIF up and running also for control traffic, we now
> expect that the interfaces are up and running and do not touch bring
> them up or down at the end of the test. This is also documented in the
> drivers/net/README.rst.
>
> The ethtool_rmon.sh script can still be used in the older fashion by
> passing two interfaces as command line arguments, the only restriction
> is that those interfaces need to be already up.
>
> As part of the kselftest infrastructure, this test can be run in the
> following manner:
>
>  $ make -C tools/testing/selftests/ TARGETS="drivers/net drivers/net/hw" \
>  install INSTALL_PATH=/tmp/ksft-net-drv
>  $ cd /tmp/ksft-net-drv/
>  $ cat > ./drivers/net/net.config <<EOF
>  NETIF=endpmac17
>  LOCAL_V4=17.0.0.1
>  REMOTE_V4=17.0.0.2
>  REMOTE_TYPE=ssh
>  REMOTE_ARGS=root@192.168.5.200
>  EOF
>
>  $ ./run_kselftest.sh -t drivers/net/hw:ethtool_rmon.sh
>  TAP version 13
>  1..1
>  # timeout set to 0
>  # selftests: drivers/net/hw: ethtool_rmon.sh
>  # TEST: rx-pkts64to64 on endpmac17                                    [ OK ]
>  # TEST: rx-pkts65to127 on endpmac17                                   [ OK ]
>  # TEST: rx-pkts128to255 on endpmac17                                  [ OK ]
>  # TEST: rx-pkts256to511 on endpmac17                                  [ OK ]
>  # TEST: rx-pkts512to1023 on endpmac17                                 [ OK ]
>  # TEST: rx-pkts1024to1518 on endpmac17                                [ OK ]
>  # TEST: rx-pkts1519to10240 on endpmac17                               [ OK ]
>  # TEST: tx-pkts64to64 on endpmac17                                    [ OK ]
>  # TEST: tx-pkts65to127 on endpmac17                                   [ OK ]
>  # TEST: tx-pkts128to255 on endpmac17                                  [ OK ]
>  # TEST: tx-pkts256to511 on endpmac17                                  [ OK ]
>  # TEST: tx-pkts512to1023 on endpmac17                                 [ OK ]
>  # TEST: tx-pkts1024to1518 on endpmac17                                [ OK ]
>  # TEST: tx-pkts1519to10240 on endpmac17                               [ OK ]
>  ok 1 selftests: drivers/net/hw: ethtool_rmon.sh
>
> Signed-off-by: Ioana Ciornei <ioana.ciornei@nxp.com>

Reviewed-by: Petr Machata <petrm@nvidia.com>

> @@ -119,8 +119,8 @@ setup_prepare()
>  	h2=${NETIFS[p2]}
>  
>  	for iface in "$h1" "$h2"; do
> -		netif_mtu["$iface"]=$(ip -j link show dev "$iface" | jq -r '.[0].mtu')
> -		ip link set dev "$iface" up
> +		netif_mtu["$iface"]=$(run_on "$iface" ip -j link show dev "$iface" | \
> +			jq -r '.[0].mtu')

BTW we have mtu_set() / mtu_restore() in forwarding/lib.sh

>  	done
>  }
>  
> @@ -128,10 +128,10 @@ cleanup()
>  {
>  	pre_cleanup
>  
> +	# Do not bring down the interfaces, just configure the initial MTU
>  	for iface in "$h2" "$h1"; do
> -		ip link set dev "$iface" \
> -			mtu "${netif_mtu[$iface]}" \
> -			down
> +		run_on "$iface" ip link set dev "$iface" \
> +			mtu "${netif_mtu[$iface]}"
>  	done
>  }

Re: [PATCH net-next v3 9/9] selftests: drivers: hw: add test for the ethtool standard counters

[Petr Machata]

Ioana Ciornei <ioana.ciornei@nxp.com> writes:

> Add a new selftest - ethtool_std_stats.sh - which validates the
> eth-ctrl, eth-mac and pause standard statistics exported by an
> interface. Collision related eth-mac counters as well as the error ones
> will be checked against zero since that is the most likely correct
> scenario.
>
> 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:
>
>  $ ./run_kselftest.sh -t drivers/net/hw:ethtool_std_stats.sh
>  TAP version 13
>  1..1
>  # timeout set to 0
>  # selftests: drivers/net/hw: ethtool_std_stats.sh
>  # TEST: eth-ctrl-MACControlFramesTransmitted on endpmac17             [ OK ]
>  # TEST: eth-ctrl-MACControlFramesReceived on endpmac17                [ OK ]
>  # TEST: eth-mac-FrameCheckSequenceErrors on endpmac17                 [ OK ]
>  # TEST: eth-mac-AlignmentErrors on endpmac17                          [ OK ]
>  # TEST: eth-mac-FramesLostDueToIntMACXmitError on endpmac17           [ OK ]
>  # TEST: eth-mac-CarrierSenseErrors on endpmac17 (not supported)       [SKIP]
>  # TEST: eth-mac-FramesLostDueToIntMACRcvError on endpmac17            [ OK ]
>  # TEST: eth-mac-InRangeLengthErrors on endpmac17 (not supported)      [SKIP]
>  # TEST: eth-mac-OutOfRangeLengthField on endpmac17 (not supported)    [SKIP]
>  # TEST: eth-mac-FrameTooLongErrors on endpmac17 (not supported)       [SKIP]
>  # TEST: eth-mac-FramesAbortedDueToXSColls on endpmac17 (not supported)  [SKIP]
>  # TEST: eth-mac-SingleCollisionFrames on endpmac17 (not supported)    [SKIP]
>  # TEST: eth-mac-MultipleCollisionFrames on endpmac17 (not supported)  [SKIP]
>  # TEST: eth-mac-FramesWithDeferredXmissions on endpmac17 (not supported)  [SKIP]
>  # TEST: eth-mac-LateCollisions on endpmac17 (not supported)           [SKIP]
>  # TEST: eth-mac-FramesWithExcessiveDeferral on endpmac17 (not supported)  [SKIP]
>  # TEST: eth-mac-BroadcastFramesXmittedOK on endpmac17                 [ OK ]
>  # TEST: eth-mac-OctetsTransmittedOK on endpmac17                      [ OK ]
>  # TEST: eth-mac-BroadcastFramesReceivedOK on endpmac17                [ OK ]
>  # TEST: eth-mac-OctetsReceivedOK on endpmac17                         [ OK ]
>  # TEST: eth-mac-FramesTransmittedOK on endpmac17                      [ OK ]
>  # TEST: eth-mac-MulticastFramesXmittedOK on endpmac17                 [ OK ]
>  # TEST: eth-mac-FramesReceivedOK on endpmac17                         [ OK ]
>  # TEST: eth-mac-MulticastFramesReceivedOK on endpmac17                [ OK ]
>  # TEST: pause-tx_pause_frames on endpmac17                            [ OK ]
>  # TEST: pause-rx_pause_frames on endpmac17                            [ OK ]
>  ok 1 selftests: drivers/net/hw: ethtool_std_stats.sh # SKIP
>
> 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.
>
>  $ ./run_kselftest.sh -t drivers/net/hw:ethtool_std_stats.sh
>  (...)
>  # TEST: pause-tx_pause_frames on eno0 (software sent pause frames not detected)  [XFAIL]
>  # TEST: pause-rx_pause_frames on eno0                                 [ OK ]
>
> Signed-off-by: Ioana Ciornei <ioana.ciornei@nxp.com>

Acked-by: Petr Machata <petrm@nvidia.com>

Re: [PATCH net-next v3 4/9] selftests: net: update some helpers to use run_on

[Ioana Ciornei]

On Fri, Mar 20, 2026 at 12:22:02PM +0100, Petr Machata wrote:
> 
> Ioana Ciornei <ioana.ciornei@nxp.com> writes:
> 
> > Update some helpers so that they are capable to run commands on
> > different targets than the local one. This patch makes the necesasy
> > modification for those helpers / sections of code which are needed for
> > the ethtool_rmon.sh test that will be converted in the next patches.
> >
> > For example, mac_addr_prepare() and mac_addr_restore() used when
> > STABLE_MAC_ADDRS=yes need to ensure stable MAC addresses on interfaces
> > located even in other namespaces. In order to do that, append the 'ip
> > link' commands with a 'run_on $dev' tag.
> >
> > The same run_on is necessary also when verifying if all the interfaces
> > listed in NETIFS are indeed available.
> >
> > Signed-off-by: Ioana Ciornei <ioana.ciornei@nxp.com>
> 
> This looks good, but I'm unhappy with some overlong lines again :)

Sure, I will take care of them.

Re: [PATCH net-next v3 2/9] selftests: net: add helpers for running a command on other targets

[Ioana Ciornei]

On Fri, Mar 20, 2026 at 11:58:42AM +0100, Petr Machata wrote:
> 
> Ioana Ciornei <ioana.ciornei@nxp.com> writes:
> 
> > Add a couple of helpers which can be used by tests which need to run a
> > specific bash command on a different target than the local system, be it
> > either another netns or a remote system accessible through ssh.
> >
> > The run_cmd() function decides where to execute the command passed
> > through $@ based on the env variable TARGET value while run_on() will
> > receive the target through its first argument.
> >
> > Signed-off-by: Ioana Ciornei <ioana.ciornei@nxp.com>
> > ---
> > Changes in v3:
> > - s/TARGET/CUR_TARGET
> > - always fallback on running a command locally when either TARGETS is
> >   not declared or there is no entry for a specific interface
> > Changes in v2:
> > - patch is new
> >
> >  tools/testing/selftests/net/lib.sh | 39 ++++++++++++++++++++++++++++++
> >  1 file changed, 39 insertions(+)
> >
> > diff --git a/tools/testing/selftests/net/lib.sh b/tools/testing/selftests/net/lib.sh
> > index b40694573f4c..f7c54d05758e 100644
> > --- a/tools/testing/selftests/net/lib.sh
> > +++ b/tools/testing/selftests/net/lib.sh
> > @@ -28,6 +28,10 @@ EXIT_STATUS=0
> >  # Per-test return value. Clear at the beginning of each test.
> >  RET=0
> >  
> > +# If a specific command needs to be executed on another target than local, set
> > +# this appropriately before calling run_cmd
> > +CUR_TARGET="local:"
> > +
> 
> These are new functions, they should just expose these parameters as
> actual function parameters, not as hidden environment. If it is
> desirable that run_cmd() can run local commands, maybe it makes sense to
> make the current run_cmd() a helper like __run_cmd(), which takes the
> remote-config parameter, and have run_cmd() be a wrapper:
> 
> run_cmd()
> {
> 	__run_cmd local: "$@"
> }

Why I chose to also add a function like run_cmd that makes use of an
environment variable is to support cases such as the following.

get_ifname_by_ip()
{
	local ip_addr=$1; shift

	run_cmd ip -j addr show to "$ip_addr" | jq -r '.[].ifname'
}

Helpers such as get_ifname_by_ip() (added in patch #3) that don't have
access to an interface name so that they can directly call run_on.

Do you mean that for any helpers such as the one above that need to be
run both on the local system and the remote it's better just to add a
new parameter to it that it's indeed the target on which it should be
run and get rid of the run_cmd all together?

get_ifname_by_ip()
{
	local target=$1; shift
	local ip_addr=$1; shift

	run_on "$target" ip -j addr show to "$ip_addr" | jq -r '.[].ifname'
}

Ioana

Re: [PATCH net-next v3 3/9] selftests: net: extend lib.sh to parse drivers/net/net.config

[Ioana Ciornei]

On Fri, Mar 20, 2026 at 11:19:18AM +0100, Petr Machata wrote:
> 
> Ioana Ciornei <ioana.ciornei@nxp.com> writes:
> 
> > Extend lib.sh so that it's able to parse driver/net/net.config and
> > environment variables such as NETIF, REMOTE_TYPE, LOCAL_V4 etc described
> > in drivers/net/README.rst.
> >
> > In order to make the transition towards running with a single local
> > interface smoother for the bash networking driver tests, beside sourcing
> > the net.config file also translate the new env variables into the old
> > style based on the NETIFS array. Since the NETIFS array only holds the
> > network interface names, also add a new array - TARGETS - which keeps
> > track of the target on which a specific interfaces resides - local,
> > netns or accesible through an ssh command.
> >
> > For example, a net.config which looks like below:
> >
> > 	NETIF=eth0
> > 	LOCAL_V4=192.168.1.1
> > 	REMOTE_V4=192.168.1.2
> > 	REMOTE_TYPE=ssh
> > 	REMOTE_ARGS=root@192.168.1.2
> >
> > will generate the NETIFS and TARGETS arrays with the following data.
> >
> > 	NETIFS[p1]="eth0"
> > 	NETIFS[p2]="eth2"
> >
> > 	TARGETS[eth0]="local:"
> > 	TARGETS[eth2]="ssh:root@192.168.1.2"
> >
> > The above will be true if on the remote target, the interface which has
> > the 192.168.1.2 address is named eth2.
> >
> > Since the TARGETS array is indexed by the network interface name,
> > document a new restriction README.rst which states that the remote
> > interface cannot have the same name as the local one.
> 
> Isn't this going to be a somewhat common scenario though?

It could be if you the remote target is another system just like the
DUT. But I couldn't find an easy representation for the relationship
between interfaces and their targets that also removes this restriction.

> 
> > Also keep the old way of populating the NETIFS variable based on the
> > command line arguments. This will be invoked in case NETIF is not
> > defined.
> >
> > Signed-off-by: Ioana Ciornei <ioana.ciornei@nxp.com>
> > ---
> > Changes in v3:
> > - s/TARGET/CUR_TARGET
> > - this used to be patch #2/9 in v2. Swapped the two patches so that the
> >   run_cmd used in this patch is defined earlier, not later.
> > Changes in v2:
> > - patch is new
> >
> >  .../testing/selftests/drivers/net/README.rst  |   3 +
> >  tools/testing/selftests/net/forwarding/lib.sh | 130 ++++++++++++++++--
> >  2 files changed, 124 insertions(+), 9 deletions(-)
> >
> > diff --git a/tools/testing/selftests/drivers/net/README.rst b/tools/testing/selftests/drivers/net/README.rst
> > index c94992acf10b..8d8d9d62e763 100644
> > --- a/tools/testing/selftests/drivers/net/README.rst
> > +++ b/tools/testing/selftests/drivers/net/README.rst
> > @@ -26,6 +26,9 @@ The netdevice against which tests will be run must exist, be running
> >  Refer to list of :ref:`Variables` later in this file to set up running
> >  the tests against a real device.
> >  
> > +Also, make sure that if you are using a remote machine for traffic injection,
> > +the local and remote interfaces have different names.
> > +
> >  Both modes required
> >  ~~~~~~~~~~~~~~~~~~~
> >  
> > diff --git a/tools/testing/selftests/net/forwarding/lib.sh b/tools/testing/selftests/net/forwarding/lib.sh
> > index cf40cb766c68..fb5aa56343e1 100644
> > --- a/tools/testing/selftests/net/forwarding/lib.sh
> > +++ b/tools/testing/selftests/net/forwarding/lib.sh
> > @@ -3,6 +3,7 @@
> >  
> >  ##############################################################################
> >  # Topology description. p1 looped back to p2, p3 to p4 and so on.
> > +#shellcheck disable=SC2034 # SC doesn't see our uses of global variables
> >  
> >  declare -A NETIFS=(
> >      [p1]=veth0
> > @@ -17,6 +18,26 @@ declare -A NETIFS=(
> >      [p10]=veth9
> >  )
> >  
> > +# Array indexed by the network interface name keeping track of the target on
> > +# which the interface resides. Values will be strings of the following format -
> > +# <type>:<args>.
> > +# TARGETS[eth0]="local:" - meaning that the eth0 interface is accessible locally
> > +# TARGETS[eth1]="netns:foo" - eth1 is in the foo netns
> > +# TARGETS[eth2]="ssh:root@10.0.0.2" - eth2 is accessible through running the
> > +# 'ssh root@10.0.0.2' command.
> > +declare -A TARGETS=(
> 
> This is a helper array for internal use only, not a user interface. It
> should just be declared suitably close to where it's actually
> initialized. It doesn't even need its own initialization, there's code
> down there to initialize it accordingly.

Ok. In v3 I added the fallback to run on the local system in case either
TARGETS is not declared or the entry is not valid so I can safely remove
this now. Thanks!

> 
> > +    [veth0]="local:"
> > +    [veth1]="local:"
> > +    [veth2]="local:"
> > +    [veth3]="local:"
> > +    [veth4]="local:"
> > +    [veth5]="local:"
> > +    [veth6]="local:"
> > +    [veth7]="local:"
> > +    [veth8]="local:"
> > +    [veth9]="local:"
> > +)
> > +
> >  # Port that does not have a cable connected.
> >  : "${NETIF_NO_CABLE:=eth8}"
> >  
> > @@ -340,17 +361,108 @@ fi
> >  ##############################################################################
> >  # Command line options handling
> >  
> > -count=0
> > +check_env() {
> > +	local vars_needed=("LOCAL_V4,LOCAL_V6"
> > +			   "REMOTE_V4,REMOTE_V6"
> > +			   "REMOTE_TYPE"
> > +			   "REMOTE_ARGS")
> 
> Given how much hand-rolled code is needed for cross-checking local x
> remote config symmetry anyway, I wonder if this whole thing should just
> be very simply:
> 
> 	if [[ ! (( -n "$LOCAL_V4" && -n "$REMOTE_V4") ||
> 		 ( -n "$LOCAL_V6" && -n "$REMOTE_V6" )) ]]; then
> 		echo "SKIP: Invalid environment, missing or inconsistent LOCAL_V4/REMOTE_V4/LOCAL_V6/REMOTE_V6"
>                 exit "$ksft_skip"
>         fi
> 
> Then you still need a manual check for REMOTE_TYPE and REMOTE_ARGS, but
> that's still just three checks all told like the current code, and we
> get rid of the double nested loop that's kinda tricky to understand. It
> doesn't look like the generalization really pays off here.

I took as an example the _check_env in env.py and it got to look way
worse in bash than necessary, agreed. Will simplify it, thanks!

> 
> > +	local missing=()
> > +	local choice
> > +
> > +	# If a choice has multiple comma separated options, at least one must
> > +	# exist
> > +	for choice in "${vars_needed[@]}"; do
> > +		IFS=',' read -ra entries <<< "$choice"
> 
> Entries should be declared as local.

Will do.

> > +
> > +		local found=0
> > +		for entry in "${entries[@]}"; do
> > +			if [[ -n "${!entry}" ]]; then
> > +				found=1
> > +				break
> > +			fi
> > +		done
> >  
> > -while [[ $# -gt 0 ]]; do
> > -	if [[ "$count" -eq "0" ]]; then
> > -		unset NETIFS
> > -		declare -A NETIFS
> > +		if [[ $found -eq 0 ]]; then
> > +			missing+=("$choice")
> > +		fi
> > +	done
> > +
> > +	# Make sure v4 / v6 configs are symmetric
> > +	if [[ (-n "${LOCAL_V6}" && -z "${REMOTE_V6}") || \
> > +	      (-z "${LOCAL_V6}" && -n "${REMOTE_V6}") ]]; then
> > +		missing+=("LOCAL_V6,REMOTE_V6")
> >  	fi
> > -	count=$((count + 1))
> > -	NETIFS[p$count]="$1"
> > -	shift
> > -done
> > +
> > +	if [[ (-n "${LOCAL_V4}" && -z "${REMOTE_V4}") || \
> > +	      (-z "${LOCAL_V4}" && -n "${REMOTE_V4}") ]]; then
> > +		missing+=("LOCAL_V4,REMOTE_V4")
> > +	fi
> > +
> > +	if [[ ${#missing[@]} -gt 0 ]]; then
> > +		echo "SKIP: Invalid environment, missing configuration: ${missing[*]}"
> > +		echo "Please see tools/testing/selftests/drivers/net/README.rst"
> > +		exit "$ksft_skip"
> > +	fi
> > +}
> > +
> > +get_ifname_by_ip()
> > +{
> > +	local ip_addr=$1; shift
> > +
> > +	run_cmd ip -j addr show to "$ip_addr" | jq -r '.[].ifname'
> > +}
> > +
> > +# If there is a configuration file, source it
> > +if [[ -f $net_forwarding_dir/../../drivers/net/net.config ]]; then
> > +	source "$net_forwarding_dir/../../drivers/net/net.config"
> > +fi
> 
> This is going to impact all tests though, not just those that are meant
> to work with this. If I have this file, it sets NETIF, and all my
> forwarding tests (presumably) break, because NETIFS doesn't get
> configured correctly.
> 
> The requirements for driver tests are different from forwarding tests.
> In particular, interfaces do not belong to driver tests, the test can't
> bring them up or down, reassign addresses etc., merely to use them for
> traffic. (Which you know, I'm just stating it for context.) So I think
> there needs to be an opt-in variable for this stuff. Like setting
> DRIVER_TEST_CONFORMANT=yes before sourcing the library or whatever.

I had this kind of approach initially but thought that it might be
desirable to also maintain the possibility to run the driver tests as
./test.sh eth0 eth1 as before, so I changed it.
I can move back to that approach.

> 
> Then NETIF ceases to be a tell and becomes an environment requirement
> for check_env to validate like the others.
> 
> (Another alternative is to extract a common core from lib.sh and have
> both lib.sh and net/driver/lib.sh use that. But that's more of a change
> and I'm not even sure it's the right way to slice it.)
> 
> > +# In case NETIF is specified, then the test expects to pass the arguments
> > +# through the variables specified in drivers/net/README.rst file. If not,
> > +# fallback on parsing the script arguments for interface names.
> > +if [[ -v NETIF ]]; then
> > +	if (( NUM_NETIFS > 2)); then
> > +		echo "SKIP: NETIF defined and NUM_NETIFS is bigger than 2"
> > +		exit "$ksft_skip"
> > +	fi
> > +
> > +	check_env
> > +
> > +	# Populate the NETIF and TARGETS arrays automatically based on the
> > +	# environment variables
> > +	unset NETIFS
> > +	declare -A NETIFS
> > +
> > +	NETIFS[p1]="$NETIF"
> > +	TARGETS[$NETIF]="local:"
> > +
> > +	# Locate the name of the remote interface
> > +	if [[ -v REMOTE_V4 ]]; then
> > +		remote_netif=$(CUR_TARGET="$REMOTE_TYPE:$REMOTE_ARGS" get_ifname_by_ip "$REMOTE_V4")
> > +	else
> > +		remote_netif=$(CUR_TARGET="$REMOTE_TYPE:$REMOTE_ARGS" get_ifname_by_ip "$REMOTE_V6")
> > +	fi
> > +	if [[ ! -n "$remote_netif" ]]; then
> > +		echo "SKIP: cannot find remote interface"
> > +		exit "$ksft_skip"
> > +	fi
> 
> This should check that $NETIF != $remote_netif.

Ok.

Re: [PATCH net-next v3 3/9] selftests: net: extend lib.sh to parse drivers/net/net.config

[Petr Machata]

Ioana Ciornei <ioana.ciornei@nxp.com> writes:

> On Fri, Mar 20, 2026 at 11:19:18AM +0100, Petr Machata wrote:
>> 
>> Ioana Ciornei <ioana.ciornei@nxp.com> writes:
>> 
>> > Extend lib.sh so that it's able to parse driver/net/net.config and
>> > environment variables such as NETIF, REMOTE_TYPE, LOCAL_V4 etc described
>> > in drivers/net/README.rst.
>> >
>> > In order to make the transition towards running with a single local
>> > interface smoother for the bash networking driver tests, beside sourcing
>> > the net.config file also translate the new env variables into the old
>> > style based on the NETIFS array. Since the NETIFS array only holds the
>> > network interface names, also add a new array - TARGETS - which keeps
>> > track of the target on which a specific interfaces resides - local,
>> > netns or accesible through an ssh command.
>> >
>> > For example, a net.config which looks like below:
>> >
>> > 	NETIF=eth0
>> > 	LOCAL_V4=192.168.1.1
>> > 	REMOTE_V4=192.168.1.2
>> > 	REMOTE_TYPE=ssh
>> > 	REMOTE_ARGS=root@192.168.1.2
>> >
>> > will generate the NETIFS and TARGETS arrays with the following data.
>> >
>> > 	NETIFS[p1]="eth0"
>> > 	NETIFS[p2]="eth2"
>> >
>> > 	TARGETS[eth0]="local:"
>> > 	TARGETS[eth2]="ssh:root@192.168.1.2"
>> >
>> > The above will be true if on the remote target, the interface which has
>> > the 192.168.1.2 address is named eth2.
>> >
>> > Since the TARGETS array is indexed by the network interface name,
>> > document a new restriction README.rst which states that the remote
>> > interface cannot have the same name as the local one.
>> 
>> Isn't this going to be a somewhat common scenario though?
>
> It could be if you the remote target is another system just like the
> DUT. But I couldn't find an easy representation for the relationship
> between interfaces and their targets that also removes this restriction.

So after the review of I think v1, I sat down and hacked up a PoC of a
way to specify location of remote interfaces. I didn't really have time
to work it all out, it was largely an experiment, but it seemed to work
even in some unmodified tests. But I hit the same issue: for backward
compatibility the interface names themselves serve as identifiers, and
sometimes that's the only thing you have to look up the access method,
so they have to be unique. So I understand how this comes up.

I don't think it is OK to state this as a general requirement though,
because the Python tests do not have this problem. And with a proper
library support that is not purely a backward compatible tweak, used by
a green-field test, the support could work in Bash as well.

So -- maybe the documentation should just be different? That Bash tests
have this limitation, and bail out in lib.sh giving a reason that using
the same name here and there is currently not supported.

>> > Also keep the old way of populating the NETIFS variable based on the
>> > command line arguments. This will be invoked in case NETIF is not
>> > defined.
>> >
>> > Signed-off-by: Ioana Ciornei <ioana.ciornei@nxp.com>
>> > ---
>> > Changes in v3:
>> > - s/TARGET/CUR_TARGET
>> > - this used to be patch #2/9 in v2. Swapped the two patches so that the
>> >   run_cmd used in this patch is defined earlier, not later.
>> > Changes in v2:
>> > - patch is new
>> >
>> >  .../testing/selftests/drivers/net/README.rst  |   3 +
>> >  tools/testing/selftests/net/forwarding/lib.sh | 130 ++++++++++++++++--
>> >  2 files changed, 124 insertions(+), 9 deletions(-)
>> >
>> > diff --git a/tools/testing/selftests/drivers/net/README.rst b/tools/testing/selftests/drivers/net/README.rst
>> > index c94992acf10b..8d8d9d62e763 100644
>> > --- a/tools/testing/selftests/drivers/net/README.rst
>> > +++ b/tools/testing/selftests/drivers/net/README.rst
>> > @@ -26,6 +26,9 @@ The netdevice against which tests will be run must exist, be running
>> >  Refer to list of :ref:`Variables` later in this file to set up running
>> >  the tests against a real device.
>> >  
>> > +Also, make sure that if you are using a remote machine for traffic injection,
>> > +the local and remote interfaces have different names.
>> > +
>> >  Both modes required
>> >  ~~~~~~~~~~~~~~~~~~~
>> >  
>> > diff --git a/tools/testing/selftests/net/forwarding/lib.sh b/tools/testing/selftests/net/forwarding/lib.sh
>> > index cf40cb766c68..fb5aa56343e1 100644
>> > --- a/tools/testing/selftests/net/forwarding/lib.sh
>> > +++ b/tools/testing/selftests/net/forwarding/lib.sh
>> > @@ -3,6 +3,7 @@
>> >  
>> >  ##############################################################################
>> >  # Topology description. p1 looped back to p2, p3 to p4 and so on.
>> > +#shellcheck disable=SC2034 # SC doesn't see our uses of global variables
>> >  
>> >  declare -A NETIFS=(
>> >      [p1]=veth0
>> > @@ -17,6 +18,26 @@ declare -A NETIFS=(
>> >      [p10]=veth9
>> >  )
>> >  
>> > +# Array indexed by the network interface name keeping track of the target on
>> > +# which the interface resides. Values will be strings of the following format -
>> > +# <type>:<args>.
>> > +# TARGETS[eth0]="local:" - meaning that the eth0 interface is accessible locally
>> > +# TARGETS[eth1]="netns:foo" - eth1 is in the foo netns
>> > +# TARGETS[eth2]="ssh:root@10.0.0.2" - eth2 is accessible through running the
>> > +# 'ssh root@10.0.0.2' command.
>> > +declare -A TARGETS=(
>> 
>> This is a helper array for internal use only, not a user interface. It
>> should just be declared suitably close to where it's actually
>> initialized. It doesn't even need its own initialization, there's code
>> down there to initialize it accordingly.
>
> Ok. In v3 I added the fallback to run on the local system in case either
> TARGETS is not declared or the entry is not valid so I can safely remove
> this now. Thanks!

It's fine for what it is, I just don't want it to be an API.

>> > +    [veth0]="local:"
>> > +    [veth1]="local:"
>> > +    [veth2]="local:"
>> > +    [veth3]="local:"
>> > +    [veth4]="local:"
>> > +    [veth5]="local:"
>> > +    [veth6]="local:"
>> > +    [veth7]="local:"
>> > +    [veth8]="local:"
>> > +    [veth9]="local:"
>> > +)

>> > +	if [[ (-n "${LOCAL_V4}" && -z "${REMOTE_V4}") || \
>> > +	      (-z "${LOCAL_V4}" && -n "${REMOTE_V4}") ]]; then
>> > +		missing+=("LOCAL_V4,REMOTE_V4")
>> > +	fi
>> > +
>> > +	if [[ ${#missing[@]} -gt 0 ]]; then
>> > +		echo "SKIP: Invalid environment, missing configuration: ${missing[*]}"
>> > +		echo "Please see tools/testing/selftests/drivers/net/README.rst"
>> > +		exit "$ksft_skip"
>> > +	fi
>> > +}
>> > +
>> > +get_ifname_by_ip()
>> > +{
>> > +	local ip_addr=$1; shift
>> > +
>> > +	run_cmd ip -j addr show to "$ip_addr" | jq -r '.[].ifname'
>> > +}
>> > +
>> > +# If there is a configuration file, source it
>> > +if [[ -f $net_forwarding_dir/../../drivers/net/net.config ]]; then
>> > +	source "$net_forwarding_dir/../../drivers/net/net.config"
>> > +fi
>> 
>> This is going to impact all tests though, not just those that are meant
>> to work with this. If I have this file, it sets NETIF, and all my
>> forwarding tests (presumably) break, because NETIFS doesn't get
>> configured correctly.
>> 
>> The requirements for driver tests are different from forwarding tests.
>> In particular, interfaces do not belong to driver tests, the test can't
>> bring them up or down, reassign addresses etc., merely to use them for
>> traffic. (Which you know, I'm just stating it for context.) So I think
>> there needs to be an opt-in variable for this stuff. Like setting
>> DRIVER_TEST_CONFORMANT=yes before sourcing the library or whatever.
>
> I had this kind of approach initially but thought that it might be
> desirable to also maintain the possibility to run the driver tests as
> ./test.sh eth0 eth1 as before, so I changed it.
> I can move back to that approach.

Hmm, interesting. Maybe this could work then?

: "${DRIVER_TEST_CONFORMANT:=yes}"

Then the user can override it on the command line:

DRIVER_TEST_CONFORMANT=no ./whatever.sh swp1 swp2

Re: [PATCH net-next v3 2/9] selftests: net: add helpers for running a command on other targets

[Petr Machata]

Ioana Ciornei <ioana.ciornei@nxp.com> writes:

> On Fri, Mar 20, 2026 at 11:58:42AM +0100, Petr Machata wrote:
>> 
>> Ioana Ciornei <ioana.ciornei@nxp.com> writes:
>> 
>> > Add a couple of helpers which can be used by tests which need to run a
>> > specific bash command on a different target than the local system, be it
>> > either another netns or a remote system accessible through ssh.
>> >
>> > The run_cmd() function decides where to execute the command passed
>> > through $@ based on the env variable TARGET value while run_on() will
>> > receive the target through its first argument.
>> >
>> > Signed-off-by: Ioana Ciornei <ioana.ciornei@nxp.com>
>> > ---
>> > Changes in v3:
>> > - s/TARGET/CUR_TARGET
>> > - always fallback on running a command locally when either TARGETS is
>> >   not declared or there is no entry for a specific interface
>> > Changes in v2:
>> > - patch is new
>> >
>> >  tools/testing/selftests/net/lib.sh | 39 ++++++++++++++++++++++++++++++
>> >  1 file changed, 39 insertions(+)
>> >
>> > diff --git a/tools/testing/selftests/net/lib.sh b/tools/testing/selftests/net/lib.sh
>> > index b40694573f4c..f7c54d05758e 100644
>> > --- a/tools/testing/selftests/net/lib.sh
>> > +++ b/tools/testing/selftests/net/lib.sh
>> > @@ -28,6 +28,10 @@ EXIT_STATUS=0
>> >  # Per-test return value. Clear at the beginning of each test.
>> >  RET=0
>> >  
>> > +# If a specific command needs to be executed on another target than local, set
>> > +# this appropriately before calling run_cmd
>> > +CUR_TARGET="local:"
>> > +
>> 
>> These are new functions, they should just expose these parameters as
>> actual function parameters, not as hidden environment. If it is
>> desirable that run_cmd() can run local commands, maybe it makes sense to
>> make the current run_cmd() a helper like __run_cmd(), which takes the
>> remote-config parameter, and have run_cmd() be a wrapper:
>> 
>> run_cmd()
>> {
>> 	__run_cmd local: "$@"
>> }
>
> Why I chose to also add a function like run_cmd that makes use of an
> environment variable is to support cases such as the following.
>
> get_ifname_by_ip()
> {
> 	local ip_addr=$1; shift
>
> 	run_cmd ip -j addr show to "$ip_addr" | jq -r '.[].ifname'
> }

Oh, I see, this didn't even occur to me. run_cmd() remembering the last
used target like this is... unobvious. I can't promise you it would bite
us in the ass in the future, but stuff like this tends to bite one in
their ass. If get_ifname_by_ip() needs to know where it should run, well
then that's just the way it is.

> Helpers such as get_ifname_by_ip() (added in patch #3) that don't have
> access to an interface name so that they can directly call run_on.
>
> Do you mean that for any helpers such as the one above that need to be
> run both on the local system and the remote it's better just to add a
> new parameter to it that it's indeed the target on which it should be
> run and get rid of the run_cmd all together?

run_cmd() is fine I think. It just needs to be honest about its
requirements.

> get_ifname_by_ip()
> {
> 	local target=$1; shift
> 	local ip_addr=$1; shift
>
> 	run_on "$target" ip -j addr show to "$ip_addr" | jq -r '.[].ifname'
> }

Yep.

Re: [PATCH net-next v3 9/9] selftests: drivers: hw: add test for the ethtool standard counters

[Jakub Kicinski]

On Thu, 19 Mar 2026 18:04:10 +0200 Ioana Ciornei wrote:
>  $ ./run_kselftest.sh -t drivers/net/hw:ethtool_std_stats.sh
>  TAP version 13
>  1..1
>  # timeout set to 0
>  # selftests: drivers/net/hw: ethtool_std_stats.sh
>  # TEST: eth-ctrl-MACControlFramesTransmitted on endpmac17             [ OK ]
>  # TEST: eth-ctrl-MACControlFramesReceived on endpmac17                [ OK ]
>  # TEST: eth-mac-FrameCheckSequenceErrors on endpmac17                 [ OK ]
>  # TEST: eth-mac-AlignmentErrors on endpmac17                          [ OK ]
>  # TEST: eth-mac-FramesLostDueToIntMACXmitError on endpmac17           [ OK ]
>  # TEST: eth-mac-CarrierSenseErrors on endpmac17 (not supported)       [SKIP]

The output needs to be KTAP for HW tests. NIPA tracks them on sub-case
basis, because most devices will skip at least one case. We need to be
able to catch regressions at finer granularity than in SW tests.