From patchwork Tue May 20 09:00:51 2014 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Pravin B Shelar X-Patchwork-Id: 351986 X-Patchwork-Delegate: davem@davemloft.net Return-Path: X-Original-To: patchwork-incoming@ozlabs.org Delivered-To: patchwork-incoming@ozlabs.org Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by ozlabs.org (Postfix) with ESMTP id 75CC914009C for ; Sat, 24 May 2014 03:06:58 +1000 (EST) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1753741AbaEWRGx (ORCPT ); Fri, 23 May 2014 13:06:53 -0400 Received: from na3sys009aog116.obsmtp.com ([74.125.149.240]:38324 "HELO na3sys009aog116.obsmtp.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with SMTP id S1753457AbaEWRGv (ORCPT ); Fri, 23 May 2014 13:06:51 -0400 Received: from mail-pa0-f52.google.com ([209.85.220.52]) (using TLSv1) by na3sys009aob116.postini.com ([74.125.148.12]) with SMTP ID DSNKU3+AKkmW8Oy5eGSbYFXfGJ50Qi6GdS0f@postini.com; Fri, 23 May 2014 10:06:50 PDT Received: by mail-pa0-f52.google.com with SMTP id fa1so4379264pad.39 for ; Fri, 23 May 2014 10:06:50 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20130820; h=x-gm-message-state:from:to:cc:subject:date:message-id; bh=OJemhj5xWhi1h76MkbgLnBoO+ioL7rFGlRa6p6t+TQY=; b=AYCH24qFc89xFzpjrvlwz2m88rTNKiojo/f8+bsGi4+JFK6P5EFUXoEgjQxNu6/hM8 VQkRC4npFzP2HU//TcXSE8W6N5QYHqNOdr6dvlwqodWEBlUD+STRoi/h+uLsE6rdUbf1 I46NMnU9ie7W7AeNYBEp3de1+AnaYsJQAb/YTdH7iUB12ae7fIjjIVPb3dA8t5Y2CF2X l7I+705YEYn4Bbxmw47HGh+TKphIpdGa52zMkrs966jXwvpuVy/vdo8X3OweOvoP7G5n pFbFJgjcckFROCoQAhfHFWPNn/ts0m/gBwIfenRNVH7K0YMNlCh3QwfoI1jCV3VmZfzI cKng== X-Gm-Message-State: ALoCoQm5HlzQwCi17TMY0er95vGCynzwSSHgR8WyUKMN9yhhJ1U7gtyIckGx1OduaC21BmLBH22TFSwWEKlValyG+hoMGYUXY36vs2zuM/9R1kAv79Pg6BlGHDeST3KDf0klkC7BvNnl X-Received: by 10.68.239.137 with SMTP id vs9mr7841538pbc.84.1400864810362; Fri, 23 May 2014 10:06:50 -0700 (PDT) X-Received: by 10.68.239.137 with SMTP id vs9mr7841526pbc.84.1400864810287; Fri, 23 May 2014 10:06:50 -0700 (PDT) Received: from localhost ([208.91.2.3]) by mx.google.com with ESMTPSA id si8sm16635592pab.27.2014.05.23.10.06.49 for (version=TLSv1.1 cipher=RC4-SHA bits=128/128); Fri, 23 May 2014 10:06:49 -0700 (PDT) From: Pravin B Shelar To: davem@davemloft.net Cc: dev@openvswitch.org, netdev@vger.kernel.org, Jarno Rajahalme , Pravin B Shelar Subject: [PATCH net-next 12/13] openvswitch: Minimize ovs_flow_cmd_new|set critical sections. Date: Tue, 20 May 2014 02:00:51 -0700 Message-Id: <1400576451-2522-1-git-send-email-pshelar@nicira.com> X-Mailer: git-send-email 1.7.1 Sender: netdev-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: netdev@vger.kernel.org From: Jarno Rajahalme Signed-off-by: Jarno Rajahalme Signed-off-by: Pravin B Shelar --- net/openvswitch/datapath.c | 192 +++++++++++++++++++++++++++------------------ 1 file changed, 116 insertions(+), 76 deletions(-) diff --git a/net/openvswitch/datapath.c b/net/openvswitch/datapath.c index 22665a6..6bc9d94 100644 --- a/net/openvswitch/datapath.c +++ b/net/openvswitch/datapath.c @@ -796,8 +796,7 @@ 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; + struct sw_flow *flow, *new_flow; struct sw_flow_mask mask; struct sk_buff *reply; struct datapath *dp; @@ -805,61 +804,77 @@ static int ovs_flow_cmd_new(struct sk_buff *skb, struct genl_info *info) struct sw_flow_match match; int error; - /* Extract key. */ + /* Must have key and actions. */ error = -EINVAL; if (!a[OVS_FLOW_ATTR_KEY]) goto error; + if (!a[OVS_FLOW_ATTR_ACTIONS]) + goto error; - ovs_match_init(&match, &key, &mask); + /* Most of the time we need to allocate a new flow, do it before + * locking. + */ + new_flow = ovs_flow_alloc(); + if (IS_ERR(new_flow)) { + error = PTR_ERR(new_flow); + goto error; + } + + /* Extract key. */ + ovs_match_init(&match, &new_flow->unmasked_key, &mask); error = ovs_nla_get_match(&match, a[OVS_FLOW_ATTR_KEY], a[OVS_FLOW_ATTR_MASK]); if (error) - goto error; + goto err_kfree_flow; - /* Validate actions. */ - error = -EINVAL; - if (!a[OVS_FLOW_ATTR_ACTIONS]) - goto error; + ovs_flow_mask_key(&new_flow->key, &new_flow->unmasked_key, &mask); + /* Validate actions. */ acts = ovs_nla_alloc_flow_actions(nla_len(a[OVS_FLOW_ATTR_ACTIONS])); error = PTR_ERR(acts); if (IS_ERR(acts)) - goto error; + goto err_kfree_flow; - ovs_flow_mask_key(&masked_key, &key, &mask); - error = ovs_nla_copy_actions(a[OVS_FLOW_ATTR_ACTIONS], - &masked_key, 0, &acts); + error = ovs_nla_copy_actions(a[OVS_FLOW_ATTR_ACTIONS], &new_flow->key, + 0, &acts); if (error) { OVS_NLERR("Flow actions may not be safe on all matching packets.\n"); - goto err_kfree; + goto err_kfree_acts; + } + + reply = ovs_flow_cmd_alloc_info(acts, info, false); + if (IS_ERR(reply)) { + error = PTR_ERR(reply); + goto err_kfree_acts; } ovs_lock(); dp = get_dp(sock_net(skb->sk), ovs_header->dp_ifindex); - error = -ENODEV; - if (!dp) + if (unlikely(!dp)) { + error = -ENODEV; goto err_unlock_ovs; - + } /* Check if this is a duplicate flow */ - flow = ovs_flow_tbl_lookup(&dp->table, &key); - if (!flow) { - /* Allocate flow. */ - flow = ovs_flow_alloc(); - if (IS_ERR(flow)) { - error = PTR_ERR(flow); - goto err_unlock_ovs; - } - - flow->key = masked_key; - flow->unmasked_key = key; - rcu_assign_pointer(flow->sf_acts, acts); + flow = ovs_flow_tbl_lookup(&dp->table, &new_flow->unmasked_key); + if (likely(!flow)) { + rcu_assign_pointer(new_flow->sf_acts, acts); /* Put flow in bucket. */ - error = ovs_flow_tbl_insert(&dp->table, flow, &mask); - if (error) { + error = ovs_flow_tbl_insert(&dp->table, new_flow, &mask); + if (unlikely(error)) { acts = NULL; - goto err_flow_free; + goto err_unlock_ovs; } + + if (unlikely(reply)) { + error = ovs_flow_cmd_fill_info(new_flow, + ovs_header->dp_ifindex, + reply, info->snd_portid, + info->snd_seq, 0, + OVS_FLOW_CMD_NEW); + BUG_ON(error < 0); + } + ovs_unlock(); } else { struct sw_flow_actions *old_acts; @@ -869,39 +884,45 @@ static int ovs_flow_cmd_new(struct sk_buff *skb, struct genl_info *info) * 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)) + if (unlikely(info->nlhdr->nlmsg_flags & (NLM_F_CREATE + | NLM_F_EXCL))) { + error = -EEXIST; goto err_unlock_ovs; - + } /* The unmasked key has to be the same for flow updates. */ - if (!ovs_flow_cmp_unmasked_key(flow, &match)) + if (unlikely(!ovs_flow_cmp_unmasked_key(flow, &match))) { + error = -EEXIST; 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); - } - reply = ovs_flow_cmd_build_info(flow, ovs_header->dp_ifindex, - info, OVS_FLOW_CMD_NEW, false); - ovs_unlock(); + if (unlikely(reply)) { + error = ovs_flow_cmd_fill_info(flow, + ovs_header->dp_ifindex, + reply, info->snd_portid, + info->snd_seq, 0, + OVS_FLOW_CMD_NEW); + BUG_ON(error < 0); + } + 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)); + ovs_nla_free_flow_actions(old_acts); + ovs_flow_free(new_flow, false); } + + if (reply) + ovs_notify(&dp_flow_genl_family, reply, info); return 0; -err_flow_free: - ovs_flow_free(flow, false); err_unlock_ovs: ovs_unlock(); -err_kfree: + kfree_skb(reply); +err_kfree_acts: kfree(acts); +err_kfree_flow: + ovs_flow_free(new_flow, false); error: return error; } @@ -915,7 +936,7 @@ static int ovs_flow_cmd_set(struct sk_buff *skb, struct genl_info *info) struct sw_flow_mask mask; struct sk_buff *reply = NULL; struct datapath *dp; - struct sw_flow_actions *acts = NULL; + struct sw_flow_actions *old_acts = NULL, *acts = NULL; struct sw_flow_match match; int error; @@ -942,56 +963,75 @@ static int ovs_flow_cmd_set(struct sk_buff *skb, struct genl_info *info) &masked_key, 0, &acts); if (error) { OVS_NLERR("Flow actions may not be safe on all matching packets.\n"); - goto err_kfree; + goto err_kfree_acts; + } + } + + /* Can allocate before locking if have acts. */ + if (acts) { + reply = ovs_flow_cmd_alloc_info(acts, info, false); + if (IS_ERR(reply)) { + error = PTR_ERR(reply); + goto err_kfree_acts; } } ovs_lock(); dp = get_dp(sock_net(skb->sk), ovs_header->dp_ifindex); - error = -ENODEV; - if (!dp) + if (unlikely(!dp)) { + error = -ENODEV; goto err_unlock_ovs; - + } /* Check that the flow exists. */ flow = ovs_flow_tbl_lookup(&dp->table, &key); - error = -ENOENT; - if (!flow) + if (unlikely(!flow)) { + error = -ENOENT; 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)) + if (unlikely(!ovs_flow_cmp_unmasked_key(flow, &match))) { + error = -EEXIST; goto err_unlock_ovs; - + } /* Update actions, if present. */ - if (acts) { - struct sw_flow_actions *old_acts; - + if (likely(acts)) { old_acts = ovsl_dereference(flow->sf_acts); rcu_assign_pointer(flow->sf_acts, acts); - ovs_nla_free_flow_actions(old_acts); + + if (unlikely(reply)) { + error = ovs_flow_cmd_fill_info(flow, + ovs_header->dp_ifindex, + reply, info->snd_portid, + info->snd_seq, 0, + OVS_FLOW_CMD_NEW); + BUG_ON(error < 0); + } + } else { + /* Could not alloc without acts before locking. */ + reply = ovs_flow_cmd_build_info(flow, ovs_header->dp_ifindex, + info, OVS_FLOW_CMD_NEW, false); + if (unlikely(IS_ERR(reply))) { + error = PTR_ERR(reply); + goto err_unlock_ovs; + } } - 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) { - if (!IS_ERR(reply)) - ovs_notify(&dp_flow_genl_family, reply, info); - else - genl_set_err(&dp_flow_genl_family, sock_net(skb->sk), 0, - 0, PTR_ERR(reply)); - } + if (reply) + ovs_notify(&dp_flow_genl_family, reply, info); + if (old_acts) + ovs_nla_free_flow_actions(old_acts); return 0; err_unlock_ovs: ovs_unlock(); -err_kfree: + kfree_skb(reply); +err_kfree_acts: kfree(acts); error: return error;