Re: [PATCH nft] src: collapse set element commands from parser

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

Hi Eric,

On Thu, Oct 31, 2024 at 11:14:53AM -0400, Eric Garver wrote:
> On Wed, Oct 23, 2024 at 03:34:40PM +0200, Pablo Neira Ayuso wrote:
> > 498a5f0c219d ("rule: collapse set element commands") does not help to
> > reduce memory consumption in the case of large sets defined by one
> > element per line:
> > 
> >  add element ip x y { 1.1.1.1 }
> >  add element ip x y { 1.1.1.2 }
> >  ...
> > 
> > This patch collapses set element whenver possible to reduce the number
> > of cmd objects, this reduces memory consumption by ~75%.
> > 
> > This patch also adds a special case for variables for sets similar to:
> > 
> >   be055af5c58d ("cmd: skip variable set elements when collapsing commands")
> > 
> > This patch requires this small kernel fix:
> > 
> >  commit b53c116642502b0c85ecef78bff4f826a7dd4145
> >  Author: Pablo Neira Ayuso <pablo@netfilter.org>
> >  Date:   Fri May 20 00:02:06 2022 +0200
> > 
> >     netfilter: nf_tables: set element extended ACK reporting support
> > 
> > which is included in recent -stable kernels:
> > 
> >  # cat ruleset.nft
> >  add table ip x
> >  add chain ip x y
> >  add set ip x y { type ipv4_addr; }
> >  create element ip x y { 1.1.1.1 }
> >  create element ip x y { 1.1.1.1 }
> > 
> >  # nft -f ruleset.nft
> >  ruleset.nft:5:25-31: Error: Could not process rule: File exists
> >  create element ip x y { 1.1.1.1 }
> >                          ^^^^^^^
> > 
> > there is no need to relate commands via sequence number, this allows to
> > remove the uncollapse step too.
> > 
> > Fixes: 498a5f0c219d ("rule: collapse set element commands")
> > Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
> 
> Hi Pablo,
> 
> This patch appears to introduce a performance regression for set entries
> in the JSON interface. AFAICS, the collapse code is only called from the
> CLI parser now.

Indeed, I am working on a fix for the JSON parser.

Thanks for early reporting this issue!