[ovs-dev] [PATCH v3] Handle NBLs with multiple NBs

Alin Serdean aserdean at cloudbasesolutions.com
Thu Sep 25 03:50:11 UTC 2014


Tested-by: Alin Gabriel Serdean <aserdean at cloudbasesolutions.com>
Acked-by: Alin Gabriel Serdean <aserdean at cloudbasesolutions.com>

-----Mesaj original-----
De la: Samuel Ghinet 
Trimis: Thursday, September 25, 2014 2:44 AM
Către: dev at openvswitch.org
Cc: Alin Serdean; nithin at vmware.com; eliahue at vmware.com; Ankur Sharma; ssaurabh at vmware.com
Subiect: [PATCH v3] Handle NBLs with multiple NBs

All NET_BUFFERs of a NET_BUFFER_LIST must go through the pipeline: extract, find flow, execute. Previously, for NBLs sent from NDIS, we handled only cases where the NBL had a single NB. Handling NBLs with multiple NBs was not implemented.

OvsPartialCopyToMultipleNBLs is used to make each NET_BUFFER have its own NET_BUFFER_LIST.

I have also added a few ASSERTs where the NET_BUFFER_LIST is expected to have only one NET_BUFFER."

By fixing this bug, the function OvsStartNBLIngress was also refactored, because the function was too big and intricate, and adding the new code necessary for this bug fix made it bigger and more intricate, and also more difficult to review.

The refactor meant the following modifications:
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 of 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.
o) clearer variable names

This patch also fixes a bug in OvsPartialCopyToMultipleNBLs:
The OvsPartialCopyToMultipleNBLs function creates a new link of NBLs out of one
NBL: it creates one NBL for each NB in the original NBL. The result is returned in firstNbl, which is the first in the linked-list chain of created NBLs.

This function mistakenly linked the resulting NBLs, mixing the original NBL in the chain.

This patch also addresses this bug, by making the created NBLs be linked together correctly.

Tested:
 - vxlan
 - vlan (using patch ports, as specified in INSTALL.Windows)

 using:
 - ping
 - tcp
 - tcp LSO (tcp segmentation)

Reported-at: ovs/ovs-issues#2
Signed-off-by: Samuel Ghinet <sghinet at cloudbasesolutions.com>
---
 datapath-windows/ovsext/Actions.c    |  41 +++-
 datapath-windows/ovsext/BufferMgmt.c |  42 +++-  datapath-windows/ovsext/BufferMgmt.h |  18 ++
 datapath-windows/ovsext/PacketIO.c   | 359 ++++++++++++++++++++++-------------
 datapath-windows/ovsext/Tunnel.c     |   5 +-
 datapath-windows/ovsext/User.c       |  16 +-
 datapath-windows/ovsext/Vxlan.c      |   5 +
 7 files changed, 342 insertions(+), 144 deletions(-)

diff --git a/datapath-windows/ovsext/Actions.c b/datapath-windows/ovsext/Actions.c
index 35ebfdf..e6836b0 100644
--- a/datapath-windows/ovsext/Actions.c
+++ b/datapath-windows/ovsext/Actions.c
@@ -525,6 +525,7 @@ OvsDoFlowLookupOutput(OvsForwardingContext *ovsFwdCtx)
         return NDIS_STATUS_SUCCESS;
     }
     ASSERT(vport->nicState == NdisSwitchNicStateConnected);
+    ASSERT(ovsFwdCtx->curNbl->FirstNetBuffer->Next == NULL);
 
     /* Assert that in the Rx direction, key is always setup. */
     ASSERT(ovsFwdCtx->tunnelRxNic == NULL || ovsFwdCtx->tunKey.dst != 0); @@ -597,6 +598,9 @@ OvsTunnelPortTx(OvsForwardingContext *ovsFwdCtx)
     NDIS_STATUS status = NDIS_STATUS_FAILURE;
     PNET_BUFFER_LIST newNbl = NULL;
 
