[ovs-dev] [PATCH v2] datapath-windows: Optimized spin locks acquisition
Alin Serdean
aserdean at cloudbasesolutions.com
Mon Oct 20 21:45:16 UTC 2014
Acked-by: Alin Gabriel Serdean <aserdean at cloudbasesolutions.com>
-----Mesaj original-----
De la: dev [mailto:dev-bounces at openvswitch.org] În numele Sorin Vinturis
Trimis: Thursday, October 16, 2014 10:48 AM
Către: dev at 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 <svinturis at cloudbasesolutions.com>
Reported-by: Samuel Ghinet <sghinet at 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 at openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
More information about the dev
mailing list