[1/1] net: Introduce $TST_PING

Message ID 20181008115849.30826-1-pvorel@suse.cz
State Superseded
Delegated to: Petr Vorel
Headers show
Series
  • [1/1] net: Introduce $TST_PING
Related show

Commit Message

Petr Vorel Oct. 8, 2018, 11:58 a.m.
and use it in ipneigh01.sh and tst_ping().

iputils commit ebad35f ("ping: merge `ping6` command into `ping`"),
released in s20150815 stopped providing ping6 and left it on
distributions.

Unfortunately we cannot use 'ping -${TST_IPVER}' as ping got '-6' switch
(as a part of support for IPv6) was in commit 25aaaf4 ("Allow ping to
use IPv6 addresses"), released in s20150815 (previous versions supported
only ping6).

Signed-off-by: Petr Vorel <pvorel@suse.cz>
---
 testcases/lib/tst_net.sh                        | 16 ++++++++++++----
 testcases/network/tcp_cmds/ipneigh/ipneigh01.sh |  4 ++--
 2 files changed, 14 insertions(+), 6 deletions(-)

Comments

Alexey Kodanev Oct. 18, 2018, 11:40 a.m. | #1
Hi Petr,
On 10/08/2018 02:58 PM, Petr Vorel wrote:
> and use it in ipneigh01.sh and tst_ping().
> 
> iputils commit ebad35f ("ping: merge `ping6` command into `ping`"),
> released in s20150815 stopped providing ping6 and left it on
> distributions.
> 
> Unfortunately we cannot use 'ping -${TST_IPVER}' as ping got '-6' switch
> (as a part of support for IPv6) was in commit 25aaaf4 ("Allow ping to
> use IPv6 addresses"), released in s20150815 (previous versions supported
> only ping6).
> 
> Signed-off-by: Petr Vorel <pvorel@suse.cz>
> ---
>  testcases/lib/tst_net.sh                        | 16 ++++++++++++----
>  testcases/network/tcp_cmds/ipneigh/ipneigh01.sh |  4 ++--
>  2 files changed, 14 insertions(+), 6 deletions(-)
> 
> diff --git a/testcases/lib/tst_net.sh b/testcases/lib/tst_net.sh
> index a4467da7c..278bf4c15 100644
> --- a/testcases/lib/tst_net.sh
> +++ b/testcases/lib/tst_net.sh
> @@ -18,11 +18,12 @@ TST_SETUP="tst_net_setup"
>  # Blank for an IPV4 test; 6 for an IPV6 test.
>  TST_IPV6=${TST_IPV6:-}
>  TST_IPVER=${TST_IPV6:-4}
> +TST_PING="ping$TST_IPV6"
>  
>  tst_net_parse_args()
>  {
>  	case $1 in
> -	6) TST_IPV6=6 TST_IPVER=6;;
> +	6) TST_IPV6=6 TST_IPVER=6 TST_PING="ping6"; tst_net_fix_ping_cmd;;
>  	*) $TST_PARSE_ARGS_CALLER "$1" "$2";;
>  	esac
>  }
> @@ -61,7 +62,15 @@ tst_net_setup()
>  	[ -n "$TST_SETUP_CALLER" ] && $TST_SETUP_CALLER
>  }
>  
> +tst_net_fix_ping_cmd()
> +{
> +	if [ -n "$TST_IPV6" ]; then
> +		tst_cmd_available $TST_PING || TST_PING="ping -${TST_IPVER}"
> +	fi
> +}
> +
>  [ -n "$TST_USE_LEGACY_API" ] && . test.sh || . tst_test.sh
> +tst_net_fix_ping_cmd

It is called twice, i.e. also in tst_net_parse_args(), though it should be
enough to have it only here?

