On Mon, Jun 14, 2021 at 01:58:55PM +0200, Pablo Neira Ayuso wrote: > On Mon, Jun 14, 2021 at 01:56:44PM +0200, Pablo Neira Ayuso wrote: > > On Mon, Jun 14, 2021 at 01:34:03PM +0200, Phil Sutter wrote: > > > Hi Pablo, > > > > > > On Fri, Jun 11, 2021 at 07:15:38PM +0200, Pablo Neira Ayuso wrote: > > > > Perform the table and set lookup only for non-anonymous sets, where the > > > > incremental cache update is required. > > > > > > > > The problem fixed by 7aa08d45031e ("evaluate: Perform set evaluation on > > > > implicitly declared (anonymous) sets") resurrected after the cache > > > > rework. > > > > > > > > # nft add rule x y tcp sport . tcp dport vmap { ssh . 0-65535 : accept, 0-65535 . ssh : accept } > > > > BUG: invalid range expression type concat > > > > nft: expression.c:1422: range_expr_value_low: Assertion `0' failed. > > > > Abort > > > > > > > > Add a test case to make sure this does not happen again. > > > > > > > > Fixes: 5ec5c706d993 ("cache: add hashtable cache for table") > > > > Signed-off-by: Pablo Neira Ayuso <pablo@xxxxxxxxxxxxx> > > > > > > This triggers a warning: > > > > > > evaluate.c: In function 'set_evaluate': > > > evaluate.c:3870:13: warning: 'table' may be used uninitialized in this function [-Wmaybe-uninitialized] > > > 3870 | if (set_cache_find(table, set->handle.set.name) == NULL) > > > | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ > > > > Hm, this needs to be restricted to anonymous sets, attaching patch. > > It's a false positive though, there is a check for anonymous set a few > lines before. I'll apply this patch if this makes gcc happy on your side. Actually, this code should not be there. The set is already in the cache.
diff --git a/src/evaluate.c b/src/evaluate.c index 5311963a20c5..92cc8994c809 100644 --- a/src/evaluate.c +++ b/src/evaluate.c @@ -3867,9 +3867,6 @@ static int set_evaluate(struct eval_ctx *ctx, struct set *set) } ctx->set = NULL; - if (set_cache_find(table, set->handle.set.name) == NULL) - set_cache_add(set_get(set), table); - return 0; }