datapath-windows: use ip proto for tunnel port lookup

In Actions.c, based on the IP Protocol type and L4 port of
the outer packet, we lookup the tunnel port. The function
that made this happen took the tunnel type as an argument.
Semantically, is is better to pass the IP protocol type and
let the lookup code map IP protocol type to tunnel type.

In the vport add code, we make sure that we block tunnel
port addition if there's already a tunnel port that uses
the same IP protocol type and L4 port number.

Signed-off-by: Nithin Raju <nithin@vmware.com>
Acked-by: Sairam Venugopal <vsairam@vmware.com>
Acked-by: Yin Lin <linyi@vmware.com>
Acked-by: Alin Gabriel Serdean <aserdean@cloudbasesolutions.com>
Signed-off-by: Gurucharan Shetty <guru@ovn.org>

diff --git a/datapath-windows/ovsext/Actions.c b/datapath-windows/ovsext/Actions.c
index 72ebc040c377d69dc2fda400da282c8505464f7e..7ac6bb711d14ba066971d59db9130e3dcbe553b9 100644 (file)
--- a/datapath-windows/ovsext/Actions.c
+++ b/datapath-windows/ovsext/Actions.c
@@ -215,32 +215,32 @@ OvsDetectTunnelRxPkt(OvsForwardingContext *ovsFwdCtx,
     /* XXX: we should also check for the length of the UDP payload to pick
      * packets only if they are at least VXLAN header size.
      */
+
+    /*
+     * For some of the tunnel types such as GRE, the dstPort is not applicable
+     * since GRE does not have a L4 port. We use '0' for convenience.
+     */
     if (!flowKey->ipKey.nwFrag) {
         UINT16 dstPort = htons(flowKey->ipKey.l4.tpDst);
-        switch (flowKey->ipKey.nwProto) {
-        case IPPROTO_GRE:
-            tunnelVport = OvsFindTunnelVportByPortType(ovsFwdCtx->switchContext,
-                                                       OVS_VPORT_TYPE_GRE);
-            if (tunnelVport) {
-                ovsActionStats.rxGre++;
-            }
-            break;
-        case IPPROTO_TCP:
-            tunnelVport = OvsFindTunnelVportByDstPort(ovsFwdCtx->switchContext,
-                                                      dstPort,
-                                                      OVS_VPORT_TYPE_STT);
-            if (tunnelVport) {
+
+        ASSERT(flowKey->ipKey.nwProto != IPPROTO_GRE || dstPort == 0);
+
+        tunnelVport =
+            OvsFindTunnelVportByDstPortAndNWProto(ovsFwdCtx->switchContext,
+                                                  dstPort,
+                                                  flowKey->ipKey.nwProto);
+        if (tunnelVport) {
+            switch(tunnelVport->ovsType) {
+            case OVS_VPORT_TYPE_STT:
                 ovsActionStats.rxStt++;
-            }
-            break;
-        case IPPROTO_UDP:
-            tunnelVport = OvsFindTunnelVportByDstPort(ovsFwdCtx->switchContext,
-                                                      dstPort,
-                                                      OVS_VPORT_TYPE_VXLAN);
-            if (tunnelVport) {
+                break;
+            case OVS_VPORT_TYPE_VXLAN:
                 ovsActionStats.rxVxlan++;
+                break;
+            case OVS_VPORT_TYPE_GRE:
+                ovsActionStats.rxGre++;
+                break;
             }
-            break;
         }
     }
 

diff --git a/datapath-windows/ovsext/Tunnel.c b/datapath-windows/ovsext/Tunnel.c
index 97d202099ef44afe5c797b21bf915ab4c29b01bf..c5aae1a51c3783416d6b6b49228597ee71dc1290 100644 (file)
--- a/datapath-windows/ovsext/Tunnel.c
+++ b/datapath-windows/ovsext/Tunnel.c
@@ -285,9 +285,9 @@ OvsInjectPacketThroughActions(PNET_BUFFER_LIST pNbl,
 
         SendFlags |= NDIS_SEND_FLAGS_DISPATCH_LEVEL;
 
-        vport = OvsFindTunnelVportByDstPort(gOvsSwitchContext,
-                                            htons(tunnelKey.dst_port),
-                                            OVS_VPORT_TYPE_VXLAN);
+        vport = OvsFindTunnelVportByDstPortAndType(gOvsSwitchContext,
+                                                   htons(tunnelKey.dst_port),
+                                                   OVS_VPORT_TYPE_VXLAN);
 
         if (vport == NULL){
             status = STATUS_UNSUCCESSFUL;

diff --git a/datapath-windows/ovsext/Vport.c b/datapath-windows/ovsext/Vport.c
index b4234e9634ecd3742010bd7db19fda65bc500092..fbab61b70d2363e719bad279556cc24da0e65d6f 100644 (file)
--- a/datapath-windows/ovsext/Vport.c
+++ b/datapath-windows/ovsext/Vport.c
@@ -689,9 +689,9 @@ OvsFindVportByPortNo(POVS_SWITCH_CONTEXT switchContext,
 
 
 POVS_VPORT_ENTRY
-OvsFindTunnelVportByDstPort(POVS_SWITCH_CONTEXT switchContext,
-                            UINT16 dstPort,
-                            OVS_VPORT_TYPE ovsPortType)
+OvsFindTunnelVportByDstPortAndType(POVS_SWITCH_CONTEXT switchContext,
+                                   UINT16 dstPort,
+                                   OVS_VPORT_TYPE ovsPortType)
 {
     POVS_VPORT_ENTRY vport;
     PLIST_ENTRY head, link;
@@ -708,6 +708,41 @@ OvsFindTunnelVportByDstPort(POVS_SWITCH_CONTEXT switchContext,
     return NULL;
 }
 
+POVS_VPORT_ENTRY
+OvsFindTunnelVportByDstPortAndNWProto(POVS_SWITCH_CONTEXT switchContext,
+                                      UINT16 dstPort,
+                                      UINT8 nwProto)
+{
+    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 (GetPortFromPriv(vport) == dstPort) {
+            switch (nwProto) {
+            case IPPROTO_UDP:
+                if (vport->ovsType != OVS_VPORT_TYPE_VXLAN) {
+                    continue;
+                }
+                break;
+            case IPPROTO_TCP:
+                if (vport->ovsType != OVS_VPORT_TYPE_STT) {
+                    continue;
+                }
+                break;
+            case IPPROTO_GRE:
+                break;
+            default:
+                continue;
+            }
+            return vport;
+        }
+    }
+    return NULL;
+}
+
 POVS_VPORT_ENTRY
 OvsFindTunnelVportByPortType(POVS_SWITCH_CONTEXT switchContext,
                              OVS_VPORT_TYPE ovsPortType)
@@ -2220,15 +2255,20 @@ OvsNewVportCmdHandler(POVS_USER_PARAMS_CONTEXT usrParamsCtx,
 
         if (OvsIsTunnelVportType(portType)) {
             UINT16 transportPortDest = 0;
+            UINT8 nwProto;
+            POVS_VPORT_ENTRY dupVport;
 
             switch (portType) {
             case OVS_VPORT_TYPE_GRE:
+                nwProto = IPPROTO_GRE;
                 break;
             case OVS_VPORT_TYPE_VXLAN:
                 transportPortDest = VXLAN_UDP_PORT;
+                nwProto = IPPROTO_UDP;
                 break;
             case OVS_VPORT_TYPE_STT:
                 transportPortDest = STT_TCP_PORT;
+                nwProto = IPPROTO_TCP;
                 break;
             default:
                 nlError = NL_ERROR_INVAL;
@@ -2243,6 +2283,22 @@ OvsNewVportCmdHandler(POVS_USER_PARAMS_CONTEXT usrParamsCtx,
                 }
             }
 
+            /*
+             * We don't allow two tunnel ports on identical N/W protocol and
+             * L4 port number. This is applicable even if the two ports are of
+             * different tunneling types.
+             */
+            dupVport =
+                OvsFindTunnelVportByDstPortAndNWProto(gOvsSwitchContext,
+                                                      transportPortDest,
+                                                      nwProto);
+            if (dupVport) {
+                OVS_LOG_ERROR("Vport for N/W proto and port already exists,"
+                    " type: %u, dst port: %u, name: %s", dupVport->ovsType,
+                    transportPortDest, dupVport->ovsName);
+                goto Cleanup;
+            }
+
             status = OvsInitTunnelVport(usrParamsCtx,
                                         vport,
                                         portType,
@@ -2317,6 +2373,8 @@ OvsNewVportCmdHandler(POVS_USER_PARAMS_CONTEXT usrParamsCtx,
                                    gOvsSwitchContext->dpNo);
 
     *replyLen = msgOut->nlMsg.nlmsgLen;
+    OVS_LOG_INFO("Created new vport, name: %s, type: %u", vport->ovsName,
+                 vport->ovsType);
 
 Cleanup:
     NdisReleaseRWLock(gOvsSwitchContext->dispatchLock, &lockState);

diff --git a/datapath-windows/ovsext/Vport.h b/datapath-windows/ovsext/Vport.h
index 373896ddd3994e519719af0a42b46343c36269b1..f0a9acd9ef87c7b399c3b82aa33f24b0730b8bd1 100644 (file)
--- a/datapath-windows/ovsext/Vport.h
+++ b/datapath-windows/ovsext/Vport.h
@@ -145,9 +145,12 @@ 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,
-                                             OVS_VPORT_TYPE ovsVportType);
+POVS_VPORT_ENTRY OvsFindTunnelVportByDstPortAndType(POVS_SWITCH_CONTEXT switchContext,
+                                                    UINT16 dstPort,
+                                                    OVS_VPORT_TYPE ovsPortType);
+POVS_VPORT_ENTRY OvsFindTunnelVportByDstPortAndNWProto(POVS_SWITCH_CONTEXT switchContext,
+                                                       UINT16 dstPort,
+                                                       UINT8 nwProto);
 POVS_VPORT_ENTRY OvsFindTunnelVportByPortType(POVS_SWITCH_CONTEXT switchContext,
                                               OVS_VPORT_TYPE ovsPortType);