| Message ID | 1456995071-54996-1-git-send-email-nithin@vmware.com |
|---|---|
| State | Changes Requested |
| Headers | show |
Acked-by: Sairam Venugopal <vsairam@vmware.com> On 3/3/16, 12:51 AM, "Nithin Raju" <nithin@vmware.com> wrote: >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 <nithin@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_mailma >n_listinfo_dev&d=BQIGaQ&c=Sqcl0Ez6M0X8aeM67LKIiDJAXVeAw-YihVMNtXt-uEs&r=Dc >ruz40PROJ40ROzSpxyQSLw6fcrOWpJgEcEmNR3JEQ&m=uP8XA9CichccgQAFi2F0J46OoUxEne >Gfn3OJFwYQiTQ&s=dqRv8YX_Qsg2fAdcel5qU_zdJ2AtYwY-gn0oG5a9JEY&e=
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-bounces@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 <nithin@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
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 <aserdean@cloudbasesolutions.com> Date: Monday, March 7, 2016 at 3:40 PM To: Nithin Raju <nithin@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-bounces@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 <nithin@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= >
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:nithin@vmware.com] > Trimis: Tuesday, March 8, 2016 3:20 AM > Către: Alin Serdean <aserdean@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 <aserdean@cloudbasesolutions.com> > Date: Monday, March 7, 2016 at 3:40 PM > To: Nithin Raju <nithin@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-bounces@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 <nithin@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= > >
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);
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 <nithin@vmware.com> --- datapath-windows/ovsext/Vport.c | 41 +++++++++++++++++++++++++++++------------ 1 file changed, 29 insertions(+), 12 deletions(-)