2ee3b20fcf
* Tue May 30 2023 Phil Sutter <psutter@redhat.com> [1.0.4-1.el8] - Synchronize patch level with nftables-1.0.4-10.el9 (Phil Sutter) [2211076] - Rebase onto version 1.0.4 (Phil Sutter) [2211076] Resolves: rhbz#2211076
201 lines
6.9 KiB
Diff
201 lines
6.9 KiB
Diff
From baea5b0f3199d21a8089ab792aee86621f67202c Mon Sep 17 00:00:00 2001
|
|
From: Phil Sutter <psutter@redhat.com>
|
|
Date: Thu, 9 Feb 2023 12:45:30 +0100
|
|
Subject: [PATCH] evaluate: set eval ctx for add/update statements with integer
|
|
constants
|
|
|
|
Bugzilla: https://bugzilla.redhat.com/show_bug.cgi?id=2211076
|
|
Upstream Status: nftables commit 4cc6b20d31498
|
|
|
|
commit 4cc6b20d31498d90e90ff574ce8b70276afcee8f
|
|
Author: Florian Westphal <fw@strlen.de>
|
|
Date: Mon Jan 23 19:03:28 2023 +0100
|
|
|
|
evaluate: set eval ctx for add/update statements with integer constants
|
|
|
|
Eric reports that nft asserts when using integer basetype constants with
|
|
'typeof' sets. Example:
|
|
table netdev t {
|
|
set s {
|
|
typeof ether saddr . vlan id
|
|
flags dynamic,timeout
|
|
}
|
|
|
|
chain c { }
|
|
}
|
|
|
|
loads fine. But adding a rule with add/update statement fails:
|
|
nft 'add rule netdev t c set update ether saddr . 0 @s'
|
|
nft: netlink_linearize.c:867: netlink_gen_expr: Assertion `dreg < ctx->reg_low' failed.
|
|
|
|
When the 'ether saddr . 0' concat expression is processed, there is
|
|
no set definition available anymore to deduce the required size of the
|
|
integer constant.
|
|
|
|
nft eval step then derives the required length using the data types.
|
|
'0' has integer basetype, so the deduced length is 0.
|
|
|
|
The assertion triggers because serialization step finds that it
|
|
needs one more register.
|
|
|
|
2 are needed to store the ethernet address, another register is
|
|
needed for the vlan id.
|
|
|
|
Update eval step to make the expression context store the set key
|
|
information when processing the preceeding set reference, then
|
|
let stmt_evaluate_set() preserve the existing context instead of
|
|
zeroing it again via stmt_evaluate_arg().
|
|
|
|
This makes concat expression evaluation compute the total size
|
|
needed based on the sets key definition.
|
|
|
|
Reported-by: Eric Garver <eric@garver.life>
|
|
Signed-off-by: Florian Westphal <fw@strlen.de>
|
|
|
|
Signed-off-by: Phil Sutter <psutter@redhat.com>
|
|
---
|
|
src/evaluate.c | 32 +++++++++++++++++--
|
|
.../maps/dumps/typeof_maps_concat.nft | 11 +++++++
|
|
tests/shell/testcases/maps/typeof_maps_concat | 6 ++++
|
|
.../sets/dumps/typeof_sets_concat.nft | 12 +++++++
|
|
tests/shell/testcases/sets/typeof_sets_concat | 6 ++++
|
|
5 files changed, 65 insertions(+), 2 deletions(-)
|
|
create mode 100644 tests/shell/testcases/maps/dumps/typeof_maps_concat.nft
|
|
create mode 100755 tests/shell/testcases/maps/typeof_maps_concat
|
|
create mode 100644 tests/shell/testcases/sets/dumps/typeof_sets_concat.nft
|
|
create mode 100755 tests/shell/testcases/sets/typeof_sets_concat
|
|
|
|
diff --git a/src/evaluate.c b/src/evaluate.c
|
|
index d67f915..7f81411 100644
|
|
--- a/src/evaluate.c
|
|
+++ b/src/evaluate.c
|
|
@@ -1526,6 +1526,14 @@ static int interval_set_eval(struct eval_ctx *ctx, struct set *set,
|
|
return ret;
|
|
}
|
|
|
|
+static void expr_evaluate_set_ref(struct eval_ctx *ctx, struct expr *expr)
|
|
+{
|
|
+ struct set *set = expr->set;
|
|
+
|
|
+ expr_set_context(&ctx->ectx, set->key->dtype, set->key->len);
|
|
+ ctx->ectx.key = set->key;
|
|
+}
|
|
+
|
|
static int expr_evaluate_set(struct eval_ctx *ctx, struct expr **expr)
|
|
{
|
|
struct expr *set = *expr, *i, *next;
|
|
@@ -2388,6 +2396,7 @@ static int expr_evaluate(struct eval_ctx *ctx, struct expr **expr)
|
|
case EXPR_VARIABLE:
|
|
return expr_evaluate_variable(ctx, expr);
|
|
case EXPR_SET_REF:
|
|
+ expr_evaluate_set_ref(ctx, *expr);
|
|
return 0;
|
|
case EXPR_VALUE:
|
|
return expr_evaluate_value(ctx, expr);
|
|
@@ -2550,6 +2559,25 @@ static int stmt_evaluate_arg(struct eval_ctx *ctx, struct stmt *stmt,
|
|
return __stmt_evaluate_arg(ctx, stmt, dtype, len, byteorder, expr);
|
|
}
|
|
|
|
+/* like stmt_evaluate_arg, but keep existing context created
|
|
+ * by previous expr_evaluate().
|
|
+ *
|
|
+ * This is needed for add/update statements:
|
|
+ * ctx->ectx.key has the set key, which may be needed for 'typeof'
|
|
+ * sets: the 'add/update' expression might contain integer data types.
|
|
+ *
|
|
+ * Without the key we cannot derive the element size.
|
|
+ */
|
|
+static int stmt_evaluate_key(struct eval_ctx *ctx, struct stmt *stmt,
|
|
+ const struct datatype *dtype, unsigned int len,
|
|
+ enum byteorder byteorder, struct expr **expr)
|
|
+{
|
|
+ if (expr_evaluate(ctx, expr) < 0)
|
|
+ return -1;
|
|
+
|
|
+ return __stmt_evaluate_arg(ctx, stmt, dtype, len, byteorder, expr);
|
|
+}
|
|
+
|
|
static int stmt_evaluate_verdict(struct eval_ctx *ctx, struct stmt *stmt)
|
|
{
|
|
if (stmt_evaluate_arg(ctx, stmt, &verdict_type, 0, 0, &stmt->expr) < 0)
|
|
@@ -3762,7 +3790,7 @@ static int stmt_evaluate_set(struct eval_ctx *ctx, struct stmt *stmt)
|
|
return expr_error(ctx->msgs, stmt->set.set,
|
|
"Expression does not refer to a set");
|
|
|
|
- if (stmt_evaluate_arg(ctx, stmt,
|
|
+ if (stmt_evaluate_key(ctx, stmt,
|
|
stmt->set.set->set->key->dtype,
|
|
stmt->set.set->set->key->len,
|
|
stmt->set.set->set->key->byteorder,
|
|
@@ -3805,7 +3833,7 @@ static int stmt_evaluate_map(struct eval_ctx *ctx, struct stmt *stmt)
|
|
return expr_error(ctx->msgs, stmt->map.set,
|
|
"Expression does not refer to a set");
|
|
|
|
- if (stmt_evaluate_arg(ctx, stmt,
|
|
+ if (stmt_evaluate_key(ctx, stmt,
|
|
stmt->map.set->set->key->dtype,
|
|
stmt->map.set->set->key->len,
|
|
stmt->map.set->set->key->byteorder,
|
|
diff --git a/tests/shell/testcases/maps/dumps/typeof_maps_concat.nft b/tests/shell/testcases/maps/dumps/typeof_maps_concat.nft
|
|
new file mode 100644
|
|
index 0000000..1ca98d8
|
|
--- /dev/null
|
|
+++ b/tests/shell/testcases/maps/dumps/typeof_maps_concat.nft
|
|
@@ -0,0 +1,11 @@
|
|
+table netdev t {
|
|
+ map m {
|
|
+ typeof ether saddr . vlan id : meta mark
|
|
+ size 1234
|
|
+ flags dynamic,timeout
|
|
+ }
|
|
+
|
|
+ chain c {
|
|
+ ether type != 8021q update @m { ether daddr . 123 timeout 1m : 0x0000002a } counter packets 0 bytes 0 return
|
|
+ }
|
|
+}
|
|
diff --git a/tests/shell/testcases/maps/typeof_maps_concat b/tests/shell/testcases/maps/typeof_maps_concat
|
|
new file mode 100755
|
|
index 0000000..07820b7
|
|
--- /dev/null
|
|
+++ b/tests/shell/testcases/maps/typeof_maps_concat
|
|
@@ -0,0 +1,6 @@
|
|
+#!/bin/bash
|
|
+
|
|
+set -e
|
|
+dumpfile=$(dirname $0)/dumps/$(basename $0).nft
|
|
+
|
|
+$NFT -f "$dumpfile"
|
|
diff --git a/tests/shell/testcases/sets/dumps/typeof_sets_concat.nft b/tests/shell/testcases/sets/dumps/typeof_sets_concat.nft
|
|
new file mode 100644
|
|
index 0000000..dbaf7cd
|
|
--- /dev/null
|
|
+++ b/tests/shell/testcases/sets/dumps/typeof_sets_concat.nft
|
|
@@ -0,0 +1,12 @@
|
|
+table netdev t {
|
|
+ set s {
|
|
+ typeof ether saddr . vlan id
|
|
+ size 2048
|
|
+ flags dynamic,timeout
|
|
+ }
|
|
+
|
|
+ chain c {
|
|
+ ether type != 8021q add @s { ether saddr . 0 timeout 5s } counter packets 0 bytes 0 return
|
|
+ ether type != 8021q update @s { ether daddr . 123 timeout 1m } counter packets 0 bytes 0 return
|
|
+ }
|
|
+}
|
|
diff --git a/tests/shell/testcases/sets/typeof_sets_concat b/tests/shell/testcases/sets/typeof_sets_concat
|
|
new file mode 100755
|
|
index 0000000..07820b7
|
|
--- /dev/null
|
|
+++ b/tests/shell/testcases/sets/typeof_sets_concat
|
|
@@ -0,0 +1,6 @@
|
|
+#!/bin/bash
|
|
+
|
|
+set -e
|
|
+dumpfile=$(dirname $0)/dumps/$(basename $0).nft
|
|
+
|
|
+$NFT -f "$dumpfile"
|
|
--
|
|
2.41.0.rc1
|
|
|