diff mbox series

[ovs-dev,branch-2.8,1/2] Revert "ofproto-dpif: Mark packets as "untracked" after call to ct()."

Message ID 1504209851-28202-1-git-send-email-jpettit@ovn.org
State Accepted
Headers show
Series [ovs-dev,branch-2.8,1/2] Revert "ofproto-dpif: Mark packets as "untracked" after call to ct()." | expand

Commit Message

Justin Pettit Aug. 31, 2017, 8:04 p.m. UTC
This reverts commit 8473cf69d25c4682cc6f6857b86b490a8c27cbd4.

This commit introduced a change in the conntrack API.  This affected
some existing applications, so we will delay introducing the change
until the next major release.

Signed-off-by: Justin Pettit <jpettit@ovn.org>
Requested-by: Flavio Leitner <fbl@sysclose.org>
---
 NEWS                         |  4 ----
 lib/ofp-actions.c            | 27 ++++++++++++++-------------
 ofproto/ofproto-dpif-xlate.c | 21 ++++++++++++++-------
 tests/ofproto-dpif.at        | 10 +++++-----
 tests/system-traffic.at      |  4 ++--
 utilities/ovs-ofctl.8.in     | 10 ++++------
 6 files changed, 39 insertions(+), 37 deletions(-)

Comments

Russell Bryant Aug. 31, 2017, 9:37 p.m. UTC | #1
On Thu, Aug 31, 2017 at 4:04 PM, Justin Pettit <jpettit@ovn.org> wrote:
> This reverts commit 8473cf69d25c4682cc6f6857b86b490a8c27cbd4.
>
> This commit introduced a change in the conntrack API.  This affected
> some existing applications, so we will delay introducing the change
> until the next major release.
>
> Signed-off-by: Justin Pettit <jpettit@ovn.org>
> Requested-by: Flavio Leitner <fbl@sysclose.org>
> ---
>  NEWS                         |  4 ----
>  lib/ofp-actions.c            | 27 ++++++++++++++-------------
>  ofproto/ofproto-dpif-xlate.c | 21 ++++++++++++++-------
>  tests/ofproto-dpif.at        | 10 +++++-----
>  tests/system-traffic.at      |  4 ++--
>  utilities/ovs-ofctl.8.in     | 10 ++++------
>  6 files changed, 39 insertions(+), 37 deletions(-)

Acked-by: Russell Bryant <russell@ovn.org>
Flavio Leitner Aug. 31, 2017, 10:16 p.m. UTC | #2
On Thu, 31 Aug 2017 13:04:10 -0700
Justin Pettit <jpettit@ovn.org> wrote:

> This reverts commit 8473cf69d25c4682cc6f6857b86b490a8c27cbd4.
> 
> This commit introduced a change in the conntrack API.  This affected
> some existing applications, so we will delay introducing the change
> until the next major release.
> 
> Signed-off-by: Justin Pettit <jpettit@ovn.org>
> Requested-by: Flavio Leitner <fbl@sysclose.org>
> ---

LGTM, thanks!
fbl




