[netsniff-ng] Re: trafgen: Would be it useful to have pktgen support ?

2016-08-08 Thread Tobias Klauser
On 2016-08-08 at 11:52:29 +0200, Daniel Borkmann  wrote:
> On 08/07/2016 07:46 PM, Vadim Kochan wrote:
> >Hi,
> >
> >I did not ever use Linux pktgen feature, but I just catch the
> >idea if it would be good to have option to send trafgen protocol
> >built packet via Linux pktgen ? Theoretically it is possible to create
> >simple and generic code to generate raw or pktgen packets. But
> >pktgen's kind of packets will be possible to generate only via protocol
> >header functions because with it we have all field metadata. Also
> >when dynamic fields series will be applied it may be possible also
> >convert them to pktgen's rand/inc commands.
> >
> >But again let me know if it might be useful.
> 
> I think it would be useful, I see value where trafgen would do all the
> procfs setup details internally and then runs pktgen as a backend. So the
> only thing needed for the user would be to execute trafgen normally via
> cmdline.

I also think this could be a useful feature to have.

-- 
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 netsniff-ng+unsubscr...@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.


[netsniff-ng] Re: [PATCH 06/15] trafgen: proto: Randomize proto field at runtime

2016-08-06 Thread Tobias Klauser
On 2016-08-06 at 01:44:42 +0200, Vadim Kochan  wrote:
> On Fri, Aug 05, 2016 at 09:26:24AM +0200, Tobias Klauser wrote:
> > On 2016-07-26 at 21:35:11 +0200, Vadim Kochan  wrote:
> > > +static inline unsigned int field_rand(struct proto_field *field)
> > > +{
> > > + return field->func.min + (rand() % ((field->func.max - field->func.min) 
> > > + 1));
> > > +}
> > > +
> > > +static void field_rnd_func(struct proto_field *field)
> > > +{
> > > + unsigned int val = field_rand(field);
> > > +
> > > + if (field->len == 1) {
> > > + proto_field_set_u8(field->hdr, field->id, (uint8_t) val);
> > > + } else if (field->len == 2) {
> > > + proto_field_set_be16(field->hdr, field->id, (uint16_t) val);
> > > + } else if (field->len == 4) {
> > > + proto_field_set_be32(field->hdr, field->id, (uint32_t) val);
> > > + } else if (field->len > 4) {
> > > + uint8_t *bytes = __proto_field_get_bytes(field);
> > > + uint32_t i;
> > > +
> > > + for (i = 0; i < field->len; i++, val = field_rand(field))
> > > + bytes[i] = (uint8_t) val;
> > 
> > No need for the assigment of `val' in the loop, should rather be like
> > this:
> > 
> > for (i = 0; i < field->len; i++)
> > bytes[i] = (uint8_t) field_rand(field);
> > > + }
> 
> I did so because field_rand(field) was calculated at the beginning so in
> the loop it will be calculated only on the next round. Do you think it
> is not needed ?

IMO it's easy to overread the val assignment in the for statement as it
is not such a common pattern. We'd not even save the call to
field_rand(), so I think it's worth to have this explicitely stated in
the loop body rather than the for(...) statement.

-- 
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 netsniff-ng+unsubscr...@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.


[netsniff-ng] Re: [PATCH 13/15] trafgen: parser: Add support of 'dinc' function for proto fields

2016-08-06 Thread Tobias Klauser
On 2016-08-06 at 00:43:56 +0200, Vadim Kochan  wrote:
> On Wed, Aug 03, 2016 at 04:32:06PM +0200, Tobias Klauser wrote:
> > On 2016-07-26 at 21:35:18 +0200, Vadim Kochan  wrote:
> > > +static void proto_field_func_setup(struct proto_field *field, struct 
> > > proto_field_func *func)
> > > +{
> > > + struct packet_dyn *pkt_dyn;
> > > +
> > > + proto_field_func_add(field->hdr, field->id, func);
> > > +
> > > + pkt_dyn = &packet_dyn[packetd_last];
> > > + pkt_dyn->flen++;
> > > + pkt_dyn->fields = (struct proto_field **)xrealloc(pkt_dyn->fields, 
> > > pkt_dyn->flen *
> > > + sizeof(struct proto_field *));
> > 
> > No need to case the return value of xrealloc()
> 
> Sorry, I did not get it, may be you meant "No need to cast ..." ?

Yes, sorry. That's a typo and should read "...cast the return value...".

-- 
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 netsniff-ng+unsubscr...@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.


[netsniff-ng] Re: [PATCH v3 3/3] configure: Add option to compile tools without libnl dependency

2016-08-06 Thread Tobias Klauser
On 2016-08-05 at 22:51:54 +0200, Vadim Kochan  wrote:
> Hm,
> 
> My version was to keep current strict dependency on libnl by default,
> so if there is no libnl then
> netsniff-ng & trafgen will be removed from build list, but make
> possible to skip this
> rule by --disable-libnl option. Now I am not sure there is a reason
> for --disable-libnl as if there is no libnl
> then netsniff-ng & trafgen will be added to build list, and will be
> compiled w/o libnl dependency..

Ok, but that intention of your patch wasn't really clear from the
description, sorry.

> So does --disable-libnl makes sense ?

If you have libnl-dev installed but want to compile netsniff-ng/trafgen
without libnl support it still makes sense.

IMO the current version is more in line with the behavior we already
have for geoip and libz.

Thanks
Tobias

-- 
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 netsniff-ng+unsubscr...@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.


[netsniff-ng] Re: [PATCH 10/15] trafgen: udp: Update csum at runtime if needed

2016-08-05 Thread Tobias Klauser
On 2016-08-05 at 14:34:04 +0200, Vadim Kochan  wrote:
> On Fri, Aug 5, 2016 at 3:26 PM, Tobias Klauser  wrote:
> > On 2016-07-26 at 21:35:15 +0200, Vadim Kochan  wrote:
> >> Update UDP csum field at runtime if:
> >>
> >> 1) UDP field was changed.
> >>
> >> 2) IPv4/6 source/destination addresses were changed
> >>(which is a part of UDP pseudo header), this is
> >>handled by IPv4/6 protocols.
> >>
> >> Also changed proto_lower_header(...) function to use header index
> >> to lookup lower header faster, the reason is that this function
> >> is used for updating UDP csum.
> >
> > I think the changes to proto_lower_header() should be a separate patch.
> > Also the introduction of proto_upper_header() should be separated out.
> 
> Is it not easier for you to have it in one patch with having the use
> case how it is used ? And you was squashed once my trafgen's icmpv4
> series,
> and I started to think may be it is not necesseary to split changes
> into a lot of smaller ones ...

If the patch introduces or changes  generic functionality which is/will
be used by multiple modules or subsequent patches I usually like to see
them separated.

In the icmpv4 case it was self-contained functionality and only added
code, not deleted/changed existing code. Also the resulting patch was
small enough, so I decided to squash them (as I also did for my ICMPv6
implementation).

-- 
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 netsniff-ng+unsubscr...@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.


[netsniff-ng] Re: [PATCH 10/15] trafgen: udp: Update csum at runtime if needed

2016-08-05 Thread Tobias Klauser
On 2016-07-26 at 21:35:15 +0200, Vadim Kochan  wrote:
> Update UDP csum field at runtime if:
> 
> 1) UDP field was changed.
> 
> 2) IPv4/6 source/destination addresses were changed
>(which is a part of UDP pseudo header), this is
>handled by IPv4/6 protocols.
> 
> Also changed proto_lower_header(...) function to use header index
> to lookup lower header faster, the reason is that this function
> is used for updating UDP csum.

I think the changes to proto_lower_header() should be a separate patch.
Also the introduction of proto_upper_header() should be separated out.

-- 
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 netsniff-ng+unsubscr...@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.


[netsniff-ng] Re: [PATCH v3 2/3] netsniff-ng: Allow compile without libnl

2016-08-05 Thread Tobias Klauser
On 2016-08-04 at 18:30:19 +0200, Vadim Kochan  wrote:
> There is reason to do not install libnl-xxx packages just
> for sniffing packets, for example if netsniff-ng will be compiled
> on embedded or switch system.
> 
> Hide libnl depended code if CONFIG_LIBNL=0.
> 
> In case if user will use --rfraw option the panic message will be printed,
> in case if netlink messages will be sniffed the noop dissector will be used.
> 
> Signed-off-by: Vadim Kochan 

Applied with minor modifications (due to my changes in PATCH 3/3).

-- 
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 netsniff-ng+unsubscr...@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.


[netsniff-ng] Re: [PATCH v3 1/3] trafgen: Allow to compile without libnl

2016-08-05 Thread Tobias Klauser
On 2016-08-04 at 18:30:18 +0200, Vadim Kochan  wrote:
> trafgen uses libnl only to inject mac80211 frames but
> it might be not needed in some embedded or switch environments,
> so lets make possible to disable this feature.
> 
> In case if --rfraw option will be used - user will get the panic message.
> 
> Signed-off-by: Vadim Kochan 

Applied with minor modifications (due to my changes in PATCH 3/3).

-- 
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 netsniff-ng+unsubscr...@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.


[netsniff-ng] Re: [PATCH v3 3/3] configure: Add option to compile tools without libnl dependency

2016-08-05 Thread Tobias Klauser
On 2016-08-04 at 18:30:20 +0200, Vadim Kochan  wrote:
> Add command line parsing function which allows to compile tools
> (trafgen, netsniff-ng) without libnl-xxx libraries.

But it does in its current state not allow to compile
netsniff-ng/trafgen without libnl if libnl is not installed (i.e. not
found by check_libnl).

I rewrote this patch to support this case and make it more in line with
the existing library checking/disabling.

Thanks!

-- 
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 netsniff-ng+unsubscr...@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.


[netsniff-ng] Re: [PATCH 06/15] trafgen: proto: Randomize proto field at runtime

2016-08-05 Thread Tobias Klauser
On 2016-07-26 at 21:35:11 +0200, Vadim Kochan  wrote:
> Add dynamic proto field function which can generate
> random value in specified range (default 0 - MAX_UINT32).
> 
> Signed-off-by: Vadim Kochan 
> ---
>  trafgen_proto.c | 29 +
>  trafgen_proto.h |  1 +
>  2 files changed, 30 insertions(+)
> 
> diff --git a/trafgen_proto.c b/trafgen_proto.c
> index 069aa00..f57d390 100644
> --- a/trafgen_proto.c
> +++ b/trafgen_proto.c
> @@ -486,6 +486,30 @@ static void field_inc_func(struct proto_field *field)
>   }
>  }
>  
> +static inline unsigned int field_rand(struct proto_field *field)
> +{
> + return field->func.min + (rand() % ((field->func.max - field->func.min) 
> + 1));
> +}
> +
> +static void field_rnd_func(struct proto_field *field)
> +{
> + unsigned int val = field_rand(field);
> +
> + if (field->len == 1) {
> + proto_field_set_u8(field->hdr, field->id, (uint8_t) val);
> + } else if (field->len == 2) {
> + proto_field_set_be16(field->hdr, field->id, (uint16_t) val);
> + } else if (field->len == 4) {
> + proto_field_set_be32(field->hdr, field->id, (uint32_t) val);
> + } else if (field->len > 4) {
> + uint8_t *bytes = __proto_field_get_bytes(field);
> + uint32_t i;
> +
> + for (i = 0; i < field->len; i++, val = field_rand(field))
> + bytes[i] = (uint8_t) val;

No need for the assigment of `val' in the loop, should rather be like
this:

for (i = 0; i < field->len; i++)
bytes[i] = (uint8_t) field_rand(field);
> + }
> +}
> +
>  void proto_field_func_add(struct proto_hdr *hdr, uint32_t fid,
> struct proto_field_func *func)
>  {
> @@ -522,6 +546,11 @@ void proto_field_func_add(struct proto_hdr *hdr, 
> uint32_t fid,
>  
>   field->func.update_field = field_inc_func;
>   }
> +
> + if (func->type & PROTO_FIELD_FUNC_RND) {
> + field->func.max = field->func.max ?: (uint32_t)~0 - 1;

UINT32_MAX, please. Also in all other places in this series.

> + field->func.update_field = field_rnd_func;
> + }
>  }
>  
>  void proto_field_dyn_apply(struct proto_field *field)
> diff --git a/trafgen_proto.h b/trafgen_proto.h
> index 64f4366..40817a8 100644
> --- a/trafgen_proto.h
> +++ b/trafgen_proto.h
> @@ -38,6 +38,7 @@ struct proto_hdr;
>  enum proto_field_func_t {
>   PROTO_FIELD_FUNC_INC = 1 << 0,
>   PROTO_FIELD_FUNC_MIN = 1 << 1,
> + PROTO_FIELD_FUNC_RND = 1 << 2,
>  };
>  
>  struct proto_field_func {
> -- 
> 2.6.3
> 

-- 
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 netsniff-ng+unsubscr...@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.


[netsniff-ng] Re: [PATCH v2 0/3] build: Allow compile trafgen & netsniff-ng without libnl

2016-08-04 Thread Tobias Klauser
On 2016-08-03 at 10:58:00 +0200, Vadim Kochan  wrote:
> It might be useful only if to compile trafgen or netsniff-ng in environment
> (or manually w/o needs for rfraw) where is no needed libnl (embedded/switch 
> server),
> and it would be good to have ability to disable libnl dependency at all even 
> w/o
> such features like rfraw dissect/inject & nlmsg dissecting. For example
> netsniff-ng might be used just for sniffing, and trafgen - send packets to 
> wired
> network only.
> 
> Therefor added --no-libnl option in ./confiure script which sets make option &
> CPP definition which are used to ignore libnl- related code/modules/libraries.
> 
> v2:
> 1) Reorder commits to be "configure" related changes  1st

I just realized I was wrong by requesting this. In the current
order we might break disectability if --disable-libnl is used. Your
original order of patches was correct. Care to change it again?

Sorry about that!

-- 
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 netsniff-ng+unsubscr...@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.


[netsniff-ng] Re: [PATCH v2 1/3] configure: Add option to compile tools without libnl dependency

2016-08-04 Thread Tobias Klauser
On 2016-08-03 at 10:58:01 +0200, Vadim Kochan  wrote:
> Add command line parsing function which allows to compile tools
> (trafgen, netsniff-ng) without libnl-xxx libraries.
> 
> Option --disable-libnl sets CONFIG_LIBNL=0 which means compile tools
> without libnl dependencies.
> 
> Signed-off-by: Vadim Kochan 
> ---
>  configure | 47 ---
>  1 file changed, 44 insertions(+), 3 deletions(-)
> 
> diff --git a/configure b/configure
> index 105b1ec..61b2e31 100755
> --- a/configure
> +++ b/configure
> @@ -18,8 +18,36 @@ HAVE_LIBGEOIP=0
>  HAVE_LIBZ=0
>  HAVE_TPACKET3=0
>  
> +CONFIG_LIBNL=1
> +
>  # use "CROSS_COMPILE= SYSROOT= ./configure && make" for cross 
> compilation
>  
> +usage()
> +{
> + echo "Usage: ./configure [OPTIONS]"

echo "Usage: ./configure [OPTION]..."

> + echo -e  "\t-h, --help  - print usage"
> + echo -ne "\t--disable-libnl - compile without libnl."

Please avoid using echo -e and tabs, just use two spaces to indent
(also check usage message format of autotools configure).

btw, I just noticed we already consider the DISABLE_GEOIP and
DISABLE_ZLIB environment variables. I will apply the option handling
part of this patch first and convert the existing DISABLE_* to
parameters. Then you may implement your libnl changes on top of mine.

> + echo "Some features (rfraw, nlmsg dissect) will not work in trafgen, 
> netsniff-ng."

I don't think this message is necessary. If at all it should be added to
INSTALL (and the "required/optional libraries" section updated
accordingly).

-- 
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 netsniff-ng+unsubscr...@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.


[netsniff-ng] Re: [PATCH 13/15] trafgen: parser: Add support of 'dinc' function for proto fields

2016-08-03 Thread Tobias Klauser
On 2016-07-26 at 21:35:18 +0200, Vadim Kochan  wrote:
> Add 'dinc()' function in 'field_expr' rules to be used for dynamically
> incrementing of any specified field:
> 
> SYNTAX := dinc() | dinc(inc) | dinc(min, max) | dinc(inc, min, max)

This should definitely follow the same parameter order as the existing
dinc() function!!!

> EXAMPLES:
> { udp(sport=dinc() }
> { udp(sport=dinc(1) }
> { udp(sport=dinc(5, 100, 125) }

> Signed-off-by: Vadim Kochan 
> ---
>  trafgen_parser.y | 51 +++
>  1 file changed, 51 insertions(+)
> 
> diff --git a/trafgen_parser.y b/trafgen_parser.y
> index 58ac999..7872b7c 100644
> --- a/trafgen_parser.y
> +++ b/trafgen_parser.y
> @@ -74,6 +74,7 @@ enum field_expr_type_t {
>   FIELD_EXPR_MAC,
>   FIELD_EXPR_IP4_ADDR,
>   FIELD_EXPR_IP6_ADDR,
> + FIELD_EXPR_INC,
>  };
>  
>  struct proto_field_expr {
> @@ -85,6 +86,7 @@ struct proto_field_expr {
>   struct in6_addr ip6_addr;
>   long long int number;
>   uint8_t bytes[256];
> + struct proto_field_func func;
>   } val;
>  };
>  
> @@ -125,6 +127,12 @@ static inline void __init_new_csum_slot(struct 
> packet_dyn *slot)
>   slot->slen = 0;
>  }
>  
> +static inline void __init_new_fields_slot(struct packet_dyn *slot)
> +{
> + slot->fields = NULL;
> + slot->flen = 0;
> +}
> +
>  static inline void __setup_new_counter(struct counter *c, uint8_t start,
>  uint8_t stop, uint8_t stepping,
>  int type)
> @@ -167,6 +175,7 @@ static void realloc_packet(void)
>   __init_new_counter_slot(&packet_dyn[packetd_last]);
>   __init_new_randomizer_slot(&packet_dyn[packetd_last]);
>   __init_new_csum_slot(&packet_dyn[packetd_last]);
> + __init_new_fields_slot(&packet_dyn[packetd_last]);
>  }
>  
>  struct packet *current_packet(void)
> @@ -376,6 +385,19 @@ static void proto_field_set(uint32_t fid)
>   field_expr.field = proto_field_by_id(hdr, fid);
>  }
>  
> +static void proto_field_func_setup(struct proto_field *field, struct 
> proto_field_func *func)
> +{
> + struct packet_dyn *pkt_dyn;
> +
> + proto_field_func_add(field->hdr, field->id, func);
> +
> + pkt_dyn = &packet_dyn[packetd_last];
> + pkt_dyn->flen++;
> + pkt_dyn->fields = (struct proto_field **)xrealloc(pkt_dyn->fields, 
> pkt_dyn->flen *
> + sizeof(struct proto_field *));

No need to case the return value of xrealloc()

> + pkt_dyn->fields[pkt_dyn->flen - 1] = field;
> +}
> +
>  static void proto_field_expr_eval(void)
>  {
>   struct proto_field *field = field_expr.field;
> @@ -405,6 +427,14 @@ static void proto_field_expr_eval(void)
>   (uint8_t *)&field_expr.val.ip6_addr.s6_addr);
>   break;
>  
> + case FIELD_EXPR_INC:
> + if (field_expr.val.func.min > field_expr.val.func.max)
> + panic("dinc(): min(%u) can't be greater than max(%u)\n",
> + field_expr.val.func.min, 
> field_expr.val.func.max);
> +
> + proto_field_func_setup(field, &field_expr.val.func);
> + break;
> +
>   case FIELD_EXPR_UNKNOWN:
>   default:
>   bug();
> @@ -685,6 +715,26 @@ field_expr
>field_expr.val.ip4_addr = $1; }
>   | ip6_addr { field_expr.type = FIELD_EXPR_IP6_ADDR;
>field_expr.val.ip6_addr = $1; }
> + | K_DINC '(' ')' { field_expr.type = FIELD_EXPR_INC;
> +field_expr.val.func.type = PROTO_FIELD_FUNC_INC;
> +field_expr.val.func.inc = 1; }
> + | K_DINC '(' number ')'
> + { field_expr.type = FIELD_EXPR_INC;
> +   field_expr.val.func.inc = $3; }
> + | K_DINC '(' number delimiter number ')'
> + { field_expr.type = FIELD_EXPR_INC;
> +   field_expr.val.func.type  = PROTO_FIELD_FUNC_INC;
> +   field_expr.val.func.type |= PROTO_FIELD_FUNC_MIN;
> +   field_expr.val.func.inc = 1;
> +   field_expr.val.func.min = $3;
> +   field_expr.val.func.max = $5; }
> + | K_DINC '(' number delimiter number delimiter number ')'
> + { field_expr.type = FIELD_EXPR_INC;
> +   field_expr.val.func.type  = PROTO_FIELD_FUNC_INC;
> +   field_expr.val.func.type |= PROTO_FIELD_FUNC_MIN;
> +   field_expr.val.func.inc = $3;
> +   field_expr.val.func.min = $5;
> +   field_expr.val.func.max = $7; }
>   ;
>  
>  eth_proto
> @@ -1097,6 +1147,7 @@ void cleanup_packets(void)
>   for (i = 0; i < dlen; ++i) {
>   free(packet_dyn[i].cnt);
>   free(packet_dyn[i].rnd);
> + free(packet_dyn[i].fields);
>   }
>  

