Re: [PATCH nft] evaluate: fix get element for concatenated set

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

On Tue, Sep 05, 2023 at 04:41:38PM +0200, Florian Westphal wrote:
> given:
> table ip filter {
> 	set test {
> 		type ipv4_addr . ether_addr . mark
> 		flags interval
> 		elements = { 198.51.100.0/25 . 00:0b:0c:ca:cc:10-c1:a0:c1:cc:10:00 . 0x0000006f, }
> 	}
> }
> 
> We get lookup failure:
> 
> nft get element ip filter test { 198.51.100.1 . 00:0b:0c:ca:cc:10 . 0x6f }
> Error: Could not process rule: No such file or directory
> 
> Its possible to work around this via dummy range somewhere in the key, e.g.
> nft get element ip filter test { 198.51.100.1 . 00:0b:0c:ca:cc:10 . 0x6f-0x6f }
> 
> but that shouldn't be needed, so make sure the INTERVAL flag is enabled
> for the queried element if the set is of interval type.

LGTM, comment below:

> Signed-off-by: Florian Westphal <fw@strlen.de>
> ---
>  src/evaluate.c | 11 +++++++----
>  1 file changed, 7 insertions(+), 4 deletions(-)
> 
> diff --git a/src/evaluate.c b/src/evaluate.c
> index ab3ec98739e9..b1fe7147c2e1 100644
> --- a/src/evaluate.c
> +++ b/src/evaluate.c
> @@ -4500,11 +4500,14 @@ static int setelem_evaluate(struct eval_ctx *ctx, struct cmd *cmd)
>  		return -1;
>  
>  	cmd->elem.set = set_get(set);
> +	if (set_is_interval(ctx->set->flags)) {
> +		if (!(set->flags & NFT_SET_CONCAT) &&
> +		    interval_set_eval(ctx, ctx->set, cmd->expr) < 0)
> +			return -1;
>  
> -	if (set_is_interval(ctx->set->flags) &&
> -	    !(set->flags & NFT_SET_CONCAT) &&
> -	    interval_set_eval(ctx, ctx->set, cmd->expr) < 0)
> -		return -1;
> +		if (cmd->expr->etype == EXPR_SET)

setelem_evaluate() is always called for CMD_OBJ_ELEMENTS.

I think this branch always evaluates 'true'.

                if (cmd->expr->etype == EXPR_SET)


> +			cmd->expr->set_flags |= NFT_SET_INTERVAL;

so maybe set_flags inconditionally?

> +	}
>  
>  	ctx->set = NULL;
>  
> -- 
> 2.41.0
>