netdev.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [stable 2.6.32.y] net/ipv4: fix cached ipv4 dsts never invalidated
@ 2012-10-16 20:41 Benjamin LaHaise
  2012-10-16 20:45 ` David Miller
  0 siblings, 1 reply; 4+ messages in thread
From: Benjamin LaHaise @ 2012-10-16 20:41 UTC (permalink / raw)
  To: Willy Tarreau; +Cc: stable, netdev

From: Timo Teras <timo.teras@iki.fi>
commit d11a4dc18bf41719c9f0d7ed494d295dd2973b92
Author: Timo Teräs <timo.teras@iki.fi>
Date:   Thu Mar 18 23:20:20 2010 +0000

    ipv4: check rt_genid in dst_check
    
    Xfrm_dst keeps a reference to ipv4 rtable entries on each
    cached bundle. The only way to renew xfrm_dst when the underlying
    route has changed, is to implement dst_check for this. This is
    what ipv6 side does too.
    
    The problems started after 87c1e12b5eeb7b30b4b41291bef8e0b41fc3dde9
    ("ipsec: Fix bogus bundle flowi") which fixed a bug causing xfrm_dst
    to not get reused, until that all lookups always generated new
    xfrm_dst with new route reference and path mtu worked. But after the
    fix, the old routes started to get reused even after they were expired
    causing pmtu to break (well it would occationally work if the rtable
    gc had run recently and marked the route obsolete causing dst_check to
    get called).
    
    Signed-off-by: Timo Teras <timo.teras@iki.fi>
    Acked-by: Herbert Xu <herbert@gondor.apana.org.au>
    Signed-off-by: David S. Miller <davem@davemloft.net>
---
Hi Willy,

Can you please consider applying d11a4dc18bf41719c9f0d7ed494d295dd2973b92 
to 2.6.32.y?  This fixes an issue with cached IPv4 routes never being 
invalidated.  For more details of the problem this causes, see 
http://marc.info/?l=linux-netdev&m=135015076708950&w=2 .  Thanks!

		-ben

Signed-off-by: Benjamin LaHaise <bcrl@kvack.org>

diff --git a/net/ipv4/route.c b/net/ipv4/route.c
index a770df2..32d3961 100644
--- a/net/ipv4/route.c
+++ b/net/ipv4/route.c
@@ -1441,7 +1441,7 @@ void ip_rt_redirect(__be32 old_gw, __be32 daddr, __be32 new_gw,
 					dev_hold(rt->u.dst.dev);
 				if (rt->idev)
 					in_dev_hold(rt->idev);
-				rt->u.dst.obsolete	= 0;
+				rt->u.dst.obsolete	= -1;
 				rt->u.dst.lastuse	= jiffies;
 				rt->u.dst.path		= &rt->u.dst;
 				rt->u.dst.neighbour	= NULL;
@@ -1506,7 +1506,7 @@ static struct dst_entry *ipv4_negative_advice(struct dst_entry *dst)
 	struct dst_entry *ret = dst;
 
 	if (rt) {
-		if (dst->obsolete) {
+		if (dst->obsolete > 0) {
 			ip_rt_put(rt);
 			ret = NULL;
 		} else if ((rt->rt_flags & RTCF_REDIRECTED) ||
@@ -1726,7 +1726,9 @@ static void ip_rt_update_pmtu(struct dst_entry *dst, u32 mtu)
 
 static struct dst_entry *ipv4_dst_check(struct dst_entry *dst, u32 cookie)
 {
-	return NULL;
+	if (rt_is_expired((struct rtable *)dst))
+		return NULL;
+	return dst;
 }
 
 static void ipv4_dst_destroy(struct dst_entry *dst)
@@ -1888,7 +1890,8 @@ static int ip_route_input_mc(struct sk_buff *skb, __be32 daddr, __be32 saddr,
 	if (!rth)
 		goto e_nobufs;
 
-	rth->u.dst.output= ip_rt_bug;
+	rth->u.dst.output = ip_rt_bug;
+	rth->u.dst.obsolete = -1;
 
 	atomic_set(&rth->u.dst.__refcnt, 1);
 	rth->u.dst.flags= DST_HOST;
@@ -2054,6 +2057,7 @@ static int __mkroute_input(struct sk_buff *skb,
 	rth->fl.oif 	= 0;
 	rth->rt_spec_dst= spec_dst;
 
+	rth->u.dst.obsolete = -1;
 	rth->u.dst.input = ip_forward;
 	rth->u.dst.output = ip_output;
 	rth->rt_genid = rt_genid(dev_net(rth->u.dst.dev));
@@ -2218,6 +2222,7 @@ local_input:
 		goto e_nobufs;
 
 	rth->u.dst.output= ip_rt_bug;
+	rth->u.dst.obsolete = -1;
 	rth->rt_genid = rt_genid(net);
 
 	atomic_set(&rth->u.dst.__refcnt, 1);
@@ -2444,6 +2449,7 @@ static int __mkroute_output(struct rtable **result,
 	rth->rt_spec_dst= fl->fl4_src;
 
 	rth->u.dst.output=ip_output;
+	rth->u.dst.obsolete = -1;
 	rth->rt_genid = rt_genid(dev_net(dev_out));
 
 	RT_CACHE_STAT_INC(out_slow_tot);
-- 
"Thought is the essence of where you are now."

^ permalink raw reply related	[flat|nested] 4+ messages in thread

* Re: [stable 2.6.32.y] net/ipv4: fix cached ipv4 dsts never invalidated
  2012-10-16 20:41 [stable 2.6.32.y] net/ipv4: fix cached ipv4 dsts never invalidated Benjamin LaHaise
@ 2012-10-16 20:45 ` David Miller
  2012-10-16 20:49   ` Benjamin LaHaise
  0 siblings, 1 reply; 4+ messages in thread
From: David Miller @ 2012-10-16 20:45 UTC (permalink / raw)
  To: bcrl; +Cc: w, stable, netdev

From: Benjamin LaHaise <bcrl@kvack.org>
Date: Tue, 16 Oct 2012 16:41:04 -0400

> From: Timo Teras <timo.teras@iki.fi>
> commit d11a4dc18bf41719c9f0d7ed494d295dd2973b92
> Author: Timo Teräs <timo.teras@iki.fi>
> Date:   Thu Mar 18 23:20:20 2010 +0000
> 
>     ipv4: check rt_genid in dst_check
>     
>     Xfrm_dst keeps a reference to ipv4 rtable entries on each
>     cached bundle. The only way to renew xfrm_dst when the underlying
>     route has changed, is to implement dst_check for this. This is
>     what ipv6 side does too.
>     
>     The problems started after 87c1e12b5eeb7b30b4b41291bef8e0b41fc3dde9
>     ("ipsec: Fix bogus bundle flowi") which fixed a bug causing xfrm_dst
>     to not get reused, until that all lookups always generated new
>     xfrm_dst with new route reference and path mtu worked. But after the
>     fix, the old routes started to get reused even after they were expired
>     causing pmtu to break (well it would occationally work if the rtable
>     gc had run recently and marked the route obsolete causing dst_check to
>     get called).
>     
>     Signed-off-by: Timo Teras <timo.teras@iki.fi>
>     Acked-by: Herbert Xu <herbert@gondor.apana.org.au>
>     Signed-off-by: David S. Miller <davem@davemloft.net>
> ---
> Hi Willy,
> 
> Can you please consider applying d11a4dc18bf41719c9f0d7ed494d295dd2973b92 
> to 2.6.32.y?  This fixes an issue with cached IPv4 routes never being 
> invalidated.  For more details of the problem this causes, see 
> http://marc.info/?l=linux-netdev&m=135015076708950&w=2 .  Thanks!

This is only a partial fix.

Only recently did this issue get resolved completely.

See commits surrounding:

From b42664f898c976247f7f609b8bb9c94d7475ca10 Mon Sep 17 00:00:00 2001
From: Nicolas Dichtel <nicolas.dichtel@6wind.com>
Date: Mon, 10 Sep 2012 22:09:44 +0000
Subject: [PATCH] netns: move net->ipv4.rt_genid to net->rt_genid

This commit prepares the use of rt_genid by both IPv4 and IPv6.
Initialization is left in IPv4 part.

Signed-off-by: Nicolas Dichtel <nicolas.dichtel@6wind.com>
Signed-off-by: David S. Miller <davem@davemloft.net>
---
 include/net/net_namespace.h | 10 ++++++++++
 include/net/netns/ipv4.h    |  1 -
 net/ipv4/route.c            |  9 ++-------
 3 files changed, 12 insertions(+), 8 deletions(-)

diff --git a/include/net/net_namespace.h b/include/net/net_namespace.h
index ae1cd6c..fd87963 100644
--- a/include/net/net_namespace.h
+++ b/include/net/net_namespace.h
@@ -102,6 +102,7 @@ struct net {
 #endif
 	struct netns_ipvs	*ipvs;
 	struct sock		*diag_nlsk;
+	atomic_t		rt_genid;
 };
 
 
@@ -300,5 +301,14 @@ static inline void unregister_net_sysctl_table(struct ctl_table_header *header)
 }
 #endif
 
+static inline int rt_genid(struct net *net)
+{
+	return atomic_read(&net->rt_genid);
+}
+
+static inline void rt_genid_bump(struct net *net)
+{
+	atomic_inc(&net->rt_genid);
+}
 
 #endif /* __NET_NET_NAMESPACE_H */
diff --git a/include/net/netns/ipv4.h b/include/net/netns/ipv4.h
index 1474dd6..eb24dbc 100644
--- a/include/net/netns/ipv4.h
+++ b/include/net/netns/ipv4.h
@@ -65,7 +65,6 @@ struct netns_ipv4 {
 	unsigned int sysctl_ping_group_range[2];
 	long sysctl_tcp_mem[3];
 
-	atomic_t rt_genid;
 	atomic_t dev_addr_genid;
 
 #ifdef CONFIG_IP_MROUTE
diff --git a/net/ipv4/route.c b/net/ipv4/route.c
index be27cfa9..fd9af60 100644
--- a/net/ipv4/route.c
+++ b/net/ipv4/route.c
@@ -202,11 +202,6 @@ EXPORT_SYMBOL(ip_tos2prio);
 static DEFINE_PER_CPU(struct rt_cache_stat, rt_cache_stat);
 #define RT_CACHE_STAT_INC(field) __this_cpu_inc(rt_cache_stat.field)
 
-static inline int rt_genid(struct net *net)
-{
-	return atomic_read(&net->ipv4.rt_genid);
-}
-
 #ifdef CONFIG_PROC_FS
 static void *rt_cache_seq_start(struct seq_file *seq, loff_t *pos)
 {
@@ -449,7 +444,7 @@ static inline bool rt_is_expired(const struct rtable *rth)
 
 void rt_cache_flush(struct net *net)
 {
-	atomic_inc(&net->ipv4.rt_genid);
+	rt_genid_bump(net);
 }
 
 static struct neighbour *ipv4_neigh_lookup(const struct dst_entry *dst,
@@ -2506,7 +2501,7 @@ static __net_initdata struct pernet_operations sysctl_route_ops = {
 
 static __net_init int rt_genid_init(struct net *net)
 {
-	atomic_set(&net->ipv4.rt_genid, 0);
+	atomic_set(&net->rt_genid, 0);
 	get_random_bytes(&net->ipv4.dev_addr_genid,
 			 sizeof(net->ipv4.dev_addr_genid));
 	return 0;
-- 
1.7.11.7

^ permalink raw reply related	[flat|nested] 4+ messages in thread

* Re: [stable 2.6.32.y] net/ipv4: fix cached ipv4 dsts never invalidated
  2012-10-16 20:45 ` David Miller
