[PATCH nft 0/4] second batch of typeof fixes

[PATCH nft 0/4] second batch of typeof fixes

[Florian Westphal]

This series makes typeof-sets work in corner cases such as

set s4 { typeof frag frag-off }
set s8 { typeof ip version }

frag frag-off @s4 accept
ip version @s8

Due to the shift/mask expressions needed to cope with these
delinearization can't figure out the correct payload/exthdr templates
and nft lists this as:

(frag unknown & 0xfff8 [invalid type]) >> 3 == @s4
(ip l4proto & pfsync) >> 4 == @s8

With this series, the mask/shift expressions are removed and
nft can print them in a readable way.

Florian Westphal (4):
  tests: add shift+and typeof test cases
  payload: skip templates with meta key set
  netlink_delinearize: and/shift postprocessing
  netlink_delinearize: zero shift removal

 src/netlink_delinearize.c                  | 28 +++++++++++++++++++
 src/payload.c                              |  3 ++
 .../testcases/sets/dumps/typeof_sets_0.nft | 23 +++++++++++++++
 tests/shell/testcases/sets/typeof_sets_0   | 22 +++++++++++++++
 4 files changed, 76 insertions(+)
-- 
2.32.0

[PATCH nft 1/4] tests: add shift+and typeof test cases

[Florian Westphal]

These tests work, but I omitted a few lines that do not:

in: frag frag-off @s4 accept
in: ip version @s8

out: (frag unknown & 0xfff8 [invalid type]) >> 3 == @s4
out:  (ip l4proto & pfsync) >> 4 == @s8

Next patches resolve this.

Signed-off-by: Florian Westphal <fw@strlen.de>
---
 .../shell/testcases/sets/dumps/typeof_sets_0.nft  | 15 +++++++++++++++
 tests/shell/testcases/sets/typeof_sets_0          | 14 ++++++++++++++
 2 files changed, 29 insertions(+)

diff --git a/tests/shell/testcases/sets/dumps/typeof_sets_0.nft b/tests/shell/testcases/sets/dumps/typeof_sets_0.nft
index 8f11b110552c..ad442713f6dc 100644
--- a/tests/shell/testcases/sets/dumps/typeof_sets_0.nft
+++ b/tests/shell/testcases/sets/dumps/typeof_sets_0.nft
@@ -34,6 +34,17 @@ table inet t {
 		elements = { 1, 4 }
 	}
 
+	set s8 {
+		typeof ip version
+		elements = { 4, 6 }
+	}
+
+	set s9 {
+		typeof ip hdrlength
+		elements = { 0, 1, 2, 3, 4,
+			     15 }
+	}
+
 	chain c1 {
 		osf name @s1 accept
 	}
@@ -53,4 +64,8 @@ table inet t {
 	chain c7 {
 		sctp chunk init num-inbound-streams @s7 accept
 	}
+
+	chain c9 {
+		ip hdrlength @s9 accept
+	}
 }
diff --git a/tests/shell/testcases/sets/typeof_sets_0 b/tests/shell/testcases/sets/typeof_sets_0
index 1e99e2987733..2102789e1043 100755
--- a/tests/shell/testcases/sets/typeof_sets_0
+++ b/tests/shell/testcases/sets/typeof_sets_0
@@ -40,6 +40,16 @@ EXPECTED="table inet t {
 		elements = { 1, 4 }
 	}
 
+	set s8 {
+		typeof ip version
+		elements = { 4, 6 }
+	}
+
+	set s9 {
+		typeof ip hdrlength
+		elements = { 0, 1, 2, 3, 4, 15 }
+	}
+
 	chain c1 {
 		osf name @s1 accept
 	}
@@ -59,6 +69,10 @@ EXPECTED="table inet t {
 	chain c7 {
 		sctp chunk init num-inbound-streams @s7 accept
 	}
+
+	chain c9 {
+		ip hdrlength @s9 accept
+	}
 }"
 
 set -e
-- 
2.32.0

[PATCH nft 2/4] payload: skip templates with meta key set

