openvswitch: Split ovs_flow_cmd_new_or_set().

Following patch will be easier to reason about with separate
ovs_flow_cmd_new() and ovs_flow_cmd_set() functions.

Signed-off-by: Jarno Rajahalme <jrajahalme@nicira.com>
Signed-off-by: Pravin B Shelar <pshelar@nicira.com>

1 files changed, 116 insertions, 44 deletions

diff --git a/net/openvswitch/datapath.c b/net/openvswitch/datapath.c
index f28beda8426f..22665a6144fc 100644
--- a/net/openvswitch/datapath.c
+++ b/net/openvswitch/datapath.c
@@ -792,16 +792,16 @@ static struct sk_buff *ovs_flow_cmd_build_info(const struct sw_flow *flow,
         return skb;
 }
 
-static int ovs_flow_cmd_new_or_set(struct sk_buff *skb, struct genl_info *info)
+static int ovs_flow_cmd_new(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_actions *acts;
         struct sw_flow_match match;
         int error;
 
@@ -817,23 +817,21 @@ static int ovs_flow_cmd_new_or_set(struct sk_buff *skb, struct genl_info *info)
                 goto error;
 
         /* Validate actions. */
-        if (a[OVS_FLOW_ATTR_ACTIONS]) {
-                acts = ovs_nla_alloc_flow_actions(nla_len(a[OVS_FLOW_ATTR_ACTIONS]));
-                error = PTR_ERR(acts);
-                if (IS_ERR(acts))
-                        goto error;
+        error = -EINVAL;
+        if (!a[OVS_FLOW_ATTR_ACTIONS])
+                goto error;
 
-                ovs_flow_mask_key(&masked_key, &key, &mask);
-                error = ovs_nla_copy_actions(a[OVS_FLOW_ATTR_ACTIONS],
-                                             &masked_key, 0, &acts);
-                if (error) {
-                        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) {
-                /* OVS_FLOW_CMD_NEW must have actions. */
-                error = -EINVAL;
+        acts = ovs_nla_alloc_flow_actions(nla_len(a[OVS_FLOW_ATTR_ACTIONS]));
+        error = PTR_ERR(acts);
+        if (IS_ERR(acts))
                 goto error;
+
+        ovs_flow_mask_key(&masked_key, &key, &mask);
+        error = ovs_nla_copy_actions(a[OVS_FLOW_ATTR_ACTIONS],
+                                     &masked_key, 0, &acts);
+        if (error) {
+                OVS_NLERR("Flow actions may not be safe on all matching packets.\n");
+                goto err_kfree;
         }
 
         ovs_lock();
@@ -845,11 +843,6 @@ 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();
                 if (IS_ERR(flow)) {
@@ -867,11 +860,9 @@ static int ovs_flow_cmd_new_or_set(struct sk_buff *skb, struct genl_info *info)
                         acts = NULL;
                         goto err_flow_free;
                 }
-
-                reply = ovs_flow_cmd_build_info(flow, ovs_header->dp_ifindex,
-                                                info, OVS_FLOW_CMD_NEW, false);
         } 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
@@ -879,30 +870,113 @@ static int ovs_flow_cmd_new_or_set(struct sk_buff *skb, struct genl_info *info)
                  * gets fixed.
                  */
                 error = -EEXIST;
-                if (info->genlhdr->cmd == OVS_FLOW_CMD_NEW &&
-                    info->nlhdr->nlmsg_flags & (NLM_F_CREATE | NLM_F_EXCL))
+                if (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. */
                 if (!ovs_flow_cmp_unmasked_key(flow, &match))
                         goto err_unlock_ovs;
 
-                /* Update actions, if present. */
-                if (acts) {
-                        struct sw_flow_actions *old_acts;
+                /* 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, false);
+        ovs_unlock();
+
+        if (reply) {
+                if (!IS_ERR(reply))
+                        ovs_notify(&dp_flow_genl_family, reply, info);
+                else
+                        netlink_set_err(sock_net(skb->sk)->genl_sock, 0, 0,
+                                        PTR_ERR(reply));
+        }
+        return 0;
+
+err_flow_free:
+        ovs_flow_free(flow, false);
+err_unlock_ovs:
+        ovs_unlock();
+err_kfree:
+        kfree(acts);
+error:
+        return error;
+}
 
-                        old_acts = ovsl_dereference(flow->sf_acts);
-                        rcu_assign_pointer(flow->sf_acts, acts);
-                        ovs_nla_free_flow_actions(old_acts);
+static int ovs_flow_cmd_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;
+        struct sw_flow_mask mask;
+        struct sk_buff *reply = NULL;
+        struct datapath *dp;
+        struct sw_flow_actions *acts = NULL;
+        struct sw_flow_match match;
+        int error;
+
+        /* Extract key. */
+        error = -EINVAL;
+        if (!a[OVS_FLOW_ATTR_KEY])
+                goto error;
+
+        ovs_match_init(&match, &key, &mask);
+        error = ovs_nla_get_match(&match,
+                                  a[OVS_FLOW_ATTR_KEY], a[OVS_FLOW_ATTR_MASK]);
+        if (error)
+                goto error;
+
+        /* Validate actions. */
+        if (a[OVS_FLOW_ATTR_ACTIONS]) {
+                acts = ovs_nla_alloc_flow_actions(nla_len(a[OVS_FLOW_ATTR_ACTIONS]));
+                error = PTR_ERR(acts);
+                if (IS_ERR(acts))
+                        goto error;
+
+                ovs_flow_mask_key(&masked_key, &key, &mask);
+                error = ovs_nla_copy_actions(a[OVS_FLOW_ATTR_ACTIONS],
+                                             &masked_key, 0, &acts);
+                if (error) {
+                        OVS_NLERR("Flow actions may not be safe on all matching packets.\n");
+                        goto err_kfree;
                 }
+        }
 
-                reply = ovs_flow_cmd_build_info(flow, ovs_header->dp_ifindex,
-                                                info, OVS_FLOW_CMD_NEW, false);
+        ovs_lock();
+        dp = get_dp(sock_net(skb->sk), ovs_header->dp_ifindex);
+        error = -ENODEV;
+        if (!dp)
+                goto err_unlock_ovs;
+
+        /* Check that the flow exists. */
+        flow = ovs_flow_tbl_lookup(&dp->table, &key);
+        error = -ENOENT;
+        if (!flow)
+                goto err_unlock_ovs;
+
+        /* The unmasked key has to be the same for flow updates. */
+        error = -EEXIST;
+        if (!ovs_flow_cmp_unmasked_key(flow, &match))
+                goto err_unlock_ovs;
 
-                /* Clear stats. */
-                if (a[OVS_FLOW_ATTR_CLEAR])
-                        ovs_flow_stats_clear(flow);
+        /* Update actions, if present. */
+        if (acts) {
+                struct sw_flow_actions *old_acts;
+
+                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, false);
+        /* Clear stats. */
+        if (a[OVS_FLOW_ATTR_CLEAR])
+                ovs_flow_stats_clear(flow);
         ovs_unlock();
 
         if (reply) {
@@ -915,8 +989,6 @@ 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();
 err_kfree:
@@ -1078,7 +1150,7 @@ static const struct genl_ops dp_flow_genl_ops[] = {
         { .cmd = OVS_FLOW_CMD_NEW,
           .flags = GENL_ADMIN_PERM, /* Requires CAP_NET_ADMIN privilege. */
           .policy = flow_policy,
-          .doit = ovs_flow_cmd_new_or_set
+          .doit = ovs_flow_cmd_new
         },
         { .cmd = OVS_FLOW_CMD_DEL,
           .flags = GENL_ADMIN_PERM, /* Requires CAP_NET_ADMIN privilege. */
@@ -1094,7 +1166,7 @@ static const struct genl_ops dp_flow_genl_ops[] = {
         { .cmd = OVS_FLOW_CMD_SET,
           .flags = GENL_ADMIN_PERM, /* Requires CAP_NET_ADMIN privilege. */
           .policy = flow_policy,
-          .doit = ovs_flow_cmd_new_or_set,
+          .doit = ovs_flow_cmd_set,
         },
 };