[LTP] [PATCH 0/9] tst_net.sh fixes + cleanup

[LTP] [PATCH 0/9] tst_net.sh fixes + cleanup

[Petr Vorel]

Hi,

first two commits should be merged before release (simple fixes),
the rest can wait after the release.

Kind regards,
Petr

Petr Vorel (8):
  tst_net.sh: Use tst_res_()
  tst_net.sh: Check for disabled IPv6 in legacy tests
  tst_net.sh: Remove unneeded $_tst_net_parse_variables
  tst_net.sh: Remove unneeded $TST_INIT_NETNS variable
  tst_net.sh: Rename function + add doc
  tst_net.sh: Remove unused variable ret
  tst_net.sh: Harden on disabled IPv6 via sysctl
  tst_net.sh: Move net setup into separate function

Wei Gao (1):
  tst_net.sh: Add more tst_require_cmds check

 testcases/lib/tst_net.sh | 148 ++++++++++++++++++++++++---------------
 1 file changed, 93 insertions(+), 55 deletions(-)

-- 
2.39.1


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

[LTP] [PATCH 1/9] tst_net.sh: Use tst_res_()

[Petr Vorel]

To avoid error on legacy network tests:

./tcpdump01.sh
Usage: tst_res TTYPE FNAME STRING
	TTYPE  - Test Result Type; one of TFAIL, TBROK and  TCONF.
	FNAME  - Print contents of this file after the message
	STRING - Message explaining the test result
tcpdump01 1 TINFO: initialize 'lhost' 'ltp_ns_veth2' interface

NOTE: only check in tst_net_detect_ipv6() requires to call tst_res_(),
but it's better to be consistent.

Fixes: ecc29beeda ("tst_net.sh: Harden library against deadlocks")
Signed-off-by: Petr Vorel <pvorel@suse.cz>
---
 testcases/lib/tst_net.sh | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/testcases/lib/tst_net.sh b/testcases/lib/tst_net.sh
index e6bb09d81f..925cf50e1c 100644
--- a/testcases/lib/tst_net.sh
+++ b/testcases/lib/tst_net.sh
@@ -74,15 +74,15 @@ tst_net_setup()
 }
 
 if [ "$TST_PARSE_ARGS_CALLER" = "$TST_PARSE_ARGS" ]; then
-	tst_res TWARN "TST_PARSE_ARGS_CALLER same as TST_PARSE_ARGS, unset it ($TST_PARSE_ARGS)"
+	tst_res_ TWARN "TST_PARSE_ARGS_CALLER same as TST_PARSE_ARGS, unset it ($TST_PARSE_ARGS)"
 	unset TST_PARSE_ARGS_CALLER
 fi
 if [ "$TST_SETUP_CALLER" = "$TST_SETUP" ]; then
-	tst_res TWARN "TST_SETUP_CALLER same as TST_SETUP, unset it ($TST_SETUP)"
+	tst_res_ TWARN "TST_SETUP_CALLER same as TST_SETUP, unset it ($TST_SETUP)"
 	unset TST_SETUP_CALLER
 fi
 if [ "$TST_USAGE_CALLER" = "$TST_USAGE" ]; then
-	tst_res TWARN "TST_USAGE_CALLER same as TST_USAGE, unset it ($TST_USAGE)"
+	tst_res_ TWARN "TST_USAGE_CALLER same as TST_USAGE, unset it ($TST_USAGE)"
 	unset TST_USAGE_CALLER
 fi
 
@@ -117,7 +117,7 @@ tst_net_detect_ipv6()
 		TST_NET_IPV6_ENABLED=1
 	else
 		TST_NET_IPV6_ENABLED=0
-		tst_res TINFO "IPv6 disabled on $type"
+		tst_res_ TINFO "IPv6 disabled on $type"
 	fi
 }
 
-- 
2.39.1


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

Re: [LTP] [PATCH 1/9] tst_net.sh: Use tst_res_()

[Petr Vorel]

Hi,

