Hi Nithin, Please see my answers inline.
-----Original Message----- From: Nithin Raju [mailto:nit...@vmware.com] Sent: Friday, 28 August, 2015 01:03 To: Sorin Vinturis Cc: dev@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. _______________________________________________ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