+    ASSERT(ovsFwdCtx->curNbl->Next == NULL);
+    ASSERT(ovsFwdCtx->curNbl->FirstNetBuffer->Next == NULL);
+
     /*
      * Setup the source port to be the internal port to as to facilitate the
      * second OvsLookupFlow.
@@ -631,11 +635,38 @@ OvsTunnelPortTx(OvsForwardingContext *ovsFwdCtx)
 
     if (status == NDIS_STATUS_SUCCESS) {
         ASSERT(newNbl);
+        PNET_BUFFER_LIST splitNbl, nextNbl;
+
         OvsCompleteNBLForwardingCtx(ovsFwdCtx,
-                                    L"Complete after cloning NBL for encapsulation");
-        ovsFwdCtx->curNbl = newNbl;
-        status = OvsDoFlowLookupOutput(ovsFwdCtx);
-        ASSERT(ovsFwdCtx->curNbl == NULL);
+                                    L"Complete after cloning NBL for "
+                                    L"encapsulation");
+
+        /*
+         * split NBLs: each NBL must contain only one NB.
+         * NOTE: if newNbl has only one NB => splitNbl will be newNbl
+         *
+         * XXX: we will one day need to optimize this - perhaps split the
+         * function OvsDoFlowLookupOutput in two, so that
+         * 1. we will extract flow on the first NB
+         * 2. we will split the NBL and perform execute actions on each NB
+        */
+        splitNbl = OvsSplitNblByNB(newNbl, ovsFwdCtx->switchContext, TRUE);
+        if (!splitNbl) {
+            return NDIS_STATUS_RESOURCES;
+        }
+
+        OVS_NBL_FOR_EACH_NEXT(curNbl, splitNbl, nextNbl) {
+            nextNbl = NET_BUFFER_LIST_NEXT_NBL(curNbl);
+            NET_BUFFER_LIST_NEXT_NBL(curNbl) = NULL;
+
+            ovsFwdCtx->curNbl = curNbl;
+            status = OvsDoFlowLookupOutput(ovsFwdCtx);
+            ASSERT(ovsFwdCtx->curNbl == NULL);
+
+            if (status != STATUS_SUCCESS) {
+                break;
+            }
+        }
     } else {
         /*
         * XXX: Temporary freeing of the packet until we register a @@ -1368,6 +1399,8 @@ OvsActionsExecute(POVS_SWITCH_CONTEXT switchContext,
     PNDIS_SWITCH_FORWARDING_DETAIL_NET_BUFFER_LIST_INFO fwdDetail =
         NET_BUFFER_LIST_SWITCH_FORWARDING_DETAIL(curNbl);
 
+    ASSERT(curNbl->FirstNetBuffer->Next == NULL);
+
     /* XXX: ASSERT that the flow table lock is held. */
     status = OvsInitForwardingCtx(&ovsFwdCtx, switchContext, curNbl, portNo,
                                   sendFlags, fwdDetail, completionList, diff --git a/datapath-windows/ovsext/BufferMgmt.c b/datapath-windows/ovsext/BufferMgmt.c
index e0377c1..f06045b 100644
--- a/datapath-windows/ovsext/BufferMgmt.c
+++ b/datapath-windows/ovsext/BufferMgmt.c
@@ -863,7 +863,7 @@ OvsPartialCopyToMultipleNBLs(PVOID ovsContext,
         if (prevNbl == NULL) {
             firstNbl = newNbl;
         } else {
-            NET_BUFFER_LIST_NEXT_NBL(prevNbl) = nbl;
+            NET_BUFFER_LIST_NEXT_NBL(prevNbl) = newNbl;
             NET_BUFFER_NEXT_NB(prevNb) = nb;
         }
         prevNbl = newNbl;
@@ -1533,3 +1533,43 @@ OvsGetCtxSourcePortNo(PNET_BUFFER_LIST nbl,
     *portNo = ctx->srcPortNo;
     return NDIS_STATUS_SUCCESS;
 }
+
+/*
+ * 
+-----------------------------------------------------------------------
+---
+ * OvsSplitNblByNB --
+ *      This function returns:
+ *      a) if the nblList contains only one NB: the original NBL
+ *      b) if the nblList contains multiple NBs: a clone  - partial copy where
+ *         each NB is wrapped in its own NBL.
+ *      c) on failure: NULL
+ *
+ *      This function also Completes the original NBL, if a partial copy was
+ *      created and completeOrigNbl is TRUE.
+ * 
+-----------------------------------------------------------------------
+---
+*/
+PNET_BUFFER_LIST
+OvsSplitNblByNB(PNET_BUFFER_LIST nblList,
+                POVS_SWITCH_CONTEXT switchContext,
+                BOOLEAN completeOrigNbl) {
+    PNET_BUFFER curNb;
+    PNET_BUFFER_LIST newNblList;
+
+    curNb = NET_BUFFER_LIST_FIRST_NB(nblList);
+
+    if (curNb->Next == NULL) {
+        return nblList;
+    }
+
+    newNblList = OvsPartialCopyToMultipleNBLs(switchContext, nblList, 0, 0,
+                                              TRUE);
+    if (!newNblList) {
+        return NULL;
+    }
+
+    if (completeOrigNbl) {
+        OvsCompleteNBL(switchContext, nblList, TRUE);
+    }
+
+    return newNblList;
+}
\ No newline at end of file
diff --git a/datapath-windows/ovsext/BufferMgmt.h b/datapath-windows/ovsext/BufferMgmt.h
index 915d7f5..586d4ac 100644
--- a/datapath-windows/ovsext/BufferMgmt.h
+++ b/datapath-windows/ovsext/BufferMgmt.h
@@ -17,6 +17,10 @@
 #ifndef __BUFFER_MGMT_H_
 #define __BUFFER_MGMT_H_ 1
 
+#include "precomp.h"
+
+typedef struct _OVS_SWITCH_CONTEXT *POVS_SWITCH_CONTEXT;
+
 #define MEM_ALIGN                       MEMORY_ALLOCATION_ALIGNMENT
 #define MEM_ALIGN_SIZE(_x)  ((MEM_ALIGN - 1 + (_x))/MEM_ALIGN * MEM_ALIGN)
 #define OVS_CTX_MAGIC                   0xabcd
@@ -81,6 +85,16 @@ typedef struct _OVS_NBL_POOL {  #endif  } OVS_NBL_POOL, *POVS_NBL_POOL;
 
+#define OVS_NBL_FOR_EACH(_curNbl, _nblList)       \
+for (PNET_BUFFER_LIST _curNbl = _nblList;         \
+     _curNbl != NULL;                             \
+     _curNbl = NET_BUFFER_LIST_NEXT_NBL(_curNbl))
+
+#define OVS_NBL_FOR_EACH_NEXT(_curNbl, _nblList, _nextNbl)   \
+for (PNET_BUFFER_LIST _curNbl = _nblList;                    \
+     _curNbl != NULL;                                        \
+     _curNbl = _nextNbl)
+
 
 NDIS_STATUS OvsInitBufferPool(PVOID context);  VOID OvsCleanupBufferPool(PVOID context); @@ -121,4 +135,8 @@ NDIS_STATUS OvsSetCtxSourcePortNo(PNET_BUFFER_LIST nbl, UINT32 portNo);
 
 NDIS_STATUS OvsGetCtxSourcePortNo(PNET_BUFFER_LIST nbl, UINT32 *portNo);
 
+PNET_BUFFER_LIST OvsSplitNblByNB(PNET_BUFFER_LIST nblList,
+                                 POVS_SWITCH_CONTEXT switchContext,
+                                 BOOLEAN completeOrigNbl);
+
 #endif /* __BUFFER_MGMT_H_ */
diff --git a/datapath-windows/ovsext/PacketIO.c b/datapath-windows/ovsext/PacketIO.c
index ac7862d..a6b90ef 100644
--- a/datapath-windows/ovsext/PacketIO.c
+++ b/datapath-windows/ovsext/PacketIO.c
@@ -28,6 +28,7 @@
 #include "Flow.h"
 #include "Event.h"
 #include "User.h"
+#include "BufferMgmt.h"
 
 /* Due to an imported header file */
 #pragma warning( disable:4505 )
@@ -176,149 +177,213 @@ OvsStartNBLIngressError(POVS_SWITCH_CONTEXT switchContext,
                                     sendCompleteFlags);  }
 
+/*
+ * 
+-----------------------------------------------------------------------
+---
+ *  Processes one NB, which means:
+ *  o) Extract Packet Info (i.e. flow key)
+ *  o) Find flow matching packet info
+ *  o) If a matching flow was found => execute flow's actions
+ *  o) If a matching flow was not found => queue packet to userspace
+ * 
+-----------------------------------------------------------------------
+---
+*/
 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,
+                PLIST_ENTRY missedPackets,
+                PUINT32 countMissedPackets)
 {
-    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;
+    POVS_DATAPATH datapath = &(switchContext->datapath);
 
-    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(NET_BUFFER_LIST_NEXT_NBL(curNbl) == NULL);
+    ASSERT(curNbl->FirstNetBuffer->Next == NULL);
 
-    InitializeListHead(&missedPackets);
-    OvsInitCompletionList(&completionList, switchContext, sendCompleteFlags);
+    /* 1. Extract flow key*/
+    status = OvsExtractFlow(curNbl, sourceVPort->portNo, &key, &layers, 
+ NULL);
 
-    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;
+    if (status != NDIS_STATUS_SUCCESS) {
+        RtlInitUnicodeString(&filterReason, L"OVS-Flow extract failed");
+        OvsAddPktCompletionList(completionList, TRUE, sourceVPort->portId,
+                                curNbl, 0, &filterReason);
+        return;
+    }
 
-        nextNbl = curNbl->Next;
-        curNbl->Next = NULL;
+    /*
+     * We must be at dispatch level, because the caller locked
+     * switchContext->dispatchLock.
+     */
+    ASSERT(KeGetCurrentIrql() == DISPATCH_LEVEL);
+    OvsAcquireDatapathRead(datapath, &dpLockState, TRUE);
+
+    /* 2. Find flow matching key */
+    flow = OvsLookupFlow(datapath, &key, &hash, FALSE);
+    if (flow) {
+        OvsFlowUsed(flow, 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;
+    }
 
-        /* Ethernet Header is a guaranteed safe access. */
-        curNb = NET_BUFFER_LIST_FIRST_NB(curNbl);
-        if (curNb->Next != NULL) {
-            /* XXX: This case is not handled yet. */
-            ASSERT(FALSE);
-        } else {
-            POVS_BUFFER_CONTEXT ctx;
-            OvsFlow *flow;
-
-            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.");
-
-                OvsStartNBLIngressError(switchContext, curNbl,
-                                        sendCompleteFlags, &filterReason,
-                                        NDIS_STATUS_RESOURCES);
-                NdisReleaseRWLock(switchContext->dispatchLock, &lockState);
-                continue;
-            }
-
-            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;
-            }
-
-            vport->stats.rxPackets++;
-            vport->stats.rxBytes += NET_BUFFER_DATA_LENGTH(curNb);
-
-            status = OvsExtractFlow(curNbl, vport->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, dispatch);
-
-            flow = OvsLookupFlow(datapath, &key, &hash, FALSE);
-            if (flow) {
-                OvsFlowUsed(flow, curNbl, &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, curNbl,
-                                portNo, SendFlags, &key, &hash, &layers,
-                                flow->actions, flow->actionsLen);
-                OvsReleaseDatapath(datapath, &dpLockState);
-                NdisReleaseRWLock(switchContext->dispatchLock, &lockState);
-                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, curNbl,
-                                                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");
-                }
-                goto dropit;
-            }
-
-dropit:
-            OvsAddPktCompletionList(&completionList, TRUE, sourcePort, curNbl, 0,
-                                    &filterReason);
-            NdisReleaseRWLock(switchContext->dispatchLock, &lockState);
-        }
+    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 "
+                             L"userspace");
+    } else {
+        RtlInitUnicodeString(&filterReason,
+                             L"OVS-Dropped due to failure to queue to "
+                             L"userspace");
     }
 
