[PATCH 00/10] iptraf-ng: ifstats() and detstats() cleanups

[Vitezslav Samel]

Vitezslav Samel (10):
  ifstats(): optimize: move some checks away from the hot path
  ifstats(): call init/destroy functions in LIFO order
  ifstats(): refactor various time computing
  counters.c: rename functions to have the action last
  detstats(), servmon(): use explicit counters reset functions
  detstats(): refactor rates updating and printing
  detstats(): call init/destroy functions in LIFO order
  detstats(): optimize: move some checks away from the hot path
  detstats(): refactor various time computing
  detstats(): move keypress and packet handling into separate functions

 src/counters.c |  27 ++-
 src/counters.h |   7 +-
 src/detstats.c | 619 +++++++++++++++++++++++++++++++--------------------------
 src/ifstats.c  | 101 +++++-----
 src/serv.c     |  12 +-
 5 files changed, 423 insertions(+), 343 deletions(-)

-- 
1.9.1

[Nikola Pajkovsky]

Vitezslav Samel <vitezslav(a)samel.cz&gt; writes:

>  Vitezslav Samel (10):
>    ifstats(): optimize: move some checks away from the hot path
>    ifstats(): call init/destroy functions in LIFO order
>    ifstats(): refactor various time computing
>    counters.c: rename functions to have the action last
>    detstats(), servmon(): use explicit counters reset functions
>    detstats(): refactor rates updating and printing
>    detstats(): call init/destroy functions in LIFO order
>    detstats(): optimize: move some checks away from the hot path
>    detstats(): refactor various time computing
>    detstats(): move keypress and packet handling into separate functions
>
>   src/counters.c |  27 ++-
>   src/counters.h |   7 +-
>   src/detstats.c | 619 +++++++++++++++++++++++++++++++--------------------------
>   src/ifstats.c  | 101 +++++-----
>   src/serv.c     |  12 +-
>   5 files changed, 423 insertions(+), 343 deletions(-) 
applied, thanks.

-- 
Nikola

[PATCH 01/10] ifstats(): optimize: move some checks away from the hot path

[Vitezslav Samel]

Most checks can be done with 1sec granularity. Check if time
in seconds changes and process all the needed work only there.

Signed-off-by: Vitezslav Samel <vitezslav(a)samel.cz&gt;
---
 src/ifstats.c | 68 +++++++++++++++++++++++++++--------------------------------
 1 file changed, 31 insertions(+), 37 deletions(-)

diff --git a/src/ifstats.c b/src/ifstats.c
index cd915e9..95a712a 100644
--- a/src/ifstats.c
+++ b/src/ifstats.c
@@ -506,14 +506,6 @@ void ifstats(time_t facilitytime)
 
 	unsigned long dropped = 0UL;
 
-	struct timeval tv;
-	time_t starttime = 0;
-	time_t statbegin = 0;
-	time_t now = 0;
-	struct timeval start_tv;
-	time_t startlog = 0;
-	struct timeval updtime;
-
 	initiflist(&(table.head));
 	if (!table.head) {
 		no_ifaces_error();
@@ -564,52 +556,54 @@ void ifstats(time_t facilitytime)
 		goto err;
 	}
 
+	packet_init(&pkt);
+
 	exitloop = 0;
-	gettimeofday(&tv, NULL);
-	start_tv = tv;
-	updtime = tv;
-	starttime = startlog = statbegin = tv.tv_sec;
 
