Skip to content

Commit

Permalink
datapath-windows: use correct dst port during Vxlan Tx
Browse files Browse the repository at this point in the history
A previous commit used the wrong DST port in the UDP header during Vxlan
Tx which caused Vxlan tunneling to break. Fixing it here..

Also included is a cosmetic fix in OvsDetectTunnelRxPkt() where we were
using htons() instead of ntohs(). Doesn't make a difference in practice
though.

One more change is, OvsIpHlprCbVxlan() has been nuked since it is not
used. Not sure if it is worth being resurrected.

Testing done: Ping across Vxlan tunnel and Stt tunnel.

Signed-off-by: Nithin Raju <nithin@vmware.com>
Reported-by: Eitan Eliahu <eliahue@vmware.com>
Acked-by: Alin Gabriel Serdean <aserdean@cloudbasesolutions.com>
Signed-off-by: Gurucharan Shetty <gshetty@nicira.com>
  • Loading branch information
nithinrajub authored and shettyg committed Jun 19, 2015
1 parent 2d34dbd commit 0b623ad
Show file tree
Hide file tree
Showing 4 changed files with 19 additions and 50 deletions.
9 changes: 4 additions & 5 deletions datapath-windows/ovsext/Actions.c
Expand Up @@ -207,7 +207,7 @@ OvsDetectTunnelRxPkt(OvsForwardingContext *ovsFwdCtx,
*/
if (!flowKey->ipKey.nwFrag &&
flowKey->ipKey.nwProto == IPPROTO_UDP) {
UINT16 dstPort = htons(flowKey->ipKey.l4.tpDst);
UINT16 dstPort = ntohs(flowKey->ipKey.l4.tpDst);
tunnelVport = OvsFindTunnelVportByDstPort(ovsFwdCtx->switchContext,
dstPort,
OVS_VPORT_TYPE_VXLAN);
Expand Down Expand Up @@ -654,14 +654,13 @@ OvsTunnelPortTx(OvsForwardingContext *ovsFwdCtx)
/* Do the encap. Encap function does not consume the NBL. */
switch(ovsFwdCtx->tunnelTxNic->ovsType) {
case OVS_VPORT_TYPE_VXLAN:
status = OvsEncapVxlan(ovsFwdCtx->curNbl, &ovsFwdCtx->tunKey,
ovsFwdCtx->switchContext,
status = OvsEncapVxlan(ovsFwdCtx->tunnelTxNic, ovsFwdCtx->curNbl,
&ovsFwdCtx->tunKey, ovsFwdCtx->switchContext,
&ovsFwdCtx->layers, &newNbl);
break;
case OVS_VPORT_TYPE_STT:
status = OvsEncapStt(ovsFwdCtx->tunnelTxNic, ovsFwdCtx->curNbl,
&ovsFwdCtx->tunKey,
ovsFwdCtx->switchContext,
&ovsFwdCtx->tunKey, ovsFwdCtx->switchContext,
&ovsFwdCtx->layers, &newNbl);
break;
default:
Expand Down
4 changes: 2 additions & 2 deletions datapath-windows/ovsext/Stt.c
Expand Up @@ -114,8 +114,8 @@ OvsEncapStt(POVS_VPORT_ENTRY vport,
return NDIS_STATUS_FAILURE;
}

status = OvsDoEncapStt(vport, curNbl, tunKey, &fwdInfo, layers, switchContext,
newNbl);
status = OvsDoEncapStt(vport, curNbl, tunKey, &fwdInfo, layers,
switchContext, newNbl);
return status;
}

Expand Down
53 changes: 11 additions & 42 deletions datapath-windows/ovsext/Vxlan.c
Expand Up @@ -170,7 +170,8 @@ OvsCleanupVxlanTunnel(PIRP irp,
*----------------------------------------------------------------------------
*/
static __inline NDIS_STATUS
OvsDoEncapVxlan(PNET_BUFFER_LIST curNbl,
OvsDoEncapVxlan(POVS_VPORT_ENTRY vport,
PNET_BUFFER_LIST curNbl,
OvsIPv4TunnelKey *tunKey,
POVS_FWD_INFO fwdInfo,
POVS_PACKET_HDR_INFO layers,
Expand All @@ -185,6 +186,7 @@ OvsDoEncapVxlan(PNET_BUFFER_LIST curNbl,
IPHdr *ipHdr;
UDPHdr *udpHdr;
VXLANHdr *vxlanHdr;
POVS_VXLAN_VPORT vportVxlan;
UINT32 headRoom = OvsGetVxlanTunHdrSize();
UINT32 packetLength;

Expand All @@ -211,6 +213,10 @@ OvsDoEncapVxlan(PNET_BUFFER_LIST curNbl,
}
}
}

vportVxlan = (POVS_VXLAN_VPORT) GetOvsVportPriv(vport);
ASSERT(vportVxlan);

/* If we didn't split the packet above, make a copy now */
if (*newNbl == NULL) {
*newNbl = OvsPartialCopyNBL(switchContext, curNbl, 0, headRoom,
Expand Down Expand Up @@ -274,7 +280,7 @@ OvsDoEncapVxlan(PNET_BUFFER_LIST curNbl,
/* UDP header */
udpHdr = (UDPHdr *)((PCHAR)ipHdr + sizeof *ipHdr);
udpHdr->source = htons(tunKey->flow_hash | 32768);
udpHdr->dest = htons(tunKey->dst_port);
udpHdr->dest = htons(vportVxlan->dstPort);
udpHdr->len = htons(NET_BUFFER_DATA_LENGTH(curNb) - headRoom +
sizeof *udpHdr + sizeof *vxlanHdr);
udpHdr->check = 0;
Expand Down Expand Up @@ -308,7 +314,8 @@ OvsDoEncapVxlan(PNET_BUFFER_LIST curNbl,
*----------------------------------------------------------------------------
*/
NDIS_STATUS
OvsEncapVxlan(PNET_BUFFER_LIST curNbl,
OvsEncapVxlan(POVS_VPORT_ENTRY vport,
PNET_BUFFER_LIST curNbl,
OvsIPv4TunnelKey *tunKey,
POVS_SWITCH_CONTEXT switchContext,
POVS_PACKET_HDR_INFO layers,
Expand All @@ -331,48 +338,10 @@ OvsEncapVxlan(PNET_BUFFER_LIST curNbl,
return NDIS_STATUS_FAILURE;
}

return OvsDoEncapVxlan(curNbl, tunKey, &fwdInfo, layers,
return OvsDoEncapVxlan(vport, curNbl, tunKey, &fwdInfo, layers,
switchContext, newNbl);
}


/*
*----------------------------------------------------------------------------
* OvsIpHlprCbVxlan --
* Callback function for IP helper.
* XXX: not used currently
*----------------------------------------------------------------------------
*/
static VOID
OvsIpHlprCbVxlan(PNET_BUFFER_LIST curNbl,
UINT32 inPort,
OvsIPv4TunnelKey *tunKey,
PVOID cbData1,
PVOID cbData2,
NTSTATUS result,
POVS_FWD_INFO fwdInfo)
{
OVS_PACKET_HDR_INFO layers;
OvsFlowKey key;
NDIS_STATUS status;
UNREFERENCED_PARAMETER(inPort);

status = OvsExtractFlow(curNbl, inPort, &key, &layers, NULL);
if (result == STATUS_SUCCESS) {
status = OvsDoEncapVxlan(curNbl, tunKey, fwdInfo, &layers,
(POVS_SWITCH_CONTEXT)cbData1, NULL);
} else {
status = NDIS_STATUS_FAILURE;
}

if (status != NDIS_STATUS_SUCCESS) {
// XXX: Free up the NBL;
return;
}

OvsLookupFlowOutput((POVS_SWITCH_CONTEXT)cbData1, cbData2, curNbl);
}

/*
*----------------------------------------------------------------------------
* OvsCalculateUDPChecksum
Expand Down
3 changes: 2 additions & 1 deletion datapath-windows/ovsext/Vxlan.h
Expand Up @@ -62,7 +62,8 @@ NTSTATUS OvsCleanupVxlanTunnel(PIRP irp,
NDIS_STATUS OvsSlowPathDecapVxlan(const PNET_BUFFER_LIST packet,
OvsIPv4TunnelKey *tunnelKey);

NDIS_STATUS OvsEncapVxlan(PNET_BUFFER_LIST curNbl,
NDIS_STATUS OvsEncapVxlan(POVS_VPORT_ENTRY vport,
PNET_BUFFER_LIST curNbl,
OvsIPv4TunnelKey *tunKey,
POVS_SWITCH_CONTEXT switchContext,
POVS_PACKET_HDR_INFO layers,
Expand Down

0 comments on commit 0b623ad

Please sign in to comment.