| Message ID | 20240104141109.100672-1-bpoirier@nvidia.com (mailing list archive) |
|---|---|
| State | Accepted |
| Commit | 2114e83381d3289a88378850f43069e79f848083 |
| Delegated to: | Netdev Maintainers |
| Headers | show |
| Series | [net-next] selftests: forwarding: Avoid failures to source net/lib.sh | expand |
On Thu, Jan 04, 2024 at 09:11:09AM -0500, Benjamin Poirier wrote: > The expression "source ../lib.sh" added to net/forwarding/lib.sh in commit > 25ae948b4478 ("selftests/net: add lib.sh") does not work for tests outside > net/forwarding which source net/forwarding/lib.sh (1). It also does not > work in some cases where only a subset of tests are exported (2). > > Avoid the problems mentioned above by replacing the faulty expression with > a copy of the content from net/lib.sh which is used by files under > net/forwarding. > > A more thorough solution which avoids duplicating content between > net/lib.sh and net/forwarding/lib.sh has been posted here: > https://lore.kernel.org/netdev/20231222135836.992841-1-bpoirier@nvidia.com/ > > The approach in the current patch is a stopgap solution to avoid submitting > large changes at the eleventh hour of this development cycle. > > Example of problem 1) > > tools/testing/selftests/drivers/net/bonding$ ./dev_addr_lists.sh > ./net_forwarding_lib.sh: line 41: ../lib.sh: No such file or directory > TEST: bonding cleanup mode active-backup [ OK ] > TEST: bonding cleanup mode 802.3ad [ OK ] > TEST: bonding LACPDU multicast address to slave (from bond down) [ OK ] > TEST: bonding LACPDU multicast address to slave (from bond up) [ OK ] > > An error message is printed but since the test does not use functions from > net/lib.sh, the test results are not affected. > > Example of problem 2) > > tools/testing/selftests$ make install TARGETS="net/forwarding" > tools/testing/selftests$ cd kselftest_install/net/forwarding/ > tools/testing/selftests/kselftest_install/net/forwarding$ ./pedit_ip.sh veth{0..3} > lib.sh: line 41: ../lib.sh: No such file or directory > TEST: ping [ OK ] > TEST: ping6 [ OK ] > ./pedit_ip.sh: line 135: busywait: command not found > TEST: dev veth1 ingress pedit ip src set 198.51.100.1 [FAIL] > Expected to get 10 packets, but got . > ./pedit_ip.sh: line 135: busywait: command not found > TEST: dev veth2 egress pedit ip src set 198.51.100.1 [FAIL] > Expected to get 10 packets, but got . > ./pedit_ip.sh: line 135: busywait: command not found > TEST: dev veth1 ingress pedit ip dst set 198.51.100.1 [FAIL] > Expected to get 10 packets, but got . > ./pedit_ip.sh: line 135: busywait: command not found > TEST: dev veth2 egress pedit ip dst set 198.51.100.1 [FAIL] > Expected to get 10 packets, but got . > ./pedit_ip.sh: line 135: busywait: command not found > TEST: dev veth1 ingress pedit ip6 src set 2001:db8:2::1 [FAIL] > Expected to get 10 packets, but got . > ./pedit_ip.sh: line 135: busywait: command not found > TEST: dev veth2 egress pedit ip6 src set 2001:db8:2::1 [FAIL] > Expected to get 10 packets, but got . > ./pedit_ip.sh: line 135: busywait: command not found > TEST: dev veth1 ingress pedit ip6 dst set 2001:db8:2::1 [FAIL] > Expected to get 10 packets, but got . > ./pedit_ip.sh: line 135: busywait: command not found > TEST: dev veth2 egress pedit ip6 dst set 2001:db8:2::1 [FAIL] > Expected to get 10 packets, but got . > > In this case, the test results are affected. > > Fixes: 25ae948b4478 ("selftests/net: add lib.sh") > Suggested-by: Ido Schimmel <idosch@nvidia.com> > Suggested-by: Petr Machata <petrm@nvidia.com> > Reviewed-by: Ido Schimmel <idosch@nvidia.com> > Tested-by: Petr Machata <petrm@nvidia.com> > Signed-off-by: Benjamin Poirier <bpoirier@nvidia.com> > --- > tools/testing/selftests/net/forwarding/lib.sh | 27 ++++++++++++++++++- > 1 file changed, 26 insertions(+), 1 deletion(-) > > diff --git a/tools/testing/selftests/net/forwarding/lib.sh b/tools/testing/selftests/net/forwarding/lib.sh > index 69ef2a40df21..8a61464ab6eb 100755 > --- a/tools/testing/selftests/net/forwarding/lib.sh > +++ b/tools/testing/selftests/net/forwarding/lib.sh > @@ -38,7 +38,32 @@ if [[ -f $relative_path/forwarding.config ]]; then > source "$relative_path/forwarding.config" > fi > > -source ../lib.sh > +# Kselftest framework requirement - SKIP code is 4. > +ksft_skip=4 > + > +busywait() > +{ > + local timeout=$1; shift > + > + local start_time="$(date -u +%s%3N)" > + while true > + do > + local out > + out=$("$@") > + local ret=$? > + if ((!ret)); then > + echo -n "$out" > + return 0 > + fi > + > + local current_time="$(date -u +%s%3N)" > + if ((current_time - start_time > timeout)); then > + echo -n "$out" > + return 1 > + fi > + done > +} > + > ############################################################################## > # Sanity checks > Reviewed-by: Hangbin Liu <liuhangbin@gmail.com> Thanks Hangbin
Hello: This patch was applied to netdev/net-next.git (main) by Jakub Kicinski <kuba@kernel.org>: On Thu, 4 Jan 2024 09:11:09 -0500 you wrote: > The expression "source ../lib.sh" added to net/forwarding/lib.sh in commit > 25ae948b4478 ("selftests/net: add lib.sh") does not work for tests outside > net/forwarding which source net/forwarding/lib.sh (1). It also does not > work in some cases where only a subset of tests are exported (2). > > Avoid the problems mentioned above by replacing the faulty expression with > a copy of the content from net/lib.sh which is used by files under > net/forwarding. > > [...] Here is the summary with links: - [net-next] selftests: forwarding: Avoid failures to source net/lib.sh https://git.kernel.org/netdev/net-next/c/2114e83381d3 You are awesome, thank you!
diff --git a/tools/testing/selftests/net/forwarding/lib.sh b/tools/testing/selftests/net/forwarding/lib.sh index 69ef2a40df21..8a61464ab6eb 100755 --- a/tools/testing/selftests/net/forwarding/lib.sh +++ b/tools/testing/selftests/net/forwarding/lib.sh @@ -38,7 +38,32 @@ if [[ -f $relative_path/forwarding.config ]]; then source "$relative_path/forwarding.config" fi -source ../lib.sh +# Kselftest framework requirement - SKIP code is 4. +ksft_skip=4 + +busywait() +{ + local timeout=$1; shift + + local start_time="$(date -u +%s%3N)" + while true + do + local out + out=$("$@") + local ret=$? + if ((!ret)); then + echo -n "$out" + return 0 + fi + + local current_time="$(date -u +%s%3N)" + if ((current_time - start_time > timeout)); then + echo -n "$out" + return 1 + fi + done +} + ############################################################################## # Sanity checks
The expression "source ../lib.sh" added to net/forwarding/lib.sh in commit 25ae948b4478 ("selftests/net: add lib.sh") does not work for tests outside net/forwarding which source net/forwarding/lib.sh (1). It also does not work in some cases where only a subset of tests are exported (2). Avoid the problems mentioned above by replacing the faulty expression with a copy of the content from net/lib.sh which is used by files under net/forwarding. A more thorough solution which avoids duplicating content between net/lib.sh and net/forwarding/lib.sh has been posted here: https://lore.kernel.org/netdev/20231222135836.992841-1-bpoirier@nvidia.com/ The approach in the current patch is a stopgap solution to avoid submitting large changes at the eleventh hour of this development cycle. Example of problem 1) tools/testing/selftests/drivers/net/bonding$ ./dev_addr_lists.sh ./net_forwarding_lib.sh: line 41: ../lib.sh: No such file or directory TEST: bonding cleanup mode active-backup [ OK ] TEST: bonding cleanup mode 802.3ad [ OK ] TEST: bonding LACPDU multicast address to slave (from bond down) [ OK ] TEST: bonding LACPDU multicast address to slave (from bond up) [ OK ] An error message is printed but since the test does not use functions from net/lib.sh, the test results are not affected. Example of problem 2) tools/testing/selftests$ make install TARGETS="net/forwarding" tools/testing/selftests$ cd kselftest_install/net/forwarding/ tools/testing/selftests/kselftest_install/net/forwarding$ ./pedit_ip.sh veth{0..3} lib.sh: line 41: ../lib.sh: No such file or directory TEST: ping [ OK ] TEST: ping6 [ OK ] ./pedit_ip.sh: line 135: busywait: command not found TEST: dev veth1 ingress pedit ip src set 198.51.100.1 [FAIL] Expected to get 10 packets, but got . ./pedit_ip.sh: line 135: busywait: command not found TEST: dev veth2 egress pedit ip src set 198.51.100.1 [FAIL] Expected to get 10 packets, but got . ./pedit_ip.sh: line 135: busywait: command not found TEST: dev veth1 ingress pedit ip dst set 198.51.100.1 [FAIL] Expected to get 10 packets, but got . ./pedit_ip.sh: line 135: busywait: command not found TEST: dev veth2 egress pedit ip dst set 198.51.100.1 [FAIL] Expected to get 10 packets, but got . ./pedit_ip.sh: line 135: busywait: command not found TEST: dev veth1 ingress pedit ip6 src set 2001:db8:2::1 [FAIL] Expected to get 10 packets, but got . ./pedit_ip.sh: line 135: busywait: command not found TEST: dev veth2 egress pedit ip6 src set 2001:db8:2::1 [FAIL] Expected to get 10 packets, but got . ./pedit_ip.sh: line 135: busywait: command not found TEST: dev veth1 ingress pedit ip6 dst set 2001:db8:2::1 [FAIL] Expected to get 10 packets, but got . ./pedit_ip.sh: line 135: busywait: command not found TEST: dev veth2 egress pedit ip6 dst set 2001:db8:2::1 [FAIL] Expected to get 10 packets, but got . In this case, the test results are affected. Fixes: 25ae948b4478 ("selftests/net: add lib.sh") Suggested-by: Ido Schimmel <idosch@nvidia.com> Suggested-by: Petr Machata <petrm@nvidia.com> Reviewed-by: Ido Schimmel <idosch@nvidia.com> Tested-by: Petr Machata <petrm@nvidia.com> Signed-off-by: Benjamin Poirier <bpoirier@nvidia.com> --- tools/testing/selftests/net/forwarding/lib.sh | 27 ++++++++++++++++++- 1 file changed, 26 insertions(+), 1 deletion(-)