Subject: Re: [PATCH 09/14] netfilter: conntrack: refine gc worker heuristics, redux

From: Pablo Neira Ayuso <[email-masked]>
Date: Wed, 1 Mar 2017 16:38:00 +0100

On Wed, Mar 01, 2017 at 04:02:53PM +0100, Nicolas Dichtel wrote:
> Le 27/01/2017 à 17:51, Nicolas Dichtel a écrit :
> > Le 26/01/2017 à 17:38, Pablo Neira Ayuso a écrit :
> >> From: Florian Westphal <[email-masked]>
> >>
> >> This further refines the changes made to conntrack gc_worker in
> >> commit e0df8cae6c16 ("netfilter: conntrack: refine gc worker heuristics").
> >>
> >> The main idea of that change was to reduce the scan interval when evictions
> >> take place.
> >>
> >> However, on the reporters' setup, there are 1-2 million conntrack entries
> >> in total and roughly 8k new (and closing) connections per second.
> >>
> >> In this case we'll always evict at least one entry per gc cycle and scan
> >> interval is always at 1 jiffy because of this test:
> >>
> >>  } else if (expired_count) {
> >>      gc_work->next_gc_run /= 2U;
> >>      next_run = msecs_to_jiffies(1);
> >>
> >> being true almost all the time.
> >>
> >> Given we scan ~10k entries per run its clearly wrong to reduce interval
> >> based on nonzero eviction count, it will only waste cpu cycles since a vast
> >> majorities of conntracks are not timed out.
> >>
> >> Thus only look at the ratio (scanned entries vs. evicted entries) to make
> >> a decision on whether to reduce or not.
> >>
> >> Because evictor is supposed to only kick in when system turns idle after
> >> a busy period, pick a high ratio -- this makes it 50%.  We thus keep
> >> the idea of increasing scan rate when its likely that table contains many
> >> expired entries.
> >>
> >> In order to not let timed-out entries hang around for too long
> >> (important when using event logging, in which case we want to timely
> >> destroy events), we now scan the full table within at most
> >> GC_MAX_SCAN_JIFFIES (16 seconds) even in worst-case scenario where all
> >> timed-out entries sit in same slot.
> >>
> >> I tested this with a vm under synflood (with
> >> sysctl net.netfilter.nf_conntrack_tcp_timeout_syn_recv=3).
> >>
> >> While flood is ongoing, interval now stays at its max rate
> >> (GC_MAX_SCAN_JIFFIES / GC_MAX_BUCKETS_DIV -> 125ms).
> >>
> >> With feedback from Nicolas Dichtel.
> >>
> >> Reported-by: Denys Fedoryshchenko <[email-masked]>
> >> Cc: Nicolas Dichtel <[email-masked]>
> >> Fixes: b87a2f9199ea82eaadc ("netfilter: conntrack: add gc worker to remove timed-out entries")
> >> Signed-off-by: Florian Westphal <[email-masked]>
> >> Tested-by: Nicolas Dichtel <[email-masked]>
> >> Acked-by: Nicolas Dichtel <[email-masked]>
> >> Tested-by: Denys Fedoryshchenko <[email-masked]>
> >> Signed-off-by: Pablo Neira Ayuso <[email-masked]>
> > Pablo, is it possible to queue this patch (and the previous: 08/14) for the 4.9
> > stable?
> 
> Pablo, should I submit it formally?

Just requested this to Greg, sorry this didn't happen so far.

Thanks.