[PATCH nft v3 00/18] Support for boolean binops with variable RHS operands.

[PATCH nft v3 00/18] Support for boolean binops with variable RHS operands.

[Jeremy Sowden]

Kernel support for passing mask and xor values for bitwise boolean
operations via registers allows us to support boolean binop's with
variable RHS operands: XOR expressions pass the xor value in a register;
AND expressions pass the mask value in a register; OR expressions pass
both mask and xor values in registers.

NB, OR expressions are converted to `(a & (b ^ 1)) ^ b` during
linearization (in patch 9), because it makes both linearization and
delinearization a lot simpler.  However, it involves rearranging and
allocating expressions after the evaluation phase.  Since nothing else
does this, AFAICS, I'm not sure whether it's the right thing to do.

The patch-set comprises four parts:

   1 -  7: some tidying and bug-fixes;
   8 - 10: support for variable RHS operands;
  11 - 15: updates to linearization and delinearization of payload
           expressions to work correctly with variable RHS operands;
  16 - 18: some new shell and Python test-cases.

Changes since v2:

  * patch 11 updated to stop binop length being clobbered during
    evaluation, instead of working around it during linearization.

Changes since v1:

  * patch 05 updated to treat short values as constant, rather than
    doing nothing with them.

Jeremy Sowden (18):
  evaluate: add separate variables for lshift and xor binops.
  evaluate: simplify calculation of payload size.
  evaluate: don't evaluate payloads twice.
  evaluate: convert the byte-order of payload statement arguments.
  evaluate: no need to swap byte-order for values of fewer than 16 bits.
  netlink_delinearize: set shift RHS byte-order.
  src: fix leaks.
  include: update nf_tables.h.
  src: support (de)linearization of bitwise op's with variable right
    operands.
  evaluate: allow boolean binop expressions with variable righthand
    arguments.
  evaluate: don't clobber binop bitmask lengths.
  netlink_delinearize: fix typo.
  netlink_delinearize: refactor stmt_payload_binop_postprocess.
  netlink_delinearize: add support for processing variable payload
    statement arguments.
  netlink_delinearize: add postprocessing for payload binops.
  tests: shell: remove stray debug flag.
  tests: shell: add variable binop RHS tests.
  tests: py: add variable binop RHS tests.

 include/expression.h                          |   1 +
 include/linux/netfilter/nf_tables.h           |   4 +
 src/evaluate.c                                |  77 ++--
 src/netlink_delinearize.c                     | 370 +++++++++++++-----
 src/netlink_linearize.c                       |  95 ++++-
 tests/py/any/ct.t                             |   1 +
 tests/py/any/ct.t.json                        |  37 ++
 tests/py/any/ct.t.payload                     |  33 ++
 tests/py/any/meta.t.payload                   |   4 -
 tests/py/inet/tcp.t                           |   2 +
 tests/py/inet/tcp.t.json                      |  46 ++-
 tests/py/inet/tcp.t.payload                   |  68 ++++
 tests/py/ip/ip.t                              |   3 +
 tests/py/ip/ip.t.json                         |  66 ++++
 tests/py/ip/ip.t.payload                      |  26 ++
 tests/py/ip/ip.t.payload.bridge               |  30 ++
 tests/py/ip/ip.t.payload.inet                 |  30 ++
 tests/py/ip/ip.t.payload.netdev               |  30 ++
 tests/shell/testcases/chains/0040mark_shift_0 |   2 +-
 tests/shell/testcases/chains/0040mark_shift_2 |  11 +
 .../testcases/chains/0041payload_variable_0   |  11 +
 .../testcases/chains/0041payload_variable_1   |  11 +
 .../testcases/chains/0041payload_variable_2   |  11 +
 .../testcases/chains/0041payload_variable_3   |  11 +
 .../chains/dumps/0040mark_shift_2.nft         |   6 +
 .../chains/dumps/0041payload_variable_0.nft   |   6 +
 .../chains/dumps/0041payload_variable_1.nft   |   6 +
 .../chains/dumps/0041payload_variable_2.nft   |   6 +
 .../chains/dumps/0041payload_variable_3.nft   |   6 +
 29 files changed, 873 insertions(+), 137 deletions(-)
 create mode 100755 tests/shell/testcases/chains/0040mark_shift_2
 create mode 100755 tests/shell/testcases/chains/0041payload_variable_0
 create mode 100755 tests/shell/testcases/chains/0041payload_variable_1
 create mode 100755 tests/shell/testcases/chains/0041payload_variable_2
 create mode 100755 tests/shell/testcases/chains/0041payload_variable_3
 create mode 100644 tests/shell/testcases/chains/dumps/0040mark_shift_2.nft
 create mode 100644 tests/shell/testcases/chains/dumps/0041payload_variable_0.nft
 create mode 100644 tests/shell/testcases/chains/dumps/0041payload_variable_1.nft
 create mode 100644 tests/shell/testcases/chains/dumps/0041payload_variable_2.nft
 create mode 100644 tests/shell/testcases/chains/dumps/0041payload_variable_3.nft

-- 
2.25.1

[PATCH nft v3 01/18] evaluate: add separate variables for lshift and xor binops.

[Jeremy Sowden]

stmt_evaluate_payload has distinct variables for some, but not all, the
binop expressions it creates.  Add variables for the rest.

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

diff --git a/src/evaluate.c b/src/evaluate.c
index b38ac9310656..fda30fd8001e 100644
--- a/src/evaluate.c
+++ b/src/evaluate.c
@@ -2204,7 +2204,7 @@ static int stmt_evaluate_exthdr(struct eval_ctx *ctx, struct stmt *stmt)
 
 static int stmt_evaluate_payload(struct eval_ctx *ctx, struct stmt *stmt)
 {
-	struct expr *binop, *mask, *and, *payload_bytes;
+	struct expr *mask, *and, *xor, *payload_bytes;
 	unsigned int masklen, extra_len = 0;
 	unsigned int payload_byte_size, payload_byte_offset;
 	uint8_t shift_imm, data[NFT_REG_SIZE];
@@ -2251,22 +2251,21 @@ static int stmt_evaluate_payload(struct eval_ctx *ctx, struct stmt *stmt)
 	}
 
 	if (shift_imm) {
-		struct expr *off;
+		struct expr *off, *lshift;
 
 		off = constant_expr_alloc(&payload->location,
 					  expr_basetype(payload),
 					  BYTEORDER_HOST_ENDIAN,
 					  sizeof(shift_imm), &shift_imm);
 
-		binop = binop_expr_alloc(&payload->location, OP_LSHIFT,
-					 stmt->payload.val, off);
-		binop->dtype		= payload->dtype;
-		binop->byteorder	= payload->byteorder;
+		lshift = binop_expr_alloc(&payload->location, OP_LSHIFT,
+					  stmt->payload.val, off);
+		lshift->dtype     = payload->dtype;
+		lshift->byteorder = payload->byteorder;
 
-		stmt->payload.val = binop;
+		stmt->payload.val = lshift;
 	}
 
-
 	masklen = payload_byte_size * BITS_PER_BYTE;
 	mpz_init_bitmask(ff, masklen);
 
@@ -2295,16 +2294,17 @@ static int stmt_evaluate_payload(struct eval_ctx *ctx, struct stmt *stmt)
 
 	and = binop_expr_alloc(&payload->location, OP_AND, payload_bytes, mask);
 
-	and->dtype		 = payload_bytes->dtype;
-	and->byteorder		 = payload_bytes->byteorder;
-	and->len		 = payload_bytes->len;
+	and->dtype	= payload_bytes->dtype;
+	and->byteorder	= payload_bytes->byteorder;
+	and->len	= payload_bytes->len;
+
+	xor = binop_expr_alloc(&payload->location, OP_XOR, and,
+			       stmt->payload.val);
+	xor->dtype	= payload->dtype;
+	xor->byteorder	= payload->byteorder;
+	xor->len	= mask->len;
 
-	binop = binop_expr_alloc(&payload->location, OP_XOR, and,
-				 stmt->payload.val);
-	binop->dtype		= payload->dtype;
-	binop->byteorder	= payload->byteorder;
-	binop->len		= mask->len;
-	stmt->payload.val = binop;
+	stmt->payload.val = xor;
 
 	return expr_evaluate(ctx, &stmt->payload.val);
 }
-- 
2.25.1

[PATCH nft v3 02/18] evaluate: simplify calculation of payload size.

[Jeremy Sowden]

Use div_round_up and one statement.

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 fda30fd8001e..e2eff2353657 100644
--- a/src/evaluate.c
+++ b/src/evaluate.c
@@ -2236,8 +2236,8 @@ static int stmt_evaluate_payload(struct eval_ctx *ctx, struct stmt *stmt)
 
 	shift_imm = expr_offset_shift(payload, payload->payload.offset,
 				      &extra_len);