[netsniff-ng] Re: [PATCH 05/15] trafgen: proto: Increment proto field at runtime

2016-08-03 Thread Tobias Klauser
On 2016-07-26 at 21:35:10 +0200, Vadim Kochan  wrote:
> Extended 'struct packet_dyn' with proto fields which has
> dynamically changing values at runtime.
> 
> Implement incrementing of proto field at runtime with min & max
> parameters, by default if the 'min' parameter is not specified
> then original value is used. For fields which len is greater
> than 4 - last 4 bytes are incremented as unsigned int value.
> 
> Added 'field_changed' callback for proto header which
> may be used for check if csum updating is needed. This callback
> is called after field was changed at runtime.
> 
> Added 'packet_update' callback to let proto header know
> when to apply final proto header changes at runtime (csum update).

The documentation of these callbacks would also make sense where they're
defined.

> Signed-off-by: Vadim Kochan 
> ---
>  trafgen.c   |  9 ++
>  trafgen_conf.h  |  7 
>  trafgen_proto.c | 99 
> +
>  trafgen_proto.h | 26 +++
>  4 files changed, 141 insertions(+)
> 
> diff --git a/trafgen.c b/trafgen.c
> index b76b5d7..553dfa5 100644
> --- a/trafgen.c
> +++ b/trafgen.c
> @@ -619,6 +619,15 @@ static inline void packet_apply_dyn_elements(int idx)
>   apply_randomizer(idx);
>   apply_csum16(idx);
>   }
> +
> + if (packet_dyn_has_fields(&packet_dyn[idx])) {
> + uint32_t i;
> +
> + for (i = 0; i < packet_dyn[idx].flen; i++)
> + proto_field_dyn_apply(packet_dyn[idx].fields[i]);
> +
> + proto_packet_update(idx);
> + }
>  }
>  
>  static void xmit_slowpath_or_die(struct ctx *ctx, unsigned int cpu, unsigned 
> long orig_num)
> diff --git a/trafgen_conf.h b/trafgen_conf.h
> index 934f8fe..7f3616c 100644
> --- a/trafgen_conf.h
> +++ b/trafgen_conf.h
> @@ -49,6 +49,8 @@ struct packet_dyn {
>   size_t rlen;
>   struct csum16 *csum;
>   size_t slen;
> + struct proto_field **fields;
> + uint32_t flen;
>  };
>  
>  static inline bool packet_dyn_has_elems(struct packet_dyn *p)
> @@ -61,6 +63,11 @@ static inline bool packet_dyn_has_only_csums(struct 
> packet_dyn *p)
>   return (p->clen == 0 && p->rlen == 0 && p->slen);
>  }
>  
> +static inline bool packet_dyn_has_fields(struct packet_dyn *p)
> +{
> + return !!p->flen;
> +}
> +
>  extern void compile_packets_str(char *str, bool verbose, unsigned int cpu);
>  extern void compile_packets(char *file, bool verbose, unsigned int cpu,
>   bool invoke_cpp, char *const cpp_argv[]);
> diff --git a/trafgen_proto.c b/trafgen_proto.c
> index ce389ce..069aa00 100644
> --- a/trafgen_proto.c
> +++ b/trafgen_proto.c
> @@ -419,6 +419,19 @@ void protos_init(const char *dev)
>   p->ctx = &ctx;
>  }
>  
> +void proto_packet_update(uint32_t idx)
> +{
> + struct packet *pkt = packet_get(idx);
> + ssize_t i;
> +
> + for (i = pkt->headers_count - 1; i >= 0; i--) {
> + struct proto_hdr *hdr = pkt->headers[i];
> +
> + if (hdr->packet_update)
> + hdr->packet_update(hdr);
> + }
> +}
> +
>  void proto_packet_finish(void)
>  {
>   struct proto_hdr **headers = ¤t_packet()->headers[0];
> @@ -433,3 +446,89 @@ void proto_packet_finish(void)
>   p->packet_finish(p);
>   }
>  }
> +
> +static inline unsigned int field_inc(struct proto_field *field)
> +{
> + uint32_t val;
> +
> + val = field->func.val - field->func.min;
> + val = (val + field->func.inc) % field->func.max;

Shouldn't this be

val = (val + field->func.inc) % (field->func.max - field->func.min + 1)

to be consistent with apply_counter()?

Also, I think you should probably get rid of as many pointer
dereferences as possible in these runtime functions, i.e. store max and
min in temporary variables.

> + field->func.val = val + field->func.min;
> +
> + return field->func.val;
> +}
> +
> +static void field_inc_func(struct proto_field *field)
> +{
> + if (field->len == 1) {
> + uint8_t val;
> +
> + val = field_inc(field);
> + proto_field_set_u8(field->hdr, field->id, val);

Assignment on declaration please. Or even better:

proto_field_set_u8(field->hdr, field->id, field_inc(field));

> + } else if (field->len == 2) {
> + uint16_t val;
> +
> + val = field_inc(field);
> + proto_field_set_be16(field->hdr, field->id, val);

Same.

> + } else if (field->len == 4) {
> + uint32_t val;
> +
> + val = field_inc(field);
> + proto_field_set_be32(field->hdr, field->id, val);

Same.

> + } else if (field->len > 4) {
> + uint8_t *bytes = __proto_field_get_bytes(field);
> + uint32_t val;
> +
> + bytes += field->len - 4;
> + val = field_inc(field);
> +
> + *(uint32_t *)bytes = bswap_32(val);

This part looks really odd. D

[netsniff-ng] Re: [PATCH 04/15] trafgen: proto: Force field id to be index in array

2016-08-03 Thread Tobias Klauser
On 2016-08-02 at 22:26:28 +0200, Vadim Kochan  wrote:
> Yes, bug_on(x) is good point, so I am expecting you will continue
> review the rest patches ?

Yes

-- 
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 netsniff-ng+unsubscr...@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.


[netsniff-ng] Re: [PATCH 1/3] trafgen: Allow to compile without libnl

2016-08-03 Thread Tobias Klauser
On 2016-08-02 at 22:17:41 +0200, Vadim Kochan  wrote:
> On Tue, Aug 2, 2016 at 11:23 AM, Tobias Klauser  wrote:
> > On 2016-07-31 at 23:13:16 +0200, Vadim Kochan  wrote:
> >> trafgen uses libnl only to inject mac80211 frames but
> >> it might be not needed in some embedded or switch environments,
> >> so lets make possible to disable this feature.
> >>
> >> In case if --rfraw option will be used - user will get the panic message.
> >>
> >> Signed-off-by: Vadim Kochan 
> >> ---
> >>  mac80211.c   |  1 -
> >>  mac80211.h   | 15 +++
> >>  trafgen/Makefile | 21 +++--
> >>  3 files changed, 30 insertions(+), 7 deletions(-)
> >>
> >> diff --git a/mac80211.c b/mac80211.c
> >> index f22b600..9aea5a0 100644
> >> --- a/mac80211.c
> >> +++ b/mac80211.c
> >> @@ -24,7 +24,6 @@
> >>  #include 
> >>  #include 
> >>
> >> -#include "die.h"
> >>  #include "str.h"
> >>  #include "dev.h"
> >>  #include "mac80211.h"
> >> diff --git a/mac80211.h b/mac80211.h
> >> index dea4ae0..2780c03 100644
> >> --- a/mac80211.h
> >> +++ b/mac80211.h
> >> @@ -1,7 +1,22 @@
> >>  #ifndef MAC80211_H
> >>  #define MAC80211_H
> >>
> >> +#include "die.h"
> >> +#include "config.h"
> >> +
> >> +#ifdef CONFIG_NO_LIBNL
> >> +static inline void enter_rfmon_mac80211(const char *device, char **mondev)
> >> +{
> >> +panic("enter_rfmon_mac80211: CONFIG_NO_LIBNL option needs to be 
> >> disabled\n");
> >> +}
> >> +
> >> +static inline void leave_rfmon_mac80211(const char *mondev)
> >> +{
> >> +panic("leave_rfmon_mac80211: CONFIG_NO_LIBNL option needs to be 
> >> disabled\n");
> >> +}
> >
> > These messages both have a double negative which isn't very easy to
> > understand. I suggest to call the CONFIG directive CONFIG_LIBNL instead
> > which is also more in line with the other library CONFIG_* directives we
> > have.
> >
> > Also, you use the CONFIG_NO_LIBNL without setting it anywhere, this is
> > only done in patch 3/3. Please change the series order to make the
> > configure change come first.
> 
> I really think that CONFIG_{DISABLE or ENABLE}_LIBNL might be better
> because we really enable/disable it,
> but CONFIG_LIBNL is rather about existence of package, if to use this
> way then it will be needed (probably) to add C defines for
> CONFIG_LIBNL_ROUTE, CONFIG_LIBNL_GENL.

Form the point of view of compiling nesniff-ng/trafgen there is no
difference of whether an external library "exists" or was explicitely
disabled by the user. I'd really prefer CONFIG_LIBNL for this and I
don't think more fine-grained control (CONFIG_LIBNL_*) will be needed,
as we gain relatively little compared to the amount of #ifdef complexity
we introduce.
> 

-- 
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 netsniff-ng+unsubscr...@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.


[netsniff-ng] Re: [PATCH 04/15] trafgen: proto: Force field id to be index in array

2016-08-02 Thread Tobias Klauser
On 2016-07-26 at 21:35:09 +0200, Vadim Kochan  wrote:
> Usually proto fields array is sorted in the same order as related enum,
> so id may be used as index for faster lookup, it will make
> csum field calculation little faster at runtime.
> 
> Signed-off-by: Vadim Kochan 
> ---
>  trafgen_l3.h| 4 ++--
>  trafgen_proto.c | 8 +---
>  2 files changed, 3 insertions(+), 9 deletions(-)
> 
> diff --git a/trafgen_l3.h b/trafgen_l3.h
> index a1b1523..e0c9a1c 100644
> --- a/trafgen_l3.h
> +++ b/trafgen_l3.h
> @@ -10,14 +10,14 @@ enum ip4_field {
>   IP4_LEN,
>   IP4_ID,
>   IP4_FLAGS,
> + IP4_MF,
> + IP4_DF,
>   IP4_FRAG_OFFS,
>   IP4_TTL,
>   IP4_PROTO,
>   IP4_CSUM,
>   IP4_SADDR,
>   IP4_DADDR,
> - IP4_DF,
> - IP4_MF,
>  };
>  
>  enum ip6_field {
> diff --git a/trafgen_proto.c b/trafgen_proto.c
> index a1d56cf..ce389ce 100644
> --- a/trafgen_proto.c
> +++ b/trafgen_proto.c
> @@ -118,13 +118,7 @@ void proto_header_fields_add(struct proto_hdr *hdr,
>  
>  static struct proto_field *proto_field_by_id(struct proto_hdr *hdr, uint32_t 
> fid)
>  {
> - int i;
> -
> - for (i = 0; i < hdr->fields_count; i++)
> - if (hdr->fields[i].id == fid)
> - return &hdr->fields[i];
> -
> - panic("Failed lookup field id %u for proto id %u\n", fid, hdr->id);
> + return &hdr->fields[fid];

Please add an inline comment here explaining this behaviour. Otherwise
someone will stumble upon it in the future for sure.

Also, something along the lines of

  bug_on(hdr->fields[fid].id != fid);

should be added to catch future erroneous fields definitions.

-- 
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 netsniff-ng+unsubscr...@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.


[netsniff-ng] Re: [PATCH 03/15] trafgen: proto: Move proto headers into packet

2016-08-02 Thread Tobias Klauser
On 2016-07-26 at 21:35:08 +0200, Vadim Kochan  wrote:
> Till now headers were used only for packet creation at compile time
> only, which does not allow to handle dynamic fields update at runtime.
> It needs that proto_hdr entries will be not freed after compile is done.
> 
> Signed-off-by: Vadim Kochan 
> ---
>  trafgen_conf.h   |  6 ++
>  trafgen_parser.y | 17 ++---
>  trafgen_proto.c  | 40 +++-
>  trafgen_proto.h  |  2 ++
>  4 files changed, 41 insertions(+), 24 deletions(-)
> 
> diff --git a/trafgen_conf.h b/trafgen_conf.h
> index efce29c..934f8fe 100644
> --- a/trafgen_conf.h
> +++ b/trafgen_conf.h
> @@ -5,6 +5,10 @@
>  #include 
>  #include 
>  
> +#include "trafgen_proto.h"

This produces a circular header dependency together with

  #include "trafgen_conf.h"

in trafgen_proto.h. I guess the latter is not needed, so I dropped it
and applied the patch.

-- 
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 netsniff-ng+unsubscr...@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.


[netsniff-ng] Re: [PATCH 02/15] trafgen: proto: Reference to packet from proto_hdr struct

2016-08-02 Thread Tobias Klauser
On 2016-07-26 at 21:35:07 +0200, Vadim Kochan  wrote:
> Using of current_packet() is not relevant for dynamically updated
> fields so lets keep packet index in proto_hdr struct.
> 
> Signed-off-by: Vadim Kochan 

Applied, thanks!

-- 
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 netsniff-ng+unsubscr...@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.


[netsniff-ng] Re: [PATCH 3/3] configure: Add option to compile tools without libnl dependency

2016-08-02 Thread Tobias Klauser
On 2016-07-31 at 23:13:18 +0200, Vadim Kochan  wrote:
> Add command line parsing function which allows to compile tools (trafgen, 
> netsniff-ng)
> without libnl-xxx libraries.
> 
> Option --no-libnl sets CONFIG_NO_LIBNL=1.
> 
> Signed-off-by: Vadim Kochan 
> ---
>  configure | 52 +---
>  1 file changed, 49 insertions(+), 3 deletions(-)
> 
> diff --git a/configure b/configure
> index 105b1ec..85f2178 100755
> --- a/configure
> +++ b/configure
> @@ -18,8 +18,36 @@ HAVE_LIBGEOIP=0
>  HAVE_LIBZ=0
>  HAVE_TPACKET3=0
>  
> +CONFIG_NO_LIBNL=0
> +
>  # use "CROSS_COMPILE= SYSROOT= ./configure && make" for cross 
> compilation
>  
> +usage()
> +{
> + echo "Usage: ./configure [OPTIONS]"
> + echo -e  "\t-h, --help - print usage"
> +echo -ne "\t--no-libnl - compile without libnl"

Indenting is inconsistent.

> + echo "Some features (rfraw, nlmsg dissect) will not work in trafgen, 
> netsniff-ng."
> +
> + exit 0
> +}
> +
> +while [ $# -gt 0 ]
> +do
> + case "$1" in
> + -h|--help)
> + usage
> + ;;
> + --no-libnl)
> + CONFIG_NO_LIBNL=1
> + ;;

Most autoconf configure scripts use --disable-xxx. I suggest we stick to
that scheme (even though we don't use autotools).

-- 
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 netsniff-ng+unsubscr...@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.


[netsniff-ng] Re: [PATCH 1/3] trafgen: Allow to compile without libnl

2016-08-02 Thread Tobias Klauser
On 2016-07-31 at 23:13:16 +0200, Vadim Kochan  wrote:
> trafgen uses libnl only to inject mac80211 frames but
> it might be not needed in some embedded or switch environments,
> so lets make possible to disable this feature.
> 
> In case if --rfraw option will be used - user will get the panic message.
> 
> Signed-off-by: Vadim Kochan 
> ---
>  mac80211.c   |  1 -
>  mac80211.h   | 15 +++
>  trafgen/Makefile | 21 +++--
>  3 files changed, 30 insertions(+), 7 deletions(-)
> 
> diff --git a/mac80211.c b/mac80211.c
> index f22b600..9aea5a0 100644
> --- a/mac80211.c
> +++ b/mac80211.c
> @@ -24,7 +24,6 @@
>  #include 
>  #include 
>  
> -#include "die.h"
>  #include "str.h"
>  #include "dev.h"
>  #include "mac80211.h"
> diff --git a/mac80211.h b/mac80211.h
> index dea4ae0..2780c03 100644
> --- a/mac80211.h
> +++ b/mac80211.h
> @@ -1,7 +1,22 @@
>  #ifndef MAC80211_H
>  #define MAC80211_H
>  
> +#include "die.h"
> +#include "config.h"
> +
> +#ifdef CONFIG_NO_LIBNL
> +static inline void enter_rfmon_mac80211(const char *device, char **mondev)
> +{
> +panic("enter_rfmon_mac80211: CONFIG_NO_LIBNL option needs to be 
> disabled\n");
> +}
> +
> +static inline void leave_rfmon_mac80211(const char *mondev)
> +{
> +panic("leave_rfmon_mac80211: CONFIG_NO_LIBNL option needs to be 
> disabled\n");
> +}

These messages both have a double negative which isn't very easy to
understand. I suggest to call the CONFIG directive CONFIG_LIBNL instead
which is also more in line with the other library CONFIG_* directives we
have.

Also, you use the CONFIG_NO_LIBNL without setting it anywhere, this is
only done in patch 3/3. Please change the series order to make the
configure change come first.

-- 
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 netsniff-ng+unsubscr...@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.


Re: [netsniff-ng] [PATCH] netsniff-ng: account skipped packets as 'seen' and 'dropped'

2016-07-29 Thread Tobias Klauser
On 2016-07-27 at 15:59:08 +0200, Paolo Abeni  wrote:
> The packets filtered out due to pkt_type are incoming packets
> effectively dropped and should be accounted as such.
> This patch explicitly accounts the skipped packets number in
> skip_packet() and add such number to the 'drop' and 'seen'
> counters in update_rx_stats().
> 
> Signed-off-by: Paolo Abeni 

Applied, thanks Paolo!

-- 
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 netsniff-ng+unsubscr...@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.


Re: [netsniff-ng] [PATCH 01/15] trafgen: Move applying dynamic elements to function

2016-07-27 Thread Tobias Klauser
On 2016-07-26 at 21:35:06 +0200, Vadim Kochan  wrote:
> Same code for applying dynamic elements is used in both places
> for slow & fast path modes, lets move them into one inlined function.
> 
> Signed-off-by: Vadim Kochan 

Applied, thanks.

-- 
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 netsniff-ng+unsubscr...@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.


Re: [netsniff-ng] Re: [PATCH 00/15] trafgen: Support dinc & drnd for proto fields

2016-07-27 Thread Tobias Klauser
Nice work Vadim! This looks good from a bird's-eye view, but due to the
size of the changes it will probably take me some time to review it
completely and give you feedback (with the exception of patch 1/15 which
is a no-brainer and I already applied it).

On 2016-07-27 at 13:04:45 +0200, Vadim Kochan  wrote:
> I really missed one thing related to csum - so if I have proto field
> with dynamic function then csum will be calculated
> minimum twice - after packet is build and after all dynamic fields are
> applied, well I have no particular solution for this,
> but let me know if you want to have it fixed in this series or after
> it will be applied.

I think this can go into a separate series.

Thanks
Tobias

-- 
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 netsniff-ng+unsubscr...@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.


Re: [netsniff-ng] [PATCH] netsniff-ng: increment the pkts_seen after packet type check

2016-07-21 Thread Tobias Klauser
On 2016-07-21 at 16:42:22 +0200, Paolo Abeni  wrote:
> Currently in receive_to_xmit() pkts_seen is incremented before
> after the packet type check, but failing the latter will cause
> the packet to be ignored, pretty much as if it failed to pass
> the filter.
> This change move the accunting after the check, as is currently
> done in both walk_t3_block() and recv_only_or_dump().
> 
> Signed-off-by: Paolo Abeni 

Applied, thanks!

-- 
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 netsniff-ng+unsubscr...@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.


Re: [netsniff-ng] packet accounting in netsniff-ng

2016-07-21 Thread Tobias Klauser
On 2016-07-21 at 16:01:30 +0200, Paolo Abeni  wrote:
> Hi all,
> 
> I have a couple of doubts about packet accounting in netsniff-ng:
> currently in receive_to_xmit() pkts_seen is incremented before the
> packet type check, but failing the latter will cause the packet to be
> ignored, pretty much as if it failed to pass the filter.
> In walk_t3_block() and recv_only_or_dump() the accounting is performed
> after the check, should the receive_to_xmit() do the same ?

Yes, receive_to_xmit() should definitely do the same. The current
behavior is wrong.

> Should pkts_drops be incremented if the packet type filter fails ? 

Yes, I think they should be accounted for as well.

> If there is agreement I can send patches implementing the above.

Would be much appreciated, thanks!

-- 
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 netsniff-ng+unsubscr...@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.


Re: [netsniff-ng] [PATCH] netsniff-ng: skip duplicated packets on loopback device

2016-07-21 Thread Tobias Klauser
On 2016-07-21 at 15:25:27 +0200, Paolo Abeni  wrote:
> When sniffing on the loopback device, each packet will be seen
> twice, once per direction. To avoid duplicates, explicitly
> skip OUTGOING packets received from loopback, if no packet_type
> filter is explicitly set.
> 
> Signed-off-by: Paolo Abeni 

Applied, thanks a lot Paolo.

-- 
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 netsniff-ng+unsubscr...@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.


[netsniff-ng] Re: [PATCH 0/2] trafgen: Avoid to use user defined fields for csum

2016-07-18 Thread Tobias Klauser
On 2016-07-16 at 11:39:42 +0200, Vadim Kochan  wrote:
> It is dangerous to use such fields like 'ihl' and 'len' when calculate csum
> for IPv4 & UDP headers because these fields are set from user. Instead -
> use program calculated values.
> 
> Vadim Kochan (2):
>   trafgen: ipv4: Do not use 'ihl' field when calculate csum
>   trafgen: udp: Do not use 'len' field when calculate csum

Series applied.

-- 
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 netsniff-ng+unsubscr...@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.


[netsniff-ng] Re: [PATCH] trafgen: ipv4: Set default proto as ipv6-in-ipv4 for ipv6()

2016-07-16 Thread Tobias Klauser
On 2016-07-15 at 22:21:05 +0200, Vadim Kochan  wrote:
> Set default ip proto field to IPPROTO_IPV6(41) if the higher protocol
> was specified as ipv6().
> 
> Signed-off-by: Vadim Kochan 

Applied, thanks Vadim.

-- 
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 netsniff-ng+unsubscr...@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.


[netsniff-ng] Re: [PATCH v2 0/3] trafgen: Implement ICMPv4 packet creation - part #2

2016-07-13 Thread Tobias Klauser
On 2016-07-13 at 00:01:52 +0200, Vadim Kochan  wrote:
> Add ICMPv4 protocol header creating.
> 
> Changed echorequest to echo-request, and echoreply to echo-reply
> which looks more readable.
> 
> The following ICMPv4 parameters are supported:
> 
> type
> code
> csum
> mtu
> seq
> id
> addr
> 
> v2:
> 1) Drop patch for split echorequest with "-" sign
> 
> 2) Removed keywords with special icmpv4 message type parameters except
>MTU & Address.
> 
> Vadim Kochan (3):
>   trafgen: l3: Add creating ICMPv4 header
>   trafgen: parser: Add function to build ICMPv4 header
>   trafgen: man: Add description for icmpv4() function

Series squashed and applied, thanks.

-- 
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 netsniff-ng+unsubscr...@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.


[netsniff-ng] Re: [PATCH 0/6] trafgen: Implement ICMPv4 packet creation

2016-07-11 Thread Tobias Klauser
On 2016-06-29 at 11:21:24 +0200, Vadim Kochan  wrote:
> Hi Tobias,
> 
> On Sat, Apr 30, 2016 at 6:01 PM, Tobias Klauser  wrote:
> > Hi Vadim
> >
> > Thanks for the series. I'll be travelling for a while starting tomorrow,
> > so it will probably take a while until I get to review the patches.
> >
> > Thanks
> > Tobias
> 
> 
> Will you look on this series ?

Sorry for taking so long with the review - quite busy times here :(
Please see my comments on the individual patches.

Thanks a lot for your patience!

-- 
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 netsniff-ng+unsubscr...@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.


[netsniff-ng] Re: [PATCH 5/6] trafgen: parser: Add function to build ICMPv4 header

2016-07-11 Thread Tobias Klauser
On 2016-04-30 at 16:39:19 +0200, Vadim Kochan  wrote:
> Add 'icmpv4()' function to create ICMPv4 header.
> 
> Parameters suported:
> 
> type  Set type field (default 0: Echo reply)
> code  Set code field (default 0)
> csum  Set checksum field (calculated by default)
> unusedSet 4-byte unused field (default 0)
> mtu Set mtu field for destination unreachable (default 0)
> seq Set sequence field (default 0)
> id  Set identifier field (default 0)
> addr  Set redirect address (default 0)
> 
> The following message types are supported:
> 
> net-unreach Set 'Destination network is unreachable' message type
> host-unreach  Set 'Destination host is unreachable' message type
> prot-unreachSet 'Destination protocol is unreachable' message type
> port-unreachSet 'Destination port is unreachable' message type

These two can easily be confused or typoed - but see my remark below.

> net-tos-unreach Set 'Destination network & ToS is unreachable' 
> message type
> host-tos-unreachSet 'Destination host & ToS is unreachable' message 
> type
> frag-needed Set 'Fragmentation is needed' message type
> route-failedSet 'Source route is failed' message type
> net-unknown Set 'Destination network is unknown' message type
> host-unknownSet 'Destination host is unknown' message type
> host-isolated   Set 'Source host isolated' message type
> net-forbidden   Set 'Network is adimnistratively prohibited' message 
> type
> host-forbidden  Set 'Host is administratively prohibited' message type
> pkt-filteredSet 'Packet is filtered' message type
> 
> net-redirectSet 'Redirect datagram for the network' message type
> host-redirect   Set 'Redirect datagram for the host' message type
> net-tos-redirectSet 'Redirect datagram for the ToS & network' message 
> type
> host-tos-redirect   Set 'Redirect datagram for the ToS & host' message 
> type
> 
> ttl-timeout Set 'TTL expired in transit' message type
> frag-timeoutSet 'Fragment reassembly time exceeded' message type

IMO it's a bit of an overkill to add an extra keyword for every
(possibly seldomly used) message type. I guess we can expect users to
look up the codes for these particular message types in the RFC in case
they want to use them.

-- 
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 netsniff-ng+unsubscr...@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.


[netsniff-ng] Re: [PATCH 3/6] trafgen: parser: Split echorequest & echoreply with "-"

2016-07-11 Thread Tobias Klauser
On 2016-04-30 at 16:39:17 +0200, Vadim Kochan  wrote:
> Change echorequest to echo-request, and echoreply to echo-reply
> which looks more readable.
> 
> Signed-off-by: Vadim Kochan 

NAK, I think it's OK the way it currently is. All other "2-word"
keywords also don't have a separating dash.

-- 
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 netsniff-ng+unsubscr...@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.


[netsniff-ng] Re: [PATCH 1/6] trafgen: parser: Split [e]type to separate keywords

2016-07-11 Thread Tobias Klauser
On 2016-04-30 at 16:39:15 +0200, Vadim Kochan  wrote:
> Split [e]etype to separate 'type' & 'etype' keywords,
> the reason is that 'type' might be used in other protocol
> headers (e.g. ICMP).
> 
> Signed-off-by: Vadim Kochan 

Applied, thanks.

-- 
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 netsniff-ng+unsubscr...@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.


[netsniff-ng] Re: [PATCH 2/6] trafgen: parser: Replace 'mtype' by 'type'

2016-07-11 Thread Tobias Klauser
On 2016-04-30 at 16:39:16 +0200, Vadim Kochan  wrote:
> After splitting etype & type to different tokens it is possible
> to use 'type' for ICMP type field which is used by RFC.
> 
> Signed-off-by: Vadim Kochan 

Applied, thanks.

-- 
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 netsniff-ng+unsubscr...@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.


[netsniff-ng] Re: [PATCH] netsniff-ng: Create out pcap only if first packet passed

2016-06-29 Thread Tobias Klauser
On 2016-05-16 at 18:58:21 +0200, Vadim Kochan  wrote:
> If all packets did not pass the filter then output pcap
> file will be created with pcap header, which might be not what user
> expect - to see only interested pcap files.

IMO, an output file should always be created, even if it contains no
packets. Otherwise it's not entirely obvious whether there was an error
in netsniff-ng or whether there just weren't any passed packets. Also,
in case the file can't be created (e.g. due to lacking permissions) and
the first packet to be written only arrives a large amount of time after
program start, the user would only notice by then.

BTW, an empty pcap opens just fine in e.g. Wireshark.

-- 
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 netsniff-ng+unsubscr...@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.


[netsniff-ng] Re: [PATCH] netsniff-ng: pcap_io: Print unsupported magic number

2016-06-22 Thread Tobias Klauser
On 2016-06-22 at 09:29:41 +0200, Vadim Kochan  wrote:
> Hi Tobias,
> 
> On Wed, Jun 22, 2016 at 10:25 AM, Tobias Klauser  wrote:
> > On 2016-05-16 at 19:32:46 +0200, Vadim Kochan  wrote:
> >> It might be more understandable to print unsupported
> >> pcap magic number in hexadecimal format.
> >>
> >> Signed-off-by: Vadim Kochan 
> >
> > Applied, thanks.
> 
> I see you'r back, I just I'd like to clarify something regarding
> flowtop, I am trying to implement horizontal column scrolling, so
> in case if you plan to do some release - would it be better to wait
> when scrolling will be ready for flowtop ?

Given that there wasn't that much change since 0.6.1 (and the long
inactivity period) I think we won't do a release very soon. So feel free
to submit your changes if you see fit.

Thanks

-- 
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 netsniff-ng+unsubscr...@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.


Re: [netsniff-ng] [PATCH] man: netsniff-ng: Fix usage example description

2016-06-22 Thread Tobias Klauser
On 2016-06-21 at 18:20:35 +0200, Hisao Tanabe  wrote:
> Fix the input device name that is used in the description of the usage
> example.
> 
> Signed-off-by: Hisao Tanabe 

Applied, thanks a lot.

-- 
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 netsniff-ng+unsubscr...@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.


[netsniff-ng] Re: [PATCH] netsniff-ng: pcap_io: Print unsupported magic number

2016-06-22 Thread Tobias Klauser
On 2016-05-16 at 19:32:46 +0200, Vadim Kochan  wrote:
> It might be more understandable to print unsupported
> pcap magic number in hexadecimal format.
> 
> Signed-off-by: Vadim Kochan 

Applied, thanks.

-- 
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 netsniff-ng+unsubscr...@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.


[netsniff-ng] Re: [PATCH 0/6] trafgen: Implement ICMPv4 packet creation

2016-04-30 Thread Tobias Klauser
Hi Vadim

Thanks for the series. I'll be travelling for a while starting tomorrow,
so it will probably take a while until I get to review the patches.

Thanks
Tobias

-- 
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 netsniff-ng+unsubscr...@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.


[netsniff-ng] Re: flowtop: How should be src info printed in stdout dump mode

2016-04-26 Thread Tobias Klauser
On 2016-04-26 at 12:24:45 +0200, Vadim Kochan  wrote:
> On Tue, Apr 26, 2016 at 12:00 PM, Tobias Klauser  wrote:
> > On 2016-04-26 at 10:47:18 +0200, Vadim Kochan  wrote:
> >> On Tue, Apr 26, 2016 at 10:30:19AM +0200, Tobias Klauser wrote:
> >> > On 2016-04-25 at 16:55:53 +0200, Vadim Kochan  wrote:
> >> > > On Mon, Apr 25, 2016 at 11:21:57AM +0200, Tobias Klauser wrote:
> >> > > > On 2016-04-22 at 23:53:01 +0200, Vadim Kochan  
> >> > > > wrote:
> >> > > > > Hi,
> >> > > > >
> >> > > > > I am thinking about to add dump of flows to stdout. It seems OK if
> >> > > > > to use similar table format like in curses mode by default, but in 
> >> > > > > case
> >> > > > > of src peer info (2 lines per flow) the output processing by 
> >> > > > > external scripts
> >> > > > > or text processors might be too complex with considering of row
> >> > > > > numbering.
> >> > > > >
> >> > > > > So here are my conclusions:
> >> > > > >
> >> > > > > 1) Default is OK - 1 line per flows with DST info only.
> >> > > > >
> >> > > > > 2) If "-s" option is specified - print 2 lines per flows like in 
> >> > > > > curses mode.
> >> > > > >
> >> > > > > 3) Add "-o, --oneline" option to print src & dst info in 1 row, 
> >> > > > > even
> >> > > > > if it will be overflowed in next line - this is just for external 
> >> > > > > text
> >> > > > > processing.
> >> > > >
> >> > > > I'd strongly prefer this way of implementing it - similar to the -c
> >> > > > option for ifpps. IMO, there's bo reason to care about line 
> >> > > > overflows,
> >> > > > as the main target will be script processing.
> >> > > >
> >> > > > > Also may be it would be useful to print empty columns with "*" or 
> >> > > > > "-"
> >> > > > > it will be more visually readable (probably in curses mode too) 
> >> > > > > and also be
> >> > > > > processed by awk.
> >> > > >
> >> > > > Either this (just make sure it's a character that can't appear 
> >> > > > inside a
> >> > > > field), or separate the columns using comma or semicolon.
> >> > >
> >> > >
> >> > > I) This is an example of default output 'flowtop -d':
> >> > >
> >> > > PROCESS   PID PROTO  STATE   TIME ADDRESS  
> >> > >   PORT GEO  BYTES   RATE
> >> > >
> >> > > * *   tcpTIME-WAIT50s mc.yandex.ru 
> >> > >   httpsRUS476  *
> >> > > * *   tcpTIME-WAIT51s host10.rax.ru
> >> > >   http RUS164  *
> >> > > firefox   29425   tcpESTABLISHED  53s 74.117.181.52
> >> > >   http USA  1.7kB  *
> >> > > * *   tcpTIME-WAIT52s bs.yandex.ru 
> >> > >   http RUS  2.2kB  *
> >> > > * *   tcpTIME-WAIT51s host69.rax.ru
> >> > >   http RUS  1.3kB  *
> >> > > firefox   29425   tcpESTABLISHED  53s 74.117.181.52
> >> > >   http USA  1.9kB  *
> >> > > * *   tcpTIME-WAIT51s host10.rax.ru
> >> > >   http RUS533  *
> >> > >
> >> > > II) This is an example of output 'flowtop -ds', each flow entry
> >> > > separated with additional empty line to easy separate src & dst:
> >> > >
> >> > >
> >> > > PROCESS   PID PROTO  STATE   TIME ADDRESS  
> >> > >   PORT GEO  BYTES   RATE
> >> &g

[netsniff-ng] Re: [PATCH 5/8] flowtop: Remove unused args in draw_flow_entry(...)

2016-04-26 Thread Tobias Klauser
On 2016-04-26 at 09:47:19 +0200, Vadim Kochan  wrote:
> Remove unused "screen" & "line" arguments in draw_flow_entry(...)
> function.
> 
> Signed-off-by: Vadim Kochan 

This does not really influence the rest of the series, so I already
applied this, thanks.

-- 
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 netsniff-ng+unsubscr...@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.


[netsniff-ng] Re: flowtop: How should be src info printed in stdout dump mode

2016-04-26 Thread Tobias Klauser
On 2016-04-26 at 10:47:18 +0200, Vadim Kochan  wrote:
> On Tue, Apr 26, 2016 at 10:30:19AM +0200, Tobias Klauser wrote:
> > On 2016-04-25 at 16:55:53 +0200, Vadim Kochan  wrote:
> > > On Mon, Apr 25, 2016 at 11:21:57AM +0200, Tobias Klauser wrote:
> > > > On 2016-04-22 at 23:53:01 +0200, Vadim Kochan  wrote:
> > > > > Hi,
> > > > > 
> > > > > I am thinking about to add dump of flows to stdout. It seems OK if
> > > > > to use similar table format like in curses mode by default, but in 
> > > > > case
> > > > > of src peer info (2 lines per flow) the output processing by external 
> > > > > scripts
> > > > > or text processors might be too complex with considering of row
> > > > > numbering.
> > > > > 
> > > > > So here are my conclusions:
> > > > > 
> > > > > 1) Default is OK - 1 line per flows with DST info only.
> > > > > 
> > > > > 2) If "-s" option is specified - print 2 lines per flows like in 
> > > > > curses mode.
> > > > > 
> > > > > 3) Add "-o, --oneline" option to print src & dst info in 1 row, even
> > > > > if it will be overflowed in next line - this is just for external text
> > > > > processing.
> > > > 
> > > > I'd strongly prefer this way of implementing it - similar to the -c
> > > > option for ifpps. IMO, there's bo reason to care about line overflows,
> > > > as the main target will be script processing.
> > > > 
> > > > > Also may be it would be useful to print empty columns with "*" or "-"
> > > > > it will be more visually readable (probably in curses mode too) and 
> > > > > also be
> > > > > processed by awk.
> > > > 
> > > > Either this (just make sure it's a character that can't appear inside a
> > > > field), or separate the columns using comma or semicolon.
> > > 
> > > 
> > > I) This is an example of default output 'flowtop -d':
> > > 
> > > PROCESS   PID PROTO  STATE   TIME ADDRESS 
> > >PORT GEO  BYTES   RATE 
> > >   
> > >   
> > > 
> > > * *   tcpTIME-WAIT50s mc.yandex.ru
> > >httpsRUS476  * 
> > > * *   tcpTIME-WAIT51s host10.rax.ru   
> > >http RUS164  * 
> > > firefox   29425   tcpESTABLISHED  53s 74.117.181.52   
> > >http USA  1.7kB  * 
> > > * *   tcpTIME-WAIT52s bs.yandex.ru
> > >http RUS  2.2kB  * 
> > > * *   tcpTIME-WAIT51s host69.rax.ru   
> > >http RUS  1.3kB  * 
> > > firefox   29425   tcpESTABLISHED  53s 74.117.181.52   
> > >http USA  1.9kB  * 
> > > * *   tcpTIME-WAIT51s host10.rax.ru   
> > >http RUS533  * 
> > > 
> > > II) This is an example of output 'flowtop -ds', each flow entry
> > > separated with additional empty line to easy separate src & dst:
> > > 
> > > 
> > > PROCESS   PID PROTO  STATE   TIME ADDRESS 
> > >PORT GEO  BYTES   RATE 
> > >   
> > >   
> > > mutt  30420   tcpESTABLISHED   1m angus-think 
> > >48154*3.9kB  * 
> > >   --> lo-in-f108.1e100.net
> > >imapsUSA 95.7kB  * 
> > > 
> > > * *   tcpESTABLISHED  21h angus-think 
> > >42480*3.9MB  * 
> > >

