diff mbox series

[nft,3/4] src: cache result of time() during parsing/output

Message ID 20230825132942.2733840-4-thaller@redhat.com
State Changes Requested
Delegated to: Pablo Neira
Headers show
Series add operation cache for timestamp | expand

Commit Message

Thomas Haller Aug. 25, 2023, 1:24 p.m. UTC 
When we parse/output a larger set of data, we should only call time()
once. With every call of time(), the value keeps ticking (and is subject
to time reset). Previously, one parse/output operation will make
decisions on potentially different timestamps.

Add a cache to the parse/output context, and only fetch time() once
per operation.

Signed-off-by: Thomas Haller <thaller@redhat.com>
---
 include/datatype.h |  6 ++++++
 src/datatype.c     | 16 ++++++++++++++++
 src/meta.c         |  4 ++--
 3 files changed, 24 insertions(+), 2 deletions(-)

Comments

Pablo Neira Ayuso Aug. 28, 2023, 3:02 p.m. UTC | #1 
On Fri, Aug 25, 2023 at 03:24:19PM +0200, Thomas Haller wrote:
> When we parse/output a larger set of data, we should only call time()
> once. With every call of time(), the value keeps ticking (and is subject
> to time reset). Previously, one parse/output operation will make
> decisions on potentially different timestamps.
> 
> Add a cache to the parse/output context, and only fetch time() once
> per operation.
> 
> Signed-off-by: Thomas Haller <thaller@redhat.com>
> ---
>  include/datatype.h |  6 ++++++
>  src/datatype.c     | 16 ++++++++++++++++
>  src/meta.c         |  4 ++--
>  3 files changed, 24 insertions(+), 2 deletions(-)
> 
> diff --git a/include/datatype.h b/include/datatype.h
> index 79d996edd348..abd093765703 100644
> --- a/include/datatype.h
> +++ b/include/datatype.h
> @@ -2,6 +2,7 @@
>  #define NFTABLES_DATATYPE_H
>  
>  #include <json.h>
> +#include <time.h>
>  
>  /**
>   * enum datatypes
> @@ -121,12 +122,17 @@ enum byteorder {
>  struct expr;
>  
>  struct ops_cache {
> +	time_t		time;
> +	bool		has_time;
>  };
>  
>  #define CTX_CACHE_INIT() \
>  	{ \
> +		.has_time = false, \
>  	}
>  
> +extern time_t ops_cache_get_time(struct ops_cache *cache);
> +
>  /**
>   * enum datatype_flags
>   *
> diff --git a/src/datatype.c b/src/datatype.c
> index dd6a5fbf5df8..933d832c4f4d 100644
> --- a/src/datatype.c
> +++ b/src/datatype.c
> @@ -35,6 +35,22 @@
>  
>  #include <netinet/ip_icmp.h>
>  
> +time_t ops_cache_get_time(struct ops_cache *cache)
> +{
> +	time_t t;
> +
> +	if (!cache || !cache->has_time) {
> +		t = time(NULL);
> +		if (cache) {
> +			cache->has_time = true;
> +			cache->time = time(NULL);
> +		}
> +	} else
> +		t = cache->time;
> +
> +	return t;
> +}
> +
>  static const struct datatype *datatypes[TYPE_MAX + 1] = {
>  	[TYPE_INVALID]		= &invalid_type,
>  	[TYPE_VERDICT]		= &verdict_type,
> diff --git a/src/meta.c b/src/meta.c
> index 4f383269d032..1d853b219fe6 100644
> --- a/src/meta.c
> +++ b/src/meta.c
> @@ -496,7 +496,7 @@ static void hour_type_print(const struct expr *expr, struct output_ctx *octx)
>  	time_t ts;
>  
>  	/* Obtain current tm, so that we can add tm_gmtoff */
> -	ts = time(NULL);
> +	ts = ops_cache_get_time(octx->ops_cache);

Following the idea of adding a specific time cache, I'd suggest:

        ts = nft_time_get(...);

or similar.

>  	if (ts != ((time_t) -1) && localtime_r(&ts, &cur_tm))
>  		seconds = (seconds + cur_tm.tm_gmtoff) % SECONDS_PER_DAY;
>  
> @@ -534,7 +534,7 @@ static struct error_record *hour_type_parse(struct parse_ctx *ctx,
>  	result = 0;
>  
>  	/* Obtain current tm, so that we can substract tm_gmtoff */
> -	ts = time(NULL);
> +	ts = ops_cache_get_time(ctx->ops_cache);
>  	if (ts != ((time_t) -1) && localtime_r(&ts, &cur_tm_data))
>  		cur_tm = &cur_tm_data;
>  	else
> -- 
> 2.41.0
>
diff mbox series

Patch

diff --git a/include/datatype.h b/include/datatype.h
index 79d996edd348..abd093765703 100644
--- a/include/datatype.h
+++ b/include/datatype.h
@@ -2,6 +2,7 @@ 
 #define NFTABLES_DATATYPE_H
 
 #include <json.h>
+#include <time.h>
 
 /**
  * enum datatypes
@@ -121,12 +122,17 @@  enum byteorder {
 struct expr;
 
 struct ops_cache {
+	time_t		time;
+	bool		has_time;
 };
 
 #define CTX_CACHE_INIT() \
 	{ \
+		.has_time = false, \
 	}
 
+extern time_t ops_cache_get_time(struct ops_cache *cache);
+
 /**
  * enum datatype_flags
  *
diff --git a/src/datatype.c b/src/datatype.c
index dd6a5fbf5df8..933d832c4f4d 100644
--- a/src/datatype.c
+++ b/src/datatype.c
@@ -35,6 +35,22 @@ 
 
 #include <netinet/ip_icmp.h>
 
+time_t ops_cache_get_time(struct ops_cache *cache)
+{
+	time_t t;
+
+	if (!cache || !cache->has_time) {
+		t = time(NULL);
+		if (cache) {
+			cache->has_time = true;
+			cache->time = time(NULL);
+		}
+	} else
+		t = cache->time;
+
+	return t;
+}
+
 static const struct datatype *datatypes[TYPE_MAX + 1] = {
 	[TYPE_INVALID]		= &invalid_type,
 	[TYPE_VERDICT]		= &verdict_type,
diff --git a/src/meta.c b/src/meta.c
index 4f383269d032..1d853b219fe6 100644
--- a/src/meta.c
+++ b/src/meta.c
@@ -496,7 +496,7 @@  static void hour_type_print(const struct expr *expr, struct output_ctx *octx)
 	time_t ts;
 
 	/* Obtain current tm, so that we can add tm_gmtoff */
-	ts = time(NULL);
+	ts = ops_cache_get_time(octx->ops_cache);
 	if (ts != ((time_t) -1) && localtime_r(&ts, &cur_tm))
 		seconds = (seconds + cur_tm.tm_gmtoff) % SECONDS_PER_DAY;
 
@@ -534,7 +534,7 @@  static struct error_record *hour_type_parse(struct parse_ctx *ctx,
 	result = 0;
 
 	/* Obtain current tm, so that we can substract tm_gmtoff */
-	ts = time(NULL);
+	ts = ops_cache_get_time(ctx->ops_cache);
 	if (ts != ((time_t) -1) && localtime_r(&ts, &cur_tm_data))
 		cur_tm = &cur_tm_data;
 	else