datapath: 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>

diff --git a/datapath/datapath.c b/datapath/datapath.c
index aa0ce9e..f88147e 100644 (file)
--- a/datapath/datapath.c
+++ b/datapath/datapath.c
@@ -794,16 +794,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;
 
@@ -819,23 +819,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();
@@ -847,11 +845,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)) {
@@ -869,11 +862,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
@@ -881,30 +872,114 @@ 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(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));
+       }
+       return 0;
+
+err_flow_free:
+       ovs_flow_free(flow, false);
+err_unlock_ovs:
+       ovs_unlock();
+err_kfree:
+       kfree(acts);
+error:
+       return error;
+}
+
+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;
 
-                       old_acts = ovsl_dereference(flow->sf_acts);
-                       rcu_assign_pointer(flow->sf_acts, acts);
-                       ovs_nla_free_flow_actions(old_acts);
+       /* 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;
                }
+       }
+
+       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;
 
-               reply = ovs_flow_cmd_build_info(flow, ovs_header->dp_ifindex,
-                                               info, OVS_FLOW_CMD_NEW, false);
+       /* 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;
+
+       /* Update actions, if present. */
+       if (acts) {
+               struct sw_flow_actions *old_acts;
 
-               /* Clear stats. */
-               if (a[OVS_FLOW_ATTR_CLEAR])
-                       ovs_flow_stats_clear(flow);
+               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) {
@@ -917,8 +992,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:
@@ -1081,7 +1154,7 @@ static 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. */
@@ -1097,7 +1170,7 @@ static 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,
        },
 };