[ulogd patch 0/5] nfacct and pgsql update

[ulogd patch 0/5] nfacct and pgsql update

[Eric Leblond]

This patchset adds some new features to nfacct:

 * nfacct: add variable to not zero counter after read
 * pgsql schema: add nfacct table
 * pgsql: only disable key if it starts with underscore
 * nfacct: add timestamp option

and fixes an issue found by Mr Dash Four.

 * pgsql schema: fix timestamp default value

Patchset statistics:
 doc/pgsql-ulogd2.sql              |   24 +++++++++++++++++++++++-
 input/sum/ulogd_inpflow_NFACCT.c  |   62 ++++++++++++++++++++++++++++++++++++++++++++++++++++++--------
 output/pgsql/ulogd_output_PGSQL.c |    6 ++++--
 ulogd.conf.in                     |   12 ++++++++++++
 4 files changed, 93 insertions(+), 11 deletions(-)

BR,
--
Eric

[PATCH 1/5] nfacct: add variable to not zero counter after read

[Eric Leblond]

The default nfacct input plugin zeroes counter after each read. This
is a limitation as other software can't use the counter at the same
time as ulogd2.
This patch adds the zerocounter variable to the NFACCT input plugin.
If set to zero, the counters are not zeroed.
---
 input/sum/ulogd_inpflow_NFACCT.c |   29 +++++++++++++++++++++--------
 ulogd.conf.in                    |    2 ++
 2 files changed, 23 insertions(+), 8 deletions(-)

diff --git a/input/sum/ulogd_inpflow_NFACCT.c b/input/sum/ulogd_inpflow_NFACCT.c
index f3b936f..a3bcc72 100644
--- a/input/sum/ulogd_inpflow_NFACCT.c
+++ b/input/sum/ulogd_inpflow_NFACCT.c
@@ -40,10 +40,17 @@ static struct config_keyset nfacct_kset = {
 			.options = CONFIG_OPT_NONE,
 			.u.value = 0,
 		},
+		{
+			.key	 = "zerocounter",
+			.type	 = CONFIG_TYPE_INT,
+			.options = CONFIG_OPT_NONE,
+			.u.value = 1,
+		}
 	},
-	.num_ces = 1,
+	.num_ces = 2,
 };
 #define pollint_ce(x)	(x->ces[0])
+#define zerocounter_ce(x) (x->ces[1])
 
 enum ulogd_nfacct_keys {
 	ULOGD_NFACCT_NAME,
@@ -145,13 +152,22 @@ static int nfacct_read_cb(int fd, unsigned int what, void *param)
 	return ret;
 }
 
