2023-07-17 21:19:53

by Mirsad Todorovac

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

Hi,

There seems to be a problem with net/forwarding line of 6.5-rc2 kselftests,
vanilla Torvalds tree, commit fdf0eaf11452, on Ubuntu 22.04 LTS Jammy Jellyfish.

(Confirmed on Ubuntu 22.10 Kinetic Kudu.)

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?

However, I can't seem to see what is the expected result.

The problem was confirmed in the backlogs of 6.5-rc1 and 6.4 kselftests.

It is possible that I'm doing something terribly wrong, but it is basically
the default kselftest suite on a rather minimal Ubuntu.

Please find attached the bash output from `set -x`.

Version of iproute2 is:
ii iproute2 5.15.0-1ubuntu2 amd64 networking and traffic control tools

Hope this helps.

Best regards,
Mirsad Todorovac


Attachments:
6.5-rc2-net-forwarding.out (2.61 kB)
config-6.5.0-rc2-debug-gfdf0eaf11452.xz (56.28 kB)
Download all attachments

2023-07-18 07:43:01

by Ido Schimmel

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

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

2023-07-18 11:15:32

by Petr Machata

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


Ido Schimmel <[email protected]> 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


2023-07-18 18:23:46

by Mirsad Todorovac

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

On 7/18/23 09:22, Ido Schimmel wrote:
> 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

This patch appears to work for the first testing script

root@defiant:# ./bridge_igmp.sh
TEST: IGMPv2 report 239.10.10.10 [ OK ]
TEST: IGMPv2 leave 239.10.10.10 [ OK ]
TEST: IGMPv3 report 239.10.10.10 is_include [ OK ]
TEST: IGMPv3 report 239.10.10.10 include -> allow [ OK ]
TEST: IGMPv3 report 239.10.10.10 include -> is_include [ OK ]
TEST: IGMPv3 report 239.10.10.10 include -> is_exclude [ OK ]
TEST: IGMPv3 report 239.10.10.10 include -> to_exclude [ OK ]
TEST: IGMPv3 report 239.10.10.10 exclude -> allow [ OK ]
TEST: IGMPv3 report 239.10.10.10 exclude -> is_include [ OK ]
TEST: IGMPv3 report 239.10.10.10 exclude -> is_exclude [ OK ]
TEST: IGMPv3 report 239.10.10.10 exclude -> to_exclude [ OK ]
TEST: IGMPv3 report 239.10.10.10 include -> block [ OK ]
TEST: IGMPv3 report 239.10.10.10 exclude -> block [ OK ]
TEST: IGMPv3 group 239.10.10.10 exclude timeout [ OK ]
TEST: IGMPv3 S,G port entry automatic add to a *,G port [ OK ]
root@defiant:#

However, I suggest setting tools/testing/selftest/net/forwarding/settings:timeout=150 at least,
because default 45 is premature on my box and it leaves the networking system in an undefined
state upon exit.

Best regards,
Mirsad

2023-07-18 18:40:38

by Mirsad Todorovac

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

On 7/18/23 12:35, Petr Machata wrote:
>
> Ido Schimmel <[email protected]> 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

2023-07-18 18:56:53

by Mirsad Todorovac

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

On 7/18/23 20:19, Mirsad Todorovac wrote:
> On 7/18/23 09:22, Ido Schimmel wrote:
>> 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
>
> This patch appears to work for the first testing script
>
> root@defiant:# ./bridge_igmp.sh
> TEST: IGMPv2 report 239.10.10.10                                    [ OK ]
> TEST: IGMPv2 leave 239.10.10.10                                     [ OK ]
> TEST: IGMPv3 report 239.10.10.10 is_include                         [ OK ]
> TEST: IGMPv3 report 239.10.10.10 include -> allow                   [ OK ]
> TEST: IGMPv3 report 239.10.10.10 include -> is_include              [ OK ]
> TEST: IGMPv3 report 239.10.10.10 include -> is_exclude              [ OK ]
> TEST: IGMPv3 report 239.10.10.10 include -> to_exclude              [ OK ]
> TEST: IGMPv3 report 239.10.10.10 exclude -> allow                   [ OK ]
> TEST: IGMPv3 report 239.10.10.10 exclude -> is_include              [ OK ]
> TEST: IGMPv3 report 239.10.10.10 exclude -> is_exclude              [ OK ]
> TEST: IGMPv3 report 239.10.10.10 exclude -> to_exclude              [ OK ]
> TEST: IGMPv3 report 239.10.10.10 include -> block                   [ OK ]
> TEST: IGMPv3 report 239.10.10.10 exclude -> block                   [ OK ]
> TEST: IGMPv3 group 239.10.10.10 exclude timeout                     [ OK ]
> TEST: IGMPv3 S,G port entry automatic add to a *,G port             [ OK ]
> root@defiant:#
>
> However, I suggest setting tools/testing/selftest/net/forwarding/settings:timeout=150 at least,
> because default 45 is premature on my box and it leaves the networking system in an undefined
> state upon exit.

