[Flent-users] Re: [tohojo/flent] Runners: stop stats collection iterators after test-time is completed (#237)

["Toke Høiland-Jørgensen" <notifications@github.com>]

[-- Attachment #1: Type: text/plain, Size: 2020 bytes --]

@tohojo requested changes on this pull request.

Unfortunately this approach doesn't work on openwrt; but then the existing timestamped output doesn't either, we need to use the tc_iterate C program on there anyway. So maybe that's okay, and since the binary uses a timerfd, maybe we don't even need to switch that over to using 'length' either (since the 'count' should be accurate enough when the kernel runs the timer).

Some comments on the code below...

>  }
 
 while getopts "c:I:H:t:p:f:" opt; do
     case $opt in
-        c) count="$OPTARG" ;;
+        l) length="$OPTARG" ;;

You also need to change the opts string passed to getopts (`c:` to `l:`)

> @@ -10,6 +12,7 @@ while getopts "i:c:I:C:H:" opt; do
     case $opt in
         i) interface=$OPTARG ;;
         c) count=$OPTARG ;;
+        l) length=$OPTARG ;;

Same thing as above re: changing the optstring, also here you're not removing the `count` option...

> @@ -1,7 +1,9 @@
 #!/bin/bash
+# set -x

Leftover debug setting?

> @@ -41,7 +41,8 @@ then
 fi
 
 command_string=$(cat <<EOF
-for i in \$(seq $count); do
+endtime=\$(date -d "\$length sec" +%s%N);

Here you shouldn't be escaping the `$` - we want it evaluated while setting the command, not on the remote host.

> @@ -22,7 +25,8 @@ buffer=""
 
 command_string=$(cat <<EOF
 which tc_iterate >/dev/null && exec tc_iterate $buffer -i $interface -c $count -I $interval -C $command;
-for i in \$(seq $count); do
+endtime=\$(date -d "\$length sec" +%s%N);

As above

> @@ -10,6 +12,7 @@ while getopts "i:c:I:C:H:" opt; do
     case $opt in
         i) interface=$OPTARG ;;
         c) count=$OPTARG ;;
+        l) length=$OPTARG ;;

Ah, you're still passing `-c` to the C binary. Guess that should be changed as well to support the `-l` option. Which means we'll need to detect if it understands it; ugh...

-- 
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/tohojo/flent/pull/237#pullrequestreview-778537436

[-- Attachment #2: Type: text/html, Size: 4464 bytes --]