-static int nfacct_send_request(struct nfacct_pluginstance *cpi)
+static int nfacct_send_request(struct ulogd_pluginstance *upi)
 {
+
+	struct nfacct_pluginstance *cpi =
+		(struct nfacct_pluginstance *)upi->private;
 	struct nlmsghdr *nlh;
 	char buf[MNL_SOCKET_BUFFER_SIZE];
+	int flushctr;
 
+	if (zerocounter_ce(upi->config_kset).u.value != 0) {
+		flushctr = NFNL_MSG_ACCT_GET_CTRZERO;
+	} else {
+		flushctr = NFNL_MSG_ACCT_GET;
+	}
 	cpi->seq = time(NULL);
-	nlh = nfacct_nlmsg_build_hdr(buf, NFNL_MSG_ACCT_GET_CTRZERO,
+	nlh = nfacct_nlmsg_build_hdr(buf, flushctr,
 				     NLM_F_DUMP, cpi->seq);
 
 	if (mnl_socket_sendto(cpi->nl, nlh, nlh->nlmsg_len) < 0) {
@@ -167,7 +183,7 @@ static void polling_timer_cb(struct ulogd_timer *t, void *data)
 	struct nfacct_pluginstance *cpi =
 		(struct nfacct_pluginstance *)upi->private;
 
-	nfacct_send_request(cpi);
+	nfacct_send_request(upi);
 
 	ulogd_add_timer(&cpi->timer, pollint_ce(upi->config_kset).u.value);
 }
@@ -234,12 +250,9 @@ static int destructor_nfacct(struct ulogd_pluginstance *upi)
 
 static void signal_nfacct(struct ulogd_pluginstance *upi, int signal)
 {
-	struct nfacct_pluginstance *cpi =
-		(struct nfacct_pluginstance *)upi->private;
-
 	switch (signal) {
 	case SIGUSR2:
-		nfacct_send_request(cpi);
+		nfacct_send_request(upi);
 		break;
 	}
 }
diff --git a/ulogd.conf.in b/ulogd.conf.in
index 5f19cae..879ab3c 100644
--- a/ulogd.conf.in
+++ b/ulogd.conf.in
@@ -263,3 +263,5 @@ mark = 1
 
 [acct1]
 pollinterval = 2
+# Set to zero to avoid zeroing counters after read
+#zerocounter = 0
-- 
1.7.10.4

Re: [PATCH 1/5] nfacct: add variable to not zero counter after read

[Pablo Neira Ayuso]

On Wed, Aug 01, 2012 at 11:27:12PM +0200, Eric Leblond wrote:
> The default nfacct input plugin zeroes counter after each read. This
> is a limitation as other software can't use the counter at the same
> time as ulogd2.
> This patch adds the zerocounter variable to the NFACCT input plugin.
> If set to zero, the counters are not zeroed.

Applied with minor glitches. Thanks Eric.

> ---
>  input/sum/ulogd_inpflow_NFACCT.c |   29 +++++++++++++++++++++--------
>  ulogd.conf.in                    |    2 ++
>  2 files changed, 23 insertions(+), 8 deletions(-)
> 
> diff --git a/input/sum/ulogd_inpflow_NFACCT.c b/input/sum/ulogd_inpflow_NFACCT.c
> index f3b936f..a3bcc72 100644
> --- a/input/sum/ulogd_inpflow_NFACCT.c
> +++ b/input/sum/ulogd_inpflow_NFACCT.c
> @@ -40,10 +40,17 @@ static struct config_keyset nfacct_kset = {
>  			.options = CONFIG_OPT_NONE,
>  			.u.value = 0,
>  		},
> +		{
> +			.key	 = "zerocounter",
> +			.type	 = CONFIG_TYPE_INT,
> +			.options = CONFIG_OPT_NONE,
> +			.u.value = 1,
> +		}
>  	},
> -	.num_ces = 1,
> +	.num_ces = 2,
>  };
>  #define pollint_ce(x)	(x->ces[0])
> +#define zerocounter_ce(x) (x->ces[1])
>  
>  enum ulogd_nfacct_keys {
>  	ULOGD_NFACCT_NAME,
> @@ -145,13 +152,22 @@ static int nfacct_read_cb(int fd, unsigned int what, void *param)
>  	return ret;
>  }
>  
> -static int nfacct_send_request(struct nfacct_pluginstance *cpi)
> +static int nfacct_send_request(struct ulogd_pluginstance *upi)
>  {
> +

Removed extra white line.

> +	struct nfacct_pluginstance *cpi =
> +		(struct nfacct_pluginstance *)upi->private;
>  	struct nlmsghdr *nlh;
>  	char buf[MNL_SOCKET_BUFFER_SIZE];
> +	int flushctr;
>  
> +	if (zerocounter_ce(upi->config_kset).u.value != 0) {
> +		flushctr = NFNL_MSG_ACCT_GET_CTRZERO;
> +	} else {
> +		flushctr = NFNL_MSG_ACCT_GET;
> +	}

No need for brackets in this case, removed.

>  	cpi->seq = time(NULL);
> -	nlh = nfacct_nlmsg_build_hdr(buf, NFNL_MSG_ACCT_GET_CTRZERO,
> +	nlh = nfacct_nlmsg_build_hdr(buf, flushctr,
>  				     NLM_F_DUMP, cpi->seq);

This line above doesn't need the line break to fit into 80-chars.

>  	if (mnl_socket_sendto(cpi->nl, nlh, nlh->nlmsg_len) < 0) {
> @@ -167,7 +183,7 @@ static void polling_timer_cb(struct ulogd_timer *t, void *data)
>  	struct nfacct_pluginstance *cpi =
>  		(struct nfacct_pluginstance *)upi->private;
>  
> -	nfacct_send_request(cpi);
> +	nfacct_send_request(upi);
>  
>  	ulogd_add_timer(&cpi->timer, pollint_ce(upi->config_kset).u.value);
>  }
> @@ -234,12 +250,9 @@ static int destructor_nfacct(struct ulogd_pluginstance *upi)
>  
>  static void signal_nfacct(struct ulogd_pluginstance *upi, int signal)
>  {
> -	struct nfacct_pluginstance *cpi =
> -		(struct nfacct_pluginstance *)upi->private;
> -
>  	switch (signal) {
>  	case SIGUSR2:
> -		nfacct_send_request(cpi);
> +		nfacct_send_request(upi);
>  		break;
>  	}
>  }
> diff --git a/ulogd.conf.in b/ulogd.conf.in
> index 5f19cae..879ab3c 100644
> --- a/ulogd.conf.in
> +++ b/ulogd.conf.in
> @@ -263,3 +263,5 @@ mark = 1
>  
>  [acct1]
>  pollinterval = 2
> +# Set to zero to avoid zeroing counters after read
> +#zerocounter = 0
> -- 
> 1.7.10.4
> 
> --
> To unsubscribe from this list: send the line "unsubscribe netfilter-devel" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 2/5] pgsql schema: add nfacct table

[Eric Leblond]

This patch adds a nfacct table to the postgresql schema. It enables
the storage of all counters at each poll.
---
 doc/pgsql-ulogd2.sql |   22 ++++++++++++++++++++++
 ulogd.conf.in        |    8 ++++++++
 2 files changed, 30 insertions(+)

diff --git a/doc/pgsql-ulogd2.sql b/doc/pgsql-ulogd2.sql
index 8f47bf9..0fc2b45 100644
--- a/doc/pgsql-ulogd2.sql
+++ b/doc/pgsql-ulogd2.sql
@@ -33,6 +33,7 @@ DROP TABLE IF EXISTS sctp CASCADE;
 DROP TABLE IF EXISTS icmp CASCADE;
 DROP TABLE IF EXISTS icmpv6 CASCADE;
 DROP TABLE IF EXISTS nufw CASCADE;
+DROP TABLE IF EXISTS nfacct CASCADE;
 DROP TABLE IF EXISTS ulog2_ct CASCADE;
 DROP TABLE IF EXISTS ulog2 CASCADE;
 
@@ -149,6 +150,16 @@ CREATE TABLE icmpv6 (
   icmpv6_csum integer default NULL
 ) WITH (OIDS=FALSE);
 
+CREATE TABLE nfacct (
+  sum_name varchar(128),
+  sum_pkts integer default 0,
+  sum_bytes integer default 0,
+  oob_time_sec integer default NULL,
+  oob_time_usec integer default NULL
+) WITH (OIDS=FALSE);
+
+CREATE UNIQUE INDEX unique_acct ON nfacct(sum_name, oob_time_sec, oob_time_usec);
+
 -- 
 -- VIEWS
 -- 
@@ -696,6 +707,17 @@ END
 $$ LANGUAGE plpgsql SECURITY INVOKER;
 
 
+CREATE OR REPLACE FUNCTION INSERT_NFACCT(
+                IN sum_name varchar(128),
+                IN sum_pkts integer,
+                IN sum_bytes integer,
+                IN oob_time_sec integer,
+                IN oob_time_usec integer
+        )
+RETURNS void AS $$
+        INSERT INTO nfacct (sum_name,sum_pkts,sum_bytes,oob_time_sec,oob_time_usec)
+                VALUES ($1,$2,$3,$4,$5);
+$$ LANGUAGE SQL SECURITY INVOKER;
 
 
 CREATE OR REPLACE FUNCTION DELETE_PACKET(
diff --git a/ulogd.conf.in b/ulogd.conf.in
index 879ab3c..0e45714 100644
--- a/ulogd.conf.in
+++ b/ulogd.conf.in
@@ -232,6 +232,14 @@ table="ulog2_ct"
 pass="changeme"
 procedure="INSERT_OR_REPLACE_CT"
 
+[pgsql4]
+db="nulog"
+host="localhost"
+user="nupik"
+table="nfacct"
+pass="changeme"
+procedure="INSERT_NFACCT"
+
 [dbi1]
 db="ulog2"
 dbtype="pgsql"
-- 
1.7.10.4

Re: [PATCH 2/5] pgsql schema: add nfacct table

[Pablo Neira Ayuso]

On Wed, Aug 01, 2012 at 11:27:13PM +0200, Eric Leblond wrote:
> This patch adds a nfacct table to the postgresql schema. It enables
> the storage of all counters at each poll.

Applied, thanks.

[PATCH 3/5] pgsql schema: fix timestamp default value

[Eric Leblond]

Set timestamp default value to now() not now which is the time at
table creation.

Reported-by: Mr Dash Four
---
 doc/pgsql-ulogd2.sql |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/doc/pgsql-ulogd2.sql b/doc/pgsql-ulogd2.sql
index 0fc2b45..dc954ed 100644
--- a/doc/pgsql-ulogd2.sql
+++ b/doc/pgsql-ulogd2.sql
@@ -62,7 +62,7 @@ CREATE TABLE ulog2 (
   ip_fragoff smallint default NULL,
   label smallint default NULL,
   mac_id bigint default NULL,
-  timestamp timestamp NOT NULL default 'now'
+  timestamp timestamp NOT NULL default now()
 ) WITH (OIDS=FALSE);
 
 CREATE INDEX ulog2_oob_family ON ulog2(oob_family);
-- 
1.7.10.4

Re: [PATCH 3/5] pgsql schema: fix timestamp default value

[Pablo Neira Ayuso]

On Wed, Aug 01, 2012 at 11:27:14PM +0200, Eric Leblond wrote:
> Set timestamp default value to now() not now which is the time at
> table creation.
> 
> Reported-by: Mr Dash Four

Applied, thanks.

[PATCH 4/5] pgsql: only disable key if it starts with underscore

[Eric Leblond]

ulogd2 was magically making inactive the first key of description
table. This patch improves this system by only doing so when
the key start with an undescore. This way, system like nfacct which
do not have a primary key can be implemented easily.
---
 output/pgsql/ulogd_output_PGSQL.c |    6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/output/pgsql/ulogd_output_PGSQL.c b/output/pgsql/ulogd_output_PGSQL.c
index 9529b1c..44600b0 100644
--- a/output/pgsql/ulogd_output_PGSQL.c
+++ b/output/pgsql/ulogd_output_PGSQL.c
@@ -195,8 +195,10 @@ static int get_columns_pgsql(struct ulogd_pluginstance *upi)
 		strncpy(upi->input.keys[i].name, buf, ULOGD_MAX_KEYLEN);
 	}
 
-	/* ID is a sequence */
-	upi->input.keys[0].flags |= ULOGD_KEYF_INACTIVE;
+	/* ID (starting by '.') is a sequence */
+	if (upi->input.keys[0].name[0] == '.') {
+	    upi->input.keys[0].flags |= ULOGD_KEYF_INACTIVE;
+	}
 
 	PQclear(pi->pgres);
 	return 0;
-- 
1.7.10.4

Re: [PATCH 4/5] pgsql: only disable key if it starts with underscore

[Pablo Neira Ayuso]

On Wed, Aug 01, 2012 at 11:27:15PM +0200, Eric Leblond wrote:
> ulogd2 was magically making inactive the first key of description
> table. This patch improves this system by only doing so when
> the key start with an undescore. This way, system like nfacct which
> do not have a primary key can be implemented easily.

Applied with minor glitch, thanks.

> ---
>  output/pgsql/ulogd_output_PGSQL.c |    6 ++++--
>  1 file changed, 4 insertions(+), 2 deletions(-)
> 
> diff --git a/output/pgsql/ulogd_output_PGSQL.c b/output/pgsql/ulogd_output_PGSQL.c
> index 9529b1c..44600b0 100644
> --- a/output/pgsql/ulogd_output_PGSQL.c
> +++ b/output/pgsql/ulogd_output_PGSQL.c
> @@ -195,8 +195,10 @@ static int get_columns_pgsql(struct ulogd_pluginstance *upi)
>  		strncpy(upi->input.keys[i].name, buf, ULOGD_MAX_KEYLEN);
>  	}
>  
> -	/* ID is a sequence */
> -	upi->input.keys[0].flags |= ULOGD_KEYF_INACTIVE;
> +	/* ID (starting by '.') is a sequence */
> +	if (upi->input.keys[0].name[0] == '.') {
> +	    upi->input.keys[0].flags |= ULOGD_KEYF_INACTIVE;
> +	}

