[ovs-dev] [netlink v5 56/61] datapath: Convert ODP_FLOW_* commands to use AF_NETLINK socket layer.

Ben Pfaff blp at nicira.com
Fri Jan 28 22:08:15 UTC 2011


On Fri, Jan 28, 2011 at 01:46:57PM -0800, Ben Pfaff wrote:
> Jesse, I've done it three or four ways now.  Pick one.

OK, here's the revert that I applied to "datapath: Convert ODP_DP_*
commands to use AF_NETLINK socket layer":

diff --git a/datapath/datapath.c b/datapath/datapath.c
index faf2e09..f42ead1 100644
--- a/datapath/datapath.c
+++ b/datapath/datapath.c
@@ -1451,15 +1451,15 @@ static int odp_dp_cmd_set(struct sk_buff *skb, struct genl_info *info)
 	if (IS_ERR(dp))
 		return PTR_ERR(dp);
 
-	reply = nlmsg_new(NLMSG_DEFAULT_SIZE, GFP_KERNEL);
-	if (!reply)
-		return -ENOMEM;
-
 	change_datapath(dp, info->attrs);
 
-	err = odp_dp_cmd_fill_info(dp, reply, info->snd_pid, info->snd_seq,
-				   0, ODP_DP_CMD_NEW);
-	BUG_ON(err < 0);
+	reply = odp_dp_cmd_build_info(dp, info->snd_pid, info->snd_seq, ODP_DP_CMD_NEW);
+	if (IS_ERR(reply)) {
+		err = PTR_ERR(reply);
+		netlink_set_err(INIT_NET_GENL_SOCK, 0,
+				dp_datapath_multicast_group.id, err);
+		return 0;
+	}
 
 	genl_notify(reply, genl_info_net(info), info->snd_pid,
 		    dp_datapath_multicast_group.id, info->nlhdr, GFP_KERNEL);

Here's the revert that I applied to "datapath: Convert ODP_VPORT_* to
use AF_NETLINK socket layer":

diff --git a/datapath/datapath.c b/datapath/datapath.c
index 3663b7f..4caa496 100644
--- a/datapath/datapath.c
+++ b/datapath/datapath.c
@@ -1774,27 +1774,23 @@ static int odp_vport_cmd_set(struct sk_buff *skb, struct genl_info *info)
 	if (IS_ERR(vport))
 		goto exit_unlock;
 
-	reply = nlmsg_new(NLMSG_DEFAULT_SIZE, GFP_KERNEL);
-	if (!reply)
-		goto exit_unlock;
-
 	err = 0;
 	if (a[ODP_VPORT_ATTR_OPTIONS])
 		err = vport_set_options(vport, a[ODP_VPORT_ATTR_OPTIONS]);
 	if (!err)
 		err = change_vport(vport, a);
-	if (err) {
-		kfree_skb(reply);
-		goto exit_unlock;
-	}
 
-	err = odp_vport_cmd_fill_info(vport, reply, info->snd_pid,
-				      info->snd_seq, 0, ODP_VPORT_CMD_NEW);
-	BUG_ON(err < 0);
+	reply = odp_vport_cmd_build_info(vport, info->snd_pid, info->snd_seq,
+					 ODP_VPORT_CMD_NEW);
+	if (IS_ERR(reply)) {
+		err = PTR_ERR(reply);
+		netlink_set_err(INIT_NET_GENL_SOCK, 0,
+				dp_vport_multicast_group.id, err);
+		return 0;
+	}
 
 	genl_notify(reply, genl_info_net(info), info->snd_pid,
 		    dp_vport_multicast_group.id, info->nlhdr, GFP_KERNEL);
-	err = 0;
 
 exit_unlock:
 	rtnl_unlock();

Here's the revert that I applied to "datapath: Convert ODP_FLOW_*
commands to use AF_NETLINK socket layer":

diff --git a/datapath/datapath.c b/datapath/datapath.c
index 34cbe71..8931456 100644
--- a/datapath/datapath.c
+++ b/datapath/datapath.c
@@ -879,12 +879,12 @@ error:
 	return err;
 }
 
