diff mbox series

[v4,1/1] ipneigh : Use new API

Message ID 20180523132432.20681-1-asmorodskyi@suse.com
State Accepted
Delegated to: Petr Vorel
Headers show
Series [v4,1/1] ipneigh : Use new API | expand

Commit Message

Anton Smorodskyi May 23, 2018, 1:24 p.m. UTC
Besides all obvious changes for moving to new API,
also was done :
1. more generic variable names
2. add check for del command failure
3. add input parameter "-c" which allows to control
which command will be used
---
 runtest/net.ipv6                                |  2 +-
 runtest/net.tcp_cmds                            |  3 +-
 testcases/network/tcp_cmds/ipneigh/ipneigh01.sh | 86 +++++++++++++++----------
 3 files changed, 54 insertions(+), 37 deletions(-)

Comments

Petr Vorel May 24, 2018, 11:39 a.m. UTC | #1
Hi Anton,

Thanks for your patch. I'm sorry to propose more changes:
* Thinking about it it'd be better to move creating commands to setup functions. Test
function get clearer by it (I've been told by Cyril several times to do that). I don't
like global shell variables (and upper case variables), but this is the purpose of setup
test function. And it allow us to check for command before (if you use 'ipneigh01.sh -c
foo' you get "unsupported command" first, not "command not found"
* Use tst_brk instead of tst_res, as test is run only once.
* Add command to TINFO.
* Add tst_brk TFAIL for ping (IMHO if you cannot ping the host test will fail so it's
better to reveal that it's due wrong link connection).
* Use $(tst_ipaddr rhost) instead of $rhost to 1) not introduce another global variable.
* Added git tag 'Signed-off-by: Anton Smorodskyi <asmorodskyi@suse.com>'. We use git tags in LTP.
* Add new line.

Bellow is diff, or you can see file online
https://raw.githubusercontent.com/pevik/ltp/anton/ipneigh.v4.fixes/testcases/network/tcp_cmds/ipneigh/ipneigh01.sh

Do you agree with the changes (can I push it like this)?


Kind regards,
Petr

diff --git testcases/network/tcp_cmds/ipneigh/ipneigh01.sh testcases/network/tcp_cmds/ipneigh/ipneigh01.sh
index 61e107a22..31dec6d49 100755
--- testcases/network/tcp_cmds/ipneigh/ipneigh01.sh
+++ testcases/network/tcp_cmds/ipneigh/ipneigh01.sh
@@ -28,6 +28,23 @@ TST_NEEDS_ROOT=1
 
 do_setup()
 {
+	case $CMD in
+	ip)
+		SHOW_CMD="ip neigh show"
+		DEL_CMD="ip neigh del $(tst_ipaddr rhost) dev $(tst_iface)"
+		;;
+	arp)
+		if [ -n "$TST_IPV6" ]; then
+			tst_brk TCONF "'arp' doesn't support IPv6"
+		fi
+		SHOW_CMD="arp -a"
+		DEL_CMD="arp -d $(tst_ipaddr rhost)"
+		;;
+	*)
+		tst_brk TBROK "unknown or missing command, use -c [ arp | ip ]"
+		;;
+	esac
+
 	tst_check_cmds $CMD ping$TST_IPV6
 }
 
@@ -45,39 +62,20 @@ parse_args()
 
 do_test()
 {
-	local rhost=$(tst_ipaddr rhost)
-	case $CMD in
-	ip)
-		local show_cmd="ip neigh show"
-		local del_cmd="ip neigh del $rhost dev $(tst_iface)"
-		;;
-	arp)
-		if [ -n "$TST_IPV6" ]; then
-			tst_res TCONF "'arp cmd doesn't support IPv6, skipping test-case"
-			return 1
-		fi
-		local show_cmd="arp -a"
-		local del_cmd="arp -d $rhost"
-		;;
-	*)
-		tst_res TBROK "-c is missing or have value not from list [ arp | ip ]"
-		return 1
-		;;
-	esac
-
 	local entry_name="ARP"
 	[ "$TST_IPV6" ] && entry_name="NDISC"
 
-	tst_res TINFO "Stress auto-creation of $entry_name cache entry $NUMLOOPS times"
+	tst_res TINFO "stress auto-creation $entry_name cache entry deleted with '$CMD' $NUMLOOPS times"
 
 	for i in $(seq 1 $NUMLOOPS); do
 