There is also a gotcha here: you do not delete all veths:

root@defiant:# ip link show
1: lo: <LOOPBACK,UP,LOWER_UP> mtu 65536 qdisc noqueue state UNKNOWN mode DEFAULT group default qlen 1000
link/loopback 00:00:00:00:00:00 brd 00:00:00:00:00:00
2: enp16s0: <BROADCAST,MULTICAST,UP,LOWER_UP> mtu 1500 qdisc fq_codel state UP mode DEFAULT group default qlen 1000
link/ether 9c:6b:00:01:fb:80 brd ff:ff:ff:ff:ff:ff
root@defiant:# ./bridge_igmp.sh
TEST: IGMPv2 report 239.10.10.10 [ OK ]
TEST: IGMPv2 leave 239.10.10.10 [ OK ]
TEST: IGMPv3 report 239.10.10.10 is_include [ OK ]
TEST: IGMPv3 report 239.10.10.10 include -> allow [ OK ]
TEST: IGMPv3 report 239.10.10.10 include -> is_include [ OK ]
TEST: IGMPv3 report 239.10.10.10 include -> is_exclude [ OK ]
TEST: IGMPv3 report 239.10.10.10 include -> to_exclude [ OK ]
TEST: IGMPv3 report 239.10.10.10 exclude -> allow [ OK ]
TEST: IGMPv3 report 239.10.10.10 exclude -> is_include [ OK ]
TEST: IGMPv3 report 239.10.10.10 exclude -> is_exclude [ OK ]
TEST: IGMPv3 report 239.10.10.10 exclude -> to_exclude [ OK ]
TEST: IGMPv3 report 239.10.10.10 include -> block [ OK ]
TEST: IGMPv3 report 239.10.10.10 exclude -> block [ OK ]
TEST: IGMPv3 group 239.10.10.10 exclude timeout [ OK ]
TEST: IGMPv3 S,G port entry automatic add to a *,G port [ OK ]
root@defiant:# ip link show
1: lo: <LOOPBACK,UP,LOWER_UP> mtu 65536 qdisc noqueue state UNKNOWN mode DEFAULT group default qlen 1000
link/loopback 00:00:00:00:00:00 brd 00:00:00:00:00:00
2: enp16s0: <BROADCAST,MULTICAST,UP,LOWER_UP> mtu 1500 qdisc fq_codel state UP mode DEFAULT group default qlen 1000
link/ether 9c:6b:00:01:fb:80 brd ff:ff:ff:ff:ff:ff
3: veth1@veth0: <BROADCAST,MULTICAST,M-DOWN> mtu 1500 qdisc noqueue state DOWN mode DEFAULT group default qlen 1000
link/ether b6:46:e6:4c:e4:00 brd ff:ff:ff:ff:ff:ff
4: veth0@veth1: <BROADCAST,MULTICAST,M-DOWN> mtu 1500 qdisc noqueue state DOWN mode DEFAULT group default qlen 1000
link/ether 2e:ff:7f:8a:6b:d4 brd ff:ff:ff:ff:ff:ff
5: veth3@veth2: <BROADCAST,MULTICAST,M-DOWN> mtu 1500 qdisc noqueue state DOWN mode DEFAULT group default qlen 1000
link/ether ba:33:37:81:dc:5b brd ff:ff:ff:ff:ff:ff
6: veth2@veth3: <BROADCAST,MULTICAST,M-DOWN> mtu 1500 qdisc noqueue state DOWN mode DEFAULT group default qlen 1000
link/ether f2:fd:0a:9b:94:17 brd ff:ff:ff:ff:ff:ff
root@defiant:#

Also, I ran into problems with some other tests, but for documentation sake
I will address the issue in a separate thread.

Best regards,
Mirsad Todorovac


2023-07-19 13:06:32

by Ido Schimmel

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

On Tue, Jul 18, 2023 at 08:39:33PM +0200, Mirsad Todorovac wrote:
> There is also a gotcha here: you do not delete all veths:
>
> root@defiant:# ip link show
> 1: lo: <LOOPBACK,UP,LOWER_UP> mtu 65536 qdisc noqueue state UNKNOWN mode DEFAULT group default qlen 1000
> link/loopback 00:00:00:00:00:00 brd 00:00:00:00:00:00
> 2: enp16s0: <BROADCAST,MULTICAST,UP,LOWER_UP> mtu 1500 qdisc fq_codel state UP mode DEFAULT group default qlen 1000
> link/ether 9c:6b:00:01:fb:80 brd ff:ff:ff:ff:ff:ff
> root@defiant:# ./bridge_igmp.sh