-static struct sk_buff *odp_flow_cmd_alloc_info(struct sw_flow_actions __rcu *actions)
+static struct sk_buff *odp_flow_cmd_alloc_info(struct sw_flow *flow)
 {
 	const struct sw_flow_actions *sf_acts;
 	int len;
 
-	sf_acts = rcu_dereference_protected(actions,
+	sf_acts = rcu_dereference_protected(flow->sf_acts,
 					    lockdep_genl_is_held());
 
 	len = nla_total_size(FLOW_BUFSIZE); /* ODP_FLOW_ATTR_KEY */
@@ -901,7 +901,7 @@ static struct sk_buff *odp_flow_cmd_build_info(struct sw_flow *flow, struct data
 	struct sk_buff *skb;
 	int retval;
 
-	skb = odp_flow_cmd_alloc_info(flow->sf_acts);
+	skb = odp_flow_cmd_alloc_info(flow);
 	if (!skb)
 		return ERR_PTR(-ENOMEM);
 
@@ -981,16 +981,13 @@ static int odp_flow_cmd_new_or_set(struct sk_buff *skb, struct genl_info *info)
 			goto error_free_flow;
 		rcu_assign_pointer(flow->sf_acts, acts);
 
-		/* Create reply. */
-		reply = odp_flow_cmd_build_info(flow, dp, info->snd_pid,
-						info->snd_seq, ODP_FLOW_CMD_NEW);
-		if (!reply)
-			goto error_free_flow;
-
 		/* Put flow in bucket. */
 		error = tbl_insert(table, &flow->tbl_node, hash);
 		if (error)
-			goto error_free_reply;
+			goto error_free_flow;
+
+		reply = odp_flow_cmd_build_info(flow, dp, info->snd_pid,
+						info->snd_seq, ODP_FLOW_CMD_NEW);
 	} else {
 		/* We found a matching flow. */
 		struct sw_flow_actions *old_acts;
@@ -1006,8 +1003,6 @@ static int odp_flow_cmd_new_or_set(struct sk_buff *skb, struct genl_info *info)
 		    info->nlhdr->nlmsg_flags & (NLM_F_CREATE | NLM_F_EXCL))
 			goto error;
 
-		/* Preallocate reply. */
-
 		/* Update actions. */
 		flow = flow_cast(flow_node);
 		old_acts = rcu_dereference_protected(flow->sf_acts,
@@ -1023,22 +1018,12 @@ static int odp_flow_cmd_new_or_set(struct sk_buff *skb, struct genl_info *info)
 			if (IS_ERR(new_acts))
 				goto error;
 
-			reply = odp_flow_cmd_alloc_info(new_acts);
-			error = -ENOMEM;
-			if (!reply) {
-				kfree(new_acts);
-				goto error;
-			}
-
 			rcu_assign_pointer(flow->sf_acts, new_acts);
 			flow_deferred_free_acts(old_acts);
-		} else {
-			reply = odp_flow_cmd_alloc_info(flow->sf_acts);
 		}
 
-		error = odp_flow_cmd_fill_info(flow, dp, reply, info->snd_pid,
-					       info->snd_seq, 0, ODP_FLOW_CMD_NEW);
-		BUG_ON(error < 0);
+		reply = odp_flow_cmd_build_info(flow, dp, info->snd_pid,
+						info->snd_seq, ODP_FLOW_CMD_NEW);
 
 		/* Clear stats. */
 		if (a[ODP_FLOW_ATTR_CLEAR]) {
@@ -1048,12 +1033,14 @@ static int odp_flow_cmd_new_or_set(struct sk_buff *skb, struct genl_info *info)
 		}
 	}
 
-	genl_notify(reply, genl_info_net(info), info->snd_pid,
-		    dp_flow_multicast_group.id, info->nlhdr, GFP_KERNEL);
+	if (!IS_ERR(reply))
+		genl_notify(reply, genl_info_net(info), info->snd_pid,
+			    dp_flow_multicast_group.id, info->nlhdr, GFP_KERNEL);
+	else
+		netlink_set_err(INIT_NET_GENL_SOCK, 0,
+				dp_flow_multicast_group.id, PTR_ERR(reply));
 	return 0;
 
-error_free_reply:
-	kfree_skb(reply);
 error_free_flow:
 	flow_put(flow);
 error:
@@ -1123,7 +1110,7 @@ static int odp_flow_cmd_del(struct sk_buff *skb, struct genl_info *info)
 		return -ENOENT;
 	flow = flow_cast(flow_node);
 
-	reply = odp_flow_cmd_alloc_info(flow->sf_acts);
+	reply = odp_flow_cmd_alloc_info(flow);
 	if (!reply)
 		return -ENOMEM;
 




More information about the dev mailing list