-    /* Queue the missed packets. */
-    OvsQueuePackets(OVS_DEFAULT_PACKET_QUEUE, &missedPackets, num);
-    OvsFinalizeCompletionList(&completionList);
+    OvsAddPktCompletionList(completionList, TRUE, sourceVPort->portId, curNbl,
+                            0, &filterReason); }
+
+/*
+ * 
+-----------------------------------------------------------------------
+---
+ *  Processes one NBL, which may contain one or more NBs.
+ *  Processing means: if the NBL contains multiple NBs, the NBL is 
+cloned to
+ *  multiple NBLs (i.e. split), and each NB (wrapped in a new NBL) is 
+being
+ *  processed separately. If the original NBL contains only one NB, 
+then only
+ *  one NB is processed.
+ * 
+-----------------------------------------------------------------------
+---
+*/
+static VOID
+OvsProcessOneNbl(PNET_BUFFER_LIST origNbl,
+                 POVS_SWITCH_CONTEXT switchContext,
+                 ULONG sendFlags,
+                 OvsCompletionList *completionList,
+                 PLIST_ENTRY missedPackets,
+                 PUINT32 countMissedPackets) {
+    POVS_VPORT_ENTRY sourceVPort;
+    PNET_BUFFER_LIST nextNbl, newNbl;
+    POVS_BUFFER_CONTEXT bufferContext;
+    LOCK_STATE_EX lockState;
+    BOOLEAN atDispatch;
+    NDIS_STATUS status = STATUS_SUCCESS;
+    NDIS_STRING filterReason;
+    ULONG completeFlags = OvsGetSendCompleteFlags(sendFlags);
+
+    PNDIS_SWITCH_FORWARDING_DETAIL_NET_BUFFER_LIST_INFO fwdDetail;
+    NDIS_SWITCH_PORT_ID hvSourcePortId;
+    NDIS_SWITCH_NIC_INDEX hvSourceIndex;
+
+    atDispatch = (NDIS_TEST_SEND_AT_DISPATCH_LEVEL(sendFlags) ?
+                  NDIS_RWL_AT_DISPATCH_LEVEL : 0);
+    fwdDetail = NET_BUFFER_LIST_SWITCH_FORWARDING_DETAIL(origNbl);
+    hvSourcePortId = fwdDetail->SourcePortId;
+    hvSourceIndex = (NDIS_SWITCH_NIC_INDEX)fwdDetail->SourceNicIndex;
+
+    /* create buffer context */
+    bufferContext = OvsInitExternalNBLContext(switchContext, origNbl,
+        hvSourcePortId == switchContext->externalPortId);
+    if (bufferContext == NULL) {
+        RtlInitUnicodeString(&filterReason,
+                             L"Cannot allocate external NBL context.");
+        status = NDIS_STATUS_RESOURCES;
+
+        OvsStartNBLIngressError(switchContext, origNbl, completeFlags,
+                                &filterReason,
+                                status);
+        return;
+    }
+
+    /*
+     * split NBLs: each NBL must contain only one NB.
+     * NOTE: if origNbl has only one NB => newNbl will be origNbl
+    */
+    newNbl = OvsSplitNblByNB(origNbl, switchContext, TRUE);
+    if (!newNbl) {
+        RtlInitUnicodeString(&filterReason, L"Cannot allocate new NBL: partial"
+                             L"copy NB to multiple NBLs.");
+        status = NDIS_STATUS_RESOURCES;
+        newNbl = origNbl;
+        goto Cleanup;
+    }
+
+    /*
+     * Take the DispatchLock so none of the VPORTs disconnect while
+     * we are setting hyper-v switch ports as destinations to the NBL
+     *
+     * XXX: we may be waiting very long here (given we're at DPC level) if
+     * there is a lot of network traffic and one of many VMs connects or
+     * disconnects.
+    */
+    NdisAcquireRWLockRead(switchContext->dispatchLock, &lockState, 
+ atDispatch);
+
+    sourceVPort = OvsFindVportByPortIdAndNicIndex(switchContext, hvSourcePortId,
+                                                  hvSourceIndex);
+    if (sourceVPort == NULL || sourceVPort->ovsState != OVS_STATE_CONNECTED) {
+        NdisReleaseRWLock(switchContext->dispatchLock, &lockState);
+
+        RtlInitUnicodeString(&filterReason, L"OVS-Cannot forward packet from "
+                             L"unknown source port");
+        status = NDIS_STATUS_INVALID_PORT;
+        goto Cleanup;
+    }
+
+    /* process each NB: each NB is wrapped in an NBL in the newNbl chain */
+    OVS_NBL_FOR_EACH_NEXT(curNbl, newNbl, nextNbl) {
+        nextNbl = NET_BUFFER_LIST_NEXT_NBL(curNbl);
+        NET_BUFFER_LIST_NEXT_NBL(curNbl) = NULL;
+
+        PNET_BUFFER firstNb = NET_BUFFER_LIST_FIRST_NB(curNbl);
+
+        /* update vport stats */
+        sourceVPort->stats.rxPackets++;
+        sourceVPort->stats.rxBytes += NET_BUFFER_DATA_LENGTH(firstNb);
+
+        /* process NB */
+        OvsProcessOneNb(curNbl, switchContext, sourceVPort, completionList,
+                        sendFlags, missedPackets, countMissedPackets);
+    }
+
+    NdisReleaseRWLock(switchContext->dispatchLock, &lockState);
+
+Cleanup:
+    if (NT_ERROR(status)) {
+        OVS_NBL_FOR_EACH_NEXT(curNbl, newNbl, nextNbl) {
+            nextNbl = NET_BUFFER_LIST_NEXT_NBL(curNbl);
+            NET_BUFFER_LIST_NEXT_NBL(curNbl) = NULL;
+
+            OvsAddPktCompletionList(completionList, TRUE, hvSourcePortId,
+                                    curNbl, 0, &filterReason);
+        }
+    }
 }
 
 
