IPv6: Avoid taking write lock for /proc/net/ipv6_route
authorJosh Hunt <joshhunt00@gmail.com>
Wed, 28 Dec 2011 13:23:07 +0000 (13:23 +0000)
committerDavid S. Miller <davem@davemloft.net>
Fri, 30 Dec 2011 22:07:33 +0000 (17:07 -0500)
During some debugging I needed to look into how /proc/net/ipv6_route
operated and in my digging I found its calling fib6_clean_all() which uses
"write_lock_bh(&table->tb6_lock)" before doing the walk of the table. I
found this on 2.6.32, but reading the code I believe the same basic idea
exists currently. Looking at the rtnetlink code they are only calling
"read_lock_bh(&table->tb6_lock);" via fib6_dump_table(). While I realize
reading from proc isn't the recommended way of fetching the ipv6 route
table; taking a write lock seems unnecessary and would probably cause
network performance issues.

To verify this I loaded up the ipv6 route table and then ran iperf in 3
cases:
  * doing nothing
  * reading ipv6 route table via proc
    (while :; do cat /proc/net/ipv6_route > /dev/null; done)
  * reading ipv6 route table via rtnetlink
    (while :; do ip -6 route show table all > /dev/null; done)

* Load the ipv6 route table up with:
  * for ((i = 0;i < 4000;i++)); do ip route add unreachable 2000::$i; done

* iperf commands:
  * client: iperf -i 1 -V -c <ipv6 addr>
  * server: iperf -V -s

* iperf results - 3 runs each (in Mbits/sec)
  * nothing: client: 927,927,927 server: 927,927,927
  * proc: client: 179,97,96,113 server: 142,112,133
  * iproute: client: 928,927,928 server: 927,927,927

lock_stat shows taking the write lock is causing the slowdown. Using this
info I decided to write a version of fib6_clean_all() which replaces
write_lock_bh(&table->tb6_lock) with read_lock_bh(&table->tb6_lock). With
this new function I see the same results as with my rtnetlink iperf test.

Signed-off-by: Josh Hunt <joshhunt00@gmail.com>
Signed-off-by: David S. Miller <davem@davemloft.net>
include/net/ip6_fib.h
net/ipv6/ip6_fib.c
net/ipv6/route.c

index 1e8a89f..b26bb81 100644 (file)
@@ -199,6 +199,10 @@ struct fib6_node           *fib6_locate(struct fib6_node *root,
                                             const struct in6_addr *daddr, int dst_len,
                                             const struct in6_addr *saddr, int src_len);
 
+extern void                    fib6_clean_all_ro(struct net *net,
+                                              int (*func)(struct rt6_info *, void *arg),
+                                              int prune, void *arg);
+
 extern void                    fib6_clean_all(struct net *net,
                                               int (*func)(struct rt6_info *, void *arg),
                                               int prune, void *arg);
index 246d8e4..b82bcde 100644 (file)
@@ -1462,6 +1462,26 @@ static void fib6_clean_tree(struct net *net, struct fib6_node *root,
        fib6_walk(&c.w);
 }
 
+void fib6_clean_all_ro(struct net *net, int (*func)(struct rt6_info *, void *arg),
+                   int prune, void *arg)
+{
+       struct fib6_table *table;
+       struct hlist_node *node;
+       struct hlist_head *head;
+       unsigned int h;
+
+       rcu_read_lock();
+       for (h = 0; h < FIB6_TABLE_HASHSZ; h++) {
+               head = &net->ipv6.fib_table_hash[h];
+               hlist_for_each_entry_rcu(table, node, head, tb6_hlist) {
+                       read_lock_bh(&table->tb6_lock);
+                       fib6_clean_tree(net, &table->tb6_root,
+                                       func, prune, arg);
+                       read_unlock_bh(&table->tb6_lock);
+               }
+       }
+       rcu_read_unlock();
+}
 void fib6_clean_all(struct net *net, int (*func)(struct rt6_info *, void *arg),
                    int prune, void *arg)
 {
index 4a62c47..07361df 100644 (file)
@@ -2680,7 +2680,7 @@ static int rt6_info_route(struct rt6_info *rt, void *p_arg)
 static int ipv6_route_show(struct seq_file *m, void *v)
 {
        struct net *net = (struct net *)m->private;
-       fib6_clean_all(net, rt6_info_route, 0, m);
+       fib6_clean_all_ro(net, rt6_info_route, 0, m);
        return 0;
 }