Skip to content

Commit

Permalink
datapath-windows: Incorrect assumption of the IRQL
Browse files Browse the repository at this point in the history
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 <svinturis@cloudbasesolutions.com>
Reported-by: Sorin Vinturis <svinturis@cloudbasesolutions.com>
Reported-at: openvswitch/ovs-issues#47
Acked-by: Nithin Raju <nithin@vmware.com>
Acked-by: Alin Gabriel Serdean <aserdean@cloudbasesolutions.com>
Acked-by: Eitan Eliahu <eliahue@vmware.com>
Signed-off-by: Ben Pfaff <blp@nicira.com>
  • Loading branch information
svinturis authored and blp committed Oct 6, 2014
1 parent 52a524e commit 1ce4551
Show file tree
Hide file tree
Showing 3 changed files with 12 additions and 7 deletions.
5 changes: 3 additions & 2 deletions datapath-windows/ovsext/Datapath.c
Expand Up @@ -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 */
Expand Down
12 changes: 8 additions & 4 deletions datapath-windows/ovsext/Flow.c
Expand Up @@ -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;
Expand Down Expand Up @@ -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);

Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -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);

Expand Down
2 changes: 1 addition & 1 deletion datapath-windows/ovsext/PacketIO.c
Expand Up @@ -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) {
Expand Down

0 comments on commit 1ce4551

Please sign in to comment.