Hi Nithin, For all port add/delete functionality we need to validate in three major port states: [1] The port exists on the Hyper-V Virtual Switch but there is not corresponding port in OVS. [2] The port exists in OVS user-mode but there is no corresponding VSwitch port. [3] The port is on the VSwitch and there is a corresponding OVS port. [4] The port is not connected to the switch (VM is down). [5] Special handling for Internal and External ports.
Also, for memory leak verification uninstallation should be performed after there were packets were transferred over this port. We need to perform these test with the Verifier enabled. Can you exercise these scenarios ? Thank you, Eitan -----Original Message----- From: dev [mailto:dev-boun...@openvswitch.org] On Behalf Of Nithin Raju Sent: Tuesday, November 11, 2014 7:05 PM To: dev@openvswitch.org Subject: [ovs-dev] [PATCH 7/7] datapath-windows: Fixes in HvCreatePort() to re-add a port In this patch, we update HvCreatePort() to be able to re-add a Hyper-V port. Specifically, we handle the case where the port had also been added by OVS userspace, so that when the port was previously deleted from Hyper-V, we did not deallocate the port. The key to a vport is its name. We lookup the list of vports both in the 'portIdHashArray' as well as 'portNoHashArray' to make sure that we don't have a port with the same name. Validation: - deleted an re-added a port with and without the corresponding OVS port existing - deleted, changed the name of a port, and re-added it back with and without the corresponding OVS port existing. - uninstall was succcessful. No asserts hit. Signed-off-by: Nithin Raju <nit...@vmware.com> --- datapath-windows/ovsext/Vport.c | 69 +++++++++++++++++++++++++++++++++++---- 1 files changed, 62 insertions(+), 7 deletions(-) diff --git a/datapath-windows/ovsext/Vport.c b/datapath-windows/ovsext/Vport.c index 57e4c1e..1008db3 100644 --- a/datapath-windows/ovsext/Vport.c +++ b/datapath-windows/ovsext/Vport.c @@ -81,26 +81,63 @@ HvCreatePort(POVS_SWITCH_CONTEXT switchContext, POVS_VPORT_ENTRY vport; LOCK_STATE_EX lockState; NDIS_STATUS status = NDIS_STATUS_SUCCESS; + BOOLEAN newPort = FALSE; VPORT_PORT_ENTER(portParam); NdisAcquireRWLockWrite(switchContext->dispatchLock, &lockState, 0); + /* Lookup by port ID. */ vport = OvsFindVportByPortIdAndNicIndex(switchContext, portParam->PortId, 0); - if (vport != NULL && !vport->hvDeleted) { + if (vport != NULL) { + OVS_LOG_ERROR("Port add failed due to duplicate port name, " + "port Id: %u", portParam->PortId); status = STATUS_DATA_NOT_ACCEPTED; goto create_port_done; - } else if (!vport) { + } + + /* + * Lookup by port name to see if this port with this name had been added + * (and deleted) previously. + */ + vport = OvsFindVportByHvNameW(gOvsSwitchContext, + portParam->PortFriendlyName.String, + portParam->PortFriendlyName.Length); + if (vport && vport->hvDeleted == FALSE) { + 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; + goto create_port_done; + } + + if (vport != NULL) { + ASSERT(vport->hvDeleted); + ASSERT(vport->portNo != OVS_DPPORT_NUMBER_INVALID); + + /* + * It should be possible to simply just mark this port as "not deleted" + * given that the port Id and the name are the same and also provided + * that the other properties that we cache have not changed. + */ + if (vport->portType != portParam->PortType) { + 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; + goto create_port_done; + } + vport->hvDeleted = FALSE; + } else { vport = (POVS_VPORT_ENTRY)OvsAllocateVport(); if (vport == NULL) { status = NDIS_STATUS_RESOURCES; goto create_port_done; } + newPort = TRUE; } - OvsInitVportWithPortParam(vport, portParam); - /* XXX: Dummy argument to InitHvVportCommon(). */ - InitHvVportCommon(switchContext, vport, TRUE); + InitHvVportCommon(switchContext, vport, newPort); create_port_done: NdisReleaseRWLock(switchContext->dispatchLock, &lockState); @@ -530,9 +567,27 @@ OvsFindVportByHvNameW(POVS_SWITCH_CONTEXT switchContext, } } -Cleanup: - OvsFreeMemory(wsName); + /* + * Look in the list of ports that were added from the Hyper-V switch and + * deleted. + */ + if (vport == NULL) { + for (i = 0; i < OVS_MAX_VPORT_ARRAY_SIZE; i++) { + head = &(switchContext->portNoHashArray[i]); + LIST_FORALL(head, link) { + vport = CONTAINING_RECORD(link, OVS_VPORT_ENTRY, portNoLink); + if (vport->portFriendlyName.Length == wstrSize && + RtlEqualMemory(wsName, vport->portFriendlyName.String, + vport->portFriendlyName.Length)) { + goto Cleanup; + } + + vport = NULL; + } + } + } +Cleanup: return vport; } -- 1.7.4.1 _______________________________________________ dev mailing list dev@openvswitch.org https://urldefense.proofpoint.com/v2/url?u=http-3A__openvswitch.org_mailman_listinfo_dev&d=AAIGaQ&c=Sqcl0Ez6M0X8aeM67LKIiDJAXVeAw-YihVMNtXt-uEs&r=CWsgHUxi6ExLXY798tmo3LJ4e3geGYp56lkcH-5cLCY&m=UjwzpmvKDinkcRA0sIxRgOLw9KwTzN9gWuHP_Urgzao&s=8NEUmrGhsIH6X0uqVCqsVMBO-t797MqdQfxIcESJQdE&e= _______________________________________________ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
