All of lore.kernel.org
 help / color / mirror / Atom feed
* [nft PATCH v4 00/32] Extend values assignable to packet marks and payload fields
@ 2022-04-04 12:13 Jeremy Sowden
  2022-04-04 12:13 ` [nft PATCH v4 01/32] examples: add .gitignore file Jeremy Sowden
                   ` (32 more replies)
  0 siblings, 33 replies; 50+ messages in thread
From: Jeremy Sowden @ 2022-04-04 12:13 UTC (permalink / raw)
  To: Netfilter Devel; +Cc: Kevin Darbyshire-Bryant

This patch-set extends the types of value which can be assigned to
packet marks and payload fields.  The original motivation for these
changes was Kevin Darbyshire-Bryant's wish to be able to set the
conntrack mark to a bitwise expression derived from a DSCP value:

  https://lore.kernel.org/netfilter-devel/20191203160652.44396-1-ldir@darbyshire-bryant.me.uk/#r

For example:

  nft add rule t c ct mark set ip dscp lshift 26 or 0x10

In principle, examples like this can be implemented solely by changes to
user space.  However, in some cases the payload munging leads to the
generation of multi-byte binops in host byte-order which are not
correctly eliminated during delinearization: the easiest way to fix this
was to pass the bit-length of these expressions to and from the kernel.

One of the changes required for this example is to relax the requirement
that when assigning a non-integer rvalue, its data-type must match that
of the lvalue.  I have been conservative in relaxing this: for an lvalue
of mark type, any rvalue with integer base-type may be assigned.  I did
try allowing the assignment of any rvalue of integer base-type to any
lvalue of integer base-type, but doing so caused test failures which
were sufficiently obscure that I decided wait and see if the patch-set
in its current form is positively received before spending time
diagnosing and fixing them.

Other examples came up in later discussion, such as:

  nft add rule t c ct mark set ct mark and 0xffff0000 or meta mark and 0xffff

and most recently:

  nft add rule t c ct mark set ct mark or ip dscp or 0x200

These require boolean bitwise operations with two variable operands.
Hitherto, the kernel has required that AND, OR and XOR operations be
converted in user space to mask-and-xor operations on one register and
two immediate values.  The related kernel space patch-set, however, adds
support for performing these operations directly on one register and an
immediate value, or on two registers.  This patch-set extends nftables
to make use of this functionality.

The patch-set is structured as follows.

  * Patch 1 adds a .gitignore file for examples/.
  * Patches 2-5 make some changes which I found helpful when adding
    debugging output.
  * Patch 6 updates the nf_tables.h kernel UAPI header to 5.17-rc7.
  * Patches 7-14 add support for assignments which do not require
    bitwise operations with variable RHS operands.
  * Patches 15-17 add tests for these.
  * Patches 18-30 add support for assignments which do require binops
    with variable RHS.
  * Patches 31-32 add tests for these.

Changes since v3

  * Patches 1-6 are new.
  * When I first posted a version of this work two years ago, the main
    focus was the changes necessary to implement binops with variable
    RHS operands.  My intention was to post the remaining changes,
    including support for assigning expressions of one type to those of
    another, separately.  The problem with this approach was that it led
    to rather contrived test-cases which in turn obscured the intended
    uses of the patch-set.  On this occasion, therefore, I have sent
    everything at once, and patches 7-17 are new.
  * In the previous versions, the variable RHS binops were still
    implemented as mask-and-xor operations, but the mask and xor values
    could be passed in registers.  Thus, in patches 18-30, the
    linearization and delinearization have been substantially reworked,
    and a number of other fixes have also been added.

For reference, v3 may be found here:

  https://lore.kernel.org/netfilter-devel/20200303094844.26694-1-jeremy@azazel.net/#r

Jeremy Sowden (32):
  examples: add .gitignore file
  include: add missing `#include`
  src: move `byteorder_names` array
  datatype: support `NULL` symbol-tables when printing constants
  ct: support `NULL` symbol-tables when looking up labels
  include: update nf_tables.h
  include: add new bitwise bit-length attribute to nf_tables.h
  netlink: send bit-length of bitwise binops to kernel
  netlink_delinearize: add postprocessing for payload binops
  netlink_delinearize: correct type and byte-order of shifts
  netlink_delinearize: correct length of right bitwise operand
  payload: set byte-order when completing expression
  evaluate: support shifts larger than the width of the left operand
  evaluate: relax type-checking for integer arguments in mark statements
  tests: shell: rename some test-cases
  tests: shell: add test-cases for ct and packet mark payload
    expressions
  tests: py: add test-cases for ct and packet mark payload expressions
  include: add new bitwise boolean attributes to nf_tables.h
  evaluate: don't eval unary arguments
  evaluate: prevent nested byte-order conversions
  evaluate: don't clobber binop lengths
  evaluate: insert byte-order conversions for expressions between 9 and
    15 bits
  evaluate: set eval context to leftmost bitwise operand
  netlink_delinearize: fix typo
  netlink_delinearize: refactor stmt_payload_binop_postprocess
  netlink_delinearize: add support for processing variable payload
    statement arguments
  netlink: rename bitwise operation functions
  netlink: support (de)linearization of new bitwise boolean operations
  parser_json: allow RHS ct, meta and payload expressions
  evaluate: allow binop expressions with variable right-hand operands
  tests: shell: add tests for binops with variable RHS operands
  tests: py: add tests for binops with variable RHS operands

 examples/.gitignore                           |   5 +
 include/datatype.h                            |   7 +
 include/linux/netfilter/nf_tables.h           |  49 ++-
 src/ct.c                                      |   9 +-
 src/datatype.c                                |  14 +-
 src/evaluate.c                                | 101 +++--
 src/netlink_delinearize.c                     | 408 +++++++++++++-----
 src/netlink_linearize.c                       |  66 ++-
 src/parser_json.c                             |   8 +-
 src/payload.c                                 |   1 +
 tests/py/any/ct.t                             |   1 +
 tests/py/any/ct.t.json                        |  37 ++
 tests/py/any/ct.t.payload                     |   9 +
 tests/py/inet/meta.t                          |   1 +
 tests/py/inet/meta.t.json                     |  37 ++
 tests/py/inet/meta.t.payload                  |   9 +
 tests/py/ip/ct.t                              |   3 +
 tests/py/ip/ct.t.json                         |  94 ++++
 tests/py/ip/ct.t.payload                      |  29 ++
 tests/py/ip/ip.t                              |   2 +
 tests/py/ip/ip.t.json                         |  77 +++-
 tests/py/ip/ip.t.payload                      |  28 ++
 tests/py/ip/ip.t.payload.bridge               |  32 ++
 tests/py/ip/ip.t.payload.inet                 |  32 ++
 tests/py/ip/ip.t.payload.netdev               |  32 ++
 tests/py/ip/meta.t                            |   3 +
 tests/py/ip/meta.t.json                       |  59 +++
 tests/py/ip/meta.t.payload                    |  18 +
 tests/py/ip6/ct.t                             |   7 +
 tests/py/ip6/ct.t.json                        |  93 ++++
 tests/py/ip6/ct.t.payload                     |  31 ++
 tests/py/ip6/ip6.t                            |   2 +
 tests/py/ip6/ip6.t.json                       |  76 ++++
 tests/py/ip6/ip6.t.payload.inet               |  34 ++
 tests/py/ip6/ip6.t.payload.ip6                |  30 ++
 tests/py/ip6/meta.t                           |   3 +
 tests/py/ip6/meta.t.json                      |  58 +++
 tests/py/ip6/meta.t.payload                   |  20 +
 .../{0040mark_shift_0 => 0040mark_binop_0}    |   2 +-
 .../{0040mark_shift_1 => 0040mark_binop_1}    |   2 +-
 .../shell/testcases/chains/0040mark_binop_10  |  11 +
 .../shell/testcases/chains/0040mark_binop_11  |  11 +
 .../shell/testcases/chains/0040mark_binop_12  |  11 +
 .../shell/testcases/chains/0040mark_binop_13  |  11 +
 tests/shell/testcases/chains/0040mark_binop_2 |  11 +
 tests/shell/testcases/chains/0040mark_binop_3 |  11 +
 tests/shell/testcases/chains/0040mark_binop_4 |  11 +
 tests/shell/testcases/chains/0040mark_binop_5 |  11 +
 tests/shell/testcases/chains/0040mark_binop_6 |  11 +
 tests/shell/testcases/chains/0040mark_binop_7 |  11 +
 tests/shell/testcases/chains/0040mark_binop_8 |  11 +
 tests/shell/testcases/chains/0040mark_binop_9 |  11 +
 .../testcases/chains/0044payload_binop_0      |  11 +
 .../testcases/chains/0044payload_binop_1      |  11 +
 .../testcases/chains/0044payload_binop_2      |  11 +
 .../testcases/chains/0044payload_binop_3      |  11 +
 .../testcases/chains/0044payload_binop_4      |  11 +
 .../testcases/chains/0044payload_binop_5      |  11 +
 ...0mark_shift_0.nft => 0040mark_binop_0.nft} |   2 +-
 ...0mark_shift_1.nft => 0040mark_binop_1.nft} |   2 +-
 .../chains/dumps/0040mark_binop_10.nft        |   6 +
 .../chains/dumps/0040mark_binop_11.nft        |   6 +
 .../chains/dumps/0040mark_binop_12.nft        |   6 +
 .../chains/dumps/0040mark_binop_13.nft        |   6 +
 .../chains/dumps/0040mark_binop_2.nft         |   6 +
 .../chains/dumps/0040mark_binop_3.nft         |   6 +
 .../chains/dumps/0040mark_binop_4.nft         |   6 +
 .../chains/dumps/0040mark_binop_5.nft         |   6 +
 .../chains/dumps/0040mark_binop_6.nft         |   6 +
 .../chains/dumps/0040mark_binop_7.nft         |   6 +
 .../chains/dumps/0040mark_binop_8.nft         |   6 +
 .../chains/dumps/0040mark_binop_9.nft         |   6 +
 .../chains/dumps/0044payload_binop_0.nft      |   6 +
 .../chains/dumps/0044payload_binop_1.nft      |   6 +
 .../chains/dumps/0044payload_binop_2.nft      |   6 +
 .../chains/dumps/0044payload_binop_3.nft      |   6 +
 .../chains/dumps/0044payload_binop_4.nft      |   6 +
 .../chains/dumps/0044payload_binop_5.nft      |   6 +
 78 files changed, 1660 insertions(+), 179 deletions(-)
 create mode 100644 examples/.gitignore
 create mode 100644 tests/py/ip6/ct.t
 create mode 100644 tests/py/ip6/ct.t.json
 create mode 100644 tests/py/ip6/ct.t.payload
 rename tests/shell/testcases/chains/{0040mark_shift_0 => 0040mark_binop_0} (68%)
 rename tests/shell/testcases/chains/{0040mark_shift_1 => 0040mark_binop_1} (70%)
 create mode 100755 tests/shell/testcases/chains/0040mark_binop_10
 create mode 100755 tests/shell/testcases/chains/0040mark_binop_11
 create mode 100755 tests/shell/testcases/chains/0040mark_binop_12
 create mode 100755 tests/shell/testcases/chains/0040mark_binop_13
 create mode 100755 tests/shell/testcases/chains/0040mark_binop_2
 create mode 100755 tests/shell/testcases/chains/0040mark_binop_3
 create mode 100755 tests/shell/testcases/chains/0040mark_binop_4
 create mode 100755 tests/shell/testcases/chains/0040mark_binop_5
 create mode 100755 tests/shell/testcases/chains/0040mark_binop_6
 create mode 100755 tests/shell/testcases/chains/0040mark_binop_7
 create mode 100755 tests/shell/testcases/chains/0040mark_binop_8
 create mode 100755 tests/shell/testcases/chains/0040mark_binop_9
 create mode 100755 tests/shell/testcases/chains/0044payload_binop_0
 create mode 100755 tests/shell/testcases/chains/0044payload_binop_1
 create mode 100755 tests/shell/testcases/chains/0044payload_binop_2
 create mode 100755 tests/shell/testcases/chains/0044payload_binop_3
 create mode 100755 tests/shell/testcases/chains/0044payload_binop_4
 create mode 100755 tests/shell/testcases/chains/0044payload_binop_5
 rename tests/shell/testcases/chains/dumps/{0040mark_shift_0.nft => 0040mark_binop_0.nft} (58%)
 rename tests/shell/testcases/chains/dumps/{0040mark_shift_1.nft => 0040mark_binop_1.nft} (64%)
 create mode 100644 tests/shell/testcases/chains/dumps/0040mark_binop_10.nft
 create mode 100644 tests/shell/testcases/chains/dumps/0040mark_binop_11.nft
 create mode 100644 tests/shell/testcases/chains/dumps/0040mark_binop_12.nft
 create mode 100644 tests/shell/testcases/chains/dumps/0040mark_binop_13.nft
 create mode 100644 tests/shell/testcases/chains/dumps/0040mark_binop_2.nft
 create mode 100644 tests/shell/testcases/chains/dumps/0040mark_binop_3.nft
 create mode 100644 tests/shell/testcases/chains/dumps/0040mark_binop_4.nft
 create mode 100644 tests/shell/testcases/chains/dumps/0040mark_binop_5.nft
 create mode 100644 tests/shell/testcases/chains/dumps/0040mark_binop_6.nft
 create mode 100644 tests/shell/testcases/chains/dumps/0040mark_binop_7.nft
 create mode 100644 tests/shell/testcases/chains/dumps/0040mark_binop_8.nft
 create mode 100644 tests/shell/testcases/chains/dumps/0040mark_binop_9.nft
 create mode 100644 tests/shell/testcases/chains/dumps/0044payload_binop_0.nft
 create mode 100644 tests/shell/testcases/chains/dumps/0044payload_binop_1.nft
 create mode 100644 tests/shell/testcases/chains/dumps/0044payload_binop_2.nft
 create mode 100644 tests/shell/testcases/chains/dumps/0044payload_binop_3.nft
 create mode 100644 tests/shell/testcases/chains/dumps/0044payload_binop_4.nft
 create mode 100644 tests/shell/testcases/chains/dumps/0044payload_binop_5.nft

-- 
2.35.1


^ permalink raw reply	[flat|nested] 50+ messages in thread

* [nft PATCH v4 01/32] examples: add .gitignore file
  2022-04-04 12:13 [nft PATCH v4 00/32] Extend values assignable to packet marks and payload fields Jeremy Sowden
@ 2022-04-04 12:13 ` Jeremy Sowden
  2022-04-05 11:26   ` Florian Westphal
  2022-04-04 12:13 ` [nft PATCH v4 02/32] include: add missing `#include` Jeremy Sowden
                   ` (31 subsequent siblings)
  32 siblings, 1 reply; 50+ messages in thread
From: Jeremy Sowden @ 2022-04-04 12:13 UTC (permalink / raw)
  To: Netfilter Devel; +Cc: Kevin Darbyshire-Bryant

---
 examples/.gitignore | 5 +++++
 1 file changed, 5 insertions(+)
 create mode 100644 examples/.gitignore

diff --git a/examples/.gitignore b/examples/.gitignore
new file mode 100644
index 000000000000..7b1a583c687e
--- /dev/null
+++ b/examples/.gitignore
@@ -0,0 +1,5 @@
+/.deps/
+/.libs/
+/nft-buffer
+/nft-json-file
+/*.o
-- 
2.35.1


^ permalink raw reply related	[flat|nested] 50+ messages in thread

* [nft PATCH v4 02/32] include: add missing `#include`
  2022-04-04 12:13 [nft PATCH v4 00/32] Extend values assignable to packet marks and payload fields Jeremy Sowden
  2022-04-04 12:13 ` [nft PATCH v4 01/32] examples: add .gitignore file Jeremy Sowden
@ 2022-04-04 12:13 ` Jeremy Sowden
  2022-04-04 12:13 ` [nft PATCH v4 03/32] src: move `byteorder_names` array Jeremy Sowden
                   ` (30 subsequent siblings)
  32 siblings, 0 replies; 50+ messages in thread
From: Jeremy Sowden @ 2022-04-04 12:13 UTC (permalink / raw)
  To: Netfilter Devel; +Cc: Kevin Darbyshire-Bryant

datatype.h uses bool and so should include <stdbool.h>.

Signed-off-by: Jeremy Sowden <jeremy@azazel.net>
---
 include/datatype.h | 1 +
 1 file changed, 1 insertion(+)

diff --git a/include/datatype.h b/include/datatype.h
index f5bb9dc4d937..0b90a33e4e64 100644
--- a/include/datatype.h
+++ b/include/datatype.h
@@ -1,6 +1,7 @@
 #ifndef NFTABLES_DATATYPE_H
 #define NFTABLES_DATATYPE_H
 
+#include <stdbool.h>
 #include <json.h>
 
 /**
-- 
2.35.1


^ permalink raw reply related	[flat|nested] 50+ messages in thread

* [nft PATCH v4 03/32] src: move `byteorder_names` array
  2022-04-04 12:13 [nft PATCH v4 00/32] Extend values assignable to packet marks and payload fields Jeremy Sowden
  2022-04-04 12:13 ` [nft PATCH v4 01/32] examples: add .gitignore file Jeremy Sowden
  2022-04-04 12:13 ` [nft PATCH v4 02/32] include: add missing `#include` Jeremy Sowden
@ 2022-04-04 12:13 ` Jeremy Sowden
  2022-04-04 12:13 ` [nft PATCH v4 04/32] datatype: support `NULL` symbol-tables when printing constants Jeremy Sowden
                   ` (29 subsequent siblings)
  32 siblings, 0 replies; 50+ messages in thread
From: Jeremy Sowden @ 2022-04-04 12:13 UTC (permalink / raw)
  To: Netfilter Devel; +Cc: Kevin Darbyshire-Bryant

It's useful for debugging, so move it out of evaluate.c to make it
available elsewhere.

Signed-off-by: Jeremy Sowden <jeremy@azazel.net>
---
 include/datatype.h | 6 ++++++
 src/evaluate.c     | 7 +------
 2 files changed, 7 insertions(+), 6 deletions(-)

diff --git a/include/datatype.h b/include/datatype.h
index 0b90a33e4e64..8d774a91e350 100644
--- a/include/datatype.h
+++ b/include/datatype.h
@@ -119,6 +119,12 @@ enum byteorder {
 	BYTEORDER_BIG_ENDIAN,
 };
 
+static const char *const byteorder_names[] = {
+	[BYTEORDER_INVALID]             = "invalid",
+	[BYTEORDER_HOST_ENDIAN]         = "host endian",
+	[BYTEORDER_BIG_ENDIAN]          = "big endian",
+};
+
 struct expr;
 
 /**
diff --git a/src/evaluate.c b/src/evaluate.c
index 04d42b800103..be493f85010c 100644
--- a/src/evaluate.c
+++ b/src/evaluate.c
@@ -27,6 +27,7 @@
 #include <net/if.h>
 #include <errno.h>
 
+#include <datatype.h>
 #include <expression.h>
 #include <statement.h>
 #include <netlink.h>
@@ -40,12 +41,6 @@
 
 static int expr_evaluate(struct eval_ctx *ctx, struct expr **expr);
 
-static const char * const byteorder_names[] = {
-	[BYTEORDER_INVALID]		= "invalid",
-	[BYTEORDER_HOST_ENDIAN]		= "host endian",
-	[BYTEORDER_BIG_ENDIAN]		= "big endian",
-};
-
 #define chain_error(ctx, s1, fmt, args...) \
 	__stmt_binary_error(ctx, &(s1)->location, NULL, fmt, ## args)
 #define monitor_error(ctx, s1, fmt, args...) \
-- 
2.35.1


^ permalink raw reply related	[flat|nested] 50+ messages in thread

* [nft PATCH v4 04/32] datatype: support `NULL` symbol-tables when printing constants
  2022-04-04 12:13 [nft PATCH v4 00/32] Extend values assignable to packet marks and payload fields Jeremy Sowden
                   ` (2 preceding siblings ...)
  2022-04-04 12:13 ` [nft PATCH v4 03/32] src: move `byteorder_names` array Jeremy Sowden
@ 2022-04-04 12:13 ` Jeremy Sowden
  2022-04-04 12:13 ` [nft PATCH v4 05/32] ct: support `NULL` symbol-tables when looking up labels Jeremy Sowden
                   ` (28 subsequent siblings)
  32 siblings, 0 replies; 50+ messages in thread
From: Jeremy Sowden @ 2022-04-04 12:13 UTC (permalink / raw)
  To: Netfilter Devel; +Cc: Kevin Darbyshire-Bryant

If the symbol-table passed to `symbol_constant_print` is `NULL`, fall
back to printing the expression's base-type.

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

diff --git a/src/datatype.c b/src/datatype.c
index b2e667cef2c6..668823b6c7b1 100644
--- a/src/datatype.c
+++ b/src/datatype.c
@@ -185,7 +185,7 @@ void symbolic_constant_print(const struct symbol_table *tbl,
 			     struct output_ctx *octx)
 {
 	unsigned int len = div_round_up(expr->len, BITS_PER_BYTE);
-	const struct symbolic_constant *s;
+	const struct symbolic_constant *s = NULL;
 	uint64_t val = 0;
 
 	/* Export the data in the correct byteorder for comparison */
