On Mon, Mar 30, 2026 at 01:28:49PM +0200, Petr Machata wrote:
> 
> 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.

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

Reply via email to