[ovs-dev] Please review: [PATCH 3/3] Refactor OvsStartNBLIngress
Samuel Ghinet
sghinet at cloudbasesolutions.com
Fri Aug 29 13:34:06 UTC 2014
Hello,
Below is part 3 of the patch - main refactor of the ingress path (OvsStartNBLIngress / OvsExtSendNBL)
Thank you!
Samuel Ghinet
=========================
addressing issues:
o) clearer variable names
o) merge OvsExtSendNBL and OvsStartNBLIngress into OvsExtSendNBL
(there is no reason to have functionalities in these two functions
separate)
o) all the processing for one NBL in the original NBL chain is now
processed in a separate function: OvsProcessOneNbl
o) all the processing for one NB (where the NB is wrapped inside a
NBL) is now processed in a separate function: OvsProcessOneNb
o) OvsSplitNblByNB was created to return either:
- the original NBL (if the original NBL has only one NB)
or
- a cloned / partial copy to multiple NBLs, if the original NBL had
more NBs.
---
datapath-windows/ovsext/OvsBufferMgmt.h | 5 +
datapath-windows/ovsext/OvsPacketIO.c | 379 ++++++++++++++++++--------------
2 files changed, 217 insertions(+), 167 deletions(-)
diff --git a/datapath-windows/ovsext/OvsBufferMgmt.h b/datapath-windows/ovsext/OvsBufferMgmt.h
index 7bf4ff0..8851b34 100644
--- a/datapath-windows/ovsext/OvsBufferMgmt.h
+++ b/datapath-windows/ovsext/OvsBufferMgmt.h
@@ -81,6 +81,11 @@ typedef struct _OVS_NBL_POOL {
#endif
} OVS_NBL_POOL, *POVS_NBL_POOL;
+#define OVS_NBL_FOR_EACH(curNbl, nblList) \
+for (NET_BUFFER_LIST* curNbl = nblList; \
+ curNbl != NULL; \
+ curNbl = NET_BUFFER_LIST_NEXT_NBL(curNbl))
+
#define OVS_NBL_FOR_EACH_NEXT(curNbl, nblList, nextNbl) \
for (NET_BUFFER_LIST* curNbl = nblList; \
curNbl != NULL; \
diff --git a/datapath-windows/ovsext/OvsPacketIO.c b/datapath-windows/ovsext/OvsPacketIO.c
index 2895b19..c5fa32c 100644
--- a/datapath-windows/ovsext/OvsPacketIO.c
+++ b/datapath-windows/ovsext/OvsPacketIO.c
@@ -176,187 +176,199 @@ OvsStartNBLIngressError(POVS_SWITCH_CONTEXT switchContext,
sendCompleteFlags);
}
+static PNET_BUFFER_LIST
+OvsSplitNblByNB(PNET_BUFFER_LIST nblList,
+ POVS_BUFFER_CONTEXT bufferContext,
+ POVS_SWITCH_CONTEXT switchContext)
+{
+ PNET_BUFFER curNb;
+ PNET_BUFFER_LIST newNblList;
+
+ curNb = NET_BUFFER_LIST_FIRST_NB(nblList);
+
+ if (curNb->Next == NULL) {
+ return nblList;
+ }
+
+ InterlockedDecrement((volatile LONG*)&bufferContext->refCount);
+ newNblList = OvsPartialCopyToMultipleNBLs(switchContext, nblList,
+ 0, 0, TRUE);
+ if (!newNblList) {
+ return NULL;
+ }
+
+ return newNblList;
+}
+
static VOID
-OvsStartNBLIngress(POVS_SWITCH_CONTEXT switchContext,
- PNET_BUFFER_LIST netBufferLists,
- ULONG SendFlags)
+OvsProcessOneNb(PNET_BUFFER_LIST curNbl,
+ POVS_SWITCH_CONTEXT switchContext,
+ POVS_VPORT_ENTRY sourceVPort,
+ OvsCompletionList* completionList,
+ ULONG sendFlags,
+ LIST_ENTRY* missedPackets,
+ UINT32* countMissedPackets,
+ NDIS_NET_BUFFER_LIST_8021Q_INFO* vlanTagValue)
{
- NDIS_SWITCH_PORT_ID sourcePort = 0;
- NDIS_SWITCH_NIC_INDEX sourceIndex = 0;
- PNDIS_SWITCH_FORWARDING_DETAIL_NET_BUFFER_LIST_INFO fwdDetail;
- PNET_BUFFER_LIST curNbl = NULL, nextNbl = NULL;
- ULONG sendCompleteFlags;
- UCHAR dispatch;
- LOCK_STATE_EX lockState, dpLockState;
NDIS_STATUS status;
+ OvsFlowKey key;
+ OVS_PACKET_HDR_INFO layers;
NDIS_STRING filterReason;
- LIST_ENTRY missedPackets;
- UINT32 num = 0;
- OvsCompletionList completionList;
+ LOCK_STATE_EX dpLockState;
+ OvsFlow *flow;
+ UINT64 hash;
+ BOOLEAN atDispatch;
+ OVS_DATAPATH* datapath = &(switchContext->datapath);
+
+ atDispatch = NDIS_TEST_SEND_AT_DISPATCH_LEVEL(sendFlags) ?
+ NDIS_RWL_AT_DISPATCH_LEVEL : 0;
+
+ /* 1. Extract flow key*/
+ status = OvsExtractFlow(NET_BUFFER_LIST_FIRST_NB(curNbl),
+ sourceVPort->portNo, &key, &layers,
+ NULL, vlanTagValue);
+ if (status != NDIS_STATUS_SUCCESS) {
+ RtlInitUnicodeString(&filterReason, L"OVS-Flow extract failed");
+ OvsAddPktCompletionList(completionList, TRUE, sourceVPort->portId,
+ curNbl, 0, &filterReason);
+ return;
+ }
- dispatch = NDIS_TEST_SEND_AT_DISPATCH_LEVEL(SendFlags)?
- NDIS_RWL_AT_DISPATCH_LEVEL : 0;
- sendCompleteFlags = OvsGetSendCompleteFlags(SendFlags);
- SendFlags |= NDIS_SEND_FLAGS_SWITCH_DESTINATION_GROUP;
+ ASSERT(KeGetCurrentIrql() == DISPATCH_LEVEL);
+ OvsAcquireDatapathRead(datapath, &dpLockState, atDispatch);
+
+ /* 2. Find flow matching key */
+ flow = OvsLookupFlow(datapath, &key, &hash, FALSE);
+ if (flow) {
+ OvsFlowUsed(flow, NET_BUFFER_LIST_FIRST_NB(curNbl), &layers);
+ datapath->hits++;
+
+ /*
+ * 2.a) execute actions on packet.
+ * If successful, OvsActionsExecute() consumes the NBL.
+ * Otherwise, it adds it to the completionList. No need to
+ * check the return value.
+ */
+ OvsActionsExecute(switchContext, completionList, curNbl,
+ sourceVPort->portNo, sendFlags, &key, &hash, &layers,
+ flow->actions, flow->actionsLen);
+
+ OvsReleaseDatapath(datapath, &dpLockState);
+ return;
+ }
- InitializeListHead(&missedPackets);
- OvsInitCompletionList(&completionList, switchContext, sendCompleteFlags);
+ OvsReleaseDatapath(datapath, &dpLockState);
+
+ /* 2.b) no matching flow found => queue packet to userspace */
+ datapath->misses++;
+ status = OvsCreateAndAddPackets(OVS_DEFAULT_PACKET_QUEUE,
+ /*user data*/ NULL, /*user data length*/ 0, OVS_PACKET_CMD_MISS,
+ sourceVPort->portNo,
+ (key.tunKey.dst != 0 ? &key.tunKey : NULL),
+ curNbl,
+ (sourceVPort->portId == switchContext->externalPortId),
+ &layers, switchContext,
+ missedPackets, countMissedPackets);
+ if (status == NDIS_STATUS_SUCCESS) {
+ /* Complete the packet since it was copied to user
+ * buffer.
+ */
+ RtlInitUnicodeString(&filterReason,
+ L"OVS-Dropped since packet was copied to userspace");
+ }
+ else {
+ RtlInitUnicodeString(&filterReason,
+ L"OVS-Dropped due to failure to queue to userspace");
+ }
+ OvsAddPktCompletionList(completionList, TRUE, sourceVPort->portId,
+ curNbl, 0, &filterReason);
+}
- for (curNbl = netBufferLists; curNbl != NULL; curNbl = nextNbl) {
- POVS_VPORT_ENTRY vport;
- UINT32 portNo;
- OVS_DATAPATH *datapath = &switchContext->datapath;
- OVS_PACKET_HDR_INFO layers;
- OvsFlowKey key;
- UINT64 hash;
- PNET_BUFFER curNb;
- VOID* vlanTagValue;
- NET_BUFFER_LIST* pNextNbl;
+static NDIS_STATUS
+OvsProcessOneNbl(PNET_BUFFER_LIST origNbl,
+ POVS_SWITCH_CONTEXT switchContext,
+ ULONG sendFlags,
+ OvsCompletionList* completionList,
+ LIST_ENTRY* missedPackets,
+ UINT32* countMissedPackets,
+ const WCHAR** failReason)
+{
+ POVS_VPORT_ENTRY sourceVPort;
+ NDIS_NET_BUFFER_LIST_8021Q_INFO* vlanTagValue;
+ NET_BUFFER_LIST* nextNbl, *newNbl;
+ POVS_BUFFER_CONTEXT bufferContext;
+ LOCK_STATE_EX lockState;
+ BOOLEAN atDispatch;
+ NDIS_STATUS status = STATUS_SUCCESS;
- nextNbl = curNbl->Next;
- curNbl->Next = NULL;
+ PNDIS_SWITCH_FORWARDING_DETAIL_NET_BUFFER_LIST_INFO fwdDetail;
+ NDIS_SWITCH_PORT_ID hvSourcePort = NDIS_SWITCH_DEFAULT_PORT_ID;
+ NDIS_SWITCH_NIC_INDEX hvSourceIndex = NDIS_SWITCH_DEFAULT_NIC_INDEX;
+
+ atDispatch = (NDIS_TEST_SEND_AT_DISPATCH_LEVEL(sendFlags) ?
+ NDIS_RWL_AT_DISPATCH_LEVEL : 0);
+ fwdDetail = NET_BUFFER_LIST_SWITCH_FORWARDING_DETAIL(origNbl);
+ hvSourcePort = fwdDetail->SourcePortId;
+ hvSourceIndex = (NDIS_SWITCH_NIC_INDEX)fwdDetail->SourceNicIndex;
+
+ vlanTagValue = NET_BUFFER_LIST_INFO(origNbl, Ieee8021QNetBufferListInfo);
+
+ /* create buffer context */
+ bufferContext = OvsInitExternalNBLContext(switchContext, origNbl,
+ hvSourcePort == switchContext->externalPortId);
+ if (bufferContext == NULL) {
+ *failReason = L"Cannot allocate external NBL context.";
+ status = NDIS_STATUS_RESOURCES;
+ return status;
+ }
- /* Ethernet Header is a guaranteed safe access. */
- POVS_BUFFER_CONTEXT ctx;
- OvsFlow *flow;
- NET_BUFFER_LIST* pNewNbl;
-
- fwdDetail = NET_BUFFER_LIST_SWITCH_FORWARDING_DETAIL(curNbl);
- 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) {
- RtlInitUnicodeString(&filterReason,
- L"Cannot allocate external NBL context.");
+ /*
+ * split NBLs: each NBL must contain only one NB.
+ * NOTE: if origNbl has only one NB => newNbl will be origNbl
+ */
+ newNbl = OvsSplitNblByNB(origNbl, bufferContext, switchContext);
+ if (!newNbl) {
+ *failReason = L"Cannot allocate new NBL: partial copy NB to "
+ L"multiple NBLs.";
+ status = NDIS_STATUS_RESOURCES;
+ goto Cleanup;
+ }
- 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.
+ */
+ NdisAcquireRWLockRead(switchContext->dispatchLock, &lockState,
+ atDispatch);
+
+ sourceVPort = OvsFindVportByPortIdAndNicIndex(switchContext, hvSourcePort,
+ hvSourceIndex);
+ if (sourceVPort == NULL || sourceVPort->ovsState != OVS_STATE_CONNECTED) {
+ *failReason = L"OVS-Cannot forward packet from unknown source port";
+ status = NDIS_STATUS_INVALID_PORT;
+ goto Cleanup;
+ }
- vport = OvsFindVportByPortIdAndNicIndex(switchContext, sourcePort,
- sourceIndex);
- if (vport == NULL || vport->ovsState != OVS_STATE_CONNECTED) {
- RtlInitUnicodeString(&filterReason,
- L"OVS-Cannot forward packet from unknown source port");
- goto dropit;
- } else {
- portNo = vport->portNo;
- }
+ /* update vport stats */
+ OVS_NBL_FOR_EACH(nbl, newNbl) {
+ PNET_BUFFER firstNb = NET_BUFFER_LIST_FIRST_NB(nbl);
- curNb = NET_BUFFER_LIST_FIRST_NB(curNbl);
- vlanTagValue = NET_BUFFER_LIST_INFO(curNbl,
- Ieee8021QNetBufferListInfo);
-
- if (curNb->Next == NULL) {
- vport->stats.rxPackets++;
- vport->stats.rxBytes += NET_BUFFER_DATA_LENGTH(curNb);
-
- pNewNbl = curNbl;
- } else {
- for (NET_BUFFER* curNb = NET_BUFFER_LIST_FIRST_NB(curNbl);
- curNb != NULL; curNb = NET_BUFFER_NEXT_NB(curNb)) {
- vport->stats.rxPackets++;
- vport->stats.rxBytes += NET_BUFFER_DATA_LENGTH(curNb);
- }
-
- InterlockedDecrement((volatile LONG*)&ctx->refCount);
- pNewNbl = OvsPartialCopyToMultipleNBLs(switchContext, curNbl,
- 0, 0, TRUE);
- if (!pNewNbl) {
- RtlInitUnicodeString(&filterReason,
- L"Cannot allocate new NBL: partial copy NB to "
- L"multiple NBLs.");
-
- OvsStartNBLIngressError(switchContext, curNbl,
- sendCompleteFlags, &filterReason,
- NDIS_STATUS_RESOURCES);
-
- NdisReleaseRWLock(switchContext->dispatchLock, &lockState);
- continue;
- }
- }
+ sourceVPort->stats.rxPackets++;
+ sourceVPort->stats.rxBytes += NET_BUFFER_DATA_LENGTH(firstNb);
+ }
- OVS_NBL_FOR_EACH_NEXT(pCurNbl, pNewNbl, pNextNbl) {
- pNextNbl = NET_BUFFER_LIST_NEXT_NBL(pCurNbl);
- NET_BUFFER_LIST_NEXT_NBL(pCurNbl) = NULL;
-
- status = OvsExtractFlow(NET_BUFFER_LIST_FIRST_NB(pCurNbl),
- vport->portNo, &key, &layers,
- NULL, vlanTagValue);
- if (status != NDIS_STATUS_SUCCESS) {
- RtlInitUnicodeString(&filterReason, L"OVS-Flow extract failed");
- OvsAddPktCompletionList(&completionList, TRUE, sourcePort,
- pCurNbl, 0, &filterReason);
- continue;
- }
-
- ASSERT(KeGetCurrentIrql() == DISPATCH_LEVEL);
- OvsAcquireDatapathRead(datapath, &dpLockState, dispatch);
-
- flow = OvsLookupFlow(datapath, &key, &hash, FALSE);
- if (flow) {
- OvsFlowUsed(flow, NET_BUFFER_LIST_FIRST_NB(pCurNbl), &layers);
- datapath->hits++;
-
- /* If successful, OvsActionsExecute() consumes the NBL.
- * Otherwise, it adds it to the completionList. No need to
- * check the return value. */
- OvsActionsExecute(switchContext, &completionList, pCurNbl,
- portNo, SendFlags, &key, &hash, &layers,
- flow->actions, flow->actionsLen);
-
- OvsReleaseDatapath(datapath, &dpLockState);
- continue;
- } else {
- OvsReleaseDatapath(datapath, &dpLockState);
-
- datapath->misses++;
- status = OvsCreateAndAddPackets(OVS_DEFAULT_PACKET_QUEUE,
- NULL, 0, OVS_PACKET_CMD_MISS,
- portNo,
- key.tunKey.dst != 0 ?
- (OvsIPv4TunnelKey *)&key.tunKey :
- NULL, pCurNbl,
- sourcePort ==
- switchContext->externalPortId,
- &layers, switchContext,
- &missedPackets, &num);
- if (status == NDIS_STATUS_SUCCESS) {
- /* Complete the packet since it was copied to user
- * buffer.
- */
- RtlInitUnicodeString(&filterReason,
- L"OVS-Dropped since packet was copied to userspace");
- } else {
- RtlInitUnicodeString(&filterReason,
- L"OVS-Dropped due to failure to queue to userspace");
- }
- OvsAddPktCompletionList(&completionList, TRUE, sourcePort,
- pCurNbl, 0, &filterReason);
- continue;
- }
- }
+ OVS_NBL_FOR_EACH_NEXT(curNbl, newNbl, nextNbl) {
+ nextNbl = NET_BUFFER_LIST_NEXT_NBL(curNbl);
+ NET_BUFFER_LIST_NEXT_NBL(curNbl) = NULL;
- dropit:
- NdisReleaseRWLock(switchContext->dispatchLock, &lockState);
+ OvsProcessOneNb(curNbl, switchContext, sourceVPort, completionList,
+ sendFlags, missedPackets, countMissedPackets, vlanTagValue);
}
- /* Queue the missed packets. */
- OvsQueuePackets(OVS_DEFAULT_PACKET_QUEUE, &missedPackets, num);
- OvsFinalizeCompletionList(&completionList);
+Cleanup:
+ NdisReleaseRWLock(switchContext->dispatchLock, &lockState);
+
+ return status;
}
@@ -378,9 +390,17 @@ OvsExtSendNBL(NDIS_HANDLE filterModuleContext,
POVS_SWITCH_CONTEXT switchContext;
switchContext = (POVS_SWITCH_CONTEXT) filterModuleContext;
+ PNET_BUFFER_LIST nextNbl = NULL;
+ ULONG sendCompleteFlags;
+ LIST_ENTRY missedPackets;
+ UINT32 countMissedPackets = 0;
+ OvsCompletionList completionList;
+
+ sendCompleteFlags = OvsGetSendCompleteFlags(sendFlags);
+ sendFlags |= NDIS_SEND_FLAGS_SWITCH_DESTINATION_GROUP;
+
if (switchContext->dataFlowState == OvsSwitchPaused) {
NDIS_STRING filterReason;
- ULONG sendCompleteFlags = OvsGetSendCompleteFlags(sendFlags);
RtlInitUnicodeString(&filterReason,
L"Switch state PAUSED, drop on ingress.");
@@ -392,7 +412,32 @@ OvsExtSendNBL(NDIS_HANDLE filterModuleContext,
ASSERT(switchContext->dataFlowState == OvsSwitchRunning);
- OvsStartNBLIngress(switchContext, netBufferLists, sendFlags);
+ InitializeListHead(&missedPackets);
+ OvsInitCompletionList(&completionList, switchContext, sendCompleteFlags);
+
+ OVS_NBL_FOR_EACH_NEXT(curNbl, netBufferLists, nextNbl) {
+ const WCHAR* failReason = NULL;
+ NDIS_STATUS status;
+ NDIS_STRING filterReason;
+
+ nextNbl = curNbl->Next;
+ curNbl->Next = NULL;
+
+ status = OvsProcessOneNbl(curNbl, switchContext, sendFlags,
+ &completionList, &missedPackets, &countMissedPackets, &failReason);
+ if (NT_ERROR(status)) {
+ RtlInitUnicodeString(&filterReason, failReason);
+
+ OvsStartNBLIngressError(switchContext, curNbl,
+ sendCompleteFlags, &filterReason,
+ status);
+ continue;
+ }
+ }
+
+ /* Queue the missed packets. */
+ OvsQueuePackets(OVS_DEFAULT_PACKET_QUEUE, &missedPackets, countMissedPackets);
+ OvsFinalizeCompletionList(&completionList);
}
static VOID
--
1.8.3.msysgit.0
More information about the dev
mailing list