Netfilter-Devel Archive on lore.kernel.org
 help / color / Atom feed
From: Pablo Neira Ayuso <pablo@netfilter.org>
To: Phil Sutter <phil@nwl.cc>
Cc: netfilter-devel@vger.kernel.org
Subject: Re: [iptables PATCH v3 04/11] nft-cache: Introduce cache levels
Date: Wed, 9 Oct 2019 12:29:01 +0200
Message-ID: <20191009102901.6kel2u36u3yv4myu@salvia> (raw)
In-Reply-To: <20191009093723.snbyd6xvtd5gpnto@salvia>

On Wed, Oct 09, 2019 at 11:37:23AM +0200, Pablo Neira Ayuso wrote:
> Hi Phil,
> 
> On Tue, Oct 08, 2019 at 06:14:40PM +0200, Phil Sutter wrote:
> > Replace the simple have_cache boolean by a cache level indicator
> > defining how complete the cache is. Since have_cache indicated full
> > cache (including rules), make code depending on it check for cache level
> > NFT_CL_RULES.
> > 
> > Core cache fetching routine __nft_build_cache() accepts a new level via
> > parameter and raises cache completeness to that level.
> > 
> > Signed-off-by: Phil Sutter <phil@nwl.cc>
> > ---
> >  iptables/nft-cache.c | 51 +++++++++++++++++++++++++++++++-------------
> >  iptables/nft.h       |  9 +++++++-
> >  2 files changed, 44 insertions(+), 16 deletions(-)
> > 
> > diff --git a/iptables/nft-cache.c b/iptables/nft-cache.c
> > index 5444419a5cc3b..22a87e94efd76 100644
> > --- a/iptables/nft-cache.c
> > +++ b/iptables/nft-cache.c
> > @@ -224,30 +224,49 @@ static int fetch_rule_cache(struct nft_handle *h)
> >  	return 0;
> >  }
> >  
> > -static void __nft_build_cache(struct nft_handle *h)
> > +static void __nft_build_cache(struct nft_handle *h, enum nft_cache_level level)
> >  {
> >  	uint32_t genid_start, genid_stop;
> >  
> > +	if (level <= h->cache_level)
> > +		return;
> >  retry:
> >  	mnl_genid_get(h, &genid_start);
> > -	fetch_table_cache(h);
> > -	fetch_chain_cache(h);
> > -	fetch_rule_cache(h);
> > -	h->have_cache = true;
> > -	mnl_genid_get(h, &genid_stop);
> >  
> > +	switch (h->cache_level) {
> > +	case NFT_CL_NONE:
> > +		fetch_table_cache(h);
> > +		if (level == NFT_CL_TABLES)
> > +			break;
> > +		/* fall through */
> > +	case NFT_CL_TABLES:
> 
> If the existing level is TABLES and use wants chains, then you have to
> invalidate the existing table cache, then fetch the tables and chains
> to make sure cache is consistent. I mean, extending an existing cache
> might lead to inconsistencies.
> 
> Am I missing anything?

If I'm correct, I wonder if we should go for splitting the parsing
from the evaluation phase here. Probably generate the rule by pointing
to the table and chain as string, then evaluate the ruleset update
batch to obtain the cache level in one go. This is the approach that
I had in mind with nftables, so you might avoid dumping the ruleset
over and over in an environment where dynamic updates are frequent.

The idea is to avoid fetching a cache, then canceling it by the rule
coming afterwards just because the cache is incomplete. So the cache
that is required is calculated once, then you go to the kernel and
fetch it (making sure generation number tells you that your cache is
consistent).

Makes sense to you?

  reply index

Thread overview: 22+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-10-08 16:14 [iptables PATCH v3 00/11] Improve iptables-nft performance with large rulesets Phil Sutter
2019-10-08 16:14 ` [iptables PATCH v3 01/11] nft: Pass nft_handle to flush_cache() Phil Sutter
2019-10-09  9:30   ` Pablo Neira Ayuso
2019-10-08 16:14 ` [iptables PATCH v3 02/11] nft: Avoid nested cache fetching Phil Sutter
2019-10-09  9:30   ` Pablo Neira Ayuso
2019-10-08 16:14 ` [iptables PATCH v3 03/11] nft: Extract cache routines into nft-cache.c Phil Sutter
2019-10-09  9:32   ` Pablo Neira Ayuso
2019-10-08 16:14 ` [iptables PATCH v3 04/11] nft-cache: Introduce cache levels Phil Sutter
2019-10-09  9:37   ` Pablo Neira Ayuso
2019-10-09 10:29     ` Pablo Neira Ayuso [this message]
2019-10-10 22:09       ` Phil Sutter
2019-10-11  9:28         ` Pablo Neira Ayuso
2019-10-11 11:24           ` Phil Sutter
2019-10-14 10:00             ` Pablo Neira Ayuso
2019-10-11 10:20         ` Pablo Neira Ayuso
2019-10-08 16:14 ` [iptables PATCH v3 05/11] nft-cache: Fetch only chains in nft_chain_list_get() Phil Sutter
2019-10-08 16:14 ` [iptables PATCH v3 06/11] nft-cache: Cover for multiple fetcher invocation Phil Sutter
2019-10-08 16:14 ` [iptables PATCH v3 07/11] nft-cache: Support partial cache per table Phil Sutter
2019-10-08 16:14 ` [iptables PATCH v3 08/11] nft-cache: Support partial rule cache per chain Phil Sutter
2019-10-08 16:14 ` [iptables PATCH v3 09/11] nft: Reduce cache overhead of nft_chain_builtin_init() Phil Sutter
2019-10-08 16:14 ` [iptables PATCH v3 10/11] nft: Support nft_is_table_compatible() per chain Phil Sutter
2019-10-08 16:14 ` [iptables PATCH v3 11/11] nft: Optimize flushing all chains of a table Phil Sutter

Reply instructions:

You may reply publically 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=20191009102901.6kel2u36u3yv4myu@salvia \
    --to=pablo@netfilter.org \
    --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

Netfilter-Devel Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/netfilter-devel/0 netfilter-devel/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 netfilter-devel netfilter-devel/ https://lore.kernel.org/netfilter-devel \
		netfilter-devel@vger.kernel.org
	public-inbox-index netfilter-devel

Example config snippet for mirrors

Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.kernel.vger.netfilter-devel


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git