From patchwork Wed Jan  9 15:33:08 2019
Content-Type: text/plain; charset="utf-8"
MIME-Version: 1.0
Content-Transfer-Encoding: 7bit
X-Patchwork-Submitter: David Marchand <david.marchand@redhat.com>
X-Patchwork-Id: 1022488
Return-Path: <ovs-dev-bounces@openvswitch.org>
X-Original-To: incoming@patchwork.ozlabs.org
Delivered-To: patchwork-incoming@bilbo.ozlabs.org
Authentication-Results: ozlabs.org;
	spf=pass (mailfrom) smtp.mailfrom=openvswitch.org
	(client-ip=140.211.169.12; helo=mail.linuxfoundation.org;
	envelope-from=ovs-dev-bounces@openvswitch.org;
	receiver=<UNKNOWN>)
Authentication-Results: ozlabs.org;
	dmarc=fail (p=none dis=none) header.from=redhat.com
Received: from mail.linuxfoundation.org (mail.linuxfoundation.org
	[140.211.169.12])
	(using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256
	bits)) (No client certificate requested)
	by ozlabs.org (Postfix) with ESMTPS id 43ZY7q3xc0z9sCs
	for <incoming@patchwork.ozlabs.org>;
	Thu, 10 Jan 2019 02:33:59 +1100 (AEDT)
Received: from mail.linux-foundation.org (localhost [127.0.0.1])
	by mail.linuxfoundation.org (Postfix) with ESMTP id 84558DB9;
	Wed,  9 Jan 2019 15:33:27 +0000 (UTC)
X-Original-To: dev@openvswitch.org
Delivered-To: ovs-dev@mail.linuxfoundation.org
Received: from smtp1.linuxfoundation.org (smtp1.linux-foundation.org
	[172.17.192.35])
	by mail.linuxfoundation.org (Postfix) with ESMTPS id 21F78DB8
	for <dev@openvswitch.org>; Wed,  9 Jan 2019 15:33:27 +0000 (UTC)
X-Greylist: domain auto-whitelisted by SQLgrey-1.7.6
Received: from mx1.redhat.com (mx1.redhat.com [209.132.183.28])
	by smtp1.linuxfoundation.org (Postfix) with ESMTPS id 53110850
	for <dev@openvswitch.org>; Wed,  9 Jan 2019 15:33:26 +0000 (UTC)
Received: from smtp.corp.redhat.com
	(int-mx03.intmail.prod.int.phx2.redhat.com [10.5.11.13])
	(using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits))
	(No client certificate requested)
	by mx1.redhat.com (Postfix) with ESMTPS id C60F8C0B2A2A;
	Wed,  9 Jan 2019 15:33:25 +0000 (UTC)
Received: from dmarchan.remote.csb (ovpn-117-97.ams2.redhat.com
	[10.36.117.97])
	by smtp.corp.redhat.com (Postfix) with ESMTP id B013F608E1;
	Wed,  9 Jan 2019 15:33:24 +0000 (UTC)
From: David Marchand <david.marchand@redhat.com>
To: dev@openvswitch.org
Date: Wed,  9 Jan 2019 16:33:08 +0100
Message-Id: <1547047989-28356-2-git-send-email-david.marchand@redhat.com>
In-Reply-To: <1547047989-28356-1-git-send-email-david.marchand@redhat.com>
References: <1545312814-23634-1-git-send-email-david.marchand@redhat.com>
	<1547047989-28356-1-git-send-email-david.marchand@redhat.com>
X-Scanned-By: MIMEDefang 2.79 on 10.5.11.13
X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-4.5.16
	(mx1.redhat.com [10.5.110.31]);
	Wed, 09 Jan 2019 15:33:25 +0000 (UTC)
X-Spam-Status: No, score=-6.9 required=5.0 tests=BAYES_00,RCVD_IN_DNSWL_HI
	autolearn=ham version=3.3.1
X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on
	smtp1.linux-foundation.org
Subject: [ovs-dev] [PATCH v3 1/2] conntrack: fix tcp seq adjustments when
	mangling commands
X-BeenThere: ovs-dev@openvswitch.org
X-Mailman-Version: 2.1.12
Precedence: list
List-Id: <ovs-dev.openvswitch.org>
List-Unsubscribe: <https://mail.openvswitch.org/mailman/options/ovs-dev>,
	<mailto:ovs-dev-request@openvswitch.org?subject=unsubscribe>
List-Archive: <http://mail.openvswitch.org/pipermail/ovs-dev/>
List-Post: <mailto:ovs-dev@openvswitch.org>
List-Help: <mailto:ovs-dev-request@openvswitch.org?subject=help>
List-Subscribe: <https://mail.openvswitch.org/mailman/listinfo/ovs-dev>,
	<mailto:ovs-dev-request@openvswitch.org?subject=subscribe>
MIME-Version: 1.0
Sender: ovs-dev-bounces@openvswitch.org
Errors-To: ovs-dev-bounces@openvswitch.org

The ftp alg deals with packets in two ways for the command connection:
either they are inspected and can be mangled when nat is enabled
(CT_FTP_CTL_INTEREST) or they just go through without being modified
(CT_FTP_CTL_OTHER).

For CT_FTP_CTL_INTEREST packets, we must both adjust the packet tcp seq
number by the connection current offset, then prepare for the next
packets by setting an accumulated offset in the ct object.

The tests are updated so that ftp+NAT checks send multiple commands in a
single tcp command connection: wget is not able to do this, so switch to
lftp.

