* [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).