Re: [PATCH net-next 2/3 v4] net: ipv4 sysctl option to ignore routes when nexthop link is down

2015-06-18 Thread David Miller
From: Andy Gospodarek go...@cumulusnetworks.com
Date: Mon, 15 Jun 2015 12:33:20 -0400

 @@ -1035,12 +1036,18 @@ int fib_dump_info(struct sk_buff *skb, u32 portid, 
 u32 seq, int event,
   nla_put_in_addr(skb, RTA_PREFSRC, fi-fib_prefsrc))
   goto nla_put_failure;
   if (fi-fib_nhs == 1) {
 + struct in_device *in_dev;
   if (fi-fib_nh-nh_gw 
   nla_put_in_addr(skb, RTA_GATEWAY, fi-fib_nh-nh_gw))
   goto nla_put_failure;

Please put an empty line between local variable declarations and code.

 @@ -1057,11 +1064,17 @@ int fib_dump_info(struct sk_buff *skb, u32 portid, 
 u32 seq, int event,
   goto nla_put_failure;
  
   for_nexthops(fi) {
 + struct in_device *in_dev;
   rtnh = nla_reserve_nohdr(skb, sizeof(*rtnh));
   if (!rtnh)
   goto nla_put_failure;

Likewise.
--
To unsubscribe from this list: send the line unsubscribe netdev in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH net-next 2/3 v4] net: ipv4 sysctl option to ignore routes when nexthop link is down

2015-06-15 Thread Andy Gospodarek
This feature is only enabled with the new per-interface or ipv4 global
sysctls called 'ignore_routes_with_linkdown'.

net.ipv4.conf.all.ignore_routes_with_linkdown = 0
net.ipv4.conf.default.ignore_routes_with_linkdown = 0
net.ipv4.conf.lo.ignore_routes_with_linkdown = 0
...

When the above sysctls are set, will report to userspace that a route is
dead and will no longer resolve to this nexthop when performing a fib
lookup.  This will signal to userspace that the route will not be
selected.  The signalling of a RTNH_F_DEAD is only passed to userspace
if the sysctl is enabled and link is down.  This was done as without it the
netlink listeners would have no idea whether or not a nexthop would be
selected.   The kernel only sets RTNH_F_DEAD internally if the inteface has
IFF_UP cleared.

With the new sysctl set, the following behavior can be observed
(interface p8p1 is link-down):

default via 10.0.5.2 dev p9p1
10.0.5.0/24 dev p9p1  proto kernel  scope link  src 10.0.5.15
70.0.0.0/24 dev p7p1  proto kernel  scope link  src 70.0.0.1
80.0.0.0/24 dev p8p1  proto kernel  scope link  src 80.0.0.1 dead linkdown
90.0.0.0/24 via 80.0.0.2 dev p8p1  metric 1 dead linkdown
90.0.0.0/24 via 70.0.0.2 dev p7p1  metric 2
90.0.0.1 via 70.0.0.2 dev p7p1  src 70.0.0.1
cache
local 80.0.0.1 dev lo  src 80.0.0.1
cache local
80.0.0.2 via 10.0.5.2 dev p9p1  src 10.0.5.15
cache

While the route does remain in the table (so it can be modified if
needed rather than being wiped away as it would be if IFF_UP was
cleared), the proper next-hop is chosen automatically when the link is
down.  Now interface p8p1 is linked-up:

default via 10.0.5.2 dev p9p1
10.0.5.0/24 dev p9p1  proto kernel  scope link  src 10.0.5.15
70.0.0.0/24 dev p7p1  proto kernel  scope link  src 70.0.0.1
80.0.0.0/24 dev p8p1  proto kernel  scope link  src 80.0.0.1
90.0.0.0/24 via 80.0.0.2 dev p8p1  metric 1
90.0.0.0/24 via 70.0.0.2 dev p7p1  metric 2
192.168.56.0/24 dev p2p1  proto kernel  scope link  src 192.168.56.2
90.0.0.1 via 80.0.0.2 dev p8p1  src 80.0.0.1
cache
local 80.0.0.1 dev lo  src 80.0.0.1
cache local
80.0.0.2 dev p8p1  src 80.0.0.1
cache

and the output changes to what one would expect.

If the sysctl is not set, the following output would be expected when
p8p1 is down:

default via 10.0.5.2 dev p9p1
10.0.5.0/24 dev p9p1  proto kernel  scope link  src 10.0.5.15
70.0.0.0/24 dev p7p1  proto kernel  scope link  src 70.0.0.1
80.0.0.0/24 dev p8p1  proto kernel  scope link  src 80.0.0.1 linkdown
90.0.0.0/24 via 80.0.0.2 dev p8p1  metric 1 linkdown
90.0.0.0/24 via 70.0.0.2 dev p7p1  metric 2

Since the dead flag does not appear, there should be no expectation that
the kernel would skip using this route due to link being down.

v2: Split kernel changes into 2 patches, this actually makes a
behavioral change if the sysctl is set.  Also took suggestion from Alex
to simplify code by only checking sysctl during fib lookup and
suggestion from Scott to add a per-interface sysctl.

v3: Code clean-ups to make it more readable and efficient as well as a
reverse path check fix.

v4: Drop binary sysctl

Signed-off-by: Andy Gospodarek go...@cumulusnetworks.com
Signed-off-by: Dinesh Dutt dd...@cumulusnetworks.com
---
 include/linux/inetdevice.h|  3 +++
 include/net/fib_rules.h   |  3 ++-
 include/net/ip_fib.h  | 16 +---
 include/uapi/linux/ip.h   |  1 +
 net/ipv4/devinet.c|  2 ++
 net/ipv4/fib_frontend.c   |  6 +++---
 net/ipv4/fib_rules.c  |  5 +++--
 net/ipv4/fib_semantics.c  | 29 -
 net/ipv4/fib_trie.c   |  7 +++
 net/ipv4/netfilter/ipt_rpfilter.c |  2 +-
 net/ipv4/route.c  | 10 +-
 11 files changed, 60 insertions(+), 24 deletions(-)

diff --git a/include/linux/inetdevice.h b/include/linux/inetdevice.h
index 0a21fbe..a4328ce 100644
--- a/include/linux/inetdevice.h
+++ b/include/linux/inetdevice.h
@@ -120,6 +120,9 @@ static inline void ipv4_devconf_setall(struct in_device 
*in_dev)
 || (!IN_DEV_FORWARD(in_dev)  \
  IN_DEV_ORCONF((in_dev), ACCEPT_REDIRECTS)))
 
+#define IN_DEV_IGNORE_ROUTES_WITH_LINKDOWN(in_dev) \
+   IN_DEV_CONF_GET((in_dev), IGNORE_ROUTES_WITH_LINKDOWN)
+
 #define IN_DEV_ARPFILTER(in_dev)   IN_DEV_ORCONF((in_dev), ARPFILTER)
 #define IN_DEV_ARP_ACCEPT(in_dev)  IN_DEV_ORCONF((in_dev), ARP_ACCEPT)
 #define IN_DEV_ARP_ANNOUNCE(in_dev)IN_DEV_MAXCONF((in_dev), ARP_ANNOUNCE)
diff --git a/include/net/fib_rules.h b/include/net/fib_rules.h
index 6d67383..903a55e 100644
--- a/include/net/fib_rules.h
+++ b/include/net/fib_rules.h
@@ -36,7 +36,8 @@ struct fib_lookup_arg {
void*result;
struct fib_rule *rule;
int flags;
-#define FIB_LOOKUP_NOREF   1
+#define FIB_LOOKUP_NOREF   1
+#define FIB_LOOKUP_IGNORE_LINKSTATE2
 };
 
 struct fib_rules_ops {
diff --git