| Message ID | 1438875754-28025-1-git-send-email-gospo@cumulusnetworks.com |
|---|---|
| State | Changes Requested, archived |
| Delegated to: | David Miller |
| Headers | show |
From: Andy Gospodarek <gospo@cumulusnetworks.com> Date: Thu, 6 Aug 2015 11:42:33 -0400 > Add support to track current link status of ipv6 nexthops to match > recent changes that added support for ipv4 nexthops. There was not a > field already available that could track these and no space available in > the existing rt6i_flags field, so this patch adds rt6i_nhflags to struct > rt6_info. > > Signed-off-by: Andy Gospodarek <gospo@cumulusnetworks.com> > Signed-off-by: Dinesh Dutt <ddutt@cumulusnetworks.com> This doesn't really make any sense to me. You can evaluate the state of the link at the time you look at the route at all of the places where it matters as far as I can tell. It's so expensive to walk the entire routing table every time a link goes up and down, so it's much better to take an "evaluate as needed" approach to implementing this. -- 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
On Mon, Aug 10, 2015 at 10:54:00AM -0700, David Miller wrote: > From: Andy Gospodarek <gospo@cumulusnetworks.com> > Date: Thu, 6 Aug 2015 11:42:33 -0400 > > > Add support to track current link status of ipv6 nexthops to match > > recent changes that added support for ipv4 nexthops. There was not a > > field already available that could track these and no space available in > > the existing rt6i_flags field, so this patch adds rt6i_nhflags to struct > > rt6_info. > > > > Signed-off-by: Andy Gospodarek <gospo@cumulusnetworks.com> > > Signed-off-by: Dinesh Dutt <ddutt@cumulusnetworks.com> > > This doesn't really make any sense to me. > > You can evaluate the state of the link at the time you look at the > route at all of the places where it matters as far as I can tell. > > It's so expensive to walk the entire routing table every time a link > goes up and down, so it's much better to take an "evaluate as needed" > approach to implementing this. I went this way as the idea of storing this info in a flags structure for 2 reasons: - This idea or marking on link status changes and checking for that mark during forwarding was done what was suggested by Alex et al for the ipv4 code and I wanted to keep the overall design similar. - New flags will likely be needed when switchdev support is added for ipv6 routes so going ahead and mirroring the RTNH_F* flags in the the ipv6 code seemed reasonable. I would actually be fine with what you proposed (it is closer to the first implementation), so if my justification above does not change your mind, let me know and I'll post a v2 that does not add rt6i_nhflags and simply checks netif_carrier_ok() rather than RTNH_F_LINKDOWN. -- 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
From: Andy Gospodarek <gospo@cumulusnetworks.com> Date: Mon, 10 Aug 2015 14:46:52 -0400 > I went this way as the idea of storing this info in a flags structure > for 2 reasons: > > - This idea or marking on link status changes and checking for that mark > during forwarding was done what was suggested by Alex et al for the > ipv4 code and I wanted to keep the overall design similar. > > - New flags will likely be needed when switchdev support is added for > ipv6 routes so going ahead and mirroring the RTNH_F* flags in the the > ipv6 code seemed reasonable. > > I would actually be fine with what you proposed (it is closer to the > first implementation), so if my justification above does not change your > mind, let me know and I'll post a v2 that does not add rt6i_nhflags and > simply checks netif_carrier_ok() rather than RTNH_F_LINKDOWN. Ok fair enough, if we'll need more flags later then so be it. Andy, please resubmit this series, I'll apply it. Thanks. -- 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
On Wed, Aug 12, 2015 at 02:32:26PM -0700, David Miller wrote: > From: Andy Gospodarek <gospo@cumulusnetworks.com> > Date: Mon, 10 Aug 2015 14:46:52 -0400 > > > I went this way as the idea of storing this info in a flags structure > > for 2 reasons: > > > > - This idea or marking on link status changes and checking for that mark > > during forwarding was done what was suggested by Alex et al for the > > ipv4 code and I wanted to keep the overall design similar. > > > > - New flags will likely be needed when switchdev support is added for > > ipv6 routes so going ahead and mirroring the RTNH_F* flags in the the > > ipv6 code seemed reasonable. > > > > I would actually be fine with what you proposed (it is closer to the > > first implementation), so if my justification above does not change your > > mind, let me know and I'll post a v2 that does not add rt6i_nhflags and > > simply checks netif_carrier_ok() rather than RTNH_F_LINKDOWN. > > Ok fair enough, if we'll need more flags later then so be it. > > Andy, please resubmit this series, I'll apply it. Thanks, Dave. Will do. -- 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 --git a/include/net/ip6_fib.h b/include/net/ip6_fib.h index 276328e..371780b 100644 --- a/include/net/ip6_fib.h +++ b/include/net/ip6_fib.h @@ -119,6 +119,7 @@ struct rt6_info { /* These are in a separate cache line. */ struct rt6key rt6i_dst ____cacheline_aligned_in_smp; u32 rt6i_flags; + u32 rt6i_nhflags; struct rt6key rt6i_src; struct rt6key rt6i_prefsrc; diff --git a/include/net/ip6_route.h b/include/net/ip6_route.h index 297629a..6ba5e9e 100644 --- a/include/net/ip6_route.h +++ b/include/net/ip6_route.h @@ -125,6 +125,7 @@ struct rt6_rtnl_dump_arg { int rt6_dump_route(struct rt6_info *rt, void *p_arg); void rt6_ifdown(struct net *net, struct net_device *dev); +void rt6_link_change(struct net *net, struct net_device *dev); void rt6_mtu_change(struct net_device *dev, unsigned int mtu); void rt6_remove_prefsrc(struct inet6_ifaddr *ifp); void rt6_clean_tohost(struct net *net, struct in6_addr *gateway); diff --git a/net/ipv6/addrconf.c b/net/ipv6/addrconf.c index 53e3a9d..3fc8ee2 100644 --- a/net/ipv6/addrconf.c +++ b/net/ipv6/addrconf.c @@ -3126,6 +3126,8 @@ static int addrconf_notify(struct notifier_block *this, unsigned long event, run_pending = 1; } } else { + rt6_link_change(dev_net(dev), dev); + if (!addrconf_qdisc_ok(dev)) { /* device is still not ready. */ break; diff --git a/net/ipv6/route.c b/net/ipv6/route.c index 54fccf0..a5c8c6a 100644 --- a/net/ipv6/route.c +++ b/net/ipv6/route.c @@ -1919,6 +1919,9 @@ install_route: rt->rt6i_idev = idev; rt->rt6i_table = table; + if (!netif_carrier_ok(dev)) + rt->rt6i_nhflags |= RTNH_F_LINKDOWN; + cfg->fc_nlinfo.nl_net = dev_net(dev); err = ip6_convert_metrics(&mxc, cfg); @@ -2161,6 +2164,7 @@ static void ip6_rt_copy_init(struct rt6_info *rt, struct rt6_info *ort) rt->rt6i_prefsrc = ort->rt6i_prefsrc; rt->rt6i_table = ort->rt6i_table; rt->rt6i_lwtstate = lwtstate_get(ort->rt6i_lwtstate); + rt->rt6i_nhflags = ort->rt6i_nhflags; } #ifdef CONFIG_IPV6_ROUTE_INFO @@ -2526,6 +2530,30 @@ static int fib6_ifdown(struct rt6_info *rt, void *arg) return 0; } +static int fib6_linkup(struct rt6_info *rt, void *arg) +{ + const struct arg_dev_net *adn = arg; + const struct net_device *dev = adn->dev; + + if (rt->dst.dev == dev || !dev) + rt->rt6i_nhflags &= ~RTNH_F_LINKDOWN; + return 0; +} + +static int fib6_linkdown(struct rt6_info *rt, void *arg) +{ + const struct arg_dev_net *adn = arg; + const struct net_device *dev = adn->dev; + + if (rt->dst.dev == dev || !dev) { + rt->rt6i_nhflags |= RTNH_F_LINKDOWN; + + if (rt->rt6i_flags & RTF_CACHE) + return -1; + } + return 0; +} + void rt6_ifdown(struct net *net, struct net_device *dev) { struct arg_dev_net adn = { @@ -2538,6 +2566,23 @@ void rt6_ifdown(struct net *net, struct net_device *dev) rt6_uncached_list_flush_dev(net, dev); } +void rt6_link_change(struct net *net, struct net_device *dev) +{ + struct arg_dev_net adn = { + .dev = dev, + .net = net, + }; + unsigned int flags = dev_get_flags(dev); + + if (flags & (IFF_RUNNING | IFF_LOWER_UP)) { + fib6_clean_all(net, fib6_linkup, &adn); + icmp6_clean_all(fib6_linkup, &adn); + } else { + fib6_clean_all(net, fib6_linkdown, &adn); + icmp6_clean_all(fib6_linkdown, &adn); + } +} + struct rt6_mtu_change_arg { struct net_device *dev; unsigned int mtu; @@ -2884,7 +2929,7 @@ static int rt6_fill_node(struct net *net, rtm->rtm_type = RTN_LOCAL; else rtm->rtm_type = RTN_UNICAST; - rtm->rtm_flags = 0; + rtm->rtm_flags = rt->rt6i_nhflags; rtm->rtm_scope = RT_SCOPE_UNIVERSE; rtm->rtm_protocol = rt->rt6i_protocol; if (rt->rt6i_flags & RTF_DYNAMIC)