[ovs-dev] [PATCH] datapath-windows: check source port during tunnel Tx
Alin Serdean
aserdean at cloudbasesolutions.com
Wed Aug 13 15:24:00 UTC 2014
Hey Nithin,
I will take a look over the patch after the meeting.
Thanks,
Alin.
-----Mesaj original-----
De la: dev [mailto:dev-bounces at openvswitch.org] În numele Nithin Raju
Trimis: Wednesday, August 13, 2014 8:59 AM
Către: dev at openvswitch.org; ssaurabh at vmware.com
Subiect: [ovs-dev] [PATCH] datapath-windows: check source port during tunnel Tx
In the Windows datapath, Tx tunneling functionality is implemented by checking if the outport in the action is a tunnel port. If so, the packet is encapsulated and sent out on the PIF bridge for as second flow lookup.
Basically, we don't use the hypervisor's IP stack to send out a packet, and short circuit the path ourselves. On the PIF bridge, the source port of the encapsulated packet is the VTEP port ie. the internal port.
If a Tunneling port is added to the PIF bridge (a possible misconfiguration), where the VTEP(internal) port and the external port (physical NIC) also reside, a flooding action can cause a loop, by re-injecting the packet on the same PIF bridge which again floods to the tunnel port.
In this change, we break the loop by encapsulating packets only if they are sent out by a VIF or if they originate from userspace ie. userspace generated.
We make use of the input port attribute in the packet execute ioctl.
This change is based off of the legacy datapath interface published in OvsPub.h. This interface has a input port field in the packet execute ioctl.
I looked in dpif-linux.c that uses the netlink based datapath interface and even in that case, we do add the the source port in:
dpif_linux_encode_execute() -> odp_key_from_pkt_metadata().
So, this fix is applicable when we adopt the netlink based datapath interface as well.
The Rx side of OvsDetectTunnelPkt() has only documentation updates. The fix is on the Tx side.
Validation (using dpif-windows.c):
- Was able to perform VTEP <-> VTEP ping with the configuration posted in the issue.
- Was able to perform VIF <-> VIF ping when the setup was configured correctly.
Signed-off-by: Nithin Raju <nithin at vmware.com>
Reported-by: Alin Serdean <aserdean at cloudbasesolutions.com>
Reported-at: https://github.com/openvswitch/ovs-issues/issues/20
---
datapath-windows/ovsext/OvsActions.c | 58 +++++++++++++++++++++++++-----------
datapath-windows/ovsext/OvsUser.c | 19 ++++++++----
datapath-windows/ovsext/OvsVport.h | 7 +++++
3 files changed, 60 insertions(+), 24 deletions(-)
diff --git a/datapath-windows/ovsext/OvsActions.c b/datapath-windows/ovsext/OvsActions.c
index 4a2c117..635becc 100644
--- a/datapath-windows/ovsext/OvsActions.c
+++ b/datapath-windows/ovsext/OvsActions.c
@@ -224,14 +224,18 @@ OvsDetectTunnelRxPkt(OvsForwardingContext *ovsFwdCtx,
/*
* --------------------------------------------------------------------------
* OvsDetectTunnelPkt --
- * Utility function to detect the tunnel type of a TX/RX packet.
+ * Utility function to detect if a packet is to be subjected to
+ * tunneling (Tx) or de-tunneling (Rx). Various factors such as source
+ * port, destination port, packet contents, and previously setup tunnel
+ * context are used.
*
* Result:
- * True - if the tunnel type was detected.
- * False - if not a tunnel packet or tunnel type not supported.
- *
- * if result==True, the forwarding context gets initialized with the
- * right tunnel vport.
+ * True - If the packet is to be subjected to tunneling.
+ * In case of invalid tunnel context, the tunneling functionality is
+ * a no-op and is completed within this function itself by consuming
+ * all of the tunneling context.
+ * False - If not a tunnel packet or tunnel type not supported. Caller should
+ * process the packet as a non-tunnel packet.
* --------------------------------------------------------------------------
*/
static __inline BOOLEAN
@@ -239,16 +243,17 @@ OvsDetectTunnelPkt(OvsForwardingContext *ovsFwdCtx,
const POVS_VPORT_ENTRY dstVport,
const OvsFlowKey *flowKey) {
- /*
- * The source of NBL during tunneling Rx could be the external port or if
- * it being executed from userspace, the source port is default port.
- */
-
if (OvsIsInternalVportType(dstVport->ovsType)) {
+ /*
+ * Rx:
+ * The source of NBL during tunneling Rx could be the external
+ * port or if it is being executed from userspace, the source port is
+ * default port.
+ */
BOOLEAN validSrcPort = (ovsFwdCtx->fwdDetail->SourcePortId ==
- ovsFwdCtx->switchContext->externalPortId)
- || (ovsFwdCtx->fwdDetail->SourcePortId ==
- NDIS_SWITCH_DEFAULT_PORT_ID);
+ ovsFwdCtx->switchContext->externalPortId) ||
+ (ovsFwdCtx->fwdDetail->SourcePortId ==
+ NDIS_SWITCH_DEFAULT_PORT_ID);
if (validSrcPort && OvsDetectTunnelRxPkt(ovsFwdCtx, flowKey)) {
ASSERT(ovsFwdCtx->tunnelTxNic == NULL); @@ -258,9 +263,27 @@ OvsDetectTunnelPkt(OvsForwardingContext *ovsFwdCtx,
} else if (OvsIsTunnelVportType(dstVport->ovsType)) {
ASSERT(ovsFwdCtx->tunnelTxNic == NULL);
ASSERT(ovsFwdCtx->tunnelRxNic == NULL);
- ASSERT(ovsFwdCtx->tunKey.dst != 0);
- ovsActionStats.txVxlan++;
- ovsFwdCtx->tunnelTxNic = dstVport;
+
+ /*
+ * Tx:
+ * The destination port is a tunnel port. Encapsulation must be
+ * performed only on packets that originate from a VIF port or from
+ * userspace (default port)
+ *
+ * If the packet will not be encapsulated, consume the tunnel context
+ * by clearing it.
+ */
+ if (ovsFwdCtx->srcVportNo != 0 &&
+ !OvsIsVifVportNo(ovsFwdCtx->srcVportNo)) {
+ ovsFwdCtx->tunKey.dst = 0;
+ }
+
+ /* Tunnel the packet only if tunnel context is set. */
+ if (ovsFwdCtx->tunKey.dst != 0) {
+ ovsActionStats.txVxlan++;
+ ovsFwdCtx->tunnelTxNic = dstVport;
+ }
+
return TRUE;
}
@@ -318,7 +341,6 @@ OvsAddPorts(OvsForwardingContext *ovsFwdCtx,
NET_BUFFER_DATA_LENGTH(NET_BUFFER_LIST_FIRST_NB(ovsFwdCtx->curNbl));
if (OvsDetectTunnelPkt(ovsFwdCtx, vport, flowKey)) {
- ASSERT(ovsFwdCtx->tunnelTxNic || ovsFwdCtx->tunnelRxNic);
return NDIS_STATUS_SUCCESS;
}
diff --git a/datapath-windows/ovsext/OvsUser.c b/datapath-windows/ovsext/OvsUser.c
index 8271d52..5093f20 100644
--- a/datapath-windows/ovsext/OvsUser.c
+++ b/datapath-windows/ovsext/OvsUser.c
@@ -314,6 +314,7 @@ OvsExecuteDpIoctl(PVOID inputBuffer,
PNDIS_SWITCH_FORWARDING_DETAIL_NET_BUFFER_LIST_INFO fwdDetail;
OvsFlowKey key;
OVS_PACKET_HDR_INFO layers;
+ POVS_VPORT_ENTRY vport;
if (inputLength < sizeof(*execute) || outputLength != 0) {
return STATUS_INFO_LENGTH_MISMATCH; @@ -351,8 +352,14 @@ OvsExecuteDpIoctl(PVOID inputBuffer,
}
fwdDetail = NET_BUFFER_LIST_SWITCH_FORWARDING_DETAIL(pNbl);
- fwdDetail->SourcePortId = NDIS_SWITCH_DEFAULT_PORT_ID;
- fwdDetail->SourceNicIndex = 0;
+ vport = OvsFindVportByPortNo(gOvsSwitchContext, execute->inPort);
+ if (vport) {
+ fwdDetail->SourcePortId = vport->portId;
+ fwdDetail->SourceNicIndex = vport->nicIndex;
+ } else {
+ fwdDetail->SourcePortId = NDIS_SWITCH_DEFAULT_PORT_ID;
+ fwdDetail->SourceNicIndex = 0;
+ }
// XXX: Figure out if any of the other members of fwdDetail need to be set.
ndisStatus = OvsExtractFlow(pNbl, fwdDetail->SourcePortId, &key, &layers, @@ -362,10 +369,10 @@ OvsExecuteDpIoctl(PVOID inputBuffer,
NdisAcquireRWLockRead(gOvsSwitchContext->dispatchLock, &lockState,
NDIS_RWL_AT_DISPATCH_LEVEL);
ndisStatus = OvsActionsExecute(gOvsSwitchContext, NULL, pNbl,
- 0, // XXX: we are passing 0 for srcVportNo
- NDIS_SEND_FLAGS_SWITCH_DESTINATION_GROUP,
- &key, NULL, &layers, actions,
- execute->actionsLen);
+ vport ? vport->portNo : 0,
+ NDIS_SEND_FLAGS_SWITCH_DESTINATION_GROUP,
+ &key, NULL, &layers, actions,
+ execute->actionsLen);
pNbl = NULL;
NdisReleaseRWLock(gOvsSwitchContext->dispatchLock, &lockState);
}
diff --git a/datapath-windows/ovsext/OvsVport.h b/datapath-windows/ovsext/OvsVport.h
index 8fe23f1..4ab0019 100644
--- a/datapath-windows/ovsext/OvsVport.h
+++ b/datapath-windows/ovsext/OvsVport.h
@@ -161,6 +161,13 @@ OvsIsTunnelVportNo(UINT32 portNo)
return (idx >= OVS_TUNNEL_INDEX_START && idx <= OVS_TUNNEL_INDEX_END); }
+static __inline BOOLEAN
+OvsIsVifVportNo(UINT32 portNo)
+{
+ UINT32 idx = OVS_VPORT_INDEX(portNo);
+ return (idx >= OVS_VM_VPORT_START && idx <= OVS_VM_VPORT_MAX); }
+
static __inline POVS_VPORT_ENTRY
OvsGetTunnelVport(OVS_VPORT_TYPE type)
{
--
1.9.1
_______________________________________________
dev mailing list
dev at openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
More information about the dev
mailing list