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

[PATCH v4 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

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

diff --git a/include/net/netfilter/nf_tables.h b/include/net/netfilter/nf_tables.h
index 9b624566b82d..aa33ada8728a 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>
@@ -119,6 +120,16 @@ static inline void nft_reg_store8(u32 *dreg, u8 val)
 	*(u8 *)dreg = val;
 }
 
+static inline void nft_reg_store64(u32 *dreg, u64 val)
+{
+	put_unaligned(val, (u64 *)dreg);
+}
+
+static inline u64 nft_reg_load64(u32 *sreg)
+{
+	return get_unaligned((u64 *)sreg);
+}
+
 static inline u16 nft_reg_load16(u32 *sreg)
 {
 	return *(u16 *)sreg;
diff --git a/net/netfilter/nft_byteorder.c b/net/netfilter/nft_byteorder.c
index e06318428ea0..a25a222d94c8 100644
--- a/net/netfilter/nft_byteorder.c
+++ b/net/netfilter/nft_byteorder.c
@@ -43,14 +43,14 @@ 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], cpu_to_be64(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 = be64_to_cpu(nft_reg_load64(&src[i]));
+				nft_reg_store64(&dst[i], src64);
 			}
 			break;
 		}
-- 
2.17.1

[PATCH v4 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.

This patch also introduces a new function, nft_reg_store64, to store
64-bit values in the register for comparison.

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

diff --git a/include/uapi/linux/netfilter/nf_tables.h b/include/uapi/linux/netfilter/nf_tables.h
index 82abaa183fc3..67ae55e08518 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: a UNIX timestamp
+ * @NFT_META_TIME_DAY: day of week
+ * @NFT_META_TIME_HOUR: hour of day
  */
 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,
+	NFT_META_TIME_DAY,
+	NFT_META_TIME_HOUR,
 };
 
 /**
diff --git a/net/netfilter/nft_meta.c b/net/netfilter/nft_meta.c
index f1b1d948c07b..3e665a1a744a 100644
--- a/net/netfilter/nft_meta.c
+++ b/net/netfilter/nft_meta.c
@@ -28,6 +28,27 @@
 
 static DEFINE_PER_CPU(struct rnd_state, nft_prandom_state);
 
+static u8 nft_meta_weekday(unsigned long secs)
+{
+	u8 wday;
+	unsigned int dse;
+
+	secs -= 60 * sys_tz.tz_minuteswest;
+	dse = secs / 86400;
+	wday = (4 + dse) % 7;
+
+	return wday;
+}
+
+static u32 nft_meta_hour(unsigned long secs)
+{
+	struct tm tm;
+
+	time64_to_tm(secs, 0, &tm);
+
+	return tm.tm_hour * 3600 + tm.tm_min * 60 + tm.tm_sec;
+}
+
 void nft_meta_get_eval(const struct nft_expr *expr,
 		       struct nft_regs *regs,
 		       const struct nft_pktinfo *pkt)
@@ -226,6 +247,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:
+		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 +368,15 @@ int nft_meta_get_init(const struct nft_ctx *ctx,
 		len = sizeof(u8);
 		break;
 #endif
+	case NFT_META_TIME:
+		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 v4 1/2] netfilter: Introduce new 64-bit helper functions

[Pablo Neira Ayuso]

On Tue, Aug 13, 2019 at 08:38:19PM +0200, Ander Juaristi wrote:
[...]
> diff --git a/include/net/netfilter/nf_tables.h b/include/net/netfilter/nf_tables.h
> index 9b624566b82d..aa33ada8728a 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>
> @@ -119,6 +120,16 @@ static inline void nft_reg_store8(u32 *dreg, u8 val)
>  	*(u8 *)dreg = val;
>  }
>  
> +static inline void nft_reg_store64(u32 *dreg, u64 val)
> +{
> +	put_unaligned(val, (u64 *)dreg);
> +}
> +
> +static inline u64 nft_reg_load64(u32 *sreg)
> +{
> +	return get_unaligned((u64 *)sreg);
> +}

Please, add these function definition below _load16() and _store16().
> +
>  static inline u16 nft_reg_load16(u32 *sreg)
>  {
>  	return *(u16 *)sreg;
> diff --git a/net/netfilter/nft_byteorder.c b/net/netfilter/nft_byteorder.c
> index e06318428ea0..a25a222d94c8 100644
> --- a/net/netfilter/nft_byteorder.c
> +++ b/net/netfilter/nft_byteorder.c
> @@ -43,14 +43,14 @@ void nft_byteorder_eval(const struct nft_expr *expr,
>  		switch (priv->op) {
>  		case NFT_BYTEORDER_NTOH:

This is network-to-host byteorder.

>  			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], cpu_to_be64(src64));

This looks inverted, this should be:

				nft_reg_store64(&dst[i], be64_to_cpu(src64));

right?

>  			}
>  			break;
>  		case NFT_BYTEORDER_HTON:

Here, network-to-host byteorder:

>  			for (i = 0; i < priv->len / 8; i++) {
> -				src64 = get_unaligned_be64(&src[i]);
> -				put_unaligned(src64, (u64 *)&dst[i]);
> +				src64 = be64_to_cpu(nft_reg_load64(&src[i]));

and this:

                                src64 = (__force __u64)
                                        cpu_to_be64(nft_reg_load64(&src[i]));

The (__force __u64) just makes 'sparse' happy [1].

[1] https://www.kernel.org/doc/html/v4.12/dev-tools/sparse.html

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

[Pablo Neira Ayuso]

Just a few nitpicks and we go :-)

On Tue, Aug 13, 2019 at 08:38:20PM +0200, Ander Juaristi wrote:
> diff --git a/include/uapi/linux/netfilter/nf_tables.h b/include/uapi/linux/netfilter/nf_tables.h
> index 82abaa183fc3..67ae55e08518 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: a UNIX timestamp

    * @NFT_META_TIME: time since 1970 (in nanoseconds)

Probably rename this to NFT_META_TIME_NS I'd suggest.

> + * @NFT_META_TIME_DAY: day of week

  + * @NFT_META_TIME_DAY: day of week (from 0 = Sunday to 6 = Saturday)

> + * @NFT_META_TIME_HOUR: hour of day

  + * @NFT_META_TIME_HOUR: hour of the 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,
> +	NFT_META_TIME_DAY,
> +	NFT_META_TIME_HOUR,
>  };
>  
>  /**
> diff --git a/net/netfilter/nft_meta.c b/net/netfilter/nft_meta.c
> index f1b1d948c07b..3e665a1a744a 100644
> --- a/net/netfilter/nft_meta.c
> +++ b/net/netfilter/nft_meta.c
> @@ -28,6 +28,27 @@
>  
>  static DEFINE_PER_CPU(struct rnd_state, nft_prandom_state);

Probably a few constant definitions to be used in nft_meta_weekday().

#define NFT_NETA_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

these numbers are easy to guess, but this helps along time to read
this code.

> +static u8 nft_meta_weekday(unsigned long secs)
> +{
> +	u8 wday;
> +	unsigned int dse;

Reverse definition, from longest to shortest line:

	unsigned int dse;
	u8 wday;

> +	secs -= 60 * sys_tz.tz_minuteswest;
> +	dse = secs / 86400;
> +	wday = (4 + dse) % 7;
> +
> +	return wday;
> +}
> +
> +static u32 nft_meta_hour(unsigned long secs)
> +{
> +	struct tm tm;
> +
> +	time64_to_tm(secs, 0, &tm);
> +
> +	return tm.tm_hour * 3600 + tm.tm_min * 60 + tm.tm_sec;
> +}
> +
>  void nft_meta_get_eval(const struct nft_expr *expr,
>  		       struct nft_regs *regs,
>  		       const struct nft_pktinfo *pkt)
> @@ -226,6 +247,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:
> +		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 +368,15 @@ int nft_meta_get_init(const struct nft_ctx *ctx,
>  		len = sizeof(u8);
>  		break;
>  #endif
> +	case NFT_META_TIME:
> +		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 v4 2/2] netfilter: nft_meta: support for time matching

[Florian Westphal]

Ander Juaristi <a@juaristi.eus> wrote:
> +++ b/net/netfilter/nft_meta.c
> @@ -28,6 +28,27 @@
>  
>  static DEFINE_PER_CPU(struct rnd_state, nft_prandom_state);
>  
> +static u8 nft_meta_weekday(unsigned long secs)
> +{
> +	u8 wday;
> +	unsigned int dse;
> +
> +	secs -= 60 * sys_tz.tz_minuteswest;
> +	dse = secs / 86400;

This will probably fail to compile (link) on 32bit arches.
You need to use do_div() here.

> +	wday = (4 + dse) % 7;

This is fine, as this only involves 32bit integers.

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

[Florian Westphal]

Florian Westphal <fw@strlen.de> wrote:
> Ander Juaristi <a@juaristi.eus> wrote:
> > +++ b/net/netfilter/nft_meta.c
> > @@ -28,6 +28,27 @@
> >  
> >  static DEFINE_PER_CPU(struct rnd_state, nft_prandom_state);
> >  
> > +static u8 nft_meta_weekday(unsigned long secs)
> > +{
> > +	u8 wday;
> > +	unsigned int dse;
> > +
> > +	secs -= 60 * sys_tz.tz_minuteswest;
> > +	dse = secs / 86400;
> 
> This will probably fail to compile (link) on 32bit arches.
> You need to use do_div() here.

Scratch that, this will work fine (unsigned long is not a 64 bit
type in that case).

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

[Ander Juaristi]

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



On 13/8/19 20:58, Pablo Neira Ayuso wrote:
> On Tue, Aug 13, 2019 at 08:38:19PM +0200, Ander Juaristi wrote:
> [...]
>> diff --git a/include/net/netfilter/nf_tables.h b/include/net/netfilter/nf_tables.h
>> index 9b624566b82d..aa33ada8728a 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>
>> @@ -119,6 +120,16 @@ static inline void nft_reg_store8(u32 *dreg, u8 val)
>>  	*(u8 *)dreg = val;
>>  }
>>  
>> +static inline void nft_reg_store64(u32 *dreg, u64 val)
>> +{
>> +	put_unaligned(val, (u64 *)dreg);
>> +}
>> +
>> +static inline u64 nft_reg_load64(u32 *sreg)
>> +{
>> +	return get_unaligned((u64 *)sreg);
>> +}
> 
> Please, add these function definition below _load16() and _store16().

You mean you'd like them ordered from smaller to larger?

nft_reg_store8
nft_reg_load8
nft_reg_store16
nft_reg_load16
nft_reg_store64
nft_reg_load64

>> +
>>  static inline u16 nft_reg_load16(u32 *sreg)
>>  {
>>  	return *(u16 *)sreg;
>> diff --git a/net/netfilter/nft_byteorder.c b/net/netfilter/nft_byteorder.c
>> index e06318428ea0..a25a222d94c8 100644
>> --- a/net/netfilter/nft_byteorder.c
>> +++ b/net/netfilter/nft_byteorder.c
>> @@ -43,14 +43,14 @@ void nft_byteorder_eval(const struct nft_expr *expr,
>>  		switch (priv->op) {
>>  		case NFT_BYTEORDER_NTOH:
> 
> This is network-to-host byteorder.
> 
>>  			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], cpu_to_be64(src64));
> 
> This looks inverted, this should be:
> 
> 				nft_reg_store64(&dst[i], be64_to_cpu(src64));
> 
> right?
> 
>>  			}
>>  			break;
>>  		case NFT_BYTEORDER_HTON:
> 
> Here, network-to-host byteorder:
> 
>>  			for (i = 0; i < priv->len / 8; i++) {
>> -				src64 = get_unaligned_be64(&src[i]);
>> -				put_unaligned(src64, (u64 *)&dst[i]);
>> +				src64 = be64_to_cpu(nft_reg_load64(&src[i]));
> 
> and this:
> 
>                                 src64 = (__force __u64)
>                                         cpu_to_be64(nft_reg_load64(&src[i]));
> 

My bad. Yes, I've just fixed them.

> The (__force __u64) just makes 'sparse' happy [1].
> 
> [1] https://www.kernel.org/doc/html/v4.12/dev-tools/sparse.html
> 

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

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

[Pablo Neira Ayuso]

On Thu, Aug 15, 2019 at 11:46:04AM +0200, Ander Juaristi wrote:
> 
> 
> On 13/8/19 20:58, Pablo Neira Ayuso wrote:
> > On Tue, Aug 13, 2019 at 08:38:19PM +0200, Ander Juaristi wrote:
> > [...]
> >> diff --git a/include/net/netfilter/nf_tables.h b/include/net/netfilter/nf_tables.h
> >> index 9b624566b82d..aa33ada8728a 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>
> >> @@ -119,6 +120,16 @@ static inline void nft_reg_store8(u32 *dreg, u8 val)
> >>  	*(u8 *)dreg = val;
> >>  }
> >>  
> >> +static inline void nft_reg_store64(u32 *dreg, u64 val)
> >> +{
> >> +	put_unaligned(val, (u64 *)dreg);
> >> +}
> >> +
> >> +static inline u64 nft_reg_load64(u32 *sreg)
> >> +{
> >> +	return get_unaligned((u64 *)sreg);
> >> +}
> > 
> > Please, add these function definition below _load16() and _store16().
> 
> You mean you'd like them ordered from smaller to larger?
> 
> nft_reg_store8
> nft_reg_load8
> nft_reg_store16
> nft_reg_load16
> nft_reg_store64
> nft_reg_load64

yes please.

> >> +
> >>  static inline u16 nft_reg_load16(u32 *sreg)
> >>  {
> >>  	return *(u16 *)sreg;
> >> diff --git a/net/netfilter/nft_byteorder.c b/net/netfilter/nft_byteorder.c
> >> index e06318428ea0..a25a222d94c8 100644
> >> --- a/net/netfilter/nft_byteorder.c
> >> +++ b/net/netfilter/nft_byteorder.c
> >> @@ -43,14 +43,14 @@ void nft_byteorder_eval(const struct nft_expr *expr,
> >>  		switch (priv->op) {
> >>  		case NFT_BYTEORDER_NTOH:
> > 
> > This is network-to-host byteorder.
> > 
> >>  			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], cpu_to_be64(src64));
> > 
> > This looks inverted, this should be:
> > 
> > 				nft_reg_store64(&dst[i], be64_to_cpu(src64));
> > 
> > right?
> > 
> >>  			}
> >>  			break;
> >>  		case NFT_BYTEORDER_HTON:
> > 
> > Here, network-to-host byteorder:
> > 
> >>  			for (i = 0; i < priv->len / 8; i++) {
> >> -				src64 = get_unaligned_be64(&src[i]);
> >> -				put_unaligned(src64, (u64 *)&dst[i]);
> >> +				src64 = be64_to_cpu(nft_reg_load64(&src[i]));
> > 
> > and this:
> > 
> >                                 src64 = (__force __u64)
> >                                         cpu_to_be64(nft_reg_load64(&src[i]));
> > 
> 
> My bad. Yes, I've just fixed them.

Great.