[PATCH nft 0/3] nft: fix ct zone handling in sets and maps

[PATCH nft 0/3] nft: fix ct zone handling in sets and maps

[Florian Westphal]

'ct zone' (and other expressions w. host byte order and integer dtype)
are not handled correctly on little endian platforms.

First patch adds a test case that demonstrates the problem,
patch 2 and 3 resolve this for the mapping and set key cases.

Florian Westphal (3):
  tests: extend dtype test case to cover expression with integer type
  evaluate: pick data element byte order, not dtype one
  evaluate: set evaluation context for set elements

 src/evaluate.c                                | 13 ++++--
 .../testcases/sets/0029named_ifname_dtype_0   | 41 +++++++++++++++++
 .../sets/dumps/0029named_ifname_dtype_0.nft   | 44 ++++++++++++++++++-
 3 files changed, 93 insertions(+), 5 deletions(-)

-- 
2.26.2

[PATCH nft 1/3] extend_test

[Florian Westphal]

---
 .../testcases/sets/0029named_ifname_dtype_0   | 41 +++++++++++++++++
 .../sets/dumps/0029named_ifname_dtype_0.nft   | 44 ++++++++++++++++++-
 2 files changed, 83 insertions(+), 2 deletions(-)

diff --git a/tests/shell/testcases/sets/0029named_ifname_dtype_0 b/tests/shell/testcases/sets/0029named_ifname_dtype_0
index 39b3c90cf8dc..2dbcd22bb2ce 100755
--- a/tests/shell/testcases/sets/0029named_ifname_dtype_0
+++ b/tests/shell/testcases/sets/0029named_ifname_dtype_0
@@ -13,12 +13,53 @@ EXPECTED="table inet t {
 		elements = { \"ssh\" . \"eth0\" }
 	}
 
