netfilter-devel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [nft PATCH 0/7] add payload set support for sub-byte sizes
@ 2016-07-27  0:43 Florian Westphal
  2016-07-27  0:43 ` [nft PATCH 1/7] netlink: add __binop_adjust helper Florian Westphal
                   ` (7 more replies)
  0 siblings, 8 replies; 13+ messages in thread
From: Florian Westphal @ 2016-07-27  0:43 UTC (permalink / raw)
  To: netfilter-devel

This series adds support for setting ipv6 flowlabel and e.g.
ecn/dscp header fields for ipv4 and ipv6 by adding the needed
bitwise ops (and removing them during netlink decoding).

Feedback welcome.


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

* [nft PATCH 1/7] netlink: add __binop_adjust helper
  2016-07-27  0:43 [nft PATCH 0/7] add payload set support for sub-byte sizes Florian Westphal
@ 2016-07-27  0:43 ` Florian Westphal
  2016-07-27  0:43 ` [nft PATCH 2/7] payload: print base and raw values for unknown payloads Florian Westphal
                   ` (6 subsequent siblings)
  7 siblings, 0 replies; 13+ messages in thread
From: Florian Westphal @ 2016-07-27  0:43 UTC (permalink / raw)
  To: netfilter-devel; +Cc: Florian Westphal

binop_adjust takes an expression whose LHS is expected to be
the binop expression that we use to adjust a payload expression
based on a mask (to match sub-byte headers like iphdr->version).

A followup patch has to pass the binop directly, so add
add a helper for it.

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

diff --git a/src/netlink_delinearize.c b/src/netlink_delinearize.c
index 257473a..ae87280 100644
--- a/src/netlink_delinearize.c
+++ b/src/netlink_delinearize.c
@@ -1265,10 +1265,10 @@ static void binop_adjust_one(const struct expr *binop, struct expr *value,
 	}
 }
 
-static void binop_adjust(struct expr *expr, unsigned int shift)
+static void __binop_adjust(const struct expr *binop, struct expr *right,
+			   unsigned int shift)
 {
-	const struct expr *binop = expr->left;
-	struct expr *right = expr->right, *i;
+	struct expr *i;
 
 	switch (right->ops->type) {
 	case EXPR_VALUE:
@@ -1293,11 +1293,16 @@ static void binop_adjust(struct expr *expr, unsigned int shift)
 		}
 		break;
 	default:
-		BUG("unknown expression type %s\n", expr->ops->name);
+		BUG("unknown expression type %s\n", right->ops->name);
 		break;
 	}
 }
 
+static void binop_adjust(struct expr *expr, unsigned int shift)
+{
+	__binop_adjust(expr->left, expr->right, shift);
+}
+
 static void binop_postprocess(struct rule_pp_ctx *ctx, struct expr *expr)
 {
 	struct expr *binop = expr->left;
-- 
2.7.3


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

* [nft PATCH 2/7] payload: print base and raw values for unknown payloads
  2016-07-27  0:43 [nft PATCH 0/7] add payload set support for sub-byte sizes Florian Westphal
  2016-07-27  0:43 ` [nft PATCH 1/7] netlink: add __binop_adjust helper Florian Westphal
