From 1ce4551c8c158e465734e26589d8444477cd5218 Mon Sep 17 00:00:00 2001 From: Sorin Vinturis Date: Mon, 6 Oct 2014 15:19:23 +0000 Subject: [PATCH] datapath-windows: Incorrect assumption of the IRQL Acquiring a spin lock raises the IRQL to DISPATCH_LEVEL. But in many places of the code, while holding a spin lock, there are useless checks for the current IRQL against DISPATCH_LEVEL. Also, the dispatch flag is not correctly set when calling NdisAcquireRWLockXXX() functions, which causes an extra check of the current IRQL. Signed-off-by: Sorin Vinturis Reported-by: Sorin Vinturis Reported-at: https://github.com/openvswitch/ovs-issues/issues/47 Acked-by: Nithin Raju Acked-by: Alin Gabriel Serdean Acked-by: Eitan Eliahu Signed-off-by: Ben Pfaff --- datapath-windows/ovsext/Datapath.c | 5 +++-- datapath-windows/ovsext/Flow.c | 12 ++++++++---- datapath-windows/ovsext/PacketIO.c | 2 +- 3 files changed, 12 insertions(+), 7 deletions(-) diff --git a/datapath-windows/ovsext/Datapath.c b/datapath-windows/ovsext/Datapath.c index e2d89a647..6e7a298c9 100644 --- a/datapath-windows/ovsext/Datapath.c +++ b/datapath-windows/ovsext/Datapath.c @@ -1389,8 +1389,9 @@ OvsGetVportDumpNext(POVS_USER_PARAMS_CONTEXT usrParamsCtx, * it means we have an array of pids, instead of a single pid. * ATM we assume we have one pid only. */ - - NdisAcquireRWLockRead(gOvsSwitchContext->dispatchLock, &lockState, 0); + ASSERT(KeGetCurrentIrql() == DISPATCH_LEVEL); + NdisAcquireRWLockRead(gOvsSwitchContext->dispatchLock, &lockState, + NDIS_RWL_AT_DISPATCH_LEVEL); if (gOvsSwitchContext->numVports > 0) { /* inBucket: the bucket, used for lookup */ diff --git a/datapath-windows/ovsext/Flow.c b/datapath-windows/ovsext/Flow.c index ebcf005d6..50fa954c1 100644 --- a/datapath-windows/ovsext/Flow.c +++ b/datapath-windows/ovsext/Flow.c @@ -1935,7 +1935,8 @@ OvsDoDumpFlows(OvsFlowDumpInput *dumpInput, datapath = &gOvsSwitchContext->datapath; ASSERT(datapath); - OvsAcquireDatapathRead(datapath, &dpLockState, FALSE); + ASSERT(KeGetCurrentIrql() == DISPATCH_LEVEL); + OvsAcquireDatapathRead(datapath, &dpLockState, TRUE); head = &datapath->flowTable[rowIndex]; node = head->Flink; @@ -2077,7 +2078,8 @@ OvsPutFlowIoctl(PVOID inputBuffer, datapath = &gOvsSwitchContext->datapath; ASSERT(datapath); - OvsAcquireDatapathWrite(datapath, &dpLockState, FALSE); + ASSERT(KeGetCurrentIrql() == DISPATCH_LEVEL); + OvsAcquireDatapathWrite(datapath, &dpLockState, TRUE); status = HandleFlowPut(put, datapath, stats); OvsReleaseDatapath(datapath, &dpLockState); @@ -2256,7 +2258,8 @@ OvsGetFlowIoctl(PVOID inputBuffer, datapath = &gOvsSwitchContext->datapath; ASSERT(datapath); - OvsAcquireDatapathRead(datapath, &dpLockState, FALSE); + ASSERT(KeGetCurrentIrql() == DISPATCH_LEVEL); + OvsAcquireDatapathRead(datapath, &dpLockState, TRUE); flow = OvsLookupFlow(datapath, &getInput->key, &hash, FALSE); if (!flow) { status = STATUS_INVALID_PARAMETER; @@ -2289,7 +2292,8 @@ OvsFlushFlowIoctl(UINT32 dpNo) datapath = &gOvsSwitchContext->datapath; ASSERT(datapath); - OvsAcquireDatapathWrite(datapath, &dpLockState, FALSE); + ASSERT(KeGetCurrentIrql() == DISPATCH_LEVEL); + OvsAcquireDatapathWrite(datapath, &dpLockState, TRUE); DeleteAllFlows(datapath); OvsReleaseDatapath(datapath, &dpLockState); diff --git a/datapath-windows/ovsext/PacketIO.c b/datapath-windows/ovsext/PacketIO.c index ac7862d85..87d703786 100644 --- a/datapath-windows/ovsext/PacketIO.c +++ b/datapath-windows/ovsext/PacketIO.c @@ -268,7 +268,7 @@ OvsStartNBLIngress(POVS_SWITCH_CONTEXT switchContext, } ASSERT(KeGetCurrentIrql() == DISPATCH_LEVEL); - OvsAcquireDatapathRead(datapath, &dpLockState, dispatch); + OvsAcquireDatapathRead(datapath, &dpLockState, TRUE); flow = OvsLookupFlow(datapath, &key, &hash, FALSE); if (flow) { -- 2.20.1