[LTP] [PATCH 1/2] lib/tst_net.sh: add getopts to tst_ping()

[LTP] [PATCH 1/2] lib/tst_net.sh: add getopts to tst_ping()

[Alexey Kodanev]

* Replace '-i 0' with '-f'

* Add similar checks for the flood and interval options as
  in ping02 test

Signed-off-by: Alexey Kodanev <alexey.kodanev@oracle.com>
---
 testcases/lib/tst_net.sh                      | 55 +++++++++++++------
 testcases/network/mpls/mpls02.sh              |  2 +-
 testcases/network/mpls/mpls_lib.sh            |  4 +-
 .../network/stress/icmp/icmp-uni-basic.sh     |  2 +-
 testcases/network/stress/icmp/icmp-uni-vti.sh |  2 +-
 .../network/stress/interface/if-mtu-change.sh |  2 +-
 .../network/stress/ns-tools/tst_net_stress.sh |  2 +-
 7 files changed, 45 insertions(+), 24 deletions(-)

diff --git a/testcases/lib/tst_net.sh b/testcases/lib/tst_net.sh
index 46d3fb88e..ef9354903 100644
--- a/testcases/lib/tst_net.sh
+++ b/testcases/lib/tst_net.sh
@@ -800,37 +800,58 @@ tst_netload_compare()
 	tst_res_ TPASS "$msg, in range [${threshold_low}:${threshold_hi}]%"
 }
 
-# tst_ping [IFACE] [DST ADDR] [MESSAGE SIZE ARRAY]
+tst_ping_opt_unsupported()
+{
+	ping $@ 2>&1 | grep -q "invalid option"
+}
+
+# tst_ping -c COUNT -s MESSAGE_SIZES -p PATTERN -I IFACE -H HOST
 # Check icmp connectivity
 # IFACE: source interface name or IP address
-# DST ADDR: destination IPv4 or IPv6 address
-# MESSAGE SIZE ARRAY: message size array
+# HOST: destination IPv4 or IPv6 address
+# MESSAGE_SIZES: message size array
 tst_ping()
 {
 	# The max number of ICMP echo request
-	PING_MAX="${PING_MAX:-500}"
-
-	local src_iface="${1:-$(tst_iface)}"
-	local dst_addr="${2:-$(tst_ipaddr rhost)}"; shift $(( $# >= 2 ? 2 : 0 ))
-	local msg_sizes="$*"
-	local msg="tst_ping $dst_addr iface/saddr $src_iface, msg_size"
+	local ping_count="${PING_MAX:-500}"
+	local flood_opt="-f"
+	local pattern_opt
+	local msg_sizes
+	local src_iface="$(tst_iface)"
+	local dst_addr="$(tst_ipaddr rhost)"
 	local cmd="ping"
 	local ret=0
+	local opts
+
+	local OPTIND
+	while getopts c:s:p:I:H: opt; do
+		case "$opt" in
+		c) ping_count="$OPTARG";;
+		s) msg_sizes="$OPTARG";;
+		p) pattern_opt="-p $OPTARG";;
+		I) src_iface="$OPTARG";;
+		H) dst_addr="$OPTARG";;
+		*) tst_brk_ TBROK "tst_ping: unknown option: $OPTARG";;
+		esac
+	done
 
 	echo "$dst_addr" | grep -q ':' && cmd="ping6"
 	tst_require_cmds $cmd
 
+	if tst_ping_opt_unsupported $flood_opt; then
+		flood_opt="-i 0.01"
+		[ "$pattern_opt" ] && pattern_opt="-p aa"
+
+		tst_ping_opt_unsupported -i $pattern_opt && \
+			tst_brk_ TCONF "unsupported ping version (old busybox?)"
+	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 \
-			-s $size -i 0 > /dev/null 2>&1
+		EXPECT_PASS $cmd -I $src_iface -c $ping_count -s $size \
+			$flood_opt $pattern_opt $dst_addr \>/dev/null
 		ret=$?
-		if [ $ret -eq 0 ]; then
-			tst_res_ TPASS "$msg $size: pass"
-		else
-			tst_res_ TFAIL "$msg $size: fail"
-			break
-		fi
+		[ "$ret" -ne 0 ] && break
 	done
 	return $ret
 }
diff --git a/testcases/network/mpls/mpls02.sh b/testcases/network/mpls/mpls02.sh
index c263f8aa8..2fd3ec5bf 100755
--- a/testcases/network/mpls/mpls02.sh
+++ b/testcases/network/mpls/mpls02.sh
@@ -43,7 +43,7 @@ do_test()
 	local max_size=$TST_NET_MAX_PKT
 
 	if [ "$type" = "icmp" ]; then