@ 2016-07-27  0:43 ` Florian Westphal
  2016-07-27  0:43 ` [nft PATCH 3/7] evaluate: add support to set IPv6 non-byte header fields Florian Westphal
                   ` (5 subsequent siblings)
  7 siblings, 0 replies; 13+ messages in thread
From: Florian Westphal @ 2016-07-27  0:43 UTC (permalink / raw)
  To: netfilter-devel; +Cc: Florian Westphal

We currently print 'unknown' rather than the raw offset values
for unrecognized header values.

If its unknown, prefer to print

payload @nh,0,16 set payload @nh,0,16

rather than 'unknown'.

Also add a helper to check if payload expression has a description
assigned to it.

Signed-off-by: Florian Westphal <fw@strlen.de>
---
 include/payload.h |  1 +
 src/payload.c     | 14 +++++++++++++-
 2 files changed, 14 insertions(+), 1 deletion(-)

diff --git a/include/payload.h b/include/payload.h
index 37375c1..bda3188 100644
--- a/include/payload.h
+++ b/include/payload.h
@@ -31,6 +31,7 @@ struct payload_dep_ctx {
 	struct stmt		*prev;
 };
 
+extern bool payload_is_known(const struct expr *expr);
 extern bool payload_is_stacked(const struct proto_desc *desc,
 			       const struct expr *expr);
 extern void payload_dependency_store(struct payload_dep_ctx *ctx,
diff --git a/src/payload.c b/src/payload.c
index 9ba980a..af533b2 100644
--- a/src/payload.c
+++ b/src/payload.c
@@ -26,6 +26,18 @@
 #include <gmputil.h>
 #include <utils.h>
 
+bool payload_is_known(const struct expr *expr)
+{
+	const struct proto_hdr_template *tmpl;
+	const struct proto_desc *desc;
+
+	desc = expr->payload.desc;
+	tmpl = expr->payload.tmpl;
+
+	return desc && tmpl && desc != &proto_unknown &&
+	       tmpl != &proto_unknown_template;
+}
+
 static void payload_expr_print(const struct expr *expr)
 {
 	const struct proto_desc *desc;
@@ -33,7 +45,7 @@ static void payload_expr_print(const struct expr *expr)
 
 	desc = expr->payload.desc;
 	tmpl = expr->payload.tmpl;
-	if (desc != NULL && tmpl != NULL)
+	if (payload_is_known(expr))
 		printf("%s %s", desc->name, tmpl->token);
 	else
 		printf("payload @%s,%u,%u",
-- 
2.7.3


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

* [nft PATCH 3/7] evaluate: add support to set IPv6 non-byte header fields
  2016-07-27  0:43 [nft PATCH 0/7] add payload set support for sub-byte sizes Florian Westphal
  2016-07-27  0:43 ` [nft PATCH 1/7] netlink: add __binop_adjust helper Florian Westphal
  2016-07-27  0:43 ` [nft PATCH 2/7] payload: print base and raw values for unknown payloads Florian Westphal
@ 2016-07-27  0:43 ` Florian Westphal
  2016-08-01 10:29   ` Pablo Neira Ayuso
  2016-07-27  0:43 ` [nft PATCH 4/7] netlink: decode payload statment Florian Westphal
                   ` (4 subsequent siblings)
  7 siblings, 1 reply; 13+ messages in thread
From: Florian Westphal @ 2016-07-27  0:43 UTC (permalink / raw)
  To: netfilter-devel; +Cc: Florian Westphal

'ip6 ecn set 1' will generate a zero-sized write operation.
Just like when matching on bit-sized header fields we need to
round up to a byte-sized quantity and add a mask to retain those
bits outside of the header bits that we want to change.

Example:

ip6 ecn set ce
  [ payload load 1b @ network header + 1 => reg 1 ]
  [ bitwise reg 1 = (reg=1 & 0x000000cf ) ^ 0x00000030 ]
  [ payload write reg 1 => 1b @ network header + 1 csum_type 0 csum_off 0 ]

1. Load the full byte containing the ecn bits
2 .Mask out everything *BUT* the ecn bits
3 .Set the CE mark

This patch only works if the protcol doesn't need a checksum fixup.
Will address this in a followup patch.

This also doesn't yet include the needed reverse translation.

Signed-off-by: Florian Westphal <fw@strlen.de>
---
 src/evaluate.c | 81 +++++++++++++++++++++++++++++++++++++++++++++++++++++++---
 1 file changed, 77 insertions(+), 4 deletions(-)

diff --git a/src/evaluate.c b/src/evaluate.c
index 8116735..e6d4642 100644
--- a/src/evaluate.c
+++ b/src/evaluate.c
@@ -1608,13 +1608,86 @@ static int stmt_evaluate_verdict(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;
+	unsigned int masklen, extra_len = 0;
+	unsigned int payload_byte_size;
+	uint8_t shift_imm, data[16];
+	struct expr *payload;
+	mpz_t bitmask, ff;
+
 	if (__expr_evaluate_payload(ctx, stmt->payload.expr) < 0)
 		return -1;
 
-	return stmt_evaluate_arg(ctx, stmt,
-				 stmt->payload.expr->dtype,
-				 stmt->payload.expr->len,
-				 &stmt->payload.val);
+	payload = stmt->payload.expr;
+	if (stmt_evaluate_arg(ctx, stmt, payload->dtype, payload->len,
+			      &stmt->payload.val) < 0)
+		return -1;
+
+	/* Normal case: byte sized and byte aligned */
+	if (payload->payload.offset % BITS_PER_BYTE == 0 &&
+	    payload->len % BITS_PER_BYTE == 0)
+		return 0;
+
+	shift_imm = expr_offset_shift(payload, payload->payload.offset, &extra_len);
+	if (shift_imm) {
+		struct expr *off;
+
+		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;
+
+		stmt->payload.val = binop;
+	}
+
+	payload_byte_size = round_up(payload->len, BITS_PER_BYTE) / BITS_PER_BYTE;
+	payload_byte_size += (extra_len / BITS_PER_BYTE);
+	masklen = payload_byte_size * BITS_PER_BYTE;
+	mpz_init_bitmask(ff, masklen);
+
+	mpz_init2(bitmask, masklen);
+	mpz_bitmask(bitmask, payload->len);
+	mpz_lshift_ui(bitmask, shift_imm);
+
+	mpz_xor(bitmask, ff, bitmask);
+	mpz_clear(ff);
+
+	assert(sizeof(data) * BITS_PER_BYTE >= masklen);
+	mpz_export_data(data, bitmask, BYTEORDER_HOST_ENDIAN, masklen);
+	mask = constant_expr_alloc(&payload->location, expr_basetype(payload),
+				   BYTEORDER_HOST_ENDIAN, masklen, data);
+
+	payload_bytes = payload_expr_alloc(&payload->location, NULL, 0);
+	payload_init_raw(payload_bytes, payload->payload.base,
+			 (payload->payload.offset / BITS_PER_BYTE) * BITS_PER_BYTE,
+			 payload_byte_size * BITS_PER_BYTE);
+
+	payload_bytes->payload.desc	 = payload->payload.desc;
+	payload_bytes->dtype		 = &integer_type;
+	payload_bytes->byteorder	 = payload->byteorder;
+
+	payload->len = payload_bytes->len;
+	payload->payload.offset = payload_bytes->payload.offset;
+
+	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;
+
+	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;
+
+	return expr_evaluate(ctx, &stmt->payload.val);
 }
 
 static int stmt_evaluate_flow(struct eval_ctx *ctx, struct stmt *stmt)
-- 
2.7.3


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

* [nft PATCH 4/7] netlink: decode payload statment
  2016-07-27  0:43 [nft PATCH 0/7] add payload set support for sub-byte sizes Florian Westphal
                   ` (2 preceding siblings ...)
  2016-07-27  0:43 ` [nft PATCH 3/7] evaluate: add support to set IPv6 non-byte header fields Florian Westphal
@ 2016-07-27  0:43 ` Florian Westphal
  2016-08-01 10:34   ` Pablo Neira Ayuso
  2016-07-27  0:43 ` [nft PATCH 5/7] tests: ip6 dscp, flowlabel and ecn test cases Florian Westphal
                   ` (3 subsequent siblings)
  7 siblings, 1 reply; 13+ messages in thread
From: Florian Westphal @ 2016-07-27  0:43 UTC (permalink / raw)
  To: netfilter-devel; +Cc: Florian Westphal

This allows nft to display payload set operations if the
header isn't byte aligned or has non-byte divisible sizes.

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

diff --git a/src/netlink_delinearize.c b/src/netlink_delinearize.c
index ae87280..5e28111 100644
--- a/src/netlink_delinearize.c
+++ b/src/netlink_delinearize.c
@@ -1747,6 +1747,165 @@ static void stmt_expr_postprocess(struct rule_pp_ctx *ctx)
 		expr_postprocess_range(ctx, op);
 }
 
+static void stmt_payload_binop_pp(struct rule_pp_ctx *ctx, struct expr *binop)
+{
+	struct expr *payload = binop->left;
+	struct expr *mask = binop->right;
+	unsigned int shift;
+
+	assert(payload->ops->type == EXPR_PAYLOAD);
+	if (payload_expr_trim(payload, mask, &ctx->pctx, &shift)) {
+		__binop_adjust(binop, mask, shift);
+		payload_expr_complete(payload, &ctx->pctx);
+		expr_set_type(mask, payload->dtype,
+			      payload->byteorder);
+	}
+}
+
+static void stmt_payload_postprocess(struct rule_pp_ctx *ctx)
+{
+	struct stmt *stmt = ctx->stmt;
+	struct expr *expr, *binop, *payload, *value, *mask;
+	mpz_t bitmask;
+
+	expr_postprocess(ctx, &stmt->payload.expr);
+
+	expr_set_type(stmt->payload.val,
+		      stmt->payload.expr->dtype,
+		      stmt->payload.expr->byteorder);
+
+	if (payload_is_known(stmt->payload.expr))
+		goto out;
+
+	/*
+	 * expr_postprocess() failed to decode the payload information.
+	 *
+	 * We will now check if payload.val contains a binop that might
+	 * help us to figure it out.  Check if expr is one of the following:
+	 * I)
+	 *           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
+	 * to clear but retains everything unrelated to the set operation.
+	 *
+	 * IIa)
+	 *     binop (&)
+	 * payload   mask
+	 *
+	 * User specified a zero set value -- netlink bitwise decoding
+	 * discarded the redundant "| 0" part.
+	 *
+	 * IIb)
+	 *     binop (|)
+	 * payload     value/set
+	 *
+	 * This happens when user wants to set all bits, netlink bitwise
+	 * decoding changed '(payload & mask) ^ bits_to_set' into
+	 * 'payload | bits_to_set', discarding the redundant "& 0xfff...".
+	 */
+	expr = stmt->payload.val;
+	if (expr->ops->type != EXPR_BINOP)
+		goto out;
+
+	switch (expr->left->ops->type) {
+	case EXPR_BINOP: {/* I? */
+		mpz_t tmp;
+
+		if (expr->op != OP_OR)
+			goto out;
+
+		value = expr->right;
+		if (value->ops->type != EXPR_VALUE)
+			goto out;
+
+		binop = expr->left;
+		if (binop->op != OP_AND)
+			goto out;
+
+		payload = binop->left;
+		if (payload->ops->type != EXPR_PAYLOAD)
+			goto out;
+
+		if (!payload->ops->cmp(stmt->payload.expr, payload))
+			goto out;
+
+		mask = binop->right;
+		if (mask->ops->type != EXPR_VALUE)
+			goto out;
+
+		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);
+			goto out;
+		}
+
+		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->ops->type != EXPR_VALUE)
+			goto out;
+
+		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);
+			break;
+		case OP_OR: /* IIb */
+			break;
+		default: /* No idea */
+			goto out;
+		}
+
+		stmt_payload_binop_pp(ctx, expr);
+		if (!payload_is_known(expr->left))
+			goto out;
+
+		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 */
+		goto out;
+	}
+
+ out:
+	expr_postprocess(ctx, &stmt->payload.val);
+}
+
 static void rule_parse_postprocess(struct netlink_parse_ctx *ctx, struct rule *rule)
 {
 	struct rule_pp_ctx rctx;
@@ -1763,11 +1922,7 @@ static void rule_parse_postprocess(struct netlink_parse_ctx *ctx, struct rule *r
 			stmt_expr_postprocess(&rctx);
 			break;
 		case STMT_PAYLOAD:
-			expr_postprocess(&rctx, &stmt->payload.expr);
-			expr_set_type(stmt->payload.val,
-				      stmt->payload.expr->dtype,
-				      stmt->payload.expr->byteorder);
-			expr_postprocess(&rctx, &stmt->payload.val);
+			stmt_payload_postprocess(&rctx);
 			break;
 		case STMT_FLOW:
 			expr_postprocess(&rctx, &stmt->flow.key);
-- 
2.7.3


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

* [nft PATCH 5/7] tests: ip6 dscp, flowlabel and ecn test cases
  2016-07-27  0:43 [nft PATCH 0/7] add payload set support for sub-byte sizes Florian Westphal
                   ` (3 preceding siblings ...)
  2016-07-27  0:43 ` [nft PATCH 4/7] netlink: decode payload statment Florian Westphal
@ 2016-07-27  0:43 ` Florian Westphal
  2016-07-27  0:43 ` [nft PATCH 6/7] netlink: make checksum fixup work with odd-sized header fields Florian Westphal
                   ` (2 subsequent siblings)
  7 siblings, 0 replies; 13+ messages in thread
