[ofw] [PATCH] winverbs: convert connect/accept to sync calls

[Fab Tillier]

Oh, that's too bad.  Any idea what that does to connection rate?

-Fab

> -----Original Message-----
> From: ofw-bounces at lists.openfabrics.org [mailto:ofw-
> bounces at lists.openfabrics.org] On Behalf Of Sean Hefty
> Sent: Wednesday, August 26, 2009 4:59 PM
> To: ofw at lists.openfabrics.org
> Subject: [ofw] [PATCH] winverbs: convert connect/accept to sync calls
>
> If an application calls Connect or Accept, their IRP is queued to a
> work queue for asynchronous processing.  However, if the application
> crashes or exits before the work queue can process the IRP, the cleanup
> code will call WvEpFree().  This destroys the IbCmId.
>
> When the work queue finally runs, it can access a freed IbCmId.
> This is bad.  To be safe, the IRP needs to be completed before
> WvEpFree() returns to ensure that all outstanding IRPs are completed
> and avoid accessing freed memory.
>
> One fix for this is to call 'WorkQueueFlush()' from WvEpFree() to
> ensure that the EP's work entry is not in use and all previous
> IRPs have completed before the pIbCmId is destroyed.
> However, since WorkQueueFlush() does not exist, and any implementation
> of it carries a higher risk, revert Connect/Accept to synchronous
> operation for the WinOF 2.1 release.
>
> Signed-off-by: Sean Hefty <sean.hefty at intel.com>
> ---
> This is a quick fix for winof 2.1.  Any fix that tries to support
> asynchronous connect/accept calls will have a much higher risk, so
> is deferred to after the 2.1 release.
>
> Index: core/winverbs/kernel/wv_ep.c
> ===================================================================
> --- core/winverbs/kernel/wv_ep.c        (revision 2373)
> +++ core/winverbs/kernel/wv_ep.c        (working copy)
> @@ -181,6 +181,9 @@
>
>  void WvEpFree(WV_ENDPOINT *pEndpoint)
>  {
> +       // see comment in WvEpProcessAsync()
> +       // WorkQueueFlush(&pEndpoint->pProvider->WorkQueue);
> +
>         if (pEndpoint->pIbCmId != NULL) {
>                 IbCmInterface.CM.destroy_id(pEndpoint->pIbCmId);
>         }
> @@ -644,7 +647,18 @@
>
>         work = WorkEntryFromIrp(WdfRequestWdmGetIrp(Request));
>         WorkEntryInit(work, AsyncHandler, Request);
> -       WorkQueueInsert(&pProvider->WorkQueue, work);
> +       // TODO: If app exits or crashes immediately after making a CM
> call,
> +       // then WvEpFree() will be called, which destroys
> +       // the pIbCmId.  Since the IRP is not on a queue, the system
> cannot
> +       // cancel the operation.  If the work queue thread is running,
> this can
> +       // result in it accessing the pIbCmId after it is destroyed.
> +       // The fix for this is to call 'WorkQueueFlush()' from
> WvEpFree() to
> +       // ensure that the EP's work entry is not in use and all
> previous
> +       // IRPs have completed before the pIbCmId is destroyed.
> +       // Until WorkQueueFlush is available, continue to process
> Connect
> +       // and Accept calls synchronously.
> +       //WorkQueueInsert(&pProvider->WorkQueue, work);
> +       AsyncHandler(work);
>  }
>
>  void WvEpConnect(WV_PROVIDER *pProvider, WDFREQUEST Request)