[pmacct-discussion] uninitialized req passed to plugin_requests and load_id_file in pm_pcap_cb??

2020-01-17 Thread Mikhail Sennikovsky
Hi all,

I was running through the pm_pcap_cb code, and it looks like the "req"
passed to exec_plugins(, ); at
https://github.com/pmacct/pmacct/blob/d72440dc9a7d0d0a7ed9502f1dd31b90105b1d95/src/nl.c#L167
and to load_id_file at
https://github.com/pmacct/pmacct/blob/d72440dc9a7d0d0a7ed9502f1dd31b90105b1d95/src/nl.c#L179
and below
is actually uninitialized. (See struct plugin_requests req;  at
https://github.com/pmacct/pmacct/blob/d72440dc9a7d0d0a7ed9502f1dd31b90105b1d95/src/nl.c#L51
)
Note that the exec_plugins and load_id_file actually read from req
rather than write to it.
If I'm getting this right, that code might be working just by coincidence.

Thanks,
Mikhail

___
pmacct-discussion mailing list
http://www.pmacct.net/#mailinglists


[pmacct-discussion] [PATCH 2/2] * nfprobe: per-interface flows

2020-01-17 Thread Mikhail Sennikovsky
nfprobe flow tree does not take interface index into consideration
when searching/aggregating the flow data.
This means that for the case multiple pcap interfaces are being
monitored and same src/dst ip/port traffic pattern is being handled
over several of those interfaces, this will all land in the same
FLOW entry.
This leads to the issues that flows being handled by one network
interface are actually reported via NetFlow (via Flow InputInt
and OutputInt fields)as being handled by another network interface
(held by the FLOW entry originally created for matching the given
src/dst ip/port traffic pattern).

Introduce a new nfprobe_per_interface_flows config variable to
allow taking flow interface indexes into consideration when
matching/searching for the FLOW entries in the flow cache tree.

Signed-off-by: Mikhail Sennikovsky 
---
 src/cfg.c   |  1 +
 src/cfg.h   |  1 +
 src/cfg_handlers.c  | 22 ++
 src/cfg_handlers.h  |  1 +
 src/nfprobe_plugin/nfprobe_plugin.c |  8 
 5 files changed, 33 insertions(+)

