There is no need to use gOvsCtrlLock spinlock to guard the switch
context, as there is now the switch context's reference count used
for this purpose.
Now the gOvsCtrlLock spinlock guards only one shared resource, the
OVS_OPEN_INSTANCE global instance array.
v2: Atomically test and set if the instance is in attach process, to
protect filter attach routine against multiple calls.
v3: Changed gOvsInAttach global variable type into 'LONG volatile'.
Also removed an unrelated change from SubscribeIoctl handler.
Signed-off-by: Sorin Vinturis <svintu...@cloudbasesolutions.com>
Acked-by: Nithin Raju <nit...@vmware.com>
---
datapath-windows/ovsext/Datapath.c | 15 ---------------
datapath-windows/ovsext/Flow.c | 39 +++++++++++++-------------------------
datapath-windows/ovsext/Switch.c | 11 +++--------
datapath-windows/ovsext/User.c | 16 ++++------------
datapath-windows/ovsext/Vport.c | 22 ++-------------------
5 files changed, 22 insertions(+), 81 deletions(-)
diff --git a/datapath-windows/ovsext/Datapath.c
b/datapath-windows/ovsext/Datapath.c
index 3115657..dfabbaf 100644
--- a/datapath-windows/ovsext/Datapath.c
+++ b/datapath-windows/ovsext/Datapath.c
@@ -920,14 +920,11 @@ ValidateNetlinkCmd(UINT32 devOp,
/* Validate the DP for commands that require a DP. */
if (nlFamilyOps->cmds[i].validateDpIndex == TRUE) {
- OvsAcquireCtrlLock();
if (ovsMsg->ovsHdr.dp_ifindex !=
(INT)gOvsSwitchContext->dpNo) {
status = STATUS_INVALID_PARAMETER;
- OvsReleaseCtrlLock();
goto done;
}
- OvsReleaseCtrlLock();
}
/* Validate the PID. */
@@ -1014,7 +1011,6 @@ OvsGetPidCmdHandler(POVS_USER_PARAMS_CONTEXT usrParamsCtx,
* --------------------------------------------------------------------------
* Utility function to fill up information about the datapath in a reply to
* userspace.
- * Assumes that 'gOvsCtrlLock' lock is acquired.
* --------------------------------------------------------------------------
*/
static NTSTATUS
@@ -1214,9 +1210,7 @@ HandleGetDpDump(POVS_USER_PARAMS_CONTEXT usrParamsCtx,
NlBufInit(&nlBuf, usrParamsCtx->outputBuffer,
usrParamsCtx->outputLength);
- OvsAcquireCtrlLock();
status = OvsDpFillInfo(gOvsSwitchContext, msgIn, &nlBuf);
- OvsReleaseCtrlLock();
if (status != STATUS_SUCCESS) {
*replyLen = 0;
@@ -1303,11 +1297,9 @@ HandleDpTransactionCommon(POVS_USER_PARAMS_CONTEXT
usrParamsCtx,
NlBufInit(&nlBuf, usrParamsCtx->outputBuffer, usrParamsCtx->outputLength);
- OvsAcquireCtrlLock();
if (dpAttrs[OVS_DP_ATTR_NAME] != NULL) {
if (!OvsCompareString(NlAttrGet(dpAttrs[OVS_DP_ATTR_NAME]),
OVS_SYSTEM_DP_NAME)) {
- OvsReleaseCtrlLock();
/* Creation of new datapaths is not supported. */
if (usrParamsCtx->ovsMsg->genlMsg.cmd == OVS_DP_CMD_SET) {
@@ -1319,19 +1311,16 @@ HandleDpTransactionCommon(POVS_USER_PARAMS_CONTEXT
usrParamsCtx,
goto cleanup;
}
} else if ((UINT32)msgIn->ovsHdr.dp_ifindex != gOvsSwitchContext->dpNo) {
- OvsReleaseCtrlLock();
nlError = NL_ERROR_NODEV;
goto cleanup;
}
if (usrParamsCtx->ovsMsg->genlMsg.cmd == OVS_DP_CMD_NEW) {
- OvsReleaseCtrlLock();
nlError = NL_ERROR_EXIST;
goto cleanup;
}
status = OvsDpFillInfo(gOvsSwitchContext, msgIn, &nlBuf);
- OvsReleaseCtrlLock();
*replyLen = NlBufSize(&nlBuf);
@@ -1413,7 +1402,6 @@ MapIrpOutputBuffer(PIRP irp,
* --------------------------------------------------------------------------
* Utility function to fill up information about the state of a port in a reply
* to* userspace.
- * Assumes that 'gOvsCtrlLock' lock is acquired.
* --------------------------------------------------------------------------
*/
static NTSTATUS
@@ -1517,8 +1505,6 @@ OvsReadEventCmdHandler(POVS_USER_PARAMS_CONTEXT
usrParamsCtx,
NlBufInit(&nlBuf, usrParamsCtx->outputBuffer, usrParamsCtx->outputLength);
- OvsAcquireCtrlLock();
-
/* remove an event entry from the event queue */
status = OvsRemoveEventEntry(usrParamsCtx->ovsInstance, &eventEntry);
if (status != STATUS_SUCCESS) {
@@ -1534,7 +1520,6 @@ OvsReadEventCmdHandler(POVS_USER_PARAMS_CONTEXT
usrParamsCtx,
}
cleanup:
- OvsReleaseCtrlLock();
return status;
}
diff --git a/datapath-windows/ovsext/Flow.c b/datapath-windows/ovsext/Flow.c
index d3de8cc..1f1bd2f 100644
--- a/datapath-windows/ovsext/Flow.c
+++ b/datapath-windows/ovsext/Flow.c
@@ -31,7 +31,6 @@
#pragma warning( push )
#pragma warning( disable:4127 )
-extern PNDIS_SPIN_LOCK gOvsCtrlLock;
extern POVS_SWITCH_CONTEXT gOvsSwitchContext;
extern UINT64 ovsTimeIncrementPerTick;
@@ -1995,25 +1994,23 @@ OvsDoDumpFlows(OvsFlowDumpInput *dumpInput,
BOOLEAN findNextNonEmpty = FALSE;
dpNo = dumpInput->dpNo;
- NdisAcquireSpinLock(gOvsCtrlLock);
if (gOvsSwitchContext->dpNo != dpNo) {
status = STATUS_INVALID_PARAMETER;
- goto unlock;
+ goto exit;
}
rowIndex = dumpInput->position[0];
if (rowIndex >= OVS_FLOW_TABLE_SIZE) {
dumpOutput->n = 0;
*replyLen = sizeof(*dumpOutput);
- goto unlock;
+ goto exit;
}
columnIndex = dumpInput->position[1];
datapath = &gOvsSwitchContext->datapath;
ASSERT(datapath);
- ASSERT(KeGetCurrentIrql() == DISPATCH_LEVEL);
- OvsAcquireDatapathRead(datapath, &dpLockState, TRUE);
+ OvsAcquireDatapathRead(datapath, &dpLockState, FALSE);
head = &datapath->flowTable[rowIndex];
node = head->Flink;
@@ -2062,8 +2059,7 @@ OvsDoDumpFlows(OvsFlowDumpInput *dumpInput,
dp_unlock:
OvsReleaseDatapath(datapath, &dpLockState);
-unlock:
- NdisReleaseSpinLock(gOvsCtrlLock);
+exit:
return status;
}
@@ -2124,21 +2120,18 @@ OvsPutFlowIoctl(PVOID inputBuffer,
}
dpNo = put->dpNo;
- NdisAcquireSpinLock(gOvsCtrlLock);
if (gOvsSwitchContext->dpNo != dpNo) {
status = STATUS_INVALID_PARAMETER;
- goto unlock;
+ goto exit;
}
datapath = &gOvsSwitchContext->datapath;
ASSERT(datapath);
- ASSERT(KeGetCurrentIrql() == DISPATCH_LEVEL);
- OvsAcquireDatapathWrite(datapath, &dpLockState, TRUE);
+ OvsAcquireDatapathWrite(datapath, &dpLockState, FALSE);
status = HandleFlowPut(put, datapath, stats);
OvsReleaseDatapath(datapath, &dpLockState);
-unlock:
- NdisReleaseSpinLock(gOvsCtrlLock);
+exit:
return status;
}
@@ -2305,16 +2298,14 @@ OvsGetFlowIoctl(PVOID inputBuffer,
}
dpNo = getInput->dpNo;
- NdisAcquireSpinLock(gOvsCtrlLock);
if (gOvsSwitchContext->dpNo != dpNo) {
status = STATUS_INVALID_PARAMETER;
- goto unlock;
+ goto exit;
}
datapath = &gOvsSwitchContext->datapath;
ASSERT(datapath);
- ASSERT(KeGetCurrentIrql() == DISPATCH_LEVEL);
- OvsAcquireDatapathRead(datapath, &dpLockState, TRUE);
+ OvsAcquireDatapathRead(datapath, &dpLockState, FALSE);
flow = OvsLookupFlow(datapath, &getInput->key, &hash, FALSE);
if (!flow) {
status = STATUS_INVALID_PARAMETER;
@@ -2326,8 +2317,7 @@ OvsGetFlowIoctl(PVOID inputBuffer,
dp_unlock:
OvsReleaseDatapath(datapath, &dpLockState);
-unlock:
- NdisReleaseSpinLock(gOvsCtrlLock);
+exit:
return status;
}
@@ -2338,21 +2328,18 @@ OvsFlushFlowIoctl(UINT32 dpNo)
OVS_DATAPATH *datapath = NULL;
LOCK_STATE_EX dpLockState;
- NdisAcquireSpinLock(gOvsCtrlLock);
if (gOvsSwitchContext->dpNo != dpNo) {
status = STATUS_INVALID_PARAMETER;
- goto unlock;
+ goto exit;
}
datapath = &gOvsSwitchContext->datapath;
ASSERT(datapath);
- ASSERT(KeGetCurrentIrql() == DISPATCH_LEVEL);
- OvsAcquireDatapathWrite(datapath, &dpLockState, TRUE);
+ OvsAcquireDatapathWrite(datapath, &dpLockState, FALSE);
DeleteAllFlows(datapath);
OvsReleaseDatapath(datapath, &dpLockState);
-unlock:
- NdisReleaseSpinLock(gOvsCtrlLock);
+exit:
return status;
}
diff --git a/datapath-windows/ovsext/Switch.c b/datapath-windows/ovsext/Switch.c
index 1246f70..666bc0c 100644
--- a/datapath-windows/ovsext/Switch.c
+++ b/datapath-windows/ovsext/Switch.c
@@ -35,10 +35,9 @@
#include "Debug.h"
POVS_SWITCH_CONTEXT gOvsSwitchContext;
-BOOLEAN gOvsInAttach;
+LONG volatile gOvsInAttach;
UINT64 ovsTimeIncrementPerTick;
-extern PNDIS_SPIN_LOCK gOvsCtrlLock;
extern NDIS_HANDLE gOvsExtDriverHandle;
extern NDIS_HANDLE gOvsExtDriverObject;
@@ -83,22 +82,18 @@ OvsExtAttach(NDIS_HANDLE ndisFilterHandle,
goto cleanup;
}
- NdisAcquireSpinLock(gOvsCtrlLock);
if (gOvsSwitchContext) {
- NdisReleaseSpinLock(gOvsCtrlLock);
OVS_LOG_TRACE("Exit: Failed to create OVS Switch, only one datapath is"
"supported, %p.", gOvsSwitchContext);
goto cleanup;
}
- if (gOvsInAttach) {
- NdisReleaseSpinLock(gOvsCtrlLock);
+
+ if (InterlockedCompareExchange(&gOvsInAttach, 1, 0)) {
/* Just fail the request. */
OVS_LOG_TRACE("Exit: Failed to create OVS Switch, since another attach"
"instance is in attach process.");
goto cleanup;
}
- gOvsInAttach = TRUE;
- NdisReleaseSpinLock(gOvsCtrlLock);
status = OvsInitIpHelper(ndisFilterHandle);
if (status != STATUS_SUCCESS) {
diff --git a/datapath-windows/ovsext/User.c b/datapath-windows/ovsext/User.c
index d8a657e..4fc3497 100644
--- a/datapath-windows/ovsext/User.c
+++ b/datapath-windows/ovsext/User.c
@@ -142,14 +142,12 @@ OvsCleanupPacketQueue(POVS_OPEN_INSTANCE instance)
}
/* Verify if gOvsSwitchContext exists. */
- OvsAcquireCtrlLock();
if (gOvsSwitchContext) {
/* Remove the instance from pidHashArray */
OvsAcquirePidHashLock();
OvsDelPidInstance(gOvsSwitchContext, instance->pid);
OvsReleasePidHashLock();
}
- OvsReleaseCtrlLock();
}
NTSTATUS
@@ -446,11 +444,9 @@ OvsExecuteDpIoctl(OvsPacketExecute *execute)
OVS_PACKET_HDR_INFO layers;
POVS_VPORT_ENTRY vport;
- NdisAcquireSpinLock(gOvsCtrlLock);
-
if (execute->packetLen == 0) {
status = STATUS_INVALID_PARAMETER;
- goto unlock;
+ goto exit;
}
actions = execute->actions;
@@ -465,7 +461,7 @@ OvsExecuteDpIoctl(OvsPacketExecute *execute)
execute->packetLen);
if (pNbl == NULL) {
status = STATUS_NO_MEMORY;
- goto unlock;
+ goto exit;
}
fwdDetail = NET_BUFFER_LIST_SWITCH_FORWARDING_DETAIL(pNbl);
@@ -482,9 +478,7 @@ OvsExecuteDpIoctl(OvsPacketExecute *execute)
ndisStatus = OvsExtractFlow(pNbl, fwdDetail->SourcePortId, &key, &layers,
NULL);
if (ndisStatus == NDIS_STATUS_SUCCESS) {
- ASSERT(KeGetCurrentIrql() == DISPATCH_LEVEL);
- NdisAcquireRWLockRead(gOvsSwitchContext->dispatchLock, &lockState,
- NDIS_RWL_AT_DISPATCH_LEVEL);
+ NdisAcquireRWLockRead(gOvsSwitchContext->dispatchLock, &lockState, 0);
ndisStatus = OvsActionsExecute(gOvsSwitchContext, NULL, pNbl,
vport ? vport->portNo :
OVS_DEFAULT_PORT_NO,
@@ -505,8 +499,7 @@ OvsExecuteDpIoctl(OvsPacketExecute *execute)
if (pNbl) {
OvsCompleteNBL(gOvsSwitchContext, pNbl, TRUE);
}
-unlock:
- NdisReleaseSpinLock(gOvsCtrlLock);
+exit:
return status;
}
@@ -629,7 +622,6 @@ OvsGetNextPacket(POVS_OPEN_INSTANCE instance)
/*
* ---------------------------------------------------------------------------
* Given a pid, returns the corresponding USER_PACKET_QUEUE.
- * gOvsCtrlLock must be acquired before calling this API.
* ---------------------------------------------------------------------------
*/
POVS_USER_PACKET_QUEUE
diff --git a/datapath-windows/ovsext/Vport.c b/datapath-windows/ovsext/Vport.c
index 6fdaddb..51eb95c 100644
--- a/datapath-windows/ovsext/Vport.c
+++ b/datapath-windows/ovsext/Vport.c
@@ -56,7 +56,6 @@ typedef struct _OVS_TUNFLT_INIT_CONTEXT {
extern POVS_SWITCH_CONTEXT gOvsSwitchContext;
-extern PNDIS_SPIN_LOCK gOvsCtrlLock;
static VOID OvsInitVportWithPortParam(POVS_VPORT_ENTRY vport,
PNDIS_SWITCH_PORT_PARAMETERS portParam);
@@ -1456,8 +1455,6 @@ OvsConvertIfCountedStrToAnsiStr(PIF_COUNTED_STRING wStr,
* --------------------------------------------------------------------------
* Utility function that populates a 'OVS_VPORT_EXT_INFO' structure for the
* specified vport.
- *
- * Assumes that 'gOvsCtrlLock' is held.
* --------------------------------------------------------------------------
*/
NTSTATUS
@@ -1471,9 +1468,7 @@ OvsGetExtInfoIoctl(POVS_VPORT_GET vportGet,
BOOLEAN doConvert = FALSE;
RtlZeroMemory(extInfo, sizeof (POVS_VPORT_EXT_INFO));
- ASSERT(KeGetCurrentIrql() == DISPATCH_LEVEL);
- NdisAcquireRWLockRead(gOvsSwitchContext->dispatchLock, &lockState,
- NDIS_RWL_AT_DISPATCH_LEVEL);
+ NdisAcquireRWLockRead(gOvsSwitchContext->dispatchLock, &lockState, 0);
if (vportGet->portNo == 0) {
StringCbLengthA(vportGet->name, OVS_MAX_PORT_NAME_LENGTH - 1, &len);
vport = OvsFindVportByHvNameA(gOvsSwitchContext, vportGet->name);
@@ -1601,8 +1596,6 @@ OvsGetNetdevCmdHandler(POVS_USER_PARAMS_CONTEXT
usrParamsCtx,
return STATUS_INVALID_PARAMETER;
}
- OvsAcquireCtrlLock();
-
vportGet.portNo = 0;
RtlCopyMemory(&vportGet.name, NlAttrGet(netdevAttrs[OVS_VPORT_ATTR_NAME]),
NlAttrGetSize(netdevAttrs[OVS_VPORT_ATTR_NAME]));
@@ -1610,7 +1603,6 @@ OvsGetNetdevCmdHandler(POVS_USER_PARAMS_CONTEXT
usrParamsCtx,
status = OvsGetExtInfoIoctl(&vportGet, &info);
if (status == STATUS_DEVICE_DOES_NOT_EXIST) {
nlError = NL_ERROR_NODEV;
- OvsReleaseCtrlLock();
goto cleanup;
}
@@ -1620,7 +1612,6 @@ OvsGetNetdevCmdHandler(POVS_USER_PARAMS_CONTEXT
usrParamsCtx,
if (status == STATUS_SUCCESS) {
*replyLen = msgOut->nlMsg.nlmsgLen;
}
- OvsReleaseCtrlLock();
cleanup:
if (nlError != NL_ERROR_SUCCESS) {
@@ -1827,17 +1818,13 @@ OvsGetVportDumpNext(POVS_USER_PARAMS_CONTEXT
usrParamsCtx,
msgIn = instance->dumpState.ovsMsg;
- OvsAcquireCtrlLock();
-
/*
* XXX: when we implement OVS_DP_ATTR_USER_FEATURES in datapath,
* we'll need to check the OVS_DP_F_VPORT_PIDS flag: if it is set,
* it means we have an array of pids, instead of a single pid.
* ATM we assume we have one pid only.
*/
- ASSERT(KeGetCurrentIrql() == DISPATCH_LEVEL);
- NdisAcquireRWLockRead(gOvsSwitchContext->dispatchLock, &lockState,
- NDIS_RWL_AT_DISPATCH_LEVEL);
+ NdisAcquireRWLockRead(gOvsSwitchContext->dispatchLock, &lockState, 0);
if (gOvsSwitchContext->numHvVports > 0 ||
gOvsSwitchContext->numNonHvVports > 0) {
@@ -1898,8 +1885,6 @@ OvsGetVportDumpNext(POVS_USER_PARAMS_CONTEXT usrParamsCtx,
NdisReleaseRWLock(gOvsSwitchContext->dispatchLock, &lockState);
- OvsReleaseCtrlLock();
-
/* if i < OVS_MAX_VPORT_ARRAY_SIZE => vport was found */
if (i < OVS_MAX_VPORT_ARRAY_SIZE) {
POVS_MESSAGE msgOut = (POVS_MESSAGE)usrParamsCtx->outputBuffer;
@@ -2287,8 +2272,6 @@ OvsSetVportCmdHandler(POVS_USER_PARAMS_CONTEXT
usrParamsCtx,
/* Output buffer has been validated while validating transact dev op. */
ASSERT(msgOut != NULL && usrParamsCtx->outputLength >= sizeof *msgOut);
- OvsAcquireCtrlLock();
-
NdisAcquireRWLockWrite(gOvsSwitchContext->dispatchLock, &lockState, 0);
if (vportAttrs[OVS_VPORT_ATTR_NAME] != NULL) {
PSTR portName = NlAttrGet(vportAttrs[OVS_VPORT_ATTR_NAME]);
@@ -2341,7 +2324,6 @@ OvsSetVportCmdHandler(POVS_USER_PARAMS_CONTEXT
usrParamsCtx,
Cleanup:
NdisReleaseRWLock(gOvsSwitchContext->dispatchLock, &lockState);
- OvsReleaseCtrlLock();
if (nlError != NL_ERROR_SUCCESS) {
POVS_MESSAGE_ERROR msgError = (POVS_MESSAGE_ERROR)
--
1.9.0.msysgit.0
_______________________________________________
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
