[PATCH v5 1/2] netfilter: Introduce new 64-bit helper functions

[PATCH v5 1/2] netfilter: Introduce new 64-bit helper functions

[Ander Juaristi]

Introduce new helper functions to load/store 64-bit values
onto/from registers:

 - nft_reg_store64
 - nft_reg_load64

This commit also re-orders all these helpers from smallest
to largest target bit size.

Signed-off-by: Ander Juaristi <a@juaristi.eus>
---
 include/net/netfilter/nf_tables.h | 25 ++++++++++++++++++-------
 net/netfilter/nft_byteorder.c     |  9 +++++----
 2 files changed, 23 insertions(+), 11 deletions(-)

diff --git a/include/net/netfilter/nf_tables.h b/include/net/netfilter/nf_tables.h
index 9b624566b82d..298cf4528635 100644
--- a/include/net/netfilter/nf_tables.h
+++ b/include/net/netfilter/nf_tables.h
@@ -2,6 +2,7 @@
 #ifndef _NET_NF_TABLES_H
 #define _NET_NF_TABLES_H
 
+#include <asm/unaligned.h>
 #include <linux/list.h>
 #include <linux/netfilter.h>
 #include <linux/netfilter/nfnetlink.h>
@@ -100,23 +101,28 @@ struct nft_regs {
 	};
 };
 
-/* Store/load an u16 or u8 integer to/from the u32 data register.
+/* Store/load an u8, u16 or u64 integer to/from the u32 data register.
  *
  * Note, when using concatenations, register allocation happens at 32-bit
  * level. So for store instruction, pad the rest part with zero to avoid
  * garbage values.
  */
 
-static inline void nft_reg_store16(u32 *dreg, u16 val)
+static inline void nft_reg_store8(u32 *dreg, u8 val)
 {
 	*dreg = 0;
-	*(u16 *)dreg = val;
+	*(u8 *)dreg = val;
 }
 
-static inline void nft_reg_store8(u32 *dreg, u8 val)
+static inline u8 nft_reg_load8(u32 *sreg)
+{
+	return *(u8 *)sreg;
+}
+
+static inline void nft_reg_store16(u32 *dreg, u16 val)
 {
 	*dreg = 0;
-	*(u8 *)dreg = val;
+	*(u16 *)dreg = val;
 }
 
 static inline u16 nft_reg_load16(u32 *sreg)
@@ -124,9 +130,14 @@ static inline u16 nft_reg_load16(u32 *sreg)
 	return *(u16 *)sreg;
 }
 