diff --git a/src/cfg.c b/src/cfg.c
index ddad54c..2b102bd 100644
--- a/src/cfg.c
+++ b/src/cfg.c
@@ -414,6 +414,7 @@ static const struct _dictionary_line dictionary[] = {
   {"sfprobe_ifindex", cfg_key_nfprobe_ifindex},
   {"sfprobe_ifspeed", cfg_key_sfprobe_ifspeed},
   {"sfprobe_ifindex_override", cfg_key_nfprobe_ifindex_override},
+  {"nfprobe_per_interface_flows", cfg_key_nfprobe_per_interface_flows},
   {"tee_receivers", cfg_key_tee_receivers},
   {"tee_source_ip", cfg_key_nfprobe_source_ip},
   {"tee_transparent", cfg_key_tee_transparent},
diff --git a/src/cfg.h b/src/cfg.h
index 631b19b..d652a59 100644
--- a/src/cfg.h
+++ b/src/cfg.h
@@ -550,6 +550,7 @@ struct configuration {
   int nfprobe_ifindex_type;
   int nfprobe_dont_cache;
   int nfprobe_tstamp_usec;
+  int nfprobe_per_interface_flows;
   char *sfprobe_receiver;
   char *sfprobe_agentip;
   int sfprobe_agentsubid;
diff --git a/src/cfg_handlers.c b/src/cfg_handlers.c
index eac176c..3fa6ed5 100644
--- a/src/cfg_handlers.c
+++ b/src/cfg_handlers.c
@@ -5859,6 +5859,28 @@ int cfg_key_nfprobe_dont_cache(char *filename, char 
*name, char *value_ptr)
   return changes;
 }
 
+int cfg_key_nfprobe_per_interface_flows(char *filename, char *name, char 
*value_ptr)
+{
+  struct plugins_list_entry *list = plugins_list;
+  int value, changes = 0;
+
+  value = parse_truefalse(value_ptr);
+  if (value < 0) return ERR;
+
+  if (!name) for (; list; list = list->next, changes++) 
list->cfg.nfprobe_per_interface_flows = value;
+  else {
+for (; list; list = list->next) {
+  if (!strcmp(name, list->name)) {
+list->cfg.nfprobe_per_interface_flows = value;
+changes++;
+break;
+  }
+}
+  }
+
+  return changes;
+}
+
 int cfg_key_sfprobe_receiver(char *filename, char *name, char *value_ptr)
 {
   struct plugins_list_entry *list = plugins_list;
diff --git a/src/cfg_handlers.h b/src/cfg_handlers.h
index 5ab0585..5d90a9c 100644
--- a/src/cfg_handlers.h
+++ b/src/cfg_handlers.h
@@ -288,6 +288,7 @@ extern int cfg_key_nfprobe_ifindex(char *, char *, char *);
 extern int cfg_key_nfprobe_ifindex_override(char *, char *, char *);
 extern int cfg_key_nfprobe_tstamp_usec(char *, char *, char *);
 extern int cfg_key_nfprobe_dont_cache(char *, char *, char *);
+extern int cfg_key_nfprobe_per_interface_flows(char *, char *, char *);
 extern int cfg_key_sfprobe_receiver(char *, char *, char *);
 extern int cfg_key_sfprobe_agentip(char *, char *, char *);
 extern int cfg_key_sfprobe_agentsubid(char *, char *, char *);
diff --git a/src/nfprobe_plugin/nfprobe_plugin.c 
b/src/nfprobe_plugin/nfprobe_plugin.c
index f7b0dc6..46a8166 100644
--- a/src/nfprobe_plugin/nfprobe_plugin.c
+++ b/src/nfprobe_plugin/nfprobe_plugin.c
@@ -150,6 +150,14 @@ flow_compare(struct FLOW *a, struct FLOW *b)
if (a->port[1] != b->port[1])
return (ntohs(a->port[1]) > ntohs(b->port[1]) ? 1 : -1);
 
+   if (config.nfprobe_per_interface_flows) {
+   if (a->ifindex[0] != b->ifindex[0])
+   return (a->ifindex[0] > b->ifindex[0] ? 1 : -1);
+
+   if (a->ifindex[1] != b->ifindex[1])
+   return (a->ifindex[1] > b->ifindex[1] ? 1 : -1);
+   }
+
return (0);
 }
 
-- 
2.7.4


___
pmacct-discussion mailing list
http://www.pmacct.net/#mailinglists


[pmacct-discussion] [PATCH 1/2] * pmacctd: allow configuring pcap_setdirection

2020-01-17 Thread Mikhail Sennikovsky
The pcap direction configuration was explicitly disabled
in 81fe649917036b9ef1ed4b3ea521befcaf36496b,
however even before that commit it apparently did not work,
because the pcap_setdirection must be called after pcap_activate,
not before it.

Introduce a new config variable, pcap_set_direction
to allow pmacctd do pcap_setdirection.

Signed-off-by: Mikhail Sennikovsky 
---
 src/cfg.c  |  1 +
 src/cfg.h  |  1 +
 src/cfg_handlers.c | 14 ++
 src/cfg_handlers.h |  1 +
 src/pmacctd.c  | 15 +++
 5 files changed, 24 insertions(+), 8 deletions(-)

diff --git a/src/cfg.c b/src/cfg.c
index 0dcf061..ddad54c 100644
--- a/src/cfg.c
+++ b/src/cfg.c
@@ -47,6 +47,7 @@ static const struct _dictionary_line dictionary[] = {
   {"pcap_interface_wait", cfg_key_pcap_interface_wait},
   {"pcap_direction", cfg_key_pcap_direction},
   {"pcap_ifindex", cfg_key_pcap_ifindex},
+  {"pcap_set_direction", cfg_key_pcap_set_direction},
   {"pcap_interfaces_map", cfg_key_pcap_interfaces_map},
   {"core_proc_name", cfg_key_proc_name},
   {"proc_priority", cfg_key_proc_priority},
diff --git a/src/cfg.h b/src/cfg.h
index 3641935..631b19b 100644
--- a/src/cfg.h
+++ b/src/cfg.h
@@ -474,6 +474,7 @@ struct configuration {
   char *pcap_savefile;
   int pcap_direction;
   int pcap_ifindex;
+  int pcap_set_direction;
   char *pcap_interfaces_map;
   char *pcap_if;
   int pcap_if_wait;
diff --git a/src/cfg_handlers.c b/src/cfg_handlers.c
index 0818b12..eac176c 100644
--- a/src/cfg_handlers.c
+++ b/src/cfg_handlers.c
@@ -547,6 +547,20 @@ int cfg_key_pcap_ifindex(char *filename, char *name, char 
*value_ptr)
   return changes;
 }
 
+int cfg_key_pcap_set_direction(char *filename, char *name, char *value_ptr)
+{
+  struct plugins_list_entry *list = plugins_list;
+  int value, changes = 0;
+
+  value = parse_truefalse(value_ptr);
+  if (value < 0) return ERR;
+
+  for (; list; list = list->next, changes++) list->cfg.pcap_set_direction = 
value;
+  if (name) Log(LOG_WARNING, "WARN: [%s] plugin name not supported for key 
'pcap_set_direction'. Globalized.\n", filename);
+
+  return changes;
+}
+
 int cfg_key_pcap_interfaces_map(char *filename, char *name, char *value_ptr)
 {
   struct plugins_list_entry *list = plugins_list;
diff --git a/src/cfg_handlers.h b/src/cfg_handlers.h
index 3fdd103..5ab0585 100644
--- a/src/cfg_handlers.h
+++ b/src/cfg_handlers.h
@@ -48,6 +48,7 @@ extern int cfg_key_pcap_savefile_delay(char *, char *, char 
*);
 extern int cfg_key_pcap_savefile_replay(char *, char *, char *);
 extern int cfg_key_pcap_direction(char *, char *, char *);
 extern int cfg_key_pcap_ifindex(char *, char *, char *);
+extern int cfg_key_pcap_set_direction(char *, char *, char *);
 extern int cfg_key_pcap_interfaces_map(char *, char *, char *);
 extern int cfg_key_use_ip_next_hop(char *, char *, char *);
 extern int cfg_key_decode_arista_trailer(char *, char *, char *);
diff --git a/src/pmacctd.c b/src/pmacctd.c
index 88fc367..1376a13 100644
--- a/src/pmacctd.c
+++ b/src/pmacctd.c
@@ -152,18 +152,17 @@ pcap_t *pm_pcap_open(const char *dev_ptr, int snaplen, 
int promisc,
   if (protocol)
 Log(LOG_WARNING, "WARN ( %s/core ): pcap_protocol specified but linked 
against a version of libpcap that does not support pcap_set_protocol().\n", 
config.name);
 #endif
-
-  /* XXX: rely on external filtering for now */
-/* 
-  ret = pcap_setdirection(p, direction);
-  if (ret < 0 && direction != PCAP_D_INOUT)
-Log(LOG_WARNING, "INFO ( %s/core ): direction specified but linked against 
a version of libpcap that does not support pcap_setdirection().\n", 
config.name);
-*/
-
+ 
   ret = pcap_activate(p);
   if (ret < 0)
 goto err;
 
+  if (config.pcap_set_direction) {
+ret = pcap_setdirection(p, direction);
+if (ret < 0 && direction != PCAP_D_INOUT)
+  Log(LOG_WARNING, "INFO ( %s/core ): direction specified but linked 
against a version of libpcap that does not support pcap_setdirection()\n", 
config.name);
+  }
+
   return p;
 
 err:
-- 
2.7.4


___
pmacct-discussion mailing list
http://www.pmacct.net/#mailinglists


[pmacct-discussion] [PATCH 0/2] patchset/questions to support pcap_setdirection and ifindex handling in nfprobe

2020-01-17 Thread Mikhail Sennikovsky
Hi all,

I had experienced some issues with configuring multiple pcap interfaces with 
pmacctd and wanted to clarify them with you and potentially ask for a 
better/alternative solution/configuration.
The two patches submitted here actually illustrate the problems I encountered 
and the way I had to fix them.
In my setup I have a "firewall entity" which forwards all traffic between two 
network interfaces, doing the necessary traffic firewalling/filtering, i.e. 
something like:
      +--+
<---> | InterfaceA  <->(Firewall)<->  InterfaceB | <--->
      +--+
now I want to generate a NetFlow v9 / IPFIX data for the traffic the Firewall 
instance allows in both directions, which means for all egress traffic for 
InterfaceA and all egress traffic for InterfaceB.

So I tried to use pmacctd +  its nfprobe plugin for doing that (configs are 
listed below).I configured pmacctd with multiple pcap interfaces, specifying 
pcap_interfaces_map and the nfprobe plugin (configs are listed below).
The first problem I encountered is that pmacctd does not configure pcap 
direction.
I.e. I can not use pcap filter, because I do not have any specific traffic 
pattern to determine the egress traffic for both interfaces (i.e. all traffic 
is being forwarded between those two interfaces, potentially filtered with some 
"firewall rules" in place.)
The pcap direction configuration was explicitly disabled in 
81fe649917036b9ef1ed4b3ea521befcaf36496b, however even before that commit it 
apparently did not work, because the pcap_setdirection must be called after 
pcap_activate, not before it.
So my first patch actually makes it possible to configure pmacctd to do 
pcap_setdirection by introducing a new config variable, pcap_set_direction.

Now after I ended up with pcap direction working, and configured pmacctd to 
listen for the egress traffic on both InterfaceA and InterfaceB, I faced the 
second problem, which seems to be a real bug in nfprobe plugin.
nfprobe flow tree does not take interface index into consideration when 
searching/aggregating the flow data. This means that for the case multiple pcap 
interfaces are being monitored and same src/dst ip/port traffic pattern is 
being handled over several of those interfaces, this will all land in the same 
FLOW entry. This leads to the issues that flows being handled by one network 
interface are actually reported via NetFlow (via Flow InputInt and OutputInt 
fields) as being handled by another network interface (held by the FLOW entry 
originally created for matching the given src/dst ip/port traffic pattern).
This perhaps could be worked around by having different nfprobe plugin 
instances handling InterfaceA and InterfaceB traffic, but I want to get the 
NetFlow data for both interfaces simultaneously, and I might also need to 
dynamically add/remove such interface pairs w/o restarting the pmacctd.
The latter could be easily achieved with updating the pcap_interfaces_map file 
and sending SIGUSR2 to pmacctd. The same would not be possible however if I had 
to add/remove nfprobe plugin instance configurations in pmacctd.conf
So my second patch fixes the above issue by making it possible to configure 
nfprobe to take flow interface indexes into consideration when 
matching/searching for the FLOW entries in the flow cache tree. The 
nfprobe_per_interface_flows config is introduced for that.

Would be great if someone could have a look into these two patches to see if 
they make sense, and/or give some hints on a better/proper way of making the 
similar NetFlow configuration.

Here are my configs for the reference:
pmacctd.conf:

daemonize: false
pidfile: /var/run/pmacctd.pid
syslog: daemon

pcap_interfaces_map: /path/to/pcap_interfaces.map
pcap_ifindex: map
! newly introduced config to tell pmacctd to actually do pcap_setdirection
pcap_set_direction: true
promisc: true
 
pmacctd_flow_buffer_buckets: 65536
pmacctd_flow_buffer_size: 128Mb
 
plugins: nfprobe[filtered], print[filtered_p]
plugin_pipe_size: 1048576000
plugin_buffer_size: 10485760

aggregate: src_host, dst_host, src_port, dst_port, in_iface, out_iface
aggregate[filtered]: src_host, dst_host, src_port, dst_port, in_iface, out_iface
aggregate[filtered_p]: src_host, dst_host, src_port, dst_port, in_iface, 
out_iface

pmacctd_as: file

refresh_maps: true
pre_tag_map: /path/to/pretag.map

pre_tag_filter[filtered]: -666
pre_tag_filter[filtered_p]: -666

nfprobe_source_ip: 10.11.12.23/24
nfprobe_receiver: 10.11.12.15:2055
nfprobe_version: 9
nfprobe_timeouts: maxlife=10:general=10:icmp=10:expint=10
nfprobe_maxflows: 512000
! newly introduced config to tell nfprobe plugin to also match flow interface 
indexes
! when matching/searching for the FLOW entries in the flow cache tree
nfprobe_per_interface_flows: true


pcap_interfaces.map:

ifindex=100 ifname=InterfaceA direction=out
ifindex=200 ifname=InterfaceB