Re: [PATCH net-next v4 03/10] selftests: net: extend lib.sh to parse drivers/net/net.config

Petr Machata
2026-03-30T11:50:50Z

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.
>
> 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 v4:
> - reword the entry in README.rst to mention that the different interface
> names is only a bash restriction and the python infrastructure does
> not have the same problem.
> - only declare the TARGETS array when necessary

I guess you mean at the point where it's necessary, instead of it being
a user API. Right now TARGETS is defined always, and I think that's
better than having it only defined sometimes.

> - add a new flags - DRIVER_TEST_CONFORMANT - that needs to be set by the
> test
> - rework the check_env() function so that its logic is simpler
> - source drivers/net/net.config only if DRIVER_TEST_CONFORMANT == yes
> - check that NETIF and the remote netif have different names and abort
> test is not
>
> 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 | 4 +
> tools/testing/selftests/net/forwarding/lib.sh | 106 ++++++++++++++++--
> 2 files changed, 101 insertions(+), 9 deletions(-)
>
> diff --git a/tools/testing/selftests/drivers/net/README.rst b/tools/testing/selftests/drivers/net/README.rst
> index c94992acf10b..1897aa1583ec 100644
> --- a/tools/testing/selftests/drivers/net/README.rst
> +++ b/tools/testing/selftests/drivers/net/README.rst
> @@ -26,6 +26,10 @@ 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.
>
> +The current support for bash tests restricts the use of the same interface name
> +on the local system and the remote one and will bail if this case is
> +encountered.
> +
> Both modes required
> ~~~~~~~~~~~~~~~~~~~
>
> diff --git a/tools/testing/selftests/net/forwarding/lib.sh b/tools/testing/selftests/net/forwarding/lib.sh
> index 3009ce00c5dc..93b865681840 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

The shellcheck line should be moved elsewhere. The comment is related to
NETIFS, and it's weird there's a shellcheck declaration between a
comment and the thing it's commenting.

> declare -A NETIFS=(
> [p1]=veth0
> @@ -85,6 +86,9 @@ declare -A NETIFS=(
> # e.g. a low-power board.
> : "${KSFT_MACHINE_SLOW:=no}"
>
> +# Whether the test is conforming to the requirements and usage described in
> +# drivers/net/README.rst.
> +: "${DRIVER_TEST_CONFORMANT:=no}"

Not sure this makes sense as user API either honestly. Given the
differences between requirements of the two test types I can't imagine a
non-conformant test would be runnable like that. The actual line is OK,
but it should be moved probably above the if check, so as not to
indicate it's usable as a user API.

> ##############################################################################
> # Find netifs by test-specified driver name
>
> @@ -340,17 +344,101 @@ fi
> ##############################################################################
> # Command line options handling
>
> -count=0
> +check_env() {
> + 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"
> + echo "Please see tools/testing/selftests/drivers/net/README.rst"
> + exit "$ksft_skip"
> + fi
>
> -while [[ $# -gt 0 ]]; do
> - if [[ "$count" -eq "0" ]]; then
> - unset NETIFS
> - declare -A NETIFS
> + if [[ -z "$REMOTE_TYPE" ]]; then
> + echo "SKIP: Invalid environment, missing REMOTE_TYPE"
> + exit "$ksft_skip"
> fi
> - count=$((count + 1))
> - NETIFS[p$count]="$1"
> - shift
> -done
> +
> + if [[ -z "$REMOTE_ARGS" ]]; then
> + echo "SKIP: Invalid environment, missing REMOTE_ARGS"
> + exit "$ksft_skip"
> + fi
> +}
> +
> +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'
> +}
> +
> +# Based on DRIVER_TEST_CONFORMANT, decide if to source drivers/net/net.config
> +# or not. In the "yes" case, 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 [ "${DRIVER_TEST_CONFORMANT}" = "yes" ]; then
> + if [[ -f $net_forwarding_dir/../../drivers/net/net.config ]]; then
> + source "$net_forwarding_dir/../../drivers/net/net.config"
> + fi
> +
> + if (( NUM_NETIFS > 2)); then
> + echo "SKIP: DRIVER_TEST_CONFORMANT=yes and NUM_NETIFS is bigger than 2"
> + exit "$ksft_skip"
> + fi
> +
> + check_env
> +
> + # Populate the NETIFS and TARGETS arrays automatically based on the
> + # environment variables. The TARGETS array is 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.
> +
> + unset NETIFS
> + declare -A NETIFS
> + declare -A TARGETS
> +
> + NETIFS[p1]="$NETIF"
> + TARGETS[$NETIF]="local:"
> +
> + # Locate the name of the remote interface
> + remote_target="$REMOTE_TYPE:$REMOTE_ARGS"
> + if [[ -v REMOTE_V4 ]]; then
> + remote_netif=$(get_ifname_by_ip "$remote_target" "$REMOTE_V4")
> + else
> + remote_netif=$(get_ifname_by_ip "$remote_target" "$REMOTE_V6")
> + fi
> + if [[ ! -n "$remote_netif" ]]; then
> + echo "SKIP: cannot find remote interface"
> + exit "$ksft_skip"
> + fi
> +
> + if [[ "$NETIF" == "$remote_netif" ]]; then
> + echo "SKIP: local and remote interfaces cannot have the same name"
> + exit "$ksft_skip"
> + fi
> +
> + NETIFS[p2]="$remote_netif"
> + TARGETS[$remote_netif]="$REMOTE_TYPE:$REMOTE_ARGS"
> +else
> + count=0

This leg is missing declare -A TARGETS.

Since both legs need to declare it, why not move it up above the if?

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