[PATCH net-next v4 02/10] selftests: net: add helpers for running a command on other targets

Ioana Ciornei posted 10 patches 1 week ago
There is a newer version of this series
[PATCH net-next v4 02/10] selftests: net: add helpers for running a command on other targets
Posted by Ioana Ciornei 1 week ago
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 <ioana.ciornei@nxp.com>
---
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]}"
+	fi
+
+	__run_on "$target" "$@"
+}
-- 
2.25.1
Re: [PATCH net-next v4 02/10] selftests: net: add helpers for running a command on other targets
Posted by Petr Machata 3 days, 9 hours ago
Ioana Ciornei <ioana.ciornei@nxp.com> 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 <ioana.ciornei@nxp.com>
> ---
> 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.
Re: [PATCH net-next v4 02/10] selftests: net: add helpers for running a command on other targets
Posted by Jakub Kicinski 2 days, 20 hours ago
On Mon, 30 Mar 2026 13:02:12 +0200 Petr Machata wrote:
> > +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.

Great catch, FWIW. Yes TARGETS is what ksft makefiles use to define
the group of tests. We do 

  make ... TARGETS=drivers/net ... run_tests
Re: [PATCH net-next v4 02/10] selftests: net: add helpers for running a command on other targets
Posted by Petr Machata 2 days, 8 hours ago
Jakub Kicinski <kuba@kernel.org> writes:

> On Mon, 30 Mar 2026 13:02:12 +0200 Petr Machata wrote:
>> > +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.
>
> Great catch, FWIW. Yes TARGETS is what ksft makefiles use to define
> the group of tests. We do 
>
>   make ... TARGETS=drivers/net ... run_tests

That'll do it. I thought the make command line-defined variables don't
get inherited, which is why I said shell exports, but indeed they do:

$ cat foo.mk
all:
	echo TARGETS=$(TARGETS)
	bash x.sh

$ cat x.sh
echo $TARGETS

$ make TARGETS=TARGETS -f foo.mk
echo TARGETS=TARGETS
TARGETS=TARGETS
bash x.sh
TARGETS
Re: [PATCH net-next v4 02/10] selftests: net: add helpers for running a command on other targets
Posted by Petr Machata 3 days, 8 hours ago
Petr Machata <petrm@nvidia.com> writes:

> Ioana Ciornei <ioana.ciornei@nxp.com> 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 <ioana.ciornei@nxp.com>
>> ---
>> 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()
{
	"$@"
}

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.
Re: [PATCH net-next v4 02/10] selftests: net: add helpers for running a command on other targets
Posted by Ioana Ciornei 3 days, 8 hours ago
On Mon, Mar 30, 2026 at 01:32:41PM +0200, Petr Machata wrote:
> 
> Petr Machata <petrm@nvidia.com> writes:
> 
> > Ioana Ciornei <ioana.ciornei@nxp.com> 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 <ioana.ciornei@nxp.com>
> >> ---
> >> 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.
Re: [PATCH net-next v4 02/10] selftests: net: add helpers for running a command on other targets
Posted by Petr Machata 3 days, 5 hours ago
Ioana Ciornei <ioana.ciornei@nxp.com> writes:

> On Mon, Mar 30, 2026 at 01:32:41PM +0200, Petr Machata wrote:
>> 
>> Petr Machata <petrm@nvidia.com> writes:
>> 
>> > Ioana Ciornei <ioana.ciornei@nxp.com> writes:
>> >
>> >> +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; "$@"

Indeed!

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

Yeah, I like it best as well.

Then I think you don't need to worry about the TARGETS export, because
forwarding/lib.sh will add the "array-ness" to whatever comes from the
outside, if anything, and the function will be able to use like that.