-	payload_byte_size = round_up(payload->len, BITS_PER_BYTE) / BITS_PER_BYTE;
-	payload_byte_size += (extra_len / BITS_PER_BYTE);
+	payload_byte_size = div_round_up(payload->len + extra_len,
+					 BITS_PER_BYTE);
 
 	if (need_csum && payload_byte_size & 1) {
 		payload_byte_size++;
-- 
2.25.1

[PATCH nft v3 03/18] evaluate: don't evaluate payloads twice.

[Jeremy Sowden]

Payload munging means that evaluation of payload expressions may not be
idempotent.  Add a flag to prevent them from being evaluated more than
once.

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

diff --git a/include/expression.h b/include/expression.h
index 62fbbbb5a737..87c39e5de08a 100644
--- a/include/expression.h
+++ b/include/expression.h
@@ -300,6 +300,7 @@ struct expr {
 			enum proto_bases		base;
 			unsigned int			offset;
 			bool				is_raw;
+			bool				evaluated;
 		} payload;
 		struct {
 			/* EXPR_EXTHDR */
diff --git a/src/evaluate.c b/src/evaluate.c
index e2eff2353657..a169e41bd833 100644
--- a/src/evaluate.c
+++ b/src/evaluate.c
@@ -736,6 +736,9 @@ static int expr_evaluate_payload(struct eval_ctx *ctx, struct expr **exprp)
 {
 	struct expr *expr = *exprp;
 
+	if (expr->payload.evaluated)
+		return 0;
+
 	if (__expr_evaluate_payload(ctx, expr) < 0)
 		return -1;
 
@@ -745,6 +748,8 @@ static int expr_evaluate_payload(struct eval_ctx *ctx, struct expr **exprp)
 	if (payload_needs_adjustment(expr))
 		expr_evaluate_bits(ctx, exprp);
 
+	expr->payload.evaluated = true;
+
 	return 0;
 }
 
-- 
2.25.1

[PATCH nft v3 04/18] evaluate: convert the byte-order of payload statement arguments.

[Jeremy Sowden]

Since shift operations require host byte-order, we need to be able to
convert the result of the shift back to network byte-order, in a rule
like:

  nft add rule ip t c tcp dport set tcp dport lshift 1

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

diff --git a/src/evaluate.c b/src/evaluate.c
index a169e41bd833..9b1a04f26f44 100644
--- a/src/evaluate.c
+++ b/src/evaluate.c
@@ -2225,6 +2225,11 @@ static int stmt_evaluate_payload(struct eval_ctx *ctx, struct stmt *stmt)
 			      payload->byteorder, &stmt->payload.val) < 0)
 		return -1;
 
