Re: [PATCH 4.9.y] netfilter: nat: Revert "netfilter: nat: convert nat bysrc hash to rhashtable"

[Greg KH <greg@kroah.com>]

On Sun, Nov 12, 2017 at 09:23:47PM +0100, Florian Westphal wrote:
> commit e1bf1687740ce1a3598a1c5e452b852ff2190682 upstream.
> 
> This reverts commit 870190a9ec9075205c0fa795a09fa931694a3ff1.
> 
> It was not a good idea. The custom hash table was a much better
> fit for this purpose.
> 
> A fast lookup is not essential, in fact for most cases there is no lookup
> at all because original tuple is not taken and can be used as-is.
> What needs to be fast is insertion and deletion.
> 
> rhlist removal however requires a rhlist walk.
> We can have thousands of entries in such a list if source port/addresses
> are reused for multiple flows, if this happens removal requests are so
> expensive that deletions of a few thousand flows can take several
> seconds(!).
> 
> The advantages that we got from rhashtable are:
> 1) table auto-sizing
> 2) multiple locks
> 
> 1) would be nice to have, but it is not essential as we have at
> most one lookup per new flow, so even a million flows in the bysource
> table are not a problem compared to current deletion cost.
> 2) is easy to add to custom hash table.
> 
> I tried to add hlist_node to rhlist to speed up rhltable_remove but this
> isn't doable without changing semantics.  rhltable_remove_fast will
> check that the to-be-deleted object is part of the table and that
> requires a list walk that we want to avoid.
> 
> Furthermore, using hlist_node increases size of struct rhlist_head, which
> in turn increases nf_conn size.
> 
> Link: https://bugzilla.kernel.org/show_bug.cgi?id=196821
> Reported-by: Ivan Babrou <ibobrik@gmail.com>
> Signed-off-by: Florian Westphal <fw@strlen.de>
> Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
> ---
>  Hi Greg,
> 
>  this is a 4.9.y backport of the same revert that already went into 4.13.y;
>  please consider applying this.  I briefly tested this in kvm, at very least
>  it doesn't break nat redirect in obvious ways...

Very nice, thanks for this, now queued up.

greg k-h