[PATCH nft v4 0/6] Remaining bitwise-shift-related changes

[PATCH nft v4 0/6] Remaining bitwise-shift-related changes

[Jeremy Sowden]

While most of the v3 bitwise-shift patches were applied, a couple of the
tidy-up ones and the new tests in the bitwise-shift series needed a bit
more work.  This version introduces some new changes: a patch renaming a
variable and a couple adding some Python tests.

Changes since v3:

  * the second, fifth and sixth patches are new;
  * the commit message of the third patch has been expanded;
  * one of the shell tests has been amended to include a parenthesized
    statement expression.

Jeremy Sowden (6):
  parser: add parenthesized statement expressions.
  evaluate: correct variable name.
  evaluate: change shift byte-order to host-endian.
  tests: shell: add bit-shift tests.
  tests: py: add missing JSON output.
  tests: py: add bit-shift tests.

 src/evaluate.c                                | 14 ++---
 src/parser_bison.y                            | 25 ++++-----
 tests/py/any/ct.t                             |  1 +
 tests/py/any/ct.t.json                        | 51 +++++++++++++++++++
 tests/py/any/ct.t.payload                     | 22 ++++++++
 tests/py/inet/meta.t                          |  1 +
 tests/py/inet/meta.t.json                     | 22 ++++++++
 tests/py/inet/meta.t.payload                  |  6 +++
 tests/py/ip/meta.t.json                       | 35 +++++++++++++
 tests/py/ip6/meta.t.json                      | 35 +++++++++++++
 tests/shell/testcases/chains/0040mark_shift_0 | 11 ++++
 tests/shell/testcases/chains/0040mark_shift_1 | 11 ++++
 .../chains/dumps/0040mark_shift_0.nft         |  6 +++
 .../chains/dumps/0040mark_shift_1.nft         |  6 +++
 14 files changed, 227 insertions(+), 19 deletions(-)
 create mode 100755 tests/shell/testcases/chains/0040mark_shift_0
 create mode 100755 tests/shell/testcases/chains/0040mark_shift_1
 create mode 100644 tests/shell/testcases/chains/dumps/0040mark_shift_0.nft
 create mode 100644 tests/shell/testcases/chains/dumps/0040mark_shift_1.nft

-- 
2.24.1

[PATCH nft v4 1/6] parser: add parenthesized statement expressions.

[Jeremy Sowden]

Primary and primary RHS expressions support parenthesized basic and
basic RHS expressions.  However, primary statement expressions do not
support parenthesized basic statement expressions.  Add them.

Signed-off-by: Jeremy Sowden <jeremy@azazel.net>
---
 src/parser_bison.y | 25 +++++++++++++------------
 1 file changed, 13 insertions(+), 12 deletions(-)

diff --git a/src/parser_bison.y b/src/parser_bison.y
index 799f7a308b07..45cc013cfe28 100644
--- a/src/parser_bison.y
+++ b/src/parser_bison.y
@@ -2992,18 +2992,19 @@ synproxy_sack		:	/* empty */	{ $$ = 0; }
 			}
 			;
 
-primary_stmt_expr	:	symbol_expr		{ $$ = $1; }
-			|	integer_expr		{ $$ = $1; }
-			|	boolean_expr		{ $$ = $1; }
-			|	meta_expr		{ $$ = $1; }
-			|	rt_expr			{ $$ = $1; }
-			|	ct_expr			{ $$ = $1; }
-			|	numgen_expr             { $$ = $1; }
-			|	hash_expr               { $$ = $1; }
-			|	payload_expr		{ $$ = $1; }
-			|	keyword_expr		{ $$ = $1; }
-			|	socket_expr		{ $$ = $1; }
-			|	osf_expr		{ $$ = $1; }
+primary_stmt_expr	:	symbol_expr			{ $$ = $1; }
+			|	integer_expr			{ $$ = $1; }
+			|	boolean_expr			{ $$ = $1; }
+			|	meta_expr			{ $$ = $1; }
+			|	rt_expr				{ $$ = $1; }
+			|	ct_expr				{ $$ = $1; }
+			|	numgen_expr             	{ $$ = $1; }
+			|	hash_expr               	{ $$ = $1; }
+			|	payload_expr			{ $$ = $1; }
+			|	keyword_expr			{ $$ = $1; }
+			|	socket_expr			{ $$ = $1; }
+			|	osf_expr			{ $$ = $1; }
+			|	'('	basic_stmt_expr	')'	{ $$ = $2; }
 			;
 
 shift_stmt_expr		:	primary_stmt_expr
