diff mbox

ipv6: shouldn't dump the expired routes

Message ID 52BBC5F9.4050904@cn.fujitsu.com
State Rejected, archived
Delegated to: David Miller
Headers show

Commit Message

Duan Jiong Dec. 26, 2013, 6 a.m. UTC
If we execute the command "ip -6 route show table all", those routes
that have been expired will be dumped.  But as everyone knows, those
expired routes will not be used, and they will be deleted by the kernel.
So why we still need to dump they, and just don't dump them.

Signed-off-by: Duan Jiong <duanj.fnst@cn.fujitsu.com>
---
 include/net/ip6_route.h | 2 ++
 net/ipv6/ip6_fib.c      | 3 +++
 net/ipv6/route.c        | 2 +-
 3 files changed, 6 insertions(+), 1 deletion(-)

Comments

Stephen Hemminger Dec. 26, 2013, 5:48 p.m. UTC | #1
On Thu, 26 Dec 2013 14:00:25 +0800
Duan Jiong <duanj.fnst@cn.fujitsu.com> wrote:

> 
> If we execute the command "ip -6 route show table all", those routes
> that have been expired will be dumped.  But as everyone knows, those
> expired routes will not be used, and they will be deleted by the kernel.
> So why we still need to dump they, and just don't dump them.
> 
> Signed-off-by: Duan Jiong <duanj.fnst@cn.fujitsu.com>

I can see three issues with this patch.

1. You are changing output of iproute2, and there is a slim chance somebody is
   expecting those routes in some test script.

2. Developers maybe using this to check that the expiration of routes is really
   working.

3. By making rt6_check_expired a global symbol, it can't be optimized as well in
   a potentially hot path for routing.

I am not saying the patch shouldn't go in, just raising the possibility that such
a seemingly trivial change could cause other impacts.
--
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
David Miller Dec. 31, 2013, 9:01 p.m. UTC | #2
From: Duan Jiong <duanj.fnst@cn.fujitsu.com>
Date: Thu, 26 Dec 2013 14:00:25 +0800

> If we execute the command "ip -6 route show table all", those routes
> that have been expired will be dumped.  But as everyone knows, those
> expired routes will not be used, and they will be deleted by the kernel.
> So why we still need to dump they, and just don't dump them.
> 
> Signed-off-by: Duan Jiong <duanj.fnst@cn.fujitsu.com>

I totally disagree with this change.

These routes are absolutely not guarenteed to never be used, their
inapplicability is temporary.

A router info advertisement can undo the expiration on an existing
route, so such ipv6 routes can become valid again.  Seeing them in
the route dump is therefore very useful.

I'm not applying this, sorry.
--
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
diff mbox

Patch

diff --git a/include/net/ip6_route.h b/include/net/ip6_route.h
index c2626ce..32fa188 100644
--- a/include/net/ip6_route.h
+++ b/include/net/ip6_route.h
@@ -103,6 +103,8 @@  void fib6_force_start_gc(struct net *net);
 struct rt6_info *addrconf_dst_alloc(struct inet6_dev *idev,
 				    const struct in6_addr *addr, bool anycast);
 
+bool rt6_check_expired(const struct rt6_info *rt);
+
 /*
  *	support functions for ND
  *
diff --git a/net/ipv6/ip6_fib.c b/net/ipv6/ip6_fib.c
index 5550a81..f57a6bb 100644
--- a/net/ipv6/ip6_fib.c
+++ b/net/ipv6/ip6_fib.c
@@ -278,6 +278,9 @@  static int fib6_dump_node(struct fib6_walker_t *w)
 	struct rt6_info *rt;
 
 	for (rt = w->leaf; rt; rt = rt->dst.rt6_next) {
+		if (rt6_check_expired(rt))
+			continue;
+
 		res = rt6_dump_route(rt, w->args);
 		if (res < 0) {
 			/* Frame is full, suspend walking */
diff --git a/net/ipv6/route.c b/net/ipv6/route.c
index 89b2735..abf5f05 100644
--- a/net/ipv6/route.c
+++ b/net/ipv6/route.c
@@ -348,7 +348,7 @@  static void ip6_dst_ifdown(struct dst_entry *dst, struct net_device *dev,
 	}
 }
 
-static bool rt6_check_expired(const struct rt6_info *rt)
+bool rt6_check_expired(const struct rt6_info *rt)
 {
 	if (rt->rt6i_flags & RTF_EXPIRES) {
 		if (time_after(jiffies, rt->dst.expires))