Thread (29 messages) 29 messages, 5 authors, 2012-04-18

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

From: Eric Dumazet <hidden>
Date: 2012-02-29 12:14:24

Le mercredi 29 février 2012 à 18:07 +0800, Gao feng a écrit :
quoted hunk ↗ jump to hunk
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 <redacted>
---
 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...

quoted hunk ↗ jump to hunk
+		 */
+		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.
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help