+	if (!expr_is_constant(stmt->payload.val) &&
+	    byteorder_conversion(ctx, &stmt->payload.val,
+				 payload->byteorder) < 0)
+		return -1;
+
 	need_csum = stmt_evaluate_payload_need_csum(payload);
 
 	if (!payload_needs_adjustment(payload)) {
-- 
2.25.1

[PATCH nft v3 05/18] evaluate: no need to swap byte-order for values of fewer than 16 bits.

[Jeremy Sowden]

Endianness is not meaningful for objects smaller than 2 bytes and the
byte-order conversions are no-ops in the kernel, so just update the
expression as if it were constant.

Signed-off-by: Jeremy Sowden <jeremy@azazel.net>
---
 src/evaluate.c              | 2 +-
 tests/py/any/meta.t.payload | 4 ----
 2 files changed, 1 insertion(+), 5 deletions(-)

diff --git a/src/evaluate.c b/src/evaluate.c
index 9b1a04f26f44..f4260436ae0f 100644
--- a/src/evaluate.c
+++ b/src/evaluate.c
@@ -147,7 +147,7 @@ static int byteorder_conversion(struct eval_ctx *ctx, struct expr **expr,
 				  byteorder_names[byteorder],
 				  byteorder_names[(*expr)->byteorder]);
 
-	if (expr_is_constant(*expr))
+	if (expr_is_constant(*expr) || (*expr)->len / BITS_PER_BYTE < 2)
 		(*expr)->byteorder = byteorder;
 	else {
 		op = byteorder_conversion_op(*expr, byteorder);
diff --git a/tests/py/any/meta.t.payload b/tests/py/any/meta.t.payload
index 486d7aa566ea..2af244a9e246 100644
--- a/tests/py/any/meta.t.payload
+++ b/tests/py/any/meta.t.payload
@@ -99,14 +99,12 @@ ip test-ip4 input
 # meta l4proto 33-45
 ip test-ip4 input
   [ meta load l4proto => reg 1 ]
-  [ byteorder reg 1 = hton(reg 1, 2, 1) ]
   [ cmp gte reg 1 0x00000021 ]
   [ cmp lte reg 1 0x0000002d ]
 
 # meta l4proto != 33-45
 ip test-ip4 input
   [ meta load l4proto => reg 1 ]
-  [ byteorder reg 1 = hton(reg 1, 2, 1) ]
   [ range neq reg 1 0x00000021 0x0000002d ]
 
 # meta l4proto { 33, 55, 67, 88}
@@ -865,7 +863,6 @@ __set%d test-ip4 0
 	element 00000000  : 1 [end]	element 00000021  : 0 [end]	element 00000038  : 1 [end]	element 00000042  : 0 [end]	element 00000059  : 1 [end]
 ip test-ip4 input 
   [ meta load l4proto => reg 1 ]
-  [ byteorder reg 1 = hton(reg 1, 2, 1) ]
   [ lookup reg 1 set __set%d ]
 
 # meta l4proto != { 33-55, 66-88}
@@ -874,7 +871,6 @@ __set%d test-ip4 0
 	element 00000000  : 1 [end]	element 00000021  : 0 [end]	element 00000038  : 1 [end]	element 00000042  : 0 [end]	element 00000059  : 1 [end]
 ip test-ip4 input 
   [ meta load l4proto => reg 1 ]
-  [ byteorder reg 1 = hton(reg 1, 2, 1) ]
   [ lookup reg 1 set __set%d 0x1 ]
 
 # meta skuid { 2001-2005, 3001-3005} accept
-- 
2.25.1

[PATCH nft v3 06/18] netlink_delinearize: set shift RHS byte-order.

[Jeremy Sowden]

The RHS operand for bitwise shift is in HBO.  Set this explicitly.

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 0058e2cfe42a..3c80895a43f9 100644
--- a/src/netlink_delinearize.c
+++ b/src/netlink_delinearize.c
@@ -440,6 +440,7 @@ static struct expr *netlink_parse_bitwise_shift(struct netlink_parse_ctx *ctx,
 
 	nld.value = nftnl_expr_get(nle, NFTNL_EXPR_BITWISE_DATA, &nld.len);
 	right = netlink_alloc_value(loc, &nld);
+	right->byteorder = BYTEORDER_HOST_ENDIAN;
 
 	expr = binop_expr_alloc(loc, op, left, right);
 	expr->len = left->len;
-- 
2.25.1

[PATCH nft v3 07/18] src: fix leaks.

[Jeremy Sowden]

Some bitmask variables are not cleared.

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

diff --git a/src/evaluate.c b/src/evaluate.c
index f4260436ae0f..4a23b231c74d 100644
--- a/src/evaluate.c
+++ b/src/evaluate.c
@@ -483,6 +483,7 @@ static void expr_evaluate_bits(struct eval_ctx *ctx, struct expr **exprp)
 	mask = constant_expr_alloc(&expr->location, expr_basetype(expr),
 				   BYTEORDER_HOST_ENDIAN, masklen, NULL);
 	mpz_set(mask->value, bitmask);
+	mpz_clear(bitmask);
 
 	and = binop_expr_alloc(&expr->location, OP_AND, expr, mask);
 	and->dtype	= expr->dtype;
@@ -2290,6 +2291,7 @@ static int stmt_evaluate_payload(struct eval_ctx *ctx, struct stmt *stmt)
 	mpz_export_data(data, bitmask, BYTEORDER_HOST_ENDIAN, sizeof(data));
 	mask = constant_expr_alloc(&payload->location, expr_basetype(payload),
 				   BYTEORDER_HOST_ENDIAN, masklen, data);
+	mpz_clear(bitmask);
 
 	payload_bytes = payload_expr_alloc(&payload->location, NULL, 0);
 	payload_init_raw(payload_bytes, payload->payload.base,
diff --git a/src/netlink_delinearize.c b/src/netlink_delinearize.c
index 3c80895a43f9..79efda123c14 100644
--- a/src/netlink_delinearize.c
+++ b/src/netlink_delinearize.c
@@ -2558,6 +2558,7 @@ static void stmt_payload_binop_postprocess(struct rule_pp_ctx *ctx)
 			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;
-- 
2.25.1

[PATCH nft v3 08/18] include: update nf_tables.h.

[Jeremy Sowden]

Pick up a couple of new bitwise netlink attributes.

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

diff --git a/include/linux/netfilter/nf_tables.h b/include/linux/netfilter/nf_tables.h
index 065218a20bb7..57e83e152bf3 100644
--- a/include/linux/netfilter/nf_tables.h
+++ b/include/linux/netfilter/nf_tables.h
@@ -526,6 +526,8 @@ 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_MREG: mask register (NLA_U32: nft_registers)
+ * @NFTA_BITWISE_XREG: xor register (NLA_U32: nft_registers)
  *
  * The bitwise expression supports boolean and shift operations.  It implements
  * the boolean operations by performing the following operation:
@@ -549,6 +551,8 @@ enum nft_bitwise_attributes {
 	NFTA_BITWISE_XOR,
 	NFTA_BITWISE_OP,
 	NFTA_BITWISE_DATA,
+	NFTA_BITWISE_MREG,
+	NFTA_BITWISE_XREG,
 	__NFTA_BITWISE_MAX
 };
 #define NFTA_BITWISE_MAX	(__NFTA_BITWISE_MAX - 1)
-- 
2.25.1

[PATCH nft v3 09/18] src: support (de)linearization of bitwise op's with variable right operands.

[Jeremy Sowden]

Hitherto, the kernel has required constant values for the xor and mask
attributes of boolean bitwise expressions.  This has meant that the
righthand argument of a boolean binop must be constant.  Now the kernel
supports passing mask and xor via registers, we need to be able to
generate and parse bitwise boolean expressions that do this.

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

diff --git a/src/netlink_delinearize.c b/src/netlink_delinearize.c
index 79efda123c14..4fc7b764d7a9 100644
--- a/src/netlink_delinearize.c
+++ b/src/netlink_delinearize.c
@@ -363,11 +363,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_bool_constant(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;
@@ -428,6 +429,69 @@ static struct expr *netlink_parse_bitwise_bool(struct netlink_parse_ctx *ctx,
 	return expr;
 }
 
+static struct expr *
+netlink_parse_bitwise_bool_variable(struct netlink_parse_ctx *ctx,
+				    const struct location *loc,
+				    const struct nftnl_expr *nle,
+				    enum nft_registers sreg,
+				    struct expr *left)
+
+{
+	enum nft_registers mreg, xreg;
+	struct expr *mask, *xor;
+
+	if (nftnl_expr_is_set(nle, NFTNL_EXPR_BITWISE_MASK)) {
+		/*
+		 * a ^ b = (a & 1) ^ b.
+		 */
+		xreg = netlink_parse_register(nle, NFTNL_EXPR_BITWISE_XREG);
+		xor = netlink_get_register(ctx, loc, xreg);
+		if (xor == NULL) {
+			netlink_error(ctx, loc,
+				      "Bitwise expression has no xor expression");
+			return NULL;
+		}
+		return binop_expr_alloc(loc, OP_XOR, left, xor);
+	}
+
+	if (nftnl_expr_is_set(nle, NFTNL_EXPR_BITWISE_XOR)) {
+		/*
+		 * a & b = (a & b) ^ 0.
+		 */
+		mreg = netlink_parse_register(nle, NFTNL_EXPR_BITWISE_MREG);
+		mask = netlink_get_register(ctx, loc, mreg);
+		if (mask == NULL) {
+			netlink_error(ctx, loc,
+				      "Bitwise expression has no mask expression");
+			return NULL;
+		}
+		return binop_expr_alloc(loc, OP_AND, left, mask);
+	}
+
+	/*
+	 * a | b = (a & ~b) ^ b.
+	 */
+	mreg = netlink_parse_register(nle, NFTNL_EXPR_BITWISE_MREG);
+	mask = netlink_get_register(ctx, loc, mreg);
+	if (mask == NULL) {
+		netlink_error(ctx, loc,
+			      "Bitwise expression has no mask expression");
+		return NULL;
+	}
+
+	expr_free(mask);
+
+	xreg = netlink_parse_register(nle, NFTNL_EXPR_BITWISE_XREG);
+	xor = netlink_get_register(ctx, loc, xreg);
+	if (xor == NULL) {
+		netlink_error(ctx, loc,
+			      "Bitwise expression has no xor expression");
+		return NULL;
+	}
+
+	return binop_expr_alloc(loc, OP_OR, left, xor);
+}
+
 static struct expr *netlink_parse_bitwise_shift(struct netlink_parse_ctx *ctx,
 						const struct location *loc,
 						const struct nftnl_expr *nle,
@@ -467,8 +531,13 @@ static void netlink_parse_bitwise(struct netlink_parse_ctx *ctx,
 
 	switch (op) {
 	case NFT_BITWISE_BOOL:
-		expr = netlink_parse_bitwise_bool(ctx, loc, nle, sreg,
-						  left);
+		if (nftnl_expr_is_set(nle, NFTNL_EXPR_BITWISE_MASK) &&
+		    nftnl_expr_is_set(nle, NFTNL_EXPR_BITWISE_XOR))
+			expr = netlink_parse_bitwise_bool_constant(ctx, loc, nle,
+								   sreg, left);
+		else
+			expr = netlink_parse_bitwise_bool_variable(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 de461775a7e1..b2987efbc49f 100644
--- a/src/netlink_linearize.c
+++ b/src/netlink_linearize.c
@@ -572,9 +572,9 @@ static void netlink_gen_shift(struct netlink_linearize_ctx *ctx,
 	nftnl_rule_add_expr(ctx->nlr, nle);
 }
 
-static void netlink_gen_bitwise(struct netlink_linearize_ctx *ctx,
-				const struct expr *expr,
-				enum nft_registers dreg)
+static void netlink_gen_bitwise_constant(struct netlink_linearize_ctx *ctx,
+					 const struct expr *expr,
+					 enum nft_registers dreg)
 {
 	struct nftnl_expr *nle;
 	struct nft_data_linearize nld;
@@ -642,6 +642,89 @@ static void netlink_gen_bitwise(struct netlink_linearize_ctx *ctx,
 	nftnl_rule_add_expr(ctx->nlr, nle);
 }
 
+static void netlink_gen_bitwise_variable(struct netlink_linearize_ctx *ctx,
+					 const struct expr *expr,
+					 enum nft_registers dreg)
+{
+	struct nft_data_linearize nld;
+	enum nft_registers mreg, xreg;
+	struct nftnl_expr *nle;
+	unsigned int len;
+	mpz_t m, x;
+
+	netlink_gen_expr(ctx, expr->left, dreg);
+
+	len = div_round_up(expr->len, BITS_PER_BYTE);
+
+	nle = alloc_nft_expr("bitwise");
+
+	switch (expr->op) {
+	case OP_XOR:
+		/*
+		 * a ^ b = (a & 1) ^ b.
+		 */
+		mpz_init_bitmask(m, expr->len);
+		netlink_gen_raw_data(m, expr->byteorder, len, &nld);
+		nftnl_expr_set(nle, NFTNL_EXPR_BITWISE_MASK, nld.value, nld.len);
+		mpz_clear(m);
+
+		xreg = get_register(ctx, expr->right);
+		netlink_gen_expr(ctx, expr->right, xreg);
+		netlink_put_register(nle, NFTNL_EXPR_BITWISE_XREG, xreg);
+		break;
+	case OP_AND:
+		/*
+		 * a & b = (a & b) ^ 0.
+		 */
+		mreg = get_register(ctx, expr->right);
+		netlink_gen_expr(ctx, expr->right, mreg);
+		netlink_put_register(nle, NFTNL_EXPR_BITWISE_MREG, mreg);
+
+		mpz_init_set_ui(x, 0);
+		netlink_gen_raw_data(x, expr->byteorder, len, &nld);
+		nftnl_expr_set(nle, NFTNL_EXPR_BITWISE_XOR, nld.value, nld.len);
+		mpz_clear(x);
+		break;
+	case OP_OR: {
+		/*
+		 * a | b = (a & ~b)      ^ b
+		 *       = (a & (b ^ 1)) ^ b.
+		 */
+		struct expr *one, *not;
+		unsigned long tmp;
+
+		mpz_init_bitmask(m, expr->right->len);
+		tmp = mpz_get_ui(m);
+		mpz_clear(m);
+
+		one = constant_expr_alloc(&expr->location, &integer_type,
+					  expr->right->byteorder,
+					  expr->right->len, &tmp);
+		not = binop_expr_alloc(&expr->location, OP_XOR,
+				       expr_get(expr->right), one);
+		not->len = expr->right->len;
+		mreg = get_register(ctx, not);
+		netlink_gen_expr(ctx, not, mreg);
+		netlink_put_register(nle, NFTNL_EXPR_BITWISE_MREG, mreg);
+		expr_free(not);
+
+		xreg = get_register(ctx, expr->right);
+		netlink_gen_expr(ctx, expr->right, xreg);
+		netlink_put_register(nle, NFTNL_EXPR_BITWISE_XREG, xreg);
+		break;
+	}
+	default:
+		BUG("invalid binary operation %u\n", expr->op);
+	}
+
+	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_LEN, 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)
@@ -652,7 +735,11 @@ static void netlink_gen_binop(struct netlink_linearize_ctx *ctx,
 		netlink_gen_shift(ctx, expr, dreg);
 		break;
 	default:
-		netlink_gen_bitwise(ctx, expr, dreg);
+		if (expr_is_constant(expr->right)) {
+			netlink_gen_bitwise_constant(ctx, expr, dreg);
+		} else {
+			netlink_gen_bitwise_variable(ctx, expr, dreg);
+		}
 		break;
 	}
 }
-- 
2.25.1

[PATCH nft v3 10/18] evaluate: allow boolean binop expressions with variable righthand arguments.

[Jeremy Sowden]

Hitherto, the kernel has required constant values for the xor and mask
attributes of boolean bitwise expressions.  This has meant that the
righthand argument of a boolean binop must be constant.  Now the kernel
supports passing mask and xor via registers, we can relax this
restriction.

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

diff --git a/src/evaluate.c b/src/evaluate.c
index 4a23b231c74d..1db175007c2d 100644
--- a/src/evaluate.c
+++ b/src/evaluate.c
@@ -1135,7 +1135,7 @@ static int expr_evaluate_bitwise(struct eval_ctx *ctx, struct expr **expr)
 	op->byteorder = left->byteorder;
 	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;
 }
@@ -1179,23 +1179,22 @@ 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:
-- 
2.25.1

[PATCH nft v3 11/18] evaluate: don't clobber binop bitmask lengths.

[Jeremy Sowden]

In this example:

nft --debug=netlink add rule ip t c ip dscp set ip dscp
ip t c
  [ 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 << 0x00000002 ) ]
  [ bitwise reg 1 = (reg=1 & 0x0000ffff ) ^ 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 is length is 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 | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/evaluate.c b/src/evaluate.c
index 1db175007c2d..ff168434cd8f 100644
--- a/src/evaluate.c
+++ b/src/evaluate.c
@@ -1133,7 +1133,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 ? op->len : left->len;
 
 	if (expr_is_constant(left) && expr_is_constant(op->right))
 		return constant_binop_simplify(ctx, expr);
-- 
2.25.1

[PATCH nft v3 12/18] netlink_delinearize: fix typo.

[Jeremy Sowden]

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 4fc7b764d7a9..4e5d64ede8bd 100644
--- a/src/netlink_delinearize.c
+++ b/src/netlink_delinearize.c
@@ -2530,7 +2530,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.25.1

[PATCH nft v3 13/18] netlink_delinearize: refactor stmt_payload_binop_postprocess.

[Jeremy Sowden]

Move the contents of the two major cases (I and II) into separate
functions.

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

diff --git a/src/netlink_delinearize.c b/src/netlink_delinearize.c
index 4e5d64ede8bd..e8e9e5719ee8 100644
--- a/src/netlink_delinearize.c
+++ b/src/netlink_delinearize.c
@@ -2514,6 +2514,109 @@ 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);
+	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
  *
@@ -2558,9 +2661,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;
 
@@ -2568,93 +2670,13 @@ 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)
+	case EXPR_BINOP: /* I? */
+		if (stmt_payload_binop_postprocess_i(ctx))
 			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))
-			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);
-		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)
-			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 */
+		if (stmt_payload_binop_postprocess_ii(ctx))
 			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.25.1

[PATCH nft v3 14/18] netlink_delinearize: add support for processing variable payload statement arguments.

[Jeremy Sowden]

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 | 74 +++++++++++++++++++++++++++++++++++----
 1 file changed, 68 insertions(+), 6 deletions(-)

diff --git a/src/netlink_delinearize.c b/src/netlink_delinearize.c
index e8e9e5719ee8..571cab1d932b 100644
--- a/src/netlink_delinearize.c
+++ b/src/netlink_delinearize.c
@@ -2514,7 +2514,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;
@@ -2568,6 +2568,56 @@ 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;
+
+	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;
@@ -2634,21 +2684,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)
@@ -2671,7 +2730,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;
 	case EXPR_PAYLOAD: /* II? */
-- 
2.25.1

[PATCH nft v3 15/18] netlink_delinearize: add postprocessing for payload binops.

[Jeremy Sowden]

If a user uses a variable payload expression in a payload statement, we
need to undo any munging during delinearization.

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

diff --git a/src/netlink_delinearize.c b/src/netlink_delinearize.c
index 571cab1d932b..73faa93c862e 100644
--- a/src/netlink_delinearize.c
+++ b/src/netlink_delinearize.c
@@ -2125,6 +2125,30 @@ static void relational_binop_postprocess(struct rule_pp_ctx *ctx, struct expr *e
 	}
 }
 
