[PATCH nft v2] evaluate: check that set type is identical before merging

[PATCH nft v2] evaluate: check that set type is identical before merging

[Florian Westphal]

Reject maps and sets of the same name:
 BUG: invalid range expression type catch-all set element
 nft: src/expression.c:1704: range_expr_value_low: Assertion `0' failed.

After:
Error: Cannot merge set with existing datamap of same name
  set z {
      ^

v2:
Pablo points out that we shouldn't merge datamaps (plain value) and objref
maps either, catch this too and add another test:

nft --check -f invalid_transcation_merge_map_and_objref_map
invalid_transcation_merge_map_and_objref_map:9:13-13: Error: Cannot merge objmap with existing datamap of same name

Signed-off-by: Florian Westphal <fw@strlen.de>
---
 src/evaluate.c                                | 34 +++++++++++++++++--
 ...pression_type_catch-all_set_element_assert | 18 ++++++++++
 ...valid_transcation_merge_map_and_objref_map | 13 +++++++
 3 files changed, 63 insertions(+), 2 deletions(-)
 create mode 100644 tests/shell/testcases/bogons/nft-f/invalid_range_expression_type_catch-all_set_element_assert
 create mode 100644 tests/shell/testcases/bogons/nft-f/invalid_transcation_merge_map_and_objref_map

diff --git a/src/evaluate.c b/src/evaluate.c
index 783a373b6268..3c091748f786 100644
--- a/src/evaluate.c
+++ b/src/evaluate.c
@@ -5149,6 +5149,29 @@ static int elems_evaluate(struct eval_ctx *ctx, struct set *set)
 	return 0;
 }
 
+static const char *set_type_str(const struct set *set)
+{
+	if (set_is_datamap(set->flags))
+		return "datamap";
+
+	if (set_is_objmap(set->flags))
+		return "objmap";
+
+	return "set";
+}
+
+static bool set_type_compatible(const struct set *set, const struct set *existing_set)
+{
+	if (set_is_datamap(set->flags))
+		return set_is_datamap(existing_set->flags);
+
+	if (set_is_objmap(set->flags))
+		return set_is_objmap(existing_set->flags);
+
+	assert(!set_is_map(set->flags));
+	return !set_is_map(existing_set->flags);
+}
+
 static int set_evaluate(struct eval_ctx *ctx, struct set *set)
 {
 	struct set *existing_set = NULL;
@@ -5272,8 +5295,15 @@ static int set_evaluate(struct eval_ctx *ctx, struct set *set)
 		return 0;
 	}
 
-	if (existing_set && set_is_interval(set->flags) && !set_is_interval(existing_set->flags))
-		return set_error(ctx, set, "existing %s lacks interval flag", type);
+
+	if (existing_set) {
+		if (set_is_interval(set->flags) && !set_is_interval(existing_set->flags))
+			return set_error(ctx, set,
+					 "existing %s lacks interval flag", type);
+		if (!set_type_compatible(set, existing_set))
+			return set_error(ctx, set, "Cannot merge %s with existing %s of same name",
+					set_type_str(set), set_type_str(existing_set));
+	}
 
 	set->existing_set = existing_set;
 
diff --git a/tests/shell/testcases/bogons/nft-f/invalid_range_expression_type_catch-all_set_element_assert b/tests/shell/testcases/bogons/nft-f/invalid_range_expression_type_catch-all_set_element_assert
new file mode 100644
index 000000000000..3660ac3fda9c
--- /dev/null
+++ b/tests/shell/testcases/bogons/nft-f/invalid_range_expression_type_catch-all_set_element_assert
@@ -0,0 +1,18 @@
+table ip x {
+	map z {
+		type ipv4_addr : ipv4_addr
+		flags interval
+		elements = { 10.0.0.2, * : 192.168.0.4 }
+	}
+
+	set z {
+		type ipv4_addr
+		flags interval
+		counter
+		elements = { 1.1.1.0/24 counter packets 0 bytes 0,
+			 * counter packets 0 bytes 0packets 0 bytes ipv4_addr }
+		flags interval
+		auto-merge
+		elements = { 1.1.1.1 }
+	}
+}
diff --git a/tests/shell/testcases/bogons/nft-f/invalid_transcation_merge_map_and_objref_map b/tests/shell/testcases/bogons/nft-f/invalid_transcation_merge_map_and_objref_map
new file mode 100644
index 000000000000..e1fde58553e9
--- /dev/null
+++ b/tests/shell/testcases/bogons/nft-f/invalid_transcation_merge_map_and_objref_map
@@ -0,0 +1,13 @@
+table ip x {
+        counter a { }
+
+	map m {
+		type ipv4_addr : ipv4_addr
+		elements = { 10.0.0.2 : 192.168.0.4 }
+	}
+
+        map m {
+		type ipv4_addr : counter
+                elements = { 192.168.2.2 : "a" }
+        }
+}
-- 
2.49.0

Re: [PATCH nft v2] evaluate: check that set type is identical before merging

[Pablo Neira Ayuso]

On Mon, Jun 23, 2025 at 09:37:31PM +0200, Florian Westphal wrote:
> Reject maps and sets of the same name:
>  BUG: invalid range expression type catch-all set element
>  nft: src/expression.c:1704: range_expr_value_low: Assertion `0' failed.
> 
> After:
> Error: Cannot merge set with existing datamap of same name
>   set z {
>       ^
> 
> v2:
> Pablo points out that we shouldn't merge datamaps (plain value) and objref
> maps either, catch this too and add another test:
> 
> nft --check -f invalid_transcation_merge_map_and_objref_map
> invalid_transcation_merge_map_and_objref_map:9:13-13: Error: Cannot merge objmap with existing datamap of same name
> 
> Signed-off-by: Florian Westphal <fw@strlen.de>
> ---
>  src/evaluate.c                                | 34 +++++++++++++++++--
>  ...pression_type_catch-all_set_element_assert | 18 ++++++++++
>  ...valid_transcation_merge_map_and_objref_map | 13 +++++++
>  3 files changed, 63 insertions(+), 2 deletions(-)
>  create mode 100644 tests/shell/testcases/bogons/nft-f/invalid_range_expression_type_catch-all_set_element_assert
>  create mode 100644 tests/shell/testcases/bogons/nft-f/invalid_transcation_merge_map_and_objref_map
> 
> diff --git a/src/evaluate.c b/src/evaluate.c
> index 783a373b6268..3c091748f786 100644
> --- a/src/evaluate.c
> +++ b/src/evaluate.c
> @@ -5149,6 +5149,29 @@ static int elems_evaluate(struct eval_ctx *ctx, struct set *set)
>  	return 0;
>  }
>  
> +static const char *set_type_str(const struct set *set)
> +{
> +	if (set_is_datamap(set->flags))
> +		return "datamap";
> +
> +	if (set_is_objmap(set->flags))
> +		return "objmap";
> +
> +	return "set";
> +}

"datamap" and "objmap" are internal concepts, users only see "maps"
from their side.

So I would not expose this in the error message.

Maybe you could just say map declarations are different by now. Later more
accurate error reporting on what is precisely different can be added.

Apart from the error report nitpick I don't see anything wrong with
this patch.

> +static bool set_type_compatible(const struct set *set, const struct set *existing_set)
> +{
> +	if (set_is_datamap(set->flags))
> +		return set_is_datamap(existing_set->flags);
> +
> +	if (set_is_objmap(set->flags))
> +		return set_is_objmap(existing_set->flags);
> +
> +	assert(!set_is_map(set->flags));
> +	return !set_is_map(existing_set->flags);
> +}
> +
>  static int set_evaluate(struct eval_ctx *ctx, struct set *set)
>  {
>  	struct set *existing_set = NULL;