[netsniff-ng] Re: flowtop: How should be src info printed in stdout dump mode

2016-04-26 Thread Tobias Klauser
On 2016-04-25 at 16:55:53 +0200, Vadim Kochan  wrote:
> On Mon, Apr 25, 2016 at 11:21:57AM +0200, Tobias Klauser wrote:
> > On 2016-04-22 at 23:53:01 +0200, Vadim Kochan  wrote:
> > > Hi,
> > > 
> > > I am thinking about to add dump of flows to stdout. It seems OK if
> > > to use similar table format like in curses mode by default, but in case
> > > of src peer info (2 lines per flow) the output processing by external 
> > > scripts
> > > or text processors might be too complex with considering of row
> > > numbering.
> > > 
> > > So here are my conclusions:
> > > 
> > > 1) Default is OK - 1 line per flows with DST info only.
> > > 
> > > 2) If "-s" option is specified - print 2 lines per flows like in curses 
> > > mode.
> > > 
> > > 3) Add "-o, --oneline" option to print src & dst info in 1 row, even
> > > if it will be overflowed in next line - this is just for external text
> > > processing.
> > 
> > I'd strongly prefer this way of implementing it - similar to the -c
> > option for ifpps. IMO, there's bo reason to care about line overflows,
> > as the main target will be script processing.
> > 
> > > Also may be it would be useful to print empty columns with "*" or "-"
> > > it will be more visually readable (probably in curses mode too) and also 
> > > be
> > > processed by awk.
> > 
> > Either this (just make sure it's a character that can't appear inside a
> > field), or separate the columns using comma or semicolon.
> 
> 
> I) This is an example of default output 'flowtop -d':
> 
> PROCESS   PID PROTO  STATE   TIME ADDRESS 
>PORT GEO  BYTES   RATE 
>   
>   
> 
> * *   tcpTIME-WAIT50s mc.yandex.ru
>httpsRUS476  * 
> * *   tcpTIME-WAIT51s host10.rax.ru   
>http RUS164  * 
> firefox   29425   tcpESTABLISHED  53s 74.117.181.52   
>http USA  1.7kB  * 
> * *   tcpTIME-WAIT52s bs.yandex.ru
>http RUS  2.2kB  * 
> * *   tcpTIME-WAIT51s host69.rax.ru   
>http RUS  1.3kB  * 
> firefox   29425   tcpESTABLISHED  53s 74.117.181.52   
>http USA  1.9kB  * 
> * *   tcpTIME-WAIT51s host10.rax.ru   
>http RUS533  * 
> 
> II) This is an example of output 'flowtop -ds', each flow entry
> separated with additional empty line to easy separate src & dst:
> 
> 
> PROCESS   PID PROTO  STATE   TIME ADDRESS 
>PORT GEO  BYTES   RATE 
>   
>   
> mutt  30420   tcpESTABLISHED   1m angus-think 
>48154*3.9kB  * 
>   --> lo-in-f108.1e100.net
>imapsUSA 95.7kB  * 
> 
> * *   tcpESTABLISHED  21h angus-think 
>42480*3.9MB  * 
>   --> 194.44.4.115
>httpsUKR191.0MB  * 
> 
> skype 20044   tcpESTABLISHED  48m angus-think 
>50148*  302.7kB  * 
>   --> 157.55.130.153  
>40021USA187.8kB  * 
> 
> skype 20044   tcpESTABLISHED   7h angus-think 
>51028*7.5kB  * 
>   --> 91.190.217.47   
>12350LUX  4.9kB  * 
> 
> 
> What do you think ?

