| Message ID | 1343856436-11129-6-git-send-email-eric@regit.org |
|---|---|
| State | Accepted |
| Headers | show |
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. -- 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
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,
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
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. -- 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
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. -- 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
> 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? -- 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
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