On Mon, Mar 30, 2026 at 01:32:41PM +0200, Petr Machata wrote:
>
> Petr Machata <[email protected]> writes:
>
> > Ioana Ciornei <[email protected]> writes:
> >
> >> Add a couple of helpers which can be used by tests which need to run a
> >> specific bash command on a different target than the local system, be it
> >> either another netns or a remote system accessible through ssh.
> >>
> >> The __run_on() function is passed through $1 the target on which the
> >> command should be executed while run_on() is passed the name of the
> >> interface that is then used to retrieve the target from the TARGETS
> >> array.
> >>
> >> Signed-off-by: Ioana Ciornei <[email protected]>
> >> ---
> >> Changes in v4:
> >> - reworked the helpers so that no global variable is used and
> >> information is passed only through parameters
> >> Changes in v3:
> >> - s/TARGET/CUR_TARGET
> >> - always fallback on running a command locally when either TARGETS is
> >> not declared or there is no entry for a specific interface
> >> Changes in v2:
> >> - patch is new
> >>
> >> tools/testing/selftests/net/lib.sh | 38 ++++++++++++++++++++++++++++++
> >> 1 file changed, 38 insertions(+)
> >>
> >> diff --git a/tools/testing/selftests/net/lib.sh
> >> b/tools/testing/selftests/net/lib.sh
> >> index b40694573f4c..6c0d613a4de5 100644
> >> --- a/tools/testing/selftests/net/lib.sh
> >> +++ b/tools/testing/selftests/net/lib.sh
> >> @@ -670,3 +670,41 @@ cmd_jq()
> >> # return success only in case of non-empty output
> >> [ ! -z "$output" ]
> >> }
> >> +
> >> +__run_on()
> >> +{
> >> + local target=$1; shift
> >> + local type args
> >> +
> >> + IFS=':' read -r type args <<< "$target"
> >> +
> >> + case "$type" in
> >> + netns)
> >> + # Execute command in network namespace
> >> + # args contains the namespace name
> >> + ip netns exec "$args" "$@"
> >> + ;;
> >> + ssh)
> >> + # Execute command via SSH args contains user@host
> >> + ssh -n "$args" "$@"
> >> + ;;
> >> + local|*)
> >> + # Execute command locally. This is also the fallback
> >> + # case for when the interface's target is not found in
> >> + # the TARGETS array.
> >> + "$@"
> >> + ;;
> >> + esac
> >> +}
> >> +
> >> +run_on()
> >> +{
> >> + local iface=$1; shift
> >> + local target="local:"
> >> +
> >> + if declare -p TARGETS &>/dev/null; then
> >> + target="${TARGETS[$iface]}"
> >
> > So I think Jakub's runs fail because there's a shell export somewhere
> > that gets inherited through make to the launched test. I guess it would
> > be enough for the test to validate that TARGETS is an array, because
> > those don't get inherited.
> >
> > Is there a reason not to reuse DRIVER_TEST_CONFORMANT as a tell though?
> >
> >> + fi
> >> +
> >> + __run_on "$target" "$@"
> >> +}
> >
> > Does the latter helper need to be in net/lib.sh? Since it uses TARGETS,
> > which are a forwarding/lib.sh concept, it seems misplaced there.
>
> Oh, I see, there's an invocation from mac_get() in net/lib.sh itself.
> Hummm. Not sure how to tackle this.
>
> I think lib.sh might unset TARGETS explicitly? Or declare -A, but leave
> empty? Since it's now an API, net/lib.sh needs to set it to a reasonable
> value (or erase). Then forwarding/lib.sh might in theory rely on
> existence of that variable and not have to declare it at all.
>
> Or, maybe have a stub run_on() like this to satisfy the run_on() API:
>
> run_on()
> {
> "$@"
shift; "$@"
> }
>
> And have the full-blown thing in forward/lib.sh. All the magic with
> TARGETS really belongs to forwarding/lib.sh. Bash allows function
> redefinition just fine, so a user importing just net/lib.sh would get
> the stub, and forwarding/lib.sh users would get the full thing.
I really like the idea of the stub run_on() in net/lib.sh. This would
also give me the possibility to check for DRIVER_TEST_CONFORMANT = "yes"
in the full-blown run_on() without having to move its default
definition in net/lib.sh.
I quickly changed to this approach and it seems to look good.