No need for brackets here. Also fixed indentation.

>  
>  	PQclear(pi->pgres);
>  	return 0;
> -- 
> 1.7.10.4
> 
> --
> To unsubscribe from this list: send the line "unsubscribe netfilter-devel" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 5/5] nfacct: add timestamp option

[Eric Leblond]

This patch adds a timestamp option to the nfacct plugin.
If activated, nfacct output a timestamp which is computed just
after sending the nfacct request.
---
 input/sum/ulogd_inpflow_NFACCT.c |   35 ++++++++++++++++++++++++++++++++++-
 ulogd.conf.in                    |    2 ++
 2 files changed, 36 insertions(+), 1 deletion(-)

diff --git a/input/sum/ulogd_inpflow_NFACCT.c b/input/sum/ulogd_inpflow_NFACCT.c
index a3bcc72..66e9f78 100644
--- a/input/sum/ulogd_inpflow_NFACCT.c
+++ b/input/sum/ulogd_inpflow_NFACCT.c
@@ -30,6 +30,7 @@ struct nfacct_pluginstance {
 	uint32_t		seq;
 	struct ulogd_fd		ufd;
 	struct ulogd_timer	timer;
+	struct timeval tv;
 };
 
 static struct config_keyset nfacct_kset = {
@@ -45,18 +46,27 @@ static struct config_keyset nfacct_kset = {
 			.type	 = CONFIG_TYPE_INT,
 			.options = CONFIG_OPT_NONE,
 			.u.value = 1,
+		},
+		{
+			.key	 = "timestamp",
+			.type	 = CONFIG_TYPE_INT,
+			.options = CONFIG_OPT_NONE,
+			.u.value = 0,
 		}
 	},