>  NEWS                         |  4 ----
>  lib/ofp-actions.c            | 27 ++++++++++++++-------------
>  ofproto/ofproto-dpif-xlate.c | 21 ++++++++++++++-------
>  tests/ofproto-dpif.at        | 10 +++++-----
>  tests/system-traffic.at      |  4 ++--
>  utilities/ovs-ofctl.8.in     | 10 ++++------
>  6 files changed, 39 insertions(+), 37 deletions(-)
> 
> diff --git a/NEWS b/NEWS
> index 8e6fb79d38ca..ecb32c4df80c 100644
> --- a/NEWS
> +++ b/NEWS
> @@ -74,10 +74,6 @@ v2.8.0 - xx xxx xxxx
>         Used generic encap and decap actions to implement encapsulation and
>         decapsulation of NSH header.
>         IETF NSH draft - https://datatracker.ietf.org/doc/draft-ietf-sfc-nsh/
> -     * Conntrack state is only available to the processing path that
> -       follows the "recirc_table" argument of the ct() action.  Starting
> -       in OVS 2.8, this state is now cleared for the current processing
> -       path whenever ct() is called.
>     - Fedora Packaging:
>       * OVN services are no longer restarted automatically after upgrade.
>       * ovs-vswitchd and ovsdb-server run as non-root users by default.
> diff --git a/lib/ofp-actions.c b/lib/ofp-actions.c
> index 71eb70c3c239..bfc8a805ffd5 100644
> --- a/lib/ofp-actions.c
> +++ b/lib/ofp-actions.c
> @@ -5858,19 +5858,20 @@ format_DEBUG_RECIRC(const struct ofpact_null *a OVS_UNUSED,
>   *
>   *   - Packet State:
>   *
> - *      Untracked packets have an unknown connection state.  In most
> - *      cases, packets entering the OpenFlow pipeline will initially be
> - *      in the untracked state. Untracked packets may become tracked by
> - *      executing NXAST_CT with a "recirc_table" specified. This makes
> - *      various aspects about the connection available, in particular
> - *      the connection state.
> - *
> - *      An NXAST_CT action always puts the packet into an untracked
> - *      state for the current processing path.  If "recirc_table" is
> - *      set, execution is forked and the packet passes through the
> - *      connection tracker.  The specified table's processing path is
> - *      able to match on Connection state until the end of the OpenFlow
> - *      pipeline or NXAST_CT is called again.
> + *      Untracked packets have not yet passed through the connection tracker,
> + *      and the connection state for such packets is unknown. In most cases,
> + *      packets entering the OpenFlow pipeline will initially be in the
> + *      untracked state. Untracked packets may become tracked by executing
> + *      NXAST_CT with a "recirc_table" specified. This makes various aspects
> + *      about the connection available, in particular the connection state.
> + *
> + *      Tracked packets have previously passed through the connection tracker.
> + *      These packets will remain tracked through until the end of the OpenFlow
> + *      pipeline. Tracked packets which have NXAST_CT executed with a
> + *      "recirc_table" specified will return to the tracked state.
> + *
> + *      The packet state is only significant for the duration of packet
> + *      processing within the OpenFlow pipeline.
>   *
>   *   - Connection State:
>   *
> diff --git a/ofproto/ofproto-dpif-xlate.c b/ofproto/ofproto-dpif-xlate.c
> index 9e1f837cb23e..973e760547fa 100644
> --- a/ofproto/ofproto-dpif-xlate.c
> +++ b/ofproto/ofproto-dpif-xlate.c
> @@ -5721,7 +5721,9 @@ put_ct_nat(struct xlate_ctx *ctx)
>  static void
>  compose_conntrack_action(struct xlate_ctx *ctx, struct ofpact_conntrack *ofc)
>  {
> +    ovs_u128 old_ct_label = ctx->xin->flow.ct_label;
>      ovs_u128 old_ct_label_mask = ctx->wc->masks.ct_label;
> +    uint32_t old_ct_mark = ctx->xin->flow.ct_mark;
>      uint32_t old_ct_mark_mask = ctx->wc->masks.ct_mark;
>      size_t ct_offset;
>      uint16_t zone;
> @@ -5733,7 +5735,7 @@ compose_conntrack_action(struct xlate_ctx *ctx, struct ofpact_conntrack *ofc)
>      /* Process nested actions first, to populate the key. */
>      ctx->ct_nat_action = NULL;
>      ctx->wc->masks.ct_mark = 0;
> -    ctx->wc->masks.ct_label = OVS_U128_ZERO;
> +    ctx->wc->masks.ct_label.u64.hi = ctx->wc->masks.ct_label.u64.lo = 0;
>      do_xlate_actions(ofc->actions, ofpact_ct_get_action_len(ofc), ctx);
>  
>      if (ofc->zone_src.field) {
> @@ -5759,18 +5761,23 @@ compose_conntrack_action(struct xlate_ctx *ctx, struct ofpact_conntrack *ofc)
>      ctx->ct_nat_action = NULL;
>      nl_msg_end_nested(ctx->odp_actions, ct_offset);
>  
> +    /* Restore the original ct fields in the key. These should only be exposed
> +     * after recirculation to another table. */
> +    ctx->xin->flow.ct_mark = old_ct_mark;
>      ctx->wc->masks.ct_mark = old_ct_mark_mask;
> +    ctx->xin->flow.ct_label = old_ct_label;
>      ctx->wc->masks.ct_label = old_ct_label_mask;
>  
> -    if (ofc->recirc_table != NX_CT_RECIRC_NONE) {
> +    if (ofc->recirc_table == NX_CT_RECIRC_NONE) {
> +        /* If we do not recirculate as part of this action, hide the results of
> +         * connection tracking from subsequent recirculations. */
> +        ctx->conntracked = false;
> +    } else {
> +        /* Use ct_* fields from datapath during recirculation upcall. */
>          ctx->conntracked = true;
>          compose_recirculate_and_fork(ctx, ofc->recirc_table);
> +        ctx->conntracked = false;
>      }
> -
> -    /* The ct_* fields are only available in the scope of the 'recirc_table'
> -     * call chain. */
> -    flow_clear_conntrack(&ctx->xin->flow);
> -    ctx->conntracked = false;
>  }
>  
>  static void
> diff --git a/tests/ofproto-dpif.at b/tests/ofproto-dpif.at
> index 28a7e827cac2..284a65ec6524 100644
> --- a/tests/ofproto-dpif.at
> +++ b/tests/ofproto-dpif.at
> @@ -8949,7 +8949,7 @@ OVS_WAIT_UNTIL([ovs-appctl -t ovs-ofctl exit])
>  
>  dnl Check this output. We only see the latter two packets, not the first.
>  AT_CHECK([cat ofctl_monitor.log], [0], [dnl
> -NXT_PACKET_IN (xid=0x0): table_id=6 cookie=0x0 total_len=42 reg0=0x1,reg1=0x4d2,reg2=0x1,reg3=0x1,reg4=0x1,in_port=1 (via action) data_len=42 (unbuffered)
> +NXT_PACKET_IN (xid=0x0): table_id=6 cookie=0x0 total_len=42 ct_state=new|trk,ct_zone=1,ct_nw_src=10.1.1.1,ct_nw_dst=10.1.1.2,ct_nw_proto=17,ct_tp_src=1,ct_tp_dst=2,reg0=0x1,reg1=0x4d2,reg2=0x1,reg3=0x1,reg4=0x1,in_port=1 (via action) data_len=42 (unbuffered)
>  udp,vlan_tci=0x0000,dl_src=50:54:00:00:00:09,dl_dst=50:54:00:00:00:0a,nw_src=10.1.1.1,nw_dst=10.1.1.2,nw_tos=0,nw_ecn=0,nw_ttl=64,tp_src=1,tp_dst=2 udp_csum:e9d6
>  dnl
>  NXT_PACKET_IN (xid=0x0): table_id=6 cookie=0x0 total_len=42 ct_state=est|rpl|trk,ct_zone=1,ct_mark=0x1,ct_label=0x4d2000000000000000000000000,ct_nw_src=10.1.1.1,ct_nw_dst=10.1.1.2,ct_nw_proto=17,ct_tp_src=1,ct_tp_dst=2,reg0=0x1,reg1=0x4d2,reg2=0x1,reg3=0x2,reg4=0x1,in_port=2 (via action) data_len=42 (unbuffered)
> @@ -8970,7 +8970,7 @@ OVS_WAIT_UNTIL([ovs-appctl -t ovs-ofctl exit])
>  
>  dnl Check this output. We should see both packets
>  AT_CHECK([cat ofctl_monitor.log], [0], [dnl
> -NXT_PACKET_IN (xid=0x0): table_id=6 cookie=0x0 total_len=42 reg0=0x1,reg1=0x4d2,reg2=0x1,reg3=0x1,reg4=0x1,in_port=1 (via action) data_len=42 (unbuffered)
> +NXT_PACKET_IN (xid=0x0): table_id=6 cookie=0x0 total_len=42 ct_state=new|trk,ct_zone=1,ct_nw_src=10.1.1.1,ct_nw_dst=10.1.1.2,ct_nw_proto=17,ct_tp_src=3,ct_tp_dst=2,reg0=0x1,reg1=0x4d2,reg2=0x1,reg3=0x1,reg4=0x1,in_port=1 (via action) data_len=42 (unbuffered)
>  udp,vlan_tci=0x0000,dl_src=50:54:00:00:00:09,dl_dst=50:54:00:00:00:0a,nw_src=10.1.1.1,nw_dst=10.1.1.2,nw_tos=0,nw_ecn=0,nw_ttl=64,tp_src=3,tp_dst=2 udp_csum:e9d4
>  dnl
>  NXT_PACKET_IN (xid=0x0): table_id=6 cookie=0x0 total_len=42 ct_state=est|rpl|trk,ct_zone=1,ct_mark=0x1,ct_label=0x4d2000000000000000000000000,ct_nw_src=10.1.1.1,ct_nw_dst=10.1.1.2,ct_nw_proto=17,ct_tp_src=3,ct_tp_dst=2,reg0=0x1,reg1=0x4d2,reg2=0x1,reg3=0x2,reg4=0x1,in_port=2 (via action) data_len=42 (unbuffered)
> @@ -9025,7 +9025,7 @@ AT_CHECK([cat ofctl_monitor.log], [0], [dnl
>  NXT_PACKET_IN (xid=0x0): cookie=0x0 total_len=42 in_port=1 (via action) data_len=42 (unbuffered)
>  udp,vlan_tci=0x0000,dl_src=50:54:00:00:00:09,dl_dst=50:54:00:00:00:0a,nw_src=10.1.1.1,nw_dst=10.1.1.2,nw_tos=0,nw_ecn=0,nw_ttl=64,tp_src=1,tp_dst=2 udp_csum:e9d6
>  dnl
> -NXT_PACKET_IN (xid=0x0): table_id=1 cookie=0x0 total_len=42 in_port=2 (via action) data_len=42 (unbuffered)
> +NXT_PACKET_IN (xid=0x0): table_id=1 cookie=0x0 total_len=42 ct_state=est|rpl|trk,ct_nw_src=10.1.1.1,ct_nw_dst=10.1.1.2,ct_nw_proto=17,ct_tp_src=1,ct_tp_dst=2,in_port=2 (via action) data_len=42 (unbuffered)
>  udp,vlan_tci=0x0000,dl_src=50:54:00:00:00:0a,dl_dst=50:54:00:00:00:09,nw_src=10.1.1.2,nw_dst=10.1.1.1,nw_tos=0,nw_ecn=0,nw_ttl=64,tp_src=2,tp_dst=1 udp_csum:e9d6
>  ])
>  
> @@ -9047,7 +9047,7 @@ AT_CHECK([cat ofctl_monitor.log], [0], [dnl
>  NXT_PACKET_IN (xid=0x0): cookie=0x0 total_len=42 in_port=1 (via action) data_len=42 (unbuffered)
>  udp,vlan_tci=0x0000,dl_src=50:54:00:00:00:09,dl_dst=50:54:00:00:00:0a,nw_src=10.1.1.1,nw_dst=10.1.1.2,nw_tos=0,nw_ecn=0,nw_ttl=64,tp_src=3,tp_dst=4 udp_csum:e9d2
>  dnl
> -NXT_PACKET_IN (xid=0x0): table_id=1 cookie=0x0 total_len=42 in_port=2 (via action) data_len=42 (unbuffered)
> +NXT_PACKET_IN (xid=0x0): table_id=1 cookie=0x0 total_len=42 ct_state=est|rpl|trk,ct_nw_src=10.1.1.1,ct_nw_dst=10.1.1.2,ct_nw_proto=17,ct_tp_src=3,ct_tp_dst=4,in_port=2 (via action) data_len=42 (unbuffered)
>  udp,vlan_tci=0x0000,dl_src=50:54:00:00:00:0a,dl_dst=50:54:00:00:00:09,nw_src=10.1.1.2,nw_dst=10.1.1.1,nw_tos=0,nw_ecn=0,nw_ttl=64,tp_src=4,tp_dst=3 udp_csum:e9d2
>  ])
>  
> @@ -9362,7 +9362,7 @@ OVS_WAIT_UNTIL([ovs-appctl -t ovs-ofctl exit])
>  
>  dnl Check this output. We only see the latter two packets, not the first.
>  AT_CHECK([cat ofctl_monitor.log], [0], [dnl
> -NXT_PACKET_IN (xid=0x0): table_id=1 cookie=0x0 total_len=42 in_port=1 (via action) data_len=42 (unbuffered)
> +NXT_PACKET_IN (xid=0x0): table_id=1 cookie=0x0 total_len=42 ct_state=new|trk,ct_nw_src=10.1.1.1,ct_nw_dst=10.1.1.2,ct_nw_proto=17,ct_tp_src=1,ct_tp_dst=2,in_port=1 (via action) data_len=42 (unbuffered)
>  udp,vlan_tci=0x0000,dl_src=50:54:00:00:00:09,dl_dst=50:54:00:00:00:0a,nw_src=10.1.1.1,nw_dst=10.1.1.2,nw_tos=0,nw_ecn=0,nw_ttl=64,tp_src=1,tp_dst=2 udp_csum:e9d6
>  dnl
>  NXT_PACKET_IN (xid=0x0): table_id=1 cookie=0x0 total_len=42 ct_state=est|rpl|trk,ct_nw_src=10.1.1.1,ct_nw_dst=10.1.1.2,ct_nw_proto=17,ct_tp_src=1,ct_tp_dst=2,in_port=2 (via action) data_len=42 (unbuffered)
> diff --git a/tests/system-traffic.at b/tests/system-traffic.at
> index 522eaa615834..798dd2cbd2c2 100644
> --- a/tests/system-traffic.at
> +++ b/tests/system-traffic.at
> @@ -2287,7 +2287,7 @@ dnl Ingress pipeline
>  dnl - Allow all connections from LOCAL port (commit and proceed to egress)
>  dnl - All other connections go through conntracker using the input port as
>  dnl   a connection tracking zone.
> -table=1,priority=150,in_port=LOCAL,ip,ct_state=+trk+new,action=ct(commit,table=2,zone=OXM_OF_IN_PORT[[0..15]])
> +table=1,priority=150,in_port=LOCAL,ip,ct_state=+trk+new,action=ct(commit,zone=OXM_OF_IN_PORT[[0..15]]),goto_table:2
>  table=1,priority=100,ip,action=ct(table=2,zone=OXM_OF_IN_PORT[[0..15]])
>  table=1,priority=1,action=drop
>  
> @@ -2295,7 +2295,7 @@ dnl Egress pipeline
>  dnl - Allow all connections from LOCAL port (commit and skip to output)
>  dnl - Allow other established connections to go through conntracker using
>  dnl   output port as a connection tracking zone.
> -table=2,priority=150,in_port=LOCAL,ip,ct_state=+trk+new,action=ct(commit,table=4,zone=NXM_NX_REG0[[0..15]])
> +table=2,priority=150,in_port=LOCAL,ip,ct_state=+trk+new,action=ct(commit,zone=NXM_NX_REG0[[0..15]]),goto_table:4
>  table=2,priority=100,ip,ct_state=+trk+est,action=ct(table=3,zone=NXM_NX_REG0[[0..15]])
>  table=2,priority=1,action=drop
>  
> diff --git a/utilities/ovs-ofctl.8.in b/utilities/ovs-ofctl.8.in
> index c65de97f5e2e..f6bd90374a18 100644
> --- a/utilities/ovs-ofctl.8.in
> +++ b/utilities/ovs-ofctl.8.in
> @@ -1031,13 +1031,11 @@ Restores the queue to the value it was before any \fBset_queue\fR
>  actions were applied.
>  .
>  .IP \fBct\fR
> -.IQ \fBct(\fR[\fIargument\fR][\fB,\fIargument\fR...]\fB)
> +.IQ \fBct\fB(\fR[\fIargument\fR][\fB,\fIargument\fR...]\fB)
>  Send the packet through the connection tracker.  Refer to the \fBct_state\fR
> -documentation above for possible packet and connection states. A \fBct\fR
> -action always sets the packet to an untracked state and clears out the
> -\fBct_state\fR fields for the current processing path.  Those fields are
> -only available for the processing path pointed to by the \fBtable\fR
> -argument.  The following arguments are supported:
> +documentation above for possible packet and connection states. The following
> +arguments are supported:
> +
>  .RS
>  .IP \fBcommit\fR
>  .RS
diff mbox series

Patch

diff --git a/NEWS b/NEWS
index 8e6fb79d38ca..ecb32c4df80c 100644
--- a/NEWS
+++ b/NEWS
@@ -74,10 +74,6 @@  v2.8.0 - xx xxx xxxx
        Used generic encap and decap actions to implement encapsulation and
        decapsulation of NSH header.
        IETF NSH draft - https://datatracker.ietf.org/doc/draft-ietf-sfc-nsh/
-     * Conntrack state is only available to the processing path that
-       follows the "recirc_table" argument of the ct() action.  Starting
-       in OVS 2.8, this state is now cleared for the current processing
-       path whenever ct() is called.
    - Fedora Packaging:
      * OVN services are no longer restarted automatically after upgrade.
      * ovs-vswitchd and ovsdb-server run as non-root users by default.
diff --git a/lib/ofp-actions.c b/lib/ofp-actions.c
index 71eb70c3c239..bfc8a805ffd5 100644
--- a/lib/ofp-actions.c
+++ b/lib/ofp-actions.c
@@ -5858,19 +5858,20 @@  format_DEBUG_RECIRC(const struct ofpact_null *a OVS_UNUSED,
  *
  *   - Packet State:
  *
- *      Untracked packets have an unknown connection state.  In most
- *      cases, packets entering the OpenFlow pipeline will initially be
- *      in the untracked state. Untracked packets may become tracked by
- *      executing NXAST_CT with a "recirc_table" specified. This makes
- *      various aspects about the connection available, in particular
- *      the connection state.
- *
- *      An NXAST_CT action always puts the packet into an untracked
- *      state for the current processing path.  If "recirc_table" is
- *      set, execution is forked and the packet passes through the
- *      connection tracker.  The specified table's processing path is
- *      able to match on Connection state until the end of the OpenFlow
- *      pipeline or NXAST_CT is called again.
+ *      Untracked packets have not yet passed through the connection tracker,
+ *      and the connection state for such packets is unknown. In most cases,
+ *      packets entering the OpenFlow pipeline will initially be in the
+ *      untracked state. Untracked packets may become tracked by executing
+ *      NXAST_CT with a "recirc_table" specified. This makes various aspects
+ *      about the connection available, in particular the connection state.
+ *
+ *      Tracked packets have previously passed through the connection tracker.
+ *      These packets will remain tracked through until the end of the OpenFlow
+ *      pipeline. Tracked packets which have NXAST_CT executed with a
+ *      "recirc_table" specified will return to the tracked state.
+ *
+ *      The packet state is only significant for the duration of packet
+ *      processing within the OpenFlow pipeline.
  *
  *   - Connection State:
  *
diff --git a/ofproto/ofproto-dpif-xlate.c b/ofproto/ofproto-dpif-xlate.c
index 9e1f837cb23e..973e760547fa 100644
--- a/ofproto/ofproto-dpif-xlate.c
+++ b/ofproto/ofproto-dpif-xlate.c
@@ -5721,7 +5721,9 @@  put_ct_nat(struct xlate_ctx *ctx)
 static void
 compose_conntrack_action(struct xlate_ctx *ctx, struct ofpact_conntrack *ofc)
 {
+    ovs_u128 old_ct_label = ctx->xin->flow.ct_label;
     ovs_u128 old_ct_label_mask = ctx->wc->masks.ct_label;
+    uint32_t old_ct_mark = ctx->xin->flow.ct_mark;
     uint32_t old_ct_mark_mask = ctx->wc->masks.ct_mark;
     size_t ct_offset;
     uint16_t zone;
@@ -5733,7 +5735,7 @@  compose_conntrack_action(struct xlate_ctx *ctx, struct ofpact_conntrack *ofc)
     /* Process nested actions first, to populate the key. */
     ctx->ct_nat_action = NULL;
     ctx->wc->masks.ct_mark = 0;
-    ctx->wc->masks.ct_label = OVS_U128_ZERO;
+    ctx->wc->masks.ct_label.u64.hi = ctx->wc->masks.ct_label.u64.lo = 0;
     do_xlate_actions(ofc->actions, ofpact_ct_get_action_len(ofc), ctx);
 
     if (ofc->zone_src.field) {
@@ -5759,18 +5761,23 @@  compose_conntrack_action(struct xlate_ctx *ctx, struct ofpact_conntrack *ofc)
     ctx->ct_nat_action = NULL;
     nl_msg_end_nested(ctx->odp_actions, ct_offset);
 
+    /* Restore the original ct fields in the key. These should only be exposed
+     * after recirculation to another table. */
+    ctx->xin->flow.ct_mark = old_ct_mark;
     ctx->wc->masks.ct_mark = old_ct_mark_mask;
+    ctx->xin->flow.ct_label = old_ct_label;
     ctx->wc->masks.ct_label = old_ct_label_mask;
 
-    if (ofc->recirc_table != NX_CT_RECIRC_NONE) {
+    if (ofc->recirc_table == NX_CT_RECIRC_NONE) {
+        /* If we do not recirculate as part of this action, hide the results of
+         * connection tracking from subsequent recirculations. */
+        ctx->conntracked = false;
+    } else {
+        /* Use ct_* fields from datapath during recirculation upcall. */
         ctx->conntracked = true;
         compose_recirculate_and_fork(ctx, ofc->recirc_table);
+        ctx->conntracked = false;
     }
-
-    /* The ct_* fields are only available in the scope of the 'recirc_table'
-     * call chain. */
-    flow_clear_conntrack(&ctx->xin->flow);
-    ctx->conntracked = false;
 }
 
 static void
diff --git a/tests/ofproto-dpif.at b/tests/ofproto-dpif.at
index 28a7e827cac2..284a65ec6524 100644
--- a/tests/ofproto-dpif.at
+++ b/tests/ofproto-dpif.at
@@ -8949,7 +8949,7 @@  OVS_WAIT_UNTIL([ovs-appctl -t ovs-ofctl exit])
 
 dnl Check this output. We only see the latter two packets, not the first.
 AT_CHECK([cat ofctl_monitor.log], [0], [dnl
-NXT_PACKET_IN (xid=0x0): table_id=6 cookie=0x0 total_len=42 reg0=0x1,reg1=0x4d2,reg2=0x1,reg3=0x1,reg4=0x1,in_port=1 (via action) data_len=42 (unbuffered)
+NXT_PACKET_IN (xid=0x0): table_id=6 cookie=0x0 total_len=42 ct_state=new|trk,ct_zone=1,ct_nw_src=10.1.1.1,ct_nw_dst=10.1.1.2,ct_nw_proto=17,ct_tp_src=1,ct_tp_dst=2,reg0=0x1,reg1=0x4d2,reg2=0x1,reg3=0x1,reg4=0x1,in_port=1 (via action) data_len=42 (unbuffered)
 udp,vlan_tci=0x0000,dl_src=50:54:00:00:00:09,dl_dst=50:54:00:00:00:0a,nw_src=10.1.1.1,nw_dst=10.1.1.2,nw_tos=0,nw_ecn=0,nw_ttl=64,tp_src=1,tp_dst=2 udp_csum:e9d6
 dnl
 NXT_PACKET_IN (xid=0x0): table_id=6 cookie=0x0 total_len=42 ct_state=est|rpl|trk,ct_zone=1,ct_mark=0x1,ct_label=0x4d2000000000000000000000000,ct_nw_src=10.1.1.1,ct_nw_dst=10.1.1.2,ct_nw_proto=17,ct_tp_src=1,ct_tp_dst=2,reg0=0x1,reg1=0x4d2,reg2=0x1,reg3=0x2,reg4=0x1,in_port=2 (via action) data_len=42 (unbuffered)
@@ -8970,7 +8970,7 @@  OVS_WAIT_UNTIL([ovs-appctl -t ovs-ofctl exit])
 
 dnl Check this output. We should see both packets
 AT_CHECK([cat ofctl_monitor.log], [0], [dnl
-NXT_PACKET_IN (xid=0x0): table_id=6 cookie=0x0 total_len=42 reg0=0x1,reg1=0x4d2,reg2=0x1,reg3=0x1,reg4=0x1,in_port=1 (via action) data_len=42 (unbuffered)
+NXT_PACKET_IN (xid=0x0): table_id=6 cookie=0x0 total_len=42 ct_state=new|trk,ct_zone=1,ct_nw_src=10.1.1.1,ct_nw_dst=10.1.1.2,ct_nw_proto=17,ct_tp_src=3,ct_tp_dst=2,reg0=0x1,reg1=0x4d2,reg2=0x1,reg3=0x1,reg4=0x1,in_port=1 (via action) data_len=42 (unbuffered)
 udp,vlan_tci=0x0000,dl_src=50:54:00:00:00:09,dl_dst=50:54:00:00:00:0a,nw_src=10.1.1.1,nw_dst=10.1.1.2,nw_tos=0,nw_ecn=0,nw_ttl=64,tp_src=3,tp_dst=2 udp_csum:e9d4
 dnl
 NXT_PACKET_IN (xid=0x0): table_id=6 cookie=0x0 total_len=42 ct_state=est|rpl|trk,ct_zone=1,ct_mark=0x1,ct_label=0x4d2000000000000000000000000,ct_nw_src=10.1.1.1,ct_nw_dst=10.1.1.2,ct_nw_proto=17,ct_tp_src=3,ct_tp_dst=2,reg0=0x1,reg1=0x4d2,reg2=0x1,reg3=0x2,reg4=0x1,in_port=2 (via action) data_len=42 (unbuffered)
@@ -9025,7 +9025,7 @@  AT_CHECK([cat ofctl_monitor.log], [0], [dnl
 NXT_PACKET_IN (xid=0x0): cookie=0x0 total_len=42 in_port=1 (via action) data_len=42 (unbuffered)
 udp,vlan_tci=0x0000,dl_src=50:54:00:00:00:09,dl_dst=50:54:00:00:00:0a,nw_src=10.1.1.1,nw_dst=10.1.1.2,nw_tos=0,nw_ecn=0,nw_ttl=64,tp_src=1,tp_dst=2 udp_csum:e9d6
 dnl
-NXT_PACKET_IN (xid=0x0): table_id=1 cookie=0x0 total_len=42 in_port=2 (via action) data_len=42 (unbuffered)
+NXT_PACKET_IN (xid=0x0): table_id=1 cookie=0x0 total_len=42 ct_state=est|rpl|trk,ct_nw_src=10.1.1.1,ct_nw_dst=10.1.1.2,ct_nw_proto=17,ct_tp_src=1,ct_tp_dst=2,in_port=2 (via action) data_len=42 (unbuffered)
 udp,vlan_tci=0x0000,dl_src=50:54:00:00:00:0a,dl_dst=50:54:00:00:00:09,nw_src=10.1.1.2,nw_dst=10.1.1.1,nw_tos=0,nw_ecn=0,nw_ttl=64,tp_src=2,tp_dst=1 udp_csum:e9d6
 ])
 
@@ -9047,7 +9047,7 @@  AT_CHECK([cat ofctl_monitor.log], [0], [dnl
 NXT_PACKET_IN (xid=0x0): cookie=0x0 total_len=42 in_port=1 (via action) data_len=42 (unbuffered)
 udp,vlan_tci=0x0000,dl_src=50:54:00:00:00:09,dl_dst=50:54:00:00:00:0a,nw_src=10.1.1.1,nw_dst=10.1.1.2,nw_tos=0,nw_ecn=0,nw_ttl=64,tp_src=3,tp_dst=4 udp_csum:e9d2
 dnl
-NXT_PACKET_IN (xid=0x0): table_id=1 cookie=0x0 total_len=42 in_port=2 (via action) data_len=42 (unbuffered)
+NXT_PACKET_IN (xid=0x0): table_id=1 cookie=0x0 total_len=42 ct_state=est|rpl|trk,ct_nw_src=10.1.1.1,ct_nw_dst=10.1.1.2,ct_nw_proto=17,ct_tp_src=3,ct_tp_dst=4,in_port=2 (via action) data_len=42 (unbuffered)
 udp,vlan_tci=0x0000,dl_src=50:54:00:00:00:0a,dl_dst=50:54:00:00:00:09,nw_src=10.1.1.2,nw_dst=10.1.1.1,nw_tos=0,nw_ecn=0,nw_ttl=64,tp_src=4,tp_dst=3 udp_csum:e9d2
 ])
 
@@ -9362,7 +9362,7 @@  OVS_WAIT_UNTIL([ovs-appctl -t ovs-ofctl exit])
 
 dnl Check this output. We only see the latter two packets, not the first.
 AT_CHECK([cat ofctl_monitor.log], [0], [dnl
-NXT_PACKET_IN (xid=0x0): table_id=1 cookie=0x0 total_len=42 in_port=1 (via action) data_len=42 (unbuffered)
+NXT_PACKET_IN (xid=0x0): table_id=1 cookie=0x0 total_len=42 ct_state=new|trk,ct_nw_src=10.1.1.1,ct_nw_dst=10.1.1.2,ct_nw_proto=17,ct_tp_src=1,ct_tp_dst=2,in_port=1 (via action) data_len=42 (unbuffered)
 udp,vlan_tci=0x0000,dl_src=50:54:00:00:00:09,dl_dst=50:54:00:00:00:0a,nw_src=10.1.1.1,nw_dst=10.1.1.2,nw_tos=0,nw_ecn=0,nw_ttl=64,tp_src=1,tp_dst=2 udp_csum:e9d6
 dnl
 NXT_PACKET_IN (xid=0x0): table_id=1 cookie=0x0 total_len=42 ct_state=est|rpl|trk,ct_nw_src=10.1.1.1,ct_nw_dst=10.1.1.2,ct_nw_proto=17,ct_tp_src=1,ct_tp_dst=2,in_port=2 (via action) data_len=42 (unbuffered)
diff --git a/tests/system-traffic.at b/tests/system-traffic.at
index 522eaa615834..798dd2cbd2c2 100644
--- a/tests/system-traffic.at
+++ b/tests/system-traffic.at
@@ -2287,7 +2287,7 @@  dnl Ingress pipeline
 dnl - Allow all connections from LOCAL port (commit and proceed to egress)
 dnl - All other connections go through conntracker using the input port as
 dnl   a connection tracking zone.
-table=1,priority=150,in_port=LOCAL,ip,ct_state=+trk+new,action=ct(commit,table=2,zone=OXM_OF_IN_PORT[[0..15]])
+table=1,priority=150,in_port=LOCAL,ip,ct_state=+trk+new,action=ct(commit,zone=OXM_OF_IN_PORT[[0..15]]),goto_table:2
 table=1,priority=100,ip,action=ct(table=2,zone=OXM_OF_IN_PORT[[0..15]])
 table=1,priority=1,action=drop
 
@@ -2295,7 +2295,7 @@  dnl Egress pipeline
 dnl - Allow all connections from LOCAL port (commit and skip to output)
 dnl - Allow other established connections to go through conntracker using
 dnl   output port as a connection tracking zone.
-table=2,priority=150,in_port=LOCAL,ip,ct_state=+trk+new,action=ct(commit,table=4,zone=NXM_NX_REG0[[0..15]])
+table=2,priority=150,in_port=LOCAL,ip,ct_state=+trk+new,action=ct(commit,zone=NXM_NX_REG0[[0..15]]),goto_table:4
 table=2,priority=100,ip,ct_state=+trk+est,action=ct(table=3,zone=NXM_NX_REG0[[0..15]])
 table=2,priority=1,action=drop
 
diff --git a/utilities/ovs-ofctl.8.in b/utilities/ovs-ofctl.8.in
index c65de97f5e2e..f6bd90374a18 100644
--- a/utilities/ovs-ofctl.8.in
+++ b/utilities/ovs-ofctl.8.in
@@ -1031,13 +1031,11 @@  Restores the queue to the value it was before any \fBset_queue\fR
 actions were applied.
 .
 .IP \fBct\fR
-.IQ \fBct(\fR[\fIargument\fR][\fB,\fIargument\fR...]\fB)
+.IQ \fBct\fB(\fR[\fIargument\fR][\fB,\fIargument\fR...]\fB)
 Send the packet through the connection tracker.  Refer to the \fBct_state\fR
-documentation above for possible packet and connection states. A \fBct\fR
-action always sets the packet to an untracked state and clears out the
-\fBct_state\fR fields for the current processing path.  Those fields are
-only available for the processing path pointed to by the \fBtable\fR
-argument.  The following arguments are supported:
+documentation above for possible packet and connection states. The following
+arguments are supported:
+
 .RS
 .IP \fBcommit\fR
 .RS