From d07ac93e1e154b8a5c031ed2bad97ca8b95d7fc4 Mon Sep 17 00:00:00 2001 From: Sorin Vinturis Date: Fri, 14 Nov 2014 11:35:21 +0000 Subject: datapath-windows: Remove all duplicate checks for NULL. 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 Acked-by: Eitan Eliahu Signed-off-by: Ben Pfaff --- 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 1b504a276..6de011cac 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 d2d0ae5c6..0e88d8c2d 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 fc27f7d54..96adb5c36 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 68755b9a6..2dc5f0a4d 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]), -- cgit v1.2.1