[ovs-dev] [PATCH] datapath-windows: Fix a couple of bugs during port enumeration
Alin Serdean
aserdean at cloudbasesolutions.com
Tue Mar 8 01:41:37 UTC 2016
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 at vmware.com]
> Trimis: Tuesday, March 8, 2016 3:20 AM
> Către: Alin Serdean <aserdean at cloudbasesolutions.com>;
> dev at 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 at cloudbasesolutions.com>
> Date: Monday, March 7, 2016 at 3:40 PM
> To: Nithin Raju <nithin at vmware.com>, "dev at openvswitch.org"
> <dev at 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 at openvswitch.org] În numele Nithin Raju
> >
> >> Trimis: Thursday, March 3, 2016 10:51 AM
> >
> >> Către: dev at 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 at 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 at 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=
> >
More information about the dev
mailing list