netfilter-devel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Pablo Neira Ayuso <pablo@netfilter.org>
To: Phil Sutter <phil@nwl.cc>, Florian Westphal <fw@strlen.de>,
	netfilter-devel@vger.kernel.org
Subject: Re: [nft PATCH] List handles of added rules if requested
Date: Fri, 5 May 2017 12:49:54 +0200	[thread overview]
Message-ID: <20170505104954.GA1676@salvia> (raw)
In-Reply-To: <20170504222612.GD20805@orbyte.nwl.cc>

On Fri, May 05, 2017 at 12:26:12AM +0200, Phil Sutter wrote:
> On Thu, May 04, 2017 at 04:00:42PM +0200, Pablo Neira Ayuso wrote:
> > On Thu, May 04, 2017 at 03:44:19PM +0200, Florian Westphal wrote:
> > > Pablo Neira Ayuso <pablo@netfilter.org> wrote:
> > > > On Thu, May 04, 2017 at 02:34:21PM +0200, Phil Sutter wrote:
> > > > > Being able to retrieve an added rule's handle atomically is a crucial
> > > > > feature for scripts invoking nft command: Without it, there is no way to
> > > > > be sure a handle extracted from 'nft list ruleset' command actually
> > > > > refers to the rule one has added before or that of another process which
> > > > > ran in between.
> > > > > 
> > > > > Extracting an added rule's handle itself is not an easy task already,
> > > > > since there is a chance that a given rule is printed differently than
> > > > > when it was added before. A simple example is port number vs. service
> > > > > name:
> > > > > 
> > > > > | nft add rule ip t c tcp dport { ssh, 80 } accept
> > > > > 
> > > > > There is no way to make 'nft list ruleset' return the rule just like
> > > > > this as depending on whether '-nn' was given or not, it either prints
> > > > > the set as '{ ssh, http }' or '{ 22, 80 }' but never in the mixed form
> > > > > that was used when adding it.
> > > > > 
> > > > > This patch prints an identifying string for each added rule which may be
> > > > > used as single parameter to a later 'nft delete rule' command. So a
> > > > > simple scripting example looks like this:
> > > > > 
> > > > > | handle=$(nft add rule ip t c counter)
> > > > 
> > > > This is a hack.
> > > > 
> > > > We should follow the rule description path.
> > > 
> > > You mean delete-by-name?
> > > 
> > > Its just as ugly, just a different kind of ugly.
> > 
> > Ugly?
> > 
> > This kernel patch is seriouly broken. It's sending a message to
> > userspace from the preparation phase of the commit protocol, where
> > things are not even confirmed at all...
> 
> Oh, I indeed ignored the transactional behaviour at all. But looking at
> the code, if I use NLM_F_ECHO that should be fine, right?
> 
> > > Will you delete the first match?  The last one?  All of them?
> > 
> > I already explained this Florian. Please, look at the mail archive.
> 
> While I think it's not a bad idea to allow users experienced with
> iptables to apply their muscle memory to nftables as well, I don't quite
> get what should hold us back from leveraging this feature nftables
> provides over iptables. The existence of a unique identifier is a big
> plus in my point of view, it's just not really useful yet since users
> have no safe way to get that handle for the rule they added.
>
> Are you OK with providing both alternatives in parallel? If not, why?

This does not integrate at all into the scripting features we have in
nftables. We don't want people to use bash (or like) shell scripts
anymore, they are bad, they break atomicity for us. We should extend
native nftables scripting capabilities to support what user need,
natively. Look, this will not work with nft -i either...

And this also will not work for robots using incremental updates via
nft -f. And we very much want to support such transaction like scheme,
ie. place a bunch of incremental updates in one single file and apply
that in one single transaction.

This is just covering one very specific usecase, that is, users have a
quick way to delete the rule that just added. And we have better ways
to achieve this, and that will work from all the scenarios that I
described above.

  reply	other threads:[~2017-05-05 10:50 UTC|newest]

Thread overview: 11+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2017-05-04 12:34 [nft PATCH] List handles of added rules if requested Phil Sutter
2017-05-04 13:36 ` Pablo Neira Ayuso
2017-05-04 13:44   ` Florian Westphal
2017-05-04 14:00     ` Pablo Neira Ayuso
2017-05-04 22:26       ` Phil Sutter
2017-05-05 10:49         ` Pablo Neira Ayuso [this message]
2017-05-05 11:18           ` Puustinen, Ismo
2017-05-08 17:35             ` Pablo Neira Ayuso
2017-05-05 11:56           ` Florian Westphal
2017-05-04 15:37     ` Thomas Woerner
2017-05-04 22:38       ` 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=20170505104954.GA1676@salvia \
    --to=pablo@netfilter.org \
    --cc=fw@strlen.de \
    --cc=netfilter-devel@vger.kernel.org \
    --cc=phil@nwl.cc \
    /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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).