netfilter: nft_hash: Add hash offset value

PATCH Applied Laura Garcia Liebana 3 participants 6 messages Sep 6, 2016 – Sep 13, 2016

Subsystems (2)

Networking netfilter

lore

Share & tools

lore

Laura Garcia Liebana

author

2016-09-06 06:44 UTC #0 lore raw

PATCH v2 1/1 netfilter: nft_hash: Add hash offset value

Add support to pass through an offset to the hash value. With this feature, the sysadmin is able to generate a hash with a given offset value.

Example:

	meta mark set jhash ip saddr mod 2 seed 0xabcd sum 100

This option generates marks according to the source address from 100 to 101.

Signed-off-by Laura Garcia Liebana <[email protected]>

---
Changes in v2:
- Add check for hash + sum overflow.

 include/uapi/linux/netfilter/nf_tables.h |  2 ++
 net/netfilter/nft_hash.c                 | 16 ++++++++++++++--
 2 files changed, 16 insertions(+), 2 deletions(-)

include/uapi/linux/netfilter/nf_tables.h +2 -0

index 4dbeeed..8026684 100644
@@ -764,6 +764,7 @@ enum nft_meta_keys {
  * @NFTA_HASH_LEN: source data length (NLA_U32)
  * @NFTA_HASH_MODULUS: modulus value (NLA_U32)
  * @NFTA_HASH_SEED: seed value (NLA_U32)
+ * @NFTA_HASH_SUM: Hash offset value (NLA_U32)
  */
 enum nft_hash_attributes {
 	NFTA_HASH_UNSPEC,

@@ -772,6 +773,7 @@ enum nft_hash_attributes {
 	NFTA_HASH_LEN,
 	NFTA_HASH_MODULUS,
 	NFTA_HASH_SEED,
+	NFTA_HASH_SUM,
 	__NFTA_HASH_MAX,
 };
 #define NFTA_HASH_MAX	(__NFTA_HASH_MAX - 1)

net/netfilter/nft_hash.c +14 -2

index b7e3b40..2102c94 100644
@@ -23,6 +23,7 @@ struct nft_hash {
 	u8			len;
 	u32			modulus;
 	u32			seed;
+	u32			sum;
 };
 
 static void nft_hash_eval(const struct nft_expr *expr,

@@ -35,7 +36,7 @@ static void nft_hash_eval(const struct nft_expr *expr,
 
 	h = reciprocal_scale(jhash(data, priv->len, priv->seed), priv->modulus);
 
-	regs->data[priv->dreg] = h;
+	regs->data[priv->dreg] = priv->sum + h;
 }
 
 const struct nla_policy nft_hash_policy[NFTA_HASH_MAX + 1] = {

@@ -44,6 +45,7 @@ const struct nla_policy nft_hash_policy[NFTA_HASH_MAX + 1] = {
 	[NFTA_HASH_LEN]			= { .type = NLA_U32 },
 	[NFTA_HASH_MODULUS]		= { .type = NLA_U32 },
 	[NFTA_HASH_SEED]		= { .type = NLA_U32 },
+	[NFTA_HASH_SUM]			= { .type = NLA_U32 },
 };
 
 static int nft_hash_init(const struct nft_ctx *ctx,

@@ -60,6 +62,11 @@ static int nft_hash_init(const struct nft_ctx *ctx,
 	    !tb[NFTA_HASH_MODULUS])
 		return -EINVAL;
 
+	if (tb[NFTA_HASH_SUM])
+		priv->sum = ntohl(nla_get_be32(tb[NFTA_HASH_SUM]));
+	else
+		priv->sum = 0;
+
 	priv->sreg = nft_parse_register(tb[NFTA_HASH_SREG]);
 	if (priv->sreg < 0)
 		return -ERANGE;

@@ -77,6 +84,9 @@ static int nft_hash_init(const struct nft_ctx *ctx,
 	if (priv->modulus <= 1)
 		return -ERANGE;
 
+	if (priv->sum + priv->modulus - 1 < U32_MAX)
+		return -EOVERFLOW;
+
 	priv->seed = ntohl(nla_get_be32(tb[NFTA_HASH_SEED]));
 
 	return nft_validate_register_load(priv->sreg, priv->len) &&

@@ -99,7 +109,9 @@ static int nft_hash_dump(struct sk_buff *skb,
 		goto nla_put_failure;
 	if (nft_dump_register(skb, NFTA_HASH_SEED, priv->seed))
 		goto nla_put_failure;
-
+	if (priv->sum != 0)
+		if (nla_put_be32(skb, NFTA_HASH_SUM, htonl(priv->sum)))
+			goto nla_put_failure;
 	return 0;
 
 nla_put_failure:

Signature · 1 line

2.8.1

Pablo Neira Ayuso

maintainer

2016-09-12 16:35 UTC #1 lore raw

Hi Laura,

Quoted 3 lines · Laura Garcia Liebana

Add support to pass through an offset to the hash value. With this feature, the sysadmin is able to generate a hash with a given offset value.

We've been using 'offset' to refer to this for a while, to I'm renaming this to NFTA_HASH_OFFSET.

net/netfilter/nft_hash.c static int nft_hash_init(const struct nft_ctx *ctx, · line 68

Quoted 12 lines

net/netfilter/nft_hash.c +4 -0

index b7e3b40..2102c94 100644
@@ -60,6 +62,11 @@ static int nft_hash_init(const struct nft_ctx *ctx,
 	    !tb[NFTA_HASH_MODULUS])
 		return -EINVAL;
 
+	if (tb[NFTA_HASH_SUM])
+		priv->sum = ntohl(nla_get_be32(tb[NFTA_HASH_SUM]));
+	else
+		priv->sum = 0;

priv is allocated via kzalloc, so this priv->sum = 0 is not required.

No need to resend, I have revamped your patch, keeping your authorship in place.

Quoted 1 line

From cf4cdeb8d5a2ea4cd27c1d9cf95aba4ce1e194b5 Mon Sep 17 00:00:00 2001

From: Laura Garcia Liebana <[email protected]>
Date: Tue, 6 Sep 2016 08:44:19 +0200
Subject: [PATCH] netfilter: nft_hash: Add hash offset value

Add support to pass through an offset to the hash value. With this feature, the sysadmin is able to generate a hash with a given offset value.

Example:

	meta mark set jhash ip saddr mod 2 seed 0xabcd plus 100

This option generates marks according to the source address from 100 to 101.

Signed-off-by Laura Garcia Liebana <[email protected]>

---
 include/uapi/linux/netfilter/nf_tables.h |  2 ++
 net/netfilter/nft_hash.c                 | 17 +++++++++++++----
 2 files changed, 15 insertions(+), 4 deletions(-)

include/uapi/linux/netfilter/nf_tables.h +2 -0

index 24161e2..8c653bb 100644
@@ -731,6 +731,7 @@ enum nft_meta_keys {
  * @NFTA_HASH_LEN: source data length (NLA_U32)
  * @NFTA_HASH_MODULUS: modulus value (NLA_U32)
  * @NFTA_HASH_SEED: seed value (NLA_U32)
+ * @NFTA_HASH_OFFSET: add this offset value to hash result (NLA_U32)
  */
 enum nft_hash_attributes {
 	NFTA_HASH_UNSPEC,

@@ -739,6 +740,7 @@ enum nft_hash_attributes {
 	NFTA_HASH_LEN,
 	NFTA_HASH_MODULUS,
 	NFTA_HASH_SEED,
+	NFTA_HASH_OFFSET,
 	__NFTA_HASH_MAX,
 };
 #define NFTA_HASH_MAX	(__NFTA_HASH_MAX - 1)

net/netfilter/nft_hash.c +13 -4

index 764251d..bd12f7a 100644
@@ -23,6 +23,7 @@ struct nft_hash {
 	u8			len;
 	u32			modulus;
 	u32			seed;
+	u32			offset;
 };
 
 static void nft_hash_eval(const struct nft_expr *expr,

@@ -31,10 +32,10 @@ static void nft_hash_eval(const struct nft_expr *expr,
 {
 	struct nft_hash *priv = nft_expr_priv(expr);
 	const void *data = &regs->data[priv->sreg];
+	u32 h;
 
-	regs->data[priv->dreg] =
-		reciprocal_scale(jhash(data, priv->len, priv->seed),
-				 priv->modulus);
+	h = reciprocal_scale(jhash(data, priv->len, priv->seed), priv->modulus);
+	regs->data[priv->dreg] = h + priv->offset;
 }
 
 static const struct nla_policy nft_hash_policy[NFTA_HASH_MAX + 1] = {

@@ -59,6 +60,9 @@ static int nft_hash_init(const struct nft_ctx *ctx,
 	    !tb[NFTA_HASH_MODULUS])
 		return -EINVAL;
 
+	if (tb[NFTA_HASH_OFFSET])
+		priv->offset = ntohl(nla_get_be32(tb[NFTA_HASH_OFFSET]));
+
 	priv->sreg = nft_parse_register(tb[NFTA_HASH_SREG]);
 	priv->dreg = nft_parse_register(tb[NFTA_HASH_DREG]);

@@ -72,6 +76,9 @@ static int nft_hash_init(const struct nft_ctx *ctx,
 	if (priv->modulus <= 1)
 		return -ERANGE;
 
+	if (priv->offset + priv->modulus - 1 < U32_MAX)
+		return -EOVERFLOW;
+
 	priv->seed = ntohl(nla_get_be32(tb[NFTA_HASH_SEED]));
 
 	return nft_validate_register_load(priv->sreg, len) &&

@@ -94,7 +101,9 @@ static int nft_hash_dump(struct sk_buff *skb,
 		goto nla_put_failure;
 	if (nla_put_be32(skb, NFTA_HASH_SEED, htonl(priv->seed)))
 		goto nla_put_failure;
-
+	if (priv->offset != 0)
+		if (nla_put_be32(skb, NFTA_HASH_OFFSET, htonl(priv->offset)))
+			goto nla_put_failure;
 	return 0;
 
 nla_put_failure:

Signature · 1 line

2.1.4

Laura Garcia

author

2016-09-12 16:57 UTC #2 lore raw

Quoted 10 lines · Pablo Neira Ayuso

Hi Laura,
Quoted 3 lines · Laura Garcia Liebana
Add support to pass through an offset to the hash value. With this feature, the sysadmin is able to generate a hash with a given offset value.
We've been using 'offset' to refer to this for a while, to I'm renaming this to NFTA_HASH_OFFSET.

Ok, I'll send a new patch to change this for libnftnl.

Thank you!

Pablo Neira Ayuso

maintainer Applied

2016-09-12 18:31 UTC #3 lore raw

In reply to Laura Garcia Liebana

Quoted 10 lines · Laura Garcia Liebana

Add support to pass through an offset to the hash value. With this feature, the sysadmin is able to generate a hash with a given offset value.
Example:
	meta mark set jhash ip saddr mod 2 seed 0xabcd sum 100
This option generates marks according to the source address from 100 to 101.

Applied, thanks.

I have renamed all these to _OFFSET.

Liping Zhang

2016-09-13 06:25 UTC #4 lore raw

In reply to Laura Garcia Liebana

Hi Laura,

2016-09-06 14:44 GMT+08:00 Laura Garcia Liebana <[email protected]>:

Quoted 14 lines

static int nft_hash_init(const struct nft_ctx *ctx,

net/netfilter/nft_hash.c +5 -0

@@ -60,6 +62,11 @@ static int nft_hash_init(const struct nft_ctx *ctx,
            !tb[NFTA_HASH_MODULUS])
                return -EINVAL;
 
+       if (tb[NFTA_HASH_SUM])
+               priv->sum = ntohl(nla_get_be32(tb[NFTA_HASH_SUM]));
+       else
+               priv->sum = 0;
+
        priv->sreg = nft_parse_register(tb[NFTA_HASH_SREG]);
        if (priv->sreg < 0)
                return -ERANGE;

net/netfilter/nft_hash.c static int nft_hash_init(const struct nft_ctx *ctx, · line 88

Quoted 5 lines

net/netfilter/nft_hash.c +2 -0

@@ -77,6 +84,9 @@ static int nft_hash_init(const struct nft_ctx *ctx,
        if (priv->modulus <= 1)
                return -ERANGE;
 
+       if (priv->sum + priv->modulus - 1 < U32_MAX)
+               return -EOVERFLOW;

I think this judgement here is wrong, it is likely to be true...

When two integer a and b do addition operation, and the calculation results satisfy the
following conditions: (a + b < a) or (a + b < b), then we can assure that integer overflow
happened.

So the judgement should be converted to:

     if (priv->sum + priv->modulus - 1 < priv->sum)

Quoted 4 lines

net/netfilter/nft_hash.c +1 -0

+
        priv->seed = ntohl(nla_get_be32(tb[NFTA_HASH_SEED]));
 
        return nft_validate_register_load(priv->sreg, priv->len) &&

Laura Garcia

author

2016-09-13 07:59 UTC #5 lore raw

Quoted 25 lines · Liping Zhang

Hi Laura,

2016-09-06 14:44 GMT+08:00 Laura Garcia Liebana <[email protected]>:

Quoted 14 lines

static int nft_hash_init(const struct nft_ctx *ctx,

net/netfilter/nft_hash.c +5 -0

@@ -60,6 +62,11 @@ static int nft_hash_init(const struct nft_ctx *ctx,
            !tb[NFTA_HASH_MODULUS])
                return -EINVAL;
 
+       if (tb[NFTA_HASH_SUM])
+               priv->sum = ntohl(nla_get_be32(tb[NFTA_HASH_SUM]));
+       else
+               priv->sum = 0;
+
        priv->sreg = nft_parse_register(tb[NFTA_HASH_SREG]);
        if (priv->sreg < 0)
                return -ERANGE;

net/netfilter/nft_hash.c static int nft_hash_init(const struct nft_ctx *ctx, · line 88

Quoted 5 lines

net/netfilter/nft_hash.c +2 -0

@@ -77,6 +84,9 @@ static int nft_hash_init(const struct nft_ctx *ctx,
        if (priv->modulus <= 1)
                return -ERANGE;
 
+       if (priv->sum + priv->modulus - 1 < U32_MAX)
+               return -EOVERFLOW;

I think this judgement here is wrong, it is likely to be true...

Quoted 9 lines

When two integer a and b do addition operation, and the calculation results satisfy the
following conditions: (a + b < a) or (a + b < b), then we can assure that integer overflow
happened.
So the judgement should be converted to:
     if (priv->sum + priv->modulus - 1 < priv->sum)

Absolutely true, i'll send a patch to fix that.

Thank you!

Quoted 4 lines

net/netfilter/nft_hash.c +1 -0

+
        priv->seed = ntohl(nla_get_be32(tb[NFTA_HASH_SEED]));
 
        return nft_validate_register_load(priv->sreg, priv->len) &&