@@ -193,12 +193,14 @@ void symbolic_constant_print(const struct symbol_table *tbl,
 	mpz_export_data(constant_data_ptr(val, expr->len), expr->value,
 			expr->byteorder, len);
 
-	for (s = tbl->symbols; s->identifier != NULL; s++) {
-		if (val == s->value)
-			break;
-	}
+	if (tbl != NULL)
+		for (s = tbl->symbols; s->identifier != NULL; s++) {
+			if (val == s->value)
+				break;
+		}
 
-	if (s->identifier == NULL || nft_output_numeric_symbol(octx))
+	if (s == NULL || s->identifier == NULL ||
+	    nft_output_numeric_symbol(octx))
 		return expr_basetype(expr)->print(expr, octx);
 
 	nft_print(octx, quotes ? "\"%s\"" : "%s", s->identifier);
-- 
2.35.1


^ permalink raw reply related	[flat|nested] 50+ messages in thread

* [nft PATCH v4 05/32] ct: support `NULL` symbol-tables when looking up labels
  2022-04-04 12:13 [nft PATCH v4 00/32] Extend values assignable to packet marks and payload fields Jeremy Sowden
                   ` (3 preceding siblings ...)
  2022-04-04 12:13 ` [nft PATCH v4 04/32] datatype: support `NULL` symbol-tables when printing constants Jeremy Sowden
@ 2022-04-04 12:13 ` Jeremy Sowden
  2022-04-05 11:15   ` Florian Westphal
  2022-04-04 12:13 ` [nft PATCH v4 06/32] include: update nf_tables.h Jeremy Sowden
                   ` (27 subsequent siblings)
  32 siblings, 1 reply; 50+ messages in thread
From: Jeremy Sowden @ 2022-04-04 12:13 UTC (permalink / raw)
  To: Netfilter Devel; +Cc: Kevin Darbyshire-Bryant

If the symbol-table passed to `ct_label2str` is `NULL`, return `NULL`.

Signed-off-by: Jeremy Sowden <jeremy@azazel.net>
---
 src/ct.c | 9 +++++----
 1 file changed, 5 insertions(+), 4 deletions(-)

diff --git a/src/ct.c b/src/ct.c
index e246d3039240..8c9ae7b0e04a 100644
--- a/src/ct.c
+++ b/src/ct.c
@@ -148,10 +148,11 @@ const char *ct_label2str(const struct symbol_table *ct_label_tbl,
 {
 	const struct symbolic_constant *s;
 
-	for (s = ct_label_tbl->symbols; s->identifier; s++) {
-		if (value == s->value)
-			return s->identifier;
-	}
+	if (ct_label_tbl != NULL)
+		for (s = ct_label_tbl->symbols; s->identifier; s++) {
+			if (value == s->value)
+				return s->identifier;
+		}
 
 	return NULL;
 }
-- 
2.35.1


^ permalink raw reply related	[flat|nested] 50+ messages in thread

* [nft PATCH v4 06/32] include: update nf_tables.h
  2022-04-04 12:13 [nft PATCH v4 00/32] Extend values assignable to packet marks and payload fields Jeremy Sowden
                   ` (4 preceding siblings ...)
  2022-04-04 12:13 ` [nft PATCH v4 05/32] ct: support `NULL` symbol-tables when looking up labels Jeremy Sowden
@ 2022-04-04 12:13 ` Jeremy Sowden
  2022-04-04 12:13 ` [nft PATCH v4 07/32] include: add new bitwise bit-length attribute to nf_tables.h Jeremy Sowden
                   ` (26 subsequent siblings)
  32 siblings, 0 replies; 50+ messages in thread
From: Jeremy Sowden @ 2022-04-04 12:13 UTC (permalink / raw)
  To: Netfilter Devel; +Cc: Kevin Darbyshire-Bryant

Bump it to 5.17-rc7.

Signed-off-by: Jeremy Sowden <jeremy@azazel.net>
---
 include/linux/netfilter/nf_tables.h | 28 +++++++++++++++++++++++++++-
 1 file changed, 27 insertions(+), 1 deletion(-)

diff --git a/include/linux/netfilter/nf_tables.h b/include/linux/netfilter/nf_tables.h
index 75df968d231b..466fd3f4447c 100644
--- a/include/linux/netfilter/nf_tables.h
+++ b/include/linux/netfilter/nf_tables.h
@@ -164,7 +164,10 @@ enum nft_hook_attributes {
  */
 enum nft_table_flags {
 	NFT_TABLE_F_DORMANT	= 0x1,
+	NFT_TABLE_F_OWNER	= 0x2,
 };
+#define NFT_TABLE_F_MASK	(NFT_TABLE_F_DORMANT | \
+				 NFT_TABLE_F_OWNER)
 
 /**
  * enum nft_table_attributes - nf_tables table netlink attributes
@@ -173,6 +176,7 @@ enum nft_table_flags {
  * @NFTA_TABLE_FLAGS: bitmask of enum nft_table_flags (NLA_U32)
  * @NFTA_TABLE_USE: number of chains in this table (NLA_U32)
  * @NFTA_TABLE_USERDATA: user data (NLA_BINARY)
+ * @NFTA_TABLE_OWNER: owner of this table through netlink portID (NLA_U32)
  */
 enum nft_table_attributes {
 	NFTA_TABLE_UNSPEC,
@@ -182,6 +186,7 @@ enum nft_table_attributes {
 	NFTA_TABLE_HANDLE,
 	NFTA_TABLE_PAD,
 	NFTA_TABLE_USERDATA,
+	NFTA_TABLE_OWNER,
 	__NFTA_TABLE_MAX
 };
 #define NFTA_TABLE_MAX		(__NFTA_TABLE_MAX - 1)
@@ -748,11 +753,13 @@ enum nft_dynset_attributes {
  * @NFT_PAYLOAD_LL_HEADER: link layer header
  * @NFT_PAYLOAD_NETWORK_HEADER: network header
  * @NFT_PAYLOAD_TRANSPORT_HEADER: transport header
+ * @NFT_PAYLOAD_INNER_HEADER: inner header / payload
  */
 enum nft_payload_bases {
 	NFT_PAYLOAD_LL_HEADER,
 	NFT_PAYLOAD_NETWORK_HEADER,
 	NFT_PAYLOAD_TRANSPORT_HEADER,
+	NFT_PAYLOAD_INNER_HEADER,
 };
 
 /**
@@ -891,7 +898,8 @@ enum nft_meta_keys {
 	NFT_META_OIF,
 	NFT_META_IIFNAME,
 	NFT_META_OIFNAME,
-	NFT_META_IIFTYPE,
+	NFT_META_IFTYPE,
+#define NFT_META_IIFTYPE	NFT_META_IFTYPE
 	NFT_META_OIFTYPE,
 	NFT_META_SKUID,
 	NFT_META_SKGID,
@@ -918,6 +926,7 @@ enum nft_meta_keys {
 	NFT_META_TIME_HOUR,
 	NFT_META_SDIF,
 	NFT_META_SDIFNAME,
+	__NFT_META_IIFTYPE,
 };
 
 /**
@@ -1013,6 +1022,7 @@ enum nft_rt_attributes {
  *
  * @NFTA_SOCKET_KEY: socket key to match
  * @NFTA_SOCKET_DREG: destination register
+ * @NFTA_SOCKET_LEVEL: cgroups2 ancestor level (only for cgroupsv2)
  */
 enum nft_socket_attributes {
 	NFTA_SOCKET_UNSPEC,
@@ -1029,6 +1039,7 @@ enum nft_socket_attributes {
  * @NFT_SOCKET_TRANSPARENT: Value of the IP(V6)_TRANSPARENT socket option
  * @NFT_SOCKET_MARK: Value of the socket mark
  * @NFT_SOCKET_WILDCARD: Whether the socket is zero-bound (e.g. 0.0.0.0 or ::0)
+ * @NFT_SOCKET_CGROUPV2: Match on cgroups version 2
  */
 enum nft_socket_keys {
 	NFT_SOCKET_TRANSPARENT,
@@ -1188,6 +1199,21 @@ enum nft_counter_attributes {
 };
 #define NFTA_COUNTER_MAX	(__NFTA_COUNTER_MAX - 1)
 
+/**
+ * enum nft_last_attributes - nf_tables last expression netlink attributes
+ *
+ * @NFTA_LAST_SET: last update has been set, zero means never updated (NLA_U32)
+ * @NFTA_LAST_MSECS: milliseconds since last update (NLA_U64)
+ */
+enum nft_last_attributes {
+	NFTA_LAST_UNSPEC,
+	NFTA_LAST_SET,
+	NFTA_LAST_MSECS,
+	NFTA_LAST_PAD,
+	__NFTA_LAST_MAX
+};
+#define NFTA_LAST_MAX	(__NFTA_LAST_MAX - 1)
+
 /**
  * enum nft_log_attributes - nf_tables log expression netlink attributes
  *
-- 
2.35.1


^ permalink raw reply related	[flat|nested] 50+ messages in thread

* [nft PATCH v4 07/32] include: add new bitwise bit-length attribute to nf_tables.h
  2022-04-04 12:13 [nft PATCH v4 00/32] Extend values assignable to packet marks and payload fields Jeremy Sowden
                   ` (5 preceding siblings ...)
  2022-04-04 12:13 ` [nft PATCH v4 06/32] include: update nf_tables.h Jeremy Sowden
@ 2022-04-04 12:13 ` Jeremy Sowden
  2022-04-04 12:13 ` [nft PATCH v4 08/32] netlink: send bit-length of bitwise binops to kernel Jeremy Sowden
                   ` (25 subsequent siblings)
  32 siblings, 0 replies; 50+ messages in thread
From: Jeremy Sowden @ 2022-04-04 12:13 UTC (permalink / raw)
  To: Netfilter Devel; +Cc: Kevin Darbyshire-Bryant

The kernel can now keep track of the bit-length of boolean expressions.

Signed-off-by: Jeremy Sowden <jeremy@azazel.net>
---
 include/linux/netfilter/nf_tables.h | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/include/linux/netfilter/nf_tables.h b/include/linux/netfilter/nf_tables.h
index 466fd3f4447c..f3dcc4a34ff1 100644
--- a/include/linux/netfilter/nf_tables.h
+++ b/include/linux/netfilter/nf_tables.h
@@ -561,6 +561,7 @@ enum nft_bitwise_ops {
  * @NFTA_BITWISE_OP: type of operation (NLA_U32: nft_bitwise_ops)
  * @NFTA_BITWISE_DATA: argument for non-boolean operations
  *                     (NLA_NESTED: nft_data_attributes)
+ * @NFTA_BITWISE_NBITS: length of operation in bits (NLA_U32)
  *
  * The bitwise expression supports boolean and shift operations.  It implements
  * the boolean operations by performing the following operation:
@@ -584,6 +585,7 @@ enum nft_bitwise_attributes {
 	NFTA_BITWISE_XOR,
 	NFTA_BITWISE_OP,
 	NFTA_BITWISE_DATA,
+	NFTA_BITWISE_NBITS,
 	__NFTA_BITWISE_MAX
 };
 #define NFTA_BITWISE_MAX	(__NFTA_BITWISE_MAX - 1)
-- 
2.35.1


^ permalink raw reply related	[flat|nested] 50+ messages in thread

* [nft PATCH v4 08/32] netlink: send bit-length of bitwise binops to kernel
  2022-04-04 12:13 [nft PATCH v4 00/32] Extend values assignable to packet marks and payload fields Jeremy Sowden
                   ` (6 preceding siblings ...)
  2022-04-04 12:13 ` [nft PATCH v4 07/32] include: add new bitwise bit-length attribute to nf_tables.h Jeremy Sowden
@ 2022-04-04 12:13 ` Jeremy Sowden
  2022-05-23 17:03   ` Pablo Neira Ayuso
  2022-04-04 12:13 ` [nft PATCH v4 09/32] netlink_delinearize: add postprocessing for payload binops Jeremy Sowden
                   ` (24 subsequent siblings)
  32 siblings, 1 reply; 50+ messages in thread
From: Jeremy Sowden @ 2022-04-04 12:13 UTC (permalink / raw)
  To: Netfilter Devel; +Cc: Kevin Darbyshire-Bryant

Some bitwise operations are generated when munging paylod expressions.
During delinearization, we attempt to eliminate these operations.
However, this is done before deducing the byte-order or the correct
length in bits of the operands, which means that we don't always handle
multi-byte host-endian operations correctly.  Therefore, pass the
bit-length of these expressions to the kernel in order to have it
available during delinearization.

Signed-off-by: Jeremy Sowden <jeremy@azazel.net>
---
 src/netlink_delinearize.c | 14 ++++++++++++--
 src/netlink_linearize.c   |  2 ++
 2 files changed, 14 insertions(+), 2 deletions(-)

diff --git a/src/netlink_delinearize.c b/src/netlink_delinearize.c
index a1b00dee209a..733977bc526d 100644
--- a/src/netlink_delinearize.c
+++ b/src/netlink_delinearize.c
@@ -451,20 +451,28 @@ static struct expr *netlink_parse_bitwise_bool(struct netlink_parse_ctx *ctx,
 					       const struct nftnl_expr *nle,
 					       enum nft_registers sreg,
 					       struct expr *left)
-
 {
 	struct nft_data_delinearize nld;
 	struct expr *expr, *mask, *xor, *or;
+	unsigned int nbits;
 	mpz_t m, x, o;
 
 	expr = left;
 
+	nbits = nftnl_expr_get_u32(nle, NFTNL_EXPR_BITWISE_NBITS);
+	if (nbits > 0)
+		expr->len = nbits;
+
 	nld.value = nftnl_expr_get(nle, NFTNL_EXPR_BITWISE_MASK, &nld.len);
 	mask = netlink_alloc_value(loc, &nld);
+	if (nbits > 0)
+		mpz_switch_byteorder(mask->value, div_round_up(nbits, BITS_PER_BYTE));
 	mpz_init_set(m, mask->value);
 
 	nld.value = nftnl_expr_get(nle, NFTNL_EXPR_BITWISE_XOR, &nld.len);
-	xor  = netlink_alloc_value(loc, &nld);
+	xor = netlink_alloc_value(loc, &nld);
+	if (nbits > 0)
+		mpz_switch_byteorder(xor->value, div_round_up(nbits, BITS_PER_BYTE));
 	mpz_init_set(x, xor->value);
 
 	mpz_init_set_ui(o, 0);
@@ -500,6 +508,8 @@ static struct expr *netlink_parse_bitwise_bool(struct netlink_parse_ctx *ctx,
 
 		or = netlink_alloc_value(loc, &nld);
 		mpz_set(or->value, o);
+		if (nbits > 0)
+			mpz_switch_byteorder(or->value, div_round_up(nbits, BITS_PER_BYTE));
 		expr = binop_expr_alloc(loc, OP_OR, expr, or);
 		expr->len = left->len;
 	}
diff --git a/src/netlink_linearize.c b/src/netlink_linearize.c
index c8bbcb7452b0..4793f3853bee 100644
--- a/src/netlink_linearize.c
+++ b/src/netlink_linearize.c
@@ -677,6 +677,8 @@ static void netlink_gen_bitwise(struct netlink_linearize_ctx *ctx,
 	netlink_put_register(nle, NFTNL_EXPR_BITWISE_DREG, dreg);
 	nftnl_expr_set_u32(nle, NFTNL_EXPR_BITWISE_OP, NFT_BITWISE_BOOL);
 	nftnl_expr_set_u32(nle, NFTNL_EXPR_BITWISE_LEN, len);
+	if (expr->byteorder == BYTEORDER_HOST_ENDIAN)
+		nftnl_expr_set_u32(nle, NFTNL_EXPR_BITWISE_NBITS, expr->len);
 
 	netlink_gen_raw_data(mask, expr->byteorder, len, &nld);
 	nftnl_expr_set(nle, NFTNL_EXPR_BITWISE_MASK, nld.value, nld.len);
-- 
2.35.1


^ permalink raw reply related	[flat|nested] 50+ messages in thread

* [nft PATCH v4 09/32] netlink_delinearize: add postprocessing for payload binops
  2022-04-04 12:13 [nft PATCH v4 00/32] Extend values assignable to packet marks and payload fields Jeremy Sowden
                   ` (7 preceding siblings ...)
  2022-04-04 12:13 ` [nft PATCH v4 08/32] netlink: send bit-length of bitwise binops to kernel Jeremy Sowden
@ 2022-04-04 12:13 ` Jeremy Sowden
  2022-05-23 17:19   ` Pablo Neira Ayuso
  2022-04-04 12:13 ` [nft PATCH v4 10/32] netlink_delinearize: correct type and byte-order of shifts Jeremy Sowden
                   ` (23 subsequent siblings)
  32 siblings, 1 reply; 50+ messages in thread
From: Jeremy Sowden @ 2022-04-04 12:13 UTC (permalink / raw)
  To: Netfilter Devel; +Cc: Kevin Darbyshire-Bryant

If a user uses a payload expression as a statement argument:

  nft add rule t c meta mark set ip dscp lshift 2 or 0x10

we may need to undo munging during delinearization.

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

diff --git a/src/netlink_delinearize.c b/src/netlink_delinearize.c
index 733977bc526d..12624db4c3a5 100644
--- a/src/netlink_delinearize.c
+++ b/src/netlink_delinearize.c
@@ -2454,6 +2454,42 @@ static void relational_binop_postprocess(struct rule_pp_ctx *ctx,
 	}
 }
 
+static bool payload_binop_postprocess(struct rule_pp_ctx *ctx,
+				      struct expr **exprp)
+{
+	struct expr *expr = *exprp;
+
+	if (expr->op != OP_RSHIFT)
+		return false;
+
+	if (expr->left->etype == EXPR_UNARY) {
+		/*
+		 * If the payload value was originally in a different byte-order
+		 * from the payload expression, there will be a byte-order
+		 * conversion to remove.
+		 */
+		struct expr *left = expr_get(expr->left->arg);
+		expr_free(expr->left);
+		expr->left = left;
+	}
+
+	if (expr->left->etype != EXPR_BINOP || expr->left->op != OP_AND)
+		return false;
+
+	if (expr->left->left->etype != EXPR_PAYLOAD)
+		return false;
+
+	expr_set_type(expr->right, &integer_type,
+		      BYTEORDER_HOST_ENDIAN);
+	expr_postprocess(ctx, &expr->right);
+
+	binop_postprocess(ctx, expr, &expr->left);
+	*exprp = expr_get(expr->left);
+	expr_free(expr);
+
+	return true;
+}
+
 static struct expr *string_wildcard_expr_alloc(struct location *loc,
 					       const struct expr *mask,
 					       const struct expr *expr)
@@ -2566,6 +2602,9 @@ static void expr_postprocess(struct rule_pp_ctx *ctx, struct expr **exprp)
 		expr_set_type(expr, expr->arg->dtype, !expr->arg->byteorder);
 		break;
 	case EXPR_BINOP:
+		if (payload_binop_postprocess(ctx, exprp))
+			break;
+
 		expr_postprocess(ctx, &expr->left);
 		switch (expr->op) {
 		case OP_LSHIFT:
-- 
2.35.1


^ permalink raw reply related	[flat|nested] 50+ messages in thread

* [nft PATCH v4 10/32] netlink_delinearize: correct type and byte-order of shifts
  2022-04-04 12:13 [nft PATCH v4 00/32] Extend values assignable to packet marks and payload fields Jeremy Sowden
                   ` (8 preceding siblings ...)
  2022-04-04 12:13 ` [nft PATCH v4 09/32] netlink_delinearize: add postprocessing for payload binops Jeremy Sowden
@ 2022-04-04 12:13 ` Jeremy Sowden
  2022-05-23 17:19   ` Pablo Neira Ayuso
  2022-04-04 12:13 ` [nft PATCH v4 11/32] netlink_delinearize: correct length of right bitwise operand Jeremy Sowden
                   ` (22 subsequent siblings)
  32 siblings, 1 reply; 50+ messages in thread
From: Jeremy Sowden @ 2022-04-04 12:13 UTC (permalink / raw)
  To: Netfilter Devel; +Cc: Kevin Darbyshire-Bryant

Shifts are of integer type and in HBO.

Signed-off-by: Jeremy Sowden <jeremy@azazel.net>
---
 src/netlink_delinearize.c | 13 +++++++++++--
 1 file changed, 11 insertions(+), 2 deletions(-)

diff --git a/src/netlink_delinearize.c b/src/netlink_delinearize.c
index 12624db4c3a5..8b010fe4d168 100644
--- a/src/netlink_delinearize.c
+++ b/src/netlink_delinearize.c
@@ -2618,8 +2618,17 @@ static void expr_postprocess(struct rule_pp_ctx *ctx, struct expr **exprp)
 		}
 		expr_postprocess(ctx, &expr->right);
 
-		expr_set_type(expr, expr->left->dtype,
-			      expr->left->byteorder);
+		switch (expr->op) {
+		case OP_LSHIFT:
+		case OP_RSHIFT:
+			expr_set_type(expr, &integer_type,
+				      BYTEORDER_HOST_ENDIAN);
+			break;
+		default:
+			expr_set_type(expr, expr->left->dtype,
+				      expr->left->byteorder);
+		}
+
 		break;
 	case EXPR_RELATIONAL:
 		switch (expr->left->etype) {
-- 
2.35.1


^ permalink raw reply related	[flat|nested] 50+ messages in thread

* [nft PATCH v4 11/32] netlink_delinearize: correct length of right bitwise operand
  2022-04-04 12:13 [nft PATCH v4 00/32] Extend values assignable to packet marks and payload fields Jeremy Sowden
                   ` (9 preceding siblings ...)
  2022-04-04 12:13 ` [nft PATCH v4 10/32] netlink_delinearize: correct type and byte-order of shifts Jeremy Sowden
@ 2022-04-04 12:13 ` Jeremy Sowden
  2022-05-23 17:22   ` Pablo Neira Ayuso
  2022-04-04 12:13 ` [nft PATCH v4 12/32] payload: set byte-order when completing expression Jeremy Sowden
                   ` (21 subsequent siblings)
  32 siblings, 1 reply; 50+ messages in thread
From: Jeremy Sowden @ 2022-04-04 12:13 UTC (permalink / raw)
  To: Netfilter Devel; +Cc: Kevin Darbyshire-Bryant

Set it to match the length of the left operand.

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

diff --git a/src/netlink_delinearize.c b/src/netlink_delinearize.c
index 8b010fe4d168..cf5359bf269e 100644
--- a/src/netlink_delinearize.c
+++ b/src/netlink_delinearize.c
@@ -2613,6 +2613,7 @@ static void expr_postprocess(struct rule_pp_ctx *ctx, struct expr **exprp)
 				      BYTEORDER_HOST_ENDIAN);
 			break;
 		default:
+			expr->right->len = expr->left->len;
 			expr_set_type(expr->right, expr->left->dtype,
 				      expr->left->byteorder);
 		}
-- 
2.35.1


^ permalink raw reply related	[flat|nested] 50+ messages in thread

* [nft PATCH v4 12/32] payload: set byte-order when completing expression
  2022-04-04 12:13 [nft PATCH v4 00/32] Extend values assignable to packet marks and payload fields Jeremy Sowden
                   ` (10 preceding siblings ...)
  2022-04-04 12:13 ` [nft PATCH v4 11/32] netlink_delinearize: correct length of right bitwise operand Jeremy Sowden
@ 2022-04-04 12:13 ` Jeremy Sowden
  2022-04-04 12:13 ` [nft PATCH v4 13/32] evaluate: support shifts larger than the width of the left operand Jeremy Sowden
                   ` (20 subsequent siblings)
  32 siblings, 0 replies; 50+ messages in thread
From: Jeremy Sowden @ 2022-04-04 12:13 UTC (permalink / raw)
  To: Netfilter Devel; +Cc: Kevin Darbyshire-Bryant

`payload_expr_complete` is called during netlink delinearization to fill
in missing fields in the payload expression.  However, the byte-order
was not being set.

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

diff --git a/src/payload.c b/src/payload.c
index f433c38421a4..e8fcd95d4bbe 100644
--- a/src/payload.c
+++ b/src/payload.c
@@ -857,6 +857,7 @@ void payload_expr_complete(struct expr *expr, const struct proto_ctx *ctx)
 			continue;
 
 		expr->dtype	   = tmpl->dtype;
+		expr->byteorder	   = tmpl->byteorder;
 		expr->payload.desc = desc;
 		expr->payload.tmpl = tmpl;
 		return;
-- 
2.35.1


^ permalink raw reply related	[flat|nested] 50+ messages in thread

* [nft PATCH v4 13/32] evaluate: support shifts larger than the width of the left operand
  2022-04-04 12:13 [nft PATCH v4 00/32] Extend values assignable to packet marks and payload fields Jeremy Sowden
                   ` (11 preceding siblings ...)
  2022-04-04 12:13 ` [nft PATCH v4 12/32] payload: set byte-order when completing expression Jeremy Sowden
@ 2022-04-04 12:13 ` Jeremy Sowden
  2022-05-23 17:42   ` Pablo Neira Ayuso
  2022-04-04 12:13 ` [nft PATCH v4 14/32] evaluate: relax type-checking for integer arguments in mark statements Jeremy Sowden
                   ` (19 subsequent siblings)
  32 siblings, 1 reply; 50+ messages in thread
From: Jeremy Sowden @ 2022-04-04 12:13 UTC (permalink / raw)
  To: Netfilter Devel; +Cc: Kevin Darbyshire-Bryant

If we want to left-shift a value of narrower type and assign the result
to a variable of a wider type, we are constrained to only shifting up to
the width of the narrower type.  Thus:

  add rule t c meta mark set ip dscp << 2

works, but:

  add rule t c meta mark set ip dscp << 8

does not, even though the lvalue is large enough to accommodate the
result.

Evaluation of the left-hand operand of a shift overwrites the `len`
field of the evaluation context when `expr_evaluate_primary` is called.
Instead, preserve the `len` value of the evaluation context for shifts,
and support shifts up to that size, even if they are larger than the
length of the left operand.

Update netlink_delinearize.c to handle the case where the length of a
shift expression does not match that of its left-hand operand.

Signed-off-by: Jeremy Sowden <jeremy@azazel.net>
---
 src/evaluate.c            | 23 ++++++++++++++---------
 src/netlink_delinearize.c |  4 ++--
 2 files changed, 16 insertions(+), 11 deletions(-)

diff --git a/src/evaluate.c b/src/evaluate.c
index be493f85010c..ee4da5a2b889 100644
--- a/src/evaluate.c
+++ b/src/evaluate.c
@@ -1116,14 +1116,18 @@ static int constant_binop_simplify(struct eval_ctx *ctx, struct expr **expr)
 static int expr_evaluate_shift(struct eval_ctx *ctx, struct expr **expr)
 {
 	struct expr *op = *expr, *left = op->left, *right = op->right;
+	unsigned int shift = mpz_get_uint32(right->value);
+	unsigned int op_len = left->len;
 
-	if (mpz_get_uint32(right->value) >= left->len)
-		return expr_binary_error(ctx->msgs, right, left,
-					 "%s shift of %u bits is undefined "
-					 "for type of %u bits width",
-					 op->op == OP_LSHIFT ? "Left" : "Right",
-					 mpz_get_uint32(right->value),
-					 left->len);
+	if (shift >= op_len) {
+		if (shift >= ctx->ectx.len)
+			return expr_binary_error(ctx->msgs, right, left,
+						 "%s shift of %u bits is undefined for type of %u bits width",
+						 op->op == OP_LSHIFT ? "Left" : "Right",
+						 shift,
+						 op_len);
+		op_len = ctx->ectx.len;
+	}
 
 	/* Both sides need to be in host byte order */
 	if (byteorder_conversion(ctx, &op->left, BYTEORDER_HOST_ENDIAN) < 0)
@@ -1134,7 +1138,7 @@ static int expr_evaluate_shift(struct eval_ctx *ctx, struct expr **expr)
 
 	op->dtype     = &integer_type;
 	op->byteorder = BYTEORDER_HOST_ENDIAN;
-	op->len       = left->len;
+	op->len	      = op_len;
 
 	if (expr_is_constant(left))
 		return constant_binop_simplify(ctx, expr);
@@ -1167,6 +1171,7 @@ static int expr_evaluate_binop(struct eval_ctx *ctx, struct expr **expr)
 {
 	struct expr *op = *expr, *left, *right;
 	const char *sym = expr_op_symbols[op->op];
+	unsigned int ectx_len = ctx->ectx.len;
 
 	if (expr_evaluate(ctx, &op->left) < 0)
 		return -1;
@@ -1174,7 +1179,7 @@ static int expr_evaluate_binop(struct eval_ctx *ctx, struct expr **expr)
 
 	if (op->op == OP_LSHIFT || op->op == OP_RSHIFT)
 		__expr_set_context(&ctx->ectx, &integer_type,
-				   left->byteorder, ctx->ectx.len, 0);
+				   left->byteorder, ectx_len, 0);
 	if (expr_evaluate(ctx, &op->right) < 0)
 		return -1;
 	right = op->right;
diff --git a/src/netlink_delinearize.c b/src/netlink_delinearize.c
index cf5359bf269e..9f6fdee3e92d 100644
--- a/src/netlink_delinearize.c
+++ b/src/netlink_delinearize.c
@@ -486,7 +486,7 @@ static struct expr *netlink_parse_bitwise_bool(struct netlink_parse_ctx *ctx,
 		mpz_ior(m, m, o);
 	}
 
-	if (left->len > 0 && mpz_scan0(m, 0) == left->len) {
+	if (left->len > 0 && mpz_scan0(m, 0) >= left->len) {
 		/* mask encompasses the entire value */
 		expr_free(mask);
 	} else {
@@ -536,7 +536,7 @@ static struct expr *netlink_parse_bitwise_shift(struct netlink_parse_ctx *ctx,
 	right->byteorder = BYTEORDER_HOST_ENDIAN;
 
 	expr = binop_expr_alloc(loc, op, left, right);
-	expr->len = left->len;
+	expr->len = nftnl_expr_get_u32(nle, NFTNL_EXPR_BITWISE_LEN) * BITS_PER_BYTE;
 
 	return expr;
 }
-- 
2.35.1


^ permalink raw reply related	[flat|nested] 50+ messages in thread

* [nft PATCH v4 14/32] evaluate: relax type-checking for integer arguments in mark statements
  2022-04-04 12:13 [nft PATCH v4 00/32] Extend values assignable to packet marks and payload fields Jeremy Sowden
                   ` (12 preceding siblings ...)
  2022-04-04 12:13 ` [nft PATCH v4 13/32] evaluate: support shifts larger than the width of the left operand Jeremy Sowden
@ 2022-04-04 12:13 ` Jeremy Sowden
  2022-05-23 17:33   ` Pablo Neira Ayuso
  2022-04-04 12:13 ` [nft PATCH v4 15/32] tests: shell: rename some test-cases Jeremy Sowden
                   ` (18 subsequent siblings)
  32 siblings, 1 reply; 50+ messages in thread
From: Jeremy Sowden @ 2022-04-04 12:13 UTC (permalink / raw)
  To: Netfilter Devel; +Cc: Kevin Darbyshire-Bryant

In order to be able to set ct and meta marks to values derived from
payload expressions, we need to relax the requirement that the type of
the statement argument must match that of the statement key.  Instead,
we require that the base-type of the argument is integer and that the
argument is small enough to fit.

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

diff --git a/src/evaluate.c b/src/evaluate.c
index ee4da5a2b889..f975dd197de3 100644
--- a/src/evaluate.c
+++ b/src/evaluate.c
@@ -2393,8 +2393,12 @@ static int __stmt_evaluate_arg(struct eval_ctx *ctx, struct stmt *stmt,
 					 "expression has type %s with length %d",
 					 dtype->desc, (*expr)->dtype->desc,
 					 (*expr)->len);
-	else if ((*expr)->dtype->type != TYPE_INTEGER &&
-		 !datatype_equal((*expr)->dtype, dtype))
+
+	if ((dtype->type == TYPE_MARK &&
+	     !datatype_equal(datatype_basetype(dtype), datatype_basetype((*expr)->dtype))) ||
+	    (dtype->type != TYPE_MARK &&
+	     (*expr)->dtype->type != TYPE_INTEGER &&
+	     !datatype_equal((*expr)->dtype, dtype)))
 		return stmt_binary_error(ctx, *expr, stmt,		/* verdict vs invalid? */
 					 "datatype mismatch: expected %s, "
 					 "expression has type %s",
-- 
2.35.1


^ permalink raw reply related	[flat|nested] 50+ messages in thread

* [nft PATCH v4 15/32] tests: shell: rename some test-cases
  2022-04-04 12:13 [nft PATCH v4 00/32] Extend values assignable to packet marks and payload fields Jeremy Sowden
                   ` (13 preceding siblings ...)
  2022-04-04 12:13 ` [nft PATCH v4 14/32] evaluate: relax type-checking for integer arguments in mark statements Jeremy Sowden
@ 2022-04-04 12:13 ` Jeremy Sowden
  2022-04-04 12:13 ` [nft PATCH v4 16/32] tests: shell: add test-cases for ct and packet mark payload expressions Jeremy Sowden
                   ` (17 subsequent siblings)
  32 siblings, 0 replies; 50+ messages in thread
From: Jeremy Sowden @ 2022-04-04 12:13 UTC (permalink / raw)
  To: Netfilter Devel; +Cc: Kevin Darbyshire-Bryant

The `0040mark_shift_?` tests are testing not just shifts, but binops
more generally, so name them accordingly.

Change the priorities of the chains to match the type.

Signed-off-by: Jeremy Sowden <jeremy@azazel.net>
---
 .../testcases/chains/{0040mark_shift_0 => 0040mark_binop_0}     | 2 +-
 .../testcases/chains/{0040mark_shift_1 => 0040mark_binop_1}     | 2 +-
 .../chains/dumps/{0040mark_shift_0.nft => 0040mark_binop_0.nft} | 2 +-
 .../chains/dumps/{0040mark_shift_1.nft => 0040mark_binop_1.nft} | 2 +-
 4 files changed, 4 insertions(+), 4 deletions(-)
 rename tests/shell/testcases/chains/{0040mark_shift_0 => 0040mark_binop_0} (68%)
 rename tests/shell/testcases/chains/{0040mark_shift_1 => 0040mark_binop_1} (70%)
 rename tests/shell/testcases/chains/dumps/{0040mark_shift_0.nft => 0040mark_binop_0.nft} (58%)
 rename tests/shell/testcases/chains/dumps/{0040mark_shift_1.nft => 0040mark_binop_1.nft} (64%)

diff --git a/tests/shell/testcases/chains/0040mark_shift_0 b/tests/shell/testcases/chains/0040mark_binop_0
similarity index 68%
rename from tests/shell/testcases/chains/0040mark_shift_0
rename to tests/shell/testcases/chains/0040mark_binop_0
index ef3dccfa049a..4280e33ac45a 100755
--- a/tests/shell/testcases/chains/0040mark_shift_0
+++ b/tests/shell/testcases/chains/0040mark_binop_0
@@ -4,7 +4,7 @@ set -e
 
 RULESET="
   add table t
-  add chain t c { type filter hook output priority mangle; }
+  add chain t c { type filter hook output priority filter; }
   add rule t c oif lo ct mark set (meta mark | 0x10) << 8
 "
 
diff --git a/tests/shell/testcases/chains/0040mark_shift_1 b/tests/shell/testcases/chains/0040mark_binop_1
similarity index 70%
rename from tests/shell/testcases/chains/0040mark_shift_1
rename to tests/shell/testcases/chains/0040mark_binop_1
index b609f5ef10ad..7e71f3eb43a8 100755
--- a/tests/shell/testcases/chains/0040mark_shift_1
+++ b/tests/shell/testcases/chains/0040mark_binop_1
@@ -4,7 +4,7 @@ set -e
 
 RULESET="
   add table t
-  add chain t c { type filter hook input priority mangle; }
+  add chain t c { type filter hook input priority filter; }
   add rule t c iif lo ct mark & 0xff 0x10 meta mark set ct mark >> 8
 "
 
diff --git a/tests/shell/testcases/chains/dumps/0040mark_shift_0.nft b/tests/shell/testcases/chains/dumps/0040mark_binop_0.nft
similarity index 58%
rename from tests/shell/testcases/chains/dumps/0040mark_shift_0.nft
rename to tests/shell/testcases/chains/dumps/0040mark_binop_0.nft
index 52d59d2c6da4..fc0a600a4dbe 100644
--- a/tests/shell/testcases/chains/dumps/0040mark_shift_0.nft
+++ b/tests/shell/testcases/chains/dumps/0040mark_binop_0.nft
@@ -1,6 +1,6 @@
 table ip t {
 	chain c {
-		type filter hook output priority mangle; policy accept;
+		type filter hook output priority filter; 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_binop_1.nft
similarity index 64%
rename from tests/shell/testcases/chains/dumps/0040mark_shift_1.nft
rename to tests/shell/testcases/chains/dumps/0040mark_binop_1.nft
index 56ec8dc766ca..dbaacefb93c7 100644
--- a/tests/shell/testcases/chains/dumps/0040mark_shift_1.nft
+++ b/tests/shell/testcases/chains/dumps/0040mark_binop_1.nft
@@ -1,6 +1,6 @@
 table ip t {
 	chain c {
-		type filter hook input priority mangle; policy accept;
+		type filter hook input priority filter; policy accept;
 		iif "lo" ct mark & 0x000000ff == 0x00000010 meta mark set ct mark >> 8
 	}
 }
-- 
2.35.1


^ permalink raw reply related	[flat|nested] 50+ messages in thread

* [nft PATCH v4 16/32] tests: shell: add test-cases for ct and packet mark payload expressions
  2022-04-04 12:13 [nft PATCH v4 00/32] Extend values assignable to packet marks and payload fields Jeremy Sowden
                   ` (14 preceding siblings ...)
  2022-04-04 12:13 ` [nft PATCH v4 15/32] tests: shell: rename some test-cases Jeremy Sowden
@ 2022-04-04 12:13 ` Jeremy Sowden
  2022-04-04 12:13 ` [nft PATCH v4 17/32] tests: py: " Jeremy Sowden
                   ` (16 subsequent siblings)
  32 siblings, 0 replies; 50+ messages in thread
From: Jeremy Sowden @ 2022-04-04 12:13 UTC (permalink / raw)
  To: Netfilter Devel; +Cc: Kevin Darbyshire-Bryant

Add new test-cases to verify that defining a rule that sets the ct or
packet mark to a value derived from a payload works correctly.

Signed-off-by: Jeremy Sowden <jeremy@azazel.net>
---
 tests/shell/testcases/chains/0040mark_binop_2         | 11 +++++++++++
 tests/shell/testcases/chains/0040mark_binop_3         | 11 +++++++++++
 tests/shell/testcases/chains/0040mark_binop_4         | 11 +++++++++++
 tests/shell/testcases/chains/0040mark_binop_5         | 11 +++++++++++
 tests/shell/testcases/chains/0040mark_binop_6         | 11 +++++++++++
 tests/shell/testcases/chains/0040mark_binop_7         | 11 +++++++++++
 tests/shell/testcases/chains/0040mark_binop_8         | 11 +++++++++++
 tests/shell/testcases/chains/0040mark_binop_9         | 11 +++++++++++
 .../shell/testcases/chains/dumps/0040mark_binop_2.nft |  6 ++++++
 .../shell/testcases/chains/dumps/0040mark_binop_3.nft |  6 ++++++
 .../shell/testcases/chains/dumps/0040mark_binop_4.nft |  6 ++++++
 .../shell/testcases/chains/dumps/0040mark_binop_5.nft |  6 ++++++
 .../shell/testcases/chains/dumps/0040mark_binop_6.nft |  6 ++++++
 .../shell/testcases/chains/dumps/0040mark_binop_7.nft |  6 ++++++
 .../shell/testcases/chains/dumps/0040mark_binop_8.nft |  6 ++++++
 .../shell/testcases/chains/dumps/0040mark_binop_9.nft |  6 ++++++
 16 files changed, 136 insertions(+)
 create mode 100755 tests/shell/testcases/chains/0040mark_binop_2
 create mode 100755 tests/shell/testcases/chains/0040mark_binop_3
 create mode 100755 tests/shell/testcases/chains/0040mark_binop_4
 create mode 100755 tests/shell/testcases/chains/0040mark_binop_5
 create mode 100755 tests/shell/testcases/chains/0040mark_binop_6
 create mode 100755 tests/shell/testcases/chains/0040mark_binop_7
 create mode 100755 tests/shell/testcases/chains/0040mark_binop_8
 create mode 100755 tests/shell/testcases/chains/0040mark_binop_9
 create mode 100644 tests/shell/testcases/chains/dumps/0040mark_binop_2.nft
 create mode 100644 tests/shell/testcases/chains/dumps/0040mark_binop_3.nft
 create mode 100644 tests/shell/testcases/chains/dumps/0040mark_binop_4.nft
 create mode 100644 tests/shell/testcases/chains/dumps/0040mark_binop_5.nft
 create mode 100644 tests/shell/testcases/chains/dumps/0040mark_binop_6.nft
 create mode 100644 tests/shell/testcases/chains/dumps/0040mark_binop_7.nft
 create mode 100644 tests/shell/testcases/chains/dumps/0040mark_binop_8.nft
 create mode 100644 tests/shell/testcases/chains/dumps/0040mark_binop_9.nft

diff --git a/tests/shell/testcases/chains/0040mark_binop_2 b/tests/shell/testcases/chains/0040mark_binop_2
new file mode 100755
index 000000000000..94ebe976c987
--- /dev/null
+++ b/tests/shell/testcases/chains/0040mark_binop_2
@@ -0,0 +1,11 @@
+#!/bin/bash
+
+set -e
+
+RULESET="
+  add table t
+  add chain t c { type filter hook output priority filter; }
+  add rule t c ct mark set ip dscp lshift 2 or 0x10
+"
+
+$NFT -f - <<< "$RULESET"
diff --git a/tests/shell/testcases/chains/0040mark_binop_3 b/tests/shell/testcases/chains/0040mark_binop_3
new file mode 100755
index 000000000000..b491565ca573
--- /dev/null
+++ b/tests/shell/testcases/chains/0040mark_binop_3
@@ -0,0 +1,11 @@
+#!/bin/bash
+
+set -e
+
+RULESET="
+  add table t
+  add chain t c { type filter hook input priority filter; }
+  add rule t c meta mark set ip dscp lshift 2 or 0x10
+"
+
+$NFT -f - <<< "$RULESET"
diff --git a/tests/shell/testcases/chains/0040mark_binop_4 b/tests/shell/testcases/chains/0040mark_binop_4
new file mode 100755
index 000000000000..adc5f25ba930
--- /dev/null
+++ b/tests/shell/testcases/chains/0040mark_binop_4
@@ -0,0 +1,11 @@
+#!/bin/bash
+
+set -e
+
+RULESET="
+  add table t
+  add chain t c { type filter hook output priority filter; }
+  add rule t c ct mark set ip dscp lshift 26 or 0x10
+"
+
+$NFT -f - <<< "$RULESET"
diff --git a/tests/shell/testcases/chains/0040mark_binop_5 b/tests/shell/testcases/chains/0040mark_binop_5
new file mode 100755
index 000000000000..286b7b1fc7f9
--- /dev/null
+++ b/tests/shell/testcases/chains/0040mark_binop_5
@@ -0,0 +1,11 @@
+#!/bin/bash
+
+set -e
+
+RULESET="
+  add table t
+  add chain t c { type filter hook input priority filter; }
+  add rule t c meta mark set ip dscp lshift 26 or 0x10
+"
+
+$NFT -f - <<< "$RULESET"
diff --git a/tests/shell/testcases/chains/0040mark_binop_6 b/tests/shell/testcases/chains/0040mark_binop_6
new file mode 100755
index 000000000000..9ea82952ef24
--- /dev/null
+++ b/tests/shell/testcases/chains/0040mark_binop_6
@@ -0,0 +1,11 @@
+#!/bin/bash
+
+set -e
+
+RULESET="
+  add table ip6 t
+  add chain ip6 t c { type filter hook output priority filter; }
+  add rule ip6 t c ct mark set ip6 dscp lshift 2 or 0x10
+"
+
+$NFT -f - <<< "$RULESET"
diff --git a/tests/shell/testcases/chains/0040mark_binop_7 b/tests/shell/testcases/chains/0040mark_binop_7
new file mode 100755
index 000000000000..ff9cfb55ac3e
--- /dev/null
+++ b/tests/shell/testcases/chains/0040mark_binop_7
@@ -0,0 +1,11 @@
+#!/bin/bash
+
+set -e
+
+RULESET="
+  add table ip6 t
+  add chain ip6 t c { type filter hook input priority filter; }
+  add rule ip6 t c meta mark set ip6 dscp lshift 2 or 0x10
+"
+
+$NFT -f - <<< "$RULESET"
diff --git a/tests/shell/testcases/chains/0040mark_binop_8 b/tests/shell/testcases/chains/0040mark_binop_8
new file mode 100755
index 000000000000..b348ee9367df
--- /dev/null
+++ b/tests/shell/testcases/chains/0040mark_binop_8
@@ -0,0 +1,11 @@
+#!/bin/bash
+
+set -e
+
+RULESET="
+  add table ip6 t
+  add chain ip6 t c { type filter hook output priority filter; }
+  add rule ip6 t c ct mark set ip6 dscp lshift 26 or 0x10
+"
+
+$NFT -f - <<< "$RULESET"
diff --git a/tests/shell/testcases/chains/0040mark_binop_9 b/tests/shell/testcases/chains/0040mark_binop_9
new file mode 100755
index 000000000000..d19447d42b22
--- /dev/null
+++ b/tests/shell/testcases/chains/0040mark_binop_9
@@ -0,0 +1,11 @@
+#!/bin/bash
+
+set -e
+
+RULESET="
+  add table ip6 t
+  add chain ip6 t c { type filter hook input priority filter; }
+  add rule ip6 t c meta mark set ip6 dscp lshift 26 or 0x10
+"
+
+$NFT -f - <<< "$RULESET"
diff --git a/tests/shell/testcases/chains/dumps/0040mark_binop_2.nft b/tests/shell/testcases/chains/dumps/0040mark_binop_2.nft
new file mode 100644
index 000000000000..7dc274f4e6a3
--- /dev/null
+++ b/tests/shell/testcases/chains/dumps/0040mark_binop_2.nft
@@ -0,0 +1,6 @@
+table ip t {
+	chain c {
+		type filter hook output priority filter; policy accept;
+		ct mark set ip dscp << 2 | 16
+	}
+}
diff --git a/tests/shell/testcases/chains/dumps/0040mark_binop_3.nft b/tests/shell/testcases/chains/dumps/0040mark_binop_3.nft
new file mode 100644
index 000000000000..c484f7a54948
--- /dev/null
+++ b/tests/shell/testcases/chains/dumps/0040mark_binop_3.nft
@@ -0,0 +1,6 @@
+table ip t {
+	chain c {
+		type filter hook input priority filter; policy accept;
+		meta mark set ip dscp << 2 | 16
+	}
+}
diff --git a/tests/shell/testcases/chains/dumps/0040mark_binop_4.nft b/tests/shell/testcases/chains/dumps/0040mark_binop_4.nft
new file mode 100644
index 000000000000..1bebea1683bc
--- /dev/null
+++ b/tests/shell/testcases/chains/dumps/0040mark_binop_4.nft
@@ -0,0 +1,6 @@
+table ip t {
+	chain c {
+		type filter hook output priority filter; policy accept;
+		ct mark set ip dscp << 26 | 16
+	}
+}
diff --git a/tests/shell/testcases/chains/dumps/0040mark_binop_5.nft b/tests/shell/testcases/chains/dumps/0040mark_binop_5.nft
new file mode 100644
index 000000000000..787c6cdd9231
--- /dev/null
+++ b/tests/shell/testcases/chains/dumps/0040mark_binop_5.nft
@@ -0,0 +1,6 @@
+table ip t {
+	chain c {
+		type filter hook input priority filter; policy accept;
+		meta mark set ip dscp << 26 | 16
+	}
+}
diff --git a/tests/shell/testcases/chains/dumps/0040mark_binop_6.nft b/tests/shell/testcases/chains/dumps/0040mark_binop_6.nft
new file mode 100644
index 000000000000..53940eaf2ea4
--- /dev/null
+++ b/tests/shell/testcases/chains/dumps/0040mark_binop_6.nft
@@ -0,0 +1,6 @@
+table ip6 t {
+	chain c {
+		type filter hook output priority filter; policy accept;
+		ct mark set ip6 dscp << 2 | 16
+	}
+}
diff --git a/tests/shell/testcases/chains/dumps/0040mark_binop_7.nft b/tests/shell/testcases/chains/dumps/0040mark_binop_7.nft
new file mode 100644
index 000000000000..35e12a0af66d
--- /dev/null
+++ b/tests/shell/testcases/chains/dumps/0040mark_binop_7.nft
@@ -0,0 +1,6 @@
+table ip6 t {
+	chain c {
+		type filter hook input priority filter; policy accept;
+		meta mark set ip6 dscp << 2 | 16
+	}
+}
diff --git a/tests/shell/testcases/chains/dumps/0040mark_binop_8.nft b/tests/shell/testcases/chains/dumps/0040mark_binop_8.nft
new file mode 100644
index 000000000000..f9f16c2491d4
--- /dev/null
+++ b/tests/shell/testcases/chains/dumps/0040mark_binop_8.nft
@@ -0,0 +1,6 @@
+table ip6 t {
+	chain c {
+		type filter hook output priority filter; policy accept;
+		ct mark set ip6 dscp << 26 | 16
+	}
+}
diff --git a/tests/shell/testcases/chains/dumps/0040mark_binop_9.nft b/tests/shell/testcases/chains/dumps/0040mark_binop_9.nft
new file mode 100644
index 000000000000..03c69c3f7cd2
--- /dev/null
+++ b/tests/shell/testcases/chains/dumps/0040mark_binop_9.nft
@@ -0,0 +1,6 @@
+table ip6 t {
+	chain c {
+		type filter hook input priority filter; policy accept;
+		meta mark set ip6 dscp << 26 | 16
+	}
+}
-- 
2.35.1


^ permalink raw reply related	[flat|nested] 50+ messages in thread

* [nft PATCH v4 17/32] tests: py: add test-cases for ct and packet mark payload expressions
  2022-04-04 12:13 [nft PATCH v4 00/32] Extend values assignable to packet marks and payload fields Jeremy Sowden
                   ` (15 preceding siblings ...)
  2022-04-04 12:13 ` [nft PATCH v4 16/32] tests: shell: add test-cases for ct and packet mark payload expressions Jeremy Sowden
@ 2022-04-04 12:13 ` Jeremy Sowden
  2022-04-04 12:13 ` [nft PATCH v4 18/32] include: add new bitwise boolean attributes to nf_tables.h Jeremy Sowden
                   ` (15 subsequent siblings)
  32 siblings, 0 replies; 50+ messages in thread
From: Jeremy Sowden @ 2022-04-04 12:13 UTC (permalink / raw)
  To: Netfilter Devel; +Cc: Kevin Darbyshire-Bryant

Add new test-cases to verify that defining a rule that sets the ct or
packet mark to a value derived from a payload works correctly.

Signed-off-by: Jeremy Sowden <jeremy@azazel.net>
---
 tests/py/ip/ct.t            |  2 ++
 tests/py/ip/ct.t.json       | 58 ++++++++++++++++++++++++++++++++++++
 tests/py/ip/ct.t.payload    | 18 +++++++++++
 tests/py/ip/meta.t          |  3 ++
 tests/py/ip/meta.t.json     | 59 +++++++++++++++++++++++++++++++++++++
 tests/py/ip/meta.t.payload  | 18 +++++++++++
 tests/py/ip6/ct.t           |  6 ++++
 tests/py/ip6/ct.t.json      | 58 ++++++++++++++++++++++++++++++++++++
 tests/py/ip6/ct.t.payload   | 17 +++++++++++
 tests/py/ip6/meta.t         |  3 ++
 tests/py/ip6/meta.t.json    | 58 ++++++++++++++++++++++++++++++++++++
 tests/py/ip6/meta.t.payload | 18 +++++++++++
 12 files changed, 318 insertions(+)
 create mode 100644 tests/py/ip6/ct.t
 create mode 100644 tests/py/ip6/ct.t.json
 create mode 100644 tests/py/ip6/ct.t.payload

diff --git a/tests/py/ip/ct.t b/tests/py/ip/ct.t
index a387863e0d8e..cfd9859c26b3 100644
--- a/tests/py/ip/ct.t
+++ b/tests/py/ip/ct.t
@@ -28,3 +28,5 @@ meta mark set ct original saddr . meta mark map { 1.1.1.1 . 0x00000014 : 0x00000
 meta mark set ct original ip saddr . meta mark map { 1.1.1.1 . 0x00000014 : 0x0000001e };ok
 ct original saddr . meta mark { 1.1.1.1 . 0x00000014 };fail
 ct original ip saddr . meta mark { 1.1.1.1 . 0x00000014 };ok
+ct mark set ip dscp lshift 2 or 0x10;ok;ct mark set ip dscp << 2 | 16
+ct mark set ip dscp lshift 26 or 0x10;ok;ct mark set ip dscp << 26 | 16
diff --git a/tests/py/ip/ct.t.json b/tests/py/ip/ct.t.json
index 3288413f8f3f..d0df36f1d060 100644
--- a/tests/py/ip/ct.t.json
+++ b/tests/py/ip/ct.t.json
@@ -325,3 +325,61 @@
         }
     }
 ]
+
+# ct mark set ip dscp lshift 2 or 0x10
+[
+    {
+        "mangle": {
+            "key": {
+                "ct": {
+                    "key": "mark"
+                }
+            },
+            "value": {
+                "|": [
+                    {
+                        "<<": [
+                            {
+                                "payload": {
+                                    "field": "dscp",
+                                    "protocol": "ip"
+                                }
+                            },
+                            2
+                        ]
+                    },
+                    16
+                ]
+            }
+        }
+    }
+]
+
+# ct mark set ip dscp lshift 26 or 0x10
+[
+    {
+        "mangle": {
+            "key": {
+                "ct": {
+                    "key": "mark"
+                }
+            },
+            "value": {
+                "|": [
+                    {
+                        "<<": [
+                            {
+                                "payload": {
+                                    "field": "dscp",
+                                    "protocol": "ip"
+                                }
+                            },
+                            26
+                        ]
+                    },
+                    16
+                ]
+            }
+        }
+    }
+]
diff --git a/tests/py/ip/ct.t.payload b/tests/py/ip/ct.t.payload
index 49f06a8401f5..b2aed170833e 100644
--- a/tests/py/ip/ct.t.payload
+++ b/tests/py/ip/ct.t.payload
@@ -84,3 +84,21 @@ ip
   [ ct load src_ip => reg 1 , dir original ]
   [ meta load mark => reg 9 ]
   [ lookup reg 1 set __set%d ]
+
+# ct mark set ip dscp lshift 2 or 0x10
+ip test-ip4 output
+  [ payload load 1b @ network header + 1 => reg 1 ]
+  [ bitwise reg 1 = ( reg 1 & 0x000000fc ) ^ 0x00000000 ]
+  [ bitwise reg 1 = ( reg 1 >> 0x00000002 ) ]
+  [ bitwise reg 1 = ( reg 1 << 0x00000002 ) ]
+  [ bitwise reg 1 = ( reg 1 & 0x000000ef ) ^ 0x00000010 ]
+  [ ct set mark with reg 1 ]
+
+# ct mark set ip dscp lshift 26 or 0x10
+ip
+  [ payload load 1b @ network header + 1 => reg 1 ]
+  [ bitwise reg 1 = ( reg 1 & 0x000000fc ) ^ 0x00000000 ]
+  [ bitwise reg 1 = ( reg 1 >> 0x00000002 ) ]
+  [ bitwise reg 1 = ( reg 1 << 0x0000001a ) ]
+  [ bitwise reg 1 = ( reg 1 & 0xffffffef ) ^ 0x00000010 ]
+  [ ct set mark with reg 1 ]
diff --git a/tests/py/ip/meta.t b/tests/py/ip/meta.t
index 5a05923a1ce1..3b6d82a0bc28 100644
--- a/tests/py/ip/meta.t
+++ b/tests/py/ip/meta.t
@@ -15,3 +15,6 @@ meta obrname "br0";fail
 
 meta sdif "lo" accept;ok
 meta sdifname != "vrf1" accept;ok
+
+meta mark set ip dscp lshift 2 or 0x10;ok;meta mark set ip dscp << 2 | 16
+meta mark set ip dscp lshift 26 or 0x10;ok;meta mark set ip dscp << 26 | 16
diff --git a/tests/py/ip/meta.t.json b/tests/py/ip/meta.t.json
index 3df31ce381fc..b82388dd31a8 100644
--- a/tests/py/ip/meta.t.json
+++ b/tests/py/ip/meta.t.json
@@ -156,3 +156,62 @@
         }
     }
 ]