>  
>  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)"
> @@ -570,14 +579,13 @@ tst_ping()
>  	local dst_addr="${2:-$(tst_ipaddr rhost)}"; shift $(( $# >= 2 ? 2 : 0 ))
>  	local msg_sizes="$*"
>  	local msg="tst_ping IPv${TST_IPV6:-4} iface $src_iface, msg_size"
> -	local cmd="ping$TST_IPV6"
>  	local ret=0
>  
> -	tst_test_cmds $cmd
> +	tst_test_cmds $TST_PING
>

For IPv4 tests we would eventually need tst_ping() with IPv6, so that TST_IPV6
is empty, for example for some tunnels that are working for both protocols over
IPv4, checking dst address to decide which command to use. I've not posted the
patch yet, I guess I will do it after this one.

So I think we shouldn't use TST_PING variable...  it could be the new variable
for the ping6 only, or some general wrapper for the both commands?

Or may be something like this:

ping6()
{
    ping -6 $@
}

if which ping6 >/dev/null 2>&1; then
    unset ping6
fi

  
>  	# ping cmd use 56 as default message size
>  	for size in ${msg_sizes:-"56"}; do
> -		$cmd -I $src_iface -c $PING_MAX $dst_addr \
> +		$TST_PING -I $src_iface -c $PING_MAX $dst_addr \
>  			-s $size -i 0 > /dev/null 2>&1
>  		ret=$?
>  		if [ $ret -eq 0 ]; then
> diff --git a/testcases/network/tcp_cmds/ipneigh/ipneigh01.sh b/testcases/network/tcp_cmds/ipneigh/ipneigh01.sh
> index e22e17aae..558057d9c 100755
> --- a/testcases/network/tcp_cmds/ipneigh/ipneigh01.sh
> +++ b/testcases/network/tcp_cmds/ipneigh/ipneigh01.sh
> @@ -45,7 +45,7 @@ do_setup()
>  		;;
>  	esac
>  
> -	tst_test_cmds $CMD ping$TST_IPV6
> +	tst_test_cmds $CMD $TST_PING

Are you sure it will work with the "ping -6" command as expected?


>  }
>  
>  usage()
> @@ -69,7 +69,7 @@ do_test()
>  
>  	for i in $(seq 1 $NUMLOOPS); do
>  
> -		ping$TST_IPV6 -q -c1 $(tst_ipaddr rhost) > /dev/null || \
> +		$TST_PING -q -c1 $(tst_ipaddr rhost) > /dev/null || \
>  			tst_brk TFAIL "cannot ping $(tst_ipaddr rhost)"
>  
>  		local k
>
Petr Vorel Oct. 18, 2018, 2 p.m. | #2
Hi Alexey,

> Hi Petr,

> >  [ -n "$TST_USE_LEGACY_API" ] && . test.sh || . tst_test.sh
> > +tst_net_fix_ping_cmd

> It is called twice, i.e. also in tst_net_parse_args(), though it should be
> enough to have it only here?
No. This is for hypothetical case when you manually set TST_IPV6=6 and use test
without getopts.


> For IPv4 tests we would eventually need tst_ping() with IPv6, so that TST_IPV6
> is empty, for example for some tunnels that are working for both protocols over
> IPv4, checking dst address to decide which command to use. I've not posted the
> patch yet, I guess I will do it after this one.
OK, make sense.

> So I think we shouldn't use TST_PING variable...  it could be the new variable
> for the ping6 only, or some general wrapper for the both commands?

> Or may be something like this:

> ping6()
> {
>     ping -6 $@
> }

> if which ping6 >/dev/null 2>&1; then
>     unset ping6
> fi
I have a respect to use function of the same name as binary in $PATH because
possible race conditions, but this should be safe.
Simple solution => I'll use it in v2.


> >  	# ping cmd use 56 as default message size
> >  	for size in ${msg_sizes:-"56"}; do
> > -		$cmd -I $src_iface -c $PING_MAX $dst_addr \
> > +		$TST_PING -I $src_iface -c $PING_MAX $dst_addr \
> >  			-s $size -i 0 > /dev/null 2>&1
> >  		ret=$?
> >  		if [ $ret -eq 0 ]; then
> > diff --git a/testcases/network/tcp_cmds/ipneigh/ipneigh01.sh b/testcases/network/tcp_cmds/ipneigh/ipneigh01.sh
> > index e22e17aae..558057d9c 100755
> > --- a/testcases/network/tcp_cmds/ipneigh/ipneigh01.sh
> > +++ b/testcases/network/tcp_cmds/ipneigh/ipneigh01.sh
> > @@ -45,7 +45,7 @@ do_setup()
> >  		;;
> >  	esac

> > -	tst_test_cmds $CMD ping$TST_IPV6
> > +	tst_test_cmds $CMD $TST_PING

> Are you sure it will work with the "ping -6" command as expected?
You're right, that'd be a bug. Another reason to use ping6 function.
TCONF: '-6' not found

Thanks for your review!


Kind regards,
Petr

Patch

diff --git a/testcases/lib/tst_net.sh b/testcases/lib/tst_net.sh
index a4467da7c..278bf4c15 100644
--- a/testcases/lib/tst_net.sh
+++ b/testcases/lib/tst_net.sh
@@ -18,11 +18,12 @@  TST_SETUP="tst_net_setup"
 # Blank for an IPV4 test; 6 for an IPV6 test.
 TST_IPV6=${TST_IPV6:-}
 TST_IPVER=${TST_IPV6:-4}
+TST_PING="ping$TST_IPV6"
 
 tst_net_parse_args()
 {
 	case $1 in
-	6) TST_IPV6=6 TST_IPVER=6;;
+	6) TST_IPV6=6 TST_IPVER=6 TST_PING="ping6"; tst_net_fix_ping_cmd;;
 	*) $TST_PARSE_ARGS_CALLER "$1" "$2";;
 	esac
 }
