Signed-off-by: Jarno Rajahalme <[email protected]>
---
datapath/datapath.c | 151 ++++++++++++++++++++++++++++++-------------------
datapath/flow_table.c | 2 +-
datapath/flow_table.h | 2 +-
3 files changed, 94 insertions(+), 61 deletions(-)
diff --git a/datapath/datapath.c b/datapath/datapath.c
index 92ae66a..5b12a5d 100644
--- a/datapath/datapath.c
+++ b/datapath/datapath.c
@@ -778,12 +778,34 @@ static struct sk_buff *ovs_flow_cmd_build_info(struct
sw_flow *flow,
return skb;
}
+/* Must be called while ovs_lock held. */
+static int ovs_flow_update(struct sw_flow *flow,
+ const struct sw_flow_match *match,
+ struct sw_flow_actions *acts)
+{
+ struct sw_flow_actions *old_acts;
+
+ /* The unmasked key has to be the same for flow updates. */
+ if (!ovs_flow_cmp_unmasked_key(flow, match)) {
+ OVS_NLERR("Flow modification message rejected, unmasked key
does not match.\n");
+ return -EINVAL;
+ }
+
+ /* Update actions. */
+ old_acts = ovsl_dereference(flow->sf_acts);
+ rcu_assign_pointer(flow->sf_acts, acts);
+ ovs_nla_free_flow_actions(old_acts);
+
+ return 0;
+}
+
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 *new_flow = NULL;
struct sw_flow_mask mask;
struct sk_buff *reply = NULL;
struct datapath *dp;
@@ -791,6 +813,7 @@ static int ovs_flow_cmd_new_or_set(struct sk_buff *skb,
struct genl_info *info)
struct sw_flow_match match;
bool exact_5tuple;
int error;
+ u8 cmd = info->genlhdr->cmd;
/* Extract key. */
error = -EINVAL;
@@ -817,11 +840,27 @@ static int ovs_flow_cmd_new_or_set(struct sk_buff *skb,
struct genl_info *info)
OVS_NLERR("Flow actions may not be safe on all matching
packets.\n");
goto err_kfree;
}
- } else if (info->genlhdr->cmd == OVS_FLOW_CMD_NEW) {
+ } else if (cmd == OVS_FLOW_CMD_NEW) {
error = -EINVAL;
goto error;
}
+ /* XXX: Is it OK to SET actions to NULL? */
+
+ if (info->nlhdr->nlmsg_flags & NLM_F_ECHO)
+ /* OK to pass NULL when not filling actions later. */
+ reply = ovs_flow_cmd_alloc_info(NULL, info, 0);
+ if (cmd == OVS_FLOW_CMD_NEW) {
+ /* Allocate flow. */
+ new_flow = ovs_flow_alloc(!exact_5tuple);
+ if (IS_ERR(new_flow)) {
+ error = PTR_ERR(new_flow);
+ new_flow = NULL;
+ } else {
+ new_flow->key = masked_key;
+ new_flow->unmasked_key = key;
+ }
+ }
ovs_lock();
dp = get_dp(sock_net(skb->sk), ovs_header->dp_ifindex);
error = -ENODEV;
@@ -830,71 +869,64 @@ static int ovs_flow_cmd_new_or_set(struct sk_buff *skb,
struct genl_info *info)
/* 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;
- /* Allocate flow. */
- flow = ovs_flow_alloc(!exact_5tuple);
- if (IS_ERR(flow)) {
- error = PTR_ERR(flow);
- goto err_unlock_ovs;
- }
+ if (cmd == OVS_FLOW_CMD_NEW) {
+ if (!flow) {
+ if (!new_flow) /* 'error' set above. */
+ goto err_unlock_ovs;
+ rcu_assign_pointer(new_flow->sf_acts, acts);
+ acts = NULL;
- flow->key = masked_key;
- flow->unmasked_key = key;
- rcu_assign_pointer(flow->sf_acts, acts);
+ /* Put flow in bucket. */
+ error = ovs_flow_tbl_insert(&dp->table, new_flow,
&mask);
+ if (error) {
+ goto err_unlock_ovs;
+ }
+ flow = new_flow;
+ new_flow = NULL;
+ } else {
+ /* We found a matching flow. */
+
+ /* 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->nlhdr->nlmsg_flags & (NLM_F_CREATE |
NLM_F_EXCL))
+ goto err_unlock_ovs;
- /* Put flow in bucket. */
- error = ovs_flow_tbl_insert(&dp->table, flow, &mask);
- if (error) {
+ error = ovs_flow_update(flow, &match, acts);
+ if (error)
+ goto err_unlock_ovs;
acts = NULL;
- goto err_flow_free;
}
-
- if (info->nlhdr->nlmsg_flags & NLM_F_ECHO)
- reply = ovs_flow_cmd_build_info(flow, dp, info,
- OVS_FLOW_CMD_NEW,
- 1 <<
OVS_FLOW_ATTR_STATS);
- } 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.
- */
- error = -EEXIST;
- if (info->genlhdr->cmd == OVS_FLOW_CMD_NEW &&
- info->nlhdr->nlmsg_flags & (NLM_F_CREATE | NLM_F_EXCL))
- goto err_unlock_ovs;
-
- /* The unmasked key has to be the same for flow updates. */
- error = -EINVAL;
- if (!ovs_flow_cmp_unmasked_key(flow, &match)) {
- OVS_NLERR("Flow modification message rejected, unmasked
key does not match.\n");
+ } else { /* OVS_FLOW_CMD_SET */
+ if (!flow) {
+ error = -ENOENT;
goto err_unlock_ovs;
}
-
- /* Update actions. */
- old_acts = ovsl_dereference(flow->sf_acts);
- rcu_assign_pointer(flow->sf_acts, acts);
- ovs_nla_free_flow_actions(old_acts);
-
- if (info->nlhdr->nlmsg_flags & NLM_F_ECHO)
- reply = ovs_flow_cmd_build_info(flow, dp, info,
- OVS_FLOW_CMD_NEW,
- 1 <<
OVS_FLOW_ATTR_STATS);
- /* Clear stats. */
- if (a[OVS_FLOW_ATTR_CLEAR])
- ovs_flow_stats_clear(flow);
+ /* We found a matching flow. */
+ error = ovs_flow_update(flow, &match, acts);
+ if (error)
+ goto err_unlock_ovs;
+ acts = NULL;
}
+ if (reply)
+ ovs_flow_cmd_fill_info(flow, dp, reply, info->snd_portid,
+ info->snd_seq, 0, OVS_FLOW_CMD_NEW,
+ 1 << OVS_FLOW_ATTR_STATS);
+ /* Clear stats. NEW would not set this, so there is no penalty of
+ * doing it here. */
+ if (a[OVS_FLOW_ATTR_CLEAR])
+ ovs_flow_stats_clear(flow);
ovs_unlock();
+ ovs_flow_free(new_flow, false); /* NULL if put in table. */
+ kfree(acts); /* NULL if put in the flow. */
+
if (reply) {
if (!IS_ERR(reply))
ovs_notify(reply, info, &ovs_dp_flow_multicast_group);
@@ -904,10 +936,11 @@ static int ovs_flow_cmd_new_or_set(struct sk_buff *skb,
struct genl_info *info)
}
return 0;
-err_flow_free:
- ovs_flow_free(flow, false);
err_unlock_ovs:
ovs_unlock();
+ ovs_flow_free(new_flow, false);
+ if (reply)
+ __kfree_skb(reply);
err_kfree:
kfree(acts);
error:
diff --git a/datapath/flow_table.c b/datapath/flow_table.c
index c0ba8ef..fdf22a9 100644
--- a/datapath/flow_table.c
+++ b/datapath/flow_table.c
@@ -425,7 +425,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 baaeb10..5806724 100644
--- a/datapath/flow_table.h
+++ b/datapath/flow_table.h
@@ -76,7 +76,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
_______________________________________________
dev mailing list
[email protected]
http://openvswitch.org/mailman/listinfo/dev