+	set nv {
+		type ifname . mark
+	}
+
+	set z {
+		typeof ct zone
+		elements = { 1 }
+	}
+
+	set m {
+		typeof meta mark
+		elements = { 1 }
+	}
+
+	map cz {
+		typeof meta iifname : ct zone
+		elements = { \"veth4\" : 1 }
+	}
+
+	map cm {
+		typeof meta iifname : ct mark
+		elements = { \"veth4\" : 1 }
+	}
+
 	chain c {
 		iifname @s accept
 		oifname @s accept
 		tcp dport . meta iifname @sc accept
+		meta iifname . meta mark @nv accept
 	}
 }"
 
 set -e
 $NFT -f - <<< "$EXPECTED"
+$NFT add element inet t s '{ "eth1" }'
+$NFT add element inet t s '{ "eth2", "eth3", "veth1" }'
+
+$NFT add element inet t sc '{ 80 . "eth0" }'
+$NFT add element inet t sc '{ 80 . "eth0" }' || true
+$NFT add element inet t sc '{ 80 . "eth1" }'
+$NFT add element inet t sc '{ 81 . "eth0" }'
+
+$NFT add element inet t nv '{ "eth0" . 1 }'
+$NFT add element inet t nv '{ "eth0" . 2 }'
+
+$NFT add element inet t z '{ 2, 3, 4, 5, 6 }'
+$NFT add element inet t cz '{ "eth0" : 1,  "eth1" : 2 }'
+
+$NFT add element inet t m '{ 2, 3, 4, 5, 6 }'
+$NFT add element inet t cm '{ "eth0" : 1,  "eth1" : 2 }'
diff --git a/tests/shell/testcases/sets/dumps/0029named_ifname_dtype_0.nft b/tests/shell/testcases/sets/dumps/0029named_ifname_dtype_0.nft
index 23ff89bb90e4..55cd4f262b35 100644
--- a/tests/shell/testcases/sets/dumps/0029named_ifname_dtype_0.nft
+++ b/tests/shell/testcases/sets/dumps/0029named_ifname_dtype_0.nft
@@ -1,17 +1,57 @@
 table inet t {
 	set s {
 		type ifname
-		elements = { "eth0" }
+		elements = { "eth0",
+			     "eth1",
+			     "eth2",
+			     "eth3",
+			     "veth1" }
 	}
 
 	set sc {
 		type inet_service . ifname
-		elements = { 22 . "eth0" }
+		elements = { 22 . "eth0",
+			     80 . "eth0",
+			     81 . "eth0",
+			     80 . "eth1" }
+	}
+
+	set nv {
+		type ifname . mark
+		elements = { "eth0" . 0x00000001,
+			     "eth0" . 0x00000002 }
+	}
+
+	set z {
+		typeof ct zone
+		elements = { 1, 2, 3, 4, 5,
+			     6 }
+	}
+
+	set m {
+		typeof meta mark
+		elements = { 0x00000001, 0x00000002, 0x00000003, 0x00000004, 0x00000005,
+			     0x00000006 }
+	}
+
+	map cz {
+		typeof iifname : ct zone
+		elements = { "eth0" : 1,
+			     "eth1" : 2,
+			     "veth4" : 1 }
+	}
+
+	map cm {
+		typeof iifname : ct mark
+		elements = { "eth0" : 0x00000001,
+			     "eth1" : 0x00000002,
+			     "veth4" : 0x00000001 }
 	}
 
 	chain c {
 		iifname @s accept
 		oifname @s accept
 		tcp dport . iifname @sc accept
+		iifname . meta mark @nv accept
 	}
 }
-- 
2.26.2

[PATCH nft 1/3] tests: extend dtype test case to cover expression with integer type

[Florian Westphal]

... nft doesn't handle this correctly at the moment: they are added
as netowkr byte order (invalid byte order).

ct zone has integer_type, the byte order has to be taken from the expression.

Signed-off-by: Florian Westphal <fw@strlen.de>
---
 .../testcases/sets/0029named_ifname_dtype_0   | 41 +++++++++++++++++
 .../sets/dumps/0029named_ifname_dtype_0.nft   | 44 ++++++++++++++++++-
 2 files changed, 83 insertions(+), 2 deletions(-)

diff --git a/tests/shell/testcases/sets/0029named_ifname_dtype_0 b/tests/shell/testcases/sets/0029named_ifname_dtype_0
index 39b3c90cf8dc..2dbcd22bb2ce 100755
--- a/tests/shell/testcases/sets/0029named_ifname_dtype_0
+++ b/tests/shell/testcases/sets/0029named_ifname_dtype_0
@@ -13,12 +13,53 @@ EXPECTED="table inet t {
 		elements = { \"ssh\" . \"eth0\" }
 	}
 
+	set nv {
+		type ifname . mark
+	}
+
+	set z {
+		typeof ct zone
+		elements = { 1 }
+	}
+
+	set m {
+		typeof meta mark
+		elements = { 1 }
+	}
+
+	map cz {
+		typeof meta iifname : ct zone
+		elements = { \"veth4\" : 1 }
+	}
+
+	map cm {
+		typeof meta iifname : ct mark
+		elements = { \"veth4\" : 1 }
+	}
+
 	chain c {
 		iifname @s accept
 		oifname @s accept
 		tcp dport . meta iifname @sc accept
+		meta iifname . meta mark @nv accept
 	}
 }"
 
 set -e
 $NFT -f - <<< "$EXPECTED"
+$NFT add element inet t s '{ "eth1" }'
+$NFT add element inet t s '{ "eth2", "eth3", "veth1" }'
+
+$NFT add element inet t sc '{ 80 . "eth0" }'
+$NFT add element inet t sc '{ 80 . "eth0" }' || true
+$NFT add element inet t sc '{ 80 . "eth1" }'
+$NFT add element inet t sc '{ 81 . "eth0" }'
+
+$NFT add element inet t nv '{ "eth0" . 1 }'
+$NFT add element inet t nv '{ "eth0" . 2 }'
+
+$NFT add element inet t z '{ 2, 3, 4, 5, 6 }'
+$NFT add element inet t cz '{ "eth0" : 1,  "eth1" : 2 }'
+
+$NFT add element inet t m '{ 2, 3, 4, 5, 6 }'
+$NFT add element inet t cm '{ "eth0" : 1,  "eth1" : 2 }'
diff --git a/tests/shell/testcases/sets/dumps/0029named_ifname_dtype_0.nft b/tests/shell/testcases/sets/dumps/0029named_ifname_dtype_0.nft
index 23ff89bb90e4..55cd4f262b35 100644
--- a/tests/shell/testcases/sets/dumps/0029named_ifname_dtype_0.nft
+++ b/tests/shell/testcases/sets/dumps/0029named_ifname_dtype_0.nft
@@ -1,17 +1,57 @@
 table inet t {
 	set s {
 		type ifname
-		elements = { "eth0" }
+		elements = { "eth0",
+			     "eth1",
+			     "eth2",
+			     "eth3",
+			     "veth1" }
 	}
 
 	set sc {
 		type inet_service . ifname
-		elements = { 22 . "eth0" }
+		elements = { 22 . "eth0",
+			     80 . "eth0",
+			     81 . "eth0",
+			     80 . "eth1" }
+	}
+
+	set nv {
+		type ifname . mark
+		elements = { "eth0" . 0x00000001,
+			     "eth0" . 0x00000002 }
+	}
+
+	set z {
+		typeof ct zone
+		elements = { 1, 2, 3, 4, 5,
+			     6 }
+	}
+
+	set m {
+		typeof meta mark
+		elements = { 0x00000001, 0x00000002, 0x00000003, 0x00000004, 0x00000005,
+			     0x00000006 }
+	}
+
+	map cz {
+		typeof iifname : ct zone
+		elements = { "eth0" : 1,
+			     "eth1" : 2,
+			     "veth4" : 1 }
+	}
+
+	map cm {
+		typeof iifname : ct mark
+		elements = { "eth0" : 0x00000001,
+			     "eth1" : 0x00000002,
+			     "veth4" : 0x00000001 }
 	}
 
 	chain c {
 		iifname @s accept
 		oifname @s accept
 		tcp dport . iifname @sc accept
+		iifname . meta mark @nv accept
 	}
 }
-- 
2.26.2

[PATCH nft 2/3] evaluate: pick data element byte order, not dtype one

[Florian Westphal]

Some expressions have integer base type, not a specific one, e.g. 'ct zone'.
In that case nft used the wrong byte order.

Without this, nft adds
elements = { "eth0" : 256, "eth1" : 512, "veth4" : 256 }
instead of 1, 2, 3.

This is not a 'display bug', the added elements have wrong byte order.

Signed-off-by: Florian Westphal <fw@strlen.de>
---
 src/evaluate.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/evaluate.c b/src/evaluate.c
index 1d5db4dacd82..123fc7ab1a28 100644
--- a/src/evaluate.c
+++ b/src/evaluate.c
@@ -1596,7 +1596,7 @@ static int expr_evaluate_mapping(struct eval_ctx *ctx, struct expr **expr)
 		else
 			datalen = set->data->len;
 
-		expr_set_context(&ctx->ectx, set->data->dtype, datalen);
+		__expr_set_context(&ctx->ectx, set->data->dtype, set->data->byteorder, datalen, 0);
 	} else {
 		assert((set->flags & NFT_SET_MAP) == 0);
 	}
-- 
2.26.2

[PATCH nft 3/3] evaluate: set evaluation context for set elements

[Florian Westphal]

This resolves same issue as previous patch when such
expression is used as a set key:

        set z {
                typeof ct zone
-               elements = { 1, 512, 768, 1024, 1280, 1536 }
+               elements = { 1, 2, 3, 4, 5, 6 }
        }

Signed-off-by: Florian Westphal <fw@strlen.de>
---
 src/evaluate.c | 11 +++++++++--
 1 file changed, 9 insertions(+), 2 deletions(-)

diff --git a/src/evaluate.c b/src/evaluate.c
index 123fc7ab1a28..0b251ab5554c 100644
--- a/src/evaluate.c
+++ b/src/evaluate.c
@@ -1391,8 +1391,15 @@ static int expr_evaluate_set_elem(struct eval_ctx *ctx, struct expr **expr)
 {
 	struct expr *elem = *expr;
 
-	if (ctx->set && __expr_evaluate_set_elem(ctx, elem) < 0)
-		return -1;
+	if (ctx->set) {
+		const struct expr *key;
+
+		if (__expr_evaluate_set_elem(ctx, elem) < 0)
+			return -1;
+
+		key = ctx->set->key;
+		__expr_set_context(&ctx->ectx, key->dtype, key->byteorder, key->len, 0);
+	}
 
 	if (expr_evaluate(ctx, &elem->key) < 0)
 		return -1;
-- 
2.26.2

Re: [PATCH nft 0/3] nft: fix ct zone handling in sets and maps

[Pablo Neira Ayuso]

On Wed, Feb 03, 2021 at 05:57:03PM +0100, Florian Westphal wrote:
> 'ct zone' (and other expressions w. host byte order and integer dtype)
> are not handled correctly on little endian platforms.
> 
> First patch adds a test case that demonstrates the problem,
> patch 2 and 3 resolve this for the mapping and set key cases.

Series LGTM, thanks Florian.