[LTP] [PATCH v3 1/2] net: Add tst_net_run helper

[LTP] [PATCH v3 1/2] net: Add tst_net_run helper

[Petr Vorel]

and use it in tst_set_sysctl

Signed-off-by: Petr Vorel <pvorel@suse.cz>
---
Changes v2->v3:
* Add -l and -r params and use it in tst_set_sysctl
Escape '>' (ROD foo \> bar.log) can be used for this, but it'd be better
to do it automatically.

Changes v1->v2:
* tst_net_driver removed and created generic command tst_net_run

TODO: For ROD it does not handle escape for >. E.g. it'd be nice to
handle this:
ROD foo \> bar.log
as passing the escape will not work for sh (non-safe local) and for
tst_rhost_run. Do we care?

Kind regards,
Petr
---
 testcases/lib/tst_net.sh | 43 +++++++++++++++++++++++++++++++---------
 1 file changed, 34 insertions(+), 9 deletions(-)

diff --git a/testcases/lib/tst_net.sh b/testcases/lib/tst_net.sh
index d1206e285..aa5a05825 100644
--- a/testcases/lib/tst_net.sh
+++ b/testcases/lib/tst_net.sh
@@ -191,6 +191,37 @@ tst_rhost_run()
 	return $ret
 }
 
+# Run command on both lhost and rhost.
+# tst_net_run [-s] [-l LPARAM] [-r RPARAM] CMD [ARG [ARG2]]
+# LPARAM: parameter passed to $1 in lhost
+# RPARAM: parameter passed to $1 in rhost
+tst_net_run()
+{
+	local cmd
+	local lparams
+	local rparams
+	local safe
+
+	local OPTIND
+	while getopts l:r:s opt; do
+		case "$opt" in
+		l) lparams="$OPTARG" ;;
+		r) rparams="$OPTARG" ;;
+		s) safe="-s" ;;
+		*) tst_brk_ TBROK "tst_net_run: unknown option: $OPTARG" ;;
+		esac
+	done
+	shift $((OPTIND - 1))
+
+	cmd="$1"
+	shift
+	cmd="$cmd $lparams $@"
+	[ -n "$safe" ] && ROD $cmd || sh -c "$cmd"
+
+	cmd="$cmd $rparams $@"
+	tst_rhost_run $safe -c "$cmd"
+}
+
 EXPECT_RHOST_PASS()
 {
 	tst_rhost_run -c "$*" > /dev/null
@@ -642,16 +673,10 @@ tst_set_sysctl()
 	local safe=
 	[ "$3" = "safe" ] && safe="-s"
 
-	local add_opt=
-	[ "$TST_USE_NETNS" = "yes" ] && add_opt="-e"
-
-	if [ "$safe" ]; then
-		ROD sysctl -q -w $name=$value
-	else
-		sysctl -q -w $name=$value
-	fi
+	local rparam=
+	[ "$TST_USE_NETNS" = "yes" ] && rparam="-e"
 
-	tst_rhost_run $safe -c "sysctl -q -w $add_opt $name=$value"
+	tst_net_run $safe -r $rparam sysctl -q -w $name=$value
 }
 
 tst_cleanup_rhost()
-- 
2.19.1

[LTP] [PATCH v3 2/2] net/ipsec: Add check for xfrm_user

[Petr Vorel]

which is required by ip xfrm command.
As ipsec_lib.sh is used for other commands than it,
check it only when used. Call in cleanup function could trigger infinite
loop, therefore check only once.

Signed-off-by: Petr Vorel <pvorel@suse.cz>
---
 testcases/network/stress/ipsec/ipsec_lib.sh | 11 +++++++++++
 1 file changed, 11 insertions(+)

diff --git a/testcases/network/stress/ipsec/ipsec_lib.sh b/testcases/network/stress/ipsec/ipsec_lib.sh
index b099fdeaf..a9ac5bc50 100644
--- a/testcases/network/stress/ipsec/ipsec_lib.sh
+++ b/testcases/network/stress/ipsec/ipsec_lib.sh
@@ -89,6 +89,14 @@ TST_SETUP=${TST_SETUP:-ipsec_lib_setup}
 TST_USAGE=ipsec_lib_usage
 . tst_net.sh
 
