From ad9197112ae58f338491e6344a9f66073e4bce95 Mon Sep 17 00:00:00 2001 From: Jesse Gross Date: Sun, 5 Dec 2010 11:22:04 -0800 Subject: [PATCH] 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 Acked-by: Ben Pfaff --- datapath/datapath.c | 5 +++-- datapath/dp_sysfs_dp.c | 6 ++++-- datapath/dp_sysfs_if.c | 5 +++-- datapath/tunnel.c | 34 ++++++++++++++++++---------------- 4 files changed, 28 insertions(+), 22 deletions(-) diff --git a/datapath/datapath.c b/datapath/datapath.c index c633e846..b44c9679 100644 --- 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 6cbd8641..587b8bc2 100644 --- 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//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 fc7c2d8e..c688bc4b 100644 --- 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//brport/bridge to * /sys/class/net/. */ - 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 98241291..239e604c 100644 --- 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; } -- 2.30.2