[ovs-dev] datapath-windows: Use layers info to extract IP header in IpFragment
diff mbox series

Message ID 20181109190833.8072-1-kumaranand@vmware.com
State Accepted
Delegated to: Alin Gabriel Serdean
Headers show
Series
  • [ovs-dev] datapath-windows: Use layers info to extract IP header in IpFragment
Related show

Commit Message

Anand Kumar Nov. 9, 2018, 7:08 p.m. UTC
- Rely on layers l3Offset field to get offset of IP header.
- Aslo fix passing 'newNbl' to IP fragment which is not required.
- Fixed including a header file twice.

Signed-off-by: Anand Kumar <kumaranand@vmware.com>
---
 datapath-windows/ovsext/BufferMgmt.c | 16 ++++------
 datapath-windows/ovsext/Conntrack.c  | 11 +++----
 datapath-windows/ovsext/Conntrack.h  |  1 -
 datapath-windows/ovsext/IpFragment.c | 62 ++++++++++++++++--------------------
 datapath-windows/ovsext/IpFragment.h |  4 +--
 5 files changed, 41 insertions(+), 53 deletions(-)

Comments

Alin Gabriel Serdean Dec. 17, 2018, 1:45 p.m. UTC | #1
> -----Mesaj original-----
> De la: ovs-dev-bounces@openvswitch.org <ovs-dev-
> bounces@openvswitch.org> În numele Anand Kumar
> Trimis: Friday, November 9, 2018 9:09 PM
> Către: dev@openvswitch.org
> Subiect: [ovs-dev] [PATCH] datapath-windows: Use layers info to extract IP
> header in IpFragment
> 
> - Rely on layers l3Offset field to get offset of IP header.
> - Aslo fix passing 'newNbl' to IP fragment which is not required.
> - Fixed including a header file twice.
> 
> Signed-off-by: Anand Kumar <kumaranand@vmware.com>
> ---
>  datapath-windows/ovsext/BufferMgmt.c | 16 ++++------  datapath-
> windows/ovsext/Conntrack.c  | 11 +++----  datapath-
> windows/ovsext/Conntrack.h  |  1 -  datapath-
> windows/ovsext/IpFragment.c | 62 ++++++++++++++++--------------------
>  datapath-windows/ovsext/IpFragment.h |  4 +--
>  5 files changed, 41 insertions(+), 53 deletions(-)
> 
Thanks a lot for the change!

Acked-by: Alin Gabriel Serdean <aserdean@ovn.org>

Patch
diff mbox series

