[LTP] [PATCH 1/1] testscripts/network.sh: Don't load tst_net.sh

[LTP] [PATCH 1/1] testscripts/network.sh: Don't load tst_net.sh

[Petr Vorel]

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(-)

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
-- 
2.39.1


-- 
Mailing list info: https://lists.linux.it/listinfo/ltp

Re: [LTP] [PATCH 1/1] testscripts/network.sh: Don't load tst_net.sh

[Petr Vorel]

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

-- 
Mailing list info: https://lists.linux.it/listinfo/ltp

Re: [LTP] [PATCH 1/1] testscripts/network.sh: Don't load tst_net.sh

[Petr Vorel]

> 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

-- 
Mailing list info: https://lists.linux.it/listinfo/ltp

Re: [LTP] [PATCH 1/1] testscripts/network.sh: Don't load tst_net.sh

[Cyril Hrubis]

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.

-- 
Cyril Hrubis
chrubis@suse.cz

-- 
Mailing list info: https://lists.linux.it/listinfo/ltp

Re: [LTP] [PATCH 1/1] testscripts/network.sh: Don't load tst_net.sh

[Petr Vorel]

> 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/

-- 
Mailing list info: https://lists.linux.it/listinfo/ltp

Re: [LTP] [PATCH 1/1] testscripts/network.sh: Don't load tst_net.sh

[Petr Vorel]

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

-- 
Mailing list info: https://lists.linux.it/listinfo/ltp

Re: [LTP] [PATCH 1/1] testscripts/network.sh: Don't load tst_net.sh

[Cyril Hrubis]

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>

-- 
Cyril Hrubis
chrubis@suse.cz

-- 
Mailing list info: https://lists.linux.it/listinfo/ltp

Re: [LTP] [PATCH 1/1] testscripts/network.sh: Don't load tst_net.sh

[Petr Vorel]

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

-- 
Mailing list info: https://lists.linux.it/listinfo/ltp