On Thu, Oct 22, 2015 at 10:03:58AM +0200, Tobias Klauser wrote: > On 2015-10-20 at 19:46:07 +0200, Vadim Kochan <[email protected]> wrote: > > Calculate & print the rate of src/dst bytes & pkts. > > Also changed refresh flows time to 1s so the rate > > info will be not disappeared very soon. > > Looks good to me in general and I like the idea. A few minor comments > below. > > > Signed-off-by: Vadim Kochan <[email protected]> > > --- > > flowtop.c | 90 > > ++++++++++++++++++++++++++++++++++++++++++++++++++++++++------- > > 1 file changed, 80 insertions(+), 10 deletions(-) > > > > diff --git a/flowtop.c b/flowtop.c > > index 9df4bdb..c348e33 100644 > > --- a/flowtop.c > > +++ b/flowtop.c > > @@ -19,6 +19,7 @@ > > #include <curses.h> > > #include <dirent.h> > > #include <sys/stat.h> > > +#include <sys/time.h> > > #include <sys/fsuid.h> > > #include <urcu.h> > > #include <libgen.h> > > @@ -64,6 +65,11 @@ struct flow_entry { > > unsigned int procnum; > > bool is_visible; > > struct nf_conntrack *ct; > > + struct timeval last_update; > > + double rate_bytes_src; > > + double rate_bytes_dst; > > + double rate_pkts_src; > > + double rate_pkts_dst; > > }; > > > > struct flow_list { > > @@ -197,6 +203,18 @@ static const struct nfct_filter_ipv6 filter_ipv6 = { > > .mask = { 0xffffffff, 0xffffffff, 0xffffffff, 0xffffffff }, > > }; > > > > +static double time_after_us(struct timeval *tv) > > No need to return a double here. Make this an int64_t... > > > +{ > > + struct timeval now; > > + > > + gettimeofday(&now, NULL); > > + > > + now.tv_sec -= tv->tv_sec; > > + now.tv_usec -= tv->tv_usec; > > + > > + return (now.tv_sec * 1000000.0 + now.tv_usec); > > +} > > + > > static void signal_handler(int number) > > { > > switch (number) { > > @@ -252,6 +270,28 @@ static void version(void) > > die(); > > } > > > > +static void flow_entry_update_time(struct flow_entry *n) > > +{ > > + gettimeofday(&n->last_update, NULL); > > +} > > + > > +static void flow_entry_calc_rate(struct flow_entry *n, struct nf_conntrack > > *ct) > > +{ > > + uint64_t bytes_src = nfct_get_attr_u64(ct, ATTR_ORIG_COUNTER_BYTES); > > + uint64_t bytes_dst = nfct_get_attr_u64(ct, ATTR_REPL_COUNTER_BYTES); > > + uint64_t pkts_src = nfct_get_attr_u64(ct, ATTR_ORIG_COUNTER_PACKETS); > > + uint64_t pkts_dst = nfct_get_attr_u64(ct, ATTR_REPL_COUNTER_PACKETS); > > + double sec = time_after_us(&n->last_update) / 1000000.0; > > ...here it will become a double anyhow. > > > + > > + if (sec <= 0) > > + return; > > + > > + n->rate_bytes_src = (bytes_src - n->bytes_src) / sec; > > + n->rate_bytes_dst = (bytes_dst - n->bytes_dst) / sec; > > + n->rate_pkts_src = (pkts_src - n->pkts_src) / sec; > > + n->rate_pkts_dst = (pkts_dst - n->pkts_dst) / sec; > > I don't think we need the alignment here, it's readable enough without > the extra spaces. > > > +} > > + > > static inline struct flow_entry *flow_entry_xalloc(void) > > { > > return xzmalloc(sizeof(struct flow_entry)); > > @@ -293,6 +333,7 @@ static void flow_list_new_entry(struct flow_list *fl, > > struct nf_conntrack *ct) > > > > n->ct = nfct_clone(ct); > > > > + flow_entry_update_time(n); > > flow_entry_from_ct(n, ct); > > flow_entry_get_extended(n); > > > > @@ -736,25 +777,48 @@ static uint16_t presenter_get_port(uint16_t src, > > uint16_t dst, bool is_tcp) > > static char *bandw2str(double bytes, char *buf, size_t len) > > { > > if (bytes > 1000000000.) > > - snprintf(buf, len, "%.1fG", bytes / 1000000000.); > > + snprintf(buf, len, "%.1fGb", bytes / 1000000000.); > > else if (bytes > 1000000.) > > - snprintf(buf, len, "%.1fM", bytes / 1000000.); > > + snprintf(buf, len, "%.1fMb", bytes / 1000000.); > > else if (bytes > 1000.) > > - snprintf(buf, len, "%.1fK", bytes / 1000.); > > + snprintf(buf, len, "%.1fKb", bytes / 1000.); > > else > > - snprintf(buf, len, "%g", bytes); > > + snprintf(buf, len, "%g bytes", bytes); > > Please split this change out into a separate preparatory patch as it is > not directly related to the rate change. This makes it easier to review > and bisect. > > > > > return buf; > > } > > > > -static void presenter_print_counters(uint64_t bytes, uint64_t pkts, int > > color) > > +static char *rate2str(double rate, char *buf, size_t len) > > +{ > > + if (rate > 1000000000.) > > + snprintf(buf, len, "%.1fGb/s", rate / 1000000000.); > > + else if (rate > 1000000.) > > + snprintf(buf, len, "%.1fMb/s", rate / 1000000.); > > + else if (rate > 1000.) > > + snprintf(buf, len, "%.1fKb/s", rate / 1000.); > > + else > > + snprintf(buf, len, "%gB/s", rate); > > + > > + return buf; > > +} > > + > > +static void presenter_print_counters(uint64_t bytes, uint64_t pkts, > > + double rate_bytes, double rate_pkts, > > + int color) > > { > > char bytes_str[64]; > > > > printw(" -> ("); > > attron(COLOR_PAIR(color)); > > - printw("%"PRIu64" pkts, ", pkts); > > - printw("%s bytes", bandw2str(bytes, bytes_str, sizeof(bytes_str) - 1)); > > + printw("%"PRIu64" pkts", pkts); > > + if (rate_pkts) { > > + printw("(%.1fpps)", rate_pkts); > > + } > > Omit the braces here. > > > + printw(", %s", bandw2str(bytes, bytes_str, sizeof(bytes_str) - 1)); > > + if (rate_bytes) { > > + printw("(%s)", rate2str(rate_bytes, bytes_str, > > + sizeof(bytes_str) - 1)); > > + } > > Same here. > > > attroff(COLOR_PAIR(color)); > > printw(")"); > > } > > @@ -872,7 +936,9 @@ static void presenter_screen_do_line(WINDOW *screen, > > struct flow_entry *n, > > } > > > > if (n->pkts_src > 0 && n->bytes_src > 0) > > - presenter_print_counters(n->bytes_src, n->pkts_src, 1); > > + presenter_print_counters(n->bytes_src, n->pkts_src, > > + n->rate_bytes_src, > > + n->rate_pkts_src, 1); > > > > printw(" => "); > > } > > @@ -898,7 +964,9 @@ static void presenter_screen_do_line(WINDOW *screen, > > struct flow_entry *n, > > } > > > > if (n->pkts_dst > 0 && n->bytes_dst > 0) > > - presenter_print_counters(n->bytes_dst, n->pkts_dst, 2); > > + presenter_print_counters(n->bytes_dst, n->pkts_dst, > > + n->rate_bytes_dst, > > + n->rate_pkts_dst, 2); > > } > > > > static inline bool presenter_flow_wrong_state(struct flow_entry *n) > > @@ -1176,6 +1244,8 @@ static int flow_update_cb(enum nf_conntrack_msg_type > > type, > > if (!n) > > return NFCT_CB_CONTINUE; > > > > + flow_entry_calc_rate(n, ct); > > + flow_entry_update_time(n); > > flow_entry_from_ct(n, ct); > > > > return NFCT_CB_CONTINUE; > > @@ -1372,7 +1442,7 @@ static void *collector(void *null __maybe_unused) > > while (!sigint) { > > int status; > > > > - usleep(300000); > > + usleep(1000000); > > > > collector_refresh_flows(ct_update); > > > > -- > > 2.4.2 > >
Thanks Tobias, I will follow your suggestions! -- You received this message because you are subscribed to the Google Groups "netsniff-ng" group. To unsubscribe from this group and stop receiving emails from it, send an email to [email protected]. For more options, visit https://groups.google.com/d/optout.
