From patchwork Mon Aug 31 17:46:37 2015 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Sorin Vinturis X-Patchwork-Id: 512562 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Received: from archives.nicira.com (li376-54.members.linode.com [96.126.127.54]) by ozlabs.org (Postfix) with ESMTP id B1785140082 for ; Tue, 1 Sep 2015 03:47:40 +1000 (AEST) Received: from archives.nicira.com (localhost [127.0.0.1]) by archives.nicira.com (Postfix) with ESMTP id BD4FF10908; Mon, 31 Aug 2015 10:47:39 -0700 (PDT) X-Original-To: dev@openvswitch.org Delivered-To: dev@openvswitch.org Received: from mx1e3.cudamail.com (mx1.cudamail.com [69.90.118.67]) by archives.nicira.com (Postfix) with ESMTPS id 07B63108EB for ; Mon, 31 Aug 2015 10:47:38 -0700 (PDT) Received: from bar2.cudamail.com (localhost [127.0.0.1]) by mx1e3.cudamail.com (Postfix) with ESMTPS id 7259F420557 for ; Mon, 31 Aug 2015 11:47:37 -0600 (MDT) X-ASG-Debug-ID: 1441043256-03dc535c69259990001-byXFYA Received: from mx1-pf1.cudamail.com ([192.168.24.1]) by bar2.cudamail.com with ESMTP id UuHkCLMyNLi0y7gT (version=TLSv1 cipher=DHE-RSA-AES256-SHA bits=256 verify=NO) for ; Mon, 31 Aug 2015 11:47:36 -0600 (MDT) X-Barracuda-Envelope-From: svinturis@cloudbasesolutions.com X-Barracuda-RBL-Trusted-Forwarder: 192.168.24.1 Received: from unknown (HELO cbssmtp1.cloudbase.local) (91.232.152.5) by mx1-pf1.cudamail.com with SMTP; 31 Aug 2015 17:47:35 -0000 Received-SPF: pass (mx1-pf1.cudamail.com: SPF record at cloudbasesolutions.com designates 91.232.152.5 as permitted sender) X-Barracuda-Apparent-Source-IP: 91.232.152.5 X-Barracuda-RBL-IP: 91.232.152.5 Received: from localhost (localhost [127.0.0.1]) by cbssmtp1.cloudbase.local (Postfix) with ESMTP id 2887D409CA for ; Mon, 31 Aug 2015 20:47:34 +0300 (EEST) X-Virus-Scanned: amavisd-new at cloudbasesolutions.com Received: from cbssmtp1.cloudbase.local ([127.0.0.1]) by localhost (cbssmtp1.cloudbase.local [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id 6gXIO2auLy15 for ; Mon, 31 Aug 2015 20:47:13 +0300 (EEST) Received: from CBSEX1.cloudbase.local (unknown [10.77.78.3]) by cbssmtp1.cloudbase.local (Postfix) with ESMTP id 01D13409CB for ; Mon, 31 Aug 2015 20:46:38 +0300 (EEST) Received: from CBSEX1.cloudbase.local ([10.77.78.3]) by CBSEX1.cloudbase.local ([10.77.78.3]) with mapi id 14.03.0224.002; Mon, 31 Aug 2015 19:46:37 +0200 X-CudaMail-Envelope-Sender: svinturis@cloudbasesolutions.com From: Sorin Vinturis To: "dev@openvswitch.org" X-CudaMail-MID: CM-E1-830051511 X-CudaMail-DTE: 083115 X-CudaMail-Originating-IP: 91.232.152.5 Thread-Topic: [PATCH v3 2/2] datapath-windows: Support for IRP cancelling mechanism X-ASG-Orig-Subj: [##CM-E1-830051511##][PATCH v3 2/2] datapath-windows: Support for IRP cancelling mechanism Thread-Index: AQHQ5BT7IHzwm8e6GkuXyKW0JmYHDQ== Date: Mon, 31 Aug 2015 17:46:37 +0000 Message-ID: <1441043184-3354-3-git-send-email-svinturis@cloudbasesolutions.com> References: <1441043184-3354-1-git-send-email-svinturis@cloudbasesolutions.com> In-Reply-To: <1441043184-3354-1-git-send-email-svinturis@cloudbasesolutions.com> Accept-Language: en-US, it-IT Content-Language: en-US X-MS-Has-Attach: X-MS-TNEF-Correlator: x-originating-ip: [10.77.78.1] MIME-Version: 1.0 X-GBUdb-Analysis: 0, 91.232.152.5, Ugly c=0.214287 p=-0.5 Source Normal X-MessageSniffer-Rules: 0-0-0-25188-c X-Barracuda-Connect: UNKNOWN[192.168.24.1] X-Barracuda-Start-Time: 1441043256 X-Barracuda-Encrypted: DHE-RSA-AES256-SHA X-Barracuda-URL: https://web.cudamail.com:443/cgi-mod/mark.cgi X-Virus-Scanned: by bsmtpd at cudamail.com X-Barracuda-BRTS-Status: 1 X-Barracuda-Spam-Score: 0.10 X-Barracuda-Spam-Status: No, SCORE=0.10 using per-user scores of TAG_LEVEL=3.0 QUARANTINE_LEVEL=1000.0 KILL_LEVEL=3.0 tests=RDNS_NONE X-Barracuda-Spam-Report: Code version 3.2, rules version 3.2.3.22093 Rule breakdown below pts rule name description ---- ---------------------- -------------------------------------------------- 0.10 RDNS_NONE Delivered to trusted network by a host with no rDNS Subject: [ovs-dev] [PATCH v3 2/2] datapath-windows: Support for IRP cancelling mechanism 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: , Errors-To: dev-bounces@openvswitch.org Sender: "dev" Under certain circumstances, we might need to cancel a pending IRP that has been submitted and not yet responded. This might occur when the request takes too long to complete or when the process which initiated the request terminated, leaving the request outstanding. This patch provides this missing piece by adding support for IRP cancelling mechanism. Signed-off-by: Sorin Vinturis Reported-by: Sorin Vinturis Reported-at: https://github.com/openvswitch/ovs-issues/issues/95 v2: Before pending an IRP, check if it has been cancelled. Acked-by: Nithin Raju --- datapath-windows/ovsext/Datapath.c | 4 - datapath-windows/ovsext/TunnelFilter.c | 198 ++++++++++++++++++++++++++++++--- 2 files changed, 180 insertions(+), 22 deletions(-) diff --git a/datapath-windows/ovsext/Datapath.c b/datapath-windows/ovsext/Datapath.c index 8c72533..b7bbf80 100644 --- a/datapath-windows/ovsext/Datapath.c +++ b/datapath-windows/ovsext/Datapath.c @@ -918,10 +918,6 @@ done: exit: /* Should not complete a pending IRP unless proceesing is completed. */ if (status == STATUS_PENDING) { - /* STATUS_PENDING is returned by the NL handler when the request is - * to be processed later, so we mark the IRP as pending and complete - * it in another thread when the request is processed. */ - IoMarkIrpPending(irp); return status; } return OvsCompleteIrpRequest(irp, (ULONG_PTR)replyLen, status); diff --git a/datapath-windows/ovsext/TunnelFilter.c b/datapath-windows/ovsext/TunnelFilter.c index 6cf9a94..ec14c64 100644 --- a/datapath-windows/ovsext/TunnelFilter.c +++ b/datapath-windows/ovsext/TunnelFilter.c @@ -168,6 +168,10 @@ static VOID OvsTunnelFilterThreadStop(POVS_TUNFLT_THREAD_CONTEXT threadCtx, BOOLEAN signalEvent); static NTSTATUS OvsTunnelFilterThreadInit(POVS_TUNFLT_THREAD_CONTEXT threadCtx); static VOID OvsTunnelFilterThreadUninit(POVS_TUNFLT_THREAD_CONTEXT threadCtx); +static VOID OvsTunnelFilterSetIrpContext(POVS_TUNFLT_REQUEST_LIST listRequests, + POVS_TUNFLT_REQUEST request); +static VOID OvsTunnelFilterCancelIrp(PDEVICE_OBJECT DeviceObject, + PIRP Irp); /* * Callout driver global variables @@ -955,20 +959,49 @@ OvsTunnelFilterExecuteAction(HANDLE engineSession, /* * -------------------------------------------------------------------------- - * This function pops the head item from the requests list while holding - * the list's spinlock. + * This function pops the head request from the queue while holding the + * queue lock. If the request has already been cancelled or is about to be + * cancelled, the function retrieves the next valid request. + * + * Returns a pointer to the OVS_TUNFLT_REQUEST_LIST request object retrieved + * from the queue. * -------------------------------------------------------------------------- */ POVS_TUNFLT_REQUEST OvsTunnelFilterRequestPop(POVS_TUNFLT_REQUEST_LIST listRequests) { POVS_TUNFLT_REQUEST request = NULL; + PLIST_ENTRY link, next, head; NdisAcquireSpinLock(&listRequests->spinlock); if (!IsListEmpty(&listRequests->head)) { - request = (POVS_TUNFLT_REQUEST)RemoveHeadList(&listRequests->head); - listRequests->numEntries--; + head = &listRequests->head; + LIST_FORALL_SAFE(head, link, next) { + PDRIVER_CANCEL oldCancelRoutine; + + request = CONTAINING_RECORD(link, OVS_TUNFLT_REQUEST, entry); + if (request->irp) { + oldCancelRoutine = IoSetCancelRoutine(request->irp, NULL); + if (oldCancelRoutine == NULL) { + /* + * The Cancel routine for the current IRP is running. The + * request is to be completed by the Cancel routine. Leave + * this request alone and go to the next one. + */ + continue; + } else { + /* + * The Cancel routine cannot run now and cannot already have + * started to run. This request can be processed. + */ + } + } + + RemoveEntryList(&request->entry); + listRequests->numEntries--; + break; + } } NdisReleaseSpinLock(&listRequests->spinlock); @@ -978,20 +1011,78 @@ OvsTunnelFilterRequestPop(POVS_TUNFLT_REQUEST_LIST listRequests) /* * -------------------------------------------------------------------------- - * This function pushes the received request to the list while holding the - * request list spinlock. + * This function pushes the received request to the queue, marks the IRP as + * pending and sets its Cancel routine, while holding the queue lock. + * + * Returns STATUS_CANCELLED if the IRP has already been cancelled. Otherwise, + * STATUS_SUCCESS is returned. * -------------------------------------------------------------------------- */ -VOID +NTSTATUS OvsTunnelFilterRequestPush(POVS_TUNFLT_REQUEST_LIST listRequests, POVS_TUNFLT_REQUEST request) { + NTSTATUS status = STATUS_SUCCESS; + PIRP irp = request->irp; + PDRIVER_CANCEL oldCancelRoutine; + BOOLEAN cancelled = FALSE; + NdisAcquireSpinLock(&listRequests->spinlock); - InsertTailList(&listRequests->head, &(request->entry)); - listRequests->numEntries++; + if (irp) { + /* + * Mark the IRP pending to indicate that the request may complete on + * a different thread. + */ + IoMarkIrpPending(irp); + + /* + * Set the Cancel routine for the pending IRP, before checking the + * Cancel flag. + */ + oldCancelRoutine = IoSetCancelRoutine(irp, OvsTunnelFilterCancelIrp); + ASSERT(oldCancelRoutine == NULL); + + if (irp->Cancel) { + /* + * The IRP has already been cancelled. + * Determine wheather the Cancel routine has started to run. + */ + oldCancelRoutine = IoSetCancelRoutine(irp, NULL); + if (oldCancelRoutine) { + /* + * The I/O Manager has not called the Cancel routine and it + * won't be called anymore, because we just set it to NULL. + * Return STATUS_CANCELLED and complete the request after + * releasing the lock. + */ + status = STATUS_CANCELLED; + cancelled = TRUE; + } else { + /* + * The Cancel routine has already started to run, but it is + * blocked while it waits for the queue lock. Release the lock + * and return STATUS_SUCCESS to avoid completing the request. + * It will be completed in the Cancel routine. + */ + } + } else { + /* + * The IRP has not been cancelled, so set its context used in the + * Cancel routine. + */ + OvsTunnelFilterSetIrpContext(listRequests, request); + } + } + + if (!cancelled) { + InsertTailList(&listRequests->head, &(request->entry)); + listRequests->numEntries++; + } NdisReleaseSpinLock(&listRequests->spinlock); + + return status; } /* @@ -1004,20 +1095,25 @@ OvsTunnelFilterRequestPush(POVS_TUNFLT_REQUEST_LIST listRequests, * calculated based on the received destination port. * -------------------------------------------------------------------------- */ -VOID +NTSTATUS OvsTunnelFilterThreadPush(POVS_TUNFLT_REQUEST request) { + NTSTATUS status = STATUS_SUCCESS; UINT32 threadIndex; threadIndex = request->port % OVS_TUNFLT_MAX_THREADS; - OvsTunnelFilterRequestPush( + status = OvsTunnelFilterRequestPush( &gTunnelThreadCtx[threadIndex].listRequests, request); - KeSetEvent(&gTunnelThreadCtx[threadIndex].requestEvent, - IO_NO_INCREMENT, - FALSE); + if (NT_SUCCESS(status)) { + KeSetEvent(&gTunnelThreadCtx[threadIndex].requestEvent, + IO_NO_INCREMENT, + FALSE); + } + + return status; } VOID @@ -1313,7 +1409,7 @@ OvsTunnelFilterThreadUninit(POVS_TUNFLT_THREAD_CONTEXT threadCtx) * -------------------------------------------------------------------------- * This function creates a new tunnel filter request and push it to a thread * queue. If the thread stop event is signaled, the request is completed with - * STATUS_CANCELLED without pushing it to any queue. + * STATUS_REQUEST_ABORTED without pushing it to any queue. * -------------------------------------------------------------------------- */ NTSTATUS @@ -1326,6 +1422,7 @@ OvsTunnelFilterQueueRequest(PIRP irp, { POVS_TUNFLT_REQUEST request = NULL; NTSTATUS status = STATUS_PENDING; + NTSTATUS result = STATUS_SUCCESS; BOOLEAN error = TRUE; UINT64 timeout = 0; @@ -1338,8 +1435,8 @@ OvsTunnelFilterQueueRequest(PIRP irp, FALSE, (LARGE_INTEGER *)&timeout)) { /* The stop event is signaled. Completed the IRP with - * STATUS_CANCELLED. */ - status = STATUS_CANCELLED; + * STATUS_REQUEST_ABORTED. */ + status = STATUS_REQUEST_ABORTED; break; } @@ -1370,7 +1467,11 @@ OvsTunnelFilterQueueRequest(PIRP irp, request->callback = callback; request->context = tunnelContext; - OvsTunnelFilterThreadPush(request); + result = OvsTunnelFilterThreadPush(request); + if (!NT_SUCCESS(result)) { + status = result; + break; + } error = FALSE; } while (error); @@ -1466,3 +1567,64 @@ OvsTunnelFilterDelete(PIRP irp, callback, tunnelContext); } + +/* + * -------------------------------------------------------------------------- + * This function sets the context for the IRP. The context is used by the + * Cancel routine, in order to identify the request object, corresponding to + * the IRP, to be completed and to have access to the queue lock to remove + * the request link from the queue. + * -------------------------------------------------------------------------- + */ +VOID +OvsTunnelFilterSetIrpContext(POVS_TUNFLT_REQUEST_LIST listRequests, + POVS_TUNFLT_REQUEST request) +{ + PIRP irp = request->irp; + + if (irp) { + /* Set the IRP's DriverContext to be used for later. */ + irp->Tail.Overlay.DriverContext[0] = (PVOID)request; + irp->Tail.Overlay.DriverContext[1] = (PVOID)listRequests; + } +} + +/* + * -------------------------------------------------------------------------- + * This function is the Cancel routine to be called by the I/O Manager in the + * case when the IRP is cancelled. + * -------------------------------------------------------------------------- + */ +VOID +OvsTunnelFilterCancelIrp(PDEVICE_OBJECT DeviceObject, + PIRP irp) +{ + POVS_TUNFLT_REQUEST request = + (POVS_TUNFLT_REQUEST)irp->Tail.Overlay.DriverContext[0]; + POVS_TUNFLT_REQUEST_LIST listRequests = + (POVS_TUNFLT_REQUEST_LIST)irp->Tail.Overlay.DriverContext[1]; + + DBG_UNREFERENCED_PARAMETER(DeviceObject); + + /* Release the global cancel spinlock. */ + IoReleaseCancelSpinLock(irp->CancelIrql); + + /* Clear the cancel routine from the IRP. */ + IoSetCancelRoutine(irp, NULL); + + NdisAcquireSpinLock(&listRequests->spinlock); + + /* Remove the request from the corresponding tunnel filter thread queue. */ + RemoveEntryList(&request->entry); + listRequests->numEntries--; + + NdisReleaseSpinLock(&listRequests->spinlock); + + /* We are done with this IRP, so complete it with STATUS_CANCELLED. */ + OvsTunnelFilterCompleteRequest(request->irp, + request->callback, + request->context, + STATUS_CANCELLED); + + OvsFreeMemory(request); +}