-		tst_ping $ip_loc $ip_rmt 10 100 1000 2000 $max_size
+		tst_ping -I $ip_loc -H $ip_rmt -s "10 100 1000 2000 $max_size"
 	else
 		tst_netload -S $ip_loc -H $ip_rmt -T $type -n 10 -N 10
 		tst_netload -S $ip_loc -H $ip_rmt -T $type -A $max_size
diff --git a/testcases/network/mpls/mpls_lib.sh b/testcases/network/mpls/mpls_lib.sh
index 8ebedba0f..30e069581 100755
--- a/testcases/network/mpls/mpls_lib.sh
+++ b/testcases/network/mpls/mpls_lib.sh
@@ -87,8 +87,8 @@ mpls_virt_test()
 	local max_size=$TST_NET_MAX_PKT
 
 	if [ "$type" = "icmp" ]; then
-		tst_ping $ip_virt_local $ip_virt_remote 10 100 1000 2000 $max_size
-		tst_ping $ip6_virt_local $ip6_virt_remote 10 100 1000 2000 $max_size
+		tst_ping -I $ip_virt_local -H $ip_virt_remote -s "10 100 1000 2000 $max_size"
+		tst_ping -I $ip6_virt_local -H $ip6_virt_remote -s "10 100 1000 2000 $max_size"
 	else
 		tst_netload -S $ip_virt_local -H $ip_virt_remote -T $type -n 10 -N 10
 		tst_netload -S $ip6_virt_local -H $ip6_virt_remote -T $type -n 10 -N 10
diff --git a/testcases/network/stress/icmp/icmp-uni-basic.sh b/testcases/network/stress/icmp/icmp-uni-basic.sh
index 5980b81e5..2ae616cc3 100755
--- a/testcases/network/stress/icmp/icmp-uni-basic.sh
+++ b/testcases/network/stress/icmp/icmp-uni-basic.sh
@@ -19,7 +19,7 @@ do_setup()
 
 do_test()
 {
-	tst_ping $(tst_iface) $(tst_ipaddr rhost) $2
+	tst_ping -s $2
 }
 
 tst_run
diff --git a/testcases/network/stress/icmp/icmp-uni-vti.sh b/testcases/network/stress/icmp/icmp-uni-vti.sh
index 24eca177c..18bc71cfb 100755
--- a/testcases/network/stress/icmp/icmp-uni-vti.sh
+++ b/testcases/network/stress/icmp/icmp-uni-vti.sh
@@ -18,7 +18,7 @@ do_setup()
 
 do_test()
 {
-	tst_ping $tst_vti $ip_rmt_tun $2
+	tst_ping -I $tst_vti -H $ip_rmt_tun -s $2
 }
 
 tst_run
diff --git a/testcases/network/stress/interface/if-mtu-change.sh b/testcases/network/stress/interface/if-mtu-change.sh
index b945fb6ce..8112cdf0e 100755
--- a/testcases/network/stress/interface/if-mtu-change.sh
+++ b/testcases/network/stress/interface/if-mtu-change.sh
@@ -77,7 +77,7 @@ test_body()
 
 		tst_sleep $CHANGE_INTERVAL
 
-		tst_ping $(tst_ipaddr) $(tst_ipaddr rhost) "1 1000 65507"
+		tst_ping -s "1 1000 65507"
 	done
 }
 
diff --git a/testcases/network/stress/ns-tools/tst_net_stress.sh b/testcases/network/stress/ns-tools/tst_net_stress.sh
index ae040bcf1..4b00ee7f8 100644
--- a/testcases/network/stress/ns-tools/tst_net_stress.sh
+++ b/testcases/network/stress/ns-tools/tst_net_stress.sh
@@ -69,7 +69,7 @@ check_connectivity()
 
 	tst_res TINFO "ping through $src_iface iface to ${dst_addr}$cnt_msg"
 
-	tst_ping $src_iface $dst_addr
+	tst_ping -I $src_iface -H $dst_addr
 }
 
 # check_connectivity_interval CNT [RESTORE] [SRC_IFACE] [DST_ADDR]
-- 
2.20.1

[LTP] [PATCH 2/2] network/ping02: use tst_ping() from the library

[Alexey Kodanev]

Signed-off-by: Alexey Kodanev <alexey.kodanev@oracle.com>
---
 testcases/network/tcp_cmds/ping/ping02.sh | 30 ++---------------------
 1 file changed, 2 insertions(+), 28 deletions(-)

