On Mon, May 11, 2015 at 5:42 AM, Sorin Vinturis
<svintu...@cloudbasesolutions.com> wrote:
> At the moment the OVS extension supports only one VXLAN tunnel that
> is cached in the extension switch context. Replaced the latter
> cached pointer with an array list that contains all VXLAN tunnel
> vports.
>
> 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/64
> Acked-by: Eitan Eliahu <elia...@vmware.com>
The patch does not apply on tip of master. Please rebase.
> ---
> datapath-windows/ovsext/Actions.c | 13 +++++++++----
> datapath-windows/ovsext/Switch.c | 16 +++++++++++-----
> datapath-windows/ovsext/Switch.h | 6 ++++--
> datapath-windows/ovsext/Tunnel.c | 3 ++-
> datapath-windows/ovsext/Vport.c | 40
> +++++++++++++++++++++++++++++++++------
> datapath-windows/ovsext/Vport.h | 21 +++++---------------
> datapath-windows/ovsext/Vxlan.c | 2 +-
> datapath-windows/ovsext/Vxlan.h | 2 +-
> 8 files changed, 67 insertions(+), 36 deletions(-)
>
> diff --git a/datapath-windows/ovsext/Actions.c
> b/datapath-windows/ovsext/Actions.c
> index a93fe03..79e464c 100644
> --- a/datapath-windows/ovsext/Actions.c
> +++ b/datapath-windows/ovsext/Actions.c
> @@ -184,6 +184,9 @@ OvsInitForwardingCtx(OvsForwardingContext *ovsFwdCtx,
> }
>
> /*
> + * XXX: When we search for the tunnelVport we also need to specify the
> + * tunnelling protocol or the L4 protocol as key as well, because there are
> + * different protocols that can use the same destination port.
> * --------------------------------------------------------------------------
> * OvsDetectTunnelRxPkt --
> * Utility function for an RX packet to detect its tunnel type.
> @@ -203,16 +206,17 @@ OvsDetectTunnelRxPkt(OvsForwardingContext *ovsFwdCtx,
> * packets only if they are at least VXLAN header size.
> */
> if (!flowKey->ipKey.nwFrag &&
> - flowKey->ipKey.nwProto == IPPROTO_UDP &&
> - flowKey->ipKey.l4.tpDst == VXLAN_UDP_PORT_NBO) {
> - tunnelVport = ovsFwdCtx->switchContext->vxlanVport;
> - ovsActionStats.rxVxlan++;
> + flowKey->ipKey.nwProto == IPPROTO_UDP) {
> + UINT16 dstPort = htons(flowKey->ipKey.l4.tpDst);
> + tunnelVport = OvsFindTunnelVportByDstPort(ovsFwdCtx->switchContext,
> + dstPort);
> }
>
> // We might get tunnel packets even before the tunnel gets initialized.
> if (tunnelVport) {
> ASSERT(ovsFwdCtx->tunnelRxNic == NULL);
> ovsFwdCtx->tunnelRxNic = tunnelVport;
> + ovsActionStats.rxVxlan++;
> return TRUE;
> }
>
> @@ -1318,6 +1322,7 @@ OvsExecuteSetAction(OvsForwardingContext *ovsFwdCtx,
> status = OvsTunnelAttrToIPv4TunnelKey((PNL_ATTR)a, &tunKey);
> ASSERT(status == NDIS_STATUS_SUCCESS);
> tunKey.flow_hash = (uint16)(hash ? *hash : OvsHashFlow(key));
> + tunKey.dst_port = key->ipKey.l4.tpDst;
> RtlCopyMemory(&ovsFwdCtx->tunKey, &tunKey, sizeof ovsFwdCtx->tunKey);
>
> break;
> diff --git a/datapath-windows/ovsext/Switch.c
> b/datapath-windows/ovsext/Switch.c
> index 416bcc0..f877854 100644
> --- a/datapath-windows/ovsext/Switch.c
> +++ b/datapath-windows/ovsext/Switch.c
> @@ -367,6 +367,8 @@ OvsInitSwitchContext(POVS_SWITCH_CONTEXT switchContext)
> sizeof(LIST_ENTRY) * OVS_MAX_VPORT_ARRAY_SIZE, OVS_SWITCH_POOL_TAG);
> switchContext->pidHashArray = (PLIST_ENTRY)OvsAllocateMemoryWithTag(
> sizeof(LIST_ENTRY) * OVS_MAX_PID_ARRAY_SIZE, OVS_SWITCH_POOL_TAG);
> + switchContext->tunnelVportsArray = (PLIST_ENTRY)OvsAllocateMemoryWithTag(
> + sizeof(LIST_ENTRY) * OVS_MAX_VPORT_ARRAY_SIZE, OVS_SWITCH_POOL_TAG);
> status = OvsAllocateFlowTable(&switchContext->datapath, switchContext);
>
> if (status == NDIS_STATUS_SUCCESS) {
> @@ -377,7 +379,8 @@ OvsInitSwitchContext(POVS_SWITCH_CONTEXT switchContext)
> switchContext->portNoHashArray == NULL ||
> switchContext->ovsPortNameHashArray == NULL ||
> switchContext->portIdHashArray== NULL ||
> - switchContext->pidHashArray == NULL) {
> + switchContext->pidHashArray == NULL ||
> + switchContext->tunnelVportsArray == NULL) {
> if (switchContext->dispatchLock) {
> NdisFreeRWLock(switchContext->dispatchLock);
> }
> @@ -398,6 +401,10 @@ OvsInitSwitchContext(POVS_SWITCH_CONTEXT switchContext)
> OVS_SWITCH_POOL_TAG);
> }
>
> + if (switchContext->tunnelVportsArray) {
> + OvsFreeMemory(switchContext->tunnelVportsArray);
> + }
> +
> OvsDeleteFlowTable(&switchContext->datapath);
> OvsCleanupBufferPool(switchContext);
>
> @@ -407,12 +414,9 @@ OvsInitSwitchContext(POVS_SWITCH_CONTEXT switchContext)
>
> for (i = 0; i < OVS_MAX_VPORT_ARRAY_SIZE; i++) {
> InitializeListHead(&switchContext->ovsPortNameHashArray[i]);
> - }
> - for (i = 0; i < OVS_MAX_VPORT_ARRAY_SIZE; i++) {
> InitializeListHead(&switchContext->portIdHashArray[i]);
> - }
> - for (i = 0; i < OVS_MAX_VPORT_ARRAY_SIZE; i++) {
> InitializeListHead(&switchContext->portNoHashArray[i]);
> + InitializeListHead(&switchContext->tunnelVportsArray[i]);
> }
>
> for (i = 0; i < OVS_MAX_PID_ARRAY_SIZE; i++) {
> @@ -465,6 +469,8 @@ OvsDeleteSwitchContext(POVS_SWITCH_CONTEXT switchContext)
> OvsFreeMemoryWithTag(switchContext->pidHashArray,
> OVS_SWITCH_POOL_TAG);
> switchContext->pidHashArray = NULL;
> + OvsFreeMemory(switchContext->tunnelVportsArray);
> + switchContext->tunnelVportsArray = NULL;
> OvsDeleteFlowTable(&switchContext->datapath);
> OvsCleanupBufferPool(switchContext);
>
> diff --git a/datapath-windows/ovsext/Switch.h
> b/datapath-windows/ovsext/Switch.h
> index 6ec34e1..8e1eb5f 100644
> --- a/datapath-windows/ovsext/Switch.h
> +++ b/datapath-windows/ovsext/Switch.h
> @@ -132,8 +132,6 @@ typedef struct _OVS_SWITCH_CONTEXT
> POVS_VPORT_ENTRY virtualExternalVport; // the virtual adapter
> vport
> POVS_VPORT_ENTRY internalVport;
>
> - POVS_VPORT_ENTRY vxlanVport;
> -
> /*
> * 'portIdHashArray' ONLY contains ports that exist on the Hyper-V
> switch,
> * namely: VIF (vNIC) ports, external port and Hyper-V internal port.
> @@ -148,11 +146,15 @@ typedef struct _OVS_SWITCH_CONTEXT
> * exist on the Hyper-V switch, and 'numNonHvVports' counts such ports in
> * 'portNoHashArray'.
> *
> + * 'tunnelVportsArray' contains tunnel ports that are added from OVS
> + * userspace. Currently only VXLAN tunnels are added in this list.
> + *
> * 'ovsPortNameHashArray' contains the same entries as 'portNoHashArray'
> but
> * hashed on a different key.
> */
> PLIST_ENTRY portIdHashArray; // based on Hyper-V
> portId
> PLIST_ENTRY portNoHashArray; // based on ovs port
> number
> + PLIST_ENTRY tunnelVportsArray; // based on ovs dst port
> number
> PLIST_ENTRY ovsPortNameHashArray; // based on ovsName
> PLIST_ENTRY pidHashArray; // based on packet pids
> NDIS_SPIN_LOCK pidHashLock; // Lock for pidHash table
> diff --git a/datapath-windows/ovsext/Tunnel.c
> b/datapath-windows/ovsext/Tunnel.c
> index fed58f1..002f180 100644
> --- a/datapath-windows/ovsext/Tunnel.c
> +++ b/datapath-windows/ovsext/Tunnel.c
> @@ -285,7 +285,8 @@ OvsInjectPacketThroughActions(PNET_BUFFER_LIST pNbl,
>
> SendFlags |= NDIS_SEND_FLAGS_DISPATCH_LEVEL;
>
> - vport = gOvsSwitchContext->vxlanVport;
> + vport = OvsFindTunnelVportByDstPort(gOvsSwitchContext,
> + htons(tunnelKey.dst_port));
>
> if (vport == NULL){
> status = STATUS_UNSUCCESSFUL;
> diff --git a/datapath-windows/ovsext/Vport.c b/datapath-windows/ovsext/Vport.c
> index e3a1803..aafbbcc 100644
> --- a/datapath-windows/ovsext/Vport.c
> +++ b/datapath-windows/ovsext/Vport.c
> @@ -591,6 +591,25 @@ OvsFindVportByPortNo(POVS_SWITCH_CONTEXT switchContext,
>
>
> POVS_VPORT_ENTRY
> +OvsFindTunnelVportByDstPort(POVS_SWITCH_CONTEXT switchContext,
> + UINT16 dstPort)
> +{
> + POVS_VPORT_ENTRY vport;
> + PLIST_ENTRY head, link;
> + UINT32 hash = OvsJhashBytes((const VOID *)&dstPort, sizeof(dstPort),
> + OVS_HASH_BASIS);
> + head = &(switchContext->tunnelVportsArray[hash & OVS_VPORT_MASK]);
> + LIST_FORALL(head, link) {
> + vport = CONTAINING_RECORD(link, OVS_VPORT_ENTRY, tunnelVportLink);
> + if (((POVS_VXLAN_VPORT)vport->priv)->dstPort == dstPort) {
> + return vport;
> + }
> + }
> + return NULL;
> +}
> +
> +
> +POVS_VPORT_ENTRY
> OvsFindVportByOvsName(POVS_SWITCH_CONTEXT switchContext,
> PSTR name)
> {
> @@ -1038,8 +1057,8 @@ InitHvVportCommon(POVS_SWITCH_CONTEXT switchContext,
> * --------------------------------------------------------------------------
> * Functionality common to any port added from OVS userspace.
> *
> - * Inserts the port into 'portIdHashArray', 'ovsPortNameHashArray' and caches
> - * the pointer in the 'switchContext' if needed.
> + * Inserts the port into 'portNoHashArray', 'ovsPortNameHashArray' and in
> + * 'tunnelVportsArray' if appropriate.
> * --------------------------------------------------------------------------
> */
> NDIS_STATUS
> @@ -1050,9 +1069,17 @@ InitOvsVportCommon(POVS_SWITCH_CONTEXT switchContext,
>
> switch(vport->ovsType) {
> case OVS_VPORT_TYPE_VXLAN:
> - switchContext->vxlanVport = vport;
> + {
> + POVS_VXLAN_VPORT vxlanVport = (POVS_VXLAN_VPORT)vport->priv;
> + hash = OvsJhashBytes(&vxlanVport->dstPort,
> + sizeof(vxlanVport->dstPort),
> + OVS_HASH_BASIS);
> + InsertHeadList(
> + &gOvsSwitchContext->tunnelVportsArray[hash & OVS_VPORT_MASK],
> + &vport->tunnelVportLink);
> switchContext->numNonHvVports++;
> break;
> + }
> case OVS_VPORT_TYPE_INTERNAL:
> if (vport->isBridgeInternal) {
> switchContext->numNonHvVports++;
> @@ -1152,7 +1179,8 @@ OvsRemoveAndDeleteVport(PVOID usrParamsContext,
> OvsTunnelVportPendingUninit,
> tunnelContext);
>
> - switchContext->vxlanVport = NULL;
> + RemoveEntryList(&vport->tunnelVportLink);
> + InitializeListHead(&vport->tunnelVportLink);
> break;
> }
> case OVS_VPORT_TYPE_GRE:
> @@ -1352,6 +1380,7 @@ OvsClearAllSwitchVports(POVS_SWITCH_CONTEXT
> switchContext)
> OvsRemoveAndDeleteVport(NULL, switchContext, vport, TRUE, TRUE);
> }
> }
> +
> /*
> * Remove 'virtualExternalVport' as well. This port is not part of the
> * 'portIdHashArray'.
> @@ -1361,9 +1390,9 @@ OvsClearAllSwitchVports(POVS_SWITCH_CONTEXT
> switchContext)
> (POVS_VPORT_ENTRY)switchContext->virtualExternalVport, TRUE,
> TRUE);
> }
>
> +
> 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;
> @@ -1377,7 +1406,6 @@ OvsClearAllSwitchVports(POVS_SWITCH_CONTEXT
> switchContext)
>
> ASSERT(switchContext->virtualExternalVport == NULL);
> ASSERT(switchContext->internalVport == NULL);
> - ASSERT(switchContext->vxlanVport == NULL);
> }
>
>
> diff --git a/datapath-windows/ovsext/Vport.h b/datapath-windows/ovsext/Vport.h
> index 4cfda30..086f03f 100644
> --- a/datapath-windows/ovsext/Vport.h
> +++ b/datapath-windows/ovsext/Vport.h
> @@ -84,6 +84,7 @@ typedef struct _OVS_VPORT_ENTRY {
> LIST_ENTRY ovsNameLink;
> LIST_ENTRY portIdLink;
> LIST_ENTRY portNoLink;
> + LIST_ENTRY tunnelVportLink;
>
> OVS_VPORT_STATE ovsState;
> OVS_VPORT_TYPE ovsType;
> @@ -135,10 +136,8 @@ typedef struct _OVS_VPORT_ENTRY {
>
> struct _OVS_SWITCH_CONTEXT;
>
> -POVS_VPORT_ENTRY
> -OvsFindVportByPortNo(struct _OVS_SWITCH_CONTEXT *switchContext,
> - UINT32 portNo);
> -
> +POVS_VPORT_ENTRY OvsFindVportByPortNo(POVS_SWITCH_CONTEXT switchContext,
> + UINT32 portNo);
> /* "name" is null-terminated */
> POVS_VPORT_ENTRY OvsFindVportByOvsName(POVS_SWITCH_CONTEXT switchContext,
> PSTR name);
> @@ -147,6 +146,8 @@ POVS_VPORT_ENTRY
> OvsFindVportByHvNameA(POVS_SWITCH_CONTEXT switchContext,
> POVS_VPORT_ENTRY OvsFindVportByPortIdAndNicIndex(POVS_SWITCH_CONTEXT
> switchContext,
> NDIS_SWITCH_PORT_ID portId,
> NDIS_SWITCH_NIC_INDEX
> index);
> +POVS_VPORT_ENTRY OvsFindTunnelVportByDstPort(POVS_SWITCH_CONTEXT
> switchContext,
> + UINT16 dstPort);
>
> NDIS_STATUS OvsAddConfiguredSwitchPorts(struct _OVS_SWITCH_CONTEXT
> *switchContext);
> NDIS_STATUS OvsInitConfiguredSwitchNics(struct _OVS_SWITCH_CONTEXT
> *switchContext);
> @@ -180,18 +181,6 @@ OvsIsTunnelVportType(OVS_VPORT_TYPE ovsType)
> ovsType == OVS_VPORT_TYPE_GRE64;
> }
>
> -static __inline POVS_VPORT_ENTRY
> -OvsGetTunnelVport(POVS_SWITCH_CONTEXT switchContext,
> - OVS_VPORT_TYPE ovsType)
> -{
> - switch(ovsType) {
> - case OVS_VPORT_TYPE_VXLAN:
> - return switchContext->vxlanVport;
> - default:
> - return NULL;
> - }
> -}
> -
> static __inline BOOLEAN
> OvsIsInternalVportType(OVS_VPORT_TYPE ovsType)
> {
> diff --git a/datapath-windows/ovsext/Vxlan.c b/datapath-windows/ovsext/Vxlan.c
> index 963d033..0474fcd 100644
> --- a/datapath-windows/ovsext/Vxlan.c
> +++ b/datapath-windows/ovsext/Vxlan.c
> @@ -264,7 +264,7 @@ OvsDoEncapVxlan(PNET_BUFFER_LIST curNbl,
> /* UDP header */
> udpHdr = (UDPHdr *)((PCHAR)ipHdr + sizeof *ipHdr);
> udpHdr->source = htons(tunKey->flow_hash | 32768);
> - udpHdr->dest = VXLAN_UDP_PORT_NBO;
> + udpHdr->dest = htons(tunKey->dst_port);
> udpHdr->len = htons(NET_BUFFER_DATA_LENGTH(curNb) - headRoom +
> sizeof *udpHdr + sizeof *vxlanHdr);
> udpHdr->check = 0;
> diff --git a/datapath-windows/ovsext/Vxlan.h b/datapath-windows/ovsext/Vxlan.h
> index 248a5dc..0e28304 100644
> --- a/datapath-windows/ovsext/Vxlan.h
> +++ b/datapath-windows/ovsext/Vxlan.h
> @@ -19,7 +19,7 @@
>
> #include "NetProto.h"
> typedef struct _OVS_VXLAN_VPORT {
> - UINT32 dstPort;
> + UINT16 dstPort;
> UINT64 inPkts;
> UINT64 outPkts;
> UINT64 slowInPkts;
> --
> 1.9.0.msysgit.0
> _______________________________________________
> dev mailing list
> dev@openvswitch.org
> http://openvswitch.org/mailman/listinfo/dev
_______________________________________________
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