[...]

> root@defiant:# ip link show
> 1: lo: <LOOPBACK,UP,LOWER_UP> mtu 65536 qdisc noqueue state UNKNOWN mode DEFAULT group default qlen 1000
> link/loopback 00:00:00:00:00:00 brd 00:00:00:00:00:00
> 2: enp16s0: <BROADCAST,MULTICAST,UP,LOWER_UP> mtu 1500 qdisc fq_codel state UP mode DEFAULT group default qlen 1000
> link/ether 9c:6b:00:01:fb:80 brd ff:ff:ff:ff:ff:ff
> 3: veth1@veth0: <BROADCAST,MULTICAST,M-DOWN> mtu 1500 qdisc noqueue state DOWN mode DEFAULT group default qlen 1000
> link/ether b6:46:e6:4c:e4:00 brd ff:ff:ff:ff:ff:ff
> 4: veth0@veth1: <BROADCAST,MULTICAST,M-DOWN> mtu 1500 qdisc noqueue state DOWN mode DEFAULT group default qlen 1000
> link/ether 2e:ff:7f:8a:6b:d4 brd ff:ff:ff:ff:ff:ff
> 5: veth3@veth2: <BROADCAST,MULTICAST,M-DOWN> mtu 1500 qdisc noqueue state DOWN mode DEFAULT group default qlen 1000
> link/ether ba:33:37:81:dc:5b brd ff:ff:ff:ff:ff:ff
> 6: veth2@veth3: <BROADCAST,MULTICAST,M-DOWN> mtu 1500 qdisc noqueue state DOWN mode DEFAULT group default qlen 1000
> link/ether f2:fd:0a:9b:94:17 brd ff:ff:ff:ff:ff:ff
> root@defiant:#

These tests can be run with veth pairs or with loop backed physical
ports. We can't delete the latter and I don't see a clean way to delete
the veth pairs.

The following patch [1] changes the default to not create interfaces so
that by default these tests will be skipped [2]. Those who care about
running the tests can create a forwarding.config file (using
forwarding.config.sample as an example) and either create the veth pairs
themselves or opt-in for the interfaces to be created automatically
(setting NETIF_CREATE=yes), but not deleted.

[1]
diff --git a/tools/testing/selftests/net/forwarding/forwarding.config.sample b/tools/testing/selftests/net/forwarding/forwarding.config.sample
index 4a546509de90..b72f08dfd491 100644
--- a/tools/testing/selftests/net/forwarding/forwarding.config.sample
+++ b/tools/testing/selftests/net/forwarding/forwarding.config.sample
@@ -36,8 +36,9 @@ PAUSE_ON_FAIL=no
PAUSE_ON_CLEANUP=no
# Type of network interface to create
NETIF_TYPE=veth
-# Whether to create virtual interfaces (veth) or not
-NETIF_CREATE=yes
+# Whether to create virtual interfaces (veth) or not. Created interfaces are
+# not automatically deleted
+NETIF_CREATE=no
# Timeout (in seconds) before ping exits regardless of how many packets have
# been sent or received
PING_TIMEOUT=5
diff --git a/tools/testing/selftests/net/forwarding/lib.sh b/tools/testing/selftests/net/forwarding/lib.sh
index 9ddb68dd6a08..1b1bc634c63e 100755
--- a/tools/testing/selftests/net/forwarding/lib.sh
+++ b/tools/testing/selftests/net/forwarding/lib.sh
@@ -17,7 +17,7 @@ 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}
+NETIF_CREATE=${NETIF_CREATE:=no}
MCD=${MCD:=smcrouted}
MC_CLI=${MC_CLI:=smcroutectl}
PING_COUNT=${PING_COUNT:=10}

[2]
# ./bridge_igmp.sh
SKIP: could not find all required interfaces

2023-07-21 20:59:00

by Mirsad Todorovac

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

