From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from out4-smtp.messagingengine.com ([66.111.4.28]:54323 "EHLO out4-smtp.messagingengine.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751836AbdKMIgm (ORCPT ); Mon, 13 Nov 2017 03:36:42 -0500 Date: Mon, 13 Nov 2017 09:36:49 +0100 From: Greg KH To: Florian Westphal Cc: stable@vger.kernel.org, g.nault@alphalink.fr, pablo@netfilter.org Subject: Re: [PATCH 4.9.y] netfilter: nat: Revert "netfilter: nat: convert nat bysrc hash to rhashtable" Message-ID: <20171113083649.GC22719@kroah.com> References: <20171112202347.14762-1-fw@strlen.de> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20171112202347.14762-1-fw@strlen.de> Sender: stable-owner@vger.kernel.org List-ID: 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 > Signed-off-by: Florian Westphal > Signed-off-by: Pablo Neira Ayuso > --- > 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