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

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

[Florian Westphal]

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.

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)
+			cmd->expr->set_flags |= NFT_SET_INTERVAL;
+	}
 
 	ctx->set = NULL;
 
-- 
2.41.0

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

[Pablo Neira Ayuso]

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
>