| Message ID | 20201015122056.20715-2-alexey.kodanev@oracle.com |
|---|---|
| State | Accepted |
| Delegated to: | Petr Vorel |
| Headers | show |
| Series | [1/5] lib/tst_net: add generic tst_netload_compare() | expand |
> Add TST_NETLOAD_RUN_COUNT to control how many times netstress > test will be run to calculate the mean time value. Default is 5. > This value will divide the total number of requests in order > not to significantly increase the time for the test after this > patch. > Moreover, one of the runs can fail once, it will produce only a > warning. The test will broke after the second failure. It can > be useful to make sure we have reproducible results. > Signed-off-by: Alexey Kodanev <alexey.kodanev@oracle.com> > --- > testcases/lib/tst_net.sh | 95 ++++++++++++++++++++++++++-------------- > 1 file changed, 62 insertions(+), 33 deletions(-) > diff --git a/testcases/lib/tst_net.sh b/testcases/lib/tst_net.sh > index b29e076c3..1912b984d 100644 > --- a/testcases/lib/tst_net.sh > +++ b/testcases/lib/tst_net.sh > @@ -623,9 +623,11 @@ tst_wait_ipv6_dad() > done > } > -tst_dump_rhost_cmd() > +tst_netload_brk() > { > tst_rhost_run -c "cat $TST_TMPDIR/netstress.log" > + cat tst_netload.log > + tst_brk_ $1 $2 > } > # Run network load test, see 'netstress -h' for option description > @@ -640,6 +642,7 @@ tst_netload() > # common options for client and server > local cs_opts= > + local run_cnt="$TST_NETLOAD_RUN_COUNT" > local c_num="$TST_NETLOAD_CLN_NUMBER" > local c_requests="$TST_NETLOAD_CLN_REQUESTS" > local c_opts= > @@ -692,51 +695,76 @@ tst_netload() > local expect_ret=0 > [ "$expect_res" != "pass" ] && expect_ret=3 > - tst_rhost_run -c "pkill -9 netstress\$" > - s_opts="${cs_opts}${s_opts}-R $s_replies -B $TST_TMPDIR" > - tst_res_ TINFO "run server 'netstress $s_opts'" > - tst_rhost_run -c "netstress $s_opts" > tst_netload.log 2>&1 > - if [ $? -ne 0 ]; then > - cat tst_netload.log > - local ttype="TFAIL" > - grep -e 'CONF:' tst_netload.log && ttype="TCONF" > - tst_brk_ $ttype "server failed" > + local was_failure=0 nit: I prefer have local variables at the top and boolean like variables as empty vs. "1" (tested with [ "$foo" = 1 ] (see: if [ "$bind_to_device" = 1 -a "$TST_NETLOAD_BINDTODEVICE" = 1 ]; then few lines above). This style is used in tst_test.sh, which is consistent, but style in tst_net.sh varies on this a lot. It's just a style and it wasn't introduced before this commit, thus feel free to ignore it, but it'd be nice to be consistent in library file. > + if [ "$run_cnt" -lt 2 ]; then maybe: if [ "$run_cnt" -lt 1 ]; then BTW we should also check all numeric variables (TST_NETLOAD_CLN_REQUESTS, ...) with tst_is_int (but don't bother with it now). > + run_cnt=1 > + was_failure=1 Hm, $was_failure set before loop. Shouldn't it be inside for i in $(seq 1 $run_cnt); do loop? And updated on each failure (be a error counter, not boolean)? > fi > - local port=$(tst_rhost_run -s -c "cat $TST_TMPDIR/netstress_port") > - c_opts="${cs_opts}${c_opts}-a $c_num -r $c_requests -d $rfile -g $port" > + s_opts="${cs_opts}${s_opts}-R $s_replies -B $TST_TMPDIR" > + c_opts="${cs_opts}${c_opts}-a $c_num -r $((c_requests / run_cnt)) -d $rfile" > + > + tst_res_ TINFO "run server 'netstress $s_opts'" > + tst_res_ TINFO "run client 'netstress -l $c_opts' $run_cnt times" > - tst_res_ TINFO "run client 'netstress -l $c_opts'" > - netstress -l $c_opts > tst_netload.log 2>&1 || ret=$? > tst_rhost_run -c "pkill -9 netstress\$" > + rm -f tst_netload.log > + > + local res=0 > + local passed=0 > + > + for i in $(seq 1 $run_cnt); do > + tst_rhost_run -c "netstress $s_opts" > tst_netload.log 2>&1 > + if [ $? -ne 0 ]; then > + cat tst_netload.log > + local ttype="TFAIL" > + grep -e 'CONF:' tst_netload.log && ttype="TCONF" > + tst_brk_ $ttype "server failed" > + fi > - if [ "$expect_ret" -ne 0 ]; then > - if [ $((ret & expect_ret)) -ne 0 ]; then > - tst_res_ TPASS "netstress failed as expected" > - else > - tst_res_ TFAIL "expected '$expect_res' but ret: '$ret'" > + local port=$(tst_rhost_run -s -c "cat $TST_TMPDIR/netstress_port") > + netstress -l ${c_opts} -g $port > tst_netload.log 2>&1 > + ret=$? > + tst_rhost_run -c "pkill -9 netstress\$" > + > + if [ "$expect_ret" -ne 0 ]; then > + if [ $((ret & expect_ret)) -ne 0 ]; then > + tst_res_ TPASS "netstress failed as expected" > + else > + tst_res_ TFAIL "expected '$expect_res' but ret: '$ret'" > + fi > + return $ret > + fi > + > + if [ "$ret" -ne 0 ]; then > + [ $((ret & 32)) -ne 0 ] && \ > + tst_netload_brk TCONF "not supported configuration" > + > + [ $((ret & 3)) -ne 0 -a $was_failure -gt 0 ] && \ > + tst_netload_brk TFAIL "expected '$expect_res' but ret: '$ret'" Instead the 2 lines above maybe this? Or am I missing something? if [ $((ret & 3)) -ne 0 ]; then was_failure=$((was_failure+1)) fi [ $was_failure -gt 0 ] && \ tst_netload_brk TFAIL "expected '$expect_res' but ret: '$ret'" > + > + tst_res_ TWARN "netstress failed, ret: $ret" > + was_failure=1 > + continue > fi > - return $ret > - fi ... Kind regards, Petr
On 20.10.2020 17:39, Petr Vorel wrote: >> Add TST_NETLOAD_RUN_COUNT to control how many times netstress >> test will be run to calculate the mean time value. Default is 5. > >> This value will divide the total number of requests in order >> not to significantly increase the time for the test after this >> patch. > >> Moreover, one of the runs can fail once, it will produce only a >> warning. The test will broke after the second failure. It can >> be useful to make sure we have reproducible results. > >> Signed-off-by: Alexey Kodanev <alexey.kodanev@oracle.com> >> --- >> testcases/lib/tst_net.sh | 95 ++++++++++++++++++++++++++-------------- >> 1 file changed, 62 insertions(+), 33 deletions(-) > >> diff --git a/testcases/lib/tst_net.sh b/testcases/lib/tst_net.sh >> index b29e076c3..1912b984d 100644 >> --- a/testcases/lib/tst_net.sh >> +++ b/testcases/lib/tst_net.sh >> @@ -623,9 +623,11 @@ tst_wait_ipv6_dad() >> done >> } > >> -tst_dump_rhost_cmd() >> +tst_netload_brk() >> { >> tst_rhost_run -c "cat $TST_TMPDIR/netstress.log" >> + cat tst_netload.log >> + tst_brk_ $1 $2 >> } > >> # Run network load test, see 'netstress -h' for option description >> @@ -640,6 +642,7 @@ tst_netload() >> # common options for client and server >> local cs_opts= > >> + local run_cnt="$TST_NETLOAD_RUN_COUNT" >> local c_num="$TST_NETLOAD_CLN_NUMBER" >> local c_requests="$TST_NETLOAD_CLN_REQUESTS" >> local c_opts= >> @@ -692,51 +695,76 @@ tst_netload() >> local expect_ret=0 >> [ "$expect_res" != "pass" ] && expect_ret=3 > >> - tst_rhost_run -c "pkill -9 netstress\$" >> - s_opts="${cs_opts}${s_opts}-R $s_replies -B $TST_TMPDIR" >> - tst_res_ TINFO "run server 'netstress $s_opts'" >> - tst_rhost_run -c "netstress $s_opts" > tst_netload.log 2>&1 >> - if [ $? -ne 0 ]; then >> - cat tst_netload.log >> - local ttype="TFAIL" >> - grep -e 'CONF:' tst_netload.log && ttype="TCONF" >> - tst_brk_ $ttype "server failed" >> + local was_failure=0 > nit: I prefer have local variables at the top and boolean like variables as empty vs. > "1" (tested with [ "$foo" = 1 ] (see: if [ "$bind_to_device" = 1 -a "$TST_NETLOAD_BINDTODEVICE" = 1 ]; then > few lines above). > > This style is used in tst_test.sh, which is consistent, but style in tst_net.sh > varies on this a lot. It's just a style and it wasn't introduced before this > commit, thus feel free to ignore it, but it'd be nice to be consistent in > library file. > >> + if [ "$run_cnt" -lt 2 ]; then > maybe: if [ "$run_cnt" -lt 1 ]; then > > BTW we should also check all numeric variables (TST_NETLOAD_CLN_REQUESTS, ...) > with tst_is_int (but don't bother with it now). > >> + run_cnt=1 >> + was_failure=1 > Hm, $was_failure set before loop. Shouldn't it be inside for i in $(seq 1 > $run_cnt); do loop? And updated on each failure (be a error counter, not boolean)? It is set to 1 for the special case, i.e. when run_cnt is 1, in that case we should tbrok the test at once. I don't see how the error counter will be better? >> fi > >> - local port=$(tst_rhost_run -s -c "cat $TST_TMPDIR/netstress_port") >> - c_opts="${cs_opts}${c_opts}-a $c_num -r $c_requests -d $rfile -g $port" >> + s_opts="${cs_opts}${s_opts}-R $s_replies -B $TST_TMPDIR" >> + c_opts="${cs_opts}${c_opts}-a $c_num -r $((c_requests / run_cnt)) -d $rfile" >> + >> + tst_res_ TINFO "run server 'netstress $s_opts'" >> + tst_res_ TINFO "run client 'netstress -l $c_opts' $run_cnt times" > >> - tst_res_ TINFO "run client 'netstress -l $c_opts'" >> - netstress -l $c_opts > tst_netload.log 2>&1 || ret=$? >> tst_rhost_run -c "pkill -9 netstress\$" >> + rm -f tst_netload.log >> + >> + local res=0 >> + local passed=0 >> + >> + for i in $(seq 1 $run_cnt); do >> + tst_rhost_run -c "netstress $s_opts" > tst_netload.log 2>&1 >> + if [ $? -ne 0 ]; then >> + cat tst_netload.log >> + local ttype="TFAIL" >> + grep -e 'CONF:' tst_netload.log && ttype="TCONF" >> + tst_brk_ $ttype "server failed" >> + fi > >> - if [ "$expect_ret" -ne 0 ]; then >> - if [ $((ret & expect_ret)) -ne 0 ]; then >> - tst_res_ TPASS "netstress failed as expected" >> - else >> - tst_res_ TFAIL "expected '$expect_res' but ret: '$ret'" >> + local port=$(tst_rhost_run -s -c "cat $TST_TMPDIR/netstress_port") >> + netstress -l ${c_opts} -g $port > tst_netload.log 2>&1 >> + ret=$? >> + tst_rhost_run -c "pkill -9 netstress\$" >> + >> + if [ "$expect_ret" -ne 0 ]; then >> + if [ $((ret & expect_ret)) -ne 0 ]; then >> + tst_res_ TPASS "netstress failed as expected" >> + else >> + tst_res_ TFAIL "expected '$expect_res' but ret: '$ret'" >> + fi >> + return $ret >> + fi >> + >> + if [ "$ret" -ne 0 ]; then >> + [ $((ret & 32)) -ne 0 ] && \ >> + tst_netload_brk TCONF "not supported configuration" >> + >> + [ $((ret & 3)) -ne 0 -a $was_failure -gt 0 ] && \ >> + tst_netload_brk TFAIL "expected '$expect_res' but ret: '$ret'" > Instead the 2 lines above maybe this? Or am I missing something? > > if [ $((ret & 3)) -ne 0 ]; then > was_failure=$((was_failure+1)) > fi > [ $was_failure -gt 0 ] && \ > tst_netload_brk TFAIL "expected '$expect_res' but ret: '$ret'" With this, it should exit on the first error, as it was before this patch... And I'm expecting to do this only on the second one, if run_cnt >= 2. >> + >> + tst_res_ TWARN "netstress failed, ret: $ret" >> + was_failure=1 >> + continue >> fi >> - return $ret >> - fi > ... > > Kind regards, > Petr >
Hi Alexey, ... > >> + if [ "$run_cnt" -lt 2 ]; then > > maybe: if [ "$run_cnt" -lt 1 ]; then > > BTW we should also check all numeric variables (TST_NETLOAD_CLN_REQUESTS, ...) > > with tst_is_int (but don't bother with it now). > >> + run_cnt=1 > >> + was_failure=1 > > Hm, $was_failure set before loop. Shouldn't it be inside for i in $(seq 1 > > $run_cnt); do loop? And updated on each failure (be a error counter, not boolean)? > It is set to 1 for the special case, i.e. when run_cnt is 1, in that case > we should tbrok the test at once. I don't see how the error counter will be > better? I'm sorry, I just misinterpret the code, please ignore my comment. Kind regards, Petr
diff --git a/testcases/lib/tst_net.sh b/testcases/lib/tst_net.sh index b29e076c3..1912b984d 100644 --- a/testcases/lib/tst_net.sh +++ b/testcases/lib/tst_net.sh @@ -623,9 +623,11 @@ tst_wait_ipv6_dad() done } -tst_dump_rhost_cmd() +tst_netload_brk() { tst_rhost_run -c "cat $TST_TMPDIR/netstress.log" + cat tst_netload.log + tst_brk_ $1 $2 } # Run network load test, see 'netstress -h' for option description @@ -640,6 +642,7 @@ tst_netload() # common options for client and server local cs_opts= + local run_cnt="$TST_NETLOAD_RUN_COUNT" local c_num="$TST_NETLOAD_CLN_NUMBER" local c_requests="$TST_NETLOAD_CLN_REQUESTS" local c_opts= @@ -692,51 +695,76 @@ tst_netload() local expect_ret=0 [ "$expect_res" != "pass" ] && expect_ret=3 - tst_rhost_run -c "pkill -9 netstress\$" - s_opts="${cs_opts}${s_opts}-R $s_replies -B $TST_TMPDIR" - tst_res_ TINFO "run server 'netstress $s_opts'" - tst_rhost_run -c "netstress $s_opts" > tst_netload.log 2>&1 - if [ $? -ne 0 ]; then - cat tst_netload.log - local ttype="TFAIL" - grep -e 'CONF:' tst_netload.log && ttype="TCONF" - tst_brk_ $ttype "server failed" + local was_failure=0 + if [ "$run_cnt" -lt 2 ]; then + run_cnt=1 + was_failure=1 fi - local port=$(tst_rhost_run -s -c "cat $TST_TMPDIR/netstress_port") - c_opts="${cs_opts}${c_opts}-a $c_num -r $c_requests -d $rfile -g $port" + s_opts="${cs_opts}${s_opts}-R $s_replies -B $TST_TMPDIR" + c_opts="${cs_opts}${c_opts}-a $c_num -r $((c_requests / run_cnt)) -d $rfile" + + tst_res_ TINFO "run server 'netstress $s_opts'" + tst_res_ TINFO "run client 'netstress -l $c_opts' $run_cnt times" - tst_res_ TINFO "run client 'netstress -l $c_opts'" - netstress -l $c_opts > tst_netload.log 2>&1 || ret=$? tst_rhost_run -c "pkill -9 netstress\$" + rm -f tst_netload.log + + local res=0 + local passed=0 + + for i in $(seq 1 $run_cnt); do + tst_rhost_run -c "netstress $s_opts" > tst_netload.log 2>&1 + if [ $? -ne 0 ]; then + cat tst_netload.log + local ttype="TFAIL" + grep -e 'CONF:' tst_netload.log && ttype="TCONF" + tst_brk_ $ttype "server failed" + fi - if [ "$expect_ret" -ne 0 ]; then - if [ $((ret & expect_ret)) -ne 0 ]; then - tst_res_ TPASS "netstress failed as expected" - else - tst_res_ TFAIL "expected '$expect_res' but ret: '$ret'" + local port=$(tst_rhost_run -s -c "cat $TST_TMPDIR/netstress_port") + netstress -l ${c_opts} -g $port > tst_netload.log 2>&1 + ret=$? + tst_rhost_run -c "pkill -9 netstress\$" + + if [ "$expect_ret" -ne 0 ]; then + if [ $((ret & expect_ret)) -ne 0 ]; then + tst_res_ TPASS "netstress failed as expected" + else + tst_res_ TFAIL "expected '$expect_res' but ret: '$ret'" + fi + return $ret + fi + + if [ "$ret" -ne 0 ]; then + [ $((ret & 32)) -ne 0 ] && \ + tst_netload_brk TCONF "not supported configuration" + + [ $((ret & 3)) -ne 0 -a $was_failure -gt 0 ] && \ + tst_netload_brk TFAIL "expected '$expect_res' but ret: '$ret'" + + tst_res_ TWARN "netstress failed, ret: $ret" + was_failure=1 + continue fi - return $ret - fi + + [ ! -f $rfile ] && \ + tst_netload_brk TFAIL "can't read $rfile" + + res="$((res + $(cat $rfile)))" + passed=$((passed + 1)) + done if [ "$ret" -ne 0 ]; then - tst_dump_rhost_cmd - cat tst_netload.log - [ $((ret & 3)) -ne 0 ] && \ - tst_brk_ TFAIL "expected '$expect_res' but ret: '$ret'" - [ $((ret & 32)) -ne 0 ] && \ - tst_brk_ TCONF "not supported configuration" [ $((ret & 4)) -ne 0 ] && \ tst_res_ TWARN "netstress has warnings" + tst_netload_brk TFAIL "expected '$expect_res' but ret: '$ret'" fi - if [ ! -f $rfile ]; then - tst_dump_rhost_cmd - cat tst_netload.log - tst_brk_ TFAIL "can't read $rfile" - fi + res=$((res / $passed)) + echo "$res" > $rfile - tst_res_ TPASS "netstress passed, time spent '$(cat $rfile)' ms" + tst_res_ TPASS "netstress passed, mean time '$res' ms" return $ret } @@ -938,6 +966,7 @@ export TST_NET_DATAROOT="$LTPROOT/testcases/bin/datafiles" export TST_NETLOAD_CLN_REQUESTS="${TST_NETLOAD_CLN_REQUESTS:-10000}" export TST_NETLOAD_CLN_NUMBER="${TST_NETLOAD_CLN_NUMBER:-2}" export TST_NETLOAD_BINDTODEVICE="${TST_NETLOAD_BINDTODEVICE-1}" +export TST_NETLOAD_RUN_COUNT="${TST_NETLOAD_RUN_COUNT:-5}" export HTTP_DOWNLOAD_DIR="${HTTP_DOWNLOAD_DIR:-/var/www/html}" export FTP_DOWNLOAD_DIR="${FTP_DOWNLOAD_DIR:-/var/ftp}" export FTP_UPLOAD_DIR="${FTP_UPLOAD_DIR:-/var/ftp/pub}"
Add TST_NETLOAD_RUN_COUNT to control how many times netstress test will be run to calculate the mean time value. Default is 5. This value will divide the total number of requests in order not to significantly increase the time for the test after this patch. Moreover, one of the runs can fail once, it will produce only a warning. The test will broke after the second failure. It can be useful to make sure we have reproducible results. Signed-off-by: Alexey Kodanev <alexey.kodanev@oracle.com> --- testcases/lib/tst_net.sh | 95 ++++++++++++++++++++++++++-------------- 1 file changed, 62 insertions(+), 33 deletions(-)