+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_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);
+	*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)
@@ -2258,6 +2282,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.25.1

[PATCH nft v3 16/18] tests: shell: remove stray debug flag.

[Jeremy Sowden]

0040mark_shift_0 was passing --debug=eval to nft.  Remove it.

Signed-off-by: Jeremy Sowden <jeremy@azazel.net>
---
 tests/shell/testcases/chains/0040mark_shift_0 | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tests/shell/testcases/chains/0040mark_shift_0 b/tests/shell/testcases/chains/0040mark_shift_0
index 55447f0b9737..ef3dccfa049a 100755
--- a/tests/shell/testcases/chains/0040mark_shift_0
+++ b/tests/shell/testcases/chains/0040mark_shift_0
@@ -8,4 +8,4 @@ RULESET="
   add rule t c oif lo ct mark set (meta mark | 0x10) << 8
 "
 
-$NFT --debug=eval -f - <<< "$RULESET"
+$NFT -f - <<< "$RULESET"
-- 
2.25.1

[PATCH nft v3 17/18] tests: shell: add variable binop RHS tests.

[Jeremy Sowden]

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

Signed-off-by: Jeremy Sowden <jeremy@azazel.net>
---
 tests/shell/testcases/chains/0040mark_shift_2         | 11 +++++++++++
 tests/shell/testcases/chains/0041payload_variable_0   | 11 +++++++++++
 tests/shell/testcases/chains/0041payload_variable_1   | 11 +++++++++++
 tests/shell/testcases/chains/0041payload_variable_2   | 11 +++++++++++
 tests/shell/testcases/chains/0041payload_variable_3   | 11 +++++++++++
 .../shell/testcases/chains/dumps/0040mark_shift_2.nft |  6 ++++++
 .../testcases/chains/dumps/0041payload_variable_0.nft |  6 ++++++
 .../testcases/chains/dumps/0041payload_variable_1.nft |  6 ++++++
 .../testcases/chains/dumps/0041payload_variable_2.nft |  6 ++++++
 .../testcases/chains/dumps/0041payload_variable_3.nft |  6 ++++++
 10 files changed, 85 insertions(+)
 create mode 100755 tests/shell/testcases/chains/0040mark_shift_2
 create mode 100755 tests/shell/testcases/chains/0041payload_variable_0
 create mode 100755 tests/shell/testcases/chains/0041payload_variable_1
 create mode 100755 tests/shell/testcases/chains/0041payload_variable_2
 create mode 100755 tests/shell/testcases/chains/0041payload_variable_3
 create mode 100644 tests/shell/testcases/chains/dumps/0040mark_shift_2.nft
 create mode 100644 tests/shell/testcases/chains/dumps/0041payload_variable_0.nft
 create mode 100644 tests/shell/testcases/chains/dumps/0041payload_variable_1.nft
 create mode 100644 tests/shell/testcases/chains/dumps/0041payload_variable_2.nft
 create mode 100644 tests/shell/testcases/chains/dumps/0041payload_variable_3.nft

