NACK/Cmnt: [SRU][v2][J][PATCH] netfilter: conntrack: adopt safer max chain length

[Khalid Elmously]

On 2023-03-15 09:33:07 , Stefan Bader wrote:
> On 14.03.23 19:21, Khalid Elmously wrote:
> > From: Eric Dumazet <edumazet at google.com>
> > 
> > BugLink: https://bugs.launchpad.net/bug/2011616
> 
> The bug reference leads nowhere for me. But also, this change appears to be
> a v6.3-rc2. So it should not _only_ be applied to Jammy but also to Kinetic,
> Lunar. Maybe OEM kernels as well but I admit its hard to keep track of them.
> 

Apologies about the bad link.
Yes good catch about K/L - saved us from a future regression.


> -Stefan
> 
> > 
> > Customers using GKE 1.25 and 1.26 are facing conntrack issues
> > root caused to commit c9c3b6811f74 ("netfilter: conntrack: make
> > max chain length random").
> > 
> > Even if we assume Uniform Hashing, a bucket often reachs 8 chained
> > items while the load factor of the hash table is smaller than 0.5
> > 
> > With a limit of 16, we reach load factors of 3.
> > With a limit of 32, we reach load factors of 11.
> > With a limit of 40, we reach load factors of 15.
> > With a limit of 50, we reach load factors of 24.
> > 
> > This patch changes MIN_CHAINLEN to 50, to minimize risks.
> > 
> > Ideally, we could in the future add a cushion based on expected
> > load factor (2 * nf_conntrack_max / nf_conntrack_buckets),
> > because some setups might expect unusual values.
> > 
> > Fixes: c9c3b6811f74 ("netfilter: conntrack: make max chain length random")
> > Signed-off-by: Eric Dumazet <edumazet at google.com>
> > Signed-off-by: Pablo Neira Ayuso <pablo at netfilter.org>
> > (cherry picked from commit c77737b736ceb50fdf150434347dbd81ec76dbb1)
> > Signed-off-by: Khalid Elmously <khalid.elmously at canonical.com>
> > ---
> >   net/netfilter/nf_conntrack_core.c | 4 ++--
> >   1 file changed, 2 insertions(+), 2 deletions(-)
> > 
> > diff --git a/net/netfilter/nf_conntrack_core.c b/net/netfilter/nf_conntrack_core.c
> > index 43ea8cfd374bb..7ff0da5f998a0 100644
> > --- a/net/netfilter/nf_conntrack_core.c
> > +++ b/net/netfilter/nf_conntrack_core.c
> > @@ -96,8 +96,8 @@ static DEFINE_MUTEX(nf_conntrack_mutex);
> >   #define GC_SCAN_MAX_DURATION	msecs_to_jiffies(10)
> >   #define GC_SCAN_EXPIRED_MAX	(64000u / HZ)
> > -#define MIN_CHAINLEN	8u
> > -#define MAX_CHAINLEN	(32u - MIN_CHAINLEN)
> > +#define MIN_CHAINLEN	50u
> > +#define MAX_CHAINLEN	(80u - MIN_CHAINLEN)
> >   static struct conntrack_gc_work conntrack_gc_work;
> 
> -- 
> - Stefan
>