On 7/19/23 14:59, Ido Schimmel wrote:
> On Tue, Jul 18, 2023 at 08:39:33PM +0200, Mirsad Todorovac wrote:
>> There is also a gotcha here: you do not delete all veths:
>>
>> root@defiant:# ip link show
>> 1: lo: <LOOPBACK,UP,LOWER_UP> mtu 65536 qdisc noqueue state UNKNOWN mode DEFAULT group default qlen 1000
>> link/loopback 00:00:00:00:00:00 brd 00:00:00:00:00:00
>> 2: enp16s0: <BROADCAST,MULTICAST,UP,LOWER_UP> mtu 1500 qdisc fq_codel state UP mode DEFAULT group default qlen 1000
>> link/ether 9c:6b:00:01:fb:80 brd ff:ff:ff:ff:ff:ff
>> root@defiant:# ./bridge_igmp.sh
>
> [...]
>
>> root@defiant:# ip link show
>> 1: lo: <LOOPBACK,UP,LOWER_UP> mtu 65536 qdisc noqueue state UNKNOWN mode DEFAULT group default qlen 1000
>> link/loopback 00:00:00:00:00:00 brd 00:00:00:00:00:00
>> 2: enp16s0: <BROADCAST,MULTICAST,UP,LOWER_UP> mtu 1500 qdisc fq_codel state UP mode DEFAULT group default qlen 1000
>> link/ether 9c:6b:00:01:fb:80 brd ff:ff:ff:ff:ff:ff
>> 3: veth1@veth0: <BROADCAST,MULTICAST,M-DOWN> mtu 1500 qdisc noqueue state DOWN mode DEFAULT group default qlen 1000
>> link/ether b6:46:e6:4c:e4:00 brd ff:ff:ff:ff:ff:ff
>> 4: veth0@veth1: <BROADCAST,MULTICAST,M-DOWN> mtu 1500 qdisc noqueue state DOWN mode DEFAULT group default qlen 1000
>> link/ether 2e:ff:7f:8a:6b:d4 brd ff:ff:ff:ff:ff:ff
>> 5: veth3@veth2: <BROADCAST,MULTICAST,M-DOWN> mtu 1500 qdisc noqueue state DOWN mode DEFAULT group default qlen 1000
>> link/ether ba:33:37:81:dc:5b brd ff:ff:ff:ff:ff:ff
>> 6: veth2@veth3: <BROADCAST,MULTICAST,M-DOWN> mtu 1500 qdisc noqueue state DOWN mode DEFAULT group default qlen 1000
>> link/ether f2:fd:0a:9b:94:17 brd ff:ff:ff:ff:ff:ff
>> root@defiant:#
>
> These tests can be run with veth pairs or with loop backed physical
> ports. We can't delete the latter and I don't see a clean way to delete
> the veth pairs.
>
> The following patch [1] changes the default to not create interfaces so
> that by default these tests will be skipped [2]. Those who care about
> running the tests can create a forwarding.config file (using
> forwarding.config.sample as an example) and either create the veth pairs
> themselves or opt-in for the interfaces to be created automatically
> (setting NETIF_CREATE=yes), but not deleted.
>
> [1]
> diff --git a/tools/testing/selftests/net/forwarding/forwarding.config.sample b/tools/testing/selftests/net/forwarding/forwarding.config.sample
> index 4a546509de90..b72f08dfd491 100644
> --- a/tools/testing/selftests/net/forwarding/forwarding.config.sample
> +++ b/tools/testing/selftests/net/forwarding/forwarding.config.sample
> @@ -36,8 +36,9 @@ PAUSE_ON_FAIL=no
> PAUSE_ON_CLEANUP=no
> # Type of network interface to create
> NETIF_TYPE=veth
> -# Whether to create virtual interfaces (veth) or not
> -NETIF_CREATE=yes
> +# Whether to create virtual interfaces (veth) or not. Created interfaces are
> +# not automatically deleted
> +NETIF_CREATE=no
> # Timeout (in seconds) before ping exits regardless of how many packets have
> # been sent or received
> PING_TIMEOUT=5
> diff --git a/tools/testing/selftests/net/forwarding/lib.sh b/tools/testing/selftests/net/forwarding/lib.sh
> index 9ddb68dd6a08..1b1bc634c63e 100755
> --- a/tools/testing/selftests/net/forwarding/lib.sh
> +++ b/tools/testing/selftests/net/forwarding/lib.sh
> @@ -17,7 +17,7 @@ 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}
> +NETIF_CREATE=${NETIF_CREATE:=no}
> MCD=${MCD:=smcrouted}
> MC_CLI=${MC_CLI:=smcroutectl}
> PING_COUNT=${PING_COUNT:=10}
>
> [2]
> # ./bridge_igmp.sh
> SKIP: could not find all required interfaces

Hi, Ido,

Please would you consider making a formal patch, so the vanilla 'make kselftest'
run would have some sensible defaults?

I am really interested in the network stack working right, though I really do
not have much opportunities testing like high speed links and bonded eth adapters :-(

Maybe that'll come :-/

Kind regards,
Mirsad