datapath: Don't directly access RCU protected pointers.

If RTNL lock is used to protected updates to RCU data structures
then it isn't necessary to use rcu_dereference() to access them if
RTNL is held.  This adds rtnl_dereference() to access these pointers
which has several benefits: documents the locking expectations;
checks that RTNL actually is held when run with lockdep; makes
sparse not complain about directly accessing RCU pointers.

Signed-off-by: Jesse Gross <jesse@nicira.com>
Acked-by: Ben Pfaff <blp@nicira.com>

diff --git a/datapath/datapath.c b/datapath/datapath.c
index c633e84..b44c967 100644 (file)
--- a/datapath/datapath.c
+++ b/datapath/datapath.c
@@ -96,7 +96,7 @@ static struct datapath *get_dp_locked(int dp_idx)
 /* Must be called with rcu_read_lock or RTNL lock. */
 const char *dp_name(const struct datapath *dp)
 {
-       return vport_get_name(dp->ports[ODPP_LOCAL]);
+       return vport_get_name(rcu_dereference_rtnl(dp->ports[ODPP_LOCAL]));
 }
 
 static inline size_t br_nlmsg_size(void)
@@ -139,7 +139,8 @@ static int dp_fill_ifinfo(struct sk_buff *skb,
        hdr->ifi_change = 0;
 
        NLA_PUT_STRING(skb, IFLA_IFNAME, vport_get_name(port));
-       NLA_PUT_U32(skb, IFLA_MASTER, vport_get_ifindex(dp->ports[ODPP_LOCAL]));
+       NLA_PUT_U32(skb, IFLA_MASTER,
+               vport_get_ifindex(rtnl_dereference(dp->ports[ODPP_LOCAL])));
        NLA_PUT_U32(skb, IFLA_MTU, vport_get_mtu(port));
 #ifdef IFLA_OPERSTATE
        NLA_PUT_U8(skb, IFLA_OPERSTATE,

diff --git a/datapath/dp_sysfs_dp.c b/datapath/dp_sysfs_dp.c
index 6cbd864..587b8bc 100644 (file)
--- a/datapath/dp_sysfs_dp.c
+++ b/datapath/dp_sysfs_dp.c
@@ -350,7 +350,8 @@ static struct attribute_group bridge_group = {
  */
 int dp_sysfs_add_dp(struct datapath *dp)
 {
-       struct kobject *kobj = vport_get_kobj(dp->ports[ODPP_LOCAL]);
+       struct kobject *kobj =
+               vport_get_kobj(rtnl_dereference(dp->ports[ODPP_LOCAL]));
        int err;
 
        /* Create /sys/class/net/<devname>/bridge directory. */
@@ -379,7 +380,8 @@ int dp_sysfs_add_dp(struct datapath *dp)
 
 int dp_sysfs_del_dp(struct datapath *dp)
 {
-       struct kobject *kobj = vport_get_kobj(dp->ports[ODPP_LOCAL]);
+       struct kobject *kobj =
+               vport_get_kobj(rtnl_dereference(dp->ports[ODPP_LOCAL]));
 
        kobject_del(&dp->ifobj);
        sysfs_remove_group(kobj, &bridge_group);

diff --git a/datapath/dp_sysfs_if.c b/datapath/dp_sysfs_if.c
index fc7c2d8..c688bc4 100644 (file)
--- a/datapath/dp_sysfs_if.c
+++ b/datapath/dp_sysfs_if.c
@@ -220,8 +220,9 @@ int dp_sysfs_add_if(struct vport *p)
 
        /* Create symlink from /sys/class/net/<devname>/brport/bridge to
         * /sys/class/net/<bridgename>. */
-       err = sysfs_create_link(&p->kobj, vport_get_kobj(dp->ports[ODPP_LOCAL]),
-                               SYSFS_BRIDGE_PORT_LINK); /* "bridge" */
+       err = sysfs_create_link(&p->kobj,
+               vport_get_kobj(rtnl_dereference(dp->ports[ODPP_LOCAL])),
+               SYSFS_BRIDGE_PORT_LINK); /* "bridge" */
        if (err)
                goto err_del;
 

diff --git a/datapath/tunnel.c b/datapath/tunnel.c
index 9824129..239e604 100644 (file)
--- a/datapath/tunnel.c
+++ b/datapath/tunnel.c
@@ -132,7 +132,7 @@ static void assign_config_rcu(struct vport *vport,
        struct tnl_vport *tnl_vport = tnl_vport_priv(vport);
        struct tnl_mutable_config *old_config;
 
-       old_config = tnl_vport->mutable;
+       old_config = rtnl_dereference(tnl_vport->mutable);
        rcu_assign_pointer(tnl_vport->mutable, new_config);
        call_rcu(&old_config->rcu, free_config_rcu);
 }
@@ -210,9 +210,9 @@ static u32 mutable_hash(const struct tnl_mutable_config *mutable)
 
 static void check_table_empty(void)
 {
-       if (tbl_count(port_table) == 0) {
-               struct tbl *old_table = port_table;
+       struct tbl *old_table = rtnl_dereference(port_table);
 
+       if (tbl_count(old_table) == 0) {
                cancel_delayed_work_sync(&cache_cleaner_wq);
                rcu_assign_pointer(port_table, NULL);
                tbl_deferred_destroy(old_table, NULL);
@@ -221,6 +221,7 @@ static void check_table_empty(void)
 
 static int add_port(struct vport *vport)
 {
+       struct tbl *cur_table = rtnl_dereference(port_table);
        struct tnl_vport *tnl_vport = tnl_vport_priv(vport);
        int err;
 
@@ -234,25 +235,25 @@ static int add_port(struct vport *vport)
                rcu_assign_pointer(port_table, new_table);
                schedule_cache_cleaner();
 
-       } else if (tbl_count(port_table) > tbl_n_buckets(port_table)) {
-               struct tbl *old_table = port_table;
+       } else if (tbl_count(cur_table) > tbl_n_buckets(cur_table)) {
                struct tbl *new_table;
 
-               new_table = tbl_expand(old_table);
+               new_table = tbl_expand(cur_table);
                if (IS_ERR(new_table))
                        return PTR_ERR(new_table);
 
                rcu_assign_pointer(port_table, new_table);
-               tbl_deferred_destroy(old_table, NULL);
+               tbl_deferred_destroy(cur_table, NULL);
        }
 
-       err = tbl_insert(port_table, &tnl_vport->tbl_node, mutable_hash(tnl_vport->mutable));
+       err = tbl_insert(rtnl_dereference(port_table), &tnl_vport->tbl_node,
+                        mutable_hash(rtnl_dereference(tnl_vport->mutable)));
        if (err) {
                check_table_empty();
                return err;
        }
 
-       (*find_port_pool(tnl_vport->mutable))++;
+       (*find_port_pool(rtnl_dereference(tnl_vport->mutable)))++;
 
        return 0;
 }
@@ -260,6 +261,7 @@ static int add_port(struct vport *vport)
 static int move_port(struct vport *vport, struct tnl_mutable_config *new_mutable)
 {
        int err;
+       struct tbl *cur_table = rtnl_dereference(port_table);
        struct tnl_vport *tnl_vport = tnl_vport_priv(vport);
        u32 hash;
 
@@ -272,21 +274,21 @@ static int move_port(struct vport *vport, struct tnl_mutable_config *new_mutable
         * finding tunnels or the possibility of failure.  However, if we do
         * find a tunnel it will always be consistent.
         */
-       err = tbl_remove(port_table, &tnl_vport->tbl_node);
+       err = tbl_remove(cur_table, &tnl_vport->tbl_node);
        if (err)
                return err;
 
-       err = tbl_insert(port_table, &tnl_vport->tbl_node, hash);
+       err = tbl_insert(cur_table, &tnl_vport->tbl_node, hash);
        if (err) {
-               (*find_port_pool(tnl_vport->mutable))--;
+               (*find_port_pool(rtnl_dereference(tnl_vport->mutable)))--;
                check_table_empty();
                return err;
        }
 
 table_updated:
-       (*find_port_pool(tnl_vport->mutable))--;
+       (*find_port_pool(rtnl_dereference(tnl_vport->mutable)))--;
        assign_config_rcu(vport, new_mutable);
-       (*find_port_pool(tnl_vport->mutable))++;
+       (*find_port_pool(rtnl_dereference(tnl_vport->mutable)))++;
 
        return 0;
 }
@@ -296,12 +298,12 @@ static int del_port(struct vport *vport)
        struct tnl_vport *tnl_vport = tnl_vport_priv(vport);
        int err;
 
-       err = tbl_remove(port_table, &tnl_vport->tbl_node);
+       err = tbl_remove(rtnl_dereference(port_table), &tnl_vport->tbl_node);
        if (err)
                return err;
 
        check_table_empty();
-       (*find_port_pool(tnl_vport->mutable))--;
+       (*find_port_pool(rtnl_dereference(tnl_vport->mutable)))--;
 
        return 0;
 }