Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

misc: Align tc_iterate behavior #312

Closed
wants to merge 2 commits into from
Closed

misc: Align tc_iterate behavior #312

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
634603c
misc: Align tc_iterate behavior
Oct 28, 2024
58ac3ce
misc: tc_iterate: ensure interval is greater 0
Oct 28, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 1 addition & 2 deletions flent/runners.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2311,12 +2311,11 @@ def find_binary(self, interface, interval, length, host='localhost'):
interface = 'eth0'

return "{bash} {script} -i {interface} -I {interval:.2f} " \
"-c {count:.0f} -l {length} -H {host}".format(
"-l {length} -H {host}".format(
bash=bash,
script=script,
interface=interface,
interval=interval,
count=length // interval + 1,
length=length,
host=host)

Expand Down
6 changes: 2 additions & 4 deletions flent/scripts/tc_iterate.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,16 +1,14 @@
#!/bin/bash

interface=eth0
count=10
length=20
interval=0.1
command=qdisc
host=localhost

while getopts "i:c:l:I:C:H:" opt; do
while getopts "i:l:I:C:H:" opt; do
case $opt in
i) interface=$OPTARG ;;
c) count=$OPTARG ;;
l) length=$OPTARG ;;
I) interval=$OPTARG ;;
C) command=$OPTARG ;;
Expand All @@ -23,7 +21,7 @@ buffer=""


command_string=$(cat <<EOF
which tc_iterate >/dev/null && exec tc_iterate $buffer -i $interface -c $count -I $interval -C $command;
which tc_iterate >/dev/null && exec tc_iterate $buffer -i $interface -l $length -I $interval -C $command;
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

OK, so one thing that comes to mind as a way of keeping backwards compatibility, is that instead of unconditionally exec'ing when the binary exists, we do a regular call to the binary, and exit the script on success. That way, if it's an old binary without the -l option, we will fall back to the script. The only potential problem with this is that if the binary runs, but still exits non-zero, we'll get duplicate results. This could happen if it explicitly killed; however, it seems that that results in a different exit code, so maybe if we only react to the return value in usage() (255), that would work?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

another option could be to check the output of the binary when called with --help (or similar) and basically grep if -l or -c is in the output. This way we could distinguish between the binaries.
Or we just keep -c and calculate the count in the script instead of in the C-code. This would be probably the minimal fix. But either way, depending on your preference I am happy to implement it :)

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh yeah, that's a good point actually: why is this change needed at all? You're basically just moving the same calculation from python to C?

Copy link
Author

@jkoeppeler jkoeppeler Nov 1, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I thought doing it in C makes it easier if one (like me) wants to directly execute the C-binary. And then it is just more comfortable to pass a length parameter instead of deriving the count from the duration. But if preferred, then I would just add the calculation to the python code (or in the bash script?)
And as far as I see it: The python code is just passing the length as the count parameter which results in different behavior because the tc_iterate binary will capture 10 data points instead of capturing for 10 seconds.

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You're executing it manually? Why? :)

And, well, the line this patch removes from the python file already contains the exact same calculation? :D

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Well, :D you are correct, sorry I misread the code. Will close this PR then.

endtime=\$(date -d "$length sec" +%s%N);
while (( \$(date +%s%N) <= \$endtime )); do
tc -s $command show dev $interface;
Expand Down
20 changes: 13 additions & 7 deletions misc/tc_iterate.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@
#include <iconv.h>
#include <fcntl.h>
#include <math.h>
#include <float.h>
#include <sys/types.h>
#include <sys/wait.h>
#include <sys/timerfd.h>
Expand All @@ -54,7 +55,7 @@
#define NSEC_PER_SEC (1000000000.0)

struct arg {
int count;
int length;
struct timespec interval;
double finterval;
char *interface;
Expand All @@ -66,7 +67,7 @@ typedef struct arg args;

static const struct option long_options[] = {
{ "interface", required_argument , NULL , 'i' } ,
{ "count" , required_argument , NULL , 'c' } ,
{ "length" , required_argument , NULL , 'l' } ,
{ "interval" , required_argument , NULL , 'I' } ,
{ "command" , required_argument , NULL , 'C' } ,
{ "help" , no_argument , NULL , 'h' } ,
Expand All @@ -80,7 +81,7 @@ void usage (char *err) {
"\t-h --help \n"
"\t-b --buffer \n"
"\t-i --interface [eth0*,wlan0,etc]\n"
"\t-c --count [number of iterations]\n"
"\t-l --length [duration in seconds]\n"
"\t-I --interval [fractional number of seconds]\n"
"\t-C --command [qdisc]\n");
exit(-1);
Expand All @@ -90,13 +91,13 @@ static void defaults(args *a) {
a->interface = "eth0";
a->command = "qdisc";
a->finterval=.2;
a->count=10;
a->length=20;
a->interval.tv_nsec = 0;
a->interval.tv_sec = 0;
a->buffer = 0;
}

#define QSTRING "i:c:I:C:hb"
#define QSTRING "i:l:I:C:hb"

int process_options(int argc, char **argv, args *o)
{
Expand All @@ -113,7 +114,7 @@ int process_options(int argc, char **argv, args *o)

switch (opt)
{
case 'c': o->count = strtoul(optarg,NULL,10); break;
case 'l': o->length = strtoul(optarg,NULL,10); break;
case 'I': o->finterval = strtod(optarg,NULL); break;
case 'C': o->command = optarg; break;
case 'i': o->interface = optarg; break;
Expand All @@ -123,6 +124,10 @@ int process_options(int argc, char **argv, args *o)
default: usage(NULL);
}
}

if (o->finterval <= 0.0)
usage("Interval should be > 0 !\n");

o->interval.tv_sec = floor(o->finterval);
o->interval.tv_nsec = (long long) ((o->finterval - o->interval.tv_sec) * NSEC_PER_SEC);
return 0;
Expand Down Expand Up @@ -193,6 +198,7 @@ int forkit(args *a)
sprintf(cmd,"%s show dev %s\n",a->command,a->interface);
int csize = strlen(cmd);
int ctr = 0;
int length_count = a->length/a->finterval;
timerfd_settime(timer,0,&new_value,NULL); // relative timer

do {
Expand All @@ -206,7 +212,7 @@ int forkit(args *a)
result(out,0,BUFFERSIZE,buffer);
perror("reading cmd output");
}
} while (ctr < a->count);
} while (ctr < length_count);
close(tool);
close(in);
if(a->buffer) {
Expand Down
Loading