* Mon Mar 03 2025 Eric Garver <egarver@redhat.com> [1.1.1-4.el10] - evaluate: allow to re-use existing metered set [RHEL-75507] Resolves: RHEL-75507
272 lines
7.7 KiB
Diff
272 lines
7.7 KiB
Diff
From b3c1312b5815b004614d79eae2ad731c6883ce6f Mon Sep 17 00:00:00 2001
|
|
From: Florian Westphal <fw@strlen.de>
|
|
Date: Wed, 22 Jan 2025 10:18:04 +0100
|
|
Subject: [PATCH] evaluate: allow to re-use existing metered set
|
|
|
|
JIRA: https://issues.redhat.com/browse/RHEL-75507
|
|
Upstream Status: nftables commit 639a111e91341cffdc6d86b847aa654646c799cf
|
|
|
|
commit 639a111e91341cffdc6d86b847aa654646c799cf
|
|
Author: Florian Westphal <fw@strlen.de>
|
|
Date: Wed Jan 22 10:18:04 2025 +0100
|
|
|
|
evaluate: allow to re-use existing metered set
|
|
|
|
Blamed commit translates old meter syntax (which used to allocate an
|
|
anonymous set) to dynamic sets.
|
|
|
|
A side effect of this is that re-adding a meter rule after chain was
|
|
flushed results in an error, unlike anonymous sets named sets are not
|
|
impacted by the flush.
|
|
|
|
Refine this: if a set of the same name exists and is compatible, then
|
|
re-use it instead of returning an error.
|
|
|
|
Also pick up the reproducer kindly provided by the reporter and place it
|
|
in the shell test directory.
|
|
|
|
Fixes: b8f8ddfff733 ("evaluate: translate meter into dynamic set")
|
|
Reported-by: Yi Chen <yiche@redhat.com>
|
|
Signed-off-by: Florian Westphal <fw@strlen.de>
|
|
|
|
Signed-off-by: Eric Garver <egarver@redhat.com>
|
|
---
|
|
src/evaluate.c | 43 +++++--
|
|
.../sets/dumps/meter_set_reuse.json-nft | 105 ++++++++++++++++++
|
|
.../testcases/sets/dumps/meter_set_reuse.nft | 11 ++
|
|
tests/shell/testcases/sets/meter_set_reuse | 20 ++++
|
|
4 files changed, 170 insertions(+), 9 deletions(-)
|
|
create mode 100644 tests/shell/testcases/sets/dumps/meter_set_reuse.json-nft
|
|
create mode 100644 tests/shell/testcases/sets/dumps/meter_set_reuse.nft
|
|
create mode 100755 tests/shell/testcases/sets/meter_set_reuse
|
|
|
|
diff --git a/src/evaluate.c b/src/evaluate.c
|
|
index 593a0140e92a..c9cbaa6ae648 100644
|
|
--- a/src/evaluate.c
|
|
+++ b/src/evaluate.c
|
|
@@ -3338,7 +3338,7 @@ static int stmt_evaluate_payload(struct eval_ctx *ctx, struct stmt *stmt)
|
|
|
|
static int stmt_evaluate_meter(struct eval_ctx *ctx, struct stmt *stmt)
|
|
{
|
|
- struct expr *key, *set, *setref;
|
|
+ struct expr *key, *setref;
|
|
struct set *existing_set;
|
|
struct table *table;
|
|
|
|
@@ -3349,7 +3349,9 @@ static int stmt_evaluate_meter(struct eval_ctx *ctx, struct stmt *stmt)
|
|
return table_not_found(ctx);
|
|
|
|
existing_set = set_cache_find(table, stmt->meter.name);
|
|
- if (existing_set)
|
|
+ if (existing_set &&
|
|
+ (!set_is_meter_compat(existing_set->flags) ||
|
|
+ set_is_map(existing_set->flags)))
|
|
return cmd_error(ctx, &stmt->location,
|
|
"%s; meter '%s' overlaps an existing %s '%s' in family %s",
|
|
strerror(EEXIST),
|
|
@@ -3370,17 +3372,40 @@ static int stmt_evaluate_meter(struct eval_ctx *ctx, struct stmt *stmt)
|
|
|
|
/* Declare an empty set */
|
|
key = stmt->meter.key;
|
|
- set = set_expr_alloc(&key->location, NULL);
|
|
- set->set_flags |= NFT_SET_EVAL;
|
|
- if (key->timeout)
|
|
- set->set_flags |= NFT_SET_TIMEOUT;
|
|
+ if (existing_set) {
|
|
+ if ((existing_set->flags & NFT_SET_TIMEOUT) && !key->timeout)
|
|
+ return expr_error(ctx->msgs, stmt->meter.key,
|
|
+ "existing set '%s' has timeout flag",
|
|
+ stmt->meter.name);
|
|
+
|
|
+ if ((existing_set->flags & NFT_SET_TIMEOUT) == 0 && key->timeout)
|
|
+ return expr_error(ctx->msgs, stmt->meter.key,
|
|
+ "existing set '%s' lacks timeout flag",
|
|
+ stmt->meter.name);
|
|
+
|
|
+ if (stmt->meter.size > 0 && existing_set->desc.size != stmt->meter.size)
|
|
+ return expr_error(ctx->msgs, stmt->meter.key,
|
|
+ "existing set '%s' has size %u, meter has %u",
|
|
+ stmt->meter.name, existing_set->desc.size,
|
|
+ stmt->meter.size);
|
|
+ setref = set_ref_expr_alloc(&key->location, existing_set);
|
|
+ } else {
|
|
+ struct expr *set;
|
|
+
|
|
+ set = set_expr_alloc(&key->location, existing_set);
|
|
+ if (key->timeout)
|
|
+ set->set_flags |= NFT_SET_TIMEOUT;
|
|
+
|
|
+ set->set_flags |= NFT_SET_EVAL;
|
|
+ setref = implicit_set_declaration(ctx, stmt->meter.name,
|
|
+ expr_get(key), NULL, set, 0);
|
|
+ if (setref)
|
|
+ setref->set->desc.size = stmt->meter.size;
|
|
+ }
|
|
|
|
- setref = implicit_set_declaration(ctx, stmt->meter.name,
|
|
- expr_get(key), NULL, set, 0);
|
|
if (!setref)
|
|
return -1;
|
|
|
|
- setref->set->desc.size = stmt->meter.size;
|
|
stmt->meter.set = setref;
|
|
|
|
if (stmt_evaluate(ctx, stmt->meter.stmt) < 0)
|
|
diff --git a/tests/shell/testcases/sets/dumps/meter_set_reuse.json-nft b/tests/shell/testcases/sets/dumps/meter_set_reuse.json-nft
|
|
new file mode 100644
|
|
index 000000000000..ab4ac06184d0
|
|
--- /dev/null
|
|
+++ b/tests/shell/testcases/sets/dumps/meter_set_reuse.json-nft
|
|
@@ -0,0 +1,105 @@
|
|
+{
|
|
+ "nftables": [
|
|
+ {
|
|
+ "metainfo": {
|
|
+ "version": "VERSION",
|
|
+ "release_name": "RELEASE_NAME",
|
|
+ "json_schema_version": 1
|
|
+ }
|
|
+ },
|
|
+ {
|
|
+ "table": {
|
|
+ "family": "ip",
|
|
+ "name": "filter",
|
|
+ "handle": 0
|
|
+ }
|
|
+ },
|
|
+ {
|
|
+ "chain": {
|
|
+ "family": "ip",
|
|
+ "table": "filter",
|
|
+ "name": "input",
|
|
+ "handle": 0
|
|
+ }
|
|
+ },
|
|
+ {
|
|
+ "set": {
|
|
+ "family": "ip",
|
|
+ "name": "http1",
|
|
+ "table": "filter",
|
|
+ "type": [
|
|
+ "inet_service",
|
|
+ "ipv4_addr"
|
|
+ ],
|
|
+ "handle": 0,
|
|
+ "size": 65535,
|
|
+ "flags": [
|
|
+ "dynamic"
|
|
+ ]
|
|
+ }
|
|
+ },
|
|
+ {
|
|
+ "rule": {
|
|
+ "family": "ip",
|
|
+ "table": "filter",
|
|
+ "chain": "input",
|
|
+ "handle": 0,
|
|
+ "expr": [
|
|
+ {
|
|
+ "match": {
|
|
+ "op": "==",
|
|
+ "left": {
|
|
+ "payload": {
|
|
+ "protocol": "tcp",
|
|
+ "field": "dport"
|
|
+ }
|
|
+ },
|
|
+ "right": 80
|
|
+ }
|
|
+ },
|
|
+ {
|
|
+ "set": {
|
|
+ "op": "add",
|
|
+ "elem": {
|
|
+ "concat": [
|
|
+ {
|
|
+ "payload": {
|
|
+ "protocol": "tcp",
|
|
+ "field": "dport"
|
|
+ }
|
|
+ },
|
|
+ {
|
|
+ "payload": {
|
|
+ "protocol": "ip",
|
|
+ "field": "saddr"
|
|
+ }
|
|
+ }
|
|
+ ]
|
|
+ },
|
|
+ "set": "@http1",
|
|
+ "stmt": [
|
|
+ {
|
|
+ "limit": {
|
|
+ "rate": 200,
|
|
+ "burst": 5,
|
|
+ "per": "second",
|
|
+ "inv": true
|
|
+ }
|
|
+ }
|
|
+ ]
|
|
+ }
|
|
+ },
|
|
+ {
|
|
+ "counter": {
|
|
+ "packets": 0,
|
|
+ "bytes": 0
|
|
+ }
|
|
+ },
|
|
+ {
|
|
+ "drop": null
|
|
+ }
|
|
+ ]
|
|
+ }
|
|
+ }
|
|
+ ]
|
|
+}
|
|
diff --git a/tests/shell/testcases/sets/dumps/meter_set_reuse.nft b/tests/shell/testcases/sets/dumps/meter_set_reuse.nft
|
|
new file mode 100644
|
|
index 000000000000..f911acaffb85
|
|
--- /dev/null
|
|
+++ b/tests/shell/testcases/sets/dumps/meter_set_reuse.nft
|
|
@@ -0,0 +1,11 @@
|
|
+table ip filter {
|
|
+ set http1 {
|
|
+ type inet_service . ipv4_addr
|
|
+ size 65535
|
|
+ flags dynamic
|
|
+ }
|
|
+
|
|
+ chain input {
|
|
+ tcp dport 80 add @http1 { tcp dport . ip saddr limit rate over 200/second burst 5 packets } counter packets 0 bytes 0 drop
|
|
+ }
|
|
+}
|
|
diff --git a/tests/shell/testcases/sets/meter_set_reuse b/tests/shell/testcases/sets/meter_set_reuse
|
|
new file mode 100755
|
|
index 000000000000..94eccc1a7b82
|
|
--- /dev/null
|
|
+++ b/tests/shell/testcases/sets/meter_set_reuse
|
|
@@ -0,0 +1,20 @@
|
|
+#!/bin/bash
|
|
+
|
|
+set -e
|
|
+
|
|
+addrule()
|
|
+{
|
|
+ $NFT add rule ip filter input tcp dport 80 meter http1 { tcp dport . ip saddr limit rate over 200/second } counter drop
|
|
+}
|
|
+
|
|
+$NFT add table filter
|
|
+$NFT add chain filter input
|
|
+addrule
|
|
+
|
|
+$NFT list meters
|
|
+
|
|
+# This used to remove the anon set, but not anymore
|
|
+$NFT flush chain filter input
|
|
+
|
|
+# This re-add should work.
|
|
+addrule
|
|
--
|
|
2.48.1
|
|
|