Two things come to mind:

1) bytes and rate - if applicable - should be printed as raw byte count
   (not shortened to kB, MB etc) in the stdout mode. Th

[netsniff-ng] Re: flowtop: How should be src info printed in stdout dump mode

2016-04-25 Thread Tobias Klauser
On 2016-04-22 at 23:53:01 +0200, Vadim Kochan  wrote:
> Hi,
> 
> I am thinking about to add dump of flows to stdout. It seems OK if
> to use similar table format like in curses mode by default, but in case
> of src peer info (2 lines per flow) the output processing by external scripts
> or text processors might be too complex with considering of row
> numbering.
> 
> So here are my conclusions:
> 
> 1) Default is OK - 1 line per flows with DST info only.
> 
> 2) If "-s" option is specified - print 2 lines per flows like in curses mode.
> 
> 3) Add "-o, --oneline" option to print src & dst info in 1 row, even
> if it will be overflowed in next line - this is just for external text
> processing.

I'd strongly prefer this way of implementing it - similar to the -c
option for ifpps. IMO, there's bo reason to care about line overflows,
as the main target will be script processing.

> Also may be it would be useful to print empty columns with "*" or "-"
> it will be more visually readable (probably in curses mode too) and also be
> processed by awk.

Either this (just make sure it's a character that can't appear inside a
field), or separate the columns using comma or semicolon.

-- 
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 netsniff-ng+unsubscr...@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.


[netsniff-ng] Re: [PATCH] flowtop: man: Add how-to activate conntrack by modprobe

2016-04-22 Thread Tobias Klauser
On 2016-04-22 at 06:34:43 +0200, Vadim Kochan  wrote:
> Add another tip how to activate conntrack mechanism by
> loading required kernel modules via modprobe. This info
> might be used to make these modules load automatically at startup.
> 
> Signed-off-by: Vadim Kochan 

Applied, thanks.

-- 
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 netsniff-ng+unsubscr...@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.


[netsniff-ng] Re: [PATCH v3 0/6] flowtop: Layout change to 1-row view (part #2)

2016-04-22 Thread Tobias Klauser
On 2016-04-21 at 20:47:37 +0200, Vadim Kochan  wrote:
> This is a 2nd part of series about changing flowtop layout to 1-row
> view. This version is mostly a refactoring of flows refreshing to
> make it more generic by using UI table API.
> 
> v3:
> 1) Fixed headers including
> 
> 2) Fixed trailing tab in ui.c
> 
> 3) Replaced panic(...) by bug_on(true) in ui table column lookup function.

Note that you can also use bug() as a shorthand for bug_on(true). I
changed your patches accordingly before applying.

> 4) Clear screen on init.
> v2:
> 1) Add UI table widget with generic implementation for
>print list items in table style. Add new UI module with
>generic table API.
> 
> 2) Add Linux-like list API used from liburcu but with redefinitions
>to the Linux naming.
> 
> 3) Get rid of clear & refresh screen each time while flows printing.
> 
> Vadim Kochan (6):
>   list: Add re-defined double-linked list API from liburcu
>   ui: Implement UI table for flows printing
>   flowtop: Use new UI table API for draw flows list
>   ui: Print empty rows when clear table
>   flowtop: Get rid of clear() & refresh() calls
>   flowtop: Simplify flows refresh delay

Series applied, thanks Vadim!

-- 
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 netsniff-ng+unsubscr...@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.


[netsniff-ng] Re: [PATCH] flowtop: Fix compilation error when build without geoip

2016-04-22 Thread Tobias Klauser
On 2016-04-22 at 06:05:47 +0200, Vadim Kochan  wrote:
> Commit (f61f39d geoip: Allow to get country 3-code) added
> new helpers without considering HAVE_GEOIP definition.
> 
> Fixed by adding dummy functions for get 3-code country name
> in case if HAVE_GEOIP is not defined.
> 
> Tested only by manualy disabling HAVE_GEOIP in config.h
> and geoip.c compilation in Config file.
> 
> Fixes: f61f39d (geoip: Allow to get country 3-code)
> Reported-by: Daniel Borkmann 
> Signed-off-by: Vadim Kochan 

Applied. Thanks Daniel and Vadim!

-- 
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 netsniff-ng+unsubscr...@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.


[netsniff-ng] Re: [PATCH v2 07/11] ui: Implement UI table for flows printing

2016-04-21 Thread Tobias Klauser
On 2016-04-21 at 14:04:05 +0200, Vadim Kochan  wrote:
> On Thu, Apr 21, 2016 at 2:53 PM, Tobias Klauser  wrote:
> > On 2016-04-17 at 19:31:30 +0200, Vadim Kochan  wrote:
> >> Add new module ui.c which is responsible to render
> >> different kinds of UI widgets - tables, etc.
> >>
> >> Implemented generic API for print table-like list of elements.
> >> This table API might be used for print flows in curses or text mode.
> >>
> >> Signed-off-by: Vadim Kochan 
> >
> > Looks good in general, a few minor nits below.
> >
> >> ---
> >>  ui.c | 142 
> >> +++
> >>  ui.h |  53 +
> >
> > How about calling the module ui_table, which is a bit more specific?
> > Also all function names already start with "ui_table_".
> 
> I thought that ui.c might keep other UI helpers (widgets), so I called
> it just "ui", so if
> there will be some other UI widget, then ui_xxx naming will be still OK.

Hm, ok. I just think UI might be a bit too generic (also given that it
is only used by flowtop). But we can still rename/split later on if need
be...

> >> +static struct ui_col *ui_table_col_get(struct ui_table *tbl, uint32_t id)
> >> +{
> >> + struct ui_col *col;
> >> +
> >> + list_for_each_entry(col, &tbl->cols, entry) {
> >> + if (col->id == id)
> >> + return col;
> >> + }
> >> +
> >> + /* Should not happen in normal case */
> >> + panic("Invalid column id %u\n", id);
> >
> > I'd rather omit the panic() here, return NULL and gracefully handle (if
> > possible) NULL returns in the callers.
> >
> 
> Hm, for me it looks like more programmers's issue so may be
> bug_on(...) will be better ?
> But I will still think on it more ...

In my opinion we should never panic() based on a programming error but
only use it e.g. if syscalls fail.  bug_on() is certainly better suited
here.

-- 
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 netsniff-ng+unsubscr...@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.


[netsniff-ng] Re: [PATCH v2 07/11] ui: Implement UI table for flows printing

2016-04-21 Thread Tobias Klauser
On 2016-04-17 at 19:31:30 +0200, Vadim Kochan  wrote:
> Add new module ui.c which is responsible to render
> different kinds of UI widgets - tables, etc.
> 
> Implemented generic API for print table-like list of elements.
> This table API might be used for print flows in curses or text mode.
> 
> Signed-off-by: Vadim Kochan 

Looks good in general, a few minor nits below.

> ---
>  ui.c | 142 
> +++
>  ui.h |  53 +

How about calling the module ui_table, which is a bit more specific?
Also all function names already start with "ui_table_".

>  2 files changed, 195 insertions(+)
>  create mode 100644 ui.c
>  create mode 100644 ui.h
> 
> diff --git a/ui.c b/ui.c
> new file mode 100644
> index 000..d5fe1a7
> --- /dev/null
> +++ b/ui.c
> @@ -0,0 +1,142 @@
> +/*
> + * netsniff-ng - the packet sniffing beast
> + * Subject to the GPL, version 2.
> + */
> +
> +#include "ui.h"
> +#include "xmalloc.h"
> +
> +#include 

Global headers should come first, i.e.

#include 

#include "ui.h"
#include "xmalloc.h"

> +void ui_table_init(struct ui_table *tbl)
> +{
> + memset(tbl, 0, sizeof(*tbl));
> +
> + getsyx(tbl->y, tbl->x);
> +
> + tbl->rows_y  = tbl->y;
> + tbl->width   = COLS;
> + tbl->col_pad = 1;
> +
> + INIT_LIST_HEAD(&tbl->cols);
> +}
> +
> +void ui_table_uninit(struct ui_table *tbl)
> +{
> + struct ui_col *col, *tmp;
> +
> + list_for_each_entry_safe(col, tmp, &tbl->cols, entry)
> + xfree(col);
> +}
> +
> +void ui_table_pos_set(struct ui_table *tbl, int y, int x)
> +{
> + tbl->y  = y;
> + tbl->x  = x;
> + tbl->rows_y = y;
> +}
> +
> +static struct ui_col *ui_table_col_get(struct ui_table *tbl, uint32_t id)
> +{
> + struct ui_col *col;
> +
> + list_for_each_entry(col, &tbl->cols, entry) {
> + if (col->id == id)
> + return col;
> + }
> +
> + /* Should not happen in normal case */
> + panic("Invalid column id %u\n", id);

I'd rather omit the panic() here, return NULL and gracefully handle (if
possible) NULL returns in the callers.

> +}
> +
> +static void __ui_table_pos_update(struct ui_table *tbl)
> +{
> + struct ui_col *col;
> + uint32_t pos = tbl->x;
> +
> + list_for_each_entry(col, &tbl->cols, entry) {
> + col->pos  = pos;
> + pos  += col->len + tbl->col_pad;
> + }
> +}
> +
> +void ui_table_col_add(struct ui_table *tbl, uint32_t id, char *name, 
> uint32_t len)
> +{
> + struct ui_col *col = xzmalloc(sizeof(*col));
> +
> + col->id= id;
> + col->name  = name;
> + col->len   = len;
> + col->align = UI_ALIGN_LEFT;
> +
> + list_add_tail(&col->entry, &tbl->cols);
> +
> + __ui_table_pos_update(tbl);
> +}
> +
> +void ui_table_col_color_set(struct ui_table *tbl, int col_id, int color)
> +{
> + struct ui_col *col = ui_table_col_get(tbl, col_id);
> +
> + col->color = color;
> +}
> +
> +void ui_table_col_align_set(struct ui_table *tbl, int col_id, enum ui_align 
> align)
> +{
> + struct ui_col *col = ui_table_col_get(tbl, col_id);
> +
> + col->align = align;
> +}
> +
> +void ui_table_row_add(struct ui_table *tbl)
> +{
> + tbl->rows_y++;
> +}
> +
> +void ui_table_clear(struct ui_table *tbl)
> +{
> + tbl->rows_y = tbl->y;
> +}
> +
> +#define UI_ALIGN_COL(col) (((col)->align == UI_ALIGN_LEFT) ? "%-*.*s" : 
> "%*.*s")
> +
> +static void __ui_table_row_print(struct ui_table *tbl, struct ui_col *col,
> +  const char *str)
> +{
> + mvprintw(tbl->rows_y, col->pos, UI_ALIGN_COL(col), col->len, col->len, 
> str);
> + mvprintw(tbl->rows_y, col->pos + col->len, "%*s", tbl->col_pad, " ");
> +}
> +
> +void ui_table_row_print(struct ui_table *tbl, uint32_t col_id, const char 
> *str)
> +{
> + struct ui_col *col = ui_table_col_get(tbl, col_id);
> +
> + attron(col->color);
> + __ui_table_row_print(tbl, col, str);
> + attroff(col->color);
> +}
> +
> +void ui_table_header_color_set(struct ui_table *tbl, int color)
> +{
> + tbl->hdr_color = color;
> +}
> +
> +void ui_table_header_print(struct ui_table *tbl)
> +{
> + struct ui_col *col;
> + int max_width = tbl->width;
> + int width = 0;
> +
> + attron(tbl->hdr_color);
> +
> + mvprintw(tbl->y, tbl->x, "%-*.*s", max_width - tbl->x, max_width - 
> tbl->x, "");
> + mvprintw(tbl->y, tbl->x, "");
> +
> + list_for_each_entry(col, &tbl->cols, entry) {
> + __ui_table_row_print(tbl, col, col->name);
> + width += col->len + tbl->col_pad;
> + }
> + 

Trailing whitespace (tab) here.

> + mvprintw(tbl->y, width, "%*s", max_width - width, " ");
> + attroff(tbl->hdr_color);
> +}
> diff --git a/ui.h b/ui.h
> new file mode 100644
> index 000..02d1da2
> --- /dev/null
> +++ b/ui.h
> @@ -0,0 +1,53 @@
> +#ifndef UI_H
> +#define UI_H
> +
> +#include "list.h"
> +
> +#include 
> +#include 

Glo

[netsniff-ng] Re: [PATCH v2 00/11] flowtop: Layout change to 1-row view

2016-04-21 Thread Tobias Klauser
On 2016-04-17 at 19:31:23 +0200, 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.
> 
> v2:
> 1) Add UI table widget with generic implementation for
>print list items in table style. Add new UI module with
>generic table API.
> 
> 2) Add Linux-like list API used from liburcu but with redefinitions
>to the Linux naming.
> 
> 3) Get rid of clear & refresh screen each time while flows printing.
> 
> Vadim Kochan (11):
>   geoip: Allow to get country 3-code
>   flowtop: Change flows layout to 1-row view
>   flowtop: Add display option to show src info
>   screen: Add helpers to easy use color by name
>   flowtop: Use new colors naming & helpers
>   list: Add re-defined double-linked list API from liburcu
>   ui: Implement UI table for flows printing
>   flowtop: Use new UI table API for draw flows list
>   ui: Print empty rows when clear table
>   flowtop: Get rid of clear() & refresh() calls
>   flowtop: Simplify flows refresh delay

Applied patches 1-5, waiting for the respin with the remaining ones.

Thanks a lot!

-- 
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 netsniff-ng+unsubscr...@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.


[netsniff-ng] Re: [PATCH v2 10/11] flowtop: Get rid of clear() & refresh() calls

2016-04-21 Thread Tobias Klauser
On 2016-04-21 at 11:45:01 +0200, Vadim Kochan  wrote:
> On Sun, Apr 17, 2016 at 8:31 PM, Vadim Kochan  wrote:
> > Don't use refresh() & clear() as we draw entire screen
> > and flows table will be filled with empty rows.
> >
> > Signed-off-by: Vadim Kochan 
> > ---
> >  flowtop.c | 11 ---
> >  1 file changed, 4 insertions(+), 7 deletions(-)
> >
> > diff --git a/flowtop.c b/flowtop.c
> > index 32459aa..3060917 100644
> > --- a/flowtop.c
> > +++ b/flowtop.c
> > @@ -1093,9 +1093,6 @@ static void draw_flows(WINDOW *screen, struct 
> > flow_list *fl,
> > int skip = skip_lines;
> > struct flow_entry *n;
> >
> > -   wclear(screen);
> > -   clear();
> > -
> > rcu_read_lock();
> >
> > n = rcu_dereference(fl->head);
> > @@ -1124,6 +1121,7 @@ static void draw_flows(WINDOW *screen, struct 
> > flow_list *fl,
> > line += row_width;
> > }
> >
> > +   mvwprintw(screen, 1, 0, "%*s", COLS - 1, " ");
> > mvwprintw(screen, 1, 2, "Kernel netfilter flows(%u) for ", flows);
> >
> > if (what & INCLUDE_IPV4)
> > @@ -1254,6 +1252,7 @@ static void flows_table_init(struct ui_table *tbl)
> > ui_table_init(tbl);
> >
> > ui_table_pos_set(tbl, 3, 0);
> > +   ui_table_height_set(tbl, LINES - 3);
> >
> > ui_table_col_add(tbl, TBL_FLOW_PROCESS, "PROCESS", 13);
> > ui_table_col_add(tbl, TBL_FLOW_PID, "PID", 7);
> > @@ -1356,6 +1355,8 @@ static void presenter(void)
> > break;
> > }
> >
> > +   draw_header(screen);
> > +
> > if (!redraw_flows)
> > redraw_flows = time_passed_us >= 1 * USEC_PER_SEC;
> >
> > @@ -1369,15 +1370,11 @@ static void presenter(void)
> > time_passed_us += time_sleep_us;
> > }
> >
> > -   draw_header(screen);
> > -
> > if (show_help)
> > draw_help(screen);
> >
> > draw_footer(screen);
> >
> > -   wrefresh(screen);
> > -   refresh();
> > usleep(time_sleep_us);
> > }
> > rcu_unregister_thread();
> > --
> > 2.6.3
> >
> 
> Sorry, I just tested on another terminal color settings and I checked
> that at least 1 clear() is needed on init.
> 
> I will fix it and re-send series, or you can ignore last 2 patches.

Sorry, I only had time to review patches 1-5 so far. These look good and
I'll apply them. I'll wait for the revised series until reviewing the
other patches.

-- 
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 netsniff-ng+unsubscr...@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.


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

2016-04-17 Thread Tobias Klauser
On 2016-04-17 at 09:06:35 +0200, Vadim Kochan  wrote:
> Hi,
> 
> I prepared new series for flowtop, I changed state names to this:
> 
>   static const char *const tcp_state2str[TCP_CONNTRACK_MAX] = {
>   [TCP_CONNTRACK_NONE]= "NONE",
>   [TCP_CONNTRACK_SYN_SENT]= "SYN-SNT",
>   [TCP_CONNTRACK_SYN_RECV]= "SYN-RECV",
>   [TCP_CONNTRACK_ESTABLISHED] = "ESTAB",
>   [TCP_CONNTRACK_FIN_WAIT]= "FIN-WAIT",
>   [TCP_CONNTRACK_CLOSE_WAIT]  = "CLO-WAIT",
>   [TCP_CONNTRACK_LAST_ACK]= "LAST-ACK",
>   [TCP_CONNTRACK_TIME_WAIT]   = "TIMEWAIT",

It's a bit inconsistent without the dash (i.e. "TIME-WAIT"), but I
understand you want to keep it at max. 8 chars.

>   [TCP_CONNTRACK_CLOSE]   = "CLOSE",
>   [TCP_CONNTRACK_SYN_SENT2]   = "SYN-SNT2",
>   };
> 
>   static const char *const dccp_state2str[DCCP_CONNTRACK_MAX] = {
>   [DCCP_CONNTRACK_NONE]   = "NONE",
>   [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",
>   };
> 
>   static const char *const sctp_state2str[SCTP_CONNTRACK_MAX] = {
>   [SCTP_CONNTRACK_NONE]   = "NONE",
>   [SCTP_CONNTRACK_CLOSED] = "CLOSED",
>   [SCTP_CONNTRACK_COOKIE_WAIT]= "CK-WAIT",
>   [SCTP_CONNTRACK_COOKIE_ECHOED]  = "CK-ECHO",
>   [SCTP_CONNTRACK_ESTABLISHED]= "ESTAB",
>   [SCTP_CONNTRACK_SHUTDOWN_SENT]  = "SHUT-SNT",
>   [SCTP_CONNTRACK_SHUTDOWN_RECD]  = "SHUT-RCV",
>   [SCTP_CONNTRACK_SHUTDOWN_ACK_SENT] = "SHUT-ACK",
>   };
> 
> I really think that now they are more understandable (even CK-WAIT), and no
> need to update man page or add help screen. I allocated 8 maximum chars
> for the "STATE" column.

I still think it might be worthwhile to document these states (or at
least their mappings to the corresponding libnetfilter values)
somewhere at some point. But that doesn't necessarily need to be part of
your current patch :)

