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:
table auto-sizing
multiple locks
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