@@ -340,9 +405,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."); @@ -354,7 +427,22 @@ 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) {
+        nextNbl = curNbl->Next;
+        curNbl->Next = NULL;
+
+        OvsProcessOneNbl(curNbl, switchContext, sendFlags, &completionList,
+                         &missedPackets, &countMissedPackets);
+    }
+
+    /* Queue the missed packets. */
+    OvsQueuePackets(OVS_DEFAULT_PACKET_QUEUE, &missedPackets,
+                    countMissedPackets);
+    /* Complete all packets that were added to the completion list */
+    OvsFinalizeCompletionList(&completionList);
 }
 
 static VOID
@@ -382,7 +470,8 @@ OvsCompleteNBLIngress(POVS_SWITCH_CONTEXT switchContext,
 
     /* Complete the NBL's that were sent by the upper layer. */
     if (newList.dropNbl != NULL) {
-        NdisFSendNetBufferListsComplete(switchContext->NdisFilterHandle, newList.dropNbl,
+        NdisFSendNetBufferListsComplete(switchContext->NdisFilterHandle,
+                                        newList.dropNbl,
                                         sendCompleteFlags);
     }
 }
diff --git a/datapath-windows/ovsext/Tunnel.c b/datapath-windows/ovsext/Tunnel.c
index 2e7da10..d52d788 100644
--- a/datapath-windows/ovsext/Tunnel.c
+++ b/datapath-windows/ovsext/Tunnel.c
@@ -227,6 +227,7 @@ OvsInjectPacketThroughActions(PNET_BUFFER_LIST pNbl,
     OVS_DATAPATH *datapath = &gOvsSwitchContext->datapath;
 
     ASSERT(gOvsSwitchContext);
+    ASSERT(pNbl->FirstNetBuffer->Next == NULL);
 
     /* Fill the tunnel key */
     status = OvsSlowPathDecapVxlan(pNbl, &tunnelKey); @@ -306,8 +307,8 @@ OvsInjectPacketThroughActions(PNET_BUFFER_LIST pNbl,
             datapath->hits++;
 
             OvsActionsExecute(gOvsSwitchContext, &completionList, pNbl,
-                            portNo, SendFlags, &key, &hash, &layers,
-                            flow->actions, flow->actionsLen);
+                              portNo, SendFlags, &key, &hash, &layers,
+                              flow->actions, flow->actionsLen);
 
             OvsReleaseDatapath(datapath, &dpLockState);
         } else {
diff --git a/datapath-windows/ovsext/User.c b/datapath-windows/ovsext/User.c index 612a4bd..2a015b8 100644
--- a/datapath-windows/ovsext/User.c
+++ b/datapath-windows/ovsext/User.c
@@ -360,10 +360,15 @@ OvsExecuteDpIoctl(PVOID inputBuffer,
         fwdDetail->SourcePortId = NDIS_SWITCH_DEFAULT_PORT_ID;
         fwdDetail->SourceNicIndex = 0;
     }
-    // XXX: Figure out if any of the other members of fwdDetail need to be set.
+    /*
+     * XXX: Figure out if any of the other members of fwdDetail need to be
+     * set.
+     */
+
+    ASSERT(pNbl->FirstNetBuffer->Next == NULL);
 
     ndisStatus = OvsExtractFlow(pNbl, fwdDetail->SourcePortId, &key, &layers,
-                              NULL);
+                                NULL);
     if (ndisStatus == NDIS_STATUS_SUCCESS) {
         ASSERT(KeGetCurrentIrql() == DISPATCH_LEVEL);
         NdisAcquireRWLockRead(gOvsSwitchContext->dispatchLock, &lockState, @@ -763,6 +768,11 @@ OvsQueuePackets(UINT32 queueId,
 
     OVS_LOG_LOUD("Enter: queueId %u, numELems: %u",
                   queueId, numElems);
+
+    if (numElems == 0) {
+        return;
+    }
+
     if (queue == NULL) {
         goto cleanup;
     }
@@ -828,6 +838,8 @@ OvsCreateAndAddPackets(UINT32 queueId,
     PNET_BUFFER_LIST newNbl = NULL;
     PNET_BUFFER nb;
 
+    ASSERT(nbl->FirstNetBuffer->Next == NULL);
+
     if (hdrInfo->isTcp) {
         NDIS_TCP_LARGE_SEND_OFFLOAD_NET_BUFFER_LIST_INFO tsoInfo;
         UINT32 packetLength;
diff --git a/datapath-windows/ovsext/Vxlan.c b/datapath-windows/ovsext/Vxlan.c index 3a1291c..4cdbfe4 100644
--- a/datapath-windows/ovsext/Vxlan.c
+++ b/datapath-windows/ovsext/Vxlan.c
@@ -115,6 +115,8 @@ OvsDoEncapVxlan(PNET_BUFFER_LIST curNbl,
     UINT32 headRoom = OvsGetVxlanTunHdrSize();
     UINT32 packetLength;
 
+    ASSERT(curNbl->FirstNetBuffer->Next == NULL);
+
     /*
      * XXX: the assumption currently is that the NBL is owned by OVS, and
      * headroom has already been allocated as part of allocating the NBL and @@ -286,6 +288,8 @@ OvsIpHlprCbVxlan(PNET_BUFFER_LIST curNbl,
     NDIS_STATUS status;
     UNREFERENCED_PARAMETER(inPort);
 
+    ASSERT(curNbl->FirstNetBuffer->Next == NULL);
+
     status = OvsExtractFlow(curNbl, inPort, &key, &layers, NULL);
     if (result == STATUS_SUCCESS) {
         status = OvsDoEncapVxlan(curNbl, tunKey, fwdInfo, &layers, @@ -456,6 +460,7 @@ OvsSlowPathDecapVxlan(const PNET_BUFFER_LIST packet,
     OVS_PACKET_HDR_INFO layers;
 
     layers.value = 0;
+    ASSERT(packet->FirstNetBuffer->Next == NULL);
 
     do {
         nh = OvsGetIp(packet, layers.l3Offset, &ip_storage);
--
1.8.3.msysgit.0




More information about the dev mailing list