Patchwork [ulogd,12/13] nfct: introduce new out keys for ipfix timestamp

login
register
mail settings
Submitter Ken-ichirou MATSUZAWA
Date April 28, 2014, 11:56 a.m.
Message ID <20140428115621.GM12523@gmail.com>
Download mbox | patch
Permalink /patch/343369/
State Changes Requested
Delegated to: Eric Leblond
Headers show

Comments

Ken-ichirou MATSUZAWA - April 28, 2014, 11:56 a.m.
Signed-off-by Ken-ichirou MATSUZAWA <chamas@h4.dion.ne.jp>
---
 input/flow/ulogd_inpflow_NFCT.c | 23 +++++++++++++++++++++++
 1 file changed, 23 insertions(+)
Eric Leblond - June 1, 2014, 10:28 a.m.
Hi,

First of all, sorry for the delay in the review. Don't hesitate to ping
me back to speed up the answer.

I'm agree with most of your patches but not with this one:
      * It causes useless computation for most output supporting NFCT
        (which don't use this value).
      * It will also cause the log all keys module like JSON to log a
        supplementary (ad useless) fields

It will be better to introduce a new filter module. That will produce
the wanted key from the existing timestamp key.

On Mon, 2014-04-28 at 20:56 +0900, Ken-ichirou MATSUZAWA wrote:
> Signed-off-by Ken-ichirou MATSUZAWA <chamas@h4.dion.ne.jp>
> ---
>  input/flow/ulogd_inpflow_NFCT.c | 23 +++++++++++++++++++++++
>  1 file changed, 23 insertions(+)
> 
> diff --git a/input/flow/ulogd_inpflow_NFCT.c b/input/flow/ulogd_inpflow_NFCT.c
> index 4f4301e..b31c46f 100644
> --- a/input/flow/ulogd_inpflow_NFCT.c
> +++ b/input/flow/ulogd_inpflow_NFCT.c
> @@ -186,8 +186,10 @@ enum nfct_keys {
>  	NFCT_CT_EVENT,
>  	NFCT_FLOW_START_SEC,
>  	NFCT_FLOW_START_USEC,
> +	NFCT_FLOW_START_MCSEC,
>  	NFCT_FLOW_END_SEC,
>  	NFCT_FLOW_END_USEC,
> +	NFCT_FLOW_END_MCSEC,
>  	NFCT_OOB_FAMILY,
>  	NFCT_OOB_PROTOCOL,
>  	NFCT_CT,
> @@ -379,6 +381,11 @@ static struct ulogd_key nfct_okeys[] = {
>  		.type 	= ULOGD_RET_UINT32,
>  		.flags 	= ULOGD_RETF_NONE,
>  		.name	= "flow.start.usec",
> +	},
> +	{
> +		.type 	= ULOGD_RET_UINT64,
> +		.flags 	= ULOGD_RETF_NONE,
> +		.name	= "flow.start.mcsec",
>  		.ipfix	= {
>  			.vendor		= IPFIX_VENDOR_IETF,
>  			.field_id	= IPFIX_flowStartMicroSeconds,
> @@ -397,6 +404,11 @@ static struct ulogd_key nfct_okeys[] = {
>  		.type	= ULOGD_RET_UINT32,
>  		.flags	= ULOGD_RETF_NONE,
>  		.name	= "flow.end.usec",
> +	},
> +	{
> +		.type	= ULOGD_RET_UINT64,
> +		.flags	= ULOGD_RETF_NONE,
> +		.name	= "flow.end.mcsec",
>  		.ipfix	= {
>  			.vendor		= IPFIX_VENDOR_IETF,
>  			.field_id	= IPFIX_flowEndMicroSeconds,
> @@ -485,6 +497,13 @@ static int compare(const void *data1, const void *data2)
>  	return nfct_cmp(u1->ct, ct, NFCT_CMP_ORIG | NFCT_CMP_REPL);
>  }
>  
> +static inline uint64_t tv2ntp(const struct timeval t)
> +{
> +	/* RFC7101 - 6.1.10. dateTimeNanoseconds */
> +	return (uint64_t) (t.tv_sec << 32)
> +		+ (t.tv_usec << 32) / (NSEC_PER_SEC / 1000);
> +}
> +
>  /* only the main_upi plugin instance contains the correct private data. */
>  static int propagate_ct(struct ulogd_pluginstance *main_upi,
>  			struct ulogd_pluginstance *upi,
> @@ -579,12 +598,16 @@ static int propagate_ct(struct ulogd_pluginstance *main_upi,
>  				     ts->time[START].tv_sec);
>  			okey_set_u32(&ret[NFCT_FLOW_START_USEC],
>  				     ts->time[START].tv_usec);
> +			okey_set_u64(&ret[NFCT_FLOW_START_MCSEC],
> +				     tv2ntp(ts->time[START]));
>  		}
>  		if (ts->time[STOP].tv_sec) {
>  			okey_set_u32(&ret[NFCT_FLOW_END_SEC],
>  				     ts->time[STOP].tv_sec);
>  			okey_set_u32(&ret[NFCT_FLOW_END_USEC],
>  				     ts->time[STOP].tv_usec);
> +			okey_set_u64(&ret[NFCT_FLOW_END_MCSEC],
> +				     tv2ntp(ts->time[STOP]));
>  		}
>  	}
>  	okey_set_ptr(&ret[NFCT_CT], cpi->ct);


BR,
Pablo Neira - June 2, 2014, 9:52 a.m.
On Sun, Jun 01, 2014 at 12:28:32PM +0200, Eric Leblond wrote:
> On Mon, 2014-04-28 at 20:56 +0900, Ken-ichirou MATSUZAWA wrote:
> > Signed-off-by Ken-ichirou MATSUZAWA <chamas@h4.dion.ne.jp>
> > ---
> >  input/flow/ulogd_inpflow_NFCT.c | 23 +++++++++++++++++++++++
> >  1 file changed, 23 insertions(+)
> > 
> > diff --git a/input/flow/ulogd_inpflow_NFCT.c b/input/flow/ulogd_inpflow_NFCT.c
> > index 4f4301e..b31c46f 100644
> > --- a/input/flow/ulogd_inpflow_NFCT.c
> > +++ b/input/flow/ulogd_inpflow_NFCT.c
> > @@ -186,8 +186,10 @@ enum nfct_keys {
> >  	NFCT_CT_EVENT,
> >  	NFCT_FLOW_START_SEC,
> >  	NFCT_FLOW_START_USEC,
> > +	NFCT_FLOW_START_MCSEC,

Perhaps you can consolidate this and provide just one single keys in
milliseconds?

> >  	NFCT_FLOW_END_SEC,
> >  	NFCT_FLOW_END_USEC,
> > +	NFCT_FLOW_END_MCSEC,
> >  	NFCT_OOB_FAMILY,
> >  	NFCT_OOB_PROTOCOL,
> >  	NFCT_CT,

It will just need to adapt other extensions to use it.
--
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

diff --git a/input/flow/ulogd_inpflow_NFCT.c b/input/flow/ulogd_inpflow_NFCT.c
index 4f4301e..b31c46f 100644
--- a/input/flow/ulogd_inpflow_NFCT.c
+++ b/input/flow/ulogd_inpflow_NFCT.c
@@ -186,8 +186,10 @@  enum nfct_keys {
 	NFCT_CT_EVENT,
 	NFCT_FLOW_START_SEC,
 	NFCT_FLOW_START_USEC,
+	NFCT_FLOW_START_MCSEC,
 	NFCT_FLOW_END_SEC,
 	NFCT_FLOW_END_USEC,
+	NFCT_FLOW_END_MCSEC,
 	NFCT_OOB_FAMILY,
 	NFCT_OOB_PROTOCOL,
 	NFCT_CT,
@@ -379,6 +381,11 @@  static struct ulogd_key nfct_okeys[] = {
 		.type 	= ULOGD_RET_UINT32,
 		.flags 	= ULOGD_RETF_NONE,
 		.name	= "flow.start.usec",
+	},
+	{
+		.type 	= ULOGD_RET_UINT64,
+		.flags 	= ULOGD_RETF_NONE,
+		.name	= "flow.start.mcsec",
 		.ipfix	= {
 			.vendor		= IPFIX_VENDOR_IETF,
 			.field_id	= IPFIX_flowStartMicroSeconds,
@@ -397,6 +404,11 @@  static struct ulogd_key nfct_okeys[] = {
 		.type	= ULOGD_RET_UINT32,
 		.flags	= ULOGD_RETF_NONE,
 		.name	= "flow.end.usec",
+	},
+	{
+		.type	= ULOGD_RET_UINT64,
+		.flags	= ULOGD_RETF_NONE,
+		.name	= "flow.end.mcsec",
 		.ipfix	= {
 			.vendor		= IPFIX_VENDOR_IETF,
 			.field_id	= IPFIX_flowEndMicroSeconds,
@@ -485,6 +497,13 @@  static int compare(const void *data1, const void *data2)
 	return nfct_cmp(u1->ct, ct, NFCT_CMP_ORIG | NFCT_CMP_REPL);
 }
 
+static inline uint64_t tv2ntp(const struct timeval t)
+{
+	/* RFC7101 - 6.1.10. dateTimeNanoseconds */
+	return (uint64_t) (t.tv_sec << 32)
+		+ (t.tv_usec << 32) / (NSEC_PER_SEC / 1000);
+}
+
 /* only the main_upi plugin instance contains the correct private data. */
 static int propagate_ct(struct ulogd_pluginstance *main_upi,
 			struct ulogd_pluginstance *upi,
@@ -579,12 +598,16 @@  static int propagate_ct(struct ulogd_pluginstance *main_upi,
 				     ts->time[START].tv_sec);
 			okey_set_u32(&ret[NFCT_FLOW_START_USEC],
 				     ts->time[START].tv_usec);
+			okey_set_u64(&ret[NFCT_FLOW_START_MCSEC],
+				     tv2ntp(ts->time[START]));
 		}
 		if (ts->time[STOP].tv_sec) {
 			okey_set_u32(&ret[NFCT_FLOW_END_SEC],
 				     ts->time[STOP].tv_sec);
 			okey_set_u32(&ret[NFCT_FLOW_END_USEC],
 				     ts->time[STOP].tv_usec);
+			okey_set_u64(&ret[NFCT_FLOW_END_MCSEC],
+				     tv2ntp(ts->time[STOP]));
 		}
 	}
 	okey_set_ptr(&ret[NFCT_CT], cpi->ct);