From: Florian Westphal @ 2016-07-27  0:43 UTC (permalink / raw)
  To: netfilter-devel; +Cc: Florian Westphal

Signed-off-by: Florian Westphal <fw@strlen.de>
---
 tests/py/ip6/ip6.t              | 12 +++++++
 tests/py/ip6/ip6.t.payload.inet | 71 +++++++++++++++++++++++++++++++++++++++++
 tests/py/ip6/ip6.t.payload.ip6  | 57 +++++++++++++++++++++++++++++++++
 3 files changed, 140 insertions(+)

diff --git a/tests/py/ip6/ip6.t b/tests/py/ip6/ip6.t
index 0a58fa8..7dea2f7 100644
--- a/tests/py/ip6/ip6.t
+++ b/tests/py/ip6/ip6.t
@@ -145,3 +145,15 @@ ip6 daddr != ::1234:1234:1234:1234:1234:1234:1234-1234:1234::1234:1234:1234:1234
 # limit impact to lo
 iif lo ip6 daddr set ::1;ok
 iif lo ip6 hoplimit set 1;ok
+iif lo ip6 dscp set af42;ok
+iif lo ip6 dscp set 63;ok;iif lo ip6 dscp set 0x3f
+iif lo ip6 ecn set ect0;ok
+iif lo ip6 ecn set ce;ok
+
+iif lo ip6 flowlabel set 0;ok
+iif lo ip6 flowlabel set 12345;ok
+iif lo ip6 flowlabel set 0xfffff;ok;iif lo ip6 flowlabel set 1048575
+
+iif lo ip6 ecn set 4;fail
+iif lo ip6 dscp set 64;fail
+iif lo ip6 flowlabel set 1048576;fail
diff --git a/tests/py/ip6/ip6.t.payload.inet b/tests/py/ip6/ip6.t.payload.inet
index 45bdd09..bf21b5b 100644
--- a/tests/py/ip6/ip6.t.payload.inet
+++ b/tests/py/ip6/ip6.t.payload.inet
@@ -530,3 +530,74 @@ inet test-inet input
   [ cmp eq reg 1 0x0000000a ]
   [ immediate reg 1 0x00000001 ]
   [ payload write reg 1 => 1b @ network header + 7 csum_type 0 csum_off 0 ]