> What do you think ?

Looks good to me.

Cheers
Tobias

-- 
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 netsniff-ng+unsubscr...@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.


[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 doc

[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",
> - [SCTP_CONNTRACK_ESTABLISHED]= "ESTABLISHED",
> - [SCTP_CONNTRACK_SHUT

[netsniff-ng] [ANNOUNCE] netsniff-ng 0.6.1

2016-03-22 Thread Tobias Klauser
It is our pleasure to announce the release of netsniff-ng 0.6.1.

As always, the summary of changes and the short log of all changes since
v0.6.0 can be found below.

Thanks to everybody who contributed to this release:

  Vadim Kochan, Thomas Fleischmann, Reiner Herrmann, Erik Bengtsson,
  and Daniel Borkmann.

This release also means that the tree is again open for new features.

Happy packet sniffing!

---

netsniff-ng 0.6.1 (aisatsana) has been released to the public.

It can be fetched via Git:

   git clone git://github.com/netsniff-ng/netsniff-ng.git
   git checkout v0.6.1

Or via HTTP from one of our mirrors:

   http://pub.netsniff-ng.org/netsniff-ng/netsniff-ng-0.6.1.tar.gz
   http://mirror.distanz.ch/netsniff-ng/netsniff-ng-0.6.1.tar.gz
   http://github.com/netsniff-ng/netsniff-ng/archive/v0.6.1.tar.gz

The release can be verified via Git (see README):

   git tag -v v0.6.1

Major high-level changes since the last release (v0.6.0) are:

1) Newly added trafgen configuration language which allows to specify
   the packets in a more convenient format. See trafgen(8) for more
   details on the supported protocols and keywords. Contributed by Vadim
   Kochan.

2) Additional runtime commands for flowtop which allow to toggle/filter
   display of flows. From Vadim Kochan.

3) Command line options to pass macro definitions to trafgen and bpfc.
   From Vadim Kochan.

4) Made the build of all netsniff-ng tools reproducible (stable link
   order). From Reiner Herrmann.

5) Fix download of GeoIP databases so the files don't get corrupted.
   From Tobias Klauser.

Contributions since last release:

 64  Vadim Kochan
 49  Tobias Klauser
  1  Thomas Fleischmann
  1  Reiner Herrmann
  1  Erik Bengtsson
  1  Daniel Borkmann

Git changelog since last release:

Vadim Kochan (64):
  build: configure: Check for libnl-route
  flowtop: Add runtime command to show help window
  flowtop: Add runtime command to change rate units
  flowtop: Add runtime command to show only active flows
  str: Add converting cmdline args vector to str module
  trafgen: Allow to build packet from command line
  netsniff-ng: Allow to specify compiled BPF from stdin
  bpfc, trafgen: Do not close stdin when "-" is specified
  bpfc: man: Add example how to filter rtnetlink by attributes
  flowtop: Add header line with tool name & version
  netsniff-ng: nlmsg: Print not dissected attribute type number
  netsniff-ng: nlmsg: Print generic netlink ctrl family info
  netsniff-ng: nlmsg: Check message length before dissect rtnl
  bpfc: Do not panic if bpf file is not valid
  flowtop: Add runtime commands to filter flows by proto
  cpp: Add cpp.c module to invoke C preprocessor
  bpfc: Invoke C preprocessor from cpp.c module
  trafgen: Invoke C preprocessor from cpp.c module
  cpp: Use /tmp folder for output files
  trafgen: Make sure yyin is set before close it
  netsniff-ng: nlmsg: Print genl ops & mcast groups attributes
  proc: Add function to execute process with argv list
  cpp: Use new proc_exec function to invoke cpp
  cpp: Allow to pass additional cpp options
  str: Add helper to extend dynamically argv list
  bpfc: Add option to pass macro/define for C preprocessor
  trafgen: Added option to pass macro/define for C preprocessor
  flowtop: Show selected proto family
  flowtop: Indicate if 'active' flows mode is selected
  flowtop: Refresh flows if filter was changed while flows loading
  trafgen: Add option to specify packets sending rate
  flowtop: Use single function to update flow entry
  flowtop: Use one nfct handle for dump & refresh flows
  trafgen: Move gap feature into shaper logic
  trafgen: Simplify 'gap' option unit parsing
  trafgen: Simplify ring size unit parsing
  dissectors: arp: Print hardware & protocol addresses
  trafgen: Export set_fill func
  trafgen: Add helper to get current packet
  trafgen: Add basic protocol generation logic
  dev: Add function to get device hardware address
  trafgen: proto: Add function to set field from device MAC
  trafgen: l2: Add Ethernet protocol header generation
  str: Add str2mac helper function
  trafgen: parser: Add syntax to generate Ethernet header fields
  trafgen: proto: Add functon to fill field with device ipv4 address
  trafgen: l2: Add ARP header generation logic
  trafgen: parser: Add syntax to generate ARP header fields
  trafgen: l3: Add IPv4 header generation backend
  trafgen: parser: Add syntax for IPv4 protocol generation
  trafgen: l4: Add UDP header generation logic
  trafgen: parser: Add syntax to build UDP header
  trafgen: man: Add help for Ethernet, ARP, IPv4, UDP headers
  trafgen: proto: Simplify getting lower protocol after init
  trafgen: proto: Add set_next_proto c

[netsniff-ng] Re: flowtop: Flows visual separating

2016-02-26 Thread Tobias Klauser
On 2016-02-25 at 17:35:56 +0100, Vadim Kochan  wrote:
> On Sat, Feb 20, 2016 at 8:47 PM, Vadim Kochan  wrote:
> > On Sat, Feb 20, 2016 at 7:49 PM, Daniel Borkmann  
> > wrote:
> >> Hi Vadim,
> >>
> >> thanks for looking into this, appreciate it!
> >>
> >> On 02/20/2016 03:28 PM, Vadim Kochan wrote:
> >>>
> >>> On Sat, Feb 20, 2016 at 1:25 AM, Vadim Kochan  wrote:
> 
>  Hi,
> 
>  I tried to come up with visual separating of printed flows as currently
>  its not easy to identify separate flow entry, so I did some changes and
>  I am not sure if it looks good so I atached the screenshot.
> 
>  Regards,
> >>>
> >>>
> >>> I attached another version of odd & even flows entries style, here I
> >>> used cyan & white colors and it seem looks
> >>> better as here is no such contrast like in case with black & white
> >>> background colors (like in previous example),
> >>> also here 'country' color changed to magenta as it looks better on
> >>> white & cyan background colors.
> >>
> >>
> >> Not particularly a fan of these background colors, but I understand
> >> you'd like to improve usability on this. How about making flowtop
> >> look and navigation more like top or htop? Perhaps some of this info
> >> can be collapsed?

Fully agree with Daniel, I'm not a big fan of too much background color
(or even color in general) in TUI interfaces either. I'd certainly
prefer if you'd go for a top/htop like interface in that case.

> >>
> >> Thanks,
> >> Daniel
> >
> > Well, if to follow these *top-like tools then we need to print less
> > info. Curently we print:
> >
> > 1) process name
> > 2) flow state
> > 3) application proto name
> > 4) duration time
> > 5) src/dst hostname
> > 6) geo info
> > 7) pkts/bytes stats (counters & rate)

top/htop allow you to select the columns to display. We could define a
sensible set of default columns (or even add additional ones in case we
detect a wide enough window) and then let the user add/remove other
columns.

> >
> > We can have 2 modes for flows visualization:
> >
> > 1) Short mode (1 row per entry) (default):
> >  a) process name
> >  b) flow state (but with shortest names)
> >  c) application proto name
> >  d) src/dst info hostnames (or only dst with country if it feets)
> >  e) mixed stats
> >
> > 2) Extended mode, like in current implementation
> > (but maybe be changed to color scheme which I sent in previous 
> > example).

I don't think it's necessary to have 2 modes if we go for selectable
columns.

> > Also there might be hot-keys to
> > 1) expand 1-row entry into 3-row mode
> > 2) switch between 1- & 3- row mode for all entries.

In case the user's window isn't wide enough to hold all columns, this
would be a nice option to display additional information.

Cheers
Tobias

-- 
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 netsniff-ng+unsubscr...@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.


[netsniff-ng] Re: [PATCH v2 0/4] trafgen: Add MPLS header creating

2016-02-23 Thread Tobias Klauser
On 2016-02-23 at 13:38:42 +0100, Vadim Kochan  wrote:
> Thanks!
> 
> So icmpv4 will appear in next version, I have patches but need to rebase them.

That'd be great if you could send them once the tree is open again, thanks.

-- 
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 netsniff-ng+unsubscr...@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.


[netsniff-ng] [ANNOUNCE] Tree closed for new features until v0.6.1 release

2016-02-23 Thread Tobias Klauser
As previously announced in the last release message [1], we slightly
changed our development model to do releases a bit more often and to
ditch the -rc releases.

[1] http://thread.gmane.org/gmane.linux.network.netsniff-ng/1258

In that vein, we'd like to close the tree for new features as of today,
which means we'll only take bug fixes and small, non-intrusive
fixes/cleanups we deem safe for inclusion in v0.6.1. The release is
planned in about two weeks time. Afterwards the tree is open again for
new features and more experimental changes.

We'd like to ask everybody who's interested in the netsniff-ng toolkit
to give the current git HEAD some good testing.

I'd especially like to point out the awesome new trafgen protocol
creation functions contributed by Vadim Kochan which make writing
trafgen config files for commonly used protocols a lot more convenient.
See the respective page section in trafgen(8) for the basic usage.

Thanks
Tobias

-- 
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 netsniff-ng+unsubscr...@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.


signature.asc
Description: Digital signature


[netsniff-ng] Re: [PATCH v2 0/4] trafgen: Add MPLS header creating

2016-02-23 Thread Tobias Klauser
On 2016-02-08 at 07:01:50 +0100, Vadim Kochan  wrote:
> Add 'mpls()' function to create MPLS header with fields:
> 
> Label, TClass, Bottom-Stack, TTL
> 
> By default EtherType is set to MPLS Unicast (0x8847).
> Added man changes as well. By default bottom-stack (S-flag) is
> set to 1 but resets to 0 after the lower MPLS was added. As future
> extensions for 'mpls()' function might be:
> 
> 1) Allow to mark MPLS header as multicast via setting EtherType to 0x8848 
> like:
>
>   mpls(mc) or mpls(mcast)
> 
> 2) Add parameters for specific label values:
> 
>   mpls(alert)
> 
> Also fixed issue with incorrect field bit-masking which uses only
> OR to merge specified & original value which does not allow
> to set 0 value, now the original value is AND-ed with reversed field mask, so
> the required bits are reset to 0s in original value and then OR-ed with
> specified value. The problem actually apperead while setting S-flag to 0
> when the lower MPLS header was added.
> 
> Small additional fixes in trafgen.8 file like:
> 
> 1) Changed ip -> ipv4 in UDP Echo example
> 
> 2) Add line break for VLAN section in sentence about EtherType.
> 
> v2:
> Add 'exp' parameter which is the same as 'tc|tclass'.
> 
> Vadim Kochan (4):
>   trafgen: proto: Fix bad field masking
>   trafgen: l2: Add MPLS header generation
>   trafgen: parser: Add syntax for MPLS header creating
>   trafgen: man: Add description for 'mpls()' function

Sorry for the delay, the series is now applied. Thanks Vadim!

Please note that I'd like to close the tree for new features today and
I'll only take bug fixes (and small non-intrusive fixes Daniel or I deem
safe for inclusion) for the next release. The release of v0.6.1 is
planned in about two weeks. I'll also send an official message about the
tree being closed to the mailing list, as previously discussed :)

Thanks

-- 
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 netsniff-ng+unsubscr...@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.


[netsniff-ng] Re: [PATCH v2 0/4] trafgen: Add MPLS header creating

2016-02-15 Thread Tobias Klauser
Hi Vadim

Thanks a lot for the patches. As I'll still be offline occassionally
during this week, I'll probably only get around reviewing them by the
end of this week.

Thanks
Tobias

-- 
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 netsniff-ng+unsubscr...@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.


[netsniff-ng] Re: [PATCH 0/7] trafgen: Add VLAN header creating

2016-02-03 Thread Tobias Klauser
On 2016-02-02 at 17:27:26 +0100, Vadim Kochan  wrote:
> On Tue, Feb 2, 2016 at 6:20 PM, Vadim Kochan  wrote:
> > On Tue, Feb 2, 2016 at 5:54 PM, Tobias Klauser  wrote:
> >> On 2016-02-01 at 18:01:34 +0100, Vadim Kochan  wrote:
> >>> Add 'vlan()' function for creating VLAN header 802.1q or 802.1ad.
> >>> There is a tricky change which changes struct proto_field.offset
> >>> uint16_t -> int16_t to allow negative values which is used in VLAN
> >>> protocol header by using -2 offset for TPID field which overlaps with
> >>> ether type field.
> >>>
> >>> Additionally series has changes for simplify setting correct upper 
> >>> protocol
> >>> id by lower layer by delegating this to the lower protocol. So upper 
> >>> protocol
> >>> specifies it's proto_id to the lower protocol which decides which field & 
> >>> value
> >>> to set.
> >>>
> >>> Vadim Kochan (7):
> >>>   trafgen: proto: Simplify getting lower protocol after init
> >>>   trafgen: proto: Add set_next_proto callback to struct proto_hdr
> >>>   trafgen: eth: Add setting next protocol id
> >>>   trafgen: ipv4: Add setting next protocol id
> >>>   trafgen: l2: Add VLAN header generation
> >>>   trafgen: parser: Add syntax for VLAN header creating
> >>>   trafgen: man: Add help for VLAN header function
> >>
> >> Series applied, thanks Vadim!
> >>
> >> Please note that I took out the VLAN ID setting as a pure number. I
> >> think it is a bit inconsistent with the rest of the functions and typing
> >> an additional 3 characters ('id=') shouldn't hurt too much and IMO also
> >> helps keep the configuration fields more readable. Hope that's OK with
> >> you.
> >
> > Well, I just was thinking that it would be good to have simple form of
> > adding VLAN header 'vlan(1)'
> > This form might relate to any small header where is some main numeric
> > parameter (e.g mpls).
> >
> > And really I think that 'vlan(100)' is enough readable and understandable.
> >
> > Regards,
> > Vadim Kochan
> 
> BTW, let me know if it would be helpful for you to have remote branch
> with changes on github too aside with patches via email ?

I for myself prefer to receive them by e-mail as this also allows Daniel
and other developers to comment on them and it is more consistent with
my workflow and I already have a bunch of scripts to integrate patches
into an own branch in my repo.

Thanks

-- 
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 netsniff-ng+unsubscr...@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.


[netsniff-ng] Re: trafgen: Automatic padding for Ethernet header

2016-02-03 Thread Tobias Klauser
Hi Vadim

On 2016-02-02 at 20:26:34 +0100, Vadim Kochan  wrote:
> I am thinking if it is OK to add automatic padding for Ethernet frames
> by filling 0s if packet len is less than 64 bytes ?

I don't think this is needed. Users might want to send explicitly send
less than 64 bytes on purpose (i.e. to test whether padding works
correct in their newly developed network driver). IMO they shouldn't be
required to disable padding in that case.

> If yes, then also there should be function 'nopad()' which will disable
> padding.
> 
> Of course there is still possibility to use 'fill()' function for this,
> but may be it would be better to do it automatically to be sure that
> packet will be sent successfully.

Yes, I'd say having a fill() function around is enough in that case.

> But if Linux guarantee such padding then it is not needed at all.

Yes, the Linux network stack will pad the frames to the length required
by the underlying medium.

Tobias

-- 
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 netsniff-ng+unsubscr...@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.


[netsniff-ng] Re: [PATCH 0/7] trafgen: Add VLAN header creating

2016-02-02 Thread Tobias Klauser
On 2016-02-01 at 18:01:34 +0100, Vadim Kochan  wrote:
> Add 'vlan()' function for creating VLAN header 802.1q or 802.1ad.
> There is a tricky change which changes struct proto_field.offset
> uint16_t -> int16_t to allow negative values which is used in VLAN
> protocol header by using -2 offset for TPID field which overlaps with
> ether type field.
> 
> Additionally series has changes for simplify setting correct upper protocol
> id by lower layer by delegating this to the lower protocol. So upper protocol
> specifies it's proto_id to the lower protocol which decides which field & 
> value
> to set.
> 
> Vadim Kochan (7):
>   trafgen: proto: Simplify getting lower protocol after init
>   trafgen: proto: Add set_next_proto callback to struct proto_hdr
>   trafgen: eth: Add setting next protocol id
>   trafgen: ipv4: Add setting next protocol id
>   trafgen: l2: Add VLAN header generation
>   trafgen: parser: Add syntax for VLAN header creating
>   trafgen: man: Add help for VLAN header function

Series applied, thanks Vadim!

Please note that I took out the VLAN ID setting as a pure number. I
think it is a bit inconsistent with the rest of the functions and typing
an additional 3 characters ('id=') shouldn't hurt too much and IMO also
helps keep the configuration fields more readable. Hope that's OK with
you.

-- 
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 netsniff-ng+unsubscr...@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.


Re: [netsniff-ng] Replay pcap file on Xenomai kernel in real time

2016-02-01 Thread Tobias Klauser
On 2016-02-01 at 15:38:28 +0100, Umair Ali  wrote:
> Thanks a lot for the help offer. I can understand what you want to say. 
> 
> I need your feedback on the code which I have written. The flow of the code 
> is as follows
> - Reader the pcap file. the pcap file contained the captured Sampled Values 
> packets (IEC 61850 9-2 SV, Datalink layer lever)
> - The length of each packet is fixed which is 126 bytes and pcap only contain 
> SV packets.
> - The code below is opening pcap file and then mapping the file and closing 
> the pcap file
> - the Variable packet_index is used to point the start of every packet
> 
> It is very small code and it will few minutes to read. Please find the 
> attached file. I will appreciate your feedback and points of suggestions. 
> Thanks a lot once again and sorry for bugging you too much.

Sorry, but reviewing a random dump of code (that doesn't even compile)
goes beyond the scope of what I'd consider support/help for netsniff-ng.

As said, I'm happy to help with any problems you encounter specific to
the netsniff-ng toolkit or help you get patches integrated that make
netsniff-ng work better in your environment/application.

Good luck
Tobias

-- 
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 netsniff-ng+unsubscr...@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.


Re: [netsniff-ng] Replay pcap file on Xenomai kernel in real time

2016-02-01 Thread Tobias Klauser
Hi Ali

On 2016-02-01 at 11:39:38 +0100, Umair Ali  wrote:
> Hello Tobias,
> 
> I have read the read_pcap function. I am not expert in C just a beginner. I 
> have understand the basic flow from the read_pcap function but still not 
> fully. I am using the real time kernel Xenomai 3.0.1 and it uses the real 
> time Ethernet driver and hence the name of the Ethernet interface is 'rteth'. 
> If you read this link 
> https://xenomai.org/2014/08/porting-a-linux-application-to-xenomai-dual-kernel/
>  which explains that how I can port the Linux application to the xenomai. As 
> I am not expert therefore I am asking you that can I port the netsniff-ng 
> tool to xenomai. I can ask the xenomai expert but you have build this tool 
> and you can easily understand therefore I asked you. Or can I send the packet 
> after 10 盜ecs in non-real time Linux. Can you provide me the separate code of 
> reading the pcap file so I can run on xenomai. Sorry for being asking so many 
> questions.

I'm not really familiar with Xenomai (or other RT Linux systems for that
matter), so I can't really tell you what changes (if any) are needed to
have netsniff-ng running on such a system. In case you see any changes
necessary in order to support running netsniff-ng on Xenomai, feel free
to send patches and/or change requests.