merged slightly modified (move tst_res_() definition before it's use)
+ merge another commit: s/tst_brk/tst_brk_/.

Kind regards,
Petr

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

[LTP] [PATCH 2/9] tst_net.sh: Check for disabled IPv6 in legacy tests

[Petr Vorel]

Before tests timeout:

    # ./tcpdump01.sh -6

    tcpdump01 1 TINFO: initialize 'lhost' 'ltp_ns_veth2' interface
    tcpdump01 1 TINFO: add local addr 10.0.0.2/24
    tcpdump01 1 TINFO: initialize 'rhost' 'ltp_ns_veth1' interface
    tcpdump01 1 TINFO: add remote addr 10.0.0.1/24
    tcpdump01 1 TINFO: Network config (local -- remote):
    tcpdump01 1 TINFO: ltp_ns_veth2 -- ltp_ns_veth1
    tcpdump01 1 TINFO: 10.0.0.2/24 -- 10.0.0.1/24
    tcpdump01 1 TINFO: fd00:1:1:1::2/64/ -- fd00:1:1:1::1/64/
    tcpdump01 1 TINFO: start ping6 in background
    tcpdump01 1 TINFO: running tcpdump...
    (timeout)

With this patch:
    tcpdump01 1 TINFO: initialize 'lhost' 'ltp_ns_veth2' interface
    ...
    tcpdump01 1 TCONF: IPv6 disabled

Signed-off-by: Petr Vorel <pvorel@suse.cz>
---
 testcases/lib/tst_net.sh | 1 +
 1 file changed, 1 insertion(+)

diff --git a/testcases/lib/tst_net.sh b/testcases/lib/tst_net.sh
index 925cf50e1c..b76734210a 100644
--- a/testcases/lib/tst_net.sh
+++ b/testcases/lib/tst_net.sh
@@ -1094,6 +1094,7 @@ export NS_ICMPV6_SENDER_DATA_MAXSIZE=1452
 # in the following document: testcases/network/stress/README
 
 if [ -n "$TST_USE_LEGACY_API" ]; then
+	[ "$TST_IPVER" = 6 ] && tst_net_require_ipv6
 	tst_net_remote_tmpdir
 fi
 
-- 
2.39.1


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

Re: [LTP] [PATCH 2/9] tst_net.sh: Check for disabled IPv6 in legacy tests

[Petr Vorel]

Hi,

merged, with change: using [ "$TST_IPV6" ] instead of [ "$TST_IPVER" = 6 ] in
both cases where tst_net_require_ipv6 is used.

Kind regards,
Petr

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

[LTP] [PATCH 3/9] tst_net.sh: Remove unneeded $_tst_net_parse_variables

[Petr Vorel]

$_tst_net_parse_variables guarder (originally introduced as
$TST_PARSE_VARIABLES in 9aab3ffd55) was not needed any more since
a93c34f914 which introduced $TST_LIB_NET_LOADED guarder at the top of
the file.

was used as a guarder to protect running
configuration in case tst_net.sh was loaded more times.

Fixes: a93c34f914 ("tst_net.sh: Introduce TST_LIB_NET_LOADED")
Signed-off-by: Petr Vorel <pvorel@suse.cz>
---
 testcases/lib/tst_net.sh | 54 ++++++++++++++++++----------------------
 1 file changed, 24 insertions(+), 30 deletions(-)

diff --git a/testcases/lib/tst_net.sh b/testcases/lib/tst_net.sh
index b76734210a..1b3a7e6683 100644
--- a/testcases/lib/tst_net.sh
+++ b/testcases/lib/tst_net.sh
@@ -1005,47 +1005,41 @@ IPV6_RHOST="${IPV6_RHOST:-fd00:1:1:1::1/64}"
 # tst_net_ip_prefix -h
 # tst_net_iface_prefix -h
 # tst_net_vars -h
-if [ -z "$_tst_net_parse_variables" ]; then
-	eval $(tst_net_ip_prefix $IPV4_LHOST || echo "exit $?")
-	eval $(tst_net_ip_prefix -r $IPV4_RHOST || echo "exit $?")
+eval $(tst_net_ip_prefix $IPV4_LHOST || echo "exit $?")
+eval $(tst_net_ip_prefix -r $IPV4_RHOST || echo "exit $?")
 
-	[ "$TST_NET_IPV6_ENABLED" = 1 ] && tst_net_detect_ipv6 rhost
+[ "$TST_NET_IPV6_ENABLED" = 1 ] && tst_net_detect_ipv6 rhost
 
-	if [ "$TST_NET_IPV6_ENABLED" = 1 ]; then
-		eval $(tst_net_ip_prefix $IPV6_LHOST || echo "exit $?")
-		eval $(tst_net_ip_prefix -r $IPV6_RHOST || echo "exit $?")
-	fi
+if [ "$TST_NET_IPV6_ENABLED" = 1 ]; then
+	eval $(tst_net_ip_prefix $IPV6_LHOST || echo "exit $?")
+	eval $(tst_net_ip_prefix -r $IPV6_RHOST || echo "exit $?")
 fi
 
 [ -n "$TST_USE_NETNS" -a "$TST_INIT_NETNS" != "no" ] && init_ltp_netspace
 
-if [ -z "$_tst_net_parse_variables" ]; then
-	eval $(tst_net_iface_prefix $IPV4_LHOST || echo "exit $?")
-	eval $(tst_rhost_run -c 'tst_net_iface_prefix -r '$IPV4_RHOST \
-		|| echo "exit $?")
-
-	if [ "$TST_NET_IPV6_ENABLED" = 1 ]; then
-		eval $(tst_net_iface_prefix $IPV6_LHOST || echo "exit $?")
-		eval $(tst_rhost_run -c 'tst_net_iface_prefix -r '$IPV6_RHOST \
-			|| echo "exit $?")
-	fi
-
-	eval $(tst_net_vars $IPV4_LHOST/$IPV4_LPREFIX \
-		$IPV4_RHOST/$IPV4_RPREFIX || echo "exit $?")
+eval $(tst_net_iface_prefix $IPV4_LHOST || echo "exit $?")
+eval $(tst_rhost_run -c 'tst_net_iface_prefix -r '$IPV4_RHOST \
+	|| echo "exit $?")
 
-	if [ "$TST_NET_IPV6_ENABLED" = 1 ]; then
-		eval $(tst_net_vars $IPV6_LHOST/$IPV6_LPREFIX \
-			$IPV6_RHOST/$IPV6_RPREFIX || echo "exit $?")
-	fi
+if [ "$TST_NET_IPV6_ENABLED" = 1 ]; then
+	eval $(tst_net_iface_prefix $IPV6_LHOST || echo "exit $?")
+	eval $(tst_rhost_run -c 'tst_net_iface_prefix -r '$IPV6_RHOST \
+		|| echo "exit $?")
+fi
 
-	tst_res_ TINFO "Network config (local -- remote):"
-	tst_res_ TINFO "$LHOST_IFACES -- $RHOST_IFACES"
-	tst_res_ TINFO "$IPV4_LHOST/$IPV4_LPREFIX -- $IPV4_RHOST/$IPV4_RPREFIX"
-	tst_res_ TINFO "$IPV6_LHOST/$IPV6_LPREFIX -- $IPV6_RHOST/$IPV6_RPREFIX"
+eval $(tst_net_vars $IPV4_LHOST/$IPV4_LPREFIX \
+	$IPV4_RHOST/$IPV4_RPREFIX || echo "exit $?")
 
-	export _tst_net_parse_variables="yes"
+if [ "$TST_NET_IPV6_ENABLED" = 1 ]; then
+	eval $(tst_net_vars $IPV6_LHOST/$IPV6_LPREFIX \
+		$IPV6_RHOST/$IPV6_RPREFIX || echo "exit $?")
 fi
 
+tst_res_ TINFO "Network config (local -- remote):"
+tst_res_ TINFO "$LHOST_IFACES -- $RHOST_IFACES"
+tst_res_ TINFO "$IPV4_LHOST/$IPV4_LPREFIX -- $IPV4_RHOST/$IPV4_RPREFIX"
+tst_res_ TINFO "$IPV6_LHOST/$IPV6_LPREFIX -- $IPV6_RHOST/$IPV6_RPREFIX"
+
 export TST_NET_DATAROOT="$LTPROOT/testcases/bin/datafiles"
 
 export TST_NETLOAD_CLN_REQUESTS="${TST_NETLOAD_CLN_REQUESTS:-10000}"
-- 
2.39.1


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

[LTP] [PATCH 4/9] tst_net.sh: Remove unneeded $TST_INIT_NETNS variable

[Petr Vorel]

Similarly to the previous commit, also $TST_INIT_NETNS guarder is not
needed since a93c34f914 which introduced $TST_LIB_NET_LOADED guarder at
the top of the file.

Also use tst_net_use_netns() instead of the variable (encapsulation).

Fixes: a93c34f914 ("tst_net.sh: Introduce TST_LIB_NET_LOADED")

Signed-off-by: Petr Vorel <pvorel@suse.cz>
---
 testcases/lib/tst_net.sh | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/testcases/lib/tst_net.sh b/testcases/lib/tst_net.sh
index 1b3a7e6683..425f0b8b7e 100644
--- a/testcases/lib/tst_net.sh
+++ b/testcases/lib/tst_net.sh
@@ -150,8 +150,6 @@ init_ltp_netspace()
 	LHOST_IFACES="${LHOST_IFACES:-ltp_ns_veth2}"
 	RHOST_IFACES="${RHOST_IFACES:-ltp_ns_veth1}"
 
-	export TST_INIT_NETNS="no"
-
 	pid="$(echo $(readlink /var/run/netns/ltp_ns) | cut -f3 -d'/')"
 	export LTP_NETNS="${LTP_NETNS:-ns_exec $pid net,mnt}"
 
@@ -1015,7 +1013,7 @@ if [ "$TST_NET_IPV6_ENABLED" = 1 ]; then
 	eval $(tst_net_ip_prefix -r $IPV6_RHOST || echo "exit $?")
 fi
 
-[ -n "$TST_USE_NETNS" -a "$TST_INIT_NETNS" != "no" ] && init_ltp_netspace
+tst_net_use_netns && init_ltp_netspace
 
 eval $(tst_net_iface_prefix $IPV4_LHOST || echo "exit $?")
 eval $(tst_rhost_run -c 'tst_net_iface_prefix -r '$IPV4_RHOST \
-- 
2.39.1


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

[LTP] [PATCH 5/9] tst_net.sh: Add more tst_require_cmds check

[Petr Vorel]

From: Wei Gao <wegao@suse.com>

More strict check for ns_xxx etc will help avoid following issue:
https://github.com/linux-test-project/ltp/issues/991

Suggested-by: Petr Vorel <pvorel@suse.cz>
Signed-off-by: Wei Gao <wegao@suse.com>
[ removed checks in tst_rhost_run() - need to be fixed differently,
check for more tools ]
Signed-off-by: Petr Vorel <pvorel@suse.cz>
---
 testcases/lib/tst_net.sh | 1 +
 1 file changed, 1 insertion(+)

diff --git a/testcases/lib/tst_net.sh b/testcases/lib/tst_net.sh
index 425f0b8b7e..6cb9f02a5f 100644
--- a/testcases/lib/tst_net.sh
+++ b/testcases/lib/tst_net.sh
@@ -1003,6 +1003,7 @@ IPV6_RHOST="${IPV6_RHOST:-fd00:1:1:1::1/64}"
 # tst_net_ip_prefix -h
 # tst_net_iface_prefix -h
 # tst_net_vars -h
+tst_require_cmds tst_net_iface_prefix tst_net_ip_prefix tst_net_vars
 eval $(tst_net_ip_prefix $IPV4_LHOST || echo "exit $?")
 eval $(tst_net_ip_prefix -r $IPV4_RHOST || echo "exit $?")
 
-- 
2.39.1


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

Re: [LTP] [PATCH 5/9] tst_net.sh: Add more tst_require_cmds check

[Petr Vorel]

Hi all,

merged up to this patch (3 more are missing + I'll send one more extra).

Kind regards,
Petr

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

[LTP] [PATCH 6/9] tst_net.sh: Rename function + add doc

[Petr Vorel]

Rename tst_net_detect_ipv6() to tst_net_detect_ipv6_cmdline() because
there will be another function to detect IPv6 disabled via sysctl.

Signed-off-by: Petr Vorel <pvorel@suse.cz>
---
 testcases/lib/tst_net.sh | 9 ++++++---
 1 file changed, 6 insertions(+), 3 deletions(-)

diff --git a/testcases/lib/tst_net.sh b/testcases/lib/tst_net.sh
index 6cb9f02a5f..3a9c16f070 100644
--- a/testcases/lib/tst_net.sh
+++ b/testcases/lib/tst_net.sh
@@ -100,7 +100,10 @@ tst_brk_()
 	[ -z "$TST_USE_LEGACY_API" ] && tst_brk $@ || tst_brkm $@
 }
 
-tst_net_detect_ipv6()
+# detect IPv6 not disabled via ipv6.disable=1 kernel cmdline parameter
+# or via CONFIG_IPV6=y kernel configure option
+# $TST_NET_IPV6_ENABLED: 1 (enabled), 0 (disabled)
+tst_net_detect_ipv6_cmdline()
 {
 	local type="${1:-lhost}"
 	local cmd='[ -f /proc/net/if_inet6 ]'
@@ -970,7 +973,7 @@ tst_default_max_pkt()
 [ -n "$TST_USE_LEGACY_API" ] && . test.sh || . tst_test.sh
 
 # detect IPv6 support on lhost for tests which don't use test links
-tst_net_detect_ipv6
+tst_net_detect_ipv6_cmdline
 
 [ -n "$TST_NET_SKIP_VARIABLE_INIT" ] && return 0
 
@@ -1007,7 +1010,7 @@ tst_require_cmds tst_net_iface_prefix tst_net_ip_prefix tst_net_vars
 eval $(tst_net_ip_prefix $IPV4_LHOST || echo "exit $?")
 eval $(tst_net_ip_prefix -r $IPV4_RHOST || echo "exit $?")
 
-[ "$TST_NET_IPV6_ENABLED" = 1 ] && tst_net_detect_ipv6 rhost
+[ "$TST_NET_IPV6_ENABLED" = 1 ] && tst_net_detect_ipv6_cmdline rhost
 
 if [ "$TST_NET_IPV6_ENABLED" = 1 ]; then
 	eval $(tst_net_ip_prefix $IPV6_LHOST || echo "exit $?")
-- 
2.39.1


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

Re: [LTP] [PATCH 6/9] tst_net.sh: Rename function + add doc

[Cyril Hrubis]

Hi!
What I didn't get here is why do we put the code to detect IPV6 into two
different functions and then call it twice nearly from the same place in
the library. Is there a reason why can't we put the sysctl check right
after the proc check in the current ipv6 detection function?

-- 
Cyril Hrubis
chrubis@suse.cz

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

Re: [LTP] [PATCH 6/9] tst_net.sh: Rename function + add doc

[Petr Vorel]

Hi Cyril,

first, thanks for you review!

> Hi!
> What I didn't get here is why do we put the code to detect IPV6 into two
> different functions and then call it twice nearly from the same place in
> the library. Is there a reason why can't we put the sysctl check right
> after the proc check in the current ipv6 detection function?

Well, the old check (cmdline check for ipv6.disable=1) must be run before
creating interfaces. But the new sysctl based tests test interfaces
(${L,R}HOST_IFACES).  Technically it could be run just after ipv6.disable=1
check, because it's needed only for the non-default ssh based two host setup [1].
But ${L,R}HOST_IFACES initialization:

export LHOST_IFACES="${LHOST_IFACES:-eth0}"
export RHOST_IFACES="${RHOST_IFACES:-eth0}"

would have to be moved before this check and run only for ssh based test
(the default netns based single host will create interfaces in init_ltp_netspace()
and for them ipv6.disable=1 check is good enough). I wanted to keep variables in
single place, but I'm ok to add this exception. Or we can keep variables where
they are and run this check on ${LHOST_IFACES:-} and ${RHOST_IFACES:-} (i.e.
only if these interfaces are set externally - which should be only on ssh two
host based setup).

I also thought that having to run this new check separately could be useful
(setup can change any time). But as nobody required that /complained it's
needed, let's ignore this.

Kind regards,
Petr

[1] https://github.com/linux-test-project/ltp/tree/master/testcases/network#single-host-configuration

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

Re: [LTP] [PATCH 6/9] tst_net.sh: Rename function + add doc

[Cyril Hrubis]

Hi!
> > What I didn't get here is why do we put the code to detect IPV6 into two
> > different functions and then call it twice nearly from the same place in
> > the library. Is there a reason why can't we put the sysctl check right
> > after the proc check in the current ipv6 detection function?
> 
> Well, the old check (cmdline check for ipv6.disable=1) must be run before
> creating interfaces. But the new sysctl based tests test interfaces
> (${L,R}HOST_IFACES).  Technically it could be run just after ipv6.disable=1
> check, because it's needed only for the non-default ssh based two host setup [1].
> But ${L,R}HOST_IFACES initialization:
> 
> export LHOST_IFACES="${LHOST_IFACES:-eth0}"
> export RHOST_IFACES="${RHOST_IFACES:-eth0}"
> 
> would have to be moved before this check and run only for ssh based test
> (the default netns based single host will create interfaces in init_ltp_netspace()
> and for them ipv6.disable=1 check is good enough). I wanted to keep variables in
> single place, but I'm ok to add this exception. Or we can keep variables where
> they are and run this check on ${LHOST_IFACES:-} and ${RHOST_IFACES:-} (i.e.
> only if these interfaces are set externally - which should be only on ssh two
> host based setup).
> 
> I also thought that having to run this new check separately could be useful
> (setup can change any time). But as nobody required that /complained it's
> needed, let's ignore this.

Ah, it's more complicated than a single switch, there is actually a
master swtich, default settings and per interface settings.

i.e. /proc/sys/net/ipv6/conf/default/disable_ipv6
     /proc/sys/net/ipv6/conf/all/disable_ipv6
     /proc/sys/net/ipv6/conf/${IFACE}/disable_ipv6

Since the deafult value is I suppose inherited by newly added interfaces
we have to check if ipv6 is disabled by sysctl in the case of network
namespaces as well, right? Technically we could probably check the
default/disable_ipv6 before we attempt to create new interfaces, but I
guess that we woudl rather want to have the same code both for ssh and
namespaces.

-- 
Cyril Hrubis
chrubis@suse.cz

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

Re: [LTP] [PATCH 6/9] tst_net.sh: Rename function + add doc

[Petr Vorel]

> Hi!
> > > What I didn't get here is why do we put the code to detect IPV6 into two
> > > different functions and then call it twice nearly from the same place in
> > > the library. Is there a reason why can't we put the sysctl check right
> > > after the proc check in the current ipv6 detection function?

> > Well, the old check (cmdline check for ipv6.disable=1) must be run before
> > creating interfaces. But the new sysctl based tests test interfaces
> > (${L,R}HOST_IFACES).  Technically it could be run just after ipv6.disable=1
> > check, because it's needed only for the non-default ssh based two host setup [1].
> > But ${L,R}HOST_IFACES initialization:

> > export LHOST_IFACES="${LHOST_IFACES:-eth0}"
> > export RHOST_IFACES="${RHOST_IFACES:-eth0}"

> > would have to be moved before this check and run only for ssh based test
> > (the default netns based single host will create interfaces in init_ltp_netspace()
> > and for them ipv6.disable=1 check is good enough). I wanted to keep variables in
> > single place, but I'm ok to add this exception. Or we can keep variables where
> > they are and run this check on ${LHOST_IFACES:-} and ${RHOST_IFACES:-} (i.e.
> > only if these interfaces are set externally - which should be only on ssh two
> > host based setup).

> > I also thought that having to run this new check separately could be useful
> > (setup can change any time). But as nobody required that /complained it's
> > needed, let's ignore this.

> Ah, it's more complicated than a single switch, there is actually a
> master swtich, default settings and per interface settings.

> i.e. /proc/sys/net/ipv6/conf/default/disable_ipv6
>      /proc/sys/net/ipv6/conf/all/disable_ipv6
>      /proc/sys/net/ipv6/conf/${IFACE}/disable_ipv6

> Since the deafult value is I suppose inherited by newly added interfaces
> we have to check if ipv6 is disabled by sysctl in the case of network
> namespaces as well, right? Technically we could probably check the
> default/disable_ipv6 before we attempt to create new interfaces, but I
> guess that we would rather want to have the same code both for ssh and
> namespaces.

Good catch, yes, conf/default/disable_ipv6 is inherited, thus
mandatory for check altogether with conf/${IFACE}/disable_ipv6 
(for ${LHOST_IFACES:-} and ${RHOST_IFACES:-} - ssh).
We can ignore conf/all/disable_ipv6 [1]:

	Reading this value does not have any particular meaning. It does not say
	whether IPv6 support is enabled or disabled. Returned value can be 1 also in
	the case when some interface has disable_ipv6 set to 0 and has configured
	IPv6 addresses.

I'd add this check before calling init_ltp_netspace(). That expects:
1) nobody changes this setup (one day we may want to test this functionality,
thus even LTP itself can change this setup) 2) any NIC without IPv6 means no
IPv6 testing, right?

BTW at least network tests which use virt_lib.sh requires IPv6 for IPv4 tests.
ip6_virt_remote="$(TST_IPV6=6 tst_ipaddr_un rhost)"
which is then used in virt_setup():

# ./macsec02.sh
macsec02 1 TINFO: setup IPsec transport/esp_aead des3_ede
macsec02 1 TINFO: setup local macsec with 'icvlen 16 encodingsa 0 replay on window 300 encrypt on protect on'
macsec02 1 TINFO: setup rhost macsec with 'icvlen 16 encodingsa 0 replay on window 300 encrypt on protect on'
RTNETLINK answers: Operation not supported
macsec02 1 TBROK: ip addr add ::2/64 dev ltp_v0 nodad failed

# ./vlan03.sh
RTNETLINK answers: Operation not supported
vlan03 1 TBROK: ip addr add ::2/64 dev ltp_v0 nodad failed

These tests would have to be either split to IPv4 (default) and IPv6 (-6)
or entirely depend on IPv6 (obviously first is better, but more work).

Kind regards,
Petr

[1] https://docs.kernel.org/networking/ip-sysctl.html

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

Re: [LTP] [PATCH 6/9] tst_net.sh: Rename function + add doc

[Petr Vorel]

Hi Cyril,

...
> Ah, it's more complicated than a single switch, there is actually a
> master swtich, default settings and per interface settings.

> i.e. /proc/sys/net/ipv6/conf/default/disable_ipv6
>      /proc/sys/net/ipv6/conf/all/disable_ipv6
>      /proc/sys/net/ipv6/conf/${IFACE}/disable_ipv6

> Since the deafult value is I suppose inherited by newly added interfaces
> we have to check if ipv6 is disabled by sysctl in the case of network
> namespaces as well, right? Technically we could probably check the
> default/disable_ipv6 before we attempt to create new interfaces, but I
> guess that we woudl rather want to have the same code both for ssh and
> namespaces.

In the end I decided to add check for net.ipv6.conf.all.disable_ipv6=1 in
tst_init_iface() (calls ip link set ...). This way it should be tested only when
needed. That means we need 2 functions.

I wonder if it still makes sense to check net.ipv6.conf.$iface.disable_ipv6
(IMHO needed for two host based setup with IPv6 non-friendly configuration).
I'd be for this check, but it could be in function which checks cmdline.

Kind regards,
Petr

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

[LTP] [PATCH 7/9] tst_net.sh: Remove unused variable ret

[Petr Vorel]

$? is enough in tst_net_detect_ipv6_cmdline().

Signed-off-by: Petr Vorel <pvorel@suse.cz>
---
 testcases/lib/tst_net.sh | 9 +++------
 1 file changed, 3 insertions(+), 6 deletions(-)

diff --git a/testcases/lib/tst_net.sh b/testcases/lib/tst_net.sh
index 3a9c16f070..518a3ed579 100644
--- a/testcases/lib/tst_net.sh
+++ b/testcases/lib/tst_net.sh
@@ -100,23 +100,20 @@ tst_brk_()
 	[ -z "$TST_USE_LEGACY_API" ] && tst_brk $@ || tst_brkm $@
 }
 