diff --git a/datapath-windows/ovsext/BufferMgmt.c b/datapath-windows/ovsext/BufferMgmt.c
index 448cd76..c163836 100644
--- a/datapath-windows/ovsext/BufferMgmt.c
+++ b/datapath-windows/ovsext/BufferMgmt.c
@@ -1101,9 +1101,9 @@  nblcopy_error:
 
 NDIS_STATUS
 GetIpHeaderInfo(PNET_BUFFER_LIST curNbl,
+                const POVS_PACKET_HDR_INFO hdrInfo,
                 UINT32 *hdrSize)
 {
-    CHAR *ethBuf[sizeof(EthHdr)];
     EthHdr *eth;
     IPHdr *ipHdr;
     PNET_BUFFER curNb;
@@ -1111,16 +1111,14 @@  GetIpHeaderInfo(PNET_BUFFER_LIST curNbl,
     curNb = NET_BUFFER_LIST_FIRST_NB(curNbl);
     ASSERT(NET_BUFFER_NEXT_NB(curNb) == NULL);
 
-    eth = (EthHdr *)NdisGetDataBuffer(curNb, ETH_HEADER_LENGTH,
-                                      (PVOID)&ethBuf, 1, 0);
+    eth = (EthHdr *)NdisGetDataBuffer(curNb,
+                                      hdrInfo->l4Offset,
+                                      NULL, 1, 0);
     if (eth == NULL) {
         return NDIS_STATUS_INVALID_PACKET;
     }
-    ipHdr = (IPHdr *)((PCHAR)eth + ETH_HEADER_LENGTH);
-    if (ipHdr == NULL) {
-        return NDIS_STATUS_INVALID_PACKET;
-    }
-    *hdrSize = (UINT32)(ETH_HEADER_LENGTH + (ipHdr->ihl * 4));
+    ipHdr = (IPHdr *)((PCHAR)eth + hdrInfo->l3Offset);
+    *hdrSize = (UINT32)(hdrInfo->l3Offset + (ipHdr->ihl * 4));
     return NDIS_STATUS_SUCCESS;
 }
 
@@ -1380,7 +1378,7 @@  OvsFragmentNBL(PVOID ovsContext,
 
     /* Figure out the header size */
     if (isIpFragment) {
-        status = GetIpHeaderInfo(nbl, &hdrSize);
+        status = GetIpHeaderInfo(nbl, hdrInfo, &hdrSize);
     } else {
         status = GetSegmentHeaderInfo(nbl, hdrInfo, &hdrSize, &seqNumber);
     }
diff --git a/datapath-windows/ovsext/Conntrack.c b/datapath-windows/ovsext/Conntrack.c
index 5be8e4d..bc00b60 100644
--- a/datapath-windows/ovsext/Conntrack.c
+++ b/datapath-windows/ovsext/Conntrack.c
@@ -489,10 +489,8 @@  OvsCtEntryDelete(POVS_CT_ENTRY entry, BOOLEAN forceDelete)
 
 static __inline NDIS_STATUS
 OvsDetectCtPacket(OvsForwardingContext *fwdCtx,
-                  OvsFlowKey *key,
-                  PNET_BUFFER_LIST *newNbl)
+                  OvsFlowKey *key)
 {
-    /* Currently we support only Unfragmented TCP packets */
     switch (ntohs(key->l2.dlType)) {
     case ETH_TYPE_IPV4:
         if (key->ipKey.nwFrag != OVS_FRAG_TYPE_NONE) {
@@ -500,8 +498,8 @@  OvsDetectCtPacket(OvsForwardingContext *fwdCtx,
                                           &fwdCtx->curNbl,
                                           fwdCtx->completionList,
                                           fwdCtx->fwdDetail->SourcePortId,
-                                          key->tunKey.tunnelId,
-                                          newNbl);
+                                          &fwdCtx->layers,
+                                          key->tunKey.tunnelId);
         }
         if (key->ipKey.nwProto == IPPROTO_TCP
             || key->ipKey.nwProto == IPPROTO_UDP
@@ -1010,11 +1008,10 @@  OvsExecuteConntrackAction(OvsForwardingContext *fwdCtx,
     PCHAR helper = NULL;
     NAT_ACTION_INFO natActionInfo;
     OVS_PACKET_HDR_INFO *layers = &fwdCtx->layers;
-    PNET_BUFFER_LIST newNbl = NULL;
     NDIS_STATUS status;
 
     memset(&natActionInfo, 0, sizeof natActionInfo);
-    status = OvsDetectCtPacket(fwdCtx, key, &newNbl);
+    status = OvsDetectCtPacket(fwdCtx, key);
     if (status != NDIS_STATUS_SUCCESS) {
         return status;
     }
diff --git a/datapath-windows/ovsext/Conntrack.h b/datapath-windows/ovsext/Conntrack.h
index c3d317f..bc6580d 100644
--- a/datapath-windows/ovsext/Conntrack.h
+++ b/datapath-windows/ovsext/Conntrack.h
@@ -21,7 +21,6 @@ 
 #include "Actions.h"
 #include "Debug.h"
 #include "Flow.h"
-#include "Actions.h"
 #include <stddef.h>
 
 #ifdef OVS_DBG_MOD
diff --git a/datapath-windows/ovsext/IpFragment.c b/datapath-windows/ovsext/IpFragment.c
index bb2cfe0..afb8e50 100644
--- a/datapath-windows/ovsext/IpFragment.c
+++ b/datapath-windows/ovsext/IpFragment.c
@@ -140,7 +140,7 @@  OvsIpv4Reassemble(POVS_SWITCH_CONTEXT switchContext,
                   OvsCompletionList *completionList,
                   NDIS_SWITCH_PORT_ID sourcePort,
                   POVS_IPFRAG_ENTRY entry,
-                  PNET_BUFFER_LIST *newNbl)
+                  POVS_PACKET_HDR_INFO layers)
 {
     NDIS_STATUS status = NDIS_STATUS_SUCCESS;
     NDIS_STRING filterReason;
@@ -148,29 +148,27 @@  OvsIpv4Reassemble(POVS_SWITCH_CONTEXT switchContext,
     PNET_BUFFER curNb;
     EthHdr *eth;
     IPHdr *ipHdr, *newIpHdr;
-    CHAR *ethBuf[sizeof(EthHdr)];
     CHAR *packetBuf;
-    UINT16 ipHdrLen, packetHeader;
     POVS_FRAGMENT_LIST head = NULL;
+    PNET_BUFFER_LIST newNbl = NULL;
+    UINT16 ipHdrLen, packetHeader;
     UINT32 packetLen;
 
     curNb = NET_BUFFER_LIST_FIRST_NB(*curNbl);
     ASSERT(NET_BUFFER_NEXT_NB(curNb) == NULL);
 
-    eth = (EthHdr*)NdisGetDataBuffer(curNb, ETH_HEADER_LENGTH,
-                                     (PVOID)&ethBuf, 1, 0);
+    eth = (EthHdr*)NdisGetDataBuffer(curNb, layers->l4Offset,
+                                     NULL, 1, 0);
     if (eth == NULL) {
         return NDIS_STATUS_INVALID_PACKET;
     }
-    ipHdr = (IPHdr *)((PCHAR)eth + ETH_HEADER_LENGTH);
-    if (ipHdr == NULL) {
-        return NDIS_STATUS_INVALID_PACKET;
-    }
+
+    ipHdr = (IPHdr *)((PCHAR)eth + layers->l3Offset);
     ipHdrLen = ipHdr->ihl * 4;
     if (ipHdrLen + entry->totalLen > MAX_IPDATAGRAM_SIZE) {
         return NDIS_STATUS_INVALID_LENGTH;
     }
-    packetLen = ETH_HEADER_LENGTH + ipHdrLen + entry->totalLen;
+    packetLen = layers->l3Offset + ipHdrLen + entry->totalLen;
     packetBuf = (CHAR*)OvsAllocateMemoryWithTag(packetLen,
                                                 OVS_IPFRAG_POOL_TAG);
     if (packetBuf == NULL) {
@@ -179,18 +177,18 @@  OvsIpv4Reassemble(POVS_SWITCH_CONTEXT switchContext,
     }
 
     /* copy Ethernet header */
-    NdisMoveMemory(packetBuf, eth, ETH_HEADER_LENGTH);
+    NdisMoveMemory(packetBuf, eth, layers->l3Offset);
     /* copy ipv4 header to packet buff */
-    NdisMoveMemory(packetBuf + ETH_HEADER_LENGTH, ipHdr, ipHdrLen);
+    NdisMoveMemory(packetBuf + layers->l3Offset, ipHdr, ipHdrLen);
 
     /* update new ip header */
-    newIpHdr = (IPHdr *)(packetBuf + ETH_HEADER_LENGTH);
+    newIpHdr = (IPHdr *)(packetBuf + layers->l3Offset);
     newIpHdr->frag_off = 0;
-    newIpHdr->tot_len = htons(packetLen - ETH_HEADER_LENGTH);
+    newIpHdr->tot_len = htons(packetLen - layers->l3Offset);
     newIpHdr->check = 0;
-    newIpHdr->check = IPChecksum((UINT8 *)packetBuf + ETH_HEADER_LENGTH,
+    newIpHdr->check = IPChecksum((UINT8 *)packetBuf + layers->l3Offset,
                                  ipHdrLen, 0);
-    packetHeader = ETH_HEADER_LENGTH + ipHdrLen;
+    packetHeader = layers->l3Offset + ipHdrLen;
     head = entry->head;
     while (head) {
         if ((UINT32)(packetHeader + head->offset) > packetLen) {
@@ -202,8 +200,8 @@  OvsIpv4Reassemble(POVS_SWITCH_CONTEXT switchContext,
         head = head->next;
     }
     /* Create new nbl from the flat buffer */
-    *newNbl = OvsAllocateNBLFromBuffer(switchContext, packetBuf, packetLen);
-    if (*newNbl == NULL) {
+    newNbl = OvsAllocateNBLFromBuffer(switchContext, packetBuf, packetLen);
+    if (newNbl == NULL) {
         OVS_LOG_ERROR("Insufficient resources, failed to allocate newNbl");
         status = NDIS_STATUS_RESOURCES;
         goto cleanup;
@@ -219,9 +217,9 @@  OvsIpv4Reassemble(POVS_SWITCH_CONTEXT switchContext,
         OvsCompleteNBL(switchContext, *curNbl, TRUE);
     }
     /* Store mru in the ovs buffer context. */
-    ctx = (POVS_BUFFER_CONTEXT)NET_BUFFER_LIST_CONTEXT_DATA_START(*newNbl);
+    ctx = (POVS_BUFFER_CONTEXT)NET_BUFFER_LIST_CONTEXT_DATA_START(newNbl);
     ctx->mru = entry->mru;
-    *curNbl = *newNbl;
+    *curNbl = newNbl;
 cleanup:
     OvsFreeMemoryWithTag(packetBuf, OVS_IPFRAG_POOL_TAG);
     entry->markedForDelete = TRUE;
@@ -240,12 +238,11 @@  OvsProcessIpv4Fragment(POVS_SWITCH_CONTEXT switchContext,
                        PNET_BUFFER_LIST *curNbl,
                        OvsCompletionList *completionList,
                        NDIS_SWITCH_PORT_ID sourcePort,
-                       ovs_be64 tunnelId,
-                       PNET_BUFFER_LIST *newNbl)
+                       POVS_PACKET_HDR_INFO layers,
+                       ovs_be64 tunnelId)
 {
     NDIS_STATUS status = NDIS_STATUS_PENDING;
     PNET_BUFFER curNb;
-    CHAR *ethBuf[sizeof(EthHdr)];
     UINT16 offset, flags;
     UINT16 payloadLen, ipHdrLen;
     UINT32 hash;
@@ -260,16 +257,13 @@  OvsProcessIpv4Fragment(POVS_SWITCH_CONTEXT switchContext,
     curNb = NET_BUFFER_LIST_FIRST_NB(*curNbl);
     ASSERT(NET_BUFFER_NEXT_NB(curNb) == NULL);
 
-    eth = (EthHdr*)NdisGetDataBuffer(curNb, ETH_HEADER_LENGTH,
-                                     (PVOID)&ethBuf, 1, 0);
+    eth = (EthHdr*)NdisGetDataBuffer(curNb, layers->l4Offset,
+                                     NULL, 1, 0);
     if (eth == NULL) {
         return NDIS_STATUS_INVALID_PACKET;
     }
 
-    ipHdr = (IPHdr *)((PCHAR)eth + ETH_HEADER_LENGTH);
-    if (ipHdr == NULL) {
-        return NDIS_STATUS_INVALID_PACKET;
-    }
+    ipHdr = (IPHdr *)((PCHAR)eth + layers->l3Offset);
     ipHdrLen = ipHdr->ihl * 4;
     payloadLen = ntohs(ipHdr->tot_len) - ipHdrLen;
     offset = ntohs(ipHdr->frag_off) & IP_OFFSET;
@@ -303,7 +297,7 @@  OvsProcessIpv4Fragment(POVS_SWITCH_CONTEXT switchContext,
     }
 
     /* Copy payload from nbl to fragment storage. */
-    if (OvsGetPacketBytes(*curNbl, payloadLen, ETH_HEADER_LENGTH + ipHdrLen,
+    if (OvsGetPacketBytes(*curNbl, payloadLen, layers->l3Offset + ipHdrLen,
                           fragStorage->pbuff) == NULL) {
         status = NDIS_STATUS_RESOURCES;
         goto payload_copy_error;
@@ -326,7 +320,7 @@  OvsProcessIpv4Fragment(POVS_SWITCH_CONTEXT switchContext,
         NdisMoveMemory(&(entry->fragKey), &fragKey,
                        sizeof(OVS_IPFRAG_KEY));
         /* Init MRU. */
-        entry->mru = ETH_HEADER_LENGTH + ipHdrLen + payloadLen;
+        entry->mru = layers->l3Offset + ipHdrLen + payloadLen;
         entry->recvdLen += fragStorage->len;
         entry->head = entry->tail = fragStorage;
         entry->numFragments = 1;
@@ -404,15 +398,15 @@  found:
         }
 
         /*Update Maximum Receive Unit */
-        entry->mru = entry->mru > (ETH_HEADER_LENGTH + ipHdrLen + payloadLen) ?
-            entry->mru : (ETH_HEADER_LENGTH + ipHdrLen + payloadLen);
+        entry->mru = entry->mru > (layers->l3Offset + ipHdrLen + payloadLen) ?
+            entry->mru : (layers->l3Offset + ipHdrLen + payloadLen);
         entry->numFragments++;
         if (!flags) {
             entry->totalLen = offset + payloadLen;
         }
         if (entry->recvdLen == entry->totalLen) {
             status = OvsIpv4Reassemble(switchContext, curNbl, completionList,
-                                       sourcePort, entry, newNbl);
+                                       sourcePort, entry, layers);
         }
         NdisReleaseSpinLock(&(entry->lockObj));
         return status;
diff --git a/datapath-windows/ovsext/IpFragment.h b/datapath-windows/ovsext/IpFragment.h
index 2b23051..433bad8 100644
--- a/datapath-windows/ovsext/IpFragment.h
+++ b/datapath-windows/ovsext/IpFragment.h
@@ -68,8 +68,8 @@  NDIS_STATUS OvsProcessIpv4Fragment(POVS_SWITCH_CONTEXT switchContext,
                                    PNET_BUFFER_LIST *curNbl,
                                    OvsCompletionList *completionList,
                                    NDIS_SWITCH_PORT_ID sourcePort,
-                                   ovs_be64 tunnelId,
-                                   PNET_BUFFER_LIST *newNbl);
+                                   POVS_PACKET_HDR_INFO layers,
+                                   ovs_be64 tunnelId);
 NDIS_STATUS OvsInitIpFragment(POVS_SWITCH_CONTEXT context);
 VOID OvsCleanupIpFragment(VOID);
 #endif /* __IPFRAGMENT_H_ */