[ULOGD PATCH 0/6] Fix difference between ULOG and NFLOG

[ULOGD PATCH 0/6] Fix difference between ULOG and NFLOG

[Eric Leblond]

Hello,

This patchset contains modification of ulogd2 and kernel relative to
a problem to hardware address logging.
HW address logging is different between ULOG and NFLOG:
 * NFLOG sends MAC address in userspace (dev_parse_header content)
 * ULOG sends complete link header (skb_mac_header content)

The kernel patch modifies NFLOG to have it log the whole
hardware header like ULOG.

Ulogd2 patches contains some code cleaning and a rework of MAC2STR plugin
which implements the support of actual NFLOG and ULOG logging.

Ulogd2 patchset statistics:
 doc/mysql-ulogd2.sql              |   32 ++++++-----
 doc/pgsql-ulogd2.sql              |   13 +++-
 filter/ulogd_filter_MAC2STR.c     |  117 +++++++++++++++++++++++++++++++------
 input/flow/ulogd_inpflow_NFCT.c   |   38 ++++++------
 input/packet/ulogd_inppkt_NFLOG.c |   87 ++++++++++++++-------------
 input/packet/ulogd_inppkt_ULOG.c  |   70 +++++++++++-----------
 ulogd.conf.in                     |    9 +++
 7 files changed, 234 insertions(+), 132 deletions(-)

BR,
--
Eric Leblond
INL: http://www.inl.fr/
NuFW: http://www.nufw.org/

[ULOGD PATCH 1/6] Suppress spaces at end of line.

[Eric Leblond]

The following patch suppress useless spaces at end of lines.

Signed-off-by: Eric Leblond <eric@inl.fr>
---
 input/flow/ulogd_inpflow_NFCT.c   |   38 +++++++++++++-------------
 input/packet/ulogd_inppkt_NFLOG.c |   52 ++++++++++++++++++------------------
 input/packet/ulogd_inppkt_ULOG.c  |   46 ++++++++++++++++----------------
 3 files changed, 68 insertions(+), 68 deletions(-)

