Pablo Neira Ayuso <pablo@xxxxxxxxxxxxx> wrote:
> On Fri, May 24, 2019 at 11:06:34PM +0200, Florian Westphal wrote:
> > Pablo Neira Ayuso <pablo@xxxxxxxxxxxxx> wrote:
> > > On Sun, May 19, 2019 at 07:18:38PM +0200, Florian Westphal wrote:
> > > > convert "ip saddr { 1.1.1.1 }" to "ip saddr 1.1.1.1".
> > > > Both do the same, but second form is faster since no single-element
> > > > anon set is created.
> > > >
> > > > Fix up the remaining test cases to expect transformations of the form
> > > > "meta l4proto { 33-55}" to "meta l4proto 33-55".
> > >
> > > Last time we discussed this I think we agreed to spew a warning for
> > > this to educate people on this.
> >
> > I decided against it.
> > Why adding a warning? We do not change what the rule does, and we do
> > not collapse different rules into one.
>
> Yes, there is no semantic change.
>
> Should we also do transparent transformation for
>
> ct state { established,new }
>
> I have seen rulesets like this too.
Hmm, to what should this be transformed?
ct state established,new ?
We can do it for this one, established and new can never be set at the
same time (or any other state for that matter).
[..]
> > Full ruleset optimization should, IMO, be done transparently as
> > well if we're confident such transformations are done correctly.
> >
> > I would however only do it for nft -f, not for "nft add rule ..."
>
> Why only for nft -f and not for "nft add rule" ?
I think if you do a 'nft add rule' then you expect to add a rule and
not to modify an existing one.
There is also the performance aspect, for 'nft add' you can just
build the rule object and pass that to kernel, but for optimizing
you need to fetch what is there and perform analysis on it.
I should have be more specific, with "nft -f" i meant a full rule
file, not a partial one (esp. one that has a 'flush ruleset'), or
something that at least contains a complete subset, e.g. a new
chain with n rules.
We could add a command line switch for this and allow optimizing in
any case, or add a new 'nft compact' or 'nft optimize' command that
would fetch the ruleset and print the 'optimized' version what nft
thinks can be coalesced/omitted etc.
What do you think?
> We can add a new parameter to optimize rulesets, we can start with
> something simple, ie.
>
> * collapse consecutive several rules that come with the same
> selectors, only values change.
>
> * turn { 22 } into 22.
>
> * turn ct state {new, established } into ct new,established.
>
> > I think a trivial one such as s/{ 22 }/22/ should just be done automatically.
>
> We could, yes. But I would prefer if we place all optimizations under
> some new option and we document them.
>
> I was expecting to have a discussion on this during the NFWS :-).
Sure, lets do that.
