[PATCH nft] evaluate: prevent merge of sets with incompatible keys

[Florian Westphal <fw@strlen.de>]

Its not enough to check for interval flag, this would assert in interval
code due to concat being passed to the interval code:
BUG: unhandled key type 13

After fix:
same_set_name_but_different_keys_assert:8:6-7: Error: set already exists with
different datatype (concatenation of (IPv4 address, network interface index) vs
network interface index)
        set s4 {
            ^^

This also improves error verbosity when mixing datamap and objref maps:

invalid_transcation_merge_map_and_objref_map:9:13-13:
Error: map already exists with different datatype (IPv4 address vs string)

.. instead of 'Cannot merge map with incompatible existing map of same name'.
The 'Cannot merge map with incompatible existing map of same name' check
is kept in place to catch when ruleset contains a set and map with same name
and same key definition.

Signed-off-by: Florian Westphal <fw@strlen.de>
---
 Followup to previous
 '[nft,v2] evaluate: check that set type is identical before merging',
  it has the welcome side effect to improve error reporting as well.

 src/evaluate.c                                      | 12 ++++++++++++
 src/intervals.c                                     |  2 +-
 .../nft-f/same_set_name_but_different_keys_assert   | 13 +++++++++++++
 3 files changed, 26 insertions(+), 1 deletion(-)
 create mode 100644 tests/shell/testcases/bogons/nft-f/same_set_name_but_different_keys_assert

diff --git a/src/evaluate.c b/src/evaluate.c
index fc9d82f73b68..a2d5d7c29514 100644
--- a/src/evaluate.c
+++ b/src/evaluate.c
@@ -5304,6 +5304,18 @@ static int set_evaluate(struct eval_ctx *ctx, struct set *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->data && existing_set->data &&
+		    !datatype_equal(existing_set->data->dtype, set->data->dtype))
+			return set_error(ctx, set,
+					 "%s already exists with different datatype (%s vs %s)",
+					 type, existing_set->data->dtype->desc,
+					 set->data->dtype->desc);
+		if (!datatype_equal(existing_set->key->dtype, set->key->dtype))
+			return set_error(ctx, set,
+					 "%s already exists with different datatype (%s vs %s)",
+					 type, existing_set->key->dtype->desc,
+					 set->key->dtype->desc);
+		/* Catch attempt to merge set and map */
 		if (!set_type_compatible(set, existing_set))
 			return set_error(ctx, set, "Cannot merge %s with incompatible existing %s of same name",
 					type,
diff --git a/src/intervals.c b/src/intervals.c
index bf125a0c59d3..e5bbb0384964 100644
--- a/src/intervals.c
+++ b/src/intervals.c
@@ -70,7 +70,7 @@ static void setelem_expr_to_range(struct expr *expr)
 		expr->key = key;
 		break;
 	default:
-		BUG("unhandled key type %d\n", expr->key->etype);
+		BUG("unhandled key type %s\n", expr_name(expr->key));
 	}
 }
 
diff --git a/tests/shell/testcases/bogons/nft-f/same_set_name_but_different_keys_assert b/tests/shell/testcases/bogons/nft-f/same_set_name_but_different_keys_assert
new file mode 100644
index 000000000000..8fcfdf5cba03
--- /dev/null
+++ b/tests/shell/testcases/bogons/nft-f/same_set_name_but_different_keys_assert
@@ -0,0 +1,13 @@
+table ip t {
+	set s4 {
+		type ipv4_addr . iface_index
+		flags interval
+		elements = { 127.0.0.1 . "lo" }
+	}
+
+	set s4 {
+		type iface_index
+		flags interval
+		elements = { "lo" }
+	}
+}
-- 
2.49.0