diff mbox series

[ovs-dev,v1,2/2] datapath-windows: Alg support for ftp and tftp in conntrack

Message ID 20220624110909.88892-2-svc.ovs-community@vmware.com
State Superseded
Headers show
Series [ovs-dev,v1,1/2] datapath-windows:Fix icmp related error code. | expand

Checks

Context Check Description
ovsrobot/apply-robot success apply and check: success
ovsrobot/github-robot-_Build_and_Test success github build: passed
ovsrobot/intel-ovs-compilation success test: success

Commit Message

ldejing June 24, 2022, 11:09 a.m. UTC
From: ldejing <ldejing@vmware.com>

This patch mainly support alg field in ct action when process
ftp/tftp traffic. Tftp with alg mainly parse the tftp packet
 (IPv4/IPv6), extract connect info from the tftp packet and
 create the related connection. For ftp, previous version has
 supported process of ftp traffic. However, previous version
 regard traffic from or to port 21 as ftp traffic, this is
 incorrect in some scenario. This version adds alg field in ct for
 ftp traffic, we could use ct(alg=ftp) to process any ftp traffic
 from/to any port.

IPv4/IPv6.

Test cases:
    1) ftp ipv4/ipv6 use alg field in the normal and nat scenario.
    2) tftp ipv4/ipv6 use alg field in the normal and nat scenario.

Signed-off-by: ldejing <ldejing@vmware.com>
---
 Documentation/intro/install/windows.rst     | 186 ++++++++++++++------
 datapath-windows/ovsext/Actions.c           |   1 -
 datapath-windows/ovsext/Conntrack-ftp.c     | 109 ++++++------
 datapath-windows/ovsext/Conntrack-related.c |  30 +++-
 datapath-windows/ovsext/Conntrack-tcp.c     |  53 ++++++
 datapath-windows/ovsext/Conntrack.c         |  77 ++++++--
 datapath-windows/ovsext/Conntrack.h         |  19 +-
 include/windows/netinet/in.h                |   1 +
 8 files changed, 348 insertions(+), 128 deletions(-)
diff mbox series

Patch

diff --git a/Documentation/intro/install/windows.rst b/Documentation/intro/install/windows.rst
index 0a392d781..303e22ee6 100644
--- a/Documentation/intro/install/windows.rst
+++ b/Documentation/intro/install/windows.rst
@@ -852,78 +852,164 @@  related state.
 
    normal scenario
    Vif38(20::1, ofport:2)->Vif40(20:2, ofport:3)
-   Vif38Name="podvif38"
-   Vif40Name="podvif40"
+   Vif38Name="podvif70"
+   Vif40Name="Ethernet1"
    Vif38Port=2
-   Vif38Address="20::1"
-   Vif38MacAddressCli="00-15-5D-F0-01-0b"
+   Vif38Address="20::88"
    Vif40Port=3
-   Vif40Address="20::2"
-   Vif40MacAddressCli="00-15-5D-F0-01-0C"
+   Vif40Address="20::45"
+   Vif40MacAddressCli="00-50-56-98-9d-97"
+   Vif38MacAddressCli="00-15-5D-F0-01-0B"
    Protocol="tcp6"
-   > netsh int ipv6 set neighbors $Vif38Name $Vif40Address \
-     $Vif40MacAddressCli
-   > netsh int ipv6 set neighbors $Vif40Name $Vif38Address \
-     $Vif38MacAddressCli
-   > ovs-ofctl del-flows br-int --strict "table=0,priority=0"
-   > ovs-ofctl add-flow br-int "table=0,priority=1,$Protocol \
+   > netsh int ipv6 set neighbors $Vif38Name $Vif40Address $Vif40MacAddressCli
+   > netsh int ipv6 set neighbors $Vif42Name $Vif38Ip $Vif38MacAddressCli
+   > ovs-ofctl del-flows br-test --strict "table=0,priority=0"
+   > ovs-ofctl add-flow br-test "table=0,priority=1,$Protocol
      actions=ct(table=1)"
-   > ovs-ofctl add-flow br-int "table=1,priority=1,ct_state=+new+trk-est, \
+   > ovs-ofctl add-flow br-test "table=1,priority=1,tp_dst=21, $Protocol,\
+     actions=ct(commit,table=2,alg=ftp)"
+   > ovs-ofctl add-flow br-test "table=1,priority=1,tp_src=21, $Protocol,\
+     actions=ct(commit,table=2,alg=ftp)"
+   > ovs-ofctl add-flow br-test "table=1,priority=1, ct_state=+new+trk+rel,\
      $Protocol,actions=ct(commit,table=2)"
-   > ovs-ofctl add-flow br-int "table=1,priority=1, \
-     ct_state=-new+trk+est-rel, $Protocol,actions=ct(commit,table=2)"
-   > ovs-ofctl add-flow br-int "table=1,priority=1, \
-     ct_state=-new+trk+est+rel, $Protocol,actions=ct(commit,table=2)"
-   > ovs-ofctl add-flow br-int "table=2,priority=1,ip6, \
+   > ovs-ofctl add-flow br-test "table=1,priority=1, \
+     ct_state=-new+trk+est+rel,$Protocol,actions=ct(commit,table=2)"
+   > ovs-ofctl add-flow br-test "table=2,priority=1,ip6,\
      ipv6_dst=$Vif38Address,$Protocol,actions=output:$Vif38Port"
