Re: [PATCH net-next v4 03/10] selftests: net: extend lib.sh to parse drivers/net/net.config

Ioana Ciornei
2026-03-30T12:28:46Z

On Mon, Mar 30, 2026 at 01:28:49PM +0200, Petr Machata wrote:
>
> Ioana Ciornei <ioana.ciornei@nxp.com> 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
> > REMOTE_ARGS=root@192.168.1.2
> >
> > will generate the NETIFS and TARGETS arrays with the following data.
> >
> > NETIFS[p1]="eth0"
> > NETIFS[p2]="eth2"
> >
> > TARGETS[eth0]="local:"
> > TARGETS[eth2]="ssh:root@192.168.1.2"
> >
> > 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 <ioana.ciornei@nxp.com>
> > ---
> > 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.

Yes.

(...)

> > 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.

Ok.

>
> > 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.

Ok, will move it before the if statement.

(...)
> > +
> > + 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?

Ok, will declare it above the if.

Thanks!
Ioana