@ 2012-10-16 20:49   ` Benjamin LaHaise
  2012-10-16 21:22     ` Willy Tarreau
  0 siblings, 1 reply; 4+ messages in thread
From: Benjamin LaHaise @ 2012-10-16 20:49 UTC (permalink / raw)
  To: David Miller; +Cc: w, stable, netdev

On Tue, Oct 16, 2012 at 04:45:39PM -0400, David Miller wrote:
> This is only a partial fix.
> 
> Only recently did this issue get resolved completely.
> 
> See commits surrounding:

Thanks for the pointers David.  I'll cherry pick / backport the relevant 
changes for Willy as well.

		-ben
-- 
"Thought is the essence of where you are now."

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [stable 2.6.32.y] net/ipv4: fix cached ipv4 dsts never invalidated
  2012-10-16 20:49   ` Benjamin LaHaise
@ 2012-10-16 21:22     ` Willy Tarreau
  0 siblings, 0 replies; 4+ messages in thread
From: Willy Tarreau @ 2012-10-16 21:22 UTC (permalink / raw)
  To: Benjamin LaHaise; +Cc: David Miller, stable, netdev

On Tue, Oct 16, 2012 at 04:49:55PM -0400, Benjamin LaHaise wrote:
> On Tue, Oct 16, 2012 at 04:45:39PM -0400, David Miller wrote:
> > This is only a partial fix.
> > 
> > Only recently did this issue get resolved completely.
> > 
> > See commits surrounding:
> 
> Thanks for the pointers David.  I'll cherry pick / backport the relevant 
> changes for Willy as well.

They will be much welcome, thank you guys !

Willy

^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2012-10-16 21:24 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2012-10-16 20:41 [stable 2.6.32.y] net/ipv4: fix cached ipv4 dsts never invalidated Benjamin LaHaise
2012-10-16 20:45 ` David Miller
2012-10-16 20:49   ` Benjamin LaHaise
2012-10-16 21:22     ` Willy Tarreau

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).