-	.num_ces = 2,
+	.num_ces = 3,
 };
 #define pollint_ce(x)	(x->ces[0])
 #define zerocounter_ce(x) (x->ces[1])
+#define timestamp_ce(x) (x->ces[2])
 
 enum ulogd_nfacct_keys {
 	ULOGD_NFACCT_NAME,
 	ULOGD_NFACCT_PKTS,
 	ULOGD_NFACCT_BYTES,
 	ULOGD_NFACCT_RAW,
+	ULOGD_NFACCT_TIME_SEC,
+	ULOGD_NFACCT_TIME_USEC,
 };
 
 static struct ulogd_key nfacct_okeys[] = {
@@ -80,12 +90,27 @@ static struct ulogd_key nfacct_okeys[] = {
 		.flags	= ULOGD_RETF_NONE,
 		.name	= "sum",
 	},
+	[ULOGD_NFACCT_TIME_SEC] = {
+		.type = ULOGD_RET_UINT32,
+		.flags = ULOGD_RETF_NONE,
+		.name = "oob.time.sec",
+		.ipfix = {
+			.vendor = IPFIX_VENDOR_IETF,
+			.field_id = 22
+		},
+	},
+	[ULOGD_NFACCT_TIME_USEC] = {
+		.type = ULOGD_RET_UINT32,
+		.flags = ULOGD_RETF_NONE,
+		.name = "oob.time.usec",
+	},
 };
 
 static void
 propagate_nfacct(struct ulogd_pluginstance *upi, struct nfacct *nfacct)
 {
 	struct ulogd_key *ret = upi->output.keys;
+	struct nfacct_pluginstance *cpi = (struct nfacct_pluginstance *) upi->private;
 
 	okey_set_ptr(&ret[ULOGD_NFACCT_NAME],
 			(void *)nfacct_attr_get_str(nfacct, NFACCT_ATTR_NAME));
@@ -94,6 +119,10 @@ propagate_nfacct(struct ulogd_pluginstance *upi, struct nfacct *nfacct)
 	okey_set_u64(&ret[ULOGD_NFACCT_BYTES],
 			nfacct_attr_get_u64(nfacct, NFACCT_ATTR_BYTES));
 	okey_set_ptr(&ret[ULOGD_NFACCT_RAW], nfacct);
+	if (timestamp_ce(upi->config_kset).u.value != 0) {
+		okey_set_u32(&ret[ULOGD_NFACCT_TIME_SEC], cpi->tv.tv_sec);
+		okey_set_u32(&ret[ULOGD_NFACCT_TIME_USEC], cpi->tv.tv_usec);
+	}
 
 	ulogd_propagate_results(upi);
 }
