On 4/1/16 2:09 AM, Julian Anastasov wrote:
diff --git a/net/ipv4/fib_semantics.c b/net/ipv4/fib_semantics.c index d97268e8ff10..6d423faff0ce 100644 --- a/net/ipv4/fib_semantics.c +++ b/net/ipv4/fib_semantics.c @@ -1559,17 +1559,45 @@ int fib_sync_up(struct net_device *dev, unsigned int nh_flags) }#ifdef CONFIG_IP_ROUTE_MULTIPATH +static bool fib_good_nh(const struct fib_nh *nh, struct net_device *dev) +{ + struct neighbour *n = NULL; + int state = NUD_NONE;Looks like we can do it even better. If we use NUD_REACHABLE here...+ + if (nh->nh_scope == RT_SCOPE_LINK) { + rcu_read_lock_bh(); + + n = __neigh_lookup_noref(&arp_tbl, &nh->nh_gw, dev); + if (n) + state = n->nud_state; + + rcu_read_unlock_bh(); + } + + /* outside of rcu locking using n only as a boolean + * on whether a neighbor entry existed + */ + if (!n || (state & NUD_VALID))then check for '!n' is not needed. For bool type 'return state & NUD_VALID;' should work.
good simplification.
+ return true; + + return false; +} void fib_select_multipath(struct fib_result *res, int hash) { struct fib_info *fi = res->fi; + struct net_device *dev = fi->fib_dev; + struct net *net = fi->fib_net; for_nexthops(fi) { if (hash > atomic_read(&nh->nh_upper_bound)) continue; - res->nh_sel = nhsel; - return; + if (!net->ipv4.sysctl_fib_multipath_use_neigh || + fib_good_nh(nh, dev)) {This dev is from first nexthop. Better fib_good_nh to use nh->nh_dev instead, it is present for all nexthops in multipath route. We should not copy the bugs from fib_detect_death.
ack. Will fix.
+ res->nh_sel = nhsel; + return; + } } endfor_nexthops(fi);So, you dropped the idea to give full chance for fallback? Now if last nexthop fails we do not fallback at all. We promised to prefer reachable nexthops.
I'll save the first nhsel not selected and if all fails return it. That keeps the fallback inline with what happens today.