+tst_ipsec_check_driver()
+{
+	if [ -z "$ipsec_driver_checked" ]; then
+		export ipsec_driver_checked=1
+		tst_net_run "tst_test_drivers xfrm_user"
+	fi
+}
+
 get_key()
 {
 	local bits=$1
@@ -110,6 +118,7 @@ tst_ipsec_setup()
 # tst_ipsec_cleanup: flush ipsec state and policy rules
 tst_ipsec_cleanup()
 {
+	tst_ipsec_check_driver
 	ip xfrm state flush
 	ip xfrm policy flush
 	tst_rhost_run -c "ip xfrm state flush && ip xfrm policy flush"
@@ -175,6 +184,7 @@ tst_ipsec()
 	local p="proto $IPSEC_PROTO"
 	[ "$IPSEC_PROTO" = "esp_aead" ] && p="proto esp"
 
+	tst_ipsec_check_driver
 	ipsec_set_algoline
 
 	if [ $target = lhost ]; then
@@ -234,6 +244,7 @@ tst_ipsec_vti()
 		tst_brk TCONF "iproute doesn't support 'vti'"
 
 	ipsec_set_algoline
+	tst_ipsec_check_driver
 
 	local o_dir="src $src dst $dst"
 	local i_dir="src $dst dst $src"
-- 
2.19.1

[LTP] [PATCH v3 2/2] net/ipsec: Add check for xfrm_user

[Alexey Kodanev]

On 08.11.2018 17:24, Petr Vorel wrote:
> which is required by ip xfrm command.
> As ipsec_lib.sh is used for other commands than it,
> check it only when used. Call in cleanup function could trigger infinite
> loop, therefore check only once.

What about adding cleanup_xfrm flag to cleanup() instead, something like this:

diff --git a/testcases/network/stress/ipsec/ipsec_lib.sh b/testcases/network/stress/ipsec/ipsec_lib.sh
index b099fde..28d457b 100644
--- a/testcases/network/stress/ipsec/ipsec_lib.sh
+++ b/testcases/network/stress/ipsec/ipsec_lib.sh
@@ -81,6 +81,11 @@ ipsec_lib_setup()
        cleanup_vti=
        ALG=
        ALGR=
+
+       if [ -n "$IPSEC_MODE" ]; then
+               tst_net_run "tst_test_drivers xfrm_user"
+               cleanup_xfrm=1
+       fi
 }
 
 TST_OPTS="l:m:p:s:S:k:A:e:a:c:r:"
@@ -110,6 +115,8 @@ tst_ipsec_setup()
 # tst_ipsec_cleanup: flush ipsec state and policy rules
 tst_ipsec_cleanup()
 {
+       [ -z "$cleanup_xfrm" ] && return
+
        ip xfrm state flush
        ip xfrm policy flush
        tst_rhost_run -c "ip xfrm state flush && ip xfrm policy flush"



> 
> Signed-off-by: Petr Vorel <pvorel@suse.cz>
> ---
>  testcases/network/stress/ipsec/ipsec_lib.sh | 11 +++++++++++
>  1 file changed, 11 insertions(+)
> 
> diff --git a/testcases/network/stress/ipsec/ipsec_lib.sh b/testcases/network/stress/ipsec/ipsec_lib.sh
> index b099fdeaf..a9ac5bc50 100644
> --- a/testcases/network/stress/ipsec/ipsec_lib.sh
> +++ b/testcases/network/stress/ipsec/ipsec_lib.sh
> @@ -89,6 +89,14 @@ TST_SETUP=${TST_SETUP:-ipsec_lib_setup}
>  TST_USAGE=ipsec_lib_usage
>  . tst_net.sh
>  
> +tst_ipsec_check_driver()
> +{
> +	if [ -z "$ipsec_driver_checked" ]; then
> +		export ipsec_driver_checked=1
> +		tst_net_run "tst_test_drivers xfrm_user"
> +	fi
> +}
> +
>  get_key()
>  {
>  	local bits=$1
> @@ -110,6 +118,7 @@ tst_ipsec_setup()
>  # tst_ipsec_cleanup: flush ipsec state and policy rules
>  tst_ipsec_cleanup()
>  {
> +	tst_ipsec_check_driver
>  	ip xfrm state flush
>  	ip xfrm policy flush
>  	tst_rhost_run -c "ip xfrm state flush && ip xfrm policy flush"
> @@ -175,6 +184,7 @@ tst_ipsec()
>  	local p="proto $IPSEC_PROTO"
>  	[ "$IPSEC_PROTO" = "esp_aead" ] && p="proto esp"
>  
> +	tst_ipsec_check_driver
>  	ipsec_set_algoline
>  
>  	if [ $target = lhost ]; then
> @@ -234,6 +244,7 @@ tst_ipsec_vti()
>  		tst_brk TCONF "iproute doesn't support 'vti'"
>  
>  	ipsec_set_algoline
> +	tst_ipsec_check_driver
>  
>  	local o_dir="src $src dst $dst"
>  	local i_dir="src $dst dst $src"
> 

[LTP] [PATCH v3 1/2] net: Add tst_net_run helper

[Alexey Kodanev]

Hi Petr,
On 08.11.2018 17:24, Petr Vorel wrote:
...  
> +# Run command on both lhost and rhost.
> +# tst_net_run [-s] [-l LPARAM] [-r RPARAM] CMD [ARG [ARG2]]
> +# LPARAM: parameter passed to $1 in lhost
> +# RPARAM: parameter passed to $1 in rhost
> +tst_net_run()
> +{
> +	local cmd
> +	local lparams
> +	local rparams
> +	local safe
> +
> +	local OPTIND
> +	while getopts l:r:s opt; do
> +		case "$opt" in
> +		l) lparams="$OPTARG" ;;
> +		r) rparams="$OPTARG" ;;
> +		s) safe="-s" ;;
> +		*) tst_brk_ TBROK "tst_net_run: unknown option: $OPTARG" ;;
> +		esac
> +	done
> +	shift $((OPTIND - 1))
> +
> +	cmd="$1"
> +	shift
> +	cmd="$cmd $lparams $@"
> +	[ -n "$safe" ] && ROD $cmd || sh -c "$cmd"
                                      ^
                                  just $cmd?

> +
> +	cmd="$cmd $rparams $@"
               ^
So, now it is "$cmd $lparams $@", we need to store $1 in a separate variable.


> +	tst_rhost_run $safe -c "$cmd"
> +}
> +