@@ -174,6 +203,10 @@ static int nfacct_send_request(struct ulogd_pluginstance *upi)
 		ulogd_log(ULOGD_ERROR, "Cannot send netlink message\n");
 		return -1;
 	}
+	if (timestamp_ce(upi->config_kset).u.value != 0) {
+		/* Compute time of query */
+		gettimeofday(&cpi->tv, NULL);
+	}
 	return 0;
 }
 
diff --git a/ulogd.conf.in b/ulogd.conf.in
index 0e45714..e99212f 100644
--- a/ulogd.conf.in
+++ b/ulogd.conf.in
@@ -273,3 +273,5 @@ mark = 1
 pollinterval = 2
 # Set to zero to avoid zeroing counters after read
 #zerocounter = 0
+# Output timestamp if set to 1 (default 0)
+#timestamp = 1
-- 
1.7.10.4

Re: [PATCH 5/5] nfacct: add timestamp option

[Pablo Neira Ayuso]

On Wed, Aug 01, 2012 at 11:27:16PM +0200, Eric Leblond wrote:
> This patch adds a timestamp option to the nfacct plugin.
> If activated, nfacct output a timestamp which is computed just
> after sending the nfacct request.

I think it makes sense to make it unconditionally.

The dump of the counters without the time doesn't make too much sense
to me?