-# detect IPv6 not disabled via ipv6.disable=1 kernel cmdline parameter
-# or via CONFIG_IPV6=y kernel configure option
-# $TST_NET_IPV6_ENABLED: 1 (enabled), 0 (disabled)
+# Detect IPv6 disabled via ipv6.disable=1 kernel cmdline parameter
+# or via CONFIG_IPV6=y kernel configure option.
 tst_net_detect_ipv6_cmdline()
 {
 	local type="${1:-lhost}"
 	local cmd='[ -f /proc/net/if_inet6 ]'
-	local ret
 
 	if [ "$type" = "lhost" ]; then
 		$cmd
 	else
 		tst_rhost_run -c "$cmd"
 	fi
-	ret=$?
 
-	if [ $ret -eq 0 ]; then
+	if [ $? -eq 0 ]; then
 		TST_NET_IPV6_ENABLED=1
 	else
 		TST_NET_IPV6_ENABLED=0
-- 
2.39.1


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

[LTP] [PATCH 8/9] tst_net.sh: Harden on disabled IPv6 via sysctl

[Petr Vorel]

Detect IPv6 can be disabled via sysctl (for both netns or on ssh based
links) and tests than fails:

    # sysctl net.ipv6.conf.ltp_ns_veth2.disable_ipv6=1
    # TST_NET_RHOST_RUN_DEBUG=1 ./ping02.sh -6
    ping02 1 TINFO: tst_rhost_run: cmd: [ -f /proc/net/if_inet6 ]
    ping02 1 TINFO: NETNS: sh -c " [ -f /proc/net/if_inet6 ] || echo RTERR" 2>&1
    ping02 1 TINFO: initialize 'lhost' 'ltp_ns_veth2' interface
    ping02 1 TINFO: add local addr 10.0.0.2/24
    ping02 1 TINFO: add local addr fd00:1:1:1::2/64
    RTNETLINK answers: Permission denied
    ping02 1 TINFO: initialize 'rhost' 'ltp_ns_veth1' interface
    ...
    ping02 1 TINFO: timeout per run is 0h 5m 0s
    ping6: connect: Network is unreachable
    ping02 1 TFAIL: ping6 -I ltp_ns_veth2 -c 3 -s 8 -f -p 000102030405060708090a0b0c0d0e0f fd00:1:1:1::1 >/dev/null failed unexpectedly

Therefore detect IPv6 disabled via sysctl:
1) tst_init_iface - to avoid setting net.ipv6.conf.$iface.accept_dad=0.
tst_init_iface is called by tst_restore_ipaddr, which is called by
init_ltp_netspace (netns) at the start of testing.
2) For ssh are all devices tested at the start.

