From: Ben Pfaff <blp@nicira.com>
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;
 }