diff --git a/tests/shell/testcases/chains/0040mark_shift_2 b/tests/shell/testcases/chains/0040mark_shift_2
new file mode 100755
index 000000000000..2ff3418bdd3f
--- /dev/null
+++ b/tests/shell/testcases/chains/0040mark_shift_2
@@ -0,0 +1,11 @@
+#!/bin/bash
+
+set -e
+
+RULESET="
+  add table t
+  add chain t c { type filter hook input priority mangle; }
+  add rule t c ct mark set ct mark and 0xffff0000 or meta mark and 0xffff
+"
+
+$NFT -f - <<< "$RULESET"
diff --git a/tests/shell/testcases/chains/0041payload_variable_0 b/tests/shell/testcases/chains/0041payload_variable_0
new file mode 100755
index 000000000000..c9819ff4ab88
--- /dev/null
+++ b/tests/shell/testcases/chains/0041payload_variable_0
@@ -0,0 +1,11 @@
+#!/bin/bash
+
+set -e
+
+RULESET="
+  add table t
+  add chain t c { type filter hook output priority mangle; }
+  add rule ip t c tcp dport set tcp dport
+"
+
+$NFT -f - <<< "$RULESET"
diff --git a/tests/shell/testcases/chains/0041payload_variable_1 b/tests/shell/testcases/chains/0041payload_variable_1
new file mode 100755
index 000000000000..e9b1e1dde515
--- /dev/null
+++ b/tests/shell/testcases/chains/0041payload_variable_1
@@ -0,0 +1,11 @@
+#!/bin/bash
+
+set -e
+
+RULESET="
+  add table t
+  add chain t c { type filter hook output priority mangle; }
+  add rule ip t c tcp dport set tcp dport lshift 1
+"
+
+$NFT -f - <<< "$RULESET"
diff --git a/tests/shell/testcases/chains/0041payload_variable_2 b/tests/shell/testcases/chains/0041payload_variable_2
new file mode 100755
index 000000000000..5a458ef5d525
--- /dev/null
+++ b/tests/shell/testcases/chains/0041payload_variable_2
@@ -0,0 +1,11 @@
+#!/bin/bash
+
+set -e
+
+RULESET="
+  add table t
+  add chain t c { type filter hook output priority mangle; }
+  add rule ip t c ip dscp set ip dscp
+"
+
+$NFT -f - <<< "$RULESET"
diff --git a/tests/shell/testcases/chains/0041payload_variable_3 b/tests/shell/testcases/chains/0041payload_variable_3
new file mode 100755
index 000000000000..0375399c3d0f
--- /dev/null
+++ b/tests/shell/testcases/chains/0041payload_variable_3
@@ -0,0 +1,11 @@
+#!/bin/bash
+
+set -e
+
+RULESET="
+  add table t
+  add chain t c { type filter hook output priority mangle; }
+  add rule ip t c ip dscp set ip dscp or 0x3
+"
+
+$NFT -f - <<< "$RULESET"
diff --git a/tests/shell/testcases/chains/dumps/0040mark_shift_2.nft b/tests/shell/testcases/chains/dumps/0040mark_shift_2.nft
new file mode 100644
index 000000000000..14f2d1685706
--- /dev/null
+++ b/tests/shell/testcases/chains/dumps/0040mark_shift_2.nft
@@ -0,0 +1,6 @@
+table ip t {
+	chain c {
+		type filter hook input priority mangle; policy accept;
+		ct mark set ct mark & 0xffff0000 | meta mark & 0x0000ffff
+	}
+}
diff --git a/tests/shell/testcases/chains/dumps/0041payload_variable_0.nft b/tests/shell/testcases/chains/dumps/0041payload_variable_0.nft
new file mode 100644
index 000000000000..731949e6b355
--- /dev/null
+++ b/tests/shell/testcases/chains/dumps/0041payload_variable_0.nft
@@ -0,0 +1,6 @@
+table ip t {
+	chain c {
+		type filter hook output priority mangle; policy accept;
+		tcp dport set tcp dport
+	}
+}
diff --git a/tests/shell/testcases/chains/dumps/0041payload_variable_1.nft b/tests/shell/testcases/chains/dumps/0041payload_variable_1.nft
new file mode 100644
index 000000000000..2cd87a4dc8e1
--- /dev/null
+++ b/tests/shell/testcases/chains/dumps/0041payload_variable_1.nft
@@ -0,0 +1,6 @@
+table ip t {
+	chain c {
+		type filter hook output priority mangle; policy accept;
+		tcp dport set tcp dport << 1
+	}
+}
diff --git a/tests/shell/testcases/chains/dumps/0041payload_variable_2.nft b/tests/shell/testcases/chains/dumps/0041payload_variable_2.nft
new file mode 100644
index 000000000000..d1e7adb92c4c
--- /dev/null
+++ b/tests/shell/testcases/chains/dumps/0041payload_variable_2.nft
@@ -0,0 +1,6 @@
+table ip t {
+	chain c {
+		type filter hook output priority mangle; policy accept;
+		ip dscp set ip dscp
+	}
+}
diff --git a/tests/shell/testcases/chains/dumps/0041payload_variable_3.nft b/tests/shell/testcases/chains/dumps/0041payload_variable_3.nft
new file mode 100644
index 000000000000..52b3a833175a
--- /dev/null
+++ b/tests/shell/testcases/chains/dumps/0041payload_variable_3.nft
@@ -0,0 +1,6 @@
+table ip t {
+	chain c {
+		type filter hook output priority mangle; policy accept;
+		ip dscp set ip dscp | 0x03
+	}
+}
-- 
2.25.1

[PATCH nft v3 18/18] tests: py: add variable binop RHS tests.

[Jeremy Sowden]

Add some tests to validate setting payload fields and 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       | 33 ++++++++++++++++
 tests/py/inet/tcp.t             |  2 +
 tests/py/inet/tcp.t.json        | 46 +++++++++++++++++++++-
 tests/py/inet/tcp.t.payload     | 68 +++++++++++++++++++++++++++++++++
 tests/py/ip/ip.t                |  3 ++
 tests/py/ip/ip.t.json           | 66 ++++++++++++++++++++++++++++++++
 tests/py/ip/ip.t.payload        | 26 +++++++++++++
 tests/py/ip/ip.t.payload.bridge | 30 +++++++++++++++
 tests/py/ip/ip.t.payload.inet   | 30 +++++++++++++++
 tests/py/ip/ip.t.payload.netdev | 30 +++++++++++++++
 12 files changed, 371 insertions(+), 1 deletion(-)

diff --git a/tests/py/any/ct.t b/tests/py/any/ct.t
index f65d275987cd..0581c6a4fd8f 100644
--- a/tests/py/any/ct.t
+++ b/tests/py/any/ct.t
@@ -59,6 +59,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 59ac27c3055c..aca7c3243cc0 100644
--- a/tests/py/any/ct.t.json
+++ b/tests/py/any/ct.t.json
@@ -779,6 +779,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 661591257804..17a1c382ea65 100644
--- a/tests/py/any/ct.t.payload
+++ b/tests/py/any/ct.t.payload
@@ -359,6 +359,39 @@ 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 ) ^ 0xffffffff ]
+  [ meta load mark => reg 3 ]
+  [ bitwise reg 3 = (reg=3 & 0x0000ffff ) ^ 0x00000000 ]
+  [ bitwise reg 1 = (reg=1 & reg 2 ) ^ reg 3 ]
+  [ ct set mark with reg 1 ]
+
+# ct mark set ct mark and 0xffff0000 or meta mark and 0xffff
+ip6
+  [ ct load mark => reg 1 ]
+  [ bitwise reg 1 = (reg=1 & 0xffff0000 ) ^ 0x00000000 ]
+  [ meta load mark => reg 2 ]
+  [ bitwise reg 2 = (reg=2 & 0x0000ffff ) ^ 0xffffffff ]
+  [ meta load mark => reg 3 ]
+  [ bitwise reg 3 = (reg=3 & 0x0000ffff ) ^ 0x00000000 ]
+  [ bitwise reg 1 = (reg=1 & reg 2 ) ^ reg 3 ]
+  [ ct set mark with reg 1 ]
+
+# ct mark set ct mark and 0xffff0000 or meta mark and 0xffff
+inet
+  [ ct load mark => reg 1 ]
+  [ bitwise reg 1 = (reg=1 & 0xffff0000 ) ^ 0x00000000 ]
+  [ meta load mark => reg 2 ]
+  [ bitwise reg 2 = (reg=2 & 0x0000ffff ) ^ 0xffffffff ]
+  [ meta load mark => reg 3 ]
+  [ bitwise reg 3 = (reg=3 & 0x0000ffff ) ^ 0x00000000 ]
+  [ bitwise reg 1 = (reg=1 & reg 2 ) ^ reg 3 ]
+  [ 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/tcp.t b/tests/py/inet/tcp.t
index e0a83e2b4152..081248643981 100644
--- a/tests/py/inet/tcp.t
+++ b/tests/py/inet/tcp.t
@@ -7,6 +7,8 @@
 *netdev;test-netdev;ingress
 
 tcp dport set {1, 2, 3};fail
+tcp dport set tcp dport;ok
+tcp dport set tcp dport lshift 1;ok;tcp dport set tcp dport << 1
 
 tcp dport 22;ok
 tcp dport != 233;ok
diff --git a/tests/py/inet/tcp.t.json b/tests/py/inet/tcp.t.json
index babe59208925..d8375fbe1e85 100644
--- a/tests/py/inet/tcp.t.json
+++ b/tests/py/inet/tcp.t.json
@@ -1,3 +1,48 @@
+# tcp dport set tcp dport
+[
+    {
+        "mangle": {
+            "key": {
+                "payload": {
+                    "field": "dport",
+                    "protocol": "tcp"
+                }
+            },
+            "value": {
+                "payload": {
+                    "field": "dport",
+                    "protocol": "tcp"
+                }
+            }
+        }
+    }
+]
+
+# tcp dport set tcp dport lshift 1
+[
+    {
+        "mangle": {
+            "key": {
+                "payload": {
+                    "field": "dport",
+                    "protocol": "tcp"
+                }
+            },
+            "value": {
+                "<<": [
+                    {
+                        "payload": {
+                            "field": "dport",
+                            "protocol": "tcp"
+                        }
+                    },
+                    1
+                ]
+            }
+        }
+    }
+]
+
 # tcp dport 22
 [
     {
@@ -1636,4 +1681,3 @@
         }
     }
 ]