-static inline u8 nft_reg_load8(u32 *sreg)
+static inline void nft_reg_store64(u32 *dreg, u64 val)
 {
-	return *(u8 *)sreg;
+	put_unaligned(val, (u64 *)dreg);
+}
+
+static inline u64 nft_reg_load64(u32 *sreg)
+{
+	return get_unaligned((u64 *)sreg);
 }
 
 static inline void nft_data_copy(u32 *dst, const struct nft_data *src,
diff --git a/net/netfilter/nft_byteorder.c b/net/netfilter/nft_byteorder.c
index e06318428ea0..12bed3f7bbc6 100644
--- a/net/netfilter/nft_byteorder.c
+++ b/net/netfilter/nft_byteorder.c
@@ -43,14 +43,15 @@ void nft_byteorder_eval(const struct nft_expr *expr,
 		switch (priv->op) {
 		case NFT_BYTEORDER_NTOH:
 			for (i = 0; i < priv->len / 8; i++) {
-				src64 = get_unaligned((u64 *)&src[i]);
-				put_unaligned_be64(src64, &dst[i]);
+				src64 = nft_reg_load64(&src[i]);
+				nft_reg_store64(&dst[i], be64_to_cpu(src64));
 			}
 			break;
 		case NFT_BYTEORDER_HTON:
 			for (i = 0; i < priv->len / 8; i++) {
-				src64 = get_unaligned_be64(&src[i]);
-				put_unaligned(src64, (u64 *)&dst[i]);
+				src64 = (__force __u64)
+					cpu_to_be64(nft_reg_load64(&src[i]));
+				nft_reg_store64(&dst[i], src64);
 			}
 			break;
 		}
-- 
2.17.1

[PATCH v5 2/2] netfilter: nft_meta: support for time matching

[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).

All values are taken as unsigned 64-bit integers.

The 'time' keyword is internally converted to nanoseconds by nft in
userspace, and hence the timestamp is taken in nanoseconds as well.

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

diff --git a/include/uapi/linux/netfilter/nf_tables.h b/include/uapi/linux/netfilter/nf_tables.h
index 82abaa183fc3..b83b62eb4b01 100644
--- a/include/uapi/linux/netfilter/nf_tables.h
+++ b/include/uapi/linux/netfilter/nf_tables.h
@@ -799,6 +799,9 @@ enum nft_exthdr_attributes {
  * @NFT_META_OIFKIND: packet output interface kind name (dev->rtnl_link_ops->kind)
  * @NFT_META_BRI_IIFPVID: packet input bridge port pvid
  * @NFT_META_BRI_IIFVPROTO: packet input bridge vlan proto
+ * @NFT_META_TIME_NS: time since epoch (in nanoseconds)
+ * @NFT_META_TIME_DAY: day of week (from 0 = Sunday to 6 = Saturday)
+ * @NFT_META_TIME_HOUR: hour of day (in seconds)
  */
 enum nft_meta_keys {
 	NFT_META_LEN,
@@ -831,6 +834,9 @@ enum nft_meta_keys {
 	NFT_META_OIFKIND,
 	NFT_META_BRI_IIFPVID,
 	NFT_META_BRI_IIFVPROTO,
+	NFT_META_TIME_NS,
+	NFT_META_TIME_DAY,
+	NFT_META_TIME_HOUR,
 };
 
 /**
diff --git a/net/netfilter/nft_meta.c b/net/netfilter/nft_meta.c
index f1b1d948c07b..096b831e3758 100644
--- a/net/netfilter/nft_meta.c
+++ b/net/netfilter/nft_meta.c
@@ -26,8 +26,36 @@
 
 #include <uapi/linux/netfilter_bridge.h> /* NF_BR_PRE_ROUTING */
 
+#define NFT_META_SECS_PER_MINUTE	60
+#define NFT_META_SECS_PER_HOUR		3600
+#define NFT_META_SECS_PER_DAY		86400
+#define NFT_META_DAYS_PER_WEEK		7
+
 static DEFINE_PER_CPU(struct rnd_state, nft_prandom_state);
 
+static u8 nft_meta_weekday(unsigned long secs)
+{
+	unsigned int dse;
+	u8 wday;
+
+	secs -= NFT_META_SECS_PER_MINUTE * sys_tz.tz_minuteswest;
+	dse = secs / NFT_META_SECS_PER_DAY;
+	wday = (4 + dse) % NFT_META_DAYS_PER_WEEK;
+
+	return wday;
+}
+
+static u32 nft_meta_hour(unsigned long secs)
+{
+	struct tm tm;
+
+	time64_to_tm(secs, 0, &tm);
+
+	return tm.tm_hour * NFT_META_SECS_PER_HOUR
+		+ tm.tm_min * NFT_META_SECS_PER_MINUTE
+		+ tm.tm_sec;
+}
+
 void nft_meta_get_eval(const struct nft_expr *expr,
 		       struct nft_regs *regs,
 		       const struct nft_pktinfo *pkt)
@@ -226,6 +254,15 @@ 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_NS:
+		nft_reg_store64(dest, ktime_get_real_ns());
+		break;
+	case NFT_META_TIME_DAY:
+		nft_reg_store8(dest, nft_meta_weekday(get_seconds()));
+		break;
+	case NFT_META_TIME_HOUR:
+		*dest = nft_meta_hour(get_seconds());
+		break;
 	default:
 		WARN_ON(1);
 		goto err;
@@ -338,6 +375,15 @@ int nft_meta_get_init(const struct nft_ctx *ctx,
 		len = sizeof(u8);
 		break;
 #endif
+	case NFT_META_TIME_NS:
+		len = sizeof(u64);
+		break;
+	case NFT_META_TIME_DAY:
+		len = sizeof(u8);
+		break;
+	case NFT_META_TIME_HOUR:
+		len = sizeof(u32);
+		break;
 	default:
 		return -EOPNOTSUPP;
 	}
-- 
2.17.1

Re: [PATCH v5 2/2] netfilter: nft_meta: support for time matching

[Jones Desougi]

The naming of the new meta keys seem a bit confusing.

On Sat, Aug 17, 2019 at 1:19 PM Ander Juaristi <a@juaristi.eus> wrote:
>
> 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).
>
> All values are taken as unsigned 64-bit integers.
>
> The 'time' keyword is internally converted to nanoseconds by nft in
> userspace, and hence the timestamp is taken in nanoseconds as well.
>
> Signed-off-by: Ander Juaristi <a@juaristi.eus>
> ---
>  include/uapi/linux/netfilter/nf_tables.h |  6 ++++
>  net/netfilter/nft_meta.c                 | 46 ++++++++++++++++++++++++
>  2 files changed, 52 insertions(+)
>
> diff --git a/include/uapi/linux/netfilter/nf_tables.h b/include/uapi/linux/netfilter/nf_tables.h
> index 82abaa183fc3..b83b62eb4b01 100644
> --- a/include/uapi/linux/netfilter/nf_tables.h
> +++ b/include/uapi/linux/netfilter/nf_tables.h
> @@ -799,6 +799,9 @@ enum nft_exthdr_attributes {
>   * @NFT_META_OIFKIND: packet output interface kind name (dev->rtnl_link_ops->kind)
>   * @NFT_META_BRI_IIFPVID: packet input bridge port pvid
>   * @NFT_META_BRI_IIFVPROTO: packet input bridge vlan proto
> + * @NFT_META_TIME_NS: time since epoch (in nanoseconds)
> + * @NFT_META_TIME_DAY: day of week (from 0 = Sunday to 6 = Saturday)

This would be clearer as NFT_META_TIME_WEEKDAY. Just day can mean a
lot of things.
Matches nicely with the added nft_meta_weekday function too.

> + * @NFT_META_TIME_HOUR: hour of day (in seconds)

This isn't really an hour, so why call it that (confuses unit at least)?
Something like NFT_META_TIME_TIMEOFDAY? Alternatively TIMEINDAY.
Presumably the added nft_meta_hour function also derives its name from
this, but otherwise has nothing to do with hours.

>   */
>  enum nft_meta_keys {
>         NFT_META_LEN,
...

Re: [PATCH v5 2/2] netfilter: nft_meta: support for time matching

[Ander Juaristi]

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

On 17/8/19 15:43, Jones Desougi wrote:
>> diff --git a/include/uapi/linux/netfilter/nf_tables.h b/include/uapi/linux/netfilter/nf_tables.h
>> index 82abaa183fc3..b83b62eb4b01 100644
>> --- a/include/uapi/linux/netfilter/nf_tables.h
>> +++ b/include/uapi/linux/netfilter/nf_tables.h
>> @@ -799,6 +799,9 @@ enum nft_exthdr_attributes {
>>   * @NFT_META_OIFKIND: packet output interface kind name (dev->rtnl_link_ops->kind)
>>   * @NFT_META_BRI_IIFPVID: packet input bridge port pvid
>>   * @NFT_META_BRI_IIFVPROTO: packet input bridge vlan proto
>> + * @NFT_META_TIME_NS: time since epoch (in nanoseconds)
>> + * @NFT_META_TIME_DAY: day of week (from 0 = Sunday to 6 = Saturday)
>   
> This would be clearer as NFT_META_TIME_WEEKDAY. Just day can mean a
> lot of things.
> Matches nicely with the added nft_meta_weekday function too.

I agree with you here. Seems to me WEEKDAY is clearer.

>   
>> + * @NFT_META_TIME_HOUR: hour of day (in seconds)
>   
> This isn't really an hour, so why call it that (confuses unit at least)?
> Something like NFT_META_TIME_TIMEOFDAY? Alternatively TIMEINDAY.
> Presumably the added nft_meta_hour function also derives its name from
> this, but otherwise has nothing to do with hours.
>   

But not so sure on this one. TIMEOFDAY sounds to me equivalent to HOUR,
though less explicit (more ambiguous).

>>   */
>>  enum nft_meta_keys {
>>         NFT_META_LEN,
> ...
>   



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

Re: [PATCH v5 2/2] netfilter: nft_meta: support for time matching

[Jones Desougi]

On Sun, Aug 18, 2019 at 8:22 PM Ander Juaristi <a@juaristi.eus> wrote:
>
> On 17/8/19 15:43, Jones Desougi wrote:
> >> diff --git a/include/uapi/linux/netfilter/nf_tables.h b/include/uapi/linux/netfilter/nf_tables.h
> >> index 82abaa183fc3..b83b62eb4b01 100644
> >> --- a/include/uapi/linux/netfilter/nf_tables.h
> >> +++ b/include/uapi/linux/netfilter/nf_tables.h
> >> @@ -799,6 +799,9 @@ enum nft_exthdr_attributes {
> >>   * @NFT_META_OIFKIND: packet output interface kind name (dev->rtnl_link_ops->kind)
> >>   * @NFT_META_BRI_IIFPVID: packet input bridge port pvid
> >>   * @NFT_META_BRI_IIFVPROTO: packet input bridge vlan proto
> >> + * @NFT_META_TIME_NS: time since epoch (in nanoseconds)
> >> + * @NFT_META_TIME_DAY: day of week (from 0 = Sunday to 6 = Saturday)
> >
> > This would be clearer as NFT_META_TIME_WEEKDAY. Just day can mean a
> > lot of things.
> > Matches nicely with the added nft_meta_weekday function too.
>
> I agree with you here. Seems to me WEEKDAY is clearer.
>
> >
> >> + * @NFT_META_TIME_HOUR: hour of day (in seconds)
> >
> > This isn't really an hour, so why call it that (confuses unit at least)?
> > Something like NFT_META_TIME_TIMEOFDAY? Alternatively TIMEINDAY.
> > Presumably the added nft_meta_hour function also derives its name from
> > this, but otherwise has nothing to do with hours.
> >
>
> But not so sure on this one. TIMEOFDAY sounds to me equivalent to HOUR,
> though less explicit (more ambiguous).

HOUR is a unit, much like NS, but its use is quite different with no
clear hint as to how. Unlike the latter it's also not the unit of the
value. From that perspective the name comes up empty of meaning. If
you already know what it means, the name can be put in context, but
that's not explicit at all.

That said, weekday is more important.


>
> >>   */
> >>  enum nft_meta_keys {
> >>         NFT_META_LEN,
> > ...
> >
>
>

Re: [PATCH v5 2/2] netfilter: nft_meta: support for time matching

[Florian Westphal]

Jones Desougi <jones.desougi+netfilter@gmail.com> wrote:
> On Sun, Aug 18, 2019 at 8:22 PM Ander Juaristi <a@juaristi.eus> wrote:
> >
> > On 17/8/19 15:43, Jones Desougi wrote:
> > >> diff --git a/include/uapi/linux/netfilter/nf_tables.h b/include/uapi/linux/netfilter/nf_tables.h
> > >> index 82abaa183fc3..b83b62eb4b01 100644
> > >> --- a/include/uapi/linux/netfilter/nf_tables.h
> > >> +++ b/include/uapi/linux/netfilter/nf_tables.h
> > >> @@ -799,6 +799,9 @@ enum nft_exthdr_attributes {
> > >>   * @NFT_META_OIFKIND: packet output interface kind name (dev->rtnl_link_ops->kind)
> > >>   * @NFT_META_BRI_IIFPVID: packet input bridge port pvid
> > >>   * @NFT_META_BRI_IIFVPROTO: packet input bridge vlan proto
> > >> + * @NFT_META_TIME_NS: time since epoch (in nanoseconds)
> > >> + * @NFT_META_TIME_DAY: day of week (from 0 = Sunday to 6 = Saturday)
> > >
> > > This would be clearer as NFT_META_TIME_WEEKDAY. Just day can mean a
> > > lot of things.
> > > Matches nicely with the added nft_meta_weekday function too.
> >
> > I agree with you here. Seems to me WEEKDAY is clearer.
> >
> > >
> > >> + * @NFT_META_TIME_HOUR: hour of day (in seconds)
> > >
> > > This isn't really an hour, so why call it that (confuses unit at least)?
> > > Something like NFT_META_TIME_TIMEOFDAY? Alternatively TIMEINDAY.
> > > Presumably the added nft_meta_hour function also derives its name from
> > > this, but otherwise has nothing to do with hours.
> > >
> >
> > But not so sure on this one. TIMEOFDAY sounds to me equivalent to HOUR,
> > though less explicit (more ambiguous).
> 
> HOUR is a unit, much like NS, but its use is quite different with no
> clear hint as to how. Unlike the latter it's also not the unit of the
> value. From that perspective the name comes up empty of meaning. If
> you already know what it means, the name can be put in context, but
> that's not explicit at all.

If the NFT_META_TIME_* names are off, then those for the
frontend are too.

I think
meta time <iso-date>
meta hour <relative to this day>
meta day <weekday>

are fine, and thus so are the uapi enums.

Examples:

meta time < "2019-06-06 17:20:20" drop
meta hour 11:00-17:00 accept
meta day "Sat" drop

What would you suggest as alternatives?

Re: [PATCH v5 1/2] netfilter: Introduce new 64-bit helper functions

[Pablo Neira Ayuso]

On Sat, Aug 17, 2019 at 01:17:52PM +0200, Ander Juaristi wrote:
> Introduce new helper functions to load/store 64-bit values
> onto/from registers:
> 
>  - nft_reg_store64
>  - nft_reg_load64
> 
> This commit also re-orders all these helpers from smallest
> to largest target bit size.

Applied, thanks.

Re: [PATCH v5 2/2] netfilter: nft_meta: support for time matching

[Pablo Neira Ayuso]

On Sat, Aug 17, 2019 at 01:17:53PM +0200, Ander Juaristi wrote:
> 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).
> 
> All values are taken as unsigned 64-bit integers.
> 
> The 'time' keyword is internally converted to nanoseconds by nft in
> userspace, and hence the timestamp is taken in nanoseconds as well.

Also applied, thanks.

Re: [PATCH v5 2/2] netfilter: nft_meta: support for time matching

[Jones Desougi]

Sorry, been away.

On Tue, Aug 20, 2019 at 9:27 PM Florian Westphal <fw@strlen.de> wrote:
>
> Jones Desougi <jones.desougi+netfilter@gmail.com> wrote:
> > On Sun, Aug 18, 2019 at 8:22 PM Ander Juaristi <a@juaristi.eus> wrote:
> > >
> > > On 17/8/19 15:43, Jones Desougi wrote:
> > > >> diff --git a/include/uapi/linux/netfilter/nf_tables.h b/include/uapi/linux/netfilter/nf_tables.h
> > > >> index 82abaa183fc3..b83b62eb4b01 100644
> > > >> --- a/include/uapi/linux/netfilter/nf_tables.h
> > > >> +++ b/include/uapi/linux/netfilter/nf_tables.h
> > > >> @@ -799,6 +799,9 @@ enum nft_exthdr_attributes {
> > > >>   * @NFT_META_OIFKIND: packet output interface kind name (dev->rtnl_link_ops->kind)
> > > >>   * @NFT_META_BRI_IIFPVID: packet input bridge port pvid
> > > >>   * @NFT_META_BRI_IIFVPROTO: packet input bridge vlan proto
> > > >> + * @NFT_META_TIME_NS: time since epoch (in nanoseconds)
> > > >> + * @NFT_META_TIME_DAY: day of week (from 0 = Sunday to 6 = Saturday)
> > > >
> > > > This would be clearer as NFT_META_TIME_WEEKDAY. Just day can mean a
> > > > lot of things.
> > > > Matches nicely with the added nft_meta_weekday function too.
> > >
> > > I agree with you here. Seems to me WEEKDAY is clearer.
> > >
> > > >
> > > >> + * @NFT_META_TIME_HOUR: hour of day (in seconds)
> > > >
> > > > This isn't really an hour, so why call it that (confuses unit at least)?
> > > > Something like NFT_META_TIME_TIMEOFDAY? Alternatively TIMEINDAY.
> > > > Presumably the added nft_meta_hour function also derives its name from
> > > > this, but otherwise has nothing to do with hours.
> > > >
> > >
> > > But not so sure on this one. TIMEOFDAY sounds to me equivalent to HOUR,
> > > though less explicit (more ambiguous).
> >
> > HOUR is a unit, much like NS, but its use is quite different with no
> > clear hint as to how. Unlike the latter it's also not the unit of the
> > value. From that perspective the name comes up empty of meaning. If
> > you already know what it means, the name can be put in context, but
> > that's not explicit at all.
>
> If the NFT_META_TIME_* names are off, then those for the
> frontend are too.

In part, but they are not that tightly bound. Or shouldn't be. See
regarding hour below.

>
> I think
> meta time <iso-date>
> meta hour <relative to this day>
> meta day <weekday>
>
> are fine, and thus so are the uapi enums.
>
> Examples:
>
> meta time < "2019-06-06 17:20:20" drop
> meta hour 11:00-17:00 accept
> meta day "Sat" drop
>
> What would you suggest as alternatives?

Using weekday here as well would be the obvious choice, e.g.
meta weekday "Sat" drop
Not being explicit seems bad to me, especially in the kernel interface.

However, for hour the context is different.
Hour is relevant in this frontend interface, since the time
specification is hour anchored.
An alternative implementation might use nautic 'five bells in morning
watch' or whatever for the user, but it would still translate to time
of day in seconds in the kernel interface.