Still that breaks our prerequisites (elementname of the VIFs are unique) and does not inform the user regarding the error behind it.
We could write a wrapper over Enable-Vmswitchextension, i.e. Enable-OvsExtension, in which we require a switch name as a mandatory field, in that way we can avoid the extension to be run on two or more switches, and also check the elementnames of the VIFs and inform the user if the activation failed and why. Alin. > -----Mesaj original----- > De la: Nithin Raju [mailto:nit...@vmware.com] > Trimis: Tuesday, March 8, 2016 3:20 AM > Către: Alin Serdean <aserd...@cloudbasesolutions.com>; > dev@openvswitch.org > Subiect: Re: [ovs-dev] [PATCH] datapath-windows: Fix a couple of bugs > during port enumeration > > 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_ma > >>ilm > >>an_listinfo_dev&d=BQIGaQ&c=Sqcl0Ez6M0X8aeM67LKIiDJAXVeAw- > YihVMNtXt-uEs > >>&r= > >>pNHQcdr7B40b4h6Yb7FIedI1dnBsxdDuTLBYD3JqV80&m=5E- > FADBd5JWQH2vVPvcRK-8u > >>jX3 Ma1xwLzc2VrN06zw&s=0OHOm5aNuplL8bo- > j1kiPwPIyOgoR8Km9YqDE9TCZSI&e= > > _______________________________________________ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