-   > ovs-ofctl add-flow br-int "table=2,priority=1,ip6, \
+   > ovs-ofctl add-flow br-test "table=2,priority=1,ip6,\
      ipv6_dst=$Vif40Address,$Protocol,actions=output:$Vif40Port"
 
+
 ::
 
    nat scenario
    Vif38(20::1, ofport:2) -> nat address(20::9) -> Vif42(21::3, ofport:4)
    Due to not construct flow to return neighbor mac address, we set the
    neighbor mac address manually
+   Vif38Name="podvif70"
+   Vif42Name="Ethernet1"
+   Vif38Ip="20::88"
    Vif38Port=2
-   Vif42Port=4
-   Vif38Name="podvif38"
-   Vif42Name="podvif42"
+   Vif42Port=3
    NatAddress="20::9"
    NatMacAddress="aa:bb:cc:dd:ee:ff"
    NatMacAddressForCli="aa-bb-cc-dd-ee-ff"
    Vif42Ip="21::3"
-   Vif38MacAddress="00:15:5D:F0:01:0B"
-   Vif42MacAddress="00:15:5D:F0:01:0D"
+   Vif38MacAddress="00:15:5D:F0:01:14"
+   Vif38MacAddressCli="00-15-5D-F0-01-14"
+   Vif42MacAddress="00:50:56:98:9d:97"
    Protocol="tcp6"
-   > netsh int ipv6 set neighbors $Vif38Name $NatAddress \
-     $NatMacAddressForCli
-   > netsh int ipv6 set neighbors $Vif42Name $NatAddress \
-     $NatMacAddressForCli
-   > ovs-ofctl del-flows br-int --strict "table=0,priority=0"
-   > ovs-ofctl add-flow br-int "table=0,priority=2,ipv6, \
-     dl_dst=$NatMacAddress,ct_state=-trk,$Protocol \
-     actions=ct(table=1,zone=456,nat)"
-   > ovs-ofctl add-flow br-int "table=0,priority=1,ipv6, \
-     ct_state=-trk,ip6,$Protocol actions=ct(nat, zone=456,table=1)"
-   > ovs-ofctl add-flow br-int "table=1,ipv6,in_port=$Vif38Port, \
-     ipv6_dst=$NatAddress,ct_state=+trk+new,$Protocol \
-     actions=ct(commit,nat(dst=$Vif42Ip),zone=456, \
-     exec(set_field:1->ct_mark)),mod_dl_src=$NatMacAddress, \
-     mod_dl_dst=$Vif42MacAddress,output:$Vif42Port"
-   > ovs-ofctl add-flow br-int "table=1,ipv6,ct_state=+dnat,$Protocol, \
-     action=resubmit(,2)"
-   > ovs-ofctl add-flow br-int "table=1,ipv6,ct_state=+trk+snat, \
-     $Protocol,action=resubmit(,2)"
-   > ovs-ofctl add-flow br-int "table=1,ipv6,ct_state=+trk+rel,$Protocol, \
-     action=resubmit(,2)"
-   > ovs-ofctl add-flow br-int "table=2,ipv6,in_port=$Vif38Port, \
-     ipv6_dst=$Vif42Ip,$Protocol, actions=mod_dl_src=$NatMacAddress, \
+   netsh int ipv6 set neighbors $Vif38Name $NatAddress $NatMacAddressForCli
+   netsh int ipv6 set neighbors $Vif42Name $Vif38Ip $Vif38MacAddressCli
+   > ovs-ofctl del-flows br-test --strict "table=0,priority=0"
+   > ovs-ofctl add-flow br-test "table=0,priority=2,ipv6,ipv6_dst=$NatAddress,\
+     ct_state=-trk,$Protocol actions=ct(table=1,zone=456)"
+   > ovs-ofctl add-flow br-test "table=0,priority=1,ipv6,ipv6_dst=$Vif38Ip,\
+     ct_state=-trk,ip6,$Protocol actions=ct(zone=456,table=1)"
+   > ovs-ofctl add-flow br-test "table=1,priority=2,ipv6,in_port=$Vif38Port,\
+     ipv6_dst=$NatAddress,ct_state=+trk-rel,tp_dst=21,$Protocol \
+     actions=ct(commit,alg=ftp,nat(dst=$Vif42Ip),zone=456, \
+     exec(set_field:1->ct_mark)),mod_dl_src=$NatMacAddress,\
      mod_dl_dst=$Vif42MacAddress,output:$Vif42Port"
