> On Jul 20, 2015, at 12:00 PM, Sorin Vinturis 
> <svintu...@cloudbasesolutions.com> wrote:
> 
> In order to support IRP cancelling mechanism for pending IRPs, all
> tunnel filter requests, VXLAN create/delete tunnel, need to be
> processed iteratively.
> 
> Signed-off-by: Sorin Vinturis <svintu...@cloudbasesolutions.com>

hi Sorin,
I had a question regarding latency of processing the requests. Looks good 
otherwise. A v4 should be good to go.

> -    do
> -    {
> +    do {
>         if (!InterlockedCompareExchange(
>             (LONG volatile *)&threadCtx->listRequests.numEntries, 0, 0)) {
>             OVS_LOG_INFO("Nothing to do... request list is empty.");
> @@ -1076,38 +1056,25 @@ 
> OvsTunnelFilterRequestListProcess(POVS_TUNFLT_THREAD_CONTEXT threadCtx)
>         }
>         inTransaction = TRUE;
> 
> -        InitializeListHead(&head);
> -        OvsTunnelFilterRequestPopList(&threadCtx->listRequests, &head, 
> &count);
> -
> -        LIST_FORALL_SAFE(&head, link, next) {
> -            request = CONTAINING_RECORD(link, OVS_TUNFLT_REQUEST, entry);
> +        while (NULL != (request = OvsTunnelFilterRequestPop(
> +            &threadCtx->listRequests))) {

minor: indentation can be improved to:
       while (NULL !=
              (request = OvsTunnelFilterRequestPop(&threadCtx->listRequests))) {

> 
>             status = OvsTunnelFilterExecuteAction(threadCtx->engineSession,
>                                                   request);
> +
> +            /* Complete the IRP with the last operation status. */
> +            OvsTunnelFilterCompleteRequest(request, status);
> +
> +            OvsFreeMemory(request);
> +            request = NULL;
> +
>             if (!NT_SUCCESS(status)) {
> -                RemoveEntryList(&request->entry);
> -                count--;
> -
> -                /* Complete the IRP with the failure status. */
> -                OvsTunnelFilterCompleteRequest(request->irp,
> -                                               request->callback,
> -                                               request->context,
> -                                               status);
> -                OvsFreeMemory(request);
> -                request = NULL;
> -            } else {
> -                error = FALSE;
> +                break;
>             }
>         }
> 

What happens if there are 10 requests to process, and the 5th one fails. You’d 
bail out of processing commit the 4 and go back into calling 
KeWaitForMultipleObjects() in OvsTunnelFilterThreadProc(). What is the trigger 
to processing the remaining 5 request, esp. if they are legitimate requests and 
there’s a good chance that they’ll succeed.

Also, if you are worried about the number of requests being committed per 
transaction, maybe you should submit them in batches.

Does KeSetEvent() queue up wakeup events?

-- Nithin
_______________________________________________
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev

Reply via email to