-- 
2.24.1

[PATCH nft v4 2/6] evaluate: correct variable name.

[Jeremy Sowden]

Rename the `lshift` variable used to store an right-shift expression to
`rshift`.

Signed-off-by: Jeremy Sowden <jeremy@azazel.net>
---
 src/evaluate.c | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/src/evaluate.c b/src/evaluate.c
index 09dd493f0757..966582e44a7d 100644
--- a/src/evaluate.c
+++ b/src/evaluate.c
@@ -450,7 +450,7 @@ static uint8_t expr_offset_shift(const struct expr *expr, unsigned int offset,
 
 static void expr_evaluate_bits(struct eval_ctx *ctx, struct expr **exprp)
 {
-	struct expr *expr = *exprp, *and, *mask, *lshift, *off;
+	struct expr *expr = *exprp, *and, *mask, *rshift, *off;
 	unsigned masklen, len = expr->len, extra_len = 0;
 	uint8_t shift;
 	mpz_t bitmask;
@@ -490,12 +490,12 @@ static void expr_evaluate_bits(struct eval_ctx *ctx, struct expr **exprp)
 					  BYTEORDER_BIG_ENDIAN,
 					  sizeof(shift), &shift);
 
-		lshift = binop_expr_alloc(&expr->location, OP_RSHIFT, and, off);
-		lshift->dtype		= expr->dtype;
-		lshift->byteorder	= expr->byteorder;
-		lshift->len		= masklen;
+		rshift = binop_expr_alloc(&expr->location, OP_RSHIFT, and, off);
+		rshift->dtype		= expr->dtype;
+		rshift->byteorder	= expr->byteorder;
+		rshift->len		= masklen;
 
-		*exprp = lshift;
+		*exprp = rshift;
 	} else
 		*exprp = and;
 
-- 
2.24.1

[PATCH nft v4 3/6] evaluate: change shift byte-order to host-endian.

[Jeremy Sowden]

The byte-order of the righthand operands of the right-shifts generated
for payload and exthdr expressions is big-endian.  However, all right
operands should be host-endian.  Since evaluation of the shift binop
will insert a byte-order conversion to enforce this, change the
endianness in order to avoid the extra operation.

Signed-off-by: Jeremy Sowden <jeremy@azazel.net>
---
 src/evaluate.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/evaluate.c b/src/evaluate.c
