[PATCH nft v3 0/9] bitwise shift support

[PATCH nft v3 0/9] bitwise shift support

[Jeremy Sowden]

The kernel supports bitwise shift operations.  This patch-set adds the
support to nft.  There are a few preliminary housekeeping patches.

Changes since v2:

  * set the type and byte-order of righthand shift operands to integer
    and host-endian during delinearization;
  * always set the length of righthand shift operands to 32 bits during
    linearization.

Changes since v1:

  * update to the final kernel and libnftnl API's;
  * update nf_tables.h in a separate patch;
  * change byte-order of payload shifts generated by expr_evaluate_bits.

Jeremy Sowden (9):
  Update gitignore.
  src: white-space fixes.
  netlink_delinearize: fix typo.
  netlink_delinearize: remove commented out pr_debug statement.
  parser: add parenthesized statement expressions.
  evaluate: change shift byte-order to host-endian.
  include: update nf_tables.h.
  netlink: add support for handling shift expressions.
  tests: shell: add bit-shift tests.

 .gitignore                                    |  9 ++
 include/linux/netfilter/nf_tables.h           | 23 +++++
 src/evaluate.c                                | 13 ++-
 src/netlink_delinearize.c                     | 93 +++++++++++++++----
 src/netlink_linearize.c                       | 52 ++++++++++-
 src/parser_bison.y                            | 25 ++---
 tests/shell/testcases/chains/0040mark_shift_0 | 11 +++
 tests/shell/testcases/chains/0040mark_shift_1 | 11 +++
 .../chains/dumps/0040mark_shift_0.nft         |  6 ++
 .../chains/dumps/0040mark_shift_1.nft         |  6 ++
 10 files changed, 208 insertions(+), 41 deletions(-)
 create mode 100755 tests/shell/testcases/chains/0040mark_shift_0
 create mode 100755 tests/shell/testcases/chains/0040mark_shift_1
 create mode 100644 tests/shell/testcases/chains/dumps/0040mark_shift_0.nft
 create mode 100644 tests/shell/testcases/chains/dumps/0040mark_shift_1.nft

-- 
2.24.1

[PATCH nft v3 1/9] Update gitignore.

[Jeremy Sowden]

Add ctags and etags tag files, and Emacs back-up files.

Signed-off-by: Jeremy Sowden <jeremy@azazel.net>
---
 .gitignore | 9 +++++++++
 1 file changed, 9 insertions(+)

diff --git a/.gitignore b/.gitignore
index 2cb1e2afd45c..6b37b1237037 100644
--- a/.gitignore
+++ b/.gitignore
@@ -19,3 +19,12 @@ libtool
 
 # Debian package build temporary files
 build-stamp
+
+# Tag files for Vim and Emacs.
+TAGS
+tags
+
+# Emacs back-up files.
+*~
+\#*\#
+.\#*
-- 
2.24.1

[PATCH nft v3 2/9] src: white-space fixes.

[Jeremy Sowden]

Remove some trailing white-space and fix some indentation.

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

diff --git a/src/evaluate.c b/src/evaluate.c
index 9d5fdaf0ef3e..5bd0858cbee1 100644
--- a/src/evaluate.c
+++ b/src/evaluate.c
@@ -2336,14 +2336,13 @@ static int stmt_evaluate_meta(struct eval_ctx *ctx, struct stmt *stmt)
 static int stmt_evaluate_ct(struct eval_ctx *ctx, struct stmt *stmt)
 {
 	if (stmt_evaluate_arg(ctx, stmt,
-				 stmt->ct.tmpl->dtype,
-				 stmt->ct.tmpl->len,
-				 stmt->ct.tmpl->byteorder,
-				 &stmt->ct.expr) < 0)
+			      stmt->ct.tmpl->dtype,
+			      stmt->ct.tmpl->len,
+			      stmt->ct.tmpl->byteorder,
+			      &stmt->ct.expr) < 0)
 		return -1;
 
-	if (stmt->ct.key == NFT_CT_SECMARK &&
-	    expr_is_constant(stmt->ct.expr))
+	if (stmt->ct.key == NFT_CT_SECMARK && expr_is_constant(stmt->ct.expr))
 		return stmt_error(ctx, stmt,
 				  "ct secmark must not be set to constant value");
 