+
+# iif lo ip6 dscp set af42
+inet test-inet input
+  [ meta load iif => reg 1 ]
+  [ cmp eq reg 1 0x00000001 ]
+  [ meta load nfproto => reg 1 ]
+  [ cmp eq reg 1 0x0000000a ]
+  [ payload load 2b @ network header + 0 => reg 1 ]
+  [ bitwise reg 1 = (reg=1 & 0x00003ff0 ) ^ 0x00000009 ]
+  [ payload write reg 1 => 2b @ network header + 0 csum_type 0 csum_off 0 ]
+
+# iif lo ip6 dscp set 63
+inet test-inet input
+  [ meta load iif => reg 1 ]
+  [ cmp eq reg 1 0x00000001 ]
+  [ meta load nfproto => reg 1 ]
+  [ cmp eq reg 1 0x0000000a ]
+  [ payload load 2b @ network header + 0 => reg 1 ]
+  [ bitwise reg 1 = (reg=1 & 0x00003ff0 ) ^ 0x0000c00f ]
+  [ payload write reg 1 => 2b @ network header + 0 csum_type 0 csum_off 0 ]
+
+# iif lo ip6 ecn set ect0
+inet test-inet input
+  [ meta load iif => reg 1 ]
+  [ cmp eq reg 1 0x00000001 ]
+  [ meta load nfproto => reg 1 ]
+  [ cmp eq reg 1 0x0000000a ]
+  [ payload load 1b @ network header + 1 => reg 1 ]
+  [ bitwise reg 1 = (reg=1 & 0x000000cf ) ^ 0x00000020 ]
+  [ payload write reg 1 => 1b @ network header + 1 csum_type 0 csum_off 0 ]
+
+# iif lo ip6 ecn set ce
+inet test-inet input
+  [ meta load iif => reg 1 ]
+  [ cmp eq reg 1 0x00000001 ]
+  [ meta load nfproto => reg 1 ]
+  [ cmp eq reg 1 0x0000000a ]
+  [ payload load 1b @ network header + 1 => reg 1 ]
+  [ bitwise reg 1 = (reg=1 & 0x000000cf ) ^ 0x00000030 ]
+  [ payload write reg 1 => 1b @ network header + 1 csum_type 0 csum_off 0 ]
+
+# iif lo ip6 flowlabel set 0
+inet test-inet input
+  [ meta load iif => reg 1 ]
+  [ cmp eq reg 1 0x00000001 ]
+  [ meta load nfproto => reg 1 ]
+  [ cmp eq reg 1 0x0000000a ]
+  [ payload load 3b @ network header + 1 => reg 1 ]
+  [ bitwise reg 1 = (reg=1 & 0x000000f0 ) ^ 0x00000000 ]
+  [ payload write reg 1 => 3b @ network header + 1 csum_type 0 csum_off 0 ]
+
+# iif lo ip6 flowlabel set 12345
+inet test-inet input
+  [ meta load iif => reg 1 ]
+  [ cmp eq reg 1 0x00000001 ]
+  [ meta load nfproto => reg 1 ]
+  [ cmp eq reg 1 0x0000000a ]
+  [ payload load 3b @ network header + 1 => reg 1 ]
+  [ bitwise reg 1 = (reg=1 & 0x000000f0 ) ^ 0x00393000 ]
+  [ payload write reg 1 => 3b @ network header + 1 csum_type 0 csum_off 0 ]
+
+# iif lo ip6 flowlabel set 0xfffff
+inet test-inet input
+  [ meta load iif => reg 1 ]
+  [ cmp eq reg 1 0x00000001 ]
+  [ meta load nfproto => reg 1 ]
+  [ cmp eq reg 1 0x0000000a ]
+  [ payload load 3b @ network header + 1 => reg 1 ]
+  [ bitwise reg 1 = (reg=1 & 0x000000f0 ) ^ 0x00ffff0f ]
+  [ payload write reg 1 => 3b @ network header + 1 csum_type 0 csum_off 0 ]
+
diff --git a/tests/py/ip6/ip6.t.payload.ip6 b/tests/py/ip6/ip6.t.payload.ip6
index 7e158a8..70d2ea0 100644
--- a/tests/py/ip6/ip6.t.payload.ip6
+++ b/tests/py/ip6/ip6.t.payload.ip6
@@ -392,3 +392,60 @@ ip6 test-ip6 input
   [ cmp eq reg 1 0x00000001 ]
   [ immediate reg 1 0x00000001 ]
   [ payload write reg 1 => 1b @ network header + 7 csum_type 0 csum_off 0 ]
+
+# iif lo ip6 dscp set af42
+ip6 test-ip6 input
+  [ meta load iif => reg 1 ]
+  [ cmp eq reg 1 0x00000001 ]
+  [ payload load 2b @ network header + 0 => reg 1 ]
+  [ bitwise reg 1 = (reg=1 & 0x00003ff0 ) ^ 0x00000009 ]
+  [ payload write reg 1 => 2b @ network header + 0 csum_type 0 csum_off 0 ]
+
+# iif lo ip6 dscp set 63
+ip6 test-ip6 input
+  [ meta load iif => reg 1 ]
+  [ cmp eq reg 1 0x00000001 ]
+  [ payload load 2b @ network header + 0 => reg 1 ]
+  [ bitwise reg 1 = (reg=1 & 0x00003ff0 ) ^ 0x0000c00f ]
+  [ payload write reg 1 => 2b @ network header + 0 csum_type 0 csum_off 0 ]
+
+# iif lo ip6 ecn set ect0
+ip6 test-ip6 input
+  [ meta load iif => reg 1 ]
+  [ cmp eq reg 1 0x00000001 ]
+  [ payload load 1b @ network header + 1 => reg 1 ]
+  [ bitwise reg 1 = (reg=1 & 0x000000cf ) ^ 0x00000020 ]
+  [ payload write reg 1 => 1b @ network header + 1 csum_type 0 csum_off 0 ]
+
+# iif lo ip6 ecn set ce
+ip6 test-ip6 input
+  [ meta load iif => reg 1 ]
+  [ cmp eq reg 1 0x00000001 ]
+  [ payload load 1b @ network header + 1 => reg 1 ]
+  [ bitwise reg 1 = (reg=1 & 0x000000cf ) ^ 0x00000030 ]
+  [ payload write reg 1 => 1b @ network header + 1 csum_type 0 csum_off 0 ]
+
+# iif lo ip6 flowlabel set 0
+ip6 test-ip6 input
+  [ meta load iif => reg 1 ]
+  [ cmp eq reg 1 0x00000001 ]
+  [ payload load 3b @ network header + 1 => reg 1 ]
+  [ bitwise reg 1 = (reg=1 & 0x000000f0 ) ^ 0x00000000 ]
+  [ payload write reg 1 => 3b @ network header + 1 csum_type 0 csum_off 0 ]
+
+# iif lo ip6 flowlabel set 12345
+ip6 test-ip6 input
+  [ meta load iif => reg 1 ]
+  [ cmp eq reg 1 0x00000001 ]
+  [ payload load 3b @ network header + 1 => reg 1 ]
+  [ bitwise reg 1 = (reg=1 & 0x000000f0 ) ^ 0x00393000 ]
+  [ payload write reg 1 => 3b @ network header + 1 csum_type 0 csum_off 0 ]
+
+# iif lo ip6 flowlabel set 0xfffff
+ip6 test-ip6 input
+  [ meta load iif => reg 1 ]
+  [ cmp eq reg 1 0x00000001 ]
+  [ payload load 3b @ network header + 1 => reg 1 ]
+  [ bitwise reg 1 = (reg=1 & 0x000000f0 ) ^ 0x00ffff0f ]
+  [ payload write reg 1 => 3b @ network header + 1 csum_type 0 csum_off 0 ]
+
-- 
2.7.3


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

* [nft PATCH 6/7] netlink: make checksum fixup work with odd-sized header fields
  2016-07-27  0:43 [nft PATCH 0/7] add payload set support for sub-byte sizes Florian Westphal
                   ` (4 preceding siblings ...)
  2016-07-27  0:43 ` [nft PATCH 5/7] tests: ip6 dscp, flowlabel and ecn test cases Florian Westphal
