[LTP] [PATCH] testcases/lib: introduce sysctl wrapper

[LTP] [PATCH] testcases/lib: introduce sysctl wrapper

[Nikita Yushchenko via ltp]

Linux sysctl utility does not propagate errors returned from proc writes
to it's exit code:

> sysctl -q -w net.ipv4.tcp_congestion_control=dctcp
sysctl: setting key "net.ipv4.tcp_congestion_control": Operation not permitted
> echo $?
0

This causes the tests to not notice issues.

Workaround that by using a wrapper instead of sysctl. The wrapper calls
sysctl with given arguments, and returns non-zero exit status if either
sysctl returned non-zero exist status, or sysctl's stderr was not empty.

Signed-off-by: Nikita Yushchenko <nikita.yushchenko@virtuozzo.com>
---
 testcases/lib/Makefile       |  2 +-
 testcases/lib/sysctl_wrapper | 10 ++++++++++
 testcases/lib/tst_net.sh     |  6 +++---
 3 files changed, 14 insertions(+), 4 deletions(-)
 create mode 100644 testcases/lib/sysctl_wrapper

diff --git a/testcases/lib/Makefile b/testcases/lib/Makefile
index f2de0c832..ecf4bdc2d 100644
--- a/testcases/lib/Makefile
+++ b/testcases/lib/Makefile
@@ -6,7 +6,7 @@ top_srcdir		?= ../..
 
 include $(top_srcdir)/include/mk/testcases.mk
 
-INSTALL_TARGETS		:= *.sh
+INSTALL_TARGETS		:= *.sh sysctl_wrapper
 
 MAKE_TARGETS		:= tst_sleep tst_random tst_checkpoint tst_rod tst_kvcmp\
 			   tst_device tst_net_iface_prefix tst_net_ip_prefix tst_net_vars\
diff --git a/testcases/lib/sysctl_wrapper b/testcases/lib/sysctl_wrapper
new file mode 100644
index 000000000..59e7fb7c0
--- /dev/null
+++ b/testcases/lib/sysctl_wrapper
@@ -0,0 +1,10 @@
+#!/bin/sh
+
+stderr=$(mktemp)
+err=0
+sysctl $* 2>$stderr || err=1
+test -s $stderr && err=1
+cat $stderr >&2
+rm -f $stderr
+
+exit $err
diff --git a/testcases/lib/tst_net.sh b/testcases/lib/tst_net.sh
index 047686dc3..c7b835cbf 100644
--- a/testcases/lib/tst_net.sh
+++ b/testcases/lib/tst_net.sh
@@ -517,7 +517,7 @@ tst_init_iface()
 		ip link set $iface down || return $?
 		ip route flush dev $iface || return $?
 		ip addr flush dev $iface || return $?
-		sysctl -qw net.ipv6.conf.$iface.accept_dad=0 || return $?
+		sysctl_wrapper -qw net.ipv6.conf.$iface.accept_dad=0 || return $?
 		ip link set $iface up
 		return $?
 	fi
@@ -529,7 +529,7 @@ tst_init_iface()
 	tst_rhost_run -c "ip link set $iface down" || return $?
 	tst_rhost_run -c "ip route flush dev $iface" || return $?
 	tst_rhost_run -c "ip addr flush dev $iface" || return $?
-	tst_rhost_run -c "sysctl -qw net.ipv6.conf.$iface.accept_dad=0" || return $?
+	tst_rhost_run -c "sysctl_wrapper -qw net.ipv6.conf.$iface.accept_dad=0" || return $?
 	tst_rhost_run -c "ip link set $iface up"
 }
 
@@ -922,7 +922,7 @@ tst_set_sysctl()
 	local rparam=
 	[ "$TST_USE_NETNS" = "yes" ] && rparam="-i -r '-e'"
 
-	tst_net_run $safe -q $rparam "sysctl" "-q -w $name=$value"
+	tst_net_run $safe -q $rparam "sysctl"_wrapper "-q -w $name=$value"
 }
 
 tst_cleanup_rhost()
-- 
2.30.2


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

