[PATCH nft 1/3] netlink: move binop postprocess to extra function

[PATCH nft 1/3] netlink: move binop postprocess to extra function

[Florian Westphal]

Just move the payload trim part to a separate function.
Next patch will add a second call site to deal with map ops
that use a lookup based on a binop result.

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

diff --git a/src/netlink_delinearize.c b/src/netlink_delinearize.c
index 3f01781..6876f02 100644
--- a/src/netlink_delinearize.c
+++ b/src/netlink_delinearize.c
@@ -1170,6 +1170,41 @@ static struct expr *binop_tree_to_list(struct expr *list, struct expr *expr)
 	return list;
 }
 
+static void binop_postprocess(struct rule_pp_ctx *ctx, struct expr *expr)
+{
+	struct expr *binop = expr->left, *value = expr->right;
+
+	struct expr *payload = binop->left;
+	struct expr *mask = binop->right;
+	unsigned int shift;
+
+	if (payload_expr_trim(payload, mask, &ctx->pctx, &shift)) {
+		/* mask is implicit, binop needs to be removed.
+		 *
+		 * Fix all values of the expression according to the mask
+		 * and then process the payload instruction using the real
+		 * sizes and offsets we're interested in.
+		 *
+		 * Finally, convert the expression to 1) by replacing
+		 * the binop with the binop payload expr.
+		 */
+		if (value->ops->type == EXPR_VALUE) {
+			assert(value->len >= expr->left->right->len);
+			mpz_rshift_ui(value->value, shift);
+			value->len = payload->len;
+		}
+
+		payload_match_postprocess(ctx, expr, payload);
+
+		assert(expr->left->ops->type == EXPR_BINOP);
+
+		assert(binop->left == payload);
+		expr->left = expr_get(payload);
+		expr_free(binop);
+	}
+}
+
+
 static void relational_binop_postprocess(struct rule_pp_ctx *ctx, struct expr *expr)
 {
 	struct expr *binop = expr->left, *value = expr->right;
@@ -1202,10 +1237,6 @@ static void relational_binop_postprocess(struct rule_pp_ctx *ctx, struct expr *e
 	} else if (binop->op == OP_AND &&
 		   binop->left->ops->type == EXPR_PAYLOAD &&
 		   binop->right->ops->type == EXPR_VALUE) {
-		struct expr *payload = binop->left;
-		struct expr *mask = binop->right;
-		unsigned int shift;
-
 		/*
 		 * This *might* be a payload match testing header fields that
 		 * have non byte divisible offsets and/or bit lengths.
@@ -1229,30 +1260,7 @@ static void relational_binop_postprocess(struct rule_pp_ctx *ctx, struct expr *e
 		 * payload_expr_trim will figure out if the mask is needed to match
 		 * templates.
 		 */
-		if (payload_expr_trim(payload, mask, &ctx->pctx, &shift)) {
-			/* mask is implicit, binop needs to be removed.
-			 *
-			 * Fix all values of the expression according to the mask
-			 * and then process the payload instruction using the real
-			 * sizes and offsets we're interested in.
-			 *
-			 * Finally, convert the expression to 1) by replacing
-			 * the binop with the binop payload expr.
-			 */
-			if (value->ops->type == EXPR_VALUE) {
-				assert(value->len >= expr->left->right->len);
-				mpz_rshift_ui(value->value, shift);
-				value->len = payload->len;
-			}
-
-			payload_match_postprocess(ctx, expr, payload);
-
-			assert(expr->left->ops->type == EXPR_BINOP);
-
-			assert(binop->left == payload);
-			expr->left = expr_get(payload);
-			expr_free(binop);
-		}
+		binop_postprocess(ctx, expr);
 	}
 }
 
-- 
2.4.10

[PATCH nft 2/3] tests: add two map test cases

[Florian Westphal]

One normal map lookup, one with an explicit binop.
The latter is supposed to also work with the followup patch applied.

Signed-off-by: Florian Westphal <fw@strlen.de>
---
 tests/py/inet/map.t                |  8 ++++++++
 tests/py/inet/map.t.payload.inet   | 11 +++++++++++
 tests/py/inet/map.t.payload.ip     |  9 +++++++++
 tests/py/inet/map.t.payload.netdev | 11 +++++++++++
 tests/py/ip6/map.t                 |  5 +++++
 tests/py/ip6/map.t.payload         | 10 ++++++++++
 6 files changed, 54 insertions(+)
 create mode 100644 tests/py/inet/map.t
 create mode 100644 tests/py/inet/map.t.payload.inet
 create mode 100644 tests/py/inet/map.t.payload.ip
 create mode 100644 tests/py/inet/map.t.payload.netdev
 create mode 100644 tests/py/ip6/map.t
 create mode 100644 tests/py/ip6/map.t.payload

