From mboxrd@z Thu Jan 1 00:00:00 1970 From: Nicolas Cavallari Subject: Re: [PATCHv2] ipv4: Do not cache routing failures due to disabled forwarding. Date: Tue, 23 Sep 2014 10:34:08 +0200 Message-ID: <54213080.9000803@green-communications.fr> References: <54143FB2.9000600@green-communications.fr> <1410776893-5284-1-git-send-email-nicolas.cavallari@green-communications.fr> <20140916.145435.714034191389495557.davem@davemloft.net> <541894F6.3090903@green-communications.fr> Mime-Version: 1.0 Content-Type: text/plain; charset=windows-1252 Content-Transfer-Encoding: 7bit Cc: netdev@vger.kernel.org, kuznet@ms2.inr.ac.ru, jmorris@namei.org, yoshfuji@linux-ipv6.org, kaber@trash.net To: David Miller Return-path: Received: from mout.kundenserver.de ([212.227.126.187]:64112 "EHLO mout.kundenserver.de" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1753541AbaIWIe7 (ORCPT ); Tue, 23 Sep 2014 04:34:59 -0400 In-Reply-To: <541894F6.3090903@green-communications.fr> Sender: netdev-owner@vger.kernel.org List-ID: On 16/09/2014 21:52, Nicolas Cavallari wrote: > On 16/09/2014 20:54, David Miller wrote: >> From: Nicolas Cavallari >> Date: Mon, 15 Sep 2014 12:28:13 +0200 >> >>> If we cache them, the kernel will reuse them, independently of >>> whether forwarding is enabled or not. Which means that if forwarding is >>> disabled on the input interface where the first routing request comes >>> from, then that unreachable result will be cached and reused for >>> other interfaces, even if forwarding is enabled on them. >>> >>> This can be verified with two interfaces A and B and an output interface >>> C, where B has forwarding enabled, but not A and trying >>> ip route get $dst iif A from $src && ip route get $dst iif B from $src >>> >>> Signed-off-by: Nicolas Cavallari >>> --- >>> v2: simplify patch using julian anastasov's suggestion. >> >> This also disables caching for the cases of a simple fib lookup failure. > > Caching is already not done on a fib lookup failure. On which fib_info > would you cache anyway ? res.fi is already NULL in this case. > >> Handle cached route invalidation the way it's meant to be, by bumping >> the rt_genid. >> >> diff --git a/net/ipv4/devinet.c b/net/ipv4/devinet.c >> index 214882e..aa4e63c 100644 >> --- a/net/ipv4/devinet.c >> +++ b/net/ipv4/devinet.c >> @@ -1965,6 +1965,8 @@ static void inet_forward_change(struct net *net) >> } >> rcu_read_unlock(); >> } >> + >> + rt_cache_flush(net); >> } >> >> static int devinet_conf_ifindex(struct net *net, struct ipv4_devconf *cnf) > > This doesn't solve the problem. Flushing the cache only changes how the > bug manifests: > > ip route flush cache > ip route get 10.0.0.1 from 10.0.0.2 iif A > # unreachable (because forwarding on A is disabled). This is cached. > ip route get 10.0.0.1 from 10.0.0.2 iif B > # unreachable (at first fib_lookup finds it reachable and forwarding on > B is enabled. And then there is a valid cache entry, which is reused > blindly even if it says "unreachable"). > > ip route flush cache > ip route get 10.0.0.1 from 10.0.0.2 iif B > # reachable (because forwarding on B is enabled). This is cached > ip route get 10.0.0.1 from 10.0.0.2 iif A > # reachable (forwarding on A is disabled, so it is unreachable > initially. But there is a valid cache entry, which says "reachable", > which is reused) Any news on this patch ? Is there anything I can do so that this bug is finally fixed ?