[Florian Westphal]

meta templates are only there for ease of use (input/parsing).

When listing, they should be ignored:
 set s4 { typeof ip version elements = { 1, } }
 chain c4 { ip version @s4 accept }

gets listed as 'ip l4proto ...' which is nonsensical.

 after this patch we get:
in: ip version @s4
out: (@nh,0,8 & 0xf0) >> 4 == @s4

.. which is (marginally) better.

Next patch adds support for payload decoding.

Signed-off-by: Florian Westphal <fw@strlen.de>
---
 src/payload.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/src/payload.c b/src/payload.c
index d9e0d4254f19..79008762825f 100644
--- a/src/payload.c
+++ b/src/payload.c
@@ -811,6 +811,9 @@ void payload_expr_complete(struct expr *expr, const struct proto_ctx *ctx)
 		    tmpl->len    != expr->len)
 			continue;
 
+		if (tmpl->meta_key && i == 0)
+			continue;
+
 		if (tmpl->icmp_dep && ctx->th_dep.icmp.type &&
 		    ctx->th_dep.icmp.type != icmp_dep_to_type(tmpl->icmp_dep))
 			continue;
-- 
2.32.0

[PATCH nft 3/4] netlink_delinearize: and/shift postprocessing

[Florian Westphal]

Before this patch:
in:  frag frag-off @s4
in:  ip version @s8

out: (@nh,0,8 & 0xf0) >> 4 == @s8
out: (frag unknown & 0xfff8 [invalid type]) >> 3 == @s4

after:
out: frag frag-off >> 0 == @s4
out: ip version >> 0 == @s8

Next patch adds support for zero-shift removal.

Signed-off-by: Florian Westphal <fw@strlen.de>
---
 src/netlink_delinearize.c | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/src/netlink_delinearize.c b/src/netlink_delinearize.c
index 1f820e68e9f1..e37a34f37ba2 100644
--- a/src/netlink_delinearize.c
+++ b/src/netlink_delinearize.c
@@ -2414,6 +2414,13 @@ static void relational_binop_postprocess(struct rule_pp_ctx *ctx,
 		 * templates.
 		 */
 		binop_postprocess(ctx, expr, &expr->left);
+	} else if (binop->op == OP_RSHIFT && binop->left->op == OP_AND &&
+		   binop->right->etype == EXPR_VALUE && binop->left->right->etype == EXPR_VALUE) {
+		/* Handle 'ip version @s4' and similar, i.e. set lookups where the lhs needs
+		 * fixups to mask out unwanted bits AND a shift.
+		 */
+
+		binop_postprocess(ctx, binop, &binop->left);
 	}
 }
 
-- 
2.32.0

[PATCH nft 4/4] netlink_delinearize: zero shift removal

[Florian Westphal]

Remove shifts-by-0.  These can occur after binop postprocessing
has adjusted the RHS value to account for a mask operation.

Example: frag frag-off @s4

Is internally represented via:

  [ exthdr load ipv6 2b @ 44 + 2 => reg 1 ]
  [ bitwise reg 1 = ( reg 1 & 0x0000f8ff ) ^ 0x00000000 ]
  [ bitwise reg 1 = ( reg 1 >> 0x00000003 ) ]
  [ lookup reg 1 set s ]

First binop masks out unwanted parts of the 16-bit field.
Second binop needs to left-shift so that lookups in the set will work.

When decoding, the first binop is removed after the exthdr load
has been adjusted accordingly.  Constant propagation adjusts the
shift-value to 0 on removal.  This change then gets rid of the
shift-by-0 entirely.

After this change, 'frag frag-off @s4' input is shown as-is.

Signed-off-by: Florian Westphal <fw@strlen.de>
---
 src/netlink_delinearize.c                     | 21 +++++++++++++++++++
 .../testcases/sets/dumps/typeof_sets_0.nft    |  8 +++++++
 tests/shell/testcases/sets/typeof_sets_0      |  8 +++++++
 3 files changed, 37 insertions(+)

