datapath-windows:adjust Offset when processing packet in POP_VLAN action

In one typical setup, on the Windows VM running OVS Windows Kernel, a Geneva
packet with 8021.q VLAN tag is received. Then it may do POP_VLAN action
processing in Actions.c, if the packet does not have Ieee8021QNetBufferListInfo
in the oob of the packet, it will be processed by function OvsPopVlanInPktBuf().
In the function it will go on remove VLAN header present in the nbl, but related
layers is never readjusted for the offset value at this moment. As a result, it
will cause function OvsValidateIPChecksum drop the packet.

Reported-at:https://github.com/openvswitch/ovs-issues/issues/225
Signed-off-by: wilsonpeng <pweisong@vmware.com>
Signed-off-by: Alin-Gabriel Serdean <aserdean@ovn.org>

1 files changed, 15 insertions, 3 deletions

diff --git a/datapath-windows/ovsext/Actions.c b/datapath-windows/ovsext/Actions.c
index e130c2f96..90ecb59f0 100644
--- a/datapath-windows/ovsext/Actions.c
+++ b/datapath-windows/ovsext/Actions.c
@@ -1112,9 +1112,9 @@ OvsPopFieldInPacketBuf(OvsForwardingContext *ovsFwdCtx,
      * should split the function and refactor. */
     if (!bufferData) {
         EthHdr *ethHdr = (EthHdr *)bufferStart;
-        /* If the frame is not VLAN make it a no op */
         if (ethHdr->Type != ETH_TYPE_802_1PQ_NBO) {
-            return NDIS_STATUS_SUCCESS;
+            OVS_LOG_ERROR("Invalid ethHdr type %u, nbl %p", ethHdr->Type, ovsFwdCtx->curNbl);
+            return NDIS_STATUS_INVALID_PACKET;
         }
     }
     RtlMoveMemory(bufferStart + shiftLength, bufferStart, shiftOffset);
@@ -1137,6 +1137,9 @@ OvsPopFieldInPacketBuf(OvsForwardingContext *ovsFwdCtx,
 static __inline NDIS_STATUS
 OvsPopVlanInPktBuf(OvsForwardingContext *ovsFwdCtx)
 {
+    NDIS_STATUS status;
+    OVS_PACKET_HDR_INFO* layers = &ovsFwdCtx->layers;
+
     /*
      * Declare a dummy vlanTag structure since we need to compute the size
      * of shiftLength. The NDIS one is a unionized structure.
@@ -1145,7 +1148,15 @@ OvsPopVlanInPktBuf(OvsForwardingContext *ovsFwdCtx)
     UINT32 shiftLength = sizeof(vlanTag.TagHeader);
     UINT32 shiftOffset = sizeof(DL_EUI48) + sizeof(DL_EUI48);
 
-    return OvsPopFieldInPacketBuf(ovsFwdCtx, shiftOffset, shiftLength, NULL);
+    status = OvsPopFieldInPacketBuf(ovsFwdCtx, shiftOffset, shiftLength,
+                                    NULL);
+
+    if (status == NDIS_STATUS_SUCCESS) {
+        layers->l3Offset -= (UINT16) shiftLength;
+        layers->l4Offset -= (UINT16) shiftLength;
+    }
+
+    return status;
 }
 
 
@@ -2100,6 +2111,7 @@ OvsDoExecuteActions(POVS_SWITCH_CONTEXT switchContext,
                  */
                 status = OvsPopVlanInPktBuf(&ovsFwdCtx);
                 if (status != NDIS_STATUS_SUCCESS) {
+                    OVS_LOG_ERROR("OVS-pop vlan action failed status = %lu", status);
                     dropReason = L"OVS-pop vlan action failed";
                     goto dropit;
                 }