All of lore.kernel.org
 help / color / mirror / Atom feed
From: Florian Westphal <fw@strlen.de>
To: Nicolas Dichtel <nicolas.dichtel@6wind.com>
Cc: pablo@netfilter.org, fw@strlen.de, davem@davemloft.net,
	netdev@vger.kernel.org, netfilter-devel@vger.kernel.org
Subject: Re: [PATCH net] conntrack: perform a full scan in gc
Date: Tue, 18 Oct 2016 10:47:18 +0200	[thread overview]
Message-ID: <20161018084718.GB29405@breakpoint.cc> (raw)
In-Reply-To: <1476779435-30503-1-git-send-email-nicolas.dichtel@6wind.com>

Nicolas Dichtel <nicolas.dichtel@6wind.com> wrote:
> After commit b87a2f9199ea ("netfilter: conntrack: add gc worker to remove
> timed-out entries"), netlink conntrack deletion events may be sent with a
> huge delay (5 minutes).
> 
> There is two ways to evict conntrack:
>  - during a conntrack lookup;
>  - during a conntrack dump.
> Let's do a full scan of conntrack entries after a period of inactivity
> (no conntrack lookup).
> 
> CC: Florian Westphal <fw@strlen.de>
> Signed-off-by: Nicolas Dichtel <nicolas.dichtel@6wind.com>
> ---
> 
> Here is another proposal to try to fix the problem.
> Comments are welcomed,
> Nicolas

Hmm, I don't think its good idea in practice.
If goal is to avoid starving arbitrary 'dead' ct for too long,
then simple ping will defeat the logic here, because...

>  net/netfilter/nf_conntrack_core.c | 11 +++++++++--
>  1 file changed, 9 insertions(+), 2 deletions(-)
> 
> diff --git a/net/netfilter/nf_conntrack_core.c b/net/netfilter/nf_conntrack_core.c
> index ba6a1d421222..3dbb27bd9582 100644
> --- a/net/netfilter/nf_conntrack_core.c
> +++ b/net/netfilter/nf_conntrack_core.c
> @@ -87,6 +87,7 @@ static __read_mostly bool nf_conntrack_locks_all;
>  #define GC_MAX_BUCKETS		8192u
>  #define GC_INTERVAL		(5 * HZ)
>  #define GC_MAX_EVICTS		256u
> +static bool gc_full_scan = true;
>  
>  static struct conntrack_gc_work conntrack_gc_work;
>  
> @@ -511,6 +512,7 @@ ____nf_conntrack_find(struct net *net, const struct nf_conntrack_zone *zone,
>  	unsigned int bucket, hsize;
>  
>  begin:
> +	gc_full_scan = false;

... we do periodic lookup (but always in same slot), so no full scan is
triggered.

If you think its useful, consider sending patch that rescheds worker
instantly in case budget expired, otherwise I will do this later this
week.

[ I am aware doing instant restart might be too late, but at least we
  would then reap more entries once we stumble upon large number of
  expired ones ].


  reply	other threads:[~2016-10-18  8:48 UTC|newest]

Thread overview: 19+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-10-10 10:18 [PATCH net 0/2] conntrack update Nicolas Dichtel
2016-10-10 10:18 ` [PATCH net 1/2] conntrack: remove obsolete sysctl (nf_conntrack_events_retry_timeout) Nicolas Dichtel
2016-10-10 13:57   ` Florian Westphal
2016-10-17 15:39     ` Pablo Neira Ayuso
2016-10-10 10:18 ` [PATCH net 2/2] conntrack: enable to tune gc parameters Nicolas Dichtel
2016-10-10 14:04   ` Florian Westphal
2016-10-10 15:24     ` Nicolas Dichtel
2016-10-13 20:43       ` Florian Westphal
2016-10-14 10:12         ` Nicolas Dichtel
2016-10-14 10:37           ` Florian Westphal
2016-10-14 10:53             ` Pablo Neira Ayuso
2016-10-14 11:16               ` Florian Westphal
2016-10-18  8:30               ` [PATCH net] conntrack: perform a full scan in gc Nicolas Dichtel
2016-10-18  8:47                 ` Florian Westphal [this message]
2016-10-18 10:06                   ` Nicolas Dichtel
2016-10-18 12:37                     ` [PATCH net] conntrack: restart gc immediately if GC_MAX_EVICTS is reached Nicolas Dichtel
2016-10-19 16:02                       ` Florian Westphal
2016-10-19 16:14                       ` Pablo Neira Ayuso
2016-10-20  8:50                     ` [PATCH net] conntrack: perform a full scan in gc Nicolas Dichtel

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=20161018084718.GB29405@breakpoint.cc \
    --to=fw@strlen.de \
    --cc=davem@davemloft.net \
    --cc=netdev@vger.kernel.org \
    --cc=netfilter-devel@vger.kernel.org \
    --cc=nicolas.dichtel@6wind.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 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.