-   > ovs-ofctl add-flow br-int "table=2,ipv6,in_port=$Vif42Port, \
-     ct_state=-new+est,ct_mark=1,ct_zone=456,$Protocol, \
-     actions=mod_dl_src=$NatMacAddress,mod_dl_dst=$Vif38MacAddress, \
+   > ovs-ofctl add-flow br-test "table=1,priority=1,ipv6,ct_state=+trk-rel,\
+     ipv6_dst=$Vif38Ip,$Protocol,action=ct(nat,alg=ftp,zone=456,table=2)"
+   > ovs-ofctl add-flow br-test "table=1,ipv6,ct_state=+trk+rel,\
+     ipv6_dst=$NatAddress,$Protocol,\
+     action=ct(table=2,commit,nat(dst=$Vif42Ip),\
+     zone=456, exec(set_field:1->ct_mark))"
+   > ovs-ofctl add-flow br-test "table=1,ipv6,ct_state=+trk+rel,$Protocol,\
+     ipv6_dst=$Vif38Ip, action=ct(nat,zone=456,table=2)"
+   > ovs-ofctl add-flow br-test "table=2,ipv6,ipv6_dst=$Vif42Ip,$Protocol,\
+     actions=mod_dl_src=$NatMacAddress, mod_dl_dst=$Vif42MacAddress,\
+     output:$Vif42Port"
+   > ovs-ofctl add-flow br-test "table=2,ipv6,ipv6_dst=$Vif38Ip,\
+     ct_state=-new+est,ct_mark=1,ct_zone=456,$Protocol,\
+     actions=mod_dl_src=$NatMacAddress,mod_dl_dst=$Vif38MacAddress,\
      output:$Vif38Port"
+   > ovs-ofctl add-flow br-test "table=2,ipv6,ipv6_dst=$Vif38Ip,\
+     ct_state=+new,ct_mark=1,ct_zone=456,$Protocol,\
+     actions=mod_dl_src=$NatMacAddress,\
+     mod_dl_dst=$Vif38MacAddress, output:$Vif38Port"
+
+Tftp same with ftp, it also contains a related connection, we could use
+following follow test the tftp connection.
+
+::
+
+   normal scenario
+   Vif38Name="podvif70"
+   Vif40Name="Ethernet1"
+   Vif38Port=2
+   Vif38Address="20::88"
+   Vif40Port=3
+   Vif40Address="20::45"
+   Vif40MacAddressCli="00-50-56-98-9d-97"
+   Vif38MacAddressCli="00-15-5D-F0-01-14"
+   Protocol="udp6"
+   netsh int ipv6 set neighbors $Vif38Name $Vif40Address $Vif40MacAddressCli
+   netsh int ipv6 set neighbors $Vif40Name $Vif38Address $Vif38MacAddressCli
+   > ovs-ofctl del-flows br-test --strict "table=0,priority=0"
+   > ovs-ofctl add-flow br-test "table=0,priority=1,$Protocol,
+     ipv6_src=$Vif38Address actions=ct(table=1)"
+   > ovs-ofctl add-flow br-test "table=0,priority=1,$Protocol,
+     ipv6_src=$Vif40Address actions=ct(table=1)"
+   > ovs-ofctl add-flow br-test "table=1,priority=1,ct_state=+new+trk-est,
+     tp_dst=69,$Protocol,udp6 actions=ct(commit,alg=tftp,table=2)"
+   > ovs-ofctl add-flow br-test "table=1,priority=1,ct_state=-new+trk+est-rel,\
+     udp6 $Protocol,actions=ct(commit,table=2)"
+   > ovs-ofctl add-flow br-test "table=1,priority=1,ct_state=-new+trk+est+rel,\
+     $Protocol,actions=ct(commit,table=2)"
+   > ovs-ofctl add-flow br-test "table=1,priority=1,ct_state=+new+trk+rel,\
+     $Protocol,actions=ct(commit,table=2)"
+   > ovs-ofctl add-flow br-test "table=2,priority=1,ip6,\
+     ipv6_dst=$Vif38Address,$Protocol,actions=output:$Vif38Port"
+   > ovs-ofctl add-flow br-test "table=2,priority=1,ip6,\
+     ipv6_dst=$Vif40Address,$Protocol,actions=output:$Vif40Port"
+
+::
+
+   nat scenario
+   Vif38Name="podvif70"
+   Vif42Name="Ethernet1"
+   Vif38Ip="20::88"
+   Vif38Port=2
+   Vif42Port=3
+   NatAddress="20::9"
+   NatMacAddress="aa:bb:cc:dd:ee:ff"
+   NatMacAddressForCli="aa-bb-cc-dd-ee-ff"
+   Vif42Ip="21::3"
+   Vif38MacAddress="00:15:5D:F0:01:14"
+   Vif38MacAddressCli="00-15-5D-F0-01-14"
+   Vif42MacAddress="00:50:56:98:9d:97"
+   Protocol="ip6"
+   netsh int ipv6 set neighbors $Vif38Name $NatAddress $NatMacAddressForCli
+   netsh int ipv6 set neighbors $Vif42Name $Vif38Ip $Vif38MacAddressCli
+   > ovs-ofctl del-flows br-test --strict "table=0,priority=0"
+   > ovs-ofctl add-flow br-test "table=0,priority=2,ipv6,\
+     dl_dst=$NatMacAddress,ct_state=-trk,$Protocol \
+     actions=ct(table=1,zone=456)"
+   > ovs-ofctl add-flow br-test "table=0,priority=1,ipv6,ct_state=-trk,ip6,\
+     $Protocol actions=ct(table=1,zone=456)"
+   > ovs-ofctl add-flow br-test "table=1,in_port=$Vif38Port,\
+     ipv6_dst=$NatAddress,ct_state=+trk+new-rel,$Protocol,udp6\
+     actions=ct(commit,alg=tftp,nat(dst=$Vif42Ip),zone=456,\
+     exec(set_field:1->ct_mark)),mod_dl_src=$NatMacAddress,\
+     mod_dl_dst=$Vif42MacAddress,output:$Vif42Port"
+   > ovs-ofctl add-flow br-test "table=1,ipv6,in_port=$Vif42Port,\
+     ipv6_dst=$Vif38Ip,ct_state=+trk+rel-rpl,$Protocol\
+     actions=ct(commit,nat(src=$NatAddress),zone=456,\
+     exec(set_field:1->ct_mark)),mod_dl_src=$NatMacAddress,\
+     mod_dl_dst=$Vif38MacAddress,output:$Vif38Port"
+   > ovs-ofctl add-flow br-test "table=1,ipv6,ct_state=+trk+rel+est+rpl,\
+     $Protocol,action=ct(nat,table=2,zone=456)"
+   > ovs-ofctl add-flow br-test "table=2,ipv6,in_port=$Vif38Port,\
+     ct_state=+rel+dnat,ipv6_dst=$Vif42Ip,$Protocol,\
+     actions=mod_dl_src=$NatMacAddress,mod_dl_dst=$Vif42MacAddress,\
+     output:$Vif42Port"
+   > ovs-ofctl add-flow br-test "table=2,ipv6,in_port=$Vif42Port,\
+     ct_state=-new+est,$Protocol,actions=mod_dl_src=$NatMacAddress,\
+     mod_dl_dst=$Vif38MacAddress,output:$Vif38Port"
+
 
 .. note::
 
