* [LTP] [PATCH v2 1/1] ipneigh : Use new API
@ 2018-05-14 11:49 Anton Smorodskyi
2018-05-16 9:09 ` Petr Vorel
0 siblings, 1 reply; 4+ messages in thread
From: Anton Smorodskyi @ 2018-05-14 11:49 UTC (permalink / raw)
To: ltp
Besides all obvious changes for moving to new API,
also was done :
1. more generic variable names
2. add check for del command failure
---
testcases/network/tcp_cmds/ipneigh/ipneigh01.sh | 65 ++++++++++++-------------
1 file changed, 32 insertions(+), 33 deletions(-)
diff --git a/testcases/network/tcp_cmds/ipneigh/ipneigh01.sh b/testcases/network/tcp_cmds/ipneigh/ipneigh01.sh
index 9af3aa31e..621863454 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
@@ -18,8 +19,8 @@
TCID=ipneigh01
NUMLOOPS=${NUMLOOPS:-50}
-TST_TOTAL=2
-TST_USE_LEGACY_API=1
+TST_TESTFUNC=do_test
+TST_CNT=2
. tst_net.sh
do_setup()
@@ -30,55 +31,53 @@ do_setup()
do_test()
{
- local arp_show_cmd="$1"
- local arp_del_cmd="$2"
+ local rhost=$(tst_ipaddr rhost)
+ case $1 in
+ 1)
+ local show_cmd="ip neigh show"
+ local del_cmd="ip neigh del $rhost dev $(tst_iface)"
+ ;;
+ 2)
+ 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"
+ ;;
+ esac
local entry_name
[ "$TST_IPV6" ] && entry_name="NDISC" || entry_name="ARP"
- 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'"
+ tst_res TINFO "Stress auto-creation of $entry_name cache entry"
+ tst_res TINFO "by pinging '$rhost' and deleting entry again"
+ tst_res TINFO "with '$del_cmd'"
for i in $(seq 1 $NUMLOOPS); do
ping$TST_IPV6 -q -c1 $rhost > /dev/null
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
- if [ $? -eq 0 ]; then
- ret=0
- break;
+ $show_cmd | grep -q $rhost
+ if [ $? -ne 0 ]; then
+ tst_brk TFAIL "$entry_name entry '$rhost' not listed"
fi
tst_sleep 100ms
done
- [ "$ret" -ne 0 ] && \
- tst_brkm TFAIL "$entry_name entry '$rhost' not listed"
+ $del_cmd
+ if [ $? -ne 0 ]; then
+ tst_brk TFAIL "fail to delete entry"
+ fi
- $arp_del_cmd
-
- $arp_show_cmd | grep -q "${rhost}.*$(tst_hwaddr rhost)" && \
- tst_brkm TFAIL "'$arp_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
- 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
\ No newline at end of file
--
2.13.6
^ permalink raw reply related [flat|nested] 4+ messages in thread
* [LTP] [PATCH v2 1/1] ipneigh : Use new API
2018-05-14 11:49 [LTP] [PATCH v2 1/1] ipneigh : Use new API Anton Smorodskyi
@ 2018-05-16 9:09 ` Petr Vorel
2018-05-16 13:05 ` Anton Smorodskyi
0 siblings, 1 reply; 4+ messages in thread
From: Petr Vorel @ 2018-05-16 9:09 UTC (permalink / raw)
To: ltp
Hi Anton,
> Besides all obvious changes for moving to new API,
> also was done :
> 1. more generic variable names
> 2. add check for del command failure
> ---
> testcases/network/tcp_cmds/ipneigh/ipneigh01.sh | 65 ++++++++++++-------------
> 1 file changed, 32 insertions(+), 33 deletions(-)
I'm sorry to NACK your patch, there needs to be some changes to done.
Some change I'd like to have is to pass this script command (-c [ arp | ip ]) and run test
only once, i.e. TST_CNT=1 (then it does not need to be included).
Then in runtest files it'd be:
diff --git runtest/net.ipv6 runtest/net.ipv6
-ipneigh601 ipneigh01.sh -6
+ipneigh6_ip ipneigh01.sh -6 -c ip
diff --git runtest/net.tcp_cmds runtest/net.tcp_cmds
-ipneigh01 ipneigh01.sh
+ipneigh01_arp ipneigh01.sh -c arp
+ipneigh01_ip ipneigh01.sh -c ip
By this test is 1) cleaner 2) we get rid of TCONF for IPv6.
> diff --git a/testcases/network/tcp_cmds/ipneigh/ipneigh01.sh b/testcases/network/tcp_cmds/ipneigh/ipneigh01.sh
...
> TCID=ipneigh01
In new API you don't use TCID.
> NUMLOOPS=${NUMLOOPS:-50}
It'd be better to use some variable from tst_net.sh, but not sure which one (NS_TIMES? Or
create new one for TCP tests?), so feel free to ignore it.
> -TST_TOTAL=2
> -TST_USE_LEGACY_API=1
> +TST_TESTFUNC=do_test
> +TST_CNT=2
> . tst_net.sh
> do_setup()
> @@ -30,55 +31,53 @@ do_setup()
> do_test()
> {
> - local arp_show_cmd="$1"
> - local arp_del_cmd="$2"
> + local rhost=$(tst_ipaddr rhost)
> + case $1 in
> + 1)
> + local show_cmd="ip neigh show"
> + local del_cmd="ip neigh del $rhost dev $(tst_iface)"
> + ;;
> + 2)
> + if [ -n "$TST_IPV6" ] then
^ Missing semicolon
if [ -n "$TST_IPV6" ]; then
Test don't run without that. I'd fix that before commit, but there are other issues.
> + tst_res TCONF "'arp cmd doesn't support IPv6, skipping test-case"
You still do TCONF.
> + return 1
> + fi
> + local show_cmd="arp -a"
> + local del_cmd="arp -d $rhost"
> + ;;
> + esac
> local entry_name
> [ "$TST_IPV6" ] && entry_name="NDISC" || entry_name="ARP"
> - 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'"
> + tst_res TINFO "Stress auto-creation of $entry_name cache entry"
> + tst_res TINFO "by pinging '$rhost' and deleting entry again"
> + tst_res TINFO "with '$del_cmd'"
Maybe you could specify number of times: $NUMLOOPS.
tst_res TINFO "with '$del_cmd' $NUMLOOPS times"
> for i in $(seq 1 $NUMLOOPS); do
> ping$TST_IPV6 -q -c1 $rhost > /dev/null
It looks to me better using tst_ping() from tst_net.sh (API function, other tests use it).
Then we could get rid of checking in do_setup:
tst_check_cmds arp grep ping$TST_IPV6
> 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
> - if [ $? -eq 0 ]; then
> - ret=0
> - break;
> + $show_cmd | grep -q $rhost
> + if [ $? -ne 0 ]; then
> + tst_brk TFAIL "$entry_name entry '$rhost' not listed"
> fi
You test to run much longer by removing break. By running it 30times until it fails. IMHO
that's not needed, the "stress" of the test is done by upper loop.
> tst_sleep 100ms
> done
...
Kind regards,
Petr
^ permalink raw reply [flat|nested] 4+ messages in thread
* [LTP] [PATCH v2 1/1] ipneigh : Use new API
2018-05-16 9:09 ` Petr Vorel
@ 2018-05-16 13:05 ` Anton Smorodskyi
2018-05-16 14:00 ` Petr Vorel
0 siblings, 1 reply; 4+ messages in thread
From: Anton Smorodskyi @ 2018-05-16 13:05 UTC (permalink / raw)
To: ltp
Hi Petr,
Thanks for review !
> I'm sorry to NACK your patch, there needs to be some changes to done.
>
> Some change I'd like to have is to pass this script command (-c [ arp | ip ]) and run test
> only once, i.e. TST_CNT=1 (then it does not need to be included).
> Then in runtest files it'd be:
>
> diff --git runtest/net.ipv6 runtest/net.ipv6
> -ipneigh601 ipneigh01.sh -6
> +ipneigh6_ip ipneigh01.sh -6 -c ip
> diff --git runtest/net.tcp_cmds runtest/net.tcp_cmds
> -ipneigh01 ipneigh01.sh
> +ipneigh01_arp ipneigh01.sh -c arp
> +ipneigh01_ip ipneigh01.sh -c ip
>
> By this test is 1) cleaner 2) we get rid of TCONF for IPv6.
ok , make sense .
>> + tst_res TCONF "'arp cmd doesn't support IPv6, skipping test-case"
> You still do TCONF.
Yes and it make perfect sense now because I am doing exit just after
this message , so it is ok
>
>> ping$TST_IPV6 -q -c1 $rhost > /dev/null
> It looks to me better using tst_ping() from tst_net.sh (API function, other tests use it).
> Then we could get rid of checking in do_setup:
> tst_check_cmds arp grep ping$TST_IPV6
ok , was not aware about that , will change this
--
Anton Smorodskyi <asmorodskyi@suse.com>
QA Engineer
SUSE Linux GmbH, Maxfeldstr. 5, D-90409 Nürnberg
Tel: +49-911-74053-0; Fax: +49-911-7417755; https://www.suse.com/
SUSE Linux GmbH, GF: Felix Imendörffer, Jane Smithard,
Graham Norton, HRB 21284 (AG Nürnberg)
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.linux.it/pipermail/ltp/attachments/20180516/db2a3670/attachment.html>
^ permalink raw reply [flat|nested] 4+ messages in thread
* [LTP] [PATCH v2 1/1] ipneigh : Use new API
2018-05-16 13:05 ` Anton Smorodskyi
@ 2018-05-16 14:00 ` Petr Vorel
0 siblings, 0 replies; 4+ messages in thread
From: Petr Vorel @ 2018-05-16 14:00 UTC (permalink / raw)
To: ltp
Hi Anton,
...
> > Some change I'd like to have is to pass this script command (-c [ arp | ip ]) and run test
> > only once, i.e. TST_CNT=1 (then it does not need to be included).
> > Then in runtest files it'd be:
> > diff --git runtest/net.ipv6 runtest/net.ipv6
> > -ipneigh601 ipneigh01.sh -6
> > +ipneigh6_ip ipneigh01.sh -6 -c ip
> > diff --git runtest/net.tcp_cmds runtest/net.tcp_cmds
> > -ipneigh01 ipneigh01.sh
> > +ipneigh01_arp ipneigh01.sh -c arp
> > +ipneigh01_ip ipneigh01.sh -c ip
> > By this test is 1) cleaner 2) we get rid of TCONF for IPv6.
> ok , make sense .
> > > + tst_res TCONF "'arp cmd doesn't support IPv6, skipping test-case"
^ left quote, which was in previous version, please remove it.
> > You still do TCONF.
> Yes and it make perfect sense now because I am doing exit just after this
> message , so it is ok
OK, it make sense to check for it. But if you pass 1 command in the test, you can have it as:
tst_res TCONF "arp doesn't support IPv6"
Kind regards,
Petr
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2018-05-16 14:00 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-05-14 11:49 [LTP] [PATCH v2 1/1] ipneigh : Use new API Anton Smorodskyi
2018-05-16 9:09 ` Petr Vorel
2018-05-16 13:05 ` Anton Smorodskyi
2018-05-16 14:00 ` Petr Vorel
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.