[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