From: David Schweikert Date: Thu, 16 Feb 2017 09:26:25 +0000 (+0100) Subject: discard late packets, auto-adjust timeout for -c/-C/-l, fixes #32 X-Git-Url: https://git.gsnw.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=7592cc2a19063c1b68499b8d140ed3bb6bad4b0e;p=fping.git discard late packets, auto-adjust timeout for -c/-C/-l, fixes #32 --- diff --git a/ChangeLog b/ChangeLog index fc361d6..3ce3b8b 100644 --- a/ChangeLog +++ b/ChangeLog @@ -24,6 +24,18 @@ fping -n NAME NAME->IP->IPNAME NAME fping -d NAME NAME->IP->IPNAME NAME->IP->IPNAME + * INCOMPATIBILITY WARNING 3: + fping will now reject replies, if they arrive after the defined timeout + for reply packets, specified with -t. This change is relevant only for + for the counting and looping modes, where now the measured times should + be more consistent (see github issue #32 for details). + + To prevent loosing reply packets because of this change, the default + timeout in counting and looping modes is now automatically adjusted + to the period interval (up to 2000ms), but it can be overriden with + the -t option. The default timeout for non-looping/counting modes + remains 500ms. + * (feature) Unified 'fping' and 'fping6' into one binary (#80) * (feature) Long option names for all options * (feature) --enable-ipv6 is now default @@ -32,6 +44,8 @@ * (feature) Keep original name if a hostname is given with -n/--name * (feature) Option -d/--rdns now always does a rdns-lookup, even for names (name->IP->name), as '-n' was doing until now + * (feature) Enforce -t timeout on reply packets, by discarding late packets (#32) + * (feature) Auto-adjust timeout for -c/-C/-l mode to value of -p * (bugfix) Fix compatibility issue with GNU Hurd * (other) A C99 compiler is now required * (other) Option parsing with optparse (https://github.com/skeeto/optparse) diff --git a/ci/test-09-option-r-t.pl b/ci/test-09-option-r-t.pl index 2ea7a68..bd118a3 100755 --- a/ci/test-09-option-r-t.pl +++ b/ci/test-09-option-r-t.pl @@ -1,6 +1,6 @@ #!/usr/bin/perl -w -use Test::Command tests => 21; +use Test::Command tests => 23; use Test::More; # -R random bytes @@ -103,4 +103,10 @@ $cmd->stdout_is_eq(""); $cmd->stderr_is_eq("fping: can't parse source address: bla\n"); } -# fping -t tested in test-4-options-a-b.pl +# (note: fping -t also tested in test-4-options-a-b.pl) + +{ +my $cmd = Test::Command->new(cmd => "fping -c 2 -p 100 -t 200 127.0.0.1"); +$cmd->exit_is_num(0); +$cmd->stderr_like(qr{^fping: warning: timeout \(-t\) value larger than period \(-p\) produces unexpected results\n.*}); +} diff --git a/doc/fping.pod b/doc/fping.pod index 4baed41..9dcf19b 100644 --- a/doc/fping.pod +++ b/doc/fping.pod @@ -199,11 +199,19 @@ Set source address. =item B<-t>, B<--timeout>=I -Initial target timeout in milliseconds (default 500). In the default mode, this -is the amount of time that B waits for a response to its first request. -Successive timeouts are multiplied by the backoff factor specified with B<-B>. -Note that this option has no effect looping or counting modes (B<-l>, B<-c>, or -B<-C>). +Initial target timeout in milliseconds. In the default, non-loop mode, the +default timeout is 500ms, and it represents the amount of time that B +waits for a response to its first request. Successive timeouts are multiplied +by the backoff factor specified with B<-B>. + +In loop/count mode, the default timeout is automatically adjusted to match +the "period" value (but not more than 2000ms). You can still adjust the timeout +value with this option, if you wish to, but note that setting a value larger +than "period" produces inconsistent results, because the timeout value can +be respected only for the last ping. + +Also note that any received replies that are larger than the timeout value, will +be discarded. =item B<-T> I diff --git a/src/fping.c b/src/fping.c index ee2da5f..a9b175b 100644 --- a/src/fping.c +++ b/src/fping.c @@ -282,7 +282,7 @@ int generate_flag = 0; /* flag for IP list generation */ int verbose_flag, quiet_flag, stats_flag, unreachable_flag, alive_flag; int elapsed_flag, version_flag, count_flag, loop_flag, netdata_flag; int per_recv_flag, report_all_rtts_flag, name_flag, addr_flag, backoff_flag, rdns_flag; -int multif_flag; +int multif_flag, timeout_flag; int outage_flag = 0; int timestamp_flag = 0; int random_data_flag = 0; @@ -453,6 +453,7 @@ int main(int argc, char** argv) case 't': if (!(timeout = (unsigned int)atoi(optparse_state.optarg) * 100)) usage(1); + timeout_flag = 1; break; @@ -729,6 +730,22 @@ int main(int argc, char** argv) trials = (count > retry + 1) ? count : retry + 1; + /* auto-tune default timeout for count/loop modes + * see also github #32 */ + if (loop_flag || count_flag) { + if(!timeout_flag) { + timeout = perhost_interval; + if(timeout > AUTOTUNE_TIMEOUT_MAX*100) { + timeout = AUTOTUNE_TIMEOUT_MAX*100; + } + } + else { + if(timeout > perhost_interval && (loop_flag || (count_flag && count > 1))) { + fprintf(stderr, "%s: warning: timeout (-t) value larger than period (-p) produces unexpected results\n", prog); + } + } + } + #if defined(DEBUG) || defined(_DEBUG) if (debugging & DBG_TRACE) trace_flag = 1; @@ -1995,7 +2012,7 @@ int wait_for_reply(long wait_time) ); if (result <= 0) { - return 0; + return 1; } gettimeofday(¤t_time, &tz); @@ -2046,6 +2063,13 @@ int wait_for_reply(long wait_time) this_count = seqmap_value->ping_count; this_reply = timeval_diff(&recv_time, sent_time); + + /* discard reply if delay is larger than timeout + * (see also: github #32) */ + if(this_reply > h->timeout) { + return 1; + } + if (loop_flag || h->resp_times[this_count] == RESP_WAITING) { /* only for non-duplicates: */ h->waiting = 0; @@ -2684,9 +2708,9 @@ void usage(int is_error) fprintf(out, "Probing options:\n"); fprintf(out, " -4, --ipv4 only ping IPv4 addresses\n"); fprintf(out, " -6, --ipv6 only ping IPv6 addresses\n"); - fprintf(out, " -b, --size=BYTES amount of ping data to send, in bytes (default %d)\n", DEFAULT_PING_DATA_SIZE); + fprintf(out, " -b, --size=BYTES amount of ping data to send, in bytes (default: %d)\n", DEFAULT_PING_DATA_SIZE); fprintf(out, " -B, --backoff=N set exponential backoff factor to N\n"); - fprintf(out, " -c, --count=N count of pings to send to each target (default %d)\n", count); + fprintf(out, " -c, --count=N count of pings to send to each target (default: %d)\n", count); fprintf(out, " -f, --file=FILE read list of targets from a file ( - means stdin)\n"); fprintf(out, " -g, --generate generate target list (only if no -f specified)\n"); fprintf(out, " (give start and end IP in the target list, or a CIDR address)\n"); @@ -2699,12 +2723,13 @@ void usage(int is_error) fprintf(out, " -m, --all use all IPs of provided hostnames (e.g. IPv4 and IPv6), use with -A\n"); fprintf(out, " -M, --dontfrag set the Don't Fragment flag\n"); fprintf(out, " -O, --tos=N set the type of service (tos) flag on the ICMP packets\n"); - fprintf(out, " -p, --period=MSEC interval between ping packets to one target (in millisec)\n"); - fprintf(out, " (in looping and counting modes, default %d)\n", perhost_interval / 100); - fprintf(out, " -r, --retry=N number of retries (default %d)\n", DEFAULT_RETRY); + fprintf(out, " -p, --period=MSEC interval between ping packets to one target (in ms)\n"); + fprintf(out, " (in looping and counting modes, default: %d ms)\n", perhost_interval / 100); + fprintf(out, " -r, --retry=N number of retries (default: %d)\n", DEFAULT_RETRY); fprintf(out, " -R, --random random packet data (to foil link data compression)\n"); fprintf(out, " -S, --src=IP set source address\n"); - fprintf(out, " -t, --timeout=MSEC individual target initial timeout (in millisec) (default %d)\n", timeout / 100); + fprintf(out, " -t, --timeout=MSEC individual target initial timeout (in ms)\n"); + fprintf(out, " (default: %d ms, except with -l/-c/-C where it is the -p period)\n", timeout / 100); fprintf(out, "\n"); fprintf(out, "Output options:\n"); fprintf(out, " -a, --alive show targets that are alive\n"); @@ -2712,7 +2737,7 @@ void usage(int is_error) fprintf(out, " -C, --vcount=N same as -c, report results in verbose format\n"); fprintf(out, " -D, --timestamp print timestamp before each output line\n"); fprintf(out, " -e, --elapsed show elapsed time on return packets\n"); - fprintf(out, " -i, --interval=MSEC interval between sending ping packets (in ms) (default %d)\n", interval / 100); + fprintf(out, " -i, --interval=MSEC interval between sending ping packets (default: %d ms)\n", interval / 100); fprintf(out, " -n, --name show targets by name (-d is equivalent)\n"); fprintf(out, " -N, --netdata output compatible for netdata (-l -Q are required)\n"); fprintf(out, " -o, --outage show the accumulated outage time (lost packets * packet interval)\n"); diff --git a/src/options.h b/src/options.h index bbc93d1..75a1277 100644 --- a/src/options.h +++ b/src/options.h @@ -29,6 +29,7 @@ #ifndef DEFAULT_TIMEOUT #define DEFAULT_TIMEOUT 500 /* individual host timeouts */ +#define AUTOTUNE_TIMEOUT_MAX 2000 #endif