-
diff --git a/tests/py/inet/tcp.t.payload b/tests/py/inet/tcp.t.payload
index 55f1bc2eff87..13de1ff80722 100644
--- a/tests/py/inet/tcp.t.payload
+++ b/tests/py/inet/tcp.t.payload
@@ -1,3 +1,71 @@
+# tcp dport set tcp dport
+ip
+  [ meta load l4proto => reg 1 ]
+  [ cmp eq reg 1 0x00000006 ]
+  [ payload load 2b @ transport header + 2 => reg 1 ]
+  [ payload write reg 1 => 2b @ transport header + 2 csum_type 1 csum_off 16 csum_flags 0x0 ]
+
+# tcp dport set tcp dport
+ip6
+  [ meta load l4proto => reg 1 ]
+  [ cmp eq reg 1 0x00000006 ]
+  [ payload load 2b @ transport header + 2 => reg 1 ]
+  [ payload write reg 1 => 2b @ transport header + 2 csum_type 1 csum_off 16 csum_flags 0x0 ]
+
+# tcp dport set tcp dport
+inet
+  [ meta load l4proto => reg 1 ]
+  [ cmp eq reg 1 0x00000006 ]
+  [ payload load 2b @ transport header + 2 => reg 1 ]
+  [ payload write reg 1 => 2b @ transport header + 2 csum_type 1 csum_off 16 csum_flags 0x0 ]
+
+# tcp dport set tcp dport
+netdev
+  [ meta load l4proto => reg 1 ]
+  [ cmp eq reg 1 0x00000006 ]
+  [ payload load 2b @ transport header + 2 => reg 1 ]
+  [ payload write reg 1 => 2b @ transport header + 2 csum_type 1 csum_off 16 csum_flags 0x0 ]
+
+# tcp dport set tcp dport lshift 1
+ip
+  [ meta load l4proto => reg 1 ]
+  [ cmp eq reg 1 0x00000006 ]
+  [ payload load 2b @ transport header + 2 => reg 1 ]
+  [ byteorder reg 1 = ntoh(reg 1, 2, 2) ]
+  [ bitwise reg 1 = ( reg 1 << 0x00000001 ) ]
+  [ byteorder reg 1 = hton(reg 1, 2, 2) ]
+  [ payload write reg 1 => 2b @ transport header + 2 csum_type 1 csum_off 16 csum_flags 0x0 ]
+
+# tcp dport set tcp dport lshift 1
+ip6
+  [ meta load l4proto => reg 1 ]
+  [ cmp eq reg 1 0x00000006 ]
+  [ payload load 2b @ transport header + 2 => reg 1 ]
+  [ byteorder reg 1 = ntoh(reg 1, 2, 2) ]
+  [ bitwise reg 1 = ( reg 1 << 0x00000001 ) ]
+  [ byteorder reg 1 = hton(reg 1, 2, 2) ]
+  [ payload write reg 1 => 2b @ transport header + 2 csum_type 1 csum_off 16 csum_flags 0x0 ]
+
+# tcp dport set tcp dport lshift 1
+inet
+  [ meta load l4proto => reg 1 ]
+  [ cmp eq reg 1 0x00000006 ]
+  [ payload load 2b @ transport header + 2 => reg 1 ]
+  [ byteorder reg 1 = ntoh(reg 1, 2, 2) ]
+  [ bitwise reg 1 = ( reg 1 << 0x00000001 ) ]
+  [ byteorder reg 1 = hton(reg 1, 2, 2) ]
+  [ payload write reg 1 => 2b @ transport header + 2 csum_type 1 csum_off 16 csum_flags 0x0 ]
+
+# tcp dport set tcp dport lshift 1
+netdev
+  [ meta load l4proto => reg 1 ]
+  [ cmp eq reg 1 0x00000006 ]
+  [ payload load 2b @ transport header + 2 => reg 1 ]
+  [ byteorder reg 1 = ntoh(reg 1, 2, 2) ]
+  [ bitwise reg 1 = ( reg 1 << 0x00000001 ) ]
+  [ byteorder reg 1 = hton(reg 1, 2, 2) ]
+  [ payload write reg 1 => 2b @ transport header + 2 csum_type 1 csum_off 16 csum_flags 0x0 ]
+
 # tcp dport 22
 inet test-inet input
   [ meta load l4proto => reg 1 ]
diff --git a/tests/py/ip/ip.t b/tests/py/ip/ip.t
index 0421d01bf6e4..1a5fb5c0efb3 100644
--- a/tests/py/ip/ip.t
+++ b/tests/py/ip/ip.t
@@ -135,3 +135,6 @@ 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 ip dscp;ok
+iif "lo" ip dscp set ip dscp or 0x3;ok;iif "lo" ip dscp set ip dscp | 0x03
diff --git a/tests/py/ip/ip.t.json b/tests/py/ip/ip.t.json
index 3131ab790c04..4e0cef9357c8 100644
--- a/tests/py/ip/ip.t.json
+++ b/tests/py/ip/ip.t.json
@@ -1836,3 +1836,69 @@
     }
 ]
 
+# iif "lo" ip dscp set ip dscp
+[
+    {
+        "match": {
+            "left": {
+                "meta": {
+                    "key": "iif"
+                }
+            },
+            "op": "==",
+            "right": "lo"
+        }
+    },
+    {
+        "mangle": {
+            "key": {
+                "payload": {
+                    "field": "dscp",
+                    "protocol": "ip"
+                }
+            },
+            "value": {
+                "payload": {
+                    "field": "dscp",
+                    "protocol": "ip"
+                }
+            }
+        }
+    }
+]
+
+# iif "lo" ip dscp set ip dscp or 0x3
+[
+    {
+        "match": {
+            "left": {
+                "meta": {
+                    "key": "iif"
+                }
+            },
+            "op": "==",
+            "right": "lo"
+        }
+    },
+    {
+        "mangle": {
+            "key": {
+                "payload": {
+                    "field": "dscp",
+                    "protocol": "ip"
+                }
+            },
+            "value": {
+                "|": [
+                    {
+                        "payload": {
+                            "field": "dscp",
+                            "protocol": "ip"
+                        }
+                    },
+                    3
+                ]
+            }
+        }
+    }
+]
diff --git a/tests/py/ip/ip.t.payload b/tests/py/ip/ip.t.payload
index d627b22f2614..d6c5d14d52ac 100644
--- a/tests/py/ip/ip.t.payload
+++ b/tests/py/ip/ip.t.payload
@@ -614,3 +614,29 @@ ip test-ip4 input
   [ bitwise reg 1 = (reg=1 & 0x000000ff ) ^ 0x00000100 ]
   [ payload write reg 1 => 2b @ network header + 8 csum_type 1 csum_off 10 csum_flags 0x1 ]
 
+# iif "lo" ip dscp set ip dscp
+ip
+  [ 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 << 0x00000002 ) ]
+  [ bitwise reg 1 = (reg=1 & 0x0000ffff ) ^ 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 or 0x3
+ip
+  [ 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 & 0x000000fc ) ^ 0x00000003 ]
+  [ bitwise reg 2 = ( reg 2 << 0x00000002 ) ]
+  [ bitwise reg 1 = (reg=1 & 0x0000ffff ) ^ reg 2 ]
+  [ payload write reg 1 => 2b @ network header + 0 csum_type 1 csum_off 10 csum_flags 0x0 ]
diff --git a/tests/py/ip/ip.t.payload.bridge b/tests/py/ip/ip.t.payload.bridge
index 91a4fde382e6..5a99142a9704 100644
--- a/tests/py/ip/ip.t.payload.bridge
+++ b/tests/py/ip/ip.t.payload.bridge
@@ -784,3 +784,33 @@ 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 ip dscp
+bridge
+  [ 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 << 0x00000002 ) ]
+  [ bitwise reg 1 = (reg=1 & 0x0000ffff ) ^ 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 or 0x3
+bridge
+  [ 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 & 0x000000fc ) ^ 0x00000003 ]
+  [ bitwise reg 2 = ( reg 2 << 0x00000002 ) ]
+  [ bitwise reg 1 = (reg=1 & 0x0000ffff ) ^ reg 2 ]
+  [ payload write reg 1 => 2b @ network header + 0 csum_type 1 csum_off 10 csum_flags 0x0 ]
diff --git a/tests/py/ip/ip.t.payload.inet b/tests/py/ip/ip.t.payload.inet
index b9cb28a22e7a..5440ceeb33f9 100644
--- a/tests/py/ip/ip.t.payload.inet
+++ b/tests/py/ip/ip.t.payload.inet
@@ -796,3 +796,33 @@ inet test-inet input
   [ bitwise reg 1 = (reg=1 & 0x000000ff ) ^ 0x00000100 ]
   [ payload write reg 1 => 2b @ network header + 8 csum_type 1 csum_off 10 csum_flags 0x1 ]
 
+# iif "lo" ip dscp set ip dscp
+inet
+  [ 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 << 0x00000002 ) ]
+  [ bitwise reg 1 = (reg=1 & 0x0000ffff ) ^ 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 or 0x3
+inet
+  [ 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 & 0x000000fc ) ^ 0x00000003 ]
+  [ bitwise reg 2 = ( reg 2 << 0x00000002 ) ]
+  [ bitwise reg 1 = (reg=1 & 0x0000ffff ) ^ reg 2 ]
+  [ payload write reg 1 => 2b @ network header + 0 csum_type 1 csum_off 10 csum_flags 0x0 ]
diff --git a/tests/py/ip/ip.t.payload.netdev b/tests/py/ip/ip.t.payload.netdev
index 588e5ca2a3e3..2e125158ee6e 100644
--- a/tests/py/ip/ip.t.payload.netdev
+++ b/tests/py/ip/ip.t.payload.netdev
@@ -896,3 +896,33 @@ netdev test-netdev ingress
   [ bitwise reg 1 = (reg=1 & 0x000000ff ) ^ 0x00000100 ]
   [ payload write reg 1 => 2b @ network header + 8 csum_type 1 csum_off 10 csum_flags 0x1 ]
 
+# iif "lo" ip dscp set ip dscp
+netdev
+  [ 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 << 0x00000002 ) ]
+  [ bitwise reg 1 = (reg=1 & 0x0000ffff ) ^ 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 or 0x3
+netdev
+  [ 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 & 0x000000fc ) ^ 0x00000003 ]
+  [ bitwise reg 2 = ( reg 2 << 0x00000002 ) ]
+  [ bitwise reg 1 = (reg=1 & 0x0000ffff ) ^ reg 2 ]
+  [ payload write reg 1 => 2b @ network header + 0 csum_type 1 csum_off 10 csum_flags 0x0 ]
-- 
2.25.1