diff --git a/src/netlink_delinearize.c b/src/netlink_delinearize.c
index 154353b8161a..387e4b046c6b 100644
--- a/src/netlink_delinearize.c
+++ b/src/netlink_delinearize.c
@@ -171,7 +171,7 @@ static void netlink_parse_immediate(struct netlink_parse_ctx *ctx,
 	struct expr *expr;
 
 	if (nftnl_expr_is_set(nle, NFTNL_EXPR_IMM_VERDICT)) {
-		nld.verdict = nftnl_expr_get_u32(nle, NFTNL_EXPR_IMM_VERDICT); 
+		nld.verdict = nftnl_expr_get_u32(nle, NFTNL_EXPR_IMM_VERDICT);
 		if  (nftnl_expr_is_set(nle, NFTNL_EXPR_IMM_CHAIN)) {
 			nld.chain = nftnl_expr_get(nle, NFTNL_EXPR_IMM_CHAIN,
 						   &nld.len);
diff --git a/src/netlink_linearize.c b/src/netlink_linearize.c
index 498326d0087a..d5e177d5e75c 100644
--- a/src/netlink_linearize.c
+++ b/src/netlink_linearize.c
@@ -1243,7 +1243,7 @@ static void netlink_gen_queue_stmt(struct netlink_linearize_ctx *ctx,
 }
 
 static void netlink_gen_ct_stmt(struct netlink_linearize_ctx *ctx,
-				  const struct stmt *stmt)
+				const struct stmt *stmt)
 {
 	struct nftnl_expr *nle;
 	enum nft_registers sreg;
-- 
2.24.1

[PATCH nft v3 3/9] netlink_delinearize: fix typo.

[Jeremy Sowden]

s/Of/If/ in comment describing function.

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 387e4b046c6b..8b9b5c808384 100644
--- a/src/netlink_delinearize.c
+++ b/src/netlink_delinearize.c
@@ -2352,7 +2352,7 @@ static void stmt_payload_binop_pp(struct rule_pp_ctx *ctx, struct expr *binop)
  * the original payload expression because it has an odd size or
  * a non-byte divisible offset/length.
  *
- * Of that was the case, the 'value' expression is not a value but
+ * If that was the case, the 'value' expression is not a value but
  * a binop expression with a munged payload expression on the left
  * and a mask to clear the real payload offset/length.
  *
-- 
2.24.1

[PATCH nft v3 4/9] netlink_delinearize: remove commented out pr_debug statement.

[Jeremy Sowden]

The statement doesn't compile, so remove it.

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

diff --git a/src/netlink_delinearize.c b/src/netlink_delinearize.c
index 8b9b5c808384..8f2a5dfacd3e 100644
--- a/src/netlink_delinearize.c
+++ b/src/netlink_delinearize.c
@@ -2047,8 +2047,6 @@ static void expr_postprocess(struct rule_pp_ctx *ctx, struct expr **exprp)
 {
 	struct expr *expr = *exprp, *i;
 
-	//pr_debug("%s len %u\n", expr->ops->name, expr->len);
-
 	switch (expr->etype) {
 	case EXPR_MAP:
 		switch (expr->map->etype) {
-- 
2.24.1

[PATCH nft v3 5/9] parser: add parenthesized statement expressions.

[Jeremy Sowden]

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

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

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

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

[Jeremy Sowden]

The byte-order of the righthand operands of the right-shifts generated
for payload and exthdr expressions is big-endian.  However, all right
shift operands should be host-endian.

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 5bd0858cbee1..52719f56abea 100644
--- a/src/evaluate.c
+++ b/src/evaluate.c
@@ -487,7 +487,7 @@ static void expr_evaluate_bits(struct eval_ctx *ctx, struct expr **exprp)
 	if (shift) {
 		off = constant_expr_alloc(&expr->location,
 					  expr_basetype(expr),
-					  BYTEORDER_BIG_ENDIAN,
+					  BYTEORDER_HOST_ENDIAN,
 					  sizeof(shift), &shift);
 
 		lshift = binop_expr_alloc(&expr->location, OP_RSHIFT, and, off);
-- 
2.24.1

[PATCH nft v3 7/9] include: update nf_tables.h.

[Jeremy Sowden]

The kernel UAPI header includes a couple of new bitwise netlink
attributes and an enum.

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

diff --git a/include/linux/netfilter/nf_tables.h b/include/linux/netfilter/nf_tables.h
index 42ed5ca39477..261864736b26 100644
--- a/include/linux/netfilter/nf_tables.h
+++ b/include/linux/netfilter/nf_tables.h
@@ -144,12 +144,14 @@ enum nft_list_attributes {
  * @NFTA_HOOK_HOOKNUM: netfilter hook number (NLA_U32)
  * @NFTA_HOOK_PRIORITY: netfilter hook priority (NLA_U32)
  * @NFTA_HOOK_DEV: netdevice name (NLA_STRING)
+ * @NFTA_HOOK_DEVS: list of netdevices (NLA_NESTED)
  */
 enum nft_hook_attributes {
 	NFTA_HOOK_UNSPEC,
 	NFTA_HOOK_HOOKNUM,
 	NFTA_HOOK_PRIORITY,
 	NFTA_HOOK_DEV,
+	NFTA_HOOK_DEVS,
 	__NFTA_HOOK_MAX
 };
 #define NFTA_HOOK_MAX		(__NFTA_HOOK_MAX - 1)
@@ -482,6 +484,20 @@ enum nft_immediate_attributes {
 };
 #define NFTA_IMMEDIATE_MAX	(__NFTA_IMMEDIATE_MAX - 1)
 
+/**
+ * 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_LSHIFT: left-shift operation
+ * @NFT_BITWISE_RSHIFT: right-shift operation
+ */
+enum nft_bitwise_ops {
+	NFT_BITWISE_BOOL,
+	NFT_BITWISE_LSHIFT,
+	NFT_BITWISE_RSHIFT,
+};
+
 /**
  * enum nft_bitwise_attributes - nf_tables bitwise expression netlink attributes
  *
@@ -490,6 +506,9 @@ enum nft_immediate_attributes {
  * @NFTA_BITWISE_LEN: length of operands (NLA_U32)
  * @NFTA_BITWISE_MASK: mask value (NLA_NESTED: nft_data_attributes)
  * @NFTA_BITWISE_XOR: xor value (NLA_NESTED: nft_data_attributes)
+ * @NFTA_BITWISE_OP: type of operation (NLA_U32: nft_bitwise_ops)
+ * @NFTA_BITWISE_DATA: argument for non-boolean operations
+ *                     (NLA_NESTED: nft_data_attributes)
  *
  * The bitwise expression supports boolean and shift operations.  It implements
  * the boolean operations by performing the following operation:
@@ -511,6 +530,8 @@ enum nft_bitwise_attributes {
 	NFTA_BITWISE_LEN,
 	NFTA_BITWISE_MASK,
 	NFTA_BITWISE_XOR,
+	NFTA_BITWISE_OP,
+	NFTA_BITWISE_DATA,
 	__NFTA_BITWISE_MAX
 };
 #define NFTA_BITWISE_MAX	(__NFTA_BITWISE_MAX - 1)
@@ -1521,6 +1542,7 @@ enum nft_object_attributes {
  * @NFTA_FLOWTABLE_HOOK: netfilter hook configuration(NLA_U32)
  * @NFTA_FLOWTABLE_USE: number of references to this flow table (NLA_U32)
  * @NFTA_FLOWTABLE_HANDLE: object handle (NLA_U64)
+ * @NFTA_FLOWTABLE_FLAGS: flags (NLA_U32)
  */
 enum nft_flowtable_attributes {
 	NFTA_FLOWTABLE_UNSPEC,
@@ -1530,6 +1552,7 @@ enum nft_flowtable_attributes {
 	NFTA_FLOWTABLE_USE,
 	NFTA_FLOWTABLE_HANDLE,
 	NFTA_FLOWTABLE_PAD,
+	NFTA_FLOWTABLE_FLAGS,
 	__NFTA_FLOWTABLE_MAX
 };
 #define NFTA_FLOWTABLE_MAX	(__NFTA_FLOWTABLE_MAX - 1)
-- 
2.24.1

[PATCH nft v3 8/9] netlink: add support for handling shift expressions.

[Jeremy Sowden]

The kernel supports bitwise shift operations, so add support to the
netlink linearization and delinearization code.  The number of bits (the
righthand operand) is expected to be a 32-bit value in host endianness.

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

diff --git a/src/netlink_delinearize.c b/src/netlink_delinearize.c
index 8f2a5dfacd3e..4dcaaba6218a 100644
--- a/src/netlink_delinearize.c
+++ b/src/netlink_delinearize.c
@@ -356,22 +356,17 @@ static void netlink_parse_lookup(struct netlink_parse_ctx *ctx,
 	ctx->stmt = expr_stmt_alloc(loc, expr);
 }
 
-static void netlink_parse_bitwise(struct netlink_parse_ctx *ctx,
-				  const struct location *loc,
-				  const struct nftnl_expr *nle)
+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)
+
 {
 	struct nft_data_delinearize nld;
-	enum nft_registers sreg, dreg;
-	struct expr *expr, *left, *mask, *xor, *or;
+	struct expr *expr, *mask, *xor, *or;
 	mpz_t m, x, o;
 
-	sreg = netlink_parse_register(nle, NFTNL_EXPR_BITWISE_SREG);
-	left = netlink_get_register(ctx, loc, sreg);
-	if (left == NULL)
-		return netlink_error(ctx, loc,
-				     "Bitwise expression has no left "
-				     "hand side");
-
 	expr = left;
 
 	nld.value = nftnl_expr_get(nle, NFTNL_EXPR_BITWISE_MASK, &nld.len);
@@ -423,6 +418,62 @@ static void netlink_parse_bitwise(struct netlink_parse_ctx *ctx,
 	mpz_clear(x);
 	mpz_clear(o);
 
+	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_registers sreg,
+						struct expr *left)
+{
+	struct nft_data_delinearize nld;
+	struct expr *expr, *right;
+
+	nld.value = nftnl_expr_get(nle, NFTNL_EXPR_BITWISE_DATA, &nld.len);
+	right = netlink_alloc_value(loc, &nld);
+
+	expr = binop_expr_alloc(loc, op, left, right);
+	expr->len = left->len;
+
+	return expr;
+}
+
+static void netlink_parse_bitwise(struct netlink_parse_ctx *ctx,
+				  const struct location *loc,
+				  const struct nftnl_expr *nle)
+{
+	enum nft_registers sreg, dreg;
+	struct expr *expr, *left;
+	enum nft_bitwise_ops op;
+
+	sreg = netlink_parse_register(nle, NFTNL_EXPR_BITWISE_SREG);
+	left = netlink_get_register(ctx, loc, sreg);
+	if (left == NULL)
+		return netlink_error(ctx, loc,
+				     "Bitwise expression has no left "
+				     "hand side");
+
+	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);
+		break;
+	case NFT_BITWISE_LSHIFT:
+		expr = netlink_parse_bitwise_shift(ctx, loc, nle, OP_LSHIFT,
+						   sreg, left);
+		break;
+	case NFT_BITWISE_RSHIFT:
+		expr = netlink_parse_bitwise_shift(ctx, loc, nle, OP_RSHIFT,
+						   sreg, left);
+		break;
+	default:
+		BUG("invalid bitwise operation %u\n", op);
+	}
+
 	dreg = netlink_parse_register(nle, NFTNL_EXPR_BITWISE_DREG);
 	netlink_set_register(ctx, dreg, expr);
 }
@@ -2091,8 +2142,16 @@ static void expr_postprocess(struct rule_pp_ctx *ctx, struct expr **exprp)
 		break;
 	case EXPR_BINOP:
 		expr_postprocess(ctx, &expr->left);
-		expr_set_type(expr->right, expr->left->dtype,
-			      expr->left->byteorder);
+		switch (expr->op) {
+		case OP_LSHIFT:
+		case OP_RSHIFT:
+			expr_set_type(expr->right, &integer_type,
+				      BYTEORDER_HOST_ENDIAN);
+			break;
+		default:
+			expr_set_type(expr->right, expr->left->dtype,
+				      expr->left->byteorder);
+		}
 		expr_postprocess(ctx, &expr->right);
 
 		expr_set_type(expr, expr->left->dtype,
diff --git a/src/netlink_linearize.c b/src/netlink_linearize.c
index d5e177d5e75c..1b9abb379577 100644
--- a/src/netlink_linearize.c
+++ b/src/netlink_linearize.c
@@ -545,9 +545,36 @@ 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_binop(struct netlink_linearize_ctx *ctx,
+static void netlink_gen_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;
+	unsigned int len = div_round_up(expr->len, BITS_PER_BYTE);
+	struct nft_data_linearize nld;
+	struct nftnl_expr *nle;
+
+	netlink_gen_expr(ctx, expr->left, dreg);
+
+	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, op);
+	nftnl_expr_set_u32(nle, NFTNL_EXPR_BITWISE_LEN, len);
+
+	netlink_gen_raw_data(expr->right->value, expr->right->byteorder,
+			     sizeof(uint32_t), &nld);
+
+	nftnl_expr_set(nle, NFTNL_EXPR_BITWISE_DATA, nld.value,
+		       nld.len);
+
+	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)
 {
 	struct nftnl_expr *nle;
 	struct nft_data_linearize nld;
@@ -562,8 +589,9 @@ static void netlink_gen_binop(struct netlink_linearize_ctx *ctx,
 	mpz_init(val);
 	mpz_init(tmp);
 
-	binops[n++] = left = (void *)expr;
-	while (left->etype == EXPR_BINOP && left->left != NULL)
+	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))
 		binops[n++] = left = left->left;
 	n--;
 
@@ -598,6 +626,7 @@ static void netlink_gen_binop(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_LEN, len);
 
 	netlink_gen_raw_data(mask, expr->byteorder, len, &nld);
@@ -613,6 +642,21 @@ static void netlink_gen_binop(struct netlink_linearize_ctx *ctx,
 	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)
+{
+	switch(expr->op) {
+	case OP_LSHIFT:
+	case OP_RSHIFT:
+		netlink_gen_shift(ctx, expr, dreg);
+		break;
+	default:
+		netlink_gen_bitwise(ctx, expr, dreg);
+		break;
+	}
+}
+
 static enum nft_byteorder_ops netlink_gen_unary_op(enum ops op)
 {
 	switch (op) {
-- 
2.24.1

[PATCH nft v3 9/9] tests: shell: add bit-shift tests.

[Jeremy Sowden]

Add a couple of tests for setting the CT mark to a bitwise expression
derived from the packet mark and vice versa.

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

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

Re: [PATCH nft v3 9/9] tests: shell: add bit-shift tests.

[Pablo Neira Ayuso]

On Sun, Jan 19, 2020 at 10:57:10PM +0000, Jeremy Sowden wrote:
> Add a couple of tests for setting the CT mark to a bitwise expression
> derived from the packet mark and vice versa.

Probably tests/py for this instead?

It also checks for the netlink bytecode, which is good to catch for
regressions in the future.

Thanks.

Re: [PATCH nft v3 9/9] tests: shell: add bit-shift tests.

[Jeremy Sowden]

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

On 2020-01-28, at 20:20:36 +0100, Pablo Neira Ayuso wrote:
> On Sun, Jan 19, 2020 at 10:57:10PM +0000, Jeremy Sowden wrote:
> > Add a couple of tests for setting the CT mark to a bitwise
> > expression derived from the packet mark and vice versa.
>
> Probably tests/py for this instead?
>
> It also checks for the netlink bytecode, which is good to catch for
> regressions in the future.

Will do.

J.

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

Re: [PATCH nft v3 0/9] bitwise shift support

[Pablo Neira Ayuso]

On Sun, Jan 19, 2020 at 10:57:01PM +0000, Jeremy Sowden wrote:
> The kernel supports bitwise shift operations.  This patch-set adds the
> support to nft.  There are a few preliminary housekeeping patches.

Actually, this batch goes in the direction of adding the basic
lshift/right support.

# nft --debug=netlink add rule x y tcp dport set tcp dport lshift 1
ip x y 
  [ 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 ) ]
  [ payload write reg 1 => 2b @ transport header + 2 csum_type 1 csum_off 16 csum_flags 0x0 ]

I'm applying patches 1, 2, 3, 4, 7 and 8.

Regarding patch 5, it would be good to restore the parens when
listing.

Patch 6, I guess it will break something else. Did you run tests/py to
check this?

Patch 9, I'm skipping until 5 and 6 are sorted out.

Thanks.

Re: [PATCH nft v3 0/9] bitwise shift support

[Jeremy Sowden]

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

On 2020-01-28, at 20:09:45 +0100, Pablo Neira Ayuso wrote:
> On Sun, Jan 19, 2020 at 10:57:01PM +0000, Jeremy Sowden wrote:
> > The kernel supports bitwise shift operations.  This patch-set adds
> > the support to nft.  There are a few preliminary housekeeping
> > patches.
>
> Actually, this batch goes in the direction of adding the basic
> lshift/right support.
>
> # nft --debug=netlink add rule x y tcp dport set tcp dport lshift 1
> ip x y
>   [ 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 ) ]
>   [ payload write reg 1 => 2b @ transport header + 2 csum_type 1
> csum_off 16 csum_flags 0x0 ]
>
> I'm applying patches 1, 2, 3, 4, 7 and 8.
>
> Regarding patch 5, it would be good to restore the parens when
> listing.

Will do.

> Patch 6, I guess it will break something else. Did you run tests/py to
> check this?

I did and I got the same results before and after applying it.  I'll
take another look.

> Patch 9, I'm skipping until 5 and 6 are sorted out.

J.

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

Re: [PATCH nft v3 0/9] bitwise shift support

[Jeremy Sowden]

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

On 2020-02-01, at 12:32:23 +0000, Jeremy Sowden wrote:
> On 2020-01-28, at 20:09:45 +0100, Pablo Neira Ayuso wrote:
> > On Sun, Jan 19, 2020 at 10:57:01PM +0000, Jeremy Sowden wrote:
> > > The kernel supports bitwise shift operations.  This patch-set adds
> > > the support to nft.  There are a few preliminary housekeeping
> > > patches.
> >
> > Actually, this batch goes in the direction of adding the basic
> > lshift/right support.
> >
> > # nft --debug=netlink add rule x y tcp dport set tcp dport lshift 1
> > ip x y
> >   [ 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 ) ]
> >   [ payload write reg 1 => 2b @ transport header + 2 csum_type 1
> > csum_off 16 csum_flags 0x0 ]
> >
> > I'm applying patches 1, 2, 3, 4, 7 and 8.
> >
> > Regarding patch 5, it would be good to restore the parens when
> > listing.
>
> Will do.

This is already handled by the same code that does it for the other
parenthesized expressions (src/expression.c, ll. 600ff.):

  static void binop_arg_print(const struct expr *op, const struct expr *arg,
                              struct output_ctx *octx)
  {
          bool prec = false;

          if (arg->etype == EXPR_BINOP &&
              expr_binop_precedence[op->op] != 0 &&
              expr_binop_precedence[op->op] < expr_binop_precedence[arg->op])
                  prec = 1;

          if (prec)
                  nft_print(octx, "(");
          expr_print(arg, octx);
          if (prec)
                  nft_print(octx, ")");
  }

> > Patch 6, I guess it will break something else. Did you run tests/py
> > to check this?
>
> I did and I got the same results before and after applying it.  I'll
> take another look.

Evaluation of the shift expression inserts a byte-order conversion if
necessary to enforce host endianness, so by changing it we just avoid
the addition of the extra operation.  I've rewritten the commit message.

> > Patch 9, I'm skipping until 5 and 6 are sorted out.

I've tweaked the shell test-cases to include a parenthesized expression,
and added some matching Python ones.

I'll send a new version out soon.

J.

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