From b8155080abc546d3d0cac3f316929904e8737bc8 Mon Sep 17 00:00:00 2001 From: Nithin Raju Date: Fri, 26 Jun 2015 11:51:28 -0700 Subject: datapath-windows: Code refactoring and fixes in Vport.c In this patch, there a couple of fixes and some code refactoring: 1. During deletion of "internal" and "external" in OvsRemoveAndDeleteVport(), we need to check if 'hvDelete' is TRUE before updating the data structures. Added code comments explaining the same. 2. Added a OvsRemoveTunnelPort() that gets called from OvsRemoveAndDeletePort() for the special processing for tunnel ports. 3. Folded in OvsCleanupVportCommon() back into OvsRemoveAndDeletePort(), since we only need a part of the functionality of OvsCleanupVportCommon() to be called from OvsTunnelVportPendingUninit(), and not the entire function. 4. Renamed OvsTunnelVportPendingUninit() to OvsTunnelVportPendingRemove() since it is basically a "pending" version of OvsVportTunnelRemove(). Validation: - Add external port from Hyper-V, add external port from OVS, remove external port from OVS, remove external port from Hyper-V. No ASSERT hit. - Add external port from Hyper-V, add external port from OVS, remove external port from Hyper-V, remove external port from OVS. No ASSERT hit. - Vxlan tunnel port creation/deletion - Stt tunnel port creation/deletion - Ping on Vxlan/Stt tunnels - Ovs Extension load/unload. There's an unrelated issue I found that is reported in: https://github.com/openvswitch/ovs-issues/issues/86 Signed-off-by: Nithin Raju V Reported-at: https://github.com/openvswitch/ovs-issues/issues/79 Reported-by: Alin Gabriel Serdean Reported-by: Nithin Raju Acked-by: Alin Gabriel Serdean Signed-off-by: Ben Pfaff --- datapath-windows/ovsext/Vport.c | 229 ++++++++++++++++++++-------------------- 1 file changed, 117 insertions(+), 112 deletions(-) (limited to 'datapath-windows/ovsext') diff --git a/datapath-windows/ovsext/Vport.c b/datapath-windows/ovsext/Vport.c index bff3ff686..a2762125d 100644 --- a/datapath-windows/ovsext/Vport.c +++ b/datapath-windows/ovsext/Vport.c @@ -84,15 +84,15 @@ static POVS_VPORT_ENTRY OvsFindVportByHvNameW(POVS_SWITCH_CONTEXT switchContext, static NDIS_STATUS InitHvVportCommon(POVS_SWITCH_CONTEXT switchContext, POVS_VPORT_ENTRY vport, BOOLEAN newPort); -static VOID OvsCleanupVportCommon(POVS_SWITCH_CONTEXT switchContext, - POVS_VPORT_ENTRY vport, - BOOLEAN hvSwitchPort, - BOOLEAN hvDelete, - BOOLEAN ovsDelete); +static NTSTATUS OvsRemoveTunnelVport(POVS_USER_PARAMS_CONTEXT usrParamsCtx, + POVS_SWITCH_CONTEXT switchContext, + POVS_VPORT_ENTRY vport, + BOOLEAN hvDelete, + BOOLEAN ovsDelete); static VOID OvsTunnelVportPendingInit(PVOID context, NTSTATUS status, UINT32 *replyLen); -static VOID OvsTunnelVportPendingUninit(PVOID context, +static VOID OvsTunnelVportPendingRemove(PVOID context, NTSTATUS status, UINT32 *replyLen); @@ -201,7 +201,7 @@ HvUpdatePort(POVS_SWITCH_CONTEXT switchContext, /* Store the nic and the OVS states as Nic Create won't be called */ ovsState = vport->ovsState; nicState = vport->nicState; - + /* * Currently only the port friendly name is being updated * Make sure that no other properties are changed @@ -1124,16 +1124,75 @@ InitOvsVportCommon(POVS_SWITCH_CONTEXT switchContext, return STATUS_SUCCESS; } -static VOID -OvsCleanupVportCommon(POVS_SWITCH_CONTEXT switchContext, - POVS_VPORT_ENTRY vport, - BOOLEAN hvSwitchPort, - BOOLEAN hvDelete, - BOOLEAN ovsDelete) + +/* + * -------------------------------------------------------------------------- + * Provides functionality that is partly complementatry to + * InitOvsVportCommon()/InitHvVportCommon(). + * + * 'hvDelete' indicates if caller is removing the vport as a result of the + * port being removed on the Hyper-V switch. + * 'ovsDelete' indicates if caller is removing the vport as a result of the + * port being removed from OVS userspace. + * -------------------------------------------------------------------------- + */ +NTSTATUS +OvsRemoveAndDeleteVport(PVOID usrParamsContext, + POVS_SWITCH_CONTEXT switchContext, + POVS_VPORT_ENTRY vport, + BOOLEAN hvDelete, + BOOLEAN ovsDelete) { + POVS_USER_PARAMS_CONTEXT usrParamsCtx = + (POVS_USER_PARAMS_CONTEXT)usrParamsContext; + BOOLEAN hvSwitchPort = FALSE; BOOLEAN deletedOnOvs = FALSE; BOOLEAN deletedOnHv = FALSE; + switch (vport->ovsType) { + case OVS_VPORT_TYPE_INTERNAL: + if (!vport->isBridgeInternal) { + if (hvDelete && vport->isPresentOnHv == FALSE) { + switchContext->internalPortId = 0; + switchContext->internalVport = NULL; + OvsInternalAdapterDown(); + } + hvSwitchPort = TRUE; + } + break; + case OVS_VPORT_TYPE_VXLAN: + { + NTSTATUS status; + status = OvsRemoveTunnelVport(usrParamsCtx, switchContext, vport, + hvDelete, ovsDelete); + if (status != STATUS_SUCCESS) { + return status; + } + } + case OVS_VPORT_TYPE_STT: + OvsCleanupSttTunnel(vport); + break; + case OVS_VPORT_TYPE_GRE: + case OVS_VPORT_TYPE_GRE64: + break; + case OVS_VPORT_TYPE_NETDEV: + if (vport->isExternal) { + if (vport->nicIndex == 0) { + /* Such a vport is not part of any of the hash tables, since it + * is not exposed to userspace. See Vport.h for explanation. */ + ASSERT(hvDelete == TRUE); + ASSERT(switchContext->numPhysicalNics == 0); + switchContext->virtualExternalPortId = 0; + switchContext->virtualExternalVport = NULL; + OvsFreeMemoryWithTag(vport, OVS_VPORT_POOL_TAG); + return STATUS_SUCCESS; + } + } + hvSwitchPort = TRUE; + default: + break; + } + /* * 'hvDelete' == TRUE indicates that the port should be removed from the * 'portIdHashArray', while 'ovsDelete' == TRUE indicates that the port @@ -1151,6 +1210,12 @@ OvsCleanupVportCommon(POVS_SWITCH_CONTEXT switchContext, if (hvDelete && !deletedOnHv) { vport->isPresentOnHv = TRUE; + if (vport->isExternal) { + ASSERT(vport->nicIndex != 0); + ASSERT(switchContext->numPhysicalNics); + switchContext->numPhysicalNics--; + } + /* Remove the port from the relevant lists. */ RemoveEntryList(&vport->portIdLink); InitializeListHead(&vport->portIdLink); @@ -1181,114 +1246,50 @@ OvsCleanupVportCommon(POVS_SWITCH_CONTEXT switchContext, if (deletedOnHv && deletedOnOvs) { if (hvSwitchPort) { switchContext->numHvVports--; - } - else { + } else { switchContext->numNonHvVports--; } OvsFreeMemoryWithTag(vport, OVS_VPORT_POOL_TAG); } + + return STATUS_SUCCESS; } -/* - * -------------------------------------------------------------------------- - * Provides functionality that is partly complementatry to - * InitOvsVportCommon()/InitHvVportCommon(). - * - * 'hvDelete' indicates if caller is removing the vport as a result of the - * port being removed on the Hyper-V switch. - * 'ovsDelete' indicates if caller is removing the vport as a result of the - * port being removed from OVS userspace. - * -------------------------------------------------------------------------- - */ -NTSTATUS -OvsRemoveAndDeleteVport(PVOID usrParamsContext, - POVS_SWITCH_CONTEXT switchContext, - POVS_VPORT_ENTRY vport, - BOOLEAN hvDelete, - BOOLEAN ovsDelete) +static NTSTATUS +OvsRemoveTunnelVport(POVS_USER_PARAMS_CONTEXT usrParamsCtx, + POVS_SWITCH_CONTEXT switchContext, + POVS_VPORT_ENTRY vport, + BOOLEAN hvDelete, + BOOLEAN ovsDelete) { - NTSTATUS status = STATUS_SUCCESS; - POVS_USER_PARAMS_CONTEXT usrParamsCtx = - (POVS_USER_PARAMS_CONTEXT)usrParamsContext; - BOOLEAN hvSwitchPort = FALSE; + POVS_TUNFLT_INIT_CONTEXT tunnelContext = NULL; + PIRP irp = NULL; - if (vport->isExternal) { - if (vport->nicIndex == 0) { - ASSERT(switchContext->numPhysicalNics == 0); - switchContext->virtualExternalPortId = 0; - switchContext->virtualExternalVport = NULL; - OvsFreeMemoryWithTag(vport, OVS_VPORT_POOL_TAG); - return STATUS_SUCCESS; - } else { - ASSERT(switchContext->numPhysicalNics); - switchContext->numPhysicalNics--; - hvSwitchPort = TRUE; - } + tunnelContext = OvsAllocateMemory(sizeof(*tunnelContext)); + if (tunnelContext == NULL) { + return STATUS_INSUFFICIENT_RESOURCES; } + RtlZeroMemory(tunnelContext, sizeof(*tunnelContext)); - switch (vport->ovsType) { - case OVS_VPORT_TYPE_INTERNAL: - if (!vport->isBridgeInternal) { - switchContext->internalPortId = 0; - switchContext->internalVport = NULL; - OvsInternalAdapterDown(); - hvSwitchPort = TRUE; - } - break; - case OVS_VPORT_TYPE_VXLAN: - { - POVS_TUNFLT_INIT_CONTEXT tunnelContext = NULL; - PIRP irp = NULL; + tunnelContext->switchContext = switchContext; + tunnelContext->hvSwitchPort = FALSE; + tunnelContext->hvDelete = hvDelete; + tunnelContext->ovsDelete = ovsDelete; + tunnelContext->vport = vport; - tunnelContext = OvsAllocateMemory(sizeof(*tunnelContext)); - if (tunnelContext == NULL) { - status = STATUS_INSUFFICIENT_RESOURCES; - break; - } - RtlZeroMemory(tunnelContext, sizeof(*tunnelContext)); - - tunnelContext->switchContext = switchContext; - tunnelContext->hvSwitchPort = hvSwitchPort; - tunnelContext->hvDelete = hvDelete; - tunnelContext->ovsDelete = ovsDelete; - tunnelContext->vport = vport; - - if (usrParamsCtx) { - tunnelContext->inputBuffer = usrParamsCtx->inputBuffer; - tunnelContext->outputBuffer = usrParamsCtx->outputBuffer; - tunnelContext->outputLength = usrParamsCtx->outputLength; - irp = usrParamsCtx->irp; - } - - status = OvsCleanupVxlanTunnel(irp, - vport, - OvsTunnelVportPendingUninit, - tunnelContext); - break; - } - case OVS_VPORT_TYPE_STT: - OvsCleanupSttTunnel(vport); - break; - case OVS_VPORT_TYPE_GRE: - case OVS_VPORT_TYPE_GRE64: - break; - case OVS_VPORT_TYPE_NETDEV: - hvSwitchPort = TRUE; - default: - break; - } - - if (STATUS_SUCCESS == status) { - OvsCleanupVportCommon(switchContext, - vport, - hvSwitchPort, - hvDelete, - ovsDelete); + if (usrParamsCtx) { + tunnelContext->inputBuffer = usrParamsCtx->inputBuffer; + tunnelContext->outputBuffer = usrParamsCtx->outputBuffer; + tunnelContext->outputLength = usrParamsCtx->outputLength; + irp = usrParamsCtx->irp; } - return status; + return OvsCleanupVxlanTunnel(irp, vport, OvsTunnelVportPendingRemove, + tunnelContext); } + + NDIS_STATUS OvsAddConfiguredSwitchPorts(POVS_SWITCH_CONTEXT switchContext) { @@ -2489,7 +2490,7 @@ Cleanup: } static VOID -OvsTunnelVportPendingUninit(PVOID context, +OvsTunnelVportPendingRemove(PVOID context, NTSTATUS status, UINT32 *replyLen) { @@ -2522,11 +2523,15 @@ OvsTunnelVportPendingUninit(PVOID context, } } - OvsCleanupVportCommon(switchContext, - vport, - tunnelContext->hvSwitchPort, - tunnelContext->hvDelete, - tunnelContext->ovsDelete); + ASSERT(vport->isPresentOnHv == TRUE); + ASSERT(vport->portNo != OVS_DPPORT_NUMBER_INVALID); + + /* Remove the port from the relevant lists. */ + switchContext->numNonHvVports--; + RemoveEntryList(&vport->ovsNameLink); + RemoveEntryList(&vport->portNoLink); + RemoveEntryList(&vport->tunnelVportLink); + OvsFreeMemoryWithTag(vport, OVS_VPORT_POOL_TAG); NdisReleaseRWLock(switchContext->dispatchLock, &lockState); } -- cgit v1.2.1