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

