From: Phil Sutter <phil@nwl.cc>
To: Pablo Neira Ayuso <pablo@netfilter.org>
Cc: netfilter-devel@vger.kernel.org, Jeff Kletsky <netfilter@allycomm.com>
Subject: Re: [nft PATCH] Enable automerge feature for anonymous sets
Date: Wed, 7 Feb 2018 10:53:36 +0100 [thread overview]
Message-ID: <20180207095336.GK2285@orbyte.nwl.cc> (raw)
In-Reply-To: <20180206233943.6shg7qeg2mg6u2gg@salvia>
Hi Pablo,
On Wed, Feb 07, 2018 at 12:39:43AM +0100, Pablo Neira Ayuso wrote:
> On Tue, Feb 06, 2018 at 07:18:47PM +0100, Phil Sutter wrote:
> > Automatic merging of adjacent/overlapping ranges upon insertion has
> > clear benefits performance- and readability-wise. The drawbacks which
> > led to disabling it by default don't apply to anonymous sets since they
> > are read-only anyway, so enable this feature for them again.
>
> Question is, why someone would be adding elements with overlapping
> ranges in an anonymous set? Then, when listing the ruleset you will
> get something different to what you've added. This would also be
> inconsistent with regards to the existing behaviour in named sets,
> where this is turned off by default.
Recapping the pros and cons of automatic merging the discussion brought
up (I probably miss some):
+ Overlaps may be hard to spot (and avoid) if they sit in different
defines used to fill a single set.
- Items added to a set can't be removed again if they were merged with
others added at the same time.
>From those, I assume that if one uses defines to manage e.g. white/black
lists, a named set is probably not used either. The con above applies
only to named sets since anonymous ones can't be changed. Therefore I
considered automatic merging happening in anonymous sets to be
desirable.
> For named sets, that are useful to maintain white/blacklists, I
> understand this simplifies complexity for people dealing with them.
> But not sure for anonymous sets.
I'm not entirely sure about that since there are multiple ways to see
things. AFAIR, Jeff's point was when he combines set elements from
different sources he doesn't want to check them manually for potential
overlaps (or even can't). So there automatic merging is a good thing. If
my fail2ban script can't delete a range previously added after it
timeouts, it fails and recovery is not trivial (find the merged element
and divide it again). Here, merging is a bad thing. Overall though, I
wonder whether using interval sets for white/black lists isn't a bad
idea at all. :)
> @Jeff: Is this also useful to you in the anonymous set use-case? IIRC
> we agreed that this was good for named sets, but not for anonymous
> sets.
I'm curious to hearing his thoughts as well.
Cheers, Phil
next prev parent reply other threads:[~2018-02-07 9:53 UTC|newest]
Thread overview: 5+ messages / expand[flat|nested] mbox.gz Atom feed top
2018-02-06 18:18 [nft PATCH] Enable automerge feature for anonymous sets Phil Sutter
2018-02-06 23:39 ` Pablo Neira Ayuso
2018-02-07 9:51 ` Jozsef Kadlecsik
2018-02-07 9:53 ` Phil Sutter [this message]
2018-02-15 15:28 ` Pablo Neira Ayuso
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=20180207095336.GK2285@orbyte.nwl.cc \
--to=phil@nwl.cc \
--cc=netfilter-devel@vger.kernel.org \
--cc=netfilter@allycomm.com \
--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 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).