lib: Add support for tftp ct helper.

The kernel datapath provides support for TFTP helpers, so add support
for this ALG to the commandline and OpenFlow encoding/decoding.

Signed-off-by: Joe Stringer <joe@ovn.org>
Acked-by: Daniele Di Proietto <diproiettod@vmware.com>
Acked-by: Ben Pfaff <blp@ovn.org>

13 files changed, 178 insertions, 32 deletions

diff --git a/Documentation/intro/install/general.rst b/Documentation/intro/install/general.rst
index f40ba9163..95b70a132 100644
--- a/Documentation/intro/install/general.rst
+++ b/Documentation/intro/install/general.rst
@@ -125,6 +125,10 @@ The datapath tests for userspace and Linux datapaths also rely upon:
 
 - netcat. Several common implementations are known to work.
 
+- curl. Version 7.47.0 is known to work. Earlier versions should also work.
+
+- tftpy. Version 0.6.2 is known to work. Earlier versions should also work.
+
 The ovs-vswitchd.conf.db(5) manpage will include an E-R diagram, in formats
 other than plain text, only if you have the following:
 
diff --git a/NEWS b/NEWS
index aead685f1..580a9d7bc 100644
--- a/NEWS
+++ b/NEWS
@@ -26,6 +26,7 @@ Post-v2.6.0
        "selection_method" and related options in ovs-ofctl(8) for
        details.
      * The "sample" action now supports "ingress" and "egress" options.
+     * The "ct" action now supports the TFTP ALG where support is available.
    - ovs-ofctl:
      * 'bundle' command now supports packet-out messages.
      * New syntax for 'ovs-ofctl packet-out' command, which uses the
diff --git a/Vagrantfile b/Vagrantfile
index 1fe60ecf4..ef25c77c1 100644
--- a/Vagrantfile
+++ b/Vagrantfile
@@ -9,9 +9,9 @@ $bootstrap_fedora = <<SCRIPT
 dnf -y update
 dnf -y install autoconf automake openssl-devel libtool \
                python-twisted-core python-zope-interface \
-               desktop-file-utils groff graphviz rpmdevtools nc \
+               desktop-file-utils groff graphviz rpmdevtools nc curl \
                wget python-six pyftpdlib checkpolicy selinux-policy-devel \
-               libcap-ng-devel kernel-devel-`uname -r` ethtool
+               libcap-ng-devel kernel-devel-`uname -r` ethtool python-tftpy
 echo "search extra update built-in" >/etc/depmod.d/search_path.conf
 SCRIPT
 
@@ -23,10 +23,10 @@ aptitude -y install -R \
 		debhelper dh-autoreconf uuid-runtime \
 		autoconf automake libtool \
 		python-all python-twisted-core python-twisted-conch \
-		xdg-utils groff graphviz netcat \
+		xdg-utils groff graphviz netcat curl \
 		wget python-six ethtool \
 		libcap-ng-dev libssl-dev python-dev openssl \
-		python-pyftpdlib python-flake8 \
+		python-pyftpdlib python-flake8 python-tftpy \
 		linux-headers-`uname -r`
 SCRIPT
 
@@ -34,7 +34,7 @@ $bootstrap_centos = <<SCRIPT
 yum -y update
 yum -y install autoconf automake openssl-devel libtool \
                python-twisted-core python-zope-interface \
-               desktop-file-utils groff graphviz rpmdevtools nc \
+               desktop-file-utils groff graphviz rpmdevtools nc curl \
                wget python-six pyftpdlib checkpolicy selinux-policy-devel \
                libcap-ng-devel kernel-devel-`uname -r` ethtool
 SCRIPT
