From: Ben Pfaff Date: Tue, 30 Dec 2008 21:03:48 +0000 (-0800) Subject: Properly lock dp_mutex around changes to the datapath. X-Git-Url: https://pintos-os.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=701c04342b498b0200c3b4b998eade0649a57921;p=openvswitch Properly lock dp_mutex around changes to the datapath. We weren't locking dp_mutex() here but it really is necessary. See the comment on dp_mutex itself for details. This actually restores some of the locking removed by commit 47b8652d "Simplify use of dp_mutex." That commit is correct that we can take dp_mutex at a high level in dp_genl_openflow(), but it removes locking from functions that are not called through dp_genl_openflow(): in particular any Netlink command other than DP_GENL_C_OPENFLOW does not go through that function, so those commands need to acquire the mutex themselves. --- diff --git a/datapath/datapath.c b/datapath/datapath.c index e10b5206..e5f011d1 100644 --- a/datapath/datapath.c +++ b/datapath/datapath.c @@ -258,25 +258,27 @@ static int new_dp(int dp_idx, const char *dp_name) struct datapath *dp; int err; + mutex_lock(&dp_mutex); if (dp_idx == -1) dp_idx = gen_dp_idx(); + err = -EINVAL; if (dp_idx < 0 || dp_idx >= DP_MAX) - return -EINVAL; + goto err_unlock; + err = -ENODEV; if (!try_module_get(THIS_MODULE)) - return -ENODEV; + goto err_unlock; /* Exit early if a datapath with that number already exists. */ - if (dps[dp_idx]) { - err = -EEXIST; - goto err_unlock; - } + err = -EEXIST; + if (dps[dp_idx]) + goto err_put; err = -ENOMEM; dp = kzalloc(sizeof *dp, GFP_KERNEL); if (dp == NULL) - goto err_unlock; + goto err_put; dp->dp_idx = dp_idx; @@ -304,6 +306,7 @@ static int new_dp(int dp_idx, const char *dp_name) goto err_destroy_chain; dps[dp_idx] = dp; + mutex_unlock(&dp_mutex); return 0; @@ -315,9 +318,11 @@ err_destroy_dp_dev: dp_dev_destroy(dp); err_free_dp: kfree(dp); -err_unlock: +err_put: module_put(THIS_MODULE); - return err; +err_unlock: + mutex_unlock(&dp_mutex); + return err; } /* Find and return a free port number under 'dp'. */ @@ -1172,6 +1177,7 @@ static int dp_genl_del(struct sk_buff *skb, struct genl_info *info) struct datapath *dp; int err; + mutex_lock(&dp_mutex); dp = lookup_dp(info); if (IS_ERR(dp)) err = PTR_ERR(dp); @@ -1179,6 +1185,7 @@ static int dp_genl_del(struct sk_buff *skb, struct genl_info *info) del_dp(dp); err = 0; } + mutex_unlock(&dp_mutex); return err; } @@ -1256,10 +1263,11 @@ static int dp_genl_add_del_port(struct sk_buff *skb, struct genl_info *info) return -EINVAL; /* Get datapath. */ + mutex_lock(&dp_mutex); dp = lookup_dp(info); if (IS_ERR(dp)) { err = PTR_ERR(dp); - goto out; + goto out_unlock; } /* Get interface to add/remove. */ @@ -1267,7 +1275,7 @@ static int dp_genl_add_del_port(struct sk_buff *skb, struct genl_info *info) nla_data(info->attrs[DP_GENL_A_PORTNAME])); if (!port) { err = -ENOENT; - goto out; + goto out_unlock; } /* Execute operation. */ @@ -1283,7 +1291,8 @@ static int dp_genl_add_del_port(struct sk_buff *skb, struct genl_info *info) out_put: dev_put(port); -out: +out_unlock: + mutex_unlock(&dp_mutex); return err; }