+
+# meta mark set ip dscp lshift 2 or 0x10
+[
+    {
+        "mangle": {
+            "key": {
+                "meta": {
+                    "key": "mark"
+                }
+            },
+            "value": {
+                "|": [
+                    {
+                        "<<": [
+                            {
+                                "payload": {
+                                    "field": "dscp",
+                                    "protocol": "ip"
+                                }
+                            },
+                            2
+                        ]
+                    },
+                    16
+                ]
+            }
+        }
+    }
+]
+
+
+# meta mark set ip dscp lshift 26 or 0x10
+[
+    {
+        "mangle": {
+            "key": {
+                "meta": {
+                    "key": "mark"
+                }
+            },
+            "value": {
+                "|": [
+                    {
+                        "<<": [
+                            {
+                                "payload": {
+                                    "field": "dscp",
+                                    "protocol": "ip"
+                                }
+                            },
+                            26
+                        ]
+                    },
+                    16
+                ]
+            }
+        }
+    }
+]
diff --git a/tests/py/ip/meta.t.payload b/tests/py/ip/meta.t.payload
index afde5cc13ac5..49d8330272f6 100644
--- a/tests/py/ip/meta.t.payload
+++ b/tests/py/ip/meta.t.payload
@@ -51,3 +51,21 @@ ip test-ip4 input
   [ cmp eq reg 1 0x00000011 ]
   [ payload load 2b @ transport header + 2 => reg 1 ]
   [ cmp eq reg 1 0x00004300 ]
+
+# meta mark set ip dscp lshift 2 or 0x10
+ip test-ip4 input
+  [ payload load 1b @ network header + 1 => reg 1 ]
+  [ bitwise reg 1 = ( reg 1 & 0x000000fc ) ^ 0x00000000 ]
+  [ bitwise reg 1 = ( reg 1 >> 0x00000002 ) ]
+  [ bitwise reg 1 = ( reg 1 << 0x00000002 ) ]
+  [ bitwise reg 1 = ( reg 1 & 0x000000ef ) ^ 0x00000010 ]
+  [ meta set mark with reg 1 ]
+
+# meta mark set ip dscp lshift 26 or 0x10
+ip
+  [ payload load 1b @ network header + 1 => reg 1 ]
+  [ bitwise reg 1 = ( reg 1 & 0x000000fc ) ^ 0x00000000 ]
+  [ bitwise reg 1 = ( reg 1 >> 0x00000002 ) ]
+  [ bitwise reg 1 = ( reg 1 << 0x0000001a ) ]
+  [ bitwise reg 1 = ( reg 1 & 0xffffffef ) ^ 0x00000010 ]
+  [ meta set mark with reg 1 ]
diff --git a/tests/py/ip6/ct.t b/tests/py/ip6/ct.t
new file mode 100644
index 000000000000..0a141ffaf961
--- /dev/null
+++ b/tests/py/ip6/ct.t
@@ -0,0 +1,6 @@
+:output;type filter hook output priority 0
+
+*ip6;test-ip6;output
+
+ct mark set ip6 dscp lshift 2 or 0x10;ok;ct mark set ip6 dscp << 2 | 16
+ct mark set ip6 dscp lshift 26 or 0x10;ok;ct mark set ip6 dscp << 26 | 16
diff --git a/tests/py/ip6/ct.t.json b/tests/py/ip6/ct.t.json
new file mode 100644
index 000000000000..7739e131343e
--- /dev/null
+++ b/tests/py/ip6/ct.t.json
@@ -0,0 +1,58 @@
+# ct mark set ip6 dscp lshift 2 or 0x10
+[
+    {
+        "mangle": {
+            "key": {
+                "ct": {
+                    "key": "mark"
+                }
+            },
+            "value": {
+                "|": [
+                    {
+                        "<<": [
+                            {
+                                "payload": {
+                                    "field": "dscp",
+                                    "protocol": "ip6"
+                                }
+                            },
+                            2
+                        ]
+                    },
+                    16
+                ]
+            }
+        }
+    }
+]
+
+# ct mark set ip6 dscp lshift 26 or 0x10
+[
+    {
+        "mangle": {
+            "key": {
+                "ct": {
+                    "key": "mark"
+                }
+            },
+            "value": {
+                "|": [
+                    {
+                        "<<": [
+                            {
+                                "payload": {
+                                    "field": "dscp",
+                                    "protocol": "ip6"
+                                }
+                            },
+                            26
+                        ]
+                    },
+                    16
+                ]
+            }
+        }
+    }
+]
+
diff --git a/tests/py/ip6/ct.t.payload b/tests/py/ip6/ct.t.payload
new file mode 100644
index 000000000000..580c8d8d5712
--- /dev/null
+++ b/tests/py/ip6/ct.t.payload
@@ -0,0 +1,17 @@
+# ct mark set ip6 dscp lshift 2 or 0x10
+ip6 test-ip6 output
+  [ payload load 2b @ network header + 0 => reg 1 ]
+  [ bitwise reg 1 = ( reg 1 & 0x0000c00f ) ^ 0x00000000 ]
+  [ bitwise reg 1 = ( reg 1 >> 0x00000006 ) ]
+  [ bitwise reg 1 = ( reg 1 << 0x00000002 ) ]
+  [ bitwise reg 1 = ( reg 1 & 0x00000fef ) ^ 0x00000010 ]
+  [ ct set mark with reg 1 ]
+
+# ct mark set ip6 dscp lshift 26 or 0x10
+ip6 test-ip6 output
+  [ payload load 2b @ network header + 0 => reg 1 ]
+  [ bitwise reg 1 = ( reg 1 & 0x0000c00f ) ^ 0x00000000 ]
+  [ bitwise reg 1 = ( reg 1 >> 0x00000006 ) ]
+  [ bitwise reg 1 = ( reg 1 << 0x0000001a ) ]
+  [ bitwise reg 1 = ( reg 1 & 0xffffffef ) ^ 0x00000010 ]
+  [ ct set mark with reg 1 ]
diff --git a/tests/py/ip6/meta.t b/tests/py/ip6/meta.t
index 471e14811975..90d588580a43 100644
--- a/tests/py/ip6/meta.t
+++ b/tests/py/ip6/meta.t
@@ -14,3 +14,6 @@ meta protocol ip6 udp dport 67;ok;udp dport 67
 
 meta sdif "lo" accept;ok
 meta sdifname != "vrf1" accept;ok
+
+meta mark set ip6 dscp lshift 2 or 0x10;ok;meta mark set ip6 dscp << 2 | 16
+meta mark set ip6 dscp lshift 26 or 0x10;ok;meta mark set ip6 dscp << 26 | 16
diff --git a/tests/py/ip6/meta.t.json b/tests/py/ip6/meta.t.json
index 351320d70f7c..5bd8b07bbc90 100644
--- a/tests/py/ip6/meta.t.json
+++ b/tests/py/ip6/meta.t.json
@@ -194,3 +194,61 @@
         }
     }
 ]
+
+# meta mark set ip6 dscp lshift 2 or 0x10
+[
+    {
+        "mangle": {
+            "key": {
+                "meta": {
+                    "key": "mark"
+                }
+            },
+            "value": {
+                "|": [
+                    {
+                        "<<": [
+                            {
+                                "payload": {
+                                    "field": "dscp",
+                                    "protocol": "ip6"
+                                }
+                            },
+                            2
+                        ]
+                    },
+                    16
+                ]
+            }
+        }
+    }
+]
+
+# meta mark set ip6 dscp lshift 26 or 0x10
+[
+    {
+        "mangle": {
+            "key": {
+                "meta": {
+                    "key": "mark"
+                }
+            },
+            "value": {
+                "|": [
+                    {
+                        "<<": [
+                            {
+                                "payload": {
+                                    "field": "dscp",
+                                    "protocol": "ip6"
+                                }
+                            },
+                            26
+                        ]
+                    },
+                    16
+                ]
+            }
+        }
+    }
+]
diff --git a/tests/py/ip6/meta.t.payload b/tests/py/ip6/meta.t.payload
index 0e3db6ba07f9..49d7b42b0179 100644
--- a/tests/py/ip6/meta.t.payload
+++ b/tests/py/ip6/meta.t.payload
@@ -60,3 +60,21 @@ ip6 test-ip6 input
   [ cmp eq reg 1 0x00000011 ]
   [ payload load 2b @ transport header + 2 => reg 1 ]
   [ cmp eq reg 1 0x00004300 ]
+
+# meta mark set ip6 dscp lshift 2 or 0x10
+ip6 test-ip6 input
+  [ payload load 2b @ network header + 0 => reg 1 ]
+  [ bitwise reg 1 = ( reg 1 & 0x0000c00f ) ^ 0x00000000 ]
+  [ bitwise reg 1 = ( reg 1 >> 0x00000006 ) ]
+  [ bitwise reg 1 = ( reg 1 << 0x00000002 ) ]
+  [ bitwise reg 1 = ( reg 1 & 0x00000fef ) ^ 0x00000010 ]
+  [ meta set mark with reg 1 ]
+
+# meta mark set ip6 dscp lshift 26 or 0x10
+ip6 test-ip6 input
+  [ payload load 2b @ network header + 0 => reg 1 ]
+  [ bitwise reg 1 = ( reg 1 & 0x0000c00f ) ^ 0x00000000 ]
+  [ bitwise reg 1 = ( reg 1 >> 0x00000006 ) ]
+  [ bitwise reg 1 = ( reg 1 << 0x0000001a ) ]
+  [ bitwise reg 1 = ( reg 1 & 0xffffffef ) ^ 0x00000010 ]
+  [ meta set mark with reg 1 ]
-- 
2.35.1


^ permalink raw reply related	[flat|nested] 50+ messages in thread

* [nft PATCH v4 18/32] include: add new bitwise boolean attributes to nf_tables.h
  2022-04-04 12:13 [nft PATCH v4 00/32] Extend values assignable to packet marks and payload fields Jeremy Sowden
                   ` (16 preceding siblings ...)
  2022-04-04 12:13 ` [nft PATCH v4 17/32] tests: py: " Jeremy Sowden
@ 2022-04-04 12:13 ` Jeremy Sowden
  2022-04-04 12:13 ` [nft PATCH v4 19/32] evaluate: don't eval unary arguments Jeremy Sowden
                   ` (14 subsequent siblings)
  32 siblings, 0 replies; 50+ messages in thread
From: Jeremy Sowden @ 2022-04-04 12:13 UTC (permalink / raw)
  To: Netfilter Devel; +Cc: Kevin Darbyshire-Bryant

The kernel now has native support for AND, OR and XOR bitwise
operations.

Signed-off-by: Jeremy Sowden <jeremy@azazel.net>
---
 include/linux/netfilter/nf_tables.h | 19 ++++++++++++++++---
 1 file changed, 16 insertions(+), 3 deletions(-)

diff --git a/include/linux/netfilter/nf_tables.h b/include/linux/netfilter/nf_tables.h
index f3dcc4a34ff1..cd3e9e4ac646 100644
--- a/include/linux/netfilter/nf_tables.h
+++ b/include/linux/netfilter/nf_tables.h
@@ -539,16 +539,27 @@ enum nft_immediate_attributes {
 /**
  * enum nft_bitwise_ops - nf_tables bitwise operations
  *
- * @NFT_BITWISE_BOOL: mask-and-xor operation used to implement NOT, AND, OR and
- *                    XOR boolean operations
+ * @NFT_BITWISE_MASK_XOR: mask-and-xor operation used to implement NOT, AND, OR
+ *                        and XOR boolean operations
  * @NFT_BITWISE_LSHIFT: left-shift operation
  * @NFT_BITWISE_RSHIFT: right-shift operation
+ * @NFT_BITWISE_AND: and operation
+ * @NFT_BITWISE_OR: or operation
+ * @NFT_BITWISE_XOR: xor operation
  */
 enum nft_bitwise_ops {
-	NFT_BITWISE_BOOL,
+	NFT_BITWISE_MASK_XOR,
 	NFT_BITWISE_LSHIFT,
 	NFT_BITWISE_RSHIFT,
+	NFT_BITWISE_AND,
+	NFT_BITWISE_OR,
+	NFT_BITWISE_XOR,
 };
+/*
+ * Old name for NFT_BITWISE_MASK_XOR, predating the addition of NFT_BITWISE_AND,
+ * NFT_BITWISE_OR and NFT_BITWISE_XOR.  Retained for backwards-compatibility.
+ */
+#define NFT_BITWISE_BOOL NFT_BITWISE_MASK_XOR
 
 /**
  * enum nft_bitwise_attributes - nf_tables bitwise expression netlink attributes
@@ -562,6 +573,7 @@ enum nft_bitwise_ops {
  * @NFTA_BITWISE_DATA: argument for non-boolean operations
  *                     (NLA_NESTED: nft_data_attributes)
  * @NFTA_BITWISE_NBITS: length of operation in bits (NLA_U32)
+ * @NFTA_BITWISE_SREG2: second source register (NLA_U32: nft_registers)
  *
  * The bitwise expression supports boolean and shift operations.  It implements
  * the boolean operations by performing the following operation:
@@ -586,6 +598,7 @@ enum nft_bitwise_attributes {
 	NFTA_BITWISE_OP,
 	NFTA_BITWISE_DATA,
 	NFTA_BITWISE_NBITS,
+	NFTA_BITWISE_SREG2,
 	__NFTA_BITWISE_MAX
 };
 #define NFTA_BITWISE_MAX	(__NFTA_BITWISE_MAX - 1)
-- 
2.35.1


^ permalink raw reply related	[flat|nested] 50+ messages in thread

* [nft PATCH v4 19/32] evaluate: don't eval unary arguments
  2022-04-04 12:13 [nft PATCH v4 00/32] Extend values assignable to packet marks and payload fields Jeremy Sowden
                   ` (17 preceding siblings ...)
  2022-04-04 12:13 ` [nft PATCH v4 18/32] include: add new bitwise boolean attributes to nf_tables.h Jeremy Sowden
@ 2022-04-04 12:13 ` Jeremy Sowden
  2022-04-04 12:13 ` [nft PATCH v4 20/32] evaluate: prevent nested byte-order conversions Jeremy Sowden
                   ` (13 subsequent siblings)
  32 siblings, 0 replies; 50+ messages in thread
From: Jeremy Sowden @ 2022-04-04 12:13 UTC (permalink / raw)
  To: Netfilter Devel; +Cc: Kevin Darbyshire-Bryant

When a unary expression is inserted to implement a byte-order
conversion, the expression being converted has already been evaluated
and so `expr_evaluate_unary` doesn't need to do so.

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