diff --git a/src/netlink_delinearize.c b/src/netlink_delinearize.c
index e37a34f37ba2..6a003cf7051b 100644
--- a/src/netlink_delinearize.c
+++ b/src/netlink_delinearize.c
@@ -2322,6 +2322,20 @@ static void map_binop_postprocess(struct rule_pp_ctx *ctx, struct expr *expr)
 		binop_postprocess(ctx, expr, &expr->map);
 }
 
+static bool is_shift_by_zero(const struct expr *binop)
+{
+	struct expr *rhs;
+
+	if (binop->op != OP_RSHIFT && binop->op != OP_LSHIFT)
+		return false;
+
+	rhs = binop->right;
+	if (rhs->etype != EXPR_VALUE || rhs->len > 64)
+		return false;
+
+	return mpz_get_uint64(binop->right->value) == 0;
+}
+
 static void relational_binop_postprocess(struct rule_pp_ctx *ctx,
 					 struct expr **exprp)
 {
@@ -2421,6 +2435,13 @@ static void relational_binop_postprocess(struct rule_pp_ctx *ctx,
 		 */
 
 		binop_postprocess(ctx, binop, &binop->left);
+		if (is_shift_by_zero(binop)) {
+			struct expr *lhs = binop->left;
+
+			expr_get(lhs);
+			expr_free(binop);
+			expr->left = lhs;
+		}
 	}
 }
 
diff --git a/tests/shell/testcases/sets/dumps/typeof_sets_0.nft b/tests/shell/testcases/sets/dumps/typeof_sets_0.nft
index ad442713f6dc..e397a6345462 100644
--- a/tests/shell/testcases/sets/dumps/typeof_sets_0.nft
+++ b/tests/shell/testcases/sets/dumps/typeof_sets_0.nft
@@ -53,6 +53,10 @@ table inet t {
 		vlan id @s2 accept
 	}
 
+	chain c4 {
+		frag frag-off @s4 accept
+	}
+
 	chain c5 {
 		ip option ra value @s5 accept
 	}
@@ -65,6 +69,10 @@ table inet t {
 		sctp chunk init num-inbound-streams @s7 accept
 	}
 
+	chain c8 {
+		ip version @s8 accept
+	}
+
 	chain c9 {
 		ip hdrlength @s9 accept
 	}
diff --git a/tests/shell/testcases/sets/typeof_sets_0 b/tests/shell/testcases/sets/typeof_sets_0
index 2102789e1043..be906cdcc842 100755
--- a/tests/shell/testcases/sets/typeof_sets_0
+++ b/tests/shell/testcases/sets/typeof_sets_0
@@ -58,6 +58,10 @@ EXPECTED="table inet t {
 		ether type vlan vlan id @s2 accept
 	}
 
+	chain c4 {
+		frag frag-off @s4 accept
+	}
+
 	chain c5 {
 		ip option ra value @s5 accept
 	}
@@ -70,6 +74,10 @@ EXPECTED="table inet t {
 		sctp chunk init num-inbound-streams @s7 accept
 	}
 
+	chain c8 {
+		ip version @s8 accept
+	}
+
 	chain c9 {
 		ip hdrlength @s9 accept
 	}
-- 
2.32.0

Re: [PATCH nft 0/4] second batch of typeof fixes

[Pablo Neira Ayuso]

On Tue, Dec 07, 2021 at 04:16:55PM +0100, Florian Westphal wrote:
> This series makes typeof-sets work in corner cases such as
> 
> set s4 { typeof frag frag-off }
> set s8 { typeof ip version }
> 
> frag frag-off @s4 accept
> ip version @s8
> 
> Due to the shift/mask expressions needed to cope with these
> delinearization can't figure out the correct payload/exthdr templates
> and nft lists this as:
> 
> (frag unknown & 0xfff8 [invalid type]) >> 3 == @s4
> (ip l4proto & pfsync) >> 4 == @s8
> 
> With this series, the mask/shift expressions are removed and
> nft can print them in a readable way.

LGTM, thanks