As for the pcap reading code: It's all in the sources, so you can dig it
up from there. If you have specific questions about it, I'll happily try
to answer them. But I'm afraid, I won't be able to guide you step by
step through the code nor provide you with example code (that goes
beyond what's already in the netsniff-ng sources) specific to your use
case.

Regards
Tobias

-- 
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 netsniff-ng+unsubscr...@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.


Re: [netsniff-ng] Synchronize file creation between netsniff-ng processes possible?

2016-01-29 Thread Tobias Klauser
On 2016-01-28 at 22:04:45 +0100, e.bengts...@gmail.com  
wrote:
> Sounds good. Thanks!

Patch now applied, thanks a lot Erik!

Tobias

-- 
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 netsniff-ng+unsubscr...@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.


[netsniff-ng] Re: [PATCH 0/4] trafgen: Add IPv4 and UDP protocol generation

2016-01-29 Thread Tobias Klauser
On 2016-01-29 at 09:05:24 +0100, Vadim Kochan  wrote:
> On Fri, Jan 29, 2016 at 08:48:59AM +0100, Tobias Klauser wrote:
> > On 2016-01-28 at 23:06:23 +0100, Vadim Kochan  wrote:
> > > Reworded commit message of 12-14 patches from series:
> > > 
> > > "[PATCH v3 00/16] trafgen: Add proto header generation"
> > > 
> > > 1) Added parameters & default values description.
> > > 2) Functionality was not changed.
> > 
> > Perfect, thanks a lot! Series now applied. I also took the manpage patch
> > from your previous series and I'll directly fold in my few minor changes.
> 
> BTW,
> 
> 1) I think that few more protocol header functions might be added (VLAN
>& TCP) before release (btw when do you plan do make release ?).

IPv6 would also be nice. But I think they're now fairly easy to add with
the existing infrastructure.

As for the release. I'd like to give the current changes a few weeks to
get some testing by others. As I'll be offline for some days beginning
of and mid February, I'd like to target a release sometime around end of
February, which means the tree will close for new features ~2 weeks
before the release.

> 2) I just realized that currently protocol functions are used at packet
>compile time and checksum's will be not re-calculated if dynamic
>functions (dinc,drand) changed some of the header or payload bytes.
>So as future improvement it needs to add some runtime logic for
>protocol fuctions. It will be needed also if to extend protocol
>functions with dynamic values too, like:
> 
>{ ip(sa=192.168.1.0/24) }
>{ udp(sp=2000...3000) }
> 
>really I don't know yet how to implement such syntax but this is just
>for future thinking.

Yes, supporting recalculation of checksums would certainly be nice and
shouldn't be that hard to implement. Would be nice to get this in before
release...

As for the "dynamic values" you propose above. What is the expected
behavior of this? Would you generate multiple packets (255 and 1000 in
the above examples)? Do you see a use case for this or shouldn't this
better be done by preprocessing the trafgen config file with a hand
crafted script?

> 3) Also as I mentioned in '... Xenomai ...' thread, what about idea to
>extend trfagen for altering ingress packets via protocol functions ?
>So currently I see it that in ingress mode protocol functions will
>change only parameters which were specified, the default will be
>ignored. Again I am not sure how useful it might be.

How is this related to Xenomai?

Well, trafgen currently doesn't support ingress packets. Or did you mean
netsniff-ng? In any case, I think this will be quite hard to get right
without having to implement a lot of protocol parsing logic (which of
course could be partially reused from the dissectors). Also you could
only do it on a per-packet level instead of i.e. per flow. I'd like to
see a clear benefit over kernel-level forwarding/packet-mangling
facilites for this feature before attempting to add it.

-- 
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 netsniff-ng+unsubscr...@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.


[netsniff-ng] Re: [PATCH 0/4] trafgen: Add IPv4 and UDP protocol generation

2016-01-28 Thread Tobias Klauser
On 2016-01-28 at 23:06:23 +0100, Vadim Kochan  wrote:
> Reworded commit message of 12-14 patches from series:
> 
> "[PATCH v3 00/16] trafgen: Add proto header generation"
> 
> 1) Added parameters & default values description.
> 2) Functionality was not changed.

Perfect, thanks a lot! Series now applied. I also took the manpage patch
from your previous series and I'll directly fold in my few minor changes.

-- 
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 netsniff-ng+unsubscr...@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.


Re: [netsniff-ng] Synchronize file creation between netsniff-ng processes possible?

2016-01-28 Thread Tobias Klauser
On 2016-01-28 at 13:17:30 +0100, e.bengts...@gmail.com  
wrote:
> 
> 
> Den onsdag 27 januari 2016 kl. 18:02:23 UTC+1 skrev e.ben...@gmail.com:
> >
> >
> >
> > Den onsdag 27 januari 2016 kl. 16:17:46 UTC+1 skrev Tobias Klauser:
> >>
> >> Hi again 
> >>
> >> On 2016-01-27 at 16:10:30 +0100, Tobias Klauser  
> >> wrote: 
> >> > On 2016-01-27 at 14:40:55 +0100, Erik Bengtsson  
> >> wrote: 
> >> > > By "start time" I mean file creation time. It is totally possible to 
> >> post 
> >> > > process all log files and synchronize them using pcap timestamps, as 
> >> you 
> >> > > suggested, but I'm afraid that it will be less effective for us since 
> >> there 
> >> > > will be a huge amount of data to move around and process. 
> >> > 
> >> > Ah ok, I see. Thanks for clarifying. 
> >> > 
> >> > In that case, I'm afraid there currently is no method to synchronize 
> >> the file 
> >> > creation among multiple instances of netsniff-ng. As Daniel suggested, 
> >> > timerfd might be an option to implement a feature along these lines. If 
> >> > you want, feel free to have a look into it - patches are gladly 
> >> accepted 
> >> > :) 
> >>
> >> Something which I completely forgot about... 
> >>
> >> There might be an option (though a bit curde) to solve this using the 
> >> permature rotation caused by SIGHUP. You could set up a separate task 
> >> simultaneously sending a SIGHUP every minute to your netsniff-ng 
> >> processes. See commit 46289df6bc8f573bc01be4fb4aa93343ecc6d50a 
> >> ("netsniff-ng: Rotate pcap files prematurely on SIGHUP") for details. 
> >>
> >> Tobias 
> >>
> >
> > That sounds really interesting! :-)
> >
> > I ended up using signal USR2 since i didn't want to change how SIGHUP was 
> > used. When USR2 is received, the current time is saved and used when naming 
> > the next file (which is created when the next packet is received).
> >
> > A patch is included if you want to have a look.
> >
> > / Erik
> >  
> >
> 
> I've continued working on this a bit and have a solution that seems to be 
> working. The patch is included if someone wants to have a look.

Nice, thanks a lot. I think we don't even need to have a command line
option for this but could just make it the default behavior to record
the timestamp of the sighup in the filename. The file creation time will
still reflect the actual time of when the first packet arrived.

I'll adjust the patch accordingly and commit it with your Signed-off-by,
if that's ok with you.

Thanks
Tobias

-- 
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 netsniff-ng+unsubscr...@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.


[netsniff-ng] Re: [PATCH v3 00/16] trafgen: Add proto header generation

2016-01-28 Thread Tobias Klauser
On 2016-01-26 at 21:24:56 +0100, Vadim Kochan  wrote:
> Add new trafgen proto generation framework which allows to describe
> proto header fields and easy build the proto header by set/get proto
> header fields value.
> 
> Meanwhile implemented Ethernet, ARP, IPv4 & UDP proto headers generation,
> with fixed header size. Each proto has its own syntax rules
> to specify header field, but really looks similary:
> 
> { (=,, =) }
> 
> Proto statement might be combined with other packet funcs like:
> 
> { ip4(mf, proto=0x1), fill(0xff, 100) }
> 
> Each proto generates some default header so it is not possible to fill packet
> only with some set of header fields and rest - via fill(..) func.

I applied patches 1-11 from this series now. There were still a few
edges which I want to have a deeper look at, but any changes there can
be done in follow-up patches by myself.

As for patches 12-16: I didn't apply them because I'd like to see the
commit messages for the parser syntax parts (patches  a bit extended. I didn't 
do
this myself due to lack of time...

Please have a look at the amended commit messages for patches 8 and 11
I edited them quite heavily to include documentation for all supported
keywords, including default values. Please resend the patches for IPv4
and UDP syntax with similar commit messages.

For commit messages in general - and also for user visible messages and
documentation - please don't use abbreviations such as 'func' or 'proto'
but always spell out the full word. This makes it much easier to read
IMO.

As for the manpage part, I'll respond with detailed review comments
there, but in general it looks very good.

Thanks a lot!

-- 
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 netsniff-ng+unsubscr...@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.


Re: [netsniff-ng] Synchronize file creation between netsniff-ng processes possible?

2016-01-27 Thread Tobias Klauser
Hi again

On 2016-01-27 at 16:10:30 +0100, Tobias Klauser  wrote:
> On 2016-01-27 at 14:40:55 +0100, Erik Bengtsson  wrote:
> > By "start time" I mean file creation time. It is totally possible to post
> > process all log files and synchronize them using pcap timestamps, as you
> > suggested, but I'm afraid that it will be less effective for us since there
> > will be a huge amount of data to move around and process.
> 
> Ah ok, I see. Thanks for clarifying.
> 
> In that case, I'm afraid there currently is no method to synchronize the file
> creation among multiple instances of netsniff-ng. As Daniel suggested,
> timerfd might be an option to implement a feature along these lines. If
> you want, feel free to have a look into it - patches are gladly accepted
> :)

Something which I completely forgot about...

There might be an option (though a bit curde) to solve this using the
permature rotation caused by SIGHUP. You could set up a separate task
simultaneously sending a SIGHUP every minute to your netsniff-ng
processes. See commit 46289df6bc8f573bc01be4fb4aa93343ecc6d50a
("netsniff-ng: Rotate pcap files prematurely on SIGHUP") for details.

Tobias

-- 
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 netsniff-ng+unsubscr...@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.


Re: [netsniff-ng] Replay pcap file on Xenomai kernel in real time

2016-01-27 Thread Tobias Klauser
On 2016-01-27 at 15:15:01 +0100, Umair Ali  wrote:
> Hi Tobias,
> 
> Thanks for the quick reply. I have read pcap_mm.c file but cannot understand 
> the flow of the code. Can you explain me the flow that once the pcap file is 
> open using mmap then how it is further processed to extract packet by packet 
> and replay. Is it possible with netsniff-ng to send packet every 5micro secs 
> or less. 

The mmap pcap functions (like the scatter-gather and the standard file
i/o functions) are wired up in struct pcap_file_ops *pcap_ops and then
used by the respective functions in netsniff-ng.cx according to the pcap
access method is set in ctx->pcap (PCAP_OPS_MM in case of mmap).
read_pcap in netsniff-ng.c is probably most interesting to you.

HTH
Tobias

-- 
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 netsniff-ng+unsubscr...@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.


Re: [netsniff-ng] Replay pcap file on Xenomai kernel in real time

2016-01-27 Thread Tobias Klauser
[resending with Ali in Cc in case he's not subscribed to the list]

Hi Ali

On 2016-01-27 at 13:01:55 +0100, Umair Ali  wrote:
> Hello there,
> 
> I am working on the project of replaying the pcap files in the real time over 
> the network. For this purpose I am using the real time Linux kernel 'Xenomai 
> v 3.0.1'. My idea is to write a code in C which will read the pcap file 
> packet by packet and then send the packet as raw Ethernet packet over the 
> real time interface. When the pcap libraries are used on the xenomai then the 
> process of reading pcap files does not behave in real time any more. Moreover 
> the xenomai uses the mmap for reading the files in real time. I have tried to 
> use the mmap technique to read pcap file but it is not perfect and works for 
> small files. I have read the netsniff.ng tool uses the same mmap technique to 
> read the pcap file for replaying the pcap file. My question is that how mmap 
> is used in netsniff-ng tool to read the pcap file packet by packet. Can you 
> give me the C code as an example to read the pcap file using mmap packet by 
> packet. I shall be highly thankful.

Have a look at pcap_mm.c, the mmap base pcap read/write functions are
defined there.

Hope that helps
Tobias

-- 
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 netsniff-ng+unsubscr...@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.


Re: [netsniff-ng] Synchronize file creation between netsniff-ng processes possible?

2016-01-27 Thread Tobias Klauser
[resending with Erik in Cc in case he's not subscribed to the list]

Hi Erik

On 2016-01-26 at 11:12:04 +0100, e.bengts...@gmail.com  
wrote:
> Hi everyone!
> 
> I've googled a bit but not found any good answer to my question. Maybe you 
> can help me out?
> 
> Currently I'm running five netsniff processes capturing data from five 
> different interfaces. It works really well with the limiting factor SSD write 
> speed, not netsniff itself. To make post processing easier, it would be nice 
> if all log files were the same size and each set of files had the same start 
> time.
> 
> It seems like netsniff only checks file size / time when a packet is received 
> so the start time within a set of files drift more or less depending on the 
> busload.

What do you mean by start time? The time of the first packet in the
file? Or the file creation time?

The timestamps of the packets stem directly from the kernel and are set
at the time the packet arrives. But since these are all in the same
timebase (kernel time) in all the files you should be able to align
them, no?

As for the size, yes this is true. The file size is only checked
whenever a new packet arrives.

> Any ideas on how to synchronize the processes?

As outlined above, you should be able to synchronize after capturing
based on the pcap timestamps. Or didn't I understand your problem right?

Best
Tobias

-- 
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 netsniff-ng+unsubscr...@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.


Re: [netsniff-ng] Replay pcap file on Xenomai kernel in real time

2016-01-27 Thread Tobias Klauser
Hi Ali

On 2016-01-27 at 13:01:55 +0100, Umair Ali  wrote:
> Hello there,
> 
> I am working on the project of replaying the pcap files in the real time over 
> the network. For this purpose I am using the real time Linux kernel 'Xenomai 
> v 3.0.1'. My idea is to write a code in C which will read the pcap file 
> packet by packet and then send the packet as raw Ethernet packet over the 
> real time interface. When the pcap libraries are used on the xenomai then the 
> process of reading pcap files does not behave in real time any more. Moreover 
> the xenomai uses the mmap for reading the files in real time. I have tried to 
> use the mmap technique to read pcap file but it is not perfect and works for 
> small files. I have read the netsniff.ng tool uses the same mmap technique to 
> read the pcap file for replaying the pcap file. My question is that how mmap 
> is used in netsniff-ng tool to read the pcap file packet by packet. Can you 
> give me the C code as an example to read the pcap file using mmap packet by 
> packet. I shall be highly thankful.

Have a look at pcap_mm.c, the mmap base pcap read/write functions are
defined there.

Hope that helps
Tobias

-- 
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 netsniff-ng+unsubscr...@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.


Re: [netsniff-ng] Synchronize file creation between netsniff-ng processes possible?

2016-01-27 Thread Tobias Klauser
Hi Erik

On 2016-01-26 at 11:12:04 +0100, e.bengts...@gmail.com  
wrote:
> Hi everyone!
> 
> I've googled a bit but not found any good answer to my question. Maybe you 
> can help me out?
> 
> Currently I'm running five netsniff processes capturing data from five 
> different interfaces. It works really well with the limiting factor SSD write 
> speed, not netsniff itself. To make post processing easier, it would be nice 
> if all log files were the same size and each set of files had the same start 
> time.
> 
> It seems like netsniff only checks file size / time when a packet is received 
> so the start time within a set of files drift more or less depending on the 
> busload.

What do you mean by start time? The time of the first packet in the
file? Or the file creation time?

The timestamps of the packets stem directly from the kernel and are set
at the time the packet arrives. But since these are all in the same
timebase (kernel time) in all the files you should be able to align
them, no?

As for the size, yes this is true. The file size is only checked
whenever a new packet arrives.

> Any ideas on how to synchronize the processes?

As outlined above, you should be able to synchronize after capturing
based on the pcap timestamps. Or didn't I understand your problem right?

Best
Tobias

-- 
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 netsniff-ng+unsubscr...@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.


[netsniff-ng] Re: [PATCH] netsniff-ng: arp: Print HW & proto addresses

2016-01-26 Thread Tobias Klauser
On 2016-01-21 at 22:23:26 +0100, Vadim Kochan  wrote:
> In case if HW type is Ethernet and proto is IPv4 then
> print sender/target MAC & IP addresses.
> 
> Signed-off-by: Vadim Kochan 

Applied with minor modifications, thanks.

-- 
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 netsniff-ng+unsubscr...@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.


[netsniff-ng] Re: [PATCH v2 11/16] trafgen: parser: Add syntax to generate ARP header fields

2016-01-26 Thread Tobias Klauser
On 2016-01-26 at 09:47:32 +0100, Vadim Kochan  wrote:
> On Tue, Jan 26, 2016 at 10:25 AM, Tobias Klauser  wrote:
> > On 2016-01-26 at 00:11:53 +0100, Vadim Kochan  wrote:
> >> Add syntax to generate ARP header fields:
> >>
> >> { arp(op=req, sip=1.1.1.1, smac=11:22:33:44:55:66) }
> >> { arp() }
> >>
> >> Signed-off-by: Vadim Kochan 
> >> ---
> >>
> >>  %%
> >>
> >> @@ -107,7 +109,16 @@ mac  
> >> ({mac_hex}:{mac_hex}:{mac_hex}:{mac_hex}:{mac_hex}:{mac_hex})
> >>  "saddr"|"sa" { return K_SADDR; }
> >>  "prot"[o]?   { return K_PROT; }
> >>
> >
> > Shouldn't we allow to specify htype, ptype, hlen and plen as well (as a
> > user might want to set non-conforming values)?
> >
> Well meanwhile it was easier to implement for me the Ethernet-IPv4
> form of ARP (which is more generic and
> used by masezahn too if I am not mistaken), and it looks a little
> tricky to allow full-flexible ARP header crafting.
> I'd like to add such ability on later work, if I will not dig into
> mac80211 headers ... :-)

Ok, I see :-) But htpye/ptype should still be possible without having
dynamic header length, no?

> >> +"sha"|"smac" { return K_SHA; }
> >> +"spa"|"sip"  { return K_SPA; }
> >> +"tha"|"tmac" { return K_THA; }
> >> +"tpa"|"tip"  { return K_TPA; }
> >> +"req"{ return K_REQ; }
> >
> > Please add "request" as well.
> >
> Sure.
> 
> >>   ;
> >>
> >> +arp_proto
> >> + : arp '(' arp_param_list ')' { }
> >> + ;
> >> +
> >> +arp_param_list
> >> + : { }
> >> + | arp_field { }
> >> + | arp_field delimiter arp_param_list { }
> >> + ;
> >> +
> >> +arp_field
> >> + : K_OPER  skip_white '=' skip_white K_REQ
> >> + { proto_field_set_be16(hdr, ARP_OPER, ARPOP_REQUEST); }
> >> + | K_OPER  skip_white '=' skip_white K_RESP
> >> + { proto_field_set_be16(hdr, ARP_OPER, ARPOP_REPLY); }
> >
> > Would be nice to allow numeric values here as well (again, to be able to
> > specify values not conforming to the standard).
> >
> 
> Right I will change it to the form:
> 
> arp(request, ...)
> arp(reply, ...)
> arp(op=)

Looks good.

> Not sure if the following is also will be needed considering above's forms:
> 
> arp(op=request)

I'd keep it in for consistency reasons.

-- 
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 netsniff-ng+unsubscr...@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.


[netsniff-ng] Re: [PATCH v2 15/16] trafgen: parser: Add syntax to build UDP header

2016-01-26 Thread Tobias Klauser
On 2016-01-26 at 00:11:57 +0100, Vadim Kochan  wrote:
> Added trafgen syntax to set UDP header fields:
> 
> { udp(sport=111, dport=222) }
> 
> Signed-off-by: Vadim Kochan 
> ---
>  trafgen_lexer.l  |  4 
>  trafgen_parser.y | 29 +
>  2 files changed, 33 insertions(+)
> 
> diff --git a/trafgen_lexer.l b/trafgen_lexer.l
> index 26876b4..98afc91 100644
> --- a/trafgen_lexer.l
> +++ b/trafgen_lexer.l
> @@ -131,9 +131,13 @@ ip_addr  ([0-9]+\.[0-9]+\.[0-9]+\.[0-9]+)
>  "df" { return K_DF; }
>  "mf" { return K_MF; }
>  
> +"sp"|"sport" { return K_SPORT; }
> +"dp"|"dport" { return K_DPORT; }

What about the length and checksum fields?

-- 
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 netsniff-ng+unsubscr...@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.


[netsniff-ng] Re: [PATCH v2 03/16] trafgen: Add basic proto generation logic