diff --git a/input/flow/ulogd_inpflow_NFCT.c b/input/flow/ulogd_inpflow_NFCT.c
index f28e3fa..58bb0e2 100644
--- a/input/flow/ulogd_inpflow_NFCT.c
+++ b/input/flow/ulogd_inpflow_NFCT.c
@@ -156,7 +156,7 @@ static struct ulogd_key nfct_okeys[] = {
 		.type 	= ULOGD_RET_IPADDR,
 		.flags 	= ULOGD_RETF_NONE,
 		.name	= "orig.ip.saddr",
-		.ipfix	= { 
+		.ipfix	= {
 			.vendor = IPFIX_VENDOR_IETF,
 			.field_id = IPFIX_sourceIPv4Address,
 		},
@@ -174,7 +174,7 @@ static struct ulogd_key nfct_okeys[] = {
 		.type	= ULOGD_RET_UINT8,
 		.flags	= ULOGD_RETF_NONE,
 		.name	= "orig.ip.protocol",
-		.ipfix	= { 
+		.ipfix	= {
 			.vendor = IPFIX_VENDOR_IETF,
 			.field_id = IPFIX_protocolIdentifier,
 		},
@@ -201,7 +201,7 @@ static struct ulogd_key nfct_okeys[] = {
 		.type	= ULOGD_RET_UINT32,
 		.flags	= ULOGD_RETF_NONE,
 		.name	= "orig.raw.pktlen",
-		.ipfix	= { 
+		.ipfix	= {
 			.vendor 	= IPFIX_VENDOR_IETF,
 			.field_id 	= IPFIX_octetTotalCount,
 			/* FIXME: this could also be octetDeltaCount */
@@ -211,7 +211,7 @@ static struct ulogd_key nfct_okeys[] = {
 		.type	= ULOGD_RET_UINT32,
 		.flags	= ULOGD_RETF_NONE,
 		.name	= "orig.raw.pktcount",
-		.ipfix	= { 
+		.ipfix	= {
 			.vendor 	= IPFIX_VENDOR_IETF,
 			.field_id 	= IPFIX_packetTotalCount,
 			/* FIXME: this could also be packetDeltaCount */
@@ -221,7 +221,7 @@ static struct ulogd_key nfct_okeys[] = {
 		.type 	= ULOGD_RET_IPADDR,
 		.flags 	= ULOGD_RETF_NONE,
 		.name	= "reply.ip.saddr",
-		.ipfix	= { 
+		.ipfix	= {
 			.vendor = IPFIX_VENDOR_IETF,
 			.field_id = IPFIX_sourceIPv4Address,
 		},
@@ -239,7 +239,7 @@ static struct ulogd_key nfct_okeys[] = {
 		.type	= ULOGD_RET_UINT8,
 		.flags	= ULOGD_RETF_NONE,
 		.name	= "reply.ip.protocol",
-		.ipfix	= { 
+		.ipfix	= {
 			.vendor = IPFIX_VENDOR_IETF,
 			.field_id = IPFIX_protocolIdentifier,
 		},
@@ -266,7 +266,7 @@ static struct ulogd_key nfct_okeys[] = {
 		.type	= ULOGD_RET_UINT32,
 		.flags	= ULOGD_RETF_NONE,
 		.name	= "reply.raw.pktlen",
-		.ipfix	= { 
+		.ipfix	= {
 			.vendor 	= IPFIX_VENDOR_IETF,
 			.field_id 	= IPFIX_octetTotalCount,
 			/* FIXME: this could also be octetDeltaCount */
@@ -276,7 +276,7 @@ static struct ulogd_key nfct_okeys[] = {
 		.type	= ULOGD_RET_UINT32,
 		.flags	= ULOGD_RETF_NONE,
 		.name	= "reply.raw.pktcount",
-		.ipfix	= { 
+		.ipfix	= {
 			.vendor 	= IPFIX_VENDOR_IETF,
 			.field_id 	= IPFIX_packetTotalCount,
 			/* FIXME: this could also be packetDeltaCount */
@@ -442,7 +442,7 @@ static int propagate_ct(struct ulogd_pluginstance *upi,
 			struct ct_timestamp *ts)
 {
 	struct ulogd_key *ret = upi->output.keys;
-	
+
 	ret[NFCT_CT_EVENT].u.value.ui32 = type;
 	ret[NFCT_CT_EVENT].flags |= ULOGD_RETF_VALID;
 
@@ -569,7 +569,7 @@ static int propagate_ct(struct ulogd_pluginstance *upi,
 
 	if (ts) {
 		if (ts->time[START].tv_sec) {
-			ret[NFCT_FLOW_START_SEC].u.value.ui32 = 
+			ret[NFCT_FLOW_START_SEC].u.value.ui32 =
 				ts->time[START].tv_sec;
 			ret[NFCT_FLOW_START_SEC].flags |= ULOGD_RETF_VALID;
 
@@ -618,7 +618,7 @@ static int event_handler(enum nf_conntrack_msg_type type,
 			 void *data)
 {
 	struct ulogd_pluginstance *upi = data;
-	struct nfct_pluginstance *cpi = 
+	struct nfct_pluginstance *cpi =
 				(struct nfct_pluginstance *) upi->private;
 	struct ct_timestamp *ts = NULL;
 	struct ct_timestamp tmp = {
@@ -732,7 +732,7 @@ static int read_cb_nfct(int fd, unsigned int what, void *param)
 					  "`netlink_socket_buffer_size' and "
 					  "`netlink_socket_buffer_maxsize'\n");
 			}
-			
+
 			/* internal hash can deal with refresh */
 			if (usehash_ce(upi->config_kset).u.value != 0) {
 				nfct_send(cpi->ovh, NFCT_Q_DUMP, &family);
@@ -816,7 +816,7 @@ static int read_cb_ovh(int fd, unsigned int what, void *param)
 static int get_ctr_zero(struct ulogd_pluginstance *upi)
 {
 	int family = 0; /* any */
-	struct nfct_pluginstance *cpi = 
+	struct nfct_pluginstance *cpi =
 			(struct nfct_pluginstance *)upi->private;
 
 	return nfct_query(cpi->cth, NFCT_Q_DUMP_RESET, &family);
@@ -825,7 +825,7 @@ static int get_ctr_zero(struct ulogd_pluginstance *upi)
 static void getctr_timer_cb(struct ulogd_timer *t, void *data)
 {
 	struct ulogd_pluginstance *upi = data;
-	struct nfct_pluginstance *cpi = 
+	struct nfct_pluginstance *cpi =
 			(struct nfct_pluginstance *)upi->private;
 
 	get_ctr_zero(upi);
@@ -835,10 +835,10 @@ static void getctr_timer_cb(struct ulogd_timer *t, void *data)
 static int configure_nfct(struct ulogd_pluginstance *upi,
 			  struct ulogd_pluginstance_stack *stack)
 {
-	struct nfct_pluginstance *cpi = 
+	struct nfct_pluginstance *cpi =
 			(struct nfct_pluginstance *)upi->private;
 	int ret;
-	
+
 	ret = config_parse_file(upi->id, upi->config_kset);
 	if (ret < 0)
 		return ret;
@@ -865,7 +865,7 @@ static void overrun_timeout(struct ulogd_timer *a, void *data)
 
 static int constructor_nfct(struct ulogd_pluginstance *upi)
 {
-	struct nfct_pluginstance *cpi = 
+	struct nfct_pluginstance *cpi =
 			(struct nfct_pluginstance *)upi->private;
 
 	cpi->cth = nfct_open(NFNL_SUBSYS_CTNETLINK,
@@ -931,7 +931,7 @@ static int constructor_nfct(struct ulogd_pluginstance *upi)
 			return -1;
 		}
 	}
-	
+
 	return 0;
 }
 
@@ -939,7 +939,7 @@ static int destructor_nfct(struct ulogd_pluginstance *pi)
 {
 	struct nfct_pluginstance *cpi = (void *) pi;
 	int rc;
-	
+
 	hashtable_destroy(cpi->ct_active);
 
 	rc = nfct_close(cpi->cth);
diff --git a/input/packet/ulogd_inppkt_NFLOG.c b/input/packet/ulogd_inppkt_NFLOG.c
index 8c2aab6..95a2956 100644
--- a/input/packet/ulogd_inppkt_NFLOG.c
+++ b/input/packet/ulogd_inppkt_NFLOG.c
@@ -136,10 +136,10 @@ enum nflog_keys {
 };
 
 static struct ulogd_key output_keys[] = {
-	{ 
-		.type = ULOGD_RET_RAW, 
+	{
+		.type = ULOGD_RET_RAW,
 		.flags = ULOGD_RETF_NONE,
-		.name = "raw.mac", 
+		.name = "raw.mac",
 		.ipfix = {
 			.vendor = IPFIX_VENDOR_IETF,
 			.field_id = IPFIX_sourceMacAddress,
@@ -158,7 +158,7 @@ static struct ulogd_key output_keys[] = {
 		.type = ULOGD_RET_UINT32,
 		.flags = ULOGD_RETF_NONE,
 		.name = "raw.pktlen",
-		.ipfix = { 
+		.ipfix = {
 			.vendor = IPFIX_VENDOR_NETFILTER,
 			.field_id = IPFIX_NF_rawpacket_length,
 		},
@@ -167,32 +167,32 @@ static struct ulogd_key output_keys[] = {
 		.type = ULOGD_RET_UINT32,
 		.flags = ULOGD_RETF_NONE,
 		.name = "raw.pktcount",
-		.ipfix = { 
+		.ipfix = {
 			.vendor = IPFIX_VENDOR_IETF,
 			.field_id = IPFIX_packetDeltaCount,
 		},
 	},
 	{
 		.type = ULOGD_RET_STRING,
-		.flags = ULOGD_RETF_NONE, 
-		.name = "oob.prefix", 
+		.flags = ULOGD_RETF_NONE,
+		.name = "oob.prefix",
 		.ipfix = {
 			.vendor = IPFIX_VENDOR_NETFILTER,
-			.field_id = IPFIX_NF_prefix,  
+			.field_id = IPFIX_NF_prefix,
 		},
 	},
-	{ 	.type = ULOGD_RET_UINT32, 
-		.flags = ULOGD_RETF_NONE, 
-		.name = "oob.time.sec", 
-		.ipfix = { 
-			.vendor = IPFIX_VENDOR_IETF, 
-			.field_id = IPFIX_flowStartSeconds, 
+	{ 	.type = ULOGD_RET_UINT32,
+		.flags = ULOGD_RETF_NONE,
+		.name = "oob.time.sec",
+		.ipfix = {
+			.vendor = IPFIX_VENDOR_IETF,
+			.field_id = IPFIX_flowStartSeconds,
 		},
 	},
 	{
 		.type = ULOGD_RET_UINT32,
 		.flags = ULOGD_RETF_NONE,
-		.name = "oob.time.usec", 
+		.name = "oob.time.usec",
 		.ipfix = {
 			.vendor = IPFIX_VENDOR_IETF,
 			.field_id = IPFIX_flowStartMicroSeconds,
@@ -201,7 +201,7 @@ static struct ulogd_key output_keys[] = {
 	{
 		.type = ULOGD_RET_UINT32,
 		.flags = ULOGD_RETF_NONE,
-		.name = "oob.mark", 
+		.name = "oob.mark",
 		.ipfix = {
 			.vendor = IPFIX_VENDOR_NETFILTER,
 			.field_id = IPFIX_NF_mark,
@@ -210,7 +210,7 @@ static struct ulogd_key output_keys[] = {
 	{
 		.type = ULOGD_RET_UINT32,
 		.flags = ULOGD_RETF_NONE,
-		.name = "oob.ifindex_in", 
+		.name = "oob.ifindex_in",
 		.ipfix = {
 			.vendor = IPFIX_VENDOR_IETF,
 			.field_id = IPFIX_ingressInterface,
@@ -219,7 +219,7 @@ static struct ulogd_key output_keys[] = {
 	{
 		.type = ULOGD_RET_UINT32,
 		.flags = ULOGD_RETF_NONE,
-		.name = "oob.ifindex_out", 
+		.name = "oob.ifindex_out",
 		.ipfix = {
 			.vendor = IPFIX_VENDOR_IETF,
 			.field_id = IPFIX_egressInterface,
@@ -234,10 +234,10 @@ static struct ulogd_key output_keys[] = {
 			.field_id = IPFIX_NF_hook,
 		},
 	},
-	{ 
-		.type = ULOGD_RET_UINT16, 
-		.flags = ULOGD_RETF_NONE, 
-		.name = "raw.mac_len", 
+	{
+		.type = ULOGD_RET_UINT16,
+		.flags = ULOGD_RETF_NONE,
+		.name = "raw.mac_len",
 	},
 	{
 		.type = ULOGD_RET_UINT32,
@@ -285,7 +285,7 @@ static struct ulogd_key output_keys[] = {
 
 };
 
-static inline int 
+static inline int
 interp_packet(struct ulogd_pluginstance *upi, struct nflog_data *ldata)
 {
 	struct ulogd_key *ret = upi->output.keys;
@@ -489,9 +489,9 @@ static int start(struct ulogd_pluginstance *upi)
 
 	if (unbind_ce(upi->config_kset).u.value > 0) {
 		ulogd_log(ULOGD_NOTICE, "forcing unbind of existing log "
-			  "handler for protocol %d\n", 
+			  "handler for protocol %d\n",
 			  af_ce(upi->config_kset).u.value);
-		if (nflog_unbind_pf(ui->nful_h, 
+		if (nflog_unbind_pf(ui->nful_h,
 				    af_ce(upi->config_kset).u.value) < 0) {
 			ulogd_log(ULOGD_ERROR, "unable to force-unbind "
 				  "existing log handler for protocol %d\n",
@@ -537,7 +537,7 @@ static int start(struct ulogd_pluginstance *upi)
 			ulogd_log(ULOGD_ERROR, "unable to set flags 0x%x\n",
 				  flags);
 	}
-	
+
 	nflog_callback_register(ui->nful_gh, &msg_cb, upi);
 
 	ui->nful_fd.fd = nflog_fd(ui->nful_h);
diff --git a/input/packet/ulogd_inppkt_ULOG.c b/input/packet/ulogd_inppkt_ULOG.c
index 0b58393..2f65813 100644
--- a/input/packet/ulogd_inppkt_ULOG.c
+++ b/input/packet/ulogd_inppkt_ULOG.c
@@ -83,10 +83,10 @@ enum ulog_keys {
 };
 
 static struct ulogd_key output_keys[] = {
-	{ 
-		.type = ULOGD_RET_RAW, 
-		.flags = ULOGD_RETF_NONE, 
-		.name = "raw.mac", 
+	{
+		.type = ULOGD_RET_RAW,
+		.flags = ULOGD_RETF_NONE,
+		.name = "raw.mac",
 		.ipfix = {
 			.vendor = IPFIX_VENDOR_IETF,
 			.field_id = IPFIX_sourceMacAddress,
@@ -105,7 +105,7 @@ static struct ulogd_key output_keys[] = {
 		.type = ULOGD_RET_UINT32,
 		.flags = ULOGD_RETF_NONE,
 		.name = "raw.pktlen",
-		.ipfix = { 
+		.ipfix = {
 			.vendor = IPFIX_VENDOR_IETF,
 			.field_id = 1
 		},
@@ -114,43 +114,43 @@ static struct ulogd_key output_keys[] = {
 		.type = ULOGD_RET_UINT32,
 		.flags = ULOGD_RETF_NONE,
 		.name = "raw.pktcount",
-		.ipfix = { 
+		.ipfix = {
 			.vendor = IPFIX_VENDOR_IETF,
 			.field_id = 2
 		},
 	},
 	{
 		.type = ULOGD_RET_STRING,
-		.flags = ULOGD_RETF_NONE, 
-		.name = "oob.prefix", 
+		.flags = ULOGD_RETF_NONE,
+		.name = "oob.prefix",
 	},
-	{ 	.type = ULOGD_RET_UINT32, 
-		.flags = ULOGD_RETF_NONE, 
-		.name = "oob.time.sec", 
-		.ipfix = { 
-			.vendor = IPFIX_VENDOR_IETF, 
-			.field_id = 22 
+	{ 	.type = ULOGD_RET_UINT32,
+		.flags = ULOGD_RETF_NONE,
+		.name = "oob.time.sec",
+		.ipfix = {
+			.vendor = IPFIX_VENDOR_IETF,
+			.field_id = 22
 		},
 	},
 	{
 		.type = ULOGD_RET_UINT32,
 		.flags = ULOGD_RETF_NONE,
-		.name = "oob.time.usec", 
+		.name = "oob.time.usec",
 	},
 	{
 		.type = ULOGD_RET_UINT32,
 		.flags = ULOGD_RETF_NONE,
-		.name = "oob.mark", 
+		.name = "oob.mark",
 	},
 	{
 		.type = ULOGD_RET_STRING,
 		.flags = ULOGD_RETF_NONE,
-		.name = "oob.in", 
+		.name = "oob.in",
 	},
 	{
 		.type = ULOGD_RET_STRING,
 		.flags = ULOGD_RETF_NONE,
-		.name = "oob.out", 
+		.name = "oob.out",
 	},
 	{
 		.type = ULOGD_RET_UINT8,
@@ -161,10 +161,10 @@ static struct ulogd_key output_keys[] = {
 			.field_id = IPFIX_NF_hook,
 		},
 	},
-	{ 
-		.type = ULOGD_RET_UINT16, 
-		.flags = ULOGD_RETF_NONE, 
-		.name = "raw.mac_len", 
+	{
+		.type = ULOGD_RET_UINT16,
+		.flags = ULOGD_RETF_NONE,
+		.name = "raw.mac_len",
 	},
 	{
 		.type = ULOGD_RET_UINT8,
-- 
1.5.5.3

Re: [ULOGD PATCH 1/6] Suppress spaces at end of line.

[Pablo Neira Ayuso]

Eric Leblond wrote:
> The following patch suppress useless spaces at end of lines.

Applied. Thanks.

-- 
"Los honestos son inadaptados sociales" -- Les Luthiers

[ULOGD PATCH 2/6] Convert struct to [KEY] = {} format.

[Eric Leblond]

This patch converts NFLOG and ULOG structure to the [KEY} = {}
discuss sooner with Patrick on the ML.

Signed-off-by: Eric Leblond <eric@inl.fr>
---
 input/packet/ulogd_inppkt_NFLOG.c |   39 +++++++++++++++++++------------------
 input/packet/ulogd_inppkt_ULOG.c  |   31 +++++++++++++++--------------
 2 files changed, 36 insertions(+), 34 deletions(-)

diff --git a/input/packet/ulogd_inppkt_NFLOG.c b/input/packet/ulogd_inppkt_NFLOG.c
index 95a2956..728b410 100644
--- a/input/packet/ulogd_inppkt_NFLOG.c
+++ b/input/packet/ulogd_inppkt_NFLOG.c
@@ -136,7 +136,7 @@ enum nflog_keys {
 };
 
 static struct ulogd_key output_keys[] = {
-	{
+	[NFLOG_KEY_RAW_MAC] = {
 		.type = ULOGD_RET_RAW,
 		.flags = ULOGD_RETF_NONE,
 		.name = "raw.mac",
@@ -145,7 +145,7 @@ static struct ulogd_key output_keys[] = {
 			.field_id = IPFIX_sourceMacAddress,
 		},
 	},
-	{
+	[NFLOG_KEY_RAW_PCKT] = {
 		.type = ULOGD_RET_RAW,
 		.flags = ULOGD_RETF_NONE,
 		.name = "raw.pkt",
@@ -154,7 +154,7 @@ static struct ulogd_key output_keys[] = {
 			.field_id = IPFIX_NF_rawpacket,
 		},
 	},
-	{
+	[NFLOG_KEY_RAW_PCKTLEN] = {
 		.type = ULOGD_RET_UINT32,
 		.flags = ULOGD_RETF_NONE,
 		.name = "raw.pktlen",
@@ -163,7 +163,7 @@ static struct ulogd_key output_keys[] = {
 			.field_id = IPFIX_NF_rawpacket_length,
 		},
 	},
-	{
+	[NFLOG_KEY_RAW_PCKTCOUNT] = {
 		.type = ULOGD_RET_UINT32,
 		.flags = ULOGD_RETF_NONE,
 		.name = "raw.pktcount",
@@ -172,7 +172,7 @@ static struct ulogd_key output_keys[] = {
 			.field_id = IPFIX_packetDeltaCount,
 		},
 	},
-	{
+	[NFLOG_KEY_OOB_PREFIX] = {
 		.type = ULOGD_RET_STRING,
 		.flags = ULOGD_RETF_NONE,
 		.name = "oob.prefix",
@@ -181,7 +181,8 @@ static struct ulogd_key output_keys[] = {
 			.field_id = IPFIX_NF_prefix,
 		},
 	},
-	{ 	.type = ULOGD_RET_UINT32,
+	[NFLOG_KEY_OOB_TIME_SEC] = {
+		.type = ULOGD_RET_UINT32,
 		.flags = ULOGD_RETF_NONE,
 		.name = "oob.time.sec",
 		.ipfix = {
@@ -189,7 +190,7 @@ static struct ulogd_key output_keys[] = {
 			.field_id = IPFIX_flowStartSeconds,
 		},
 	},
-	{
+	[NFLOG_KEY_OOB_TIME_USEC] = {
 		.type = ULOGD_RET_UINT32,
 		.flags = ULOGD_RETF_NONE,
 		.name = "oob.time.usec",
@@ -198,7 +199,7 @@ static struct ulogd_key output_keys[] = {
 			.field_id = IPFIX_flowStartMicroSeconds,
 		},
 	},
-	{
+	[NFLOG_KEY_OOB_MARK] = {
 		.type = ULOGD_RET_UINT32,
 		.flags = ULOGD_RETF_NONE,
 		.name = "oob.mark",
@@ -207,7 +208,7 @@ static struct ulogd_key output_keys[] = {
 			.field_id = IPFIX_NF_mark,
 		},
 	},
-	{
+	[NFLOG_KEY_OOB_IFINDEX_IN] = {
 		.type = ULOGD_RET_UINT32,
 		.flags = ULOGD_RETF_NONE,
 		.name = "oob.ifindex_in",
@@ -216,7 +217,7 @@ static struct ulogd_key output_keys[] = {
 			.field_id = IPFIX_ingressInterface,
 		},
 	},
-	{
+	[NFLOG_KEY_OOB_IFINDEX_OUT] = {
 		.type = ULOGD_RET_UINT32,
 		.flags = ULOGD_RETF_NONE,
 		.name = "oob.ifindex_out",
@@ -225,7 +226,7 @@ static struct ulogd_key output_keys[] = {
 			.field_id = IPFIX_egressInterface,
 		},
 	},
-	{
+	[NFLOG_KEY_OOB_HOOK] = {
 		.type = ULOGD_RET_UINT8,
 		.flags = ULOGD_RETF_NONE,
 		.name = "oob.hook",
@@ -234,12 +235,12 @@ static struct ulogd_key output_keys[] = {
 			.field_id = IPFIX_NF_hook,
 		},
 	},
-	{
+	[NFLOG_KEY_RAW_MAC_LEN] = {
 		.type = ULOGD_RET_UINT16,
 		.flags = ULOGD_RETF_NONE,
 		.name = "raw.mac_len",
 	},
-	{
+	[NFLOG_KEY_OOB_SEQ_LOCAL] = {
 		.type = ULOGD_RET_UINT32,
 		.flags = ULOGD_RETF_NONE,
 		.name = "oob.seq.local",
@@ -248,7 +249,7 @@ static struct ulogd_key output_keys[] = {
 			.field_id = IPFIX_NF_seq_local,
 		},
 	},
-	{
+	[NFLOG_KEY_OOB_SEQ_GLOBAL] = {
 		.type = ULOGD_RET_UINT32,
 		.flags = ULOGD_RETF_NONE,
 		.name = "oob.seq.global",
@@ -257,27 +258,27 @@ static struct ulogd_key output_keys[] = {
 			.field_id = IPFIX_NF_seq_global,
 		},
 	},
-	{
+	[NFLOG_KEY_OOB_FAMILY] = {
 		.type = ULOGD_RET_UINT8,
 		.flags = ULOGD_RETF_NONE,
 		.name = "oob.family",
 	},
-	{
+	[NFLOG_KEY_OOB_PROTOCOL] = {
 		.type = ULOGD_RET_UINT16,
 		.flags = ULOGD_RETF_NONE,
 		.name = "oob.protocol",
 	},
-	{
+	[NFLOG_KEY_OOB_UID] = {
 		.type = ULOGD_RET_UINT32,
 		.flags = ULOGD_RETF_NONE,
 		.name = "oob.uid",
 	},
-	{
+	[NFLOG_KEY_OOB_GID] = {
 		.type = ULOGD_RET_UINT32,
 		.flags = ULOGD_RETF_NONE,
 		.name = "oob.gid",
 	},
-	{
+	[NFLOG_KEY_RAW_LABEL] = {
 		.type = ULOGD_RET_UINT8,
 		.flags = ULOGD_RETF_NONE,
 		.name = "raw.label",
diff --git a/input/packet/ulogd_inppkt_ULOG.c b/input/packet/ulogd_inppkt_ULOG.c
index 2f65813..97b9972 100644
--- a/input/packet/ulogd_inppkt_ULOG.c
+++ b/input/packet/ulogd_inppkt_ULOG.c
@@ -83,7 +83,7 @@ enum ulog_keys {
 };
 
 static struct ulogd_key output_keys[] = {
-	{
+	[ULOG_KEY_RAW_MAC] = {
 		.type = ULOGD_RET_RAW,
 		.flags = ULOGD_RETF_NONE,
 		.name = "raw.mac",
@@ -92,7 +92,7 @@ static struct ulogd_key output_keys[] = {
 			.field_id = IPFIX_sourceMacAddress,
 		},
 	},
-	{
+	[ULOG_KEY_RAW_PCKT] = {
 		.type = ULOGD_RET_RAW,
 		.flags = ULOGD_RETF_NONE,
 		.name = "raw.pkt",
@@ -101,7 +101,7 @@ static struct ulogd_key output_keys[] = {
 			.field_id = 1,
 			},
 	},
-	{
+	[ULOG_KEY_RAW_PCKTLEN] = {
 		.type = ULOGD_RET_UINT32,
 		.flags = ULOGD_RETF_NONE,
 		.name = "raw.pktlen",
@@ -110,7 +110,7 @@ static struct ulogd_key output_keys[] = {
 			.field_id = 1
 		},
 	},
-	{
+	[ULOG_KEY_RAW_PCKTCOUNT] = {
 		.type = ULOGD_RET_UINT32,
 		.flags = ULOGD_RETF_NONE,
 		.name = "raw.pktcount",
@@ -119,12 +119,13 @@ static struct ulogd_key output_keys[] = {
 			.field_id = 2
 		},
 	},
-	{
+	[ULOG_KEY_OOB_PREFIX] = {
 		.type = ULOGD_RET_STRING,
 		.flags = ULOGD_RETF_NONE,
 		.name = "oob.prefix",
 	},
-	{ 	.type = ULOGD_RET_UINT32,
+	[ULOG_KEY_OOB_TIME_SEC] = {
+		.type = ULOGD_RET_UINT32,
 		.flags = ULOGD_RETF_NONE,
 		.name = "oob.time.sec",
 		.ipfix = {
@@ -132,27 +133,27 @@ static struct ulogd_key output_keys[] = {
 			.field_id = 22
 		},
 	},
-	{
+	[ULOG_KEY_OOB_TIME_USEC] = {
 		.type = ULOGD_RET_UINT32,
 		.flags = ULOGD_RETF_NONE,
 		.name = "oob.time.usec",
 	},
-	{
+	[ULOG_KEY_OOB_MARK] = {
 		.type = ULOGD_RET_UINT32,
 		.flags = ULOGD_RETF_NONE,
 		.name = "oob.mark",
 	},
-	{
+	[ULOG_KEY_OOB_IN] = {
 		.type = ULOGD_RET_STRING,
 		.flags = ULOGD_RETF_NONE,
 		.name = "oob.in",
 	},
-	{
+	[ULOG_KEY_OOB_OUT] = {
 		.type = ULOGD_RET_STRING,
 		.flags = ULOGD_RETF_NONE,
 		.name = "oob.out",
 	},
-	{
+	[ULOG_KEY_OOB_HOOK] = {
 		.type = ULOGD_RET_UINT8,
 		.flags = ULOGD_RETF_NONE,
 		.name = "oob.hook",
@@ -161,22 +162,22 @@ static struct ulogd_key output_keys[] = {
 			.field_id = IPFIX_NF_hook,
 		},
 	},
-	{
+	[ULOG_KEY_RAW_MAC_LEN] = {
 		.type = ULOGD_RET_UINT16,
 		.flags = ULOGD_RETF_NONE,
 		.name = "raw.mac_len",
 	},
-	{
+	[ULOG_KEY_OOB_FAMILY] = {
 		.type = ULOGD_RET_UINT8,
 		.flags = ULOGD_RETF_NONE,
 		.name = "oob.family",
 	},
-	{
+	[ULOG_KEY_OOB_PROTOCOL] = {
 		.type = ULOGD_RET_UINT16,
 		.flags = ULOGD_RETF_NONE,
 		.name = "oob.protocol",
 	},
-	{
+	[ULOG_KEY_RAW_LABEL] = {
 		.type = ULOGD_RET_UINT8,
 		.flags = ULOGD_RETF_NONE,
 		.name = "raw.label",
-- 
1.5.5.3

Re: [ULOGD PATCH 2/6] Convert struct to [KEY] = {} format.

[Pablo Neira Ayuso]

Eric Leblond wrote:
> This patch converts NFLOG and ULOG structure to the [KEY} = {}
> discuss sooner with Patrick on the ML.

Applied. Thanks.

-- 
"Los honestos son inadaptados sociales" -- Les Luthiers

[ULOGD PATCH 3/6] Fix warning about unused variable if NFLOG_GID is not available.

[Eric Leblond]

This patch fixes a warning about an unused variable if NFLOG_GID is not available
in libnetfilter_log.

Signed-off-by: Eric Leblond <eric@inl.fr>
---
 input/packet/ulogd_inppkt_NFLOG.c |    2 ++
 1 files changed, 2 insertions(+), 0 deletions(-)

diff --git a/input/packet/ulogd_inppkt_NFLOG.c b/input/packet/ulogd_inppkt_NFLOG.c
index 728b410..9689929 100644
--- a/input/packet/ulogd_inppkt_NFLOG.c
+++ b/input/packet/ulogd_inppkt_NFLOG.c
@@ -302,7 +302,9 @@ interp_packet(struct ulogd_pluginstance *upi, struct nflog_data *ldata)
 	u_int32_t outdev = nflog_get_outdev(ldata);
 	u_int32_t seq;
 	u_int32_t uid;
+#ifdef HAVE_NFLOG_GET_GID
 	u_int32_t gid;
+#endif
 
 	ret[NFLOG_KEY_OOB_FAMILY].u.value.ui8 = af_ce(upi->config_kset).u.value;
 	ret[NFLOG_KEY_OOB_FAMILY].flags |= ULOGD_RETF_VALID;
-- 
1.5.5.3

Re: [ULOGD PATCH 3/6] Fix warning about unused variable if NFLOG_GID is not available.

[Pablo Neira Ayuso]

Eric Leblond wrote:
> This patch fixes a warning about an unused variable if NFLOG_GID is not available
> in libnetfilter_log.

AFAIK, this warning happens if we use an old version of the library, right?

As I'm about to conclude a massive release of libraries and the tools, I
think that forcing the use of a recent library version is better as it
also contains several bugfixes.

-- 
"Los honestos son inadaptados sociales" -- Les Luthiers

Re: [ULOGD PATCH 3/6] Fix warning about unused variable if NFLOG_GID is not available.

[Eric Leblond]

[-- Attachment #1: Type: text/plain, Size: 924 bytes --]

Hello,

On Monday, 2008 June 23 at 16:25:36 +0200, Pablo Neira Ayuso wrote:
> Eric Leblond wrote:
> > This patch fixes a warning about an unused variable if NFLOG_GID is not available
> > in libnetfilter_log.
> 
> AFAIK, this warning happens if we use an old version of the library, right?


Yes.

> As I'm about to conclude a massive release of libraries and the tools, I
> think that forcing the use of a recent library version is better as it
> also contains several bugfixes.

Users prefer to use the library packaged in their system and it will
take some time before every distribution includes the required
libnfnetlink_log. This patch finishes to provide a clean backward
compatibility at a really low cost.

Furthermore, I don't think a gcc warning about "unused variable" will
force a lot of people to upgrade ;)

BR,
-- 
Eric Leblond
INL: http://www.inl.fr/
NuFW: http://www.nufw.org/

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 189 bytes --]

Re: [ULOGD PATCH 3/6] Fix warning about unused variable if NFLOG_GID is not available.

[Eric Leblond]

[-- Attachment #1: Type: text/plain, Size: 1092 bytes --]

Hello,

On Friday, 2008 June 27 at 21:41:33 +0200, Eric Leblond wrote:
> Hello,
> > As I'm about to conclude a massive release of libraries and the tools, I
> > think that forcing the use of a recent library version is better as it
> > also contains several bugfixes.

I did not really understand the meaning of the word "forcing" till I discover
that you've forced the use of at least libnfnetlink 0.0.39 in configure.in.

IMHO, this is too strict and not an easy step into ulogd2 acceptance:
Instead of having a single software to compile, users will need to
compile almost all libnf* library :(

> Users prefer to use the library packaged in their system and it will
> take some time before every distribution includes the required
> libnfnetlink_log. This patch finishes to provide a clean backward
> compatibility at a really low cost.
> 
> Furthermore, I don't think a gcc warning about "unused variable" will
> force a lot of people to upgrade ;)

But a strict configure.in will :/

BR,
-- 
Eric Leblond
INL: http://www.inl.fr/
NuFW: http://www.nufw.org/

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 189 bytes --]

Re: [ULOGD PATCH 3/6] Fix warning about unused variable if NFLOG_GID is not available.

[Pablo Neira Ayuso]

Eric Leblond wrote:
> On Friday, 2008 June 27 at 21:41:33 +0200, Eric Leblond wrote:
>> Hello,
>>> As I'm about to conclude a massive release of libraries and the tools, I
>>> think that forcing the use of a recent library version is better as it
>>> also contains several bugfixes.
> 
> I did not really understand the meaning of the word "forcing" till I discover
> that you've forced the use of at least libnfnetlink 0.0.39 in configure.in.
> 
> IMHO, this is too strict and not an easy step into ulogd2 acceptance:
> Instead of having a single software to compile, users will need to
> compile almost all libnf* library :(

This latest library release-set includes several fixes. Having the
lastest version installed always suppose an extra effort for the users.
Moreover, bumping the dependencies is also a way to force packagers to
upgrade.

>> Users prefer to use the library packaged in their system and it will
>> take some time before every distribution includes the required
>> libnfnetlink_log. This patch finishes to provide a clean backward
>> compatibility at a really low cost.

The libraries are backward compatible. We did not break the API nor ABI
so that users with the own applications are not forced to upgrade.

The thing changes if we discuss about the userspace tools. I think that
they must use the latest library release. Thus, we avoid having people
that report problems that has been already fixed.

>> Furthermore, I don't think a gcc warning about "unused variable" will
>> force a lot of people to upgrade ;)
> 
> But a strict configure.in will :/

The gcc warning is anecdotic, it was not the main reason to bump the
dependencies.

Looking at the current state of the libraries, I think that it's the
best for now. Later on, once they get more stable and well-tested, we
may change this policy.

-- 
"Los honestos son inadaptados sociales" -- Les Luthiers

Re: [ULOGD PATCH 3/6] Fix warning about unused variable if NFLOG_GID is not available.

[Eric Leblond]

[-- Attachment #1: Type: text/plain, Size: 2926 bytes --]

Hi,

On Saturday, 2008 June 28 at 11:54:14 +0200, Pablo Neira Ayuso wrote:
> Eric Leblond wrote:
> > On Friday, 2008 June 27 at 21:41:33 +0200, Eric Leblond wrote:
> >> Hello,
> >>> As I'm about to conclude a massive release of libraries and the tools, I
> >>> think that forcing the use of a recent library version is better as it
> >>> also contains several bugfixes.
> > 
> > I did not really understand the meaning of the word "forcing" till I discover
> > that you've forced the use of at least libnfnetlink 0.0.39 in configure.in.
> > 
> > IMHO, this is too strict and not an easy step into ulogd2 acceptance:
> > Instead of having a single software to compile, users will need to
> > compile almost all libnf* library :(
> 
> This latest library release-set includes several fixes. Having the
> lastest version installed always suppose an extra effort for the users.
> Moreover, bumping the dependencies is also a way to force packagers to
> upgrade.
> 
> >> Users prefer to use the library packaged in their system and it will
> >> take some time before every distribution includes the required
> >> libnfnetlink_log. This patch finishes to provide a clean backward
> >> compatibility at a really low cost.
> 
> The libraries are backward compatible. We did not break the API nor ABI
> so that users with the own applications are not forced to upgrade.

I know that. The point is really not here. Work done in this area is
clearly good.

> The thing changes if we discuss about the userspace tools. I think that
> they must use the latest library release. Thus, we avoid having people
> that report problems that has been already fixed.

I really don't like this policy because it is too efficient: You will have
any report before required version of library get into the distributions
because people won't test it it there is too much work to build a test
system.
 
> >> Furthermore, I don't think a gcc warning about "unused variable" will
> >> force a lot of people to upgrade ;)
> > 
> > But a strict configure.in will :/
> 
> The gcc warning is anecdotic, it was not the main reason to bump the
> dependencies.

I know that, I was just talking about the discussed patch. But after
looking at the changelog and after using ulogd2 without problem with
older libraries, I don't think the dependencies bump was needed. If a
bug in the library was preventing ulogd2 to work correctly, I will be
the first to ask for a dependencies bump but it is far from being the
case here.

> Looking at the current state of the libraries, I think that it's the
> best for now. Later on, once they get more stable and well-tested, we
> may change this policy.

Please revert commit 3178606785161296dc5a1bd4d42d965db8b3e2cd if you
want to apply this strict policy. This code is now useless.

BR,
-- 
Eric Leblond
INL: http://www.inl.fr/
NuFW: http://www.nufw.org/

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 189 bytes --]

Re: [ULOGD PATCH 3/6] Fix warning about unused variable if NFLOG_GID is not available.

[Pablo Neira Ayuso]

Eric Leblond wrote:
> On Saturday, 2008 June 28 at 11:54:14 +0200, Pablo Neira Ayuso wrote:
>> The thing changes if we discuss about the userspace tools. I think that
>> they must use the latest library release. Thus, we avoid having people
>> that report problems that has been already fixed.
> 
> I really don't like this policy because it is too efficient: You will have
> any report before required version of library get into the distributions
> because people won't test it it there is too much work to build a test
> system.

Let's see it from this point: Assume that my policy consists of reducing
people homework to keep it easier to test ulogd2. Then, later on, I may
receive reports on problems that are supposed-to-be-fixed.

This report, consequently, will increase our workload since we'll have
to request in the ML: "please upgrade to lastest and confirm that it's
fixed, a similar problem is supposed to be fixed in lastest". Then, wait
for their ACK, and so on.

Moreover, since ulogd2 uses three libraries, the hypothetical reported
problem can be in any of those previous library versions. As the users
can use whatever combination of libraries, we'll have to look into the
repository and check if that library suffers from that problem or not.
As said, more work for us.

In short: I prefer reducing my workload if this only implies a sweet
push to make people do their homework - and, after all, they only have
to recompile the libraries which is not that much work.

>>>> Furthermore, I don't think a gcc warning about "unused variable" will
>>>> force a lot of people to upgrade ;)
>>> But a strict configure.in will :/
>> The gcc warning is anecdotic, it was not the main reason to bump the
>> dependencies.
> 
> I know that, I was just talking about the discussed patch. But after
> looking at the changelog and after using ulogd2 without problem with
> older libraries, I don't think the dependencies bump was needed. If a
> bug in the library was preventing ulogd2 to work correctly, I will be
> the first to ask for a dependencies bump but it is far from being the
> case here.
> 
>> Looking at the current state of the libraries, I think that it's the
>> best for now. Later on, once they get more stable and well-tested, we
>> may change this policy.
> 
> Please revert commit 3178606785161296dc5a1bd4d42d965db8b3e2cd if you
> want to apply this strict policy. This code is now useless.

Done. Thanks.

-- 
"Los honestos son inadaptados sociales" -- Les Luthiers

Re: [ULOGD PATCH 3/6] Fix warning about unused variable if NFLOG_GID is not available.

[Pablo Neira Ayuso]

Eric Leblond wrote:
> On Saturday, 2008 June 28 at 11:54:14 +0200, Pablo Neira Ayuso wrote:
>> The thing changes if we discuss about the userspace tools. I think that
>> they must use the latest library release. Thus, we avoid having people
>> that report problems that has been already fixed.
> 
> I really don't like this policy because it is too efficient: You will have
> any report before required version of library get into the distributions
> because people won't test it it there is too much work to build a test
> system.

Let's see it from this point: Assume that my policy consists of reducing
people homework to keep it easier to test ulogd2. Then, later on, I may
receive reports on problems that are supposed-to-be-fixed.

This report, consequently, will increase our workload since we'll have
to request in the ML: "please upgrade to lastest and confirm that it's
fixed, a similar problem is supposed to be fixed in lastest". Then, wait
for their ACK, and so on.

Moreover, since ulogd2 uses three libraries, the hypothetical reported
problem can be in any of those previous library versions. As the users
can use whatever combination of libraries, we'll have to look into the
repository and check if that library suffers from that problem or not.
As said, more work for us.

In short: I prefer reducing my workload if this only implies a sweet
push to make people do their homework - and, after all, they only have
to recompile the libraries which is not that much work.

>>>> Furthermore, I don't think a gcc warning about "unused variable" will
>>>> force a lot of people to upgrade ;)
>>> But a strict configure.in will :/
>> The gcc warning is anecdotic, it was not the main reason to bump the
>> dependencies.
> 
> I know that, I was just talking about the discussed patch. But after
> looking at the changelog and after using ulogd2 without problem with
> older libraries, I don't think the dependencies bump was needed. If a
> bug in the library was preventing ulogd2 to work correctly, I will be
> the first to ask for a dependencies bump but it is far from being the
> case here.
> 
>> Looking at the current state of the libraries, I think that it's the
>> best for now. Later on, once they get more stable and well-tested, we
>> may change this policy.
> 
> Please revert commit 3178606785161296dc5a1bd4d42d965db8b3e2cd if you
> want to apply this strict policy. This code is now useless.

Done. Thanks.

-- 
"Los honestos son inadaptados sociales" -- Les Luthiers

[ULOGD PATCH 4/6] Introduce datatype and hwaddrlen param to MAC2STR plugin.

[Eric Leblond]

This patch introduces a hwaddrlen and datatype parameters in
MAC2STR configuration. This is a workaround differences between
ULOG and NFLOG:
 * NFLOG sends MAC address in userspace (dev_parse_header content)
 * ULOG sends complete link header (skb_mac_header content)
And they use the same field (raw.mac) for that. Database schema
needs mac.saddr.str and/or mac.daddr.str fields. With datatype set
0 only mac.daddr is logged. Both fields are filled when datatype is
set to 1. hwaddrlen param can be used to change the length of
link header address (default is 6 which is used by ethernet header).

Signed-off-by: Eric Leblond <eric@inl.fr>
---
 filter/ulogd_filter_MAC2STR.c |  117 ++++++++++++++++++++++++++++++++++------
 ulogd.conf.in                 |    9 +++
 2 files changed, 108 insertions(+), 18 deletions(-)

diff --git a/filter/ulogd_filter_MAC2STR.c b/filter/ulogd_filter_MAC2STR.c
index 0035886..0d09baa 100644
--- a/filter/ulogd_filter_MAC2STR.c
+++ b/filter/ulogd_filter_MAC2STR.c
@@ -30,6 +30,34 @@
 
 #define IPADDR_LENGTH 128
 
+enum mac2str_kset {
+	MAC2STR_LEN,
+	MAC2STR_TYPE,
+};
+
+enum mac2str_datatype {
+	MAC2STR_ONLY_SMAC,
+	MAC2STR_FULL_MAC,
+};
+
+static struct config_keyset libulog_kset = {
+	.num_ces = 2,
+	.ces = {
+		[MAC2STR_LEN] = {
+			.key 	 = "hwaddrlen",
+			.type 	 = CONFIG_TYPE_INT,
+			.options = CONFIG_OPT_NONE,
+			.u.value = 6,
+		},
+		[MAC2STR_TYPE] = {
+			.key 	 = "datatype",
+			.type 	 = CONFIG_TYPE_INT,
+			.options = CONFIG_OPT_NONE,
+			.u.value = MAC2STR_ONLY_SMAC,
+		}
+	}
+};
+
 enum input_keys {
 	KEY_RAW_MAC,
 	KEY_RAW_MACLEN,
@@ -37,6 +65,7 @@ enum input_keys {
 
 enum output_keys {
 	KEY_MAC_SADDR,
+	KEY_MAC_DADDR,
 };
 
 static struct ulogd_key mac2str_inp[] = {
@@ -45,22 +74,48 @@ static struct ulogd_key mac2str_inp[] = {
 		.flags = ULOGD_RETF_NONE,
 		.name = "raw.mac",
 	},
-	[KEY_RAW_MACLEN] = { 
-		.type = ULOGD_RET_UINT16, 
-		.flags = ULOGD_RETF_NONE, 
-		.name = "raw.mac_len", 
+	[KEY_RAW_MACLEN] = {
+		.type = ULOGD_RET_UINT16,
+		.flags = ULOGD_RETF_NONE,
+		.name = "raw.mac_len",
 	},
 
 };
 
 static struct ulogd_key mac2str_keys[] = {
-	{
+	[KEY_MAC_SADDR] = {
 		.type = ULOGD_RET_STRING,
 		.flags = ULOGD_RETF_FREE,
 		.name = "mac.saddr.str",
 	},
+	[KEY_MAC_DADDR] = {
+		.type = ULOGD_RET_STRING,
+		.flags = ULOGD_RETF_FREE,
+		.name = "mac.daddr.str",
+	},
 };
 
+static int parse_header(struct ulogd_key *ret, unsigned char *mac, int hwlen, int type)
+{
+	char *mac_str = NULL;
+	char *buf_cur = NULL;
+	mac_str = calloc(hwlen/sizeof(char)*3, sizeof(char));
+	buf_cur = mac_str;
+	int i;
+
+	if (mac_str == NULL)
+		return ULOGD_IRET_ERR;
+
+	for (i = 0; i < hwlen; i++)
+		buf_cur += sprintf(buf_cur, "%02x%c", mac[i],
+				i == hwlen - 1 ? 0 : ':');
+
+	ret[type].u.value.ptr = mac_str;
+	ret[type].flags |= ULOGD_RETF_VALID;
+
+	return ULOGD_IRET_OK;
+}
+
 static int interp_mac2str(struct ulogd_pluginstance *pi)
 {
 	struct ulogd_key *ret = pi->output.keys;
@@ -69,24 +124,48 @@ static int interp_mac2str(struct ulogd_pluginstance *pi)
 	if (pp_is_valid(inp, KEY_RAW_MAC)) {
 		unsigned char *mac = (unsigned char *) GET_VALUE(inp, KEY_RAW_MAC).ptr;
 		int len = GET_VALUE(inp, KEY_RAW_MACLEN).ui16;
-		char *mac_str = calloc(len/sizeof(char)*3, sizeof(char));
-		char *buf_cur = mac_str;
-		int i;
-		
-		if (mac_str == NULL)
-			return ULOGD_IRET_ERR;
-
-		for (i = 0; i < len; i++)
-			buf_cur += sprintf(buf_cur, "%02x%c", mac[i],
-					   i == len - 1 ? 0 : ':');
-
-		ret[KEY_MAC_SADDR].u.value.ptr = mac_str;
-		ret[KEY_MAC_SADDR].flags |= ULOGD_RETF_VALID;
+		int hwlen = pi->config_kset->ces[MAC2STR_LEN].u.value;
+		int retp;
+
+		if (hwlen) {
+			if (len < hwlen)
+				return ULOGD_IRET_ERR;
+		} else {
+			hwlen = len;
+		}
+
+		switch (pi->config_kset->ces[MAC2STR_TYPE].u.value) {
+		case MAC2STR_ONLY_SMAC:
+			retp = parse_header(ret, mac, hwlen, KEY_MAC_SADDR);
+			if (retp != ULOGD_IRET_OK)
+				return retp;
+			break;
+		case MAC2STR_FULL_MAC:
+			retp = parse_header(ret, mac, hwlen, KEY_MAC_DADDR);
+			if (retp != ULOGD_IRET_OK)
+				return retp;
+			retp = parse_header(ret, mac + hwlen, hwlen, KEY_MAC_SADDR);
+			if (retp != ULOGD_IRET_OK)
+				return retp;
+			break;
+		default:
+			break;
+		}
 	}
 
 	return ULOGD_IRET_OK;
 }
 
+static int configure(struct ulogd_pluginstance *upi,
+		     struct ulogd_pluginstance_stack *stack)
+{
+	ulogd_log(ULOGD_DEBUG, "parsing config file section `%s', "
+		  "plugin `%s'\n", upi->id, upi->plugin->name);
+
+	config_parse_file(upi->id, upi->config_kset);
+	return 0;
+}
+
 static struct ulogd_plugin mac2str_pluging = {
 	.name = "MAC2STR",
 	.input = {
@@ -100,6 +179,8 @@ static struct ulogd_plugin mac2str_pluging = {
 		.type = ULOGD_DTYPE_PACKET,
 		},
 	.interp = &interp_mac2str,
+	.config_kset = &libulog_kset,
+	.configure = &configure,
 	.version = ULOGD_VERSION,
 };
 
diff --git a/ulogd.conf.in b/ulogd.conf.in
index a32234d..c7714ee 100644
--- a/ulogd.conf.in
+++ b/ulogd.conf.in
@@ -188,3 +188,12 @@ sync = 1
 
 [mark1]
 mark = 1
+
+[mac2str1]
+# datatype:
+#  * Set to 0 for NFLOG version with saddr log only
+#  * set to 1 for ULOG and NFLOG version with full header log
+datatype = 0
+# Set hwaddrlen to length of hardware address (default 6 for ethernet)
+#hwaddrlen = 6
+
-- 
1.5.5.3

Re: [ULOGD PATCH 4/6] Introduce datatype and hwaddrlen param to MAC2STR plugin.

[Pablo Neira Ayuso]

Eric Leblond wrote:
> This patch introduces a hwaddrlen and datatype parameters in
> MAC2STR configuration. This is a workaround differences between
> ULOG and NFLOG:
>  * NFLOG sends MAC address in userspace (dev_parse_header content)
>  * ULOG sends complete link header (skb_mac_header content)
> And they use the same field (raw.mac) for that. Database schema
> needs mac.saddr.str and/or mac.daddr.str fields. With datatype set
> 0 only mac.daddr is logged. Both fields are filled when datatype is
> set to 1.

Why not split KEY_RAW_MAC into two keys in the input plugins, eg.
KEY_RAW_SMAC and KEY_RAW_DMAC? Thus, we inconditionally dump the source
and the destination MAC if they are present.

I'm not a big fun of adding options that can confuse users.

-- 
"Los honestos son inadaptados sociales" -- Les Luthiers

[ULOGD PATCH 0/6] rework mac address related issues

[Eric Leblond]

Hello,

This is a rewrite of my previous patchset following your suggestion to modify
input plugins instead of adding options the filter plugin.

The patches:
 "Specify that NFLOG only return mac saddr."
 "Output mac.saddr and mac.daddr in ULOG plugin."
modify the INPUT plugins to have them use raw.mac.saddr and raw.mac.daddr
fields. NFLOG code is trivial but ULOG patch is more difficult because ULOG
manually parses the hardware header to extract source and destination
addresses. This has raise the necessity to have an option to specify the length
of an hardware address in this plugin. The option is set by default to 6 which
is the default for almost all sort of interfaces.

Parsing in ULOG is completed by the patch:
 "Parse oob protocol in ULOG when possible."
It make ULOG parse the end of the hardware header to extrac the protocol.

The patch "Adapt MAC2STR to NFLOG and ULOG input key change." is a simple
adaptation of MAC2STR plugin to the new keys.

Last two patches:
 "Add destination mac_addr to database."
 "Sync PRINTPKT with mac modification."
sync output plugins with these modifications.

Patchset statistics:
 doc/mysql-ulogd2.sql              |   32 ++++++++++-------
 doc/pgsql-ulogd2.sql              |   17 +++++++--
 filter/ulogd_filter_MAC2STR.c     |   69 +++++++++++++++++++++++++-----------
 include/ulogd/printpkt.h          |    3 +-
 input/packet/ulogd_inppkt_NFLOG.c |   10 +++---
 input/packet/ulogd_inppkt_ULOG.c  |   48 +++++++++++++++++++------
 util/printpkt.c                   |   26 +++++++++++---
 7 files changed, 143 insertions(+), 62 deletions(-)

BR,
--
Eric Leblond
INL: http://www.inl.fr/
NuFW: http://www.nufw.org/

[ULOGD PATCH 1/6] Specify that NFLOG only return mac saddr.

[Eric Leblond]

The following patch modifies NFLOG to return a key raw.mac.saddr as NFLOG
kernel module only returns this field.

Signed-off-by: Eric Leblond <eric@inl.fr>
---
 input/packet/ulogd_inppkt_NFLOG.c |   10 +++++-----
 1 files changed, 5 insertions(+), 5 deletions(-)

diff --git a/input/packet/ulogd_inppkt_NFLOG.c b/input/packet/ulogd_inppkt_NFLOG.c
index 728b410..f239e42 100644
--- a/input/packet/ulogd_inppkt_NFLOG.c
+++ b/input/packet/ulogd_inppkt_NFLOG.c
@@ -114,7 +114,7 @@ static struct config_keyset libulog_kset = {
 #define nlsockbufmaxsize_ce(x) (x->ces[9])
 
 enum nflog_keys {
-	NFLOG_KEY_RAW_MAC = 0,
+	NFLOG_KEY_RAW_MAC_SADDR = 0,
 	NFLOG_KEY_RAW_PCKT,
 	NFLOG_KEY_RAW_PCKTLEN,
 	NFLOG_KEY_RAW_PCKTCOUNT,
@@ -136,10 +136,10 @@ enum nflog_keys {
 };
 
 static struct ulogd_key output_keys[] = {
-	[NFLOG_KEY_RAW_MAC] = {
+	[NFLOG_KEY_RAW_MAC_SADDR] = {
 		.type = ULOGD_RET_RAW,
 		.flags = ULOGD_RETF_NONE,
-		.name = "raw.mac",
+		.name = "raw.mac.saddr",
 		.ipfix = {
 			.vendor = IPFIX_VENDOR_IETF,
 			.field_id = IPFIX_sourceMacAddress,
@@ -319,8 +319,8 @@ interp_packet(struct ulogd_pluginstance *upi, struct nflog_data *ldata)
 	}
 
 	if (hw) {
-		ret[NFLOG_KEY_RAW_MAC].u.value.ptr = hw->hw_addr;
-		ret[NFLOG_KEY_RAW_MAC].flags |= ULOGD_RETF_VALID;
+		ret[NFLOG_KEY_RAW_MAC_SADDR].u.value.ptr = hw->hw_addr;
+		ret[NFLOG_KEY_RAW_MAC_SADDR].flags |= ULOGD_RETF_VALID;
 		ret[NFLOG_KEY_RAW_MAC_LEN].u.value.ui16 = ntohs(hw->hw_addrlen);
 		ret[NFLOG_KEY_RAW_MAC_LEN].flags |= ULOGD_RETF_VALID;
 	}
-- 
1.5.4.3

[ULOGD PATCH 2/6] Adapt MAC2STR to NFLOG and ULOG input key change.

[Eric Leblond]

MAC2STR was only dealing with the raw.mac key. This patch modifies
its behaviour to be able to convert raw.mac.saddr and raw.mac.daddr
fields.

Signed-off-by: Eric Leblond <eric@inl.fr>
---
 filter/ulogd_filter_MAC2STR.c |   69 ++++++++++++++++++++++++++++------------
 1 files changed, 48 insertions(+), 21 deletions(-)

diff --git a/filter/ulogd_filter_MAC2STR.c b/filter/ulogd_filter_MAC2STR.c
index 0035886..bb40c2b 100644
--- a/filter/ulogd_filter_MAC2STR.c
+++ b/filter/ulogd_filter_MAC2STR.c
@@ -31,19 +31,26 @@
 #define IPADDR_LENGTH 128
 
 enum input_keys {
-	KEY_RAW_MAC,
+	KEY_RAW_MAC_SADDR,
+	KEY_RAW_MAC_DADDR,
 	KEY_RAW_MACLEN,
 };
 
 enum output_keys {
 	KEY_MAC_SADDR,
+	KEY_MAC_DADDR,
 };
 
 static struct ulogd_key mac2str_inp[] = {
-	[KEY_RAW_MAC] = {
+	[KEY_RAW_MAC_SADDR] = {
 		.type = ULOGD_RET_RAW,
-		.flags = ULOGD_RETF_NONE,
-		.name = "raw.mac",
+		.flags = ULOGD_RETF_NONE|ULOGD_KEYF_OPTIONAL,
+		.name = "raw.mac.saddr",
+	},
+	[KEY_RAW_MAC_DADDR] = {
+		.type = ULOGD_RET_RAW,
+		.flags = ULOGD_RETF_NONE|ULOGD_KEYF_OPTIONAL,
+		.name = "raw.mac.daddr",
 	},
 	[KEY_RAW_MACLEN] = { 
 		.type = ULOGD_RET_UINT16, 
@@ -54,34 +61,54 @@ static struct ulogd_key mac2str_inp[] = {
 };
 
 static struct ulogd_key mac2str_keys[] = {
-	{
+	[KEY_MAC_SADDR] = {
 		.type = ULOGD_RET_STRING,
 		.flags = ULOGD_RETF_FREE,
 		.name = "mac.saddr.str",
 	},
+	[KEY_MAC_DADDR] = {
+		.type = ULOGD_RET_STRING,
+		.flags = ULOGD_RETF_FREE,
+		.name = "mac.daddr.str",
+	},
 };
 
+static int parse_mac2str(struct ulogd_key *ret, struct ulogd_key *inp, int key)
+{
+	unsigned char *mac = (unsigned char *) GET_VALUE(inp, key).ptr;
+	int len = GET_VALUE(inp, KEY_RAW_MACLEN).ui16;
+	char *mac_str = calloc(len/sizeof(char)*3, sizeof(char));
+	char *buf_cur = mac_str;
+	int i;
+
+	if (mac_str == NULL)
+		return ULOGD_IRET_ERR;
+
+	for (i = 0; i < len; i++)
+		buf_cur += sprintf(buf_cur, "%02x%c", mac[i],
+				i == len - 1 ? 0 : ':');
+
+	ret[key].u.value.ptr = mac_str;
+	ret[key].flags |= ULOGD_RETF_VALID;
+
+	return ULOGD_IRET_OK;
+}
+
 static int interp_mac2str(struct ulogd_pluginstance *pi)
 {
 	struct ulogd_key *ret = pi->output.keys;
 	struct ulogd_key *inp = pi->input.keys;
+	int retc;
 
-	if (pp_is_valid(inp, KEY_RAW_MAC)) {
-		unsigned char *mac = (unsigned char *) GET_VALUE(inp, KEY_RAW_MAC).ptr;
-		int len = GET_VALUE(inp, KEY_RAW_MACLEN).ui16;
-		char *mac_str = calloc(len/sizeof(char)*3, sizeof(char));
-		char *buf_cur = mac_str;
-		int i;
-		
-		if (mac_str == NULL)
-			return ULOGD_IRET_ERR;
-
-		for (i = 0; i < len; i++)
-			buf_cur += sprintf(buf_cur, "%02x%c", mac[i],
-					   i == len - 1 ? 0 : ':');
-
-		ret[KEY_MAC_SADDR].u.value.ptr = mac_str;
-		ret[KEY_MAC_SADDR].flags |= ULOGD_RETF_VALID;
+	if (pp_is_valid(inp, KEY_RAW_MAC_SADDR)) {
+		retc = parse_mac2str(ret, inp, KEY_RAW_MAC_SADDR);
+		if (retc != ULOGD_IRET_OK)
+			return retc;
+	}
+	if (pp_is_valid(inp, KEY_RAW_MAC_DADDR)) {
+		retc = parse_mac2str(ret, inp, KEY_RAW_MAC_DADDR);
+		if (retc != ULOGD_IRET_OK)
+			return retc;
 	}
 
 	return ULOGD_IRET_OK;
-- 
1.5.4.3

[ULOGD PATCH 3/6] Output mac.saddr and mac.daddr in ULOG plugin.

[Eric Leblond]

This patch modifies output key of ULOG by providing a parsing of
source and destination mac address instead of having a simply
sending raw.mac through the stack.
The introduction of the config key "hwaddrlen" was necessary to be able
to know the length of an hardware address which is used to compute the
length of an address and the offset of destination address

Signed-off-by: Eric Leblond <eric@inl.fr>
---
 input/packet/ulogd_inppkt_ULOG.c |   37 ++++++++++++++++++++++++++++---------
 1 files changed, 28 insertions(+), 9 deletions(-)

diff --git a/input/packet/ulogd_inppkt_ULOG.c b/input/packet/ulogd_inppkt_ULOG.c
index c00d9bf..8abaa60 100644
--- a/input/packet/ulogd_inppkt_ULOG.c
+++ b/input/packet/ulogd_inppkt_ULOG.c
@@ -35,7 +35,7 @@ struct ulog_input {
 /* configuration entries */
 
 static struct config_keyset libulog_kset = {
-	.num_ces = 4,
+	.num_ces = 5,
 	.ces = {
 	{
 		.key 	 = "bufsize",
@@ -61,11 +61,17 @@ static struct config_keyset libulog_kset = {
 		.options = CONFIG_OPT_NONE,
 		.u.value = 0,
 	},
-
+	{
+		.key     = "hwaddrlen",
+		.type    = CONFIG_TYPE_INT,
+		.options = CONFIG_OPT_NONE,
+		.u.value = 6, /* ETH_ALEN */
+	},
 	}
 };
 enum ulog_keys {
-	ULOG_KEY_RAW_MAC = 0,
+	ULOG_KEY_RAW_MAC_SADDR = 0,
+	ULOG_KEY_RAW_MAC_DADDR,
 	ULOG_KEY_RAW_PCKT,
 	ULOG_KEY_RAW_PCKTLEN,
 	ULOG_KEY_RAW_PCKTCOUNT,
@@ -83,15 +89,24 @@ enum ulog_keys {
 };
 
 static struct ulogd_key output_keys[] = {
-	[ULOG_KEY_RAW_MAC] = {
+	[ULOG_KEY_RAW_MAC_SADDR] = {
 		.type = ULOGD_RET_RAW,
 		.flags = ULOGD_RETF_NONE,
-		.name = "raw.mac",
+		.name = "raw.mac.saddr",
 		.ipfix = {
 			.vendor = IPFIX_VENDOR_IETF,
 			.field_id = IPFIX_sourceMacAddress,
 		},
 	},
+	[ULOG_KEY_RAW_MAC_DADDR] = {
+		.type = ULOGD_RET_RAW,
+		.flags = ULOGD_RETF_NONE,
+		.name = "raw.mac.daddr",
+		.ipfix = {
+			.vendor = IPFIX_VENDOR_IETF,
+			.field_id = IPFIX_destinationMacAddress,
+		},
+	},
 	[ULOG_KEY_RAW_PCKT] = {
 		.type = ULOGD_RET_RAW,
 		.flags = ULOGD_RETF_NONE,
@@ -189,10 +204,14 @@ static int interp_packet(struct ulogd_pluginstance *ip, ulog_packet_msg_t *pkt)
 {
 	struct ulogd_key *ret = ip->output.keys;
 
-	if (pkt->mac_len) {
-		ret[ULOG_KEY_RAW_MAC].u.value.ptr = pkt->mac;
-		ret[ULOG_KEY_RAW_MAC].flags |= ULOGD_RETF_VALID;
-		ret[ULOG_KEY_RAW_MAC_LEN].u.value.ui16 = pkt->mac_len;
+	if (pkt->mac_len >= 2 * ip->config_kset->ces[4].u.value) {
+		ret[ULOG_KEY_RAW_MAC_DADDR].u.value.ptr = pkt->mac;
+		ret[ULOG_KEY_RAW_MAC_DADDR].flags |= ULOGD_RETF_VALID;
+		ret[ULOG_KEY_RAW_MAC_SADDR].u.value.ptr = pkt->mac +
+			ip->config_kset->ces[4].u.value;
+		ret[ULOG_KEY_RAW_MAC_SADDR].flags |= ULOGD_RETF_VALID;
+		ret[ULOG_KEY_RAW_MAC_LEN].u.value.ui16 =
+			ip->config_kset->ces[4].u.value;
 		ret[ULOG_KEY_RAW_MAC_LEN].flags |= ULOGD_RETF_VALID;
 	}
 
-- 
1.5.4.3

[ULOGD PATCH] Output mac.saddr, mac.daddr and oob.protocol in ULOG plugin.

[Eric Leblond]

Hello,

Following live discussion with Pablo, here's a resend of my patch
implementing his idea about header parsing. This permits to get rid
of the addition of a new configuration variable in ULOG plugin.

This patch modifies output key of ULOG by providing a parsing of
source, destination mac address and hardware protocol instead
of having a simply sending raw.mac through the stack.

This patch replaces 3 and 4/6 of my previous patchset.

BR,

Signed-off-by: Eric Leblond <eric@inl.fr>
---
 input/packet/ulogd_inppkt_ULOG.c |   50 ++++++++++++++++++++++++++++---------
 1 files changed, 38 insertions(+), 12 deletions(-)

diff --git a/input/packet/ulogd_inppkt_ULOG.c b/input/packet/ulogd_inppkt_ULOG.c
index c00d9bf..7978eb6 100644
--- a/input/packet/ulogd_inppkt_ULOG.c
+++ b/input/packet/ulogd_inppkt_ULOG.c
@@ -65,7 +65,8 @@ static struct config_keyset libulog_kset = {
 	}
 };
 enum ulog_keys {
-	ULOG_KEY_RAW_MAC = 0,
+	ULOG_KEY_RAW_MAC_SADDR = 0,
+	ULOG_KEY_RAW_MAC_DADDR,
 	ULOG_KEY_RAW_PCKT,
 	ULOG_KEY_RAW_PCKTLEN,
 	ULOG_KEY_RAW_PCKTCOUNT,
@@ -83,15 +84,24 @@ enum ulog_keys {
 };
 
 static struct ulogd_key output_keys[] = {
-	[ULOG_KEY_RAW_MAC] = {
+	[ULOG_KEY_RAW_MAC_SADDR] = {
 		.type = ULOGD_RET_RAW,
 		.flags = ULOGD_RETF_NONE,
-		.name = "raw.mac",
+		.name = "raw.mac.saddr",
 		.ipfix = {
 			.vendor = IPFIX_VENDOR_IETF,
 			.field_id = IPFIX_sourceMacAddress,
 		},
 	},
+	[ULOG_KEY_RAW_MAC_DADDR] = {
+		.type = ULOGD_RET_RAW,
+		.flags = ULOGD_RETF_NONE,
+		.name = "raw.mac.daddr",
+		.ipfix = {
+			.vendor = IPFIX_VENDOR_IETF,
+			.field_id = IPFIX_destinationMacAddress,
+		},
+	},
 	[ULOG_KEY_RAW_PCKT] = {
 		.type = ULOGD_RET_RAW,
 		.flags = ULOGD_RETF_NONE,
@@ -188,13 +198,32 @@ static struct ulogd_key output_keys[] = {
 static int interp_packet(struct ulogd_pluginstance *ip, ulog_packet_msg_t *pkt)
 {
 	struct ulogd_key *ret = ip->output.keys;
-
-	if (pkt->mac_len) {
-		ret[ULOG_KEY_RAW_MAC].u.value.ptr = pkt->mac;
-		ret[ULOG_KEY_RAW_MAC].flags |= ULOGD_RETF_VALID;
-		ret[ULOG_KEY_RAW_MAC_LEN].u.value.ui16 = pkt->mac_len;
-		ret[ULOG_KEY_RAW_MAC_LEN].flags |= ULOGD_RETF_VALID;
-	}
+	int hwlen = 0;
+
+	/* compute hardware header length hwlen from mac_len, we suppose
+	 * header field is [ HW DST ADDR | HW SRC ADDR | PROTO ] with
+	 * proto code on two bytes.
+	 * Thus, we have:
+	 * 	hwlen = (mac_len - 2) / 2
+	 */
+	if ((pkt->mac_len % 2) || (pkt->mac_len < 2)) {
+		ulogd_log(ULOGD_NOTICE, "Invalid  mac_len (%d), rejecting"
+			  " packet", pkt->mac_len);
+		return ULOGD_IRET_ERR;
+	} else
+		hwlen = (pkt->mac_len - 2) / 2;
+
+	ret[ULOG_KEY_RAW_MAC_DADDR].u.value.ptr = pkt->mac;
+	ret[ULOG_KEY_RAW_MAC_DADDR].flags |= ULOGD_RETF_VALID;
+	ret[ULOG_KEY_RAW_MAC_SADDR].u.value.ptr = pkt->mac + hwlen;
+	ret[ULOG_KEY_RAW_MAC_SADDR].flags |= ULOGD_RETF_VALID;
+
+	ret[ULOG_KEY_RAW_MAC_LEN].u.value.ui16 = hwlen;
+	ret[ULOG_KEY_RAW_MAC_LEN].flags |= ULOGD_RETF_VALID;
+
+	ret[ULOG_KEY_OOB_PROTOCOL].u.value.ui16 = ntohs(
+			*(uint16_t *)(pkt->mac + 2 * hwlen));
+	ret[ULOG_KEY_OOB_PROTOCOL].flags |= ULOGD_RETF_VALID;
 
 	ret[ULOG_KEY_RAW_LABEL].u.value.ui8 = ip->config_kset->ces[3].u.value;
 	ret[ULOG_KEY_RAW_LABEL].flags |= ULOGD_RETF_VALID;
@@ -235,9 +264,6 @@ static int interp_packet(struct ulogd_pluginstance *ip, ulog_packet_msg_t *pkt)
 	/* ULOG is IPv4 only */
 	ret[ULOG_KEY_OOB_FAMILY].u.value.ui8 = AF_INET;
 	ret[ULOG_KEY_OOB_FAMILY].flags |= ULOGD_RETF_VALID;
-	/* Undef in ULOG but necessary */
-	ret[ULOG_KEY_OOB_PROTOCOL].u.value.ui16 = 0;
-	ret[ULOG_KEY_OOB_PROTOCOL].flags |= ULOGD_RETF_VALID;
 
 	ulogd_propagate_results(ip);
 	return 0;
-- 
1.5.4.3

Re: [ULOGD PATCH] Output mac.saddr, mac.daddr and oob.protocol in ULOG plugin.

[Patrick McHardy]

Eric Leblond wrote:
> +	/* compute hardware header length hwlen from mac_len, we suppose
> +	 * header field is [ HW DST ADDR | HW SRC ADDR | PROTO ] with
> +	 * proto code on two bytes.
> +	 * Thus, we have:
> +	 * 	hwlen = (mac_len - 2) / 2
> +	 */
> +	if ((pkt->mac_len % 2) || (pkt->mac_len < 2)) {
> +		ulogd_log(ULOGD_NOTICE, "Invalid  mac_len (%d), rejecting"
> +			  " packet", pkt->mac_len);
> +		return ULOGD_IRET_ERR;
> +	} else
> +		hwlen = (pkt->mac_len - 2) / 2;


Thats not correct. The mac header (which is badly named) includes
the entire device specific header, for IPIP tunnels for example its
the outer IP header. So this needs device specific logic.

[ULOGD PATCH] rework, fill MAC address in ULOG for ethernet.

[Eric Leblond]

This patch introduces a parsing of the hardware header field based on
the length of the field. It currently only detects ethernet header and
fill mac.saddr and mac.daddr properly.

With this behaviour it may be impossible to support all kind of devices
but ULOG will soon be deprecated in favor of NFLOG.

Signed-off-by: Eric Leblond <eric@inl.fr>
---
 input/packet/ulogd_inppkt_ULOG.c |   68 +++++++++++++++++++++++++++++++-------
 1 files changed, 56 insertions(+), 12 deletions(-)

diff --git a/input/packet/ulogd_inppkt_ULOG.c b/input/packet/ulogd_inppkt_ULOG.c
index c00d9bf..742c43f 100644
--- a/input/packet/ulogd_inppkt_ULOG.c
+++ b/input/packet/ulogd_inppkt_ULOG.c
@@ -7,6 +7,7 @@
 #include <stdlib.h>
 #include <arpa/inet.h>
 #include <string.h>
+#include <linux/if_ether.h>
 
 
 #include <ulogd/ulogd.h>
@@ -65,7 +66,8 @@ static struct config_keyset libulog_kset = {
 	}
 };
 enum ulog_keys {
-	ULOG_KEY_RAW_MAC = 0,
+	ULOG_KEY_RAW_MAC_SADDR = 0,
+	ULOG_KEY_RAW_MAC_DADDR,
 	ULOG_KEY_RAW_PCKT,
 	ULOG_KEY_RAW_PCKTLEN,
 	ULOG_KEY_RAW_PCKTCOUNT,
@@ -83,15 +85,24 @@ enum ulog_keys {
 };
 
 static struct ulogd_key output_keys[] = {
-	[ULOG_KEY_RAW_MAC] = {
+	[ULOG_KEY_RAW_MAC_SADDR] = {
 		.type = ULOGD_RET_RAW,
 		.flags = ULOGD_RETF_NONE,
-		.name = "raw.mac",
+		.name = "raw.mac.saddr",
 		.ipfix = {
 			.vendor = IPFIX_VENDOR_IETF,
 			.field_id = IPFIX_sourceMacAddress,
 		},
 	},
+	[ULOG_KEY_RAW_MAC_DADDR] = {
+		.type = ULOGD_RET_RAW,
+		.flags = ULOGD_RETF_NONE,
+		.name = "raw.mac.daddr",
+		.ipfix = {
+			.vendor = IPFIX_VENDOR_IETF,
+			.field_id = IPFIX_destinationMacAddress,
+		},
+	},
 	[ULOG_KEY_RAW_PCKT] = {
 		.type = ULOGD_RET_RAW,
 		.flags = ULOGD_RETF_NONE,
@@ -185,16 +196,52 @@ static struct ulogd_key output_keys[] = {
 
 };
 
+
+static int parse_macheader(struct ulogd_key *ret, ulog_packet_msg_t *pkt,
+			   )
+{
+	int hwlen;
+
+	switch (pkt->mac_len) {
+		case (2 * ETH_ALEN +2):
+			hwlen = ETH_ALEN;
+			break;
+		default:
+			ulogd_log(ULOGD_DEBUG, "Unknown mac_len (%d), "
+					"rejecting packet", pkt->mac_len);
+			ret[ULOG_KEY_OOB_PROTOCOL].u.value.ui16 = 0;
+			ret[ULOG_KEY_OOB_PROTOCOL].flags |= ULOGD_RETF_VALID;
+			return ULOGD_IRET_OK;
+	}
+
+
+	ret[ULOG_KEY_RAW_MAC_DADDR].u.value.ptr = pkt->mac;
+	ret[ULOG_KEY_RAW_MAC_DADDR].flags |= ULOGD_RETF_VALID;
+	ret[ULOG_KEY_RAW_MAC_SADDR].u.value.ptr = pkt->mac + hwlen;
+	ret[ULOG_KEY_RAW_MAC_SADDR].flags |= ULOGD_RETF_VALID;
+
+	ret[ULOG_KEY_RAW_MAC_LEN].u.value.ui16 = hwlen;
+	ret[ULOG_KEY_RAW_MAC_LEN].flags |= ULOGD_RETF_VALID;
+
+	ret[ULOG_KEY_OOB_PROTOCOL].u.value.ui16 = ntohs(
+			*(uint16_t *)(pkt->mac + 2 * hwlen));
+	ret[ULOG_KEY_OOB_PROTOCOL].flags |= ULOGD_RETF_VALID;
+
+	return ULOGD_IRET_OK;
+}
+
+
+
 static int interp_packet(struct ulogd_pluginstance *ip, ulog_packet_msg_t *pkt)
 {
 	struct ulogd_key *ret = ip->output.keys;
+	int hwlen = 0;
+	int fret;
 
-	if (pkt->mac_len) {
-		ret[ULOG_KEY_RAW_MAC].u.value.ptr = pkt->mac;
-		ret[ULOG_KEY_RAW_MAC].flags |= ULOGD_RETF_VALID;
-		ret[ULOG_KEY_RAW_MAC_LEN].u.value.ui16 = pkt->mac_len;
-		ret[ULOG_KEY_RAW_MAC_LEN].flags |= ULOGD_RETF_VALID;
-	}
+
+	fret = parse_macheader(ret, pkt);
+	if (fret != ULOGD_IRET_OK)
+		return fret;
 
 	ret[ULOG_KEY_RAW_LABEL].u.value.ui8 = ip->config_kset->ces[3].u.value;
 	ret[ULOG_KEY_RAW_LABEL].flags |= ULOGD_RETF_VALID;
@@ -235,9 +282,6 @@ static int interp_packet(struct ulogd_pluginstance *ip, ulog_packet_msg_t *pkt)
 	/* ULOG is IPv4 only */
 	ret[ULOG_KEY_OOB_FAMILY].u.value.ui8 = AF_INET;
 	ret[ULOG_KEY_OOB_FAMILY].flags |= ULOGD_RETF_VALID;
-	/* Undef in ULOG but necessary */
-	ret[ULOG_KEY_OOB_PROTOCOL].u.value.ui16 = 0;
-	ret[ULOG_KEY_OOB_PROTOCOL].flags |= ULOGD_RETF_VALID;
 
 	ulogd_propagate_results(ip);
 	return 0;
-- 
1.5.4.3

Re: [ULOGD PATCH] rework, fill MAC address in ULOG for ethernet.

[Pablo Neira Ayuso]

Eric Leblond wrote:
> This patch introduces a parsing of the hardware header field based on
> the length of the field. It currently only detects ethernet header and
> fill mac.saddr and mac.daddr properly.
> 
> With this behaviour it may be impossible to support all kind of devices
> but ULOG will soon be deprecated in favor of NFLOG.

Since we do not have more information in ULOG, I see this as a best try
to detect what kind of layer 2 header is there. The other choice is not
to include layer 2 information at all if we use ulog as input which also
seems reasonable to me. If users want new features they have to migrate
from ulog to nflog IMO.

I'm willing to finish the compatibility layer in libnetfilter_queue,
this could probably help to deprecate it.

If Patrick does not have any objection, I'll apply this and the previous
layer 2 related patches.

-- 
"Los honestos son inadaptados sociales" -- Les Luthiers

Re: [ULOGD PATCH] rework, fill MAC address in ULOG for ethernet.

[Patrick McHardy]

Pablo Neira Ayuso wrote:
> Eric Leblond wrote:
>> This patch introduces a parsing of the hardware header field based on
>> the length of the field. It currently only detects ethernet header and
>> fill mac.saddr and mac.daddr properly.
>>
>> With this behaviour it may be impossible to support all kind of devices
>> but ULOG will soon be deprecated in favor of NFLOG.
> 
> Since we do not have more information in ULOG, I see this as a best try
> to detect what kind of layer 2 header is there. The other choice is not
> to include layer 2 information at all if we use ulog as input which also
> seems reasonable to me. If users want new features they have to migrate
> from ulog to nflog IMO.
> 
> I'm willing to finish the compatibility layer in libnetfilter_queue,
> this could probably help to deprecate it.
> 
> If Patrick does not have any objection, I'll apply this and the previous
> layer 2 related patches.

One objection:

> +static int parse_macheader(struct ulogd_key *ret, ulog_packet_msg_t *pkt,
> +			   )
> +{
> +	int hwlen;
> +
> +	switch (pkt->mac_len) {
> +		case (2 * ETH_ALEN +2):
> +			hwlen = ETH_ALEN;
> +			break;
> +		default:
> +			ulogd_log(ULOGD_DEBUG, "Unknown mac_len (%d), "
> +					"rejecting packet", pkt->mac_len);
> +			ret[ULOG_KEY_OOB_PROTOCOL].u.value.ui16 = 0;
> +			ret[ULOG_KEY_OOB_PROTOCOL].flags |= ULOGD_RETF_VALID;
> +			return ULOGD_IRET_OK;

This appears to be breaking logging for anything but ethernet
packets. We can't do that, especially since ulogd1 has long
been in maintenance-only mode.

Re: [ULOGD PATCH] rework, fill MAC address in ULOG for ethernet.

[Pablo Neira Ayuso]

Patrick McHardy wrote:
> Pablo Neira Ayuso wrote:
>> Eric Leblond wrote:
>>> This patch introduces a parsing of the hardware header field based on
>>> the length of the field. It currently only detects ethernet header and
>>> fill mac.saddr and mac.daddr properly.
>>>
>>> With this behaviour it may be impossible to support all kind of devices
>>> but ULOG will soon be deprecated in favor of NFLOG.
>>
>> Since we do not have more information in ULOG, I see this as a best try
>> to detect what kind of layer 2 header is there. The other choice is not
>> to include layer 2 information at all if we use ulog as input which also
>> seems reasonable to me. If users want new features they have to migrate
>> from ulog to nflog IMO.
>>
>> I'm willing to finish the compatibility layer in libnetfilter_queue,
>> this could probably help to deprecate it.
>>
>> If Patrick does not have any objection, I'll apply this and the previous
>> layer 2 related patches.
> 
> One objection:
> 
>> +static int parse_macheader(struct ulogd_key *ret, ulog_packet_msg_t
>> *pkt,
>> +               )
>> +{
>> +    int hwlen;
>> +
>> +    switch (pkt->mac_len) {
>> +        case (2 * ETH_ALEN +2):
>> +            hwlen = ETH_ALEN;
>> +            break;
>> +        default:
>> +            ulogd_log(ULOGD_DEBUG, "Unknown mac_len (%d), "
>> +                    "rejecting packet", pkt->mac_len);
>> +            ret[ULOG_KEY_OOB_PROTOCOL].u.value.ui16 = 0;
>> +            ret[ULOG_KEY_OOB_PROTOCOL].flags |= ULOGD_RETF_VALID;
>> +            return ULOGD_IRET_OK;
> 
> This appears to be breaking logging for anything but ethernet
> packets. We can't do that, especially since ulogd1 has long
> been in maintenance-only mode.

Hm, this patch applies to ulogd2, probably you got confused with the
subject?

But indeed, I agree with you. Eric, I think that it's better to drop any
effort in trying to add layer 2 support for ulogd2 if the user use ULOG
as input logger. If users want new features, eg. the layer 2 information
in their log messages, they should upgrade to NFLOG.

-- 
"Los honestos son inadaptados sociales" -- Les Luthiers

Re: [ULOGD PATCH] rework, fill MAC address in ULOG for ethernet.

[Patrick McHardy]

Pablo Neira Ayuso wrote:
> Patrick McHardy wrote:
>> Pablo Neira Ayuso wrote:
>>> Eric Leblond wrote:
>>>> This patch introduces a parsing of the hardware header field based on
>>>> the length of the field. It currently only detects ethernet header and
>>>> fill mac.saddr and mac.daddr properly.
>>>>
>>>> With this behaviour it may be impossible to support all kind of devices
>>>> but ULOG will soon be deprecated in favor of NFLOG.
>>> Since we do not have more information in ULOG, I see this as a best try
>>> to detect what kind of layer 2 header is there. The other choice is not
>>> to include layer 2 information at all if we use ulog as input which also
>>> seems reasonable to me. If users want new features they have to migrate
>>> from ulog to nflog IMO.
>>>
>>> I'm willing to finish the compatibility layer in libnetfilter_queue,
>>> this could probably help to deprecate it.
>>>
>>> If Patrick does not have any objection, I'll apply this and the previous
>>> layer 2 related patches.
>> One objection:
>>
>>> +static int parse_macheader(struct ulogd_key *ret, ulog_packet_msg_t
>>> *pkt,
>>> +               )
>>> +{
>>> +    int hwlen;
>>> +
>>> +    switch (pkt->mac_len) {
>>> +        case (2 * ETH_ALEN +2):
>>> +            hwlen = ETH_ALEN;
>>> +            break;
>>> +        default:
>>> +            ulogd_log(ULOGD_DEBUG, "Unknown mac_len (%d), "
>>> +                    "rejecting packet", pkt->mac_len);
>>> +            ret[ULOG_KEY_OOB_PROTOCOL].u.value.ui16 = 0;
>>> +            ret[ULOG_KEY_OOB_PROTOCOL].flags |= ULOGD_RETF_VALID;
>>> +            return ULOGD_IRET_OK;
>> This appears to be breaking logging for anything but ethernet
>> packets. We can't do that, especially since ulogd1 has long
>> been in maintenance-only mode.
> 
> Hm, this patch applies to ulogd2, probably you got confused with the
> subject?

Indeed, I mixed that up.

> But indeed, I agree with you. Eric, I think that it's better to drop any
> effort in trying to add layer 2 support for ulogd2 if the user use ULOG
> as input logger. If users want new features, eg. the layer 2 information
> in their log messages, they should upgrade to NFLOG.

Agreed, I really don't see how this can be done without breaking
things. The necessary information is not present in the ULOG
messages.

Re: [ULOGD PATCH] rework, fill MAC address in ULOG for ethernet.

[Eric Leblond]

[-- Attachment #1: Type: text/plain, Size: 1520 bytes --]

Hello,

On Wednesday, 2008 July  9 at 13:10:34 +0200, Patrick McHardy wrote:
> Pablo Neira Ayuso wrote:
>> Patrick McHardy wrote:
>>> Pablo Neira Ayuso wrote:
>>>> +            ret[ULOG_KEY_OOB_PROTOCOL].u.value.ui16 = 0;
>>>> +            ret[ULOG_KEY_OOB_PROTOCOL].flags |= ULOGD_RETF_VALID;
>>>> +            return ULOGD_IRET_OK;
>>> This appears to be breaking logging for anything but ethernet
>>> packets. We can't do that, especially since ulogd1 has long
>>> been in maintenance-only mode.
>>
>> Hm, this patch applies to ulogd2, probably you got confused with the
>> subject?
>
> Indeed, I mixed that up.
>
>> But indeed, I agree with you. Eric, I think that it's better to drop any
>> effort in trying to add layer 2 support for ulogd2 if the user use ULOG
>> as input logger. If users want new features, eg. the layer 2 information
>> in their log messages, they should upgrade to NFLOG.
>
> Agreed, I really don't see how this can be done without breaking
> things. The necessary information is not present in the ULOG
> messages.

I think the proposed patch is a best effort work. It should be enough
for most users as ethernet headers are the more common. But I agree, we
can't have a full working solution with ULOG.

I will try to find some time to rework my NFLOG patch to see if it is
possible to cleanly send the source and destination hardware address to
userspace via NFLOG.

IMHO, this patch is not harmfull and will help some people. It may worth
to be push in the git tree.

BR,
--
Eric Leblond

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 189 bytes --]

Re: [ULOGD PATCH] rework, fill MAC address in ULOG for ethernet.

[Patrick McHardy]

Eric Leblond wrote:
> I think the proposed patch is a best effort work. It should be enough
> for most users as ethernet headers are the more common. But I agree, we
> can't have a full working solution with ULOG.
> 
> I will try to find some time to rework my NFLOG patch to see if it is
> possible to cleanly send the source and destination hardware address to
> userspace via NFLOG.
> 
> IMHO, this patch is not harmfull and will help some people. It may worth
> to be push in the git tree.

> +			ulogd_log(ULOGD_DEBUG, "Unknown mac_len (%d), "
> +					"rejecting packet", pkt->mac_len);

This comment made me believe that it would break header
printing for non-ethernet packets. If that is not true,
I have no objections.

Re: [ULOGD PATCH] rework, fill MAC address in ULOG for ethernet.

[Eric Leblond]

Hello,

On Wednesday, 2008 July  9 at 23:18:30 +0200, Patrick McHardy wrote:
> Eric Leblond wrote:
>> I think the proposed patch is a best effort work. It should be enough
>> for most users as ethernet headers are the more common. But I agree, we
>> can't have a full working solution with ULOG.
>>
>> I will try to find some time to rework my NFLOG patch to see if it is
>> possible to cleanly send the source and destination hardware address to
>> userspace via NFLOG.

I don't see any other mean but to get hardware address from device and
not from skb. I really don't like it, as this can be done from userspace
directly. I will thus stop working on adding destination address to
NFLOG messages.

>> IMHO, this patch is not harmfull and will help some people. It may worth
>> to be push in the git tree.
>
>> +			ulogd_log(ULOGD_DEBUG, "Unknown mac_len (%d), "
>> +					"rejecting packet", pkt->mac_len);
>
> This comment made me believe that it would break header
> printing for non-ethernet packets. If that is not true,
> I have no objections.

Hmm, you're right but user using non-ethernet packets can upgrade to
NFLOG ;)

BR,
-- 
Eric Leblond
INL: http://www.inl.fr/
NuFW: http://www.nufw.org/

Re: [ULOGD PATCH] rework, fill MAC address in ULOG for ethernet.

[Patrick McHardy]

Eric Leblond wrote:
> Hello,
> 
> On Wednesday, 2008 July  9 at 23:18:30 +0200, Patrick McHardy wrote:
>> Eric Leblond wrote:
>>> I think the proposed patch is a best effort work. It should be enough
>>> for most users as ethernet headers are the more common. But I agree, we
>>> can't have a full working solution with ULOG.
>>>
>>> I will try to find some time to rework my NFLOG patch to see if it is
>>> possible to cleanly send the source and destination hardware address to
>>> userspace via NFLOG.
> 
> I don't see any other mean but to get hardware address from device and
> not from skb. I really don't like it, as this can be done from userspace
> directly. I will thus stop working on adding destination address to
> NFLOG messages.
> 
>>> IMHO, this patch is not harmfull and will help some people. It may worth
>>> to be push in the git tree.
>>> +			ulogd_log(ULOGD_DEBUG, "Unknown mac_len (%d), "
>>> +					"rejecting packet", pkt->mac_len);
>> This comment made me believe that it would break header
>> printing for non-ethernet packets. If that is not true,
>> I have no objections.
> 
> Hmm, you're right but user using non-ethernet packets can upgrade to
> NFLOG ;)

I assume thats not meant seriously :) But we should integrate
new features into the new things, not break old things and
expect users to upgrade :)

Re: [ULOGD PATCH] rework, fill MAC address in ULOG for ethernet.

[Eric Leblond]

[-- Attachment #1: Type: text/plain, Size: 1448 bytes --]

Hello,

On Friday, 2008 July 11 at 16:08:50 +0200, Patrick McHardy wrote:
>>> This comment made me believe that it would break header
>>> printing for non-ethernet packets. If that is not true,
>>> I have no objections.
>>
>> Hmm, you're right but user using non-ethernet packets can upgrade to
>> NFLOG ;)
>
> I assume thats not meant seriously :) But we should integrate
> new features into the new things, not break old things and
> expect users to upgrade :)

Well, we still have a problem here:
 * ULOG logs raw hardware header
 * NFLOG logs source hardware address only
 * SQL output module only support real MAC address (at least for PGSQL which
 has a dedicated type)

I'm not able to find a clean and easy way to fix this.

Here's some possible solutions:
 * Modify mac type in database to support everything:
  * Performance impact
  * Different datatypes in same field :(
 * NFLOG modification to log full hardware header:
  * Will revert to a non parsable field :(
 * ULOG header parsing:
  * Incomplete and untrustable by design
  * May be able to retrieve source HW address info in most cases
  * ULOG could output fields raw.mac and raw.mac.saddr to ensure
  compatibility with older applications

IMHO last solution is the less worst one. If it seems ok for you, I will
provide a patchset implementing this solution.

BR,
-- 
Eric Leblond
INL: http://www.inl.fr/
NuFW: http://www.nufw.org/

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 197 bytes --]

Re: [ULOGD PATCH] rework, fill MAC address in ULOG for ethernet.

[Patrick McHardy]

Eric Leblond wrote:
> Hello,
> 
> On Friday, 2008 July 11 at 16:08:50 +0200, Patrick McHardy wrote:
>>>> This comment made me believe that it would break header
>>>> printing for non-ethernet packets. If that is not true,
>>>> I have no objections.
>>> Hmm, you're right but user using non-ethernet packets can upgrade to
>>> NFLOG ;)
>> I assume thats not meant seriously :) But we should integrate
>> new features into the new things, not break old things and
>> expect users to upgrade :)
> 
> Well, we still have a problem here:
>  * ULOG logs raw hardware header
>  * NFLOG logs source hardware address only
>  * SQL output module only support real MAC address (at least for PGSQL which
>  has a dedicated type)
> 
> I'm not able to find a clean and easy way to fix this.
> 
> Here's some possible solutions:
>  * Modify mac type in database to support everything:
>   * Performance impact
>   * Different datatypes in same field :(

It seems we already have a problem here since we can't put
other link layer headers in the database.

>  * NFLOG modification to log full hardware header:
>   * Will revert to a non parsable field :(

How about extending nfnetlink_log to include the full MAC header
and link layer type? That will make it a superset of ULOG.

>  * ULOG header parsing:
>   * Incomplete and untrustable by design
>   * May be able to retrieve source HW address info in most cases
>   * ULOG could output fields raw.mac and raw.mac.saddr to ensure
>   compatibility with older applications

Why don't you do something similar to your previous patch,
but instead of refusing unknown header lengths, treat
them as before?

> IMHO last solution is the less worst one. If it seems ok for you, I will
> provide a patchset implementing this solution.

I don't think I understand the problem well enough yet
to make that decision. Generally, I'm fine with anything
that doesn't break existing features and doesn't stand
in the way of correctly handling non-ethernet headers.

netfilter: Send complete hardware header in NFLOG.

[Eric Leblond]

This patch adds some fields to NFLOG to be able to send the complete
hardware header with all necessary informations.
It sends to userspace:
 * the type of hardware link
 * the lenght of hardware header
 * the hardware header
---
 include/linux/netfilter/nfnetlink_log.h |    3 +++
 net/netfilter/nfnetlink_log.c           |    8 ++++++++
 2 files changed, 11 insertions(+), 0 deletions(-)

diff --git a/include/linux/netfilter/nfnetlink_log.h b/include/linux/netfilter/nfnetlink_log.h
index a857213..f661731 100644
--- a/include/linux/netfilter/nfnetlink_log.h
+++ b/include/linux/netfilter/nfnetlink_log.h
@@ -48,6 +48,9 @@ enum nfulnl_attr_type {
 	NFULA_SEQ,			/* instance-local sequence number */
 	NFULA_SEQ_GLOBAL,		/* global sequence number */
 	NFULA_GID,			/* group id of socket */
+	NFULA_HWTYPE,			/* hardware type */
+	NFULA_HWHEADER,			/* hardware header */
+	NFULA_HWLEN,			/* hardware header length */
 
 	__NFULA_MAX
 };
diff --git a/net/netfilter/nfnetlink_log.c b/net/netfilter/nfnetlink_log.c
index b8173af..204dcae 100644
--- a/net/netfilter/nfnetlink_log.c
+++ b/net/netfilter/nfnetlink_log.c
@@ -453,6 +453,14 @@ __build_packet_message(struct nfulnl_instance *inst,
 		}
 	}
 
+	if (indev && skb_mac_header_was_set(skb)) {
+		NLA_PUT_U16(inst->skb, NFULA_HWTYPE, htons(skb->dev->type));
+		NLA_PUT_U16(inst->skb, NFULA_HWLEN,
+			    htons(skb->dev->hard_header_len));
+		NLA_PUT(inst->skb, NFULA_HWHEADER, skb->dev->hard_header_len,
+			skb_mac_header(skb));
+	}
+
 	if (skb->tstamp.tv64) {
 		struct nfulnl_msg_packet_timestamp ts;
 		struct timeval tv = ktime_to_timeval(skb->tstamp);
-- 
1.5.6

Re: netfilter: Send complete hardware header in NFLOG.

[Patrick McHardy]

[-- Attachment #1: Type: text/plain, Size: 395 bytes --]

Eric Leblond wrote:
> This patch adds some fields to NFLOG to be able to send the complete
> hardware header with all necessary informations.
> It sends to userspace:
>  * the type of hardware link
>  * the lenght of hardware header
>  * the hardware header

Looks good. I've applied it with a minor change to use the endian
aware netlink macros.

I need a Signed-off-by: line from you though.


[-- Attachment #2: x --]
[-- Type: text/plain, Size: 692 bytes --]

diff --git a/net/netfilter/nfnetlink_log.c b/net/netfilter/nfnetlink_log.c
index 204dcae..9a35b57 100644
--- a/net/netfilter/nfnetlink_log.c
+++ b/net/netfilter/nfnetlink_log.c
@@ -454,9 +454,9 @@ __build_packet_message(struct nfulnl_instance *inst,
 	}
 
 	if (indev && skb_mac_header_was_set(skb)) {
-		NLA_PUT_U16(inst->skb, NFULA_HWTYPE, htons(skb->dev->type));
-		NLA_PUT_U16(inst->skb, NFULA_HWLEN,
-			    htons(skb->dev->hard_header_len));
+		NLA_PUT_BE16(inst->skb, NFULA_HWTYPE, htons(skb->dev->type));
+		NLA_PUT_BE16(inst->skb, NFULA_HWLEN,
+			     htons(skb->dev->hard_header_len));
 		NLA_PUT(inst->skb, NFULA_HWHEADER, skb->dev->hard_header_len,
 			skb_mac_header(skb));
 	}

netfilter: Send complete hardware header in NFLOG.

[Eric Leblond]

This patch adds some fields to NFLOG to be able to send the complete
hardware header with all necessary informations.
It sends to userspace:
 * the type of hardware link
 * the length of hardware header
 * the hardware header

Signed-off-by: Eric Leblond <eric@inl.fr>
---
 include/linux/netfilter/nfnetlink_log.h |    3 +++
 net/netfilter/nfnetlink_log.c           |    8 ++++++++
 2 files changed, 11 insertions(+), 0 deletions(-)

diff --git a/include/linux/netfilter/nfnetlink_log.h b/include/linux/netfilter/nfnetlink_log.h
index a857213..f661731 100644
--- a/include/linux/netfilter/nfnetlink_log.h
+++ b/include/linux/netfilter/nfnetlink_log.h
@@ -48,6 +48,9 @@ enum nfulnl_attr_type {
 	NFULA_SEQ,			/* instance-local sequence number */
 	NFULA_SEQ_GLOBAL,		/* global sequence number */
 	NFULA_GID,			/* group id of socket */
+	NFULA_HWTYPE,			/* hardware type */
+	NFULA_HWHEADER,			/* hardware header */
+	NFULA_HWLEN,			/* hardware header length */
 
 	__NFULA_MAX
 };
diff --git a/net/netfilter/nfnetlink_log.c b/net/netfilter/nfnetlink_log.c
index b8173af..9a35b57 100644
--- a/net/netfilter/nfnetlink_log.c
+++ b/net/netfilter/nfnetlink_log.c
@@ -453,6 +453,14 @@ __build_packet_message(struct nfulnl_instance *inst,
 		}
 	}
 
+	if (indev && skb_mac_header_was_set(skb)) {
+		NLA_PUT_BE16(inst->skb, NFULA_HWTYPE, htons(skb->dev->type));
+		NLA_PUT_BE16(inst->skb, NFULA_HWLEN,
+			     htons(skb->dev->hard_header_len));
+		NLA_PUT(inst->skb, NFULA_HWHEADER, skb->dev->hard_header_len,
+			skb_mac_header(skb));
+	}
+
 	if (skb->tstamp.tv64) {
 		struct nfulnl_msg_packet_timestamp ts;
 		struct timeval tv = ktime_to_timeval(skb->tstamp);
-- 
1.5.4.3

Re: netfilter: Send complete hardware header in NFLOG.

[Patrick McHardy]

Eric Leblond wrote:
> This patch adds some fields to NFLOG to be able to send the complete
> hardware header with all necessary informations.
> It sends to userspace:
>  * the type of hardware link
>  * the length of hardware header
>  * the hardware header
> 
> Signed-off-by: Eric Leblond <eric@inl.fr>

Thanks, added to the commit message.

[libnetfilter_log PATCH] Add parsing function for raw hardware header.

[Eric Leblond]

This patch adds support for the parsing of NFULA_HWTYPE, NFULA_HWHEADER,
and NFULA_HWLEN part of the NFLOG message. This permits to have a ULOG
like behaviour but thanks to NFULA_HWTYPE we will be able to parse the
message cleanly in userspace.

Signed-off-by: Eric Leblond <eric@inl.fr>
---
 include/libnetfilter_log/libnetfilter_log.h    |    5 +++++
 include/libnetfilter_log/linux_nfnetlink_log.h |    3 +++
 src/libnetfilter_log.c                         |   16 ++++++++++++++++
 3 files changed, 24 insertions(+), 0 deletions(-)

diff --git a/include/libnetfilter_log/libnetfilter_log.h b/include/libnetfilter_log/libnetfilter_log.h
index 4e5d763..aa0da72 100644
--- a/include/libnetfilter_log/libnetfilter_log.h
+++ b/include/libnetfilter_log/libnetfilter_log.h
@@ -48,6 +48,11 @@ extern int nflog_handle_packet(struct nflog_handle *h, char *buf, int len);
 
 
 extern struct nfulnl_msg_packet_hdr *nflog_get_msg_packet_hdr(struct nflog_data *nfad);
+
+extern u_int16_t nflog_get_hwtype(struct nflog_data *nfad);
+extern u_int16_t nflog_get_msg_packet_hwhdrlen(struct nflog_data *nfad);
+extern char *nflog_get_msg_packet_hwhdr(struct nflog_data *nfad);
+
 extern u_int32_t nflog_get_nfmark(struct nflog_data *nfad);
 extern int nflog_get_timestamp(struct nflog_data *nfad, struct timeval *tv);
 extern u_int32_t nflog_get_indev(struct nflog_data *nfad);
diff --git a/include/libnetfilter_log/linux_nfnetlink_log.h b/include/libnetfilter_log/linux_nfnetlink_log.h
index f8e174d..4c802c8 100644
--- a/include/libnetfilter_log/linux_nfnetlink_log.h
+++ b/include/libnetfilter_log/linux_nfnetlink_log.h
@@ -54,6 +54,9 @@ enum nfulnl_attr_type {
 	NFULA_SEQ,			/* instance-local sequence number */
 	NFULA_SEQ_GLOBAL,		/* global sequence number */
 	NFULA_GID,			/* group id of socket */
+	NFULA_HWTYPE,			/* hardware type */
+	NFULA_HWHEADER,			/* hardware header */
+	NFULA_HWLEN,			/* hardware header length */
 
 	__NFULA_MAX
 };
diff --git a/src/libnetfilter_log.c b/src/libnetfilter_log.c
index 2019b0c..6c0936e 100644
--- a/src/libnetfilter_log.c
+++ b/src/libnetfilter_log.c
@@ -385,6 +385,22 @@ struct nfulnl_msg_packet_hdr *nflog_get_msg_packet_hdr(struct nflog_data *nfad)
 					 struct nfulnl_msg_packet_hdr);
 }
 
+
+u_int16_t nflog_get_hwtype(struct nflog_data *nfad)
+{
+	return ntohs(nfnl_get_data(nfad->nfa, NFULA_HWTYPE, u_int16_t));
+}
+
+u_int16_t nflog_get_msg_packet_hwhdrlen(struct nflog_data *nfad)
+{
+	return ntohs(nfnl_get_data(nfad->nfa, NFULA_HWLEN, u_int16_t));
+}
+
+char *nflog_get_msg_packet_hwhdr(struct nflog_data *nfad)
+{
+	return nfnl_get_pointer_to_data(nfad->nfa, NFULA_HWHEADER, char);
+}
+
 u_int32_t nflog_get_nfmark(struct nflog_data *nfad)
 {
 	return ntohl(nfnl_get_data(nfad->nfa, NFULA_MARK, u_int32_t));
-- 
1.5.4.3

Re: [libnetfilter_log PATCH] Add parsing function for raw hardware header.

[Patrick McHardy]

Eric Leblond wrote:
> This patch adds support for the parsing of NFULA_HWTYPE, NFULA_HWHEADER,
> and NFULA_HWLEN part of the NFLOG message. This permits to have a ULOG
> like behaviour but thanks to NFULA_HWTYPE we will be able to parse the
> message cleanly in userspace.

Applied, thanks.

[ULOGD2 PATCH] Sync NFLOG input plugin with NFLOG modification related to hardware header.

[Eric Leblond]

This patch modifies ulogd2 to synchronize NFLOG with ULOG relatively to the
definition of raw.mac. It fills the field with the complete hardware header
instead of simply using the source hardware header.

This is a preliminary work for an implementation of an hardware header parsing.

Signed-off-by: Eric Leblond <eric@inl.fr>
---
 input/packet/ulogd_inppkt_NFLOG.c |   10 +++++-----
 1 files changed, 5 insertions(+), 5 deletions(-)

diff --git a/input/packet/ulogd_inppkt_NFLOG.c b/input/packet/ulogd_inppkt_NFLOG.c
index cb58661..b69a99f 100644
--- a/input/packet/ulogd_inppkt_NFLOG.c
+++ b/input/packet/ulogd_inppkt_NFLOG.c
@@ -290,9 +290,8 @@ static inline int
 interp_packet(struct ulogd_pluginstance *upi, struct nflog_data *ldata)
 {
 	struct ulogd_key *ret = upi->output.keys;
-
+	
 	struct nfulnl_msg_packet_hdr *ph = nflog_get_msg_packet_hdr(ldata);
-	struct nfulnl_msg_packet_hw *hw = nflog_get_packet_hw(ldata);
 	char *payload;
 	int payload_len = nflog_get_payload(ldata, &payload);
 	char *prefix = nflog_get_prefix(ldata);
@@ -318,10 +317,11 @@ interp_packet(struct ulogd_pluginstance *upi, struct nflog_data *ldata)
 		ret[NFLOG_KEY_OOB_PROTOCOL].flags |= ULOGD_RETF_VALID;
 	}
 
-	if (hw) {
-		ret[NFLOG_KEY_RAW_MAC].u.value.ptr = hw->hw_addr;
+	if (nflog_get_msg_packet_hwhdrlen(ldata)) {
+		ret[NFLOG_KEY_RAW_MAC].u.value.ptr = nflog_get_msg_packet_hwhdr(ldata);
 		ret[NFLOG_KEY_RAW_MAC].flags |= ULOGD_RETF_VALID;
-		ret[NFLOG_KEY_RAW_MAC_LEN].u.value.ui16 = ntohs(hw->hw_addrlen);
+		ret[NFLOG_KEY_RAW_MAC_LEN].u.value.ui16 =
+			nflog_get_msg_packet_hwhdrlen(ldata);
 		ret[NFLOG_KEY_RAW_MAC_LEN].flags |= ULOGD_RETF_VALID;
 	}
 
-- 
1.5.4.3

Re: [ULOGD2 PATCH] Sync NFLOG input plugin with NFLOG modification related to hardware header.

[Pablo Neira Ayuso]

Eric Leblond wrote:
> This patch modifies ulogd2 to synchronize NFLOG with ULOG relatively to the
> definition of raw.mac. It fills the field with the complete hardware header
> instead of simply using the source hardware header.
> 
> This is a preliminary work for an implementation of an hardware header parsing.
> 
> Signed-off-by: Eric Leblond <eric@inl.fr>
> ---
>  input/packet/ulogd_inppkt_NFLOG.c |   10 +++++-----
>  1 files changed, 5 insertions(+), 5 deletions(-)
> 
> diff --git a/input/packet/ulogd_inppkt_NFLOG.c b/input/packet/ulogd_inppkt_NFLOG.c
> index cb58661..b69a99f 100644
> --- a/input/packet/ulogd_inppkt_NFLOG.c
> +++ b/input/packet/ulogd_inppkt_NFLOG.c
> @@ -290,9 +290,8 @@ static inline int
>  interp_packet(struct ulogd_pluginstance *upi, struct nflog_data *ldata)
>  {
>  	struct ulogd_key *ret = upi->output.keys;
> -
> +	
>  	struct nfulnl_msg_packet_hdr *ph = nflog_get_msg_packet_hdr(ldata);
> -	struct nfulnl_msg_packet_hw *hw = nflog_get_packet_hw(ldata);
>  	char *payload;
>  	int payload_len = nflog_get_payload(ldata, &payload);
>  	char *prefix = nflog_get_prefix(ldata);
> @@ -318,10 +317,11 @@ interp_packet(struct ulogd_pluginstance *upi, struct nflog_data *ldata)
>  		ret[NFLOG_KEY_OOB_PROTOCOL].flags |= ULOGD_RETF_VALID;
>  	}
>  
> -	if (hw) {
> -		ret[NFLOG_KEY_RAW_MAC].u.value.ptr = hw->hw_addr;
> +	if (nflog_get_msg_packet_hwhdrlen(ldata)) {
> +		ret[NFLOG_KEY_RAW_MAC].u.value.ptr = nflog_get_msg_packet_hwhdr(ldata);
>  		ret[NFLOG_KEY_RAW_MAC].flags |= ULOGD_RETF_VALID;
> -		ret[NFLOG_KEY_RAW_MAC_LEN].u.value.ui16 = ntohs(hw->hw_addrlen);
> +		ret[NFLOG_KEY_RAW_MAC_LEN].u.value.ui16 =
> +			nflog_get_msg_packet_hwhdrlen(ldata);
>  		ret[NFLOG_KEY_RAW_MAC_LEN].flags |= ULOGD_RETF_VALID;

OK, but we also need a key for the hw header type here to fix the
MAC2STR plugin - and probably rename it by ETHERNET2STR or HWADDR2STR if
we want to enable generic link layer header parsing.

Anyhow, the main problem is the database back-end. I think that we'll
have to log the whole hardware header again together with the hardware
type. But then, the MAC2STR plugin would not be useful for the database
output - still it may be for the plain text output.

-- 
"Los honestos son inadaptados sociales" -- Les Luthiers

[ULOGD PATCH 4/6] Parse oob protocol in ULOG when possible.

[Eric Leblond]

ULOG API does not provide the value of oob.protocol (hardware protocol).
This patch parses the hardware mac header by using ethernet address packet
structure to go to the value of the field and store it inside the oob.protocol
key.

Signed-off-by: Eric Leblond <eric@inl.fr>
---
 input/packet/ulogd_inppkt_ULOG.c |   11 ++++++++---
 1 files changed, 8 insertions(+), 3 deletions(-)

diff --git a/input/packet/ulogd_inppkt_ULOG.c b/input/packet/ulogd_inppkt_ULOG.c
index 8abaa60..6b0ccc2 100644
--- a/input/packet/ulogd_inppkt_ULOG.c
+++ b/input/packet/ulogd_inppkt_ULOG.c
@@ -215,6 +215,14 @@ static int interp_packet(struct ulogd_pluginstance *ip, ulog_packet_msg_t *pkt)
 		ret[ULOG_KEY_RAW_MAC_LEN].flags |= ULOGD_RETF_VALID;
 	}
 
+	if (pkt->mac_len >= 2 * ip->config_kset->ces[4].u.value
+			    + sizeof(uint16_t)) {
+		ret[ULOG_KEY_OOB_PROTOCOL].u.value.ui16 = ntohs(*(uint16_t *)(
+			pkt->mac + 2 *  ip->config_kset->ces[4].u.value));
+	} else
+		ret[ULOG_KEY_OOB_PROTOCOL].u.value.ui16 = 0;
+	ret[ULOG_KEY_OOB_PROTOCOL].flags |= ULOGD_RETF_VALID;
+
 	ret[ULOG_KEY_RAW_LABEL].u.value.ui8 = ip->config_kset->ces[3].u.value;
 	ret[ULOG_KEY_RAW_LABEL].flags |= ULOGD_RETF_VALID;
 
@@ -254,9 +262,6 @@ static int interp_packet(struct ulogd_pluginstance *ip, ulog_packet_msg_t *pkt)
 	/* ULOG is IPv4 only */
 	ret[ULOG_KEY_OOB_FAMILY].u.value.ui8 = AF_INET;
 	ret[ULOG_KEY_OOB_FAMILY].flags |= ULOGD_RETF_VALID;
-	/* Undef in ULOG but necessary */
-	ret[ULOG_KEY_OOB_PROTOCOL].u.value.ui16 = 0;
-	ret[ULOG_KEY_OOB_PROTOCOL].flags |= ULOGD_RETF_VALID;
 
 	ulogd_propagate_results(ip);
 	return 0;
-- 
1.5.4.3

[ULOGD PATCH 5/6] Add destination mac_addr to database.

[Eric Leblond]

From: Pierre Chifflier <chifflier@inl.fr>

This patch adds support for logging the destination mac address in
the SQL databases.
A tuple (mac_saddr,mac_daddr,mac_proto) is logged only once.

Signed-off-by: Eric Leblond <eric@inl.fr>
---
 doc/mysql-ulogd2.sql |   32 ++++++++++++++++++--------------
 doc/pgsql-ulogd2.sql |   17 +++++++++++++----
 2 files changed, 31 insertions(+), 18 deletions(-)

diff --git a/doc/mysql-ulogd2.sql b/doc/mysql-ulogd2.sql
index 8659c38..43a63d0 100644
--- a/doc/mysql-ulogd2.sql
+++ b/doc/mysql-ulogd2.sql
@@ -75,11 +75,12 @@ ALTER TABLE ulog2 ADD KEY `timestamp` (`timestamp`);
 CREATE TABLE `mac` (
   `_mac_id` bigint unsigned NOT NULL auto_increment,
   `mac_saddr` varchar(32) default NULL,
+  `mac_daddr` varchar(32) default NULL,
   `mac_protocol` smallint(5) default NULL,
   UNIQUE KEY `key_id` (`_mac_id`)
 ) ENGINE=INNODB;
 
-ALTER TABLE mac ADD UNIQUE KEY `mac_saddr` (`mac_saddr`,`mac_protocol`);
+ALTER TABLE mac ADD UNIQUE KEY `mac_addr` (`mac_saddr`,`mac_daddr`,`mac_protocol`);
 ALTER TABLE mac ADD KEY `index_mac_id` (`_mac_id`);
 
 CREATE TABLE `tcp` (
@@ -176,8 +177,8 @@ CREATE SQL SECURITY INVOKER VIEW `ulog` AS
         oob_prefix,
         oob_mark,
         oob_in,
-	oob_out,
-	oob_family,
+        oob_out,
+        oob_family,
         ip_saddr AS ip_saddr_bin,
         ip_daddr AS ip_daddr_bin,
         ip_protocol,
@@ -209,14 +210,15 @@ CREATE SQL SECURITY INVOKER VIEW `ulog` AS
         icmp_echoseq,
         icmp_gateway,
         icmp_fragmtu,
-	icmpv6_type,
-	icmpv6_code,
-	icmpv6_echoid,
-	icmpv6_echoseq,
-	icmpv6_csum,
-	mac_saddr as mac_saddr_str,
-	mac_protocol as oob_protocol,
-	label as raw_label
+        icmpv6_type,
+        icmpv6_code,
+        icmpv6_echoid,
+        icmpv6_echoseq,
+        icmpv6_csum,
+        mac_saddr as mac_saddr_str,
+        mac_daddr as mac_daddr_str,
+        mac_protocol as oob_protocol,
+        label as raw_label
         FROM ulog2 LEFT JOIN tcp ON ulog2._id = tcp._tcp_id LEFT JOIN udp ON ulog2._id = udp._udp_id
                 LEFT JOIN icmp ON ulog2._id = icmp._icmp_id LEFT JOIN mac ON ulog2._id = mac._mac_id
                 LEFT JOIN icmpv6 ON ulog2._id = icmpv6._icmpv6_id;
@@ -607,13 +609,14 @@ delimiter $$
 DROP FUNCTION IF EXISTS INSERT_OR_SELECT_MAC;
 CREATE FUNCTION INSERT_OR_SELECT_MAC(
 		`_saddr` varchar(32),
+		`_daddr` varchar(32),
 		`_protocol` smallint(5)
 		) RETURNS bigint unsigned
 NOT DETERMINISTIC
 READS SQL DATA
 BEGIN
-	INSERT IGNORE INTO mac (mac_saddr, mac_protocol) VALUES (_saddr, _protocol);
-	SELECT _mac_id FROM mac WHERE mac_saddr = _saddr AND mac_protocol = _protocol INTO @last_id;
+	INSERT IGNORE INTO mac (mac_saddr, mac_daddr, mac_protocol) VALUES (_saddr, _daddr, _protocol);
+	SELECT _mac_id FROM mac WHERE mac_saddr = _saddr AND mac_daddr = _daddr AND mac_protocol = _protocol INTO @last_id;
 	RETURN @last_id;
 END
 $$
@@ -666,6 +669,7 @@ CREATE FUNCTION INSERT_PACKET_FULL(
 		icmpv6_echoseq smallint(5) unsigned,
 		icmpv6_csum int(10) unsigned,
 		mac_saddr varchar(32),
+		mac_daddr varchar(32),
 		mac_protocol smallint(5),
 		_label tinyint(4) unsigned
 		) RETURNS bigint unsigned
@@ -690,7 +694,7 @@ BEGIN
 				       icmpv6_echoseq, icmpv6_csum);
 	END IF;
 	IF mac_protocol IS NOT NULL THEN
-		SET @mac_id = INSERT_OR_SELECT_MAC(mac_saddr, mac_protocol);
+		SET @mac_id = INSERT_OR_SELECT_MAC(mac_saddr, mac_daddr, mac_protocol);
 		IF @mac_id IS NOT NULL THEN
 			UPDATE ulog2 SET mac_id = @mac_id WHERE _id = @lastid;
 		END IF;
diff --git a/doc/pgsql-ulogd2.sql b/doc/pgsql-ulogd2.sql
index b7e0038..83b1c28 100644
--- a/doc/pgsql-ulogd2.sql
+++ b/doc/pgsql-ulogd2.sql
@@ -72,11 +72,13 @@ CREATE SEQUENCE mac__id_seq;
 CREATE TABLE mac (
   _mac_id bigint PRIMARY KEY UNIQUE NOT NULL DEFAULT nextval('mac__id_seq'),
   mac_saddr macaddr NOT NULL,
+  mac_daddr macaddr default NULL,
   mac_protocol smallint default NULL
 ) WITH (OIDS=FALSE);
 
 CREATE INDEX mac_saddr ON mac(mac_saddr);
-CREATE UNIQUE INDEX unique_mac ON mac(mac_saddr,mac_protocol);
+CREATE INDEX mac_daddr ON mac(mac_daddr);
+CREATE UNIQUE INDEX unique_mac ON mac(mac_saddr,mac_daddr,mac_protocol);
 
 CREATE TABLE tcp (
   _tcp_id bigint PRIMARY KEY UNIQUE NOT NULL,
@@ -196,6 +198,7 @@ CREATE OR REPLACE VIEW ulog AS
         icmpv6_echoseq,
         icmpv6_csum,
         mac_saddr AS mac_saddr_str,
+        mac_daddr AS mac_daddr_str,
         mac_protocol AS oob_protocol,
         label AS raw_label
         FROM ulog2 LEFT JOIN tcp ON ulog2._id = tcp._tcp_id LEFT JOIN udp ON ulog2._id = udp._udp_id
@@ -445,15 +448,20 @@ $$ LANGUAGE SQL SECURITY INVOKER;
 
 CREATE OR REPLACE FUNCTION INSERT_OR_SELECT_MAC(
                 IN in_mac_saddr macaddr,
+                IN in_mac_daddr macaddr,
                 IN in_mac_protocol integer
         )
 RETURNS bigint AS $$
 DECLARE
         _id bigint;
 BEGIN
-        SELECT INTO _id _mac_id FROM mac WHERE mac_saddr = $1 AND mac_protocol = $2;
+        IF $2 IS NULL THEN
+                SELECT INTO _id _mac_id FROM mac WHERE mac_saddr = $1 AND mac_daddr IS NULL AND mac_protocol = $3;
+        ELSE
+                SELECT INTO _id _mac_id FROM mac WHERE mac_saddr = $1 AND mac_daddr = $2 AND mac_protocol = $3;
+        END IF;
         IF NOT FOUND THEN
-                INSERT INTO mac (mac_saddr,mac_protocol) VALUES ($1,$2) RETURNING _mac_id INTO _id;
+                INSERT INTO mac (mac_saddr,mac_daddr,mac_protocol) VALUES ($1,$2,$3) RETURNING _mac_id INTO _id;
                 RETURN _id;
         END IF;
         RETURN _id;
@@ -508,6 +516,7 @@ CREATE OR REPLACE FUNCTION INSERT_PACKET_FULL(
                 IN icmpv6_echoseq integer,
                 IN icmpv6_csum integer,
                 IN mac_saddr varchar(32),
+                IN mac_daddr varchar(32),
                 IN mac_protocol integer,
                 IN label integer
         )
@@ -527,7 +536,7 @@ BEGIN
                 PERFORM INSERT_ICMPV6(t_id,$40,$41,$42,$43,$44);
         END IF;
         IF (mac_saddr IS NOT NULL) THEN
-                t_mac_id = INSERT_OR_SELECT_MAC($45::macaddr,$46);
+                t_mac_id = INSERT_OR_SELECT_MAC($45::macaddr,$46::macaddr,$47);
                 UPDATE ulog2 SET mac_id = t_mac_id WHERE _id = t_id;
         END IF;
         RETURN t_id;
-- 
1.5.4.3

[ULOGD PATCH 6/6] Sync PRINTPKT with mac modification.

[Eric Leblond]

This patch synchronizes the PRINTPKT plugin with the replacement
of raw.mac by raw.mac.saddr and raw.mac.daddr. It now displays
a SMAC and a DMAC field in the message.

Signed-off-by: Eric Leblond <eric@inl.fr>
---
 include/ulogd/printpkt.h |    3 ++-
 util/printpkt.c          |   26 +++++++++++++++++++++-----
 2 files changed, 23 insertions(+), 6 deletions(-)

diff --git a/include/ulogd/printpkt.h b/include/ulogd/printpkt.h
index 6207d5b..7294743 100644
--- a/include/ulogd/printpkt.h
+++ b/include/ulogd/printpkt.h
@@ -10,7 +10,8 @@ enum pkt_keys {
 	KEY_OOB_UID,
 	KEY_OOB_GID,
 	KEY_OOB_MARK,
-	KEY_RAW_MAC,
+	KEY_RAW_MAC_SADDR,
+	KEY_RAW_MAC_DADDR,
 	KEY_RAW_MACLEN,
 	KEY_IP_SADDR,
 	KEY_IP_DADDR,
diff --git a/util/printpkt.c b/util/printpkt.c
index 90fec0e..d08ec92 100644
--- a/util/printpkt.c
+++ b/util/printpkt.c
@@ -52,7 +52,12 @@ struct ulogd_key printpkt_keys[] = {
 				    .flags = ULOGD_KEYF_OPTIONAL
 				  },
 	[KEY_OOB_MARK]		= { .name = "oob.mark", },
-	[KEY_RAW_MAC]		= { .name = "raw.mac", },
+	[KEY_RAW_MAC_SADDR]	= { .name = "raw.mac.saddr",
+				    .flags = ULOGD_KEYF_OPTIONAL
+				  },
+	[KEY_RAW_MAC_DADDR]	= { .name = "raw.mac.daddr", 
+				    .flags = ULOGD_KEYF_OPTIONAL
+				  },
 	[KEY_RAW_MACLEN]	= { .name = "raw.mac_len", },
 	[KEY_IP_SADDR]		= { .name = "ip.saddr.str", },
 	[KEY_IP_DADDR]		= { .name = "ip.daddr.str", },
@@ -430,16 +435,27 @@ int printpkt_print(struct ulogd_key *res, char *buf)
 				   (char *) GET_VALUE(res, KEY_OOB_OUT).ptr);
 
 	/* FIXME: configurable */
-	if (pp_is_valid(res, KEY_RAW_MAC)) {
-		unsigned char *mac = (unsigned char *) GET_VALUE(res, KEY_RAW_MAC).ptr;
+	if (pp_is_valid(res, KEY_RAW_MAC_SADDR)) {
+		unsigned char *mac = (unsigned char *) GET_VALUE(res, KEY_RAW_MAC_SADDR).ptr;
+		int i, len = GET_VALUE(res, KEY_RAW_MACLEN).ui16;
+
+		buf_cur += sprintf(buf_cur, "SMAC=");
+		for (i = 0; i < len; i++)
+			buf_cur += sprintf(buf_cur, "%02x%c", mac[i],
+					   i == len - 1 ? ' ' : ':');
+	} else
+		buf_cur += sprintf(buf_cur, "SMAC= ");
+
+	if (pp_is_valid(res, KEY_RAW_MAC_DADDR)) {
+		unsigned char *mac = (unsigned char *) GET_VALUE(res, KEY_RAW_MAC_DADDR).ptr;
 		int i, len = GET_VALUE(res, KEY_RAW_MACLEN).ui16;
 
-		buf_cur += sprintf(buf_cur, "MAC=");
+		buf_cur += sprintf(buf_cur, "DMAC=");
 		for (i = 0; i < len; i++)
 			buf_cur += sprintf(buf_cur, "%02x%c", mac[i],
 					   i == len - 1 ? ' ' : ':');
 	} else
-		buf_cur += sprintf(buf_cur, "MAC= ");
+		buf_cur += sprintf(buf_cur, "DMAC= ");
 
 	switch (GET_VALUE(res, KEY_OOB_FAMILY).ui8) {
 	case AF_INET:
-- 
1.5.4.3

[ULOGD PATCH 5/6] Add destination mac_addr to database

[Eric Leblond]

From: Pierre Chifflier <chifflier@inl.fr>

This patch adds support for logging the destination mac address in
the SQL databases.
A tuple (mac_saddr,mac_daddr,mac_proto) is logged only once.

Signed-off-by: Pierre Chifflier <chifflier@inl.fr>
Signed-off-by: Eric Leblond <eric@inl.fr>
---
 doc/mysql-ulogd2.sql |   32 ++++++++++++++++++--------------
 doc/pgsql-ulogd2.sql |   13 +++++++++----
 2 files changed, 27 insertions(+), 18 deletions(-)

diff --git a/doc/mysql-ulogd2.sql b/doc/mysql-ulogd2.sql
index 8659c38..43a63d0 100644
--- a/doc/mysql-ulogd2.sql
+++ b/doc/mysql-ulogd2.sql
@@ -75,11 +75,12 @@ ALTER TABLE ulog2 ADD KEY `timestamp` (`timestamp`);
 CREATE TABLE `mac` (
   `_mac_id` bigint unsigned NOT NULL auto_increment,
   `mac_saddr` varchar(32) default NULL,
+  `mac_daddr` varchar(32) default NULL,
   `mac_protocol` smallint(5) default NULL,
   UNIQUE KEY `key_id` (`_mac_id`)
 ) ENGINE=INNODB;
 
-ALTER TABLE mac ADD UNIQUE KEY `mac_saddr` (`mac_saddr`,`mac_protocol`);
+ALTER TABLE mac ADD UNIQUE KEY `mac_addr` (`mac_saddr`,`mac_daddr`,`mac_protocol`);
 ALTER TABLE mac ADD KEY `index_mac_id` (`_mac_id`);
 
 CREATE TABLE `tcp` (
@@ -176,8 +177,8 @@ CREATE SQL SECURITY INVOKER VIEW `ulog` AS
         oob_prefix,
         oob_mark,
         oob_in,
-	oob_out,
-	oob_family,
+        oob_out,
+        oob_family,
         ip_saddr AS ip_saddr_bin,
         ip_daddr AS ip_daddr_bin,
         ip_protocol,
@@ -209,14 +210,15 @@ CREATE SQL SECURITY INVOKER VIEW `ulog` AS
         icmp_echoseq,
         icmp_gateway,
         icmp_fragmtu,
-	icmpv6_type,
-	icmpv6_code,
-	icmpv6_echoid,
-	icmpv6_echoseq,
-	icmpv6_csum,
-	mac_saddr as mac_saddr_str,
-	mac_protocol as oob_protocol,
-	label as raw_label
+        icmpv6_type,
+        icmpv6_code,
+        icmpv6_echoid,
+        icmpv6_echoseq,
+        icmpv6_csum,
+        mac_saddr as mac_saddr_str,
+        mac_daddr as mac_daddr_str,
+        mac_protocol as oob_protocol,
+        label as raw_label
         FROM ulog2 LEFT JOIN tcp ON ulog2._id = tcp._tcp_id LEFT JOIN udp ON ulog2._id = udp._udp_id
                 LEFT JOIN icmp ON ulog2._id = icmp._icmp_id LEFT JOIN mac ON ulog2._id = mac._mac_id
                 LEFT JOIN icmpv6 ON ulog2._id = icmpv6._icmpv6_id;
@@ -607,13 +609,14 @@ delimiter $$
 DROP FUNCTION IF EXISTS INSERT_OR_SELECT_MAC;
 CREATE FUNCTION INSERT_OR_SELECT_MAC(
 		`_saddr` varchar(32),
+		`_daddr` varchar(32),
 		`_protocol` smallint(5)
 		) RETURNS bigint unsigned
 NOT DETERMINISTIC
 READS SQL DATA
 BEGIN
-	INSERT IGNORE INTO mac (mac_saddr, mac_protocol) VALUES (_saddr, _protocol);
-	SELECT _mac_id FROM mac WHERE mac_saddr = _saddr AND mac_protocol = _protocol INTO @last_id;
+	INSERT IGNORE INTO mac (mac_saddr, mac_daddr, mac_protocol) VALUES (_saddr, _daddr, _protocol);
+	SELECT _mac_id FROM mac WHERE mac_saddr = _saddr AND mac_daddr = _daddr AND mac_protocol = _protocol INTO @last_id;
 	RETURN @last_id;
 END
 $$
@@ -666,6 +669,7 @@ CREATE FUNCTION INSERT_PACKET_FULL(
 		icmpv6_echoseq smallint(5) unsigned,
 		icmpv6_csum int(10) unsigned,
 		mac_saddr varchar(32),
+		mac_daddr varchar(32),
 		mac_protocol smallint(5),
 		_label tinyint(4) unsigned
 		) RETURNS bigint unsigned
@@ -690,7 +694,7 @@ BEGIN
 				       icmpv6_echoseq, icmpv6_csum);
 	END IF;
 	IF mac_protocol IS NOT NULL THEN
-		SET @mac_id = INSERT_OR_SELECT_MAC(mac_saddr, mac_protocol);
+		SET @mac_id = INSERT_OR_SELECT_MAC(mac_saddr, mac_daddr, mac_protocol);
 		IF @mac_id IS NOT NULL THEN
 			UPDATE ulog2 SET mac_id = @mac_id WHERE _id = @lastid;
 		END IF;
diff --git a/doc/pgsql-ulogd2.sql b/doc/pgsql-ulogd2.sql
index b7e0038..51917fa 100644
--- a/doc/pgsql-ulogd2.sql
+++ b/doc/pgsql-ulogd2.sql
@@ -72,11 +72,13 @@ CREATE SEQUENCE mac__id_seq;
 CREATE TABLE mac (
   _mac_id bigint PRIMARY KEY UNIQUE NOT NULL DEFAULT nextval('mac__id_seq'),
   mac_saddr macaddr NOT NULL,
+  mac_daddr macaddr default NULL,
   mac_protocol smallint default NULL
 ) WITH (OIDS=FALSE);
 
 CREATE INDEX mac_saddr ON mac(mac_saddr);
-CREATE UNIQUE INDEX unique_mac ON mac(mac_saddr,mac_protocol);
+CREATE INDEX mac_daddr ON mac(mac_daddr);
+CREATE UNIQUE INDEX unique_mac ON mac(mac_saddr,mac_daddr,mac_protocol);
 
 CREATE TABLE tcp (
   _tcp_id bigint PRIMARY KEY UNIQUE NOT NULL,
@@ -196,6 +198,7 @@ CREATE OR REPLACE VIEW ulog AS
         icmpv6_echoseq,
         icmpv6_csum,
         mac_saddr AS mac_saddr_str,
+        mac_daddr AS mac_daddr_str,
         mac_protocol AS oob_protocol,
         label AS raw_label
         FROM ulog2 LEFT JOIN tcp ON ulog2._id = tcp._tcp_id LEFT JOIN udp ON ulog2._id = udp._udp_id
@@ -445,15 +448,16 @@ $$ LANGUAGE SQL SECURITY INVOKER;
 
 CREATE OR REPLACE FUNCTION INSERT_OR_SELECT_MAC(
                 IN in_mac_saddr macaddr,
+                IN in_mac_daddr macaddr,
                 IN in_mac_protocol integer
         )
 RETURNS bigint AS $$
 DECLARE
         _id bigint;
 BEGIN
-        SELECT INTO _id _mac_id FROM mac WHERE mac_saddr = $1 AND mac_protocol = $2;
+        SELECT INTO _id _mac_id FROM mac WHERE mac_saddr = $1 AND mac_daddr = $2 AND mac_protocol = $3;
         IF NOT FOUND THEN
-                INSERT INTO mac (mac_saddr,mac_protocol) VALUES ($1,$2) RETURNING _mac_id INTO _id;
+                INSERT INTO mac (mac_saddr,mac_daddr,mac_protocol) VALUES ($1,$2,$3) RETURNING _mac_id INTO _id;
                 RETURN _id;
         END IF;
         RETURN _id;
@@ -508,6 +512,7 @@ CREATE OR REPLACE FUNCTION INSERT_PACKET_FULL(
                 IN icmpv6_echoseq integer,
                 IN icmpv6_csum integer,
                 IN mac_saddr varchar(32),
+                IN mac_daddr varchar(32),
                 IN mac_protocol integer,
                 IN label integer
         )
@@ -527,7 +532,7 @@ BEGIN
                 PERFORM INSERT_ICMPV6(t_id,$40,$41,$42,$43,$44);
         END IF;
         IF (mac_saddr IS NOT NULL) THEN
-                t_mac_id = INSERT_OR_SELECT_MAC($45::macaddr,$46);
+                t_mac_id = INSERT_OR_SELECT_MAC($45::macaddr,$46::macaddr,$47);
                 UPDATE ulog2 SET mac_id = t_mac_id WHERE _id = t_id;
         END IF;
         RETURN t_id;
-- 
1.5.5.3

Re: [ULOGD PATCH 5/6] Add destination mac_addr to database

[Pablo Neira Ayuso]

Eric Leblond wrote:
> From: Pierre Chifflier <chifflier@inl.fr>
> 
> This patch adds support for logging the destination mac address in
> the SQL databases.
> A tuple (mac_saddr,mac_daddr,mac_proto) is logged only once.

I have kept back this patch. BTW, please do not mix cleanups with the
supposed intention of the patch, I see several cleanups in this patch.
Better split it into two patches.

-- 
"Los honestos son inadaptados sociales" -- Les Luthiers

[ULOGD PATCH 6/6] Suppress verbose debug message in ULOG.

[Eric Leblond]

ULOG input plugin was displaying a message for each received packet. This
patch suppress the message.

Signed-off-by: Eric Leblond <eric@inl.fr>
---
 input/packet/ulogd_inppkt_ULOG.c |    1 -
 1 files changed, 0 insertions(+), 1 deletions(-)

diff --git a/input/packet/ulogd_inppkt_ULOG.c b/input/packet/ulogd_inppkt_ULOG.c
index 97b9972..c00d9bf 100644
--- a/input/packet/ulogd_inppkt_ULOG.c
+++ b/input/packet/ulogd_inppkt_ULOG.c
@@ -270,7 +270,6 @@ static int ulog_read_cb(int fd, unsigned int what, void *param)
 		}
 		while ((upkt = ipulog_get_packet(u->libulog_h,
 						 u->libulog_buf, len))) {
-			ulogd_log(ULOGD_DEBUG, "==> ulog packet received\n");
 			/* since we support the re-use of one instance in
 			 * several different stacks, we duplicate the message
 			 * to let them know */
-- 
1.5.5.3

Re: [ULOGD PATCH 6/6] Suppress verbose debug message in ULOG.

[Pablo Neira Ayuso]

Eric Leblond wrote:
> ULOG input plugin was displaying a message for each received packet. This
> patch suppress the message.

Applied. Thanks Eric and Pierre.

-- 
"Los honestos son inadaptados sociales" -- Les Luthiers

netfilter: Make NFLOG dump all hardware header.

[Eric Leblond]

This patch synchronises behaviour of ULOG and NFLOG relatively to the
hardware header:
 * NFLOG sends MAC address to userspace (dev_parse_header content)
 * ULOG sends complete link header (skb_mac_header content)
This patch double the size of hardware header for NFLOG and fill it
with the complete hardware header.
---
 include/linux/netfilter/nfnetlink_log.h |    4 +++-
 net/netfilter/nfnetlink_log.c           |   14 ++++++++------
 2 files changed, 11 insertions(+), 7 deletions(-)

diff --git a/include/linux/netfilter/nfnetlink_log.h b/include/linux/netfilter/nfnetlink_log.h
index a857213..21b7ad5 100644
--- a/include/linux/netfilter/nfnetlink_log.h
+++ b/include/linux/netfilter/nfnetlink_log.h
@@ -21,10 +21,12 @@ struct nfulnl_msg_packet_hdr {
 	u_int8_t	_pad;
 };
 
+#define MAX_HWHEADER_LEN 16
+
 struct nfulnl_msg_packet_hw {
 	__be16		hw_addrlen;
 	u_int16_t	_pad;
-	u_int8_t	hw_addr[8];
+	u_int8_t	hw_addr[MAX_HWHEADER_LEN];
 };
 
 struct nfulnl_msg_packet_timestamp {
diff --git a/net/netfilter/nfnetlink_log.c b/net/netfilter/nfnetlink_log.c
index 7efa40d..de4af6c 100644
--- a/net/netfilter/nfnetlink_log.c
+++ b/net/netfilter/nfnetlink_log.c
@@ -444,13 +444,15 @@ __build_packet_message(struct nfulnl_instance *inst,
 	if (skb->mark)
 		NLA_PUT_BE32(inst->skb, NFULA_MARK, htonl(skb->mark));
 
-	if (indev && skb->dev) {
+
+	if (indev && indev->hard_header_len > 0
+	    && skb->mac_header != skb->network_header
+	    && indev->hard_header_len <= MAX_HWHEADER_LEN * sizeof(char)) {
 		struct nfulnl_msg_packet_hw phw;
-		int len = dev_parse_header(skb, phw.hw_addr);
-		if (len > 0) {
-			phw.hw_addrlen = htons(len);
-			NLA_PUT(inst->skb, NFULA_HWADDR, sizeof(phw), &phw);
-		}
+		memcpy(phw.hw_addr, skb_mac_header(skb), indev->hard_header_len);
+
+		phw.hw_addrlen = htons(indev->hard_header_len);
+		NLA_PUT(inst->skb, NFULA_HWADDR, sizeof(phw), &phw);
 	}
 
 	if (skb->tstamp.tv64) {
-- 
1.5.5.3

Re: netfilter: Make NFLOG dump all hardware header.

[Patrick McHardy]

Eric Leblond wrote:
> This patch synchronises behaviour of ULOG and NFLOG relatively to the
> hardware header:
>  * NFLOG sends MAC address to userspace (dev_parse_header content)
>  * ULOG sends complete link header (skb_mac_header content)
> This patch double the size of hardware header for NFLOG and fill it
> with the complete hardware header.

This looks like an API breaking patch. It would be better to use
a new attribute for this. Also this structure encapsulation is a
bad idea IMO, it should just use a NLA_BINARY attribute.

> +	if (indev && indev->hard_header_len > 0
> +	    && skb->mac_header != skb->network_header
> +	    && indev->hard_header_len <= MAX_HWHEADER_LEN * sizeof(char)) {

skb_mac_header_was_set()?

[PATCH] Change packet hw header struct accordingly to NFLOG update.

[Eric Leblond]

This patch synchronizes libnetfilter_log with my last patch.

Sent for completeness: kernel patch will need rework.
---
 include/libnetfilter_log/linux_nfnetlink_log.h |    4 +++-
 1 files changed, 3 insertions(+), 1 deletions(-)

diff --git a/include/libnetfilter_log/linux_nfnetlink_log.h b/include/libnetfilter_log/linux_nfnetlink_log.h
index f8e174d..83beb2d 100644
--- a/include/libnetfilter_log/linux_nfnetlink_log.h
+++ b/include/libnetfilter_log/linux_nfnetlink_log.h
@@ -25,10 +25,12 @@ struct nfulnl_msg_packet_hdr {
 	u_int8_t	_pad;
 } __attribute__ ((packed));
 
+#define MAX_HWHEADER_LEN 16
+
 struct nfulnl_msg_packet_hw {
 	u_int16_t	hw_addrlen;
 	u_int16_t	_pad;
-	u_int8_t	hw_addr[8];
+	u_int8_t	hw_addr[MAX_HWHEADER_LEN];
 } __attribute__ ((packed));
 
 struct nfulnl_msg_packet_timestamp {
-- 
1.5.5.3