-	packet_init(&pkt);
+	struct timeval now;
+	gettimeofday(&now, NULL);
+	struct timeval last_time = now;
+	struct timeval last_update = now;
+	time_t statbegin = now.tv_sec;
+	time_t last_log = now.tv_sec;
 
 	while (!exitloop) {
-		gettimeofday(&tv, NULL);
-		now = tv.tv_sec;
+		gettimeofday(&now, NULL);
 
-		if ((now - starttime) >= 1) {
-			unsigned long msecs;
-
-			msecs = timeval_diff_msec(&tv, &start_tv);
+		if (now.tv_sec > last_time.tv_sec) {
+			unsigned long msecs = timeval_diff_msec(&now, &last_time);
 			updaterates(&table, msecs);
 			showrates(&table);
-			printelapsedtime(statbegin, now, LINES - 3, 1,
-					 table.borderwin);
+
+			printelapsedtime(statbegin, now.tv_sec, LINES - 3, 1, table.borderwin);
+
 			dropped += packet_get_dropped(fd);
 			print_packet_drops(dropped, table.borderwin, LINES - 3, 49);
-			starttime = now;
-			start_tv = tv;
-		}
-		if (logging) {
-			check_rotate_flag(&logfile);
-			if ((now - startlog) >= options.logspan) {
-				writegstatlog(&table,
-					      time(NULL) - statbegin,
-					      logfile);
-				startlog = now;
+
+			if (logging) {
+				check_rotate_flag(&logfile);
+				if ((now.tv_sec - last_log) >= options.logspan) {
+					writegstatlog(&table,
+						      time(NULL) - statbegin,
+						      logfile);
+					last_log = now.tv_sec;
+				}
 			}
+
+			if ((facilitytime != 0)
+			    && (((now.tv_sec - statbegin) / 60) >= facilitytime))
+				exitloop = 1;
+
+			last_time = now;
 		}
-		if (screen_update_needed(&tv, &updtime)) {
+		if (screen_update_needed(&now, &last_update)) {
 			print_if_entries(&table);
 			update_panels();
 			doupdate();
 
-			updtime = tv;
+			last_update = now;
 		}
 
-		if ((facilitytime != 0)
-		    && (((now - statbegin) / 60) >= facilitytime))
-			exitloop = 1;
-
 		if (packet_get(fd, &pkt, &ch, table.statwin) == -1) {
 			write_error("Packet receive failed");
 			exitloop = 1;
-- 
1.9.1

[PATCH 02/10] ifstats(): call init/destroy functions in LIFO order

[Vitezslav Samel]

... to prevent use of uninitialized variables.

Signed-off-by: Vitezslav Samel <vitezslav(a)samel.cz&gt;
---
 src/ifstats.c | 33 +++++++++++++++++----------------
 1 file changed, 17 insertions(+), 16 deletions(-)

diff --git a/src/ifstats.c b/src/ifstats.c
index 95a712a..b525534 100644
--- a/src/ifstats.c
+++ b/src/ifstats.c
@@ -520,6 +520,12 @@ void ifstats(time_t facilitytime)
 		promisc_set_list(&promisc);
 	}
 
+	fd = socket(PF_PACKET, SOCK_RAW, htons(ETH_P_ALL));
+	if(fd == -1) {
+		write_error("Unable to obtain monitoring socket");
+		goto err;
+	}
+
 	if (logging) {
 		if (strcmp(current_logfile, "") == 0) {
 			strcpy(current_logfile, GSTATLOG);
@@ -550,12 +556,6 @@ void ifstats(time_t facilitytime)
 	update_panels();
 	doupdate();
 
-	fd = socket(PF_PACKET, SOCK_RAW, htons(ETH_P_ALL));
-	if(fd == -1) {
-		write_error("Unable to obtain monitoring socket");
-		goto err;
-	}
-
 	packet_init(&pkt);
 
 	exitloop = 0;
@@ -617,8 +617,18 @@ void ifstats(time_t facilitytime)
 			ifstats_process_packet(&table, &pkt);
 
 	}
-	close(fd);
+	packet_destroy(&pkt);
 
+	if (logging) {
+		signal(SIGUSR1, SIG_DFL);
+		writegstatlog(&table, time(NULL) - statbegin, logfile);
+		writelog(logging, logfile,
+			 "******** General interface statistics stopped ********");
+		fclose(logfile);
+	}
+	strcpy(current_logfile, "");
+
+	close(fd);
 err:
 	if (options.promisc) {
 		promisc_restore_list(&promisc);
@@ -632,16 +642,7 @@ err:
 	update_panels();
 	doupdate();
 
-	if (logging) {
-		signal(SIGUSR1, SIG_DFL);
-		writegstatlog(&table, time(NULL) - statbegin, logfile);
-		writelog(logging, logfile,
-			 "******** General interface statistics stopped ********");
-		fclose(logfile);
-	}
 	destroyiflist(table.head);
-	packet_destroy(&pkt);
-	strcpy(current_logfile, "");
 }
 
 void selectiface(char *ifname, int withall, int *aborted)
-- 
1.9.1

[PATCH 03/10] ifstats(): refactor various time computing

[Vitezslav Samel]

Signed-off-by: Vitezslav Samel <vitezslav(a)samel.cz&gt;
---
 src/ifstats.c | 28 +++++++++++++++-------------
 1 file changed, 15 insertions(+), 13 deletions(-)

diff --git a/src/ifstats.c b/src/ifstats.c
index b525534..4e8482f 100644
--- a/src/ifstats.c
+++ b/src/ifstats.c
@@ -564,8 +564,15 @@ void ifstats(time_t facilitytime)
 	gettimeofday(&now, NULL);
 	struct timeval last_time = now;
 	struct timeval last_update = now;
-	time_t statbegin = now.tv_sec;
-	time_t last_log = now.tv_sec;
+
+	time_t starttime = now.tv_sec;
+	time_t endtime = INT_MAX;
+	if (facilitytime != 0)
+		endtime = now.tv_sec + facilitytime * 60;
+
+	time_t log_next = INT_MAX;
+	if (logging)
+		log_next = now.tv_sec + options.logspan;
 
 	while (!exitloop) {
 		gettimeofday(&now, NULL);
@@ -575,23 +582,18 @@ void ifstats(time_t facilitytime)
 			updaterates(&table, msecs);
 			showrates(&table);
 
-			printelapsedtime(statbegin, now.tv_sec, LINES - 3, 1, table.borderwin);
+			printelapsedtime(starttime, now.tv_sec, LINES - 3, 1, table.borderwin);
 
 			dropped += packet_get_dropped(fd);
 			print_packet_drops(dropped, table.borderwin, LINES - 3, 49);
 
-			if (logging) {
+			if (logging && (now.tv_sec > log_next)) {
 				check_rotate_flag(&logfile);
-				if ((now.tv_sec - last_log) >= options.logspan) {
-					writegstatlog(&table,
-						      time(NULL) - statbegin,
-						      logfile);
-					last_log = now.tv_sec;
-				}
+				writegstatlog(&table, now.tv_sec - starttime, logfile);
+				log_next = now.tv_sec + options.logspan;
 			}
 
-			if ((facilitytime != 0)
-			    && (((now.tv_sec - statbegin) / 60) >= facilitytime))
+			if (now.tv_sec > endtime)
 				exitloop = 1;
 
 			last_time = now;
@@ -621,7 +623,7 @@ void ifstats(time_t facilitytime)
 
 	if (logging) {
 		signal(SIGUSR1, SIG_DFL);
-		writegstatlog(&table, time(NULL) - statbegin, logfile);
+		writegstatlog(&table, time(NULL) - starttime, logfile);
 		writelog(logging, logfile,
 			 "******** General interface statistics stopped ********");
 		fclose(logfile);
-- 
1.9.1

[PATCH 04/10] counters.c: rename functions to have the action last

[Vitezslav Samel]

Signed-off-by: Vitezslav Samel <vitezslav(a)samel.cz&gt;
---
 src/counters.c | 10 +++++-----
 src/counters.h |  4 ++--
 src/detstats.c | 24 ++++++++++++------------
 src/serv.c     |  8 ++++----
 4 files changed, 23 insertions(+), 23 deletions(-)

diff --git a/src/counters.c b/src/counters.c
index f3281ec..89dc0cd 100644
--- a/src/counters.c
+++ b/src/counters.c
@@ -3,7 +3,7 @@
 
 #include "counters.h"
 
-void update_pkt_counter(struct pkt_counter *count, int bytes)
+void pkt_counter_update(struct pkt_counter *count, int bytes)
 {
 	if (count) {
 		count->pc_packets++;
@@ -11,13 +11,13 @@ void update_pkt_counter(struct pkt_counter *count, int bytes)
 	}
 }
 
-void update_proto_counter(struct proto_counter *proto_counter, int outgoing, int bytes)
+void proto_counter_update(struct proto_counter *proto_counter, int outgoing, int bytes)
 {
 	if (proto_counter) {
-		update_pkt_counter(&proto_counter->proto_total, bytes);
+		pkt_counter_update(&proto_counter->proto_total, bytes);
 		if (outgoing)
-			update_pkt_counter(&proto_counter->proto_out, bytes);
+			pkt_counter_update(&proto_counter->proto_out, bytes);
 		else
-			update_pkt_counter(&proto_counter->proto_in, bytes);
+			pkt_counter_update(&proto_counter->proto_in, bytes);
 	}
 }
diff --git a/src/counters.h b/src/counters.h
index e1dfbab..ab4cec6 100644
--- a/src/counters.h
+++ b/src/counters.h
@@ -12,8 +12,8 @@ struct proto_counter {
 	struct pkt_counter proto_out;
 };
 
-void update_pkt_counter(struct pkt_counter *count, int bytes);
-void update_proto_counter(struct proto_counter *proto_counter, int outgoing,
+void pkt_counter_update(struct pkt_counter *count, int bytes);
+void proto_counter_update(struct proto_counter *proto_counter, int outgoing,
 			  int bytes);
 
 #endif	/* IPTRAF_NG_COUNTERS_H */
diff --git a/src/detstats.c b/src/detstats.c
index 049541b..bb5ce32 100644
--- a/src/detstats.c
+++ b/src/detstats.c
@@ -537,33 +537,33 @@ void detstats(char *iface, time_t facilitytime)
 		}
 
 		outgoing = (pkt.from->sll_pkttype == PACKET_OUTGOING);
-		update_proto_counter(&ifcounts.total, outgoing, pkt.pkt_len);
+		proto_counter_update(&ifcounts.total, outgoing, pkt.pkt_len);
 		if (pkt.from->sll_pkttype == PACKET_BROADCAST) {
-			update_proto_counter(&ifcounts.bcast, outgoing, pkt.pkt_len);
-			update_pkt_counter(&span_bcast, pkt.pkt_len);
+			proto_counter_update(&ifcounts.bcast, outgoing, pkt.pkt_len);
+			pkt_counter_update(&span_bcast, pkt.pkt_len);
 		}
 
-		update_proto_counter(&span, outgoing, pkt.pkt_len);
+		proto_counter_update(&span, outgoing, pkt.pkt_len);
 
 		/* account network layer protocol */
 		switch(pkt.pkt_protocol) {
 		case ETH_P_IP:
 			if (pkt_result == CHECKSUM_ERROR) {
-				update_pkt_counter(&ifcounts.bad, pkt.pkt_len);
+				pkt_counter_update(&ifcounts.bad, pkt.pkt_len);
 				continue;
 			}
 
 			iplen = ntohs(pkt.iphdr->tot_len);
 
-			update_proto_counter(&ifcounts.ipv4, outgoing, iplen);
+			proto_counter_update(&ifcounts.ipv4, outgoing, iplen);
 			break;
 		case ETH_P_IPV6:
 			iplen = ntohs(pkt.ip6_hdr->ip6_plen) + 40;
 
-			update_proto_counter(&ifcounts.ipv6, outgoing, iplen);
+			proto_counter_update(&ifcounts.ipv6, outgoing, iplen);
 			break;
 		default:
-			update_proto_counter(&ifcounts.nonip, outgoing, pkt.pkt_len);
+			proto_counter_update(&ifcounts.nonip, outgoing, pkt.pkt_len);
 			continue;
 		}
 
@@ -572,17 +572,17 @@ void detstats(char *iface, time_t facilitytime)
 		/* account transport layer protocol */
 		switch (ip_protocol) {
 		case IPPROTO_TCP:
-			update_proto_counter(&ifcounts.tcp, outgoing, iplen);
+			proto_counter_update(&ifcounts.tcp, outgoing, iplen);
 			break;
 		case IPPROTO_UDP:
-			update_proto_counter(&ifcounts.udp, outgoing, iplen);
+			proto_counter_update(&ifcounts.udp, outgoing, iplen);
 			break;
 		case IPPROTO_ICMP:
 		case IPPROTO_ICMPV6:
-			update_proto_counter(&ifcounts.icmp, outgoing, iplen);
+			proto_counter_update(&ifcounts.icmp, outgoing, iplen);
 			break;
 		default:
-			update_proto_counter(&ifcounts.other, outgoing, iplen);
+			proto_counter_update(&ifcounts.other, outgoing, iplen);
 			break;
 		}
 	}
diff --git a/src/serv.c b/src/serv.c
index 90bac8c..7e27b84 100644
--- a/src/serv.c
+++ b/src/serv.c
@@ -360,8 +360,8 @@ static void updateportent(struct portlist *list, unsigned int
protocol,
 		if (sport_listent == NULL)
 			return;
 
-		update_proto_counter(&sport_listent->serv_count, PORT_OUTGOING, br);
-		update_proto_counter(&sport_listent->span, PORT_OUTGOING, br);
+		proto_counter_update(&sport_listent->serv_count, PORT_OUTGOING, br);
+		proto_counter_update(&sport_listent->span, PORT_OUTGOING, br);
 	}
 
 	if (goodport(dport, ports)) {
@@ -374,8 +374,8 @@ static void updateportent(struct portlist *list, unsigned int
protocol,
 		if (dport_listent == NULL)
 			return;
 
-		update_proto_counter(&dport_listent->serv_count, PORT_INCOMING, br);
-		update_proto_counter(&dport_listent->span, PORT_INCOMING, br);
+		proto_counter_update(&dport_listent->serv_count, PORT_INCOMING, br);
+		proto_counter_update(&dport_listent->span, PORT_INCOMING, br);
 	}
 }
 
-- 
1.9.1

[PATCH 05/10] detstats(), servmon(): use explicit counters reset functions

[Vitezslav Samel]

... instead of memset(struct, 0, sizeof(struct)).

Signed-off-by: Vitezslav Samel <vitezslav(a)samel.cz&gt;
---
 src/counters.c | 17 +++++++++++++++++
 src/counters.h |  3 +++
 src/detstats.c |  9 +++++----
 src/serv.c     |  4 ++--
 4 files changed, 27 insertions(+), 6 deletions(-)

diff --git a/src/counters.c b/src/counters.c
index 89dc0cd..f71880c 100644
--- a/src/counters.c
+++ b/src/counters.c
@@ -1,6 +1,8 @@
 /* For terms of usage/redistribution/modification see the LICENSE file */
 /* For authors and contributors see the AUTHORS file */
 
+#include "iptraf-ng-compat.h"
+
 #include "counters.h"
 
 void pkt_counter_update(struct pkt_counter *count, int bytes)
@@ -11,6 +13,12 @@ void pkt_counter_update(struct pkt_counter *count, int bytes)
 	}
 }
 
+void pkt_counter_reset(struct pkt_counter *count)
+{
+	if (count)
+		memset(count, 0, sizeof(*count));
+}
+
 void proto_counter_update(struct proto_counter *proto_counter, int outgoing, int bytes)
 {
 	if (proto_counter) {
@@ -21,3 +29,12 @@ void proto_counter_update(struct proto_counter *proto_counter, int
outgoing, int
 			pkt_counter_update(&proto_counter->proto_in, bytes);
 	}
 }
+
+void proto_counter_reset(struct proto_counter *proto_counter)
+{
+	if (proto_counter) {
+		pkt_counter_reset(&proto_counter->proto_total);
+		pkt_counter_reset(&proto_counter->proto_out);
+		pkt_counter_reset(&proto_counter->proto_in);
+	}
+}
diff --git a/src/counters.h b/src/counters.h
index ab4cec6..1746d0c 100644
--- a/src/counters.h
+++ b/src/counters.h
@@ -13,7 +13,10 @@ struct proto_counter {
 };
 
 void pkt_counter_update(struct pkt_counter *count, int bytes);
+void pkt_counter_reset(struct pkt_counter *count);
+
 void proto_counter_update(struct proto_counter *proto_counter, int outgoing,
 			  int bytes);
+void proto_counter_reset(struct proto_counter *proto_counter);
 
 #endif	/* IPTRAF_NG_COUNTERS_H */
diff --git a/src/detstats.c b/src/detstats.c
index bb5ce32..e560f67 100644
--- a/src/detstats.c
+++ b/src/detstats.c
@@ -351,8 +351,8 @@ void detstats(char *iface, time_t facilitytime)
 	update_panels();
 	doupdate();
 
-	memset(&span, 0, sizeof(span));
-	memset(&span_bcast, 0, sizeof(span_bcast));
+	proto_counter_reset(&span);
+	pkt_counter_reset(&span_bcast);
 	rate_alloc(&rate, 5);
 	rate_alloc(&rate_in, 5);
 	rate_alloc(&rate_out, 5);
@@ -426,8 +426,9 @@ void detstats(char *iface, time_t facilitytime)
 			rate_add_rate(&pps_rate_bcast, span_bcast.pc_packets, msecs);
 			pps_bcast = rate_get_average(&pps_rate_bcast);
 
-			memset(&span, 0, sizeof(span));
-			memset(&span_bcast, 0, sizeof(span_bcast));
+			proto_counter_reset(&span);
+			pkt_counter_reset(&span_bcast);
+
 			starttime = now;
 			start_tv = tv;
 
diff --git a/src/serv.c b/src/serv.c
index 7e27b84..0907495 100644
--- a/src/serv.c
+++ b/src/serv.c
@@ -215,7 +215,7 @@ static struct portlistent *addtoportlist(struct portlist *list,
 
 	servlook(port, protocol, ptemp->servname, 10);
 
-	memset(&ptemp->serv_count, 0, sizeof(ptemp->serv_count));
+	proto_counter_reset(&ptemp->serv_count);
 
 	list->count++;
 	ptemp->idx = list->count;
@@ -774,7 +774,7 @@ static void update_serv_rates(struct portlist *list, unsigned long
msecs)
 		rate_add_rate(&ple->rate_in, ple->span.proto_in.pc_bytes, msecs);
 		rate_add_rate(&ple->rate_out, ple->span.proto_out.pc_bytes, msecs);
 
-		memset(&ple->span, 0, sizeof(ple->span));
+		proto_counter_reset(&ple->span);
 	}
 }
 
-- 
1.9.1

[PATCH 06/10] detstats(): refactor rates updating and printing

[Vitezslav Samel]

Signed-off-by: Vitezslav Samel <vitezslav(a)samel.cz&gt;
---
 src/detstats.c | 316 +++++++++++++++++++++++++++++++++------------------------
 1 file changed, 186 insertions(+), 130 deletions(-)

diff --git a/src/detstats.c b/src/detstats.c
index e560f67..5b316e2 100644
--- a/src/detstats.c
+++ b/src/detstats.c
@@ -40,6 +40,35 @@ struct ifcounts {
 	struct proto_counter udp;
 	struct proto_counter icmp;
 	struct proto_counter other;
+
+	struct proto_counter span;
+	struct pkt_counter span_bcast;
+};
+
+struct ifrates {
+	struct rate rate;
+	struct rate rate_in;
+	struct rate rate_out;
+	struct rate rate_bcast;
+	unsigned long activity;
+	unsigned long peakactivity;
+	unsigned long activity_in;
+	unsigned long peakactivity_in;
+	unsigned long activity_out;
+	unsigned long peakactivity_out;
+	unsigned long activity_bcast;
+
+	struct rate pps_rate;
+	struct rate pps_rate_in;
+	struct rate pps_rate_out;
+	struct rate pps_rate_bcast;
+	unsigned long pps;
+	unsigned long peakpps;
+	unsigned long pps_in;
+	unsigned long peakpps_in;
+	unsigned long pps_out;
+	unsigned long peakpps_out;
+	unsigned long pps_bcast;
 };
 
 /* USR1 log-rotation signal handlers */
@@ -50,11 +79,8 @@ static void rotate_dstat_log(int s __unused)
 	signal(SIGUSR1, rotate_dstat_log);
 }
 
-static void writedstatlog(char *ifname,
-		   unsigned long peakactivity, unsigned long peakpps,
-		   unsigned long peakactivity_in, unsigned long peakpps_in,
-		   unsigned long peakactivity_out, unsigned long peakpps_out,
-		   struct ifcounts *ts, unsigned long nsecs, FILE *fd)
+static void writedstatlog(char *ifname, struct ifcounts *ts,
+			  struct ifrates *ifrates, unsigned long nsecs, FILE *fd)
 {
 	char atime[TIME_TARGET_MAX];
 
@@ -146,14 +172,14 @@ static void writedstatlog(char *ifname,
 		rate_print(ts->total.proto_out.pc_bytes / nsecs, bps_string, sizeof(bps_string));
 		rate_print_pps(ts->total.proto_out.pc_packets / nsecs, pps_string,
sizeof(pps_string));
 		fprintf(fd, "  Outgoing:\t%s, %s\n", bps_string, pps_string);
-		rate_print(peakactivity, bps_string, sizeof(bps_string));
-		rate_print_pps(peakpps, pps_string, sizeof(pps_string));
+		rate_print(ifrates->peakactivity, bps_string, sizeof(bps_string));
+		rate_print_pps(ifrates->peakpps, pps_string, sizeof(pps_string));
 		fprintf(fd, "\nPeak total activity: %s, %s\n", bps_string, pps_string);
-		rate_print(peakactivity_in, bps_string, sizeof(bps_string));
-		rate_print_pps(peakpps_in, pps_string, sizeof(pps_string));
+		rate_print(ifrates->peakactivity_in, bps_string, sizeof(bps_string));
+		rate_print_pps(ifrates->peakpps_in, pps_string, sizeof(pps_string));
 		fprintf(fd, "Peak incoming rate: %s, %s\n", bps_string, pps_string);
-		rate_print(peakactivity_out, bps_string, sizeof(bps_string));
-		rate_print_pps(peakpps_out, pps_string, sizeof(pps_string));
+		rate_print(ifrates->peakactivity_out, bps_string, sizeof(bps_string));
+		rate_print_pps(ifrates->peakpps_out, pps_string, sizeof(pps_string));
 		fprintf(fd, "Peak outgoing rate: %s, %s\n\n", bps_string, pps_string);
 	}
 	fprintf(fd, "IP checksum errors: %llu\n\n", ts->bad.pc_packets);
@@ -161,6 +187,141 @@ static void writedstatlog(char *ifname,
 	fflush(fd);
 }
 
+static void ifcounts_init(struct ifcounts *ifcounts)
+{
+	if (ifcounts == NULL)
+		return;
+
+	proto_counter_reset(&ifcounts->total);
+	pkt_counter_reset(&ifcounts->bad);
+	proto_counter_reset(&ifcounts->ipv4);
+	proto_counter_reset(&ifcounts->ipv6);
+	proto_counter_reset(&ifcounts->nonip);
+	proto_counter_reset(&ifcounts->bcast);
+
+	proto_counter_reset(&ifcounts->tcp);
+	proto_counter_reset(&ifcounts->udp);
+	proto_counter_reset(&ifcounts->icmp);
+	proto_counter_reset(&ifcounts->other);
+
+	proto_counter_reset(&ifcounts->span);
+	pkt_counter_reset(&ifcounts->span_bcast);
+}
+
+static void ifrates_init(struct ifrates *ifrates)
+{
+	if (ifrates == NULL)
+		return;
+
+	rate_alloc(&ifrates->rate, 5);
+	rate_alloc(&ifrates->rate_in, 5);
+	rate_alloc(&ifrates->rate_out, 5);
+	rate_alloc(&ifrates->rate_bcast, 5);
+
+	ifrates->activity = 0UL;
+	ifrates->peakactivity = 0UL;
+	ifrates->activity_in = 0UL;
+	ifrates->peakactivity_in = 0UL;
+	ifrates->activity_out = 0UL;
+	ifrates->peakactivity_out = 0UL;
+	ifrates->activity_bcast = 0UL;
+
+	rate_alloc(&ifrates->pps_rate, 5);
+	rate_alloc(&ifrates->pps_rate_in, 5);
+	rate_alloc(&ifrates->pps_rate_out, 5);
+	rate_alloc(&ifrates->pps_rate_bcast, 5);
+
+	ifrates->pps = 0UL;
+	ifrates->peakpps = 0UL;
+	ifrates->pps_in = 0UL;
+	ifrates->peakpps_in = 0UL;
+	ifrates->pps_out = 0UL;
+	ifrates->peakpps_out = 0UL;
+	ifrates->pps_bcast = 0UL;
+}
+
+static void ifrates_destroy(struct ifrates *ifrates)
+{
+	if (ifrates == NULL)
+		return;
+
+	rate_destroy(&ifrates->pps_rate_bcast);
+	rate_destroy(&ifrates->pps_rate_out);
+	rate_destroy(&ifrates->pps_rate_in);
+	rate_destroy(&ifrates->pps_rate);
+
+	rate_destroy(&ifrates->rate_bcast);
+	rate_destroy(&ifrates->rate_out);
+	rate_destroy(&ifrates->rate_in);
+	rate_destroy(&ifrates->rate);
+}
+
+static void ifrates_update(struct ifrates *ifrates, struct ifcounts *ifcounts,
+			   unsigned long msecs)
+{
+	rate_add_rate(&ifrates->rate, ifcounts->span.proto_total.pc_bytes, msecs);
+	ifrates->activity = rate_get_average(&ifrates->rate);
+	rate_add_rate(&ifrates->rate_in, ifcounts->span.proto_in.pc_bytes, msecs);
+	ifrates->activity_in = rate_get_average(&ifrates->rate_in);
+	rate_add_rate(&ifrates->rate_out, ifcounts->span.proto_out.pc_bytes, msecs);
+	ifrates->activity_out = rate_get_average(&ifrates->rate_out);
+	rate_add_rate(&ifrates->rate_bcast, ifcounts->span_bcast.pc_bytes, msecs);
+	ifrates->activity_bcast = rate_get_average(&ifrates->rate_bcast);
+
+	rate_add_rate(&ifrates->pps_rate, ifcounts->span.proto_total.pc_packets,
msecs);
+	ifrates->pps = rate_get_average(&ifrates->pps_rate);
+	rate_add_rate(&ifrates->pps_rate_in, ifcounts->span.proto_in.pc_packets,
msecs);
+	ifrates->pps_in = rate_get_average(&ifrates->pps_rate_in);
+	rate_add_rate(&ifrates->pps_rate_out, ifcounts->span.proto_out.pc_packets,
msecs);
+	ifrates->pps_out = rate_get_average(&ifrates->pps_rate_out);
+	rate_add_rate(&ifrates->pps_rate_bcast, ifcounts->span_bcast.pc_packets,
msecs);
+	ifrates->pps_bcast = rate_get_average(&ifrates->pps_rate_bcast);
+
+	proto_counter_reset(&ifcounts->span);
+	pkt_counter_reset(&ifcounts->span_bcast);
+
+	if (ifrates->activity > ifrates->peakactivity)
+		ifrates->peakactivity = ifrates->activity;
+
+	if (ifrates->activity_in > ifrates->peakactivity_in)
+		ifrates->peakactivity_in = ifrates->activity_in;
+
+	if (ifrates->activity_out > ifrates->peakactivity_out)
+		ifrates->peakactivity_out = ifrates->activity_out;
+
+	if (ifrates->pps > ifrates->peakpps)
+		ifrates->peakpps = ifrates->pps;
+
+	if (ifrates->pps_in > ifrates->peakpps_in)
+		ifrates->peakpps_in = ifrates->pps_in;
+
+	if (ifrates->pps_out > ifrates->peakpps_out)
+		ifrates->peakpps_out = ifrates->pps_out;
+}
+
+static void ifrates_show(struct ifrates *ifrates, WINDOW *statwin)
+{
+	char buf[64];
+
+	wattrset(statwin, HIGHATTR);
+	rate_print(ifrates->activity, buf, sizeof(buf));
+	mvwprintw(statwin, 14, 19, "%s", buf);
+	rate_print_pps(ifrates->pps, buf, sizeof(buf));
+	mvwprintw(statwin, 15, 19, "%s", buf);
+	rate_print(ifrates->activity_in, buf, sizeof(buf));
+	mvwprintw(statwin, 17, 19, "%s", buf);
+	rate_print_pps(ifrates->pps_in, buf, sizeof(buf));
+	mvwprintw(statwin, 18, 19, "%s", buf);
+	rate_print(ifrates->activity_out, buf, sizeof(buf));
+	mvwprintw(statwin, 20, 19, "%s", buf);
+	rate_print_pps(ifrates->pps_out, buf, sizeof(buf));
+	mvwprintw(statwin, 21, 19, "%s", buf);
+	rate_print(ifrates->activity_bcast, buf, sizeof(buf));
+	mvwprintw(statwin, 14, 64, "%s", buf);
+	rate_print_pps(ifrates->pps_bcast, buf, sizeof(buf));
+	mvwprintw(statwin, 15, 64, "%s", buf);
+}
+
 static void printdetlabels(WINDOW * win)
 {
 	wattrset(win, BOXATTR);
@@ -258,6 +419,7 @@ void detstats(char *iface, time_t facilitytime)
 	unsigned int iplen = 0;
 
 	struct ifcounts ifcounts;
+	struct ifrates ifrates;
 
 	int ch;
 
@@ -269,25 +431,6 @@ void detstats(char *iface, time_t facilitytime)
 	time_t statbegin;
 	time_t startlog;
 
-	struct proto_counter span;
-	struct pkt_counter span_bcast;
-
-	struct rate rate;
-	struct rate rate_in;
-	struct rate rate_out;
-	struct rate rate_bcast;
-	unsigned long peakactivity = 0;
-	unsigned long peakactivity_in = 0;
-	unsigned long peakactivity_out = 0;
-
-	struct rate pps_rate;
-	struct rate pps_rate_in;
-	struct rate pps_rate_out;
-	struct rate pps_rate_bcast;
-	unsigned long peakpps = 0;
-	unsigned long peakpps_in = 0;
-	unsigned long peakpps_out = 0;
-
 	int fd;
 
 	struct pkt_hdr pkt;
@@ -320,8 +463,6 @@ void detstats(char *iface, time_t facilitytime)
 	update_panels();
 	doupdate();
 
-	memset(&ifcounts, 0, sizeof(struct ifcounts));
-
 	if (logging) {
 		if (strcmp(current_logfile, "") == 0) {
 			snprintf(current_logfile, 64, "%s-%s.log", DSTATLOG,
@@ -346,23 +487,14 @@ void detstats(char *iface, time_t facilitytime)
 			 "******** Detailed interface statistics started ********");
 	}
 
+	ifcounts_init(&ifcounts);
+	ifrates_init(&ifrates);
+
 	printdetlabels(statwin);
 	printdetails(&ifcounts, statwin);
 	update_panels();
 	doupdate();
 
-	proto_counter_reset(&span);
-	pkt_counter_reset(&span_bcast);
-	rate_alloc(&rate, 5);
-	rate_alloc(&rate_in, 5);
-	rate_alloc(&rate_out, 5);
-	rate_alloc(&rate_bcast, 5);
-
-	rate_alloc(&pps_rate, 5);
-	rate_alloc(&pps_rate_in, 5);
-	rate_alloc(&pps_rate_out, 5);
-	rate_alloc(&pps_rate_bcast, 5);
-
 	gettimeofday(&tv, NULL);
 	start_tv = tv;
 	updtime = tv;
@@ -393,12 +525,9 @@ void detstats(char *iface, time_t facilitytime)
 		now = tv.tv_sec;
 
 		if ((now - starttime) >= 1) {
-			char buf[64];
-			unsigned long activity, activity_in, activity_out;
-			unsigned long activity_bcast;
-			unsigned long pps, pps_in, pps_out;
-			unsigned long pps_bcast;
-			unsigned long msecs;
+			unsigned long msecs = timeval_diff_msec(&tv, &start_tv);
+			ifrates_update(&ifrates, &ifcounts, msecs);
+			ifrates_show(&ifrates, statwin);
 
 			wattrset(statwin, BOXATTR);
 			printelapsedtime(statbegin, now, LINES - 3, 1, statwin);
@@ -406,76 +535,14 @@ void detstats(char *iface, time_t facilitytime)
 			dropped += packet_get_dropped(fd);
 			print_packet_drops(dropped, statwin, LINES - 3, 49);
 
-			msecs = timeval_diff_msec(&tv, &start_tv);
-
-			rate_add_rate(&rate, span.proto_total.pc_bytes, msecs);
-			activity = rate_get_average(&rate);
-			rate_add_rate(&rate_in, span.proto_in.pc_bytes, msecs);
-			activity_in = rate_get_average(&rate_in);
-			rate_add_rate(&rate_out, span.proto_out.pc_bytes, msecs);
-			activity_out = rate_get_average(&rate_out);
-			rate_add_rate(&rate_bcast, span_bcast.pc_bytes, msecs);
-			activity_bcast = rate_get_average(&rate_bcast);
-
-			rate_add_rate(&pps_rate, span.proto_total.pc_packets, msecs);
-			pps = rate_get_average(&pps_rate);
-			rate_add_rate(&pps_rate_in, span.proto_in.pc_packets, msecs);
-			pps_in = rate_get_average(&pps_rate_in);
-			rate_add_rate(&pps_rate_out, span.proto_out.pc_packets, msecs);
-			pps_out = rate_get_average(&pps_rate_out);
-			rate_add_rate(&pps_rate_bcast, span_bcast.pc_packets, msecs);
-			pps_bcast = rate_get_average(&pps_rate_bcast);
-
-			proto_counter_reset(&span);
-			pkt_counter_reset(&span_bcast);
-
 			starttime = now;
 			start_tv = tv;
-
-			wattrset(statwin, HIGHATTR);
-			rate_print(activity, buf, sizeof(buf));
-			mvwprintw(statwin, 14, 19, "%s", buf);
-			rate_print_pps(pps, buf, sizeof(buf));
-			mvwprintw(statwin, 15, 19, "%s", buf);
-			rate_print(activity_in, buf, sizeof(buf));
-			mvwprintw(statwin, 17, 19, "%s", buf);
-			rate_print_pps(pps_in, buf, sizeof(buf));
-			mvwprintw(statwin, 18, 19, "%s", buf);
-			rate_print(activity_out, buf, sizeof(buf));
-			mvwprintw(statwin, 20, 19, "%s", buf);
-			rate_print_pps(pps_out, buf, sizeof(buf));
-			mvwprintw(statwin, 21, 19, "%s", buf);
-			rate_print(activity_bcast, buf, sizeof(buf));
-			mvwprintw(statwin, 14, 64, "%s", buf);
-			rate_print_pps(pps_bcast, buf, sizeof(buf));
-			mvwprintw(statwin, 15, 64, "%s", buf);
-
-			if (activity > peakactivity)
-				peakactivity = activity;
-
-			if (activity_in > peakactivity_in)
-				peakactivity_in = activity_in;
-
-			if (activity_out > peakactivity_out)
-				peakactivity_out = activity_out;
-
-			if (pps > peakpps)
-				peakpps = pps;
-
-			if (pps_in > peakpps_in)
-				peakpps_in = pps_in;
-
-			if (pps_out > peakpps_out)
-				peakpps_out = pps_out;
 		}
 		if (logging) {
 			check_rotate_flag(&logfile);
 			if ((now - startlog) >= options.logspan) {
-				writedstatlog(iface,
-					      peakactivity, peakpps,
-					      peakactivity_in, peakpps_in,
-					      peakactivity_out, peakpps_out,
-					      &ifcounts, time(NULL) - statbegin,
+				writedstatlog(iface, &ifcounts, &ifrates,
+					      time(NULL) - statbegin,
 					      logfile);
 
 				startlog = now;
@@ -541,10 +608,10 @@ void detstats(char *iface, time_t facilitytime)
 		proto_counter_update(&ifcounts.total, outgoing, pkt.pkt_len);
 		if (pkt.from->sll_pkttype == PACKET_BROADCAST) {
 			proto_counter_update(&ifcounts.bcast, outgoing, pkt.pkt_len);
-			pkt_counter_update(&span_bcast, pkt.pkt_len);
+			pkt_counter_update(&ifcounts.span_bcast, pkt.pkt_len);
 		}
 
-		proto_counter_update(&span, outgoing, pkt.pkt_len);
+		proto_counter_update(&ifcounts.span, outgoing, pkt.pkt_len);
 
 		/* account network layer protocol */
 		switch(pkt.pkt_protocol) {
@@ -592,15 +659,7 @@ err_close:
 	close(fd);
 
 err:
-	rate_destroy(&pps_rate_bcast);
-	rate_destroy(&pps_rate_out);
-	rate_destroy(&pps_rate_in);
-	rate_destroy(&pps_rate);
-
-	rate_destroy(&rate_bcast);
-	rate_destroy(&rate_out);
-	rate_destroy(&rate_in);
-	rate_destroy(&rate);
+	ifrates_destroy(&ifrates);
 
 	if (options.promisc) {
 		promisc_restore_list(&promisc);
@@ -609,11 +668,8 @@ err:
 
 	if (logging) {
 		signal(SIGUSR1, SIG_DFL);
-		writedstatlog(iface,
-			      peakactivity, peakpps, peakactivity_in,
-			      peakpps_in, peakactivity_out, peakpps_out,
-			      &ifcounts, time(NULL) - statbegin,
-			      logfile);
+		writedstatlog(iface, &ifcounts, &ifrates,
+			      time(NULL) - statbegin, logfile);
 		writelog(logging, logfile,
 			 "******** Detailed interface statistics stopped ********");
 		fclose(logfile);
-- 
1.9.1

[PATCH 07/10] detstats(): call init/destroy functions in LIFO order

[Vitezslav Samel]

... to prevent use of uninitialized variables.

Signed-off-by: Vitezslav Samel <vitezslav(a)samel.cz&gt;
---
 src/detstats.c | 86 ++++++++++++++++++++++++++++++----------------------------
 1 file changed, 44 insertions(+), 42 deletions(-)

diff --git a/src/detstats.c b/src/detstats.c
index 5b316e2..6d66082 100644
--- a/src/detstats.c
+++ b/src/detstats.c
@@ -208,6 +208,11 @@ static void ifcounts_init(struct ifcounts *ifcounts)
 	pkt_counter_reset(&ifcounts->span_bcast);
 }
 
+static void ifcounts_destroy(struct ifcounts *ifcounts __unused)
+{
+	/* do nothing for now */
+}
+
 static void ifrates_init(struct ifrates *ifrates)
 {
 	if (ifrates == NULL)
@@ -442,12 +447,6 @@ void detstats(char *iface, time_t facilitytime)
 		return;
 	}
 
-	LIST_HEAD(promisc);
-	if (options.promisc) {
-		promisc_init(&promisc, iface);
-		promisc_set_list(&promisc);
-	}
-
 	move(LINES - 1, 1);
 	stdexitkeyhelp();
 	statwin = newwin(LINES - 2, COLS, 1, 0);
@@ -460,9 +459,30 @@ void detstats(char *iface, time_t facilitytime)
 	wmove(statwin, 0, 1);
 	wprintw(statwin, " Statistics for %s ", iface);
 	wattrset(statwin, STDATTR);
+	leaveok(statwin, TRUE);
+
 	update_panels();
 	doupdate();
 
+	LIST_HEAD(promisc);
+	if (options.promisc) {
+		promisc_init(&promisc, iface);
+		promisc_set_list(&promisc);
+	}
+
+	fd = socket(PF_PACKET, SOCK_RAW, htons(ETH_P_ALL));
+	if(fd == -1) {
+		write_error("Unable to obtain monitoring socket");
+		goto err;
+	}
+	if(dev_bind_ifname(fd, iface) == -1) {
+		write_error("Unable to bind interface on the socket");
+		goto err_close;
+	}
+
+	ifcounts_init(&ifcounts);
+	ifrates_init(&ifrates);
+
 	if (logging) {
 		if (strcmp(current_logfile, "") == 0) {
 			snprintf(current_logfile, 64, "%s-%s.log", DSTATLOG,
@@ -487,39 +507,21 @@ void detstats(char *iface, time_t facilitytime)
 			 "******** Detailed interface statistics started ********");
 	}
 
-	ifcounts_init(&ifcounts);
-	ifrates_init(&ifrates);
-
 	printdetlabels(statwin);
 	printdetails(&ifcounts, statwin);
 	update_panels();
 	doupdate();
 
+	packet_init(&pkt);
+
+	exitloop = 0;
+
 	gettimeofday(&tv, NULL);
 	start_tv = tv;
 	updtime = tv;
 	starttime = startlog = statbegin = tv.tv_sec;
 
-	leaveok(statwin, TRUE);
-
-	fd = socket(PF_PACKET, SOCK_RAW, htons(ETH_P_ALL));
-	if(fd == -1) {
-		write_error("Unable to obtain monitoring socket");
-		goto err;
-	}
-	if(dev_bind_ifname(fd, iface) == -1) {
-		write_error("Unable to bind interface on the socket");
-		goto err_close;
-	}
-
-	exitloop = 0;
-
-	packet_init(&pkt);
-
-	/*
-	 * Data-gathering loop
-	 */
-
+	/* data-gathering loop */
 	while (!exitloop) {
 		gettimeofday(&tv, NULL);
 		now = tv.tv_sec;
@@ -654,17 +656,7 @@ void detstats(char *iface, time_t facilitytime)
 			break;
 		}
 	}
-
-err_close:
-	close(fd);
-
-err:
-	ifrates_destroy(&ifrates);
-
-	if (options.promisc) {
-		promisc_restore_list(&promisc);
-		promisc_destroy(&promisc);
-	}
+	packet_destroy(&pkt);
 
 	if (logging) {
 		signal(SIGUSR1, SIG_DFL);
@@ -674,11 +666,21 @@ err:
 			 "******** Detailed interface statistics stopped ********");
 		fclose(logfile);
 	}
+	strcpy(current_logfile, "");
+
+	ifrates_destroy(&ifrates);
+	ifcounts_destroy(&ifcounts);
+
+err_close:
+	close(fd);
+err:
+	if (options.promisc) {
+		promisc_restore_list(&promisc);
+		promisc_destroy(&promisc);
+	}
 
 	del_panel(statpanel);
 	delwin(statwin);
-	strcpy(current_logfile, "");
-	packet_destroy(&pkt);
 	update_panels();
 	doupdate();
 }
-- 
1.9.1

[PATCH 08/10] detstats(): optimize: move some checks away from the hot path

[Vitezslav Samel]

Signed-off-by: Vitezslav Samel <vitezslav(a)samel.cz&gt;
---
 src/detstats.c | 30 +++++++++++++++---------------
 1 file changed, 15 insertions(+), 15 deletions(-)

diff --git a/src/detstats.c b/src/detstats.c
index 6d66082..7f8c693 100644
--- a/src/detstats.c
+++ b/src/detstats.c
@@ -537,20 +537,24 @@ void detstats(char *iface, time_t facilitytime)
 			dropped += packet_get_dropped(fd);
 			print_packet_drops(dropped, statwin, LINES - 3, 49);
 
+			if ((facilitytime != 0)
+			    && (((now - statbegin) / 60) >= facilitytime))
+				exitloop = 1;
+
+			if (logging) {
+				check_rotate_flag(&logfile);
+				if ((now - startlog) >= options.logspan) {
+					writedstatlog(iface, &ifcounts, &ifrates,
+						      time(NULL) - statbegin,
+						      logfile);
+
+					startlog = now;
+				}
+			}
+
 			starttime = now;
 			start_tv = tv;
 		}
-		if (logging) {
-			check_rotate_flag(&logfile);
-			if ((now - startlog) >= options.logspan) {
-				writedstatlog(iface, &ifcounts, &ifrates,
-					      time(NULL) - statbegin,
-					      logfile);
-
-				startlog = now;
-			}
-		}
-
 		if (screen_update_needed(&tv, &updtime)) {
 			printdetails(&ifcounts, statwin);
 			update_panels();
@@ -559,10 +563,6 @@ void detstats(char *iface, time_t facilitytime)
 			updtime = tv;
 		}
 
-		if ((facilitytime != 0)
-		    && (((now - statbegin) / 60) >= facilitytime))
-			exitloop = 1;
-
 		if (packet_get(fd, &pkt, &ch, statwin) == -1) {
 			write_error("Packet receive failed");
 			exitloop = 1;
-- 
1.9.1

[PATCH 09/10] detstats(): refactor various time computing

[Vitezslav Samel]

Signed-off-by: Vitezslav Samel <vitezslav(a)samel.cz&gt;
---
 src/detstats.c | 59 +++++++++++++++++++++++++++-------------------------------
 1 file changed, 27 insertions(+), 32 deletions(-)

diff --git a/src/detstats.c b/src/detstats.c
index 7f8c693..e98cd29 100644
--- a/src/detstats.c
+++ b/src/detstats.c
@@ -428,14 +428,6 @@ void detstats(char *iface, time_t facilitytime)
 
 	int ch;
 
-	struct timeval tv;
-	struct timeval start_tv;
-	struct timeval updtime;
-	time_t starttime;
-	time_t now;
-	time_t statbegin;
-	time_t startlog;
-
 	int fd;
 
 	struct pkt_hdr pkt;
@@ -516,51 +508,54 @@ void detstats(char *iface, time_t facilitytime)
 
 	exitloop = 0;
 
-	gettimeofday(&tv, NULL);
-	start_tv = tv;
-	updtime = tv;
-	starttime = startlog = statbegin = tv.tv_sec;
+	struct timeval now;
+	gettimeofday(&now, NULL);
+	struct timeval last_time = now;
+	struct timeval last_update = now;
+
+	time_t starttime = now.tv_sec;
+	time_t endtime = INT_MAX;
+	if (facilitytime != 0)
+		endtime = now.tv_sec + facilitytime * 60;
+
+	time_t log_next = INT_MAX;
+	if (logging)
+		log_next = now.tv_sec + options.logspan;
 
 	/* data-gathering loop */
 	while (!exitloop) {
-		gettimeofday(&tv, NULL);
-		now = tv.tv_sec;
+		gettimeofday(&now, NULL);
 
-		if ((now - starttime) >= 1) {
-			unsigned long msecs = timeval_diff_msec(&tv, &start_tv);
+		if (now.tv_sec > last_time.tv_sec) {
+			unsigned long msecs = timeval_diff_msec(&now, &last_time);
 			ifrates_update(&ifrates, &ifcounts, msecs);
 			ifrates_show(&ifrates, statwin);
 
 			wattrset(statwin, BOXATTR);
-			printelapsedtime(statbegin, now, LINES - 3, 1, statwin);
+			printelapsedtime(starttime, now.tv_sec, LINES - 3, 1, statwin);
 
 			dropped += packet_get_dropped(fd);
 			print_packet_drops(dropped, statwin, LINES - 3, 49);
 
-			if ((facilitytime != 0)
-			    && (((now - statbegin) / 60) >= facilitytime))
+			if (now.tv_sec > endtime)
 				exitloop = 1;
 
-			if (logging) {
+			if (logging && (now.tv_sec > log_next)) {
 				check_rotate_flag(&logfile);
-				if ((now - startlog) >= options.logspan) {
-					writedstatlog(iface, &ifcounts, &ifrates,
-						      time(NULL) - statbegin,
-						      logfile);
-
-					startlog = now;
-				}
+				writedstatlog(iface, &ifcounts, &ifrates,
+					      now.tv_sec - starttime,
+					      logfile);
+				log_next = now.tv_sec + options.logspan;
 			}
 
-			starttime = now;
-			start_tv = tv;
+			last_time = now;
 		}
-		if (screen_update_needed(&tv, &updtime)) {
+		if (screen_update_needed(&now, &last_update)) {
 			printdetails(&ifcounts, statwin);
 			update_panels();
 			doupdate();
 
-			updtime = tv;
+			last_update = now;
 		}
 
 		if (packet_get(fd, &pkt, &ch, statwin) == -1) {
@@ -661,7 +656,7 @@ void detstats(char *iface, time_t facilitytime)
 	if (logging) {
 		signal(SIGUSR1, SIG_DFL);
 		writedstatlog(iface, &ifcounts, &ifrates,
-			      time(NULL) - statbegin, logfile);
+			      time(NULL) - starttime, logfile);
 		writelog(logging, logfile,
 			 "******** Detailed interface statistics stopped ********");
 		fclose(logfile);
-- 
1.9.1

[PATCH 10/10] detstats(): move keypress and packet handling into separate functions

[Vitezslav Samel]

Signed-off-by: Vitezslav Samel <vitezslav(a)samel.cz&gt;
---
 src/detstats.c | 187 ++++++++++++++++++++++++++++++---------------------------
 1 file changed, 98 insertions(+), 89 deletions(-)

diff --git a/src/detstats.c b/src/detstats.c
index e98cd29..f70dae4 100644
--- a/src/detstats.c
+++ b/src/detstats.c
@@ -409,9 +409,100 @@ static void printdetails(struct ifcounts *ifcounts, WINDOW * win)
 	mvwprintw(win, 19, 65, "%8lu", ifcounts->bad.pc_packets);
 }
 
-/*
- * The detailed interface statistics function
- */
+static void detstats_process_key(int ch)
+{
+	switch (ch) {
+	case 12:
+	case 'l':
+	case 'L':
+		tx_refresh_screen();
+		break;
+	case 'Q':
+	case 'q':
+	case 'X':
+	case 'x':
+	case 24:
+	case 27:
+		exitloop = 1;
+		break;
+	case ERR:
+		/* no key ready, do nothing */
+		/* fall through */
+	default:
+		/* do nothing */
+		break;
+	}
+}
+
+static void detstats_process_packet(struct ifcounts *ifcounts, struct pkt_hdr *pkt)
+{
+	int pkt_result = packet_process(pkt, NULL, NULL, NULL,
+					MATCH_OPPOSITE_USECONFIG,
+					options.v6inv4asv6);
+
+	switch (pkt_result) {
+	case PACKET_OK:			/* we only handle these */
+	case MORE_FRAGMENTS:
+	case CHECKSUM_ERROR:
+		break;
+	default:			/* drop others */
+		return;
+	}
+
+	int outgoing = (pkt->from->sll_pkttype == PACKET_OUTGOING);
+	proto_counter_update(&ifcounts->total, outgoing, pkt->pkt_len);
+	if (pkt->from->sll_pkttype == PACKET_BROADCAST) {
+		proto_counter_update(&ifcounts->bcast, outgoing, pkt->pkt_len);
+		pkt_counter_update(&ifcounts->span_bcast, pkt->pkt_len);
+	}
+
+	proto_counter_update(&ifcounts->span, outgoing, pkt->pkt_len);
+
+	unsigned int iplen = 0;
+
+	/* account network layer protocol */
+	switch(pkt->pkt_protocol) {
+	case ETH_P_IP:
+		if (pkt_result == CHECKSUM_ERROR) {
+			pkt_counter_update(&ifcounts->bad, pkt->pkt_len);
+			return;
+		}
+
+		iplen = ntohs(pkt->iphdr->tot_len);
+
+		proto_counter_update(&ifcounts->ipv4, outgoing, iplen);
+		break;
+	case ETH_P_IPV6:
+		iplen = ntohs(pkt->ip6_hdr->ip6_plen) + 40;
+
+		proto_counter_update(&ifcounts->ipv6, outgoing, iplen);
+		break;
+	default:
+		proto_counter_update(&ifcounts->nonip, outgoing, pkt->pkt_len);
+		return;
+	}
+
+	__u8 ip_protocol = pkt_ip_protocol(pkt);
+
+	/* account transport layer protocol */
+	switch (ip_protocol) {
+	case IPPROTO_TCP:
+		proto_counter_update(&ifcounts->tcp, outgoing, iplen);
+		break;
+	case IPPROTO_UDP:
+		proto_counter_update(&ifcounts->udp, outgoing, iplen);
+		break;
+	case IPPROTO_ICMP:
+	case IPPROTO_ICMPV6:
+		proto_counter_update(&ifcounts->icmp, outgoing, iplen);
+		break;
+	default:
+		proto_counter_update(&ifcounts->other, outgoing, iplen);
+		break;
+	}
+}
+
+/* detailed interface statistics function */
 void detstats(char *iface, time_t facilitytime)
 {
 	int logging = options.logging;
@@ -421,8 +512,6 @@ void detstats(char *iface, time_t facilitytime)
 
 	FILE *logfile = NULL;
 
-	unsigned int iplen = 0;
-
 	struct ifcounts ifcounts;
 	struct ifrates ifrates;
 
@@ -564,92 +653,12 @@ void detstats(char *iface, time_t facilitytime)
 			break;
 		}
 
-		switch (ch) {
-		case ERR:
-			/* no key ready, do nothing */
-			break;
-		case 12:
-		case 'l':
-		case 'L':
-			tx_refresh_screen();
-			break;
-
-		case 'Q':
-		case 'q':
-		case 'X':
-		case 'x':
-		case 24:
-		case 27:
-			exitloop = 1;
-			break;
-		}
-		if (pkt.pkt_len <= 0)
-			continue;
-
-		int outgoing;
-
-		int pkt_result = packet_process(&pkt, NULL, NULL, NULL,
-						MATCH_OPPOSITE_USECONFIG,
-						options.v6inv4asv6);
-
-		switch (pkt_result) {
-		case PACKET_OK:			/* we only handle these */
-		case MORE_FRAGMENTS:
-		case CHECKSUM_ERROR:
-			break;
-		default:			/* drop others */
-			continue;
-		}
-
-		outgoing = (pkt.from->sll_pkttype == PACKET_OUTGOING);
-		proto_counter_update(&ifcounts.total, outgoing, pkt.pkt_len);
-		if (pkt.from->sll_pkttype == PACKET_BROADCAST) {
-			proto_counter_update(&ifcounts.bcast, outgoing, pkt.pkt_len);
-			pkt_counter_update(&ifcounts.span_bcast, pkt.pkt_len);
-		}
-
-		proto_counter_update(&ifcounts.span, outgoing, pkt.pkt_len);
+		if (ch != ERR)
+			detstats_process_key(ch);
 
-		/* account network layer protocol */
-		switch(pkt.pkt_protocol) {
-		case ETH_P_IP:
-			if (pkt_result == CHECKSUM_ERROR) {
-				pkt_counter_update(&ifcounts.bad, pkt.pkt_len);
-				continue;
-			}
-
-			iplen = ntohs(pkt.iphdr->tot_len);
-
-			proto_counter_update(&ifcounts.ipv4, outgoing, iplen);
-			break;
-		case ETH_P_IPV6:
-			iplen = ntohs(pkt.ip6_hdr->ip6_plen) + 40;
-
-			proto_counter_update(&ifcounts.ipv6, outgoing, iplen);
-			break;
-		default:
-			proto_counter_update(&ifcounts.nonip, outgoing, pkt.pkt_len);
-			continue;
-		}
+		if (pkt.pkt_len > 0)
+			detstats_process_packet(&ifcounts, &pkt);
 
-		__u8 ip_protocol = pkt_ip_protocol(&pkt);
-
-		/* account transport layer protocol */
-		switch (ip_protocol) {
-		case IPPROTO_TCP:
-			proto_counter_update(&ifcounts.tcp, outgoing, iplen);
-			break;
-		case IPPROTO_UDP:
-			proto_counter_update(&ifcounts.udp, outgoing, iplen);
-			break;
-		case IPPROTO_ICMP:
-		case IPPROTO_ICMPV6:
-			proto_counter_update(&ifcounts.icmp, outgoing, iplen);
-			break;
-		default:
-			proto_counter_update(&ifcounts.other, outgoing, iplen);
-			break;
-		}
 	}
 	packet_destroy(&pkt);
 
-- 
1.9.1