Re: [PROBLEM] selftests: net/forwarding/*.sh: 'Command line is not complete. Try option "help"'

[Mirsad Todorovac]

On 7/18/23 12:35, Petr Machata wrote:
> Ido Schimmel <idosch@xxxxxxxxxx> writes:
>
> > On Mon, Jul 17, 2023 at 10:51:04PM +0200, Mirsad Todorovac wrote:
> > > Tests fail with error message:
> > >
> > > Command line is not complete. Try option "help"
> > > Failed to create netif
> > >
> > > The script
> > >
> > > # tools/testing/seltests/net/forwarding/bridge_igmp.sh
> > >
> > > bash `set -x` ends with an error:
> > >
> > > ++ create_netif_veth
> > > ++ local i
> > > ++ (( i = 1 ))
> > > ++ (( i <= NUM_NETIFS ))
> > > ++ local j=2
> > > ++ ip link show dev
> > > ++ [[ 255 -ne 0 ]]
> > > ++ ip link add type veth peer name
> > > Command line is not complete. Try option "help"
> > > ++ [[ 255 -ne 0 ]]
> > > ++ echo 'Failed to create netif'
> > > Failed to create netif
> > > ++ exit 1
> > >
> > > The problem seems to be linked with this piece of code of "lib.sh":
> > >
> > > create_netif_veth()
> > > {
> > >          local i
> > >
> > >          for ((i = 1; i <= NUM_NETIFS; ++i)); do
> > >                  local j=$((i+1))
> > >
> > >                  ip link show dev ${NETIFS[p$i]} &> /dev/null
> > >                  if [[ $? -ne 0 ]]; then
> > >                          ip link add ${NETIFS[p$i]} type veth \
> > >                                  peer name ${NETIFS[p$j]}
> > >                          if [[ $? -ne 0 ]]; then
> > >                                  echo "Failed to create netif"
> > >                                  exit 1
> > >                          fi
> > >                  fi
> > >                  i=$j
> > >          done
> > > }
> > >
> > > Somehow, ${NETIFS[p$i]} is evaluated to an empty string?
> >
> > You need to provide a configuration file in
> > tools/testing/selftests/net/forwarding/forwarding.config. See
> > tools/testing/selftests/net/forwarding/forwarding.config.sample for
> > example.
> >
> > Another option is to provide the interfaces on the command line.
> >
> > ./bridge_igmp.sh veth0 veth1 veth2 veth3
> >
> > If no configuration file is present, we can try to assume that the
> > tests are meant to be run with veth pairs and not with physical
> > loopbacks. Something like:
> >
> > diff --git a/tools/testing/selftests/net/forwarding/lib.sh b/tools/testing/selftests/net/forwarding/lib.sh
> > index 71f7c0c49677..5b0183013017 100755
> > --- a/tools/testing/selftests/net/forwarding/lib.sh
> > +++ b/tools/testing/selftests/net/forwarding/lib.sh
> > @@ -16,8 +16,6 @@ TEAMD=${TEAMD:=teamd}
> >   WAIT_TIME=${WAIT_TIME:=5}
> >   PAUSE_ON_FAIL=${PAUSE_ON_FAIL:=no}
> >   PAUSE_ON_CLEANUP=${PAUSE_ON_CLEANUP:=no}
> > -NETIF_TYPE=${NETIF_TYPE:=veth}
> > -NETIF_CREATE=${NETIF_CREATE:=yes}
> >   MCD=${MCD:=smcrouted}
> >   MC_CLI=${MC_CLI:=smcroutectl}
> >   PING_COUNT=${PING_COUNT:=10}
> > @@ -30,6 +28,20 @@ REQUIRE_MZ=${REQUIRE_MZ:=yes}
> >   REQUIRE_MTOOLS=${REQUIRE_MTOOLS:=no}
> >   STABLE_MAC_ADDRS=${STABLE_MAC_ADDRS:=no}
> >   TCPDUMP_EXTRA_FLAGS=${TCPDUMP_EXTRA_FLAGS:=}
> > +NETIF_TYPE=${NETIF_TYPE:=veth}
> > +NETIF_CREATE=${NETIF_CREATE:=yes}
> > +declare -A NETIFS=(
> > +       [p1]=veth0
> > +       [p2]=veth1
> > +       [p3]=veth2
> > +       [p4]=veth3
> > +       [p5]=veth4
> > +       [p6]=veth5
> > +       [p7]=veth6
> > +       [p8]=veth7
> > +       [p9]=veth8
> > +       [p10]=veth9
> > +)
> >   
> >   relative_path="${BASH_SOURCE%/*}"
> >   if [[ "$relative_path" == "${BASH_SOURCE}" ]]; then
>
> Or maybe this so that we get the exactly right number of interfaces?
>
> diff --git a/tools/testing/selftests/net/forwarding/lib.sh b/tools/testing/selftests/net/forwarding/lib.sh
> index 8491c97475ab..4fefdf9716dc 100755
> --- a/tools/testing/selftests/net/forwarding/lib.sh
> +++ b/tools/testing/selftests/net/forwarding/lib.sh
> @@ -36,6 +36,16 @@ if [[ "$relative_path" == "${BASH_SOURCE}" ]]; then
>   	relative_path="."
>   fi
>   
> +if [[ ! -v NUM_NETIFS ]]; then
> +	echo "SKIP: importer does not define \"NUM_NETIFS\""
> +	exit $ksft_skip
> +fi
> +
> +declare -A NETIFS
> +for i in $(seq $NUM_NETIFS); do
> +	NETIFS[p$i]=veth$i
> +done
> +
>   if [[ -f $relative_path/forwarding.config ]]; then
>   	source "$relative_path/forwarding.config"
>   fi
> @@ -195,11 +205,6 @@ if [[ "$REQUIRE_MTOOLS" = "yes" ]]; then
>   	require_command mreceive
>   fi
>   
> -if [[ ! -v NUM_NETIFS ]]; then
> -	echo "SKIP: importer does not define \"NUM_NETIFS\""
> -	exit $ksft_skip
> -fi
> -
>   ##############################################################################
>   # Command line options handling
>   

This leaves the user with the output:

root@defiant:# ./bridge_igmp.sh
SKIP: could not find all required interfaces
root@defiant:#

Arguably it might be prudent to offer some sensible defaults, as a novice developer
running all selftests might not have the net stack insight required to modify those,
but be lucky instead to get away with the `make kselftest` run ... :-)

Best regards,
Mirsad Todorovac