diff --git a/src/evaluate.c b/src/evaluate.c
index f975dd197de3..1b252076e124 100644
--- a/src/evaluate.c
+++ b/src/evaluate.c
@@ -1025,13 +1025,9 @@ static int expr_evaluate_range(struct eval_ctx *ctx, struct expr **expr)
  */
 static int expr_evaluate_unary(struct eval_ctx *ctx, struct expr **expr)
 {
-	struct expr *unary = *expr, *arg;
+	struct expr *unary = *expr, *arg = unary->arg;
 	enum byteorder byteorder;
 
-	if (expr_evaluate(ctx, &unary->arg) < 0)
-		return -1;
-	arg = unary->arg;
-
 	assert(!expr_is_constant(arg));
 	assert(expr_basetype(arg)->type == TYPE_INTEGER);
 	assert(arg->etype != EXPR_UNARY);
-- 
2.35.1


^ permalink raw reply related	[flat|nested] 50+ messages in thread

* [nft PATCH v4 20/32] evaluate: prevent nested byte-order conversions
  2022-04-04 12:13 [nft PATCH v4 00/32] Extend values assignable to packet marks and payload fields Jeremy Sowden
                   ` (18 preceding siblings ...)
  2022-04-04 12:13 ` [nft PATCH v4 19/32] evaluate: don't eval unary arguments Jeremy Sowden
@ 2022-04-04 12:13 ` Jeremy Sowden
  2022-04-04 12:13 ` [nft PATCH v4 21/32] evaluate: don't clobber binop lengths Jeremy Sowden
                   ` (12 subsequent siblings)
  32 siblings, 0 replies; 50+ messages in thread
From: Jeremy Sowden @ 2022-04-04 12:13 UTC (permalink / raw)
  To: Netfilter Devel; +Cc: Kevin Darbyshire-Bryant

There is a an assertion in `expr_evaluate_unary` that checks that the
operand of the unary operation is not itself a unary expression.  Add a
check to `byteorder_conversion` to ensure that this is the case by
removing an existing unary operation, rather than adding a second one.

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

diff --git a/src/evaluate.c b/src/evaluate.c
index 1b252076e124..3f697eb1dd43 100644
--- a/src/evaluate.c
+++ b/src/evaluate.c
@@ -144,6 +144,14 @@ static int byteorder_conversion(struct eval_ctx *ctx, struct expr **expr,
 	if ((*expr)->etype == EXPR_CONCAT)
 		return 0;
 
+	/* Remove existing conversion */
+	if ((*expr)->etype == EXPR_UNARY) {
+		struct expr *unary = *expr;
+		*expr = expr_get((*expr)->arg);
+		expr_free(unary);
+		return 0;
+	}
+
 	if (expr_basetype(*expr)->type != TYPE_INTEGER)
 		return expr_error(ctx->msgs, *expr,
 			 	  "Byteorder mismatch: expected %s, got %s",
-- 
2.35.1


^ permalink raw reply related	[flat|nested] 50+ messages in thread

* [nft PATCH v4 21/32] evaluate: don't clobber binop lengths
  2022-04-04 12:13 [nft PATCH v4 00/32] Extend values assignable to packet marks and payload fields Jeremy Sowden
                   ` (19 preceding siblings ...)
  2022-04-04 12:13 ` [nft PATCH v4 20/32] evaluate: prevent nested byte-order conversions Jeremy Sowden
@ 2022-04-04 12:13 ` Jeremy Sowden
  2022-04-04 12:14 ` [nft PATCH v4 22/32] evaluate: insert byte-order conversions for expressions between 9 and 15 bits Jeremy Sowden
                   ` (11 subsequent siblings)
  32 siblings, 0 replies; 50+ messages in thread
From: Jeremy Sowden @ 2022-04-04 12:13 UTC (permalink / raw)
  To: Netfilter Devel; +Cc: Kevin Darbyshire-Bryant

Binops with variable RHS operands will make it possible to do thing like
this:

  nft add rule t c ip dscp set ip dscp and 0xc

However, the netlink dump reveals a problem:

  [ payload load 2b @ network header + 0 => reg 1 ]
  [ bitwise reg 1 = ( reg 1 & 0x000003ff ) ^ 0x00000000 ]
  [ payload load 1b @ network header + 1 => reg 2 ]
  [ bitwise reg 2 = ( reg 2 & 0x0000003c ) ^ 0x00000000 ]
  [ bitwise reg 2 = ( reg 2 >> 0x00000002 ) ]
  [ bitwise reg 2 = ( reg 2 & 0x0000000c ) ^ 0x00000000 ]
  [ bitwise reg 2 = ( reg 2 << 0x00000002 ) ]
  [ bitwise reg 1 = ( reg 1 ^ reg 2 ) ]
  [ payload write reg 1 => 2b @ network header + 0 csum_type 1 csum_off 10 csum_flags 0x0 ]

The mask at line 4 should be 0xfc, not 0x3c.

Evaluation of the payload expression munges it from `ip dscp` to
`(ip dscp & 0xfc) >> 2`, because although `ip dscp` is only 6 bits long,
those 6 bits are the top bits in a byte, and to make the arithmetic
simpler when we perform comparisons and assignments, we mask and shift
the field.  When the AND expression is allocated, its length is
correctly set to 8.  However, when a binop is evaluated, it is assumed
that the length has not been set and it always set to the length of the
left operand, incorrectly to 6 in this case.  When the bitwise netlink
expression is generated, the length of the AND is used to generate the
mask, 0x3f, used in combining the binop's.  The upshot of this is that
the original mask gets mangled to 0x3c.

We can fix this by changing the evaluation of binops only to set the
op's length if it is not already set.

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

diff --git a/src/evaluate.c b/src/evaluate.c
index 3f697eb1dd43..e19f6300fe2c 100644
--- a/src/evaluate.c
+++ b/src/evaluate.c
@@ -1121,7 +1121,7 @@ static int expr_evaluate_shift(struct eval_ctx *ctx, struct expr **expr)
 {
 	struct expr *op = *expr, *left = op->left, *right = op->right;
 	unsigned int shift = mpz_get_uint32(right->value);
-	unsigned int op_len = left->len;
+	unsigned int op_len = op->len ? : left->len;
 
 	if (shift >= op_len) {
 		if (shift >= ctx->ectx.len)
@@ -1158,7 +1158,7 @@ static int expr_evaluate_bitwise(struct eval_ctx *ctx, struct expr **expr)
 
 	op->dtype     = left->dtype;
 	op->byteorder = left->byteorder;
-	op->len	      = left->len;
+	op->len	      = op->len ? : left->len;
 
 	if (expr_is_constant(left))
 		return constant_binop_simplify(ctx, expr);
-- 
2.35.1


^ permalink raw reply related	[flat|nested] 50+ messages in thread

* [nft PATCH v4 22/32] evaluate: insert byte-order conversions for expressions between 9 and 15 bits
  2022-04-04 12:13 [nft PATCH v4 00/32] Extend values assignable to packet marks and payload fields Jeremy Sowden
                   ` (20 preceding siblings ...)
  2022-04-04 12:13 ` [nft PATCH v4 21/32] evaluate: don't clobber binop lengths Jeremy Sowden
@ 2022-04-04 12:14 ` Jeremy Sowden
  2022-04-04 12:14 ` [nft PATCH v4 23/32] evaluate: set eval context to leftmost bitwise operand Jeremy Sowden
                   ` (10 subsequent siblings)
  32 siblings, 0 replies; 50+ messages in thread
From: Jeremy Sowden @ 2022-04-04 12:14 UTC (permalink / raw)
  To: Netfilter Devel; +Cc: Kevin Darbyshire-Bryant

Round up expression lengths when determining whether to insert a
byte-order conversion.  For example, if one is masking a network header
which spans a byte boundary, the mask will span two bytes and so it will
need to be in NBO.

Fixes: bb03cbcd18a1 ("evaluate: no need to swap byte-order for values of fewer than 16 bits.")
Signed-off-by: Jeremy Sowden <jeremy@azazel.net>
---
 src/evaluate.c              | 2 +-
 tests/py/ip6/ct.t.payload   | 2 ++
 tests/py/ip6/meta.t.payload | 2 ++
 3 files changed, 5 insertions(+), 1 deletion(-)

diff --git a/src/evaluate.c b/src/evaluate.c
index e19f6300fe2c..6b1e295d216a 100644
--- a/src/evaluate.c
+++ b/src/evaluate.c
@@ -158,7 +158,7 @@ static int byteorder_conversion(struct eval_ctx *ctx, struct expr **expr,
 				  byteorder_names[byteorder],
 				  byteorder_names[(*expr)->byteorder]);
 
-	if (expr_is_constant(*expr) || (*expr)->len / BITS_PER_BYTE < 2)
+	if (expr_is_constant(*expr) || div_round_up((*expr)->len, BITS_PER_BYTE) < 2)
 		(*expr)->byteorder = byteorder;
 	else {
 		op = byteorder_conversion_op(*expr, byteorder);
diff --git a/tests/py/ip6/ct.t.payload b/tests/py/ip6/ct.t.payload
index 580c8d8d5712..a0565d14e15e 100644
--- a/tests/py/ip6/ct.t.payload
+++ b/tests/py/ip6/ct.t.payload
@@ -3,6 +3,7 @@ ip6 test-ip6 output
   [ payload load 2b @ network header + 0 => reg 1 ]
   [ bitwise reg 1 = ( reg 1 & 0x0000c00f ) ^ 0x00000000 ]
   [ bitwise reg 1 = ( reg 1 >> 0x00000006 ) ]
+  [ byteorder reg 1 = ntoh(reg 1, 2, 1) ]
   [ bitwise reg 1 = ( reg 1 << 0x00000002 ) ]
   [ bitwise reg 1 = ( reg 1 & 0x00000fef ) ^ 0x00000010 ]
   [ ct set mark with reg 1 ]
@@ -12,6 +13,7 @@ ip6 test-ip6 output
   [ payload load 2b @ network header + 0 => reg 1 ]
   [ bitwise reg 1 = ( reg 1 & 0x0000c00f ) ^ 0x00000000 ]
   [ bitwise reg 1 = ( reg 1 >> 0x00000006 ) ]
+  [ byteorder reg 1 = ntoh(reg 1, 2, 1) ]
   [ bitwise reg 1 = ( reg 1 << 0x0000001a ) ]
   [ bitwise reg 1 = ( reg 1 & 0xffffffef ) ^ 0x00000010 ]
   [ ct set mark with reg 1 ]
diff --git a/tests/py/ip6/meta.t.payload b/tests/py/ip6/meta.t.payload
index 49d7b42b0179..3cb0a587a5e7 100644
--- a/tests/py/ip6/meta.t.payload
+++ b/tests/py/ip6/meta.t.payload
@@ -66,6 +66,7 @@ ip6 test-ip6 input
   [ payload load 2b @ network header + 0 => reg 1 ]
   [ bitwise reg 1 = ( reg 1 & 0x0000c00f ) ^ 0x00000000 ]
   [ bitwise reg 1 = ( reg 1 >> 0x00000006 ) ]
+  [ byteorder reg 1 = ntoh(reg 1, 2, 1) ]
   [ bitwise reg 1 = ( reg 1 << 0x00000002 ) ]
   [ bitwise reg 1 = ( reg 1 & 0x00000fef ) ^ 0x00000010 ]
   [ meta set mark with reg 1 ]
@@ -75,6 +76,7 @@ ip6 test-ip6 input
   [ payload load 2b @ network header + 0 => reg 1 ]
   [ bitwise reg 1 = ( reg 1 & 0x0000c00f ) ^ 0x00000000 ]
   [ bitwise reg 1 = ( reg 1 >> 0x00000006 ) ]
+  [ byteorder reg 1 = ntoh(reg 1, 2, 1) ]
   [ bitwise reg 1 = ( reg 1 << 0x0000001a ) ]
   [ bitwise reg 1 = ( reg 1 & 0xffffffef ) ^ 0x00000010 ]
   [ meta set mark with reg 1 ]
-- 
2.35.1


^ permalink raw reply related	[flat|nested] 50+ messages in thread

* [nft PATCH v4 23/32] evaluate: set eval context to leftmost bitwise operand
  2022-04-04 12:13 [nft PATCH v4 00/32] Extend values assignable to packet marks and payload fields Jeremy Sowden
                   ` (21 preceding siblings ...)
  2022-04-04 12:14 ` [nft PATCH v4 22/32] evaluate: insert byte-order conversions for expressions between 9 and 15 bits Jeremy Sowden
@ 2022-04-04 12:14 ` Jeremy Sowden
  2022-04-04 12:14 ` [nft PATCH v4 24/32] netlink_delinearize: fix typo Jeremy Sowden
                   ` (9 subsequent siblings)
  32 siblings, 0 replies; 50+ messages in thread
From: Jeremy Sowden @ 2022-04-04 12:14 UTC (permalink / raw)
  To: Netfilter Devel; +Cc: Kevin Darbyshire-Bryant

A bitwise expression currently derives its type and size from its
left operand.  Thus:

  ct mark & 0xff

has type `mark` and size `32`.

However, currently, something like:

  ct mark | ip dscp | 0x200

will fail because, although evaluation is left-associative, and
therefore this expression will be evaluated as:

 (ct mark | ip dscp) | 0x200

after the evaluation of `ct mark | ip dscp`, the evaluation context
contains the size and data-type of the `ip dscp` expression and so
`0x200` is out of range.

Instead, reset the evaluation context to the values from the left-hand
operand once both operands have been evaluated.

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

diff --git a/src/evaluate.c b/src/evaluate.c
index 6b1e295d216a..02bfde2a2ded 100644
--- a/src/evaluate.c
+++ b/src/evaluate.c
@@ -1153,6 +1153,8 @@ static int expr_evaluate_bitwise(struct eval_ctx *ctx, struct expr **expr)
 {
 	struct expr *op = *expr, *left = op->left;
 
+	expr_evaluate_primary(ctx, &left);
+
 	if (byteorder_conversion(ctx, &op->right, left->byteorder) < 0)
 		return -1;
 
-- 
2.35.1


^ permalink raw reply related	[flat|nested] 50+ messages in thread

* [nft PATCH v4 24/32] netlink_delinearize: fix typo
  2022-04-04 12:13 [nft PATCH v4 00/32] Extend values assignable to packet marks and payload fields Jeremy Sowden
                   ` (22 preceding siblings ...)
  2022-04-04 12:14 ` [nft PATCH v4 23/32] evaluate: set eval context to leftmost bitwise operand Jeremy Sowden
@ 2022-04-04 12:14 ` Jeremy Sowden
  2022-04-04 12:14 ` [nft PATCH v4 25/32] netlink_delinearize: refactor stmt_payload_binop_postprocess Jeremy Sowden
                   ` (8 subsequent siblings)
  32 siblings, 0 replies; 50+ messages in thread
From: Jeremy Sowden @ 2022-04-04 12:14 UTC (permalink / raw)
  To: Netfilter Devel; +Cc: Kevin Darbyshire-Bryant

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

diff --git a/src/netlink_delinearize.c b/src/netlink_delinearize.c
index 9f6fdee3e92d..8f19594a1633 100644
--- a/src/netlink_delinearize.c
+++ b/src/netlink_delinearize.c
@@ -2878,7 +2878,7 @@ static void stmt_payload_binop_pp(struct rule_pp_ctx *ctx, struct expr *binop)
  * a binop expression with a munged payload expression on the left
  * and a mask to clear the real payload offset/length.
  *
- * So chech if we have one of the following binops:
+ * So check if we have one of the following binops:
  * I)
  *           binop (|)
  *       binop(&)   value/set
-- 
2.35.1


^ permalink raw reply related	[flat|nested] 50+ messages in thread

* [nft PATCH v4 25/32] netlink_delinearize: refactor stmt_payload_binop_postprocess
  2022-04-04 12:13 [nft PATCH v4 00/32] Extend values assignable to packet marks and payload fields Jeremy Sowden
                   ` (23 preceding siblings ...)
  2022-04-04 12:14 ` [nft PATCH v4 24/32] netlink_delinearize: fix typo Jeremy Sowden
@ 2022-04-04 12:14 ` Jeremy Sowden
  2022-04-04 12:14 ` [nft PATCH v4 26/32] netlink_delinearize: add support for processing variable payload statement arguments Jeremy Sowden
                   ` (7 subsequent siblings)
  32 siblings, 0 replies; 50+ messages in thread
From: Jeremy Sowden @ 2022-04-04 12:14 UTC (permalink / raw)
  To: Netfilter Devel; +Cc: Kevin Darbyshire-Bryant

We are about to add support for a new payload binop that needs to be
post-processed, so move the contents of the two major cases (I and II)
into separate functions to keep the function size reasonable.

Signed-off-by: Jeremy Sowden <jeremy@azazel.net>
---
 src/netlink_delinearize.c | 191 +++++++++++++++++++++-----------------
 1 file changed, 108 insertions(+), 83 deletions(-)

diff --git a/src/netlink_delinearize.c b/src/netlink_delinearize.c
index 8f19594a1633..4036646d57ac 100644
--- a/src/netlink_delinearize.c
+++ b/src/netlink_delinearize.c
@@ -2862,6 +2862,110 @@ static void stmt_payload_binop_pp(struct rule_pp_ctx *ctx, struct expr *binop)
 	}
 }
 
+static bool stmt_payload_binop_postprocess_i(struct rule_pp_ctx *ctx)
+{
+	struct expr *expr, *binop, *payload, *value, *mask;
+	struct stmt *stmt = ctx->stmt;
+	mpz_t tmp, bitmask;
+
+	expr = stmt->payload.val;
+
+	if (expr->op != OP_OR)
+		return false;
+
+	value = expr->right;
+	if (value->etype != EXPR_VALUE)
+		return false;
+
+	binop = expr->left;
+	if (binop->op != OP_AND)
+		return false;
+
+	payload = binop->left;
+	if (payload->etype != EXPR_PAYLOAD)
+		return false;
+
+	if (!payload_expr_cmp(stmt->payload.expr, payload))
+		return false;
+
+	mask = binop->right;
+	if (mask->etype != EXPR_VALUE)
+		return false;
+
+	mpz_init(tmp);
+	mpz_set(tmp, mask->value);
+
+	mpz_init_bitmask(bitmask, payload->len);
+	mpz_xor(bitmask, bitmask, mask->value);
+	mpz_xor(bitmask, bitmask, value->value);
+	mpz_set(mask->value, bitmask);
+	mpz_clear(bitmask);
+
+	binop_postprocess(ctx, expr, &expr->left);
+	if (!payload_is_known(payload))
+		mpz_set(mask->value, tmp);
+	else {
+		expr_free(stmt->payload.expr);
+		stmt->payload.expr = expr_get(payload);
+		stmt->payload.val = expr_get(expr->right);
+		expr_free(expr);
+	}
+
+	mpz_clear(tmp);
+
+	return true;
+}
+
+static bool stmt_payload_binop_postprocess_ii(struct rule_pp_ctx *ctx)
+{
+	struct expr *expr, *payload, *value;
+	struct stmt *stmt = ctx->stmt;
+	mpz_t bitmask;
+
+	expr = stmt->payload.val;
+
+	value = expr->right;
+	if (value->etype != EXPR_VALUE)
+		return false;
+
+	switch (expr->op) {
+	case OP_AND: /* IIa */
+		payload = expr->left;
+		mpz_init_bitmask(bitmask, payload->len);
+		mpz_xor(bitmask, bitmask, value->value);
+		mpz_set(value->value, bitmask);
+		mpz_clear(bitmask);
+		break;
+	case OP_OR: /* IIb */
+		break;
+	default: /* No idea */
+		return false;
+	}
+
+	stmt_payload_binop_pp(ctx, expr);
+	if (!payload_is_known(expr->left))
+		return false;
+
+	expr_free(stmt->payload.expr);
+
+	switch (expr->op) {
+	case OP_AND:
+		/* Mask was used to match payload, i.e.
+		 * user asked to set zero value.
+		 */
+		mpz_set_ui(value->value, 0);
+		break;
+	default:
+		break;
+	}
+
+	stmt->payload.expr = expr_get(expr->left);
+	stmt->payload.val = expr_get(expr->right);
+	expr_free(expr);
+
+	return true;
+}
+
 /**
  * stmt_payload_binop_postprocess - decode payload set binop
  *
@@ -2906,9 +3010,8 @@ static void stmt_payload_binop_pp(struct rule_pp_ctx *ctx, struct expr *binop)
  */
 static void stmt_payload_binop_postprocess(struct rule_pp_ctx *ctx)
 {
-	struct expr *expr, *binop, *payload, *value, *mask;
+	struct expr *expr;
 	struct stmt *stmt = ctx->stmt;
-	mpz_t bitmask;
 
 	expr = stmt->payload.val;
 
@@ -2916,93 +3019,15 @@ static void stmt_payload_binop_postprocess(struct rule_pp_ctx *ctx)
 		return;
 
 	switch (expr->left->etype) {
-	case EXPR_BINOP: {/* I? */
-		mpz_t tmp;
-
-		if (expr->op != OP_OR)
-			return;
-
-		value = expr->right;
-		if (value->etype != EXPR_VALUE)
-			return;
-
-		binop = expr->left;
-		if (binop->op != OP_AND)
-			return;
-
-		payload = binop->left;
-		if (payload->etype != EXPR_PAYLOAD)
-			return;
-
-		if (!payload_expr_cmp(stmt->payload.expr, payload))
+	case EXPR_BINOP: /* I? */
+		if (stmt_payload_binop_postprocess_i(ctx))
 			return;
 
-		mask = binop->right;
-		if (mask->etype != EXPR_VALUE)
-			return;
-
-		mpz_init(tmp);
-		mpz_set(tmp, mask->value);
-
-		mpz_init_bitmask(bitmask, payload->len);
-		mpz_xor(bitmask, bitmask, mask->value);
-		mpz_xor(bitmask, bitmask, value->value);
-		mpz_set(mask->value, bitmask);
-		mpz_clear(bitmask);
-
-		binop_postprocess(ctx, expr, &expr->left);
-		if (!payload_is_known(payload)) {
-			mpz_set(mask->value, tmp);
-			mpz_clear(tmp);
-			return;
-		}
-
-		mpz_clear(tmp);
-		expr_free(stmt->payload.expr);
-		stmt->payload.expr = expr_get(payload);
-		stmt->payload.val = expr_get(expr->right);
-		expr_free(expr);
 		break;
-	}
 	case EXPR_PAYLOAD: /* II? */
-		value = expr->right;
-		if (value->etype != EXPR_VALUE)
+		if (stmt_payload_binop_postprocess_ii(ctx))
 			return;
 
-		switch (expr->op) {
-		case OP_AND: /* IIa */
-			payload = expr->left;
-			mpz_init_bitmask(bitmask, payload->len);
-			mpz_xor(bitmask, bitmask, value->value);
-			mpz_set(value->value, bitmask);
-			mpz_clear(bitmask);
-			break;
-		case OP_OR: /* IIb */
-			break;
-		default: /* No idea */
-			return;
-		}
-
-		stmt_payload_binop_pp(ctx, expr);
-		if (!payload_is_known(expr->left))
-			return;
-
-		expr_free(stmt->payload.expr);
-
-		switch (expr->op) {
-		case OP_AND:
-			/* Mask was used to match payload, i.e.
-			 * user asked to set zero value.
-			 */
-			mpz_set_ui(value->value, 0);
-			break;
-		default:
-			break;
-		}
-
-		stmt->payload.expr = expr_get(expr->left);
-		stmt->payload.val = expr_get(expr->right);
-		expr_free(expr);
 		break;
 	default: /* No idea */
 		break;
-- 
2.35.1


^ permalink raw reply related	[flat|nested] 50+ messages in thread

* [nft PATCH v4 26/32] netlink_delinearize: add support for processing variable payload statement arguments
  2022-04-04 12:13 [nft PATCH v4 00/32] Extend values assignable to packet marks and payload fields Jeremy Sowden
                   ` (24 preceding siblings ...)
  2022-04-04 12:14 ` [nft PATCH v4 25/32] netlink_delinearize: refactor stmt_payload_binop_postprocess Jeremy Sowden
@ 2022-04-04 12:14 ` Jeremy Sowden
  2022-04-04 12:14 ` [nft PATCH v4 27/32] netlink: rename bitwise operation functions Jeremy Sowden
                   ` (6 subsequent siblings)
  32 siblings, 0 replies; 50+ messages in thread
From: Jeremy Sowden @ 2022-04-04 12:14 UTC (permalink / raw)
  To: Netfilter Devel; +Cc: Kevin Darbyshire-Bryant

If a user uses a variable payload expression in a payload statement, the
structure of the statement value is not handled by the existing
statement postprocessing function, so we need to extend it.

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

diff --git a/src/netlink_delinearize.c b/src/netlink_delinearize.c
index 4036646d57ac..e7042d6ae940 100644
--- a/src/netlink_delinearize.c
+++ b/src/netlink_delinearize.c
@@ -2862,7 +2862,7 @@ static void stmt_payload_binop_pp(struct rule_pp_ctx *ctx, struct expr *binop)
 	}
 }
 
-static bool stmt_payload_binop_postprocess_i(struct rule_pp_ctx *ctx)
+static bool stmt_payload_binop_postprocess_i_a(struct rule_pp_ctx *ctx)
 {
 	struct expr *expr, *binop, *payload, *value, *mask;
 	struct stmt *stmt = ctx->stmt;
@@ -2916,6 +2916,67 @@ static bool stmt_payload_binop_postprocess_i(struct rule_pp_ctx *ctx)
 	return true;
 }
 
+static bool stmt_payload_binop_postprocess_i_b(struct rule_pp_ctx *ctx)
+{
+	struct expr *expr, *payload, *mask, *xor;
+	struct stmt *stmt = ctx->stmt;
+	unsigned int shift;
+	mpz_t tmp, bitmask;
+
+	expr = stmt->payload.val;
+
+	if (expr->op != OP_XOR)
+		return false;
+
+	if (expr->left->etype != EXPR_BINOP)
+		return false;
+
+	if (expr->left->op != OP_AND)
+		return false;
+
+	if (expr->right->etype == EXPR_UNARY) {
+		/*
+		 * If the payload value was originally in a different byte-order
+		 * from the payload expression, there will be a byte-order
+		 * conversion to remove.
+		 */
+		xor = expr_get(expr->right->arg);
+		expr_free(expr->right);
+		expr->right = xor;
+	} else
+		xor = expr->right;
+
+	mask    = expr->left->right;
+	payload = expr->left->left;
+
+	mpz_init(tmp);
+	mpz_set(tmp, mask->value);
+
+	mpz_init_bitmask(bitmask, payload->len);
+	mpz_xor(bitmask, bitmask, mask->value);
+	mpz_set(mask->value, bitmask);
+	mpz_clear(bitmask);
+
+	if (payload_expr_trim(payload, mask, &ctx->pctx, &shift))
+		payload_match_postprocess(ctx, expr->left, payload);
+
+	if (!payload_is_known(payload)) {
+		mpz_set(mask->value, tmp);
+	} else {
+		if (shift) {
+			expr->right = expr_get(xor->left);
+			expr_free(xor);
+		}
+		expr_free(stmt->payload.expr);
+		stmt->payload.expr = expr_get(payload);
+		stmt->payload.val = expr_get(expr->right);
+		expr_free(expr);
+	}
+
+	mpz_clear(tmp);
+	return true;
+}
+
 static bool stmt_payload_binop_postprocess_ii(struct rule_pp_ctx *ctx)
 {
 	struct expr *expr, *payload, *value;
@@ -2983,21 +3044,30 @@ static bool stmt_payload_binop_postprocess_ii(struct rule_pp_ctx *ctx)
  * and a mask to clear the real payload offset/length.
  *
  * So check if we have one of the following binops:
- * I)
+ *
+ * Ia)
  *           binop (|)
  *       binop(&)   value/set
  * payload   value(mask)
  *
- * This is the normal case, the | RHS is the value the user wants
- * to set, the & RHS is the mask value that discards bits we need
+ * This is the normal constant case, the | RHS is the value the user
+ * wants to set, the & RHS is the mask value that discards bits we need
  * to clear but retains everything unrelated to the set operation.
  *
+ * Ib)
+ *         binop (^)
+ *       binop(&)   value/set
+ * payload   value(mask)
+ *
+ * The user wants to set a variable payload argument.  The ^ RHS is the
+ * variable expression.  The mask is as above.
+ *
  * IIa)
  *     binop (&)
  * payload   mask
  *
  * User specified a zero set value -- netlink bitwise decoding
- * discarded the redundant "| 0" part.  This is identical to I),
+ * discarded the redundant "| 0" part.  This is identical to Ia),
  * we can just set value to 0 after we inferred the real payload size.
  *
  * IIb)
@@ -3020,7 +3090,10 @@ static void stmt_payload_binop_postprocess(struct rule_pp_ctx *ctx)
 
 	switch (expr->left->etype) {
 	case EXPR_BINOP: /* I? */
-		if (stmt_payload_binop_postprocess_i(ctx))
+		if (stmt_payload_binop_postprocess_i_a(ctx))
+			return;
+
+		if (stmt_payload_binop_postprocess_i_b(ctx))
 			return;
 
 		break;
-- 
2.35.1


^ permalink raw reply related	[flat|nested] 50+ messages in thread

* [nft PATCH v4 27/32] netlink: rename bitwise operation functions
  2022-04-04 12:13 [nft PATCH v4 00/32] Extend values assignable to packet marks and payload fields Jeremy Sowden
                   ` (25 preceding siblings ...)
  2022-04-04 12:14 ` [nft PATCH v4 26/32] netlink_delinearize: add support for processing variable payload statement arguments Jeremy Sowden
@ 2022-04-04 12:14 ` Jeremy Sowden
  2022-04-04 12:14 ` [nft PATCH v4 28/32] netlink: support (de)linearization of new bitwise boolean operations Jeremy Sowden
                   ` (5 subsequent siblings)
  32 siblings, 0 replies; 50+ messages in thread
From: Jeremy Sowden @ 2022-04-04 12:14 UTC (permalink / raw)
  To: Netfilter Devel; +Cc: Kevin Darbyshire-Bryant

In the next few patches we add support for doing AND, OR and XOR
operations directly in the kernel, so rename a couple of functions and
an enum constant related to mask-and-xor boolean operations.

Signed-off-by: Jeremy Sowden <jeremy@azazel.net>
---
 src/netlink_delinearize.c | 17 +++++++++--------
 src/netlink_linearize.c   | 18 +++++++++---------
 2 files changed, 18 insertions(+), 17 deletions(-)

diff --git a/src/netlink_delinearize.c b/src/netlink_delinearize.c
index e7042d6ae940..63f6febb457d 100644
--- a/src/netlink_delinearize.c
+++ b/src/netlink_delinearize.c
@@ -446,11 +446,12 @@ static void netlink_parse_lookup(struct netlink_parse_ctx *ctx,
 	ctx->stmt = expr_stmt_alloc(loc, expr);
 }
 
-static struct expr *netlink_parse_bitwise_bool(struct netlink_parse_ctx *ctx,
-					       const struct location *loc,
-					       const struct nftnl_expr *nle,
-					       enum nft_registers sreg,
-					       struct expr *left)
+static struct expr *
+netlink_parse_bitwise_mask_xor(struct netlink_parse_ctx *ctx,
+			       const struct location *loc,
+			       const struct nftnl_expr *nle,
+			       enum nft_registers sreg,
+			       struct expr *left)
 {
 	struct nft_data_delinearize nld;
 	struct expr *expr, *mask, *xor, *or;
@@ -559,9 +560,9 @@ static void netlink_parse_bitwise(struct netlink_parse_ctx *ctx,
 	op = nftnl_expr_get_u32(nle, NFTNL_EXPR_BITWISE_OP);
 
 	switch (op) {
-	case NFT_BITWISE_BOOL:
-		expr = netlink_parse_bitwise_bool(ctx, loc, nle, sreg,
-						  left);
+	case NFT_BITWISE_MASK_XOR:
+		expr = netlink_parse_bitwise_mask_xor(ctx, loc, nle, sreg,
+						      left);
 		break;
 	case NFT_BITWISE_LSHIFT:
 		expr = netlink_parse_bitwise_shift(ctx, loc, nle, OP_LSHIFT,
diff --git a/src/netlink_linearize.c b/src/netlink_linearize.c
index 4793f3853bee..478bad073c82 100644
--- a/src/netlink_linearize.c
+++ b/src/netlink_linearize.c
@@ -594,9 +594,9 @@ static void combine_binop(mpz_t mask, mpz_t xor, const mpz_t m, const mpz_t x)
 	mpz_and(mask, mask, m);
 }
 
-static void netlink_gen_shift(struct netlink_linearize_ctx *ctx,
-			      const struct expr *expr,
-			      enum nft_registers dreg)
+static void netlink_gen_bitwise_shift(struct netlink_linearize_ctx *ctx,
+				      const struct expr *expr,
+				      enum nft_registers dreg)
 {
 	enum nft_bitwise_ops op = expr->op == OP_LSHIFT ?
 		NFT_BITWISE_LSHIFT : NFT_BITWISE_RSHIFT;
@@ -621,9 +621,9 @@ static void netlink_gen_shift(struct netlink_linearize_ctx *ctx,
 	nft_rule_add_expr(ctx, nle, &expr->location);
 }
 
-static void netlink_gen_bitwise(struct netlink_linearize_ctx *ctx,
-				const struct expr *expr,
-				enum nft_registers dreg)
+static void netlink_gen_bitwise_mask_xor(struct netlink_linearize_ctx *ctx,
+					 const struct expr *expr,
+					 enum nft_registers dreg)
 {
 	struct nftnl_expr *nle;
 	struct nft_data_linearize nld;
@@ -675,7 +675,7 @@ static void netlink_gen_bitwise(struct netlink_linearize_ctx *ctx,
 	nle = alloc_nft_expr("bitwise");
 	netlink_put_register(nle, NFTNL_EXPR_BITWISE_SREG, dreg);
 	netlink_put_register(nle, NFTNL_EXPR_BITWISE_DREG, dreg);
-	nftnl_expr_set_u32(nle, NFTNL_EXPR_BITWISE_OP, NFT_BITWISE_BOOL);
+	nftnl_expr_set_u32(nle, NFTNL_EXPR_BITWISE_OP, NFT_BITWISE_MASK_XOR);
 	nftnl_expr_set_u32(nle, NFTNL_EXPR_BITWISE_LEN, len);
 	if (expr->byteorder == BYTEORDER_HOST_ENDIAN)
 		nftnl_expr_set_u32(nle, NFTNL_EXPR_BITWISE_NBITS, expr->len);
@@ -700,10 +700,10 @@ static void netlink_gen_binop(struct netlink_linearize_ctx *ctx,
 	switch(expr->op) {
 	case OP_LSHIFT:
 	case OP_RSHIFT:
-		netlink_gen_shift(ctx, expr, dreg);
+		netlink_gen_bitwise_shift(ctx, expr, dreg);
 		break;
 	default:
-		netlink_gen_bitwise(ctx, expr, dreg);
+		netlink_gen_bitwise_mask_xor(ctx, expr, dreg);
 		break;
 	}
 }
-- 
2.35.1


^ permalink raw reply related	[flat|nested] 50+ messages in thread

* [nft PATCH v4 28/32] netlink: support (de)linearization of new bitwise boolean operations
  2022-04-04 12:13 [nft PATCH v4 00/32] Extend values assignable to packet marks and payload fields Jeremy Sowden
                   ` (26 preceding siblings ...)
  2022-04-04 12:14 ` [nft PATCH v4 27/32] netlink: rename bitwise operation functions Jeremy Sowden
@ 2022-04-04 12:14 ` Jeremy Sowden
  2022-04-04 12:14 ` [nft PATCH v4 29/32] parser_json: allow RHS ct, meta and payload expressions Jeremy Sowden
                   ` (4 subsequent siblings)
  32 siblings, 0 replies; 50+ messages in thread
From: Jeremy Sowden @ 2022-04-04 12:14 UTC (permalink / raw)
  To: Netfilter Devel; +Cc: Kevin Darbyshire-Bryant

Hitherto, all boolean bitwise operationss have been converted to the
form:

  dst = (src & mask) ^ xor

and constant values have been required for `xor` and `mask`.  This has
meant that the right-hand operand of a boolean binop must be constant.
The kernel now supports performing AND, OR and XOR operations directly,
on one register and an immediate value or on two registers, so we need
to be able to generate and parse bitwise boolean expressions of this
form.

If a boolean operation has a constant RHS, we continue to send a
mask-and-xor expression to the kernel.

Signed-off-by: Jeremy Sowden <jeremy@azazel.net>
---
 src/netlink_delinearize.c | 50 ++++++++++++++++++++++++++++++++++-----
 src/netlink_linearize.c   | 48 +++++++++++++++++++++++++++++++++++--
 2 files changed, 90 insertions(+), 8 deletions(-)

diff --git a/src/netlink_delinearize.c b/src/netlink_delinearize.c
index 63f6febb457d..73b95cc52763 100644
--- a/src/netlink_delinearize.c
+++ b/src/netlink_delinearize.c
@@ -522,10 +522,43 @@ netlink_parse_bitwise_mask_xor(struct netlink_parse_ctx *ctx,
 	return expr;
 }
 
+static struct expr *netlink_parse_bitwise_bool(struct netlink_parse_ctx *ctx,
+					       const struct location *loc,
+					       const struct nftnl_expr *nle,
+					       enum nft_bitwise_ops op,
+					       enum nft_registers sreg,
+					       struct expr *left)
+{
+	enum nft_registers sreg2;
+	struct expr *right, *expr;
+	unsigned nbits;
+
+	sreg2 = netlink_parse_register(nle, NFTNL_EXPR_BITWISE_SREG2);
+	right = netlink_get_register(ctx, loc, sreg2);
+	if (right == NULL) {
+		netlink_error(ctx, loc,
+			      "Bitwise expression has no right-hand expression");
+		return NULL;
+	}
+
+	expr = binop_expr_alloc(loc,
+				op == NFT_BITWISE_XOR ? OP_XOR :
+				op == NFT_BITWISE_AND ? OP_AND : OP_OR,
+				left, right);
+
+	nbits = nftnl_expr_get_u32(nle, NFTNL_EXPR_BITWISE_NBITS);
+	if (nbits > 0)
+		expr->len = nbits;
+	else if (left->len > 0)
+		expr->len = left->len;
+
+	return expr;
+}
+
 static struct expr *netlink_parse_bitwise_shift(struct netlink_parse_ctx *ctx,
 						const struct location *loc,
 						const struct nftnl_expr *nle,
-						enum ops op,
+						enum nft_bitwise_ops op,
 						enum nft_registers sreg,
 						struct expr *left)
 {
@@ -536,7 +569,9 @@ static struct expr *netlink_parse_bitwise_shift(struct netlink_parse_ctx *ctx,
 	right = netlink_alloc_value(loc, &nld);
 	right->byteorder = BYTEORDER_HOST_ENDIAN;
 
-	expr = binop_expr_alloc(loc, op, left, right);
+	expr = binop_expr_alloc(loc,
+				op == NFT_BITWISE_LSHIFT ? OP_LSHIFT : OP_RSHIFT,
+				left, right);
 	expr->len = nftnl_expr_get_u32(nle, NFTNL_EXPR_BITWISE_LEN) * BITS_PER_BYTE;
 
 	return expr;
@@ -564,12 +599,15 @@ static void netlink_parse_bitwise(struct netlink_parse_ctx *ctx,
 		expr = netlink_parse_bitwise_mask_xor(ctx, loc, nle, sreg,
 						      left);
 		break;
-	case NFT_BITWISE_LSHIFT:
-		expr = netlink_parse_bitwise_shift(ctx, loc, nle, OP_LSHIFT,
-						   sreg, left);
+	case NFT_BITWISE_XOR:
+	case NFT_BITWISE_AND:
+	case NFT_BITWISE_OR:
+		expr = netlink_parse_bitwise_bool(ctx, loc, nle, op,
+						  sreg, left);
 		break;
+	case NFT_BITWISE_LSHIFT:
 	case NFT_BITWISE_RSHIFT:
-		expr = netlink_parse_bitwise_shift(ctx, loc, nle, OP_RSHIFT,
+		expr = netlink_parse_bitwise_shift(ctx, loc, nle, op,
 						   sreg, left);
 		break;
 	default:
diff --git a/src/netlink_linearize.c b/src/netlink_linearize.c
index 478bad073c82..7292c42eda8b 100644
--- a/src/netlink_linearize.c
+++ b/src/netlink_linearize.c
@@ -640,7 +640,8 @@ static void netlink_gen_bitwise_mask_xor(struct netlink_linearize_ctx *ctx,
 
 	binops[n++] = left = (struct expr *) expr;
 	while (left->etype == EXPR_BINOP && left->left != NULL &&
-	       (left->op == OP_AND || left->op == OP_OR || left->op == OP_XOR))
+	       (left->op == OP_AND || left->op == OP_OR || left->op == OP_XOR) &&
+	       expr_is_constant(left->right))
 		binops[n++] = left = left->left;
 	n--;
 
@@ -693,6 +694,46 @@ static void netlink_gen_bitwise_mask_xor(struct netlink_linearize_ctx *ctx,
 	nft_rule_add_expr(ctx, nle, &expr->location);
 }
 
+static void netlink_gen_bitwise_bool(struct netlink_linearize_ctx *ctx,
+				     const struct expr *expr,
+				     enum nft_registers dreg)
+{
+	enum nft_registers sreg2;
+	struct nftnl_expr *nle;
+	unsigned int len;
+
+	nle = alloc_nft_expr("bitwise");
+
+	switch (expr->op) {
+	case OP_XOR:
+		nftnl_expr_set_u32(nle, NFTNL_EXPR_BITWISE_OP, NFT_BITWISE_XOR);
+		break;
+	case OP_AND:
+		nftnl_expr_set_u32(nle, NFTNL_EXPR_BITWISE_OP, NFT_BITWISE_AND);
+		break;
+	case OP_OR:
+		nftnl_expr_set_u32(nle, NFTNL_EXPR_BITWISE_OP, NFT_BITWISE_OR);
+		break;
+	default:
+		BUG("invalid binary operation %u\n", expr->op);
+	}
+
+	netlink_gen_expr(ctx, expr->left, dreg);
+	netlink_put_register(nle, NFTNL_EXPR_BITWISE_SREG, dreg);
+	netlink_put_register(nle, NFTNL_EXPR_BITWISE_DREG, dreg);
+
+	sreg2 = get_register(ctx, expr->right);
+	netlink_gen_expr(ctx, expr->right, sreg2);
+	netlink_put_register(nle, NFTNL_EXPR_BITWISE_SREG2, sreg2);
+
+	len = div_round_up(expr->len, BITS_PER_BYTE);
+	nftnl_expr_set_u32(nle, NFTNL_EXPR_BITWISE_LEN, len);
+	if (expr->byteorder == BYTEORDER_HOST_ENDIAN)
+		nftnl_expr_set_u32(nle, NFTNL_EXPR_BITWISE_NBITS, expr->len);
+
+	nftnl_rule_add_expr(ctx->nlr, nle);
+}
+
 static void netlink_gen_binop(struct netlink_linearize_ctx *ctx,
 			      const struct expr *expr,
 			      enum nft_registers dreg)
@@ -703,7 +744,10 @@ static void netlink_gen_binop(struct netlink_linearize_ctx *ctx,
 		netlink_gen_bitwise_shift(ctx, expr, dreg);
 		break;
 	default:
-		netlink_gen_bitwise_mask_xor(ctx, expr, dreg);
+		if (expr_is_constant(expr->right))
+			netlink_gen_bitwise_mask_xor(ctx, expr, dreg);
+		else
+			netlink_gen_bitwise_bool(ctx, expr, dreg);
 		break;
 	}
 }
-- 
2.35.1


^ permalink raw reply related	[flat|nested] 50+ messages in thread

* [nft PATCH v4 29/32] parser_json: allow RHS ct, meta and payload expressions
  2022-04-04 12:13 [nft PATCH v4 00/32] Extend values assignable to packet marks and payload fields Jeremy Sowden
                   ` (27 preceding siblings ...)
  2022-04-04 12:14 ` [nft PATCH v4 28/32] netlink: support (de)linearization of new bitwise boolean operations Jeremy Sowden
@ 2022-04-04 12:14 ` Jeremy Sowden
  2022-04-04 12:14 ` [nft PATCH v4 30/32] evaluate: allow binop expressions with variable right-hand operands Jeremy Sowden
                   ` (3 subsequent siblings)
  32 siblings, 0 replies; 50+ messages in thread
From: Jeremy Sowden @ 2022-04-04 12:14 UTC (permalink / raw)
  To: Netfilter Devel; +Cc: Kevin Darbyshire-Bryant

Support for binops with variable RHS's will amke it possible to have ct,
meta and payload expressions in the RHS.  Relax the JSON parser accordingly.

Fix a typo in an adjacent comment.

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

diff --git a/src/parser_json.c b/src/parser_json.c
index fb401009a499..664a77c66165 100644
--- a/src/parser_json.c
+++ b/src/parser_json.c
@@ -1446,20 +1446,20 @@ static struct expr *json_parse_expr(struct json_ctx *ctx, json_t *root)
 		{ "concat", json_parse_concat_expr, CTX_F_RHS | CTX_F_STMT | CTX_F_DTYPE | CTX_F_SET_RHS | CTX_F_SES | CTX_F_MAP },
 		{ "set", json_parse_set_expr, CTX_F_RHS | CTX_F_STMT }, /* allow this as stmt expr because that allows set references */
 		{ "map", json_parse_map_expr, CTX_F_STMT | CTX_F_PRIMARY | CTX_F_SET_RHS },
-		/* below three are multiton_rhs_expr */
+		/* below two are multiton_rhs_expr */
 		{ "prefix", json_parse_prefix_expr, CTX_F_RHS | CTX_F_SET_RHS | CTX_F_STMT | CTX_F_CONCAT },
 		{ "range", json_parse_range_expr, CTX_F_RHS | CTX_F_SET_RHS | CTX_F_STMT | CTX_F_CONCAT },
-		{ "payload", json_parse_payload_expr, CTX_F_STMT | CTX_F_PRIMARY | CTX_F_SET_RHS | CTX_F_MANGLE | CTX_F_SES | CTX_F_MAP | CTX_F_CONCAT },
+		{ "payload", json_parse_payload_expr, CTX_F_RHS | CTX_F_STMT | CTX_F_PRIMARY | CTX_F_SET_RHS | CTX_F_MANGLE | CTX_F_SES | CTX_F_MAP | CTX_F_CONCAT },
 		{ "exthdr", json_parse_exthdr_expr, CTX_F_PRIMARY | CTX_F_SET_RHS | CTX_F_SES | CTX_F_MAP | CTX_F_CONCAT },
 		{ "tcp option", json_parse_tcp_option_expr, CTX_F_PRIMARY | CTX_F_SET_RHS | CTX_F_MANGLE | CTX_F_SES | CTX_F_CONCAT },
 		{ "ip option", json_parse_ip_option_expr, CTX_F_PRIMARY | CTX_F_SET_RHS | CTX_F_MANGLE | CTX_F_SES | CTX_F_CONCAT },
 		{ "sctp chunk", json_parse_sctp_chunk_expr, CTX_F_PRIMARY | CTX_F_SET_RHS | CTX_F_MANGLE | CTX_F_SES | CTX_F_CONCAT },
-		{ "meta", json_parse_meta_expr, CTX_F_STMT | CTX_F_PRIMARY | CTX_F_SET_RHS | CTX_F_MANGLE | CTX_F_SES | CTX_F_MAP | CTX_F_CONCAT },
+		{ "meta", json_parse_meta_expr, CTX_F_RHS | CTX_F_STMT | CTX_F_PRIMARY | CTX_F_SET_RHS | CTX_F_MANGLE | CTX_F_SES | CTX_F_MAP | CTX_F_CONCAT },
 		{ "osf", json_parse_osf_expr, CTX_F_STMT | CTX_F_PRIMARY | CTX_F_MAP | CTX_F_CONCAT },
 		{ "ipsec", json_parse_xfrm_expr, CTX_F_PRIMARY | CTX_F_MAP | CTX_F_CONCAT },
 		{ "socket", json_parse_socket_expr, CTX_F_PRIMARY | CTX_F_CONCAT },
 		{ "rt", json_parse_rt_expr, CTX_F_STMT | CTX_F_PRIMARY | CTX_F_SET_RHS | CTX_F_SES | CTX_F_MAP | CTX_F_CONCAT },
-		{ "ct", json_parse_ct_expr, CTX_F_STMT | CTX_F_PRIMARY | CTX_F_SET_RHS | CTX_F_MANGLE | CTX_F_SES | CTX_F_MAP | CTX_F_CONCAT },
+		{ "ct", json_parse_ct_expr, CTX_F_RHS | CTX_F_STMT | CTX_F_PRIMARY | CTX_F_SET_RHS | CTX_F_MANGLE | CTX_F_SES | CTX_F_MAP | CTX_F_CONCAT },
 		{ "numgen", json_parse_numgen_expr, CTX_F_STMT | CTX_F_PRIMARY | CTX_F_SET_RHS | CTX_F_SES | CTX_F_MAP | CTX_F_CONCAT },
 		/* below two are hash expr */
 		{ "jhash", json_parse_hash_expr, CTX_F_STMT | CTX_F_PRIMARY | CTX_F_SET_RHS | CTX_F_SES | CTX_F_MAP | CTX_F_CONCAT },
-- 
2.35.1


^ permalink raw reply related	[flat|nested] 50+ messages in thread

* [nft PATCH v4 30/32] evaluate: allow binop expressions with variable right-hand operands
  2022-04-04 12:13 [nft PATCH v4 00/32] Extend values assignable to packet marks and payload fields Jeremy Sowden
                   ` (28 preceding siblings ...)
  2022-04-04 12:14 ` [nft PATCH v4 29/32] parser_json: allow RHS ct, meta and payload expressions Jeremy Sowden
@ 2022-04-04 12:14 ` Jeremy Sowden
  2022-04-04 12:14 ` [nft PATCH v4 31/32] tests: shell: add tests for binops with variable RHS operands Jeremy Sowden
                   ` (2 subsequent siblings)
  32 siblings, 0 replies; 50+ messages in thread
From: Jeremy Sowden @ 2022-04-04 12:14 UTC (permalink / raw)
  To: Netfilter Devel; +Cc: Kevin Darbyshire-Bryant

Hitherto, the kernel has required constant values for the `xor` and
`mask` attributes of boolean bitwise expressions.  This has meant that
the right-hand operand of a boolean binop must be constant.  Now the
kernel has support for AND, OR and XOR operations with right-hand
operands passed via registers, we can relax this restriction.  Allow
non-constant right-hand operands if the left-hand operand is not
constant, e.g.:

  ct mark & 0xffff0000 | meta mark & 0xffff

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

diff --git a/src/evaluate.c b/src/evaluate.c
index 02bfde2a2ded..4fff788f45fb 100644
--- a/src/evaluate.c
+++ b/src/evaluate.c
@@ -1162,16 +1162,18 @@ static int expr_evaluate_bitwise(struct eval_ctx *ctx, struct expr **expr)
 	op->byteorder = left->byteorder;
 	op->len	      = op->len ? : left->len;
 
-	if (expr_is_constant(left))
+	if (expr_is_constant(left) && expr_is_constant(op->right))
 		return constant_binop_simplify(ctx, expr);
 	return 0;
 }
 
 /*
- * Binop expression: both sides must be of integer base type. The left
- * hand side may be either constant or non-constant; in case its constant
- * it must be a singleton. The ride hand side must always be a constant
- * singleton.
+ * Binop expression: both sides must be of integer base type. The left-hand side
+ * may be either constant or non-constant; if it is constant, it must be a
+ * singleton.  For bitwise operations, the right-hand side must be constant if
+ * the left-hand side is constant; the right-hand side may be constant or
+ * non-constant, if the left-hand side is non-constant; for shifts, the
+ * right-hand side must be constant; if it is constant, it must be a singleton.
  */
 static int expr_evaluate_binop(struct eval_ctx *ctx, struct expr **expr)
 {
@@ -1207,27 +1209,36 @@ static int expr_evaluate_binop(struct eval_ctx *ctx, struct expr **expr)
 					 "for %s expressions",
 					 sym, expr_name(left));
 
-	if (!expr_is_constant(right))
-		return expr_binary_error(ctx->msgs, right, op,
-					 "Right hand side of binary operation "
-					 "(%s) must be constant", sym);
-
-	if (!expr_is_singleton(right))
-		return expr_binary_error(ctx->msgs, left, op,
-					 "Binary operation (%s) is undefined "
-					 "for %s expressions",
-					 sym, expr_name(right));
-
 	/* The grammar guarantees this */
 	assert(expr_basetype(left) == expr_basetype(right));
 
 	switch (op->op) {
 	case OP_LSHIFT:
 	case OP_RSHIFT:
+		if (!expr_is_constant(right))
+			return expr_binary_error(ctx->msgs, right, op,
+						 "Right hand side of binary operation "
+						 "(%s) must be constant", sym);
+
+		if (!expr_is_singleton(right))
+			return expr_binary_error(ctx->msgs, left, op,
+						 "Binary operation (%s) is undefined "
+						 "for %s expressions",
+						 sym, expr_name(right));
 		return expr_evaluate_shift(ctx, expr);
 	case OP_AND:
 	case OP_XOR:
 	case OP_OR:
+		if (expr_is_constant(left) && !expr_is_constant(right))
+			return expr_binary_error(ctx->msgs, right, op,
+						 "Right hand side of binary operation "
+						 "(%s) must be constant", sym);
+
+		if (expr_is_constant(right) && !expr_is_singleton(right))
+			return expr_binary_error(ctx->msgs, left, op,
+						 "Binary operation (%s) is undefined "
+						 "for %s expressions",
+						 sym, expr_name(right));
 		return expr_evaluate_bitwise(ctx, expr);
 	default:
 		BUG("invalid binary operation %u\n", op->op);
-- 
2.35.1


^ permalink raw reply related	[flat|nested] 50+ messages in thread

* [nft PATCH v4 31/32] tests: shell: add tests for binops with variable RHS operands
  2022-04-04 12:13 [nft PATCH v4 00/32] Extend values assignable to packet marks and payload fields Jeremy Sowden
                   ` (29 preceding siblings ...)
  2022-04-04 12:14 ` [nft PATCH v4 30/32] evaluate: allow binop expressions with variable right-hand operands Jeremy Sowden
@ 2022-04-04 12:14 ` Jeremy Sowden
  2022-04-04 12:14 ` [nft PATCH v4 32/32] tests: py: " Jeremy Sowden
  2022-04-09  8:30 ` [nft PATCH v4 00/32] Extend values assignable to packet marks and payload fields Kevin 'ldir' Darbyshire-Bryant
  32 siblings, 0 replies; 50+ messages in thread
From: Jeremy Sowden @ 2022-04-04 12:14 UTC (permalink / raw)
  To: Netfilter Devel; +Cc: Kevin Darbyshire-Bryant

Add tests to validate setting marks with statement arguments that include
binops with variable RHS operands.

Signed-off-by: Jeremy Sowden <jeremy@azazel.net>
---
 tests/shell/testcases/chains/0040mark_binop_10        | 11 +++++++++++
 tests/shell/testcases/chains/0040mark_binop_11        | 11 +++++++++++
 tests/shell/testcases/chains/0040mark_binop_12        | 11 +++++++++++
 tests/shell/testcases/chains/0040mark_binop_13        | 11 +++++++++++
 tests/shell/testcases/chains/0044payload_binop_0      | 11 +++++++++++
 tests/shell/testcases/chains/0044payload_binop_1      | 11 +++++++++++
 tests/shell/testcases/chains/0044payload_binop_2      | 11 +++++++++++
 tests/shell/testcases/chains/0044payload_binop_3      | 11 +++++++++++
 tests/shell/testcases/chains/0044payload_binop_4      | 11 +++++++++++
 tests/shell/testcases/chains/0044payload_binop_5      | 11 +++++++++++
 .../testcases/chains/dumps/0040mark_binop_10.nft      |  6 ++++++
 .../testcases/chains/dumps/0040mark_binop_11.nft      |  6 ++++++
 .../testcases/chains/dumps/0040mark_binop_12.nft      |  6 ++++++
 .../testcases/chains/dumps/0040mark_binop_13.nft      |  6 ++++++
 .../testcases/chains/dumps/0044payload_binop_0.nft    |  6 ++++++
 .../testcases/chains/dumps/0044payload_binop_1.nft    |  6 ++++++
 .../testcases/chains/dumps/0044payload_binop_2.nft    |  6 ++++++
 .../testcases/chains/dumps/0044payload_binop_3.nft    |  6 ++++++
 .../testcases/chains/dumps/0044payload_binop_4.nft    |  6 ++++++
 .../testcases/chains/dumps/0044payload_binop_5.nft    |  6 ++++++
 20 files changed, 170 insertions(+)
 create mode 100755 tests/shell/testcases/chains/0040mark_binop_10
 create mode 100755 tests/shell/testcases/chains/0040mark_binop_11
 create mode 100755 tests/shell/testcases/chains/0040mark_binop_12
 create mode 100755 tests/shell/testcases/chains/0040mark_binop_13
 create mode 100755 tests/shell/testcases/chains/0044payload_binop_0
 create mode 100755 tests/shell/testcases/chains/0044payload_binop_1
 create mode 100755 tests/shell/testcases/chains/0044payload_binop_2
 create mode 100755 tests/shell/testcases/chains/0044payload_binop_3
 create mode 100755 tests/shell/testcases/chains/0044payload_binop_4
 create mode 100755 tests/shell/testcases/chains/0044payload_binop_5
 create mode 100644 tests/shell/testcases/chains/dumps/0040mark_binop_10.nft
 create mode 100644 tests/shell/testcases/chains/dumps/0040mark_binop_11.nft
 create mode 100644 tests/shell/testcases/chains/dumps/0040mark_binop_12.nft
 create mode 100644 tests/shell/testcases/chains/dumps/0040mark_binop_13.nft
 create mode 100644 tests/shell/testcases/chains/dumps/0044payload_binop_0.nft
 create mode 100644 tests/shell/testcases/chains/dumps/0044payload_binop_1.nft
 create mode 100644 tests/shell/testcases/chains/dumps/0044payload_binop_2.nft
 create mode 100644 tests/shell/testcases/chains/dumps/0044payload_binop_3.nft
 create mode 100644 tests/shell/testcases/chains/dumps/0044payload_binop_4.nft
 create mode 100644 tests/shell/testcases/chains/dumps/0044payload_binop_5.nft

diff --git a/tests/shell/testcases/chains/0040mark_binop_10 b/tests/shell/testcases/chains/0040mark_binop_10
new file mode 100755
index 000000000000..8e9bc6ad4329
--- /dev/null
+++ b/tests/shell/testcases/chains/0040mark_binop_10
@@ -0,0 +1,11 @@
+#!/bin/bash
+
+set -e
+
+RULESET="
+  add table t
+  add chain t c { type filter hook output priority filter; }
+  add rule t c ct mark set ct mark and 0xffff0000 or meta mark and 0xffff
+"
+
+$NFT -f - <<< "$RULESET"
diff --git a/tests/shell/testcases/chains/0040mark_binop_11 b/tests/shell/testcases/chains/0040mark_binop_11
new file mode 100755
index 000000000000..7825b0827851
--- /dev/null
+++ b/tests/shell/testcases/chains/0040mark_binop_11
@@ -0,0 +1,11 @@
+#!/bin/bash
+
+set -e
+
+RULESET="
+  add table t
+  add chain t c { type filter hook input priority filter; }
+  add rule t c meta mark set ct mark and 0xffff0000 or meta mark and 0xffff
+"
+
+$NFT -f - <<< "$RULESET"
diff --git a/tests/shell/testcases/chains/0040mark_binop_12 b/tests/shell/testcases/chains/0040mark_binop_12
new file mode 100755
index 000000000000..aa27cdc5303c
--- /dev/null
+++ b/tests/shell/testcases/chains/0040mark_binop_12
@@ -0,0 +1,11 @@
+#!/bin/bash
+
+set -e
+
+RULESET="
+  add table ip6 t
+  add chain ip6 t c { type filter hook output priority filter; }
+  add rule ip6 t c ct mark set ct mark and 0xffff0000 or meta mark and 0xffff
+"
+
+$NFT -f - <<< "$RULESET"
diff --git a/tests/shell/testcases/chains/0040mark_binop_13 b/tests/shell/testcases/chains/0040mark_binop_13
new file mode 100755
index 000000000000..53a7e2ec6c6f
--- /dev/null
+++ b/tests/shell/testcases/chains/0040mark_binop_13
@@ -0,0 +1,11 @@
+#!/bin/bash
+
+set -e
+
+RULESET="
+  add table ip6 t
+  add chain ip6 t c { type filter hook input priority filter; }
+  add rule ip6 t c meta mark set ct mark and 0xffff0000 or meta mark and 0xffff
+"
+
+$NFT -f - <<< "$RULESET"
diff --git a/tests/shell/testcases/chains/0044payload_binop_0 b/tests/shell/testcases/chains/0044payload_binop_0
new file mode 100755
index 000000000000..81b8cbaa961f
--- /dev/null
+++ b/tests/shell/testcases/chains/0044payload_binop_0
@@ -0,0 +1,11 @@
+#!/bin/bash
+
+set -e
+
+RULESET="
+  add table t
+  add chain t c { type filter hook output priority filter; }
+  add rule t c ip dscp set (ct mark & 0xfc000000) >> 26
+"
+
+$NFT -f - <<< "$RULESET"
diff --git a/tests/shell/testcases/chains/0044payload_binop_1 b/tests/shell/testcases/chains/0044payload_binop_1
new file mode 100755
index 000000000000..1d69b6f78654
--- /dev/null
+++ b/tests/shell/testcases/chains/0044payload_binop_1
@@ -0,0 +1,11 @@
+#!/bin/bash
+
+set -e
+
+RULESET="
+  add table t
+  add chain t c { type filter hook output priority filter; }
+  add rule t c ip dscp set ip dscp and 0xc
+"
+
+$NFT -f - <<< "$RULESET"
diff --git a/tests/shell/testcases/chains/0044payload_binop_2 b/tests/shell/testcases/chains/0044payload_binop_2
new file mode 100755
index 000000000000..2d09d24479d0
--- /dev/null
+++ b/tests/shell/testcases/chains/0044payload_binop_2
@@ -0,0 +1,11 @@
+#!/bin/bash
+
+set -e
+
+RULESET="
+  add table t
+  add chain t c { type filter hook output priority filter; }
+  add rule t c ct mark set ct mark | ip dscp | 0x200 counter
+"
+
+$NFT -f - <<< "$RULESET"
diff --git a/tests/shell/testcases/chains/0044payload_binop_3 b/tests/shell/testcases/chains/0044payload_binop_3
new file mode 100755
index 000000000000..7752af238409
--- /dev/null
+++ b/tests/shell/testcases/chains/0044payload_binop_3
@@ -0,0 +1,11 @@
+#!/bin/bash
+
+set -e
+
+RULESET="
+  add table ip6 t
+  add chain ip6 t c { type filter hook output priority filter; }
+  add rule ip6 t c ip6 dscp set (ct mark & 0xfc000000) >> 26
+"
+
+$NFT -f - <<< "$RULESET"
diff --git a/tests/shell/testcases/chains/0044payload_binop_4 b/tests/shell/testcases/chains/0044payload_binop_4
new file mode 100755
index 000000000000..2c7792e9f929
--- /dev/null
+++ b/tests/shell/testcases/chains/0044payload_binop_4
@@ -0,0 +1,11 @@
+#!/bin/bash
+
+set -e
+
+RULESET="
+  add table ip6 t
+  add chain ip6 t c { type filter hook output priority filter; }
+  add rule ip6 t c ip6 dscp set ip6 dscp and 0xc
+"
+
+$NFT -f - <<< "$RULESET"
diff --git a/tests/shell/testcases/chains/0044payload_binop_5 b/tests/shell/testcases/chains/0044payload_binop_5
new file mode 100755
index 000000000000..aa82cd1c299e
--- /dev/null
+++ b/tests/shell/testcases/chains/0044payload_binop_5
@@ -0,0 +1,11 @@
+#!/bin/bash
+
+set -e
+
+RULESET="
+  add table ip6 t
+  add chain ip6 t c { type filter hook output priority filter; }
+  add rule ip6 t c ct mark set ct mark | ip6 dscp | 0x200 counter
+"
+
+$NFT -f - <<< "$RULESET"
diff --git a/tests/shell/testcases/chains/dumps/0040mark_binop_10.nft b/tests/shell/testcases/chains/dumps/0040mark_binop_10.nft
new file mode 100644
index 000000000000..5566f7298461
--- /dev/null
+++ b/tests/shell/testcases/chains/dumps/0040mark_binop_10.nft
@@ -0,0 +1,6 @@
+table ip t {
+	chain c {
+		type filter hook output priority filter; policy accept;
+		ct mark set ct mark & 0xffff0000 | meta mark & 0x0000ffff
+	}
+}
diff --git a/tests/shell/testcases/chains/dumps/0040mark_binop_11.nft b/tests/shell/testcases/chains/dumps/0040mark_binop_11.nft
new file mode 100644
index 000000000000..719980d55341
--- /dev/null
+++ b/tests/shell/testcases/chains/dumps/0040mark_binop_11.nft
@@ -0,0 +1,6 @@
+table ip t {
+	chain c {
+		type filter hook input priority filter; policy accept;
+		meta mark set ct mark & 0xffff0000 | meta mark & 0x0000ffff
+	}
+}
diff --git a/tests/shell/testcases/chains/dumps/0040mark_binop_12.nft b/tests/shell/testcases/chains/dumps/0040mark_binop_12.nft
new file mode 100644
index 000000000000..bd589fe549f7
--- /dev/null
+++ b/tests/shell/testcases/chains/dumps/0040mark_binop_12.nft
@@ -0,0 +1,6 @@
+table ip6 t {
+	chain c {
+		type filter hook output priority filter; policy accept;
+		ct mark set ct mark & 0xffff0000 | meta mark & 0x0000ffff
+	}
+}
diff --git a/tests/shell/testcases/chains/dumps/0040mark_binop_13.nft b/tests/shell/testcases/chains/dumps/0040mark_binop_13.nft
new file mode 100644
index 000000000000..2b046b128fb2
--- /dev/null
+++ b/tests/shell/testcases/chains/dumps/0040mark_binop_13.nft
@@ -0,0 +1,6 @@
+table ip6 t {
+	chain c {
+		type filter hook input priority filter; policy accept;
+		meta mark set ct mark & 0xffff0000 | meta mark & 0x0000ffff
+	}
+}
diff --git a/tests/shell/testcases/chains/dumps/0044payload_binop_0.nft b/tests/shell/testcases/chains/dumps/0044payload_binop_0.nft
new file mode 100644
index 000000000000..5aaf1353bdc8
--- /dev/null
+++ b/tests/shell/testcases/chains/dumps/0044payload_binop_0.nft
@@ -0,0 +1,6 @@
+table ip t {
+	chain c {
+		type filter hook output priority filter; policy accept;
+		ip dscp set (ct mark & 0xfc000000) >> 26
+	}
+}
diff --git a/tests/shell/testcases/chains/dumps/0044payload_binop_1.nft b/tests/shell/testcases/chains/dumps/0044payload_binop_1.nft
new file mode 100644
index 000000000000..54f744b54a3a
--- /dev/null
+++ b/tests/shell/testcases/chains/dumps/0044payload_binop_1.nft
@@ -0,0 +1,6 @@
+table ip t {
+	chain c {
+		type filter hook output priority filter; policy accept;
+		ip dscp set ip dscp & af12
+	}
+}
diff --git a/tests/shell/testcases/chains/dumps/0044payload_binop_2.nft b/tests/shell/testcases/chains/dumps/0044payload_binop_2.nft
new file mode 100644
index 000000000000..ed347bb2788a
--- /dev/null
+++ b/tests/shell/testcases/chains/dumps/0044payload_binop_2.nft
@@ -0,0 +1,6 @@
+table ip t {
+	chain c {
+		type filter hook output priority filter; policy accept;
+		ct mark set ct mark | ip dscp | 0x00000200 counter packets 0 bytes 0
+	}
+}
diff --git a/tests/shell/testcases/chains/dumps/0044payload_binop_3.nft b/tests/shell/testcases/chains/dumps/0044payload_binop_3.nft
new file mode 100644
index 000000000000..64da4a77cb5c
--- /dev/null
+++ b/tests/shell/testcases/chains/dumps/0044payload_binop_3.nft
@@ -0,0 +1,6 @@
+table ip6 t {
+	chain c {
+		type filter hook output priority filter; policy accept;
+		ip6 dscp set (ct mark & 0xfc000000) >> 26
+	}
+}
diff --git a/tests/shell/testcases/chains/dumps/0044payload_binop_4.nft b/tests/shell/testcases/chains/dumps/0044payload_binop_4.nft
new file mode 100644
index 000000000000..e863f44ef991
--- /dev/null
+++ b/tests/shell/testcases/chains/dumps/0044payload_binop_4.nft
@@ -0,0 +1,6 @@
+table ip6 t {
+	chain c {
+		type filter hook output priority filter; policy accept;
+		ip6 dscp set ip6 dscp & af12
+	}
+}
diff --git a/tests/shell/testcases/chains/dumps/0044payload_binop_5.nft b/tests/shell/testcases/chains/dumps/0044payload_binop_5.nft
new file mode 100644
index 000000000000..ccdb93d74a9a
--- /dev/null
+++ b/tests/shell/testcases/chains/dumps/0044payload_binop_5.nft
@@ -0,0 +1,6 @@
+table ip6 t {
+	chain c {
+		type filter hook output priority filter; policy accept;
+		ct mark set ct mark | ip6 dscp | 0x00000200 counter packets 0 bytes 0
+	}
+}
-- 
2.35.1


^ permalink raw reply related	[flat|nested] 50+ messages in thread

* [nft PATCH v4 32/32] tests: py: add tests for binops with variable RHS operands
  2022-04-04 12:13 [nft PATCH v4 00/32] Extend values assignable to packet marks and payload fields Jeremy Sowden
                   ` (30 preceding siblings ...)
  2022-04-04 12:14 ` [nft PATCH v4 31/32] tests: shell: add tests for binops with variable RHS operands Jeremy Sowden
@ 2022-04-04 12:14 ` Jeremy Sowden
  2022-04-09  8:30 ` [nft PATCH v4 00/32] Extend values assignable to packet marks and payload fields Kevin 'ldir' Darbyshire-Bryant
  32 siblings, 0 replies; 50+ messages in thread
From: Jeremy Sowden @ 2022-04-04 12:14 UTC (permalink / raw)
  To: Netfilter Devel; +Cc: Kevin Darbyshire-Bryant

Add some tests to validate setting marks with statement arguments that include
binops with variable RHS operands.

Signed-off-by: Jeremy Sowden <jeremy@azazel.net>
---
 tests/py/any/ct.t               |  1 +
 tests/py/any/ct.t.json          | 37 ++++++++++++++++
 tests/py/any/ct.t.payload       |  9 ++++
 tests/py/inet/meta.t            |  1 +
 tests/py/inet/meta.t.json       | 37 ++++++++++++++++
 tests/py/inet/meta.t.payload    |  9 ++++
 tests/py/ip/ct.t                |  1 +
 tests/py/ip/ct.t.json           | 36 +++++++++++++++
 tests/py/ip/ct.t.payload        | 11 +++++
 tests/py/ip/ip.t                |  2 +
 tests/py/ip/ip.t.json           | 77 ++++++++++++++++++++++++++++++++-
 tests/py/ip/ip.t.payload        | 28 ++++++++++++
 tests/py/ip/ip.t.payload.bridge | 32 ++++++++++++++
 tests/py/ip/ip.t.payload.inet   | 32 ++++++++++++++
 tests/py/ip/ip.t.payload.netdev | 32 ++++++++++++++
 tests/py/ip6/ct.t               |  1 +
 tests/py/ip6/ct.t.json          | 35 +++++++++++++++
 tests/py/ip6/ct.t.payload       | 12 +++++
 tests/py/ip6/ip6.t              |  2 +
 tests/py/ip6/ip6.t.json         | 76 ++++++++++++++++++++++++++++++++
 tests/py/ip6/ip6.t.payload.inet | 34 +++++++++++++++
 tests/py/ip6/ip6.t.payload.ip6  | 30 +++++++++++++
 22 files changed, 534 insertions(+), 1 deletion(-)

diff --git a/tests/py/any/ct.t b/tests/py/any/ct.t
index f73fa4e7aedb..3e0e473f55b7 100644
--- a/tests/py/any/ct.t
+++ b/tests/py/any/ct.t
@@ -61,6 +61,7 @@ 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 ct mark and 0xffff0000 or meta mark and 0xffff;ok;ct mark set ct mark & 0xffff0000 | meta mark & 0x0000ffff
 
 ct mark set {0x11333, 0x11};fail
 ct zone set {123, 127};fail
diff --git a/tests/py/any/ct.t.json b/tests/py/any/ct.t.json
index a2a06025992c..4d6043190201 100644
--- a/tests/py/any/ct.t.json
+++ b/tests/py/any/ct.t.json
@@ -817,6 +817,43 @@
     }
 ]
 
+# ct mark set ct mark and 0xffff0000 or meta mark and 0xffff
+[
+    {
+        "mangle": {
+            "key": {
+                "ct": {
+                    "key": "mark"
+                }
+            },
+            "value": {
+                "|": [
+                    {
+                        "&": [
+                            {
+                                "ct": {
+                                    "key": "mark"
+                                }
+                            },
+                            4294901760
+                        ]
+                    },
+                    {
+                        "&": [
+                            {
+                                "meta": {
+                                    "key": "mark"
+                                }
+                            },
+                            65535
+                        ]
+                    }
+                ]
+            }
+        }
+    }
+]
+
 # ct expiration 30s
 [
     {
diff --git a/tests/py/any/ct.t.payload b/tests/py/any/ct.t.payload
index ed868e53277d..1523e54d1307 100644
--- a/tests/py/any/ct.t.payload
+++ b/tests/py/any/ct.t.payload
@@ -336,6 +336,15 @@ ip test-ip4 output
   [ lookup reg 1 set __map%d dreg 1 ]
   [ ct set mark with reg 1 ]
 
+# ct mark set ct mark and 0xffff0000 or meta mark and 0xffff
+ip
+  [ ct load mark => reg 1 ]
+  [ bitwise reg 1 = ( reg 1 & 0xffff0000 ) ^ 0x00000000 ]
+  [ meta load mark => reg 2 ]
+  [ bitwise reg 2 = ( reg 2 & 0x0000ffff ) ^ 0x00000000 ]
+  [ bitwise reg 1 = ( reg 1 | reg 2 ) ]
+  [ ct set mark with reg 1 ]
+
 # ct original bytes > 100000
 ip test-ip4 output
   [ ct load bytes => reg 1 , dir original ]
diff --git a/tests/py/inet/meta.t b/tests/py/inet/meta.t
index 423cc5f32cba..a389a3ee5123 100644
--- a/tests/py/inet/meta.t
+++ b/tests/py/inet/meta.t
@@ -21,3 +21,4 @@ meta secpath missing;ok;meta ipsec missing
 meta ibrname "br0";fail
 meta obrname "br0";fail
 meta mark set ct mark >> 8;ok
+meta mark set ct mark and 0xffff0000 or meta mark and 0xffff;ok;meta mark set ct mark & 0xffff0000 | meta mark & 0x0000ffff
diff --git a/tests/py/inet/meta.t.json b/tests/py/inet/meta.t.json
index 723a36f74946..9e0484388adf 100644
--- a/tests/py/inet/meta.t.json
+++ b/tests/py/inet/meta.t.json
@@ -236,6 +236,43 @@
     }
 ]
 
+# meta mark set ct mark and 0xffff0000 or meta mark and 0xffff
+[
+    {
+        "mangle": {
+            "key": {
+                "meta": {
+                    "key": "mark"
+                }
+            },
+            "value": {
+                "|": [
+                    {
+                        "&": [
+                            {
+                                "ct": {
+                                    "key": "mark"
+                                }
+                            },
+                            4294901760
+                        ]
+                    },
+                    {
+                        "&": [
+                            {
+                                "meta": {
+                                    "key": "mark"
+                                }
+                            },
+                            65535
+                        ]
+                    }
+                ]
+            }
+        }
+    }
+]
+
 # meta protocol ip udp dport 67
 [
     {
diff --git a/tests/py/inet/meta.t.payload b/tests/py/inet/meta.t.payload
index fd0545490b78..737878294d1e 100644
--- a/tests/py/inet/meta.t.payload
+++ b/tests/py/inet/meta.t.payload
@@ -80,6 +80,15 @@ inet test-inet input
   [ bitwise reg 1 = ( reg 1 >> 0x00000008 ) ]
   [ meta set mark with reg 1 ]
 
+# meta mark set ct mark and 0xffff0000 or meta mark and 0xffff
+inet test-inet input
+  [ ct load mark => reg 1 ]
+  [ bitwise reg 1 = ( reg 1 & 0xffff0000 ) ^ 0x00000000 ]
+  [ meta load mark => reg 2 ]
+  [ bitwise reg 2 = ( reg 2 & 0x0000ffff ) ^ 0x00000000 ]
+  [ bitwise reg 1 = ( reg 1 | reg 2 ) ]
+  [ meta set mark with reg 1 ]
+
 # meta protocol ip udp dport 67
 inet test-inet input
   [ meta load protocol => reg 1 ]
diff --git a/tests/py/ip/ct.t b/tests/py/ip/ct.t
index cfd9859c26b3..b13c58d2df72 100644
--- a/tests/py/ip/ct.t
+++ b/tests/py/ip/ct.t
@@ -30,3 +30,4 @@ ct original saddr . meta mark { 1.1.1.1 . 0x00000014 };fail
 ct original ip saddr . meta mark { 1.1.1.1 . 0x00000014 };ok
 ct mark set ip dscp lshift 2 or 0x10;ok;ct mark set ip dscp << 2 | 16
 ct mark set ip dscp lshift 26 or 0x10;ok;ct mark set ip dscp << 26 | 16
+ct mark set ct mark or ip dscp or 0x200 counter;ok;ct mark set ct mark | ip dscp | 0x00000200 counter
diff --git a/tests/py/ip/ct.t.json b/tests/py/ip/ct.t.json
index d0df36f1d060..6abaa4c19e04 100644
--- a/tests/py/ip/ct.t.json
+++ b/tests/py/ip/ct.t.json
@@ -383,3 +383,39 @@
         }
     }
 ]
+
+# ct mark set ct mark or ip dscp or 0x200 counter
+[
+    {
+        "mangle": {
+            "key": {
+                "ct": {
+                    "key": "mark"
+                }
+            },
+            "value": {
+                "|": [
+                    {
+                        "|": [
+                            {
+                                "ct": {
+                                    "key": "mark"
+                                }
+                            },
+                            {
+                                "payload": {
+                                    "field": "dscp",
+                                    "protocol": "ip"
+                                }
+                            }
+                        ]
+                    },
+                    512
+                ]
+            }
+        }
+    },
+    {
+        "counter": null
+    }
+]
diff --git a/tests/py/ip/ct.t.payload b/tests/py/ip/ct.t.payload
index b2aed170833e..c2340b2e5fc6 100644
--- a/tests/py/ip/ct.t.payload
+++ b/tests/py/ip/ct.t.payload
@@ -102,3 +102,14 @@ ip
   [ bitwise reg 1 = ( reg 1 << 0x0000001a ) ]
   [ bitwise reg 1 = ( reg 1 & 0xffffffef ) ^ 0x00000010 ]
   [ ct set mark with reg 1 ]
+
+# ct mark set ct mark or ip dscp or 0x200 counter
+ip test-ip4 output
+  [ ct load mark => reg 1 ]
+  [ payload load 1b @ network header + 1 => reg 2 ]
+  [ bitwise reg 2 = ( reg 2 & 0x000000fc ) ^ 0x00000000 ]
+  [ bitwise reg 2 = ( reg 2 >> 0x00000002 ) ]
+  [ bitwise reg 1 = ( reg 1 | reg 2 ) ]
+  [ bitwise reg 1 = ( reg 1 & 0xfffffdff ) ^ 0x00000200 ]
+  [ ct set mark with reg 1 ]
+  [ counter pkts 0 bytes 0 ]
diff --git a/tests/py/ip/ip.t b/tests/py/ip/ip.t
index d5a4d8a5e46e..6ef1be3a8ddb 100644
--- a/tests/py/ip/ip.t
+++ b/tests/py/ip/ip.t
@@ -124,6 +124,8 @@ iif "lo" ip protocol set 1;ok
 
 iif "lo" ip dscp set af23;ok
 iif "lo" ip dscp set cs0;ok
+iif "lo" ip dscp set (meta mark & 0xfc000000) >> 26;ok
+iif "lo" ip dscp set ip dscp & 0xc;ok;iif "lo" ip dscp set ip dscp & af12
 
 ip saddr . ip daddr { 192.0.2.1 . 10.0.0.1-10.0.0.2 };ok
 ip saddr . ip daddr vmap { 192.168.5.1-192.168.5.128 . 192.168.6.1-192.168.6.128 : accept };ok
diff --git a/tests/py/ip/ip.t.json b/tests/py/ip/ip.t.json
index b1085035a000..1adbf9323b7a 100644
--- a/tests/py/ip/ip.t.json
+++ b/tests/py/ip/ip.t.json
@@ -1596,6 +1596,82 @@
     }
 ]
 
+# iif "lo" ip dscp set (meta mark & 0xfc000000) >> 26
+[
+    {
+        "match": {
+            "left": {
+                "meta": {
+                    "key": "iif"
+                }
+            },
+            "op": "==",
+            "right": "lo"
+        }
+    },
+    {
+        "mangle": {
+            "key": {
+                "payload": {
+                    "field": "dscp",
+                    "protocol": "ip"
+                }
+            },
+            "value": {
+                ">>": [
+                    {
+                        "&": [
+                            {
+                                "meta": {
+                                    "key": "mark"
+                                }
+                            },
+                            4227858432
+                        ]
+                    },
+                    26
+                ]
+            }
+        }
+    }
+]
+
+# iif "lo" ip dscp set ip dscp & 0xc
+[
+    {
+        "match": {
+            "left": {
+                "meta": {
+                    "key": "iif"
+                }
+            },
+            "op": "==",
+            "right": "lo"
+        }
+    },
+    {
+        "mangle": {
+            "key": {
+                "payload": {
+                    "field": "dscp",
+                    "protocol": "ip"
+                }
+            },
+            "value": {
+                "&": [
+                    {
+                        "payload": {
+                            "field": "dscp",
+                            "protocol": "ip"
+                        }
+                    },
+                    "af12"
+                ]
+            }
+        }
+    }
+]
+
 # ip saddr . ip daddr { 192.0.2.1 . 10.0.0.1-10.0.0.2 }
 [
     {
@@ -1684,4 +1760,3 @@
         }
     }
 ]
-
diff --git a/tests/py/ip/ip.t.payload b/tests/py/ip/ip.t.payload
index b9fcb5158e9d..7e955d07ebc9 100644
--- a/tests/py/ip/ip.t.payload
+++ b/tests/py/ip/ip.t.payload
@@ -490,6 +490,34 @@ ip test-ip4 input
   [ bitwise reg 1 = ( reg 1 & 0x000003ff ) ^ 0x00000000 ]
   [ payload write reg 1 => 2b @ network header + 0 csum_type 1 csum_off 10 csum_flags 0x0 ]
 
+# iif "lo" ip dscp set (meta mark & 0xfc000000) >> 26
+ip test-ip4 input
+  [ meta load iif => reg 1 ]
+  [ cmp eq reg 1 0x00000001 ]
+  [ payload load 2b @ network header + 0 => reg 1 ]
+  [ bitwise reg 1 = ( reg 1 & 0x000003ff ) ^ 0x00000000 ]
+  [ meta load mark => reg 2 ]
+  [ bitwise reg 2 = ( reg 2 & 0xfc000000 ) ^ 0x00000000 ]
+  [ bitwise reg 2 = ( reg 2 >> 0x0000001a ) ]
+  [ bitwise reg 2 = ( reg 2 << 0x00000002 ) ]
+  [ byteorder reg 2 = hton(reg 2, 4, 4) ]
+  [ bitwise reg 1 = ( reg 1 ^ reg 2 ) ]
+  [ payload write reg 1 => 2b @ network header + 0 csum_type 1 csum_off 10 csum_flags 0x0 ]
+
+# iif "lo" ip dscp set ip dscp & 0xc
+ip test-ip4 input
+  [ meta load iif => reg 1 ]
+  [ cmp eq reg 1 0x00000001 ]
+  [ payload load 2b @ network header + 0 => reg 1 ]
+  [ bitwise reg 1 = ( reg 1 & 0x000003ff ) ^ 0x00000000 ]
+  [ payload load 1b @ network header + 1 => reg 2 ]
+  [ bitwise reg 2 = ( reg 2 & 0x000000fc ) ^ 0x00000000 ]
+  [ bitwise reg 2 = ( reg 2 >> 0x00000002 ) ]
+  [ bitwise reg 2 = ( reg 2 & 0x0000000c ) ^ 0x00000000 ]
+  [ bitwise reg 2 = ( reg 2 << 0x00000002 ) ]
+  [ bitwise reg 1 = ( reg 1 ^ reg 2 ) ]
+  [ payload write reg 1 => 2b @ network header + 0 csum_type 1 csum_off 10 csum_flags 0x0 ]
+
 # iif "lo" ip ttl set 23
 ip test-ip4 input
   [ meta load iif => reg 1 ]
diff --git a/tests/py/ip/ip.t.payload.bridge b/tests/py/ip/ip.t.payload.bridge
index c6f8d4e5575b..fd3603a68e9b 100644
--- a/tests/py/ip/ip.t.payload.bridge
+++ b/tests/py/ip/ip.t.payload.bridge
@@ -662,6 +662,38 @@ bridge test-bridge input
   [ bitwise reg 1 = ( reg 1 & 0x000003ff ) ^ 0x00000000 ]
   [ payload write reg 1 => 2b @ network header + 0 csum_type 1 csum_off 10 csum_flags 0x0 ]
 
+# iif "lo" ip dscp set (meta mark & 0xfc000000) >> 26
+bridge test-bridge input
+  [ meta load iif => reg 1 ]
+  [ cmp eq reg 1 0x00000001 ]
+  [ meta load protocol => reg 1 ]
+  [ cmp eq reg 1 0x00000008 ]
+  [ payload load 2b @ network header + 0 => reg 1 ]
+  [ bitwise reg 1 = ( reg 1 & 0x000003ff ) ^ 0x00000000 ]
+  [ meta load mark => reg 2 ]
+  [ bitwise reg 2 = ( reg 2 & 0xfc000000 ) ^ 0x00000000 ]
+  [ bitwise reg 2 = ( reg 2 >> 0x0000001a ) ]
+  [ bitwise reg 2 = ( reg 2 << 0x00000002 ) ]
+  [ byteorder reg 2 = hton(reg 2, 4, 4) ]
+  [ bitwise reg 1 = ( reg 1 ^ reg 2 ) ]
+  [ payload write reg 1 => 2b @ network header + 0 csum_type 1 csum_off 10 csum_flags 0x0 ]
+
+# iif "lo" ip dscp set ip dscp & 0xc
+bridge test-bridge input
+  [ meta load iif => reg 1 ]
+  [ cmp eq reg 1 0x00000001 ]
+  [ meta load protocol => reg 1 ]
+  [ cmp eq reg 1 0x00000008 ]
+  [ payload load 2b @ network header + 0 => reg 1 ]
+  [ bitwise reg 1 = ( reg 1 & 0x000003ff ) ^ 0x00000000 ]
+  [ payload load 1b @ network header + 1 => reg 2 ]
+  [ bitwise reg 2 = ( reg 2 & 0x000000fc ) ^ 0x00000000 ]
+  [ bitwise reg 2 = ( reg 2 >> 0x00000002 ) ]
+  [ bitwise reg 2 = ( reg 2 & 0x0000000c ) ^ 0x00000000 ]
+  [ bitwise reg 2 = ( reg 2 << 0x00000002 ) ]
+  [ bitwise reg 1 = ( reg 1 ^ reg 2 ) ]
+  [ payload write reg 1 => 2b @ network header + 0 csum_type 1 csum_off 10 csum_flags 0x0 ]
+
 # ip saddr . ip daddr { 192.0.2.1 . 10.0.0.1-10.0.0.2 }
 __set%d test-bridge 87 size 1
 __set%d test-bridge 0
diff --git a/tests/py/ip/ip.t.payload.inet b/tests/py/ip/ip.t.payload.inet
index e26d0dac47be..7f92423ab051 100644
--- a/tests/py/ip/ip.t.payload.inet
+++ b/tests/py/ip/ip.t.payload.inet
@@ -642,6 +642,38 @@ inet test-inet input
   [ bitwise reg 1 = ( reg 1 & 0x000003ff ) ^ 0x00000000 ]
   [ payload write reg 1 => 2b @ network header + 0 csum_type 1 csum_off 10 csum_flags 0x0 ]
 
+# iif "lo" ip dscp set (meta mark & 0xfc000000) >> 26
+inet test-inet input
+  [ meta load iif => reg 1 ]
+  [ cmp eq reg 1 0x00000001 ]
+  [ meta load nfproto => reg 1 ]
+  [ cmp eq reg 1 0x00000002 ]
+  [ payload load 2b @ network header + 0 => reg 1 ]
+  [ bitwise reg 1 = ( reg 1 & 0x000003ff ) ^ 0x00000000 ]
+  [ meta load mark => reg 2 ]
+  [ bitwise reg 2 = ( reg 2 & 0xfc000000 ) ^ 0x00000000 ]
+  [ bitwise reg 2 = ( reg 2 >> 0x0000001a ) ]
+  [ bitwise reg 2 = ( reg 2 << 0x00000002 ) ]
+  [ byteorder reg 2 = hton(reg 2, 4, 4) ]
+  [ bitwise reg 1 = ( reg 1 ^ reg 2 ) ]
+  [ payload write reg 1 => 2b @ network header + 0 csum_type 1 csum_off 10 csum_flags 0x0 ]
+
+# iif "lo" ip dscp set ip dscp & 0xc
+inet test-inet input
+  [ meta load iif => reg 1 ]
+  [ cmp eq reg 1 0x00000001 ]
+  [ meta load nfproto => reg 1 ]
+  [ cmp eq reg 1 0x00000002 ]
+  [ payload load 2b @ network header + 0 => reg 1 ]
+  [ bitwise reg 1 = ( reg 1 & 0x000003ff ) ^ 0x00000000 ]
+  [ payload load 1b @ network header + 1 => reg 2 ]
+  [ bitwise reg 2 = ( reg 2 & 0x000000fc ) ^ 0x00000000 ]
+  [ bitwise reg 2 = ( reg 2 >> 0x00000002 ) ]
+  [ bitwise reg 2 = ( reg 2 & 0x0000000c ) ^ 0x00000000 ]
+  [ bitwise reg 2 = ( reg 2 << 0x00000002 ) ]
+  [ bitwise reg 1 = ( reg 1 ^ reg 2 ) ]
+  [ payload write reg 1 => 2b @ network header + 0 csum_type 1 csum_off 10 csum_flags 0x0 ]
+
 # iif "lo" ip ttl set 23
 inet test-inet input
   [ meta load iif => reg 1 ]
diff --git a/tests/py/ip/ip.t.payload.netdev b/tests/py/ip/ip.t.payload.netdev
index de990f5bba12..74fc696f31fe 100644
--- a/tests/py/ip/ip.t.payload.netdev
+++ b/tests/py/ip/ip.t.payload.netdev
@@ -642,6 +642,38 @@ netdev test-netdev ingress
   [ bitwise reg 1 = ( reg 1 & 0x000003ff ) ^ 0x00000000 ]
   [ payload write reg 1 => 2b @ network header + 0 csum_type 1 csum_off 10 csum_flags 0x0 ]
 
+# iif "lo" ip dscp set (meta mark & 0xfc000000) >> 26
+netdev test-netdev ingress
+  [ meta load iif => reg 1 ]
+  [ cmp eq reg 1 0x00000001 ]
+  [ meta load protocol => reg 1 ]
+  [ cmp eq reg 1 0x00000008 ]
+  [ payload load 2b @ network header + 0 => reg 1 ]
+  [ bitwise reg 1 = ( reg 1 & 0x000003ff ) ^ 0x00000000 ]
+  [ meta load mark => reg 2 ]
+  [ bitwise reg 2 = ( reg 2 & 0xfc000000 ) ^ 0x00000000 ]
+  [ bitwise reg 2 = ( reg 2 >> 0x0000001a ) ]
+  [ bitwise reg 2 = ( reg 2 << 0x00000002 ) ]
+  [ byteorder reg 2 = hton(reg 2, 4, 4) ]
+  [ bitwise reg 1 = ( reg 1 ^ reg 2 ) ]
+  [ payload write reg 1 => 2b @ network header + 0 csum_type 1 csum_off 10 csum_flags 0x0 ]
+
+# iif "lo" ip dscp set ip dscp & 0xc
+netdev test-netdev ingress
+  [ meta load iif => reg 1 ]
+  [ cmp eq reg 1 0x00000001 ]
+  [ meta load protocol => reg 1 ]
+  [ cmp eq reg 1 0x00000008 ]
+  [ payload load 2b @ network header + 0 => reg 1 ]
+  [ bitwise reg 1 = ( reg 1 & 0x000003ff ) ^ 0x00000000 ]
+  [ payload load 1b @ network header + 1 => reg 2 ]
+  [ bitwise reg 2 = ( reg 2 & 0x000000fc ) ^ 0x00000000 ]
+  [ bitwise reg 2 = ( reg 2 >> 0x00000002 ) ]
+  [ bitwise reg 2 = ( reg 2 & 0x0000000c ) ^ 0x00000000 ]
+  [ bitwise reg 2 = ( reg 2 << 0x00000002 ) ]
+  [ bitwise reg 1 = ( reg 1 ^ reg 2 ) ]
+  [ payload write reg 1 => 2b @ network header + 0 csum_type 1 csum_off 10 csum_flags 0x0 ]
+
 # iif "lo" ip ttl set 23
 netdev test-netdev ingress
   [ meta load iif => reg 1 ]
diff --git a/tests/py/ip6/ct.t b/tests/py/ip6/ct.t
index 0a141ffaf961..782353666d0f 100644
--- a/tests/py/ip6/ct.t
+++ b/tests/py/ip6/ct.t
@@ -4,3 +4,4 @@
 
 ct mark set ip6 dscp lshift 2 or 0x10;ok;ct mark set ip6 dscp << 2 | 16
 ct mark set ip6 dscp lshift 26 or 0x10;ok;ct mark set ip6 dscp << 26 | 16
+ct mark set ct mark or ip6 dscp or 0x200 counter;ok;ct mark set ct mark | ip6 dscp | 0x00000200 counter
diff --git a/tests/py/ip6/ct.t.json b/tests/py/ip6/ct.t.json
index 7739e131343e..d1753b5d2a17 100644
--- a/tests/py/ip6/ct.t.json
+++ b/tests/py/ip6/ct.t.json
@@ -56,3 +56,38 @@
     }
 ]
 
+# ct mark set ct mark or ip6 dscp or 0x200 counter
+[
+    {
+        "mangle": {
+            "key": {
+                "ct": {
+                    "key": "mark"
+                }
+            },
+            "value": {
+                "|": [
+                    {
+                        "|": [
+                            {
+                                "ct": {
+                                    "key": "mark"
+                                }
+                            },
+                            {
+                                "payload": {
+                                    "field": "dscp",
+                                    "protocol": "ip6"
+                                }
+                            }
+                        ]
+                    },
+                    512
+                ]
+            }
+        }
+    },
+    {
+        "counter": null
+    }
+]
diff --git a/tests/py/ip6/ct.t.payload b/tests/py/ip6/ct.t.payload
index a0565d14e15e..861e330f2df2 100644
--- a/tests/py/ip6/ct.t.payload
+++ b/tests/py/ip6/ct.t.payload
@@ -17,3 +17,15 @@ ip6 test-ip6 output
   [ bitwise reg 1 = ( reg 1 << 0x0000001a ) ]
   [ bitwise reg 1 = ( reg 1 & 0xffffffef ) ^ 0x00000010 ]
   [ ct set mark with reg 1 ]
+
+# ct mark set ct mark or ip6 dscp or 0x200 counter
+ip6 test-ip6 output
+  [ ct load mark => reg 1 ]
+  [ payload load 2b @ network header + 0 => reg 2 ]
+  [ bitwise reg 2 = ( reg 2 & 0x0000c00f ) ^ 0x00000000 ]
+  [ bitwise reg 2 = ( reg 2 >> 0x00000006 ) ]
+  [ byteorder reg 2 = ntoh(reg 2, 2, 1) ]
+  [ bitwise reg 1 = ( reg 1 | reg 2 ) ]
+  [ bitwise reg 1 = ( reg 1 & 0xfffffdff ) ^ 0x00000200 ]
+  [ ct set mark with reg 1 ]
+  [ counter pkts 0 bytes 0 ]
diff --git a/tests/py/ip6/ip6.t b/tests/py/ip6/ip6.t
index 2ffe318e1e6d..6222ffb7d885 100644
--- a/tests/py/ip6/ip6.t
+++ b/tests/py/ip6/ip6.t
@@ -141,6 +141,8 @@ iif "lo" ip6 daddr set ::1;ok
 iif "lo" ip6 hoplimit set 1;ok
 iif "lo" ip6 dscp set af42;ok
 iif "lo" ip6 dscp set 63;ok;iif "lo" ip6 dscp set 0x3f
+iif "lo" ip6 dscp set (ct mark & 0xfc000000) >> 26;ok
+iif "lo" ip6 dscp set ip6 dscp & 0xc;ok;iif "lo" ip6 dscp set ip6 dscp & af12
 iif "lo" ip6 ecn set ect0;ok
 iif "lo" ip6 ecn set ce;ok
 
diff --git a/tests/py/ip6/ip6.t.json b/tests/py/ip6/ip6.t.json
index cf802175b792..b9658274968a 100644
--- a/tests/py/ip6/ip6.t.json
+++ b/tests/py/ip6/ip6.t.json
@@ -1437,6 +1437,82 @@
     }
 ]
 