Signed-off-by: David Marchand <david.marchand@redhat.com>
---
Changelog since v2:
- skip tests relying on lftp when absent
- removed unneeded temp seq_skew variable by moving conn_seq_skew_set
  at the end of the function and rely on the value in the conn object,
  then removed unneeded do_seq_skew_adj

---
 Vagrantfile             |  9 ++++---
 Vagrantfile-FreeBSD     |  2 +-
 lib/conntrack.c         | 69 ++++++++++++++++++++++++-------------------------
 tests/atlocal.in        |  3 +++
 tests/system-traffic.at | 12 ++++++++-
 5 files changed, 55 insertions(+), 40 deletions(-)

diff --git a/Vagrantfile b/Vagrantfile
index 0192f66..fbd772a 100644
--- a/Vagrantfile
+++ b/Vagrantfile
@@ -12,7 +12,8 @@ dnf -y install autoconf automake openssl-devel libtool \
                python-twisted python-zope-interface \
                desktop-file-utils groff graphviz rpmdevtools nc curl \
                wget python-six pyftpdlib checkpolicy selinux-policy-devel \
-               libcap-ng-devel kernel-devel-`uname -r` ethtool python-tftpy
+               libcap-ng-devel kernel-devel-`uname -r` ethtool python-tftpy \
+               lftp
 echo "search extra update built-in" >/etc/depmod.d/search_path.conf
 SCRIPT
 
@@ -28,7 +29,8 @@ aptitude -y install -R \
                 wget python-six ethtool \
                 libcap-ng-dev libssl-dev python-dev openssl \
                 python-pyftpdlib python-flake8 python-tftpy \
-                linux-headers-`uname -r`
+                linux-headers-`uname -r` \
+                lftp
 SCRIPT
 
 $bootstrap_centos = <<SCRIPT
@@ -37,7 +39,8 @@ yum -y install autoconf automake openssl-devel libtool \
                python-twisted-core python-zope-interface \
                desktop-file-utils groff graphviz rpmdevtools nc curl \
                wget python-six pyftpdlib checkpolicy selinux-policy-devel \
-               libcap-ng-devel kernel-devel-`uname -r` ethtool net-tools
+               libcap-ng-devel kernel-devel-`uname -r` ethtool net-tools \
+               lftp
 SCRIPT
 
 $configure_ovs = <<SCRIPT
diff --git a/Vagrantfile-FreeBSD b/Vagrantfile-FreeBSD
index 8f00abe..52599ee 100644
--- a/Vagrantfile-FreeBSD
+++ b/Vagrantfile-FreeBSD
@@ -12,7 +12,7 @@ Vagrant.require_version ">=1.7.0"
 $bootstrap_freebsd = <<SCRIPT
 sed  -e 's/\#DEFAULT_ALWAYS_YES = false/DEFAULT_ALWAYS_YES = true/g' -e 's/\#ASSUME_ALWAYS_YES = false/ASSUME_ALWAYS_YES = true/g' /usr/local/etc/pkg.conf > /tmp/pkg.conf
 mv -f /tmp/pkg.conf /usr/local/etc/pkg.conf
-pkg install automake libtool wget python py27-six gmake
+pkg install automake libtool wget python py27-six gmake lftp
 SCRIPT
 
 $configure_ovs = <<SCRIPT
