In the ingress datapath the creation of a missed packet doesn't need to be guarded by the dispatchLock. Same for the NBL context initialization.
In the OvsInjectPacketThroughActions function extracting flow parameters (OvsExtractFlow) from the current packet doesn't need to be guarded. Nor by the vports lock, nor by the datapath lock. I also removed from the datapath lock the creation of the queue packet (OvsCreateQueuePacket) in case the flow lookup fails. Also in the OvsGetNetdevCmdHandler function I have removed the dispatchLock acquisition that guarded OvsGetExtInfoIoctl call, due to the fact that the latter function uses the same lock to protect vports handling. Signed-off-by: Sorin Vinturis <svintu...@cloudbasesolutions.com> Reported-by: Samuel Ghinet <sghi...@cloudbasesolutions.com> Reported-at: https://github.com/openvswitch/ovs-issues/issues/19 --- datapath-windows/ovsext/PacketIO.c | 30 +++++++++++++++--------------- datapath-windows/ovsext/Tunnel.c | 27 +++++++++++++-------------- datapath-windows/ovsext/User.c | 10 ++++------ datapath-windows/ovsext/Vport.c | 4 ---- 4 files changed, 32 insertions(+), 39 deletions(-) diff --git a/datapath-windows/ovsext/PacketIO.c b/datapath-windows/ovsext/PacketIO.c index 87d7037..00786a3 100644 --- a/datapath-windows/ovsext/PacketIO.c +++ b/datapath-windows/ovsext/PacketIO.c @@ -227,14 +227,6 @@ OvsStartNBLIngress(POVS_SWITCH_CONTEXT switchContext, sourcePort = fwdDetail->SourcePortId; sourceIndex = (NDIS_SWITCH_NIC_INDEX)fwdDetail->SourceNicIndex; - /* Take the DispatchLock so none of the VPORTs disconnect while - * we are setting destination ports. - * - * XXX: acquire/release the dispatch lock for a "batch" of packets - * rather than for each packet. */ - NdisAcquireRWLockRead(switchContext->dispatchLock, &lockState, - dispatch); - ctx = OvsInitExternalNBLContext(switchContext, curNbl, sourcePort == switchContext->externalPortId); if (ctx == NULL) { @@ -244,10 +236,17 @@ OvsStartNBLIngress(POVS_SWITCH_CONTEXT switchContext, OvsStartNBLIngressError(switchContext, curNbl, sendCompleteFlags, &filterReason, NDIS_STATUS_RESOURCES); - NdisReleaseRWLock(switchContext->dispatchLock, &lockState); continue; } + /* Take the DispatchLock so none of the VPORTs disconnect while + * we are setting destination ports. + * + * XXX: acquire/release the dispatch lock for a "batch" of packets + * rather than for each packet. */ + NdisAcquireRWLockRead(switchContext->dispatchLock, &lockState, + dispatch); + vport = OvsFindVportByPortIdAndNicIndex(switchContext, sourcePort, sourceIndex); if (vport == NULL || vport->ovsState != OVS_STATE_CONNECTED) { @@ -261,14 +260,15 @@ OvsStartNBLIngress(POVS_SWITCH_CONTEXT switchContext, vport->stats.rxPackets++; vport->stats.rxBytes += NET_BUFFER_DATA_LENGTH(curNb); - status = OvsExtractFlow(curNbl, vport->portNo, &key, &layers, NULL); + NdisReleaseRWLock(switchContext->dispatchLock, &lockState); + + status = OvsExtractFlow(curNbl, portNo, &key, &layers, NULL); if (status != NDIS_STATUS_SUCCESS) { RtlInitUnicodeString(&filterReason, L"OVS-Flow extract failed"); goto dropit; } - ASSERT(KeGetCurrentIrql() == DISPATCH_LEVEL); - OvsAcquireDatapathRead(datapath, &dpLockState, TRUE); + OvsAcquireDatapathRead(datapath, &dpLockState, FALSE); flow = OvsLookupFlow(datapath, &key, &hash, FALSE); if (flow) { @@ -280,13 +280,14 @@ OvsStartNBLIngress(POVS_SWITCH_CONTEXT switchContext, OvsActionsExecute(switchContext, &completionList, curNbl, portNo, SendFlags, &key, &hash, &layers, flow->actions, flow->actionsLen); + OvsReleaseDatapath(datapath, &dpLockState); - NdisReleaseRWLock(switchContext->dispatchLock, &lockState); continue; } else { + datapath->misses++; + OvsReleaseDatapath(datapath, &dpLockState); - datapath->misses++; status = OvsCreateAndAddPackets(OVS_DEFAULT_PACKET_QUEUE, NULL, 0, OVS_PACKET_CMD_MISS, portNo, @@ -312,7 +313,6 @@ OvsStartNBLIngress(POVS_SWITCH_CONTEXT switchContext, dropit: OvsAddPktCompletionList(&completionList, TRUE, sourcePort, curNbl, 0, &filterReason); - NdisReleaseRWLock(switchContext->dispatchLock, &lockState); } } diff --git a/datapath-windows/ovsext/Tunnel.c b/datapath-windows/ovsext/Tunnel.c index 64e279c..cfdb308 100644 --- a/datapath-windows/ovsext/Tunnel.c +++ b/datapath-windows/ovsext/Tunnel.c @@ -277,29 +277,32 @@ OvsInjectPacketThroughActions(PNET_BUFFER_LIST pNbl, curNb = NET_BUFFER_LIST_FIRST_NB(pNbl); ASSERT(curNb->Next == NULL); - NdisAcquireRWLockRead(gOvsSwitchContext->dispatchLock, &lockState, dispatch); - - /* Lock the flowtable for the duration of accessing the flow */ - OvsAcquireDatapathRead(datapath, &dpLockState, NDIS_RWL_AT_DISPATCH_LEVEL); - SendFlags |= NDIS_SEND_FLAGS_DISPATCH_LEVEL; + NdisAcquireRWLockRead(gOvsSwitchContext->dispatchLock, &lockState, dispatch); + vport = gOvsSwitchContext->vxlanVport; if (vport == NULL){ status = STATUS_UNSUCCESSFUL; - goto unlockAndDrop; + NdisReleaseRWLock(gOvsSwitchContext->dispatchLock, &lockState); + goto dropit; } ASSERT(vport->ovsType == OVS_VPORT_TYPE_VXLAN); portNo = vport->portNo; + NdisReleaseRWLock(gOvsSwitchContext->dispatchLock, &lockState); + status = OvsExtractFlow(pNbl, portNo, &key, &layers, &tunnelKey); if (status != NDIS_STATUS_SUCCESS) { - goto unlockAndDrop; + goto dropit; } + /* Lock the flowtable for the duration of accessing the flow */ + OvsAcquireDatapathRead(datapath, &dpLockState, dispatch); + flow = OvsLookupFlow(datapath, &key, &hash, FALSE); if (flow) { OvsFlowUsed(flow, pNbl, &layers); @@ -314,6 +317,8 @@ OvsInjectPacketThroughActions(PNET_BUFFER_LIST pNbl, POVS_PACKET_QUEUE_ELEM elem; datapath->misses++; + OvsReleaseDatapath(datapath, &dpLockState); + elem = OvsCreateQueuePacket(1, NULL, 0, OVS_PACKET_CMD_MISS, portNo, &key.tunKey, pNbl, curNb, TRUE, &layers); @@ -324,18 +329,12 @@ OvsInjectPacketThroughActions(PNET_BUFFER_LIST pNbl, } else { status = STATUS_INSUFFICIENT_RESOURCES; } - goto unlockAndDrop; + goto dropit; } - - NdisReleaseRWLock(gOvsSwitchContext->dispatchLock, &lockState); - } return status; -unlockAndDrop: - OvsReleaseDatapath(datapath, &dpLockState); - NdisReleaseRWLock(gOvsSwitchContext->dispatchLock, &lockState); dropit: pNbl = OvsCompleteNBL(gOvsSwitchContext, pNbl, TRUE); ASSERT(pNbl == NULL); diff --git a/datapath-windows/ovsext/User.c b/datapath-windows/ovsext/User.c index 26cd431..6d896b0 100644 --- a/datapath-windows/ovsext/User.c +++ b/datapath-windows/ovsext/User.c @@ -421,16 +421,14 @@ OvsCancelIrpDatapath(PDEVICE_OBJECT deviceObject, if (fileObject == NULL) { goto done; } - NdisAcquireSpinLock(gOvsCtrlLock); instance = (POVS_OPEN_INSTANCE)fileObject->FsContext; - if (instance) { - queue = instance->packetQueue; + if (instance == NULL) { + goto done; } - if (instance == NULL || queue == NULL) { - NdisReleaseSpinLock(gOvsCtrlLock); + queue = instance->packetQueue; + if (queue == NULL) { goto done; } - NdisReleaseSpinLock(gOvsCtrlLock); NdisAcquireSpinLock(&queue->queueLock); if (queue->pendingIrp == irp) { queue->pendingIrp = NULL; diff --git a/datapath-windows/ovsext/Vport.c b/datapath-windows/ovsext/Vport.c index 0522cfd..07750ca 100644 --- a/datapath-windows/ovsext/Vport.c +++ b/datapath-windows/ovsext/Vport.c @@ -1095,7 +1095,6 @@ OvsGetNetdevCmdHandler(POVS_USER_PARAMS_CONTEXT usrParamsCtx, NL_ERROR nlError = NL_ERROR_SUCCESS; OVS_VPORT_GET vportGet; OVS_VPORT_EXT_INFO info; - LOCK_STATE_EX lockState; static const NL_POLICY ovsNetdevPolicy[] = { [OVS_WIN_NETDEV_ATTR_NAME] = { .type = NL_A_STRING, @@ -1129,15 +1128,12 @@ OvsGetNetdevCmdHandler(POVS_USER_PARAMS_CONTEXT usrParamsCtx, RtlCopyMemory(&vportGet.name, NlAttrGet(netdevAttrs[OVS_VPORT_ATTR_NAME]), NlAttrGetSize(netdevAttrs[OVS_VPORT_ATTR_NAME])); - NdisAcquireRWLockRead(gOvsSwitchContext->dispatchLock, &lockState, 0); status = OvsGetExtInfoIoctl(&vportGet, &info); if (status == STATUS_DEVICE_DOES_NOT_EXIST) { nlError = NL_ERROR_NODEV; - NdisReleaseRWLock(gOvsSwitchContext->dispatchLock, &lockState); OvsReleaseCtrlLock(); goto cleanup; } - NdisReleaseRWLock(gOvsSwitchContext->dispatchLock, &lockState); status = CreateNetlinkMesgForNetdev(&info, msgIn, usrParamsCtx->outputBuffer, usrParamsCtx->outputLength, -- 1.9.0.msysgit.0 _______________________________________________ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
