ipv6: update fn_sernum after route is inserted to tree
authorWei Wang <weiwan@google.com>
Fri, 6 Oct 2017 19:06:07 +0000 (12:06 -0700)
committerDavid S. Miller <davem@davemloft.net>
Sat, 7 Oct 2017 20:22:58 +0000 (21:22 +0100)
fib6_add() logic currently calls fib6_add_1() to figure out what node
should be used for the newly added route and then call
fib6_add_rt2node() to insert the route to the node.
And during the call of fib6_add_1(), fn_sernum is updated for all nodes
that share the same prefix as the new route.
This does not have issue in the current code because reader thread will
not be able to access the tree while writer thread is inserting new
route to it. However, it is not the case once we transition to use RCU.
Reader thread could potentially see the new fn_sernum before the new
route is inserted. As a result, reader thread's route lookup will return
a stale route with the new fn_sernum.

In order to solve this issue, we remove all the update of fn_sernum in
fib6_add_1(), and instead, introduce a new function that updates fn_sernum
for all related nodes and call this functions once the route is
successfully inserted to the tree.
Also, smp_wmb() is used after a route is successfully inserted into the
fib tree and right before the updated of fn->sernum. And smp_rmb() is
used right after fn->sernum is accessed in rt6_get_cookie_safe(). This
is to guarantee that when the reader thread sees the new fn->sernum, the
new route is already inserted in the tree in memory.

Signed-off-by: Wei Wang <weiwan@google.com>
Signed-off-by: Martin KaFai Lau <kafai@fb.com>
Signed-off-by: Eric Dumazet <edumazet@google.com>
Signed-off-by: David S. Miller <davem@davemloft.net>
include/net/ip6_fib.h
net/ipv6/ip6_fib.c

index d0b7283073e3c8402ffc4916544274bfb3a48184..6bf929b509515569c5d9978f9b560350f758c664 100644 (file)
@@ -220,6 +220,8 @@ static inline bool rt6_get_cookie_safe(const struct rt6_info *rt,
 
        if (fn) {
                *cookie = fn->fn_sernum;
+               /* pairs with smp_wmb() in fib6_update_sernum_upto_root() */
+               smp_rmb();
                status = true;
        }
 
index eee392f7b1f60fdd3c782b4ee5ef4abb9b75dfb0..f604b311cc3e3167c426a7c7d75e90bcbbbabd20 100644 (file)
@@ -585,7 +585,7 @@ out:
 static struct fib6_node *fib6_add_1(struct fib6_node *root,
                                     struct in6_addr *addr, int plen,
                                     int offset, int allow_create,
-                                    int replace_required, int sernum,
+                                    int replace_required,
                                     struct netlink_ext_ack *extack)
 {
        struct fib6_node *fn, *in, *ln;
@@ -631,8 +631,6 @@ static struct fib6_node *fib6_add_1(struct fib6_node *root,
                                fn->leaf = NULL;
                        }
 
-                       fn->fn_sernum = sernum;
-
                        return fn;
                }
 
@@ -641,7 +639,6 @@ static struct fib6_node *fib6_add_1(struct fib6_node *root,
                 */
 
                /* Try to walk down on tree. */
-               fn->fn_sernum = sernum;
                dir = addr_bit_set(addr, fn->fn_bit);
                pn = fn;
                fn = dir ? fn->right : fn->left;
@@ -677,7 +674,6 @@ static struct fib6_node *fib6_add_1(struct fib6_node *root,
        ln->fn_bit = plen;
 
        ln->parent = pn;
-       ln->fn_sernum = sernum;
 
        if (dir)
                pn->right = ln;
@@ -737,8 +733,6 @@ insert_above:
                in->leaf = fn->leaf;
                atomic_inc(&in->leaf->rt6i_ref);
 
-               in->fn_sernum = sernum;
-
                /* update parent pointer */
                if (dir)
                        pn->right = in;
@@ -750,8 +744,6 @@ insert_above:
                ln->parent = in;
                fn->parent = in;
 
-               ln->fn_sernum = sernum;
-
                if (addr_bit_set(addr, bit)) {
                        in->right = ln;
                        in->left  = fn;
@@ -776,8 +768,6 @@ insert_above:
 
                ln->parent = pn;
 
-               ln->fn_sernum = sernum;
-
                if (dir)
                        pn->right = ln;
                else
@@ -1079,6 +1069,20 @@ void fib6_force_start_gc(struct net *net)
                          jiffies + net->ipv6.sysctl.ip6_rt_gc_interval);
 }
 
+static void fib6_update_sernum_upto_root(struct rt6_info *rt,
+                                        int sernum)
+{
+       struct fib6_node *fn = rcu_dereference_protected(rt->rt6i_node,
+                               lockdep_is_held(&rt->rt6i_table->tb6_lock));
+
+       /* paired with smp_rmb() in rt6_get_cookie_safe() */
+       smp_wmb();
+       while (fn) {
+               fn->fn_sernum = sernum;
+               fn = fn->parent;
+       }
+}
+
 /*
  *     Add routing information to the routing tree.
  *     <destination addr>/<source addr>
@@ -1111,7 +1115,7 @@ int fib6_add(struct fib6_node *root, struct rt6_info *rt,
 
        fn = fib6_add_1(root, &rt->rt6i_dst.addr, rt->rt6i_dst.plen,
                        offsetof(struct rt6_info, rt6i_dst), allow_create,
-                       replace_required, sernum, extack);
+                       replace_required, extack);
        if (IS_ERR(fn)) {
                err = PTR_ERR(fn);
                fn = NULL;
@@ -1145,15 +1149,13 @@ int fib6_add(struct fib6_node *root, struct rt6_info *rt,
                        sfn->leaf = info->nl_net->ipv6.ip6_null_entry;
                        atomic_inc(&info->nl_net->ipv6.ip6_null_entry->rt6i_ref);
                        sfn->fn_flags = RTN_ROOT;
-                       sfn->fn_sernum = sernum;
 
                        /* Now add the first leaf node to new subtree */
 
                        sn = fib6_add_1(sfn, &rt->rt6i_src.addr,
                                        rt->rt6i_src.plen,
                                        offsetof(struct rt6_info, rt6i_src),
-                                       allow_create, replace_required, sernum,
-                                       extack);
+                                       allow_create, replace_required, extack);
 
                        if (IS_ERR(sn)) {
                                /* If it is failed, discard just allocated
@@ -1172,8 +1174,7 @@ int fib6_add(struct fib6_node *root, struct rt6_info *rt,
                        sn = fib6_add_1(fn->subtree, &rt->rt6i_src.addr,
                                        rt->rt6i_src.plen,
                                        offsetof(struct rt6_info, rt6i_src),
-                                       allow_create, replace_required, sernum,
-                                       extack);
+                                       allow_create, replace_required, extack);
 
                        if (IS_ERR(sn)) {
                                err = PTR_ERR(sn);
@@ -1190,8 +1191,10 @@ int fib6_add(struct fib6_node *root, struct rt6_info *rt,
 #endif
 
        err = fib6_add_rt2node(fn, rt, info, mxc);
-       if (!err)
+       if (!err) {
+               fib6_update_sernum_upto_root(rt, sernum);
                fib6_start_gc(info->nl_net, rt);
+       }
 
 out:
        if (err) {