[netsniff-ng] Re: [PATCH 2/5] flowtop: Change flows layout to 1-row view

2016-04-17 Thread Vadim Kochan 
On Tue, Mar 29, 2016 at 03:38:58PM +0200, Tobias Klauser wrote:
> On 2016-03-29 at 15:32:43 +0200, Vadim Kochan  wrote:
> > On Tue, Mar 29, 2016 at 4:23 PM, Tobias Klauser  wrote:
> > > On 2016-03-23 at 22:00:44 +0100, Vadim Kochan  wrote:
> > >> Changed flows list layout to look more a top-like output
> > >> with header and in 1 line. When -s option is specified
> > >> then layout changes to 2 lines view including with src peer
> > >> info and dst under it on next line.
> > >>
> > >> Also shortified flow state names to allocate less space.
> > >>
> > >> Removed presenter_get_port be cause ports are printed for both peers
> > >> separately.
> > >>
> > >> The flow duration time is printed in very short form in one of the
> > >> units:
> > >> XXd - days
> > >> XXh - hours
> > >> XXm - minutes
> > >> XXs - seconds
> > >>
> > >> the reason is that it is enough to have actually generic understanding
> > >> about flow time in the biggest time unit.
> > >>
> > >> Signed-off-by: Vadim Kochan 
> > >> ---
> > >>  flowtop.c | 405 
> > >> ++
> > >>  1 file changed, 194 insertions(+), 211 deletions(-)
> > >>
> > >> diff --git a/flowtop.c b/flowtop.c
> > >> index 4c15c06..8201321 100644
> > >> --- a/flowtop.c
> > >> +++ b/flowtop.c
> > >> @@ -62,6 +62,7 @@ struct flow_entry {
> > >>   uint64_t pkts_dst, bytes_dst;
> > >>   uint64_t timestamp_start, timestamp_stop;
> > >>   char country_src[128], country_dst[128];
> > >> + char country_code_src[4], country_code_dst[4];
> > >>   char city_src[128], city_dst[128];
> > >>   char rev_dns_src[256], rev_dns_dst[256];
> > >>   char procname[256];
> > >> @@ -166,11 +167,6 @@ static const char *copyright = "Please report bugs 
> > >> to  > >>   "This is free software: you are free to change and redistribute 
> > >> it.\n"
> > >>   "There is NO WARRANTY, to the extent permitted by law.";
> > >>
> > >> -static const char *const l3proto2str[AF_MAX] = {
> > >> - [AF_INET]   = "ipv4",
> > >> - [AF_INET6]  = "ipv6",
> > >> -};
> > >
> > > Why remove L3 protocol information from the output? I consider this
> > > quite useful. Could we somehow combine this with L4 Proto information in
> > > a generic way?
> > 
> > I thought it will be easy to identify ipvX version by IPvX address format.
> 
> True, didn't think of it that way. I'm fine with omitting it in that case...
> 
> > >
> > >> -
> > >>  static const char *const l4proto2str[IPPROTO_MAX] = {
> > >>   [IPPROTO_TCP]   = "tcp",
> > >>   [IPPROTO_UDP]   = "udp",
> > >> @@ -194,40 +190,40 @@ static const char *const l4proto2str[IPPROTO_MAX] 
> > >> = {
> > >>  };
> > >>
> > >>  static const char *const tcp_state2str[TCP_CONNTRACK_MAX] = {
> > >> - [TCP_CONNTRACK_NONE]= "NOSTATE",
> > >> - [TCP_CONNTRACK_SYN_SENT]= "SYN_SENT",
> > >> - [TCP_CONNTRACK_SYN_RECV]= "SYN_RECV",
> > >> - [TCP_CONNTRACK_ESTABLISHED] = "ESTABLISHED",
> > >> - [TCP_CONNTRACK_FIN_WAIT]= "FIN_WAIT",
> > >> - [TCP_CONNTRACK_CLOSE_WAIT]  = "CLOSE_WAIT",
> > >> - [TCP_CONNTRACK_LAST_ACK]= "LAST_ACK",
> > >> - [TCP_CONNTRACK_TIME_WAIT]   = "TIME_WAIT",
> > >> - [TCP_CONNTRACK_CLOSE]   = "CLOSE",
> > >> - [TCP_CONNTRACK_SYN_SENT2]   = "SYN_SENT2",
> > >> + [TCP_CONNTRACK_NONE]= "NO",
> > >> + [TCP_CONNTRACK_SYN_SENT]= "SS",
> > >> + [TCP_CONNTRACK_SYN_RECV]= "SR",
> > >> + [TCP_CONNTRACK_ESTABLISHED] = "EST",
> > >> + [TCP_CONNTRACK_FIN_WAIT]= "FWT",
> > >> + [TCP_CONNTRACK_CLOSE_WAIT]  = "CWT",
> > >> + [TCP_CONNTRACK_LAST_ACK]= "LAC",
> > >> + [TCP_CONNTRACK_TIME_WAIT]   = "TWT",
> > >> + [TCP_CONNTRACK_CLOSE]   = "CLO",
> > >> + [TCP_CONNTRACK_SYN_SENT2]   = "SS2",
> > >
> > >
> > > These abbreviations are no longer easy to grasp for the user without
> > > looking at this struct in the source. We should either keep the long
> > > names (if possible) of at least add corresponding documentation about
> > > the abbreviations to the manpage. Same goes for dccp_state2str and
> > > sctp_state2str below.
> > 
> > OK, what do you prefer ? I just tried to minimize the column width to
> > fit into at least 100-sized screen.
> > But OK, I will try to see how much character long names will occupy.
> 
> I'd prefer the long version, but if you can't fit it into the width
> without too many compromises I think the abbreviations are fine as well.
> But they should be documented at least, maybe even in the help screen.
> 
> > >>
> > >> -static void draw_flow_entry(WINDOW *screen, const struct flow_entry *n,
> > >> - unsigned int *line)
> > >>

[netsniff-ng] Re: [PATCH 2/5] flowtop: Change flows layout to 1-row view

2016-03-29 Thread Tobias Klauser 
On 2016-03-29 at 15:32:43 +0200, Vadim Kochan  wrote:
> On Tue, Mar 29, 2016 at 4:23 PM, Tobias Klauser  wrote:
> > On 2016-03-23 at 22:00:44 +0100, Vadim Kochan  wrote:
> >> Changed flows list layout to look more a top-like output
> >> with header and in 1 line. When -s option is specified
> >> then layout changes to 2 lines view including with src peer
> >> info and dst under it on next line.
> >>
> >> Also shortified flow state names to allocate less space.
> >>
> >> Removed presenter_get_port be cause ports are printed for both peers
> >> separately.
> >>
> >> The flow duration time is printed in very short form in one of the
> >> units:
> >> XXd - days
> >> XXh - hours
> >> XXm - minutes
> >> XXs - seconds
> >>
> >> the reason is that it is enough to have actually generic understanding
> >> about flow time in the biggest time unit.
> >>
> >> Signed-off-by: Vadim Kochan 
> >> ---
> >>  flowtop.c | 405 
> >> ++
> >>  1 file changed, 194 insertions(+), 211 deletions(-)
> >>
> >> diff --git a/flowtop.c b/flowtop.c
> >> index 4c15c06..8201321 100644
> >> --- a/flowtop.c
> >> +++ b/flowtop.c
> >> @@ -62,6 +62,7 @@ struct flow_entry {
> >>   uint64_t pkts_dst, bytes_dst;
> >>   uint64_t timestamp_start, timestamp_stop;
> >>   char country_src[128], country_dst[128];
> >> + char country_code_src[4], country_code_dst[4];
> >>   char city_src[128], city_dst[128];
> >>   char rev_dns_src[256], rev_dns_dst[256];
> >>   char procname[256];
> >> @@ -166,11 +167,6 @@ static const char *copyright = "Please report bugs to 
> >>  >>   "This is free software: you are free to change and redistribute 
> >> it.\n"
> >>   "There is NO WARRANTY, to the extent permitted by law.";
> >>
> >> -static const char *const l3proto2str[AF_MAX] = {
> >> - [AF_INET]   = "ipv4",
> >> - [AF_INET6]  = "ipv6",
> >> -};
> >
> > Why remove L3 protocol information from the output? I consider this
> > quite useful. Could we somehow combine this with L4 Proto information in
> > a generic way?
> 
> I thought it will be easy to identify ipvX version by IPvX address format.

True, didn't think of it that way. I'm fine with omitting it in that case...

> >
> >> -
> >>  static const char *const l4proto2str[IPPROTO_MAX] = {
> >>   [IPPROTO_TCP]   = "tcp",
> >>   [IPPROTO_UDP]   = "udp",
> >> @@ -194,40 +190,40 @@ static const char *const l4proto2str[IPPROTO_MAX] = {
> >>  };
> >>
> >>  static const char *const tcp_state2str[TCP_CONNTRACK_MAX] = {
> >> - [TCP_CONNTRACK_NONE]= "NOSTATE",
> >> - [TCP_CONNTRACK_SYN_SENT]= "SYN_SENT",
> >> - [TCP_CONNTRACK_SYN_RECV]= "SYN_RECV",
> >> - [TCP_CONNTRACK_ESTABLISHED] = "ESTABLISHED",
> >> - [TCP_CONNTRACK_FIN_WAIT]= "FIN_WAIT",
> >> - [TCP_CONNTRACK_CLOSE_WAIT]  = "CLOSE_WAIT",
> >> - [TCP_CONNTRACK_LAST_ACK]= "LAST_ACK",
> >> - [TCP_CONNTRACK_TIME_WAIT]   = "TIME_WAIT",
> >> - [TCP_CONNTRACK_CLOSE]   = "CLOSE",
> >> - [TCP_CONNTRACK_SYN_SENT2]   = "SYN_SENT2",
> >> + [TCP_CONNTRACK_NONE]= "NO",
> >> + [TCP_CONNTRACK_SYN_SENT]= "SS",
> >> + [TCP_CONNTRACK_SYN_RECV]= "SR",
> >> + [TCP_CONNTRACK_ESTABLISHED] = "EST",
> >> + [TCP_CONNTRACK_FIN_WAIT]= "FWT",
> >> + [TCP_CONNTRACK_CLOSE_WAIT]  = "CWT",
> >> + [TCP_CONNTRACK_LAST_ACK]= "LAC",
> >> + [TCP_CONNTRACK_TIME_WAIT]   = "TWT",
> >> + [TCP_CONNTRACK_CLOSE]   = "CLO",
> >> + [TCP_CONNTRACK_SYN_SENT2]   = "SS2",
> >
> >
> > These abbreviations are no longer easy to grasp for the user without
> > looking at this struct in the source. We should either keep the long
> > names (if possible) of at least add corresponding documentation about
> > the abbreviations to the manpage. Same goes for dccp_state2str and
> > sctp_state2str below.
> 
> OK, what do you prefer ? I just tried to minimize the column width to
> fit into at least 100-sized screen.
> But OK, I will try to see how much character long names will occupy.

I'd prefer the long version, but if you can't fit it into the width
without too many compromises I think the abbreviations are fine as well.
But they should be documented at least, maybe even in the help screen.

> >>
> >> -static void draw_flow_entry(WINDOW *screen, const struct flow_entry *n,
> >> - unsigned int *line)
> >> +static void draw_flow_entry(WINDOW *scr, const struct flow_entry *n, int 
> >> line)
> >>  {
> >> + const char *str = NULL;
> >>   char tmp[128];
> >> - const char *pname = NULL;
> >> - uint16_t port;
> >>
> >> - mvwprintw(screen, *line, 2, "");
> >> + mvwprintw(scr,

[netsniff-ng] Re: [PATCH 2/5] flowtop: Change flows layout to 1-row view

2016-03-29 Thread Vadim Kochan 
On Tue, Mar 29, 2016 at 4:23 PM, Tobias Klauser  wrote:
> On 2016-03-23 at 22:00:44 +0100, Vadim Kochan  wrote:
>> Changed flows list layout to look more a top-like output
>> with header and in 1 line. When -s option is specified
>> then layout changes to 2 lines view including with src peer
>> info and dst under it on next line.
>>
>> Also shortified flow state names to allocate less space.
>>
>> Removed presenter_get_port be cause ports are printed for both peers
>> separately.
>>
>> The flow duration time is printed in very short form in one of the
>> units:
>> XXd - days
>> XXh - hours
>> XXm - minutes
>> XXs - seconds
>>
>> the reason is that it is enough to have actually generic understanding
>> about flow time in the biggest time unit.
>>
>> Signed-off-by: Vadim Kochan 
>> ---
>>  flowtop.c | 405 
>> ++
>>  1 file changed, 194 insertions(+), 211 deletions(-)
>>
>> diff --git a/flowtop.c b/flowtop.c
>> index 4c15c06..8201321 100644
>> --- a/flowtop.c
>> +++ b/flowtop.c
>> @@ -62,6 +62,7 @@ struct flow_entry {
>>   uint64_t pkts_dst, bytes_dst;
>>   uint64_t timestamp_start, timestamp_stop;
>>   char country_src[128], country_dst[128];
>> + char country_code_src[4], country_code_dst[4];
>>   char city_src[128], city_dst[128];
>>   char rev_dns_src[256], rev_dns_dst[256];
>>   char procname[256];
>> @@ -166,11 +167,6 @@ static const char *copyright = "Please report bugs to 
>> >   "This is free software: you are free to change and redistribute it.\n"
>>   "There is NO WARRANTY, to the extent permitted by law.";
>>
>> -static const char *const l3proto2str[AF_MAX] = {
>> - [AF_INET]   = "ipv4",
>> - [AF_INET6]  = "ipv6",
>> -};
>
> Why remove L3 protocol information from the output? I consider this
> quite useful. Could we somehow combine this with L4 Proto information in
> a generic way?

I thought it will be easy to identify ipvX version by IPvX address format.

>
>> -
>>  static const char *const l4proto2str[IPPROTO_MAX] = {
>>   [IPPROTO_TCP]   = "tcp",
>>   [IPPROTO_UDP]   = "udp",
>> @@ -194,40 +190,40 @@ static const char *const l4proto2str[IPPROTO_MAX] = {
>>  };
>>
>>  static const char *const tcp_state2str[TCP_CONNTRACK_MAX] = {
>> - [TCP_CONNTRACK_NONE]= "NOSTATE",
>> - [TCP_CONNTRACK_SYN_SENT]= "SYN_SENT",
>> - [TCP_CONNTRACK_SYN_RECV]= "SYN_RECV",
>> - [TCP_CONNTRACK_ESTABLISHED] = "ESTABLISHED",
>> - [TCP_CONNTRACK_FIN_WAIT]= "FIN_WAIT",
>> - [TCP_CONNTRACK_CLOSE_WAIT]  = "CLOSE_WAIT",
>> - [TCP_CONNTRACK_LAST_ACK]= "LAST_ACK",
>> - [TCP_CONNTRACK_TIME_WAIT]   = "TIME_WAIT",
>> - [TCP_CONNTRACK_CLOSE]   = "CLOSE",
>> - [TCP_CONNTRACK_SYN_SENT2]   = "SYN_SENT2",
>> + [TCP_CONNTRACK_NONE]= "NO",
>> + [TCP_CONNTRACK_SYN_SENT]= "SS",
>> + [TCP_CONNTRACK_SYN_RECV]= "SR",
>> + [TCP_CONNTRACK_ESTABLISHED] = "EST",
>> + [TCP_CONNTRACK_FIN_WAIT]= "FWT",
>> + [TCP_CONNTRACK_CLOSE_WAIT]  = "CWT",
>> + [TCP_CONNTRACK_LAST_ACK]= "LAC",
>> + [TCP_CONNTRACK_TIME_WAIT]   = "TWT",
>> + [TCP_CONNTRACK_CLOSE]   = "CLO",
>> + [TCP_CONNTRACK_SYN_SENT2]   = "SS2",
>
>
> These abbreviations are no longer easy to grasp for the user without
> looking at this struct in the source. We should either keep the long
> names (if possible) of at least add corresponding documentation about
> the abbreviations to the manpage. Same goes for dccp_state2str and
> sctp_state2str below.

OK, what do you prefer ? I just tried to minimize the column width to
fit into at least 100-sized screen.
But OK, I will try to see how much character long names will occupy.

>>
>> -static void draw_flow_entry(WINDOW *screen, const struct flow_entry *n,
>> - unsigned int *line)
>> +static void draw_flow_entry(WINDOW *scr, const struct flow_entry *n, int 
>> line)
>>  {
>> + const char *str = NULL;
>>   char tmp[128];
>> - const char *pname = NULL;
>> - uint16_t port;
>>
>> - mvwprintw(screen, *line, 2, "");
>> + mvwprintw(scr, line, 0, "");
>>
>> - /* PID, application name */
>> - if (n->procnum > 0) {
>> - slprintf(tmp, sizeof(tmp), "%s(%d)", n->procname, n->procnum);
>> + /* Application */
>> + attron(COLOR_PAIR(3));
>> + printw("%-*.*s", 10, 10, n->procname);
>
> There might be several processes with the same name, how would we now
> differentiate them?

What about separate column "PID" ?


Regards,
Vadim Kochan

-- 
You received this message because you are subscribed to the Google Groups 
"netsniff-ng" group.
To unsubscribe from this group and stop receiving emails

[netsniff-ng] Re: [PATCH 2/5] flowtop: Change flows layout to 1-row view

2016-03-29 Thread Tobias Klauser 
On 2016-03-23 at 22:00:44 +0100, Vadim Kochan  wrote:
> Changed flows list layout to look more a top-like output
> with header and in 1 line. When -s option is specified
> then layout changes to 2 lines view including with src peer
> info and dst under it on next line.
> 
> Also shortified flow state names to allocate less space.
> 
> Removed presenter_get_port be cause ports are printed for both peers
> separately.
> 
> The flow duration time is printed in very short form in one of the
> units:
> XXd - days
> XXh - hours
> XXm - minutes
> XXs - seconds
> 
> the reason is that it is enough to have actually generic understanding
> about flow time in the biggest time unit.
> 
> Signed-off-by: Vadim Kochan 
> ---
>  flowtop.c | 405 
> ++
>  1 file changed, 194 insertions(+), 211 deletions(-)
> 
> diff --git a/flowtop.c b/flowtop.c
> index 4c15c06..8201321 100644
> --- a/flowtop.c
> +++ b/flowtop.c
> @@ -62,6 +62,7 @@ struct flow_entry {
>   uint64_t pkts_dst, bytes_dst;
>   uint64_t timestamp_start, timestamp_stop;
>   char country_src[128], country_dst[128];
> + char country_code_src[4], country_code_dst[4];
>   char city_src[128], city_dst[128];
>   char rev_dns_src[256], rev_dns_dst[256];
>   char procname[256];
> @@ -166,11 +167,6 @@ static const char *copyright = "Please report bugs to 
>    "This is free software: you are free to change and redistribute it.\n"
>   "There is NO WARRANTY, to the extent permitted by law.";
>  
> -static const char *const l3proto2str[AF_MAX] = {
> - [AF_INET]   = "ipv4",
> - [AF_INET6]  = "ipv6",
> -};

Why remove L3 protocol information from the output? I consider this
quite useful. Could we somehow combine this with L4 Proto information in
a generic way?

> -
>  static const char *const l4proto2str[IPPROTO_MAX] = {
>   [IPPROTO_TCP]   = "tcp",
>   [IPPROTO_UDP]   = "udp",
> @@ -194,40 +190,40 @@ static const char *const l4proto2str[IPPROTO_MAX] = {
>  };
>  
>  static const char *const tcp_state2str[TCP_CONNTRACK_MAX] = {
> - [TCP_CONNTRACK_NONE]= "NOSTATE",
> - [TCP_CONNTRACK_SYN_SENT]= "SYN_SENT",
> - [TCP_CONNTRACK_SYN_RECV]= "SYN_RECV",
> - [TCP_CONNTRACK_ESTABLISHED] = "ESTABLISHED",
> - [TCP_CONNTRACK_FIN_WAIT]= "FIN_WAIT",
> - [TCP_CONNTRACK_CLOSE_WAIT]  = "CLOSE_WAIT",
> - [TCP_CONNTRACK_LAST_ACK]= "LAST_ACK",
> - [TCP_CONNTRACK_TIME_WAIT]   = "TIME_WAIT",
> - [TCP_CONNTRACK_CLOSE]   = "CLOSE",
> - [TCP_CONNTRACK_SYN_SENT2]   = "SYN_SENT2",
> + [TCP_CONNTRACK_NONE]= "NO",
> + [TCP_CONNTRACK_SYN_SENT]= "SS",
> + [TCP_CONNTRACK_SYN_RECV]= "SR",
> + [TCP_CONNTRACK_ESTABLISHED] = "EST",
> + [TCP_CONNTRACK_FIN_WAIT]= "FWT",
> + [TCP_CONNTRACK_CLOSE_WAIT]  = "CWT",
> + [TCP_CONNTRACK_LAST_ACK]= "LAC",
> + [TCP_CONNTRACK_TIME_WAIT]   = "TWT",
> + [TCP_CONNTRACK_CLOSE]   = "CLO",
> + [TCP_CONNTRACK_SYN_SENT2]   = "SS2",


These abbreviations are no longer easy to grasp for the user without
looking at this struct in the source. We should either keep the long
names (if possible) of at least add corresponding documentation about
the abbreviations to the manpage. Same goes for dccp_state2str and
sctp_state2str below.

>  };
>  
>  static const char *const dccp_state2str[DCCP_CONNTRACK_MAX] = {
> - [DCCP_CONNTRACK_NONE]   = "NOSTATE",
> - [DCCP_CONNTRACK_REQUEST]= "REQUEST",
> - [DCCP_CONNTRACK_RESPOND]= "RESPOND",
> - [DCCP_CONNTRACK_PARTOPEN]   = "PARTOPEN",
> - [DCCP_CONNTRACK_OPEN]   = "OPEN",
> - [DCCP_CONNTRACK_CLOSEREQ]   = "CLOSEREQ",
> - [DCCP_CONNTRACK_CLOSING]= "CLOSING",
> - [DCCP_CONNTRACK_TIMEWAIT]   = "TIMEWAIT",
> - [DCCP_CONNTRACK_IGNORE] = "IGNORE",
> - [DCCP_CONNTRACK_INVALID]= "INVALID",
> + [DCCP_CONNTRACK_NONE]   = "NO",
> + [DCCP_CONNTRACK_REQUEST]= "REQ",
> + [DCCP_CONNTRACK_RESPOND]= "RES",
> + [DCCP_CONNTRACK_PARTOPEN]   = "POP",
> + [DCCP_CONNTRACK_OPEN]   = "OPN",
> + [DCCP_CONNTRACK_CLOSEREQ]   = "CLQ",
> + [DCCP_CONNTRACK_CLOSING]= "CLN",
> + [DCCP_CONNTRACK_TIMEWAIT]   = "TWT",
> + [DCCP_CONNTRACK_IGNORE] = "IGN",
> + [DCCP_CONNTRACK_INVALID]= "INV",
>  };
>  
>  static const char *const sctp_state2str[SCTP_CONNTRACK_MAX] = {
> - [SCTP_CONNTRACK_NONE]   = "NOSTATE",
> - [SCTP_CONNTRACK_CLOSED] = "CLOSED",
> - [SCTP_CONNTRACK_COOKIE_WAIT]= "COOKIE_WAIT",
> - [SCTP_CONNTRACK_COOKIE_ECHOED]  = "COOKIE_ECHOED",
> -