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

[Pablo Neira Ayuso <pablo@netfilter.org>]

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;