index 966582e44a7d..ef2dcb5ce78f 100644
--- a/src/evaluate.c
+++ b/src/evaluate.c
@@ -487,7 +487,7 @@ static void expr_evaluate_bits(struct eval_ctx *ctx, struct expr **exprp)
 	if (shift) {
 		off = constant_expr_alloc(&expr->location,
 					  expr_basetype(expr),
-					  BYTEORDER_BIG_ENDIAN,
+					  BYTEORDER_HOST_ENDIAN,
 					  sizeof(shift), &shift);
 
 		rshift = binop_expr_alloc(&expr->location, OP_RSHIFT, and, off);
-- 
2.24.1

[PATCH nft v4 4/6] tests: shell: add bit-shift tests.

[Jeremy Sowden]

Add a couple of shell test-cases for setting the CT mark to a bitwise
expression derived from the packet mark and vice versa.

Signed-off-by: Jeremy Sowden <jeremy@azazel.net>
---
 tests/shell/testcases/chains/0040mark_shift_0         | 11 +++++++++++
 tests/shell/testcases/chains/0040mark_shift_1         | 11 +++++++++++
 .../shell/testcases/chains/dumps/0040mark_shift_0.nft |  6 ++++++
 .../shell/testcases/chains/dumps/0040mark_shift_1.nft |  6 ++++++
 4 files changed, 34 insertions(+)
 create mode 100755 tests/shell/testcases/chains/0040mark_shift_0
 create mode 100755 tests/shell/testcases/chains/0040mark_shift_1
 create mode 100644 tests/shell/testcases/chains/dumps/0040mark_shift_0.nft
 create mode 100644 tests/shell/testcases/chains/dumps/0040mark_shift_1.nft

diff --git a/tests/shell/testcases/chains/0040mark_shift_0 b/tests/shell/testcases/chains/0040mark_shift_0
new file mode 100755
index 000000000000..55447f0b9737
--- /dev/null
+++ b/tests/shell/testcases/chains/0040mark_shift_0
@@ -0,0 +1,11 @@
+#!/bin/bash
+
+set -e
+
+RULESET="
+  add table t
+  add chain t c { type filter hook output priority mangle; }
+  add rule t c oif lo ct mark set (meta mark | 0x10) << 8
+"
+
+$NFT --debug=eval -f - <<< "$RULESET"
diff --git a/tests/shell/testcases/chains/0040mark_shift_1 b/tests/shell/testcases/chains/0040mark_shift_1
new file mode 100755
index 000000000000..b609f5ef10ad
--- /dev/null
+++ b/tests/shell/testcases/chains/0040mark_shift_1
@@ -0,0 +1,11 @@
+#!/bin/bash
+
+set -e
+
+RULESET="
+  add table t
+  add chain t c { type filter hook input priority mangle; }
+  add rule t c iif lo ct mark & 0xff 0x10 meta mark set ct mark >> 8
+"
+
+$NFT -f - <<< "$RULESET"
diff --git a/tests/shell/testcases/chains/dumps/0040mark_shift_0.nft b/tests/shell/testcases/chains/dumps/0040mark_shift_0.nft
new file mode 100644
index 000000000000..52d59d2c6da4
--- /dev/null
+++ b/tests/shell/testcases/chains/dumps/0040mark_shift_0.nft
@@ -0,0 +1,6 @@
+table ip t {
+	chain c {
+		type filter hook output priority mangle; policy accept;
+		oif "lo" ct mark set (meta mark | 0x00000010) << 8
+	}
+}
diff --git a/tests/shell/testcases/chains/dumps/0040mark_shift_1.nft b/tests/shell/testcases/chains/dumps/0040mark_shift_1.nft
new file mode 100644
index 000000000000..56ec8dc766ca
--- /dev/null
+++ b/tests/shell/testcases/chains/dumps/0040mark_shift_1.nft
@@ -0,0 +1,6 @@
+table ip t {
+	chain c {
+		type filter hook input priority mangle; policy accept;
+		iif "lo" ct mark & 0x000000ff == 0x00000010 meta mark set ct mark >> 8
+	}
+}
-- 
2.24.1

[PATCH nft v4 5/6] tests: py: add missing JSON output.

[Jeremy Sowden]

The JSON output was missing for some existing tests.

Signed-off-by: Jeremy Sowden <jeremy@azazel.net>
---
 tests/py/any/ct.t.json   | 23 +++++++++++++++++++++++
 tests/py/ip/meta.t.json  | 35 +++++++++++++++++++++++++++++++++++
 tests/py/ip6/meta.t.json | 35 +++++++++++++++++++++++++++++++++++
 3 files changed, 93 insertions(+)

diff --git a/tests/py/any/ct.t.json b/tests/py/any/ct.t.json
index 7c16f9df2195..8d56db2aaedb 100644
--- a/tests/py/any/ct.t.json
+++ b/tests/py/any/ct.t.json
@@ -499,6 +499,29 @@
     }
 ]
 
+# ct mark set ct mark or 0x00000001
+[
+    {
+        "mangle": {
+            "key": {
+                "ct": {
+                    "key": "mark"
+                }
+            },
+            "value": {
+                "|": [
+                    {
+                        "ct": {
+                            "key": "mark"
+                        }
+                    },
+                    1
+                ]
+            }
+        }
+    }
+]
+
 # ct mark 0x00000032
 [
     {
diff --git a/tests/py/ip/meta.t.json b/tests/py/ip/meta.t.json
index f873aa88598b..f83864f672d5 100644
--- a/tests/py/ip/meta.t.json
+++ b/tests/py/ip/meta.t.json
@@ -105,3 +105,38 @@
     }
 ]
 
