netflow tools - Mar 2006 - Softflowd patches for ICMP type/code and DESTDIR support

Greetings,

First I must say thanks to Damien for this very useful program.  I have
recently started using softflowd and I found a few minor problems with it.

1. The Makefile doesn''t support ''make install
DESTDIR='' which is very
useful for building RPMs (more on that in another message).  I''ve
attached
a small patch that adds this support.

2. When Cisco routers generate Netflow v5 for ICMP, they encode the ICMP
type and code into the Netflow destination port field as type*256 + code.
Unfortunately softflowd does not do this, so you have no way of knowing
what ICMP it is logging - until now!  The other attached patch enables
the same ICMP type/code reporting you get with Cisco Netflow.

These patches are against softflowd 0.9.7.  Enjoy.

-- 
Steve Snodgrass * ssnodgra at pheran.com * Network and Unix Guru(?) at Large
Geek Code: GCS d? s: a C++ U++++$ P+++ L++ w PS+ 5++ b++ DI+ D++ e++ r+++ y+*
"If you want to be somebody else, change your mind."  -Sister Hazel
-------------- next part --------------
diff -ur softflowd-0.9.7.orig/Makefile.in softflowd-0.9.7/Makefile.in
--- softflowd-0.9.7.orig/Makefile.in	2004-09-29 00:14:35.000000000 -0400
+++ softflowd-0.9.7/Makefile.in	2006-02-15 15:30:48.000000000 -0500
@@ -49,8 +49,9 @@
 	strip $(TARGETS)
 
 install:
-	$(INSTALL) -m 0755 -s softflowd $(sbindir)/softflowd
-	$(INSTALL) -m 0755 -s softflowctl $(sbindir)/softflowctl
-	$(INSTALL) -m 0644 softflowd.8 $(mandir)/man8/softflowd.8
-	$(INSTALL) -m 0644 softflowctl.8 $(mandir)/man8/softflowctl.8
-
+	[ -d $(DESTDIR)$(sbindir) ] || mkdir -p $(DESTDIR)$(sbindir)
+	[ -d $(DESTDIR)$(mandir)/man8 ] || mkdir -p $(DESTDIR)$(mandir)/man8
+	$(INSTALL) -m 0755 -s softflowd $(DESTDIR)$(sbindir)/softflowd
+	$(INSTALL) -m 0755 -s softflowctl $(DESTDIR)$(sbindir)/softflowctl
+	$(INSTALL) -m 0644 softflowd.8 $(DESTDIR)$(mandir)/man8/softflowd.8
+	$(INSTALL) -m 0644 softflowctl.8 $(DESTDIR)$(mandir)/man8/softflowctl.8
-------------- next part --------------
diff -ur softflowd-0.9.7.orig/common.h softflowd-0.9.7/common.h
--- softflowd-0.9.7.orig/common.h	2005-01-14 23:08:56.000000000 -0500
+++ softflowd-0.9.7/common.h	2006-03-03 15:23:30.000000000 -0500
@@ -41,6 +41,7 @@
 #include <netinet/in_systm.h>
 #include <netinet/ip.h>
 #include <netinet/ip6.h>
+#include <netinet/ip_icmp.h>
 #include <netinet/tcp.h>
 #include <netinet/udp.h>
 #include <arpa/inet.h>
diff -ur softflowd-0.9.7.orig/softflowd.c softflowd-0.9.7/softflowd.c
--- softflowd-0.9.7.orig/softflowd.c	2005-01-09 20:50:07.000000000 -0500
+++ softflowd-0.9.7/softflowd.c	2006-03-03 16:36:44.000000000 -0500
@@ -282,6 +282,7 @@
 {
 	const struct tcphdr *tcp = (const struct tcphdr *)pkt;
 	const struct udphdr *udp = (const struct udphdr *)pkt;
+	const struct icmphdr *icmp = (const struct icmphdr *)pkt;
 
 	/*
 	 * XXX to keep flow in proper canonical format, it may be necessary
@@ -306,6 +307,11 @@
 		flow->port[ndx] = udp->uh_sport;
 		flow->port[ndx ^ 1] = udp->uh_dport;
 		break;
+	case IPPROTO_ICMP:
+		/* Encode ICMP type * 256 + code into dest port like Cisco routers */
+		flow->port[ndx] = 0;
+		flow->port[ndx ^ 1] = htons(icmp->type * 256 + icmp->code);
+		break;
 	}
 	return (0);
 }

