From patchwork Wed Aug 24 02:18:50 2016 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Joe Stringer X-Patchwork-Id: 662109 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Received: from archives.nicira.com (archives.nicira.com [96.126.127.54]) by ozlabs.org (Postfix) with ESMTP id 3sJrbK4053z9sdm for ; Wed, 24 Aug 2016 12:19:09 +1000 (AEST) Received: from archives.nicira.com (localhost [127.0.0.1]) by archives.nicira.com (Postfix) with ESMTP id C053E10671; Tue, 23 Aug 2016 19:19:08 -0700 (PDT) X-Original-To: dev@openvswitch.org Delivered-To: dev@openvswitch.org Received: from mx3v3.cudamail.com (mx3.cudamail.com [64.34.241.5]) by archives.nicira.com (Postfix) with ESMTPS id 5730510661 for ; Tue, 23 Aug 2016 19:19:07 -0700 (PDT) Received: from bar6.cudamail.com (localhost [127.0.0.1]) by mx3v3.cudamail.com (Postfix) with ESMTPS id E2E431613BC for ; Tue, 23 Aug 2016 20:19:06 -0600 (MDT) X-ASG-Debug-ID: 1472005145-0b32373e3a01a50001-byXFYA Received: from mx1-pf2.cudamail.com ([192.168.24.2]) by bar6.cudamail.com with ESMTP id e8UgAQWLJKEO5ut4 (version=TLSv1 cipher=DHE-RSA-AES256-SHA bits=256 verify=NO) for ; Tue, 23 Aug 2016 20:19:06 -0600 (MDT) X-Barracuda-Envelope-From: joe@ovn.org X-Barracuda-RBL-Trusted-Forwarder: 192.168.24.2 Received: from unknown (HELO relay2-d.mail.gandi.net) (217.70.183.194) by mx1-pf2.cudamail.com with ESMTPS (DHE-RSA-AES256-SHA encrypted); 24 Aug 2016 02:19:05 -0000 Received-SPF: pass (mx1-pf2.cudamail.com: SPF record at ovn.org designates 217.70.183.194 as permitted sender) X-Barracuda-Apparent-Source-IP: 217.70.183.194 X-Barracuda-RBL-IP: 217.70.183.194 Received: from mfilter22-d.gandi.net (mfilter22-d.gandi.net [217.70.178.150]) by relay2-d.mail.gandi.net (Postfix) with ESMTP id E2670C5A4E; Wed, 24 Aug 2016 04:19:03 +0200 (CEST) X-Virus-Scanned: Debian amavisd-new at mfilter22-d.gandi.net Received: from relay2-d.mail.gandi.net ([IPv6:::ffff:217.70.183.194]) by mfilter22-d.gandi.net (mfilter22-d.gandi.net [::ffff:10.0.15.180]) (amavisd-new, port 10024) with ESMTP id iTRdmYCRXLaw; Wed, 24 Aug 2016 04:19:02 +0200 (CEST) X-Originating-IP: 208.91.1.34 Received: from archer.eng.vmware.com (unknown [208.91.1.34]) (Authenticated sender: joe@ovn.org) by relay2-d.mail.gandi.net (Postfix) with ESMTPSA id 5D917C5A46; Wed, 24 Aug 2016 04:19:01 +0200 (CEST) X-CudaMail-Envelope-Sender: joe@ovn.org From: Joe Stringer To: dev@openvswitch.org X-CudaMail-Whitelist-To: dev@openvswitch.org X-CudaMail-MID: CM-E2-822091700 X-CudaMail-DTE: 082316 X-CudaMail-Originating-IP: 217.70.183.194 Date: Tue, 23 Aug 2016 19:18:50 -0700 X-ASG-Orig-Subj: [##CM-E2-822091700##][PATCH] ofproto-dpif-upcall: Replace ukeys for deleted flows. Message-Id: <20160824021850.11359-1-joe@ovn.org> X-Mailer: git-send-email 2.9.3 X-Barracuda-Connect: UNKNOWN[192.168.24.2] X-Barracuda-Start-Time: 1472005146 X-Barracuda-Encrypted: DHE-RSA-AES256-SHA X-Barracuda-URL: https://web.cudamail.com:443/cgi-mod/mark.cgi X-ASG-Whitelist: Header =?UTF-8?B?eFwtY3VkYW1haWxcLXdoaXRlbGlzdFwtdG8=?= X-Virus-Scanned: by bsmtpd at cudamail.com X-Barracuda-BRTS-Status: 1 Subject: [ovs-dev] [PATCH] ofproto-dpif-upcall: Replace ukeys for deleted flows. X-BeenThere: dev@openvswitch.org X-Mailman-Version: 2.1.16 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , MIME-Version: 1.0 Errors-To: dev-bounces@openvswitch.org Sender: "dev" If a revalidator dumps/revalidates a flow during the 'dump' phase, resulting in the deletion of the flow, then ukey->flow_exists is set to false, and the ukey is kept around until the 'sweep' phase. The ukey is kept around to ensure that cases like duplicated dumps from the datapaths do not result in multiple attribution of the same stats. However, if an upcall for this flow comes for a handler between the revalidator 'dump' and 'sweep' phases, the handler will lookup the ukey and find that the ukey exists, then skip installing a new flow entirely. As a result, for this period all traffic for the flow is slowpathed. If there is a lot of traffic hitting this flow, then it will all be handled in userspace until the 'sweep' phase. Eventually the revalidators will reach the sweep phase and delete the ukey, and subsequently the handlers should install a new flow. To reduce the slowpathing of this traffic during flow table transitions, allow the handler to identify this case during miss upcall handling and replace the existing ukey with a new ukey. The handler will then be able to install a flow for this traffic, allowing the traffic flow to return to the fastpath. There are three situations where the existing ukey has "flow_exists" set to false: * Firstly, the flow for the ukey might not yet have been installed. In this case, the other handler that set up this ukey will be holding the lock for the ukey. So, if we attempt to grab the lock and fail then another handler is already setting up the flow and we can safely skip flow install. * Secondly, a revalidator could be currently deleting the flow. In this case, the revalidator holds the ukey lock so the handler will fail to grab it. This is fine, if traffic continues to miss then a subsequent miss upcall will hit the third case. * Thirdly, the flow may have been recently deleted by a revalidator thread. In this case, we can grab the lock. From the handler thread we swap the original key out for a new one and rcu-defer its deletion. Signed-off-by: Joe Stringer --- ofproto/ofproto-dpif-upcall.c | 29 +++++++++++++++++++++++++++++ 1 file changed, 29 insertions(+) diff --git a/ofproto/ofproto-dpif-upcall.c b/ofproto/ofproto-dpif-upcall.c index 042a50a9f179..5a0ecc2a6fe6 100644 --- a/ofproto/ofproto-dpif-upcall.c +++ b/ofproto/ofproto-dpif-upcall.c @@ -50,6 +50,7 @@ COVERAGE_DEFINE(dumped_duplicate_flow); COVERAGE_DEFINE(dumped_new_flow); COVERAGE_DEFINE(handler_duplicate_upcall); COVERAGE_DEFINE(upcall_ukey_contention); +COVERAGE_DEFINE(upcall_ukey_replace); COVERAGE_DEFINE(revalidate_missed_dp_flow); /* A thread that reads upcalls from dpif, forwards each upcall's packet, @@ -1569,6 +1570,33 @@ ukey_create_from_dpif_flow(const struct udpif *udpif, return 0; } +static bool +try_ukey_replace(struct umap *umap, struct udpif_key *old_ukey, + struct udpif_key *new_ukey) + OVS_REQUIRES(umap->mutex) + OVS_TRY_LOCK(true, new_ukey->mutex) +{ + bool replaced = false; + + if (!ovs_mutex_trylock(&old_ukey->mutex)) { + if (!old_ukey->flow_exists) { + /* The flow was deleted during the current revalidator dump, + * but its ukey won't be cleaned up until the sweep phase. + * In the mean time, we are receiving upcalls for this traffic. + * Expedite the flow install by replacing the ukey. */ + COVERAGE_INC(upcall_ukey_replace); + ovs_mutex_lock(&new_ukey->mutex); + cmap_replace(&umap->cmap, &old_ukey->cmap_node, + &new_ukey->cmap_node, new_ukey->hash); + ovsrcu_postpone(ukey_delete__, old_ukey); + replaced = true; + } + ovs_mutex_unlock(&old_ukey->mutex); + } + + return replaced; +} + /* Attempts to insert a ukey into the shared ukey maps. * * On success, returns true, installs the ukey and returns it in a locked @@ -1591,6 +1619,7 @@ ukey_install_start(struct udpif *udpif, struct udpif_key *new_ukey) if (old_ukey->key_len == new_ukey->key_len && !memcmp(old_ukey->key, new_ukey->key, new_ukey->key_len)) { COVERAGE_INC(handler_duplicate_upcall); + locked = try_ukey_replace(umap, old_ukey, new_ukey); } else { struct ds ds = DS_EMPTY_INITIALIZER;