[ovs-dev] [PATCH] datapath-windows: Removed all duplicate checking of

Eitan Eliahu eliahue at vmware.com
Fri Nov 14 17:48:26 UTC 2014


Acked-by: Eitan Eliahu <eliahue at vmware.com>

Thanks!
Eitan


-----Original Message-----
From: dev [mailto:dev-bounces at openvswitch.org] On Behalf Of Sorin Vinturis
Sent: Friday, November 14, 2014 3:35 AM
To: dev at openvswitch.org
Subject: [ovs-dev] [PATCH] datapath-windows: Removed all duplicate checking of

Right now the gOvsSwitchContext pointer is checked against NULL in a lot of places of the OVS extension code. This check should be done only once to avoid wasteful checks. Thus I have added the check in the dispatch routine, before doing any processing, and removed all other checks from the rest of the code.

Signed-off-by: Sorin Vinturis <svinturis at cloudbasesolutions.com>
---
 datapath-windows/ovsext/Datapath.c | 66 ++++++--------------------------------
 datapath-windows/ovsext/Flow.c     | 12 +++----
 datapath-windows/ovsext/User.c     | 11 -------
 datapath-windows/ovsext/Vport.c    |  4 ---
 4 files changed, 13 insertions(+), 80 deletions(-)

diff --git a/datapath-windows/ovsext/Datapath.c b/datapath-windows/ovsext/Datapath.c
index 1b504a2..6de011c 100644
--- a/datapath-windows/ovsext/Datapath.c
+++ b/datapath-windows/ovsext/Datapath.c
@@ -470,8 +470,7 @@ OvsGetOpenInstance(PFILE_OBJECT fileObject,
     POVS_OPEN_INSTANCE instance = (POVS_OPEN_INSTANCE)fileObject->FsContext;
     ASSERT(instance);
     ASSERT(instance->fileObject == fileObject);
-    if (gOvsSwitchContext == NULL ||
-        gOvsSwitchContext->dpNo != dpNo) {
+    if (gOvsSwitchContext->dpNo != dpNo) {
         return NULL;
     }
     return instance;
@@ -653,7 +652,6 @@ NTSTATUS
 OvsDeviceControl(PDEVICE_OBJECT deviceObject,
                  PIRP irp)
 {
-
     PIO_STACK_LOCATION irpSp;
     NTSTATUS status = STATUS_SUCCESS;
     PFILE_OBJECT fileObject;
@@ -690,6 +688,12 @@ 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;
+    }
+
     /* Concurrent netlink operations are not supported. */
     if (InterlockedCompareExchange((LONG volatile *)&instance->inUse, 1, 0)) {
         status = STATUS_RESOURCE_IN_USE; @@ -891,7 +895,7 @@ ValidateNetlinkCmd(UINT32 devOp,
             /* Validate the DP for commands that require a DP. */
             if (nlFamilyOps->cmds[i].validateDpIndex == TRUE) {
                 OvsAcquireCtrlLock();
-                if (!gOvsSwitchContext || ovsMsg->ovsHdr.dp_ifindex !=
+                if (ovsMsg->ovsHdr.dp_ifindex !=
                                           (INT)gOvsSwitchContext->dpNo) {
                     status = STATUS_INVALID_PARAMETER;
                     OvsReleaseCtrlLock(); @@ -1184,13 +1188,6 @@ HandleGetDpDump(POVS_USER_PARAMS_CONTEXT usrParamsCtx,
                   usrParamsCtx->outputLength);
 
         OvsAcquireCtrlLock();
-        if (!gOvsSwitchContext) {
-            /* Treat this as a dump done. */
-            OvsReleaseCtrlLock();
-            *replyLen = 0;
-            FreeUserDumpState(instance);
-            return STATUS_SUCCESS;
-        }
         status = OvsDpFillInfo(gOvsSwitchContext, msgIn, &nlBuf);
         OvsReleaseCtrlLock();
 
@@ -1280,8 +1277,7 @@ HandleDpTransactionCommon(POVS_USER_PARAMS_CONTEXT usrParamsCtx,
 
     OvsAcquireCtrlLock();
     if (dpAttrs[OVS_DP_ATTR_NAME] != NULL) {
-        if (!gOvsSwitchContext &&
-            !OvsCompareString(NlAttrGet(dpAttrs[OVS_DP_ATTR_NAME]),
+        if (!OvsCompareString(NlAttrGet(dpAttrs[OVS_DP_ATTR_NAME]),
                               OVS_SYSTEM_DP_NAME)) {
             OvsReleaseCtrlLock();
 
@@ -1495,13 +1491,6 @@ OvsGetVportDumpNext(POVS_USER_PARAMS_CONTEXT usrParamsCtx,
     msgIn = instance->dumpState.ovsMsg;
 
     OvsAcquireCtrlLock();
-    if (!gOvsSwitchContext) {
-        /* Treat this as a dump done. */
-        OvsReleaseCtrlLock();
-        *replyLen = 0;
-        FreeUserDumpState(instance);
-        return STATUS_SUCCESS;
-    }
 
     /*
      * XXX: when we implement OVS_DP_ATTR_USER_FEATURES in datapath, @@ -1629,13 +1618,6 @@ OvsGetVport(POVS_USER_PARAMS_CONTEXT usrParamsCtx,
         return STATUS_INVALID_BUFFER_SIZE;
     }
 
-    OvsAcquireCtrlLock();
-    if (!gOvsSwitchContext) {
-        OvsReleaseCtrlLock();
-        return STATUS_INVALID_PARAMETER;
-    }
-    OvsReleaseCtrlLock();
-
     NdisAcquireRWLockRead(gOvsSwitchContext->dispatchLock, &lockState, 0);
     if (vportAttrs[OVS_VPORT_ATTR_NAME] != NULL) {
         portName = NlAttrGet(vportAttrs[OVS_VPORT_ATTR_NAME]);
@@ -1776,13 +1758,6 @@ OvsNewVportCmdHandler(POVS_USER_PARAMS_CONTEXT usrParamsCtx,
         return STATUS_INVALID_PARAMETER;
     }
 
-    OvsAcquireCtrlLock();
-    if (!gOvsSwitchContext) {
-        OvsReleaseCtrlLock();
-        return STATUS_INVALID_PARAMETER;
-    }
-    OvsReleaseCtrlLock();
-
     portName = NlAttrGet(vportAttrs[OVS_VPORT_ATTR_NAME]);
     portNameLen = NlAttrGetSize(vportAttrs[OVS_VPORT_ATTR_NAME]);
     portType = NlAttrGetU32(vportAttrs[OVS_VPORT_ATTR_TYPE]);
@@ -1962,10 +1937,6 @@ OvsSetVportCmdHandler(POVS_USER_PARAMS_CONTEXT usrParamsCtx,
     }
 
     OvsAcquireCtrlLock();
-    if (!gOvsSwitchContext) {
-        OvsReleaseCtrlLock();
-        return STATUS_INVALID_PARAMETER;
-    }
 
     NdisAcquireRWLockWrite(gOvsSwitchContext->dispatchLock, &lockState, 0);
     if (vportAttrs[OVS_VPORT_ATTR_NAME] != NULL) { @@ -2071,13 +2042,6 @@ OvsDeleteVportCmdHandler(POVS_USER_PARAMS_CONTEXT usrParamsCtx,
         return STATUS_NDIS_INVALID_LENGTH;
     }
 
-    OvsAcquireCtrlLock();
-    if (!gOvsSwitchContext) {
-        OvsReleaseCtrlLock();
-        return STATUS_INVALID_PARAMETER;
-    }
-    OvsReleaseCtrlLock();
-
     NdisAcquireRWLockWrite(gOvsSwitchContext->dispatchLock, &lockState, 0);
     if (vportAttrs[OVS_VPORT_ATTR_NAME] != NULL) {
         portName = NlAttrGet(vportAttrs[OVS_VPORT_ATTR_NAME]);
@@ -2285,10 +2249,6 @@ OvsReadEventCmdHandler(POVS_USER_PARAMS_CONTEXT usrParamsCtx,
     NlBufInit(&nlBuf, usrParamsCtx->outputBuffer, usrParamsCtx->outputLength);
 
     OvsAcquireCtrlLock();
-    if (!gOvsSwitchContext) {
-        status = STATUS_SUCCESS;
-        goto cleanup;
-    }
 
     /* remove an event entry from the event queue */
     status = OvsRemoveEventEntry(usrParamsCtx->ovsInstance, &eventEntry); @@ -2337,14 +2297,6 @@ OvsReadPacketCmdHandler(POVS_USER_PARAMS_CONTEXT usrParamsCtx,
     /* Output buffer has been validated while validating read dev op. */
     ASSERT(msgOut != NULL && usrParamsCtx->outputLength >= sizeof *msgOut);
 
-    OvsAcquireCtrlLock();
-    if (!gOvsSwitchContext) {
-        status = STATUS_SUCCESS;
-        OvsReleaseCtrlLock();
-        return status;
-    }
-    OvsReleaseCtrlLock();
-
     /* Read a packet from the instance queue */
     status = OvsReadDpIoctl(instance->fileObject, usrParamsCtx->outputBuffer,
                             usrParamsCtx->outputLength, replyLen); diff --git a/datapath-windows/ovsext/Flow.c b/datapath-windows/ovsext/Flow.c index d2d0ae5..0e88d8c 100644
--- a/datapath-windows/ovsext/Flow.c
+++ b/datapath-windows/ovsext/Flow.c
@@ -1985,8 +1985,7 @@ OvsDoDumpFlows(OvsFlowDumpInput *dumpInput,
 
     dpNo = dumpInput->dpNo;
     NdisAcquireSpinLock(gOvsCtrlLock);
-    if (gOvsSwitchContext == NULL ||
-        gOvsSwitchContext->dpNo != dpNo) {
+    if (gOvsSwitchContext->dpNo != dpNo) {
         status = STATUS_INVALID_PARAMETER;
         goto unlock;
     }
@@ -2137,8 +2136,7 @@ OvsPutFlowIoctl(PVOID inputBuffer,
 
     dpNo = put->dpNo;
     NdisAcquireSpinLock(gOvsCtrlLock);
-    if (gOvsSwitchContext == NULL ||
-        gOvsSwitchContext->dpNo != dpNo) {
+    if (gOvsSwitchContext->dpNo != dpNo) {
         status = STATUS_INVALID_PARAMETER;
         goto unlock;
     }
@@ -2319,8 +2317,7 @@ OvsGetFlowIoctl(PVOID inputBuffer,
 
     dpNo = getInput->dpNo;
     NdisAcquireSpinLock(gOvsCtrlLock);
-    if (gOvsSwitchContext == NULL ||
-        gOvsSwitchContext->dpNo != dpNo) {
+    if (gOvsSwitchContext->dpNo != dpNo) {
         status = STATUS_INVALID_PARAMETER;
         goto unlock;
     }
@@ -2353,8 +2350,7 @@ OvsFlushFlowIoctl(UINT32 dpNo)
     LOCK_STATE_EX dpLockState;
 
     NdisAcquireSpinLock(gOvsCtrlLock);
-    if (gOvsSwitchContext == NULL ||
-        gOvsSwitchContext->dpNo != dpNo) {
+    if (gOvsSwitchContext->dpNo != dpNo) {
         status = STATUS_INVALID_PARAMETER;
         goto unlock;
     }
diff --git a/datapath-windows/ovsext/User.c b/datapath-windows/ovsext/User.c index fc27f7d..96adb5c 100644
--- a/datapath-windows/ovsext/User.c
+++ b/datapath-windows/ovsext/User.c
@@ -155,13 +155,6 @@ OvsSubscribeDpIoctl(PVOID instanceP,
     POVS_USER_PACKET_QUEUE queue;
     POVS_OPEN_INSTANCE instance = (POVS_OPEN_INSTANCE)instanceP;
 
-    OvsAcquireCtrlLock();
-    if (!gOvsSwitchContext) {
-        OvsReleaseCtrlLock();
-        return STATUS_INVALID_PARAMETER;
-    }
-    OvsReleaseCtrlLock();
-
     if (instance->packetQueue && !join) {
         /* unsubscribe */
         OvsCleanupPacketQueue(instance); @@ -445,10 +438,6 @@ OvsExecuteDpIoctl(OvsPacketExecute *execute)
     POVS_VPORT_ENTRY vport;
 
     NdisAcquireSpinLock(gOvsCtrlLock);
-    if (gOvsSwitchContext == NULL) {
-        status = STATUS_INVALID_PARAMETER;
-        goto unlock;
-    }
 
     if (execute->packetLen == 0) {
         status = STATUS_INVALID_PARAMETER; diff --git a/datapath-windows/ovsext/Vport.c b/datapath-windows/ovsext/Vport.c index 68755b9..2dc5f0a 100644
--- a/datapath-windows/ovsext/Vport.c
+++ b/datapath-windows/ovsext/Vport.c
@@ -1373,10 +1373,6 @@ OvsGetNetdevCmdHandler(POVS_USER_PARAMS_CONTEXT usrParamsCtx,
     }
 
     OvsAcquireCtrlLock();
-    if (!gOvsSwitchContext) {
-        OvsReleaseCtrlLock();
-        return STATUS_INVALID_PARAMETER;
-    }
 
     vportGet.portNo = 0;
     RtlCopyMemory(&vportGet.name, NlAttrGet(netdevAttrs[OVS_VPORT_ATTR_NAME]),
--
1.9.0.msysgit.0
_______________________________________________
dev mailing list
dev at openvswitch.org
https://urldefense.proofpoint.com/v2/url?u=http-3A__openvswitch.org_mailman_listinfo_dev&d=AAIGaQ&c=Sqcl0Ez6M0X8aeM67LKIiDJAXVeAw-YihVMNtXt-uEs&r=CWsgHUxi6ExLXY798tmo3LJ4e3geGYp56lkcH-5cLCY&m=UZO-YUDVFD8m4qcHQTELMPrV42LuveNW1GW4-AXTsUQ&s=fp--QsWd028_QIQsxFsF8dUbjlLHcRw6EMZhUG6lYU0&e= 


More information about the dev mailing list