Unless I am reading wrong: OvsAddConfiguredSwitchPorts and OvsInitConfiguredSwitchNics only fail if we could not allocate memory or could not issue an OID request.
I am not in favor of reporting back to NDIS gracefully if any of the above conditions was broken even once. Thanks, Alin. > -----Mesaj original----- > De la: dev [mailto:dev-boun...@openvswitch.org] În numele Nithin Raju > Trimis: Thursday, March 3, 2016 10:51 AM > Către: dev@openvswitch.org > Subiect: [ovs-dev] [PATCH] datapath-windows: Fix a couple of bugs during > port enumeration > > 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 _______________________________________________ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
