Re: [ovs-dev] [PATCH] datapath-windows: Fix a couple of bugs during port enumeration
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
> duringport 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,
> >
&
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 po
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_N
