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

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

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.

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