From mboxrd@z Thu Jan 1 00:00:00 1970 From: Nicolas Dichtel Subject: Re: [PATCH net] conntrack: perform a full scan in gc Date: Thu, 20 Oct 2016 10:50:28 +0200 Message-ID: <28501df9-1ca3-a0a4-f897-ef906f8a5f09@6wind.com> References: <20161014105327.GA8655@salvia> <1476779435-30503-1-git-send-email-nicolas.dichtel@6wind.com> <20161018084718.GB29405@breakpoint.cc> <34d607cc-5556-89c7-673d-be8cb0a4758a@6wind.com> Reply-To: nicolas.dichtel@6wind.com Mime-Version: 1.0 Content-Type: text/plain; charset=windows-1252 Content-Transfer-Encoding: 8bit Cc: pablo@netfilter.org, davem@davemloft.net, netdev@vger.kernel.org, netfilter-devel@vger.kernel.org To: Florian Westphal Return-path: In-Reply-To: <34d607cc-5556-89c7-673d-be8cb0a4758a@6wind.com> Sender: netfilter-devel-owner@vger.kernel.org List-Id: netdev.vger.kernel.org Le 18/10/2016 à 12:06, Nicolas Dichtel a écrit : > Le 18/10/2016 à 10:47, Florian Westphal a écrit : >> Nicolas Dichtel 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 >>> Signed-off-by: Nicolas Dichtel >>> --- >>> >>> 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. > Yes, I was wondering about that. My first idea was to have that bool per bucket > and force a scan of the bucket instead of the whole table. FYI, I'am off for about two weeks, but we really need to find a way to fix that before the release goes out.