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

Reply via email to