diff mbox series

[net-next,12/16] ipv6: update fn_sernum after route is inserted to tree

Message ID 20171006190611.110633-13-tracywwnj@gmail.com
State Accepted, archived
Delegated to: David Miller
Headers show
Series ipv6: replace rwlock with rcu and spinlock in fib6 table | expand

Commit Message

Wei Wang Oct. 6, 2017, 7:06 p.m. UTC 
From: Wei Wang <weiwan@google.com>

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>
---
 include/net/ip6_fib.h |  2 ++
 net/ipv6/ip6_fib.c    | 39 +++++++++++++++++++++------------------
 2 files changed, 23 insertions(+), 18 deletions(-)
diff mbox series

Patch

diff --git a/include/net/ip6_fib.h b/include/net/ip6_fib.h
index d0b7283073e3..6bf929b50951 100644
--- a/include/net/ip6_fib.h
+++ b/include/net/ip6_fib.h
@@ -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;
 	}
 
diff --git a/net/ipv6/ip6_fib.c b/net/ipv6/ip6_fib.c
index eee392f7b1f6..f604b311cc3e 100644
--- a/net/ipv6/ip6_fib.c
+++ b/net/ipv6/ip6_fib.c
@@ -585,7 +585,7 @@  static int inet6_dump_fib(struct sk_buff *skb, struct netlink_callback *cb)
 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 @@  static struct fib6_node *fib6_add_1(struct fib6_node *root,
 		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 @@  static struct fib6_node *fib6_add_1(struct fib6_node *root,
 		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 @@  static struct fib6_node *fib6_add_1(struct fib6_node *root,
 
 		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) {