@ 2016-07-27  0:43 ` Florian Westphal
  2016-07-27  0:43 ` [nft PATCH 7/7] tests: ip payload set support for ecn and dscp Florian Westphal
  2016-08-01 10:35 ` [nft PATCH 0/7] add payload set support for sub-byte sizes Pablo Neira Ayuso
  7 siblings, 0 replies; 13+ messages in thread
From: Florian Westphal @ 2016-07-27  0:43 UTC (permalink / raw)
  To: netfilter-devel; +Cc: Florian Westphal

The kernel checksum functions want even-sized lengths except for
the last block at the end of the data.

This means that

nft --debug=netlink add rule filter output ip ecn set 1

must generate a two byte read and a two byte write:

[ payload load 2b @ network header + 0 => reg 1 ]
[ bitwise reg 1 = (reg=1 & 0x0000fcff ) ^ 0x00000100 ]
[ payload write reg 1 => 2b @ network header + 0 csum_type 1 csum_off 10 ]

Otherwise, while a one-byte write is enough, the kernel will
generate invalid checksums (unless checksum is offloaded).

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

diff --git a/src/evaluate.c b/src/evaluate.c
index e6d4642..eca46f7 100644
--- a/src/evaluate.c
+++ b/src/evaluate.c
@@ -1606,14 +1606,24 @@ static int stmt_evaluate_verdict(struct eval_ctx *ctx, struct stmt *stmt)
 	return 0;
 }
 
+static bool stmt_evaluate_payload_need_csum(const struct expr *payload)
+{
+	const struct proto_desc *desc;
+
+	desc = payload->payload.desc;
+
+	return desc && desc->checksum_key;
+}
+
 static int stmt_evaluate_payload(struct eval_ctx *ctx, struct stmt *stmt)
 {
 	struct expr *binop, *mask, *and, *payload_bytes;
 	unsigned int masklen, extra_len = 0;
-	unsigned int payload_byte_size;
+	unsigned int payload_byte_size, payload_byte_offset;
 	uint8_t shift_imm, data[16];
 	struct expr *payload;
 	mpz_t bitmask, ff;
+	bool need_csum;
 
 	if (__expr_evaluate_payload(ctx, stmt->payload.expr) < 0)
 		return -1;
@@ -1623,10 +1633,18 @@ static int stmt_evaluate_payload(struct eval_ctx *ctx, struct stmt *stmt)
 			      &stmt->payload.val) < 0)
 		return -1;
 
+	need_csum = stmt_evaluate_payload_need_csum(payload);
+
 	/* Normal case: byte sized and byte aligned */
 	if (payload->payload.offset % BITS_PER_BYTE == 0 &&
-	    payload->len % BITS_PER_BYTE == 0)
-		return 0;
+	    payload->len % BITS_PER_BYTE == 0) {
+
+		if (!need_csum || ((payload->len / BITS_PER_BYTE) & 1) == 0)
+			return 0;
+		 /* Can't deal with odd checksum fixup in kernel */
+	}
+
+	payload_byte_offset = payload->payload.offset / BITS_PER_BYTE;
 
 	shift_imm = expr_offset_shift(payload, payload->payload.offset, &extra_len);
 	if (shift_imm) {
@@ -1647,6 +1665,16 @@ static int stmt_evaluate_payload(struct eval_ctx *ctx, struct stmt *stmt)
 
 	payload_byte_size = round_up(payload->len, BITS_PER_BYTE) / BITS_PER_BYTE;
 	payload_byte_size += (extra_len / BITS_PER_BYTE);
+
+	if (need_csum && payload_byte_size & 1) {
+		payload_byte_size++;
+
+		if (payload_byte_offset & 1) { /* prefer 16bit aligned fetch */
+			payload_byte_offset--;
+			assert(payload->payload.offset >= BITS_PER_BYTE);
+		}
+	}
+
 	masklen = payload_byte_size * BITS_PER_BYTE;
 	mpz_init_bitmask(ff, masklen);
 
@@ -1664,7 +1692,7 @@ static int stmt_evaluate_payload(struct eval_ctx *ctx, struct stmt *stmt)
 
 	payload_bytes = payload_expr_alloc(&payload->location, NULL, 0);
 	payload_init_raw(payload_bytes, payload->payload.base,
-			 (payload->payload.offset / BITS_PER_BYTE) * BITS_PER_BYTE,
+			 payload_byte_offset * BITS_PER_BYTE,
 			 payload_byte_size * BITS_PER_BYTE);
 
 	payload_bytes->payload.desc	 = payload->payload.desc;
-- 
2.7.3


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

* [nft PATCH 7/7] tests: ip payload set support for ecn and dscp
  2016-07-27  0:43 [nft PATCH 0/7] add payload set support for sub-byte sizes Florian Westphal
                   ` (5 preceding siblings ...)
  2016-07-27  0:43 ` [nft PATCH 6/7] netlink: make checksum fixup work with odd-sized header fields Florian Westphal
@ 2016-07-27  0:43 ` Florian Westphal
  2016-08-01 10:35 ` [nft PATCH 0/7] add payload set support for sub-byte sizes Pablo Neira Ayuso
  7 siblings, 0 replies; 13+ messages in thread
From: Florian Westphal @ 2016-07-27  0:43 UTC (permalink / raw)
  To: netfilter-devel; +Cc: Florian Westphal

Signed-off-by: Florian Westphal <fw@strlen.de>
---
 tests/py/ip/ip.t                |  5 +++++
 tests/py/ip/ip.t.payload        | 32 ++++++++++++++++++++++++++++++
 tests/py/ip/ip.t.payload.inet   | 42 +++++++++++++++++++++++++++++++++++++++
 tests/py/ip/ip.t.payload.netdev | 44 +++++++++++++++++++++++++++++++++++++++++
 4 files changed, 123 insertions(+)

diff --git a/tests/py/ip/ip.t b/tests/py/ip/ip.t
index 90e117a..8fa53be 100644
--- a/tests/py/ip/ip.t
+++ b/tests/py/ip/ip.t
@@ -126,3 +126,8 @@ ip hdrlength 16;fail
 iif lo ip daddr set 127.0.0.1;ok
 iif lo ip checksum set 0;ok
 iif lo ip id set 0;ok
+iif lo ip ecn set 1;ok;iif lo ip ecn set ect1
+iif lo ip ecn set ce;ok
+
+iif lo ip dscp set af23;ok
+iif lo ip dscp set cs0;ok
diff --git a/tests/py/ip/ip.t.payload b/tests/py/ip/ip.t.payload
index d6ef540..75e85c1 100644
--- a/tests/py/ip/ip.t.payload
+++ b/tests/py/ip/ip.t.payload
@@ -437,3 +437,35 @@ ip test-ip4 input
   [ immediate reg 1 0x00000000 ]
   [ payload write reg 1 => 2b @ network header + 4 csum_type 1 csum_off 10 ]
 
+# iif lo ip ecn set 1
+ip test-ip4 input
+  [ meta load iif => reg 1 ]
+  [ cmp eq reg 1 0x00000001 ]
+  [ payload load 2b @ network header + 0 => reg 1 ]
+  [ bitwise reg 1 = (reg=1 & 0x0000fcff ) ^ 0x00000100 ]
+  [ payload write reg 1 => 2b @ network header + 0 csum_type 1 csum_off 10 ]
+
+# iif lo ip ecn set ce
+ip test-ip4 input
+  [ meta load iif => reg 1 ]
+  [ cmp eq reg 1 0x00000001 ]
+  [ payload load 2b @ network header + 0 => reg 1 ]
+  [ bitwise reg 1 = (reg=1 & 0x0000fcff ) ^ 0x00000300 ]
+  [ payload write reg 1 => 2b @ network header + 0 csum_type 1 csum_off 10 ]
+
+# iif lo ip dscp set af23
+ip test-ip4 input
+  [ meta load iif => reg 1 ]
+  [ cmp eq reg 1 0x00000001 ]
+  [ payload load 2b @ network header + 0 => reg 1 ]
+  [ bitwise reg 1 = (reg=1 & 0x000003ff ) ^ 0x00005800 ]
+  [ payload write reg 1 => 2b @ network header + 0 csum_type 1 csum_off 10 ]
+
+# iif lo ip dscp set cs0
+ip test-ip4 input
+  [ meta load iif => reg 1 ]
+  [ cmp eq reg 1 0x00000001 ]
+  [ payload load 2b @ network header + 0 => reg 1 ]
+  [ bitwise reg 1 = (reg=1 & 0x000003ff ) ^ 0x00000000 ]
+  [ payload write reg 1 => 2b @ network header + 0 csum_type 1 csum_off 10 ]
+
diff --git a/tests/py/ip/ip.t.payload.inet b/tests/py/ip/ip.t.payload.inet
index 6c78536..5cdf2a6 100644
--- a/tests/py/ip/ip.t.payload.inet
+++ b/tests/py/ip/ip.t.payload.inet
@@ -573,3 +573,45 @@ inet test-inet input
   [ meta load nfproto => reg 1 ]
   [ cmp eq reg 1 0x00000002 ]
   [ immediate reg 1 0x00000000 ]
+  [ payload write reg 1 => 2b @ network header + 4 csum_type 1 csum_off 10 ]
+
+# iif lo ip ecn set 1
+inet test-inet input
+  [ meta load iif => reg 1 ]
+  [ cmp eq reg 1 0x00000001 ]
+  [ meta load nfproto => reg 1 ]
+  [ cmp eq reg 1 0x00000002 ]
+  [ payload load 2b @ network header + 0 => reg 1 ]
+  [ bitwise reg 1 = (reg=1 & 0x0000fcff ) ^ 0x00000100 ]
+  [ payload write reg 1 => 2b @ network header + 0 csum_type 1 csum_off 10 ]
+
+# iif lo ip ecn set ce
+inet test-netdev ingress
+  [ 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 & 0x0000fcff ) ^ 0x00000300 ]
+  [ payload write reg 1 => 2b @ network header + 0 csum_type 1 csum_off 10 ]
+
+# iif lo ip dscp set af23
+inet test-inet input
+  [ meta load iif => reg 1 ]
+  [ cmp eq reg 1 0x00000001 ]
+  [ meta load nfproto => reg 1 ]
+  [ cmp eq reg 1 0x00000002 ]
+  [ payload load 2b @ network header + 0 => reg 1 ]
+  [ bitwise reg 1 = (reg=1 & 0x000003ff ) ^ 0x00005800 ]
+  [ payload write reg 1 => 2b @ network header + 0 csum_type 1 csum_off 10 ]
+
+# iif lo ip dscp set cs0
+inet test-inet input
+  [ meta load iif => reg 1 ]
+  [ cmp eq reg 1 0x00000001 ]
+  [ meta load nfproto => reg 1 ]
+  [ cmp eq reg 1 0x00000002 ]
+  [ payload load 2b @ network header + 0 => reg 1 ]
+  [ bitwise reg 1 = (reg=1 & 0x000003ff ) ^ 0x00000000 ]
+  [ payload write reg 1 => 2b @ network header + 0 csum_type 1 csum_off 10 ]
+
diff --git a/tests/py/ip/ip.t.payload.netdev b/tests/py/ip/ip.t.payload.netdev
index 714b0d2..134fb73 100644
--- a/tests/py/ip/ip.t.payload.netdev
+++ b/tests/py/ip/ip.t.payload.netdev
@@ -671,3 +671,47 @@ netdev test-netdev ingress
   [ meta load iif => reg 1 ]
   [ cmp eq reg 1 0x00000001 ]
   [ meta load protocol => reg 1 ]
+  [ cmp eq reg 1 0x00000008 ]
+  [ immediate reg 1 0x00000000 ]
+  [ payload write reg 1 => 2b @ network header + 4 csum_type 1 csum_off 10 ]
+
+# iif lo ip ecn set 1
+netdev test-netdev ingress
+  [ meta load iif => reg 1 ]
+  [ cmp eq reg 1 0x00000001 ]
+  [ meta load protocol => reg 1 ]
+  [ cmp eq reg 1 0x00000008 ]
+  [ payload load 2b @ network header + 0 => reg 1 ]
+  [ bitwise reg 1 = (reg=1 & 0x0000fcff ) ^ 0x00000100 ]
+  [ payload write reg 1 => 2b @ network header + 0 csum_type 1 csum_off 10 ]
+
+# iif lo ip ecn set ce
+netdev test-netdev ingress
+  [ meta load iif => reg 1 ]
+  [ cmp eq reg 1 0x00000001 ]
+  [ meta load protocol => reg 1 ]
+  [ cmp eq reg 1 0x00000008 ]
+  [ payload load 2b @ network header + 0 => reg 1 ]
+  [ bitwise reg 1 = (reg=1 & 0x0000fcff ) ^ 0x00000300 ]
+  [ payload write reg 1 => 2b @ network header + 0 csum_type 1 csum_off 10 ]
+
+# iif lo ip dscp set af23
+netdev test-netdev ingress
+  [ meta load iif => reg 1 ]
+  [ cmp eq reg 1 0x00000001 ]
+  [ meta load protocol => reg 1 ]
+  [ cmp eq reg 1 0x00000008 ]
+  [ payload load 2b @ network header + 0 => reg 1 ]
+  [ bitwise reg 1 = (reg=1 & 0x000003ff ) ^ 0x00005800 ]
+  [ payload write reg 1 => 2b @ network header + 0 csum_type 1 csum_off 10 ]
+
+# iif lo ip dscp set cs0
+netdev test-netdev ingress
+  [ meta load iif => reg 1 ]
+  [ cmp eq reg 1 0x00000001 ]
+  [ meta load protocol => reg 1 ]
+  [ cmp eq reg 1 0x00000008 ]
+  [ payload load 2b @ network header + 0 => reg 1 ]
+  [ bitwise reg 1 = (reg=1 & 0x000003ff ) ^ 0x00000000 ]
+  [ payload write reg 1 => 2b @ network header + 0 csum_type 1 csum_off 10 ]
+
-- 
2.7.3


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

* Re: [nft PATCH 3/7] evaluate: add support to set IPv6 non-byte header fields
  2016-07-27  0:43 ` [nft PATCH 3/7] evaluate: add support to set IPv6 non-byte header fields Florian Westphal
