Re: [nft PATCH] evaluate: Reject set stmt refs to sets without dynamic flag

[Phil Sutter <phil@nwl.cc>]

Hi Pablo,

On Thu, May 18, 2023 at 11:41:10AM +0200, Pablo Neira Ayuso wrote:
> On Wed, May 10, 2023 at 01:48:01PM +0200, Phil Sutter wrote:
> > Hi Pablo,
> > 
> > On Wed, May 10, 2023 at 12:54:28PM +0200, Pablo Neira Ayuso wrote:
> > > On Wed, May 03, 2023 at 12:50:22PM +0200, Phil Sutter wrote:
> > > > This is a revert of commit 8d443adfcc8c1 ("evaluate: attempt to set_eval
> > > > flag if dynamic updates requested"), implementing the alternative
> > > > mentioned in the comment it added.
> > > > 
> > > > Reason is the inconsistent behaviour when applying the same ruleset
> > > > twice: In the first call, the set lacking 'dynamic' flag does not exist
> > > > and is therefore added to the cache. Consequently, both the 'add set'
> > > > command and the set statement point at the same set object. In the
> > > > second call, a set with same name exists already, so the object created
> > > > for 'add set' command is not added to cache and consequently not updated
> > > > with the missing flag. The kernel thus rejects the NEWSET request as the
> > > > existing set differs from the new one.
> > > 
> > > # cat test.nft
> > > flush ruleset
> > 
> > Just remove this 'flush ruleset' call, then it should trigger.
> 
> I cannot reproduce it yet :(

This is very odd.

> # cat test.nft
> table ip test {
>         set dlist {
>                 type ipv4_addr
>                 size 65535
>         }
> 
>         chain output {
>                 type filter hook output priority filter; policy accept;
>                 udp dport 1234 update @dlist { ip daddr } counter packets 0 bytes 0
>         }
> }
> # nft -f test.nft
> # nft -f test.nft

On my side, this second call emits:

| /tmp/test.nft:2:6-10: Error: Could not process rule: File exists
|         set dlist {
|             ^^^^^

> Maybe I need a specific kernel? I am trying with latest.

IIUC, it is not kernel-related. I can reproduce with:

nftables @ d486c9e626405 ("datatype: add hint error handler")
libnftnl @ libnftnl-1.2.5
linux @ b50a8b0d57ab1 ("net: openvswitch: Use struct_size()")

Cheers, Phil