Let me know.

Re: [PATCH 5/5] nfacct: add timestamp option

[Eric Leblond]

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

Hello,

Le vendredi 03 août 2012 à 11:35 +0200, Pablo Neira Ayuso a écrit :
> On Wed, Aug 01, 2012 at 11:27:16PM +0200, Eric Leblond wrote:
> > This patch adds a timestamp option to the nfacct plugin.
> > If activated, nfacct output a timestamp which is computed just
> > after sending the nfacct request.
> 
> I think it makes sense to make it unconditionally.
> 
> The dump of the counters without the time doesn't make too much sense
> to me?
> 
> Let me know.

I think it was not originally added because some output format did not
need it. Maybe we can make output of a timestamp by default and let
people who want to spare the cycles of gettimeofday() set it to 0.

BR,
-- 
Eric Leblond 
Blog: http://home.regit.org/ - Portfolio: http://regit.500px.com/

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 198 bytes --]

Re: [PATCH 5/5] nfacct: add timestamp option

[Pablo Neira Ayuso]

On Fri, Aug 03, 2012 at 11:43:58AM +0200, Eric Leblond wrote:
> Hello,
> 
> Le vendredi 03 août 2012 à 11:35 +0200, Pablo Neira Ayuso a écrit :
> > On Wed, Aug 01, 2012 at 11:27:16PM +0200, Eric Leblond wrote:
> > > This patch adds a timestamp option to the nfacct plugin.
> > > If activated, nfacct output a timestamp which is computed just
> > > after sending the nfacct request.
> > 
> > I think it makes sense to make it unconditionally.
> > 
> > The dump of the counters without the time doesn't make too much sense
> > to me?
> > 
> > Let me know.
> 
> I think it was not originally added because some output format did not
> need it.

I guess you refer to GPRINT.

> Maybe we can make output of a timestamp by default and let
> people who want to spare the cycles of gettimeofday() set it to 0.

