[ovs-dev] [PATCH] datapath-windows: Fix the hash length when using recirculation

[Alin Serdean]

Current implementation of hashing does not take into consideration the
value of recirculation.

This patch updates the length of the hash to include the value of recirculation
in the hash itself.

To make sure the length is a multiple of 8 include the dphash in the
calculation.

Also clean some unnecessary code.

Signed-off-by: Alin Gabriel Serdean <aserdean at cloudbasesolutions.com>
---
 datapath-windows/ovsext/Actions.c |  6 ++++++
 datapath-windows/ovsext/Flow.c    | 24 +++++++++++++-----------
 datapath-windows/ovsext/Flow.h    |  4 ++--
 datapath-windows/ovsext/User.c    |  9 +++------
 4 files changed, 24 insertions(+), 19 deletions(-)

diff --git a/datapath-windows/ovsext/Actions.c b/datapath-windows/ovsext/Actions.c
index a91454d..00b3625 100644
--- a/datapath-windows/ovsext/Actions.c
+++ b/datapath-windows/ovsext/Actions.c
@@ -1964,6 +1964,12 @@ OvsDoRecirc(POVS_SWITCH_CONTEXT switchContext,
 
     status = OvsExtractFlow(ovsFwdCtx.curNbl, ovsFwdCtx.srcVportNo, key,
                             &ovsFwdCtx.layers, NULL);
+
+    if (key->recircId || key->dpHash) {
+        key->l2.keyLen += sizeof(key->recircId);
+        key->l2.keyLen += sizeof(key->dpHash);
+    }
+
     if (status != NDIS_STATUS_SUCCESS) {
         OvsCompleteNBLForwardingCtx(&ovsFwdCtx,
             L"OVS-Dropped due to extract flow failure");
diff --git a/datapath-windows/ovsext/Flow.c b/datapath-windows/ovsext/Flow.c
index 02c41b7..3a397ce 100644
--- a/datapath-windows/ovsext/Flow.c
+++ b/datapath-windows/ovsext/Flow.c
@@ -1380,13 +1380,20 @@ _MapKeyAttrToFlowPut(PNL_ATTR *keyAttrs,
 {
     _MapTunAttrToFlowPut(keyAttrs, tunnelAttrs, destKey);
 
+    /* ==== L3 + L4. ==== */
+    destKey->l2.keyLen = OVS_WIN_TUNNEL_KEY_SIZE + OVS_L2_KEY_SIZE
+                         - destKey->l2.offset;
+
     if (keyAttrs[OVS_KEY_ATTR_RECIRC_ID]) {
         destKey->recircId = NlAttrGetU32(keyAttrs[OVS_KEY_ATTR_RECIRC_ID]);
-        destKey->l2.keyLen += sizeof(destKey->recircId);
     }
 
     if (keyAttrs[OVS_KEY_ATTR_DP_HASH]) {
         destKey->dpHash = NlAttrGetU32(keyAttrs[OVS_KEY_ATTR_DP_HASH]);
+    }
+
+    if (destKey->recircId || destKey->dpHash) {
+        destKey->l2.keyLen += sizeof(destKey->recircId);
         destKey->l2.keyLen += sizeof(destKey->dpHash);
     }
 
@@ -1413,10 +1420,6 @@ _MapKeyAttrToFlowPut(PNL_ATTR *keyAttrs,
         destKey->l2.vlanTci = NlAttrGetU16(keyAttrs[OVS_KEY_ATTR_VLAN]);
     }
 
-    /* ==== L3 + L4. ==== */
-    destKey->l2.keyLen = OVS_WIN_TUNNEL_KEY_SIZE + OVS_L2_KEY_SIZE
-                         - destKey->l2.offset;
-
     switch (ntohs(destKey->l2.dlType)) {
     case ETH_TYPE_IPV4: {
 
@@ -1757,23 +1760,22 @@ DeleteAllFlows(OVS_DATAPATH *datapath)
     }
 }
 
-NDIS_STATUS
+VOID
 OvsGetFlowMetadata(OvsFlowKey *key,
                    PNL_ATTR *keyAttrs)
 {
-    NDIS_STATUS status = NDIS_STATUS_SUCCESS;
-
     if (keyAttrs[OVS_KEY_ATTR_RECIRC_ID]) {
         key->recircId = NlAttrGetU32(keyAttrs[OVS_KEY_ATTR_RECIRC_ID]);
-        key->l2.keyLen += sizeof(key->recircId);
     }
 
     if (keyAttrs[OVS_KEY_ATTR_DP_HASH]) {
         key->dpHash = NlAttrGetU32(keyAttrs[OVS_KEY_ATTR_DP_HASH]);
-        key->l2.keyLen += sizeof(key->dpHash);
     }
 
-    return status;
+    if (key->recircId || key->dpHash) {
+        key->l2.keyLen += sizeof(key->recircId);
+        key->l2.keyLen += sizeof(key->dpHash);
+    }
 }
 
 /*
diff --git a/datapath-windows/ovsext/Flow.h b/datapath-windows/ovsext/Flow.h
index 310c472..486e449 100644
--- a/datapath-windows/ovsext/Flow.h
+++ b/datapath-windows/ovsext/Flow.h
@@ -51,8 +51,8 @@ NDIS_STATUS OvsDeleteFlowTable(OVS_DATAPATH *datapath);
 NDIS_STATUS OvsAllocateFlowTable(OVS_DATAPATH *datapath,
                                  POVS_SWITCH_CONTEXT switchContext);
 
-NDIS_STATUS OvsGetFlowMetadata(OvsFlowKey *key,
-                               PNL_ATTR *keyAttrs);
+VOID OvsGetFlowMetadata(OvsFlowKey *key,
+                        PNL_ATTR *keyAttrs);
 NDIS_STATUS OvsExtractFlow(const NET_BUFFER_LIST *pkt, UINT32 inPort,
                            OvsFlowKey *flow, POVS_PACKET_HDR_INFO layers,
                            OvsIPv4TunnelKey *tunKey);
diff --git a/datapath-windows/ovsext/User.c b/datapath-windows/ovsext/User.c
index 6b2d94a..902e35a 100644
--- a/datapath-windows/ovsext/User.c
+++ b/datapath-windows/ovsext/User.c
@@ -430,13 +430,11 @@ OvsExecuteDpIoctl(OvsPacketExecute *execute)
     }
     // XXX: Figure out if any of the other members of fwdDetail need to be set.
 
-    status = OvsGetFlowMetadata(&key, execute->keyAttrs);
-    if (status != STATUS_SUCCESS) {
-        goto dropit;
-    }
-
     ndisStatus = OvsExtractFlow(pNbl, fwdDetail->SourcePortId, &key, &layers,
                                 NULL);
+
+    OvsGetFlowMetadata(&key, execute->keyAttrs);
+
     if (ndisStatus == NDIS_STATUS_SUCCESS) {
         NdisAcquireRWLockRead(gOvsSwitchContext->dispatchLock, &lockState, 0);
         ndisStatus = OvsActionsExecute(gOvsSwitchContext, NULL, pNbl,
@@ -456,7 +454,6 @@ OvsExecuteDpIoctl(OvsPacketExecute *execute)
         }
     }
 
-dropit:
     if (pNbl) {
         OvsCompleteNBL(gOvsSwitchContext, pNbl, TRUE);
     }
-- 
1.9.5.msysgit.0