diff --git a/tests/py/inet/map.t b/tests/py/inet/map.t
new file mode 100644
index 0000000..f48afcd
--- /dev/null
+++ b/tests/py/inet/map.t
@@ -0,0 +1,8 @@
+:input;type filter hook input priority 0
+:ingress;type filter hook ingress device lo priority 0
+
+*ip;test-ip4;input
+*inet;test-inet;input
+*netdev;test-netdev;ingress
+
+mark set ip saddr map { 10.2.3.2 : 0x0000002a, 10.2.3.1 : 0x00000017};ok;mark set ip saddr map { 10.2.3.1 : 0x00000017, 10.2.3.2 : 0x0000002a}
diff --git a/tests/py/inet/map.t.payload.inet b/tests/py/inet/map.t.payload.inet
new file mode 100644
index 0000000..73e68b6
--- /dev/null
+++ b/tests/py/inet/map.t.payload.inet
@@ -0,0 +1,11 @@
+# mark set ip saddr map { 10.2.3.2 : 0x0000002a, 10.2.3.1 : 0x00000017}
+map%d test-inet b
+map%d test-inet 0
+	element 0203020a  : 0000002a 0 [end]	element 0103020a  : 00000017 0 [end]
+inet test-inet input
+  [ meta load nfproto => reg 1 ]
+  [ cmp eq reg 1 0x00000002 ]
+  [ payload load 4b @ network header + 12 => reg 1 ]
+  [ lookup reg 1 set map%d dreg 1 ]
+  [ meta set mark with reg 1 ]
+
diff --git a/tests/py/inet/map.t.payload.ip b/tests/py/inet/map.t.payload.ip
new file mode 100644
index 0000000..54b9583
--- /dev/null
+++ b/tests/py/inet/map.t.payload.ip
@@ -0,0 +1,9 @@
+# mark set ip saddr map { 10.2.3.2 : 0x0000002a, 10.2.3.1 : 0x00000017}
+map%d test-ip b
+map%d test-ip 0
+	element 0103020a  : 00000017 0 [end]	element 0203020a  : 0000002a 0 [end]
+ip test-ip input
+  [ payload load 4b @ network header + 12 => reg 1 ]
+  [ lookup reg 1 set map%d dreg 1 ]
+  [ meta set mark with reg 1 ]
+
diff --git a/tests/py/inet/map.t.payload.netdev b/tests/py/inet/map.t.payload.netdev
new file mode 100644
index 0000000..27a3ca8
--- /dev/null
+++ b/tests/py/inet/map.t.payload.netdev
@@ -0,0 +1,11 @@
+# mark set ip saddr map { 10.2.3.2 : 0x0000002a, 10.2.3.1 : 0x00000017}
+map%d test-netdev b
+map%d test-netdev 0
+	element 0103020a  : 00000017 0 [end]	element 0203020a  : 0000002a 0 [end]
+netdev test-netdev ingress
+  [ meta load protocol => reg 1 ]
+  [ cmp eq reg 1 0x00000008 ]
+  [ payload load 4b @ network header + 12 => reg 1 ]
+  [ lookup reg 1 set map%d dreg 1 ]
+  [ meta set mark with reg 1 ]
+
diff --git a/tests/py/ip6/map.t b/tests/py/ip6/map.t
new file mode 100644
index 0000000..3377f8d
--- /dev/null
+++ b/tests/py/ip6/map.t
@@ -0,0 +1,5 @@
+:input;type filter hook input priority 0
+*ip6;test-ip6;input
+
+mark set ip6 saddr and ::ffff map { ::2 : 0x0000002a, ::ffff : 0x00000017};ok;mark set ip6 saddr & ::ffff map { ::2 : 0x0000002a, ::ffff : 0x00000017}
+
diff --git a/tests/py/ip6/map.t.payload b/tests/py/ip6/map.t.payload
new file mode 100644
index 0000000..db7df27
--- /dev/null
+++ b/tests/py/ip6/map.t.payload
@@ -0,0 +1,10 @@
+# mark set ip6 saddr and ::ffff map { ::2 : 0x0000002a, ::ffff : 0x00000017}
+map%d test-ip6 b
+map%d test-ip6 0
+	element 00000000 00000000 00000000 02000000  : 0000002a 0 [end]	element 00000000 00000000 00000000 ffff0000  : 00000017 0 [end]
+ip6 test-ip6 input
+  [ payload load 16b @ network header + 8 => reg 1 ]
+  [ bitwise reg 1 = (reg=1 & 0x00000000 0x00000000 0x00000000 0xffff0000 ) ^ 0x00000000 0x00000000 0x00000000 0x00000000 ]
+  [ lookup reg 1 set map%d dreg 1 ]
+  [ meta set mark with reg 1 ]
+
-- 
2.4.10