2016-01-26 Thread Tobias Klauser
On 2016-01-26 at 00:11:45 +0100, Vadim Kochan  wrote:
[...]
> +void proto_header_init(enum proto_id pid)
> +{
> + struct proto_hdr *hdr = proto_header_by_id(pid);
> + struct proto_hdr *new;
> +
> + if (headers_count >= PROTO_MAX_LAYERS)
> + panic("Too much proto headers\n");
> +
> + new = malloc(sizeof(*new));

Use xmalloc here, as we would otherwise dereference NULL.

-- 
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 netsniff-ng+unsubscr...@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.


[netsniff-ng] Re: [PATCH v2 11/16] trafgen: parser: Add syntax to generate ARP header fields

2016-01-26 Thread Tobias Klauser
On 2016-01-26 at 00:11:53 +0100, Vadim Kochan  wrote:
> Add syntax to generate ARP header fields:
> 
> { arp(op=req, sip=1.1.1.1, smac=11:22:33:44:55:66) }
> { arp() }
> 
> Signed-off-by: Vadim Kochan 
> ---
>  trafgen_lexer.l  | 15 +++
>  trafgen_parser.y | 38 +-
>  2 files changed, 52 insertions(+), 1 deletion(-)
> 
> diff --git a/trafgen_lexer.l b/trafgen_lexer.l
> index ac4fec1..e89d8c8 100644
> --- a/trafgen_lexer.l
> +++ b/trafgen_lexer.l
> @@ -15,6 +15,7 @@
>  #include 
>  #include 
>  #include 
> +#include 
>  
>  #include "trafgen_parser.tab.h"
>  #include "xmalloc.h"
> @@ -78,6 +79,7 @@ number_ascii([a-zA-Z])
>  
>  mac_hex  ([a-fA-F0-9]+)
>  mac  ({mac_hex}:{mac_hex}:{mac_hex}:{mac_hex}:{mac_hex}:{mac_hex})
> +ip_addr  ([0-9]+\.[0-9]+\.[0-9]+\.[0-9]+)
>  
>  %%
>  
> @@ -107,7 +109,16 @@ mac  
> ({mac_hex}:{mac_hex}:{mac_hex}:{mac_hex}:{mac_hex}:{mac_hex})
>  "saddr"|"sa" { return K_SADDR; }
>  "prot"[o]?   { return K_PROT; }
>  

Shouldn't we allow to specify htype, ptype, hlen and plen as well (as a
user might want to set non-conforming values)?

> +"sha"|"smac" { return K_SHA; }
> +"spa"|"sip"  { return K_SPA; }
> +"tha"|"tmac" { return K_THA; }
> +"tpa"|"tip"  { return K_TPA; }
> +"req"{ return K_REQ; }

Please add "request" as well.

> +"resp"|"reply"   { return K_RESP; }

I'd stick to the term "reply" as this is the one used in RFC 826 and
drop "resp" alltogether.

> +"op" { return K_OPER; }

Please add "oper" as well.

> +
>  "eth"{ return K_ETH; }
> +"arp"{ return K_ARP; }
>  
>  [ ]*"-"[ ]*  { return '-'; }
>  [ ]*"+"[ ]*  { return '+'; }
> @@ -161,6 +172,10 @@ mac  
> ({mac_hex}:{mac_hex}:{mac_hex}:{mac_hex}:{mac_hex}:{mac_hex})
>   panic("Failed to parse MAC addres %s\n", yytext);
> return mac; }
>  
> +{ip_addr}{ if (inet_pton(AF_INET, yytext, &yylval.ip_addr) != 1)
> + panic("Failed to parse IPv4 address %s\n", yytext);
> +   return ip_addr; };
> +
>  "'\\x"[a-fA-F0-9]{2}"'" { yylval.number = strtol(yytext + 3, NULL, 16);
> return number; }
>  
> diff --git a/trafgen_parser.y b/trafgen_parser.y
> index df1b1a6..5c9dcd9 100644
> --- a/trafgen_parser.y
> +++ b/trafgen_parser.y
> @@ -17,6 +17,8 @@
>  #include 
>  #include 
>  #include 
> +#include 
> +#include 
>  
>  #include "xmalloc.h"
>  #include "trafgen_parser.tab.h"
> @@ -337,6 +339,7 @@ static void proto_add(enum proto_id pid)
>  %}
>  
>  %union {
> + struct in_addr ip_addr;
>   long long int number;
>   uint8_t bytes[256];
>   char *str;
> @@ -346,15 +349,19 @@ static void proto_add(enum proto_id pid)
>  %token K_CPU K_CSUMIP K_CSUMUDP K_CSUMTCP K_CSUMUDP6 K_CSUMTCP6 K_CONST8 
> K_CONST16 K_CONST32 K_CONST64
>  
>  %token K_DADDR K_SADDR K_PROT
> +%token K_OPER K_SHA K_SPA K_THA K_TPA K_REQ K_RESP
> +
>  %token K_ETH
> +%token K_ARP
>  
>  %token ',' '{' '}' '(' ')' '[' ']' ':' '-' '+' '*' '/' '%' '&' '|' '<' '>' 
> '^'
>  
> -%token number string mac
> +%token number string mac ip_addr
>  
>  %type  number expression
>  %type  string
>  %type  mac
> +%type  ip_addr
>  
>  %left '-' '+' '*' '/' '%' '&' '|' '<' '>' '^'
>  
> @@ -566,6 +573,7 @@ ddec
>  
>  proto
>   : eth_proto { }
> + | arp_proto { }
>   ;
>  
>  eth_proto
> @@ -591,6 +599,34 @@ eth_field
>   { proto_field_set_be16(hdr, ETH_PROTO_ID, $5); }
>   ;
>  
> +arp_proto
> + : arp '(' arp_param_list ')' { }
> + ;
> +
> +arp_param_list
> + : { }
> + | arp_field { }
> + | arp_field delimiter arp_param_list { }
> + ;
> +
> +arp_field
> + : K_OPER  skip_white '=' skip_white K_REQ
> + { proto_field_set_be16(hdr, ARP_OPER, ARPOP_REQUEST); }
> + | K_OPER  skip_white '=' skip_white K_RESP
> + { proto_field_set_be16(hdr, ARP_OPER, ARPOP_REPLY); }

Would be nice to allow numeric values here as well (again, to be able to
specify values not conforming to the standard).

> + | K_SHA skip_white '=' skip_white mac
> + { proto_field_set_bytes(hdr, ARP_SHA, $5); }
> + | K_THA skip_white '=' skip_white mac
> + { proto_field_set_bytes(hdr, ARP_THA, $5); }
> + | K_SPA skip_white '=' skip_white ip_addr
> + { proto_field_set_u32(hdr, ARP_SPA, $5.s_addr); }
> + | K_TPA skip_white '=' skip_white ip_addr
> + { proto_field_set_u32(hdr, ARP_TPA, $5.s_addr); }
> + ;
> +arp
> + : K_ARP { proto_add(PROTO_ARP); }
> + ;
> +
>  %%
>  
>  static void finalize_packet(void)
> -- 
> 2.6.3
> 

-- 
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 netsniff-ng+unsubscr...@googlegroups.com.
For more options, visit https://groups.google.com/d/op

[netsniff-ng] Re: [PATCH 00/13] trafgen: Add proto header generation

2016-01-25 Thread Tobias Klauser
On 2016-01-25 at 10:06:04 +0100, Vadim Kochan  wrote:
> On Mon, Jan 25, 2016 at 09:56:37AM +0100, Tobias Klauser wrote:
> > On 2016-01-21 at 00:19:48 +0100, Vadim Kochan  wrote:
> > > Add new trafgen proto generation framework which allows to describe
> > > proto header fields and easy build the proto header by set/get proto
> > > header fields value.
> > 
> > This is great, thanks a lot for doing this! I only had the chance to
> > have a quick look at the series and couldn't find any major problems,
> > except for one thing: Please add a patch for trafgen.8, adding a section
> > which shortly describes the keywords, supported protocols, default
> > values for fields not specified etc. Having the user dig through git
> > commit messages or parser grammars to figure out how to use these
> > features is not very nice :)
> > 
> > Once you add that I think we can safely apply the series and fix any
> > remaining issues in follow-up patches. I'll also reply to some of the
> > patches with some minor comments which you might want to consider for v2
> > along with the changes you already announced.
> > 
> > Thanks!
> 
> BTW, I reworked to make possible to specify headers in any order and
> multiple times (which was not possible in previous version), so it
> allows to make such constructions like:
> 
> { ip(), ip(proto=0x1) }
> 
> which builds ip-in-ip header (including Ethernet). And now user
> responsible to specify the order of headers, but lower layers still will
> be added automatically if they were not specified by user.
> 
> Regarding man changes, well I did not include it because I am not good
> in English so to do not block this series in case of English issues I
> decided to work on it separately, but OK - I will try to document it.

I'm not a native English speaker/writer either, so no worries :) We can
still wordsmith on the documentation later on, or based on your patch.
The important part to me is that the keywords and the rules for protocol
default values are documented somewhere.

-- 
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 netsniff-ng+unsubscr...@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.


[netsniff-ng] Re: [PATCH 04/13] trafgen: Add basic proto generation logic

2016-01-25 Thread Tobias Klauser
On 2016-01-25 at 10:14:10 +0100, Vadim Kochan  wrote:
> On Mon, Jan 25, 2016 at 10:15:24AM +0100, Tobias Klauser wrote:
> > On 2016-01-21 at 00:19:52 +0100, Vadim Kochan  wrote:
> > > Add new trafgen_proto.c module with basic proto
> > > header fields generation logic.
> > > 
> > > Each proto must implement proto_gen struct and register it
> > > to the global proto list.
> > > 
> > > Proto header consist from set of fields, and each field must be
> > > described via proto_field struct by specifying unique id, len,
> > > offset (relative to the header). Small fields ( < 8 bits) can be
> > > described via left shift & mask.
> > > 
> > > On header_init required fields must be added to the packet and
> > > initialized with default values.
> > > 
> > > header_finish callback is invoked from upper to lower proto
> > > and some final calculations might be performed (total len, checksum).
> > > 
> > > Proto generation API provides easy proto field setters/getters to easy
> > > craft the packet via parser.
> > > 
> > > Signed-off-by: Vadim Kochan 
> > > ---
> > >  trafgen.c|   3 +
> > >  trafgen/Makefile |   1 +
> > >  trafgen_proto.c  | 288 
> > > +++
> > >  trafgen_proto.h  |  82 
> > >  4 files changed, 374 insertions(+)
> > >  create mode 100644 trafgen_proto.c
> > >  create mode 100644 trafgen_proto.h
> > > 
> > > diff --git a/trafgen.c b/trafgen.c
> > > index c74a973..949f909 100644
> > > --- a/trafgen.c
> > > +++ b/trafgen.c
> > > @@ -54,6 +54,7 @@
> > >  #include "timer.h"
> > >  #include "ring_tx.h"
> > >  #include "csum.h"
> > > +#include "trafgen_proto.h"
> > >  
> > >  #ifndef timeval_to_timespec
> > >  #define timeval_to_timespec(tv, ts) { \
> > > @@ -1215,6 +1216,8 @@ int main(int argc, char **argv)
> > >   register_signal(SIGTERM, signal_handler);
> > >   register_signal(SIGHUP, signal_handler);
> > >  
> > > + protos_init(ctx.device);
> > > +
> > >   if (prio_high) {
> > >   set_proc_prio(-20);
> > >   set_sched_status(SCHED_FIFO, 
> > > sched_get_priority_max(SCHED_FIFO));
> > > diff --git a/trafgen/Makefile b/trafgen/Makefile
> > > index bc256b2..2ea684f 100644
> > > --- a/trafgen/Makefile
> > > +++ b/trafgen/Makefile
> > > @@ -19,6 +19,7 @@ trafgen-objs =  xmalloc.o \
> > >   timer.o \
> > >   sysctl.o \
> > >   cpp.o \
> > > + trafgen_proto.o \
> > >   trafgen_lexer.yy.o \
> > >   trafgen_parser.tab.o \
> > >   trafgen.o
> > > diff --git a/trafgen_proto.c b/trafgen_proto.c
> > > new file mode 100644
> > > index 000..caf2685
> > > --- /dev/null
> > > +++ b/trafgen_proto.c
> > > @@ -0,0 +1,288 @@
> > > +/*
> > > + * netsniff-ng - the packet sniffing beast
> > > + * Subject to the GPL, version 2.
> > > + */
> > > +
> > > +#include 
> > > +#include 
> > > +
> > > +#include "xmalloc.h"
> > > +#include "trafgen_conf.h"
> > > +#include "trafgen_proto.h"
> > > +
> > > +#define field_shift_and_mask(f, v) (((v) << (f)->shift) & \
> > > + ((f)->mask ? (f)->mask : (0x)))
> > > +
> > > +#define field_unmask_and_unshift(f, v) (((v) & \
> > > + ((f)->mask ? (f)->mask : (0x))) >> (f)->shift)
> > > +
> > > +static struct proto_ctx ctx;
> > > +
> > > +#define PROTO_MAX_LAYERS 8
> > > +
> > > +static struct proto_gen *headers[PROTO_MAX_LAYERS];
> > > +static int headers_count;
> > > +
> > > +static struct proto_gen *protos;
> > > +
> > > +struct proto_gen *proto_get_by_id(enum proto_id id)
> > > +{
> > > + struct proto_gen *p = protos;
> > > +
> > > + for (; p; p = p->next)
> > > + if (p->id == id)
> > > + return p;
> > > +
> > > + panic("Can't lookup proto by id %u\n", id);
> > 
> > Why do panic here? Wouldn't it be better to return NULL and let the
> > callers handle it gracefully?
> 
> Well, just because it should not happen in normal case but only when
> adding new proto syntax (parser strictly specifies the proto id).

I see, makes sense to keep it in that case.

-- 
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 netsniff-ng+unsubscr...@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.


[netsniff-ng] Re: [PATCH 04/13] trafgen: Add basic proto generation logic

2016-01-25 Thread Tobias Klauser
On 2016-01-21 at 00:19:52 +0100, Vadim Kochan  wrote:
> Add new trafgen_proto.c module with basic proto
> header fields generation logic.
> 
> Each proto must implement proto_gen struct and register it
> to the global proto list.
> 
> Proto header consist from set of fields, and each field must be
> described via proto_field struct by specifying unique id, len,
> offset (relative to the header). Small fields ( < 8 bits) can be
> described via left shift & mask.
> 
> On header_init required fields must be added to the packet and
> initialized with default values.
> 
> header_finish callback is invoked from upper to lower proto
> and some final calculations might be performed (total len, checksum).
> 
> Proto generation API provides easy proto field setters/getters to easy
> craft the packet via parser.
> 
> Signed-off-by: Vadim Kochan 
> ---
>  trafgen.c|   3 +
>  trafgen/Makefile |   1 +
>  trafgen_proto.c  | 288 
> +++
>  trafgen_proto.h  |  82 
>  4 files changed, 374 insertions(+)
>  create mode 100644 trafgen_proto.c
>  create mode 100644 trafgen_proto.h
> 
> diff --git a/trafgen.c b/trafgen.c
> index c74a973..949f909 100644
> --- a/trafgen.c
> +++ b/trafgen.c
> @@ -54,6 +54,7 @@
>  #include "timer.h"
>  #include "ring_tx.h"
>  #include "csum.h"
> +#include "trafgen_proto.h"
>  
>  #ifndef timeval_to_timespec
>  #define timeval_to_timespec(tv, ts) { \
> @@ -1215,6 +1216,8 @@ int main(int argc, char **argv)
>   register_signal(SIGTERM, signal_handler);
>   register_signal(SIGHUP, signal_handler);
>  
> + protos_init(ctx.device);
> +
>   if (prio_high) {
>   set_proc_prio(-20);
>   set_sched_status(SCHED_FIFO, 
> sched_get_priority_max(SCHED_FIFO));
> diff --git a/trafgen/Makefile b/trafgen/Makefile
> index bc256b2..2ea684f 100644
> --- a/trafgen/Makefile
> +++ b/trafgen/Makefile
> @@ -19,6 +19,7 @@ trafgen-objs =  xmalloc.o \
>   timer.o \
>   sysctl.o \
>   cpp.o \
> + trafgen_proto.o \
>   trafgen_lexer.yy.o \
>   trafgen_parser.tab.o \
>   trafgen.o
> diff --git a/trafgen_proto.c b/trafgen_proto.c
> new file mode 100644
> index 000..caf2685
> --- /dev/null
> +++ b/trafgen_proto.c
> @@ -0,0 +1,288 @@
> +/*
> + * netsniff-ng - the packet sniffing beast
> + * Subject to the GPL, version 2.
> + */
> +
> +#include 
> +#include 
> +
> +#include "xmalloc.h"
> +#include "trafgen_conf.h"
> +#include "trafgen_proto.h"
> +
> +#define field_shift_and_mask(f, v) (((v) << (f)->shift) & \
> + ((f)->mask ? (f)->mask : (0x)))
> +
> +#define field_unmask_and_unshift(f, v) (((v) & \
> + ((f)->mask ? (f)->mask : (0x))) >> (f)->shift)
> +
> +static struct proto_ctx ctx;
> +
> +#define PROTO_MAX_LAYERS 8
> +
> +static struct proto_gen *headers[PROTO_MAX_LAYERS];
> +static int headers_count;
> +
> +static struct proto_gen *protos;
> +
> +struct proto_gen *proto_get_by_id(enum proto_id id)
> +{
> + struct proto_gen *p = protos;
> +
> + for (; p; p = p->next)
> + if (p->id == id)
> + return p;
> +
> + panic("Can't lookup proto by id %u\n", id);

Why do panic here? Wouldn't it be better to return NULL and let the
callers handle it gracefully?

> +}
> +
> +void proto_register(struct proto_gen *prot)
> +{
> + prot->next = protos;
> + protos = prot;
> +
> + prot->fields = NULL;
> + prot->fields_count = 0;
> +}
> +
> +static void proto_fields_realloc(struct proto_gen *prot, int count)
> +{
> + prot->fields = xrealloc(prot->fields, count * sizeof(*prot->fields));
> + prot->fields_count = count;
> +}
> +
> +void proto_fields_add(enum proto_id pid, struct proto_field *fields, int 
> count)
> +{
> + struct proto_gen *prot = proto_get_by_id(pid);
> + struct packet *pkt = current_packet();
> + struct proto_field *f;
> + int i;
> +
> + if (!prot->fields)
> + prot->pkt_offset = pkt->len;
> +
> + proto_fields_realloc(prot, prot->fields_count + count);
> +
> + for (i = 0; count >= 1; count--, i++) {
> + f = &prot->fields[prot->fields_count - count];
> +
> + f->id = fields[i].id;
> + f->len = fields[i].len;
> + f->is_set = false;
> + f->shift = fields[i].shift;
> + f->mask = fields[i].mask;
> + f->pkt_offset = prot->pkt_offset + fields[i].offset;
> +
> + if (f->pkt_offset + f->len > pkt->len)
> + set_fill(0, (f->pkt_offset + f->len) - pkt->len);
> + }
> +}
> +
> +static struct proto_field *proto_field_by_id(struct proto_gen *prot, int fid)
> +{
> + int i;
> +
> + for (i = 0; i < prot->fields_count; i++)
> + if (prot->fields[i].id == fid)
> + return &prot->fields[i];
> +
> + panic("Failed lookup field id %u for proto id

[netsniff-ng] Re: [PATCH 07/13] str: Add str2mac helper function

2016-01-25 Thread Tobias Klauser
On 2016-01-21 at 00:19:55 +0100, Vadim Kochan  wrote:
> Add func for convert string to MAC address.
> 
> Signed-off-by: Vadim Kochan 
> ---
>  str.c | 25 +
>  str.h |  1 +
>  2 files changed, 26 insertions(+)
> 
> diff --git a/str.c b/str.c
> index e4d8722..aca069a 100644
> --- a/str.c
> +++ b/str.c
> @@ -7,6 +7,7 @@
>  #include 
>  #include 
>  #include 
> +#include 
>  
>  #include "str.h"
>  #include "die.h"
> @@ -129,3 +130,27 @@ void argv_free(char **argv)
>  
>   free(tmp);
>  }
> +
> +int str2mac(char *str, uint8_t *mac)
   ^
   \-- should be const

-- 
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 netsniff-ng+unsubscr...@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.


[netsniff-ng] Re: [PATCH 10/13] trafgen: l2: Add ARP header generation logic

2016-01-25 Thread Tobias Klauser
On 2016-01-21 at 00:19:58 +0100, Vadim Kochan  wrote:
> Add ARP proto header fields generation via src mac, src ip,
> dst mac, dst ip & operaion.
> 
> By default Ethernet proto will be initialized, and ARP probe
> request is filled.
> 
> Signed-off-by: Vadim Kochan 
> ---
>  trafgen_l2.c | 45 +
>  trafgen_l2.h | 12 
>  2 files changed, 57 insertions(+)
> 
> diff --git a/trafgen_l2.c b/trafgen_l2.c
> index 5d245b8..adee113 100644
> --- a/trafgen_l2.c
> +++ b/trafgen_l2.c
> @@ -3,10 +3,15 @@
>   * Subject to the GPL, version 2.
>   */
>  
> +#include 
> +#include 
> +
>  #include "built_in.h"
>  #include "trafgen_l2.h"
>  #include "trafgen_proto.h"
>  
> +#define ETH_BCAST { 0xff, 0xff, 0xff, 0xff, 0xff, 0xff }
> +
>  struct proto_field eth_fields[] = {
>   { .id = ETH_DST_ADDR, .len = 6, },
>   { .id = ETH_SRC_ADDR, .len = 6, .offset = 6 },
> @@ -25,7 +30,47 @@ static struct proto_gen eth_prot = {
>   .header_init= eth_header_init,
>  };
>  
> +static struct proto_field arp_fields[] = {
> + { .id = ARP_HTYPE, .len = 2 },
> + { .id = ARP_PTYPE, .len = 2, .offset = 2 },
> + { .id = ARP_HLEN,  .len = 1, .offset = 4 },
> + { .id = ARP_PLEN,  .len = 1, .offset = 5 },
> + { .id = ARP_OPER,  .len = 2, .offset = 6 },
> + { .id = ARP_SHA,   .len = 6, .offset = 8 },
> + { .id = ARP_SPA,   .len = 4, .offset = 14 },
> + { .id = ARP_THA,   .len = 6, .offset = 18 },
> + { .id = ARP_TPA,   .len = 4, .offset = 24 },
> +};
> +
> +static void arp_header_init(struct proto_gen *prot)
> +{
> + uint8_t mac[6] = ETH_BCAST;
> +
> + proto_header_init(PROTO_ETH);
> +
> + proto_field_set_default_bytes(PROTO_ETH, ETH_DST_ADDR, mac);
> + proto_field_set_default_be16(PROTO_ETH, ETH_PROTO_ID, ETH_P_ARP);
> +
> + proto_fields_add(prot->id, arp_fields, array_size(arp_fields));
> +
> + /* Generate probe request by default */
> + proto_field_set_default_be16(prot->id, ARP_HTYPE, ARPHRD_ETHER);
> + proto_field_set_default_be16(prot->id, ARP_PTYPE, ETH_P_IP);
> + proto_field_set_default_u8(prot->id, ARP_HLEN, 6);
> + proto_field_set_default_u8(prot->id, ARP_PLEN, 4);
> + proto_field_set_default_be16(prot->id, ARP_OPER, ARPOP_REQUEST);
> +proto_field_set_default_dev_mac(prot->id, ARP_SHA);

Please use tab to indent here as well.

-- 
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 netsniff-ng+unsubscr...@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.


[netsniff-ng] Re: [PATCH 13/13] trafgen: parser: Add syntax for IPv4 proto

2016-01-25 Thread Tobias Klauser
On 2016-01-21 at 00:20:01 +0100, Vadim Kochan  wrote:
> Add syntax to specify IPv4 header fields:
> 
> { ip(df, mf, frag=100, prot=0x1, ecn=2, dscp=20) }
> 
> Signed-off-by: Vadim Kochan 
> ---
>  trafgen_lexer.l  | 15 +++
>  trafgen_parser.y | 51 +++
>  2 files changed, 66 insertions(+)
> 
> diff --git a/trafgen_lexer.l b/trafgen_lexer.l
> index 9bbd982..b9bcd10 100644
> --- a/trafgen_lexer.l
> +++ b/trafgen_lexer.l
> @@ -117,8 +117,23 @@ ip_addr  ([0-9]+\.[0-9]+\.[0-9]+\.[0-9]+)
>  "resp"|"reply"   { return F_RESP; }
>  "op" { return F_OPER; }
>  
> +"ihl"{ return F_IHL; }
> +"ver"{ return F_VER; }

Maybe add "version" here as well?

> +"ttl"{ return F_TTL; }
> +"dscp"   { return F_DSCP; }
> +"ecn"{ return F_ECN; }
> +"tos"{ return F_TOS; }
> +"len"{ return F_LEN; }

"length"?

> +"id" { return F_ID; }
> +"flags"  { return F_FLAGS; }
> +"frag"   { return F_FRAG; }
> +"csum"   { return F_CSUM; }
> +"df" { return F_DF; }
> +"mf" { return F_MF; }
> +
>  "eth"{ return P_ETH; }
>  "arp"{ return P_ARP; }
> +"ip" { return P_IP4; }

I think this should be "ip4" or "ipv4" (or both), in order to be
consistent with a possible future extension to support IPv6.

-- 
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 netsniff-ng+unsubscr...@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.


[netsniff-ng] Re: [PATCH 00/13] trafgen: Add proto header generation

2016-01-25 Thread Tobias Klauser
On 2016-01-21 at 00:19:48 +0100, Vadim Kochan  wrote:
> Add new trafgen proto generation framework which allows to describe
> proto header fields and easy build the proto header by set/get proto
> header fields value.

This is great, thanks a lot for doing this! I only had the chance to
have a quick look at the series and couldn't find any major problems,
except for one thing: Please add a patch for trafgen.8, adding a section
which shortly describes the keywords, supported protocols, default
values for fields not specified etc. Having the user dig through git
commit messages or parser grammars to figure out how to use these
features is not very nice :)

Once you add that I think we can safely apply the series and fix any
remaining issues in follow-up patches. I'll also reply to some of the
patches with some minor comments which you might want to consider for v2
along with the changes you already announced.

Thanks!

-- 
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 netsniff-ng+unsubscr...@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.


[netsniff-ng] Re: [PATCH 0/2] trafgen: Small changes for unit parsing (gap, ring size)

2016-01-14 Thread Tobias Klauser
On 2016-01-14 at 00:22:39 +0100, Vadim Kochan  wrote:
> Small simplification of unit parsing for gap & ring size option by using
> strtoul for setting start of unit name instead of checking it char by char.
> 
> Vadim Kochan (2):
>   trafgen: Simplify 'gap' option unit parsing
>   trafgen: Simplify ring size unit parsing

Series applied, thanks Vadim.

-- 
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 netsniff-ng+unsubscr...@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.


[netsniff-ng] Re: [PATCH] trafgen: Move gap feature into shaper logic

2016-01-12 Thread Tobias Klauser
On 2016-01-07 at 07:34:46 +0100, Vadim Kochan  wrote:
> Move gap feature into rate shaper, as these features
> means the same - delay the packet sending.
> 
> Signed-off-by: Vadim Kochan 

Applied, thanks.

-- 
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 netsniff-ng+unsubscr...@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.


[netsniff-ng] Re: [PATCH 1/2] flowtop: Use one func to update flow entry

2016-01-11 Thread Tobias Klauser
On 2016-01-05 at 20:42:39 +0100, Vadim Kochan  wrote:
> Seems there is no need to have 2 separate handlers
> for the flow updating, so use one which was used
> for flow refreshing. Significant change is that new entry
> will be not added if it was not found in the list, but such
> case should not happen.

The sentence above sounds rather strange by itself. It would be nice to
explain _why_ this is the case (i.e the NFCT_T_NEW event).

Otherwise the patch looks good to me.

Applied with an updated description.

-- 
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 netsniff-ng+unsubscr...@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.


[netsniff-ng] Re: [PATCH 2/2] flowtop: Use one nfct handle for dump & refresh flows

2016-01-11 Thread Tobias Klauser
On 2016-01-05 at 20:42:40 +0100, Vadim Kochan  wrote:
> Simplify dump & flows refreshing via one nfct handle, which is enough.
> 
> Signed-off-by: Vadim Kochan 

Applied, thanks.

-- 
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 netsniff-ng+unsubscr...@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.


[netsniff-ng] Re: [PATCH v2] trafgen: Add option to specify packets sending rate

2016-01-05 Thread Tobias Klauser
On 2015-12-28 at 10:19:12 +0100, Vadim Kochan  wrote:
> On Fri, Dec 25, 2015 at 03:19:59AM +0200, Vadim Kochan wrote:
> > On Wed, Dec 23, 2015 at 10:58:32PM +0200, Vadim Kochan wrote:
> > > On Wed, Dec 23, 2015 at 10:31:23PM +0200, Vadim Kochan wrote:
> > > > Added -b,--rate option in units of:
> > > > 
> > > > pps/B/kB/MB/kBit/Mbit/Gbit/KiB/MiB/GiB
> > > > 
> > > > to specify rate at which packets will be sent.
> > > > Similary to -t,--gap option the packets will be sent
> > > > in slow mode with 1 CPU.
> > > > 
> > > > Tested with ifpps.
> > > > 
> > > > Signed-off-by: Vadim Kochan 

[...]

> Sorry, may be I confused you, but I think this patch seems OK, you can
> ignore my message that it does not work for mtu > 1500 as I was
> performing wrong tests where rate was much less than packet len. So you
> can review it.

Ok, patch applied now. Thanks!

-- 
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 netsniff-ng+unsubscr...@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.


[netsniff-ng] Re: [PATCH 0/3] flowtop: Misc updates: filter state, refresh flag reset

2015-12-21 Thread Tobias Klauser
On 2015-12-16 at 21:12:06 +0100, Vadim Kochan  wrote:
> There are just few different changes like:
> 1) Show family in filter status bar
> 2) Show if 'Active' flows mode is selected in filter status bar
> 3) Reset do_refresh flag immideately if it is enabled to make able
>refresh flows again if this flag was changed while refreshing.
> 
> Vadim Kochan (3):
>   flowtop: Show selected proto family
>   flowtop: Indicate if 'active' flows mode is selected
>   flowtop: Refresh flows if filter was changed while flows loading