+# meta sdif "lo" accept
+[
+    {
+        "match": {
+            "left": {
+                "meta": {
+                    "key": "sdif"
+                }
+            },
+            "op": "==",
+            "right": "lo"
+        }
+    },
+    {
+        "accept": null
+    }
+]
+
+# meta sdifname != "vrf1" accept
+[
+    {
+        "match": {
+            "left": {
+                "meta": {
+                    "key": "sdifname"
+                }
+            },
+            "op": "!=",
+            "right": "vrf1"
+        }
+    },
+    {
+        "accept": null
+    }
+]
diff --git a/tests/py/ip6/meta.t.json b/tests/py/ip6/meta.t.json
index 29cf9fd2d0cf..e72350f375e9 100644
--- a/tests/py/ip6/meta.t.json
+++ b/tests/py/ip6/meta.t.json
@@ -105,3 +105,38 @@
     }
 ]
 
+# meta sdif "lo" accept
+[
+    {
+        "match": {
+            "left": {
+                "meta": {
+                    "key": "sdif"
+                }
+            },
+            "op": "==",
+            "right": "lo"
+        }
+    },
+    {
+        "accept": null
+    }
+]
+
+# meta sdifname != "vrf1" accept
+[
+    {
+        "match": {
+            "left": {
+                "meta": {
+                    "key": "sdifname"
+                }
+            },
+            "op": "!=",
+            "right": "vrf1"
+        }
+    },
+    {
+        "accept": null
+    }
+]
-- 
2.24.1

[PATCH nft v4 6/6] tests: py: add bit-shift tests.

[Jeremy Sowden]

Add a couple of Python test-cases for setting the CT mark to a bitwise
expression derived from the packet mark and vice versa.

Signed-off-by: Jeremy Sowden <jeremy@azazel.net>
---
 tests/py/any/ct.t            |  1 +
 tests/py/any/ct.t.json       | 28 ++++++++++++++++++++++++++++
 tests/py/any/ct.t.payload    | 21 +++++++++++++++++++++
 tests/py/inet/meta.t         |  1 +
 tests/py/inet/meta.t.json    | 22 ++++++++++++++++++++++
 tests/py/inet/meta.t.payload |  6 ++++++
 6 files changed, 79 insertions(+)

diff --git a/tests/py/any/ct.t b/tests/py/any/ct.t
index ebc086445567..f65d275987cd 100644
--- a/tests/py/any/ct.t
+++ b/tests/py/any/ct.t
@@ -57,6 +57,7 @@ ct mark set 0x11333 and 0x11;ok;ct mark set 0x00000011
 ct mark set 0x12 or 0x11;ok;ct mark set 0x00000013
 ct mark set 0x11;ok;ct mark set 0x00000011
 ct mark set mark;ok;ct mark set meta mark
+ct mark set (meta mark | 0x10) << 8;ok;ct mark set (meta mark | 0x00000010) << 8
 ct mark set mark map { 1 : 10, 2 : 20, 3 : 30 };ok;ct mark set meta mark map { 0x00000003 : 0x0000001e, 0x00000002 : 0x00000014, 0x00000001 : 0x0000000a}
 
 ct mark set {0x11333, 0x11};fail
diff --git a/tests/py/any/ct.t.json b/tests/py/any/ct.t.json
index 8d56db2aaedb..59ac27c3055c 100644
--- a/tests/py/any/ct.t.json
+++ b/tests/py/any/ct.t.json
@@ -724,6 +724,34 @@
     }
 ]
 