On Fri, 3 Mar 2006, Steve Snodgrass wrote:
> Greetings,
> 
> First I must say thanks to Damien for this very useful program.  I have
> recently started using softflowd and I found a few minor problems with it.
Thanks!
> 1. The Makefile doesn''t support ''make install
DESTDIR='' which is very
> useful for building RPMs (more on that in another message).  I''ve
attached
> a small patch that adds this support.
Applied.
> 2. When Cisco routers generate Netflow v5 for ICMP, they encode the ICMP
> type and code into the Netflow destination port field as type*256 + code.
> Unfortunately softflowd does not do this, so you have no way of knowing
> what ICMP it is logging - until now!  The other attached patch enables
> the same ICMP type/code reporting you get with Cisco Netflow.
Thanks for this. I tweaked the patch slightly because "struct icmphdr"
appears to be a Linuxism, and is not present on OpenBSD or Solaris. 
What was committed uses "struct icmp" which is everywhere.

Please give this a try - it might need some incantation of _BSD_SOURCE
defined on glibc, or maybe not.

-d

Index: common.h
==================================================================RCS file:
/var/cvs/softflowd/common.h,v
retrieving revision 1.22
diff -u -p -r1.22 common.h
--- common.h	15 Jan 2005 04:08:56 -0000	1.22
+++ common.h	14 Mar 2006 22:56:16 -0000
@@ -41,6 +41,7 @@
 #include <netinet/in_systm.h>
 #include <netinet/ip.h>
 #include <netinet/ip6.h>
+#include <netinet/ip_icmp.h>
 #include <netinet/tcp.h>
 #include <netinet/udp.h>
 #include <arpa/inet.h>
Index: softflowd.c
==================================================================RCS file:
/var/cvs/softflowd/softflowd.c,v
retrieving revision 1.90
diff -u -p -r1.90 softflowd.c
--- softflowd.c	14 Mar 2006 22:51:48 -0000	1.90
+++ softflowd.c	14 Mar 2006 23:04:09 -0000
@@ -285,6 +285,7 @@ transport_to_flowrec(struct FLOW *flow, 
 {
 	const struct tcphdr *tcp = (const struct tcphdr *)pkt;
 	const struct udphdr *udp = (const struct udphdr *)pkt;
+	const struct icmp *icmp = (const struct icmp *)pkt;
 
 	/*
 	 * XXX to keep flow in proper canonical format, it may be necessary
@@ -308,6 +309,15 @@ transport_to_flowrec(struct FLOW *flow, 
 			return (isfrag ? 0 : 1);
 		flow->port[ndx] = udp->uh_sport;
 		flow->port[ndx ^ 1] = udp->uh_dport;
+		break;
+	case IPPROTO_ICMP:
+		/*
+		 * Encode ICMP type * 256 + code into dest port like
+		 * Cisco routers
+		 */
+		flow->port[ndx] = 0;
+		flow->port[ndx ^ 1] = htons(icmp->icmp_type * 256 +
+		    icmp->icmp_code);
 		break;
 	}
 	return (0);

On Wed, Mar 15, 2006 at 10:09:57AM +1100, Damien Miller
wrote:
> > 2. When Cisco routers generate Netflow v5 for ICMP, they encode the
ICMP
> > type and code into the Netflow destination port field as type*256 +
code.
> > Unfortunately softflowd does not do this, so you have no way of
knowing
> > what ICMP it is logging - until now!  The other attached patch enables
> > the same ICMP type/code reporting you get with Cisco Netflow.
> 
> Thanks for this. I tweaked the patch slightly because "struct
icmphdr"
> appears to be a Linuxism, and is not present on OpenBSD or Solaris. 
> What was committed uses "struct icmp" which is everywhere.
> 
> Please give this a try - it might need some incantation of _BSD_SOURCE
> defined on glibc, or maybe not.
You''re absolutely right, sorry about that.  I just checked an old
Solaris 8
box and it only has "struct icmp" as well.  I did a compile on Linux
with
struct icmp and it worked fine (with no additional defines), so your patch
should be good.

-- 
Steve Snodgrass * ssnodgra at pheran.com * Network and Unix Guru(?) at Large
Geek Code: GCS d? s: a C++ U++++$ P+++ L++ w PS+ 5++ b++ DI+ D++ e++ r+++ y+*
"If you want to be somebody else, change your mind."  -Sister Hazel