diff --git a/testcases/network/tcp_cmds/ping/ping02.sh b/testcases/network/tcp_cmds/ping/ping02.sh
index d4290c873..07a713531 100755
--- a/testcases/network/tcp_cmds/ping/ping02.sh
+++ b/testcases/network/tcp_cmds/ping/ping02.sh
@@ -2,41 +2,15 @@
 # SPDX-License-Identifier: GPL-2.0-or-later
 # Copyright (c) 2016-2019 Oracle and/or its affiliates. All Rights Reserved.
 
-TST_SETUP="do_setup"
 TST_TESTFUNC="do_test"
 TST_NEEDS_ROOT=1
 
 . tst_net.sh
 
-do_setup()
-{
-	COUNT=${COUNT:-3}
-	PACKETSIZES=${PACKETSIZES:-"8 16 32 64 128 256 512 1024 2048 4064"}
-
-	PING=ping${TST_IPV6}
-
-	tst_require_cmds $PING
-
-	ping_opts="-f -p 000102030405060708090a0b0c0d0e0f"
-	ipaddr=$(tst_ipaddr rhost)
-
-	if ! $PING -c 1 -f $ipaddr >/dev/null 2>&1; then
-		ping_opts="-i 0.01 -p aa"
-		if $PING -i 2>&1 | grep -q "invalid option"; then
-			tst_brk TCONF "unsupported ping version (old busybox?)"
-		fi
-	fi
-}
-
 do_test()
 {
-	local s
-
-	tst_res TINFO "flood $PING: ICMP packets with options '$ping_opts'"
-
-	for s in $PACKETSIZES; do
-		EXPECT_PASS $PING -c $COUNT -s $s $ipaddr $ping_opts \>/dev/null
-	done
+	tst_ping -s "${PACKETSIZES:-8 16 32 64 128 256 512 1024 2048 4064}" \
+		 -p "000102030405060708090a0b0c0d0e0f" -c "${COUNT:-3}"
 }
 
 tst_run
-- 
2.20.1

[LTP] [PATCH 1/2] lib/tst_net.sh: add getopts to tst_ping()

[Petr Vorel]

Hi Alexey,

> * Replace '-i 0' with '-f'

> * Add similar checks for the flood and interval options as
>   in ping02 test

Reviewed-by: Petr Vorel <pvorel@suse.cz>

Did a brief testing for both iputils ping and busybox ping.

Kind regards,
Petr

[LTP] [PATCH 2/2] network/ping02: use tst_ping() from the library

[Petr Vorel]

Hi Alexey,

Reviewed-by: Petr Vorel <pvorel@suse.cz>

Kind regards,
Petr

[LTP] [PATCH 1/2] lib/tst_net.sh: add getopts to tst_ping()

[Petr Vorel]

Hi Alexey,

