Hi Alin, Valid point. I’ll update the code. The change I was trying to make was as follows. If user forgot to assign OVS names to for VIFs, the first VIF gets added to the hash tables but subsequent ones will throw errors - either a NDIS_STATUS_DATA_NOT_ACCEPTED or NDIS_STATUS_ADAPTER_NOT_FOUND. For these two specific errors, I didn’t want to panic NDIS by reporting error since it is an OVS logic to not add these ports/NICs.
I agree that things like memory allocation are more serious issues and should be reported to NDIS. -- Nithin -----Original Message----- From: Alin Serdean <aserd...@cloudbasesolutions.com> Date: Monday, March 7, 2016 at 3:40 PM To: Nithin Raju <nit...@vmware.com>, "dev@openvswitch.org" <dev@openvswitch.org> Subject: RE: [ovs-dev] [PATCH] datapath-windows: Fix a couple of bugs during port enumeration >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 > >> >>https://urldefense.proofpoint.com/v2/url?u=http-3A__openvswitch.org_mailm >>an_listinfo_dev&d=BQIGaQ&c=Sqcl0Ez6M0X8aeM67LKIiDJAXVeAw-YihVMNtXt-uEs&r= >>pNHQcdr7B40b4h6Yb7FIedI1dnBsxdDuTLBYD3JqV80&m=5E-FADBd5JWQH2vVPvcRK-8ujX3 >>Ma1xwLzc2VrN06zw&s=0OHOm5aNuplL8bo-j1kiPwPIyOgoR8Km9YqDE9TCZSI&e= > _______________________________________________ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
