Ioana Ciornei <[email protected]> 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
>       [email protected]
>
> will generate the NETIFS and TARGETS arrays with the following data.
>
>       NETIFS[p1]="eth0"
>       NETIFS[p2]="eth2"
>
>       TARGETS[eth0]="local:"
>       TARGETS[eth2]="ssh:[email protected]"
>
> 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 <[email protected]>
> ---
> 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:[email protected]" - eth2 is accessible through
> +     # running the 'ssh [email protected]' 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


Reply via email to