[PATCH] ipv6: Fix problem with expired dst cache

[PATCH] ipv6: Fix problem with expired dst cache

[Gao feng]

if the ipv6 dst cache copy from the dst witch generated by ICMPV6 RA packet.
this dst cache will not be checked expire because it has no RTF_EXPIRES flag
So this dst cache always be used until the dst gc run.

add a pointer in struct rt6_info,point to where the dst cache copy from.
in func rt6_check_expired check if rt6->info->rt6i_copy is expired.

Signed-off-by: Gao feng <gaofeng@cn.fujitsu.com>
---
 include/net/ip6_fib.h |    1 +
 net/ipv6/route.c      |   16 ++++++++++++++--
 2 files changed, 15 insertions(+), 2 deletions(-)

diff --git a/include/net/ip6_fib.h b/include/net/ip6_fib.h
index b26bb81..3da4d58c 100644
--- a/include/net/ip6_fib.h
+++ b/include/net/ip6_fib.h
@@ -106,6 +106,7 @@ struct rt6_info {
 	u32				rt6i_metric;
 	u32				rt6i_peer_genid;
 
+	struct rt6_info			*rt6i_copy;
 	struct inet6_dev		*rt6i_idev;
 	struct inet_peer		*rt6i_peer;
 
diff --git a/net/ipv6/route.c b/net/ipv6/route.c
index 8c2e3ab..939d06a 100644
--- a/net/ipv6/route.c
+++ b/net/ipv6/route.c
@@ -316,8 +316,15 @@ static void ip6_dst_ifdown(struct dst_entry *dst, struct net_device *dev,
 
 static __inline__ int rt6_check_expired(const struct rt6_info *rt)
 {
-	return (rt->rt6i_flags & RTF_EXPIRES) &&
-		time_after(jiffies, rt->dst.expires);
+	if ((rt->rt6i_flags & RTF_EXPIRES) &&
+		time_after(jiffies, rt->dst.expires))
+		return 1;
+
+	if (rt->rt6i_copy && (rt->rt6i_copy->rt6i_flags & RTF_EXPIRES) &&
+		time_after(jiffies, rt->rt6i_copy->dst.expires))
+		return 1;
+
+	return 0;
 }
 
 static inline int rt6_need_strict(const struct in6_addr *daddr)
@@ -1804,6 +1811,11 @@ static struct rt6_info *ip6_rt_copy(const struct rt6_info *ort,
 		rt->rt6i_gateway = ort->rt6i_gateway;
 		rt->rt6i_flags = ort->rt6i_flags & ~RTF_EXPIRES;
 		rt->rt6i_metric = 0;
+
+		if (ort->rt6i_copy)
+			rt->rt6i_copy = ort->rt6i_copy;
+		else
+			rt->rt6i_copy = ort;
 
 #ifdef CONFIG_IPV6_SUBTREES
 		memcpy(&rt->rt6i_src, &ort->rt6i_src, sizeof(struct rt6key));
-- 
1.7.5.4

Re: [PATCH] ipv6: Fix problem with expired dst cache

[David Miller]

From: Gao feng <gaofeng@cn.fujitsu.com>
Date: Fri, 24 Feb 2012 14:20:04 +0800

> diff --git a/include/net/ip6_fib.h b/include/net/ip6_fib.h
> index b26bb81..3da4d58c 100644
> --- a/include/net/ip6_fib.h
> +++ b/include/net/ip6_fib.h
> @@ -106,6 +106,7 @@ struct rt6_info {
>  	u32				rt6i_metric;
>  	u32				rt6i_peer_genid;
>  
> +	struct rt6_info			*rt6i_copy;
>  	struct inet6_dev		*rt6i_idev;
>  	struct inet_peer		*rt6i_peer;

This bloats up every route and cached entry in the machine, find
another way.

> +	if ((rt->rt6i_flags & RTF_EXPIRES) &&
> +		time_after(jiffies, rt->dst.expires))

Poorly formatted, correct way is:

	if ((rt->rt6i_flags & RTF_EXPIRES) &&
	    time_after(jiffies, rt->dst.expires))

> +	if (rt->rt6i_copy && (rt->rt6i_copy->rt6i_flags & RTF_EXPIRES) &&
> +		time_after(jiffies, rt->rt6i_copy->dst.expires))

Same problem.

Re: [PATCH] ipv6: Fix problem with expired dst cache

[Eric Dumazet]

Le vendredi 24 février 2012 à 14:20 +0800, Gao feng a écrit :
> if the ipv6 dst cache copy from the dst witch generated by ICMPV6 RA packet.
> this dst cache will not be checked expire because it has no RTF_EXPIRES flag
> So this dst cache always be used until the dst gc run.
> 
> add a pointer in struct rt6_info,point to where the dst cache copy from.
> in func rt6_check_expired check if rt6->info->rt6i_copy is expired.
> 

Sorry, I really dont understand what you are saying.

Also, adding a pointer to a structure without holding a reference on it
is suspicious.

> Signed-off-by: Gao feng <gaofeng@cn.fujitsu.com>
> ---
>  include/net/ip6_fib.h |    1 +
>  net/ipv6/route.c      |   16 ++++++++++++++--
>  2 files changed, 15 insertions(+), 2 deletions(-)
> 
> diff --git a/include/net/ip6_fib.h b/include/net/ip6_fib.h
> index b26bb81..3da4d58c 100644
> --- a/include/net/ip6_fib.h
> +++ b/include/net/ip6_fib.h
> @@ -106,6 +106,7 @@ struct rt6_info {
>  	u32				rt6i_metric;
>  	u32				rt6i_peer_genid;
>  
> +	struct rt6_info			*rt6i_copy;
>  	struct inet6_dev		*rt6i_idev;
>  	struct inet_peer		*rt6i_peer;
>  
> diff --git a/net/ipv6/route.c b/net/ipv6/route.c
> index 8c2e3ab..939d06a 100644
> --- a/net/ipv6/route.c
> +++ b/net/ipv6/route.c
> @@ -316,8 +316,15 @@ static void ip6_dst_ifdown(struct dst_entry *dst, struct net_device *dev,
>  
>  static __inline__ int rt6_check_expired(const struct rt6_info *rt)
>  {
> -	return (rt->rt6i_flags & RTF_EXPIRES) &&
> -		time_after(jiffies, rt->dst.expires);
> +	if ((rt->rt6i_flags & RTF_EXPIRES) &&
> +		time_after(jiffies, rt->dst.expires))
> +		return 1;
> +
> +	if (rt->rt6i_copy && (rt->rt6i_copy->rt6i_flags & RTF_EXPIRES) &&
> +		time_after(jiffies, rt->rt6i_copy->dst.expires))
> +		return 1;
> +
> +	return 0;
>  }
>  
>  static inline int rt6_need_strict(const struct in6_addr *daddr)
> @@ -1804,6 +1811,11 @@ static struct rt6_info *ip6_rt_copy(const struct rt6_info *ort,
>  		rt->rt6i_gateway = ort->rt6i_gateway;
>  		rt->rt6i_flags = ort->rt6i_flags & ~RTF_EXPIRES;
>  		rt->rt6i_metric = 0;
> +
> +		if (ort->rt6i_copy)
> +			rt->rt6i_copy = ort->rt6i_copy;
> +		else
> +			rt->rt6i_copy = ort;
>  
>  #ifdef CONFIG_IPV6_SUBTREES
>  		memcpy(&rt->rt6i_src, &ort->rt6i_src, sizeof(struct rt6key));

Re: [PATCH] ipv6: Fix problem with expired dst cache

[Gao feng]

于 2012年02月24日 14:47, David Miller 写道:
> From: Gao feng <gaofeng@cn.fujitsu.com>
> Date: Fri, 24 Feb 2012 14:20:04 +0800
> 
>> diff --git a/include/net/ip6_fib.h b/include/net/ip6_fib.h
>> index b26bb81..3da4d58c 100644
>> --- a/include/net/ip6_fib.h
>> +++ b/include/net/ip6_fib.h
>> @@ -106,6 +106,7 @@ struct rt6_info {
>>  	u32				rt6i_metric;
>>  	u32				rt6i_peer_genid;
>>  
>> +	struct rt6_info			*rt6i_copy;
>>  	struct inet6_dev		*rt6i_idev;
>>  	struct inet_peer		*rt6i_peer;
> 
> This bloats up every route and cached entry in the machine, find
> another way.

Another way is set dst cache's RTF_EXPIRES flag and expires in ip6_rt_copy
and when receive RA packet,update all the related dst cache's expires.
I don't think this is a good idea.

> 
>> +	if ((rt->rt6i_flags & RTF_EXPIRES) &&
>> +		time_after(jiffies, rt->dst.expires))
> 
> Poorly formatted, correct way is:
> 
> 	if ((rt->rt6i_flags & RTF_EXPIRES) &&
> 	    time_after(jiffies, rt->dst.expires))
> 
>> +	if (rt->rt6i_copy && (rt->rt6i_copy->rt6i_flags & RTF_EXPIRES) &&
>> +		time_after(jiffies, rt->rt6i_copy->dst.expires))
> 
> Same problem.
> 

Re: [PATCH] ipv6: Fix problem with expired dst cache

[Gao feng]

于 2012年02月24日 14:51, Eric Dumazet 写道:
> Le vendredi 24 février 2012 à 14:20 +0800, Gao feng a écrit :
>> if the ipv6 dst cache copy from the dst witch generated by ICMPV6 RA packet.
>> this dst cache will not be checked expire because it has no RTF_EXPIRES flag
>> So this dst cache always be used until the dst gc run.
>>
>> add a pointer in struct rt6_info,point to where the dst cache copy from.
>> in func rt6_check_expired check if rt6->info->rt6i_copy is expired.
>>
> 
> Sorry, I really dont understand what you are saying.

Just because ipv6 dst cache has no RTF_EXPIRES flag,
so even this dst cache has expired,it will be used until the gc run.

> 
> Also, adding a pointer to a structure without holding a reference on it
> is suspicious.
> 

OK,I think there is no use to hold a reference.
the dst cache will be deleted before the pointer,
So this pointer will be always usefull.

Re: [PATCH] ipv6: Fix problem with expired dst cache

[Gao feng]

于 2012年02月24日 14:47, David Miller 写道:
> From: Gao feng <gaofeng@cn.fujitsu.com>
> Date: Fri, 24 Feb 2012 14:20:04 +0800
> 
>> diff --git a/include/net/ip6_fib.h b/include/net/ip6_fib.h
>> index b26bb81..3da4d58c 100644
>> --- a/include/net/ip6_fib.h
>> +++ b/include/net/ip6_fib.h
>> @@ -106,6 +106,7 @@ struct rt6_info {
>>  	u32				rt6i_metric;
>>  	u32				rt6i_peer_genid;
>>  
>> +	struct rt6_info			*rt6i_copy;
>>  	struct inet6_dev		*rt6i_idev;
>>  	struct inet_peer		*rt6i_peer;
> 
> This bloats up every route and cached entry in the machine, find
> another way.
> 

How about calling rt6_get_dflt_route to find out the default dst,
and check if it has expired when we use the cache dst?

[PATCH V2] ipv6: Fix problem with expired dst cache

[Gao feng]

If the ipv6 dst cache which copy from the dst generated by ICMPV6 RA packet.
this dst cache will not check expire because it has no RTF_EXPIRES flag.
So this dst cache will always be used until the dst gc run.

Change the struct dst_entry,add a union contains new pointer from and expires.
When rt6_info.rt6i_flags has no RTF_EXPIRES flag,the dst.expires has no use.
we can use this field to point to where the dst cache copy from.
The dst.from is only used in IPV6.

In func rt6_check_expired check if rt6_info.dst.from is expired.

In func ip6_rt_copy only set dst.from when the ort has flag RTF_ADDRCONF
and RTF_DEFAULT.

Signed-off-by: Gao feng <gaofeng@cn.fujitsu.com>
---
 include/net/dst.h |   11 ++++++++++-
 net/ipv6/route.c  |   22 +++++++++++++++++++---
 2 files changed, 29 insertions(+), 4 deletions(-)

diff --git a/include/net/dst.h b/include/net/dst.h
index 344c8dd..eae92f9 100644
--- a/include/net/dst.h
+++ b/include/net/dst.h
@@ -35,7 +35,16 @@ struct dst_entry {
 	struct net_device       *dev;
 	struct  dst_ops	        *ops;
 	unsigned long		_metrics;
-	unsigned long		expires;
+
+	union {
+		unsigned long		expires;
+		/*
+		 *from is used only for dst cache witch copy form
+		 *the dst generated by ipv6 RA.
+		 */
+		void			*from;
+	};
+
 	struct dst_entry	*path;
 	struct neighbour __rcu	*_neighbour;
 #ifdef CONFIG_XFRM
diff --git a/net/ipv6/route.c b/net/ipv6/route.c
index 8c2e3ab..cd365fe 100644
--- a/net/ipv6/route.c
+++ b/net/ipv6/route.c
@@ -316,8 +316,18 @@ static void ip6_dst_ifdown(struct dst_entry *dst, struct net_device *dev,
 
 static __inline__ int rt6_check_expired(const struct rt6_info *rt)
 {
-	return (rt->rt6i_flags & RTF_EXPIRES) &&
-		time_after(jiffies, rt->dst.expires);
+	struct rt6_info *ort = NULL;
+
+	if (rt->rt6i_flags & RTF_EXPIRES) {
+		if (time_after(jiffies, rt->dst.expires))
+			return 1;
+	} else if (rt->dst.from) {
+		ort = (struct rt6_info *) rt->dst.from;
+		return (ort->rt6i_flags & RTF_EXPIRES) &&
+			time_after(jiffies, ort->dst.expires);
+	}
+
+	return 0;
 }
 
 static inline int rt6_need_strict(const struct in6_addr *daddr)
@@ -608,6 +618,7 @@ int rt6_route_rcv(struct net_device *dev, u8 *opt, int len,
 
 	if (rt) {
 		if (!addrconf_finite_timeout(lifetime)) {
+			rt->dst.expires = 0;
 			rt->rt6i_flags &= ~RTF_EXPIRES;
 		} else {
 			rt->dst.expires = jiffies + HZ * lifetime;
@@ -1799,7 +1810,12 @@ static struct rt6_info *ip6_rt_copy(const struct rt6_info *ort,
 		if (rt->rt6i_idev)
 			in6_dev_hold(rt->rt6i_idev);
 		rt->dst.lastuse = jiffies;
-		rt->dst.expires = 0;
+
+		if ((ort->rt6i_flags & (RTF_ADDRCONF | RTF_DEFAULT)) ==
+		    (RTF_ADDRCONF | RTF_DEFAULT))
+			rt->dst.from = (void *)ort;
+		else
+			rt->dst.expires = 0;
 
 		rt->rt6i_gateway = ort->rt6i_gateway;
 		rt->rt6i_flags = ort->rt6i_flags & ~RTF_EXPIRES;
-- 
1.7.5.4

Re: [PATCH V2] ipv6: Fix problem with expired dst cache

[Gao feng]

Hi David.

于 2012年02月27日 14:36, Gao feng 写道:
> If the ipv6 dst cache which copy from the dst generated by ICMPV6 RA packet.
> this dst cache will not check expire because it has no RTF_EXPIRES flag.
> So this dst cache will always be used until the dst gc run.
> 
> Change the struct dst_entry,add a union contains new pointer from and expires.
> When rt6_info.rt6i_flags has no RTF_EXPIRES flag,the dst.expires has no use.
> we can use this field to point to where the dst cache copy from.
> The dst.from is only used in IPV6.
> 
> In func rt6_check_expired check if rt6_info.dst.from is expired.
> 
> In func ip6_rt_copy only set dst.from when the ort has flag RTF_ADDRCONF
> and RTF_DEFAULT.
> 

I will send the v3 PATCH.
This version has some changes.
1,hold the ort in ip6_rt_copy and release it in ip6_dst_destroy just as Eric said.
2,add some functions to operate the RTF_EXPIRES flag and expires(from).
3,change the code to use the functions added in 2.

[PATCH] ipv6: Fix problem with expired dst cache

[Gao feng]

If the ipv6 dst cache which copy from the dst generated by ICMPV6 RA packet.
this dst cache will not check expire because it has no RTF_EXPIRES flag.
So this dst cache will always be used until the dst gc run.

Change the struct dst_entry,add a union contains new pointer from and expires.
When rt6_info.rt6i_flags has no RTF_EXPIRES flag,the dst.expires has no use.
we can use this field to point to where the dst cache copy from.
The dst.from is only used in IPV6.

In func rt6_check_expired check if rt6_info.dst.from is expired.

In func ip6_rt_copy only set dst.from when the ort has flag RTF_ADDRCONF
and RTF_DEFAULT.then hold the ort.

In func ip6_dst_destroy release the ort.

Add some functions to operate the RTF_EXPIRES flag and expires(from)
and change the code to use these new adding functions.

Signed-off-by: Gao feng <gaofeng@cn.fujitsu.com>
---
 include/net/dst.h     |   11 ++++++++++-
 include/net/ip6_fib.h |   41 +++++++++++++++++++++++++++++++++++++++++
 net/ipv6/addrconf.c   |    9 +++------
 net/ipv6/ip6_fib.c    |    3 +--
 net/ipv6/route.c      |   47 +++++++++++++++++++++++++++++------------------
 5 files changed, 84 insertions(+), 27 deletions(-)

diff --git a/include/net/dst.h b/include/net/dst.h
index 344c8dd..5147839 100644
--- a/include/net/dst.h
+++ b/include/net/dst.h
@@ -35,7 +35,16 @@ struct dst_entry {
 	struct net_device       *dev;
 	struct  dst_ops	        *ops;
 	unsigned long		_metrics;
-	unsigned long		expires;
+
+	union {
+		unsigned long		expires;
+		/*
+		 * from is used only for dst cache witch copy form
+		 * the dst generated by ipv6 RA.
+		 * from is set only when rt6_info has no RTF_EXPIRES flag.
+		 */
+		void			*from;
+	};
 	struct dst_entry	*path;
 	struct neighbour __rcu	*_neighbour;
 #ifdef CONFIG_XFRM
diff --git a/include/net/ip6_fib.h b/include/net/ip6_fib.h
index b26bb81..86cf1ac 100644
--- a/include/net/ip6_fib.h
+++ b/include/net/ip6_fib.h
@@ -123,6 +123,47 @@ static inline struct inet6_dev *ip6_dst_idev(struct dst_entry *dst)
 	return ((struct rt6_info *)dst)->rt6i_idev;
 }
 
+static inline void rt6_clean_expires(struct rt6_info *rt)
+{
+	if (!(rt->rt6i_flags & RTF_EXPIRES) && rt->dst.from)
+		dst_release(&rt->dst);
+
+	rt->rt6i_flags &= ~RTF_EXPIRES;
+	rt->dst.expires = 0;
+}
+
+static inline void rt6_set_expires(struct rt6_info *rt, unsigned long expires)
+{
+	if (!(rt->rt6i_flags & RTF_EXPIRES) && rt->dst.from)
+		dst_release(&rt->dst);
+
+	rt->rt6i_flags |= RTF_EXPIRES;
+	rt->dst.expires = expires;
+}
+
+static inline void rt6_update_expires(struct rt6_info *rt, int timeout)
+{
+	if (!(rt->rt6i_flags & RTF_EXPIRES) && rt->dst.from)
+		dst_release(&rt->dst);
+
+	dst_set_expires(&rt->dst, timeout);
+	rt->rt6i_flags |= RTF_EXPIRES;
+}
+
+static inline void rt6_set_from(struct rt6_info *rt, struct rt6_info *from)
+{
+	if (!(rt->rt6i_flags & RTF_EXPIRES) && rt->dst.from) {
+		if (from == rt->dst.from)
+			return;
+		else
+			dst_release((struct dst_entry *) &rt->dst.from);
+	}
+
+	rt->rt6i_flags &= ~RTF_EXPIRES;
+	rt->dst.from = (void *) from;
+	dst_hold(&from->dst);
+}
+
 struct fib6_walker_t {
 	struct list_head lh;
 	struct fib6_node *root, *node;
diff --git a/net/ipv6/addrconf.c b/net/ipv6/addrconf.c
index c02280a..b5cb3b1 100644
--- a/net/ipv6/addrconf.c
+++ b/net/ipv6/addrconf.c
@@ -799,8 +799,7 @@ static void ipv6_del_addr(struct inet6_ifaddr *ifp)
 				ip6_del_rt(rt);
 				rt = NULL;
 			} else if (!(rt->rt6i_flags & RTF_EXPIRES)) {
-				rt->dst.expires = expires;
-				rt->rt6i_flags |= RTF_EXPIRES;
+				rt6_set_expires(rt, expires);
 			}
 		}
 		dst_release(&rt->dst);
@@ -1883,11 +1882,9 @@ void addrconf_prefix_rcv(struct net_device *dev, u8 *opt, int len, bool sllao)
 				rt = NULL;
 			} else if (addrconf_finite_timeout(rt_expires)) {
 				/* not infinity */
-				rt->dst.expires = jiffies + rt_expires;
-				rt->rt6i_flags |= RTF_EXPIRES;
+				rt6_set_expires(rt, jiffies + rt_expires);
 			} else {
-				rt->rt6i_flags &= ~RTF_EXPIRES;
-				rt->dst.expires = 0;
+				rt6_clean_expires(rt);
 			}
 		} else if (valid_lft) {
 			clock_t expires = 0;
diff --git a/net/ipv6/ip6_fib.c b/net/ipv6/ip6_fib.c
index b82bcde..9a4f51e 100644
--- a/net/ipv6/ip6_fib.c
+++ b/net/ipv6/ip6_fib.c
@@ -675,8 +675,7 @@ static int fib6_add_rt2node(struct fib6_node *fn, struct rt6_info *rt,
 					return -EEXIST;
 				iter->dst.expires = rt->dst.expires;
 				if (!(rt->rt6i_flags & RTF_EXPIRES)) {
-					iter->rt6i_flags &= ~RTF_EXPIRES;
-					iter->dst.expires = 0;
+					rt6_clean_expires(iter);
 				}
 				return -EEXIST;
 			}
diff --git a/net/ipv6/route.c b/net/ipv6/route.c
index 8c2e3ab..1f6d40a 100644
--- a/net/ipv6/route.c
+++ b/net/ipv6/route.c
@@ -62,7 +62,7 @@
 #include <linux/sysctl.h>
 #endif
 
-static struct rt6_info *ip6_rt_copy(const struct rt6_info *ort,
+static struct rt6_info *ip6_rt_copy(struct rt6_info *ort,
 				    const struct in6_addr *dest);
 static struct dst_entry	*ip6_dst_check(struct dst_entry *dst, u32 cookie);
 static unsigned int	 ip6_default_advmss(const struct dst_entry *dst);
@@ -272,6 +272,10 @@ static void ip6_dst_destroy(struct dst_entry *dst)
 		rt->rt6i_idev = NULL;
 		in6_dev_put(idev);
 	}
+
+	if (!(rt->rt6i_flags & RTF_EXPIRES) && dst->from)
+		dst_release((struct dst_entry *)dst->from);
+
 	if (peer) {
 		rt->rt6i_peer = NULL;
 		inet_putpeer(peer);
@@ -316,8 +320,18 @@ static void ip6_dst_ifdown(struct dst_entry *dst, struct net_device *dev,
 
 static __inline__ int rt6_check_expired(const struct rt6_info *rt)
 {
-	return (rt->rt6i_flags & RTF_EXPIRES) &&
-		time_after(jiffies, rt->dst.expires);
+	struct rt6_info *ort = NULL;
+
+	if (rt->rt6i_flags & RTF_EXPIRES) {
+		if (time_after(jiffies, rt->dst.expires))
+			return 1;
+	} else if (rt->dst.from) {
+		ort = (struct rt6_info *) rt->dst.from;
+		return (ort->rt6i_flags & RTF_EXPIRES) &&
+			time_after(jiffies, ort->dst.expires);
+	}
+
+	return 0;
 }
 
 static inline int rt6_need_strict(const struct in6_addr *daddr)
@@ -608,10 +622,9 @@ int rt6_route_rcv(struct net_device *dev, u8 *opt, int len,
 
 	if (rt) {
 		if (!addrconf_finite_timeout(lifetime)) {
-			rt->rt6i_flags &= ~RTF_EXPIRES;
+			rt6_clean_expires(rt);
 		} else {
-			rt->dst.expires = jiffies + HZ * lifetime;
-			rt->rt6i_flags |= RTF_EXPIRES;
+			rt6_set_expires(rt, jiffies + HZ * lifetime);
 		}
 		dst_release(&rt->dst);
 	}
@@ -717,7 +730,7 @@ int ip6_ins_rt(struct rt6_info *rt)
 	return __ip6_ins_rt(rt, &info);
 }
 
-static struct rt6_info *rt6_alloc_cow(const struct rt6_info *ort,
+static struct rt6_info *rt6_alloc_cow(struct rt6_info *ort,
 				      const struct in6_addr *daddr,
 				      const struct in6_addr *saddr)
 {
@@ -934,10 +947,9 @@ struct dst_entry *ip6_blackhole_route(struct net *net, struct dst_entry *dst_ori
 		rt->rt6i_idev = ort->rt6i_idev;
 		if (rt->rt6i_idev)
 			in6_dev_hold(rt->rt6i_idev);
-		rt->dst.expires = 0;
 
 		rt->rt6i_gateway = ort->rt6i_gateway;
-		rt->rt6i_flags = ort->rt6i_flags & ~RTF_EXPIRES;
+		rt6_clean_expires(rt);
 		rt->rt6i_metric = 0;
 
 		memcpy(&rt->rt6i_dst, &ort->rt6i_dst, sizeof(struct rt6key));
@@ -1000,8 +1012,7 @@ static void ip6_link_failure(struct sk_buff *skb)
 	rt = (struct rt6_info *) skb_dst(skb);
 	if (rt) {
 		if (rt->rt6i_flags & RTF_CACHE) {
-			dst_set_expires(&rt->dst, 0);
-			rt->rt6i_flags |= RTF_EXPIRES;
+			rt6_update_expires(rt, 0);
 		} else if (rt->rt6i_node && (rt->rt6i_flags & RTF_DEFAULT))
 			rt->rt6i_node->fn_sernum = -1;
 	}
@@ -1716,8 +1727,8 @@ again:
 			features |= RTAX_FEATURE_ALLFRAG;
 			dst_metric_set(&rt->dst, RTAX_FEATURES, features);
 		}
-		dst_set_expires(&rt->dst, net->ipv6.sysctl.ip6_rt_mtu_expires);
-		rt->rt6i_flags |= RTF_MODIFIED|RTF_EXPIRES;
+		rt6_update_expires(rt, net->ipv6.sysctl.ip6_rt_mtu_expires);
+		rt->rt6i_flags |= RTF_MODIFIED;
 		goto out;
 	}
 
@@ -1745,8 +1756,8 @@ again:
 		 * which is 10 mins. After 10 mins the decreased pmtu is expired
 		 * and detecting PMTU increase will be automatically happened.
 		 */
-		dst_set_expires(&nrt->dst, net->ipv6.sysctl.ip6_rt_mtu_expires);
-		nrt->rt6i_flags |= RTF_DYNAMIC|RTF_EXPIRES;
+		rt6_update_expires(nrt, net->ipv6.sysctl.ip6_rt_mtu_expires);
+		nrt->rt6i_flags |= RTF_DYNAMIC;
 
 		ip6_ins_rt(nrt);
 	}
@@ -1779,7 +1790,7 @@ void rt6_pmtu_discovery(const struct in6_addr *daddr, const struct in6_addr *sad
  *	Misc support functions
  */
 
-static struct rt6_info *ip6_rt_copy(const struct rt6_info *ort,
+static struct rt6_info *ip6_rt_copy(struct rt6_info *ort,
 				    const struct in6_addr *dest)
 {
 	struct net *net = dev_net(ort->dst.dev);
@@ -1799,10 +1810,10 @@ static struct rt6_info *ip6_rt_copy(const struct rt6_info *ort,
 		if (rt->rt6i_idev)
 			in6_dev_hold(rt->rt6i_idev);
 		rt->dst.lastuse = jiffies;
-		rt->dst.expires = 0;
 
 		rt->rt6i_gateway = ort->rt6i_gateway;
-		rt->rt6i_flags = ort->rt6i_flags & ~RTF_EXPIRES;
+		rt6_set_from(rt, ort);
+
 		rt->rt6i_metric = 0;
 
 #ifdef CONFIG_IPV6_SUBTREES
-- 
1.7.5.4

Re: [PATCH] ipv6: Fix problem with expired dst cache

[Gao feng]

于 2012年02月29日 17:45, Gao feng 写道:

> -static struct rt6_info *rt6_alloc_cow(const struct rt6_info *ort,
> +static struct rt6_info *rt6_alloc_cow(struct rt6_info *ort,
>  				      const struct in6_addr *daddr,
>  				      const struct in6_addr *saddr)
>  {
> @@ -934,10 +947,9 @@ struct dst_entry *ip6_blackhole_route(struct net *net, struct dst_entry *dst_ori
>  		rt->rt6i_idev = ort->rt6i_idev;
>  		if (rt->rt6i_idev)
>  			in6_dev_hold(rt->rt6i_idev);
> -		rt->dst.expires = 0;
>  
>  		rt->rt6i_gateway = ort->rt6i_gateway;
> -		rt->rt6i_flags = ort->rt6i_flags & ~RTF_EXPIRES;
> +		rt6_clean_expires(rt);


sorry for this,Please ignore this patch,I will resend it.

[PATCH v3] ipv6: Fix problem with expired dst cache

[Gao feng]

If the ipv6 dst cache which copy from the dst generated by ICMPV6 RA packet.
this dst cache will not check expire because it has no RTF_EXPIRES flag.
So this dst cache will always be used until the dst gc run.

Change the struct dst_entry,add a union contains new pointer from and expires.
When rt6_info.rt6i_flags has no RTF_EXPIRES flag,the dst.expires has no use.
we can use this field to point to where the dst cache copy from.
The dst.from is only used in IPV6.

In func rt6_check_expired check if rt6_info.dst.from is expired.

In func ip6_rt_copy only set dst.from when the ort has flag RTF_ADDRCONF
and RTF_DEFAULT.then hold the ort.

In func ip6_dst_destroy release the ort.

Add some functions to operate the RTF_EXPIRES flag and expires(from)
and change the code to use these new adding functions.

Signed-off-by: Gao feng <gaofeng@cn.fujitsu.com>
---
 include/net/dst.h     |   11 ++++++++++-
 include/net/ip6_fib.h |   41 +++++++++++++++++++++++++++++++++++++++++
 net/ipv6/addrconf.c   |    9 +++------
 net/ipv6/ip6_fib.c    |    3 +--
 net/ipv6/route.c      |   49 +++++++++++++++++++++++++++++++------------------
 5 files changed, 86 insertions(+), 27 deletions(-)

diff --git a/include/net/dst.h b/include/net/dst.h
index 344c8dd..5147839 100644
--- a/include/net/dst.h
+++ b/include/net/dst.h
@@ -35,7 +35,16 @@ struct dst_entry {
 	struct net_device       *dev;
 	struct  dst_ops	        *ops;
 	unsigned long		_metrics;
-	unsigned long		expires;
+
+	union {
+		unsigned long		expires;
+		/*
+		 * from is used only for dst cache witch copy form
+		 * the dst generated by ipv6 RA.
+		 * from is set only when rt6_info has no RTF_EXPIRES flag.
+		 */
+		void			*from;
+	};
 	struct dst_entry	*path;
 	struct neighbour __rcu	*_neighbour;
 #ifdef CONFIG_XFRM
diff --git a/include/net/ip6_fib.h b/include/net/ip6_fib.h
index b26bb81..86cf1ac 100644
--- a/include/net/ip6_fib.h
+++ b/include/net/ip6_fib.h
@@ -123,6 +123,47 @@ static inline struct inet6_dev *ip6_dst_idev(struct dst_entry *dst)
 	return ((struct rt6_info *)dst)->rt6i_idev;
 }
 
+static inline void rt6_clean_expires(struct rt6_info *rt)
+{
+	if (!(rt->rt6i_flags & RTF_EXPIRES) && rt->dst.from)
+		dst_release(&rt->dst);
+
+	rt->rt6i_flags &= ~RTF_EXPIRES;
+	rt->dst.expires = 0;
+}
+
+static inline void rt6_set_expires(struct rt6_info *rt, unsigned long expires)
+{
+	if (!(rt->rt6i_flags & RTF_EXPIRES) && rt->dst.from)
+		dst_release(&rt->dst);
+
+	rt->rt6i_flags |= RTF_EXPIRES;
+	rt->dst.expires = expires;
+}
+
+static inline void rt6_update_expires(struct rt6_info *rt, int timeout)
+{
+	if (!(rt->rt6i_flags & RTF_EXPIRES) && rt->dst.from)
+		dst_release(&rt->dst);
+
+	dst_set_expires(&rt->dst, timeout);
+	rt->rt6i_flags |= RTF_EXPIRES;
+}
+
+static inline void rt6_set_from(struct rt6_info *rt, struct rt6_info *from)
+{
+	if (!(rt->rt6i_flags & RTF_EXPIRES) && rt->dst.from) {
+		if (from == rt->dst.from)
+			return;
+		else
+			dst_release((struct dst_entry *) &rt->dst.from);
+	}
+
+	rt->rt6i_flags &= ~RTF_EXPIRES;
+	rt->dst.from = (void *) from;
+	dst_hold(&from->dst);
+}
+
 struct fib6_walker_t {
 	struct list_head lh;
 	struct fib6_node *root, *node;
diff --git a/net/ipv6/addrconf.c b/net/ipv6/addrconf.c
index c02280a..b5cb3b1 100644
--- a/net/ipv6/addrconf.c
+++ b/net/ipv6/addrconf.c
@@ -799,8 +799,7 @@ static void ipv6_del_addr(struct inet6_ifaddr *ifp)
 				ip6_del_rt(rt);
 				rt = NULL;
 			} else if (!(rt->rt6i_flags & RTF_EXPIRES)) {
-				rt->dst.expires = expires;
-				rt->rt6i_flags |= RTF_EXPIRES;
+				rt6_set_expires(rt, expires);
 			}
 		}
 		dst_release(&rt->dst);
@@ -1883,11 +1882,9 @@ void addrconf_prefix_rcv(struct net_device *dev, u8 *opt, int len, bool sllao)
 				rt = NULL;
 			} else if (addrconf_finite_timeout(rt_expires)) {
 				/* not infinity */
-				rt->dst.expires = jiffies + rt_expires;
-				rt->rt6i_flags |= RTF_EXPIRES;
+				rt6_set_expires(rt, jiffies + rt_expires);
 			} else {
-				rt->rt6i_flags &= ~RTF_EXPIRES;
-				rt->dst.expires = 0;
+				rt6_clean_expires(rt);
 			}
 		} else if (valid_lft) {
 			clock_t expires = 0;
diff --git a/net/ipv6/ip6_fib.c b/net/ipv6/ip6_fib.c
index b82bcde..9a4f51e 100644
--- a/net/ipv6/ip6_fib.c
+++ b/net/ipv6/ip6_fib.c
@@ -675,8 +675,7 @@ static int fib6_add_rt2node(struct fib6_node *fn, struct rt6_info *rt,
 					return -EEXIST;
 				iter->dst.expires = rt->dst.expires;
 				if (!(rt->rt6i_flags & RTF_EXPIRES)) {
-					iter->rt6i_flags &= ~RTF_EXPIRES;
-					iter->dst.expires = 0;
+					rt6_clean_expires(iter);
 				}
 				return -EEXIST;
 			}
diff --git a/net/ipv6/route.c b/net/ipv6/route.c
index 8c2e3ab..302a773 100644
--- a/net/ipv6/route.c
+++ b/net/ipv6/route.c
@@ -62,7 +62,7 @@
 #include <linux/sysctl.h>
 #endif
 
-static struct rt6_info *ip6_rt_copy(const struct rt6_info *ort,
+static struct rt6_info *ip6_rt_copy(struct rt6_info *ort,
 				    const struct in6_addr *dest);
 static struct dst_entry	*ip6_dst_check(struct dst_entry *dst, u32 cookie);
 static unsigned int	 ip6_default_advmss(const struct dst_entry *dst);
@@ -272,6 +272,10 @@ static void ip6_dst_destroy(struct dst_entry *dst)
 		rt->rt6i_idev = NULL;
 		in6_dev_put(idev);
 	}
+
+	if (!(rt->rt6i_flags & RTF_EXPIRES) && dst->from)
+		dst_release((struct dst_entry *)dst->from);
+
 	if (peer) {
 		rt->rt6i_peer = NULL;
 		inet_putpeer(peer);
@@ -316,8 +320,18 @@ static void ip6_dst_ifdown(struct dst_entry *dst, struct net_device *dev,
 
 static __inline__ int rt6_check_expired(const struct rt6_info *rt)
 {
-	return (rt->rt6i_flags & RTF_EXPIRES) &&
-		time_after(jiffies, rt->dst.expires);
+	struct rt6_info *ort = NULL;
+
+	if (rt->rt6i_flags & RTF_EXPIRES) {
+		if (time_after(jiffies, rt->dst.expires))
+			return 1;
+	} else if (rt->dst.from) {
+		ort = (struct rt6_info *) rt->dst.from;
+		return (ort->rt6i_flags & RTF_EXPIRES) &&
+			time_after(jiffies, ort->dst.expires);
+	}
+
+	return 0;
 }
 
 static inline int rt6_need_strict(const struct in6_addr *daddr)
@@ -608,10 +622,9 @@ int rt6_route_rcv(struct net_device *dev, u8 *opt, int len,
 
 	if (rt) {
 		if (!addrconf_finite_timeout(lifetime)) {
-			rt->rt6i_flags &= ~RTF_EXPIRES;
+			rt6_clean_expires(rt);
 		} else {
-			rt->dst.expires = jiffies + HZ * lifetime;
-			rt->rt6i_flags |= RTF_EXPIRES;
+			rt6_set_expires(rt, jiffies + HZ * lifetime);
 		}
 		dst_release(&rt->dst);
 	}
@@ -717,7 +730,7 @@ int ip6_ins_rt(struct rt6_info *rt)
 	return __ip6_ins_rt(rt, &info);
 }
 
-static struct rt6_info *rt6_alloc_cow(const struct rt6_info *ort,
+static struct rt6_info *rt6_alloc_cow(struct rt6_info *ort,
 				      const struct in6_addr *daddr,
 				      const struct in6_addr *saddr)
 {
@@ -934,10 +947,10 @@ struct dst_entry *ip6_blackhole_route(struct net *net, struct dst_entry *dst_ori
 		rt->rt6i_idev = ort->rt6i_idev;
 		if (rt->rt6i_idev)
 			in6_dev_hold(rt->rt6i_idev);
-		rt->dst.expires = 0;
 
 		rt->rt6i_gateway = ort->rt6i_gateway;
-		rt->rt6i_flags = ort->rt6i_flags & ~RTF_EXPIRES;
+		rt->rt6i_flags = ort->rt6i_flags;
+		rt6_clean_expires(rt);
 		rt->rt6i_metric = 0;
 
 		memcpy(&rt->rt6i_dst, &ort->rt6i_dst, sizeof(struct rt6key));
@@ -1000,8 +1013,7 @@ static void ip6_link_failure(struct sk_buff *skb)
 	rt = (struct rt6_info *) skb_dst(skb);
 	if (rt) {
 		if (rt->rt6i_flags & RTF_CACHE) {
-			dst_set_expires(&rt->dst, 0);
-			rt->rt6i_flags |= RTF_EXPIRES;
+			rt6_update_expires(rt, 0);
 		} else if (rt->rt6i_node && (rt->rt6i_flags & RTF_DEFAULT))
 			rt->rt6i_node->fn_sernum = -1;
 	}
@@ -1716,8 +1728,8 @@ again:
 			features |= RTAX_FEATURE_ALLFRAG;
 			dst_metric_set(&rt->dst, RTAX_FEATURES, features);
 		}
-		dst_set_expires(&rt->dst, net->ipv6.sysctl.ip6_rt_mtu_expires);
-		rt->rt6i_flags |= RTF_MODIFIED|RTF_EXPIRES;
+		rt6_update_expires(rt, net->ipv6.sysctl.ip6_rt_mtu_expires);
+		rt->rt6i_flags |= RTF_MODIFIED;
 		goto out;
 	}
 
@@ -1745,8 +1757,8 @@ again:
 		 * which is 10 mins. After 10 mins the decreased pmtu is expired
 		 * and detecting PMTU increase will be automatically happened.
 		 */
-		dst_set_expires(&nrt->dst, net->ipv6.sysctl.ip6_rt_mtu_expires);
-		nrt->rt6i_flags |= RTF_DYNAMIC|RTF_EXPIRES;
+		rt6_update_expires(nrt, net->ipv6.sysctl.ip6_rt_mtu_expires);
+		nrt->rt6i_flags |= RTF_DYNAMIC;
 
 		ip6_ins_rt(nrt);
 	}
@@ -1779,7 +1791,7 @@ void rt6_pmtu_discovery(const struct in6_addr *daddr, const struct in6_addr *sad
  *	Misc support functions
  */
 
-static struct rt6_info *ip6_rt_copy(const struct rt6_info *ort,
+static struct rt6_info *ip6_rt_copy(struct rt6_info *ort,
 				    const struct in6_addr *dest)
 {
 	struct net *net = dev_net(ort->dst.dev);
@@ -1799,10 +1811,11 @@ static struct rt6_info *ip6_rt_copy(const struct rt6_info *ort,
 		if (rt->rt6i_idev)
 			in6_dev_hold(rt->rt6i_idev);
 		rt->dst.lastuse = jiffies;
-		rt->dst.expires = 0;
 
 		rt->rt6i_gateway = ort->rt6i_gateway;
-		rt->rt6i_flags = ort->rt6i_flags & ~RTF_EXPIRES;
+		rt->rt6i_flags = ort->rt6i_flags;
+		rt6_set_from(rt, ort);
+
 		rt->rt6i_metric = 0;
 
 #ifdef CONFIG_IPV6_SUBTREES
-- 
1.7.5.4

Re: [PATCH v3] ipv6: Fix problem with expired dst cache

[Eric Dumazet]

Le mercredi 29 février 2012 à 18:07 +0800, Gao feng a écrit :
> If the ipv6 dst cache which copy from the dst generated by ICMPV6 RA packet.
> this dst cache will not check expire because it has no RTF_EXPIRES flag.
> So this dst cache will always be used until the dst gc run.
> 
> Change the struct dst_entry,add a union contains new pointer from and expires.
> When rt6_info.rt6i_flags has no RTF_EXPIRES flag,the dst.expires has no use.
> we can use this field to point to where the dst cache copy from.
> The dst.from is only used in IPV6.
> 
> In func rt6_check_expired check if rt6_info.dst.from is expired.
> 
> In func ip6_rt_copy only set dst.from when the ort has flag RTF_ADDRCONF
> and RTF_DEFAULT.then hold the ort.
> 
> In func ip6_dst_destroy release the ort.
> 
> Add some functions to operate the RTF_EXPIRES flag and expires(from)
> and change the code to use these new adding functions.
> 
> Signed-off-by: Gao feng <gaofeng@cn.fujitsu.com>
> ---
>  include/net/dst.h     |   11 ++++++++++-
>  include/net/ip6_fib.h |   41 +++++++++++++++++++++++++++++++++++++++++
>  net/ipv6/addrconf.c   |    9 +++------
>  net/ipv6/ip6_fib.c    |    3 +--
>  net/ipv6/route.c      |   49 +++++++++++++++++++++++++++++++------------------
>  5 files changed, 86 insertions(+), 27 deletions(-)
> 
> diff --git a/include/net/dst.h b/include/net/dst.h
> index 344c8dd..5147839 100644
> --- a/include/net/dst.h
> +++ b/include/net/dst.h
> @@ -35,7 +35,16 @@ struct dst_entry {
>  	struct net_device       *dev;
>  	struct  dst_ops	        *ops;
>  	unsigned long		_metrics;
> -	unsigned long		expires;
> +
> +	union {
> +		unsigned long		expires;
> +		/*
> +		 * from is used only for dst cache witch copy form


> +		 * the dst generated by ipv6 RA.
> +		 * from is set only when rt6_info has no RTF_EXPIRES flag.


I am not an english native but really this comment should be reworded...


> +		 */
> +		void			*from;
> +	};
>  	struct dst_entry	*path;
>  	struct neighbour __rcu	*_neighbour;
>  #ifdef CONFIG_XFRM
> diff --git a/include/net/ip6_fib.h b/include/net/ip6_fib.h
> index b26bb81..86cf1ac 100644
> --- a/include/net/ip6_fib.h
> +++ b/include/net/ip6_fib.h
> @@ -123,6 +123,47 @@ static inline struct inet6_dev *ip6_dst_idev(struct dst_entry *dst)
>  	return ((struct rt6_info *)dst)->rt6i_idev;
>  }
>  
> +static inline void rt6_clean_expires(struct rt6_info *rt)
> +{
> +	if (!(rt->rt6i_flags & RTF_EXPIRES) && rt->dst.from)
> +		dst_release(&rt->dst);
> +
> +	rt->rt6i_flags &= ~RTF_EXPIRES;
> +	rt->dst.expires = 0;
> +}
> +
> +static inline void rt6_set_expires(struct rt6_info *rt, unsigned long expires)
> +{
> +	if (!(rt->rt6i_flags & RTF_EXPIRES) && rt->dst.from)
> +		dst_release(&rt->dst);
> +
> +	rt->rt6i_flags |= RTF_EXPIRES;
> +	rt->dst.expires = expires;
> +}
> +
> +static inline void rt6_update_expires(struct rt6_info *rt, int timeout)
> +{
> +	if (!(rt->rt6i_flags & RTF_EXPIRES) && rt->dst.from)
> +		dst_release(&rt->dst);
> +
> +	dst_set_expires(&rt->dst, timeout);
> +	rt->rt6i_flags |= RTF_EXPIRES;
> +}

why rt6_update_expires() takes an "int timeout", promoted to "unsigned
long expires" ? Do you have a 32bit machine by any chance ?

Why is it needed at all, it seems rt6_update_expires() is redundant with
dst_set_expires()

> +
> +static inline void rt6_set_from(struct rt6_info *rt, struct rt6_info *from)
> +{
> +	if (!(rt->rt6i_flags & RTF_EXPIRES) && rt->dst.from) {
> +		if (from == rt->dst.from)
> +			return;

after a "return;" you dont need an "else" 

> +		else
> +			dst_release((struct dst_entry *) &rt->dst.from);

Really this cast hides a real bug... Was this patch tested ?

> +	}
> +
> +	rt->rt6i_flags &= ~RTF_EXPIRES;
> +	rt->dst.from = (void *) from;
> +	dst_hold(&from->dst);

You hold a reference on the "from" dst, which is fine, but some previous
releases are done on dst_release(&rt->dst). So you dont release the
right dst and bad things happen.

I am not really convinced by this patch, too many issues in it.

Please take the time to make sure you submit a nice one on your next
submission. This part of the code is complex and need top quality
patches.

Re: [PATCH v3] ipv6: Fix problem with expired dst cache

[Gao feng]

Hi Eric:

于 2012年02月29日 20:14, Eric Dumazet 写道:
> Le mercredi 29 février 2012 à 18:07 +0800, Gao feng a écrit :

>> diff --git a/include/net/dst.h b/include/net/dst.h
>> index 344c8dd..5147839 100644
>> --- a/include/net/dst.h
>> +++ b/include/net/dst.h
>> @@ -35,7 +35,16 @@ struct dst_entry {
>>  	struct net_device       *dev;
>>  	struct  dst_ops	        *ops;
>>  	unsigned long		_metrics;
>> -	unsigned long		expires;
>> +
>> +	union {
>> +		unsigned long		expires;
>> +		/*
>> +		 * from is used only for dst cache witch copy form
> 
> 
>> +		 * the dst generated by ipv6 RA.
>> +		 * from is set only when rt6_info has no RTF_EXPIRES flag.
> 
> 
> I am not an english native but really this comment should be reworded...
> 

Sorry...I will reworded it.
> 
>> +		 */
>> +		void			*from;
>> +	};
>>  	struct dst_entry	*path;
>>  	struct neighbour __rcu	*_neighbour;
>>  #ifdef CONFIG_XFRM
>> diff --git a/include/net/ip6_fib.h b/include/net/ip6_fib.h
>> index b26bb81..86cf1ac 100644
>> --- a/include/net/ip6_fib.h
>> +++ b/include/net/ip6_fib.h
>> @@ -123,6 +123,47 @@ static inline struct inet6_dev *ip6_dst_idev(struct dst_entry *dst)
>>  	return ((struct rt6_info *)dst)->rt6i_idev;
>>  }
>>  
>> +static inline void rt6_clean_expires(struct rt6_info *rt)
>> +{
>> +	if (!(rt->rt6i_flags & RTF_EXPIRES) && rt->dst.from)
>> +		dst_release(&rt->dst);
>> +
>> +	rt->rt6i_flags &= ~RTF_EXPIRES;
>> +	rt->dst.expires = 0;
>> +}
>> +
>> +static inline void rt6_set_expires(struct rt6_info *rt, unsigned long expires)
>> +{
>> +	if (!(rt->rt6i_flags & RTF_EXPIRES) && rt->dst.from)
>> +		dst_release(&rt->dst);
>> +
>> +	rt->rt6i_flags |= RTF_EXPIRES;
>> +	rt->dst.expires = expires;
>> +}
>> +
>> +static inline void rt6_update_expires(struct rt6_info *rt, int timeout)
>> +{
>> +	if (!(rt->rt6i_flags & RTF_EXPIRES) && rt->dst.from)
>> +		dst_release(&rt->dst);
>> +
>> +	dst_set_expires(&rt->dst, timeout);
>> +	rt->rt6i_flags |= RTF_EXPIRES;
>> +}
> 
> why rt6_update_expires() takes an "int timeout", promoted to "unsigned
> long expires" ? Do you have a 32bit machine by any chance ?

Because dst_set_expires takes an "int timeout".
rt6_update_expires provides an interface to change rt->rt6i_flags and rt->dst.expires together.
Just like rt6_clean_expires,rt6_set_expires...

> 
> Why is it needed at all, it seems rt6_update_expires() is redundant with
> dst_set_expires()
> 
>> +
>> +static inline void rt6_set_from(struct rt6_info *rt, struct rt6_info *from)
>> +{
>> +	if (!(rt->rt6i_flags & RTF_EXPIRES) && rt->dst.from) {
>> +		if (from == rt->dst.from)
>> +			return;
> 
> after a "return;" you dont need an "else" 
> 
>> +		else
>> +			dst_release((struct dst_entry *) &rt->dst.from);
> 
> Really this cast hides a real bug... Was this patch tested ?
> 

I am very sorry for this,it's my fault.

>> +	}
>> +
>> +	rt->rt6i_flags &= ~RTF_EXPIRES;
>> +	rt->dst.from = (void *) from;
>> +	dst_hold(&from->dst);
> 
> You hold a reference on the "from" dst, which is fine, but some previous
> releases are done on dst_release(&rt->dst). So you dont release the
> right dst and bad things happen.
> 
> I am not really convinced by this patch, too many issues in it.
> 
> Please take the time to make sure you submit a nice one on your next
> submission. This part of the code is complex and need top quality
> patches.
> 


Thanks Eric,I will change this patch carefully,and resend it after test.

[PATCH v4] ipv6: Fix problem with expired dst cache

[Gao feng]

If the ipv6 dst cache which copy from the dst generated by ICMPV6 RA packet.
this dst cache will not check expire because it has no RTF_EXPIRES flag.
So this dst cache will always be used until the dst gc run.

Change the struct dst_entry,add a union contains new pointer from and expires.
When rt6_info.rt6i_flags has no RTF_EXPIRES flag,the dst.expires has no use.
we can use this field to point to where the dst cache copy from.
The dst.from is only used in IPV6.

In func rt6_check_expired check if rt6_info.dst.from is expired.

In func ip6_rt_copy only set dst.from when the ort has flag RTF_ADDRCONF
and RTF_DEFAULT.then hold the ort.

In func ip6_dst_destroy release the ort.

Add some functions to operate the RTF_EXPIRES flag and expires(from) together.
and change the code to use these new adding functions.

Signed-off-by: Gao feng <gaofeng@cn.fujitsu.com>
---
 include/net/dst.h     |    7 ++++++-
 include/net/ip6_fib.h |   40 ++++++++++++++++++++++++++++++++++++++++
 net/ipv6/addrconf.c   |    9 +++------
 net/ipv6/ip6_fib.c    |    3 +--
 net/ipv6/route.c      |   49 +++++++++++++++++++++++++++++++------------------
 5 files changed, 81 insertions(+), 27 deletions(-)

diff --git a/include/net/dst.h b/include/net/dst.h
index 344c8dd..33ebec3 100644
--- a/include/net/dst.h
+++ b/include/net/dst.h
@@ -35,7 +35,12 @@ struct dst_entry {
 	struct net_device       *dev;
 	struct  dst_ops	        *ops;
 	unsigned long		_metrics;
-	unsigned long		expires;
+
+	union {
+		unsigned long		expires;
+		/* point to where the dst_entry copied from */
+		void			*from;
+	};
 	struct dst_entry	*path;
 	struct neighbour __rcu	*_neighbour;
 #ifdef CONFIG_XFRM
diff --git a/include/net/ip6_fib.h b/include/net/ip6_fib.h
index b26bb81..d85ef86 100644
--- a/include/net/ip6_fib.h
+++ b/include/net/ip6_fib.h
@@ -123,6 +123,46 @@ static inline struct inet6_dev *ip6_dst_idev(struct dst_entry *dst)
 	return ((struct rt6_info *)dst)->rt6i_idev;
 }
 
+static inline void rt6_clean_expires(struct rt6_info *rt)
+{
+	if (!(rt->rt6i_flags & RTF_EXPIRES) && rt->dst.from)
+		dst_release((struct dst_entry *) rt->dst.from);
+
+	rt->rt6i_flags &= ~RTF_EXPIRES;
+	rt->dst.expires = 0;
+}
+
+static inline void rt6_set_expires(struct rt6_info *rt, unsigned long expires)
+{
+	if (!(rt->rt6i_flags & RTF_EXPIRES) && rt->dst.from)
+		dst_release((struct dst_entry *) rt->dst.from);
+
+	rt->rt6i_flags |= RTF_EXPIRES;
+	rt->dst.expires = expires;
+}
+
+static inline void rt6_update_expires(struct rt6_info *rt, int timeout)
+{
+	if (!(rt->rt6i_flags & RTF_EXPIRES) && rt->dst.from)
+		dst_release((struct dst_entry *) rt->dst.from);
+
+	dst_set_expires(&rt->dst, timeout);
+	rt->rt6i_flags |= RTF_EXPIRES;
+}
+
+static inline void rt6_set_from(struct rt6_info *rt, struct rt6_info *from)
+{
+	if (!(rt->rt6i_flags & RTF_EXPIRES) && rt->dst.from) {
+		if (from == rt->dst.from)
+			return;
+		dst_release((struct dst_entry *) rt->dst.from);
+	}
+
+	rt->rt6i_flags &= ~RTF_EXPIRES;
+	rt->dst.from = (void *) from;
+	dst_hold(&from->dst);
+}
+
 struct fib6_walker_t {
 	struct list_head lh;
 	struct fib6_node *root, *node;
diff --git a/net/ipv6/addrconf.c b/net/ipv6/addrconf.c
index c02280a..b5cb3b1 100644
--- a/net/ipv6/addrconf.c
+++ b/net/ipv6/addrconf.c
@@ -799,8 +799,7 @@ static void ipv6_del_addr(struct inet6_ifaddr *ifp)
 				ip6_del_rt(rt);
 				rt = NULL;
 			} else if (!(rt->rt6i_flags & RTF_EXPIRES)) {
-				rt->dst.expires = expires;
-				rt->rt6i_flags |= RTF_EXPIRES;
+				rt6_set_expires(rt, expires);
 			}
 		}
 		dst_release(&rt->dst);
@@ -1883,11 +1882,9 @@ void addrconf_prefix_rcv(struct net_device *dev, u8 *opt, int len, bool sllao)
 				rt = NULL;
 			} else if (addrconf_finite_timeout(rt_expires)) {
 				/* not infinity */
-				rt->dst.expires = jiffies + rt_expires;
-				rt->rt6i_flags |= RTF_EXPIRES;
+				rt6_set_expires(rt, jiffies + rt_expires);
 			} else {
-				rt->rt6i_flags &= ~RTF_EXPIRES;
-				rt->dst.expires = 0;
+				rt6_clean_expires(rt);
 			}
 		} else if (valid_lft) {
 			clock_t expires = 0;
diff --git a/net/ipv6/ip6_fib.c b/net/ipv6/ip6_fib.c
index b82bcde..9a4f51e 100644
--- a/net/ipv6/ip6_fib.c
+++ b/net/ipv6/ip6_fib.c
@@ -675,8 +675,7 @@ static int fib6_add_rt2node(struct fib6_node *fn, struct rt6_info *rt,
 					return -EEXIST;
 				iter->dst.expires = rt->dst.expires;
 				if (!(rt->rt6i_flags & RTF_EXPIRES)) {
-					iter->rt6i_flags &= ~RTF_EXPIRES;
-					iter->dst.expires = 0;
+					rt6_clean_expires(iter);
 				}
 				return -EEXIST;
 			}
diff --git a/net/ipv6/route.c b/net/ipv6/route.c
index 8c2e3ab..302a773 100644
--- a/net/ipv6/route.c
+++ b/net/ipv6/route.c
@@ -62,7 +62,7 @@
 #include <linux/sysctl.h>
 #endif
 
-static struct rt6_info *ip6_rt_copy(const struct rt6_info *ort,
+static struct rt6_info *ip6_rt_copy(struct rt6_info *ort,
 				    const struct in6_addr *dest);
 static struct dst_entry	*ip6_dst_check(struct dst_entry *dst, u32 cookie);
 static unsigned int	 ip6_default_advmss(const struct dst_entry *dst);
@@ -272,6 +272,10 @@ static void ip6_dst_destroy(struct dst_entry *dst)
 		rt->rt6i_idev = NULL;
 		in6_dev_put(idev);
 	}
+
+	if (!(rt->rt6i_flags & RTF_EXPIRES) && dst->from)
+		dst_release((struct dst_entry *)dst->from);
+
 	if (peer) {
 		rt->rt6i_peer = NULL;
 		inet_putpeer(peer);
@@ -316,8 +320,18 @@ static void ip6_dst_ifdown(struct dst_entry *dst, struct net_device *dev,
 
 static __inline__ int rt6_check_expired(const struct rt6_info *rt)
 {
-	return (rt->rt6i_flags & RTF_EXPIRES) &&
-		time_after(jiffies, rt->dst.expires);
+	struct rt6_info *ort = NULL;
+
+	if (rt->rt6i_flags & RTF_EXPIRES) {
+		if (time_after(jiffies, rt->dst.expires))
+			return 1;
+	} else if (rt->dst.from) {
+		ort = (struct rt6_info *) rt->dst.from;
+		return (ort->rt6i_flags & RTF_EXPIRES) &&
+			time_after(jiffies, ort->dst.expires);
+	}
+
+	return 0;
 }
 
 static inline int rt6_need_strict(const struct in6_addr *daddr)
@@ -608,10 +622,9 @@ int rt6_route_rcv(struct net_device *dev, u8 *opt, int len,
 
 	if (rt) {
 		if (!addrconf_finite_timeout(lifetime)) {
-			rt->rt6i_flags &= ~RTF_EXPIRES;
+			rt6_clean_expires(rt);
 		} else {
-			rt->dst.expires = jiffies + HZ * lifetime;
-			rt->rt6i_flags |= RTF_EXPIRES;
+			rt6_set_expires(rt, jiffies + HZ * lifetime);
 		}
 		dst_release(&rt->dst);
 	}
@@ -717,7 +730,7 @@ int ip6_ins_rt(struct rt6_info *rt)
 	return __ip6_ins_rt(rt, &info);
 }
 
-static struct rt6_info *rt6_alloc_cow(const struct rt6_info *ort,
+static struct rt6_info *rt6_alloc_cow(struct rt6_info *ort,
 				      const struct in6_addr *daddr,
 				      const struct in6_addr *saddr)
 {
@@ -934,10 +947,10 @@ struct dst_entry *ip6_blackhole_route(struct net *net, struct dst_entry *dst_ori
 		rt->rt6i_idev = ort->rt6i_idev;
 		if (rt->rt6i_idev)
 			in6_dev_hold(rt->rt6i_idev);
-		rt->dst.expires = 0;
 
 		rt->rt6i_gateway = ort->rt6i_gateway;
-		rt->rt6i_flags = ort->rt6i_flags & ~RTF_EXPIRES;
+		rt->rt6i_flags = ort->rt6i_flags;
+		rt6_clean_expires(rt);
 		rt->rt6i_metric = 0;
 
 		memcpy(&rt->rt6i_dst, &ort->rt6i_dst, sizeof(struct rt6key));
@@ -1000,8 +1013,7 @@ static void ip6_link_failure(struct sk_buff *skb)
 	rt = (struct rt6_info *) skb_dst(skb);
 	if (rt) {
 		if (rt->rt6i_flags & RTF_CACHE) {
-			dst_set_expires(&rt->dst, 0);
-			rt->rt6i_flags |= RTF_EXPIRES;
+			rt6_update_expires(rt, 0);
 		} else if (rt->rt6i_node && (rt->rt6i_flags & RTF_DEFAULT))
 			rt->rt6i_node->fn_sernum = -1;
 	}
@@ -1716,8 +1728,8 @@ again:
 			features |= RTAX_FEATURE_ALLFRAG;
 			dst_metric_set(&rt->dst, RTAX_FEATURES, features);
 		}
-		dst_set_expires(&rt->dst, net->ipv6.sysctl.ip6_rt_mtu_expires);
-		rt->rt6i_flags |= RTF_MODIFIED|RTF_EXPIRES;
+		rt6_update_expires(rt, net->ipv6.sysctl.ip6_rt_mtu_expires);
+		rt->rt6i_flags |= RTF_MODIFIED;
 		goto out;
 	}
 
@@ -1745,8 +1757,8 @@ again:
 		 * which is 10 mins. After 10 mins the decreased pmtu is expired
 		 * and detecting PMTU increase will be automatically happened.
 		 */
-		dst_set_expires(&nrt->dst, net->ipv6.sysctl.ip6_rt_mtu_expires);
-		nrt->rt6i_flags |= RTF_DYNAMIC|RTF_EXPIRES;
+		rt6_update_expires(nrt, net->ipv6.sysctl.ip6_rt_mtu_expires);
+		nrt->rt6i_flags |= RTF_DYNAMIC;
 
 		ip6_ins_rt(nrt);
 	}
@@ -1779,7 +1791,7 @@ void rt6_pmtu_discovery(const struct in6_addr *daddr, const struct in6_addr *sad
  *	Misc support functions
  */
 
-static struct rt6_info *ip6_rt_copy(const struct rt6_info *ort,
+static struct rt6_info *ip6_rt_copy(struct rt6_info *ort,
 				    const struct in6_addr *dest)
 {
 	struct net *net = dev_net(ort->dst.dev);
@@ -1799,10 +1811,11 @@ static struct rt6_info *ip6_rt_copy(const struct rt6_info *ort,
 		if (rt->rt6i_idev)
 			in6_dev_hold(rt->rt6i_idev);
 		rt->dst.lastuse = jiffies;
-		rt->dst.expires = 0;
 
 		rt->rt6i_gateway = ort->rt6i_gateway;
-		rt->rt6i_flags = ort->rt6i_flags & ~RTF_EXPIRES;
+		rt->rt6i_flags = ort->rt6i_flags;
+		rt6_set_from(rt, ort);
+
 		rt->rt6i_metric = 0;
 
 #ifdef CONFIG_IPV6_SUBTREES
-- 
1.7.5.4

Re: [PATCH v4] ipv6: Fix problem with expired dst cache

[David Miller]

From: Gao feng <gaofeng@cn.fujitsu.com>
Date: Mon, 5 Mar 2012 11:53:48 +0800

> +		/* point to where the dst_entry copied from */
> +		void			*from;

So instead of using a real type here, your just going to cast
this thing to and from "struct dst_entry *" a thousand times?

That's terrible, there is zero value from using a void pointer
here, it just makes the code look ugly.

Re: [PATCH v4] ipv6: Fix problem with expired dst cache

[Gao feng]

于 2012年03月05日 13:05, David Miller 写道:
> From: Gao feng <gaofeng@cn.fujitsu.com>
> Date: Mon, 5 Mar 2012 11:53:48 +0800
> 
>> +		/* point to where the dst_entry copied from */
>> +		void			*from;
> 
> So instead of using a real type here, your just going to cast
> this thing to and from "struct dst_entry *" a thousand times?
> 
> That's terrible, there is zero value from using a void pointer
> here, it just makes the code look ugly.
> --
> To unsubscribe from this list: send the line "unsubscribe netdev" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 

Hi David
You are right.I will resend this patch changes void * to dst_entry *.

thanks for your patience.

[PATCH v5] ipv6: Fix problem with expired dst cache

[Gao feng]

If the ipv6 dst cache which copy from the dst generated by ICMPV6 RA packet.
this dst cache will not check expire because it has no RTF_EXPIRES flag.
So this dst cache will always be used until the dst gc run.

Change the struct dst_entry,add a union contains new pointer from and expires.
When rt6_info.rt6i_flags has no RTF_EXPIRES flag,the dst.expires has no use.
we can use this field to point to where the dst cache copy from.
The dst.from is only used in IPV6.

In func rt6_check_expired check if rt6_info.dst.from is expired.

In func ip6_rt_copy only set dst.from when the ort has flag RTF_ADDRCONF
and RTF_DEFAULT.then hold the ort.

In func ip6_dst_destroy release the ort.

Add some functions to operate the RTF_EXPIRES flag and expires(from) together.
and change the code to use these new adding functions.

Signed-off-by: Gao feng <gaofeng@cn.fujitsu.com>
---
 include/net/dst.h     |    7 ++++++-
 include/net/ip6_fib.h |   42 ++++++++++++++++++++++++++++++++++++++++++
 net/ipv6/addrconf.c   |    9 +++------
 net/ipv6/ip6_fib.c    |    3 +--
 net/ipv6/route.c      |   49 +++++++++++++++++++++++++++++++------------------
 5 files changed, 83 insertions(+), 27 deletions(-)

diff --git a/include/net/dst.h b/include/net/dst.h
index 344c8dd..864cd18 100644
--- a/include/net/dst.h
+++ b/include/net/dst.h
@@ -35,7 +35,12 @@ struct dst_entry {
 	struct net_device       *dev;
 	struct  dst_ops	        *ops;
 	unsigned long		_metrics;
-	unsigned long		expires;
+
+	union {
+		unsigned long		expires;
+		/* point to where the dst_entry copied from */
+		struct dst_entry	*from;
+	};
 	struct dst_entry	*path;
 	struct neighbour __rcu	*_neighbour;
 #ifdef CONFIG_XFRM
diff --git a/include/net/ip6_fib.h b/include/net/ip6_fib.h
index b26bb81..07e578e 100644
--- a/include/net/ip6_fib.h
+++ b/include/net/ip6_fib.h
@@ -123,6 +123,48 @@ static inline struct inet6_dev *ip6_dst_idev(struct dst_entry *dst)
 	return ((struct rt6_info *)dst)->rt6i_idev;
 }
 
+static inline void rt6_clean_expires(struct rt6_info *rt)
+{
+	if (!(rt->rt6i_flags & RTF_EXPIRES) && rt->dst.from)
+		dst_release(rt->dst.from);
+
+	rt->rt6i_flags &= ~RTF_EXPIRES;
+	rt->dst.expires = 0;
+}
+
+static inline void rt6_set_expires(struct rt6_info *rt, unsigned long expires)
+{
+	if (!(rt->rt6i_flags & RTF_EXPIRES) && rt->dst.from)
+		dst_release(rt->dst.from);
+
+	rt->rt6i_flags |= RTF_EXPIRES;
+	rt->dst.expires = expires;
+}
+
+static inline void rt6_update_expires(struct rt6_info *rt, int timeout)
+{
+	if (!(rt->rt6i_flags & RTF_EXPIRES) && rt->dst.from)
+		dst_release(rt->dst.from);
+
+	dst_set_expires(&rt->dst, timeout);
+	rt->rt6i_flags |= RTF_EXPIRES;
+}
+
+static inline void rt6_set_from(struct rt6_info *rt, struct rt6_info *from)
+{
+	struct dst_entry *new = (struct dst_entry *) from;
+
+	if (!(rt->rt6i_flags & RTF_EXPIRES) && rt->dst.from) {
+		if (new == rt->dst.from)
+			return;
+		dst_release(rt->dst.from);
+	}
+
+	rt->rt6i_flags &= ~RTF_EXPIRES;
+	rt->dst.from = new;
+	dst_hold(new);
+}
+
 struct fib6_walker_t {
 	struct list_head lh;
 	struct fib6_node *root, *node;
diff --git a/net/ipv6/addrconf.c b/net/ipv6/addrconf.c
index c02280a..b5cb3b1 100644
--- a/net/ipv6/addrconf.c
+++ b/net/ipv6/addrconf.c
@@ -799,8 +799,7 @@ static void ipv6_del_addr(struct inet6_ifaddr *ifp)
 				ip6_del_rt(rt);
 				rt = NULL;
 			} else if (!(rt->rt6i_flags & RTF_EXPIRES)) {
-				rt->dst.expires = expires;
-				rt->rt6i_flags |= RTF_EXPIRES;
+				rt6_set_expires(rt, expires);
 			}
 		}
 		dst_release(&rt->dst);
@@ -1883,11 +1882,9 @@ void addrconf_prefix_rcv(struct net_device *dev, u8 *opt, int len, bool sllao)
 				rt = NULL;
 			} else if (addrconf_finite_timeout(rt_expires)) {
 				/* not infinity */
-				rt->dst.expires = jiffies + rt_expires;
-				rt->rt6i_flags |= RTF_EXPIRES;
+				rt6_set_expires(rt, jiffies + rt_expires);
 			} else {
-				rt->rt6i_flags &= ~RTF_EXPIRES;
-				rt->dst.expires = 0;
+				rt6_clean_expires(rt);
 			}
 		} else if (valid_lft) {
 			clock_t expires = 0;
diff --git a/net/ipv6/ip6_fib.c b/net/ipv6/ip6_fib.c
index b82bcde..9a4f51e 100644
--- a/net/ipv6/ip6_fib.c
+++ b/net/ipv6/ip6_fib.c
@@ -675,8 +675,7 @@ static int fib6_add_rt2node(struct fib6_node *fn, struct rt6_info *rt,
 					return -EEXIST;
 				iter->dst.expires = rt->dst.expires;
 				if (!(rt->rt6i_flags & RTF_EXPIRES)) {
-					iter->rt6i_flags &= ~RTF_EXPIRES;
-					iter->dst.expires = 0;
+					rt6_clean_expires(iter);
 				}
 				return -EEXIST;
 			}
diff --git a/net/ipv6/route.c b/net/ipv6/route.c
index 8c2e3ab..dd472f3 100644
--- a/net/ipv6/route.c
+++ b/net/ipv6/route.c
@@ -62,7 +62,7 @@
 #include <linux/sysctl.h>
 #endif
 
-static struct rt6_info *ip6_rt_copy(const struct rt6_info *ort,
+static struct rt6_info *ip6_rt_copy(struct rt6_info *ort,
 				    const struct in6_addr *dest);
 static struct dst_entry	*ip6_dst_check(struct dst_entry *dst, u32 cookie);
 static unsigned int	 ip6_default_advmss(const struct dst_entry *dst);
@@ -272,6 +272,10 @@ static void ip6_dst_destroy(struct dst_entry *dst)
 		rt->rt6i_idev = NULL;
 		in6_dev_put(idev);
 	}
+
+	if (!(rt->rt6i_flags & RTF_EXPIRES) && dst->from)
+		dst_release(dst->from);
+
 	if (peer) {
 		rt->rt6i_peer = NULL;
 		inet_putpeer(peer);
@@ -316,8 +320,18 @@ static void ip6_dst_ifdown(struct dst_entry *dst, struct net_device *dev,
 
 static __inline__ int rt6_check_expired(const struct rt6_info *rt)
 {
-	return (rt->rt6i_flags & RTF_EXPIRES) &&
-		time_after(jiffies, rt->dst.expires);
+	struct rt6_info *ort = NULL;
+
+	if (rt->rt6i_flags & RTF_EXPIRES) {
+		if (time_after(jiffies, rt->dst.expires))
+			return 1;
+	} else if (rt->dst.from) {
+		ort = (struct rt6_info *) rt->dst.from;
+		return (ort->rt6i_flags & RTF_EXPIRES) &&
+			time_after(jiffies, ort->dst.expires);
+	}
+
+	return 0;
 }
 
 static inline int rt6_need_strict(const struct in6_addr *daddr)
@@ -608,10 +622,9 @@ int rt6_route_rcv(struct net_device *dev, u8 *opt, int len,
 
 	if (rt) {
 		if (!addrconf_finite_timeout(lifetime)) {
-			rt->rt6i_flags &= ~RTF_EXPIRES;
+			rt6_clean_expires(rt);
 		} else {
-			rt->dst.expires = jiffies + HZ * lifetime;
-			rt->rt6i_flags |= RTF_EXPIRES;
+			rt6_set_expires(rt, jiffies + HZ * lifetime);
 		}
 		dst_release(&rt->dst);
 	}
@@ -717,7 +730,7 @@ int ip6_ins_rt(struct rt6_info *rt)
 	return __ip6_ins_rt(rt, &info);
 }
 
-static struct rt6_info *rt6_alloc_cow(const struct rt6_info *ort,
+static struct rt6_info *rt6_alloc_cow(struct rt6_info *ort,
 				      const struct in6_addr *daddr,
 				      const struct in6_addr *saddr)
 {
@@ -934,10 +947,10 @@ struct dst_entry *ip6_blackhole_route(struct net *net, struct dst_entry *dst_ori
 		rt->rt6i_idev = ort->rt6i_idev;
 		if (rt->rt6i_idev)
 			in6_dev_hold(rt->rt6i_idev);
-		rt->dst.expires = 0;
 
 		rt->rt6i_gateway = ort->rt6i_gateway;
-		rt->rt6i_flags = ort->rt6i_flags & ~RTF_EXPIRES;
+		rt->rt6i_flags = ort->rt6i_flags;
+		rt6_clean_expires(rt);
 		rt->rt6i_metric = 0;
 
 		memcpy(&rt->rt6i_dst, &ort->rt6i_dst, sizeof(struct rt6key));
@@ -1000,8 +1013,7 @@ static void ip6_link_failure(struct sk_buff *skb)
 	rt = (struct rt6_info *) skb_dst(skb);
 	if (rt) {
 		if (rt->rt6i_flags & RTF_CACHE) {
-			dst_set_expires(&rt->dst, 0);
-			rt->rt6i_flags |= RTF_EXPIRES;
+			rt6_update_expires(rt, 0);
 		} else if (rt->rt6i_node && (rt->rt6i_flags & RTF_DEFAULT))
 			rt->rt6i_node->fn_sernum = -1;
 	}
@@ -1716,8 +1728,8 @@ again:
 			features |= RTAX_FEATURE_ALLFRAG;
 			dst_metric_set(&rt->dst, RTAX_FEATURES, features);
 		}
-		dst_set_expires(&rt->dst, net->ipv6.sysctl.ip6_rt_mtu_expires);
-		rt->rt6i_flags |= RTF_MODIFIED|RTF_EXPIRES;
+		rt6_update_expires(rt, net->ipv6.sysctl.ip6_rt_mtu_expires);
+		rt->rt6i_flags |= RTF_MODIFIED;
 		goto out;
 	}
 
@@ -1745,8 +1757,8 @@ again:
 		 * which is 10 mins. After 10 mins the decreased pmtu is expired
 		 * and detecting PMTU increase will be automatically happened.
 		 */
-		dst_set_expires(&nrt->dst, net->ipv6.sysctl.ip6_rt_mtu_expires);
-		nrt->rt6i_flags |= RTF_DYNAMIC|RTF_EXPIRES;
+		rt6_update_expires(nrt, net->ipv6.sysctl.ip6_rt_mtu_expires);
+		nrt->rt6i_flags |= RTF_DYNAMIC;
 
 		ip6_ins_rt(nrt);
 	}
@@ -1779,7 +1791,7 @@ void rt6_pmtu_discovery(const struct in6_addr *daddr, const struct in6_addr *sad
  *	Misc support functions
  */
 
-static struct rt6_info *ip6_rt_copy(const struct rt6_info *ort,
+static struct rt6_info *ip6_rt_copy(struct rt6_info *ort,
 				    const struct in6_addr *dest)
 {
 	struct net *net = dev_net(ort->dst.dev);
@@ -1799,10 +1811,11 @@ static struct rt6_info *ip6_rt_copy(const struct rt6_info *ort,
 		if (rt->rt6i_idev)
 			in6_dev_hold(rt->rt6i_idev);
 		rt->dst.lastuse = jiffies;
-		rt->dst.expires = 0;
 
 		rt->rt6i_gateway = ort->rt6i_gateway;
-		rt->rt6i_flags = ort->rt6i_flags & ~RTF_EXPIRES;
+		rt->rt6i_flags = ort->rt6i_flags;
+		rt6_set_from(rt, ort);
+
 		rt->rt6i_metric = 0;
 
 #ifdef CONFIG_IPV6_SUBTREES
-- 
1.7.5.4

Re: [PATCH v5] ipv6: Fix problem with expired dst cache

[RongQing Li]

2012/3/5 Gao feng <gaofeng@cn.fujitsu.com>:
> If the ipv6 dst cache which copy from the dst generated by ICMPV6 RA packet.
> this dst cache will not check expire because it has no RTF_EXPIRES flag.
> So this dst cache will always be used until the dst gc run.
>
> Change the struct dst_entry,add a union contains new pointer from and expires.
> When rt6_info.rt6i_flags has no RTF_EXPIRES flag,the dst.expires has no use.
> we can use this field to point to where the dst cache copy from.
> The dst.from is only used in IPV6.
>
> In func rt6_check_expired check if rt6_info.dst.from is expired.
>
> In func ip6_rt_copy only set dst.from when the ort has flag RTF_ADDRCONF
> and RTF_DEFAULT.then hold the ort.
>
> In func ip6_dst_destroy release the ort.
>
> Add some functions to operate the RTF_EXPIRES flag and expires(from) together.
> and change the code to use these new adding functions.
>
> Signed-off-by: Gao feng <gaofeng@cn.fujitsu.com>
> ---
>  include/net/dst.h     |    7 ++++++-
>  include/net/ip6_fib.h |   42 ++++++++++++++++++++++++++++++++++++++++++
>  net/ipv6/addrconf.c   |    9 +++------
>  net/ipv6/ip6_fib.c    |    3 +--
>  net/ipv6/route.c      |   49 +++++++++++++++++++++++++++++++------------------
>  5 files changed, 83 insertions(+), 27 deletions(-)
>
> diff --git a/include/net/dst.h b/include/net/dst.h
> index 344c8dd..864cd18 100644
> --- a/include/net/dst.h
> +++ b/include/net/dst.h
> @@ -35,7 +35,12 @@ struct dst_entry {
>        struct net_device       *dev;
>        struct  dst_ops         *ops;
>        unsigned long           _metrics;
> -       unsigned long           expires;
> +
> +       union {
> +               unsigned long           expires;
> +               /* point to where the dst_entry copied from */
> +               struct dst_entry        *from;
> +       };
>        struct dst_entry        *path;
>        struct neighbour __rcu  *_neighbour;
>  #ifdef CONFIG_XFRM
> diff --git a/include/net/ip6_fib.h b/include/net/ip6_fib.h
> index b26bb81..07e578e 100644
> --- a/include/net/ip6_fib.h
> +++ b/include/net/ip6_fib.h
> @@ -123,6 +123,48 @@ static inline struct inet6_dev *ip6_dst_idev(struct dst_entry *dst)
>        return ((struct rt6_info *)dst)->rt6i_idev;
>  }
>
> +static inline void rt6_clean_expires(struct rt6_info *rt)
> +{
> +       if (!(rt->rt6i_flags & RTF_EXPIRES) && rt->dst.from)
> +               dst_release(rt->dst.from);
> +
> +       rt->rt6i_flags &= ~RTF_EXPIRES;
> +       rt->dst.expires = 0;
> +}
> +
> +static inline void rt6_set_expires(struct rt6_info *rt, unsigned long expires)
> +{
> +       if (!(rt->rt6i_flags & RTF_EXPIRES) && rt->dst.from)
> +               dst_release(rt->dst.from);
> +
> +       rt->rt6i_flags |= RTF_EXPIRES;
> +       rt->dst.expires = expires;
> +}
> +
> +static inline void rt6_update_expires(struct rt6_info *rt, int timeout)
> +{
> +       if (!(rt->rt6i_flags & RTF_EXPIRES) && rt->dst.from)
> +               dst_release(rt->dst.from);
> +
> +       dst_set_expires(&rt->dst, timeout);
> +       rt->rt6i_flags |= RTF_EXPIRES;
> +}
> +

Calling dst_set_expires() in  rt6_update_expires() will double release
rt->dst.from?

-Roy

> +static inline void rt6_set_from(struct rt6_info *rt, struct rt6_info *from)
> +{
> +       struct dst_entry *new = (struct dst_entry *) from;
> +
> +       if (!(rt->rt6i_flags & RTF_EXPIRES) && rt->dst.from) {
> +               if (new == rt->dst.from)
> +                       return;
> +               dst_release(rt->dst.from);
> +       }
> +
> +       rt->rt6i_flags &= ~RTF_EXPIRES;
> +       rt->dst.from = new;
> +       dst_hold(new);
> +}
> +
>  struct fib6_walker_t {
>        struct list_head lh;
>        struct fib6_node *root, *node;
> diff --git a/net/ipv6/addrconf.c b/net/ipv6/addrconf.c
> index c02280a..b5cb3b1 100644
> --- a/net/ipv6/addrconf.c
> +++ b/net/ipv6/addrconf.c
> @@ -799,8 +799,7 @@ static void ipv6_del_addr(struct inet6_ifaddr *ifp)
>                                ip6_del_rt(rt);
>                                rt = NULL;
>                        } else if (!(rt->rt6i_flags & RTF_EXPIRES)) {
> -                               rt->dst.expires = expires;
> -                               rt->rt6i_flags |= RTF_EXPIRES;
> +                               rt6_set_expires(rt, expires);
>                        }
>                }
>                dst_release(&rt->dst);
> @@ -1883,11 +1882,9 @@ void addrconf_prefix_rcv(struct net_device *dev, u8 *opt, int len, bool sllao)
>                                rt = NULL;
>                        } else if (addrconf_finite_timeout(rt_expires)) {
>                                /* not infinity */
> -                               rt->dst.expires = jiffies + rt_expires;
> -                               rt->rt6i_flags |= RTF_EXPIRES;
> +                               rt6_set_expires(rt, jiffies + rt_expires);
>                        } else {
> -                               rt->rt6i_flags &= ~RTF_EXPIRES;
> -                               rt->dst.expires = 0;
> +                               rt6_clean_expires(rt);
>                        }
>                } else if (valid_lft) {
>                        clock_t expires = 0;
> diff --git a/net/ipv6/ip6_fib.c b/net/ipv6/ip6_fib.c
> index b82bcde..9a4f51e 100644
> --- a/net/ipv6/ip6_fib.c
> +++ b/net/ipv6/ip6_fib.c
> @@ -675,8 +675,7 @@ static int fib6_add_rt2node(struct fib6_node *fn, struct rt6_info *rt,
>                                        return -EEXIST;
>                                iter->dst.expires = rt->dst.expires;
>                                if (!(rt->rt6i_flags & RTF_EXPIRES)) {
> -                                       iter->rt6i_flags &= ~RTF_EXPIRES;
> -                                       iter->dst.expires = 0;
> +                                       rt6_clean_expires(iter);
>                                }
>                                return -EEXIST;
>                        }
> diff --git a/net/ipv6/route.c b/net/ipv6/route.c
> index 8c2e3ab..dd472f3 100644
> --- a/net/ipv6/route.c
> +++ b/net/ipv6/route.c
> @@ -62,7 +62,7 @@
>  #include <linux/sysctl.h>
>  #endif
>
> -static struct rt6_info *ip6_rt_copy(const struct rt6_info *ort,
> +static struct rt6_info *ip6_rt_copy(struct rt6_info *ort,
>                                    const struct in6_addr *dest);
>  static struct dst_entry        *ip6_dst_check(struct dst_entry *dst, u32 cookie);
>  static unsigned int     ip6_default_advmss(const struct dst_entry *dst);
> @@ -272,6 +272,10 @@ static void ip6_dst_destroy(struct dst_entry *dst)
>                rt->rt6i_idev = NULL;
>                in6_dev_put(idev);
>        }
> +
> +       if (!(rt->rt6i_flags & RTF_EXPIRES) && dst->from)
> +               dst_release(dst->from);
> +
>        if (peer) {
>                rt->rt6i_peer = NULL;
>                inet_putpeer(peer);
> @@ -316,8 +320,18 @@ static void ip6_dst_ifdown(struct dst_entry *dst, struct net_device *dev,
>
>  static __inline__ int rt6_check_expired(const struct rt6_info *rt)
>  {
> -       return (rt->rt6i_flags & RTF_EXPIRES) &&
> -               time_after(jiffies, rt->dst.expires);
> +       struct rt6_info *ort = NULL;
> +
> +       if (rt->rt6i_flags & RTF_EXPIRES) {
> +               if (time_after(jiffies, rt->dst.expires))
> +                       return 1;
> +       } else if (rt->dst.from) {
> +               ort = (struct rt6_info *) rt->dst.from;
> +               return (ort->rt6i_flags & RTF_EXPIRES) &&
> +                       time_after(jiffies, ort->dst.expires);
> +       }
> +
> +       return 0;
>  }
>
>  static inline int rt6_need_strict(const struct in6_addr *daddr)
> @@ -608,10 +622,9 @@ int rt6_route_rcv(struct net_device *dev, u8 *opt, int len,
>
>        if (rt) {
>                if (!addrconf_finite_timeout(lifetime)) {
> -                       rt->rt6i_flags &= ~RTF_EXPIRES;
> +                       rt6_clean_expires(rt);
>                } else {
> -                       rt->dst.expires = jiffies + HZ * lifetime;
> -                       rt->rt6i_flags |= RTF_EXPIRES;
> +                       rt6_set_expires(rt, jiffies + HZ * lifetime);
>                }
>                dst_release(&rt->dst);
>        }
> @@ -717,7 +730,7 @@ int ip6_ins_rt(struct rt6_info *rt)
>        return __ip6_ins_rt(rt, &info);
>  }
>
> -static struct rt6_info *rt6_alloc_cow(const struct rt6_info *ort,
> +static struct rt6_info *rt6_alloc_cow(struct rt6_info *ort,
>                                      const struct in6_addr *daddr,
>                                      const struct in6_addr *saddr)
>  {
> @@ -934,10 +947,10 @@ struct dst_entry *ip6_blackhole_route(struct net *net, struct dst_entry *dst_ori
>                rt->rt6i_idev = ort->rt6i_idev;
>                if (rt->rt6i_idev)
>                        in6_dev_hold(rt->rt6i_idev);
> -               rt->dst.expires = 0;
>
>                rt->rt6i_gateway = ort->rt6i_gateway;
> -               rt->rt6i_flags = ort->rt6i_flags & ~RTF_EXPIRES;
> +               rt->rt6i_flags = ort->rt6i_flags;
> +               rt6_clean_expires(rt);
>                rt->rt6i_metric = 0;
>
>                memcpy(&rt->rt6i_dst, &ort->rt6i_dst, sizeof(struct rt6key));
> @@ -1000,8 +1013,7 @@ static void ip6_link_failure(struct sk_buff *skb)
>        rt = (struct rt6_info *) skb_dst(skb);
>        if (rt) {
>                if (rt->rt6i_flags & RTF_CACHE) {
> -                       dst_set_expires(&rt->dst, 0);
> -                       rt->rt6i_flags |= RTF_EXPIRES;
> +                       rt6_update_expires(rt, 0);
>                } else if (rt->rt6i_node && (rt->rt6i_flags & RTF_DEFAULT))
>                        rt->rt6i_node->fn_sernum = -1;
>        }
> @@ -1716,8 +1728,8 @@ again:
>                        features |= RTAX_FEATURE_ALLFRAG;
>                        dst_metric_set(&rt->dst, RTAX_FEATURES, features);
>                }
> -               dst_set_expires(&rt->dst, net->ipv6.sysctl.ip6_rt_mtu_expires);
> -               rt->rt6i_flags |= RTF_MODIFIED|RTF_EXPIRES;
> +               rt6_update_expires(rt, net->ipv6.sysctl.ip6_rt_mtu_expires);
> +               rt->rt6i_flags |= RTF_MODIFIED;
>                goto out;
>        }
>
> @@ -1745,8 +1757,8 @@ again:
>                 * which is 10 mins. After 10 mins the decreased pmtu is expired
>                 * and detecting PMTU increase will be automatically happened.
>                 */
> -               dst_set_expires(&nrt->dst, net->ipv6.sysctl.ip6_rt_mtu_expires);
> -               nrt->rt6i_flags |= RTF_DYNAMIC|RTF_EXPIRES;
> +               rt6_update_expires(nrt, net->ipv6.sysctl.ip6_rt_mtu_expires);
> +               nrt->rt6i_flags |= RTF_DYNAMIC;
>
>                ip6_ins_rt(nrt);
>        }
> @@ -1779,7 +1791,7 @@ void rt6_pmtu_discovery(const struct in6_addr *daddr, const struct in6_addr *sad
>  *     Misc support functions
>  */
>
> -static struct rt6_info *ip6_rt_copy(const struct rt6_info *ort,
> +static struct rt6_info *ip6_rt_copy(struct rt6_info *ort,
>                                    const struct in6_addr *dest)
>  {
>        struct net *net = dev_net(ort->dst.dev);
> @@ -1799,10 +1811,11 @@ static struct rt6_info *ip6_rt_copy(const struct rt6_info *ort,
>                if (rt->rt6i_idev)
>                        in6_dev_hold(rt->rt6i_idev);
>                rt->dst.lastuse = jiffies;
> -               rt->dst.expires = 0;
>
>                rt->rt6i_gateway = ort->rt6i_gateway;
> -               rt->rt6i_flags = ort->rt6i_flags & ~RTF_EXPIRES;
> +               rt->rt6i_flags = ort->rt6i_flags;
> +               rt6_set_from(rt, ort);
> +
>                rt->rt6i_metric = 0;
>
>  #ifdef CONFIG_IPV6_SUBTREES
> --
> 1.7.5.4
>
> --
> To unsubscribe from this list: send the line "unsubscribe netdev" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v5] ipv6: Fix problem with expired dst cache

[RongQing Li]

> Calling dst_set_expires() in  rt6_update_expires() will double release
> rt->dst.from?
>
> -Roy

Sorry,  Please ignore it.

Re: [PATCH v5] ipv6: Fix problem with expired dst cache

[David Miller]

From: Gao feng <gaofeng@cn.fujitsu.com>
Date: Mon, 5 Mar 2012 15:16:02 +0800

> If the ipv6 dst cache which copy from the dst generated by ICMPV6 RA packet.
> this dst cache will not check expire because it has no RTF_EXPIRES flag.
> So this dst cache will always be used until the dst gc run.
> 
> Change the struct dst_entry,add a union contains new pointer from and expires.
> When rt6_info.rt6i_flags has no RTF_EXPIRES flag,the dst.expires has no use.
> we can use this field to point to where the dst cache copy from.
> The dst.from is only used in IPV6.
> 
> In func rt6_check_expired check if rt6_info.dst.from is expired.
> 
> In func ip6_rt_copy only set dst.from when the ort has flag RTF_ADDRCONF
> and RTF_DEFAULT.then hold the ort.
> 
> In func ip6_dst_destroy release the ort.
> 
> Add some functions to operate the RTF_EXPIRES flag and expires(from) together.
> and change the code to use these new adding functions.
> 
> Signed-off-by: Gao feng <gaofeng@cn.fujitsu.com>

I see some unprotected access to dst.expires in the router discovery
code in net/ipv6/ndisc.c, doesn't that need to be updated?

There are probably some more similar cases elsewhere in the ipv6 code
too.

Re: [PATCH v5] ipv6: Fix problem with expired dst cache

[Gao feng]

于 2012年03月17日 13:33, David Miller 写道:
> From: Gao feng <gaofeng@cn.fujitsu.com>
> Date: Mon, 5 Mar 2012 15:16:02 +0800
> 
>> If the ipv6 dst cache which copy from the dst generated by ICMPV6 RA packet.
>> this dst cache will not check expire because it has no RTF_EXPIRES flag.
>> So this dst cache will always be used until the dst gc run.
>>
>> Change the struct dst_entry,add a union contains new pointer from and expires.
>> When rt6_info.rt6i_flags has no RTF_EXPIRES flag,the dst.expires has no use.
>> we can use this field to point to where the dst cache copy from.
>> The dst.from is only used in IPV6.
>>
>> In func rt6_check_expired check if rt6_info.dst.from is expired.
>>
>> In func ip6_rt_copy only set dst.from when the ort has flag RTF_ADDRCONF
>> and RTF_DEFAULT.then hold the ort.
>>
>> In func ip6_dst_destroy release the ort.
>>
>> Add some functions to operate the RTF_EXPIRES flag and expires(from) together.
>> and change the code to use these new adding functions.
>>
>> Signed-off-by: Gao feng <gaofeng@cn.fujitsu.com>
> 
> I see some unprotected access to dst.expires in the router discovery
> code in net/ipv6/ndisc.c, doesn't that need to be updated?
> 
> There are probably some more similar cases elsewhere in the ipv6 code
> too.
> 

Hi David

I only search the RTF_EXPIRES flag and forget the expires.I will do this.

BUT what confuse me is that, in func ip6_rt_copy should we do rt6_set_from in any case
or only when the ort has flag RTF_ADDRCONF and RTF_DEFAULT?

thanks.

Re: [PATCH v5] ipv6: Fix problem with expired dst cache

[David Miller]

From: Gao feng <gaofeng@cn.fujitsu.com>
Date: Mon, 19 Mar 2012 08:49:51 +0800

> BUT what confuse me is that, in func ip6_rt_copy should we do
> rt6_set_from in any case or only when the ort has flag RTF_ADDRCONF
> and RTF_DEFAULT?

Your guess is as good as mine, unfortunately.  A lot of code in
this area is hard to decipher.

For example, I've spent the past several months trying to figure out
which kinds of ipv6 routes have explicit neighbour entries attached at
route insert time, which do not, etc.

[PATCH v6] ipv6: fix problem with expired dst cache

[Gao feng]

If the ipv6 dst cache which copy from the dst generated by ICMPV6 RA packet.
this dst cache will not check expire because it has no RTF_EXPIRES flag.
So this dst cache will always be used until the dst gc run.

Change the struct dst_entry,add a union contains new pointer from and expires.
When rt6_info.rt6i_flags has no RTF_EXPIRES flag,the dst.expires has no use.
we can use this field to point to where the dst cache copy from.
The dst.from is only used in IPV6.

rt6_check_expired check if rt6_info.dst.from is expired.

ip6_rt_copy only set dst.from when the ort has flag RTF_ADDRCONF
and RTF_DEFAULT.then hold the ort.

ip6_dst_destroy release the ort.

Add some functions to operate the RTF_EXPIRES flag and expires(from) together.
and change the code to use these new adding functions.

Changes from v5:
modify ip6_route_add and ndisc_router_discovery to use new adding functions.

Only set dst.from when the ort has flag RTF_ADDRCONF
and RTF_DEFAULT.then hold the ort.

Signed-off-by: Gao feng <gaofeng@cn.fujitsu.com>
---
 include/net/dst.h     |    6 +++-
 include/net/ip6_fib.h |   42 +++++++++++++++++++++++++++++
 net/ipv6/addrconf.c   |    9 ++----
 net/ipv6/ip6_fib.c    |    9 +++---
 net/ipv6/ndisc.c      |    3 +-
 net/ipv6/route.c      |   71 ++++++++++++++++++++++++++++++------------------
 6 files changed, 99 insertions(+), 41 deletions(-)

diff --git a/include/net/dst.h b/include/net/dst.h
index 59c5d18..ff4da42 100644
--- a/include/net/dst.h
+++ b/include/net/dst.h
@@ -36,7 +36,11 @@ struct dst_entry {
 	struct net_device       *dev;
 	struct  dst_ops	        *ops;
 	unsigned long		_metrics;
-	unsigned long		expires;
+	union {
+		unsigned long           expires;
+		/* point to where the dst_entry copied from */
+		struct dst_entry        *from;
+	};
 	struct dst_entry	*path;
 	struct neighbour __rcu	*_neighbour;
 #ifdef CONFIG_XFRM
diff --git a/include/net/ip6_fib.h b/include/net/ip6_fib.h
index b26bb81..c64778f 100644
--- a/include/net/ip6_fib.h
+++ b/include/net/ip6_fib.h
@@ -123,6 +123,48 @@ static inline struct inet6_dev *ip6_dst_idev(struct dst_entry *dst)
 	return ((struct rt6_info *)dst)->rt6i_idev;
 }
 
+static inline void rt6_clean_expires(struct rt6_info *rt)
+{
+	if (!(rt->rt6i_flags & RTF_EXPIRES) && rt->dst.from)
+		dst_release(rt->dst.from);
+
+	rt->rt6i_flags &= ~RTF_EXPIRES;
+	rt->dst.expires = 0;
+}
+
+static inline void rt6_set_expires(struct rt6_info *rt, unsigned long expires)
+{
+	if (!(rt->rt6i_flags & RTF_EXPIRES) && rt->dst.from)
+		dst_release(rt->dst.from);
+
+	rt->rt6i_flags |= RTF_EXPIRES;
+	rt->dst.expires = expires;
+}
+
+static inline void rt6_update_expires(struct rt6_info *rt, int timeout)
+{
+	if (!(rt->rt6i_flags & RTF_EXPIRES) && rt->dst.from)
+		dst_release(rt->dst.from);
+
+	dst_set_expires(&rt->dst, timeout);
+	rt->rt6i_flags |= RTF_EXPIRES;
+}
+
+static inline void rt6_set_from(struct rt6_info *rt, struct rt6_info *from)
+{
+	struct dst_entry *new = (struct dst_entry *) from;
+
+	if (!(rt->rt6i_flags & RTF_EXPIRES) && rt->dst.from) {
+		if (new == rt->dst.from)
+			return;
+		dst_release(rt->dst.from);
+	}
+
+	rt->rt6i_flags &= ~RTF_EXPIRES;
+	rt->dst.from = new;
+	dst_hold(new);
+}
+
 struct fib6_walker_t {
 	struct list_head lh;
 	struct fib6_node *root, *node;
diff --git a/net/ipv6/addrconf.c b/net/ipv6/addrconf.c
index 6a3bb60..7d5cb97 100644
--- a/net/ipv6/addrconf.c
+++ b/net/ipv6/addrconf.c
@@ -803,8 +803,7 @@ static void ipv6_del_addr(struct inet6_ifaddr *ifp)
 				ip6_del_rt(rt);
 				rt = NULL;
 			} else if (!(rt->rt6i_flags & RTF_EXPIRES)) {
-				rt->dst.expires = expires;
-				rt->rt6i_flags |= RTF_EXPIRES;
+				rt6_set_expires(rt, expires);
 			}
 		}
 		dst_release(&rt->dst);
@@ -1887,11 +1886,9 @@ void addrconf_prefix_rcv(struct net_device *dev, u8 *opt, int len, bool sllao)
 				rt = NULL;
 			} else if (addrconf_finite_timeout(rt_expires)) {
 				/* not infinity */
-				rt->dst.expires = jiffies + rt_expires;
-				rt->rt6i_flags |= RTF_EXPIRES;
+				rt6_set_expires(rt, jiffies + rt_expires);
 			} else {
-				rt->rt6i_flags &= ~RTF_EXPIRES;
-				rt->dst.expires = 0;
+				rt6_clean_expires(rt);
 			}
 		} else if (valid_lft) {
 			clock_t expires = 0;
diff --git a/net/ipv6/ip6_fib.c b/net/ipv6/ip6_fib.c
index 5b27fbc..dbf6c10 100644
--- a/net/ipv6/ip6_fib.c
+++ b/net/ipv6/ip6_fib.c
@@ -673,11 +673,10 @@ static int fib6_add_rt2node(struct fib6_node *fn, struct rt6_info *rt,
 					    &rt->rt6i_gateway)) {
 				if (!(iter->rt6i_flags & RTF_EXPIRES))
 					return -EEXIST;
-				iter->dst.expires = rt->dst.expires;
-				if (!(rt->rt6i_flags & RTF_EXPIRES)) {
-					iter->rt6i_flags &= ~RTF_EXPIRES;
-					iter->dst.expires = 0;
-				}
+				if (!(rt->rt6i_flags & RTF_EXPIRES))
+					rt6_clean_expires(iter);
+				else
+					rt6_set_expires(iter, rt->dst.expires);
 				return -EEXIST;
 			}
 		}
diff --git a/net/ipv6/ndisc.c b/net/ipv6/ndisc.c
index 3dcdb81..176b469 100644
--- a/net/ipv6/ndisc.c
+++ b/net/ipv6/ndisc.c
@@ -1264,8 +1264,7 @@ static void ndisc_router_discovery(struct sk_buff *skb)
 	}
 
 	if (rt)
-		rt->dst.expires = jiffies + (HZ * lifetime);
-
+		rt6_set_expires(rt, jiffies + (HZ * lifetime));
 	if (ra_msg->icmph.icmp6_hop_limit) {
 		in6_dev->cnf.hop_limit = ra_msg->icmph.icmp6_hop_limit;
 		if (rt)
diff --git a/net/ipv6/route.c b/net/ipv6/route.c
index 3992e26..27068be 100644
--- a/net/ipv6/route.c
+++ b/net/ipv6/route.c
@@ -62,7 +62,7 @@
 #include <linux/sysctl.h>
 #endif
 
-static struct rt6_info *ip6_rt_copy(const struct rt6_info *ort,
+static struct rt6_info *ip6_rt_copy(struct rt6_info *ort,
 				    const struct in6_addr *dest);
 static struct dst_entry	*ip6_dst_check(struct dst_entry *dst, u32 cookie);
 static unsigned int	 ip6_default_advmss(const struct dst_entry *dst);
@@ -285,6 +285,10 @@ static void ip6_dst_destroy(struct dst_entry *dst)
 		rt->rt6i_idev = NULL;
 		in6_dev_put(idev);
 	}
+
+	if (!(rt->rt6i_flags & RTF_EXPIRES) && dst->from)
+		dst_release(dst->from);
+
 	if (peer) {
 		rt->rt6i_peer = NULL;
 		inet_putpeer(peer);
@@ -329,8 +333,17 @@ static void ip6_dst_ifdown(struct dst_entry *dst, struct net_device *dev,
 
 static __inline__ int rt6_check_expired(const struct rt6_info *rt)
 {
-	return (rt->rt6i_flags & RTF_EXPIRES) &&
-		time_after(jiffies, rt->dst.expires);
+	struct rt6_info *ort = NULL;
+
+	if (rt->rt6i_flags & RTF_EXPIRES) {
+		if (time_after(jiffies, rt->dst.expires))
+			return 1;
+	} else if (rt->dst.from) {
+		ort = (struct rt6_info *) rt->dst.from;
+		return (ort->rt6i_flags & RTF_EXPIRES) &&
+			time_after(jiffies, ort->dst.expires);
+	}
+	return 0;
 }
 
 static inline int rt6_need_strict(const struct in6_addr *daddr)
@@ -620,12 +633,11 @@ int rt6_route_rcv(struct net_device *dev, u8 *opt, int len,
 				 (rt->rt6i_flags & ~RTF_PREF_MASK) | RTF_PREF(pref);
 
 	if (rt) {
-		if (!addrconf_finite_timeout(lifetime)) {
-			rt->rt6i_flags &= ~RTF_EXPIRES;
-		} else {
-			rt->dst.expires = jiffies + HZ * lifetime;
-			rt->rt6i_flags |= RTF_EXPIRES;
-		}
+		if (!addrconf_finite_timeout(lifetime))
+			rt6_clean_expires(rt);
+		else
+			rt6_set_expires(rt, jiffies + HZ * lifetime);
+
 		dst_release(&rt->dst);
 	}
 	return 0;
@@ -730,7 +742,7 @@ int ip6_ins_rt(struct rt6_info *rt)
 	return __ip6_ins_rt(rt, &info);
 }
 
-static struct rt6_info *rt6_alloc_cow(const struct rt6_info *ort,
+static struct rt6_info *rt6_alloc_cow(struct rt6_info *ort,
 				      const struct in6_addr *daddr,
 				      const struct in6_addr *saddr)
 {
@@ -954,10 +966,10 @@ struct dst_entry *ip6_blackhole_route(struct net *net, struct dst_entry *dst_ori
 		rt->rt6i_idev = ort->rt6i_idev;
 		if (rt->rt6i_idev)
 			in6_dev_hold(rt->rt6i_idev);
-		rt->dst.expires = 0;
 
 		rt->rt6i_gateway = ort->rt6i_gateway;
-		rt->rt6i_flags = ort->rt6i_flags & ~RTF_EXPIRES;
+		rt->rt6i_flags = ort->rt6i_flags;
+		rt6_clean_expires(rt);
 		rt->rt6i_metric = 0;
 
 		memcpy(&rt->rt6i_dst, &ort->rt6i_dst, sizeof(struct rt6key));
@@ -1019,10 +1031,9 @@ static void ip6_link_failure(struct sk_buff *skb)
 
 	rt = (struct rt6_info *) skb_dst(skb);
 	if (rt) {
-		if (rt->rt6i_flags & RTF_CACHE) {
-			dst_set_expires(&rt->dst, 0);
-			rt->rt6i_flags |= RTF_EXPIRES;
-		} else if (rt->rt6i_node && (rt->rt6i_flags & RTF_DEFAULT))
+		if (rt->rt6i_flags & RTF_CACHE)
+			rt6_update_expires(rt, 0);
+		else if (rt->rt6i_node && (rt->rt6i_flags & RTF_DEFAULT))
 			rt->rt6i_node->fn_sernum = -1;
 	}
 }
@@ -1289,9 +1300,12 @@ int ip6_route_add(struct fib6_config *cfg)
 	}
 
 	rt->dst.obsolete = -1;
-	rt->dst.expires = (cfg->fc_flags & RTF_EXPIRES) ?
-				jiffies + clock_t_to_jiffies(cfg->fc_expires) :
-				0;
+
+	if (cfg->fc_flags & RTF_EXPIRES)
+		rt6_set_expires(rt, jiffies +
+				clock_t_to_jiffies(cfg->fc_expires));
+	else
+		rt6_clean_expires(rt);
 
 	if (cfg->fc_protocol == RTPROT_UNSPEC)
 		cfg->fc_protocol = RTPROT_BOOT;
@@ -1736,8 +1750,8 @@ again:
 			features |= RTAX_FEATURE_ALLFRAG;
 			dst_metric_set(&rt->dst, RTAX_FEATURES, features);
 		}
-		dst_set_expires(&rt->dst, net->ipv6.sysctl.ip6_rt_mtu_expires);
-		rt->rt6i_flags |= RTF_MODIFIED|RTF_EXPIRES;
+		rt6_update_expires(rt, net->ipv6.sysctl.ip6_rt_mtu_expires);
+		rt->rt6i_flags |= RTF_MODIFIED;
 		goto out;
 	}
 
@@ -1765,9 +1779,8 @@ again:
 		 * which is 10 mins. After 10 mins the decreased pmtu is expired
 		 * and detecting PMTU increase will be automatically happened.
 		 */
-		dst_set_expires(&nrt->dst, net->ipv6.sysctl.ip6_rt_mtu_expires);
-		nrt->rt6i_flags |= RTF_DYNAMIC|RTF_EXPIRES;
-
+		rt6_update_expires(nrt, net->ipv6.sysctl.ip6_rt_mtu_expires);
+		nrt->rt6i_flags |= RTF_DYNAMIC;
 		ip6_ins_rt(nrt);
 	}
 out:
@@ -1799,7 +1812,7 @@ void rt6_pmtu_discovery(const struct in6_addr *daddr, const struct in6_addr *sad
  *	Misc support functions
  */
 
-static struct rt6_info *ip6_rt_copy(const struct rt6_info *ort,
+static struct rt6_info *ip6_rt_copy(struct rt6_info *ort,
 				    const struct in6_addr *dest)
 {
 	struct net *net = dev_net(ort->dst.dev);
@@ -1819,10 +1832,14 @@ static struct rt6_info *ip6_rt_copy(const struct rt6_info *ort,
 		if (rt->rt6i_idev)
 			in6_dev_hold(rt->rt6i_idev);
 		rt->dst.lastuse = jiffies;
-		rt->dst.expires = 0;
 
 		rt->rt6i_gateway = ort->rt6i_gateway;
-		rt->rt6i_flags = ort->rt6i_flags & ~RTF_EXPIRES;
+		rt->rt6i_flags = ort->rt6i_flags;
+		if ((ort->rt6i_flags & (RTF_DEFAULT | RTF_ADDRCONF)) ==
+		    (RTF_DEFAULT | RTF_ADDRCONF))
+			rt6_set_from(rt, ort);
+		else
+			rt6_clean_expires(rt);
 		rt->rt6i_metric = 0;
 
 #ifdef CONFIG_IPV6_SUBTREES
-- 
1.7.7.6

Re: [PATCH v6] ipv6: fix problem with expired dst cache

[David Miller]

From: Gao feng <gaofeng@cn.fujitsu.com>
Date: Fri, 6 Apr 2012 18:13:10 +0800

> If the ipv6 dst cache which copy from the dst generated by ICMPV6 RA packet.
> this dst cache will not check expire because it has no RTF_EXPIRES flag.
> So this dst cache will always be used until the dst gc run.
> 
> Change the struct dst_entry,add a union contains new pointer from and expires.
> When rt6_info.rt6i_flags has no RTF_EXPIRES flag,the dst.expires has no use.
> we can use this field to point to where the dst cache copy from.
> The dst.from is only used in IPV6.
> 
> rt6_check_expired check if rt6_info.dst.from is expired.
> 
> ip6_rt_copy only set dst.from when the ort has flag RTF_ADDRCONF
> and RTF_DEFAULT.then hold the ort.
> 
> ip6_dst_destroy release the ort.
> 
> Add some functions to operate the RTF_EXPIRES flag and expires(from) together.
> and change the code to use these new adding functions.
> 
> Changes from v5:
> modify ip6_route_add and ndisc_router_discovery to use new adding functions.
> 
> Only set dst.from when the ort has flag RTF_ADDRCONF
> and RTF_DEFAULT.then hold the ort.
> 
> Signed-off-by: Gao feng <gaofeng@cn.fujitsu.com>

Applied, thanks for doing this work.

[PATCH] ipv6: fix rt6_update_expires

[Jiri Bohac]

Commit 1716a961 (ipv6: fix problem with expired dst cache) broke PMTU
discovery. rt6_update_expires() calls dst_set_expires(), which only updates
dst->expires if it has not been set previously (expires == 0) or if the new
expires is earlier than the current dst->expires.

rt6_update_expires() needs to zero rt->dst.expires, otherwise it will contain
ivalid data left over from rt->dst.from and will confuse dst_set_expires().

Signed-off-by: Jiri Bohac <jbohac@suse.cz>

diff --git a/include/net/ip6_fib.h b/include/net/ip6_fib.h
index ad4126f..68c1f94 100644
--- a/include/net/ip6_fib.h
+++ b/include/net/ip6_fib.h
@@ -143,8 +143,13 @@ static inline void rt6_set_expires(struct rt6_info *rt, unsigned long expires)
 
 static inline void rt6_update_expires(struct rt6_info *rt, int timeout)
 {
-	if (!(rt->rt6i_flags & RTF_EXPIRES) && rt->dst.from)
-		dst_release(rt->dst.from);
+	if (!(rt->rt6i_flags & RTF_EXPIRES)) {
+		if (rt->dst.from)
+			dst_release(rt->dst.from);
+		/* dst_set_expires relies on expires == 0 
+                  if it has not been set previously */
+		rt->dst.expires = 0;
+	}
 
 	dst_set_expires(&rt->dst, timeout);
 	rt->rt6i_flags |= RTF_EXPIRES;

-- 
Jiri Bohac <jbohac@suse.cz>
SUSE Labs, SUSE CZ

[PATCH] ipv6: clean up rt6_clean_expires

[Jiri Bohac]

Functionally, this change is a NOP.

Semantically, rt6_clean_expires() wants to do rt->dst.from = NULL instead of
rt->dst.expires = 0. It is clearing the RTF_EXPIRES flag, so the union is going
to be treated as a pointer (dst.from) not a long (dst.expires).

Signed-off-by: Jiri Bohac <jbohac@suse.cz>

diff --git a/include/net/ip6_fib.h b/include/net/ip6_fib.h
index c64778f..ad4126f 100644
--- a/include/net/ip6_fib.h
+++ b/include/net/ip6_fib.h
@@ -129,7 +129,7 @@ static inline void rt6_clean_expires(struct rt6_info *rt)
 		dst_release(rt->dst.from);
 
 	rt->rt6i_flags &= ~RTF_EXPIRES;
-	rt->dst.expires = 0;
+	rt->dst.from = NULL;
 }
 
 static inline void rt6_set_expires(struct rt6_info *rt, unsigned long expires)

-- 
Jiri Bohac <jbohac@suse.cz>
SUSE Labs, SUSE CZ

Re: [PATCH] ipv6: fix rt6_update_expires

[Gao feng]

于 2012年04月16日 21:34, Jiri Bohac 写道:
> 
> Commit 1716a961 (ipv6: fix problem with expired dst cache) broke PMTU
> discovery. rt6_update_expires() calls dst_set_expires(), which only updates
> dst->expires if it has not been set previously (expires == 0) or if the new
> expires is earlier than the current dst->expires.
> 
> rt6_update_expires() needs to zero rt->dst.expires, otherwise it will contain
> ivalid data left over from rt->dst.from and will confuse dst_set_expires().
> 
> Signed-off-by: Jiri Bohac <jbohac@suse.cz>
> 
> diff --git a/include/net/ip6_fib.h b/include/net/ip6_fib.h
> index ad4126f..68c1f94 100644
> --- a/include/net/ip6_fib.h
> +++ b/include/net/ip6_fib.h
> @@ -143,8 +143,13 @@ static inline void rt6_set_expires(struct rt6_info *rt, unsigned long expires)
>  
>  static inline void rt6_update_expires(struct rt6_info *rt, int timeout)
>  {
> -	if (!(rt->rt6i_flags & RTF_EXPIRES) && rt->dst.from)
> -		dst_release(rt->dst.from);
> +	if (!(rt->rt6i_flags & RTF_EXPIRES)) {
> +		if (rt->dst.from)
> +			dst_release(rt->dst.from);
> +		/* dst_set_expires relies on expires == 0 
> +                  if it has not been set previously */
> +		rt->dst.expires = 0;
> +	}
>  
>  	dst_set_expires(&rt->dst, timeout);
>  	rt->rt6i_flags |= RTF_EXPIRES;
> 

looks good to me, thanks.

Re: [PATCH] ipv6: fix rt6_update_expires

[David Miller]

From: Gao feng <gaofeng@cn.fujitsu.com>
Date: Wed, 18 Apr 2012 10:24:48 +0800

> 于 2012年04月16日 21:34, Jiri Bohac 写道:
>> 
>> Commit 1716a961 (ipv6: fix problem with expired dst cache) broke PMTU
>> discovery. rt6_update_expires() calls dst_set_expires(), which only updates
>> dst->expires if it has not been set previously (expires == 0) or if the new
>> expires is earlier than the current dst->expires.
>> 
>> rt6_update_expires() needs to zero rt->dst.expires, otherwise it will contain
>> ivalid data left over from rt->dst.from and will confuse dst_set_expires().
>> 
>> Signed-off-by: Jiri Bohac <jbohac@suse.cz>
 ...
> looks good to me, thanks.

Applied.

Re: [PATCH] ipv6: clean up rt6_clean_expires

[David Miller]

From: Jiri Bohac <jbohac@suse.cz>
Date: Mon, 16 Apr 2012 15:35:41 +0200

> Functionally, this change is a NOP.
> 
> Semantically, rt6_clean_expires() wants to do rt->dst.from = NULL instead of
> rt->dst.expires = 0. It is clearing the RTF_EXPIRES flag, so the union is going
> to be treated as a pointer (dst.from) not a long (dst.expires).
> 
> Signed-off-by: Jiri Bohac <jbohac@suse.cz>

Applied.