--- Begin Message ---
Package: tcpstat
Version: 1.5-7
Severity: normal
Tags: patch
When running tcpstat in capture mode, output hangs while no packets
arrive. For example
# tcpstat -i eth0 1
should print output every second, but instead hangs while there's no
network traffic. Missing output comes up all at once once a packet
arrives. (actually even when there is traffic the output timing is off,
but less obvious)
This bug is known and documented in the man page.
Packages based on upstream v1.5 code are affected.
Here is a patch against 1.5-7 that fixes it. Contacted upstream about
it, it is unclear if new versions of the package will be released, so
posting it here.
- main loop, fix untimely display in live capture mode:
all: decoupled data logic and display logic,
process.c: changed alarm handler to catch interval timeouts.
capture_seconds logic now on top of interval
logic.
- use setitimer() instead of ualarm(): not limited to 1s (linux)
Did a bit of testing, couldn't find anything it breaks from what i can
tell (under linux at least):
tcpstat works both in capture and file mode,
-s now works in capture mode,
intervals less and greater than 1s work in capture mode.
tcpprof and packetdump look fine also.
Cheers
--
Matthieu
diff -Nru tcpstat-1.5/configure.in tcpstat-1.5_fix/configure.in
--- tcpstat-1.5/configure.in 2013-01-04 14:12:58.000000000 +0100
+++ tcpstat-1.5_fix/configure.in 2013-01-04 14:01:07.000000000 +0100
@@ -102,7 +102,7 @@
dnl ############################
dnl Checks for library functions
dnl ############################
-AC_CHECK_FUNCS(strtol strtoul ualarm perror inet_ntop)
+AC_CHECK_FUNCS(strtol strtoul setitimer perror inet_ntop)
AC_CHECK_FUNCS(snprintf, , [
echo "WARNING: You don't seem to have snprintf() (Solaris 2.5.x?)"
echo " There may be a slight security problem without it."
diff -Nru tcpstat-1.5/doc/tcpstat.1 tcpstat-1.5_fix/doc/tcpstat.1
--- tcpstat-1.5/doc/tcpstat.1 2013-01-04 14:12:58.000000000 +0100
+++ tcpstat-1.5_fix/doc/tcpstat.1 2013-01-04 14:01:01.000000000 +0100
@@ -345,11 +345,6 @@
.Pp
Please send all bug reports to this address.
.Sh BUGS
-Due to a bug in libpcap, tcpstat will hang indefinitely under Linux
-when no packets arrive. This is because the timeout in pcap_open_live()
-is ignored under Linux when the interface is idle, which causes pcap_dispatch()
-to never return.
-.Pp
Not tested with link types other than Ethernet, PPP, and "None" types.
.Pp
There may be problems reading non-IPv4 packets across platforms when
diff -Nru tcpstat-1.5/include/config.h.in tcpstat-1.5_fix/include/config.h.in
--- tcpstat-1.5/include/config.h.in 2013-01-04 14:12:58.000000000 +0100
+++ tcpstat-1.5_fix/include/config.h.in 2013-01-04 14:01:07.000000000 +0100
@@ -72,8 +72,8 @@
/* Define if you have <sys/wait.h> that is POSIX.1 compatible. */
#undef HAVE_SYS_WAIT_H
-/* Define if you have the `ualarm' function. */
-#undef HAVE_UALARM
+/* Define if you have the `setitimer' function. */
+#undef HAVE_SETITIMER
/* Define if you have the <unistd.h> header file. */
#undef HAVE_UNISTD_H
diff -Nru tcpstat-1.5/include/tcpstat.h tcpstat-1.5_fix/include/tcpstat.h
--- tcpstat-1.5/include/tcpstat.h 2002-06-01 08:42:08.000000000 +0200
+++ tcpstat-1.5_fix/include/tcpstat.h 2013-01-04 14:01:01.000000000 +0100
@@ -234,8 +234,9 @@
/* process.c protos */
int get_dump_data(char *fname, char *filter, int flags,
- Double capture_seconds, void (*hook)(packet_data *, void **),
- void **args);
+ Double interval, Double capture_seconds,
+ void (*data_hook)(packet_data *, void **), void **args,
+ void (*display_hook)(void **));
/* print_packet.c protos */
void print_packet(packet_data *p, int what_to_print);
diff -Nru tcpstat-1.5/lib/process.c tcpstat-1.5_fix/lib/process.c
--- tcpstat-1.5/lib/process.c 2002-06-01 08:42:09.000000000 +0200
+++ tcpstat-1.5_fix/lib/process.c 2013-01-04 14:01:07.000000000 +0100
@@ -30,16 +30,8 @@
#include "tcpstat.h"
#include "snoop.h"
- /* Set alarm in 10 minute (600 seconds) steps
- * This is necessary, because ualarm() is usually limited:
- * FreeBSD, Solaris: 2^32/10^6 seconds (71m 35s)
- * Linux: ??
- */
-#define SECONDS_STEP 600.0
-
packet_data pdata;
int run;
-Double seconds_left;
struct hook_and_sinker {
void (*hook)(packet_data *, void **);
@@ -48,10 +40,16 @@
bpf_u_int32 linktype;
};
-void process_catch_alarm(int a) {
- seconds_left -= SECONDS_STEP;
- if (seconds_left <= 0 || a == SIGINT) run = 0;
- else my_alarm((seconds_left > SECONDS_STEP)? SECONDS_STEP : seconds_left);
+void sigint_handler(int a){
+ run = 0;
+}
+
+static pcap_t *pd = 0;
+static int interval_timeout = 0;
+
+void interval_timeout_handler(int a) {
+ interval_timeout = 1;
+ pcap_breakloop(pd);
}
#define MAGIC_SIZE 2
@@ -311,14 +309,14 @@
* calls a user function pointing to the data
*/
int get_pcap_data(char *fname, char *filter, int flags,
- Double capture_seconds, void (*hook)(packet_data *, void **),
- void **args) {
-
- pcap_t *pd;
+ Double interval, Double capture_seconds,
+ void (*data_hook)(packet_data *, void **), void **args,
+ void (*display_hook)(void **)) {
int i;
char ebuf[PCAP_ERRBUF_SIZE];
struct bpf_program bpf_prog;
struct hook_and_sinker hs;
+ Double seconds_left = capture_seconds;
#define SNAPLEN 68 /* XXX: Doesn't belong here */
if (flags & GET_TCPD_DO_LIVE) {
@@ -350,24 +348,36 @@
fprintf(stderr, "pcap_setfilter(): %s\n", pcap_geterr(pd));
return -1;
}
- hs.hook = hook;
+ hs.hook = data_hook;
hs.args = args;
hs.proc_flags = flags;
hs.linktype = (bpf_u_int32)pcap_datalink(pd);
run = 1;
- seconds_left = capture_seconds;
/* only catch SIGINT when doing live reads, otherwise just exit */
if (flags & GET_TCPD_DO_LIVE)
- (void)signal(SIGINT, process_catch_alarm);
-
- if (capture_seconds > 0.0 && flags & GET_TCPD_DO_LIVE) {
- (void)signal(SIGALRM, process_catch_alarm);
- my_alarm((seconds_left > SECONDS_STEP)? SECONDS_STEP : seconds_left);
- }
+ (void)signal(SIGINT, sigint_handler);
+
+ if (flags & GET_TCPD_DO_LIVE && interval > 0.0) {
+ (void)signal(SIGALRM, interval_timeout_handler);
+ my_alarm(interval);
+ }
+
while (run) {
i = pcap_dispatch(pd, -1, process_pcap, (u_char *)&hs);
+ if (interval_timeout)
+ {
+ my_alarm(interval);
+ interval_timeout = 0;
+ if (display_hook)
+ display_hook(args);
+
+ seconds_left -= interval;
+ if (capture_seconds > 0.0 && seconds_left <= 0.0)
+ run = 0;
+ }
+
if (i == 0 && ! (flags & GET_TCPD_DO_LIVE)) run = 0;
if (i == -1) {
fprintf(stderr, "pcap_dispatch(): %s\n", pcap_geterr(pd) );
@@ -383,8 +393,9 @@
* calls a user function pointing to the data
*/
int get_dump_data(char *fname, char *filter, int flags,
- Double capture_seconds, void (*hook)(packet_data *, void **),
- void **args) {
+ Double interval, Double capture_seconds,
+ void (*data_hook)(packet_data *, void **), void **args,
+ void (*display_hook)(void **)) {
u_int df_type = 0;
df_type = PCAP_FILE_MAGIC; /* Default to pcap format */
@@ -416,13 +427,13 @@
switch(df_type) {
case PCAP_FILE_MAGIC:
case PCAP_FILE_MAGIC_RH: /* Try RedHat format as well. */
- return get_pcap_data(fname, filter, flags,
- capture_seconds, hook, args);
+ return get_pcap_data(fname, filter, flags, interval,
+ capture_seconds, data_hook, args, display_hook);
/* NOTREACHED */
break;
case SNOOP_FILE_MAGIC:
return get_snoop_data(fname, filter, flags,
- capture_seconds, hook, args);
+ capture_seconds, data_hook, args);
/* NOTREACHED */
break;
default:
diff -Nru tcpstat-1.5/lib/utils.c tcpstat-1.5_fix/lib/utils.c
--- tcpstat-1.5/lib/utils.c 2002-01-02 22:05:55.000000000 +0100
+++ tcpstat-1.5_fix/lib/utils.c 2013-01-04 14:01:07.000000000 +0100
@@ -147,11 +147,18 @@
}
void my_alarm(Double seconds) {
-#ifdef HAVE_UALARM
- ualarm( (u_int) (seconds*1e6 + 0.5), 0);
+#ifdef HAVE_SETITIMER
+ struct itimerval it;
+ int s = seconds;
+ it.it_interval.tv_sec = 0;
+ it.it_interval.tv_usec = 0;
+ it.it_value.tv_sec = s;
+ it.it_value.tv_usec = (seconds - s) * 1e6;
+ setitimer(ITIMER_REAL, &it, 0);
#else
- alarm( (u_int) (seconds));
-#endif /* HAVE_UALARM */
+ /* drop microseconds part ... */
+ alarm( (u_int) (seconds));
+#endif /* HAVE_SETITIMER */
}
#ifndef HAVE_INET_NTOP
diff -Nru tcpstat-1.5/src/dump.c tcpstat-1.5_fix/src/dump.c
--- tcpstat-1.5/src/dump.c 2002-09-18 23:00:29.000000000 +0200
+++ tcpstat-1.5_fix/src/dump.c 2013-01-04 14:01:01.000000000 +0100
@@ -51,7 +51,7 @@
*/
void process_file(char *fname, u_int unused) {
get_dump_data(fname, filterexpr, get_tcp_flags,
- -1.0, my_hook, NULL);
+ 1.0, -1.0, my_hook, NULL, NULL);
}
int parse_show_types(char *in) {
diff -Nru tcpstat-1.5/src/tcpprof.c tcpstat-1.5_fix/src/tcpprof.c
--- tcpstat-1.5/src/tcpprof.c 2002-01-19 05:31:37.000000000 +0100
+++ tcpstat-1.5_fix/src/tcpprof.c 2013-01-04 14:01:01.000000000 +0100
@@ -79,7 +79,7 @@
stats_initdb(s_types);
argv[0] = (void *) &s_types;
- if (get_dump_data(fname, filterexpr, flags, capture_seconds, my_hook, argv) == 0)
+ if (get_dump_data(fname, filterexpr, flags, capture_seconds, capture_seconds, my_hook, argv, NULL) == 0)
show_results(s_types);
stats_closedb();
diff -Nru tcpstat-1.5/src/tcpstat.c tcpstat-1.5_fix/src/tcpstat.c
--- tcpstat-1.5/src/tcpstat.c 2002-07-27 02:42:05.000000000 +0200
+++ tcpstat-1.5_fix/src/tcpstat.c 2013-01-04 14:01:01.000000000 +0100
@@ -149,7 +149,7 @@
/* Used in the PRINTVAL macro which follows */
char printval_str[BUF_SIZ];
int filedesc = 1;
-
+
/* Handy macro for writing bytes to arbitrary file descriptors */
#define PRINTVAL(x,y) { \
snprintf(printval_str, BUF_SIZ, (x),(y)); \
@@ -352,25 +352,31 @@
return 0;
}
+void
+check_init_stats(statistics *sp, double ts)
+{
+ if (sp->ts_interval_begin == 0.0) {
+ /* initialize stats if called for the first time */
+ reset_counters(sp);
+ sp->ts_interval_begin = ts;
+ sp->global.ts_bigbang = ts;
+ }
+}
+
+void snoop_file_display_logic(statistics *sp, double ts);
+
/*
* This fuction is called by pcap_loop (not really, but OK.) It adds
* a particular packet to the total statistical data.
*/
-void my_hook(packet_data *pd, void **args) {
+void data_hook(packet_data *pd, void **args) {
statistics *sp;
- Double ts, elapsed;
+ Double ts;
sp = (statistics *) args[0];
-
ts = (Double)pd->timestamp.tv_sec +
(Double)pd->timestamp.tv_usec/1.0e6;
-
- if (sp->ts_interval_begin == 0.0) {
- /* initialize stats if called for the first time */
- reset_counters(sp);
- sp->ts_interval_begin = ts;
- sp->global.ts_bigbang = ts;
- }
+ check_init_stats(sp, ts);
/* Before we add this packet to the statistics,
* we need to check if we are actually done counting
@@ -381,21 +387,13 @@
ts - sp->global.ts_bigbang > capture_seconds)
return;
- /* After that, we need to see if there is an interval waiting
- * to be printed.
+ /* FIXME! get_pcap_data() should take care of this so we just
+ * provide it data_hook() and display_hook() and never
+ * have to invoke display stuff from within data_hook().
*/
- elapsed = ts - sp->ts_interval_begin;
- if ( (elapsed > interval && interval != -1.0) ||
- (print_immediately && elapsed > 0.0) ) {
-
- /* We got a packet past the boundry of the last interval,
- * so update stats, and print out the last interval that
- * has already expired. There might be a packet handed to
- * this function at this point, but we can't include it in
- * the current statistic.
- */
- do_the_printing(sp, elapsed, ts);
- }
+ if (!(get_tcp_flags & GET_TCPD_DO_LIVE))
+ snoop_file_display_logic(sp, ts);
+
sp->ts_now = ts;
sp->count.packets++;
sp->global.total_bytes += (Double)pd->packet_len;
@@ -420,13 +418,49 @@
sp->min_packetsize = pd->packet_len;
}
+/* Display logic when reading from file, NOT for live captures.
+ * This should be done by get_pcap_data(), see comment above. */
+void snoop_file_display_logic(statistics *sp, double ts) {
+ double elapsed;
+
+ /* After that, we need to see if there is an interval waiting
+ * to be printed.
+ */
+ elapsed = ts - sp->ts_interval_begin;
+ if ( (elapsed > interval && interval != -1.0) ||
+ (print_immediately && elapsed > 0.0) ) {
+
+ /* We got a packet past the boundry of the last interval,
+ * so update stats, and print out the last interval that
+ * has already expired. There might be a packet handed to
+ * this function at this point, but we can't include it in
+ * the current statistic.
+ */
+ do_the_printing(sp, elapsed, ts);
+ }
+}
+
+void display_hook(void **args) {
+ statistics *sp;
+ Double ts;
+ struct timeval tv;
+
+ sp = (statistics *) args[0];
+ gettimeofday(&tv, NULL);
+ ts = (Double)tv.tv_sec +
+ (Double)tv.tv_usec/1.0e6;
+ check_init_stats(sp, ts);
+
+ do_the_printing(sp, 0.0, ts);
+}
+
/*
* process_file() gets the data, and then displays the statistics
*/
void process_file(char *fname, u_int unused) {
void *argv[2];
statistics stats;
- Double x;
+ Double x = 0.0;
signal(SIGUSR1, catch_signal);
@@ -441,10 +475,12 @@
argv[0] = (void *) &stats;
/* Main Loop */
- if (get_dump_data(fname, filterexpr, get_tcp_flags,
- capture_seconds, my_hook, argv))
+ if (get_dump_data(fname, filterexpr, get_tcp_flags, interval,
+ capture_seconds, data_hook, argv, display_hook))
return;
+ if (!(get_tcp_flags & GET_TCPD_DO_LIVE))
+ {
/* The last "interval" still needs to be computed.
* Most of the work has been done. Now do...
*/
@@ -452,6 +488,7 @@
stats.global.total_time += x;
do_the_printing(&stats, x, stats.ts_now);
+ }
switch (action) {
case ACTION_INTERVAL:
@@ -459,7 +496,8 @@
* padding. This is only useful when
* reading data from a file.
*/
- if (capture_seconds > 0.0) {
+ if (!(get_tcp_flags & GET_TCPD_DO_LIVE)&&
+ capture_seconds > 0.0) {
while (stats.ts_now + x - stats.global.ts_bigbang < capture_seconds) {
do_the_printing(&stats, x, stats.ts_now);
stats.global.total_time += x;
@@ -623,7 +661,7 @@
* are capturing.
*/
if (capture_seconds > 0.0 && capture_seconds < interval)
- interval = -1.0;
+ interval = capture_seconds;
if (filename == NULL) {
/* Default read from interface */
--- End Message ---
pgp3xgi0sHi68.pgp
