[ovs-dev] [PATCH v3] datapath-windows: Solved BSOD when uninstalling the driver (race condition)

Gurucharan Shetty shettyg at nicira.com
Tue Apr 7 22:25:23 UTC 2015


On Tue, Apr 7, 2015 at 9:35 AM, Sorin Vinturis
<svinturis at cloudbasesolutions.com> wrote:
> The BSOD occurred because the FilterAttach routine released the switch
> context, while there were IRPs in processing.
>
> The solution was to add a reference count to prevent premature deallocation of the
> global switch context structure, gOvsSwitchContext.
>
> Signed-off-by: Sorin Vinturis <svinturis at cloudbasesolutions.com>
> Reported-by: Alin Gabriel Serdean <aserdean at cloudbasesolutions.com>
> Reported-at: https://github.com/openvswitch/ovs-issues/issues/58
> Acked-by: Eitan Eliahu <eliahue at vmware.com>
Thank you all. Applied!

> ---
>  datapath-windows/ovsext/Datapath.c | 12 ++++++--
>  datapath-windows/ovsext/Switch.c   | 56 ++++++++++++++++++++++++++++++++++++++
>  datapath-windows/ovsext/Switch.h   |  6 +++-
>  3 files changed, 71 insertions(+), 3 deletions(-)
>
> diff --git a/datapath-windows/ovsext/Datapath.c b/datapath-windows/ovsext/Datapath.c
> index 8eb13f1..b692225 100644
> --- a/datapath-windows/ovsext/Datapath.c
> +++ b/datapath-windows/ovsext/Datapath.c
> @@ -701,8 +701,13 @@ OvsDeviceControl(PDEVICE_OBJECT deviceObject,
>
>      /* Check if the extension is enabled. */
>      if (NULL == gOvsSwitchContext) {
> -        status = STATUS_DEVICE_NOT_READY;
> -        goto done;
> +        status = STATUS_NOT_FOUND;
> +        goto exit;
> +    }
> +
> +    if (!OvsAcquireSwitchContext()) {
> +        status = STATUS_NOT_FOUND;
> +        goto exit;
>      }
>
>      /* Concurrent netlink operations are not supported. */
> @@ -874,6 +879,9 @@ OvsDeviceControl(PDEVICE_OBJECT deviceObject,
>      status = InvokeNetlinkCmdHandler(&usrParamsCtx, nlFamilyOps, &replyLen);
>
>  done:
> +    OvsReleaseSwitchContext(gOvsSwitchContext);
> +
> +exit:
>      KeMemoryBarrier();
>      instance->inUse = 0;
>
> diff --git a/datapath-windows/ovsext/Switch.c b/datapath-windows/ovsext/Switch.c
> index a228d8e..94565a6 100644
> --- a/datapath-windows/ovsext/Switch.c
> +++ b/datapath-windows/ovsext/Switch.c
> @@ -42,6 +42,12 @@ extern PNDIS_SPIN_LOCK gOvsCtrlLock;
>  extern NDIS_HANDLE gOvsExtDriverHandle;
>  extern NDIS_HANDLE gOvsExtDriverObject;
>
> +/*
> + * Reference count used to prevent premature deallocation of the global switch
> + * context structure, gOvsSwitchContext.
> + */
> +volatile LONG      gOvsSwitchContextRefCount = 1;
> +
>  static NDIS_STATUS OvsCreateSwitch(NDIS_HANDLE ndisFilterHandle,
>                                     POVS_SWITCH_CONTEXT *switchContextOut);
>  static NDIS_STATUS OvsInitSwitchContext(POVS_SWITCH_CONTEXT switchContext);
> @@ -420,6 +426,7 @@ OvsInitSwitchContext(POVS_SWITCH_CONTEXT switchContext)
>      switchContext->isActivateFailed = FALSE;
>      switchContext->dpNo = OVS_DP_NUMBER;
>      ovsTimeIncrementPerTick = KeQueryTimeIncrement() / 10000;
> +
>      OVS_LOG_TRACE("Exit: Succesfully initialized switchContext: %p",
>                    switchContext);
>      return NDIS_STATUS_SUCCESS;
> @@ -428,6 +435,12 @@ OvsInitSwitchContext(POVS_SWITCH_CONTEXT switchContext)
>  static VOID
>  OvsUninitSwitchContext(POVS_SWITCH_CONTEXT switchContext)
>  {
> +    OvsReleaseSwitchContext(switchContext);
> +}
> +
> +VOID
> +OvsDeleteSwitchContext(POVS_SWITCH_CONTEXT switchContext)
> +{
>      OVS_LOG_TRACE("Enter: Delete switchContext:%p", switchContext);
>
>      /* We need to do cleanup for tunnel port here. */
> @@ -450,6 +463,49 @@ OvsUninitSwitchContext(POVS_SWITCH_CONTEXT switchContext)
>      OVS_LOG_TRACE("Exit: Delete switchContext: %p", switchContext);
>  }
>
> +VOID
> +OvsReleaseSwitchContext(POVS_SWITCH_CONTEXT switchContext)
> +{
> +    LONG ref = 0;
> +    LONG newRef = 0;
> +    LONG icxRef = 0;
> +
> +    do {
> +        ref = gOvsSwitchContextRefCount;
> +        newRef = (0 == ref) ? 0 : ref - 1;
> +        icxRef = InterlockedCompareExchange(&gOvsSwitchContextRefCount,
> +                                            newRef,
> +                                            ref);
> +    } while (icxRef != ref);
> +
> +    if (ref == 1) {
> +        OvsDeleteSwitchContext(switchContext);
> +    }
> +}
> +
> +BOOLEAN
> +OvsAcquireSwitchContext(VOID)
> +{
> +    LONG ref = 0;
> +    LONG newRef = 0;
> +    LONG icxRef = 0;
> +    BOOLEAN ret = FALSE;
> +
> +    do {
> +        ref = gOvsSwitchContextRefCount;
> +        newRef = (0 == ref) ? 0 : ref + 1;
> +        icxRef = InterlockedCompareExchange(&gOvsSwitchContextRefCount,
> +                                            newRef,
> +                                            ref);
> +    } while (icxRef != ref);
> +
> +    if (ref != 0) {
> +        ret = TRUE;
> +    }
> +
> +    return ret;
> +}
> +
>  /*
>   * --------------------------------------------------------------------------
>   *  This function activates the switch by initializing it with all the runtime
> diff --git a/datapath-windows/ovsext/Switch.h b/datapath-windows/ovsext/Switch.h
> index 7960072..6ec34e1 100644
> --- a/datapath-windows/ovsext/Switch.h
> +++ b/datapath-windows/ovsext/Switch.h
> @@ -202,7 +202,6 @@ OvsAcquireDatapathWrite(OVS_DATAPATH *datapath,
>                             dispatch ? NDIS_RWL_AT_DISPATCH_LEVEL : 0);
>  }
>
> -
>  static __inline VOID
>  OvsReleaseDatapath(OVS_DATAPATH *datapath,
>                     LOCK_STATE_EX *lockState)
> @@ -211,6 +210,11 @@ OvsReleaseDatapath(OVS_DATAPATH *datapath,
>      NdisReleaseRWLock(datapath->lock, lockState);
>  }
>
> +BOOLEAN
> +OvsAcquireSwitchContext(VOID);
> +
> +VOID
> +OvsReleaseSwitchContext(POVS_SWITCH_CONTEXT switchContext);
>
>  PVOID OvsGetExternalVport();
>
> --
> 1.9.0.msysgit.0
> _______________________________________________
> dev mailing list
> dev at openvswitch.org
> http://openvswitch.org/mailman/listinfo/dev



More information about the dev mailing list