Re: [LTP] [PATCH] testcases/lib: introduce sysctl wrapper

[Nikita Yushchenko via ltp]

>> Linux sysctl utility does not propagate errors returned from proc writes
>> to it's exit code:
>>
>>> sysctl -q -w net.ipv4.tcp_congestion_control=dctcp
>> sysctl: setting key "net.ipv4.tcp_congestion_control": Operation not permitted
>>> echo $?
>> 0
> 
> I wonder why we use sysctl in the first place, all that it does in our
> case is write to /proc/sys/net/.... file. I guess that it may be easier
> if we just echo the value into the corresponding files instead.

LTP sometimes uses sysctl under 'tst_net_run' wrapper

Using shell redirection under wrapper is quite tricky if possible at all.

Nikita


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

Re: [LTP] [PATCH] testcases/lib: introduce sysctl wrapper

[Cyril Hrubis]

Hi!
> Linux sysctl utility does not propagate errors returned from proc writes
> to it's exit code:
> 
> > sysctl -q -w net.ipv4.tcp_congestion_control=dctcp
> sysctl: setting key "net.ipv4.tcp_congestion_control": Operation not permitted
> > echo $?
> 0

I wonder why we use sysctl in the first place, all that it does in our
case is write to /proc/sys/net/.... file. I guess that it may be easier
if we just echo the value into the corresponding files instead.

-- 
Cyril Hrubis
chrubis@suse.cz

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

Re: [LTP] [PATCH] testcases/lib: introduce sysctl wrapper

[Cyril Hrubis]

Hi!
> LTP sometimes uses sysctl under 'tst_net_run' wrapper
> 
> Using shell redirection under wrapper is quite tricky if possible at all.

Ah, right. But I guess that adding a simple tool that would behave just
as echo but the first parameter would be filename may be cleaner
solution than wrapping around the sysctl command.

-- 
Cyril Hrubis
chrubis@suse.cz

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

Re: [LTP] [PATCH] testcases/lib: introduce sysctl wrapper

[Petr Vorel]

Hi Nikita,

> Hi!
> > LTP sometimes uses sysctl under 'tst_net_run' wrapper

> > Using shell redirection under wrapper is quite tricky if possible at all.
Can you be more specific when sysctl with tst_net_run does not work?

> Ah, right. But I guess that adding a simple tool that would behave just
> as echo but the first parameter would be filename may be cleaner
> solution than wrapping around the sysctl command.
+1. If really requested please create C binary testcases/lib/tst_sysctl.c.
Code which use simple SAFE_FILE_PRINTF() as many C code does would be fairly
simple, more portable and probably faster than shell script. FYI we tend to
rewrite shell code into C.

Kind regards,
Petr

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

Re: [LTP] [PATCH] testcases/lib: introduce sysctl wrapper

[Cyril Hrubis]

Hi!
> > Ah, right. But I guess that adding a simple tool that would behave just
> > as echo but the first parameter would be filename may be cleaner
> > solution than wrapping around the sysctl command.
> +1. If really requested please create C binary testcases/lib/tst_sysctl.c.
> Code which use simple SAFE_FILE_PRINTF() as many C code does would be fairly
> simple, more portable and probably faster than shell script. FYI we tend to
> rewrite shell code into C.

I would even call it tst_write since it's pretty generic tool that does
write to a file.

-- 
Cyril Hrubis
chrubis@suse.cz

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

Re: [LTP] [PATCH] testcases/lib: introduce sysctl wrapper

[Petr Vorel]

> Hi!
> > > Ah, right. But I guess that adding a simple tool that would behave just
> > > as echo but the first parameter would be filename may be cleaner
> > > solution than wrapping around the sysctl command.
> > +1. If really requested please create C binary testcases/lib/tst_sysctl.c.
> > Code which use simple SAFE_FILE_PRINTF() as many C code does would be fairly
> > simple, more portable and probably faster than shell script. FYI we tend to
> > rewrite shell code into C.

> I would even call it tst_write since it's pretty generic tool that does
> write to a file.
+1, good point.

Kind regards,
Petr

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