[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