[ovs-dev] [PATCH v2 2/2] datapath-windows: Support for IRP cancelling mechanism

Sorin Vinturis svinturis at cloudbasesolutions.com
Mon Aug 31 17:21:21 UTC 2015


Hi Nithin,

Please see my answers inline.

-----Original Message-----
From: Nithin Raju [mailto:nithin at vmware.com] 
Sent: Friday, 28 August, 2015 01:03
To: Sorin Vinturis
Cc: dev at openvswitch.org
Subject: Re: [ovs-dev] [PATCH v2 2/2] datapath-windows: Support for IRP cancelling mechanism

hi Sorin,
Looks good overall.

I just one one comment about the patch which I’ve inlined.

How did you test this BTW?
SV: I've tested using instrumentation code.

> VOID
> @@ -1309,7 +1405,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
> @@ -1321,7 +1417,7 @@ OvsTunnelFilterQueueRequest(PIRP irp,
>                             PVOID tunnelContext) {
>     POVS_TUNFLT_REQUEST request = NULL;
> -    NTSTATUS            status = STATUS_PENDING;
> +    NTSTATUS            status = STATUS_SUCCESS;
>     BOOLEAN             error = TRUE;
>     UINT64              timeout = 0;
> 
> @@ -1334,8 +1430,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;
>         }
> 
> @@ -1366,7 +1462,10 @@ OvsTunnelFilterQueueRequest(PIRP irp,
>         request->callback = callback;
>         request->context = tunnelContext;
> 
> -        OvsTunnelFilterThreadPush(request);
> +        status = OvsTunnelFilterThreadPush(request);
> +        if (!NT_SUCCESS(status)) {
> +            break;
> +        }
> 
>         error = FALSE;
>     } while (error);
> @@ -1379,13 +1478,14 @@ OvsTunnelFilterQueueRequest(PIRP irp,
>         }
>     }
> 
> -    return status;
> +    /* Return pending to indicate that the IRP will be completed later. */
> +    return STATUS_PENDING;

You are returning STATUS_PENDING at the end of the function. You should be returning ‘status’.
SV: Correct. Good catch. 



More information about the dev mailing list