There are guards in both functions to avoid repeated checks (e.g.
tst_restore_ipaddr() it's also cleanup function for some tests therefore
called by both netns and ssh).

Signed-off-by: Petr Vorel <pvorel@suse.cz>
---
 testcases/lib/tst_net.sh | 40 ++++++++++++++++++++++++++++++++++++++++
 1 file changed, 40 insertions(+)

diff --git a/testcases/lib/tst_net.sh b/testcases/lib/tst_net.sh
index 518a3ed579..7ee8594a54 100644
--- a/testcases/lib/tst_net.sh
+++ b/testcases/lib/tst_net.sh
@@ -126,6 +126,43 @@ tst_net_require_ipv6()
 	[ "$TST_NET_IPV6_ENABLED" = 1 ] || tst_brk_ TCONF "IPv6 disabled"
 }
 
+# Detect IPv6 disabled via sysctl.
+tst_net_detect_ipv6_sysctl()
+{
+	[ "$TST_NET_IPV6_ENABLED" = 1 ] || return 1
+
+	local iface="$1"
+	local type="${2:-lhost}"
+	local check="sysctl -n net.ipv6.conf.$iface.disable_ipv6"
+	local disabled
+
+	if [ "$type" = "lhost" ]; then
+		disabled=$($check)
+	else
+		disabled=$(tst_rhost_run -c "$check")
+	fi
+	if [ $disabled = 1 ]; then
+		tst_res_ TINFO "IPv6 disabled on $type on $iface"
+		TST_NET_IPV6_ENABLED=0
+		return 1
+	fi
+
+	return 0
+}
+
+tst_net_check_ifaces_ipv6()
+{
+	local iface
+
+	for iface in $(tst_get_ifaces); do
+		tst_net_detect_ipv6_sysctl || return
+	done
+
+	for iface in $(tst_get_ifaces rhost); do
+		tst_net_detect_ipv6_sysctl $iface rhost || return
+	done
+}
+
 init_ltp_netspace()
 {
 	local pid
@@ -531,7 +568,9 @@ tst_init_iface()
 	local type="${1:-lhost}"
 	local link_num="${2:-0}"
 	local iface="$(tst_iface $type $link_num)"
+
 	tst_res_ TINFO "initialize '$type' '$iface' interface"
+	tst_net_detect_ipv6_sysctl $iface $type
 
 	if [ "$type" = "lhost" ]; then
 		if ip xfrm state 1>/dev/null 2>&1; then
@@ -1015,6 +1054,7 @@ if [ "$TST_NET_IPV6_ENABLED" = 1 ]; then
 fi
 
 tst_net_use_netns && init_ltp_netspace
+tst_net_check_ifaces_ipv6
 
 eval $(tst_net_iface_prefix $IPV4_LHOST || echo "exit $?")
 eval $(tst_rhost_run -c 'tst_net_iface_prefix -r '$IPV4_RHOST \
-- 
2.39.1


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

[LTP] [PATCH 9/9] tst_net.sh: Move net setup into separate function

[Petr Vorel]

Having network setup in separate function helps readability.
Test environment variables are for readability kept outside.

Signed-off-by: Petr Vorel <pvorel@suse.cz>
---
 testcases/lib/tst_net.sh | 118 ++++++++++++++++++++-------------------
 1 file changed, 61 insertions(+), 57 deletions(-)

diff --git a/testcases/lib/tst_net.sh b/testcases/lib/tst_net.sh
index 7ee8594a54..490909907a 100644
--- a/testcases/lib/tst_net.sh
+++ b/testcases/lib/tst_net.sh
@@ -1006,6 +1006,64 @@ tst_default_max_pkt()
 	echo "$((mtu + mtu / 10))"
 }
 
+# Setup LTP network.
+# Used tools:
+# * tst_net_ip_prefix
+# Strip prefix from IP address and save both If no prefix found sets
+# default prefix.
+#
+# * tst_net_iface_prefix reads prefix and interface from rtnetlink.
+# If nothing found sets default prefix value.
+#
+# * tst_net_vars exports environment variables related to test links and
+# networks that aren't reachable through the test links.
+#
+# For full list of exported environment variables see:
+# tst_net_ip_prefix -h
+# tst_net_iface_prefix -h
+# tst_net_vars -h
+tst_net_setup_network()
+{
+	tst_require_cmds tst_net_iface_prefix tst_net_ip_prefix tst_net_vars
+
+	eval $(tst_net_ip_prefix $IPV4_LHOST || echo "exit $?")
+	eval $(tst_net_ip_prefix -r $IPV4_RHOST || echo "exit $?")
+
+	[ "$TST_NET_IPV6_ENABLED" = 1 ] && tst_net_detect_ipv6_cmdline rhost
+
+	if [ "$TST_NET_IPV6_ENABLED" = 1 ]; then
+		eval $(tst_net_ip_prefix $IPV6_LHOST || echo "exit $?")
+		eval $(tst_net_ip_prefix -r $IPV6_RHOST || echo "exit $?")
+	fi
+
+	tst_net_use_netns && init_ltp_netspace
+	tst_net_check_ifaces_ipv6
+
+	eval $(tst_net_iface_prefix $IPV4_LHOST || echo "exit $?")
+	eval $(tst_rhost_run -c 'tst_net_iface_prefix -r '$IPV4_RHOST \
+		|| echo "exit $?")
+	eval $(tst_net_vars $IPV4_LHOST/$IPV4_LPREFIX \
+		$IPV4_RHOST/$IPV4_RPREFIX || echo "exit $?")
+
+	if [ "$TST_NET_IPV6_ENABLED" = 1 ]; then
+		eval $(tst_net_iface_prefix $IPV6_LHOST || echo "exit $?")
+		eval $(tst_rhost_run -c 'tst_net_iface_prefix -r '$IPV6_RHOST \
+			|| echo "exit $?")
+		eval $(tst_net_vars $IPV6_LHOST/$IPV6_LPREFIX \
+			$IPV6_RHOST/$IPV6_RPREFIX || echo "exit $?")
+	fi
+
+	tst_res_ TINFO "Network config (local -- remote):"
+	tst_res_ TINFO "$LHOST_IFACES -- $RHOST_IFACES"
+	tst_res_ TINFO "$IPV4_LHOST/$IPV4_LPREFIX -- $IPV4_RHOST/$IPV4_RPREFIX"
+	tst_res_ TINFO "$IPV6_LHOST/$IPV6_LPREFIX -- $IPV6_RHOST/$IPV6_RPREFIX"
+
+	if [ -n "$TST_USE_LEGACY_API" ]; then
+		tst_net_remote_tmpdir
+		[ "$TST_IPVER" = 6 ] && tst_net_require_ipv6
+	fi
+}
+
 [ -n "$TST_USE_LEGACY_API" ] && . test.sh || . tst_test.sh
 
 # detect IPv6 support on lhost for tests which don't use test links
@@ -1028,56 +1086,10 @@ IPV4_RHOST="${IPV4_RHOST:-10.0.0.1/24}"
 IPV6_LHOST="${IPV6_LHOST:-fd00:1:1:1::2/64}"
 IPV6_RHOST="${IPV6_RHOST:-fd00:1:1:1::1/64}"
 
-# tst_net_ip_prefix
-# Strip prefix from IP address and save both If no prefix found sets
-# default prefix.
-#
-# tst_net_iface_prefix reads prefix and interface from rtnetlink.
-# If nothing found sets default prefix value.
-#
-# tst_net_vars exports environment variables related to test links and
-# networks that aren't reachable through the test links.
-#
-# For full list of exported environment variables see:
-# tst_net_ip_prefix -h
-# tst_net_iface_prefix -h
-# tst_net_vars -h
-tst_require_cmds tst_net_iface_prefix tst_net_ip_prefix tst_net_vars
-eval $(tst_net_ip_prefix $IPV4_LHOST || echo "exit $?")
-eval $(tst_net_ip_prefix -r $IPV4_RHOST || echo "exit $?")
-
-[ "$TST_NET_IPV6_ENABLED" = 1 ] && tst_net_detect_ipv6_cmdline rhost
-
-if [ "$TST_NET_IPV6_ENABLED" = 1 ]; then
-	eval $(tst_net_ip_prefix $IPV6_LHOST || echo "exit $?")
-	eval $(tst_net_ip_prefix -r $IPV6_RHOST || echo "exit $?")
-fi
-
-tst_net_use_netns && init_ltp_netspace
-tst_net_check_ifaces_ipv6
-
-eval $(tst_net_iface_prefix $IPV4_LHOST || echo "exit $?")
-eval $(tst_rhost_run -c 'tst_net_iface_prefix -r '$IPV4_RHOST \
-	|| echo "exit $?")
-
-if [ "$TST_NET_IPV6_ENABLED" = 1 ]; then
-	eval $(tst_net_iface_prefix $IPV6_LHOST || echo "exit $?")
-	eval $(tst_rhost_run -c 'tst_net_iface_prefix -r '$IPV6_RHOST \
-		|| echo "exit $?")
-fi
-
-eval $(tst_net_vars $IPV4_LHOST/$IPV4_LPREFIX \
-	$IPV4_RHOST/$IPV4_RPREFIX || echo "exit $?")
-
-if [ "$TST_NET_IPV6_ENABLED" = 1 ]; then
-	eval $(tst_net_vars $IPV6_LHOST/$IPV6_LPREFIX \
-		$IPV6_RHOST/$IPV6_RPREFIX || echo "exit $?")
-fi
+tst_net_setup_network
 
-tst_res_ TINFO "Network config (local -- remote):"
-tst_res_ TINFO "$LHOST_IFACES -- $RHOST_IFACES"
-tst_res_ TINFO "$IPV4_LHOST/$IPV4_LPREFIX -- $IPV4_RHOST/$IPV4_RPREFIX"
-tst_res_ TINFO "$IPV6_LHOST/$IPV6_LPREFIX -- $IPV6_RHOST/$IPV6_RPREFIX"
+# More information about network parameters can be found
+# in the following document: testcases/network/stress/README
 
 export TST_NET_DATAROOT="$LTPROOT/testcases/bin/datafiles"
 
@@ -1123,14 +1135,6 @@ export RHOST_HWADDRS="${RHOST_HWADDRS:-$(tst_get_hwaddrs rhost)}"
 export NS_ICMPV4_SENDER_DATA_MAXSIZE=1472
 export NS_ICMPV6_SENDER_DATA_MAXSIZE=1452
 
-# More information about network parameters can be found
-# in the following document: testcases/network/stress/README
-
-if [ -n "$TST_USE_LEGACY_API" ]; then
-	[ "$TST_IPVER" = 6 ] && tst_net_require_ipv6
-	tst_net_remote_tmpdir
-fi
-
 if [ -z "$TST_USE_LEGACY_API" ] && ! tst_cmd_available ping6; then
 	ping6()
 	{
-- 
2.39.1


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

Re: [LTP] [PATCH 9/9] tst_net.sh: Move net setup into separate function

[Petr Vorel]

Hi all,

> Having network setup in separate function helps readability.
> Test environment variables are for readability kept outside.
FYI I rebased this (removed new functions) and merged.

Kind regards,
Petr

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