@ 2016-08-01 10:29   ` Pablo Neira Ayuso
  2016-08-01 14:23     ` Florian Westphal
  0 siblings, 1 reply; 13+ messages in thread
From: Pablo Neira Ayuso @ 2016-08-01 10:29 UTC (permalink / raw)
  To: Florian Westphal; +Cc: netfilter-devel

On Wed, Jul 27, 2016 at 02:43:12AM +0200, Florian Westphal wrote:
> 'ip6 ecn set 1' will generate a zero-sized write operation.
> Just like when matching on bit-sized header fields we need to
> round up to a byte-sized quantity and add a mask to retain those
> bits outside of the header bits that we want to change.
>
> Example:
> 
> ip6 ecn set ce
>   [ payload load 1b @ network header + 1 => reg 1 ]
>   [ bitwise reg 1 = (reg=1 & 0x000000cf ) ^ 0x00000030 ]
>   [ payload write reg 1 => 1b @ network header + 1 csum_type 0 csum_off 0 ]
> 
> 1. Load the full byte containing the ecn bits
> 2 .Mask out everything *BUT* the ecn bits
> 3 .Set the CE mark
> 
> This patch only works if the protcol doesn't need a checksum fixup.
> Will address this in a followup patch.
> 
> This also doesn't yet include the needed reverse translation.
> 
> Signed-off-by: Florian Westphal <fw@strlen.de>
> ---
>  src/evaluate.c | 81 +++++++++++++++++++++++++++++++++++++++++++++++++++++++---
>  1 file changed, 77 insertions(+), 4 deletions(-)
> 
> diff --git a/src/evaluate.c b/src/evaluate.c
> index 8116735..e6d4642 100644
> --- a/src/evaluate.c
> +++ b/src/evaluate.c
> @@ -1608,13 +1608,86 @@ static int stmt_evaluate_verdict(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;
> +	unsigned int masklen, extra_len = 0;
> +	unsigned int payload_byte_size;
> +	uint8_t shift_imm, data[16];

Instead of hardcoding to our current maximum word size, I think you
can extract this information from the ctx.

> +	struct expr *payload;
> +	mpz_t bitmask, ff;
> +
>  	if (__expr_evaluate_payload(ctx, stmt->payload.expr) < 0)
>  		return -1;
>  
> -	return stmt_evaluate_arg(ctx, stmt,
> -				 stmt->payload.expr->dtype,
> -				 stmt->payload.expr->len,
> -				 &stmt->payload.val);
> +	payload = stmt->payload.expr;
> +	if (stmt_evaluate_arg(ctx, stmt, payload->dtype, payload->len,
> +			      &stmt->payload.val) < 0)
> +		return -1;
> +
> +	/* Normal case: byte sized and byte aligned */
> +	if (payload->payload.offset % BITS_PER_BYTE == 0 &&
> +	    payload->len % BITS_PER_BYTE == 0)
> +		return 0;

This idiom is already used in expr_evaluate_payload(), so you can
probably wrap this code in a function, eg. payload_needs_adjustment()
in a follow up patch and we skip this comment on top of this function.

> +
> +	shift_imm = expr_offset_shift(payload, payload->payload.offset, &extra_len);
> +	if (shift_imm) {
> +		struct expr *off;
> +
> +		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;

This indent doesn't look correct.

I'd suggest you amend this before pushing this out.

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

* Re: [nft PATCH 4/7] netlink: decode payload statment
  2016-07-27  0:43 ` [nft PATCH 4/7] netlink: decode payload statment Florian Westphal
@ 2016-08-01 10:34   ` Pablo Neira Ayuso
  0 siblings, 0 replies; 13+ messages in thread
From: Pablo Neira Ayuso @ 2016-08-01 10:34 UTC (permalink / raw)
  To: Florian Westphal; +Cc: netfilter-devel

On Wed, Jul 27, 2016 at 02:43:13AM +0200, Florian Westphal wrote:
> This allows nft to display payload set operations if the
> header isn't byte aligned or has non-byte divisible sizes.
> 
> Signed-off-by: Florian Westphal <fw@strlen.de>
> ---
>  src/netlink_delinearize.c | 165 ++++++++++++++++++++++++++++++++++++++++++++--
>  1 file changed, 160 insertions(+), 5 deletions(-)
> 
> diff --git a/src/netlink_delinearize.c b/src/netlink_delinearize.c
> index ae87280..5e28111 100644
> --- a/src/netlink_delinearize.c
> +++ b/src/netlink_delinearize.c
> @@ -1747,6 +1747,165 @@ static void stmt_expr_postprocess(struct rule_pp_ctx *ctx)
>  		expr_postprocess_range(ctx, op);
>  }
>  
> +static void stmt_payload_binop_pp(struct rule_pp_ctx *ctx, struct expr *binop)
> +{
> +	struct expr *payload = binop->left;
> +	struct expr *mask = binop->right;
> +	unsigned int shift;
> +
> +	assert(payload->ops->type == EXPR_PAYLOAD);
> +	if (payload_expr_trim(payload, mask, &ctx->pctx, &shift)) {
> +		__binop_adjust(binop, mask, shift);
> +		payload_expr_complete(payload, &ctx->pctx);
> +		expr_set_type(mask, payload->dtype,
> +			      payload->byteorder);
> +	}
> +}
> +
> +static void stmt_payload_postprocess(struct rule_pp_ctx *ctx)
> +{
> +	struct stmt *stmt = ctx->stmt;
> +	struct expr *expr, *binop, *payload, *value, *mask;
> +	mpz_t bitmask;
> +
> +	expr_postprocess(ctx, &stmt->payload.expr);
> +
> +	expr_set_type(stmt->payload.val,
> +		      stmt->payload.expr->dtype,
> +		      stmt->payload.expr->byteorder);
> +
> +	if (payload_is_known(stmt->payload.expr))
> +		goto out;

Could you wrap the whole code below into a function, something like:

        stmt_payload_binop_postprocess(...)

(feel free to select a better name that looks consistent to what we
have around), so this looks like:

static void stmt_payload_postprocess(struct rule_pp_ctx *ctx)
{
        [...]

	expr_postprocess(ctx, &stmt->payload.expr);

	expr_set_type(stmt->payload.val,
		      stmt->payload.expr->dtype,
		      stmt->payload.expr->byteorder);

	if (!payload_is_known(stmt->payload.expr))
                stmt_payload_binop_postprocess(...);

        expr_postprocess(...);
}

So we reduce the use of gotos a bit.

> +	/*
> +	 * expr_postprocess() failed to decode the payload information.

Thus, this comment becomes a description for this new
stmt_payload_binop_postprocess() function.

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

* Re: [nft PATCH 0/7] add payload set support for sub-byte sizes
  2016-07-27  0:43 [nft PATCH 0/7] add payload set support for sub-byte sizes Florian Westphal
                   ` (6 preceding siblings ...)
  2016-07-27  0:43 ` [nft PATCH 7/7] tests: ip payload set support for ecn and dscp Florian Westphal
@ 2016-08-01 10:35 ` Pablo Neira Ayuso
  2016-08-01 15:12   ` Florian Westphal
  7 siblings, 1 reply; 13+ messages in thread
From: Pablo Neira Ayuso @ 2016-08-01 10:35 UTC (permalink / raw)
  To: Florian Westphal; +Cc: netfilter-devel

On Wed, Jul 27, 2016 at 02:43:09AM +0200, Florian Westphal wrote:
> This series adds support for setting ipv6 flowlabel and e.g.
> ecn/dscp header fields for ipv4 and ipv6 by adding the needed
> bitwise ops (and removing them during netlink decoding).

I'm hitting whitespace errors here:

        warning: 2 lines add whitespace errors

Please, amend these and other nitpicks and push these series, we can
later on refine them if needed.

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

Thanks.

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

* Re: [nft PATCH 3/7] evaluate: add support to set IPv6 non-byte header fields
  2016-08-01 10:29   ` Pablo Neira Ayuso
@ 2016-08-01 14:23     ` Florian Westphal
  0 siblings, 0 replies; 13+ messages in thread
From: Florian Westphal @ 2016-08-01 14:23 UTC (permalink / raw)
  To: Pablo Neira Ayuso; +Cc: Florian Westphal, netfilter-devel

Pablo Neira Ayuso <pablo@netfilter.org> wrote:
> On Wed, Jul 27, 2016 at 02:43:12AM +0200, Florian Westphal wrote:
> > 'ip6 ecn set 1' will generate a zero-sized write operation.
> > Just like when matching on bit-sized header fields we need to
> > round up to a byte-sized quantity and add a mask to retain those
> > bits outside of the header bits that we want to change.
> >
> > Example:
> > 
> > ip6 ecn set ce
> >   [ payload load 1b @ network header + 1 => reg 1 ]
> >   [ bitwise reg 1 = (reg=1 & 0x000000cf ) ^ 0x00000030 ]
> >   [ payload write reg 1 => 1b @ network header + 1 csum_type 0 csum_off 0 ]
> > 
> > 1. Load the full byte containing the ecn bits
> > 2 .Mask out everything *BUT* the ecn bits
> > 3 .Set the CE mark
> > 
> > This patch only works if the protcol doesn't need a checksum fixup.
> > Will address this in a followup patch.
> > 
> > This also doesn't yet include the needed reverse translation.
> > 
> > Signed-off-by: Florian Westphal <fw@strlen.de>
> > ---
> >  src/evaluate.c | 81 +++++++++++++++++++++++++++++++++++++++++++++++++++++++---
> >  1 file changed, 77 insertions(+), 4 deletions(-)
> > 
> > diff --git a/src/evaluate.c b/src/evaluate.c
> > index 8116735..e6d4642 100644
> > --- a/src/evaluate.c
> > +++ b/src/evaluate.c
> > @@ -1608,13 +1608,86 @@ static int stmt_evaluate_verdict(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;
> > +	unsigned int masklen, extra_len = 0;
> > +	unsigned int payload_byte_size;
> > +	uint8_t shift_imm, data[16];
> 
> Instead of hardcoding to our current maximum word size, I think you
> can extract this information from the ctx.

I changed it to data[NFT_REG_SIZE].

> This idiom is already used in expr_evaluate_payload(), so you can
> probably wrap this code in a function, eg. payload_needs_adjustment()
> in a follow up patch and we skip this comment on top of this function.

Okay, can do this.

> > +
> > +	shift_imm = expr_offset_shift(payload, payload->payload.offset, &extra_len);
> > +	if (shift_imm) {
> > +		struct expr *off;
> > +
> > +		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;
> 
> This indent doesn't look correct.
> 
> I'd suggest you amend this before pushing this out.

Will do, thanks!

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

* Re: [nft PATCH 0/7] add payload set support for sub-byte sizes
  2016-08-01 10:35 ` [nft PATCH 0/7] add payload set support for sub-byte sizes Pablo Neira Ayuso
@ 2016-08-01 15:12   ` Florian Westphal
  0 siblings, 0 replies; 13+ messages in thread
From: Florian Westphal @ 2016-08-01 15:12 UTC (permalink / raw)
  To: Pablo Neira Ayuso; +Cc: Florian Westphal, netfilter-devel

Pablo Neira Ayuso <pablo@netfilter.org> wrote:
> On Wed, Jul 27, 2016 at 02:43:09AM +0200, Florian Westphal wrote:
> > This series adds support for setting ipv6 flowlabel and e.g.
> > ecn/dscp header fields for ipv4 and ipv6 by adding the needed
> > bitwise ops (and removing them during netlink decoding).
> 
> I'm hitting whitespace errors here:
> 
>         warning: 2 lines add whitespace errors
> 
> Please, amend these and other nitpicks and push these series, we can
> later on refine them if needed.
> 
> Acked-by: Pablo Neira Ayuso <pablo@netfilter.org>

I pushed this to master, thanks for reviewing.

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

end of thread, other threads:[~2016-08-01 15:12 UTC | newest]

Thread overview: 13+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-07-27  0:43 [nft PATCH 0/7] add payload set support for sub-byte sizes Florian Westphal
2016-07-27  0:43 ` [nft PATCH 1/7] netlink: add __binop_adjust helper Florian Westphal
2016-07-27  0:43 ` [nft PATCH 2/7] payload: print base and raw values for unknown payloads Florian Westphal
2016-07-27  0:43 ` [nft PATCH 3/7] evaluate: add support to set IPv6 non-byte header fields Florian Westphal
2016-08-01 10:29   ` Pablo Neira Ayuso
2016-08-01 14:23     ` Florian Westphal
2016-07-27  0:43 ` [nft PATCH 4/7] netlink: decode payload statment Florian Westphal
2016-08-01 10:34   ` Pablo Neira Ayuso
2016-07-27  0:43 ` [nft PATCH 5/7] tests: ip6 dscp, flowlabel and ecn test cases Florian Westphal
2016-07-27  0:43 ` [nft PATCH 6/7] netlink: make checksum fixup work with odd-sized header fields Florian Westphal
2016-07-27  0:43 ` [nft PATCH 7/7] tests: ip payload set support for ecn and dscp Florian Westphal
2016-08-01 10:35 ` [nft PATCH 0/7] add payload set support for sub-byte sizes Pablo Neira Ayuso
2016-08-01 15:12   ` Florian Westphal

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).