Signed-off-by: Jarno Rajahalme <jrajaha...@nicira.com> --- 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 dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev