On Tue, Apr 7, 2015 at 9:35 AM, Sorin Vinturis
<[email protected]> 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 <[email protected]>
> Reported-by: Alin Gabriel Serdean <[email protected]>
> Reported-at: https://github.com/openvswitch/ovs-issues/issues/58
> Acked-by: Eitan Eliahu <[email protected]>
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
> [email protected]
> http://openvswitch.org/mailman/listinfo/dev
_______________________________________________
dev mailing list
[email protected]
http://openvswitch.org/mailman/listinfo/dev
