* [PATCH] selftests/net/forwarding: teamd command not found @ 2025-01-10 0:07 Alessandro Zanni 2025-01-10 4:09 ` Hangbin Liu 0 siblings, 1 reply; 6+ messages in thread From: Alessandro Zanni @ 2025-01-10 0:07 UTC (permalink / raw) To: davem, edumazet, kuba, pabeni, shuah Cc: Alessandro Zanni, netdev, linux-kselftest, linux-kernel Running "make kselftest TARGETS=net/forwarding" results in several occurrences of the same error: ./lib.sh: line 787: teamd: command not found Since many tests depends on teamd, this fix stops the tests if the teamd command is not installed. Signed-off-by: Alessandro Zanni <alessandro.zanni87@gmail.com> --- tools/testing/selftests/net/forwarding/lib.sh | 1 + 1 file changed, 1 insertion(+) diff --git a/tools/testing/selftests/net/forwarding/lib.sh b/tools/testing/selftests/net/forwarding/lib.sh index 7337f398f9cc..a6a74a4be4bf 100644 --- a/tools/testing/selftests/net/forwarding/lib.sh +++ b/tools/testing/selftests/net/forwarding/lib.sh @@ -784,6 +784,7 @@ team_destroy() { local if_name=$1; shift + require_command $TEAMD $TEAMD -t $if_name -k } -- 2.43.0 ^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH] selftests/net/forwarding: teamd command not found 2025-01-10 0:07 [PATCH] selftests/net/forwarding: teamd command not found Alessandro Zanni @ 2025-01-10 4:09 ` Hangbin Liu 2025-01-10 14:35 ` Alessandro Zanni 0 siblings, 1 reply; 6+ messages in thread From: Hangbin Liu @ 2025-01-10 4:09 UTC (permalink / raw) To: Alessandro Zanni Cc: davem, edumazet, kuba, pabeni, shuah, netdev, linux-kselftest, linux-kernel On Fri, Jan 10, 2025 at 01:07:44AM +0100, Alessandro Zanni wrote: > Running "make kselftest TARGETS=net/forwarding" results in several > occurrences of the same error: > ./lib.sh: line 787: teamd: command not found > > Since many tests depends on teamd, this fix stops the tests if the > teamd command is not installed. > > Signed-off-by: Alessandro Zanni <alessandro.zanni87@gmail.com> > --- > tools/testing/selftests/net/forwarding/lib.sh | 1 + > 1 file changed, 1 insertion(+) > > diff --git a/tools/testing/selftests/net/forwarding/lib.sh b/tools/testing/selftests/net/forwarding/lib.sh > index 7337f398f9cc..a6a74a4be4bf 100644 > --- a/tools/testing/selftests/net/forwarding/lib.sh > +++ b/tools/testing/selftests/net/forwarding/lib.sh > @@ -784,6 +784,7 @@ team_destroy() > { > local if_name=$1; shift > > + require_command $TEAMD > $TEAMD -t $if_name -k > } I saw team_create() has `require_command $TEAMD`. Is some test called team_destroy() before team_create()? ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] selftests/net/forwarding: teamd command not found 2025-01-10 4:09 ` Hangbin Liu @ 2025-01-10 14:35 ` Alessandro Zanni 2025-01-11 1:11 ` Hangbin Liu 2025-01-11 1:16 ` Jakub Kicinski 0 siblings, 2 replies; 6+ messages in thread From: Alessandro Zanni @ 2025-01-10 14:35 UTC (permalink / raw) To: Hangbin Liu Cc: davem, edumazet, kuba, pabeni, shuah, netdev, linux-kselftest, linux-kernel On Fri, Jan 10, 2025 at 04:09:07AM +0000, Hangbin Liu wrote: > On Fri, Jan 10, 2025 at 01:07:44AM +0100, Alessandro Zanni wrote: > > Running "make kselftest TARGETS=net/forwarding" results in several > > occurrences of the same error: > > ./lib.sh: line 787: teamd: command not found > > > > Since many tests depends on teamd, this fix stops the tests if the > > teamd command is not installed. > > > > Signed-off-by: Alessandro Zanni <alessandro.zanni87@gmail.com> > > --- > > tools/testing/selftests/net/forwarding/lib.sh | 1 + > > 1 file changed, 1 insertion(+) > > > > diff --git a/tools/testing/selftests/net/forwarding/lib.sh b/tools/testing/selftests/net/forwarding/lib.sh > > index 7337f398f9cc..a6a74a4be4bf 100644 > > --- a/tools/testing/selftests/net/forwarding/lib.sh > > +++ b/tools/testing/selftests/net/forwarding/lib.sh > > @@ -784,6 +784,7 @@ team_destroy() > > { > > local if_name=$1; shift > > > > + require_command $TEAMD > > $TEAMD -t $if_name -k > > } > > I saw team_create() has `require_command $TEAMD`. Is some test called > team_destroy() before team_create()? Actually, the errors seem to raise in the "cleanup()" method. So, first, during the test preparation the "team_create()" method is called and checks if teamd is installed. When it fails and skips the test, the "cleanup()" method calls the "team_destroy()" that raises the errors. ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] selftests/net/forwarding: teamd command not found 2025-01-10 14:35 ` Alessandro Zanni @ 2025-01-11 1:11 ` Hangbin Liu 2025-01-11 1:16 ` Jakub Kicinski 1 sibling, 0 replies; 6+ messages in thread From: Hangbin Liu @ 2025-01-11 1:11 UTC (permalink / raw) To: Alessandro Zanni Cc: davem, edumazet, kuba, pabeni, shuah, netdev, linux-kselftest, linux-kernel On Fri, Jan 10, 2025 at 03:35:23PM +0100, Alessandro Zanni wrote: > On Fri, Jan 10, 2025 at 04:09:07AM +0000, Hangbin Liu wrote: > > On Fri, Jan 10, 2025 at 01:07:44AM +0100, Alessandro Zanni wrote: > > > Running "make kselftest TARGETS=net/forwarding" results in several > > > occurrences of the same error: > > > ./lib.sh: line 787: teamd: command not found > > > > > > Since many tests depends on teamd, this fix stops the tests if the > > > teamd command is not installed. > > > > > > Signed-off-by: Alessandro Zanni <alessandro.zanni87@gmail.com> > > > --- > > > tools/testing/selftests/net/forwarding/lib.sh | 1 + > > > 1 file changed, 1 insertion(+) > > > > > > diff --git a/tools/testing/selftests/net/forwarding/lib.sh b/tools/testing/selftests/net/forwarding/lib.sh > > > index 7337f398f9cc..a6a74a4be4bf 100644 > > > --- a/tools/testing/selftests/net/forwarding/lib.sh > > > +++ b/tools/testing/selftests/net/forwarding/lib.sh > > > @@ -784,6 +784,7 @@ team_destroy() > > > { > > > local if_name=$1; shift > > > > > > + require_command $TEAMD > > > $TEAMD -t $if_name -k > > > } > > > > I saw team_create() has `require_command $TEAMD`. Is some test called > > team_destroy() before team_create()? > > Actually, the errors seem to raise in the "cleanup()" method. > > So, first, during the test preparation the "team_create()" method is called > and checks if teamd is installed. > When it fails and skips the test, the "cleanup()" method calls > the "team_destroy()" that raises the errors. > Thanks for the clarify. Reviewed-by: Hangbin Liu <liuhangbin@gmail.com> ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] selftests/net/forwarding: teamd command not found 2025-01-10 14:35 ` Alessandro Zanni 2025-01-11 1:11 ` Hangbin Liu @ 2025-01-11 1:16 ` Jakub Kicinski 2025-01-13 11:48 ` Petr Machata 1 sibling, 1 reply; 6+ messages in thread From: Jakub Kicinski @ 2025-01-11 1:16 UTC (permalink / raw) To: Alessandro Zanni, petrm Cc: Hangbin Liu, davem, edumazet, pabeni, shuah, netdev, linux-kselftest, linux-kernel On Fri, 10 Jan 2025 15:35:23 +0100 Alessandro Zanni wrote: > On Fri, Jan 10, 2025 at 04:09:07AM +0000, Hangbin Liu wrote: > > On Fri, Jan 10, 2025 at 01:07:44AM +0100, Alessandro Zanni wrote: > > > Running "make kselftest TARGETS=net/forwarding" results in several > > > occurrences of the same error: > > > ./lib.sh: line 787: teamd: command not found > > > > > > Since many tests depends on teamd, this fix stops the tests if the > > > teamd command is not installed. > > > > > > Signed-off-by: Alessandro Zanni <alessandro.zanni87@gmail.com> > > > --- > > > tools/testing/selftests/net/forwarding/lib.sh | 1 + > > > 1 file changed, 1 insertion(+) > > > > > > diff --git a/tools/testing/selftests/net/forwarding/lib.sh b/tools/testing/selftests/net/forwarding/lib.sh > > > index 7337f398f9cc..a6a74a4be4bf 100644 > > > --- a/tools/testing/selftests/net/forwarding/lib.sh > > > +++ b/tools/testing/selftests/net/forwarding/lib.sh > > > @@ -784,6 +784,7 @@ team_destroy() > > > { > > > local if_name=$1; shift > > > > > > + require_command $TEAMD > > > $TEAMD -t $if_name -k > > > } > > > > I saw team_create() has `require_command $TEAMD`. Is some test called > > team_destroy() before team_create()? > > Actually, the errors seem to raise in the "cleanup()" method. > > So, first, during the test preparation the "team_create()" method is called > and checks if teamd is installed. > When it fails and skips the test, the "cleanup()" method calls > the "team_destroy()" that raises the errors. Actually, maybe we're better off switching the team handling to the way REQUIRE_JQ REQUIRE_MZ etc are handled? That way we'll validate and exit when lib is imported / sourced. Petr, do you have a preference? -- pw-bot: cr ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] selftests/net/forwarding: teamd command not found 2025-01-11 1:16 ` Jakub Kicinski @ 2025-01-13 11:48 ` Petr Machata 0 siblings, 0 replies; 6+ messages in thread From: Petr Machata @ 2025-01-13 11:48 UTC (permalink / raw) To: Jakub Kicinski Cc: Alessandro Zanni, petrm, Hangbin Liu, davem, edumazet, pabeni, shuah, netdev, linux-kselftest, linux-kernel Jakub Kicinski <kuba@kernel.org> writes: > On Fri, 10 Jan 2025 15:35:23 +0100 Alessandro Zanni wrote: >> On Fri, Jan 10, 2025 at 04:09:07AM +0000, Hangbin Liu wrote: >> > On Fri, Jan 10, 2025 at 01:07:44AM +0100, Alessandro Zanni wrote: >> > > Running "make kselftest TARGETS=net/forwarding" results in several >> > > occurrences of the same error: >> > > ./lib.sh: line 787: teamd: command not found >> > > >> > > Since many tests depends on teamd, this fix stops the tests if the >> > > teamd command is not installed. >> > > >> > > Signed-off-by: Alessandro Zanni <alessandro.zanni87@gmail.com> >> > > --- >> > > tools/testing/selftests/net/forwarding/lib.sh | 1 + >> > > 1 file changed, 1 insertion(+) >> > > >> > > diff --git a/tools/testing/selftests/net/forwarding/lib.sh b/tools/testing/selftests/net/forwarding/lib.sh >> > > index 7337f398f9cc..a6a74a4be4bf 100644 >> > > --- a/tools/testing/selftests/net/forwarding/lib.sh >> > > +++ b/tools/testing/selftests/net/forwarding/lib.sh >> > > @@ -784,6 +784,7 @@ team_destroy() >> > > { >> > > local if_name=$1; shift >> > > >> > > + require_command $TEAMD >> > > $TEAMD -t $if_name -k >> > > } >> > >> > I saw team_create() has `require_command $TEAMD`. Is some test called >> > team_destroy() before team_create()? >> >> Actually, the errors seem to raise in the "cleanup()" method. >> >> So, first, during the test preparation the "team_create()" method is called >> and checks if teamd is installed. >> When it fails and skips the test, the "cleanup()" method calls >> the "team_destroy()" that raises the errors. > > Actually, maybe we're better off switching the team handling to the way > REQUIRE_JQ REQUIRE_MZ etc are handled? That way we'll validate and exit > when lib is imported / sourced. > > Petr, do you have a preference? Yeah, that would be better. My first though was that this is fine, because it's just for tests that don't use defer. (With defer, if the creation bails out, you never schedule the cleanup.) But if you bail out early from team_destroy(), the rest of the cleanups will be skipped, which is bad. So maybe we need something like this: $TEAMD -t $if_name -k 2>/dev/null ||: ... i.e. to just quietly tolerate failures. But that's no good either, because then you never catch issues with the cleanup itself even in valid runs. So maybe team_create() should note whether $TEAMD was found and do this conditionally, but at that point, going with REQUIRE_TEAMD is the cleaner solution. It looks like we have 7 scripts that use team_create(), so it's not crazy to just fix them all. ^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2025-01-13 12:07 UTC | newest] Thread overview: 6+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2025-01-10 0:07 [PATCH] selftests/net/forwarding: teamd command not found Alessandro Zanni 2025-01-10 4:09 ` Hangbin Liu 2025-01-10 14:35 ` Alessandro Zanni 2025-01-11 1:11 ` Hangbin Liu 2025-01-11 1:16 ` Jakub Kicinski 2025-01-13 11:48 ` Petr Machata
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).