[PATCH] nft_meta: Introduce new conditions 'time', 'day' and 'hour'

[PATCH] nft_meta: Introduce new conditions 'time', 'day' and 'hour'

[Ander Juaristi]

This patch introduces meta matches in the kernel for time (a UNIX timestamp),
day (a day of week, represented as an integer between 0-6), and
hour (an hour in the current day, or: number of seconds since midnight).

Signed-off-by: Ander Juaristi <a@juaristi.eus>
---
 include/uapi/linux/netfilter/nf_tables.h |  6 +++++
 net/netfilter/nft_meta.c                 | 32 ++++++++++++++++++++++++
 2 files changed, 38 insertions(+)

diff --git a/include/uapi/linux/netfilter/nf_tables.h b/include/uapi/linux/netfilter/nf_tables.h
index c6c8ec5c7c00..92c78813bc7d 100644
--- a/include/uapi/linux/netfilter/nf_tables.h
+++ b/include/uapi/linux/netfilter/nf_tables.h
@@ -795,6 +795,9 @@ enum nft_exthdr_attributes {
  * @NFT_META_SECPATH: boolean, secpath_exists (!!skb->sp)
  * @NFT_META_IIFKIND: packet input interface kind name (dev->rtnl_link_ops->kind)
  * @NFT_META_OIFKIND: packet output interface kind name (dev->rtnl_link_ops->kind)
+ * @NFT_META_TIME: a UNIX timestamp
+ * @NFT_META_TIME_DAY: day of week
+ * @NFT_META_TIME_HOUR: hour of day
  */
 enum nft_meta_keys {
 	NFT_META_LEN,
@@ -825,6 +828,9 @@ enum nft_meta_keys {
 	NFT_META_SECPATH,
 	NFT_META_IIFKIND,
 	NFT_META_OIFKIND,
+	NFT_META_TIME,
+	NFT_META_TIME_DAY,
+	NFT_META_TIME_HOUR,
 };
 
 /**
diff --git a/net/netfilter/nft_meta.c b/net/netfilter/nft_meta.c
index 987d2d6ce624..a684abd00597 100644
--- a/net/netfilter/nft_meta.c
+++ b/net/netfilter/nft_meta.c
@@ -50,6 +50,7 @@ void nft_meta_get_eval(const struct nft_expr *expr,
 	const struct net_device *in = nft_in(pkt), *out = nft_out(pkt);
 	struct sock *sk;
 	u32 *dest = &regs->data[priv->dreg];
+	s64 *d64;
 #ifdef CONFIG_NF_TABLES_BRIDGE
 	const struct net_bridge_port *p;
 #endif
@@ -254,6 +255,28 @@ void nft_meta_get_eval(const struct nft_expr *expr,
 			goto err;
 		strncpy((char *)dest, out->rtnl_link_ops->kind, IFNAMSIZ);
 		break;
+	case NFT_META_TIME:
+		d64 = (s64 *) dest;
+		*d64 = get_seconds();
+		break;
+	case NFT_META_TIME_DAY:
+	case NFT_META_TIME_HOUR:
+	{
+		s64 secs;
+		struct tm tm;
+
+		d64 = (s64 *) dest;
+
+		/* get timestamp in seconds, and convert to tm structure */
+		secs = get_seconds();
+		time64_to_tm(secs, 0, &tm);
+
+		if (priv->key == NFT_META_TIME_HOUR)
+			*d64 = (s64) (tm.tm_hour * 3600 + tm.tm_min * 60 + tm.tm_sec);
+		else
+			nft_reg_store8(dest, (u8) tm.tm_wday);
+	}
+		break;
 	default:
 		WARN_ON(1);
 		goto err;
@@ -371,6 +394,15 @@ static int nft_meta_get_init(const struct nft_ctx *ctx,
 		len = IFNAMSIZ;
 		break;
 #endif
+	case NFT_META_TIME:
+		len = sizeof(s64);
+		break;
+	case NFT_META_TIME_DAY:
+		len = sizeof(u8);
+		break;
+	case NFT_META_TIME_HOUR:
+		len = sizeof(s64);
+		break;
 	default:
 		return -EOPNOTSUPP;
 	}
-- 
2.17.1

Re: [PATCH] nft_meta: Introduce new conditions 'time', 'day' and 'hour'

[Florian Westphal]

Ander Juaristi <a@juaristi.eus> wrote:
> diff --git a/net/netfilter/nft_meta.c b/net/netfilter/nft_meta.c
> index 987d2d6ce624..a684abd00597 100644
> --- a/net/netfilter/nft_meta.c
> +++ b/net/netfilter/nft_meta.c
> @@ -50,6 +50,7 @@ void nft_meta_get_eval(const struct nft_expr *expr,
>  	const struct net_device *in = nft_in(pkt), *out = nft_out(pkt);
>  	struct sock *sk;
>  	u32 *dest = &regs->data[priv->dreg];
> +	s64 *d64;
>  #ifdef CONFIG_NF_TABLES_BRIDGE
>  	const struct net_bridge_port *p;
>  #endif
> @@ -254,6 +255,28 @@ void nft_meta_get_eval(const struct nft_expr *expr,
>  			goto err;
>  		strncpy((char *)dest, out->rtnl_link_ops->kind, IFNAMSIZ);
>  		break;
> +	case NFT_META_TIME:
> +		d64 = (s64 *) dest;
> +		*d64 = get_seconds();

Nit; why limit this to 1 second granularity and not use
ktime_get_real_ns()  here instead?

I don't mind, we could add NFT_META_TIME_NS if needed.

Otherwise, this looks good to me.
We could also split nft_meta_get_eval and add nft_meta_get_time_eval()
to avoid increasing size of that function but its not a huge deal\x7f
and could be done later anyway.

Re: [PATCH] nft_meta: Introduce new conditions 'time', 'day' and 'hour'

[Ander Juaristi]

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

On 24/6/19 0:56, Florian Westphal wrote:
>> @@ -254,6 +255,28 @@ void nft_meta_get_eval(const struct nft_expr *expr,
>>  			goto err;
>>  		strncpy((char *)dest, out->rtnl_link_ops->kind, IFNAMSIZ);
>>  		break;
>> +	case NFT_META_TIME:
>> +		d64 = (s64 *) dest;
>> +		*d64 = get_seconds();
> 
> Nit; why limit this to 1 second granularity and not use
> ktime_get_real_ns()  here instead?
> 
> I don't mind, we could add NFT_META_TIME_NS if needed.
> 

I don't think this would make sense.

Would require statements such as "meta time 1562005920098458691". That
is totally unfriendly to the end user.

But maybe I didn't understand what you meant here. Maybe you meant to
replace get_seconds() with ktime_get_real_ns(), and divide the result by
10e-9 to get seconds? Would that get better precision?

> Otherwise, this looks good to me.
> We could also split nft_meta_get_eval and add nft_meta_get_time_eval()
> to avoid increasing size of that function but its not a huge deal\x7f
> and could be done later anyway.
> 

[-- Attachment #2: pEpkey.asc --]
[-- Type: application/pgp-keys, Size: 6263 bytes --]

Re: [PATCH] nft_meta: Introduce new conditions 'time', 'day' and 'hour'

[Florian Westphal]

Ander Juaristi <a@juaristi.eus> wrote:
> I don't think this would make sense.
> 
> Would require statements such as "meta time 1562005920098458691". That
> is totally unfriendly to the end user.

We do not necessarily need to expose this on nft side.

e.g. user says
meta time 1562018374

and nft converts this to 1562018374000000000 internally.

Or did you mean that this might cause confusion as this
might never match at all?

In such a case, we'd have to internally rewrite
meta time 1562018374
to
meta time 1562018374-1562018375

(reg1 >= 1562018374000000000 and <= 1562018375000000000).

We could also expose/support the suffixes we support for timeouts, e.g.:
3512312s, 1000ms and so on.

> But maybe I didn't understand what you meant here. Maybe you meant to
> replace get_seconds() with ktime_get_real_ns(), and divide the result by
> 10e-9 to get seconds?

No, thats not what I meant.

I was just thinking that having ns-resolution exposed to registers
might allow to use this for e.g. sampling packet arrival time.

Its not a big deal, we can add this later when such a use case pops up
and keep seconds resolution.