Hi Sorin,
Can you please explain how the case where FilterDetach is called on a
different core concurrently with the Release and between switchContext is
checked for NULL and the swicthContext->refCount is referenced ? (where it is
marked ---->>)
It seems that FilterDetach would set gOvsSwitchContext to NULL even if there
are outstanding IRPs and the switch context has not been released as of yet.
+ do {
+ if (NULL == switchContext) {
+ break;
+ }
---->>
+ if (0 == InterlockedCompareExchange(
+ (LONG volatile *)&switchContext->refCount, 0, 0)) {
+ break;
+ }
Can you inline the acquisition and release functions?
Thanks!
Eitan
-----Original Message-----
From: dev [mailto:[email protected]] On Behalf Of Sorin Vinturis
Sent: Friday, March 20, 2015 10:28 AM
To: [email protected]
Subject: [ovs-dev] [PATCH] datapath-windows: Solved BSOD when uninstalling the
driver (race condition)
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 in the switch context to prevent
premature deallocation of the switch context structure.
Signed-off-by: Sorin Vinturis <[email protected]>
Reported-by: Alin Gabriel Serdean <[email protected]>
Reported-at:
https://urldefense.proofpoint.com/v2/url?u=https-3A__github.com_openvswitch_ovs-2Dissues_issues_58&d=AwIGaQ&c=Sqcl0Ez6M0X8aeM67LKIiDJAXVeAw-YihVMNtXt-uEs&r=CWsgHUxi6ExLXY798tmo3LJ4e3geGYp56lkcH-5cLCY&m=bKJV7k8Y9bG2Z1Hczw1uCkihZsQUivQ4nMAjrAWy20I&s=JgFv3Pq9CjJRPHRfSRgXB4ibNGyMlSl8a6kKOfNQ2_k&e=
---
datapath-windows/ovsext/Datapath.c | 10 +++++----
datapath-windows/ovsext/Switch.c | 44 ++++++++++++++++++++++++++++++++++++++
datapath-windows/ovsext/Switch.h | 12 ++++++++++-
3 files changed, 61 insertions(+), 5 deletions(-)
diff --git a/datapath-windows/ovsext/Datapath.c
b/datapath-windows/ovsext/Datapath.c
index 8eb13f1..8b561a3 100644
--- a/datapath-windows/ovsext/Datapath.c
+++ b/datapath-windows/ovsext/Datapath.c
@@ -699,10 +699,9 @@ OvsDeviceControl(PDEVICE_OBJECT deviceObject,
outputBufferLen = irpSp->Parameters.DeviceIoControl.OutputBufferLength;
inputBuffer = irp->AssociatedIrp.SystemBuffer;
- /* Check if the extension is enabled. */
- if (NULL == gOvsSwitchContext) {
- status = STATUS_DEVICE_NOT_READY;
- goto done;
+ if (!OvsAcquireSwitchContext(gOvsSwitchContext)) {
+ status = STATUS_NOT_FOUND;
+ goto exit;
}
/* Concurrent netlink operations are not supported. */ @@ -874,6 +873,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..8366944 100644
--- a/datapath-windows/ovsext/Switch.c
+++ b/datapath-windows/ovsext/Switch.c
@@ -420,6 +420,9 @@ OvsInitSwitchContext(POVS_SWITCH_CONTEXT switchContext)
switchContext->isActivateFailed = FALSE;
switchContext->dpNo = OVS_DP_NUMBER;
ovsTimeIncrementPerTick = KeQueryTimeIncrement() / 10000;
+
+ switchContext->refCount = 1;
+
OVS_LOG_TRACE("Exit: Succesfully initialized switchContext: %p",
switchContext);
return NDIS_STATUS_SUCCESS;
@@ -428,6 +431,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. */ @@ -447,9 +456,44 @@
OvsUninitSwitchContext(POVS_SWITCH_CONTEXT switchContext)
switchContext->pidHashArray = NULL;
OvsDeleteFlowTable(&switchContext->datapath);
OvsCleanupBufferPool(switchContext);
+ switchContext->refCount = 0;
OVS_LOG_TRACE("Exit: Delete switchContext: %p", switchContext); }
+VOID
+OvsReleaseSwitchContext(POVS_SWITCH_CONTEXT switchContext) {
+ if (1 == InterlockedCompareExchange(
+ (LONG volatile *)&switchContext->refCount, 0, 1)) {
+ OvsDeleteSwitchContext(switchContext);
+ }
+ else {
+ InterlockedDecrement((LONG volatile *)&switchContext->refCount);
+ }
+}
+
+BOOLEAN
+OvsAcquireSwitchContext(POVS_SWITCH_CONTEXT switchContext) {
+ BOOLEAN ret = FALSE;
+
+ do {
+ if (NULL == switchContext) {
+ break;
+ }
+
+ if (0 == InterlockedCompareExchange(
+ (LONG volatile *)&switchContext->refCount, 0, 0)) {
+ break;
+ }
+
+ InterlockedIncrement((LONG volatile *)&switchContext->refCount);
+ ret = TRUE;
+ } while (!ret);
+
+ 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..25db636 100644
--- a/datapath-windows/ovsext/Switch.h
+++ b/datapath-windows/ovsext/Switch.h
@@ -179,6 +179,12 @@ typedef struct _OVS_SWITCH_CONTEXT
volatile LONG pendingOidCount;
OVS_NBL_POOL ovsPool;
+
+ /*
+ * Reference count used to prevent premature deallocation of the switch
+ * context.
+ */
+ INT32 refCount;
} OVS_SWITCH_CONTEXT, *POVS_SWITCH_CONTEXT;
@@ -202,7 +208,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 +216,11 @@
OvsReleaseDatapath(OVS_DATAPATH *datapath,
NdisReleaseRWLock(datapath->lock, lockState); }
+BOOLEAN
+OvsAcquireSwitchContext(POVS_SWITCH_CONTEXT switchContext);
+
+VOID
+OvsReleaseSwitchContext(POVS_SWITCH_CONTEXT switchContext);
PVOID OvsGetExternalVport();
--
1.9.0.msysgit.0
_______________________________________________
dev mailing list
[email protected]
https://urldefense.proofpoint.com/v2/url?u=http-3A__openvswitch.org_mailman_listinfo_dev&d=AwIGaQ&c=Sqcl0Ez6M0X8aeM67LKIiDJAXVeAw-YihVMNtXt-uEs&r=CWsgHUxi6ExLXY798tmo3LJ4e3geGYp56lkcH-5cLCY&m=bKJV7k8Y9bG2Z1Hczw1uCkihZsQUivQ4nMAjrAWy20I&s=kQtENH_VEx8FY3DCnOS32-4BoIo4cicBvy-57X1DFfY&e=
_______________________________________________
dev mailing list
[email protected]
http://openvswitch.org/mailman/listinfo/dev
