Acked-by: Alin Gabriel Serdean <aserd...@cloudbasesolutions.com>
-----Mesaj original----- De la: dev [mailto:dev-boun...@openvswitch.org] În numele Sorin Vinturis Trimis: Thursday, October 16, 2014 10:48 AM Către: dev@openvswitch.org Subiect: [ovs-dev] [PATCH v2] datapath-windows: Optimized spin locks acquisition I refactored the OvsInjectPacketThroughActions function to make it easier to follow. Also I removed from the datapath lock the creation of the queue packet (OvsCreateQueuePacket) in case the flow lookup fails. 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 | 6 ++- datapath-windows/ovsext/Tunnel.c | 97 ++++++++++++++++++-------------------- datapath-windows/ovsext/User.c | 10 ++-- datapath-windows/ovsext/Vport.c | 4 -- 4 files changed, 54 insertions(+), 63 deletions(-) diff --git a/datapath-windows/ovsext/PacketIO.c b/datapath-windows/ovsext/PacketIO.c index 493c8cb..ce17857 100644 --- a/datapath-windows/ovsext/PacketIO.c +++ b/datapath-windows/ovsext/PacketIO.c @@ -261,7 +261,7 @@ OvsStartNBLIngress(POVS_SWITCH_CONTEXT switchContext, vport->stats.rxPackets++; vport->stats.rxBytes += NET_BUFFER_DATA_LENGTH(curNb); - status = OvsExtractFlow(curNbl, vport->portNo, &key, &layers, NULL); + status = OvsExtractFlow(curNbl, portNo, &key, &layers, + NULL); if (status != NDIS_STATUS_SUCCESS) { RtlInitUnicodeString(&filterReason, L"OVS-Flow extract failed"); goto dropit; @@ -280,13 +280,15 @@ 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(NULL, 0, OVS_PACKET_CMD_MISS, portNo, &key, curNbl, diff --git a/datapath-windows/ovsext/Tunnel.c b/datapath-windows/ovsext/Tunnel.c index eb45454..b183d59 100644 --- a/datapath-windows/ovsext/Tunnel.c +++ b/datapath-windows/ovsext/Tunnel.c @@ -225,45 +225,43 @@ OvsInjectPacketThroughActions(PNET_BUFFER_LIST pNbl, KIRQL irql; ULONG SendFlags = NDIS_SEND_FLAGS_SWITCH_DESTINATION_GROUP; OVS_DATAPATH *datapath = &gOvsSwitchContext->datapath; + POVS_VPORT_ENTRY vport; + UINT32 portNo; + OVS_PACKET_HDR_INFO layers; + OvsFlowKey key; + UINT64 hash; + PNET_BUFFER curNb; + OvsFlow *flow; + + do { + ASSERT(gOvsSwitchContext); + + /* Fill the tunnel key */ + status = OvsSlowPathDecapVxlan(pNbl, &tunnelKey); + if(!NT_SUCCESS(status)) { + break; + } - ASSERT(gOvsSwitchContext); - - /* Fill the tunnel key */ - status = OvsSlowPathDecapVxlan(pNbl, &tunnelKey); - - if(!NT_SUCCESS(status)) { - goto dropit; - } - - pNb = NET_BUFFER_LIST_FIRST_NB(pNbl); - - NdisAdvanceNetBufferDataStart(pNb, - packet->transportHeaderSize + packet->ipHeaderSize + - sizeof(VXLANHdr), - FALSE, - NULL); + pNb = NET_BUFFER_LIST_FIRST_NB(pNbl); - /* Most likely (always) dispatch irql */ - irql = KeGetCurrentIrql(); + NdisAdvanceNetBufferDataStart(pNb, + packet->transportHeaderSize + packet->ipHeaderSize + + sizeof(VXLANHdr), + FALSE, + NULL); - /* dispatch is used for datapath lock as well */ - dispatch = (irql == DISPATCH_LEVEL) ? NDIS_RWL_AT_DISPATCH_LEVEL : 0; - if (dispatch) { - sendCompleteFlags |= NDIS_SEND_COMPLETE_FLAGS_DISPATCH_LEVEL; - } + /* Most likely (always) dispatch irql */ + irql = KeGetCurrentIrql(); - InitializeListHead(&missedPackets); - OvsInitCompletionList(&completionList, gOvsSwitchContext, - sendCompleteFlags); + /* dispatch is used for datapath lock as well */ + dispatch = (irql == DISPATCH_LEVEL) ? NDIS_RWL_AT_DISPATCH_LEVEL : 0; + if (dispatch) { + sendCompleteFlags |= NDIS_SEND_COMPLETE_FLAGS_DISPATCH_LEVEL; + } - { - POVS_VPORT_ENTRY vport; - UINT32 portNo; - OVS_PACKET_HDR_INFO layers; - OvsFlowKey key; - UINT64 hash; - PNET_BUFFER curNb; - OvsFlow *flow; + InitializeListHead(&missedPackets); + OvsInitCompletionList(&completionList, gOvsSwitchContext, + sendCompleteFlags); fwdDetail = NET_BUFFER_LIST_SWITCH_FORWARDING_DETAIL(pNbl); @@ -277,18 +275,16 @@ 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){ + if (vport == NULL) { status = STATUS_UNSUCCESSFUL; - goto unlockAndDrop; + NdisReleaseRWLock(gOvsSwitchContext->dispatchLock, &lockState); + break; } ASSERT(vport->ovsType == OVS_VPORT_TYPE_VXLAN); @@ -297,9 +293,13 @@ OvsInjectPacketThroughActions(PNET_BUFFER_LIST pNbl, status = OvsExtractFlow(pNbl, portNo, &key, &layers, &tunnelKey); if (status != NDIS_STATUS_SUCCESS) { - goto unlockAndDrop; + NdisReleaseRWLock(gOvsSwitchContext->dispatchLock, &lockState); + break; } + /* Lock the flowtable for the duration of accessing the flow */ + OvsAcquireDatapathRead(datapath, &dpLockState, + NDIS_RWL_AT_DISPATCH_LEVEL); + flow = OvsLookupFlow(datapath, &key, &hash, FALSE); if (flow) { OvsFlowUsed(flow, pNbl, &layers); @@ -314,6 +314,8 @@ OvsInjectPacketThroughActions(PNET_BUFFER_LIST pNbl, POVS_PACKET_QUEUE_ELEM elem; datapath->misses++; + OvsReleaseDatapath(datapath, &dpLockState); + elem = OvsCreateQueueNlPacket(NULL, 0, OVS_PACKET_CMD_MISS, portNo, &key, pNbl, curNb, TRUE, &layers); @@ -324,21 +326,14 @@ OvsInjectPacketThroughActions(PNET_BUFFER_LIST pNbl, } else { status = STATUS_INSUFFICIENT_RESOURCES; } - goto unlockAndDrop; } NdisReleaseRWLock(gOvsSwitchContext->dispatchLock, &lockState); + } while (FALSE); - } - - return status; - -unlockAndDrop: - OvsReleaseDatapath(datapath, &dpLockState); - NdisReleaseRWLock(gOvsSwitchContext->dispatchLock, &lockState); -dropit: pNbl = OvsCompleteNBL(gOvsSwitchContext, pNbl, TRUE); ASSERT(pNbl == NULL); + return status; } diff --git a/datapath-windows/ovsext/User.c b/datapath-windows/ovsext/User.c index f24c4e3..bb1d2fc 100644 --- a/datapath-windows/ovsext/User.c +++ b/datapath-windows/ovsext/User.c @@ -538,16 +538,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 _______________________________________________ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