-		ping$TST_IPV6 -q -c1 $rhost > /dev/null
+		ping$TST_IPV6 -q -c1 $(tst_ipaddr rhost) > /dev/null || \
+			tst_brk TFAIL "cannot ping $(tst_ipaddr rhost)"
 
 		local k
 		local ret=1
 		for k in $(seq 1 30); do
-			$show_cmd | grep -q $rhost
+			$SHOW_CMD | grep -q $(tst_ipaddr rhost)
 			if [ $? -eq 0 ]; then
 				ret=0
 				break;
@@ -86,11 +84,12 @@ do_test()
 		done
 
 		[ "$ret" -ne 0 ] && \
-			tst_brk TFAIL "$entry_name entry '$rhost' not listed"
-		$del_cmd || tst_brk TFAIL "fail to delete entry"
+			tst_brk TFAIL "$entry_name entry '$(tst_ipaddr rhost)' not listed"
+
+		$DEL_CMD || tst_brk TFAIL "fail to delete entry"
 
-		$show_cmd | grep -q "${rhost}.*$(tst_hwaddr rhost)" && \
-			tst_brk TFAIL "'$del_cmd' failed, entry has " \
+		$SHOW_CMD | grep -q "${rhost}.*$(tst_hwaddr rhost)" && \
+			tst_brk TFAIL "'$DEL_CMD' failed, entry has " \
 				"$(tst_hwaddr rhost)' $i/$NUMLOOPS"
 	done
Anton Smorodskyi May 24, 2018, 11:50 a.m. UTC | #2
Hi Petr


On 05/24/2018 01:39 PM, Petr Vorel wrote:
> Bellow is diff, or you can see file online
> https://raw.githubusercontent.com/pevik/ltp/anton/ipneigh.v4.fixes/testcases/network/tcp_cmds/ipneigh/ipneigh01.sh
>
> Do you agree with the changes (can I push it like this)?
Yes
Petr Vorel May 24, 2018, 12:59 p.m. UTC | #3
Hi Anton,

> Hi Petr

> On 05/24/2018 01:39 PM, Petr Vorel wrote:
> > Bellow is diff, or you can see file online
> > https://raw.githubusercontent.com/pevik/ltp/anton/ipneigh.v4.fixes/testcases/network/tcp_cmds/ipneigh/ipneigh01.sh

> > Do you agree with the changes (can I push it like this)?
> Yes
Pushed, thanks!


Kind regards,
Petr
diff mbox series

Patch

diff --git a/runtest/net.ipv6 b/runtest/net.ipv6
index d8f85cc31..261a7254c 100644
--- a/runtest/net.ipv6
+++ b/runtest/net.ipv6
@@ -7,4 +7,4 @@  tracepath601 tracepath01.sh -6
 traceroute601 traceroute01.sh -6
 dhcpd6 dhcpd_tests.sh -6
 dnsmasq6 dnsmasq_tests.sh -6
-ipneigh601 ipneigh01.sh -6
+ipneigh6_ip ipneigh01.sh -6 -c ip
diff --git a/runtest/net.tcp_cmds b/runtest/net.tcp_cmds
index 859f48127..cfeacee5b 100644
--- a/runtest/net.tcp_cmds
+++ b/runtest/net.tcp_cmds
@@ -2,7 +2,8 @@ 
 #
 # PLEASE READ THE README FILE IN /tcp_cmds BEFORE RUNNING THESE.
 #
-ipneigh01 ipneigh01.sh
+ipneigh01_arp ipneigh01.sh -c arp
+ipneigh01_ip ipneigh01.sh -c ip
 arping01 arping01.sh
 clockdiff01 clockdiff01.sh
 ftp export TCbin=$LTPROOT/testcases/network/tcp_cmds/ftp; ftp01
diff --git a/testcases/network/tcp_cmds/ipneigh/ipneigh01.sh b/testcases/network/tcp_cmds/ipneigh/ipneigh01.sh
index 9af3aa31e..61e107a22 100755
--- a/testcases/network/tcp_cmds/ipneigh/ipneigh01.sh
+++ b/testcases/network/tcp_cmds/ipneigh/ipneigh01.sh
@@ -1,4 +1,5 @@ 
 #!/bin/sh