@@ -61,7 +62,15 @@  tst_net_setup()
 	[ -n "$TST_SETUP_CALLER" ] && $TST_SETUP_CALLER
 }
 
+tst_net_fix_ping_cmd()
+{
+	if [ -n "$TST_IPV6" ]; then
+		tst_cmd_available $TST_PING || TST_PING="ping -${TST_IPVER}"
+	fi
+}
+
 [ -n "$TST_USE_LEGACY_API" ] && . test.sh || . tst_test.sh
+tst_net_fix_ping_cmd
 
 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)"
@@ -570,14 +579,13 @@  tst_ping()
 	local dst_addr="${2:-$(tst_ipaddr rhost)}"; shift $(( $# >= 2 ? 2 : 0 ))
 	local msg_sizes="$*"
 	local msg="tst_ping IPv${TST_IPV6:-4} iface $src_iface, msg_size"
-	local cmd="ping$TST_IPV6"
 	local ret=0
 
-	tst_test_cmds $cmd
+	tst_test_cmds $TST_PING
 
 	# ping cmd use 56 as default message size
 	for size in ${msg_sizes:-"56"}; do
-		$cmd -I $src_iface -c $PING_MAX $dst_addr \
+		$TST_PING -I $src_iface -c $PING_MAX $dst_addr \
 			-s $size -i 0 > /dev/null 2>&1
 		ret=$?
 		if [ $ret -eq 0 ]; then
diff --git a/testcases/network/tcp_cmds/ipneigh/ipneigh01.sh b/testcases/network/tcp_cmds/ipneigh/ipneigh01.sh
index e22e17aae..558057d9c 100755
--- a/testcases/network/tcp_cmds/ipneigh/ipneigh01.sh
+++ b/testcases/network/tcp_cmds/ipneigh/ipneigh01.sh
@@ -45,7 +45,7 @@  do_setup()
 		;;
 	esac
 
-	tst_test_cmds $CMD ping$TST_IPV6
+	tst_test_cmds $CMD $TST_PING
 }
 
 usage()
@@ -69,7 +69,7 @@  do_test()
 
 	for i in $(seq 1 $NUMLOOPS); do
 
-		ping$TST_IPV6 -q -c1 $(tst_ipaddr rhost) > /dev/null || \
+		$TST_PING -q -c1 $(tst_ipaddr rhost) > /dev/null || \
 			tst_brk TFAIL "cannot ping $(tst_ipaddr rhost)"
 
 		local k