Re: [PATCH nft] evaluate: disallow anonymous set with empty elements

[Phil Sutter <phil@nwl.cc>]

Hi,

On Wed, Apr 10, 2019 at 01:19:25AM +0200, Pablo Neira Ayuso wrote:
> On Tue, Apr 09, 2019 at 04:03:26PM +0200, Florian Westphal wrote:
> > Phil Sutter <phil@nwl.cc> wrote:
> > > Could we maybe find a middle ground where nft still does these
> > > optimizations but prints warnings so users are notified? We might even
> > > introduce -W flag to customize behaviour (-W all (default), -W error
> > > (strict mode), -W none (suppress any non-fatal output on stderr)).
> > 
> > I like this proposal.
> > 
> > One of the broken tproxy test cases (it prints warning) does this:
> > 
> > ip daddr 0.0.0.0/0
> 
> Yes, sorry, that's my fault.
> 
> > .. and that is always true and could be removed.
> > Different "problem" of course, but it shows that there is ample
> > opportunity for pruning irrelevant expressions.
> > 
> > And breaking scripts every time we decide that something is
> > "silly" is a bad decision, imo.
> 
> Agreed, this case is slightly bit corner case as they should _not_ be
> doing enclosing single element in brackets in their scripts. But I get
> your point, better adopt a more conservative approach ;-)
> 
> > I suspect users will complain about { 1.2.3.4 } being illegal
> > "just because".
> 
> I'll explore the warning idea, it can be an initial step before we can
> fully disallow this, so users don't complain about sudden breakage :-)

What I have in mind is "dumb" scripts collecting addresses and adding a
rule matching them in an anonymous set. The case of just a single address
needs additional code, not just an adjustment of the existing one. This
is not so much a matter of bad design or missing education but one of
effort and feasibility.

Cheers, Phil