BTW -f does not require root (but effectively it's required by netns setup).

> * Replace '-i 0' with '-f'
Out of curiosity, why flood ping is better than -i 0?
More effective stressing?

Ad Busybox ping fallback -i 0.01, interesting -i 0 keeps blocked in
recvfrom():

$ strace busybox ping -i 0 localhost
setsockopt(0, SOL_SOCKET, SO_BROADCAST, [1], 4) = 0
setsockopt(0, SOL_SOCKET, SO_RCVBUF, [7280], 4) = 0
rt_sigaction(SIGINT, {sa_handler=0x55935b81f3a0, sa_mask=[INT], sa_flags=SA_RESTORER|SA_RESTART, sa_restorer=0x7f82be570af0}, {sa_handler=SIG_DFL, sa_mask=[], sa_flags=0}, 8) = 0
sendto(0, "\10\0H\25*k\0\0T\2200\357\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0"..., 64, 0, {sa_family=AF_INET, sin_port=htons(0), sin_addr=inet_addr("127.0.0.1")}, 28) = 64
rt_sigaction(SIGALRM, {sa_handler=0x55935b81f4d0, sa_mask=[ALRM], sa_flags=SA_RESTORER|SA_RESTART, sa_restorer=0x7f82be570af0}, {sa_handler=SIG_DFL, sa_mask=[], sa_flags=0}, 8) = 0
setitimer(ITIMER_REAL, {it_interval={tv_sec=0, tv_usec=0}, it_value={tv_sec=0, tv_usec=0}}, NULL) = 0
recvfrom(0, "E\0\0Tf\25@\0@\1\326\221\177\0\0\1\177\0\0\1\10\0H\25*k\0\0T\2200\357"..., 192, 0, {sa_family=AF_INET, sin_port=htons(0), sin_addr=inet_addr("127.0.0.1")}, [16]) = 84
recvfrom(0, "E\0\0Tf\26\0\0@\1\26\221\177\0\0\1\177\0\0\1\0\0P\25*k\0\0T\2200\357"..., 192, 0, {sa_family=AF_INET, sin_port=htons(0), sin_addr=inet_addr("127.0.0.1")}, [16]) = 84
write(1, "64 bytes from 127.0.0.1: seq=0 t"..., 5264 bytes from 127.0.0.1: seq=0 ttl=64 time=0.634 ms
) = 52
recvfrom(0,

Is that a bug?

Kind regards,
Petr

> * Add similar checks for the flood and interval options as
>   in ping02 test

[LTP] [PATCH 1/2] lib/tst_net.sh: add getopts to tst_ping()

[Alexey Kodanev]

On 11.12.2020 12:15, Petr Vorel wrote:
> Hi Alexey,
> 
> BTW -f does not require root (but effectively it's required by netns setup).

Hi Petr,

Are sure -f doesn't require root?

> 
>> * Replace '-i 0' with '-f'
> Out of curiosity, why flood ping is better than -i 0?
> More effective stressing?

I think it's better to use the option that is specially made
for this, isn't it? AFAIK, -f won't do verbose printing, though
we could add -q too along with -i 0...

> 
> Ad Busybox ping fallback -i 0.01, interesting -i 0 keeps blocked in
> recvfrom():
> 
> $ strace busybox ping -i 0 localhost
> setsockopt(0, SOL_SOCKET, SO_BROADCAST, [1], 4) = 0
> setsockopt(0, SOL_SOCKET, SO_RCVBUF, [7280], 4) = 0
> rt_sigaction(SIGINT, {sa_handler=0x55935b81f3a0, sa_mask=[INT], sa_flags=SA_RESTORER|SA_RESTART, sa_restorer=0x7f82be570af0}, {sa_handler=SIG_DFL, sa_mask=[], sa_flags=0}, 8) = 0
> sendto(0, "\10\0H\25*k\0\0T\2200\357\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0"..., 64, 0, {sa_family=AF_INET, sin_port=htons(0), sin_addr=inet_addr("127.0.0.1")}, 28) = 64
> rt_sigaction(SIGALRM, {sa_handler=0x55935b81f4d0, sa_mask=[ALRM], sa_flags=SA_RESTORER|SA_RESTART, sa_restorer=0x7f82be570af0}, {sa_handler=SIG_DFL, sa_mask=[], sa_flags=0}, 8) = 0
> setitimer(ITIMER_REAL, {it_interval={tv_sec=0, tv_usec=0}, it_value={tv_sec=0, tv_usec=0}}, NULL) = 0
> recvfrom(0, "E\0\0Tf\25@\0@\1\326\221\177\0\0\1\177\0\0\1\10\0H\25*k\0\0T\2200\357"..., 192, 0, {sa_family=AF_INET, sin_port=htons(0), sin_addr=inet_addr("127.0.0.1")}, [16]) = 84
> recvfrom(0, "E\0\0Tf\26\0\0@\1\26\221\177\0\0\1\177\0\0\1\0\0P\25*k\0\0T\2200\357"..., 192, 0, {sa_family=AF_INET, sin_port=htons(0), sin_addr=inet_addr("127.0.0.1")}, [16]) = 84
> write(1, "64 bytes from 127.0.0.1: seq=0 t"..., 5264 bytes from 127.0.0.1: seq=0 ttl=64 time=0.634 ms
> ) = 52
> recvfrom(0,
> 
> Is that a bug?
> 

Looks like a bug.

> Kind regards,
> Petr
> 
>> * Add similar checks for the flood and interval options as
>>   in ping02 test

[LTP] [PATCH 1/2] lib/tst_net.sh: add getopts to tst_ping()

[Petr Vorel]

Hi Alexey,

> > BTW -f does not require root (but effectively it's required by netns setup).

> Hi Petr,

> Are sure -f doesn't require root?
Sorry, I was wrong (tested on latest iputils release):

ping localhost -f
ping: cannot flood; minimal interval allowed for user is 200ms

> >> * Replace '-i 0' with '-f'
> > Out of curiosity, why flood ping is better than -i 0?
> > More effective stressing?

> I think it's better to use the option that is specially made
> for this, isn't it? AFAIK, -f won't do verbose printing, though
> we could add -q too along with -i 0...

+1

Kind regards,
Petr

[LTP] [PATCH 2/2] network/ping02: use tst_ping() from the library

[Alexey Kodanev]

On 11.12.2020 11:55, Petr Vorel wrote:
> Hi Alexey,
> 
> Reviewed-by: Petr Vorel <pvorel@suse.cz>
>

Petr, thanks for the review!

Applied.