Hm, that means that we'll have two gettimeofday in case that GPRINT is
used (specifically if people configure this wrong, ie. setting on
timestamp both in GPRINT and NFACCT, that's likely to happen IMO).

I'm still undecided, but it seems to me we provide more fine-grain
control if timestamp is calculated in the output for the nfacct case.

I think we have to agree where to calculate the timestamp (input or
output plugins?) for consistency, or at least agreed on some policy
for this.
--
To unsubscribe from this list: send the line "unsubscribe netfilter-devel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 5/5] nfacct: add timestamp option

[Pablo Neira Ayuso]

On Fri, Aug 03, 2012 at 01:24:49PM +0200, Pablo Neira Ayuso wrote:
[...]
> I'm still undecided, but it seems to me we provide more fine-grain
> control if timestamp is calculated in the output for the nfacct case.

Well, most input plugins already provide timestamp calculation. GPRINT
seems to be the exception for the output plugin case.

I have applied the patch expanding the documentation on the example
file a bit.

Thanks Eric.

Re: [PATCH 5/5] nfacct: add timestamp option

[Mr Dash Four]

Pablo Neira Ayuso wrote:
> On Wed, Aug 01, 2012 at 11:27:16PM +0200, Eric Leblond wrote:
>> This patch adds a timestamp option to the nfacct plugin.
>> If activated, nfacct output a timestamp which is computed just
>> after sending the nfacct request.
> 
> I think it makes sense to make it unconditionally.
Yep. The PGSQL output plugin can't function without it. When I have the following:

stack=acct1:NFACCT,pgsql3:PGSQL

I get the following error if timestamp is not enabled:

<7> ulogd.c:700 type mismatch between PGSQL and NFACCT in stack
<7> ulogd.c:727 cannot find key `oob.time.sec' in stack

I had to dig in further to find out why that was the case (something I could have done without), so I think it makes sense for the timestamp to always be included.

Re: [PATCH 5/5] nfacct: add timestamp option

[Mr Dash Four]

> stack=acct1:NFACCT,pgsql3:PGSQL
>
> I get the following error if timestamp is not enabled:
>
> <7> ulogd.c:700 type mismatch between PGSQL and NFACCT in stack
> <7> ulogd.c:727 cannot find key `oob.time.sec' in stack
>
> I had to dig in further to find out why that was the case (something I could have done without), so I think it makes sense for the timestamp to always be included.
>   
I am still getting a mismatch error, though there is no indication as to 
what is causing it (full debug log follows):


<1> ulogd.c:820 building new pluginstance stack (acct1:NFACCT,pgsql3:PGSQL):
<1> ulogd.c:829 tok=`acct1:NFACCT'
<1> ulogd.c:866 pushing `NFACCT' on stack
<1> ulogd.c:829 tok=`pgsql3:PGSQL'
<1> ulogd.c:866 pushing `PGSQL' on stack
<1> ulogd.c:646 traversing plugin `PGSQL'
<5> ../../util/db.c:146 (re)configuring
<1> ulogd_output_PGSQL.c:146 SELECT attname FROM nfacct
<1> ulogd_output_PGSQL.c:166 5 fields in table
<1> ulogd.c:646 traversing plugin `NFACCT'
<1> ulogd.c:662 connecting input/output keys of stack:
<1> ulogd.c:670 traversing plugin `PGSQL'
<7> ulogd.c:700 type mismatch between PGSQL and NFACCT in stack
<1> ulogd.c:627 acct1(NFACCT)
<1> ulogd.c:733 assigning `sum.name(?)' as source for PGSQL(sum.name)
<1> ulogd.c:627 acct1(NFACCT)
<1> ulogd.c:733 assigning `sum.pkts(?)' as source for PGSQL(sum.pkts)
<1> ulogd.c:627 acct1(NFACCT)
<1> ulogd.c:733 assigning `sum.bytes(?)' as source for PGSQL(sum.bytes)
<1> ulogd.c:627 acct1(NFACCT)
<1> ulogd.c:733 assigning `oob.time.sec(?)' as source for 
PGSQL(oob.time.sec)
<1> ulogd.c:627 acct1(NFACCT)
<1> ulogd.c:733 assigning `oob.time.usec(?)' as source for 
PGSQL(oob.time.usec)
<1> ulogd.c:670 traversing plugin `NFACCT'
<5> ../../util/db.c:180 starting
<1> ../../util/db.c:86 allocating 593 bytes for statement
<1> ../../util/db.c:133 stmt='SELECT INSERT_NFACCT('

Any ideas?

Also, not nfacct related, but a general question about ulogd2: What 
happens when I have my iptables NFLOG targets set/created, but the ulog 
daemon is not running - do the packets matching get discarded or do they 
get buffered (if so, how large is this buffer and is this configurable?) 
so that when ulogd2 gets started they get processed?