Re: [PATCH nft v3 00/18] Support for boolean binops with variable RHS operands.

[Florian Westphal]

Jeremy Sowden <jeremy@azazel.net> wrote:
> Kernel support for passing mask and xor values for bitwise boolean
> operations via registers allows us to support boolean binop's with
> variable RHS operands: XOR expressions pass the xor value in a register;
> AND expressions pass the mask value in a register; OR expressions pass
> both mask and xor values in registers.
> 
> NB, OR expressions are converted to `(a & (b ^ 1)) ^ b` during
> linearization (in patch 9), because it makes both linearization and
> delinearization a lot simpler.  However, it involves rearranging and
> allocating expressions after the evaluation phase.  Since nothing else
> does this, AFAICS, I'm not sure whether it's the right thing to do.
> 
> The patch-set comprises four parts:
> 
>    1 -  7: some tidying and bug-fixes;

I've pushed these to master, thanks.

Re: [PATCH nft v3 00/18] Support for boolean binops with variable RHS operands.

[Jeremy Sowden]

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

On 2020-03-05, at 11:53:40 +0100, Florian Westphal wrote:
> Jeremy Sowden <jeremy@azazel.net> wrote:
> > Kernel support for passing mask and xor values for bitwise boolean
> > operations via registers allows us to support boolean binop's with
> > variable RHS operands: XOR expressions pass the xor value in a
> > register; AND expressions pass the mask value in a register; OR
> > expressions pass both mask and xor values in registers.
> >
> > NB, OR expressions are converted to `(a & (b ^ 1)) ^ b` during
> > linearization (in patch 9), because it makes both linearization and
> > delinearization a lot simpler.  However, it involves rearranging and
> > allocating expressions after the evaluation phase.  Since nothing
> > else does this, AFAICS, I'm not sure whether it's the right thing to
> > do.
> >
> > The patch-set comprises four parts:
> >
> >    1 -  7: some tidying and bug-fixes;
>
> I've pushed these to master, thanks.

Thanks, Florian.

J.

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

Re: [PATCH nft v3 18/18] tests: py: add variable binop RHS tests.

[Pablo Neira Ayuso]

On Tue, Mar 03, 2020 at 09:48:44AM +0000, Jeremy Sowden wrote:
[...]
> diff --git a/tests/py/any/ct.t.payload b/tests/py/any/ct.t.payload
> index 661591257804..17a1c382ea65 100644
> --- a/tests/py/any/ct.t.payload
> +++ b/tests/py/any/ct.t.payload
> @@ -359,6 +359,39 @@ 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 ]

These two are: ct mark and 0xffff0000

> +  [ meta load mark => reg 2 ]
> +  [ bitwise reg 2 = (reg=2 & 0x0000ffff ) ^ 0xffffffff ]

Refetch.

> +  [ meta load mark => reg 3 ]
> +  [ bitwise reg 3 = (reg=3 & 0x0000ffff ) ^ 0x00000000 ]

These two are: meta mark and 0xffff

> +  [ bitwise reg 1 = (reg=1 & reg 2 ) ^ reg 3 ]

This one is triggering the refetch from meta load in reg 2, right?

If so, probably extend nft_bitwise to support for 'or' from two
registers would make things more simple?

     [ bitwise reg 1 = (reg 1 | reg 3) ]

This one requires two registers as input for this new OR operation.

> +  [ ct set mark with reg 1 ]
> +
[...]
> diff --git a/tests/py/ip/ip.t.payload b/tests/py/ip/ip.t.payload
> index d627b22f2614..d6c5d14d52ac 100644
> --- a/tests/py/ip/ip.t.payload
> +++ b/tests/py/ip/ip.t.payload
[...]
> +# iif "lo" ip dscp set ip dscp or 0x3
> +ip
> +  [ 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 & 0x000000fc ) ^ 0x00000003 ]
> +  [ bitwise reg 2 = ( reg 2 << 0x00000002 ) ]
> +  [ bitwise reg 1 = (reg=1 & 0x0000ffff ) ^ reg 2 ]
> +  [ payload write reg 1 => 2b @ network header + 0 csum_type 1 csum_off 10 csum_flags 0x0 ]

Probably extending nft_bitwise again is the way to go to simplify
this?

1) fetch two bytes from payload => reg 1.
2) reg 2 = ( reg 1 | 0x000c )

   userspace 0x3 << 2 => 0x0c, then extend this to two bytes => 0x000c

This is an OR with immediate value.

3) payload write reg 1

This one requires two immediates.


Then, how does 'ip dscp set ip dscp and 0x01' bytecode looks like?

1) fetch two bytes => reg 1.
2) reg 1 = (reg 1 & 0xff07) ^ 0x0

userspace 0x01 => 0x04 (after << 2). Then, 0x04 & 0xff03 = 0xff07.

This case should be possible to support it with the existing bitwise.


The delinearization path will need to calculate the closest field
matching, but there is already code for this in the userspace tree (it
was required when matching ip dscp using bitwise operation).

Would it be possible to simplify all this through new kernel
extension? If so, I'm sorry for wasting resources, this might go to a
different direction than _MREG and _XREG.

Moreover, for field updates like in these examples, I wonder if it is
worth to introduce a new syntax, ie.

        ip dscp |= 0x01
        ip dscp or_eq 0x01

        ip dscp &= 0x01
        ip dscp and_eq 0x01

| and & might be a problem for the shell, for the native nft cli this
should be fine. But this is a different issue.

Thanks.

Re: [PATCH nft v3 18/18] tests: py: add variable binop RHS tests.

[Jeremy Sowden]

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

On 2020-03-10, at 03:39:13 +0100, Pablo Neira Ayuso wrote:
> On Tue, Mar 03, 2020 at 09:48:44AM +0000, Jeremy Sowden wrote:
> [...]
> > diff --git a/tests/py/any/ct.t.payload b/tests/py/any/ct.t.payload
> > index 661591257804..17a1c382ea65 100644
> > --- a/tests/py/any/ct.t.payload
> > +++ b/tests/py/any/ct.t.payload
> > @@ -359,6 +359,39 @@ 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 ]
>
> These two are: ct mark and 0xffff0000
>
> > +  [ meta load mark => reg 2 ]
> > +  [ bitwise reg 2 = (reg=2 & 0x0000ffff ) ^ 0xffffffff ]
>
> Refetch.
>
> > +  [ meta load mark => reg 3 ]
> > +  [ bitwise reg 3 = (reg=3 & 0x0000ffff ) ^ 0x00000000 ]
>
> These two are: meta mark and 0xffff
>
> > +  [ bitwise reg 1 = (reg=1 & reg 2 ) ^ reg 3 ]
>
> This one is triggering the refetch from meta load in reg 2, right?
>
> If so, probably extend nft_bitwise to support for 'or' from two
> registers would make things more simple?
>
>      [ bitwise reg 1 = (reg 1 | reg 3) ]
>
> This one requires two registers as input for this new OR operation.
>
> > +  [ ct set mark with reg 1 ]
> > +
> [...]
> > diff --git a/tests/py/ip/ip.t.payload b/tests/py/ip/ip.t.payload
> > index d627b22f2614..d6c5d14d52ac 100644
> > --- a/tests/py/ip/ip.t.payload
> > +++ b/tests/py/ip/ip.t.payload
> [...]
> > +# iif "lo" ip dscp set ip dscp or 0x3
> > +ip
> > +  [ 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 & 0x000000fc ) ^ 0x00000003 ]
> > +  [ bitwise reg 2 = ( reg 2 << 0x00000002 ) ]
> > +  [ bitwise reg 1 = (reg=1 & 0x0000ffff ) ^ reg 2 ]
> > +  [ payload write reg 1 => 2b @ network header + 0 csum_type 1 csum_off 10 csum_flags 0x0 ]
>
> Probably extending nft_bitwise again is the way to go to simplify
> this?
>
> 1) fetch two bytes from payload => reg 1.
> 2) reg 2 = ( reg 1 | 0x000c )
>
>    userspace 0x3 << 2 => 0x0c, then extend this to two bytes => 0x000c
>
> This is an OR with immediate value.
>
> 3) payload write reg 1
>
> This one requires two immediates.
>
> Then, how does 'ip dscp set ip dscp and 0x01' bytecode looks like?
>
> 1) fetch two bytes => reg 1.
> 2) reg 1 = (reg 1 & 0xff07) ^ 0x0
>
> userspace 0x01 => 0x04 (after << 2). Then, 0x04 & 0xff03 = 0xff07.
>
> This case should be possible to support it with the existing bitwise.
>
> The delinearization path will need to calculate the closest field
> matching, but there is already code for this in the userspace tree (it
> was required when matching ip dscp using bitwise operation).
>
> Would it be possible to simplify all this through new kernel
> extension? If so, I'm sorry for wasting resources, this might go to a
> different direction than _MREG and _XREG.

No problem. :)

> Moreover, for field updates like in these examples, I wonder if it is
> worth to introduce a new syntax, ie.
>
>         ip dscp |= 0x01
>         ip dscp or_eq 0x01
>
>         ip dscp &= 0x01
>         ip dscp and_eq 0x01
>
> | and & might be a problem for the shell, for the native nft cli this
> should be fine. But this is a different issue.

