[ovs-dev] [PATCH] datapath-windows: Code refactoring and fixes in Vport.c
Nithin Raju
nithin at vmware.com
Fri Jun 19 04:57:29 UTC 2015
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 <nithin at vmware.com>V
Reported-at: https://github.com/openvswitch/ovs-issues/issues/79
Reported-by: Alin Gabriel Serdean <aserdean at cloudbasesolutions.com>
Reported-by: Nithin Raju <nithin at vmware.com>
---
datapath-windows/ovsext/Vport.c | 235 +++++++++++++++++++++-------------------
1 file changed, 124 insertions(+), 111 deletions(-)
diff --git a/datapath-windows/ovsext/Vport.c b/datapath-windows/ovsext/Vport.c
index 9139545..c8cfab9 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);
@@ -1124,16 +1124,89 @@ 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 && hvDelete) {
+ 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:
+ hvSwitchPort = TRUE;
+ default:
+ break;
+ }
+
+ if (vport->isExternal) {
+ /*
+ * vports of type external are created when:
+ * 1. External port is added (vport->nicIndex == 0). This port is not
+ * exposed to userspace, and thus cannot be deleted from OVS usersapce.
+ * 2. External NICs are added (vport->nicIndex != 0). One or more of
+ * these ports are exposed to userspace, and thus can be deleted from
+ * OVS userspace.
+ */
+ if (vport->nicIndex == 0) {
+ /* Such a vport is not part of any of the hash tables. */
+ ASSERT(hvDelete == TRUE);
+ ASSERT(switchContext->numPhysicalNics == 0);
+ switchContext->virtualExternalPortId = 0;
+ switchContext->virtualExternalVport = NULL;
+ OvsFreeMemoryWithTag(vport, OVS_VPORT_POOL_TAG);
+ return STATUS_SUCCESS;
+ } else {
+ if (hvDelete == TRUE) {
+ /* Decrementing the counters when the vport is deleted from OVS,
+ * will lead to double decrement. */
+ ASSERT(switchContext->numPhysicalNics);
+ switchContext->numPhysicalNics--;
+ }
+ hvSwitchPort = TRUE;
+ }
+ }
+
/*
* 'hvDelete' == TRUE indicates that the port should be removed from the
* 'portIdHashArray', while 'ovsDelete' == TRUE indicates that the port
@@ -1181,114 +1254,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 = 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;
- }
+ tunnelContext->switchContext = switchContext;
+ tunnelContext->hvSwitchPort = FALSE;
+ tunnelContext->hvDelete = hvDelete;
+ tunnelContext->ovsDelete = ovsDelete;
+ tunnelContext->vport = vport;
- 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 +2498,7 @@ Cleanup:
}
static VOID
-OvsTunnelVportPendingUninit(PVOID context,
+OvsTunnelVportPendingRemove(PVOID context,
NTSTATUS status,
UINT32 *replyLen)
{
@@ -2522,11 +2531,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);
}
--
1.8.5.6
More information about the dev
mailing list