[ovs-dev] [PATCH] datapath-windows: Fixed vport search functions

[Sorin Vinturis]

All OvsFindVportByXXX functions failed to check the first entry from
the vports doubly circular linked list. If the vports list had only
one element, all the latter functions exited without checking it.
This was due to the incorect usage of the loop browsing through the
vports list.

Signed-off-by: Sorin Vinturis <svinturis at cloudbasesolutions.com>
Reported-by: Sorin Vinturis <svinturis at cloudbasesolutions.com>
Reported-at: https://github.com/openvswitch/ovs-issues/issues/49

---
 datapath-windows/ovsext/Vport.c | 81 ++++++++++++++++++++++++++---------------
 1 file changed, 51 insertions(+), 30 deletions(-)

diff --git a/datapath-windows/ovsext/Vport.c b/datapath-windows/ovsext/Vport.c
index 0522cfd..81cf175 100644
--- a/datapath-windows/ovsext/Vport.c
+++ b/datapath-windows/ovsext/Vport.c
@@ -465,18 +465,31 @@ POVS_VPORT_ENTRY
 OvsFindVportByPortNo(POVS_SWITCH_CONTEXT switchContext,
                      UINT32 portNo)
 {
-    POVS_VPORT_ENTRY vport;
-    PLIST_ENTRY head, link;
-    UINT32 hash = OvsJhashBytes((const VOID *)&portNo, sizeof(portNo),
-                                OVS_HASH_BASIS);
-    head = &(switchContext->portNoHashArray[hash & OVS_VPORT_MASK]);
-    LIST_FORALL(head, link) {
-        vport = CONTAINING_RECORD(link, OVS_VPORT_ENTRY, portNoLink);
-        if (vport->portNo == portNo) {
-            return vport;
-        }
+    POVS_VPORT_ENTRY vport = NULL;
+
+    if (portNo == switchContext->externalVport->portNo) {
+        vport = switchContext->externalVport;
+    }
+    else if (portNo == switchContext->internalVport->portNo) {
+        vport = switchContext->internalVport;
+    }
+    else {
+        PLIST_ENTRY head = NULL, link = NULL;
+        UINT32 hash = OvsJhashBytes((const VOID *)&portNo, sizeof(portNo),
+                                    OVS_HASH_BASIS);
+        head = &(switchContext->portNoHashArray[hash & OVS_VPORT_MASK]);
+        link = head;
+        do {
+            vport = CONTAINING_RECORD(link, OVS_VPORT_ENTRY, portNoLink);
+            if (vport->portNo == portNo) {
+                break;
+            }
+            vport = NULL;
+            link = link->Flink;
+        } while (link != head);
     }
-    return NULL;
+
+    return vport;
 }
 
 
@@ -484,22 +497,24 @@ POVS_VPORT_ENTRY
 OvsFindVportByOvsName(POVS_SWITCH_CONTEXT switchContext,
                       PSTR name)
 {
-    POVS_VPORT_ENTRY vport;
-    PLIST_ENTRY head, link;
+    POVS_VPORT_ENTRY vport = NULL;
+    PLIST_ENTRY head = NULL, link = NULL;
     UINT32 hash;
     SIZE_T length = strlen(name) + 1;
 
     hash = OvsJhashBytes((const VOID *)name, length, OVS_HASH_BASIS);
     head = &(switchContext->ovsPortNameHashArray[hash & OVS_VPORT_MASK]);
-
-    LIST_FORALL(head, link) {
+    link = head;
+    do {
         vport = CONTAINING_RECORD(link, OVS_VPORT_ENTRY, ovsNameLink);
         if (!strcmp(name, vport->ovsName)) {
-            return vport;
+            break;
         }
-    }
+        vport = NULL;
+        link = link->Flink;
+    } while (link != head);
 
-    return NULL;
+    return vport;
 }
 
 /* OvsFindVportByHvName: "name" is assumed to be null-terminated */
@@ -508,7 +523,7 @@ OvsFindVportByHvName(POVS_SWITCH_CONTEXT switchContext,
                      PSTR name)
 {
     POVS_VPORT_ENTRY vport = NULL;
-    PLIST_ENTRY head, link;
+    PLIST_ENTRY head = NULL, link = NULL;
     /* 'portFriendlyName' is not NUL-terminated. */
     SIZE_T length = strlen(name);
     SIZE_T wstrSize = length * sizeof(WCHAR);
@@ -524,7 +539,8 @@ OvsFindVportByHvName(POVS_SWITCH_CONTEXT switchContext,
 
     for (i = 0; i < OVS_MAX_VPORT_ARRAY_SIZE; i++) {
         head = &(switchContext->portIdHashArray[i]);
-        LIST_FORALL(head, link) {
+        link = head;
+        do {
             vport = CONTAINING_RECORD(link, OVS_VPORT_ENTRY, portIdLink);
 
             /*
@@ -535,14 +551,14 @@ OvsFindVportByHvName(POVS_SWITCH_CONTEXT switchContext,
             if (vport->portFriendlyName.Length == wstrSize &&
                 RtlEqualMemory(wsName, vport->portFriendlyName.String,
                                vport->portFriendlyName.Length)) {
-                goto Cleanup;
+                break;
             }
 
             vport = NULL;
-        }
+            link = link->Flink;
+        } while (link != head);
     }
 
-Cleanup:
     OvsFreeMemory(wsName);
 
     return vport;
@@ -553,24 +569,29 @@ OvsFindVportByPortIdAndNicIndex(POVS_SWITCH_CONTEXT switchContext,
                                 NDIS_SWITCH_PORT_ID portId,
                                 NDIS_SWITCH_NIC_INDEX index)
 {
+    POVS_VPORT_ENTRY vport = NULL;
+
     if (portId == switchContext->externalPortId) {
-        return (POVS_VPORT_ENTRY)switchContext->externalVport;
+        vport = switchContext->externalVport;
     } else if (switchContext->internalPortId == portId) {
-        return (POVS_VPORT_ENTRY)switchContext->internalVport;
+        vport = switchContext->internalVport;
     } else {
         PLIST_ENTRY head, link;
-        POVS_VPORT_ENTRY vport;
         UINT32 hash;
         hash = OvsJhashWords((UINT32 *)&portId, 1, OVS_HASH_BASIS);
         head = &(switchContext->portIdHashArray[hash & OVS_VPORT_MASK]);
-        LIST_FORALL(head, link) {
+        link = head;
+        do {
             vport = CONTAINING_RECORD(link, OVS_VPORT_ENTRY, portIdLink);
             if (portId == vport->portId && index == vport->nicIndex) {
-                return vport;
+                break;
             }
-        }
-        return NULL;
+            vport = NULL;
+            link = link->Flink;
+        } while (link != head);
     }
+
+    return vport;
 }
 
 POVS_VPORT_ENTRY
-- 
1.9.0.msysgit.0