Re: [ulogd PATCH 12/13] nfct: introduce new out keys for ipfix timestamp

[Eric Leblond <eric@regit.org>]

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,
-- 
Eric Leblond <eric@regit.org>