All of lore.kernel.org
 help / color / mirror / Atom feed
From: Phil Sutter <phil@nwl.cc>
To: Pablo Neira Ayuso <pablo@netfilter.org>
Cc: netfilter-devel@vger.kernel.org, fw@strlen.de, arturo@netfilter.org
Subject: Re: [PATCH nft] evaluate: disallow anonymous set with empty elements
Date: Tue, 9 Apr 2019 15:59:25 +0200	[thread overview]
Message-ID: <20190409135925.GQ4851@orbyte.nwl.cc> (raw)
In-Reply-To: <20190409105936.23422-1-pablo@netfilter.org>

Hi Pablo,

On Tue, Apr 09, 2019 at 12:59:36PM +0200, Pablo Neira Ayuso wrote:
> Restrict this, the brackets have explicit semantics since they tell the
> kernel to represent this value as a set, which is too costly. Set for
> one single element are overkill.
> 
>  # nft add rule x y ct state { established } counter
>  Error: anonymous set with single element makes no sense, remove brackets wrapping this value
>  add rule x y ct state { established } counter
>                        ^^^^^^^^^^^^^^^
> 
> Instead, the preferred way to express this is:
> 
>  # nft add rule x y ct state established counter
> 
> Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
> ---
> I know this may break stuff outthere, but probably it's still early to
> fix this. If we keep allowing this and transparently turn this into a
> value, people will likely never understand the bracket semantics.
> Brackets are not just syntaxic sugar.

Your point makes sense, understanding that within a rule curly braces
are not a block delimiter but a set definition is key to getting along
with nft syntax.

OTOH I like how we radically optimize anonymous sets. This allows to
have rather "dumb" scripts and get by without a performance penalty.

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)).

Just an idea, not sure if feasible.

Cheers, Phil

  parent reply	other threads:[~2019-04-09 13:59 UTC|newest]

Thread overview: 6+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-04-09 10:59 [PATCH nft] evaluate: disallow anonymous set with empty elements Pablo Neira Ayuso
2019-04-09 13:02 ` Pablo Neira Ayuso
2019-04-09 13:59 ` Phil Sutter [this message]
2019-04-09 14:03   ` Florian Westphal
2019-04-09 23:19     ` Pablo Neira Ayuso
2019-04-10 13:37       ` Phil Sutter

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20190409135925.GQ4851@orbyte.nwl.cc \
    --to=phil@nwl.cc \
    --cc=arturo@netfilter.org \
    --cc=fw@strlen.de \
    --cc=netfilter-devel@vger.kernel.org \
    --cc=pablo@netfilter.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.