[LTP] [PATCH v3 1/2] net: Add tst_net_run helper

[Petr Vorel]

Hi Alexey,

> > +	[ -n "$safe" ] && ROD $cmd || sh -c "$cmd"
>                                       ^
>                                   just $cmd?
Yes, sh -c does not work for shell functions unless are exported
(tst_test_drivers() in next commit is a shell function).
I somehow overlooked it.

But sh -c is used for all test links in tst_rhost_run(),
I guess we don't want to do export -f FUNCNAME for all functions in tst_net.sh,
do we?

> > +
> > +	cmd="$cmd $rparams $@"
>                ^
> So, now it is "$cmd $lparams $@", we need to store $1 in a separate variable.
Good catch, thx!


Kind regards,
Petr

[LTP] [PATCH v3 1/2] net: Add tst_net_run helper

[Petr Vorel]

Hi Alexey,

> > > +	[ -n "$safe" ] && ROD $cmd || sh -c "$cmd"
> >                                       ^
> >                                   just $cmd?
> Yes, sh -c does not work for shell functions unless are exported
> (tst_test_drivers() in next commit is a shell function).
> I somehow overlooked it.

> But sh -c is used for all test links in tst_rhost_run(),
> I guess we don't want to do export -f FUNCNAME for all functions in tst_net.sh,
> do we?
OK, export -f FUNCNAME is a bashism, we cannot use it.
I'll check whether we can replace sh -c with brackets.
If not, tst_rhost_run() won't be able to use functions (just binaries).


Kind regards,
Petr

[LTP] [PATCH v3 1/2] net: Add tst_net_run helper

[Petr Vorel]

Hi Alexey,

> > > > +	[ -n "$safe" ] && ROD $cmd || sh -c "$cmd"
> > >                                       ^
> > >                                   just $cmd?
> > Yes, sh -c does not work for shell functions unless are exported
> > (tst_test_drivers() in next commit is a shell function).
> > I somehow overlooked it.

> > But sh -c is used for all test links in tst_rhost_run(),
> > I guess we don't want to do export -f FUNCNAME for all functions in tst_net.sh,
> > do we?
> OK, export -f FUNCNAME is a bashism, we cannot use it.
> I'll check whether we can replace sh -c with brackets.
> If not, tst_rhost_run() won't be able to use functions (just binaries).
OK, execvp() used in testcases/kernel/containers/share/ns_exec.c (used for netns) requires binary.
I guess I have to live with the fact that tst_rhost_run() is not able to call
shell functions.


Kind regards,
Petr