[PATCH nft 3/3] netlink: do binop postprocessing also for map lookups

[Florian Westphal]

nft list before patch:
mark set unknown unknown & 0xfff [invalid type] map { 3 : 0x00000017, 1 : 0x0000002a}
now:
mark set vlan id map { 3 : 0x00000017, 1 : 0x0000002a}

Signed-off-by: Florian Westphal <fw@strlen.de>
---
 src/netlink_delinearize.c          | 19 +++++++++++++++++++
 tests/py/inet/map.t                |  1 +
 tests/py/inet/map.t.payload.inet   | 12 ++++++++++++
 tests/py/inet/map.t.payload.ip     | 10 ++++++++++
 tests/py/inet/map.t.payload.netdev | 12 ++++++++++++
 5 files changed, 54 insertions(+)

diff --git a/src/netlink_delinearize.c b/src/netlink_delinearize.c
index 6876f02..76d598c 100644
--- a/src/netlink_delinearize.c
+++ b/src/netlink_delinearize.c
@@ -1204,6 +1204,17 @@ static void binop_postprocess(struct rule_pp_ctx *ctx, struct expr *expr)
 	}
 }
 
+static void map_binop_postprocess(struct rule_pp_ctx *ctx, struct expr *expr)
+{
+	struct expr *binop = expr->left;
+
+	if (binop->op != OP_AND)
+		return;
+
+	if (binop->left->ops->type == EXPR_PAYLOAD &&
+	    binop->right->ops->type == EXPR_VALUE)
+		binop_postprocess(ctx, expr);
+}
 
 static void relational_binop_postprocess(struct rule_pp_ctx *ctx, struct expr *expr)
 {
@@ -1357,6 +1368,14 @@ static void expr_postprocess(struct rule_pp_ctx *ctx, struct expr **exprp)
 
 	switch (expr->ops->type) {
 	case EXPR_MAP:
+		switch (expr->map->ops->type) {
+		case EXPR_BINOP:
+			map_binop_postprocess(ctx, expr);
+			break;
+		default:
+			break;
+		}
+
 		expr_postprocess(ctx, &expr->map);
 		expr_postprocess(ctx, &expr->mappings);
 		break;
diff --git a/tests/py/inet/map.t b/tests/py/inet/map.t
index f48afcd..5075540 100644
--- a/tests/py/inet/map.t
+++ b/tests/py/inet/map.t
@@ -6,3 +6,4 @@
 *netdev;test-netdev;ingress
 
 mark set ip saddr map { 10.2.3.2 : 0x0000002a, 10.2.3.1 : 0x00000017};ok;mark set ip saddr map { 10.2.3.1 : 0x00000017, 10.2.3.2 : 0x0000002a}
+mark set ip hdrlength map { 5 : 0x00000017, 4 : 0x00000001};ok;mark set ip hdrlength map { 4 : 0x00000001, 5 : 0x00000017}
diff --git a/tests/py/inet/map.t.payload.inet b/tests/py/inet/map.t.payload.inet
index 73e68b6..a0ff003 100644
--- a/tests/py/inet/map.t.payload.inet
+++ b/tests/py/inet/map.t.payload.inet
@@ -9,3 +9,15 @@ inet test-inet input
   [ lookup reg 1 set map%d dreg 1 ]
   [ meta set mark with reg 1 ]
 
+# mark set ip hdrlength map { 5 : 0x00000017, 4 : 0x00000001}
+map%d test-inet b
+map%d test-inet 0
+	element 00000005  : 00000017 0 [end]	element 00000004  : 00000001 0 [end]
+inet test-inet input
+  [ meta load nfproto => reg 1 ]
+  [ cmp eq reg 1 0x00000002 ]
+  [ payload load 1b @ network header + 0 => reg 1 ]
+  [ bitwise reg 1 = (reg=1 & 0x0000000f ) ^ 0x00000000 ]
+  [ lookup reg 1 set map%d dreg 1 ]
+  [ meta set mark with reg 1 ]
+
diff --git a/tests/py/inet/map.t.payload.ip b/tests/py/inet/map.t.payload.ip
index 54b9583..465a55a 100644
--- a/tests/py/inet/map.t.payload.ip
+++ b/tests/py/inet/map.t.payload.ip
@@ -7,3 +7,13 @@ ip test-ip input
   [ lookup reg 1 set map%d dreg 1 ]
   [ meta set mark with reg 1 ]
 
+# mark set ip hdrlength map { 5 : 0x00000017, 4 : 0x00000001}
+map%d test-ip4 b
+map%d test-ip4 0
+	element 00000004  : 00000001 0 [end]	element 00000005  : 00000017 0 [end]
+ip test-ip4 input
+  [ payload load 1b @ network header + 0 => reg 1 ]
+  [ bitwise reg 1 = (reg=1 & 0x0000000f ) ^ 0x00000000 ]
+  [ lookup reg 1 set map%d dreg 1 ]
+  [ meta set mark with reg 1 ]
+
diff --git a/tests/py/inet/map.t.payload.netdev b/tests/py/inet/map.t.payload.netdev
index 27a3ca8..fb9260c 100644
--- a/tests/py/inet/map.t.payload.netdev
+++ b/tests/py/inet/map.t.payload.netdev
@@ -9,3 +9,15 @@ netdev test-netdev ingress
   [ lookup reg 1 set map%d dreg 1 ]
   [ meta set mark with reg 1 ]
 
+# mark set ip hdrlength map { 5 : 0x00000017, 4 : 0x00000001}
+map%d test-netdev b
+map%d test-netdev 0
+	element 00000005  : 00000017 0 [end]	element 00000004  : 00000001 0 [end]
+netdev test-netdev ingress
+  [ meta load protocol => reg 1 ]
+  [ cmp eq reg 1 0x00000008 ]
+  [ payload load 1b @ network header + 0 => reg 1 ]
+  [ bitwise reg 1 = (reg=1 & 0x0000000f ) ^ 0x00000000 ]
+  [ lookup reg 1 set map%d dreg 1 ]
+  [ meta set mark with reg 1 ]
+
-- 
2.4.10

Re: [PATCH nft 1/3] netlink: move binop postprocess to extra function

[Pablo Neira Ayuso]

On Sun, Jan 24, 2016 at 06:58:25PM +0100, Florian Westphal wrote:
> Just move the payload trim part to a separate function.
> Next patch will add a second call site to deal with map ops
> that use a lookup based on a binop result.

Acked-by: Pablo Neira Ayuso <pablo@netfilter.org>

Re: [PATCH nft 2/3] tests: add two map test cases

[Pablo Neira Ayuso]

On Sun, Jan 24, 2016 at 06:58:26PM +0100, Florian Westphal wrote:
> One normal map lookup, one with an explicit binop.
> The latter is supposed to also work with the followup patch applied.

Acked-by: Pablo Neira Ayuso <pablo@netfilter.org>

Re: [PATCH nft 3/3] netlink: do binop postprocessing also for map lookups

[Pablo Neira Ayuso]

On Sun, Jan 24, 2016 at 06:58:27PM +0100, Florian Westphal wrote:
> nft list before patch:
> mark set unknown unknown & 0xfff [invalid type] map { 3 : 0x00000017, 1 : 0x0000002a}
> now:
> mark set vlan id map { 3 : 0x00000017, 1 : 0x0000002a}

Acked-by: Pablo Neira Ayuso <pablo@netfilter.org>

Re: [PATCH nft 3/3] netlink: do binop postprocessing also for map lookups

[Florian Westphal]

Pablo Neira Ayuso <pablo@netfilter.org> wrote:
> On Sun, Jan 24, 2016 at 06:58:27PM +0100, Florian Westphal wrote:
> > nft list before patch:
> > mark set unknown unknown & 0xfff [invalid type] map { 3 : 0x00000017, 1 : 0x0000002a}
> > now:
> > mark set vlan id map { 3 : 0x00000017, 1 : 0x0000002a}
> 
> Acked-by: Pablo Neira Ayuso <pablo@netfilter.org>

Thanks, I pushed the series to nft master.