diff --git a/include/openvswitch/ofp-actions.h b/include/openvswitch/ofp-actions.h
index 870e16f34..df9025cba 100644
--- a/include/openvswitch/ofp-actions.h
+++ b/include/openvswitch/ofp-actions.h
@@ -566,6 +566,10 @@ enum nx_conntrack_flags {
 #define IPPORT_FTP  21
 #endif
 
+#if !defined(IPPORT_TFTP)
+#define IPPORT_TFTP  69
+#endif
+
 /* OFPACT_CT.
  *
  * Used for NXAST_CT. */
diff --git a/lib/ofp-actions.c b/lib/ofp-actions.c
index 41d06fa31..512b6f707 100644
--- a/lib/ofp-actions.c
+++ b/lib/ofp-actions.c
@@ -5454,10 +5454,19 @@ parse_CT(char *arg, struct ofpbuf *ofpacts,
 static void
 format_alg(int port, struct ds *s)
 {
-    if (port == IPPORT_FTP) {
+    switch(port) {
+    case IPPORT_FTP:
         ds_put_format(s, "%salg=%sftp,", colors.param, colors.end);
-    } else if (port) {
+        break;
+    case IPPORT_TFTP:
+        ds_put_format(s, "%salg=%stftp,", colors.param, colors.end);
+        break;
+    case 0:
+        /* Don't print. */
+        break;
+    default:
         ds_put_format(s, "%salg=%s%d,", colors.param, colors.end, port);
+        break;
     }
 }
 
@@ -7170,7 +7179,8 @@ ofpact_check__(enum ofputil_protocol *usable_protocols, struct ofpact *a,
 
         if (!dl_type_is_ip_any(flow->dl_type)
             || (flow->ct_state & CS_INVALID && oc->flags & NX_CT_F_COMMIT)
-            || (oc->alg == IPPORT_FTP && flow->nw_proto != IPPROTO_TCP)) {
+            || (oc->alg == IPPORT_FTP && flow->nw_proto != IPPROTO_TCP)
+            || (oc->alg == IPPORT_TFTP && flow->nw_proto != IPPROTO_UDP)) {
             /* We can't downgrade to OF1.0 and expect inconsistent CT actions
              * be silently discarded.  Instead, datapath flow install fails, so
              * it is better to flag inconsistent CT actions as hard errors. */
diff --git a/lib/ofp-parse.c b/lib/ofp-parse.c
index 553991c77..a26e8ff8c 100644
--- a/lib/ofp-parse.c
+++ b/lib/ofp-parse.c
@@ -181,6 +181,10 @@ str_to_connhelper(const char *str, uint16_t *alg)
         *alg = IPPORT_FTP;
         return NULL;
     }
+    if (!strcmp(str, "tftp")) {
+        *alg = IPPORT_TFTP;
+        return NULL;
+    }
     return xasprintf("invalid conntrack helper \"%s\"", str);
 }
 
diff --git a/ofproto/ofproto-dpif-xlate.c b/ofproto/ofproto-dpif-xlate.c
index 2977be59b..e442956d1 100644
--- a/ofproto/ofproto-dpif-xlate.c
+++ b/ofproto/ofproto-dpif-xlate.c
@@ -4550,10 +4550,16 @@ static void
 put_ct_helper(struct ofpbuf *odp_actions, struct ofpact_conntrack *ofc)
 {
     if (ofc->alg) {
-        if (ofc->alg == IPPORT_FTP) {
+        switch(ofc->alg) {
+        case IPPORT_FTP:
             nl_msg_put_string(odp_actions, OVS_CT_ATTR_HELPER, "ftp");
-        } else {
+            break;
+        case IPPORT_TFTP:
+            nl_msg_put_string(odp_actions, OVS_CT_ATTR_HELPER, "tftp");
+            break;
+        default:
             VLOG_WARN("Cannot serialize ct_helper %d\n", ofc->alg);
+            break;
         }
     }
 }
diff --git a/tests/atlocal.in b/tests/atlocal.in
index 5b151c3d8..c0b28938c 100644
--- a/tests/atlocal.in
+++ b/tests/atlocal.in
@@ -117,12 +117,24 @@ if test "$IS_WIN32" = "yes"; then
     HAVE_PYTHON3="no"
 fi
 
-if test "$HAVE_PYTHON" = "yes" \
-   && test "x`$PYTHON $abs_top_srcdir/tests/test-l7.py --help | grep 'ftp'`" != x; then
-    HAVE_PYFTPDLIB="yes"
-else
-    HAVE_PYFTPDLIB="no"
-fi
+find_l7_lib()
+{
+    set +x
+    var=HAVE_`echo "$1" | tr '[a-z]' '[A-Z]'`
+    if test "$HAVE_PYTHON" = "yes"; then
+        result=$($PYTHON $abs_top_srcdir/tests/test-l7.py --help | grep "$1")
+        if test "x${result}" != x; then
+            eval ${var}="yes"
+        else
+            eval ${var}="no"
+        fi
+    else
+        eval ${var}="no"
+    fi
+}
+
+find_l7_lib ftp
+find_l7_lib tftp
 
 # Look for a commnand in the system. If it is found, defines
 # HAVE_COMMAND="yes", otherwise HAVE_COMMAND="no".
@@ -148,6 +160,8 @@ else
     NC_EOF_OPT="-q 1"
 fi
 
+CURL_OPT="-g -v --max-time 1 --retry 2 --retry-delay 1 --connect-timeout 1"
+
 # Turn off proxies.
 unset http_proxy
 unset https_proxy
diff --git a/tests/odp.at b/tests/odp.at
index 64aabe1d6..019897c2d 100644
--- a/tests/odp.at
+++ b/tests/odp.at
@@ -320,6 +320,7 @@ ct(commit,zone=5)
 ct(commit,mark=0xa0a0a0a0/0xfefefefe)
 ct(commit,label=0x1234567890abcdef1234567890abcdef/0xf1f2f3f4f5f6f7f8f9f0fafbfcfdfeff)
 ct(commit,helper=ftp)
+ct(commit,helper=tftp)
 ct(nat)
 ct(commit,nat(src))
 ct(commit,nat(dst))
diff --git a/tests/ofp-actions.at b/tests/ofp-actions.at
index db7385489..3881f9f82 100644
--- a/tests/ofp-actions.at
+++ b/tests/ofp-actions.at
@@ -201,6 +201,9 @@ ffff 0018 00002320 0007 001f 00010004 000000000000f009
 # actions=ct(alg=ftp)
 ffff 0018 00002320 0023 0000 00000000 0000 FF 000000 0015
 
+# actions=ct(alg=tftp)
+ffff 0018 00002320 0023 0000 00000000 0000 FF 000000 0045
+
 # actions=ct(commit,nat(src))
 ffff 0028 00002320 0023 0001 00000000 0000 FF 000000 0000 dnl
 ffff 0010 00002320 0024 00 00 0001 0000
diff --git a/tests/system-traffic.at b/tests/system-traffic.at
index a5023d38e..29dd6d632 100644
--- a/tests/system-traffic.at
+++ b/tests/system-traffic.at
@@ -1987,7 +1987,7 @@ OVS_TRAFFIC_VSWITCHD_STOP
 AT_CLEANUP
 
 AT_SETUP([conntrack - FTP])
-AT_SKIP_IF([test $HAVE_PYFTPDLIB = no])
+AT_SKIP_IF([test $HAVE_FTP = no])
 CHECK_CONNTRACK()
 CHECK_CONNTRACK_ALG()
 OVS_TRAFFIC_VSWITCHD_START()
@@ -2072,7 +2072,7 @@ OVS_TRAFFIC_VSWITCHD_STOP
 AT_CLEANUP
 
 AT_SETUP([conntrack - FTP over IPv6])
-AT_SKIP_IF([test $HAVE_PYFTPDLIB = no])
+AT_SKIP_IF([test $HAVE_FTP = no])
 CHECK_CONNTRACK()
 CHECK_CONNTRACK_ALG()
 OVS_TRAFFIC_VSWITCHD_START()
@@ -2127,7 +2127,7 @@ OVS_TRAFFIC_VSWITCHD_STOP
 AT_CLEANUP
 
 AT_SETUP([conntrack - FTP with multiple expectations])
-AT_SKIP_IF([test $HAVE_PYFTPDLIB = no])
+AT_SKIP_IF([test $HAVE_FTP = no])
 CHECK_CONNTRACK()
 CHECK_CONNTRACK_ALG()
 OVS_TRAFFIC_VSWITCHD_START()
@@ -2192,6 +2192,80 @@ tcp,orig=(src=10.1.1.1,dst=10.1.1.2,sport=<cleared>,dport=<cleared>),reply=(src=
 OVS_TRAFFIC_VSWITCHD_STOP
 AT_CLEANUP
 
+AT_SETUP([conntrack - TFTP])
+AT_SKIP_IF([test $HAVE_TFTP = no])
+CHECK_CONNTRACK()
+CHECK_CONNTRACK_ALG()
+OVS_TRAFFIC_VSWITCHD_START()
+
+ADD_NAMESPACES(at_ns0, at_ns1)
+
+ADD_VETH(p0, at_ns0, br0, "10.1.1.1/24")
+ADD_VETH(p1, at_ns1, br0, "10.1.1.2/24")
+
+dnl Allow any traffic from ns0->ns1. Only allow nd, return traffic from ns1->ns0.
+AT_DATA([flows1.txt], [dnl
+table=0,priority=1,action=drop
+table=0,priority=10,arp,action=normal
+table=0,priority=10,icmp,action=normal
+table=0,priority=100,in_port=1,udp,action=ct(alg=tftp,commit),2
+table=0,priority=100,in_port=2,udp,action=ct(table=1)
+table=1,in_port=2,udp,ct_state=+trk+est,action=1
+table=1,in_port=2,udp,ct_state=+trk+rel,action=1
+])
+
+dnl Similar policy but without allowing all traffic from ns0->ns1.
+AT_DATA([flows2.txt], [dnl
+table=0,priority=1,action=drop
+table=0,priority=10,arp,action=normal
+table=0,priority=10,icmp,action=normal
+
+dnl Allow outgoing UDP connections, and treat them as TFTP
+table=0,priority=100,in_port=1,udp,action=ct(table=1)
+table=1,in_port=1,udp,ct_state=+trk+new-rel,action=ct(commit,alg=tftp),2
+table=1,in_port=1,udp,ct_state=+trk+new+rel,action=ct(commit),2
+table=1,in_port=1,udp,ct_state=+trk+est,action=2
+
+dnl Allow incoming TFTP data connections and responses to existing connections
+table=0,priority=100,in_port=2,udp,action=ct(table=1)
+table=1,in_port=2,udp,ct_state=+trk+est,action=1
+table=1,in_port=2,udp,ct_state=+trk+new+rel,action=1
+])
+
+AT_CHECK([ovs-ofctl --bundle replace-flows br0 flows1.txt])
+
+OVS_START_L7([at_ns0], [tftp])
+OVS_START_L7([at_ns1], [tftp])
+
+dnl TFTP requests from p1->p0 should fail due to network failure.
+NS_CHECK_EXEC([at_ns1], [[curl $CURL_OPT tftp://10.1.1.1/flows1.txt -o foo 2>curl0.log]], [28])
+AT_CHECK([ovs-appctl dpctl/dump-conntrack | FORMAT_CT(10.1.1.1)], [0], [dnl
+])
+
+dnl TFTP requests from p0->p1 should work fine.
+NS_CHECK_EXEC([at_ns0], [[curl $CURL_OPT tftp://10.1.1.2/flows1.txt -o foo 2>curl1.log]])
+AT_CHECK([ovs-appctl dpctl/dump-conntrack | FORMAT_CT(10.1.1.2)], [0], [dnl
+udp,orig=(src=10.1.1.1,dst=10.1.1.2,sport=<cleared>,dport=<cleared>),reply=(src=10.1.1.2,dst=10.1.1.1,sport=<cleared>,dport=<cleared>),helper=tftp
+])
+
+dnl Try the second set of flows.
+AT_CHECK([ovs-ofctl --bundle replace-flows br0 flows2.txt])
+AT_CHECK([ovs-appctl dpctl/flush-conntrack])
+
+dnl TFTP requests from p1->p0 should fail due to network failure.
+NS_CHECK_EXEC([at_ns1], [[curl $CURL_OPT tftp://10.1.1.1/flows1.txt -o foo 2>curl2.log]], [28])
+AT_CHECK([ovs-appctl dpctl/dump-conntrack | FORMAT_CT(10.1.1.1)], [0], [dnl
+])
+
+dnl TFTP requests from p0->p1 should work fine.
+NS_CHECK_EXEC([at_ns0], [[curl $CURL_OPT tftp://10.1.1.2/flows1.txt -o foo 2>curl3.log]])
+AT_CHECK([ovs-appctl dpctl/dump-conntrack | FORMAT_CT(10.1.1.2)], [0], [dnl
+udp,orig=(src=10.1.1.1,dst=10.1.1.2,sport=<cleared>,dport=<cleared>),reply=(src=10.1.1.2,dst=10.1.1.1,sport=<cleared>,dport=<cleared>),helper=tftp
+])
+
+OVS_TRAFFIC_VSWITCHD_STOP
+AT_CLEANUP
+
 AT_BANNER([conntrack - NAT])
 
 AT_SETUP([conntrack - simple SNAT])
@@ -2524,7 +2598,7 @@ dnl Checks the implementation of conntrack with FTP ALGs in combination with
 dnl NAT, using the provided flow table.
 m4_define([CHECK_FTP_NAT],
    [AT_SETUP([conntrack - FTP NAT $1])
-    AT_SKIP_IF([test $HAVE_PYFTPDLIB = no])
+    AT_SKIP_IF([test $HAVE_FTP = no])
     CHECK_CONNTRACK()
     CHECK_CONNTRACK_NAT()
 
@@ -2736,7 +2810,7 @@ AT_CLEANUP
 
 
 AT_SETUP([conntrack - IPv6 FTP with NAT])
-AT_SKIP_IF([test $HAVE_PYFTPDLIB = no])
+AT_SKIP_IF([test $HAVE_FTP = no])
 CHECK_CONNTRACK()
 CHECK_CONNTRACK_NAT()
 OVS_TRAFFIC_VSWITCHD_START()
diff --git a/tests/test-l7.py b/tests/test-l7.py
index aed34f411..e5f473b56 100755
--- a/tests/test-l7.py
+++ b/tests/test-l7.py
@@ -1,4 +1,4 @@
-# Copyright (c) 2015 Nicira, Inc.
+# Copyright (c) 2015, 2016 Nicira, Inc.
 #
 # Licensed under the Apache License, Version 2.0 (the "License");
 # you may not use this file except in compliance with the License.
@@ -48,24 +48,42 @@ def get_ftpd():
     return server
 
 
+def get_tftpd():
+    try:
+        from tftpy import TftpServer, TftpShared
+
+        class OVSTFTPServer(TftpServer):
+            def __init__(self, listen, handler=None):
+                (ip, port) = listen
+                self.ip = ip
+                self.port = port
+                TftpServer.__init__(self, tftproot='./')
+
+            def serve_forever(self):
+                self.listen(self.ip, self.port)
+        server = [OVSTFTPServer, None, TftpShared.DEF_TFTP_PORT]
+    except ImportError:
+        server = None
+        pass
+    return server
+
+
 def main():
     SERVERS = {
         'http': [TCPServer, SimpleHTTPRequestHandler, 80],
         'http6': [TCPServerV6, SimpleHTTPRequestHandler, 80],
+        'ftp': get_ftpd(),
+        'tftp': get_tftpd(),
     }
 
-    ftpd = get_ftpd()
-    if ftpd is not None:
-        SERVERS['ftp'] = ftpd
-
-    protocols = [srv for srv in SERVERS]
+    protocols = [srv for srv in SERVERS if SERVERS[srv] is not None]
     parser = argparse.ArgumentParser(
             description='Run basic application servers.')
     parser.add_argument('proto', default='http', nargs='?',
             help='protocol to serve (%s)' % protocols)
     args = parser.parse_args()
 
-    if args.proto not in SERVERS:
+    if args.proto not in protocols:
         parser.print_help()
         exit(1)
 
diff --git a/utilities/ovs-ofctl.8.in b/utilities/ovs-ofctl.8.in
index af1eb2b7b..49b3aa5f7 100644
--- a/utilities/ovs-ofctl.8.in
+++ b/utilities/ovs-ofctl.8.in
@@ -1840,12 +1840,19 @@ The \fBcommit\fR parameter must be specified to use \fBexec(...)\fR.
 .
 .IP \fBalg=\fIalg\fR
 Specify application layer gateway \fIalg\fR to track specific connection
-types. Supported types include:
+types. If subsequent related connections are sent through the \fBct\fR
+action, then the \fBrel\fR flag in the \fBct_state\fR field will be set.
+Supported types include:
 .RS
 .IP \fBftp\fR
-Look for negotiation of FTP data connections. If a subsequent FTP data
-connection arrives which is related, the \fBct\fR action will set the
-\fBrel\fR flag in the \fBct_state\fR field for packets sent through \fBct\fR.
+Look for negotiation of FTP data connections. Specify this option for FTP
+control connections to detect related data connections and populate the
+\fBrel\fR flag for the data connections.
+.
+.IP \fBtftp\fR
+Look for negotiation of TFTP data connections. Specify this option for TFTP
+control connections to detect related data connections and populate the
+\fBrel\fR flag for the data connections.
 .RE
 .
 .IP