+# iif "lo" ip6 dscp set (ct mark & 0xfc000000) >> 26
+[
+    {
+        "match": {
+            "left": {
+                "meta": {
+                    "key": "iif"
+                }
+            },
+            "op": "==",
+            "right": "lo"
+        }
+    },
+    {
+        "mangle": {
+            "key": {
+                "payload": {
+                    "field": "dscp",
+                    "protocol": "ip6"
+                }
+            },
+            "value": {
+                ">>": [
+                    {
+                        "&": [
+                            {
+                                "ct": {
+                                    "key": "mark"
+                                }
+                            },
+                            4227858432
+                        ]
+                    },
+                    26
+                ]
+            }
+        }
+    }
+]
+
+# iif "lo" ip6 dscp set ip6 dscp & 0xc
+[
+    {
+        "match": {
+            "left": {
+                "meta": {
+                    "key": "iif"
+                }
+            },
+            "op": "==",
+            "right": "lo"
+        }
+    },
+    {
+        "mangle": {
+            "key": {
+                "payload": {
+                    "field": "dscp",
+                    "protocol": "ip6"
+                }
+            },
+            "value": {
+                "&": [
+                    {
+                        "payload": {
+                            "field": "dscp",
+                            "protocol": "ip6"
+                        }
+                    },
+                    "af12"
+                ]
+            }
+        }
+    }
+]
+
 # iif "lo" ip6 ecn set ect0
 [
     {
diff --git a/tests/py/ip6/ip6.t.payload.inet b/tests/py/ip6/ip6.t.payload.inet
index 20dfe5497367..83a95861bc06 100644
--- a/tests/py/ip6/ip6.t.payload.inet
+++ b/tests/py/ip6/ip6.t.payload.inet
@@ -589,6 +589,40 @@ inet test-inet input
   [ bitwise reg 1 = ( reg 1 & 0x00003ff0 ) ^ 0x0000c00f ]
   [ payload write reg 1 => 2b @ network header + 0 csum_type 0 csum_off 0 csum_flags 0x0 ]
 
+# iif "lo" ip6 dscp set (ct mark & 0xfc000000) >> 26
+inet test-inet input
+  [ meta load iif => reg 1 ]
+  [ cmp eq reg 1 0x00000001 ]
+  [ meta load nfproto => reg 1 ]
+  [ cmp eq reg 1 0x0000000a ]
+  [ payload load 2b @ network header + 0 => reg 1 ]
+  [ bitwise reg 1 = ( reg 1 & 0x00003ff0 ) ^ 0x00000000 ]
+  [ ct load mark => reg 2 ]
+  [ bitwise reg 2 = ( reg 2 & 0xfc000000 ) ^ 0x00000000 ]
+  [ bitwise reg 2 = ( reg 2 >> 0x0000001a ) ]
+  [ bitwise reg 2 = ( reg 2 << 0x00000006 ) ]
+  [ byteorder reg 2 = hton(reg 2, 4, 4) ]
+  [ bitwise reg 1 = ( reg 1 ^ reg 2 ) ]
+  [ payload write reg 1 => 2b @ network header + 0 csum_type 0 csum_off 0 csum_flags 0x0 ]
+
+# iif "lo" ip6 dscp set ip6 dscp & 0xc
+inet test-inet input
+  [ meta load iif => reg 1 ]
+  [ cmp eq reg 1 0x00000001 ]
+  [ meta load nfproto => reg 1 ]
+  [ cmp eq reg 1 0x0000000a ]
+  [ payload load 2b @ network header + 0 => reg 1 ]
+  [ bitwise reg 1 = ( reg 1 & 0x00003ff0 ) ^ 0x00000000 ]
+  [ payload load 2b @ network header + 0 => reg 2 ]
+  [ bitwise reg 2 = ( reg 2 & 0x0000c00f ) ^ 0x00000000 ]
+  [ byteorder reg 2 = ntoh(reg 2, 2, 1) ]
+  [ bitwise reg 2 = ( reg 2 >> 0x00000006 ) ]
+  [ bitwise reg 2 = ( reg 2 & 0x0000000c ) ^ 0x00000000 ]
+  [ bitwise reg 2 = ( reg 2 << 0x00000006 ) ]
+  [ byteorder reg 2 = hton(reg 2, 2, 1) ]
+  [ bitwise reg 1 = ( reg 1 ^ reg 2 ) ]
+  [ payload write reg 1 => 2b @ network header + 0 csum_type 0 csum_off 0 csum_flags 0x0 ]
+
 # iif "lo" ip6 ecn set ect0
 inet test-inet input
   [ meta load iif => reg 1 ]
diff --git a/tests/py/ip6/ip6.t.payload.ip6 b/tests/py/ip6/ip6.t.payload.ip6
index f8e3ca3cb622..240dfd2d8d35 100644
--- a/tests/py/ip6/ip6.t.payload.ip6
+++ b/tests/py/ip6/ip6.t.payload.ip6
@@ -439,6 +439,36 @@ ip6 test-ip6 input
   [ bitwise reg 1 = ( reg 1 & 0x00003ff0 ) ^ 0x0000c00f ]
   [ payload write reg 1 => 2b @ network header + 0 csum_type 0 csum_off 0 csum_flags 0x0 ]
 
+# iif "lo" ip6 dscp set (ct mark & 0xfc000000) >> 26
+ip6 test-ip6 input
+  [ meta load iif => reg 1 ]
+  [ cmp eq reg 1 0x00000001 ]
+  [ payload load 2b @ network header + 0 => reg 1 ]
+  [ bitwise reg 1 = ( reg 1 & 0x00003ff0 ) ^ 0x00000000 ]
+  [ ct load mark => reg 2 ]
+  [ bitwise reg 2 = ( reg 2 & 0xfc000000 ) ^ 0x00000000 ]
+  [ bitwise reg 2 = ( reg 2 >> 0x0000001a ) ]
+  [ bitwise reg 2 = ( reg 2 << 0x00000006 ) ]
+  [ byteorder reg 2 = hton(reg 2, 4, 4) ]
+  [ bitwise reg 1 = ( reg 1 ^ reg 2 ) ]
+  [ payload write reg 1 => 2b @ network header + 0 csum_type 0 csum_off 0 csum_flags 0x0 ]
+
+# iif "lo" ip6 dscp set ip6 dscp & 0xc
+ip6 test-ip6 input
+  [ meta load iif => reg 1 ]
+  [ cmp eq reg 1 0x00000001 ]
+  [ payload load 2b @ network header + 0 => reg 1 ]
+  [ bitwise reg 1 = ( reg 1 & 0x00003ff0 ) ^ 0x00000000 ]
+  [ payload load 2b @ network header + 0 => reg 2 ]
+  [ bitwise reg 2 = ( reg 2 & 0x0000c00f ) ^ 0x00000000 ]
+  [ byteorder reg 2 = ntoh(reg 2, 2, 1) ]
+  [ bitwise reg 2 = ( reg 2 >> 0x00000006 ) ]
+  [ bitwise reg 2 = ( reg 2 & 0x0000000c ) ^ 0x00000000 ]
+  [ bitwise reg 2 = ( reg 2 << 0x00000006 ) ]
+  [ byteorder reg 2 = hton(reg 2, 2, 1) ]
+  [ bitwise reg 1 = ( reg 1 ^ reg 2 ) ]
+  [ payload write reg 1 => 2b @ network header + 0 csum_type 0 csum_off 0 csum_flags 0x0 ]
+
 # iif "lo" ip6 ecn set ect0
 ip6 test-ip6 input
   [ meta load iif => reg 1 ]
-- 
2.35.1


^ permalink raw reply related	[flat|nested] 50+ messages in thread

* Re: [nft PATCH v4 05/32] ct: support `NULL` symbol-tables when looking up labels
  2022-04-04 12:13 ` [nft PATCH v4 05/32] ct: support `NULL` symbol-tables when looking up labels Jeremy Sowden
@ 2022-04-05 11:15   ` Florian Westphal
  2022-04-05 15:29     ` Jeremy Sowden
  0 siblings, 1 reply; 50+ messages in thread
From: Florian Westphal @ 2022-04-05 11:15 UTC (permalink / raw)
  To: Jeremy Sowden; +Cc: Netfilter Devel, Kevin Darbyshire-Bryant

Jeremy Sowden <jeremy@azazel.net> wrote:
> If the symbol-table passed to `ct_label2str` is `NULL`, return `NULL`.

It would be nice to describe why, not what.
Does this fix a crash when the label file is missing?

^ permalink raw reply	[flat|nested] 50+ messages in thread

* Re: [nft PATCH v4 01/32] examples: add .gitignore file
  2022-04-04 12:13 ` [nft PATCH v4 01/32] examples: add .gitignore file Jeremy Sowden
@ 2022-04-05 11:26   ` Florian Westphal
  0 siblings, 0 replies; 50+ messages in thread
From: Florian Westphal @ 2022-04-05 11:26 UTC (permalink / raw)
  To: Jeremy Sowden; +Cc: Netfilter Devel, Kevin Darbyshire-Bryant

Jeremy Sowden <jeremy@azazel.net> wrote:

Applied this one and the next, will go over this series later today.

^ permalink raw reply	[flat|nested] 50+ messages in thread

* Re: [nft PATCH v4 05/32] ct: support `NULL` symbol-tables when looking up labels
  2022-04-05 11:15   ` Florian Westphal
@ 2022-04-05 15:29     ` Jeremy Sowden
  0 siblings, 0 replies; 50+ messages in thread
From: Jeremy Sowden @ 2022-04-05 15:29 UTC (permalink / raw)
  To: Florian Westphal; +Cc: Netfilter Devel, Kevin Darbyshire-Bryant

[-- Attachment #1: Type: text/plain, Size: 1563 bytes --]

On 2022-04-05, at 13:15:45 +0200, Florian Westphal wrote:
> Jeremy Sowden <jeremy@azazel.net> wrote:
> > If the symbol-table passed to `ct_label2str` is `NULL`, return `NULL`.
> 
> It would be nice to describe why, not what.
> Does this fix a crash when the label file is missing?

I've been putting debug logging in various places while developing and
testing things.  Here's a function for dumping expressions to stderr:

  static inline void
  dbg_print_expr(const char *func, const char *name,
                 const struct expr *expr)
  {
    struct output_ctx *dbgctx = &(struct output_ctx) { .output_fp = stderr };

    nft_print(dbgctx, "%s: %s = (%s, %u, %s, %s) { ",
              func, name,
              expr_ops(expr)->name,
              expr->len,
              expr->dtype ? expr->dtype->name : "",
              byteorder_names[expr->byteorder]);
    expr_print(expr, dbgctx);
    nft_print(dbgctx, " }\n");
  }

There are two problems with this.  Firstly, the `byteorder_names` array
is defined in src/evaluate.c and so this function cannot be used
elsewhere.  Secondly, the symbol-tables in the output context are not
initialized, which results in crashes when trying to print symbolic
constants and CT labels.  Patch 3 fixes the former problem by moving the
`byteorder_names` array, and patches 4 & 5 fix the latter problem by
adding NULL checks before trying to dereference the symbol-tables.  They
seemed like small, low-impact changes that could be upstreamed, so that
I didn't have to carry them.

J.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

^ permalink raw reply	[flat|nested] 50+ messages in thread

* Re: [nft PATCH v4 00/32] Extend values assignable to packet marks and payload fields
  2022-04-04 12:13 [nft PATCH v4 00/32] Extend values assignable to packet marks and payload fields Jeremy Sowden
                   ` (31 preceding siblings ...)
  2022-04-04 12:14 ` [nft PATCH v4 32/32] tests: py: " Jeremy Sowden
@ 2022-04-09  8:30 ` Kevin 'ldir' Darbyshire-Bryant
  32 siblings, 0 replies; 50+ messages in thread
From: Kevin 'ldir' Darbyshire-Bryant @ 2022-04-09  8:30 UTC (permalink / raw)
  To: Netfilter Devel, Jeremy Sowden; +Cc: Jeremy Sowden

[-- Attachment #1: Type: text/plain, Size: 773 bytes --]



> On 4 Apr 2022, at 13:13, Jeremy Sowden <jeremy@azazel.net> wrote:
> 
> This patch-set extends the types of value which can be assigned to
> packet marks and payload fields.  The original motivation for these
> changes was Kevin Darbyshire-Bryant's wish to be able to set the
> conntrack mark to a bitwise expression derived from a DSCP value:
> 
>  https://lore.kernel.org/netfilter-devel/20191203160652.44396-1-ldir@darbyshire-bryant.me.uk/#r
> 
> For example:
> 
>  nft add rule t c ct mark set ip dscp lshift 26 or 0x10

And I’d still like to be able to do the same/similar thing :-)

Thank you Jeremy for your continued work on this, so far beyond my ability.

Cheers,

Kevin D-B

gpg: 012C ACB2 28C6 C53E 9775  9123 B3A2 389B 9DE2 334A


[-- Attachment #2: Message signed with OpenPGP --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

^ permalink raw reply	[flat|nested] 50+ messages in thread

* Re: [nft PATCH v4 08/32] netlink: send bit-length of bitwise binops to kernel
  2022-04-04 12:13 ` [nft PATCH v4 08/32] netlink: send bit-length of bitwise binops to kernel Jeremy Sowden
@ 2022-05-23 17:03   ` Pablo Neira Ayuso
  2022-11-01 18:46     ` Jeremy Sowden
  0 siblings, 1 reply; 50+ messages in thread
From: Pablo Neira Ayuso @ 2022-05-23 17:03 UTC (permalink / raw)
  To: Jeremy Sowden; +Cc: Netfilter Devel, Kevin Darbyshire-Bryant

Hi,

I'm sorry for taking time to get back to this, I have questions.

On Mon, Apr 04, 2022 at 01:13:46PM +0100, Jeremy Sowden wrote:
> Some bitwise operations are generated when munging paylod expressions.
> During delinearization, we attempt to eliminate these operations.
> However, this is done before deducing the byte-order or the correct
> length in bits of the operands, which means that we don't always handle
> multi-byte host-endian operations correctly.  Therefore, pass the
> bit-length of these expressions to the kernel in order to have it
> available during delinearization.
> 
> Signed-off-by: Jeremy Sowden <jeremy@azazel.net>
> ---
>  src/netlink_delinearize.c | 14 ++++++++++++--
>  src/netlink_linearize.c   |  2 ++
>  2 files changed, 14 insertions(+), 2 deletions(-)
> 
> diff --git a/src/netlink_delinearize.c b/src/netlink_delinearize.c
> index a1b00dee209a..733977bc526d 100644
> --- a/src/netlink_delinearize.c
> +++ b/src/netlink_delinearize.c
> @@ -451,20 +451,28 @@ static struct expr *netlink_parse_bitwise_bool(struct netlink_parse_ctx *ctx,
>  					       const struct nftnl_expr *nle,
>  					       enum nft_registers sreg,
>  					       struct expr *left)
> -
>  {
>  	struct nft_data_delinearize nld;
>  	struct expr *expr, *mask, *xor, *or;
> +	unsigned int nbits;
>  	mpz_t m, x, o;
>  
>  	expr = left;
>  
> +	nbits = nftnl_expr_get_u32(nle, NFTNL_EXPR_BITWISE_NBITS);
> +	if (nbits > 0)
> +		expr->len = nbits;

So NFTNL_EXPR_BITWISE_NBITS is signalling that this is an implicit
bitwise that has been generated to operate with a payload header bitfield?

Could you provide an example expression tree to show how this
simplifies delinearization?

> +
>  	nld.value = nftnl_expr_get(nle, NFTNL_EXPR_BITWISE_MASK, &nld.len);
>  	mask = netlink_alloc_value(loc, &nld);
> +	if (nbits > 0)
> +		mpz_switch_byteorder(mask->value, div_round_up(nbits, BITS_PER_BYTE));

What is the byteorder expected for the mask before this switch
operation?

>  	mpz_init_set(m, mask->value);
>  
>  	nld.value = nftnl_expr_get(nle, NFTNL_EXPR_BITWISE_XOR, &nld.len);
> -	xor  = netlink_alloc_value(loc, &nld);
> +	xor = netlink_alloc_value(loc, &nld);
> +	if (nbits > 0)
> +		mpz_switch_byteorder(xor->value, div_round_up(nbits, BITS_PER_BYTE));
>  	mpz_init_set(x, xor->value);
>  
>  	mpz_init_set_ui(o, 0);
> @@ -500,6 +508,8 @@ static struct expr *netlink_parse_bitwise_bool(struct netlink_parse_ctx *ctx,
>  
>  		or = netlink_alloc_value(loc, &nld);
>  		mpz_set(or->value, o);
> +		if (nbits > 0)
> +			mpz_switch_byteorder(or->value, div_round_up(nbits, BITS_PER_BYTE));
>  		expr = binop_expr_alloc(loc, OP_OR, expr, or);
>  		expr->len = left->len;
>  	}
> diff --git a/src/netlink_linearize.c b/src/netlink_linearize.c
> index c8bbcb7452b0..4793f3853bee 100644
> --- a/src/netlink_linearize.c
> +++ b/src/netlink_linearize.c
> @@ -677,6 +677,8 @@ static void netlink_gen_bitwise(struct netlink_linearize_ctx *ctx,
>  	netlink_put_register(nle, NFTNL_EXPR_BITWISE_DREG, dreg);
>  	nftnl_expr_set_u32(nle, NFTNL_EXPR_BITWISE_OP, NFT_BITWISE_BOOL);
>  	nftnl_expr_set_u32(nle, NFTNL_EXPR_BITWISE_LEN, len);
> +	if (expr->byteorder == BYTEORDER_HOST_ENDIAN)
> +		nftnl_expr_set_u32(nle, NFTNL_EXPR_BITWISE_NBITS, expr->len);

Why is this only required for host endian expressions?

>  	netlink_gen_raw_data(mask, expr->byteorder, len, &nld);
>  	nftnl_expr_set(nle, NFTNL_EXPR_BITWISE_MASK, nld.value, nld.len);
> -- 
> 2.35.1
> 

^ permalink raw reply	[flat|nested] 50+ messages in thread

* Re: [nft PATCH v4 09/32] netlink_delinearize: add postprocessing for payload binops
  2022-04-04 12:13 ` [nft PATCH v4 09/32] netlink_delinearize: add postprocessing for payload binops Jeremy Sowden
@ 2022-05-23 17:19   ` Pablo Neira Ayuso
  2022-11-01 18:46     ` Jeremy Sowden
  0 siblings, 1 reply; 50+ messages in thread
From: Pablo Neira Ayuso @ 2022-05-23 17:19 UTC (permalink / raw)
  To: Jeremy Sowden; +Cc: Netfilter Devel, Kevin Darbyshire-Bryant

On Mon, Apr 04, 2022 at 01:13:47PM +0100, Jeremy Sowden wrote:
> If a user uses a payload expression as a statement argument:
> 
>   nft add rule t c meta mark set ip dscp lshift 2 or 0x10
> 
> we may need to undo munging during delinearization.
> 
> Signed-off-by: Jeremy Sowden <jeremy@azazel.net>
> ---
>  src/netlink_delinearize.c | 39 +++++++++++++++++++++++++++++++++++++++
>  1 file changed, 39 insertions(+)
> 
> diff --git a/src/netlink_delinearize.c b/src/netlink_delinearize.c
> index 733977bc526d..12624db4c3a5 100644
> --- a/src/netlink_delinearize.c
> +++ b/src/netlink_delinearize.c
> @@ -2454,6 +2454,42 @@ static void relational_binop_postprocess(struct rule_pp_ctx *ctx,
>  	}
>  }
>  
> +static bool payload_binop_postprocess(struct rule_pp_ctx *ctx,
> +				      struct expr **exprp)
> +{
> +	struct expr *expr = *exprp;
> +
> +	if (expr->op != OP_RSHIFT)
> +		return false;
> +
> +	if (expr->left->etype == EXPR_UNARY) {
> +		/*
> +		 * If the payload value was originally in a different byte-order
> +		 * from the payload expression, there will be a byte-order
> +		 * conversion to remove.
> +		 */

The comment assumes this is a payload expression, the unary is
stripped off here...

> +		struct expr *left = expr_get(expr->left->arg);
> +		expr_free(expr->left);
> +		expr->left = left;
> +	}
> +
> +	if (expr->left->etype != EXPR_BINOP || expr->left->op != OP_AND)
> +		return false;
> +
> +	if (expr->left->left->etype != EXPR_PAYLOAD)

... but the check for payload is coming here.

I assume this postprocessing is to undo the switch from network
byteorder to host byteorder for the ip dscp of the example above?

Could you describe an example expression tree to depict this
delinearize scenario?

> +		return false;
> +
> +	expr_set_type(expr->right, &integer_type,
> +		      BYTEORDER_HOST_ENDIAN);
> +	expr_postprocess(ctx, &expr->right);
> +
> +	binop_postprocess(ctx, expr, &expr->left);
> +	*exprp = expr_get(expr->left);
> +	expr_free(expr);
> +
> +	return true;
> +}
> +
>  static struct expr *string_wildcard_expr_alloc(struct location *loc,
>  					       const struct expr *mask,
>  					       const struct expr *expr)
> @@ -2566,6 +2602,9 @@ static void expr_postprocess(struct rule_pp_ctx *ctx, struct expr **exprp)
>  		expr_set_type(expr, expr->arg->dtype, !expr->arg->byteorder);
>  		break;
>  	case EXPR_BINOP:
> +		if (payload_binop_postprocess(ctx, exprp))
> +			break;
> +
>  		expr_postprocess(ctx, &expr->left);
>  		switch (expr->op) {
>  		case OP_LSHIFT:
> -- 
> 2.35.1
> 

^ permalink raw reply	[flat|nested] 50+ messages in thread

* Re: [nft PATCH v4 10/32] netlink_delinearize: correct type and byte-order of shifts
  2022-04-04 12:13 ` [nft PATCH v4 10/32] netlink_delinearize: correct type and byte-order of shifts Jeremy Sowden
@ 2022-05-23 17:19   ` Pablo Neira Ayuso
  2022-11-01 18:47     ` Jeremy Sowden
  0 siblings, 1 reply; 50+ messages in thread
From: Pablo Neira Ayuso @ 2022-05-23 17:19 UTC (permalink / raw)
  To: Jeremy Sowden; +Cc: Netfilter Devel, Kevin Darbyshire-Bryant

On Mon, Apr 04, 2022 at 01:13:48PM +0100, Jeremy Sowden wrote:
> Shifts are of integer type and in HBO.
> 
> Signed-off-by: Jeremy Sowden <jeremy@azazel.net>
> ---
>  src/netlink_delinearize.c | 13 +++++++++++--
>  1 file changed, 11 insertions(+), 2 deletions(-)
> 
> diff --git a/src/netlink_delinearize.c b/src/netlink_delinearize.c
> index 12624db4c3a5..8b010fe4d168 100644
> --- a/src/netlink_delinearize.c
> +++ b/src/netlink_delinearize.c
> @@ -2618,8 +2618,17 @@ static void expr_postprocess(struct rule_pp_ctx *ctx, struct expr **exprp)
>  		}
>  		expr_postprocess(ctx, &expr->right);
>  
> -		expr_set_type(expr, expr->left->dtype,
> -			      expr->left->byteorder);
> +		switch (expr->op) {
> +		case OP_LSHIFT:
> +		case OP_RSHIFT:
> +			expr_set_type(expr, &integer_type,
> +				      BYTEORDER_HOST_ENDIAN);
> +			break;
> +		default:
> +			expr_set_type(expr, expr->left->dtype,
> +				      expr->left->byteorder);

This is a fix?

If so, would it be possible to provide a standalone example that shows
what this is fixing up?

> +		}
> +
>  		break;
>  	case EXPR_RELATIONAL:
>  		switch (expr->left->etype) {
> -- 
> 2.35.1
> 

^ permalink raw reply	[flat|nested] 50+ messages in thread

* Re: [nft PATCH v4 11/32] netlink_delinearize: correct length of right bitwise operand
  2022-04-04 12:13 ` [nft PATCH v4 11/32] netlink_delinearize: correct length of right bitwise operand Jeremy Sowden
@ 2022-05-23 17:22   ` Pablo Neira Ayuso
  2022-11-01 18:47     ` Jeremy Sowden
  0 siblings, 1 reply; 50+ messages in thread
From: Pablo Neira Ayuso @ 2022-05-23 17:22 UTC (permalink / raw)
  To: Jeremy Sowden; +Cc: Netfilter Devel, Kevin Darbyshire-Bryant

On Mon, Apr 04, 2022 at 01:13:49PM +0100, Jeremy Sowden wrote:
> Set it to match the length of the left operand.
> 
> Signed-off-by: Jeremy Sowden <jeremy@azazel.net>
> ---
>  src/netlink_delinearize.c | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/src/netlink_delinearize.c b/src/netlink_delinearize.c
> index 8b010fe4d168..cf5359bf269e 100644
> --- a/src/netlink_delinearize.c
> +++ b/src/netlink_delinearize.c
> @@ -2613,6 +2613,7 @@ static void expr_postprocess(struct rule_pp_ctx *ctx, struct expr **exprp)
>  				      BYTEORDER_HOST_ENDIAN);
>  			break;
>  		default:
> +			expr->right->len = expr->left->len;

This seems to be required for EXPR_BINOP (exclusing left/right shift)

I am assuming here expr->right is the value of the bitmask.

Was expr->right->len unset?

>  			expr_set_type(expr->right, expr->left->dtype,
>  				      expr->left->byteorder);
>  		}
> -- 
> 2.35.1
> 

^ permalink raw reply	[flat|nested] 50+ messages in thread

* Re: [nft PATCH v4 14/32] evaluate: relax type-checking for integer arguments in mark statements
  2022-04-04 12:13 ` [nft PATCH v4 14/32] evaluate: relax type-checking for integer arguments in mark statements Jeremy Sowden
@ 2022-05-23 17:33   ` Pablo Neira Ayuso
  0 siblings, 0 replies; 50+ messages in thread
From: Pablo Neira Ayuso @ 2022-05-23 17:33 UTC (permalink / raw)
  To: Jeremy Sowden; +Cc: Netfilter Devel, Kevin Darbyshire-Bryant

On Mon, Apr 04, 2022 at 01:13:52PM +0100, Jeremy Sowden wrote:
> In order to be able to set ct and meta marks to values derived from
> payload expressions, we need to relax the requirement that the type of
> the statement argument must match that of the statement key.  Instead,
> we require that the base-type of the argument is integer and that the
> argument is small enough to fit.

LGTM.

> Signed-off-by: Jeremy Sowden <jeremy@azazel.net>
> ---
>  src/evaluate.c | 8 ++++++--
>  1 file changed, 6 insertions(+), 2 deletions(-)
> 
> diff --git a/src/evaluate.c b/src/evaluate.c
> index ee4da5a2b889..f975dd197de3 100644
> --- a/src/evaluate.c
> +++ b/src/evaluate.c
> @@ -2393,8 +2393,12 @@ static int __stmt_evaluate_arg(struct eval_ctx *ctx, struct stmt *stmt,
>  					 "expression has type %s with length %d",
>  					 dtype->desc, (*expr)->dtype->desc,
>  					 (*expr)->len);
> -	else if ((*expr)->dtype->type != TYPE_INTEGER &&
> -		 !datatype_equal((*expr)->dtype, dtype))
> +
> +	if ((dtype->type == TYPE_MARK &&
> +	     !datatype_equal(datatype_basetype(dtype), datatype_basetype((*expr)->dtype))) ||
> +	    (dtype->type != TYPE_MARK &&
> +	     (*expr)->dtype->type != TYPE_INTEGER &&
> +	     !datatype_equal((*expr)->dtype, dtype)))
>  		return stmt_binary_error(ctx, *expr, stmt,		/* verdict vs invalid? */
>  					 "datatype mismatch: expected %s, "
>  					 "expression has type %s",
> -- 
> 2.35.1
> 

^ permalink raw reply	[flat|nested] 50+ messages in thread

* Re: [nft PATCH v4 13/32] evaluate: support shifts larger than the width of the left operand
  2022-04-04 12:13 ` [nft PATCH v4 13/32] evaluate: support shifts larger than the width of the left operand Jeremy Sowden
@ 2022-05-23 17:42   ` Pablo Neira Ayuso
  2022-11-01 18:47     ` Jeremy Sowden
  2023-02-07 12:05     ` Pablo Neira Ayuso
  0 siblings, 2 replies; 50+ messages in thread
From: Pablo Neira Ayuso @ 2022-05-23 17:42 UTC (permalink / raw)
  To: Jeremy Sowden; +Cc: Netfilter Devel, Kevin Darbyshire-Bryant

Hi,

I just tested patches 9 and 14 alone, and meta mark set ip dscp ...
now works fine.

So most of the work from 7 to 14 is to allow to use shifts.

So 8, 10, 11, 12 and 13 enable the use of shifts is meta and ct
statements?

The new _NBIT field is there to store the original length for the
payload field (6 bits, for the ip dscp case)?

On Mon, Apr 04, 2022 at 01:13:51PM +0100, Jeremy Sowden wrote:
> If we want to left-shift a value of narrower type and assign the result
> to a variable of a wider type, we are constrained to only shifting up to
> the width of the narrower type.  Thus:
> 
>   add rule t c meta mark set ip dscp << 2
> 
> works, but:
> 
>   add rule t c meta mark set ip dscp << 8
> 
> does not, even though the lvalue is large enough to accommodate the
> result.
> 
> Evaluation of the left-hand operand of a shift overwrites the `len`
> field of the evaluation context when `expr_evaluate_primary` is called.
> Instead, preserve the `len` value of the evaluation context for shifts,
> and support shifts up to that size, even if they are larger than the
> length of the left operand.
> 
> Update netlink_delinearize.c to handle the case where the length of a
> shift expression does not match that of its left-hand operand.
> 
> Signed-off-by: Jeremy Sowden <jeremy@azazel.net>
> ---
>  src/evaluate.c            | 23 ++++++++++++++---------
>  src/netlink_delinearize.c |  4 ++--
>  2 files changed, 16 insertions(+), 11 deletions(-)
> 
> diff --git a/src/evaluate.c b/src/evaluate.c
> index be493f85010c..ee4da5a2b889 100644
> --- a/src/evaluate.c
> +++ b/src/evaluate.c
> @@ -1116,14 +1116,18 @@ static int constant_binop_simplify(struct eval_ctx *ctx, struct expr **expr)
>  static int expr_evaluate_shift(struct eval_ctx *ctx, struct expr **expr)
>  {
>  	struct expr *op = *expr, *left = op->left, *right = op->right;
> +	unsigned int shift = mpz_get_uint32(right->value);
> +	unsigned int op_len = left->len;
>  
> -	if (mpz_get_uint32(right->value) >= left->len)
> -		return expr_binary_error(ctx->msgs, right, left,
> -					 "%s shift of %u bits is undefined "
> -					 "for type of %u bits width",
> -					 op->op == OP_LSHIFT ? "Left" : "Right",
> -					 mpz_get_uint32(right->value),
> -					 left->len);
> +	if (shift >= op_len) {
> +		if (shift >= ctx->ectx.len)
> +			return expr_binary_error(ctx->msgs, right, left,
> +						 "%s shift of %u bits is undefined for type of %u bits width",
> +						 op->op == OP_LSHIFT ? "Left" : "Right",
> +						 shift,
> +						 op_len);
> +		op_len = ctx->ectx.len;
> +	}
>  
>  	/* Both sides need to be in host byte order */
>  	if (byteorder_conversion(ctx, &op->left, BYTEORDER_HOST_ENDIAN) < 0)
> @@ -1134,7 +1138,7 @@ static int expr_evaluate_shift(struct eval_ctx *ctx, struct expr **expr)
>  
>  	op->dtype     = &integer_type;
>  	op->byteorder = BYTEORDER_HOST_ENDIAN;
> -	op->len       = left->len;
> +	op->len	      = op_len;
>  
>  	if (expr_is_constant(left))
>  		return constant_binop_simplify(ctx, expr);
> @@ -1167,6 +1171,7 @@ static int expr_evaluate_binop(struct eval_ctx *ctx, struct expr **expr)
>  {
>  	struct expr *op = *expr, *left, *right;
>  	const char *sym = expr_op_symbols[op->op];
> +	unsigned int ectx_len = ctx->ectx.len;
>  
>  	if (expr_evaluate(ctx, &op->left) < 0)
>  		return -1;
> @@ -1174,7 +1179,7 @@ static int expr_evaluate_binop(struct eval_ctx *ctx, struct expr **expr)
>  
>  	if (op->op == OP_LSHIFT || op->op == OP_RSHIFT)
>  		__expr_set_context(&ctx->ectx, &integer_type,
> -				   left->byteorder, ctx->ectx.len, 0);
> +				   left->byteorder, ectx_len, 0);
>  	if (expr_evaluate(ctx, &op->right) < 0)
>  		return -1;
>  	right = op->right;
> diff --git a/src/netlink_delinearize.c b/src/netlink_delinearize.c
> index cf5359bf269e..9f6fdee3e92d 100644
> --- a/src/netlink_delinearize.c
> +++ b/src/netlink_delinearize.c
> @@ -486,7 +486,7 @@ static struct expr *netlink_parse_bitwise_bool(struct netlink_parse_ctx *ctx,
>  		mpz_ior(m, m, o);
>  	}
>  
> -	if (left->len > 0 && mpz_scan0(m, 0) == left->len) {
> +	if (left->len > 0 && mpz_scan0(m, 0) >= left->len) {
>  		/* mask encompasses the entire value */
>  		expr_free(mask);
>  	} else {
> @@ -536,7 +536,7 @@ static struct expr *netlink_parse_bitwise_shift(struct netlink_parse_ctx *ctx,
>  	right->byteorder = BYTEORDER_HOST_ENDIAN;
>  
>  	expr = binop_expr_alloc(loc, op, left, right);
> -	expr->len = left->len;
> +	expr->len = nftnl_expr_get_u32(nle, NFTNL_EXPR_BITWISE_LEN) * BITS_PER_BYTE;
>  
>  	return expr;
>  }
> -- 
> 2.35.1
> 

^ permalink raw reply	[flat|nested] 50+ messages in thread

* Re: [nft PATCH v4 08/32] netlink: send bit-length of bitwise binops to kernel
  2022-05-23 17:03   ` Pablo Neira Ayuso
@ 2022-11-01 18:46     ` Jeremy Sowden
  0 siblings, 0 replies; 50+ messages in thread
From: Jeremy Sowden @ 2022-11-01 18:46 UTC (permalink / raw)
  To: Pablo Neira Ayuso; +Cc: Netfilter Devel

[-- Attachment #1: Type: text/plain, Size: 6069 bytes --]

On 2022-05-23, at 19:03:42 +0200, Pablo Neira Ayuso wrote:
> On Mon, Apr 04, 2022 at 01:13:46PM +0100, Jeremy Sowden wrote:
> > Some bitwise operations are generated when munging paylod
> > expressions.  During delinearization, we attempt to eliminate these
> > operations.  However, this is done before deducing the byte-order or
> > the correct length in bits of the operands, which means that we
> > don't always handle multi-byte host-endian operations correctly.
> > Therefore, pass the bit-length of these expressions to the kernel in
> > order to have it available during delinearization.
> >
> > Signed-off-by: Jeremy Sowden <jeremy@azazel.net>
> > ---
> >  src/netlink_delinearize.c | 14 ++++++++++++--
> >  src/netlink_linearize.c   |  2 ++
> >  2 files changed, 14 insertions(+), 2 deletions(-)
> >
> > diff --git a/src/netlink_delinearize.c b/src/netlink_delinearize.c
> > index a1b00dee209a..733977bc526d 100644
> > --- a/src/netlink_delinearize.c
> > +++ b/src/netlink_delinearize.c
> > @@ -451,20 +451,28 @@ static struct expr *netlink_parse_bitwise_bool(struct netlink_parse_ctx *ctx,
> >  					       const struct nftnl_expr *nle,
> >  					       enum nft_registers sreg,
> >  					       struct expr *left)
> > -
> >  {
> >  	struct nft_data_delinearize nld;
> >  	struct expr *expr, *mask, *xor, *or;
> > +	unsigned int nbits;
> >  	mpz_t m, x, o;
> >
> >  	expr = left;
> >
> > +	nbits = nftnl_expr_get_u32(nle, NFTNL_EXPR_BITWISE_NBITS);
> > +	if (nbits > 0)
> > +		expr->len = nbits;
>
> So NFTNL_EXPR_BITWISE_NBITS is signalling that this is an implicit
> bitwise that has been generated to operate with a payload header
> bitfield?
>
> Could you provide an example expression tree to show how this
> simplifies delinearization?

This rule:

  add rule ip6 t c ct mark set ip6 dscp lshift 2 or 0x10

has the following representation:

  ip6 t c
  [ payload load 2b @ network header + 0 => reg 1 ]
  [ bitwise reg 1 = ( reg 1 & 0x0000c00f ) ^ 0x00000000 ]
  [ bitwise reg 1 = ( reg 1 >> 0x00000006 ) ]
  [ byteorder reg 1 = ntoh(reg 1, 2, 1) ]
  [ bitwise reg 1 = ( reg 1 << 0x00000002 ) ]
  [ bitwise reg 1 = ( reg 1 & 0x00000fef ) ^ 0x00000010 ]
  [ ct set mark with reg 1 ]

This patch is intended to fix a problem with the OR expression:

  [ bitwise reg 1 = ( reg 1 & 0x00000fef ) ^ 0x00000010 ]
  
The expression has length 12 bits.  During linearization the length is
rounded up to 2 bytes.  During delinearization, the length of the
expression is compared to the size of the mask in order to determine
whether the mask can be removed:

  if (left->len > 0 && mpz_scan0(m, 0) == left->len) {
    /* mask encompasses the entire value */
    expr_free(mask);
  } else {
    mpz_set(mask->value, m);
    expr = binop_expr_alloc(loc, OP_AND, expr, mask);
    expr->len = left->len;
  }

Because the length of the expression is now 16 bits, it does not match
the width of the mask and so the mask is retained:

  table ip6 t {
    chain c {
      type filter hook output priority filter; policy accept;
      ct mark set ip6 dscp << 2 & 4095 | 16
    }
  }

> > +
> >  	nld.value = nftnl_expr_get(nle, NFTNL_EXPR_BITWISE_MASK, &nld.len);
> >  	mask = netlink_alloc_value(loc, &nld);
> > +	if (nbits > 0)
> > +		mpz_switch_byteorder(mask->value, div_round_up(nbits, BITS_PER_BYTE));
>
> What is the byteorder expected for the mask before this switch
> operation?

NBO.

> >  	mpz_init_set(m, mask->value);
> >
> >  	nld.value = nftnl_expr_get(nle, NFTNL_EXPR_BITWISE_XOR, &nld.len);
> > -	xor  = netlink_alloc_value(loc, &nld);
> > +	xor = netlink_alloc_value(loc, &nld);
> > +	if (nbits > 0)
> > +		mpz_switch_byteorder(xor->value, div_round_up(nbits, BITS_PER_BYTE));
> >  	mpz_init_set(x, xor->value);
> >
> >  	mpz_init_set_ui(o, 0);
> > @@ -500,6 +508,8 @@ static struct expr *netlink_parse_bitwise_bool(struct netlink_parse_ctx *ctx,
> >
> >  		or = netlink_alloc_value(loc, &nld);
> >  		mpz_set(or->value, o);
> > +		if (nbits > 0)
> > +			mpz_switch_byteorder(or->value, div_round_up(nbits, BITS_PER_BYTE));
> >  		expr = binop_expr_alloc(loc, OP_OR, expr, or);
> >  		expr->len = left->len;
> >  	}
> > diff --git a/src/netlink_linearize.c b/src/netlink_linearize.c
> > index c8bbcb7452b0..4793f3853bee 100644
> > --- a/src/netlink_linearize.c
> > +++ b/src/netlink_linearize.c
> > @@ -677,6 +677,8 @@ static void netlink_gen_bitwise(struct netlink_linearize_ctx *ctx,
> >  	netlink_put_register(nle, NFTNL_EXPR_BITWISE_DREG, dreg);
> >  	nftnl_expr_set_u32(nle, NFTNL_EXPR_BITWISE_OP, NFT_BITWISE_BOOL);
> >  	nftnl_expr_set_u32(nle, NFTNL_EXPR_BITWISE_LEN, len);
> > +	if (expr->byteorder == BYTEORDER_HOST_ENDIAN)
> > +		nftnl_expr_set_u32(nle, NFTNL_EXPR_BITWISE_NBITS, expr->len);
>
> Why is this only required for host endian expressions?

I wasn't sure whether keeping track of the bit length by storing it in
the kernel would be an acceptable solution.  Doing it for both byte-
orders caused failures in unrelated test-cases.  Since NBO expressions
don't come up in my use-case I decided to restrict it to HBO to start
with, and, if copying the bit-length to and from the kernel *was*
acceptable, to fix those test-failures in the next version of the
patch-set (I assumed one would be required :)).

However, in the process of replying to the questions in your response to
patch 13, I have realized that this patch may not be necessary after
all.  The problem here lies in the code which attempts to turn a mask-
and-xor expression back into the corresponding original bitwise opera-
tion.  A different solution would be to make use of the native bitwise
operations introduced by this series to avoid having to convert to and
from mask-and-xor expressions altogether.  Further explanation in the
later reply.

J.

> >  	netlink_gen_raw_data(mask, expr->byteorder, len, &nld);
> >  	nftnl_expr_set(nle, NFTNL_EXPR_BITWISE_MASK, nld.value, nld.len);
> > --
> > 2.35.1
> >
>

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

^ permalink raw reply	[flat|nested] 50+ messages in thread

* Re: [nft PATCH v4 09/32] netlink_delinearize: add postprocessing for payload binops
  2022-05-23 17:19   ` Pablo Neira Ayuso
@ 2022-11-01 18:46     ` Jeremy Sowden
  0 siblings, 0 replies; 50+ messages in thread
From: Jeremy Sowden @ 2022-11-01 18:46 UTC (permalink / raw)
  To: Pablo Neira Ayuso; +Cc: Netfilter Devel

[-- Attachment #1: Type: text/plain, Size: 3456 bytes --]

On 2022-05-23, at 19:19:08 +0200, Pablo Neira Ayuso wrote:
> On Mon, Apr 04, 2022 at 01:13:47PM +0100, Jeremy Sowden wrote:
> > If a user uses a payload expression as a statement argument:
> >
> >   nft add rule t c meta mark set ip dscp lshift 2 or 0x10
> >
> > we may need to undo munging during delinearization.
> >
> > Signed-off-by: Jeremy Sowden <jeremy@azazel.net>
> > ---
> >  src/netlink_delinearize.c | 39 +++++++++++++++++++++++++++++++++++++++
> >  1 file changed, 39 insertions(+)
> >
> > diff --git a/src/netlink_delinearize.c b/src/netlink_delinearize.c
> > index 733977bc526d..12624db4c3a5 100644
> > --- a/src/netlink_delinearize.c
> > +++ b/src/netlink_delinearize.c
> > @@ -2454,6 +2454,42 @@ static void relational_binop_postprocess(struct rule_pp_ctx *ctx,
> >  	}
> >  }
> >
> > +static bool payload_binop_postprocess(struct rule_pp_ctx *ctx,
> > +				      struct expr **exprp)
> > +{
> > +	struct expr *expr = *exprp;
> > +
> > +	if (expr->op != OP_RSHIFT)
> > +		return false;
> > +
> > +	if (expr->left->etype == EXPR_UNARY) {
> > +		/*
> > +		 * If the payload value was originally in a different byte-order
> > +		 * from the payload expression, there will be a byte-order
> > +		 * conversion to remove.
> > +		 */
>
> The comment assumes this is a payload expression, the unary is
> stripped off here...
>
> > +		struct expr *left = expr_get(expr->left->arg);
> > +		expr_free(expr->left);
> > +		expr->left = left;
> > +	}
> > +
> > +	if (expr->left->etype != EXPR_BINOP || expr->left->op != OP_AND)
> > +		return false;
> > +
> > +	if (expr->left->left->etype != EXPR_PAYLOAD)
>
> ... but the check for payload is coming here.

Will fix.

> I assume this postprocessing is to undo the switch from network
> byteorder to host byteorder for the ip dscp of the example above?
>
> Could you describe an example expression tree to depict this
> delinearize scenario?

Currently, demunging is only done for payload statement expressions, in
`stmt_payload_postprocess`.  However, this patch-set will lead to the
appearance of munged payload expressions in other contexts, such as the
example given above in the commit message:

  nft add rule t c meta mark set ip dscp lshift 2 or 0x10

The expression tree for the value assigned to `meta mark` is:

                              OR
		             /  \
                       LSHIFT    0x10
                      /      \
                RSHIFT        2
               /      \
            AND        2
           /   \
   @nh(8,8)     0xfc

and the `@nh(8,8) & 0xfc >> 2` expression needs to be demunged to `ip dscp`.

> > +		return false;
> > +
> > +	expr_set_type(expr->right, &integer_type,
> > +		      BYTEORDER_HOST_ENDIAN);
> > +	expr_postprocess(ctx, &expr->right);
> > +
> > +	binop_postprocess(ctx, expr, &expr->left);
> > +	*exprp = expr_get(expr->left);
> > +	expr_free(expr);
> > +
> > +	return true;
> > +}
> > +
> >  static struct expr *string_wildcard_expr_alloc(struct location *loc,
> >  					       const struct expr *mask,
> >  					       const struct expr *expr)
> > @@ -2566,6 +2602,9 @@ static void expr_postprocess(struct rule_pp_ctx *ctx, struct expr **exprp)
> >  		expr_set_type(expr, expr->arg->dtype, !expr->arg->byteorder);
> >  		break;
> >  	case EXPR_BINOP:
> > +		if (payload_binop_postprocess(ctx, exprp))
> > +			break;
> > +
> >  		expr_postprocess(ctx, &expr->left);
> >  		switch (expr->op) {
> >  		case OP_LSHIFT:
> > --
> > 2.35.1
> >
>

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

^ permalink raw reply	[flat|nested] 50+ messages in thread

* Re: [nft PATCH v4 10/32] netlink_delinearize: correct type and byte-order of shifts
  2022-05-23 17:19   ` Pablo Neira Ayuso
@ 2022-11-01 18:47     ` Jeremy Sowden
  0 siblings, 0 replies; 50+ messages in thread
From: Jeremy Sowden @ 2022-11-01 18:47 UTC (permalink / raw)
  To: Pablo Neira Ayuso; +Cc: Netfilter Devel

[-- Attachment #1: Type: text/plain, Size: 1779 bytes --]

On 2022-05-23, at 19:19:14 +0200, Pablo Neira Ayuso wrote:
> On Mon, Apr 04, 2022 at 01:13:48PM +0100, Jeremy Sowden wrote:
> > Shifts are of integer type and in HBO.
> > 
> > Signed-off-by: Jeremy Sowden <jeremy@azazel.net>
> > ---
> >  src/netlink_delinearize.c | 13 +++++++++++--
> >  1 file changed, 11 insertions(+), 2 deletions(-)
> > 
> > diff --git a/src/netlink_delinearize.c b/src/netlink_delinearize.c
> > index 12624db4c3a5..8b010fe4d168 100644
> > --- a/src/netlink_delinearize.c
> > +++ b/src/netlink_delinearize.c
> > @@ -2618,8 +2618,17 @@ static void expr_postprocess(struct rule_pp_ctx *ctx, struct expr **exprp)
> >  		}
> >  		expr_postprocess(ctx, &expr->right);
> >  
> > -		expr_set_type(expr, expr->left->dtype,
> > -			      expr->left->byteorder);
> > +		switch (expr->op) {
> > +		case OP_LSHIFT:
> > +		case OP_RSHIFT:
> > +			expr_set_type(expr, &integer_type,
> > +				      BYTEORDER_HOST_ENDIAN);
> > +			break;
> > +		default:
> > +			expr_set_type(expr, expr->left->dtype,
> > +				      expr->left->byteorder);
> 
> This is a fix?
> 
> If so, would it be possible to provide a standalone example that shows
> what this is fixing up?

Without this, listing a rule like:

  ct mark set ip dscp lshift 2 or 0x10

will return:

  ct mark set ip dscp << 2 | cs2

because the type of the OR's right operand will be transitively derived
from `ip dscp`.  However, this is not valid syntax:

  # nft add rule t c ct mark set ip dscp '<<' 2 '|' cs2
  Error: Could not parse integer
  add rule t c ct mark set ip dscp << 2 | cs2
                                          ^^^

> > +		}
> > +
> >  		break;
> >  	case EXPR_RELATIONAL:
> >  		switch (expr->left->etype) {
> > -- 
> > 2.35.1
> > 
> 

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

^ permalink raw reply	[flat|nested] 50+ messages in thread

* Re: [nft PATCH v4 11/32] netlink_delinearize: correct length of right bitwise operand
  2022-05-23 17:22   ` Pablo Neira Ayuso
@ 2022-11-01 18:47     ` Jeremy Sowden
  0 siblings, 0 replies; 50+ messages in thread
From: Jeremy Sowden @ 2022-11-01 18:47 UTC (permalink / raw)
  To: Pablo Neira Ayuso; +Cc: Netfilter Devel

[-- Attachment #1: Type: text/plain, Size: 1490 bytes --]

On 2022-05-23, at 19:22:02 +0200, Pablo Neira Ayuso wrote:
> On Mon, Apr 04, 2022 at 01:13:49PM +0100, Jeremy Sowden wrote:
> > Set it to match the length of the left operand.
> >
> > Signed-off-by: Jeremy Sowden <jeremy@azazel.net>
> > ---
> >  src/netlink_delinearize.c | 1 +
> >  1 file changed, 1 insertion(+)
> >
> > diff --git a/src/netlink_delinearize.c b/src/netlink_delinearize.c
> > index 8b010fe4d168..cf5359bf269e 100644
> > --- a/src/netlink_delinearize.c
> > +++ b/src/netlink_delinearize.c
> > @@ -2613,6 +2613,7 @@ static void expr_postprocess(struct rule_pp_ctx *ctx, struct expr **exprp)
> >  				      BYTEORDER_HOST_ENDIAN);
> >  			break;
> >  		default:
> > +			expr->right->len = expr->left->len;
>
> This seems to be required for EXPR_BINOP (exclusing left/right shift)
>
> I am assuming here expr->right is the value of the bitmask.
>
> Was expr->right->len unset?

Hmm.  I can't now remember what purpose this served.  I spent a lot of
time staring at the delinearization code for binops and payloads while
debugging this series, and it is possible that I just spotted that under
some circumstances the length of the right hand operand after delinea-
rization wasn't right or didn't match what it did after evaluation, and
made this change for correctness.  However, reverting it doesn't seem to
break anything, so I'm happy to drop it.

> >  			expr_set_type(expr->right, expr->left->dtype,
> >  				      expr->left->byteorder);
> >  		}
> > --
> > 2.35.1
> >
>

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

^ permalink raw reply	[flat|nested] 50+ messages in thread

* Re: [nft PATCH v4 13/32] evaluate: support shifts larger than the width of the left operand
  2022-05-23 17:42   ` Pablo Neira Ayuso
@ 2022-11-01 18:47     ` Jeremy Sowden
  2023-02-07 12:05     ` Pablo Neira Ayuso
  1 sibling, 0 replies; 50+ messages in thread
From: Jeremy Sowden @ 2022-11-01 18:47 UTC (permalink / raw)
  To: Pablo Neira Ayuso; +Cc: Netfilter Devel

[-- Attachment #1: Type: text/plain, Size: 8101 bytes --]

On 2022-05-23, at 19:42:40 +0200, Pablo Neira Ayuso wrote:
> I just tested patches 9 and 14 alone, and meta mark set ip dscp ...
> now works fine.
>
> So most of the work from 7 to 14 is to allow to use shifts.
>
> So 8, 10, 11, 12 and 13 enable the use of shifts is meta and ct
> statements?

Yes.

> The new _NBIT field is there to store the original length for the
> payload field (6 bits, for the ip dscp case)?

It's for this ip6 dscp case:

  ct mark set ip6 dscp << 2 | 16

This is linearized as:

  [ payload load 2b @ network header + 0 => reg 1 ]
  [ bitwise reg 1 = ( reg 1 & 0x0000c00f ) ^ 0x00000000 ]
  [ bitwise reg 1 = ( reg 1 >> 0x00000006 ) ]
  [ byteorder reg 1 = ntoh(reg 1, 2, 1) ]
  [ bitwise reg 1 = ( reg 1 << 0x00000002 ) ]
  [ bitwise reg 1 = ( reg 1 & 0x00000fef ) ^ 0x00000010 ]
  [ ct set mark with reg 1 ]

The problem is the last bitwise expression:

  [ bitwise reg 1 = ( reg 1 & 0x00000fef ) ^ 0x00000010 ]

`ip6 dscp` spans two octets:

  @nh(0,16) & 0xfc0 >> 6

The original length is 12 bits.  The LSHIFT expression changes the
byte-order to host-endian.

When the OR expression is evaluated, it is converted from:

  ${lhs} | 16

to:

  ${lhs} & 0xfef ^ 0x10

and when it is linearized, the bit-length is rounded up to 16 bits and
the byte-order is converted to big-endian.

During delinearization we try to turn the mask-and-xor back into its
original form before the original endianness and length are known, so
starting from:

  ${lhs} & 0xef0f ^ 0x1000

we fail to strip the mask and end up with:

  ct mark set ip6 dscp << 2 & 4095 | 16

Having gone back and reviewed this bug in the writing of this e-mail
I've realized that the introduction of native kernel bitwise op's in
patches 27-28 could obviate it.  In the current iteration of the
patch-set, the new bitwise op's are only used to support previously
unsupported bitwise expressions with variable right hand operands;
currently supported operations with constant right hand operands are
still converted to mask-and-xor operations.  If the linearization code
were changed to use the native op's for expressions with constant RHS's
too, the problematic conversion from mask-and-xor would go away.

When I tried this out, I had to make a couple of other changes to get it
working.  The big one was to register allocation.  Although netfilter
registers are 32-bits wide these days, they are currently allocated in
blocks of four for backwards-compatibility with older kernels.  Some of
the new test-cases added in this series failed because of a lack of
available registers, so I changed the allocation as follows:

  --- a/src/netlink_linearize.c
  +++ b/src/netlink_linearize.c
  @@ -97,7 +97,7 @@ static void __release_register(struct netlink_linearize_ctx *ctx,
   static enum nft_registers get_register(struct netlink_linearize_ctx *ctx,
                                         const struct expr *expr)
   {
  -       if (expr && expr->etype == EXPR_CONCAT)
  +       if (expr && expr->len)
                  return __get_register(ctx, expr->len);
          else
                  return __get_register(ctx, NFT_REG_SIZE * BITS_PER_BYTE);
  @@ -106,7 +106,7 @@ static enum nft_registers get_register(struct netlink_linearize_ctx *ctx,
   static void release_register(struct netlink_linearize_ctx *ctx,
                               const struct expr *expr)
   {
  -       if (expr && expr->etype == EXPR_CONCAT)
  +       if (expr && expr->len)
                  __release_register(ctx, expr->len);
          else
                  __release_register(ctx, NFT_REG_SIZE * BITS_PER_BYTE);

It's been seven years since the switch from 128- to 32-bit registers.
Would something like this change be acceptable?

J.

> On Mon, Apr 04, 2022 at 01:13:51PM +0100, Jeremy Sowden wrote:
> > If we want to left-shift a value of narrower type and assign the result
> > to a variable of a wider type, we are constrained to only shifting up to
> > the width of the narrower type.  Thus:
> >
> >   add rule t c meta mark set ip dscp << 2
> >
> > works, but:
> >
> >   add rule t c meta mark set ip dscp << 8
> >
> > does not, even though the lvalue is large enough to accommodate the
> > result.
> >
> > Evaluation of the left-hand operand of a shift overwrites the `len`
> > field of the evaluation context when `expr_evaluate_primary` is called.
> > Instead, preserve the `len` value of the evaluation context for shifts,
> > and support shifts up to that size, even if they are larger than the
> > length of the left operand.
> >
> > Update netlink_delinearize.c to handle the case where the length of a
> > shift expression does not match that of its left-hand operand.
> >
> > Signed-off-by: Jeremy Sowden <jeremy@azazel.net>
> > ---
> >  src/evaluate.c            | 23 ++++++++++++++---------
> >  src/netlink_delinearize.c |  4 ++--
> >  2 files changed, 16 insertions(+), 11 deletions(-)
> >
> > diff --git a/src/evaluate.c b/src/evaluate.c
> > index be493f85010c..ee4da5a2b889 100644
> > --- a/src/evaluate.c
> > +++ b/src/evaluate.c
> > @@ -1116,14 +1116,18 @@ static int constant_binop_simplify(struct eval_ctx *ctx, struct expr **expr)
> >  static int expr_evaluate_shift(struct eval_ctx *ctx, struct expr **expr)
> >  {
> >  	struct expr *op = *expr, *left = op->left, *right = op->right;
> > +	unsigned int shift = mpz_get_uint32(right->value);
> > +	unsigned int op_len = left->len;
> >
> > -	if (mpz_get_uint32(right->value) >= left->len)
> > -		return expr_binary_error(ctx->msgs, right, left,
> > -					 "%s shift of %u bits is undefined "
> > -					 "for type of %u bits width",
> > -					 op->op == OP_LSHIFT ? "Left" : "Right",
> > -					 mpz_get_uint32(right->value),
> > -					 left->len);
> > +	if (shift >= op_len) {
> > +		if (shift >= ctx->ectx.len)
> > +			return expr_binary_error(ctx->msgs, right, left,
> > +						 "%s shift of %u bits is undefined for type of %u bits width",
> > +						 op->op == OP_LSHIFT ? "Left" : "Right",
> > +						 shift,
> > +						 op_len);
> > +		op_len = ctx->ectx.len;
> > +	}
> >
> >  	/* Both sides need to be in host byte order */
> >  	if (byteorder_conversion(ctx, &op->left, BYTEORDER_HOST_ENDIAN) < 0)
> > @@ -1134,7 +1138,7 @@ static int expr_evaluate_shift(struct eval_ctx *ctx, struct expr **expr)
> >
> >  	op->dtype     = &integer_type;
> >  	op->byteorder = BYTEORDER_HOST_ENDIAN;
> > -	op->len       = left->len;
> > +	op->len	      = op_len;
> >
> >  	if (expr_is_constant(left))
> >  		return constant_binop_simplify(ctx, expr);
> > @@ -1167,6 +1171,7 @@ static int expr_evaluate_binop(struct eval_ctx *ctx, struct expr **expr)
> >  {
> >  	struct expr *op = *expr, *left, *right;
> >  	const char *sym = expr_op_symbols[op->op];
> > +	unsigned int ectx_len = ctx->ectx.len;
> >
> >  	if (expr_evaluate(ctx, &op->left) < 0)
> >  		return -1;
> > @@ -1174,7 +1179,7 @@ static int expr_evaluate_binop(struct eval_ctx *ctx, struct expr **expr)
> >
> >  	if (op->op == OP_LSHIFT || op->op == OP_RSHIFT)
> >  		__expr_set_context(&ctx->ectx, &integer_type,
> > -				   left->byteorder, ctx->ectx.len, 0);
> > +				   left->byteorder, ectx_len, 0);
> >  	if (expr_evaluate(ctx, &op->right) < 0)
> >  		return -1;
> >  	right = op->right;
> > diff --git a/src/netlink_delinearize.c b/src/netlink_delinearize.c
> > index cf5359bf269e..9f6fdee3e92d 100644
> > --- a/src/netlink_delinearize.c
> > +++ b/src/netlink_delinearize.c
> > @@ -486,7 +486,7 @@ static struct expr *netlink_parse_bitwise_bool(struct netlink_parse_ctx *ctx,
> >  		mpz_ior(m, m, o);
> >  	}
> >
> > -	if (left->len > 0 && mpz_scan0(m, 0) == left->len) {
> > +	if (left->len > 0 && mpz_scan0(m, 0) >= left->len) {
> >  		/* mask encompasses the entire value */
> >  		expr_free(mask);
> >  	} else {
> > @@ -536,7 +536,7 @@ static struct expr *netlink_parse_bitwise_shift(struct netlink_parse_ctx *ctx,
> >  	right->byteorder = BYTEORDER_HOST_ENDIAN;
> >
> >  	expr = binop_expr_alloc(loc, op, left, right);
> > -	expr->len = left->len;
> > +	expr->len = nftnl_expr_get_u32(nle, NFTNL_EXPR_BITWISE_LEN) * BITS_PER_BYTE;
> >
> >  	return expr;
> >  }
> > --
> > 2.35.1
> >
>

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

^ permalink raw reply	[flat|nested] 50+ messages in thread

* Re: [nft PATCH v4 13/32] evaluate: support shifts larger than the width of the left operand
  2022-05-23 17:42   ` Pablo Neira Ayuso
  2022-11-01 18:47     ` Jeremy Sowden
@ 2023-02-07 12:05     ` Pablo Neira Ayuso
  2023-03-04 12:00       ` Jeremy Sowden
  1 sibling, 1 reply; 50+ messages in thread
From: Pablo Neira Ayuso @ 2023-02-07 12:05 UTC (permalink / raw)
  To: Jeremy Sowden; +Cc: Netfilter Devel, Kevin Darbyshire-Bryant

Long time, no news.

On Mon, May 23, 2022 at 07:42:43PM +0200, Pablo Neira Ayuso wrote:
> Hi,
> 
> I just tested patches 9 and 14 alone, and meta mark set ip dscp ...
> now works fine.

I have applied 9 and 14, including one testcase for tests/py, so

        meta mark set ip dscp

works, so there is at least some progress on this, sorry about this :(

^ permalink raw reply	[flat|nested] 50+ messages in thread

* Re: [nft PATCH v4 13/32] evaluate: support shifts larger than the width of the left operand
  2023-02-07 12:05     ` Pablo Neira Ayuso
@ 2023-03-04 12:00       ` Jeremy Sowden
  0 siblings, 0 replies; 50+ messages in thread
From: Jeremy Sowden @ 2023-03-04 12:00 UTC (permalink / raw)
  To: Pablo Neira Ayuso; +Cc: Netfilter Devel, Kevin Darbyshire-Bryant

[-- Attachment #1: Type: text/plain, Size: 511 bytes --]

On 2023-02-07, at 13:05:38 +0100, Pablo Neira Ayuso wrote:
> Long time, no news.
> 
> On Mon, May 23, 2022 at 07:42:43PM +0200, Pablo Neira Ayuso wrote:
> > Hi,
> > 
> > I just tested patches 9 and 14 alone, and meta mark set ip dscp ...
> > now works fine.
> 
> I have applied 9 and 14, including one testcase for tests/py, so
> 
>         meta mark set ip dscp
> 
> works, so there is at least some progress on this, sorry about this :(

A step in the right direction. :) Thanks, Pablo.

J.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

^ permalink raw reply	[flat|nested] 50+ messages in thread

end of thread, other threads:[~2023-03-04 12:40 UTC | newest]

Thread overview: 50+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-04-04 12:13 [nft PATCH v4 00/32] Extend values assignable to packet marks and payload fields Jeremy Sowden
2022-04-04 12:13 ` [nft PATCH v4 01/32] examples: add .gitignore file Jeremy Sowden
2022-04-05 11:26   ` Florian Westphal
2022-04-04 12:13 ` [nft PATCH v4 02/32] include: add missing `#include` Jeremy Sowden
2022-04-04 12:13 ` [nft PATCH v4 03/32] src: move `byteorder_names` array Jeremy Sowden
2022-04-04 12:13 ` [nft PATCH v4 04/32] datatype: support `NULL` symbol-tables when printing constants Jeremy Sowden
2022-04-04 12:13 ` [nft PATCH v4 05/32] ct: support `NULL` symbol-tables when looking up labels Jeremy Sowden
2022-04-05 11:15   ` Florian Westphal
2022-04-05 15:29     ` Jeremy Sowden
2022-04-04 12:13 ` [nft PATCH v4 06/32] include: update nf_tables.h Jeremy Sowden
2022-04-04 12:13 ` [nft PATCH v4 07/32] include: add new bitwise bit-length attribute to nf_tables.h Jeremy Sowden
2022-04-04 12:13 ` [nft PATCH v4 08/32] netlink: send bit-length of bitwise binops to kernel Jeremy Sowden
2022-05-23 17:03   ` Pablo Neira Ayuso
2022-11-01 18:46     ` Jeremy Sowden
2022-04-04 12:13 ` [nft PATCH v4 09/32] netlink_delinearize: add postprocessing for payload binops Jeremy Sowden
2022-05-23 17:19   ` Pablo Neira Ayuso
2022-11-01 18:46     ` Jeremy Sowden
2022-04-04 12:13 ` [nft PATCH v4 10/32] netlink_delinearize: correct type and byte-order of shifts Jeremy Sowden
2022-05-23 17:19   ` Pablo Neira Ayuso
2022-11-01 18:47     ` Jeremy Sowden
2022-04-04 12:13 ` [nft PATCH v4 11/32] netlink_delinearize: correct length of right bitwise operand Jeremy Sowden
2022-05-23 17:22   ` Pablo Neira Ayuso
2022-11-01 18:47     ` Jeremy Sowden
2022-04-04 12:13 ` [nft PATCH v4 12/32] payload: set byte-order when completing expression Jeremy Sowden
2022-04-04 12:13 ` [nft PATCH v4 13/32] evaluate: support shifts larger than the width of the left operand Jeremy Sowden
2022-05-23 17:42   ` Pablo Neira Ayuso
2022-11-01 18:47     ` Jeremy Sowden
2023-02-07 12:05     ` Pablo Neira Ayuso
2023-03-04 12:00       ` Jeremy Sowden
2022-04-04 12:13 ` [nft PATCH v4 14/32] evaluate: relax type-checking for integer arguments in mark statements Jeremy Sowden
2022-05-23 17:33   ` Pablo Neira Ayuso
2022-04-04 12:13 ` [nft PATCH v4 15/32] tests: shell: rename some test-cases Jeremy Sowden
2022-04-04 12:13 ` [nft PATCH v4 16/32] tests: shell: add test-cases for ct and packet mark payload expressions Jeremy Sowden
2022-04-04 12:13 ` [nft PATCH v4 17/32] tests: py: " Jeremy Sowden
2022-04-04 12:13 ` [nft PATCH v4 18/32] include: add new bitwise boolean attributes to nf_tables.h Jeremy Sowden
2022-04-04 12:13 ` [nft PATCH v4 19/32] evaluate: don't eval unary arguments Jeremy Sowden
2022-04-04 12:13 ` [nft PATCH v4 20/32] evaluate: prevent nested byte-order conversions Jeremy Sowden
2022-04-04 12:13 ` [nft PATCH v4 21/32] evaluate: don't clobber binop lengths Jeremy Sowden
2022-04-04 12:14 ` [nft PATCH v4 22/32] evaluate: insert byte-order conversions for expressions between 9 and 15 bits Jeremy Sowden
2022-04-04 12:14 ` [nft PATCH v4 23/32] evaluate: set eval context to leftmost bitwise operand Jeremy Sowden
2022-04-04 12:14 ` [nft PATCH v4 24/32] netlink_delinearize: fix typo Jeremy Sowden
2022-04-04 12:14 ` [nft PATCH v4 25/32] netlink_delinearize: refactor stmt_payload_binop_postprocess Jeremy Sowden
2022-04-04 12:14 ` [nft PATCH v4 26/32] netlink_delinearize: add support for processing variable payload statement arguments Jeremy Sowden
2022-04-04 12:14 ` [nft PATCH v4 27/32] netlink: rename bitwise operation functions Jeremy Sowden
2022-04-04 12:14 ` [nft PATCH v4 28/32] netlink: support (de)linearization of new bitwise boolean operations Jeremy Sowden
2022-04-04 12:14 ` [nft PATCH v4 29/32] parser_json: allow RHS ct, meta and payload expressions Jeremy Sowden
2022-04-04 12:14 ` [nft PATCH v4 30/32] evaluate: allow binop expressions with variable right-hand operands Jeremy Sowden
2022-04-04 12:14 ` [nft PATCH v4 31/32] tests: shell: add tests for binops with variable RHS operands Jeremy Sowden
2022-04-04 12:14 ` [nft PATCH v4 32/32] tests: py: " Jeremy Sowden
2022-04-09  8:30 ` [nft PATCH v4 00/32] Extend values assignable to packet marks and payload fields Kevin 'ldir' Darbyshire-Bryant

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.