| Message ID | 20230208092327.28914-1-pvorel@suse.cz |
|---|---|
| State | Accepted |
| Headers | show |
| Series | [1/1] testscripts/network.sh: Don't load tst_net.sh | expand |
Hi, FYI this can be tested: /opt/ltp/testscripts/network.sh -6 vs. running plain test, e.g. PATH="/opt/ltp/testcases/bin:$PATH" ping02.sh -6 Kind regards, Petr
> This should not be needed any more for neither new API and legacy tests > since 04021637f4 ("tst_test.sh: Cleanup getopts usage") I suppose loading tst_net.sh was not needed even before. I wonder why this was ever needed. It was added in 6538f7ab70 ("network: merge networktests.sh/networkstress.sh into network.sh") when testcases/lib/test_net.sh (predecessor of testcases/lib/tst_net.sh) was much simpler (e.g. without netns). I suppose it was due export LTP_RSH=${LTP_RSH:-"rsh -n"} which was moved from testscripts/network.sh to testcases/lib/test_net.sh. LTP_RSH was changed to use ssh internally in 40110018e6 ("tst_net.sh: Remove rsh support"), but the variable is still used in various legacy files, which don't even use tst_net.sh. But these files aren't in any runtest file (see below), therefore it should be safe to merge this. $ for i in $(git grep -l LTP_RSH |grep -v README); do j=$(basename $i); echo "* $j ($i)"; git grep $j runtest/; done * tst_net.sh (testcases/lib/tst_net.sh) * icmp4-multi-diffip01 (testcases/network/stress/icmp/multi-diffip/icmp4-multi-diffip01) * icmp4-multi-diffnic01 (testcases/network/stress/icmp/multi-diffnic/icmp4-multi-diffnic01) * add_ipv6addr (testcases/network/stress/ns-tools/add_ipv6addr) * check_envval (testcases/network/stress/ns-tools/check_envval) * check_netem (testcases/network/stress/ns-tools/check_netem) * check_setkey (testcases/network/stress/ns-tools/check_setkey) * get_ifname (testcases/network/stress/ns-tools/get_ifname) * initialize_if (testcases/network/stress/ns-tools/initialize_if) * killall_icmp_traffic (testcases/network/stress/ns-tools/killall_icmp_traffic) * killall_tcp_traffic (testcases/network/stress/ns-tools/killall_tcp_traffic) * killall_udp_traffic (testcases/network/stress/ns-tools/killall_udp_traffic) * set_ipv4addr (testcases/network/stress/ns-tools/set_ipv4addr) * tcp4-multi-diffip01 (testcases/network/stress/tcp/multi-diffip/tcp4-multi-diffip01) * tcp4-multi-diffnic01 (testcases/network/stress/tcp/multi-diffnic/tcp4-multi-diffnic01) * tcp4-multi-diffport01 (testcases/network/stress/tcp/multi-diffport/tcp4-multi-diffport01) * tcp4-multi-sameport01 (testcases/network/stress/tcp/multi-sameport/tcp4-multi-sameport01) * tcp4-uni-basic01 (testcases/network/stress/tcp/uni-basic/tcp4-uni-basic01) * udp4-multi-diffip01 (testcases/network/stress/udp/multi-diffip/udp4-multi-diffip01) * udp4-multi-diffnic01 (testcases/network/stress/udp/multi-diffnic/udp4-multi-diffnic01) * udp4-multi-diffport01 (testcases/network/stress/udp/multi-diffport/udp4-multi-diffport01) * udp4-uni-basic01 (testcases/network/stress/udp/uni-basic/udp4-uni-basic01) Tools in testcases/network/stress/ns-tools/ will be removed once the rest of testcases/network/stress/{icmp,udp}/ is migrated to use tst_net.sh or removed. Kind regards, Petr
Hi! > > This should not be needed any more for neither new API and legacy tests > > since 04021637f4 ("tst_test.sh: Cleanup getopts usage") > I suppose loading tst_net.sh was not needed even before. I wonder why this was > ever needed. > > It was added in 6538f7ab70 ("network: merge networktests.sh/networkstress.sh > into network.sh") when testcases/lib/test_net.sh (predecessor of > testcases/lib/tst_net.sh) was much simpler (e.g. without netns). Actually it sems that it was just moved from the top of the file further down in that commit. It was in the git since the beginning when the file was added by Alexey. Maybe there were scripts that didn't include any shell library at that time, who knows. Anyways as long as all the network tests we have does include the library themselves let's go ahead and get rid of it.
> Hi! > > > This should not be needed any more for neither new API and legacy tests > > > since 04021637f4 ("tst_test.sh: Cleanup getopts usage") > > I suppose loading tst_net.sh was not needed even before. I wonder why this was > > ever needed. > > It was added in 6538f7ab70 ("network: merge networktests.sh/networkstress.sh > > into network.sh") when testcases/lib/test_net.sh (predecessor of > > testcases/lib/tst_net.sh) was much simpler (e.g. without netns). > Actually it sems that it was just moved from the top of the file further > down in that commit. It was in the git since the beginning when the file > was added by Alexey. Maybe there were scripts that didn't include any > shell library at that time, who knows. > Anyways as long as all the network tests we have does include the > library themselves let's go ahead and get rid of it. Please read further info in my later reaction [1] (TL;DR: not all include it, but these aren't in runtest files anyway). Kind regards, Petr [1] https://lore.kernel.org/ltp/Y+NzkUcAyeupRwmP@pevik/
... > > It was added in 6538f7ab70 ("network: merge networktests.sh/networkstress.sh > > into network.sh") when testcases/lib/test_net.sh (predecessor of > > testcases/lib/tst_net.sh) was much simpler (e.g. without netns). > Actually it sems that it was just moved from the top of the file further > down in that commit. It was in the git since the beginning when the file Ah, you're right, it was added in 50c8ec324 ("network: uniform network parameters") Kind regards, Petr
Hi! > Please read further info in my later reaction [1] > (TL;DR: not all include it, but these aren't in runtest files anyway). In that case: Acked-by: Cyril Hrubis <chrubis@suse.cz>
Hi Cyril, > Hi! > > Please read further info in my later reaction [1] > > (TL;DR: not all include it, but these aren't in runtest files anyway). > In that case: > Acked-by: Cyril Hrubis <chrubis@suse.cz> Merged as b45bb8924d, with updated description. I need to spent time to resolve these old tests. https://github.com/linux-test-project/ltp/issues/128 (updated this text below in the ticket description) $ git grep -l LTP_RSH |grep -v -e README -e tst_net.sh -e /ns-tools/ testcases/network/stress/icmp/multi-diffip/icmp4-multi-diffip01 testcases/network/stress/icmp/multi-diffnic/icmp4-multi-diffnic01 testcases/network/stress/tcp/multi-diffip/tcp4-multi-diffip01 testcases/network/stress/tcp/multi-diffnic/tcp4-multi-diffnic01 testcases/network/stress/tcp/multi-diffport/tcp4-multi-diffport01 testcases/network/stress/tcp/multi-sameport/tcp4-multi-sameport01 testcases/network/stress/tcp/uni-basic/tcp4-uni-basic01 testcases/network/stress/udp/multi-diffip/udp4-multi-diffip01 testcases/network/stress/udp/multi-diffnic/udp4-multi-diffnic01 testcases/network/stress/udp/multi-diffport/udp4-multi-diffport01 testcases/network/stress/udp/uni-basic/udp4-uni-basic01 These tests using similar IPsec proto/mode params, which are in runtest/net_stress.ipsec_{dccp,icmp,sctp,tcp,udp}. These new and well working tests are using ipsec_lib.sh and are testing either with ping via tst_ping() (ICMP tests) or with netstress.c via tst_netload() (the rest of the tests). The old legacy tests are using various TCP/UDP client-server and ICMP sender tools from testcases/network/stress/ns-tools/. To decide whether they can be safely deleted or should be kept (and cleanup and possibly migrated to ipsec_lib.sh) is whether their C tools they use test other kernel functionality than netstress.c and ping. There is also ns-igmp_querier.c, which is used in mcast-lib.sh, which should be cleaned (probably not a candidate to put the code into netstress.c), but that's another case. Once this all is solved, several shell scripts in testcases/network/stress/ns-tools/ can be deleted. Kind regards, Petr
diff --git a/testscripts/network.sh b/testscripts/network.sh index 15a4cc1c71..afe9c7a977 100755 --- a/testscripts/network.sh +++ b/testscripts/network.sh @@ -86,13 +86,6 @@ if [ "$OPTIND" -eq 1 ]; then fi shift $(($OPTIND - 1)) -TST_NO_DEFAULT_RUN=1 -. tst_net.sh - -# Reset variables. -# Don't break the tests which are using 'testcases/lib/cmdlib.sh' -unset TST_ID TST_LIB_LOADED TST_NO_DEFAULT_RUN - rm -f $CMDFILE for t in $TEST_CASES; do
This should not be needed any more for neither new API and legacy tests since 04021637f4 ("tst_test.sh: Cleanup getopts usage") This fixes wrong info when tests are run with network.sh: ping01 1 TINFO: using not default LTP netns: 'ns_exec 18242 net,mnt' because we don't use any custom netns. In case loading is really needed, we'd need to revert 0da2c285a1. Fixes: 0da2c285a1 ("tst_net.sh: Remove unneeded $TST_INIT_NETNS variable") Signed-off-by: Petr Vorel <pvorel@suse.cz> --- testscripts/network.sh | 7 ------- 1 file changed, 7 deletions(-)