+# Copyright (c) 2018 SUSE Linux GmbH
 # Copyright (c) 2016 Oracle and/or its affiliates. All Rights Reserved.
 # Copyright (c) International Business Machines  Corp., 2000
 # This program is free software; you can redistribute it and/or
@@ -16,29 +17,58 @@ 
 #
 # Test basic functionality of 'arp' and 'ip neigh'.
 
-TCID=ipneigh01
 NUMLOOPS=${NUMLOOPS:-50}
-TST_TOTAL=2
-TST_USE_LEGACY_API=1
+TST_TESTFUNC=do_test
+TST_SETUP=do_setup
+TST_OPTS="c:"
+TST_PARSE_ARGS="parse_args"
+TST_USAGE="usage"
+TST_NEEDS_ROOT=1
 . tst_net.sh
 
 do_setup()
 {
-	tst_require_root
-	tst_check_cmds arp grep ping$TST_IPV6
+	tst_check_cmds $CMD ping$TST_IPV6
 }
 
-do_test()
+usage()
 {
-	local arp_show_cmd="$1"
-	local arp_del_cmd="$2"
+	echo "-c [ arp | ip ] Test command"
+}
 
-	local entry_name
-	[ "$TST_IPV6" ] && entry_name="NDISC" || entry_name="ARP"
+parse_args()
+{
+	case $1 in
+	c) CMD="$2" ;;
+	esac
+}
 
-	tst_resm TINFO "Stress auto-creation of $entry_name cache entry"
-	tst_resm TINFO "by pinging '$rhost' and deleting entry again"
-	tst_resm TINFO "with '$arp_del_cmd'"
+do_test()
+{
+	local rhost=$(tst_ipaddr rhost)
+	case $CMD in
+	ip)
+		local show_cmd="ip neigh show"
+		local del_cmd="ip neigh del $rhost dev $(tst_iface)"
+		;;
+	arp)
+		if [ -n "$TST_IPV6" ]; then
+			tst_res TCONF "'arp cmd doesn't support IPv6, skipping test-case"
+			return 1
+		fi
+		local show_cmd="arp -a"
+		local del_cmd="arp -d $rhost"
+		;;
+	*)
+		tst_res TBROK "-c is missing or have value not from list [ arp | ip ]"
+		return 1
+		;;
+	esac
+
+	local entry_name="ARP"
+	[ "$TST_IPV6" ] && entry_name="NDISC"
+
+	tst_res TINFO "Stress auto-creation of $entry_name cache entry $NUMLOOPS times"
 
 	for i in $(seq 1 $NUMLOOPS); do
 
@@ -46,9 +76,8 @@  do_test()
 
 		local k
 		local ret=1
-		# wait for arp entry at least 3 seconds
 		for k in $(seq 1 30); do
-			$arp_show_cmd | grep -q $rhost
+			$show_cmd | grep -q $rhost
 			if [ $? -eq 0 ]; then
 				ret=0
 				break;
@@ -57,28 +86,15 @@  do_test()
 		done
 
 		[ "$ret" -ne 0 ] && \
-			tst_brkm TFAIL "$entry_name entry '$rhost' not listed"
+			tst_brk TFAIL "$entry_name entry '$rhost' not listed"
+		$del_cmd || tst_brk TFAIL "fail to delete entry"
 
-		$arp_del_cmd
-
-		$arp_show_cmd | grep -q "${rhost}.*$(tst_hwaddr rhost)" && \
-			tst_brkm TFAIL "'$arp_del_cmd' failed, entry has " \
-				       "$(tst_hwaddr rhost)' $i/$NUMLOOPS"
+		$show_cmd | grep -q "${rhost}.*$(tst_hwaddr rhost)" && \
+			tst_brk TFAIL "'$del_cmd' failed, entry has " \
+				"$(tst_hwaddr rhost)' $i/$NUMLOOPS"
 	done
 
-	tst_resm TPASS "verified adding/removing of $entry_name cache entry"
+	tst_res TPASS "verified adding/removing of $entry_name cache entry"
 }
 
-do_setup
-
-rhost=$(tst_ipaddr rhost)
-
-if [ -z "$TST_IPV6" ]; then
-	do_test "arp -a" "arp -d $rhost"
-else
-	tst_resm TCONF "'arp cmd doesn't support IPv6, skipping test-case"
-fi
-
-do_test "ip neigh show" "ip neigh del $rhost dev $(tst_iface)"
-
-tst_exit
+tst_run