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

[Petr Machata <petrm@nvidia.com>]

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. Keep the old way
> of populating the NETIFS variable based on the command line arguments.
> This will be invoked in case DRIVER_TEST_CONFORMANT = "no".
>
> Also 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_on() function is passed through $1 the target on which the
> command should be executed while run_on() is passed the name of the
> interface that is then used to retrieve the target from the TARGETS
> array.
>
> Also add a stub run_on() function in net/lib.sh so that users of the
> net/lib.sh are going through the stub only since neither NETIFS nor
> TARGETS are valid in that circumstance.
>
> Signed-off-by: Ioana Ciornei <ioana.ciornei@nxp.com>

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

> +else
> +	count=0
> +
> +	while [[ $# -gt 0 ]]; do
> +		if [[ "$count" -eq "0" ]]; then
> +			unset NETIFS
> +			declare -A NETIFS
> +		fi
> +		count=$((count + 1))

This is coming from the original as well, but I find the piece of code
really unobvious. I'm not going to block the patchset over this, but
this would IMHO be a better way to express the intent:

	# Prime NETIFS from the command line, but retain if none given.
	if [[ $# -gt 0 ]]; then
		unset NETIFS
		declare -A NETIFS

		while [[ $# -gt 0 ]]; do
			((count++))
			etc
		done
	fi

If there is a v6, please roll it in.

> +		NETIFS[p$count]="$1"
> +		TARGETS[$1]="local:"
> +		shift
> +	done
> +fi