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.

Signed-off-by: Sorin Vinturis <svintu...@cloudbasesolutions.com>
---
 datapath-windows/ovsext/Datapath.c | 15 ---------------
 datapath-windows/ovsext/Flow.c     | 39 +++++++++++++-------------------------
 datapath-windows/ovsext/Switch.c   |  5 -----
 datapath-windows/ovsext/User.c     | 22 ++++-----------------
 datapath-windows/ovsext/Vport.c    | 22 ++-------------------
 5 files changed, 19 insertions(+), 84 deletions(-)

diff --git a/datapath-windows/ovsext/Datapath.c 
b/datapath-windows/ovsext/Datapath.c
index 8b561a3..6a76a12 100644
--- a/datapath-windows/ovsext/Datapath.c
+++ b/datapath-windows/ovsext/Datapath.c
@@ -918,14 +918,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. */
@@ -1012,7 +1009,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
@@ -1212,9 +1208,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;
@@ -1301,11 +1295,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) {
@@ -1317,19 +1309,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);
 
@@ -1411,7 +1400,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
@@ -1515,8 +1503,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) {
@@ -1532,7 +1518,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 8366944..0c40916 100644
--- a/datapath-windows/ovsext/Switch.c
+++ b/datapath-windows/ovsext/Switch.c
@@ -38,7 +38,6 @@ POVS_SWITCH_CONTEXT gOvsSwitchContext;
 BOOLEAN 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);
         /* 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..efe52b1 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
@@ -163,12 +161,6 @@ OvsSubscribeDpIoctl(PVOID instanceP,
     if (instance->packetQueue && !join) {
         /* unsubscribe */
         OvsCleanupPacketQueue(instance);
-
-        OvsAcquirePidHashLock();
-        /* Remove the instance from pidHashArray */
-        OvsDelPidInstance(gOvsSwitchContext, pid);
-        OvsReleasePidHashLock();
-
     } else if (instance->packetQueue == NULL && join) {
         queue = (POVS_USER_PACKET_QUEUE) OvsAllocateMemory(sizeof *queue);
         if (queue == NULL) {
@@ -446,11 +438,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 +455,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 +472,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 +493,7 @@ OvsExecuteDpIoctl(OvsPacketExecute *execute)
     if (pNbl) {
         OvsCompleteNBL(gOvsSwitchContext, pNbl, TRUE);
     }
-unlock:
-    NdisReleaseSpinLock(gOvsCtrlLock);
+exit:
     return status;
 }
 
@@ -629,7 +616,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 c9dfaea..cfe757d 100644
--- a/datapath-windows/ovsext/Vport.c
+++ b/datapath-windows/ovsext/Vport.c
@@ -48,7 +48,6 @@
 #define OVS_VPORT_DEFAULT_WAIT_TIME_MICROSEC    100
 
 extern POVS_SWITCH_CONTEXT gOvsSwitchContext;
-extern PNDIS_SPIN_LOCK gOvsCtrlLock;
 
 static VOID OvsInitVportWithPortParam(POVS_VPORT_ENTRY vport,
                 PNDIS_SWITCH_PORT_PARAMETERS portParam);
@@ -1364,8 +1363,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
@@ -1379,9 +1376,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);
@@ -1509,8 +1504,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]));
@@ -1518,7 +1511,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;
     }
 
@@ -1528,7 +1520,6 @@ OvsGetNetdevCmdHandler(POVS_USER_PARAMS_CONTEXT 
usrParamsCtx,
     if (status == STATUS_SUCCESS) {
         *replyLen = msgOut->nlMsg.nlmsgLen;
     }
-    OvsReleaseCtrlLock();
 
 cleanup:
     if (nlError != NL_ERROR_SUCCESS) {
@@ -1735,17 +1726,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) {
@@ -1806,8 +1793,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;
@@ -2184,8 +2169,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]);
@@ -2238,7 +2221,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

Reply via email to