diff --git a/datapath-windows/ovsext/Actions.c b/datapath-windows/ovsext/Actions.c
index 20de4db4c..3100532e1 100644
--- a/datapath-windows/ovsext/Actions.c
+++ b/datapath-windows/ovsext/Actions.c
@@ -2378,7 +2378,6 @@  OvsDoExecuteActions(POVS_SWITCH_CONTEXT switchContext,
             }
 
             OvsExecuteHash(key, (const PNL_ATTR)a);
-
             break;
         }
 
diff --git a/datapath-windows/ovsext/Conntrack-ftp.c b/datapath-windows/ovsext/Conntrack-ftp.c
index 066723685..6775496cf 100644
--- a/datapath-windows/ovsext/Conntrack-ftp.c
+++ b/datapath-windows/ovsext/Conntrack-ftp.c
@@ -122,12 +122,9 @@  OvsCtExtractNumbers(char *buf,
  *----------------------------------------------------------------------------
  */
 NDIS_STATUS
-OvsCtHandleFtp(PNET_BUFFER_LIST curNbl,
-               OvsFlowKey *key,
-               OVS_PACKET_HDR_INFO *layers,
-               UINT64 currentTime,
-               POVS_CT_ENTRY entry,
-               BOOLEAN request)
+OvsCtHandleFtp(PNET_BUFFER_LIST curNbl, OvsFlowKey *key,
+               OVS_PACKET_HDR_INFO *layers, UINT64 currentTime,
+               POVS_CT_ENTRY entry)
 {
     NDIS_STATUS status = NDIS_STATUS_SUCCESS;
     FTP_TYPE ftpType = 0;
@@ -157,52 +154,51 @@  OvsCtHandleFtp(PNET_BUFFER_LIST curNbl,
     OvsStrlcpy((char *)ftpMsg, (char *)buf, min(len, sizeof(ftpMsg)));
     char *req = NULL;
 
-    if (request) {
-        if ((len >= 5) && (OvsStrncmp("PORT", ftpMsg, 4) == 0)) {
-            ftpType = FTP_TYPE_ACTIVE;
-            req = ftpMsg + 4;
-        } else if ((len >= 5) && (OvsStrncmp("EPRT", ftpMsg, 4) == 0)) {
-            ftpType = FTP_EXTEND_TYPE_ACTIVE;
-            req = ftpMsg + 4;
+    if ((len >= 5) && (OvsStrncmp("PORT", ftpMsg, 4) == 0)) {
+        ftpType = FTP_TYPE_ACTIVE;
+        req = ftpMsg + 4;
+    } else if ((len >= 5) && (OvsStrncmp("EPRT", ftpMsg, 4) == 0)) {
+        ftpType = FTP_EXTEND_TYPE_ACTIVE;
+        req = ftpMsg + 4;
+    }
+
+    if ((len >= 4) && (OvsStrncmp(FTP_PASV_RSP_PREFIX, ftpMsg, 3) == 0)) {
+        ftpType = FTP_TYPE_PASV;
+        /* There are various formats for PASV command. We try to support
+         * some of them. This has been addressed by RFC 2428 - EPSV.
+         * Eg:
+         *    227 Entering Passive Mode (h1,h2,h3,h4,p1,p2).
+         *    227 Entering Passive Mode (h1,h2,h3,h4,p1,p2
+         *    227 Entering Passive Mode. h1,h2,h3,h4,p1,p2
+         *    227 =h1,h2,h3,h4,p1,p2
+         */
+        char *paren;
+        paren = strchr(ftpMsg, '(');
+        if (paren) {
+            req = paren + 1;
+        } else {
+            /* PASV command without ( */
+            req = ftpMsg + 3;
+        }
+    } else if ((len >= 4) && (
+               OvsStrncmp(FTP_EXTEND_PASV_RSP_PREFIX, ftpMsg, 3) == 0)) {
+        ftpType = FTP_EXTEND_TYPE_PASV;
+        /* The ftp extended passive mode only contain port info, ip address
+         * is same with the network protocol used by control connection.
+         * 229 Entering Extended Passive Mode (|||port|)
+         * */
+        char *paren;
+        paren = strchr(ftpMsg, '|');
+        if (paren) {
+            req = paren + 3;
+        } else {
+            /* Not a valid EPSV packet. */
+            return NDIS_STATUS_INVALID_PACKET;
         }
-    } else {
-        if ((len >= 4) && (OvsStrncmp(FTP_PASV_RSP_PREFIX, ftpMsg, 3) == 0)) {
-            ftpType = FTP_TYPE_PASV;
-            /* There are various formats for PASV command. We try to support
-             * some of them. This has been addressed by RFC 2428 - EPSV.
-             * Eg:
-             *    227 Entering Passive Mode (h1,h2,h3,h4,p1,p2).
-             *    227 Entering Passive Mode (h1,h2,h3,h4,p1,p2
-             *    227 Entering Passive Mode. h1,h2,h3,h4,p1,p2
-             *    227 =h1,h2,h3,h4,p1,p2
-             */
-            char *paren;
-            paren = strchr(ftpMsg, '(');
-            if (paren) {
-                req = paren + 1;
-            } else {
-                /* PASV command without ( */
-                req = ftpMsg + 3;
-            }
-        } else if ((len >= 4) && (OvsStrncmp(FTP_EXTEND_PASV_RSP_PREFIX, ftpMsg, 3) == 0)) {
-            ftpType = FTP_EXTEND_TYPE_PASV;
-            /* The ftp extended passive mode only contain port info, ip address
-             * is same with the network protocol used by control connection.
-             * 229 Entering Extended Passive Mode (|||port|)
-             * */
-            char *paren;
-            paren = strchr(ftpMsg, '|');
-            if (paren) {
-                req = paren + 3;
-            } else {
-                /* Not a valid EPSV packet. */
-                return NDIS_STATUS_INVALID_PACKET;
-            }
 
-            if (!(*req > '0' && * req < '9')) {
-                /* Not a valid port number. */
-                return NDIS_STATUS_INVALID_PACKET;
-            }
+        if (!(*req > '0' && * req < '9')) {
+            /* Not a valid port number. */
+            return NDIS_STATUS_INVALID_PACKET;
         }
     }
 
@@ -226,8 +222,15 @@  OvsCtHandleFtp(PNET_BUFFER_LIST curNbl,
                           (arr[2] << 8) | arr[3]);
         port = ntohs(((arr[4] << 8) | arr[5]));
 
-        serverIp.ipv4 = ip;
-        clientIp.ipv4 = key->ipKey.nwDst;
+        if (ftpType == FTP_TYPE_ACTIVE) {
+            serverIp.ipv4 = key->ipKey.nwDst;
+            clientIp.ipv4 = ip;
+        }
+
+        if (ftpType == FTP_TYPE_PASV) {
+            serverIp.ipv4 = ip;
+            clientIp.ipv4 = key->ipKey.nwDst;
+        }
     } else {
         if (ftpType == FTP_EXTEND_TYPE_ACTIVE) {
             /** In ftp active mode, we need to parse string like below:
@@ -239,7 +242,7 @@  OvsCtHandleFtp(PNET_BUFFER_LIST curNbl,
             char *nextHdr = NULL;
             int index = 0;
             int isIpv6AddressFamily = 0;
-            char ftpStr[1024] = {0x00};
+            char ftpStr[512] = {0x00};
 
             RtlCopyMemory(ftpStr, req, strlen(req));
             for (curHdr = ftpStr; *curHdr != '|'; curHdr++);
diff --git a/datapath-windows/ovsext/Conntrack-related.c b/datapath-windows/ovsext/Conntrack-related.c
index f985c7631..99b1553da 100644
--- a/datapath-windows/ovsext/Conntrack-related.c
+++ b/datapath-windows/ovsext/Conntrack-related.c
@@ -40,6 +40,7 @@  OvsCtRelatedKeyAreSame(OVS_CT_KEY incomingKey, OVS_CT_KEY entryKey)
     /* FTP PASV - Client initiates the connection from unknown port */
     if ((incomingKey.dl_type == entryKey.dl_type) &&
         (incomingKey.dl_type == htons(ETH_TYPE_IPV4)) &&
+        (incomingKey.nw_proto == IPPROTO_TCP) &&
         (incomingKey.dst.addr.ipv4 == entryKey.src.addr.ipv4) &&
         (incomingKey.dst.port == entryKey.src.port) &&
         (incomingKey.src.addr.ipv4 == entryKey.dst.addr.ipv4) &&
@@ -49,6 +50,7 @@  OvsCtRelatedKeyAreSame(OVS_CT_KEY incomingKey, OVS_CT_KEY entryKey)
 
     if ((incomingKey.dl_type == entryKey.dl_type) &&
         (incomingKey.dl_type == htons(ETH_TYPE_IPV6)) &&
+        (incomingKey.nw_proto == IPPROTO_TCP) &&
         !memcmp(&(incomingKey.dst.addr.ipv6), &(entryKey.src.addr.ipv6),
                sizeof(incomingKey.dst.addr.ipv6)) &&
         (incomingKey.dst.port == entryKey.src.port) &&
@@ -65,6 +67,7 @@  OvsCtRelatedKeyAreSame(OVS_CT_KEY incomingKey, OVS_CT_KEY entryKey)
      */
     if ((incomingKey.dl_type == entryKey.dl_type) &&
         (incomingKey.dl_type == htons(ETH_TYPE_IPV4)) &&
+        (incomingKey.nw_proto == IPPROTO_TCP) &&
         (incomingKey.src.addr.ipv4 == entryKey.src.addr.ipv4) &&
         (incomingKey.dst.addr.ipv4 == entryKey.dst.addr.ipv4) &&
         (incomingKey.dst.port == entryKey.dst.port) &&
@@ -74,6 +77,7 @@  OvsCtRelatedKeyAreSame(OVS_CT_KEY incomingKey, OVS_CT_KEY entryKey)
 
     if ((incomingKey.dl_type == entryKey.dl_type) &&
         (incomingKey.dl_type == htons(ETH_TYPE_IPV6)) &&
+        (incomingKey.nw_proto == IPPROTO_TCP) &&
         !memcmp(&(incomingKey.src.addr.ipv6), &(entryKey.src.addr.ipv6),
                sizeof(incomingKey.src.addr.ipv6)) &&
         !memcmp(&(incomingKey.dst.addr.ipv6), &(entryKey.dst.addr.ipv6),
@@ -83,6 +87,31 @@  OvsCtRelatedKeyAreSame(OVS_CT_KEY incomingKey, OVS_CT_KEY entryKey)
         return TRUE;
     }
 
+    /* Tftp protocol */
+    if ((incomingKey.dl_type == entryKey.dl_type) &&
+        (incomingKey.dl_type == htons(ETH_TYPE_IPV4)) &&
+        (incomingKey.nw_proto == IPPROTO_UDP) &&
+        !memcmp(&(incomingKey.src.addr.ipv4), &(entryKey.src.addr.ipv4),
+                sizeof(incomingKey.src.addr.ipv4)) &&
+        !memcmp(&(incomingKey.dst.addr.ipv4), &(entryKey.dst.addr.ipv4),
+                sizeof(incomingKey.dst.addr.ipv4)) &&
+        (incomingKey.dst.port == entryKey.dst.port) &&
+        (incomingKey.nw_proto == entryKey.nw_proto)) {
+        return TRUE;
+    }
+
+    if ((incomingKey.dl_type == entryKey.dl_type) &&
+        (incomingKey.dl_type == htons(ETH_TYPE_IPV6)) &&
+        (incomingKey.nw_proto == IPPROTO_UDP) &&
+        !memcmp(&(incomingKey.src.addr.ipv6), &(entryKey.src.addr.ipv6),
+                sizeof(incomingKey.src.addr.ipv6)) &&
+        !memcmp(&(incomingKey.dst.addr.ipv6), &(entryKey.dst.addr.ipv6),
+                sizeof(incomingKey.dst.addr.ipv6)) &&
+        (incomingKey.dst.port == entryKey.dst.port) &&
+        (incomingKey.nw_proto == entryKey.nw_proto)) {
+        return TRUE;
+    }
+
     return FALSE;
 }
 
@@ -165,7 +194,6 @@  OvsCtRelatedEntryCreate(UINT8 ipProto,
     }
 
     UINT32 hash = OvsExtractCtRelatedKeyHash(&entry->key);
-
     NdisAcquireRWLockWrite(ovsCtRelatedLockObj, &lockState, 0);
     InsertHeadList(&ovsCtRelatedTable[hash & CT_HASH_TABLE_MASK],
                    &entry->link);
diff --git a/datapath-windows/ovsext/Conntrack-tcp.c b/datapath-windows/ovsext/Conntrack-tcp.c
index a468c3e6b..77370531c 100644
--- a/datapath-windows/ovsext/Conntrack-tcp.c
+++ b/datapath-windows/ovsext/Conntrack-tcp.c
@@ -37,6 +37,8 @@ 
  */
 
 #include "Conntrack.h"
+#include "NetProto.h"
+#include "PacketParser.h"
 #include <stddef.h>
 
 struct tcp_peer {
@@ -577,3 +579,54 @@  done:
     NlMsgEndNested(nlBuf, offset);
     return status;
 }
+
+NDIS_STATUS
+OvsCtHandleTftp(PNET_BUFFER_LIST curNbl, OvsFlowKey *key,
+                OVS_PACKET_HDR_INFO *layers, UINT64 currentTime,
+                POVS_CT_ENTRY entry)
+{
+    UDPHdr udpStorage;
+    const UDPHdr *udp = NULL;
+    struct ct_addr serverIp;
+    struct ct_addr clientIp;
+    NDIS_STATUS status = NDIS_STATUS_SUCCESS;
+
+    udp = OvsGetUdp(curNbl, layers->l4Offset, &udpStorage);
+    if (!udp) {
+        return NDIS_STATUS_INVALID_PACKET;
+    }
+
+    RtlZeroMemory(&serverIp, sizeof(serverIp));
+    RtlZeroMemory(&clientIp, sizeof(clientIp));
+
+    if (OvsCtRelatedLookup(entry->key, currentTime)) {
+        return NDIS_STATUS_SUCCESS;
+    }
+
+    if (layers->isIPv4) {
+        serverIp.ipv4 = key->ipKey.nwDst;
+        clientIp.ipv4 = key->ipKey.nwSrc;
+        status = OvsCtRelatedEntryCreate(key->ipKey.nwProto,
+                                         key->l2.dlType,
+                                         serverIp,
+                                         clientIp,
+                                         0,
+                                         udp->source,
+                                         currentTime,
+                                         entry);
+    } else {
+        serverIp.ipv6 = key->ipv6Key.ipv6Dst;
+        clientIp.ipv6 = key->ipv6Key.ipv6Src;
+        status = OvsCtRelatedEntryCreate(key->ipv6Key.nwProto,
+                                         key->l2.dlType,
+                                         serverIp,
+                                         clientIp,
+                                         0,
+                                         udp->source,
+                                         currentTime,
+                                         entry);
+    }
+
+    return status;
+}
+
diff --git a/datapath-windows/ovsext/Conntrack.c b/datapath-windows/ovsext/Conntrack.c
index f56349640..cec010f28 100644
--- a/datapath-windows/ovsext/Conntrack.c
+++ b/datapath-windows/ovsext/Conntrack.c
@@ -874,13 +874,25 @@  OvsCtSetupLookupCtx(OvsFlowKey *flowKey,
         return NDIS_STATUS_INVALID_PACKET;
     }
 
+    /* It's only designed for unNat traffic, when reverse traffic comes,
+     * find the unNat table, if found the nat entry, based on the nat entry
+     * restore the conntrack, it will be stored in the ctx->key and then use the
+     * ctx->key lookup the conntrack table to find the corresponded
+     * entry with the traffic.*/
     natEntry = OvsNatLookup(&ctx->key, TRUE);
     if (natEntry) {
-        /* Translate address first for reverse NAT */
+        /* initial direction 20::1 -> 20::9,  reverse direction 21::3 -> 20::1
+         * 20::9 could be regarded as nat ip, before convert, ctx->key value
+         * is "21::3 -> 20::1", after convert, ctx->key value is
+         * "20::9->20::1" */
         ctx->key = natEntry->ctEntry->key;
         OvsCtKeyReverse(&ctx->key);
     } else {
-        if (flowKey->l2.dlType == htons(ETH_TYPE_IPV4)) {
+        if (OvsNatLookup(&ctx->key, FALSE)) {
+            /* Do nothing here, this branch here used to exclude traffic
+             * described in https://github.com/openvswitch/ovs-issues/issues/237
+             * */
+        } else if (flowKey->l2.dlType == htons(ETH_TYPE_IPV4)) {
             OvsPickupCtTupleAsLookupKey(&(ctx->key), zone, flowKey);
         }
     }
@@ -903,6 +915,18 @@  OvsDetectFtp6Packet(OvsFlowKey *key) {
              ntohs(key->ipv6Key.l4.tpSrc) == IPPORT_FTP));
 }
 
+static __inline BOOLEAN
+OvsDetectTftpPacket(OvsFlowKey *key) {
+    return (key->ipKey.nwProto == IPPROTO_UDP &&
+            (ntohs(key->ipKey.l4.tpDst) == IPPORT_TFTP));
+}
+
+static __inline BOOLEAN
+OvsDetectTftp6Packet(OvsFlowKey *key) {
+    return (key->ipv6Key.nwProto == IPPROTO_UDP &&
+            (ntohs(key->ipv6Key.l4.tpDst) == IPPORT_TFTP));
+}
+
 /*
  *----------------------------------------------------------------------------
  * OvsProcessConntrackEntry
@@ -989,7 +1013,9 @@  OvsProcessConntrackEntry(OvsForwardingContext *fwdCtx,
     if (entry) {
         NdisAcquireSpinLock(&(entry->lock));
         if ((layers->isIPv6 && key->ipv6Key.nwProto == IPPROTO_TCP) ||
-            (!(layers->isIPv6) && key->ipKey.nwProto == IPPROTO_TCP)) {
+            (!(layers->isIPv6) && key->ipKey.nwProto == IPPROTO_TCP) ||
+            (layers->isIPv6 && key->ipv6Key.nwProto == IPPROTO_UDP) ||
+            (!(layers->isIPv6) && key->ipKey.nwProto == IPPROTO_UDP)) {
             /* Update the related bit if there is a parent */
             if (entry->parent) {
                 state |= OVS_CS_F_RELATED;
@@ -1156,12 +1182,11 @@  OvsCtExecute_(OvsForwardingContext *fwdCtx,
     NDIS_STATUS status = NDIS_STATUS_SUCCESS;
     BOOLEAN triggerUpdateEvent = FALSE;
     BOOLEAN entryCreated = FALSE;
-    BOOLEAN isFtpPacket = FALSE;
-    BOOLEAN isFtpRequestDirection = FALSE;
     POVS_CT_ENTRY entry = NULL;
     POVS_CT_ENTRY parent = NULL;
     PNET_BUFFER_LIST curNbl = fwdCtx->curNbl;
     OvsConntrackKeyLookupCtx ctx = { 0 };
+    CT_HELPER_METHOD helpMethod = CT_HELPER_NONE;
     LOCK_STATE_EX lockStateTable;
     UINT64 currentTime;
     NdisGetCurrentSystemTime((LARGE_INTEGER *) &currentTime);
@@ -1241,32 +1266,52 @@  OvsCtExecute_(OvsForwardingContext *fwdCtx,
 
     OvsCtSetMarkLabel(key, entry, mark, labels, &triggerUpdateEvent);
 
+    if (helper && RtlEqualMemory(helper, "ftp", sizeof("ftp"))) {
+        helpMethod = CT_HELPER_FTP;
+    } else if (helper && RtlEqualMemory(helper, "tftp", sizeof("tftp"))) {
+        helpMethod = CT_HELPER_TFTP;
+    }
+
+    /* This code section was added to compatible with the old version,
+     * because old version regard all traffic to port 21 as ftp traffic,
+     * no need to add alg field in ct. Thus, the code was added to keep the
+     * same behavior for ftp and tftp.*/
     if (layers->isIPv6) {
-        isFtpPacket = OvsDetectFtp6Packet(key);
-        if (ntohs(key->ipv6Key.l4.tpDst) == IPPORT_FTP) {
-            isFtpRequestDirection = TRUE;
+        if (OvsDetectFtp6Packet(key)) {
+            helpMethod = CT_HELPER_FTP;
+        }
+
+        if (OvsDetectTftp6Packet(key)) {
+            helpMethod = CT_HELPER_TFTP;
         }
     } else {
-        isFtpPacket = OvsDetectFtpPacket(key);
-        if (ntohs(key->ipKey.l4.tpDst) == IPPORT_FTP) {
-            isFtpRequestDirection = TRUE;
+        if (OvsDetectFtpPacket(key)) {
+            helpMethod = CT_HELPER_FTP;
+        }
+
+        if (OvsDetectTftpPacket(key)) {
+            helpMethod = CT_HELPER_TFTP;
         }
     }
 
-    if (isFtpPacket) {
-        /* FTP parser will always be loaded */
-        status = OvsCtHandleFtp(curNbl, key, layers, currentTime, entry,
-                                isFtpRequestDirection);
+    if (helpMethod == CT_HELPER_FTP) {
+        status = OvsCtHandleFtp(curNbl, key, layers, currentTime, entry);
         if (status != NDIS_STATUS_SUCCESS) {
             OVS_LOG_ERROR("Error while parsing the FTP packet");
         }
     }
 
+    if (helpMethod == CT_HELPER_TFTP) {
+        status = OvsCtHandleTftp(curNbl, key, layers, currentTime, entry);
+        if (status != NDIS_STATUS_SUCCESS) {
+            OVS_LOG_ERROR("Error while parsing the TFTP packet");
+        }
+    }
+
     parent = entry->parent;
     /* The entry should have the same helper name with parent's */
     if (!entry->helper_name &&
         (helper || (parent && parent->helper_name))) {
-
         helper = helper ? helper : parent->helper_name;
         entry->helper_name = OvsAllocateMemoryWithTag(strlen(helper) + 1,
                                                       OVS_CT_POOL_TAG);
diff --git a/datapath-windows/ovsext/Conntrack.h b/datapath-windows/ovsext/Conntrack.h
index b68a54f30..deb51c0bc 100644
--- a/datapath-windows/ovsext/Conntrack.h
+++ b/datapath-windows/ovsext/Conntrack.h
@@ -80,6 +80,12 @@  typedef enum _NAT_ACTION {
     NAT_ACTION_DST_PORT = 1 << 4,
 } NAT_ACTION;
 
+typedef enum _CT_HELPER_METHOD {
+    CT_HELPER_NONE = 0,
+    CT_HELPER_FTP = 1,
+    CT_HELPER_TFTP = 2,
+} CT_HELPER_METHOD;
+
 typedef struct _OVS_CT_KEY {
     struct ct_endpoint src;
     struct ct_endpoint dst;
@@ -218,11 +224,10 @@  NDIS_STATUS OvsCtRelatedEntryCreate(UINT8 ipProto,
                                     UINT64 currentTime,
                                     POVS_CT_ENTRY parent);
 POVS_CT_ENTRY OvsCtRelatedLookup(OVS_CT_KEY key, UINT64 currentTime);
-
-NDIS_STATUS OvsCtHandleFtp(PNET_BUFFER_LIST curNbl,
-                           OvsFlowKey *key,
-                           OVS_PACKET_HDR_INFO *layers,
-                           UINT64 currentTime,
-                           POVS_CT_ENTRY entry,
-                           BOOLEAN request);
+NDIS_STATUS OvsCtHandleFtp(PNET_BUFFER_LIST curNbl, OvsFlowKey *key,
+                           OVS_PACKET_HDR_INFO *layers, UINT64 currentTime,
+                           POVS_CT_ENTRY entry);
+NDIS_STATUS OvsCtHandleTftp(PNET_BUFFER_LIST curNbl, OvsFlowKey *key,
+                            OVS_PACKET_HDR_INFO *layers, UINT64 currentTime,
+                            POVS_CT_ENTRY entry);
 #endif /* __OVS_CONNTRACK_H_ */
diff --git a/include/windows/netinet/in.h b/include/windows/netinet/in.h
index e4169994b..bae9f8cee 100644
--- a/include/windows/netinet/in.h
+++ b/include/windows/netinet/in.h
@@ -19,5 +19,6 @@ 
 
 #define IPPROTO_GRE 47
 #define IPPORT_FTP 21
+#define IPPORT_TFTP 69
 
 #endif /* netinet/in.h */