[ovs-dev] [PATCH v2 09/13] datapath: Minimize ovs_flow_cmd_new_or_set critical section.
Jarno Rajahalme
jrajahalme at nicira.com
Wed Feb 12 00:07:20 UTC 2014
Signed-off-by: Jarno Rajahalme <jrajahalme at nicira.com>
---
datapath/datapath.c | 83 ++++++++++++++++++++++++-------------------------
datapath/flow_table.c | 2 +-
datapath/flow_table.h | 2 +-
3 files changed, 43 insertions(+), 44 deletions(-)
diff --git a/datapath/datapath.c b/datapath/datapath.c
index cbdd68f..1599d70 100644
--- a/datapath/datapath.c
+++ b/datapath/datapath.c
@@ -745,15 +745,11 @@ error:
return err;
}
-/* Must be called with ovs_mutex. */
-static struct sk_buff *ovs_flow_cmd_alloc_info(struct sw_flow *flow,
+static struct sk_buff *ovs_flow_cmd_alloc_info(const struct sw_flow_actions *acts,
struct genl_info *info)
{
- size_t len;
-
- len = ovs_flow_cmd_msg_size(ovsl_dereference(flow->sf_acts));
-
- return genlmsg_new_unicast(len, info, GFP_KERNEL);
+ return genlmsg_new_unicast(ovs_flow_cmd_msg_size(acts), info,
+ GFP_KERNEL);
}
/* Must be called with ovs_mutex. */
@@ -764,7 +760,7 @@ static struct sk_buff *ovs_flow_cmd_build_info(struct sw_flow *flow,
struct sk_buff *skb;
int retval;
- skb = ovs_flow_cmd_alloc_info(flow, info);
+ skb = ovs_flow_cmd_alloc_info(ovsl_dereference(flow->sf_acts), info);
if (!skb)
return ERR_PTR(-ENOMEM);
@@ -780,13 +776,14 @@ static int ovs_flow_cmd_new_or_set(struct sk_buff *skb, struct genl_info *info)
struct nlattr **a = info->attrs;
struct ovs_header *ovs_header = info->userhdr;
struct sw_flow_key key, masked_key;
- struct sw_flow *flow = NULL;
+ struct sw_flow *flow;
struct sw_flow_mask mask;
struct sk_buff *reply;
struct datapath *dp;
struct sw_flow_actions *acts = NULL;
struct sw_flow_match match;
int error;
+ u8 cmd = info->genlhdr->cmd;
/* Extract key. */
error = -EINVAL;
@@ -811,7 +808,7 @@ static int ovs_flow_cmd_new_or_set(struct sk_buff *skb, struct genl_info *info)
&masked_key, 0, &acts);
if (error) {
OVS_NLERR("Flow actions may not be safe on all matching packets.\n");
- goto err_kfree;
+ goto err_kfree_acts;
}
} else if (info->genlhdr->cmd == OVS_FLOW_CMD_SET) {
/* Need empty actions. */
@@ -825,84 +822,86 @@ static int ovs_flow_cmd_new_or_set(struct sk_buff *skb, struct genl_info *info)
goto error;
}
+ reply = ovs_flow_cmd_alloc_info(acts, info);
+ if (!reply) {
+ error = -ENOMEM;
+ goto err_kfree_acts;
+ }
+
ovs_lock();
dp = get_dp(sock_net(skb->sk), ovs_header->dp_ifindex);
error = -ENODEV;
if (!dp)
- goto err_unlock_ovs;
+ goto err_unlock;
/* Check if this is a duplicate flow */
flow = ovs_flow_tbl_lookup(&dp->table, &key);
if (!flow) {
/* Bail out if we're not allowed to create a new flow. */
error = -ENOENT;
- if (info->genlhdr->cmd == OVS_FLOW_CMD_SET)
- goto err_unlock_ovs;
+ if (cmd == OVS_FLOW_CMD_SET)
+ goto err_unlock;
/* Allocate flow. */
flow = ovs_flow_alloc();
if (IS_ERR(flow)) {
error = PTR_ERR(flow);
- goto err_unlock_ovs;
+ goto err_unlock;
}
-
flow->key = masked_key;
flow->unmasked_key = key;
rcu_assign_pointer(flow->sf_acts, acts);
+ acts = NULL;
/* Put flow in bucket. */
error = ovs_flow_tbl_insert(&dp->table, flow, &mask);
+
if (error) {
- acts = NULL;
- goto err_flow_free;
+ ovs_flow_free(flow, false);
+ goto err_unlock;
}
-
- reply = ovs_flow_cmd_build_info(flow, ovs_header->dp_ifindex,
- info, OVS_FLOW_CMD_NEW);
} else {
/* We found a matching flow. */
struct sw_flow_actions *old_acts;
- /* Bail out if we're not allowed to modify an existing flow.
- * We accept NLM_F_CREATE in place of the intended NLM_F_EXCL
- * because Generic Netlink treats the latter as a dump
- * request. We also accept NLM_F_EXCL in case that bug ever
- * gets fixed.
+ /* Bail out if we're not allowed to modify an
+ * existing flow. We accept NLM_F_CREATE in
+ * place of the intended NLM_F_EXCL because
+ * Generic Netlink treats the latter as a dump
+ * request. We also accept NLM_F_EXCL in case
+ * that bug ever gets fixed.
*/
error = -EEXIST;
- if (info->genlhdr->cmd == OVS_FLOW_CMD_NEW &&
- info->nlhdr->nlmsg_flags & (NLM_F_CREATE | NLM_F_EXCL))
- goto err_unlock_ovs;
-
+ if (cmd == OVS_FLOW_CMD_NEW
+ && info->nlhdr->nlmsg_flags & (NLM_F_CREATE | NLM_F_EXCL))
+ goto err_unlock;
/* The unmasked key has to be the same for flow updates. */
if (!ovs_flow_cmp_unmasked_key(flow, &match))
- goto err_unlock_ovs;
+ goto err_unlock;
/* Update actions. */
old_acts = ovsl_dereference(flow->sf_acts);
rcu_assign_pointer(flow->sf_acts, acts);
ovs_nla_free_flow_actions(old_acts);
- reply = ovs_flow_cmd_build_info(flow, ovs_header->dp_ifindex,
- info, OVS_FLOW_CMD_NEW);
/* Clear stats. */
if (a[OVS_FLOW_ATTR_CLEAR])
ovs_flow_stats_clear(flow);
}
+
+ error = ovs_flow_cmd_fill_info(flow, ovs_header->dp_ifindex, reply,
+ info->snd_portid, info->snd_seq, 0,
+ OVS_FLOW_CMD_NEW);
+ BUG_ON(error < 0);
ovs_unlock();
- if (!IS_ERR(reply))
- ovs_notify(reply, info, &ovs_dp_flow_multicast_group);
- else
- netlink_set_err(sock_net(skb->sk)->genl_sock, 0,
- ovs_dp_flow_multicast_group.id, PTR_ERR(reply));
+ ovs_notify(reply, info, &ovs_dp_flow_multicast_group);
return 0;
-err_flow_free:
- ovs_flow_free(flow, false);
-err_unlock_ovs:
+err_unlock:
ovs_unlock();
-err_kfree:
+ kfree_skb(reply);
+err_kfree_acts:
kfree(acts);
error:
return error;
@@ -993,7 +992,7 @@ static int ovs_flow_cmd_del(struct sk_buff *skb, struct genl_info *info)
ovs_flow_tbl_remove(&dp->table, flow);
ovs_unlock();
- reply = ovs_flow_cmd_alloc_info(flow, info);
+ reply = ovs_flow_cmd_alloc_info(ovsl_dereference(flow->sf_acts), info);
/* XXX: We may skip sending a response if kernel is out of memory.
* We could do the allocation before locking if we did not need
* to return the actions (which are useless in this case anyway). */
diff --git a/datapath/flow_table.c b/datapath/flow_table.c
index c04fb4d..b9c7ef0 100644
--- a/datapath/flow_table.c
+++ b/datapath/flow_table.c
@@ -422,7 +422,7 @@ static bool flow_cmp_masked_key(const struct sw_flow *flow,
}
bool ovs_flow_cmp_unmasked_key(const struct sw_flow *flow,
- struct sw_flow_match *match)
+ const struct sw_flow_match *match)
{
struct sw_flow_key *key = match->key;
int key_start = flow_key_start(key);
diff --git a/datapath/flow_table.h b/datapath/flow_table.h
index ca8a582..e3ab93d 100644
--- a/datapath/flow_table.h
+++ b/datapath/flow_table.h
@@ -78,7 +78,7 @@ struct sw_flow *ovs_flow_tbl_lookup(struct flow_table *,
const struct sw_flow_key *);
bool ovs_flow_cmp_unmasked_key(const struct sw_flow *flow,
- struct sw_flow_match *match);
+ const struct sw_flow_match *match);
void ovs_flow_mask_key(struct sw_flow_key *dst, const struct sw_flow_key *src,
const struct sw_flow_mask *mask);
--
1.7.10.4
More information about the dev
mailing list