[ovs-dev] [PATCH] datapath-windows: Fix a couple of bugs during port enumeration

[Nithin Raju]

While enumerating the ports on a switch, if adding one of the
ports fails, we are not handling that error gracefully. Instead,
we fail adding of all the ports. This patch rectifies that.

Signed-off-by: Nithin Raju <nit...@vmware.com>
---
 datapath-windows/ovsext/Vport.c | 41 +++++++++++++++++++++++++++++------------
 1 file changed, 29 insertions(+), 12 deletions(-)

diff --git a/datapath-windows/ovsext/Vport.c b/datapath-windows/ovsext/Vport.c
index 7b0103d..a991d10 100644
--- a/datapath-windows/ovsext/Vport.c
+++ b/datapath-windows/ovsext/Vport.c
@@ -125,7 +125,7 @@ HvCreatePort(POVS_SWITCH_CONTEXT switchContext,
     if (vport != NULL) {
         OVS_LOG_ERROR("Port add failed due to duplicate port name, "
                       "port Id: %u", portParam->PortId);
-        status = STATUS_DATA_NOT_ACCEPTED;
+        status = NDIS_STATUS_DATA_NOT_ACCEPTED;
         goto create_port_done;
     }
 
@@ -140,7 +140,7 @@ HvCreatePort(POVS_SWITCH_CONTEXT switchContext,
         OVS_LOG_ERROR("Port add failed since a port already exists on "
                       "the specified port Id: %u, ovsName: %s",
                       portParam->PortId, vport->ovsName);
-        status = STATUS_DATA_NOT_ACCEPTED;
+        status = NDIS_STATUS_DATA_NOT_ACCEPTED;
         goto create_port_done;
     }
 
@@ -157,7 +157,7 @@ HvCreatePort(POVS_SWITCH_CONTEXT switchContext,
             OVS_LOG_INFO("Port add failed due to PortType change, port Id: %u"
                          " old: %u, new: %u", portParam->PortId,
                          vport->portType, portParam->PortType);
-            status = STATUS_DATA_NOT_ACCEPTED;
+            status = NDIS_STATUS_DATA_NOT_ACCEPTED;
             goto create_port_done;
         }
         vport->isAbsentOnHv = FALSE;
@@ -365,7 +365,7 @@ HvCreateNic(POVS_SWITCH_CONTEXT switchContext,
         OVS_LOG_ERROR("Create NIC without Switch Port,"
                       " PortId: %x, NicIndex: %d",
                       nicParam->PortId, nicParam->NicIndex);
-        status = NDIS_STATUS_INVALID_PARAMETER;
+        status = NDIS_STATUS_ADAPTER_NOT_FOUND;
         goto add_nic_done;
     }
     OvsInitVportWithNicParam(switchContext, vport, nicParam);
@@ -1393,6 +1393,7 @@ OvsAddConfiguredSwitchPorts(POVS_SWITCH_CONTEXT 
switchContext)
     ULONG arrIndex;
     PNDIS_SWITCH_PORT_PARAMETERS portParam;
     PNDIS_SWITCH_PORT_ARRAY portArray = NULL;
+    BOOLEAN portAdded = FALSE;
 
     OVS_LOG_TRACE("Enter: switchContext:%p", switchContext);
 
@@ -1402,16 +1403,24 @@ OvsAddConfiguredSwitchPorts(POVS_SWITCH_CONTEXT 
switchContext)
     }
 
     for (arrIndex = 0; arrIndex < portArray->NumElements; arrIndex++) {
-         portParam = NDIS_SWITCH_PORT_AT_ARRAY_INDEX(portArray, arrIndex);
+        portParam = NDIS_SWITCH_PORT_AT_ARRAY_INDEX(portArray, arrIndex);
 
-         if (portParam->IsValidationPort) {
-             continue;
-         }
+        if (portParam->IsValidationPort) {
+            continue;
+        }
 
-         status = HvCreatePort(switchContext, portParam, 0);
-         if (status != STATUS_SUCCESS && status != STATUS_DATA_NOT_ACCEPTED) {
-             break;
-         }
+        status = HvCreatePort(switchContext, portParam, 0);
+        if (status == NDIS_STATUS_SUCCESS) {
+            portAdded = TRUE;
+        } else if (status != NDIS_STATUS_DATA_NOT_ACCEPTED) {
+            /* Any other error. */
+            break;
+        }
+    }
+
+    /* If at least one port was added, return success. */
+    if (status != NDIS_STATUS_SUCCESS && portAdded == TRUE) {
+        status = NDIS_STATUS_SUCCESS;
     }
 
 cleanup:
@@ -1438,6 +1447,7 @@ OvsInitConfiguredSwitchNics(POVS_SWITCH_CONTEXT 
switchContext)
     PNDIS_SWITCH_NIC_ARRAY nicArray = NULL;
     ULONG arrIndex;
     PNDIS_SWITCH_NIC_PARAMETERS nicParam;
+    BOOLEAN nicAdded = FALSE;
 
     OVS_LOG_TRACE("Enter: switchContext: %p", switchContext);
     /*
@@ -1459,9 +1469,16 @@ OvsInitConfiguredSwitchNics(POVS_SWITCH_CONTEXT 
switchContext)
 
         status = HvCreateNic(switchContext, nicParam);
         if (status == NDIS_STATUS_SUCCESS) {
+            nicAdded = TRUE;
             HvConnectNic(switchContext, nicParam);
         }
     }
+
+    /* If at least one NIC was added, return success. */
+    if (status != NDIS_STATUS_SUCCESS && nicAdded == TRUE) {
+        status = NDIS_STATUS_SUCCESS;
+    }
+
 cleanup:
 
     OvsFreeSwitchNicsArray(nicArray);
-- 
2.6.2

_______________________________________________
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev