ipv4: Refactor nhc evaluation in fib_table_lookup
authorDavid Ahern <dsahern@gmail.com>
Tue, 26 May 2020 18:56:17 +0000 (12:56 -0600)
committerDavid S. Miller <davem@davemloft.net>
Tue, 26 May 2020 23:06:07 +0000 (16:06 -0700)
FIB lookups can return an entry that references an external nexthop.
While walking the nexthop struct we do not want to make multiple calls
into the nexthop code which can result in 2 different structs getting
accessed - one returning the number of paths the rest of the loop
seeing a different nh_grp struct. If the nexthop group shrunk, the
result is an attempt to access a fib_nh_common that does not exist for
the new nh_grp struct but did for the old one.

To fix that move the device evaluation code to a helper that can be
used for inline fib_nh path as well as external nexthops.

Update the existing check for fi->nh in fib_table_lookup to call a
new helper, nexthop_get_nhc_lookup, which walks the external nexthop
with a single rcu dereference.

Fixes: 430a049190de ("nexthop: Add support for nexthop groups")
Signed-off-by: David Ahern <dsahern@gmail.com>
Acked-by: Nikolay Aleksandrov <nikolay@cumulusnetworks.com>
Signed-off-by: David S. Miller <davem@davemloft.net>
include/net/ip_fib.h
include/net/nexthop.h
net/ipv4/fib_trie.c

index b219a8fe0950b99a88d057816dcf1b7cb48080a8..771ce068bc966e5602601f002f25204475b1e5e1 100644 (file)
@@ -479,6 +479,8 @@ void fib_nh_common_release(struct fib_nh_common *nhc);
 void fib_alias_hw_flags_set(struct net *net, const struct fib_rt_info *fri);
 void fib_trie_init(void);
 struct fib_table *fib_trie_table(u32 id, struct fib_table *alias);
+bool fib_lookup_good_nhc(const struct fib_nh_common *nhc, int fib_flags,
+                        const struct flowi4 *flp);
 
 static inline void fib_combine_itag(u32 *itag, const struct fib_result *res)
 {
index f09e8d7d98869f9f33fbe633062393003aaf6862..9414ae46fc1c0f41bc803ecf4a9650cc78f55880 100644 (file)
@@ -233,6 +233,39 @@ struct fib_nh_common *nexthop_fib_nhc(struct nexthop *nh, int nhsel)
        return &nhi->fib_nhc;
 }
 
+/* called from fib_table_lookup with rcu_lock */
+static inline
+struct fib_nh_common *nexthop_get_nhc_lookup(const struct nexthop *nh,
+                                            int fib_flags,
+                                            const struct flowi4 *flp,
+                                            int *nhsel)
+{
+       struct nh_info *nhi;
+
+       if (nh->is_group) {
+               struct nh_group *nhg = rcu_dereference(nh->nh_grp);
+               int i;
+
+               for (i = 0; i < nhg->num_nh; i++) {
+                       struct nexthop *nhe = nhg->nh_entries[i].nh;
+
+                       nhi = rcu_dereference(nhe->nh_info);
+                       if (fib_lookup_good_nhc(&nhi->fib_nhc, fib_flags, flp)) {
+                               *nhsel = i;
+                               return &nhi->fib_nhc;
+                       }
+               }
+       } else {
+               nhi = rcu_dereference(nh->nh_info);
+               if (fib_lookup_good_nhc(&nhi->fib_nhc, fib_flags, flp)) {
+                       *nhsel = 0;
+                       return &nhi->fib_nhc;
+               }
+       }
+
+       return NULL;
+}
+
 static inline unsigned int fib_info_num_path(const struct fib_info *fi)
 {
        if (unlikely(fi->nh))
index 4f334b42553853b2cc1de001bd99a53a71de5030..248f1c1959a63c4aa22331d2a937aa5aecaf3f0e 100644 (file)
@@ -1371,6 +1371,26 @@ static inline t_key prefix_mismatch(t_key key, struct key_vector *n)
        return (key ^ prefix) & (prefix | -prefix);
 }
 
+bool fib_lookup_good_nhc(const struct fib_nh_common *nhc, int fib_flags,
+                        const struct flowi4 *flp)
+{
+       if (nhc->nhc_flags & RTNH_F_DEAD)
+               return false;
+
+       if (ip_ignore_linkdown(nhc->nhc_dev) &&
+           nhc->nhc_flags & RTNH_F_LINKDOWN &&
+           !(fib_flags & FIB_LOOKUP_IGNORE_LINKSTATE))
+               return false;
+
+       if (!(flp->flowi4_flags & FLOWI_FLAG_SKIP_NH_OIF)) {
+               if (flp->flowi4_oif &&
+                   flp->flowi4_oif != nhc->nhc_oif)
+                       return false;
+       }
+
+       return true;
+}
+
 /* should be called with rcu_read_lock */
 int fib_table_lookup(struct fib_table *tb, const struct flowi4 *flp,
                     struct fib_result *res, int fib_flags)
@@ -1503,6 +1523,7 @@ found:
        /* Step 3: Process the leaf, if that fails fall back to backtracing */
        hlist_for_each_entry_rcu(fa, &n->leaf, fa_list) {
                struct fib_info *fi = fa->fa_info;
+               struct fib_nh_common *nhc;
                int nhsel, err;
 
                if ((BITS_PER_LONG > KEYLENGTH) || (fa->fa_slen < KEYLENGTH)) {
@@ -1528,26 +1549,25 @@ out_reject:
                if (fi->fib_flags & RTNH_F_DEAD)
                        continue;
 
-               if (unlikely(fi->nh && nexthop_is_blackhole(fi->nh))) {
-                       err = fib_props[RTN_BLACKHOLE].error;
-                       goto out_reject;
+               if (unlikely(fi->nh)) {
+                       if (nexthop_is_blackhole(fi->nh)) {
+                               err = fib_props[RTN_BLACKHOLE].error;
+                               goto out_reject;
+                       }
+
+                       nhc = nexthop_get_nhc_lookup(fi->nh, fib_flags, flp,
+                                                    &nhsel);
+                       if (nhc)
+                               goto set_result;
+                       goto miss;
                }
 
                for (nhsel = 0; nhsel < fib_info_num_path(fi); nhsel++) {
-                       struct fib_nh_common *nhc = fib_info_nhc(fi, nhsel);
+                       nhc = fib_info_nhc(fi, nhsel);
 
-                       if (nhc->nhc_flags & RTNH_F_DEAD)
+                       if (!fib_lookup_good_nhc(nhc, fib_flags, flp))
                                continue;
-                       if (ip_ignore_linkdown(nhc->nhc_dev) &&
-                           nhc->nhc_flags & RTNH_F_LINKDOWN &&
-                           !(fib_flags & FIB_LOOKUP_IGNORE_LINKSTATE))
-                               continue;
-                       if (!(flp->flowi4_flags & FLOWI_FLAG_SKIP_NH_OIF)) {
-                               if (flp->flowi4_oif &&
-                                   flp->flowi4_oif != nhc->nhc_oif)
-                                       continue;
-                       }
-
+set_result:
                        if (!(fib_flags & FIB_LOOKUP_NOREF))
                                refcount_inc(&fi->fib_clntref);
 
@@ -1568,6 +1588,7 @@ out_reject:
                        return err;
                }
        }
+miss:
 #ifdef CONFIG_IP_FIB_TRIE_STATS
        this_cpu_inc(stats->semantic_match_miss);
 #endif