Series applied, thanks Vadim!

-- 
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 netsniff-ng+unsubscr...@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.


[netsniff-ng] Re: [PATCH v4 1/6] proc: Add function to execute process with argv list

2015-12-17 Thread Tobias Klauser
On 2015-12-15 at 22:09:10 +0100, Vadim Kochan  wrote:
> Add proc_exec function which executes given process with
> argv list via fork + execvp.
> 
> It allows to replace 'system' call approach which is used
> for invoking cpp and securely extend it with additional options
> like -D.
> 
> Signed-off-by: Vadim Kochan 

Series applied with some minor modifications (added const qualifiers
where applicable, changed some data types).

Thanks a lot, this is nice work!

-- 
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 netsniff-ng+unsubscr...@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.


[netsniff-ng] Re: [PATCH v4 1/6] proc: Add function to execute process with argv list

2015-12-16 Thread Tobias Klauser
On 2015-12-16 at 10:05:33 +0100, vkochan  wrote:
> Hi,
> 
> On Wed, Dec 16, 2015 at 09:34:28AM +0100, Tobias Klauser wrote:
> > On 2015-12-15 at 22:09:10 +0100, Vadim Kochan  wrote:
> > > Add proc_exec function which executes given process with
> > > argv list via fork + execvp.
> > > 
> > > It allows to replace 'system' call approach which is used
> > > for invoking cpp and securely extend it with additional options
> > > like -D.
> > > 
> > > Signed-off-by: Vadim Kochan 
> > > ---
> > 
> > For future patch series where you send multiple version, please add a
> > small changelog here (below the ---) of what changed between the
> > versions. Otherwise it's more cumbersome to determine whether something
> > has changed in the patch at all and needs a new review. This means
> > review time will be larger for the maintainers :(
> 
> Usually I do this but I was thinking that maintainer will review the
> whole submitted changes, and may be this changelog might be useless ...
> actually I assumed that maintainer will do not trust the changelog but
> will go review each line again.

Not necessarily. I want to avoid duplicating work, so I usually only
to a full review on patches which were actually changed and check
whether review comments have been addressed. Having a changelog helps to
quickly identify whether the patch author thinks they have adressed all
review comments with this.

> > For larger patch series, please also send along a cover letter (git
> > format-patch --cover-letter) where you give a short summary of the
> > overall idea of the series. In this case, adding the version changelog
> > to the cover letter is preferred.
> 
> Really I was doing that but I observed that you responded only to each
> patch so I decided that may be you skip the cover letter.

Sure, I only respond to the patches themselves, but the cover letter is
still useful. Please note that me not replying to the cover letter does not
imply I didn't read it :), I'd reply to the cover letter in case I
applied the series as a whole or if I had change requests to the overall
idea of the series.

Thanks

-- 
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 netsniff-ng+unsubscr...@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.


[netsniff-ng] Re: [PATCH v4 1/6] proc: Add function to execute process with argv list

2015-12-16 Thread Tobias Klauser
On 2015-12-15 at 22:09:10 +0100, Vadim Kochan  wrote:
> Add proc_exec function which executes given process with
> argv list via fork + execvp.
> 
> It allows to replace 'system' call approach which is used
> for invoking cpp and securely extend it with additional options
> like -D.
> 
> Signed-off-by: Vadim Kochan 
> ---

For future patch series where you send multiple version, please add a
small changelog here (below the ---) of what changed between the
versions. Otherwise it's more cumbersome to determine whether something
has changed in the patch at all and needs a new review. This means
review time will be larger for the maintainers :(

For larger patch series, please also send along a cover letter (git
format-patch --cover-letter) where you give a short summary of the
overall idea of the series. In this case, adding the version changelog
to the cover letter is preferred.

There's no need to do this for this patch series, please just keep in
mind for future patches. I'll review this series as soon as I find some
time.

Thanks

-- 
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 netsniff-ng+unsubscr...@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.


[netsniff-ng] Re: [PATCH v2] bpfc, trafgen: Allow to pass C preprocessor macro/define

2015-12-08 Thread Tobias Klauser
On 2015-12-01 at 23:04:18 +0100, Vadim Kochan  wrote:
> Add -D option for bpfc & trafgen to make possible pass -Dkey=value
> to C preprocessor. Characters like .?*()[]-^|`!#$&<>;\ are quoted
> to do not trick with shell's special symbols (e.g. process spawning),
> \ is ignored only if it goes before " - to make possible pass escaped
> text for trafgen's conf file.

Please split this up into at least 4 patches to make review easier:

1) string list helper
1) helpers to create/check/... argument list (cpp module)
2) trafgen integration
3) bpfc integration

I'm also still not a big fan of the shell escaping solution. This adds a
lot of string handling code which needs to be reviewed _very_ carefully.
If somehow possible I'd like to avoid this alltogether. Could we maybe
get rid of the call to system() alltogether and instead use fork() +
execvp() + wait() and pass the definitions args as we got them from
getopt? You'd need to use the "-o" option to cpp instead of the input
redirection, but that should be fine...

A few minor remarks below, but not a complete review (due to limited
time at the moment).

> Signed-off-by: Vadim Kochan 
> ---
>  bpf_parser.y |  6 ++--
>  bpfc.8   |  6 
>  bpfc.c   | 16 --
>  cpp.c| 97 
> +---
>  cpp.h|  2 +-
>  str.c| 10 ++
>  str.h|  1 +
>  trafgen.8|  7 
>  trafgen.c| 22 +
>  trafgen_conf.h   |  3 +-
>  trafgen_parser.y |  5 +--
>  11 files changed, 154 insertions(+), 21 deletions(-)
> 
> diff --git a/bpf_parser.y b/bpf_parser.y
> index 003c031..28e8d11 100644
> --- a/bpf_parser.y
> +++ b/bpf_parser.y
> @@ -28,7 +28,7 @@
>  #include "cpp.h"
>  
>  int compile_filter(char *file, int verbose, int bypass, int format,
> -bool invoke_cpp);
> +bool invoke_cpp, char **cpp_defs);

c
>  
>  static int curr_instr = 0;
>  
> @@ -735,7 +735,7 @@ static void pretty_printer(const struct sock_fprog *prog, 
> int format)
>  }
>  
>  int compile_filter(char *file, int verbose, int bypass, int format,
> -bool invoke_cpp)
> +bool invoke_cpp, char **cpp_defs)
>  {
>   int i;
>   struct sock_fprog res;
> @@ -745,7 +745,7 @@ int compile_filter(char *file, int verbose, int bypass, 
> int format,
>   memset(tmp_file, 0, sizeof(tmp_file));
>  
>   if (invoke_cpp) {
> - ret = cpp_exec(file, tmp_file, sizeof(tmp_file));
> + ret = cpp_exec(file, tmp_file, sizeof(tmp_file), cpp_defs);
>   if (ret) {
>   fprintf(stderr, "Failed to invoke C preprocessor!\n");
>   goto exit;
> diff --git a/bpfc.8 b/bpfc.8
> index 8a99e2e..eeb8356 100644
> --- a/bpfc.8
> +++ b/bpfc.8
> @@ -65,6 +65,12 @@ Pass the bpf program through the C preprocessor before 
> reading it in
>  bpfc. This allows #define and #include directives (e.g. to include
>  definitions from system headers) to be used in the bpf program.
>  .PP
> +.SS -D, --define
> +Add macro/define for C preprocessor to use it within bpf file. You may need 
> the shell's
> +quoting syntax to protect characters such as spaces that have a meaning in 
> the shell syntax.
> +By default the following characters .?*()[]-^|`!$&<>;\\ are escaped 
> automatically when
> +calling C preprocessor.

Please explicitly mention that this flag is only useful together with
-p/--cpp, same goes for the trafgen manpage.

> +.PP
>  .SS -f , --format 
>  Specify a different output format than the default that is netsniff-ng
>  compatible. The  specifier can be: C, netsniff-ng, xt_bpf, tcpdump.
> diff --git a/bpfc.c b/bpfc.c
> index d360cf5..fb7a9e5 100644
> --- a/bpfc.c
> +++ b/bpfc.c
> @@ -17,12 +17,14 @@
>  #include "die.h"
>  #include "bpf.h"
>  #include "config.h"
> +#include "str.h"
>  
> -static const char *short_options = "vhi:Vdbf:p";
> +static const char *short_options = "vhi:Vdbf:pD:";
>  static const struct option long_options[] = {
>   {"input",   required_argument,  NULL, 'i'},
>   {"format",  required_argument,  NULL, 'f'},
>   {"cpp", no_argument,NULL, 'p'},
> + {"define",  required_argument,  NULL, 'D'},
>   {"verbose", no_argument,NULL, 'V'},
>   {"bypass",  no_argument,NULL, 'b'},
>   {"dump",no_argument,NULL, 'd'},
> @@ -39,7 +41,7 @@ static const char *copyright = "Please report bugs to 
>"There is NO WARRANTY, to the extent permitted by law.";
>  
>  extern int compile_filter(char *file, int verbose, int bypass, int format,
> -   bool invoke_cpp);
> +   bool invoke_cpp, char **cpp_defs);
>  
>  static void __noreturn help(void)
>  {
> @@ -49,6 +51,7 @@ static void __noreturn help(void)
>"Options:\n"
>"  -i|--input   Berkeley Packet Filter file/stdin\n"
>

[netsniff-ng] Re: [PATCH 1/4] netsniff-ng: nlmsg: Resolve genl family name

2015-12-08 Thread Tobias Klauser
On 2015-11-30 at 01:05:04 +0100, Vadim Kochan  wrote:
> Print name of resolved genl family name by type

This patch does quite a bit more than the description says (i.e. the
init/uninit hooks). Please be a bit more verbose in your patch
descriptions.

> Signed-off-by: Vadim Kochan 
> ---
>  dissector_netlink.c |  3 ++
>  proto.h | 15 ++
>  proto_nlmsg.c   | 82 
> +
>  protos.h|  2 +-
>  4 files changed, 96 insertions(+), 6 deletions(-)
> 
> diff --git a/dissector_netlink.c b/dissector_netlink.c
> index 2b23a99..b4de112 100644
> --- a/dissector_netlink.c
> +++ b/dissector_netlink.c
> @@ -19,10 +19,13 @@ static inline void dissector_init_exit(int type)
>  
>  void dissector_init_netlink(int fnttype)
>  {
> + proto_ops_init(&nlmsg_ops);
> +
>   dissector_init_entry(fnttype);
>   dissector_init_exit(fnttype);
>  }
>  
>  void dissector_cleanup_netlink(void)
>  {
> + proto_ops_uninit(&nlmsg_ops);
>  }
> diff --git a/proto.h b/proto.h
> index 0cc1fd8..03a07e2 100644
> --- a/proto.h
> +++ b/proto.h
> @@ -10,6 +10,7 @@
>  
>  #include 
>  #include 
> +#include 

What is this needed for?

>  
>  #include "tprintf.h"
>  
> @@ -20,6 +21,8 @@ struct protocol {
>   const unsigned int key;
>   void (*print_full)(struct pkt_buff *pkt);
>   void (*print_less)(struct pkt_buff *pkt);
> + void (*init)(void);
> + void (*uninit)(void);

I don't think the very specific case of dissecting genl family messages
deserves the introduction of these hooks. How about just doing the init
work the first time the genl stuff is actually used?

Sorry for the brevity, I currently have a very limited bandwidth to
review patches...

-- 
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 netsniff-ng+unsubscr...@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.


<    1   2   3   4   >