+# ct mark set (meta mark | 0x10) << 8
+[
+    {
+        "mangle": {
+            "key": {
+                "ct": {
+                    "key": "mark"
+                }
+            },
+            "value": {
+                "<<": [
+                    {
+                        "|": [
+                            {
+                                "meta": {
+                                    "key": "mark"
+                                }
+                            },
+                            16
+                        ]
+                    },
+                    8
+                ]
+            }
+        }
+    }
+]
+
 # ct mark set mark map { 1 : 10, 2 : 20, 3 : 30 }
 [
     {
diff --git a/tests/py/any/ct.t.payload b/tests/py/any/ct.t.payload
index bdc6a70e3672..661591257804 100644
--- a/tests/py/any/ct.t.payload
+++ b/tests/py/any/ct.t.payload
@@ -329,6 +329,27 @@ ip test-ip4 output
   [ meta load mark => reg 1 ]
   [ ct set mark with reg 1 ]
 
+# ct mark set (meta mark | 0x10) << 8
+ip test-ip4 output
+  [ meta load mark => reg 1 ]
+  [ bitwise reg 1 = (reg=1 & 0xffffffef ) ^ 0x00000010 ]
+  [ bitwise reg 1 = ( reg 1 << 0x00000008 ) ]
+  [ ct set mark with reg 1 ]
+
+# ct mark set (meta mark | 0x10) << 8
+ip6 test-ip6 output
+  [ meta load mark => reg 1 ]
+  [ bitwise reg 1 = (reg=1 & 0xffffffef ) ^ 0x00000010 ]
+  [ bitwise reg 1 = ( reg 1 << 0x00000008 ) ]
+  [ ct set mark with reg 1 ]
+
+# ct mark set (meta mark | 0x10) << 8
+inet test-inet output
+  [ meta load mark => reg 1 ]
+  [ bitwise reg 1 = (reg=1 & 0xffffffef ) ^ 0x00000010 ]
+  [ bitwise reg 1 = ( reg 1 << 0x00000008 ) ]
+  [ ct set mark with reg 1 ]
+
 # ct mark set mark map { 1 : 10, 2 : 20, 3 : 30 }
 __map%d test-ip4 b
 __map%d test-ip4 0
diff --git a/tests/py/inet/meta.t b/tests/py/inet/meta.t
index df32332f0621..3638898b5dbb 100644
--- a/tests/py/inet/meta.t
+++ b/tests/py/inet/meta.t
@@ -16,3 +16,4 @@ meta ipsec exists;ok
 meta secpath missing;ok;meta ipsec missing
 meta ibrname "br0";fail
 meta obrname "br0";fail
+meta mark set ct mark >> 8;ok
diff --git a/tests/py/inet/meta.t.json b/tests/py/inet/meta.t.json
index 5501f0bec6ed..5c0e7d2e0e42 100644
--- a/tests/py/inet/meta.t.json
+++ b/tests/py/inet/meta.t.json
@@ -213,3 +213,25 @@
     }
 ]
 
+# meta mark set ct mark >> 8
+[
+    {
+        "mangle": {
+            "key": {
+                "meta": {
+                    "key": "mark"
+                }
+            },
+            "value": {
+                ">>": [
+                    {
+                        "ct": {
+                            "key": "mark"
+                        }
+                    },
+                    8
+                ]
+            }
+        }
+    }
+]
diff --git a/tests/py/inet/meta.t.payload b/tests/py/inet/meta.t.payload
index d7ff7e2d41fa..6ccf6d24210a 100644
--- a/tests/py/inet/meta.t.payload
+++ b/tests/py/inet/meta.t.payload
@@ -73,3 +73,9 @@ inet test-inet input
 inet test-inet input
   [ meta load secpath => reg 1 ]
   [ cmp eq reg 1 0x00000000 ]
+
+# meta mark set ct mark >> 8
+inet test-inet input
+  [ ct load mark => reg 1 ]
+  [ bitwise reg 1 = ( reg 1 >> 0x00000008 ) ]
+  [ meta set mark with reg 1 ]
-- 
2.24.1

Re: [PATCH nft v4 0/6] Remaining bitwise-shift-related changes

[Pablo Neira Ayuso]

On Mon, Feb 03, 2020 at 11:20:17AM +0000, Jeremy Sowden wrote:
> While most of the v3 bitwise-shift patches were applied, a couple of the
> tidy-up ones and the new tests in the bitwise-shift series needed a bit
> more work.  This version introduces some new changes: a patch renaming a
> variable and a couple adding some Python tests.

Series applied, thanks Jeremy.