datapath-windows: Account for VLAN tag in tunnel Decap

Decap functions for tunneling protocols do not compute
the packet header offsets correctly when there is a VLAN
tag in the L2 header. This results in incorrect checksum
computation causing the packet to be dropped.

This patch adds support to account for the VLAN tag in the
packet if its present, and makes use of the OvsExtractLayers()
function to correctly compute the header offsets for different
layers.

Testing done:
- Tested Geneve, STT, Vxlan and Gre and verified that there
  are no regressions.
- Verified that packets with VLAN tags are correctly handled
  in the decap code of all tunneling protocols. Previously,
  this would result in packet drops due to invalid checksums
  being computed.
- Verified that non-VLAN tagged packets are handled correctly.

Signed-off-by: Shashank Ram <rams@vmware.com>
Acked-by: Anand Kumar <kumaranand@vmware.com>
Signed-off-by: Alin Gabriel Serdean <aserdean@ovn.org>

diff --git a/datapath-windows/ovsext/Geneve.c b/datapath-windows/ovsext/Geneve.c
index 6dca69bc73dba2df8571f4197116ce803cab7259..210716d5aba33676e8221f957285cc8ca8a463f1 100644 (file)
--- a/datapath-windows/ovsext/Geneve.c
+++ b/datapath-windows/ovsext/Geneve.c
@@ -262,10 +262,16 @@ NDIS_STATUS OvsDecapGeneve(POVS_SWITCH_CONTEXT switchContext,
     PUINT8 bufferStart;
     PVOID optStart;
     NDIS_STATUS status;
+    OVS_PACKET_HDR_INFO layers = { 0 };
+
+    status = OvsExtractLayers(curNbl, &layers);
+    if (status != NDIS_STATUS_SUCCESS) {
+        return status;
+    }
 
     /* Check the length of the UDP payload */
     curNb = NET_BUFFER_LIST_FIRST_NB(curNbl);
-    tunnelSize = OvsGetGeneveTunHdrMinSize();
+    tunnelSize = OvsGetGeneveTunHdrSizeFromLayers(&layers);
     packetLength = NET_BUFFER_DATA_LENGTH(curNb);
     if (packetLength <= tunnelSize) {
         return NDIS_STATUS_INVALID_LENGTH;
@@ -295,13 +301,13 @@ NDIS_STATUS OvsDecapGeneve(POVS_SWITCH_CONTEXT switchContext,
 
     ethHdr = (EthHdr *)bufferStart;
     /* XXX: Handle IP options. */
-    ipHdr = (IPHdr *)((PCHAR)ethHdr + sizeof *ethHdr);
+    ipHdr = (IPHdr *)(bufferStart + layers.l3Offset);
     tunKey->src = ipHdr->saddr;
     tunKey->dst = ipHdr->daddr;
     tunKey->tos = ipHdr->tos;
     tunKey->ttl = ipHdr->ttl;
     tunKey->pad = 0;
-    udpHdr = (UDPHdr *)((PCHAR)ipHdr + sizeof *ipHdr);
+    udpHdr = (UDPHdr *)(bufferStart + layers.l4Offset);
 
     /* Validate if NIC has indicated checksum failure. */
     status = OvsValidateUDPChecksum(curNbl, udpHdr->check == 0);
@@ -312,7 +318,7 @@ NDIS_STATUS OvsDecapGeneve(POVS_SWITCH_CONTEXT switchContext,
     /* Calculate and verify UDP checksum if NIC didn't do it. */
     if (udpHdr->check != 0) {
         status = OvsCalculateUDPChecksum(curNbl, curNb, ipHdr, udpHdr,
-                                         packetLength);
+                                         packetLength, &layers);
         tunKey->flags |= OVS_TNL_F_CSUM;
         if (status != NDIS_STATUS_SUCCESS) {
             goto dropNbl;

diff --git a/datapath-windows/ovsext/Geneve.h b/datapath-windows/ovsext/Geneve.h
index 019c0dd9f2e7eafe595582acac04cfb277bd5551..db758dd070093109b3a3753da92964f6ea50183c 100644 (file)
--- a/datapath-windows/ovsext/Geneve.h
+++ b/datapath-windows/ovsext/Geneve.h
@@ -113,6 +113,12 @@ OvsGetGeneveTunHdrMaxSize(VOID)
     return OvsGetGeneveTunHdrMinSize() + TUN_OPT_MAX_LEN;
 }
 
+static __inline UINT32
+OvsGetGeneveTunHdrSizeFromLayers(POVS_PACKET_HDR_INFO layers)
+{
+    return layers->l7Offset + sizeof(GeneveHdr);
+}
+
 #define GENEVE_UDP_PORT 6081
 #define GENEVE_UDP_PORT_NBO 0xC117
 #define GENEVE_VER 0

diff --git a/datapath-windows/ovsext/Gre.c b/datapath-windows/ovsext/Gre.c
index f095742a49a8dfee182b4885210c38ad754e9a63..1f38ee794846c4a93b26ab0b40e66267c687660c 100644 (file)
--- a/datapath-windows/ovsext/Gre.c
+++ b/datapath-windows/ovsext/Gre.c
@@ -317,35 +317,42 @@ OvsDecapGre(POVS_SWITCH_CONTEXT switchContext,
     GREHdr *greHdr;
     UINT32 tunnelSize, packetLength;
     UINT32 headRoom = 0;
+    UINT32 maxGreLen;
     PUINT8 bufferStart;
     NDIS_STATUS status = NDIS_STATUS_SUCCESS;
     PCHAR tempBuf = NULL;
+    OVS_PACKET_HDR_INFO layers;
 
     ASSERT(*newNbl == NULL);
 
     *newNbl = NULL;
+    status = OvsExtractLayers(curNbl, &layers);
+    if (status != NDIS_STATUS_SUCCESS) {
+        return status;
+    }
 
     curNb = NET_BUFFER_LIST_FIRST_NB(curNbl);
     packetLength = NET_BUFFER_DATA_LENGTH(curNb);
     curMdl = NET_BUFFER_CURRENT_MDL(curNb);
-    tunnelSize = GreTunHdrSize(0);
+    tunnelSize = GreTunHdrSizeFromLayers(0, &layers);
     if (packetLength <= tunnelSize) {
         return NDIS_STATUS_INVALID_LENGTH;
     }
 
+    maxGreLen = GreMaxLengthFromLayers(&layers);
     /* Get a contiguous buffer for the maximum length of a GRE header */
-    bufferStart = NdisGetDataBuffer(curNb, OVS_MAX_GRE_LGTH, NULL, 1, 0);
+    bufferStart = NdisGetDataBuffer(curNb, maxGreLen, NULL, 1, 0);
     if (!bufferStart) {
         /* Documentation is unclear on where the packet can be fragmented.
          * For the moment allocate the buffer needed to get the maximum length
          * of a GRE header contiguous */
-        tempBuf = OvsAllocateMemoryWithTag(OVS_MAX_GRE_LGTH, OVS_GRE_POOL_TAG);
+        tempBuf = OvsAllocateMemoryWithTag(maxGreLen, OVS_GRE_POOL_TAG);
         if (!tempBuf) {
             status = NDIS_STATUS_RESOURCES;
             goto end;
         }
-        RtlZeroMemory(tempBuf, OVS_MAX_GRE_LGTH);
-        bufferStart = NdisGetDataBuffer(curNb, OVS_MAX_GRE_LGTH, tempBuf,
+        RtlZeroMemory(tempBuf, maxGreLen);
+        bufferStart = NdisGetDataBuffer(curNb, maxGreLen, tempBuf,
                                         1, 0);
         if (!bufferStart) {
             status = NDIS_STATUS_RESOURCES;
@@ -354,9 +361,9 @@ OvsDecapGre(POVS_SWITCH_CONTEXT switchContext,
     }
 
     ethHdr = (EthHdr *)bufferStart;
-    headRoom += sizeof *ethHdr;
+    headRoom += layers.l3Offset;
 
-    ipHdr = (IPHdr *)((PCHAR)ethHdr + sizeof *ethHdr);
+    ipHdr = (IPHdr *)(bufferStart + layers.l3Offset);
     tunKey->src = ipHdr->saddr;
     tunKey->dst = ipHdr->daddr;
     tunKey->tos = ipHdr->tos;
@@ -364,10 +371,10 @@ OvsDecapGre(POVS_SWITCH_CONTEXT switchContext,
     tunKey->pad = 0;
     headRoom += sizeof *ipHdr;
 
-    greHdr = (GREHdr *)((PCHAR)ipHdr + sizeof *ipHdr);
+    greHdr = (GREHdr *)(bufferStart + layers.l4Offset);
     headRoom += sizeof *greHdr;
 
-    tunnelSize = GreTunHdrSize(greHdr->flags);
+    tunnelSize = GreTunHdrSizeFromLayers(greHdr->flags, &layers);
 
     /* Verify the packet length after looking at the GRE flags*/
     if (packetLength <= tunnelSize) {
@@ -390,8 +397,8 @@ OvsDecapGre(POVS_SWITCH_CONTEXT switchContext,
         UINT16 chksum =
             CalculateChecksumNB(curNb,
                                 (UINT16)(NET_BUFFER_DATA_LENGTH(curNb) -
-                                (ipHdr->ihl * 4 + sizeof *ethHdr)),
-                                ipHdr->ihl * 4 + sizeof *ethHdr);
+                                layers.l4Offset),
+                                layers.l4Offset);
         if (prevChksum != chksum) {
             status = STATUS_NDIS_INVALID_PACKET;
             goto end;

diff --git a/datapath-windows/ovsext/Gre.h b/datapath-windows/ovsext/Gre.h
index c45df8fedfc6ba8edb55b73852c28eddcb23e484..45f3f59f3a768c0b1566105a38df591e67f5e3ad 100644 (file)
--- a/datapath-windows/ovsext/Gre.h
+++ b/datapath-windows/ovsext/Gre.h
@@ -103,4 +103,20 @@ GreTunHdrSize(UINT16 flags)
     return sum;
 }
 
+static __inline UINT32
+GreTunHdrSizeFromLayers(UINT16 flags, POVS_PACKET_HDR_INFO layers)
+{
+    UINT32 sum = layers->l4Offset + sizeof(GREHdr);
+    sum += (flags & GRE_CSUM) ? 4 : 0;
+    sum += (flags & GRE_KEY) ? 4 : 0;
+
+    return sum;
+}
+
+static __inline UINT32
+GreMaxLengthFromLayers(POVS_PACKET_HDR_INFO layers)
+{
+    return (layers->l4Offset + sizeof(GREHdr) + 12);
+}
+
 #endif /*__GRE_H_ */

diff --git a/datapath-windows/ovsext/Offload.c b/datapath-windows/ovsext/Offload.c
index 0905c8057bb2a8faae7c26508661791d78039b75..7b75e0afc8715b4746798a1dea3078ab630f514d 100644 (file)
--- a/datapath-windows/ovsext/Offload.c
+++ b/datapath-windows/ovsext/Offload.c
@@ -612,7 +612,8 @@ OvsCalculateUDPChecksum(PNET_BUFFER_LIST curNbl,
                         PNET_BUFFER curNb,
                         IPHdr *ipHdr,
                         UDPHdr *udpHdr,
-                        UINT32 packetLength)
+                        UINT32 packetLength,
+                        POVS_PACKET_HDR_INFO layers)
 {
     NDIS_TCP_IP_CHECKSUM_NET_BUFFER_LIST_INFO csumInfo;
     UINT16 checkSum;
@@ -625,16 +626,17 @@ OvsCalculateUDPChecksum(PNET_BUFFER_LIST curNbl,
 
         checkSum = udpHdr->check;
 
-        l4Payload = packetLength - sizeof(EthHdr) - ipHdr->ihl * 4;
+        l4Payload = packetLength - layers->l4Offset;
         udpHdr->check = 0;
         udpHdr->check =
             IPPseudoChecksum((UINT32 *)&ipHdr->saddr,
                              (UINT32 *)&ipHdr->daddr,
                              IPPROTO_UDP, (UINT16)l4Payload);
         udpHdr->check = CalculateChecksumNB(curNb, (UINT16)l4Payload,
-                                            sizeof(EthHdr) + ipHdr->ihl * 4);
+                                            layers->l4Offset);
         if (checkSum != udpHdr->check) {
-            OVS_LOG_TRACE("UDP checksum incorrect.");
+            OVS_LOG_ERROR("UDP checksum incorrect, expected %u, got %u",
+                          udpHdr->check, checkSum);
             return NDIS_STATUS_INVALID_PACKET;
         }
     }

diff --git a/datapath-windows/ovsext/Offload.h b/datapath-windows/ovsext/Offload.h
index d3731b171619dc777e3e24e192b0b134fffcc0a9..b2a0b6faa6c730a34048dba6932fbaed95c69aa6 100644 (file)
--- a/datapath-windows/ovsext/Offload.h
+++ b/datapath-windows/ovsext/Offload.h
@@ -39,7 +39,8 @@ NDIS_STATUS OvsCalculateUDPChecksum(PNET_BUFFER_LIST curNbl,
                                     PNET_BUFFER curNb,
                                     IPHdr *ipHdr,
                                     UDPHdr *udpHdr,
-                                    UINT32 packetLength);
+                                    UINT32 packetLength,
+                                    POVS_PACKET_HDR_INFO layers);
 
 ULONG OVSGetTcpMSS(PNET_BUFFER_LIST nbl);
 

diff --git a/datapath-windows/ovsext/Stt.c b/datapath-windows/ovsext/Stt.c
index f98070f33e0694ef0c463c01f78201d3685a9b17..8eb71676a40fe62382f648b49a7f40a0616819f1 100644 (file)
--- a/datapath-windows/ovsext/Stt.c
+++ b/datapath-windows/ovsext/Stt.c
@@ -415,9 +415,23 @@ ret_error:
  *----------------------------------------------------------------------------
  */
 static __inline NDIS_STATUS
-OvsValidateTCPChecksum(PNET_BUFFER_LIST curNbl, PNET_BUFFER curNb)
+OvsValidateTCPChecksum(PNET_BUFFER_LIST curNbl,
+                       PNET_BUFFER curNb,
+                       POVS_PACKET_HDR_INFO layers)
 {
+    PUINT8 buf;
+    PMDL curMdl;
     NDIS_TCP_IP_CHECKSUM_NET_BUFFER_LIST_INFO csumInfo;
+    NDIS_STATUS status;
+
+    curMdl = NET_BUFFER_CURRENT_MDL(curNb);
+    buf = (PUINT8)MmGetSystemAddressForMdlSafe(curMdl, LowPagePriority)
+        + NET_BUFFER_CURRENT_MDL_OFFSET(curNb);
+    if (!buf) {
+        status = NDIS_STATUS_INVALID_PACKET;
+        return status;
+    }
+
     csumInfo.Value = NET_BUFFER_LIST_INFO(curNbl,
                                           TcpIpChecksumNetBufferListInfo);
 
@@ -433,23 +447,22 @@ OvsValidateTCPChecksum(PNET_BUFFER_LIST curNbl, PNET_BUFFER curNb)
         return NDIS_STATUS_SUCCESS;
     }
 
-    EthHdr *eth = (EthHdr *)NdisGetDataBuffer(curNb, sizeof(EthHdr),
-                                              NULL, 1, 0);
-    if (eth == NULL) {
+    EthHdr *ethHdr = (EthHdr *)buf;
+    if (ethHdr == NULL) {
         return NDIS_STATUS_RESOURCES;
     }
 
-    if (eth->Type == ntohs(NDIS_ETH_TYPE_IPV4)) {
-        IPHdr *ip = (IPHdr *)((PCHAR)eth + sizeof *eth);
-        UINT32 l4Payload = ntohs(ip->tot_len) - ip->ihl * 4;
-        TCPHdr *tcp = (TCPHdr *)((PCHAR)ip + ip->ihl * 4);
+    if (ethHdr->Type == ntohs(NDIS_ETH_TYPE_IPV4)) {
+        IPHdr *ipHdr = (IPHdr *)(buf + layers->l3Offset);
+        UINT32 l4Payload = ntohs(ipHdr->tot_len) - ipHdr->ihl * 4;
+        TCPHdr *tcp = (TCPHdr *)(buf + layers->l4Offset);
         checkSum = tcp->check;
 
         tcp->check = 0;
-        tcp->check = IPPseudoChecksum(&ip->saddr, &ip->daddr,
+        tcp->check = IPPseudoChecksum(&ipHdr->saddr, &ipHdr->daddr,
                                       IPPROTO_TCP, (UINT16)l4Payload);
         tcp->check = CalculateChecksumNB(curNb, (UINT16)(l4Payload),
-                                         sizeof(EthHdr) + ip->ihl * 4);
+                                         layers->l4Offset);
         if (checkSum != tcp->check) {
             return NDIS_STATUS_INVALID_PACKET;
         }
@@ -912,18 +925,24 @@ OvsDecapStt(POVS_SWITCH_CONTEXT switchContext,
     SttHdr *sttHdr;
     char *sttBuf[STT_HDR_LEN];
     UINT32 advanceCnt, hdrLen;
+    OVS_PACKET_HDR_INFO layers = { 0 };
+
+    status = OvsExtractLayers(curNbl, &layers);
+    if (status != NDIS_STATUS_SUCCESS) {
+        return status;
+    }
 
     curNb = NET_BUFFER_LIST_FIRST_NB(curNbl);
     ASSERT(NET_BUFFER_NEXT_NB(curNb) == NULL);
 
     /* Validate the TCP Checksum */
-    status = OvsValidateTCPChecksum(curNbl, curNb);
+    status = OvsValidateTCPChecksum(curNbl, curNb, &layers);
     if (status != NDIS_STATUS_SUCCESS) {
         return status;
     }
 
     /* Skip Eth header */
-    hdrLen = sizeof(EthHdr);
+    hdrLen = layers.l3Offset;
     NdisAdvanceNetBufferDataStart(curNb, hdrLen, FALSE, NULL);
     advanceCnt = hdrLen;
 
@@ -996,7 +1015,6 @@ OvsDecapStt(POVS_SWITCH_CONTEXT switchContext,
     tunKey->pad = 0;
 
     /* Handle ECN */
-    OVS_PACKET_HDR_INFO layers = {0};
     if (0 != ipHdr->tos) {
         status = OvsExtractLayers(*newNbl, &layers);
         if (status != NDIS_STATUS_SUCCESS) {

diff --git a/datapath-windows/ovsext/Stt.h b/datapath-windows/ovsext/Stt.h
index 988a1c1eb2d7642dd4e51599657be9a96c62e263..2999dcf991074d5a5c36214337381dd594313752 100644 (file)
--- a/datapath-windows/ovsext/Stt.h
+++ b/datapath-windows/ovsext/Stt.h
@@ -115,4 +115,10 @@ OvsGetSttTunHdrSize(VOID)
                   STT_HDR_LEN;
 }
 
+static __inline UINT32
+OvsGetSttTunHdrSizeFromLayers(POVS_PACKET_HDR_INFO layers)
+{
+    return layers->l7Offset + STT_HDR_LEN;
+}
+
 #endif /*__OVS_STT_H_ */

diff --git a/datapath-windows/ovsext/Vxlan.c b/datapath-windows/ovsext/Vxlan.c
index f66a7e57fceacafbbcd2af8fad6370a3c182f91a..47c734e1e49775363931f590f59ee290f493d470 100644 (file)
--- a/datapath-windows/ovsext/Vxlan.c
+++ b/datapath-windows/ovsext/Vxlan.c
@@ -391,11 +391,17 @@ OvsDecapVxlan(POVS_SWITCH_CONTEXT switchContext,
     UINT32 tunnelSize, packetLength;
     PUINT8 bufferStart;
     NDIS_STATUS status;
+    OVS_PACKET_HDR_INFO layers = { 0 };
+
+    status = OvsExtractLayers(curNbl, &layers);
+    if (status != NDIS_STATUS_SUCCESS) {
+        return status;
+    }
 
     /* Check the length of the UDP payload */
     curNb = NET_BUFFER_LIST_FIRST_NB(curNbl);
     packetLength = NET_BUFFER_DATA_LENGTH(curNb);
-    tunnelSize = OvsGetVxlanTunHdrSize();
+    tunnelSize = OvsGetVxlanTunHdrSizeFromLayers(&layers);
     if (packetLength < tunnelSize) {
         return NDIS_STATUS_INVALID_LENGTH;
     }
@@ -424,13 +430,13 @@ OvsDecapVxlan(POVS_SWITCH_CONTEXT switchContext,
 
     ethHdr = (EthHdr *)bufferStart;
     /* XXX: Handle IP options. */
-    ipHdr = (IPHdr *)((PCHAR)ethHdr + sizeof *ethHdr);
+    ipHdr = (IPHdr *)(bufferStart + layers.l3Offset);
     tunKey->src = ipHdr->saddr;
     tunKey->dst = ipHdr->daddr;
     tunKey->tos = ipHdr->tos;
     tunKey->ttl = ipHdr->ttl;
     tunKey->pad = 0;
-    udpHdr = (UDPHdr *)((PCHAR)ipHdr + sizeof *ipHdr);
+    udpHdr = (UDPHdr *)(bufferStart + layers.l4Offset);
 
     /* Validate if NIC has indicated checksum failure. */
     status = OvsValidateUDPChecksum(curNbl, udpHdr->check == 0);
@@ -442,7 +448,7 @@ OvsDecapVxlan(POVS_SWITCH_CONTEXT switchContext,
     if (udpHdr->check != 0) {
         tunKey->flags |= OVS_TNL_F_CSUM;
         status = OvsCalculateUDPChecksum(curNbl, curNb, ipHdr, udpHdr,
-                                         packetLength);
+                                         packetLength, &layers);
         if (status != NDIS_STATUS_SUCCESS) {
             goto dropNbl;
         }

diff --git a/datapath-windows/ovsext/Vxlan.h b/datapath-windows/ovsext/Vxlan.h
index f4a8bceba5d467f26e9e34f02e6e801523157e5c..e2fb2dc08de20a3b9c2257f83af4c133210b988c 100644 (file)
--- a/datapath-windows/ovsext/Vxlan.h
+++ b/datapath-windows/ovsext/Vxlan.h
@@ -85,6 +85,12 @@ OvsGetVxlanTunHdrSize(VOID)
            sizeof (VXLANHdr);
 }
 
+static __inline UINT32
+OvsGetVxlanTunHdrSizeFromLayers(POVS_PACKET_HDR_INFO layers)
+{
+    return layers->l7Offset + sizeof(VXLANHdr);
+}
+
 #define VXLAN_UDP_PORT 4789
 #define VXLAN_UDP_PORT_NBO 0xB512