diff --git a/lib/conntrack.c b/lib/conntrack.c
index 6f6021a..4717950 100644
--- a/lib/conntrack.c
+++ b/lib/conntrack.c
@@ -3170,9 +3170,8 @@ repl_ftp_v6_addr(struct dp_packet *pkt, struct ct_addr v6_addr_rep,
 
 static void
 handle_ftp_ctl(struct conntrack *ct, const struct conn_lookup_ctx *ctx,
-               struct dp_packet *pkt,
-               const struct conn *conn_for_expectation,
-               long long now, enum ftp_ctl_pkt ftp_ctl, bool nat)
+               struct dp_packet *pkt, const struct conn *ec, long long now,
+               enum ftp_ctl_pkt ftp_ctl, bool nat)
 {
     struct ip_header *l3_hdr = dp_packet_l3(pkt);
     ovs_be32 v4_addr_rep = 0;
@@ -3180,31 +3179,24 @@ handle_ftp_ctl(struct conntrack *ct, const struct conn_lookup_ctx *ctx,
     size_t addr_offset_from_ftp_data_start = 0;
     size_t addr_size = 0;
     char *ftp_data_start;
-    bool do_seq_skew_adj = true;
     enum ct_alg_mode mode = CT_FTP_MODE_ACTIVE;
 
     if (detect_ftp_ctl_type(ctx, pkt) != ftp_ctl) {
         return;
     }
 
-    if (!nat || !conn_for_expectation->seq_skew) {
-        do_seq_skew_adj = false;
-    }
-
     struct ovs_16aligned_ip6_hdr *nh6 = dp_packet_l3(pkt);
     int64_t seq_skew = 0;
 
-    if (ftp_ctl == CT_FTP_CTL_OTHER) {
-        seq_skew = conn_for_expectation->seq_skew;
-    } else if (ftp_ctl == CT_FTP_CTL_INTEREST) {
+    if (ftp_ctl == CT_FTP_CTL_INTEREST) {
         enum ftp_ctl_pkt rc;
         if (ctx->key.dl_type == htons(ETH_TYPE_IPV6)) {
-            rc = process_ftp_ctl_v6(ct, pkt, conn_for_expectation,
+            rc = process_ftp_ctl_v6(ct, pkt, ec,
                                     &v6_addr_rep, &ftp_data_start,
                                     &addr_offset_from_ftp_data_start,
                                     &addr_size, &mode);
         } else {
-            rc = process_ftp_ctl_v4(ct, pkt, conn_for_expectation,
+            rc = process_ftp_ctl_v4(ct, pkt, ec,
                                     &v4_addr_rep, &ftp_data_start,
                                     &addr_offset_from_ftp_data_start);
         }
@@ -3217,62 +3209,64 @@ handle_ftp_ctl(struct conntrack *ct, const struct conn_lookup_ctx *ctx,
             uint16_t ip_len;
 
             if (ctx->key.dl_type == htons(ETH_TYPE_IPV6)) {
-                seq_skew = repl_ftp_v6_addr(pkt, v6_addr_rep, ftp_data_start,
-                                            addr_offset_from_ftp_data_start,
-                                            addr_size, mode);
+                if (nat) {
+                    seq_skew = repl_ftp_v6_addr(pkt, v6_addr_rep,
+                                   ftp_data_start,
+                                   addr_offset_from_ftp_data_start,
+                                   addr_size, mode);
+                }
+
                 if (seq_skew) {
                     ip_len = ntohs(nh6->ip6_ctlun.ip6_un1.ip6_un1_plen);
                     ip_len += seq_skew;
                     nh6->ip6_ctlun.ip6_un1.ip6_un1_plen = htons(ip_len);
-                    conn_seq_skew_set(ct, &conn_for_expectation->key, now,
-                                      seq_skew, ctx->reply);
                 }
             } else {
-                seq_skew = repl_ftp_v4_addr(pkt, v4_addr_rep, ftp_data_start,
-                                            addr_offset_from_ftp_data_start);
-                ip_len = ntohs(l3_hdr->ip_tot_len);
+                if (nat) {
+                    seq_skew = repl_ftp_v4_addr(pkt, v4_addr_rep,
+                                   ftp_data_start,
+                                   addr_offset_from_ftp_data_start);
+                }
                 if (seq_skew) {
+                    ip_len = ntohs(l3_hdr->ip_tot_len);
                     ip_len += seq_skew;
                     l3_hdr->ip_csum = recalc_csum16(l3_hdr->ip_csum,
                                           l3_hdr->ip_tot_len, htons(ip_len));
                     l3_hdr->ip_tot_len = htons(ip_len);
-                    conn_seq_skew_set(ct, &conn_for_expectation->key, now,
-                                      seq_skew, ctx->reply);
                 }
             }
         } else {
             OVS_NOT_REACHED();
         }
-    } else {
-        OVS_NOT_REACHED();
     }
 
     struct tcp_header *th = dp_packet_l4(pkt);
 
-    if (do_seq_skew_adj && seq_skew != 0) {
-        if (ctx->reply != conn_for_expectation->seq_skew_dir) {
+    if (nat && ec->seq_skew != 0) {
+        if (ctx->reply != ec->seq_skew_dir) {
 
             uint32_t tcp_ack = ntohl(get_16aligned_be32(&th->tcp_ack));
 
-            if ((seq_skew > 0) && (tcp_ack < seq_skew)) {
+            if ((ec->seq_skew > 0) && (tcp_ack < ec->seq_skew)) {
                 /* Should not be possible; will be marked invalid. */
                 tcp_ack = 0;
-            } else if ((seq_skew < 0) && (UINT32_MAX - tcp_ack < -seq_skew)) {
-                tcp_ack = (-seq_skew) - (UINT32_MAX - tcp_ack);
+            } else if ((ec->seq_skew < 0) &&
+                       (UINT32_MAX - tcp_ack < -ec->seq_skew)) {
+                tcp_ack = (-ec->seq_skew) - (UINT32_MAX - tcp_ack);
             } else {
-                tcp_ack -= seq_skew;
+                tcp_ack -= ec->seq_skew;
             }
             ovs_be32 new_tcp_ack = htonl(tcp_ack);
             put_16aligned_be32(&th->tcp_ack, new_tcp_ack);
         } else {
             uint32_t tcp_seq = ntohl(get_16aligned_be32(&th->tcp_seq));
-            if ((seq_skew > 0) && (UINT32_MAX - tcp_seq < seq_skew)) {
-                tcp_seq = seq_skew - (UINT32_MAX - tcp_seq);
-            } else if ((seq_skew < 0) && (tcp_seq < -seq_skew)) {
+            if ((ec->seq_skew > 0) && (UINT32_MAX - tcp_seq < ec->seq_skew)) {
+                tcp_seq = ec->seq_skew - (UINT32_MAX - tcp_seq);
+            } else if ((ec->seq_skew < 0) && (tcp_seq < -ec->seq_skew)) {
                 /* Should not be possible; will be marked invalid. */
                 tcp_seq = 0;
             } else {
-                tcp_seq += seq_skew;
+                tcp_seq += ec->seq_skew;
             }
             ovs_be32 new_tcp_seq = htonl(tcp_seq);
             put_16aligned_be32(&th->tcp_seq, new_tcp_seq);
@@ -3290,6 +3284,11 @@ handle_ftp_ctl(struct conntrack *ct, const struct conn_lookup_ctx *ctx,
     uint8_t pad = dp_packet_l2_pad_size(pkt);
     th->tcp_csum = csum_finish(
         csum_continue(tcp_csum, th, tail - (char *) th - pad));
+
+    if (seq_skew) {
+        conn_seq_skew_set(ct, &ec->key, now, seq_skew + ec->seq_skew,
+                          ctx->reply);
+    }
 }
 
 static void
diff --git a/tests/atlocal.in b/tests/atlocal.in
index 388f79e..5eff0a0 100644
--- a/tests/atlocal.in
+++ b/tests/atlocal.in
@@ -193,6 +193,9 @@ fi
 # Set HAVE_TCPDUMP
 find_command tcpdump
 
+# Set HAVE_LFTP
+find_command lftp
+
 CURL_OPT="-g -v --max-time 1 --retry 2 --retry-delay 1 --connect-timeout 1"
 
 # Determine whether "diff" supports "normal" diffs.  (busybox diff does not.)
diff --git a/tests/system-traffic.at b/tests/system-traffic.at
index b2ab800..bcaa1a5 100644
--- a/tests/system-traffic.at
+++ b/tests/system-traffic.at
@@ -4226,6 +4226,7 @@ dnl NAT, using the provided flow table.
 m4_define([CHECK_FTP_NAT],
    [AT_SETUP([conntrack - FTP NAT $1])
     AT_SKIP_IF([test $HAVE_FTP = no])
+    AT_SKIP_IF([test $HAVE_LFTP = no])
     CHECK_CONNTRACK()
     CHECK_CONNTRACK_NAT()
     CHECK_CONNTRACK_ALG()
@@ -4246,7 +4247,16 @@ m4_define([CHECK_FTP_NAT],
     OVS_START_L7([at_ns1], [ftp])
 
     dnl FTP requests from p0->p1 should work fine.
-    NS_CHECK_EXEC([at_ns0], [wget ftp://10.1.1.2 -4 --no-passive-ftp -t 3 -T 1 --retry-connrefused -v --server-response --no-remove-listing -o wget0.log -d])
+    AT_DATA([ftp.cmd], [dnl
+set net:max-retries 1
+set net:timeout 1
+set ftp:passive-mode off
+cache off
+connect ftp://anonymous:@10.1.1.2
+ls
+ls
+])
+    NS_CHECK_EXEC([at_ns0], [lftp -f ftp.cmd > lftp.log])
 
     dnl Discards CLOSE_WAIT and CLOSING
     AT_CHECK([ovs-appctl dpctl/dump-conntrack | FORMAT_CT(10.1.1.2)], [0], [$4])

From patchwork Wed Jan  9 15:33:09 2019
Content-Type: text/plain; charset="utf-8"
MIME-Version: 1.0
Content-Transfer-Encoding: 7bit
X-Patchwork-Submitter: David Marchand <david.marchand@redhat.com>
X-Patchwork-Id: 1022489
Return-Path: <ovs-dev-bounces@openvswitch.org>
X-Original-To: incoming@patchwork.ozlabs.org
Delivered-To: patchwork-incoming@bilbo.ozlabs.org
Authentication-Results: ozlabs.org;
	spf=pass (mailfrom) smtp.mailfrom=openvswitch.org
	(client-ip=140.211.169.12; helo=mail.linuxfoundation.org;
	envelope-from=ovs-dev-bounces@openvswitch.org;
	receiver=<UNKNOWN>)
Authentication-Results: ozlabs.org;
	dmarc=fail (p=none dis=none) header.from=redhat.com
Received: from mail.linuxfoundation.org (mail.linuxfoundation.org
	[140.211.169.12])
	(using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256
	bits)) (No client certificate requested)
	by ozlabs.org (Postfix) with ESMTPS id 43ZY8P1NRGz9sCh
	for <incoming@patchwork.ozlabs.org>;
	Thu, 10 Jan 2019 02:34:29 +1100 (AEDT)
Received: from mail.linux-foundation.org (localhost [127.0.0.1])
	by mail.linuxfoundation.org (Postfix) with ESMTP id 2F87EE21;
	Wed,  9 Jan 2019 15:33:30 +0000 (UTC)
X-Original-To: dev@openvswitch.org
Delivered-To: ovs-dev@mail.linuxfoundation.org
Received: from smtp1.linuxfoundation.org (smtp1.linux-foundation.org
	[172.17.192.35])
	by mail.linuxfoundation.org (Postfix) with ESMTPS id AEFF2DB8
	for <dev@openvswitch.org>; Wed,  9 Jan 2019 15:33:28 +0000 (UTC)
X-Greylist: domain auto-whitelisted by SQLgrey-1.7.6
Received: from mx1.redhat.com (mx1.redhat.com [209.132.183.28])
	by smtp1.linuxfoundation.org (Postfix) with ESMTPS id 9A3E3850
	for <dev@openvswitch.org>; Wed,  9 Jan 2019 15:33:27 +0000 (UTC)
Received: from smtp.corp.redhat.com
	(int-mx03.intmail.prod.int.phx2.redhat.com [10.5.11.13])
	(using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits))
	(No client certificate requested)
	by mx1.redhat.com (Postfix) with ESMTPS id 3536D8AE40;
	Wed,  9 Jan 2019 15:33:27 +0000 (UTC)
Received: from dmarchan.remote.csb (ovpn-117-97.ams2.redhat.com
	[10.36.117.97])
	by smtp.corp.redhat.com (Postfix) with ESMTP id 2582A608C7;
	Wed,  9 Jan 2019 15:33:25 +0000 (UTC)
From: David Marchand <david.marchand@redhat.com>
To: dev@openvswitch.org
Date: Wed,  9 Jan 2019 16:33:09 +0100
Message-Id: <1547047989-28356-3-git-send-email-david.marchand@redhat.com>
In-Reply-To: <1547047989-28356-1-git-send-email-david.marchand@redhat.com>
References: <1545312814-23634-1-git-send-email-david.marchand@redhat.com>
	<1547047989-28356-1-git-send-email-david.marchand@redhat.com>
X-Scanned-By: MIMEDefang 2.79 on 10.5.11.13
X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-4.5.16
	(mx1.redhat.com [10.5.110.25]);
	Wed, 09 Jan 2019 15:33:27 +0000 (UTC)
X-Spam-Status: No, score=-6.9 required=5.0 tests=BAYES_00,RCVD_IN_DNSWL_HI
	autolearn=ham version=3.3.1
X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on
	smtp1.linux-foundation.org
Subject: [ovs-dev] [PATCH v3 2/2] conntrack: fix expectations nat
	configuration for ftp+DNAT
X-BeenThere: ovs-dev@openvswitch.org
X-Mailman-Version: 2.1.12
Precedence: list
List-Id: <ovs-dev.openvswitch.org>
List-Unsubscribe: <https://mail.openvswitch.org/mailman/options/ovs-dev>,
	<mailto:ovs-dev-request@openvswitch.org?subject=unsubscribe>
List-Archive: <http://mail.openvswitch.org/pipermail/ovs-dev/>
List-Post: <mailto:ovs-dev@openvswitch.org>
List-Help: <mailto:ovs-dev-request@openvswitch.org?subject=help>
List-Subscribe: <https://mail.openvswitch.org/mailman/listinfo/ovs-dev>,
	<mailto:ovs-dev-request@openvswitch.org?subject=subscribe>
MIME-Version: 1.0
Sender: ovs-dev-bounces@openvswitch.org
Errors-To: ovs-dev-bounces@openvswitch.org

When configuring the nat part of an expectation, care must be taken to
look at the master nat action and direction to properly reproduce it.

DNAT tests have been added to both active and passive modes, all
ftp/tftp tests titles have been updated to reflect they are dealing with
SNAT.

Co-authored-by: Darrell Ball <dlu998@gmail.com>
Signed-off-by: Darrell Ball <dlu998@gmail.com>
Signed-off-by: David Marchand <david.marchand@redhat.com>
---
Changelog since v2:
- following Darrell comment, kept the existing passive snat test as is,
  then added new dnat tests for passive and active modes
- updated missing snat tests titles

---
 lib/conntrack.c         |  12 +++-
 tests/system-traffic.at | 162 +++++++++++++++++++++++++++++++++++++++++-------
 2 files changed, 151 insertions(+), 23 deletions(-)

diff --git a/lib/conntrack.c b/lib/conntrack.c
index 4717950..0062930 100644
--- a/lib/conntrack.c
+++ b/lib/conntrack.c
@@ -2713,21 +2713,29 @@ expectation_create(struct conntrack *ct, ovs_be16 dst_port,
     if (reply) {
         src_addr = master_conn->key.src.addr;
         dst_addr = master_conn->key.dst.addr;
+        alg_exp_node->nat_rpl_dst = true;
         if (skip_nat) {
             alg_nat_repl_addr = dst_addr;
+        } else if (master_conn->nat_info &&
+                   master_conn->nat_info->nat_action & NAT_ACTION_DST) {
+            alg_nat_repl_addr = master_conn->rev_key.src.addr;
+            alg_exp_node->nat_rpl_dst = false;
         } else {
             alg_nat_repl_addr = master_conn->rev_key.dst.addr;
         }
-        alg_exp_node->nat_rpl_dst = true;
     } else {
         src_addr = master_conn->rev_key.src.addr;
         dst_addr = master_conn->rev_key.dst.addr;
+        alg_exp_node->nat_rpl_dst = false;
         if (skip_nat) {
             alg_nat_repl_addr = src_addr;
+        } else if (master_conn->nat_info &&
+                   master_conn->nat_info->nat_action & NAT_ACTION_DST) {
+            alg_nat_repl_addr = master_conn->key.dst.addr;
+            alg_exp_node->nat_rpl_dst = true;
         } else {
             alg_nat_repl_addr = master_conn->key.src.addr;
         }
-        alg_exp_node->nat_rpl_dst = false;
     }
     if (src_ip_wc) {
         memset(&src_addr, 0, sizeof src_addr);
diff --git a/tests/system-traffic.at b/tests/system-traffic.at
index bcaa1a5..1539a25 100644
--- a/tests/system-traffic.at
+++ b/tests/system-traffic.at
@@ -4224,7 +4224,7 @@ dnl
 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_SETUP([conntrack - FTP $1])
     AT_SKIP_IF([test $HAVE_FTP = no])
     AT_SKIP_IF([test $HAVE_LFTP = no])
     CHECK_CONNTRACK()
@@ -4264,7 +4264,7 @@ ls
     OVS_TRAFFIC_VSWITCHD_STOP
     AT_CLEANUP])
 
-dnl CHECK_FTP_NAT_PRE_RECIRC(TITLE, IP_ADDR, IP_ADDR_AS_HEX)
+dnl CHECK_FTP_SNAT_PRE_RECIRC(TITLE, IP_ADDR, IP_ADDR_AS_HEX)
 dnl
 dnl Checks the implementation of conntrack with FTP ALGs in combination with
 dnl NAT, with flow tables that implement the NATing as part of handling of
@@ -4272,8 +4272,8 @@ dnl initial incoming packets - ie, the first flow is ct(nat,table=foo).
 dnl
 dnl IP_ADDR must specify the NAT address in standard "10.1.1.x" format,
 dnl and IP_ADDR_AS_HEX must specify the same address as hex, eg 0x0a0101xx.
-m4_define([CHECK_FTP_NAT_PRE_RECIRC], [dnl
-   CHECK_FTP_NAT([prerecirc $1], [$2], [dnl
+m4_define([CHECK_FTP_SNAT_PRE_RECIRC], [dnl
+    CHECK_FTP_NAT([SNAT prerecirc $1], [$2], [dnl
 dnl track all IP traffic, de-mangle non-NEW connections
 table=0 in_port=1, ip, action=ct(table=1,nat)
 table=0 in_port=2, ip, action=ct(table=2,nat)
@@ -4327,7 +4327,7 @@ tcp,orig=(src=10.1.1.2,dst=$2,sport=<cleared>,dport=<cleared>),reply=(src=10.1.1
 ])
 
 dnl Check that ct(nat,table=foo) works without TCP sequence adjustment.
-CHECK_FTP_NAT_PRE_RECIRC([], [10.1.1.9], [0x0a010109])
+CHECK_FTP_SNAT_PRE_RECIRC([], [10.1.1.9], [0x0a010109])
 
 dnl Check that ct(nat,table=foo) works with TCP sequence adjustment.
 dnl
@@ -4338,9 +4338,9 @@ dnl of 10.1.1.1 used in the test and 10.1.1.240 here), the FTP NAT ALG must
 dnl resize the packet and adjust TCP sequence numbers. This test is kept
 dnl separate from the above to easier identify issues in this code on different
 dnl kernels.
-CHECK_FTP_NAT_PRE_RECIRC([seqadj], [10.1.1.240], [0x0a0101f0])
+CHECK_FTP_SNAT_PRE_RECIRC([seqadj], [10.1.1.240], [0x0a0101f0])
 
-dnl CHECK_FTP_NAT_POST_RECIRC(TITLE, IP_ADDR, IP_ADDR_AS_HEX)
+dnl CHECK_FTP_SNAT_POST_RECIRC(TITLE, IP_ADDR, IP_ADDR_AS_HEX)
 dnl
 dnl Checks the implementation of conntrack with FTP ALGs in combination with
 dnl NAT, with flow tables that implement the NATing after the first round
@@ -4349,8 +4349,8 @@ dnl flow will implement the NATing with ct(nat..),output:foo.
 dnl
 dnl IP_ADDR must specify the NAT address in standard "10.1.1.x" format,
 dnl and IP_ADDR_AS_HEX must specify the same address as hex, eg 0x0a0101xx.
-m4_define([CHECK_FTP_NAT_POST_RECIRC], [dnl
-    CHECK_FTP_NAT([postrecirc $1], [$2], [dnl
+m4_define([CHECK_FTP_SNAT_POST_RECIRC], [dnl
+    CHECK_FTP_NAT([SNAT postrecirc $1], [$2], [dnl
 dnl track all IP traffic (this includes a helper call to non-NEW packets.)
 table=0 ip, action=ct(table=1)
 dnl
@@ -4393,7 +4393,7 @@ tcp,orig=(src=10.1.1.2,dst=$2,sport=<cleared>,dport=<cleared>),reply=(src=10.1.1
 ])
 
 dnl Check that ct(nat,table=foo) works without TCP sequence adjustment.
-CHECK_FTP_NAT_POST_RECIRC([], [10.1.1.9], [0x0a010109])
+CHECK_FTP_SNAT_POST_RECIRC([], [10.1.1.9], [0x0a010109])
 
 dnl Check that ct(nat,table=foo) works with TCP sequence adjustment.
 dnl
@@ -4404,10 +4404,10 @@ dnl of 10.1.1.1 used in the test and 10.1.1.240 here), the FTP NAT ALG must
 dnl resize the packet and adjust TCP sequence numbers. This test is kept
 dnl separate from the above to easier identify issues in this code on different
 dnl kernels.
-CHECK_FTP_NAT_POST_RECIRC([seqadj], [10.1.1.240], [0x0a0101f0])
+CHECK_FTP_SNAT_POST_RECIRC([seqadj], [10.1.1.240], [0x0a0101f0])
 
 
-dnl CHECK_FTP_NAT_ORIG_TUPLE(TITLE, IP_ADDR, IP_ADDR_AS_HEX)
+dnl CHECK_FTP_SNAT_ORIG_TUPLE(TITLE, IP_ADDR, IP_ADDR_AS_HEX)
 dnl
 dnl Checks the implementation of conntrack original direction tuple matching
 dnl with FTP ALGs in combination with NAT, with flow tables that implement
@@ -4417,8 +4417,8 @@ dnl commiting of NATed and other connections with ct(nat..),output:foo.
 dnl
 dnl IP_ADDR must specify the NAT address in standard "10.1.1.x" format,
 dnl and IP_ADDR_AS_HEX must specify the same address as hex, eg 0x0a0101xx.
-m4_define([CHECK_FTP_NAT_ORIG_TUPLE], [dnl
-    CHECK_FTP_NAT([orig tuple $1], [$2], [dnl
+m4_define([CHECK_FTP_SNAT_ORIG_TUPLE], [dnl
+    CHECK_FTP_NAT([SNAT orig tuple $1], [$2], [dnl
 dnl Store zone in reg4 and packet direction in reg3 (IN=1, OUT=2).
 dnl NAT is only applied to OUT-direction packets, so that ACL
 dnl processing can be done with non-NATted headers.
@@ -4524,13 +4524,13 @@ tcp,orig=(src=10.1.1.2,dst=$2,sport=<cleared>,dport=<cleared>),reply=(src=10.1.1
 
 dnl Check that ct(nat,table=foo) works without TCP sequence adjustment with
 dnl an ACL table based on matching on conntrack original direction tuple only.
-CHECK_FTP_NAT_ORIG_TUPLE([], [10.1.1.9], [0x0a010109])
+CHECK_FTP_SNAT_ORIG_TUPLE([], [10.1.1.9], [0x0a010109])
 
 dnl Check that ct(nat,table=foo) works with TCP sequence adjustment with
 dnl an ACL table based on matching on conntrack original direction tuple only.
-CHECK_FTP_NAT_ORIG_TUPLE([seqadj], [10.1.1.240], [0x0a0101f0])
+CHECK_FTP_SNAT_ORIG_TUPLE([seqadj], [10.1.1.240], [0x0a0101f0])
 
-AT_SETUP([conntrack - IPv4 FTP Passive with NAT])
+AT_SETUP([conntrack - IPv4 FTP Passive with SNAT])
 AT_SKIP_IF([test $HAVE_FTP = no])
 CHECK_CONNTRACK()
 CHECK_CONNTRACK_NAT()
@@ -4590,6 +4590,126 @@ 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 - IPv4 FTP Passive with DNAT])
+AT_SKIP_IF([test $HAVE_FTP = no])
+CHECK_CONNTRACK()
+CHECK_CONNTRACK_NAT()
+CHECK_CONNTRACK_ALG()
+
+OVS_TRAFFIC_VSWITCHD_START()
+
+ADD_NAMESPACES(at_ns0, at_ns1)
+
+ADD_VETH(p0, at_ns0, br0, "10.1.1.1/24")
+NS_CHECK_EXEC([at_ns0], [ip link set dev p0 address e6:66:c1:11:11:11])
+NS_CHECK_EXEC([at_ns0], [arp -s 10.1.1.2 e6:66:c1:22:22:22])
+NS_CHECK_EXEC([at_ns0], [arp -s 10.1.1.240 e6:66:c1:22:22:22])
+
+ADD_VETH(p1, at_ns1, br0, "10.1.1.240/24")
+NS_CHECK_EXEC([at_ns1], [ip link set dev p1 address e6:66:c1:22:22:22])
+NS_CHECK_EXEC([at_ns1], [arp -s 10.1.1.1 e6:66:c1:11:11:11])
+
+dnl Allow any traffic from ns0->ns1.
+AT_DATA([flows.txt], [dnl
+dnl track all IPv4 traffic and NAT any established traffic.
+table=0 priority=10 ip, action=ct(nat,table=1)
+table=0 priority=0 action=drop
+dnl
+dnl Table 1
+dnl
+dnl Allow new FTP control connections.
+table=1 in_port=1 ct_state=+new tcp nw_src=10.1.1.1 tp_dst=21 action=ct(alg=ftp,commit,nat(dst=10.1.1.240)),2
+dnl Allow related TCP connections from port 1.
+table=1 in_port=1 ct_state=+new+rel tcp nw_src=10.1.1.1 action=ct(commit,nat),2
+dnl Allow established TCP connections both ways, post-NAT match.
+table=1 in_port=1 ct_state=+est tcp nw_dst=10.1.1.240 action=2
+table=1 in_port=2 ct_state=+est tcp nw_dst=10.1.1.1 action=1
+
+dnl Allow ICMP both ways.
+table=1 priority=100 in_port=1 icmp, action=2
+table=1 priority=100 in_port=2 icmp, action=1
+table=1 priority=0, action=drop
+])
+
+AT_CHECK([ovs-ofctl --bundle add-flows br0 flows.txt])
+
+dnl Check that the stacks working to avoid races.
+OVS_WAIT_UNTIL([ip netns exec at_ns0 ping -c 1 10.1.1.240 >/dev/null])
+
+OVS_START_L7([at_ns1], [ftp])
+
+dnl FTP requests from p0->p1 should work fine.
+NS_CHECK_EXEC([at_ns0], [wget ftp://10.1.1.2 -t 3 -T 1 --retry-connrefused -v -o wget0.log])
+
+dnl Discards CLOSE_WAIT and CLOSING
+AT_CHECK([ovs-appctl dpctl/dump-conntrack | FORMAT_CT(10.1.1.2)], [0], [dnl
+tcp,orig=(src=10.1.1.1,dst=10.1.1.2,sport=<cleared>,dport=<cleared>),reply=(src=10.1.1.240,dst=10.1.1.1,sport=<cleared>,dport=<cleared>),protoinfo=(state=<cleared>)
+tcp,orig=(src=10.1.1.1,dst=10.1.1.2,sport=<cleared>,dport=<cleared>),reply=(src=10.1.1.240,dst=10.1.1.1,sport=<cleared>,dport=<cleared>),protoinfo=(state=<cleared>),helper=ftp
+])
+
+OVS_TRAFFIC_VSWITCHD_STOP
+AT_CLEANUP
+
+AT_SETUP([conntrack - IPv4 FTP Active with DNAT])
+AT_SKIP_IF([test $HAVE_FTP = no])
+CHECK_CONNTRACK()
+CHECK_CONNTRACK_NAT()
+CHECK_CONNTRACK_ALG()
+
+OVS_TRAFFIC_VSWITCHD_START()
+
+ADD_NAMESPACES(at_ns0, at_ns1)
+
+ADD_VETH(p0, at_ns0, br0, "10.1.1.1/24")
+NS_CHECK_EXEC([at_ns0], [ip link set dev p0 address e6:66:c1:11:11:11])
+NS_CHECK_EXEC([at_ns0], [arp -s 10.1.1.2 e6:66:c1:22:22:22])
+NS_CHECK_EXEC([at_ns0], [arp -s 10.1.1.240 e6:66:c1:22:22:22])
+
+ADD_VETH(p1, at_ns1, br0, "10.1.1.240/24")
+NS_CHECK_EXEC([at_ns1], [ip link set dev p1 address e6:66:c1:22:22:22])
+NS_CHECK_EXEC([at_ns1], [arp -s 10.1.1.1 e6:66:c1:11:11:11])
+
+dnl Allow any traffic from ns0->ns1.
+AT_DATA([flows.txt], [dnl
+dnl track all IPv4 traffic and NAT any established traffic.
+table=0 priority=10 ip, action=ct(nat,table=1)
+table=0 priority=0 action=drop
+dnl
+dnl Table 1
+dnl
+dnl Allow new FTP control connections.
+table=1 in_port=1 ct_state=+new tcp nw_src=10.1.1.1 tp_dst=21 action=ct(alg=ftp,commit,nat(dst=10.1.1.240)),2
+dnl Allow related TCP connections from port 1.
+table=1 in_port=2 ct_state=+new+rel tcp nw_src=10.1.1.240 action=ct(commit,nat),1
+dnl Allow established TCP connections both ways, post-NAT match.
+table=1 in_port=1 ct_state=+est tcp nw_dst=10.1.1.240 action=2
+table=1 in_port=2 ct_state=+est tcp nw_dst=10.1.1.1 action=1
+
+dnl Allow ICMP both ways.
+table=1 priority=100 in_port=1 icmp, action=2
+table=1 priority=100 in_port=2 icmp, action=1
+table=1 priority=0, action=drop
+])
+
+AT_CHECK([ovs-ofctl --bundle add-flows br0 flows.txt])
+
+dnl Check that the stacks working to avoid races.
+OVS_WAIT_UNTIL([ip netns exec at_ns0 ping -c 1 10.1.1.240 >/dev/null])
+
+OVS_START_L7([at_ns1], [ftp])
+
+dnl FTP requests from p0->p1 should work fine.
+NS_CHECK_EXEC([at_ns0], [wget ftp://10.1.1.2 --no-passive-ftp -t 3 -T 1 --retry-connrefused -v -o wget0.log])
+
+dnl Discards CLOSE_WAIT and CLOSING
+AT_CHECK([ovs-appctl dpctl/dump-conntrack | FORMAT_CT(10.1.1.2)], [0], [dnl
+tcp,orig=(src=10.1.1.1,dst=10.1.1.2,sport=<cleared>,dport=<cleared>),reply=(src=10.1.1.240,dst=10.1.1.1,sport=<cleared>,dport=<cleared>),protoinfo=(state=<cleared>),helper=ftp
+tcp,orig=(src=10.1.1.240,dst=10.1.1.1,sport=<cleared>,dport=<cleared>),reply=(src=10.1.1.1,dst=10.1.1.2,sport=<cleared>,dport=<cleared>),protoinfo=(state=<cleared>)
+])
+
+OVS_TRAFFIC_VSWITCHD_STOP
+AT_CLEANUP
+
 AT_SETUP([conntrack - IPv6 HTTP with SNAT])
 CHECK_CONNTRACK()
 CHECK_CONNTRACK_NAT()
@@ -4731,7 +4851,7 @@ udp,orig=(src=fc00::1,dst=fc00::2,sport=<cleared>,dport=<cleared>),reply=(src=fc
 OVS_TRAFFIC_VSWITCHD_STOP
 AT_CLEANUP
 
-AT_SETUP([conntrack - IPv6 FTP with NAT])
+AT_SETUP([conntrack - IPv6 FTP with SNAT])
 AT_SKIP_IF([test $HAVE_FTP = no])
 CHECK_CONNTRACK()
 CHECK_CONNTRACK_NAT()
@@ -4791,7 +4911,7 @@ tcp,orig=(src=fc00::2,dst=fc00::240,sport=<cleared>,dport=<cleared>),reply=(src=
 OVS_TRAFFIC_VSWITCHD_STOP
 AT_CLEANUP
 
-AT_SETUP([conntrack - IPv6 FTP Passive with NAT])
+AT_SETUP([conntrack - IPv6 FTP Passive with SNAT])
 AT_SKIP_IF([test $HAVE_FTP = no])
 CHECK_CONNTRACK()
 CHECK_CONNTRACK_NAT()
@@ -4852,7 +4972,7 @@ tcp,orig=(src=fc00::1,dst=fc00::2,sport=<cleared>,dport=<cleared>),reply=(src=fc
 OVS_TRAFFIC_VSWITCHD_STOP
 AT_CLEANUP
 
-AT_SETUP([conntrack - IPv6 FTP with NAT - orig tuple])
+AT_SETUP([conntrack - IPv6 FTP with SNAT - orig tuple])
 AT_SKIP_IF([test $HAVE_FTP = no])
 CHECK_CONNTRACK()
 CHECK_CONNTRACK_NAT()
@@ -4912,7 +5032,7 @@ tcp,orig=(src=fc00::2,dst=fc00::240,sport=<cleared>,dport=<cleared>),reply=(src=
 OVS_TRAFFIC_VSWITCHD_STOP
 AT_CLEANUP
 
-AT_SETUP([conntrack - IPv4 TFTP with NAT])
+AT_SETUP([conntrack - IPv4 TFTP with SNAT])
 AT_SKIP_IF([test $HAVE_TFTP = no])
 CHECK_CONNTRACK()
 CHECK_CONNTRACK_NAT()