| Message ID | 20240412151314.3365034-4-jiri@resnulli.us (mailing list archive) |
|---|---|
| State | Superseded |
| Delegated to: | Netdev Maintainers |
| Headers | show |
| Series | selftests: virtio_net: introduce initial testing infrastructure | expand |
On 2024-04-12 17:13 +0200, Jiri Pirko wrote: > From: Jiri Pirko <jiri@nvidia.com> > > Allow driver tests to work without specifying the netdevice names. > Introduce a possibility to search for available netdevices according to > set driver name. Allow test to specify the name by setting > NETIF_FIND_DRIVER variable. > > Note that user overrides this either by passing netdevice names on the > command line or by declaring NETIFS array in custom forwarding.config > configuration file. > > Signed-off-by: Jiri Pirko <jiri@nvidia.com> > --- > tools/testing/selftests/net/forwarding/lib.sh | 39 +++++++++++++++++++ > 1 file changed, 39 insertions(+) > > diff --git a/tools/testing/selftests/net/forwarding/lib.sh b/tools/testing/selftests/net/forwarding/lib.sh > index 6f6a0f13465f..06633518b3aa 100644 > --- a/tools/testing/selftests/net/forwarding/lib.sh > +++ b/tools/testing/selftests/net/forwarding/lib.sh > @@ -55,6 +55,9 @@ declare -A NETIFS=( > : "${NETIF_CREATE:=yes}" > : "${NETIF_TYPE:=veth}" > > +# Whether to find netdevice according to the specified driver. > +: "${NETIF_FIND_DRIVER:=}" > + This section of the file sets default values for variables that can be set by users in forwarding.config. NETIF_FIND_DRIVER is more like NUM_NETIFS, it is set by tests, so I don't think it should be listed there. > # Constants for ping tests: > # How many packets should be sent. > : "${PING_COUNT:=10}" > @@ -94,6 +97,42 @@ if [[ ! -v NUM_NETIFS ]]; then > exit $ksft_skip > fi > > +############################################################################## > +# Find netifs by test-specified driver name > + > +driver_name_get() > +{ > + local dev=$1; shift > + local driver_path="/sys/class/net/$dev/device/driver" > + > + if [ ! -L $driver_path ]; then > + echo "" > + else > + basename `realpath $driver_path` > + fi > +} > + > +find_netif() > +{ > + local ifnames=`ip -j -p link show | jq -e -r ".[].ifname"` -p and -e can be omitted
Fri, Apr 12, 2024 at 10:38:30PM CEST, benjamin.poirier@gmail.com wrote: >On 2024-04-12 17:13 +0200, Jiri Pirko wrote: >> From: Jiri Pirko <jiri@nvidia.com> >> >> Allow driver tests to work without specifying the netdevice names. >> Introduce a possibility to search for available netdevices according to >> set driver name. Allow test to specify the name by setting >> NETIF_FIND_DRIVER variable. >> >> Note that user overrides this either by passing netdevice names on the >> command line or by declaring NETIFS array in custom forwarding.config >> configuration file. >> >> Signed-off-by: Jiri Pirko <jiri@nvidia.com> >> --- >> tools/testing/selftests/net/forwarding/lib.sh | 39 +++++++++++++++++++ >> 1 file changed, 39 insertions(+) >> >> diff --git a/tools/testing/selftests/net/forwarding/lib.sh b/tools/testing/selftests/net/forwarding/lib.sh >> index 6f6a0f13465f..06633518b3aa 100644 >> --- a/tools/testing/selftests/net/forwarding/lib.sh >> +++ b/tools/testing/selftests/net/forwarding/lib.sh >> @@ -55,6 +55,9 @@ declare -A NETIFS=( >> : "${NETIF_CREATE:=yes}" >> : "${NETIF_TYPE:=veth}" >> >> +# Whether to find netdevice according to the specified driver. >> +: "${NETIF_FIND_DRIVER:=}" >> + > >This section of the file sets default values for variables that can be >set by users in forwarding.config. NETIF_FIND_DRIVER is more like >NUM_NETIFS, it is set by tests, so I don't think it should be listed >there. Well, currently there is a mixture of config variables and test definitions/requirements. For example REQUIRE_JQ, REQUIRE_MZ, REQUIRE_MTOOLS are not forwarding.config configurable (they are, they should not be ;)) Where do you suggest to move NETIF_FIND_DRIVER? > >> # Constants for ping tests: >> # How many packets should be sent. >> : "${PING_COUNT:=10}" >> @@ -94,6 +97,42 @@ if [[ ! -v NUM_NETIFS ]]; then >> exit $ksft_skip >> fi >> >> +############################################################################## >> +# Find netifs by test-specified driver name >> + >> +driver_name_get() >> +{ >> + local dev=$1; shift >> + local driver_path="/sys/class/net/$dev/device/driver" >> + >> + if [ ! -L $driver_path ]; then >> + echo "" >> + else >> + basename `realpath $driver_path` >> + fi >> +} >> + >> +find_netif() >> +{ >> + local ifnames=`ip -j -p link show | jq -e -r ".[].ifname"` > >-p and -e can be omitted True. Will remove them.
On 2024-04-13 15:27 +0200, Jiri Pirko wrote: > Fri, Apr 12, 2024 at 10:38:30PM CEST, benjamin.poirier@gmail.com wrote: > >On 2024-04-12 17:13 +0200, Jiri Pirko wrote: > >> From: Jiri Pirko <jiri@nvidia.com> > >> > >> Allow driver tests to work without specifying the netdevice names. > >> Introduce a possibility to search for available netdevices according to > >> set driver name. Allow test to specify the name by setting > >> NETIF_FIND_DRIVER variable. > >> > >> Note that user overrides this either by passing netdevice names on the > >> command line or by declaring NETIFS array in custom forwarding.config > >> configuration file. > >> > >> Signed-off-by: Jiri Pirko <jiri@nvidia.com> > >> --- > >> tools/testing/selftests/net/forwarding/lib.sh | 39 +++++++++++++++++++ > >> 1 file changed, 39 insertions(+) > >> > >> diff --git a/tools/testing/selftests/net/forwarding/lib.sh b/tools/testing/selftests/net/forwarding/lib.sh > >> index 6f6a0f13465f..06633518b3aa 100644 > >> --- a/tools/testing/selftests/net/forwarding/lib.sh > >> +++ b/tools/testing/selftests/net/forwarding/lib.sh > >> @@ -55,6 +55,9 @@ declare -A NETIFS=( > >> : "${NETIF_CREATE:=yes}" > >> : "${NETIF_TYPE:=veth}" > >> > >> +# Whether to find netdevice according to the specified driver. > >> +: "${NETIF_FIND_DRIVER:=}" > >> + > > > >This section of the file sets default values for variables that can be > >set by users in forwarding.config. NETIF_FIND_DRIVER is more like > >NUM_NETIFS, it is set by tests, so I don't think it should be listed > >there. > > Well, currently there is a mixture of config variables and test > definitions/requirements. For example REQUIRE_JQ, REQUIRE_MZ, REQUIRE_MTOOLS > are not forwarding.config configurable (they are, they should not be ;)) Yes, that's true. If you prefer to leave that statement there, go ahead. > Where do you suggest to move NETIF_FIND_DRIVER? I would make NETIF_FIND_DRIVER like NUM_NETIFS, ie. there's no statement setting a default value for it. And I would move the comment describing its purpose above this new part: > + > +if [[ ! -z $NETIF_FIND_DRIVER ]]; then > + unset NETIFS > + declare -A NETIFS > + find_netif > +fi > + BTW, '! -z' can be removed from that test. It's equivalent to: if [[ $NETIF_FIND_DRIVER ]]; then
Sun, Apr 14, 2024 at 09:32:46PM CEST, benjamin.poirier@gmail.com wrote: >On 2024-04-13 15:27 +0200, Jiri Pirko wrote: >> Fri, Apr 12, 2024 at 10:38:30PM CEST, benjamin.poirier@gmail.com wrote: >> >On 2024-04-12 17:13 +0200, Jiri Pirko wrote: >> >> From: Jiri Pirko <jiri@nvidia.com> >> >> >> >> Allow driver tests to work without specifying the netdevice names. >> >> Introduce a possibility to search for available netdevices according to >> >> set driver name. Allow test to specify the name by setting >> >> NETIF_FIND_DRIVER variable. >> >> >> >> Note that user overrides this either by passing netdevice names on the >> >> command line or by declaring NETIFS array in custom forwarding.config >> >> configuration file. >> >> >> >> Signed-off-by: Jiri Pirko <jiri@nvidia.com> >> >> --- >> >> tools/testing/selftests/net/forwarding/lib.sh | 39 +++++++++++++++++++ >> >> 1 file changed, 39 insertions(+) >> >> >> >> diff --git a/tools/testing/selftests/net/forwarding/lib.sh b/tools/testing/selftests/net/forwarding/lib.sh >> >> index 6f6a0f13465f..06633518b3aa 100644 >> >> --- a/tools/testing/selftests/net/forwarding/lib.sh >> >> +++ b/tools/testing/selftests/net/forwarding/lib.sh >> >> @@ -55,6 +55,9 @@ declare -A NETIFS=( >> >> : "${NETIF_CREATE:=yes}" >> >> : "${NETIF_TYPE:=veth}" >> >> >> >> +# Whether to find netdevice according to the specified driver. >> >> +: "${NETIF_FIND_DRIVER:=}" >> >> + >> > >> >This section of the file sets default values for variables that can be >> >set by users in forwarding.config. NETIF_FIND_DRIVER is more like >> >NUM_NETIFS, it is set by tests, so I don't think it should be listed >> >there. >> >> Well, currently there is a mixture of config variables and test >> definitions/requirements. For example REQUIRE_JQ, REQUIRE_MZ, REQUIRE_MTOOLS >> are not forwarding.config configurable (they are, they should not be ;)) > >Yes, that's true. If you prefer to leave that statement there, go ahead. > >> Where do you suggest to move NETIF_FIND_DRIVER? > >I would make NETIF_FIND_DRIVER like NUM_NETIFS, ie. there's no statement >setting a default value for it. And I would move the comment describing >its purpose above this new part: Ok. > >> + >> +if [[ ! -z $NETIF_FIND_DRIVER ]]; then >> + unset NETIFS >> + declare -A NETIFS >> + find_netif >> +fi >> + > >BTW, '! -z' can be removed from that test. It's equivalent to: >if [[ $NETIF_FIND_DRIVER ]]; then Ok.
diff --git a/tools/testing/selftests/net/forwarding/lib.sh b/tools/testing/selftests/net/forwarding/lib.sh index 6f6a0f13465f..06633518b3aa 100644 --- a/tools/testing/selftests/net/forwarding/lib.sh +++ b/tools/testing/selftests/net/forwarding/lib.sh @@ -55,6 +55,9 @@ declare -A NETIFS=( : "${NETIF_CREATE:=yes}" : "${NETIF_TYPE:=veth}" +# Whether to find netdevice according to the specified driver. +: "${NETIF_FIND_DRIVER:=}" + # Constants for ping tests: # How many packets should be sent. : "${PING_COUNT:=10}" @@ -94,6 +97,42 @@ if [[ ! -v NUM_NETIFS ]]; then exit $ksft_skip fi +############################################################################## +# Find netifs by test-specified driver name + +driver_name_get() +{ + local dev=$1; shift + local driver_path="/sys/class/net/$dev/device/driver" + + if [ ! -L $driver_path ]; then + echo "" + else + basename `realpath $driver_path` + fi +} + +find_netif() +{ + local ifnames=`ip -j -p link show | jq -e -r ".[].ifname"` + local count=0 + + for ifname in $ifnames + do + local driver_name=`driver_name_get $ifname` + if [[ ! -z $driver_name && $driver_name == $NETIF_FIND_DRIVER ]]; then + count=$((count + 1)) + NETIFS[p$count]="$ifname" + fi + done +} + +if [[ ! -z $NETIF_FIND_DRIVER ]]; then + unset NETIFS + declare -A NETIFS + find_netif +fi + net_forwarding_dir=$(dirname "$(readlink -e "${BASH_SOURCE[0]}")") if [[ -f $net_forwarding_dir/forwarding.config ]]; then