nftables/0014-evaluate-allow-to-re-use-existing-metered-set.patch
Eric Garver 6e830a1e31 nftables-1.1.1-4.el10
* 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
2025-03-03 15:43:38 -05:00

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