The kernel datapath supports only port 4789 for VXLAN tunnel creation. Added support in order to allow for the VXLAN tunnel port to be configurable to any port number set by the userspace.
The patch also checks to see if an existing WFP filter, for the necessary UDP tunnel port, is already created before adding a new one. This is a double check, because currently the userspace also verifies this, but in my opinion is necessary to avoid future inconsistencies. Custom VXLAN tunnel port requires the addition of a new WFP filter with the new UDP tunnel port. The cleanest way for the creation or removal of the new WFP filters is to add them in OvsInitVxlanTunnel or OvsCleanupVxlanTunnel functions. But the latter functions are running at IRQL = DISPATCH_LEVEL, due to the NDIS RW lock acquisition, and all WFP calls must be running at IRQL = PASSIVE_LEVEL. This is why the WFP filter creation/removal is made when the spinlock is released and the IRQL level is lowered at PASSIVE_LEVEL. The default VXLAN tunnel port 4789 behaviour is unchanged. The WFP filter for UDP port 4789 is added when the extension is enabled and is removed when the extension is disabled. No other NL command is able to remove it. Now it is necessary that OvsTunnelFilterUninitialize function is called after OvsClearAllSwitchVports in order to allow for the added WFP filters to be removed. OvsTunnelFilterUninitialize function closes the global engine handle used by most of the WFP calls, including filter removal. Signed-off-by: Sorin Vinturis <svintu...@cloudbasesolutions.com> Reported-by: Alin Gabriel Serdean <aserd...@cloudbasesolutions.com> Reported-at: https://github.com/openvswitch/ovs-issues/issues/66 --- datapath-windows/ovsext/Switch.c | 2 +- datapath-windows/ovsext/TunnelFilter.c | 88 ++++++++++++++++++++++++++++++++-- datapath-windows/ovsext/TunnelIntf.h | 4 ++ datapath-windows/ovsext/Vport.c | 78 ++++++++++++++++++++++++++++-- datapath-windows/ovsext/Vxlan.c | 8 +--- datapath-windows/ovsext/Vxlan.h | 1 + 6 files changed, 166 insertions(+), 15 deletions(-) diff --git a/datapath-windows/ovsext/Switch.c b/datapath-windows/ovsext/Switch.c index a228d8e..cf5e3c4 100644 --- a/datapath-windows/ovsext/Switch.c +++ b/datapath-windows/ovsext/Switch.c @@ -261,8 +261,8 @@ OvsDeleteSwitch(POVS_SWITCH_CONTEXT switchContext) if (switchContext) { dpNo = switchContext->dpNo; - OvsTunnelFilterUninitialize(gOvsExtDriverObject); OvsClearAllSwitchVports(switchContext); + OvsTunnelFilterUninitialize(gOvsExtDriverObject); OvsUninitSwitchContext(switchContext); OvsFreeMemory(switchContext); } diff --git a/datapath-windows/ovsext/TunnelFilter.c b/datapath-windows/ovsext/TunnelFilter.c index e0adc37..dac14c7 100644 --- a/datapath-windows/ovsext/TunnelFilter.c +++ b/datapath-windows/ovsext/TunnelFilter.c @@ -239,7 +239,8 @@ OvsTunnelAddFilter(PWSTR filterName, UINT64 context, const GUID *filterKey, const GUID *layerKey, - const GUID *calloutKey) + const GUID *calloutKey, + UINT64 *filterID) { NTSTATUS status = STATUS_SUCCESS; FWPM_FILTER filter = {0}; @@ -249,7 +250,9 @@ OvsTunnelAddFilter(PWSTR filterName, UNREFERENCED_PARAMETER(remotePort); UNREFERENCED_PARAMETER(direction); - filter.filterKey = *filterKey; + if (filterKey) { + filter.filterKey = *filterKey; + } filter.layerKey = *layerKey; filter.displayData.name = (wchar_t*)filterName; filter.displayData.description = (wchar_t*)filterDesc; @@ -282,7 +285,7 @@ OvsTunnelAddFilter(PWSTR filterName, status = FwpmFilterAdd(gEngineHandle, &filter, NULL, - NULL); + filterID); return status; } @@ -395,7 +398,8 @@ OvsTunnelRegisterDatagramDataCallouts(const GUID *layerKey, 0, &OVS_TUNNEL_FILTER_KEY, layerKey, - calloutKey); + calloutKey, + NULL); Exit: @@ -541,3 +545,79 @@ Exit: return status; } + +NTSTATUS +OvsTunnelAddFilterEx(UINT32 remotePort, + UINT64* filterID) +{ + NTSTATUS status = STATUS_SUCCESS; + BOOLEAN inTransaction = FALSE; + + do { + status = FwpmTransactionBegin(gEngineHandle, 0); + if (!NT_SUCCESS(status)) { + break; + } + + inTransaction = TRUE; + + status = OvsTunnelAddFilter(L"Datagram-Data OVS Filter (Inbound)", + L"address/port for UDP", + (USHORT)remotePort, + FWP_DIRECTION_INBOUND, + 0, + NULL, + &FWPM_LAYER_DATAGRAM_DATA_V4, + &OVS_TUNNEL_CALLOUT_V4, + filterID); + if (!NT_SUCCESS(status)) { + break; + } + + status = FwpmTransactionCommit(gEngineHandle); + if (!NT_SUCCESS(status)){ + break; + } + + inTransaction = FALSE; + } while (inTransaction); + + if (inTransaction) { + FwpmTransactionAbort(gEngineHandle); + } + return status; +} + +NTSTATUS +OvsTunnelRemoveFilterEx(UINT64 filterID) +{ + NTSTATUS status = STATUS_SUCCESS; + BOOLEAN inTransaction = FALSE; + + do { + if (!filterID) { + break; + } + + status = FwpmTransactionBegin(gEngineHandle, 0); + if (!NT_SUCCESS(status)) { + break; + } + inTransaction = TRUE; + + status = FwpmFilterDeleteById(gEngineHandle, filterID); + if (!NT_SUCCESS(status)) { + break; + } + + status = FwpmTransactionCommit(gEngineHandle); + + inTransaction = FALSE; + } while (inTransaction); + + if (inTransaction) { + FwpmTransactionAbort(gEngineHandle); + } + + return status; +} diff --git a/datapath-windows/ovsext/TunnelIntf.h b/datapath-windows/ovsext/TunnelIntf.h index b2bba30..5084a94 100644 --- a/datapath-windows/ovsext/TunnelIntf.h +++ b/datapath-windows/ovsext/TunnelIntf.h @@ -30,4 +30,8 @@ VOID OvsTunnelAddSystemProvider(HANDLE handle); VOID OvsTunnelRemoveSystemProvider(HANDLE handle); +NTSTATUS OvsTunnelAddFilterEx(UINT32 remotePort, UINT64* filterID); + +NTSTATUS OvsTunnelRemoveFilterEx(UINT64 filterID); + #endif /* __TUNNEL_INTF_H_ */ diff --git a/datapath-windows/ovsext/Vport.c b/datapath-windows/ovsext/Vport.c index c9dfaea..c9ba19a 100644 --- a/datapath-windows/ovsext/Vport.c +++ b/datapath-windows/ovsext/Vport.c @@ -70,6 +70,8 @@ static POVS_VPORT_ENTRY OvsFindVportByHvNameW(POVS_SWITCH_CONTEXT switchContext, static NDIS_STATUS InitHvVportCommon(POVS_SWITCH_CONTEXT switchContext, POVS_VPORT_ENTRY vport, BOOLEAN newPort); +static BOOLEAN OvsIsTunnelFilterCreated(POVS_SWITCH_CONTEXT switchContext, + UINT16 udpPortDest); /* * Functions implemented in relaton to NDIS port manipulation. @@ -1316,6 +1318,12 @@ OvsClearAllSwitchVports(POVS_SWITCH_CONTEXT switchContext) ASSERT(OvsIsTunnelVportType(vport->ovsType) || (vport->ovsType == OVS_VPORT_TYPE_INTERNAL && vport->isBridgeInternal) || vport->isPresentOnHv == TRUE); + + if (vport->priv) { + OvsTunnelRemoveFilterEx( + ((POVS_VXLAN_VPORT)vport->priv)->filterID); + } + OvsRemoveAndDeleteVport(switchContext, vport, TRUE, TRUE, NULL); } } @@ -1908,7 +1916,7 @@ Cleanup: /* * -------------------------------------------------------------------------- - * Command Handler for 'OVS_VPORT_CMD_NEW'. + * Command Handler for 'OVS_VPORT_CMD_GET'. * * The function handles the initial call to setup the dump state, as well as * subsequent calls to continue dumping data. @@ -1971,6 +1979,7 @@ OvsNewVportCmdHandler(POVS_USER_PARAMS_CONTEXT usrParamsCtx, PCHAR portName; ULONG portNameLen; UINT32 portType; + UINT16 udpPortDest = VXLAN_UDP_PORT; BOOLEAN isBridgeInternal = FALSE; BOOLEAN vportAllocated = FALSE, vportInitialized = FALSE; BOOLEAN addInternalPortAsNetdev = FALSE; @@ -2044,7 +2053,10 @@ OvsNewVportCmdHandler(POVS_USER_PARAMS_CONTEXT usrParamsCtx, vportAllocated = TRUE; if (OvsIsTunnelVportType(portType)) { - status = OvsInitTunnelVport(vport, portType, VXLAN_UDP_PORT); + PNL_ATTR attr = NlAttrFindNested(vportAttrs[OVS_VPORT_ATTR_OPTIONS], + OVS_TUNNEL_ATTR_DST_PORT); + udpPortDest = NlAttrGetU16(attr); + status = OvsInitTunnelVport(vport, portType, udpPortDest); nlError = NlMapStatusToNlErr(status); } else { OvsInitBridgeInternalVport(vport); @@ -2119,7 +2131,21 @@ OvsNewVportCmdHandler(POVS_USER_PARAMS_CONTEXT usrParamsCtx, Cleanup: NdisReleaseRWLock(gOvsSwitchContext->dispatchLock, &lockState); - if (nlError != NL_ERROR_SUCCESS) { + if (nlError == NL_ERROR_SUCCESS) { + if (VXLAN_UDP_PORT != udpPortDest && + !OvsIsTunnelFilterCreated(gOvsSwitchContext, udpPortDest)) { + /* + * All necessary calls to the WFP filtering engine must be running + * at IRQL = PASSIVE_LEVEL. Thus the new WFP tunnel filter creation + * is added here, outside the lock which raises the IRQL to + * DISPATCH_LEVEL. + */ + if (vport->priv) { + OvsTunnelAddFilterEx(udpPortDest, + &((POVS_VXLAN_VPORT)vport->priv)->filterID); + } + } + } else { POVS_MESSAGE_ERROR msgError = (POVS_MESSAGE_ERROR) usrParamsCtx->outputBuffer; @@ -2269,6 +2295,7 @@ OvsDeleteVportCmdHandler(POVS_USER_PARAMS_CONTEXT usrParamsCtx, NL_ERROR nlError = NL_ERROR_SUCCESS; PSTR portName = NULL; UINT32 portNameLen = 0; + UINT64 vxlanFilterID = 0; static const NL_POLICY ovsVportPolicy[] = { [OVS_VPORT_ATTR_PORT_NO] = { .type = NL_A_U32, .optional = TRUE }, @@ -2308,6 +2335,14 @@ OvsDeleteVportCmdHandler(POVS_USER_PARAMS_CONTEXT usrParamsCtx, nlError = NL_ERROR_NODEV; goto Cleanup; } + + if (vport->priv) { + /* + * Get VXLAN filterID before vport deletion in order to remove the + * WFP filter. + */ + vxlanFilterID = ((POVS_VXLAN_VPORT)vport->priv)->filterID; + } status = OvsCreateMsgFromVport(vport, msgIn, usrParamsCtx->outputBuffer, usrParamsCtx->outputLength, @@ -2324,6 +2359,10 @@ OvsDeleteVportCmdHandler(POVS_USER_PARAMS_CONTEXT usrParamsCtx, Cleanup: NdisReleaseRWLock(gOvsSwitchContext->dispatchLock, &lockState); + if (vxlanFilterID) { + OvsTunnelRemoveFilterEx(vxlanFilterID); + } + if (nlError != NL_ERROR_SUCCESS) { POVS_MESSAGE_ERROR msgError = (POVS_MESSAGE_ERROR) usrParamsCtx->outputBuffer; @@ -2334,3 +2373,36 @@ Cleanup: return STATUS_SUCCESS; } + +static BOOLEAN +OvsIsTunnelFilterCreated(POVS_SWITCH_CONTEXT switchContext, + UINT16 udpPortDest) +{ + for (UINT hash = 0; hash < OVS_MAX_VPORT_ARRAY_SIZE; hash++) { + PLIST_ENTRY head, link, next; + + head = &(switchContext->portNoHashArray[hash & OVS_VPORT_MASK]); + LIST_FORALL_SAFE(head, link, next) { + POVS_VPORT_ENTRY vport = NULL; + POVS_VXLAN_VPORT vxlanPort = NULL; + vport = CONTAINING_RECORD(link, OVS_VPORT_ENTRY, portNoLink); + vxlanPort = (POVS_VXLAN_VPORT)vport->priv; + if (vxlanPort) { + if ((udpPortDest == vxlanPort->dstPort) && + (0 != vxlanPort->filterID)) { + /* + * VXLAN destination port matching is not enough to decide + * if the WFP filter was created or not, because the newly + * allocated VXLAN vport is already added to the + * portNoHashArray in InitOvsVportCommon function. If the + * filterID of the VXLAN vport is zero it means that the + * WFP filter is not created yet. + */ + return TRUE; + } + } + } + } + + return FALSE; +} \ No newline at end of file diff --git a/datapath-windows/ovsext/Vxlan.c b/datapath-windows/ovsext/Vxlan.c index 1ce5af2..8f40b6a 100644 --- a/datapath-windows/ovsext/Vxlan.c +++ b/datapath-windows/ovsext/Vxlan.c @@ -57,7 +57,7 @@ NTSTATUS OvsInitVxlanTunnel(POVS_VPORT_ENTRY vport, UINT16 udpDestPort) { - POVS_VXLAN_VPORT vxlanPort; + POVS_VXLAN_VPORT vxlanPort = NULL; vxlanPort = OvsAllocateMemory(sizeof (*vxlanPort)); if (vxlanPort == NULL) { @@ -66,12 +66,6 @@ OvsInitVxlanTunnel(POVS_VPORT_ENTRY vport, RtlZeroMemory(vxlanPort, sizeof(*vxlanPort)); vxlanPort->dstPort = udpDestPort; - /* - * since we are installing the WFP filter before the port is created - * We need to check if it is the same number - * XXX should be removed later - */ - ASSERT(vxlanPort->dstPort == VXLAN_UDP_PORT); vport->priv = (PVOID)vxlanPort; return STATUS_SUCCESS; diff --git a/datapath-windows/ovsext/Vxlan.h b/datapath-windows/ovsext/Vxlan.h index d84796d..4343da1 100644 --- a/datapath-windows/ovsext/Vxlan.h +++ b/datapath-windows/ovsext/Vxlan.h @@ -24,6 +24,7 @@ typedef struct _OVS_VXLAN_VPORT { UINT64 outPkts; UINT64 slowInPkts; UINT64 slowOutPkts; + UINT64 filterID; /* * To be filled */ -- 1.9.0.msysgit.0 _______________________________________________ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