Thanks for the feedback, Pablo.

J.

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

Re: [PATCH nft v3 18/18] tests: py: add variable binop RHS tests.

[Pablo Neira Ayuso]

Hi Jeremy,

On Tue, Mar 10, 2020 at 09:30:08AM +0000, Jeremy Sowden wrote:
> On 2020-03-10, at 03:39:13 +0100, Pablo Neira Ayuso wrote:
> > On Tue, Mar 03, 2020 at 09:48:44AM +0000, Jeremy Sowden wrote:
> > [...]
> > > diff --git a/tests/py/any/ct.t.payload b/tests/py/any/ct.t.payload
> > > index 661591257804..17a1c382ea65 100644
> > > --- a/tests/py/any/ct.t.payload
> > > +++ b/tests/py/any/ct.t.payload
> > > @@ -359,6 +359,39 @@ 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 ]
> >
> > These two are: ct mark and 0xffff0000
> >
> > > +  [ meta load mark => reg 2 ]
> > > +  [ bitwise reg 2 = (reg=2 & 0x0000ffff ) ^ 0xffffffff ]
> >
> > Refetch.
> >
> > > +  [ meta load mark => reg 3 ]
> > > +  [ bitwise reg 3 = (reg=3 & 0x0000ffff ) ^ 0x00000000 ]
> >
> > These two are: meta mark and 0xffff
> >
> > > +  [ bitwise reg 1 = (reg=1 & reg 2 ) ^ reg 3 ]
> >
> > This one is triggering the refetch from meta load in reg 2, right?
> >
> > If so, probably extend nft_bitwise to support for 'or' from two
> > registers would make things more simple?
> >
> >      [ bitwise reg 1 = (reg 1 | reg 3) ]
> >
> > This one requires two registers as input for this new OR operation.
> >
> > > +  [ ct set mark with reg 1 ]
> > > +
> > [...]
> > > diff --git a/tests/py/ip/ip.t.payload b/tests/py/ip/ip.t.payload
> > > index d627b22f2614..d6c5d14d52ac 100644
> > > --- a/tests/py/ip/ip.t.payload
> > > +++ b/tests/py/ip/ip.t.payload
> > [...]
> > > +# iif "lo" ip dscp set ip dscp or 0x3
> > > +ip
> > > +  [ 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 & 0x000000fc ) ^ 0x00000003 ]
> > > +  [ bitwise reg 2 = ( reg 2 << 0x00000002 ) ]
> > > +  [ bitwise reg 1 = (reg=1 & 0x0000ffff ) ^ reg 2 ]
> > > +  [ payload write reg 1 => 2b @ network header + 0 csum_type 1 csum_off 10 csum_flags 0x0 ]
> >
> > Probably extending nft_bitwise again is the way to go to simplify
> > this?
> >
> > 1) fetch two bytes from payload => reg 1.
> > 2) reg 2 = ( reg 1 | 0x000c )
> >
> >    userspace 0x3 << 2 => 0x0c, then extend this to two bytes => 0x000c
> >
> > This is an OR with immediate value.
> >
> > 3) payload write reg 1
> >
> > This one requires two immediates.
> >
> > Then, how does 'ip dscp set ip dscp and 0x01' bytecode looks like?
> >
> > 1) fetch two bytes => reg 1.
> > 2) reg 1 = (reg 1 & 0xff07) ^ 0x0
> >
> > userspace 0x01 => 0x04 (after << 2). Then, 0x04 & 0xff03 = 0xff07.
> >
> > This case should be possible to support it with the existing bitwise.
> >
> > The delinearization path will need to calculate the closest field
> > matching, but there is already code for this in the userspace tree (it
> > was required when matching ip dscp using bitwise operation).
> >
> > Would it be possible to simplify all this through new kernel
> > extension? If so, I'm sorry for wasting resources, this might go to a
> > different direction than _MREG and _XREG.
> 
> No problem. :)
> 
> > Moreover, for field updates like in these examples, I wonder if it is
> > worth to introduce a new syntax, ie.
> >
> >         ip dscp |= 0x01
> >         ip dscp or_eq 0x01
> >
> >         ip dscp &= 0x01
> >         ip dscp and_eq 0x01
> >
> > | and & might be a problem for the shell, for the native nft cli this
> > should be fine. But this is a different issue.
> 
> Thanks for the feedback, Pablo.

Do you think it would be to keep back this one from the nf-next tree
until you evaluate an alternative way to extend nft_bitwise?

commit 8d1f378a51fcf2f5e44e06ff726a91c885d248cc
Author: Jeremy Sowden <jeremy@azazel.net>
Date:   Mon Feb 24 12:49:31 2020 +0000

    netfilter: bitwise: add support for passing mask and xor values in registers.

Thanks.

Re: [PATCH nft v3 18/18] tests: py: add variable binop RHS tests.

[Jeremy Sowden]

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

On 2020-03-11, at 14:26:13 +0100, Pablo Neira Ayuso wrote:
> Do you think it would be to keep back this one from the nf-next tree
> until you evaluate an alternative way to extend nft_bitwise?
>
> commit 8d1f378a51fcf2f5e44e06ff726a91c885d248cc
> Author: Jeremy Sowden <jeremy@azazel.net>
> Date:   Mon Feb 24 12:49:31 2020 +0000
>
>     netfilter: bitwise: add support for passing mask and xor values in registers.

If we do move away from converting all boolean op's to:

  d = (s & m) ^ x

then it seems unlikely that the new attributes will be used.

For me, it depends whether you rebase nf-next.  I'm guessing not.  In
that case, I probably wouldn't bother reverting the patch now, since
it's not big or invasive, and it wouldn't much matter if it went into
5.6 and got removed in a later patch-set.

J.

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

Re: [PATCH nft v3 18/18] tests: py: add variable binop RHS tests.

[Pablo Neira Ayuso]

On Wed, Mar 11, 2020 at 02:35:35PM +0000, Jeremy Sowden wrote:
> On 2020-03-11, at 14:26:13 +0100, Pablo Neira Ayuso wrote:
> > Do you think it would be to keep back this one from the nf-next tree
> > until you evaluate an alternative way to extend nft_bitwise?
> >
> > commit 8d1f378a51fcf2f5e44e06ff726a91c885d248cc
> > Author: Jeremy Sowden <jeremy@azazel.net>
> > Date:   Mon Feb 24 12:49:31 2020 +0000
> >
> >     netfilter: bitwise: add support for passing mask and xor values in registers.
> 
> If we do move away from converting all boolean op's to:
> 
>   d = (s & m) ^ x
> 
> then it seems unlikely that the new attributes will be used.

I see.

> For me, it depends whether you rebase nf-next.  I'm guessing not.  In
> that case, I probably wouldn't bother reverting the patch now, since
> it's not big or invasive, and it wouldn't much matter if it went into
> 5.6 and got removed in a later patch-set.

OK. I'm considering to rebase given this patch is not yet into
net-next, unless anyone here is opposed to this in order to pass a
pull-request with no add-patch-then-revert.

Regarding the new extension, we only have to be careful when updating
userspace, so only new code uses the new bitwise extension you make.
Old code will still use the old boolean approach:

    d = (s & m) ^ x

So only the payload with non-constant right-hand side will be using
your new extension for nft_bitwise.

For libnftnl, I'm inclined to revert.

Let me know, thanks.

Re: [PATCH nft v3 18/18] tests: py: add variable binop RHS tests.

[Jeremy Sowden]

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

On 2020-03-11, at 18:17:52 +0100, Pablo Neira Ayuso wrote:
> On Wed, Mar 11, 2020 at 02:35:35PM +0000, Jeremy Sowden wrote:
> > On 2020-03-11, at 14:26:13 +0100, Pablo Neira Ayuso wrote:
> > > Do you think it would be to keep back this one from the nf-next
> > > tree until you evaluate an alternative way to extend nft_bitwise?
> > >
> > > commit 8d1f378a51fcf2f5e44e06ff726a91c885d248cc
> > > Author: Jeremy Sowden <jeremy@azazel.net>
> > > Date:   Mon Feb 24 12:49:31 2020 +0000
> > >
> > >     netfilter: bitwise: add support for passing mask and xor
> > >     values in registers.
> >
> > If we do move away from converting all boolean op's to:
> >
> >   d = (s & m) ^ x
> >
> > then it seems unlikely that the new attributes will be used.
>
> I see.
>
> > For me, it depends whether you rebase nf-next.  I'm guessing not.
> > In that case, I probably wouldn't bother reverting the patch now,
> > since it's not big or invasive, and it wouldn't much matter if it
> > went into 5.6 and got removed in a later patch-set.
>
> OK. I'm considering to rebase given this patch is not yet into
> net-next, unless anyone here is opposed to this in order to pass a
> pull-request with no add-patch-then-revert.

Fbm.

> Regarding the new extension, we only have to be careful when updating
> userspace, so only new code uses the new bitwise extension you make.
> Old code will still use the old boolean approach:
>
>     d = (s & m) ^ x
>
> So only the payload with non-constant right-hand side will be using
> your new extension for nft_bitwise.

The kernel should definitely continue to support the old way of doing
things.

> For libnftnl, I'm inclined to revert.

Agreed.

> Let me know, thanks.

J.

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