diff mbox series

[ovs-dev,2/2] openflow: Add extension to flush CT by generic match

Message ID 20221020105311.114842-3-amusil@redhat.com
State Changes Requested
Headers show
Series Add extension for partial CT flush | 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

Ales Musil Oct. 20, 2022, 10:53 a.m. UTC
Add extension that allows to flush connections from CT
by specifying fields that the connections should be
matched against. This allows to match only some fields
of the connection e.g. source address for orig direrction.

Reported-at: https://bugzilla.redhat.com/2120546
Signed-off-by: Ales Musil <amusil@redhat.com>
---
 NEWS                           |  3 ++
 include/openflow/nicira-ext.h  | 26 +++++++++++++
 include/openvswitch/ofp-msgs.h |  4 ++
 include/openvswitch/ofp-util.h |  4 ++
 lib/ofp-bundle.c               |  1 +
 lib/ofp-ct-util.c              | 40 +++++++++++++++++++
 lib/ofp-ct-util.h              |  3 ++
 lib/ofp-print.c                | 16 ++++++++
 lib/ofp-util.c                 | 36 +++++++++++++++++
 lib/rconn.c                    |  1 +
 ofproto/ofproto-dpif.c         |  8 +++-
 ofproto/ofproto-provider.h     |  7 +++-
 ofproto/ofproto.c              | 24 +++++++++++-
 tests/ofp-print.at             | 71 ++++++++++++++++++++++++++++++++++
 14 files changed, 239 insertions(+), 5 deletions(-)

Comments

Ales Musil Dec. 1, 2022, 1:19 p.m. UTC | #1
On Thu, Oct 20, 2022 at 12:53 PM Ales Musil <amusil@redhat.com> wrote:

> Add extension that allows to flush connections from CT
> by specifying fields that the connections should be
> matched against. This allows to match only some fields
> of the connection e.g. source address for orig direrction.
>
> Reported-at: https://bugzilla.redhat.com/2120546
> Signed-off-by: Ales Musil <amusil@redhat.com>
> ---
>  NEWS                           |  3 ++
>  include/openflow/nicira-ext.h  | 26 +++++++++++++
>  include/openvswitch/ofp-msgs.h |  4 ++
>  include/openvswitch/ofp-util.h |  4 ++
>  lib/ofp-bundle.c               |  1 +
>  lib/ofp-ct-util.c              | 40 +++++++++++++++++++
>  lib/ofp-ct-util.h              |  3 ++
>  lib/ofp-print.c                | 16 ++++++++
>  lib/ofp-util.c                 | 36 +++++++++++++++++
>  lib/rconn.c                    |  1 +
>  ofproto/ofproto-dpif.c         |  8 +++-
>  ofproto/ofproto-provider.h     |  7 +++-
>  ofproto/ofproto.c              | 24 +++++++++++-
>  tests/ofp-print.at             | 71 ++++++++++++++++++++++++++++++++++
>  14 files changed, 239 insertions(+), 5 deletions(-)
>
> diff --git a/NEWS b/NEWS
> index 81909812e..20ffd0a2a 100644
> --- a/NEWS
> +++ b/NEWS
> @@ -25,6 +25,9 @@ Post-v3.0.0
>         DPDK 21.11.2.
>     - ovs-dpctl and related ovs-appctl commands:
>       * "flush-conntrack" is capable of handling partial 5-tuple.
> +   - OpenFlow:
> +      * New OpenFlow extension NXT_CT_FLUSH to flush connections matching
> +        the specified fields.
>
>
>  v3.0.0 - 15 Aug 2022
> diff --git a/include/openflow/nicira-ext.h b/include/openflow/nicira-ext.h
> index b68804991..90013bc36 100644
> --- a/include/openflow/nicira-ext.h
> +++ b/include/openflow/nicira-ext.h
> @@ -1064,4 +1064,30 @@ struct nx_zone_id {
>  };
>  OFP_ASSERT(sizeof(struct nx_zone_id) == 8);
>
> +/* NXT_CT_FLUSH.
> + *
> + * Flushes the connection tracking specified by 5-tuple. */
> +struct nx_ct_flush {
> +    uint8_t ip_proto;          /* IP protocol. */
> +    uint8_t family;            /* L3 address family. */
> +    ovs_be16 zone_id;          /* CT zone id. */
> +
> +    /* The orig direction section. */
> +    ovs_be32 orig_src[4];      /* CT source IPv6 or mapped IPv4 address.
> */
> +    ovs_be32 orig_dst[4];      /* CT destination IPv6 or mapped IPv4
> +                                * address. */
> +    ovs_be16 orig_src_port;    /* CT source port or ICMP id. */
> +    ovs_be16 orig_dst_port;    /* CT destination port or ICMP type and
> ICMP
> +                                * code. */
> +
> +    /* The reply direction section. */
> +    ovs_be32 reply_src[4];      /* CT source IPv6 or mapped IPv4 address.
> */
> +    ovs_be32 reply_dst[4];      /* CT destination IPv6 or mapped IPv4
> +                                * address. */
> +    ovs_be16 reply_src_port;    /* CT source port or ICMP id. */
> +    ovs_be16 reply_dst_port;    /* CT destination port or ICMP type and
> ICMP
> +                                * code. */
> +};
> +OFP_ASSERT(sizeof(struct nx_ct_flush) == 76);
> +
>  #endif /* openflow/nicira-ext.h */
> diff --git a/include/openvswitch/ofp-msgs.h
> b/include/openvswitch/ofp-msgs.h
> index 921a937e5..80f12481c 100644
> --- a/include/openvswitch/ofp-msgs.h
> +++ b/include/openvswitch/ofp-msgs.h
> @@ -526,6 +526,9 @@ enum ofpraw {
>
>      /* NXST 1.0+ (4): struct nx_ipfix_stats_reply[]. */
>      OFPRAW_NXST_IPFIX_FLOW_REPLY,
> +
> +    /* NXT 1.0+ (32): struct nx_ct_flush. */
> +    OFPRAW_NXT_CT_FLUSH,
>  };
>
>  /* Decoding messages into OFPRAW_* values. */
> @@ -772,6 +775,7 @@ enum ofptype {
>      OFPTYPE_IPFIX_FLOW_STATS_REQUEST, /* OFPRAW_NXST_IPFIX_FLOW_REQUEST */
>      OFPTYPE_IPFIX_FLOW_STATS_REPLY,   /* OFPRAW_NXST_IPFIX_FLOW_REPLY */
>      OFPTYPE_CT_FLUSH_ZONE,            /* OFPRAW_NXT_CT_FLUSH_ZONE. */
> +    OFPTYPE_CT_FLUSH,           /* OFPRAW_NXT_CT_FLUSH. */
>
>      /* Flow monitor extension. */
>      OFPTYPE_FLOW_MONITOR_CANCEL,  /* OFPRAW_NXT_FLOW_MONITOR_CANCEL.
> diff --git a/include/openvswitch/ofp-util.h
> b/include/openvswitch/ofp-util.h
> index 84937ae26..2e533fa4f 100644
> --- a/include/openvswitch/ofp-util.h
> +++ b/include/openvswitch/ofp-util.h
> @@ -65,6 +65,10 @@ struct ofpbuf *ofputil_encode_echo_reply(const struct
> ofp_header *);
>
>  struct ofpbuf *ofputil_encode_barrier_request(enum ofp_version);
>
> +struct ofpbuf *ofputil_ct_match_encode(const struct ofputil_ct_match *,
> +                                       uint16_t zone_id,
> +                                       enum ofp_version version);
> +
>  #ifdef __cplusplus
>  }
>  #endif
> diff --git a/lib/ofp-bundle.c b/lib/ofp-bundle.c
> index 0161c2bc6..941a8370e 100644
> --- a/lib/ofp-bundle.c
> +++ b/lib/ofp-bundle.c
> @@ -292,6 +292,7 @@ ofputil_is_bundlable(enum ofptype type)
>      case OFPTYPE_IPFIX_FLOW_STATS_REQUEST:
>      case OFPTYPE_IPFIX_FLOW_STATS_REPLY:
>      case OFPTYPE_CT_FLUSH_ZONE:
> +    case OFPTYPE_CT_FLUSH:
>          break;
>      }
>
> diff --git a/lib/ofp-ct-util.c b/lib/ofp-ct-util.c
> index 9112305cc..2e7f7ffc1 100644
> --- a/lib/ofp-ct-util.c
> +++ b/lib/ofp-ct-util.c
> @@ -23,7 +23,9 @@
>
>  #include "ct-dpif.h"
>  #include "ofp-ct-util.h"
> +#include "openflow/nicira-ext.h"
>  #include "openvswitch/dynamic-string.h"
> +#include "openvswitch/ofp-msgs.h"
>  #include "openvswitch/ofp-parse.h"
>  #include "openvswitch/ofp-util.h"
>  #include "openvswitch/packets.h"
> @@ -309,3 +311,41 @@ error:
>      free(copy);
>      return false;
>  }
> +
> +void
> +ofputil_ct_match_decode(struct ofputil_ct_match *match, uint16_t *zone_id,
> +                        const struct ofp_header *oh)
> +{
> +    const struct nx_ct_flush *nx_flush = ofpmsg_body(oh);
> +
> +    struct ofputil_ct_tuple *orig = &match->tuple_orig;
> +    struct ofputil_ct_tuple *reply = &match->tuple_reply;
> +
> +    *zone_id = ntohs(nx_flush->zone_id);
> +
> +    match->l3_type = nx_flush->family;
> +    match->ip_proto = nx_flush->ip_proto;
> +
> +    memcpy(&orig->src, &nx_flush->orig_src, sizeof orig->src);
> +    memcpy(&orig->dst, &nx_flush->orig_dst, sizeof orig->dst);
> +
> +    memcpy(&reply->src, &nx_flush->reply_src, sizeof reply->src);
> +    memcpy(&reply->dst, &nx_flush->reply_dst, sizeof reply->dst);
> +
> +    orig->src_port = nx_flush->orig_src_port;
> +    reply->src_port = nx_flush->reply_src_port;
> +
> +    if (match->ip_proto == IPPROTO_ICMP || match->ip_proto ==
> IPPROTO_ICMPV6) {
> +        uint16_t icmp = ntohs(nx_flush->orig_dst_port);
> +        orig->icmp_type = icmp >> 8 & 0xff;
> +        orig->icmp_code = icmp & 0xff;
> +
> +        icmp = ntohs(nx_flush->reply_dst_port);
> +        reply->icmp_type = icmp >> 8 & 0xff;
> +        reply->icmp_code = icmp & 0xff;
> +    } else {
> +        orig->dst_port = nx_flush->orig_dst_port;
> +        reply->dst_port = nx_flush->reply_dst_port;
> +    }
> +}
> +
> diff --git a/lib/ofp-ct-util.h b/lib/ofp-ct-util.h
> index 6e8f0f68a..4c6e61e2d 100644
> --- a/lib/ofp-ct-util.h
> +++ b/lib/ofp-ct-util.h
> @@ -31,4 +31,7 @@ void ofputil_ct_match_format(struct ds *ds,
>  bool ofputil_ct_match_parse(struct ofputil_ct_match *match, const char *s,
>                              struct ds *ds);
>
> +void ofputil_ct_match_decode(struct ofputil_ct_match *match, uint16_t
> *zone_id,
> +                             const struct ofp_header *oh);
> +
>  #endif // lib/ofp-ct-util.h
> diff --git a/lib/ofp-print.c b/lib/ofp-print.c
> index bd37fa17a..fd4e982b6 100644
> --- a/lib/ofp-print.c
> +++ b/lib/ofp-print.c
> @@ -36,6 +36,7 @@
>  #include "learn.h"
>  #include "multipath.h"
>  #include "netdev.h"
> +#include "ofp-ct-util.h"
>  #include "nx-match.h"
>  #include "odp-util.h"
>  #include "openflow/nicira-ext.h"
> @@ -949,6 +950,19 @@ ofp_print_nxt_ct_flush_zone(struct ds *string, const
> struct nx_zone_id *nzi)
>      return 0;
>  }
>
> +static enum ofperr
> +ofp_print_nxt_ct_flush(struct ds *string, const struct ofp_header *oh)
> +{
> +    uint16_t zone_id;
> +    struct ofputil_ct_match match = {0};
> +
> +    ofputil_ct_match_decode(&match, &zone_id, oh);
> +    ofputil_ct_match_format(string, &match);
> +    ds_put_format(string, ",zone_id=%"PRIu16, zone_id);
> +
> +    return 0;
> +}
> +
>  static enum ofperr
>  ofp_to_string__(const struct ofp_header *oh,
>                  const struct ofputil_port_map *port_map,
> @@ -1184,6 +1198,8 @@ ofp_to_string__(const struct ofp_header *oh,
>
>      case OFPTYPE_CT_FLUSH_ZONE:
>          return ofp_print_nxt_ct_flush_zone(string, ofpmsg_body(oh));
> +    case OFPTYPE_CT_FLUSH:
> +        return ofp_print_nxt_ct_flush(string, oh);
>      }
>
>      return 0;
> diff --git a/lib/ofp-util.c b/lib/ofp-util.c
> index a324ceeea..51c42357f 100644
> --- a/lib/ofp-util.c
> +++ b/lib/ofp-util.c
> @@ -237,3 +237,39 @@ ofputil_encode_barrier_request(enum ofp_version
> ofp_version)
>
>      return ofpraw_alloc(type, ofp_version, 0);
>  }
> +
> +struct ofpbuf *
> +ofputil_ct_match_encode(const struct ofputil_ct_match *match, uint16_t
> zone_id,
> +                        enum ofp_version version)
> +{
> +    struct ofpbuf *msg = ofpraw_alloc(OFPRAW_NXT_CT_FLUSH, version, 0);
> +    struct nx_ct_flush *nx_flush = ofpbuf_put_zeros(msg, sizeof
> *nx_flush);
> +    const struct ofputil_ct_tuple *orig = &match->tuple_orig;
> +    const struct ofputil_ct_tuple *reply = &match->tuple_reply;
> +
> +    nx_flush->ip_proto = match->ip_proto;
> +    nx_flush->family = match->l3_type;
> +    nx_flush->zone_id = htons(zone_id);
> +
> +    memcpy(&nx_flush->orig_src, &orig->src, sizeof nx_flush->orig_src);
> +    memcpy(&nx_flush->orig_dst, &orig->dst, sizeof nx_flush->orig_dst);
> +    memcpy(&nx_flush->reply_src, &reply->src, sizeof nx_flush->reply_src);
> +    memcpy(&nx_flush->reply_dst, &reply->dst, sizeof nx_flush->reply_dst);
> +
> +    nx_flush->orig_src_port = orig->src_port;
> +    nx_flush->reply_src_port = reply->src_port;
> +
> +    if (nx_flush->ip_proto == IPPROTO_ICMP
> +        || nx_flush->ip_proto == IPPROTO_ICMPV6) {
> +
> +        nx_flush->orig_dst_port =
> +            htons(orig->icmp_type << 8 | orig->icmp_code);
> +        nx_flush->reply_dst_port =
> +            htons(reply->icmp_type << 8 | reply->icmp_code);
> +    } else {
> +        nx_flush->orig_dst_port = orig->dst_port;
> +        nx_flush->reply_dst_port = reply->dst_port;
> +    }
> +
> +    return msg;
> +}
> diff --git a/lib/rconn.c b/lib/rconn.c
> index a96b2eb8b..4afa21515 100644
> --- a/lib/rconn.c
> +++ b/lib/rconn.c
> @@ -1426,6 +1426,7 @@ is_admitted_msg(const struct ofpbuf *b)
>      case OFPTYPE_IPFIX_FLOW_STATS_REQUEST:
>      case OFPTYPE_IPFIX_FLOW_STATS_REPLY:
>      case OFPTYPE_CT_FLUSH_ZONE:
> +    case OFPTYPE_CT_FLUSH:
>      default:
>          return true;
>      }
> diff --git a/ofproto/ofproto-dpif.c b/ofproto/ofproto-dpif.c
> index f9562dee8..29174a585 100644
> --- a/ofproto/ofproto-dpif.c
> +++ b/ofproto/ofproto-dpif.c
> @@ -5358,11 +5358,12 @@ type_set_config(const char *type, const struct
> smap *other_config)
>  }
>
>  static void
> -ct_flush(const struct ofproto *ofproto_, const uint16_t *zone)
> +ct_flush(const struct ofproto *ofproto_, const uint16_t *zone,
> +         const struct ofputil_ct_match *match)
>  {
>      struct ofproto_dpif *ofproto = ofproto_dpif_cast(ofproto_);
>
> -    ct_dpif_flush(ofproto->backer->dpif, zone, NULL);
> +    ct_dpif_flush(ofproto->backer->dpif, zone, match);
>  }
>
>  static struct ct_timeout_policy *
> @@ -5674,6 +5675,9 @@ get_datapath_cap(const char *datapath_type, struct
> smap *cap)
>      smap_add(cap, "lb_output_action", s.lb_output_action ? "true" :
> "false");
>      smap_add(cap, "ct_zero_snat", s.ct_zero_snat ? "true" : "false");
>      smap_add(cap, "add_mpls", s.add_mpls ? "true" : "false");
> +    /* The ct_tuple_flush is implemented on dpif level, so it is supported
> +     * for all backers. */
> +    smap_add(cap, "ct_flush", "true");
>  }
>
>  /* Gets timeout policy name in 'backer' based on 'zone', 'dl_type' and
> diff --git a/ofproto/ofproto-provider.h b/ofproto/ofproto-provider.h
> index 7e3fb6698..5e39234f9 100644
> --- a/ofproto/ofproto-provider.h
> +++ b/ofproto/ofproto-provider.h
> @@ -49,6 +49,7 @@
>  #include "openvswitch/ofp-port.h"
>  #include "openvswitch/ofp-switch.h"
>  #include "openvswitch/ofp-table.h"
> +#include "openvswitch/ofp-util.h"
>  #include "ovs-atomic.h"
>  #include "ovs-rcu.h"
>  #include "ovs-thread.h"
> @@ -1902,8 +1903,10 @@ struct ofproto_class {
>  /* ## Connection tracking ## */
>  /* ## ------------------- ## */
>      /* Flushes the connection tracking tables. If 'zone' is not NULL,
> -     * only deletes connections in '*zone'. */
> -    void (*ct_flush)(const struct ofproto *, const uint16_t *zone);
> +     * only deletes connections in '*zone'. If 'match' is not NULL,
> +     * deletes connections specified by the match. */
> +    void (*ct_flush)(const struct ofproto *, const uint16_t *zone,
> +                     const struct ofputil_ct_match *match);
>
>      /* Sets conntrack timeout policy specified by 'timeout_policy' to
> 'zone'
>       * in datapath type 'dp_type'. */
> diff --git a/ofproto/ofproto.c b/ofproto/ofproto.c
> index 3a527683c..1aee6b327 100644
> --- a/ofproto/ofproto.c
> +++ b/ofproto/ofproto.c
> @@ -34,6 +34,7 @@
>  #include "openvswitch/hmap.h"
>  #include "netdev.h"
>  #include "nx-match.h"
> +#include "ofp-ct-util.h"
>  #include "ofproto.h"
>  #include "ofproto-provider.h"
>  #include "openflow/nicira-ext.h"
> @@ -934,7 +935,25 @@ handle_nxt_ct_flush_zone(struct ofconn *ofconn, const
> struct ofp_header *oh)
>
>      uint16_t zone = ntohs(nzi->zone_id);
>      if (ofproto->ofproto_class->ct_flush) {
> -        ofproto->ofproto_class->ct_flush(ofproto, &zone);
> +        ofproto->ofproto_class->ct_flush(ofproto, &zone, NULL);
> +    } else {
> +        return EOPNOTSUPP;
> +    }
> +
> +    return 0;
> +}
> +
> +static enum ofperr
> +handle_nxt_ct_flush(struct ofconn *ofconn, const struct ofp_header *oh)
> +{
> +    struct ofproto *ofproto = ofconn_get_ofproto(ofconn);
> +    struct ofputil_ct_match match = {0};
> +    uint16_t zone_id;
> +
> +    ofputil_ct_match_decode(&match, &zone_id, oh);
> +
> +    if (ofproto->ofproto_class->ct_flush) {
> +        ofproto->ofproto_class->ct_flush(ofproto, &zone_id, &match);
>

I've realized that v1 doesn't allow zone_id being NULL. However I'm not
sure how to put that information into the extension struct.
I'm open to any suggestion, I was thinking about flags field, which would
grow the whole struct by 4 bytes.


>      } else {
>          return EOPNOTSUPP;
>      }
> @@ -8787,6 +8806,9 @@ handle_single_part_openflow(struct ofconn *ofconn,
> const struct ofp_header *oh,
>      case OFPTYPE_CT_FLUSH_ZONE:
>          return handle_nxt_ct_flush_zone(ofconn, oh);
>
> +    case OFPTYPE_CT_FLUSH:
> +        return handle_nxt_ct_flush(ofconn, oh);
> +
>      case OFPTYPE_HELLO:
>      case OFPTYPE_ERROR:
>      case OFPTYPE_FEATURES_REPLY:
> diff --git a/tests/ofp-print.at b/tests/ofp-print.at
> index fe41cc42c..418e98559 100644
> --- a/tests/ofp-print.at
> +++ b/tests/ofp-print.at
> @@ -4073,3 +4073,74 @@ AT_CHECK([ovs-ofctl ofp-print "\
>  NXT_CT_FLUSH_ZONE (xid=0x3): zone_id=13
>  ])
>  AT_CLEANUP
> +
> +AT_SETUP([NXT_CT_FLUSH])
> +AT_KEYWORDS([ofp-print])
> +AT_CHECK([ovs-ofctl ofp-print "\
> +01 04 00 5c 00 00 00 03 00 00 23 20 00 00 00 20 \
> +06 \
> +02 \
> +00 0d \
> +00 00 00 00 00 00 00 00 00 00 ff ff 0a 0a 00 01 \
> +00 00 00 00 00 00 00 00 00 00 ff ff 0a 0a 00 02 \
> +00 50 \
> +1f 90 \
> +00 00 00 00 00 00 00 00 00 00 ff ff 0a 0a 00 02 \
> +00 00 00 00 00 00 00 00 00 00 ff ff 0a 0a 00 01 \
> +1f 90 \
> +00 50 \
> +"], [0], [dnl
> +NXT_CT_FLUSH (xid=0x3):
> l3_type=2,ip_proto=6,orig=(src=10.10.0.1,dst=10.10.0.2,src_port=80,dst_port=8080),reply=(src=10.10.0.2,dst=10.10.0.1,src_port=8080,dst_port=80),zone_id=13
> +])
> +
> +AT_CHECK([ovs-ofctl ofp-print "\
> +01 04 00 5c 00 00 00 03 00 00 23 20 00 00 00 20 \
> +06 \
> +0a \
> +00 0d \
> +fd 18 00 00 00 00 00 00 00 00 ff ff ab cd 00 01 \
> +fd 18 00 00 00 00 00 00 00 00 ff ff ab cd 00 02 \
> +00 50 \
> +1f 90 \
> +fd 18 00 00 00 00 00 00 00 00 ff ff ab cd 00 02 \
> +fd 18 00 00 00 00 00 00 00 00 ff ff ab cd 00 01 \
> +1f 90 \
> +00 50 \
> +"], [0], [dnl
> +NXT_CT_FLUSH (xid=0x3):
> l3_type=10,ip_proto=6,orig=(src=fd18::ffff:abcd:1,dst=fd18::ffff:abcd:2,src_port=80,dst_port=8080),reply=(src=fd18::ffff:abcd:2,dst=fd18::ffff:abcd:1,src_port=8080,dst_port=80),zone_id=13
> +])
> +
> +AT_CHECK([ovs-ofctl ofp-print "\
> +01 04 00 5c 00 00 00 03 00 00 23 20 00 00 00 20 \
> +01 \
> +02 \
> +00 0d \
> +00 00 00 00 00 00 00 00 00 00 ff ff 0a 0a 00 01 \
> +00 00 00 00 00 00 00 00 00 00 ff ff 0a 0a 00 02 \
> +00 01 \
> +08 00 \
> +00 00 00 00 00 00 00 00 00 00 ff ff 0a 0a 00 02 \
> +00 00 00 00 00 00 00 00 00 00 ff ff 0a 0a 00 01 \
> +00 01 \
> +00 00 \
> +"], [0], [dnl
> +NXT_CT_FLUSH (xid=0x3):
> l3_type=2,ip_proto=1,orig=(src=10.10.0.1,dst=10.10.0.2,icmp_id=1,icmp_type=8,icmp_code=0),reply=(src=10.10.0.2,dst=10.10.0.1,icmp_id=1,icmp_type=0,icmp_code=0),zone_id=13
> +])
> +
> +AT_CHECK([ovs-ofctl ofp-print "\
> +01 04 00 5c 00 00 00 03 00 00 23 20 00 00 00 20 \
> +01 \
> +0a \
> +00 0d \
> +fd 18 00 00 00 00 00 00 00 00 ff ff ab cd 00 01 \
> +fd 18 00 00 00 00 00 00 00 00 ff ff ab cd 00 02 \
> +00 01 \
> +08 00 \
> +fd 18 00 00 00 00 00 00 00 00 ff ff ab cd 00 02 \
> +fd 18 00 00 00 00 00 00 00 00 ff ff ab cd 00 01 \
> +00 01 \
> +00 00 \
> +"], [0], [dnl
> +NXT_CT_FLUSH (xid=0x3):
> l3_type=10,ip_proto=1,orig=(src=fd18::ffff:abcd:1,dst=fd18::ffff:abcd:2,icmp_id=1,icmp_type=8,icmp_code=0),reply=(src=fd18::ffff:abcd:2,dst=fd18::ffff:abcd:1,icmp_id=1,icmp_type=0,icmp_code=0),zone_id=13
> +])
> +AT_CLEANUP
> --
> 2.37.3
>
>
Ilya Maximets Dec. 5, 2022, 9:38 p.m. UTC | #2
On 12/1/22 14:19, Ales Musil wrote:
>     +
>     +static enum ofperr
>     +handle_nxt_ct_flush(struct ofconn *ofconn, const struct ofp_header *oh)
>     +{
>     +    struct ofproto *ofproto = ofconn_get_ofproto(ofconn);
>     +    struct ofputil_ct_match match = {0};
>     +    uint16_t zone_id;
>     +
>     +    ofputil_ct_match_decode(&match, &zone_id, oh);
>     +
>     +    if (ofproto->ofproto_class->ct_flush) {
>     +        ofproto->ofproto_class->ct_flush(ofproto, &zone_id, &match);
> 
> 
> I've realized that v1 doesn't allow zone_id being NULL. However I'm not sure how to put that information into the extension struct.
> I'm open to any suggestion, I was thinking about flags field, which would grow the whole struct by 4 bytes.


IIUC, you're talking about OpenFlow interface that you created
requiring zone_id to be provided, right?

Optional arguments and multiple arguments with mixed order are
usually solved by using TLVs.  You may shrink down the
'struct nx_ct_flush' structure to only mandatory elements and
make all the 'match' fields including zone_id as TLVs with the
help of include/openvswitch/ofp-prop.h.

One more thing: I don't think we should add dpctl interface for
the flushing.  We don't have such interface for ct-flush-zone
so we should not have it for ct-flush.  Instead, what is missing
in your implementation, is the native OpenFlow interface via
ovs-ofctl, i.e. 'ovs-ofctl ct-flush' command.  And all the tests
should use it instead.  This way we will also have test coverage
for the code that will actually be used by OVN/CMS.

Best regards, Ilya Maximets.
Ales Musil Dec. 6, 2022, 6:29 a.m. UTC | #3
On Mon, Dec 5, 2022 at 10:38 PM Ilya Maximets <i.maximets@ovn.org> wrote:

> On 12/1/22 14:19, Ales Musil wrote:
> >     +
> >     +static enum ofperr
> >     +handle_nxt_ct_flush(struct ofconn *ofconn, const struct ofp_header
> *oh)
> >     +{
> >     +    struct ofproto *ofproto = ofconn_get_ofproto(ofconn);
> >     +    struct ofputil_ct_match match = {0};
> >     +    uint16_t zone_id;
> >     +
> >     +    ofputil_ct_match_decode(&match, &zone_id, oh);
> >     +
> >     +    if (ofproto->ofproto_class->ct_flush) {
> >     +        ofproto->ofproto_class->ct_flush(ofproto, &zone_id, &match);
> >
> >
> > I've realized that v1 doesn't allow zone_id being NULL. However I'm not
> sure how to put that information into the extension struct.
> > I'm open to any suggestion, I was thinking about flags field, which
> would grow the whole struct by 4 bytes.
>
>
> IIUC, you're talking about OpenFlow interface that you created
> requiring zone_id to be provided, right?
>

Yes.


>
> Optional arguments and multiple arguments with mixed order are
> usually solved by using TLVs.  You may shrink down the
> 'struct nx_ct_flush' structure to only mandatory elements and
> make all the 'match' fields including zone_id as TLVs with the
> help of include/openvswitch/ofp-prop.h.
>

That sounds good. I'll look into it, thanks.


>
> One more thing: I don't think we should add dpctl interface for
> the flushing.  We don't have such interface for ct-flush-zone
> so we should not have it for ct-flush.  Instead, what is missing
> in your implementation, is the native OpenFlow interface via
> ovs-ofctl, i.e. 'ovs-ofctl ct-flush' command.  And all the tests
> should use it instead.  This way we will also have test coverage
> for the code that will actually be used by OVN/CMS.
>

The problem is that the dpctl interface actually already exists, and we
shouldn't
just remove it, this wouldn't make sense IMO. The first patch just extends
that interface.
Adding the ovs-ofctl makes, I'll work on it for v2.


>
> Best regards, Ilya Maximets.
>
>
Thanks,
Ales.
Ilya Maximets Dec. 6, 2022, 10 a.m. UTC | #4
On 12/6/22 07:29, Ales Musil wrote:
> 
> 
> On Mon, Dec 5, 2022 at 10:38 PM Ilya Maximets <i.maximets@ovn.org <mailto:i.maximets@ovn.org>> wrote:
> 
>     On 12/1/22 14:19, Ales Musil wrote:
>     >     +
>     >     +static enum ofperr
>     >     +handle_nxt_ct_flush(struct ofconn *ofconn, const struct ofp_header *oh)
>     >     +{
>     >     +    struct ofproto *ofproto = ofconn_get_ofproto(ofconn);
>     >     +    struct ofputil_ct_match match = {0};
>     >     +    uint16_t zone_id;
>     >     +
>     >     +    ofputil_ct_match_decode(&match, &zone_id, oh);
>     >     +
>     >     +    if (ofproto->ofproto_class->ct_flush) {
>     >     +        ofproto->ofproto_class->ct_flush(ofproto, &zone_id, &match);
>     >
>     >
>     > I've realized that v1 doesn't allow zone_id being NULL. However I'm not sure how to put that information into the extension struct.
>     > I'm open to any suggestion, I was thinking about flags field, which would grow the whole struct by 4 bytes.
> 
> 
>     IIUC, you're talking about OpenFlow interface that you created
>     requiring zone_id to be provided, right?
> 
> 
> Yes.
>  
> 
> 
>     Optional arguments and multiple arguments with mixed order are
>     usually solved by using TLVs.  You may shrink down the
>     'struct nx_ct_flush' structure to only mandatory elements and
>     make all the 'match' fields including zone_id as TLVs with the
>     help of include/openvswitch/ofp-prop.h.
> 
> 
> That sounds good. I'll look into it, thanks.
>  
> 
> 
>     One more thing: I don't think we should add dpctl interface for
>     the flushing.  We don't have such interface for ct-flush-zone
>     so we should not have it for ct-flush.  Instead, what is missing
>     in your implementation, is the native OpenFlow interface via
>     ovs-ofctl, i.e. 'ovs-ofctl ct-flush' command.  And all the tests
>     should use it instead.  This way we will also have test coverage
>     for the code that will actually be used by OVN/CMS.
> 
> 
> The problem is that the dpctl interface actually already exists, and we shouldn't
> just remove it, this wouldn't make sense IMO. The first patch just extends that interface.

Hmm, OK.

> Adding the ovs-ofctl makes, I'll work on it for v2.

Thanks.  And we'll need tests for ovs-ofctl part, of course.

>  
> 
> 
>     Best regards, Ilya Maximets.
> 
> 
> Thanks,
> Ales.
> 
> -- 
> 
> Ales Musil
> 
> Senior Software Engineer - OVN Core
> 
> Red Hat EMEA <https://www.redhat.com>
> 
> amusil@redhat.com <mailto:amusil@redhat.com>    IM: amusil
> 
> <https://red.ht/sig>
>
diff mbox series

Patch

diff --git a/NEWS b/NEWS
index 81909812e..20ffd0a2a 100644
--- a/NEWS
+++ b/NEWS
@@ -25,6 +25,9 @@  Post-v3.0.0
        DPDK 21.11.2.
    - ovs-dpctl and related ovs-appctl commands:
      * "flush-conntrack" is capable of handling partial 5-tuple.
+   - OpenFlow:
+      * New OpenFlow extension NXT_CT_FLUSH to flush connections matching
+        the specified fields.
 
 
 v3.0.0 - 15 Aug 2022
diff --git a/include/openflow/nicira-ext.h b/include/openflow/nicira-ext.h
index b68804991..90013bc36 100644
--- a/include/openflow/nicira-ext.h
+++ b/include/openflow/nicira-ext.h
@@ -1064,4 +1064,30 @@  struct nx_zone_id {
 };
 OFP_ASSERT(sizeof(struct nx_zone_id) == 8);
 
+/* NXT_CT_FLUSH.
+ *
+ * Flushes the connection tracking specified by 5-tuple. */
+struct nx_ct_flush {
+    uint8_t ip_proto;          /* IP protocol. */
+    uint8_t family;            /* L3 address family. */
+    ovs_be16 zone_id;          /* CT zone id. */
+
+    /* The orig direction section. */
+    ovs_be32 orig_src[4];      /* CT source IPv6 or mapped IPv4 address. */
+    ovs_be32 orig_dst[4];      /* CT destination IPv6 or mapped IPv4
+                                * address. */
+    ovs_be16 orig_src_port;    /* CT source port or ICMP id. */
+    ovs_be16 orig_dst_port;    /* CT destination port or ICMP type and ICMP
+                                * code. */
+
+    /* The reply direction section. */
+    ovs_be32 reply_src[4];      /* CT source IPv6 or mapped IPv4 address. */
+    ovs_be32 reply_dst[4];      /* CT destination IPv6 or mapped IPv4
+                                * address. */
+    ovs_be16 reply_src_port;    /* CT source port or ICMP id. */
+    ovs_be16 reply_dst_port;    /* CT destination port or ICMP type and ICMP
+                                * code. */
+};
+OFP_ASSERT(sizeof(struct nx_ct_flush) == 76);
+
 #endif /* openflow/nicira-ext.h */
diff --git a/include/openvswitch/ofp-msgs.h b/include/openvswitch/ofp-msgs.h
index 921a937e5..80f12481c 100644
--- a/include/openvswitch/ofp-msgs.h
+++ b/include/openvswitch/ofp-msgs.h
@@ -526,6 +526,9 @@  enum ofpraw {
 
     /* NXST 1.0+ (4): struct nx_ipfix_stats_reply[]. */
     OFPRAW_NXST_IPFIX_FLOW_REPLY,
+
+    /* NXT 1.0+ (32): struct nx_ct_flush. */
+    OFPRAW_NXT_CT_FLUSH,
 };
 
 /* Decoding messages into OFPRAW_* values. */
@@ -772,6 +775,7 @@  enum ofptype {
     OFPTYPE_IPFIX_FLOW_STATS_REQUEST, /* OFPRAW_NXST_IPFIX_FLOW_REQUEST */
     OFPTYPE_IPFIX_FLOW_STATS_REPLY,   /* OFPRAW_NXST_IPFIX_FLOW_REPLY */
     OFPTYPE_CT_FLUSH_ZONE,            /* OFPRAW_NXT_CT_FLUSH_ZONE. */
+    OFPTYPE_CT_FLUSH,           /* OFPRAW_NXT_CT_FLUSH. */
 
     /* Flow monitor extension. */
     OFPTYPE_FLOW_MONITOR_CANCEL,  /* OFPRAW_NXT_FLOW_MONITOR_CANCEL.
diff --git a/include/openvswitch/ofp-util.h b/include/openvswitch/ofp-util.h
index 84937ae26..2e533fa4f 100644
--- a/include/openvswitch/ofp-util.h
+++ b/include/openvswitch/ofp-util.h
@@ -65,6 +65,10 @@  struct ofpbuf *ofputil_encode_echo_reply(const struct ofp_header *);
 
 struct ofpbuf *ofputil_encode_barrier_request(enum ofp_version);
 
+struct ofpbuf *ofputil_ct_match_encode(const struct ofputil_ct_match *,
+                                       uint16_t zone_id,
+                                       enum ofp_version version);
+
 #ifdef __cplusplus
 }
 #endif
diff --git a/lib/ofp-bundle.c b/lib/ofp-bundle.c
index 0161c2bc6..941a8370e 100644
--- a/lib/ofp-bundle.c
+++ b/lib/ofp-bundle.c
@@ -292,6 +292,7 @@  ofputil_is_bundlable(enum ofptype type)
     case OFPTYPE_IPFIX_FLOW_STATS_REQUEST:
     case OFPTYPE_IPFIX_FLOW_STATS_REPLY:
     case OFPTYPE_CT_FLUSH_ZONE:
+    case OFPTYPE_CT_FLUSH:
         break;
     }
 
diff --git a/lib/ofp-ct-util.c b/lib/ofp-ct-util.c
index 9112305cc..2e7f7ffc1 100644
--- a/lib/ofp-ct-util.c
+++ b/lib/ofp-ct-util.c
@@ -23,7 +23,9 @@ 
 
 #include "ct-dpif.h"
 #include "ofp-ct-util.h"
+#include "openflow/nicira-ext.h"
 #include "openvswitch/dynamic-string.h"
+#include "openvswitch/ofp-msgs.h"
 #include "openvswitch/ofp-parse.h"
 #include "openvswitch/ofp-util.h"
 #include "openvswitch/packets.h"
@@ -309,3 +311,41 @@  error:
     free(copy);
     return false;
 }
+
+void
+ofputil_ct_match_decode(struct ofputil_ct_match *match, uint16_t *zone_id,
+                        const struct ofp_header *oh)
+{
+    const struct nx_ct_flush *nx_flush = ofpmsg_body(oh);
+
+    struct ofputil_ct_tuple *orig = &match->tuple_orig;
+    struct ofputil_ct_tuple *reply = &match->tuple_reply;
+
+    *zone_id = ntohs(nx_flush->zone_id);
+
+    match->l3_type = nx_flush->family;
+    match->ip_proto = nx_flush->ip_proto;
+
+    memcpy(&orig->src, &nx_flush->orig_src, sizeof orig->src);
+    memcpy(&orig->dst, &nx_flush->orig_dst, sizeof orig->dst);
+
+    memcpy(&reply->src, &nx_flush->reply_src, sizeof reply->src);
+    memcpy(&reply->dst, &nx_flush->reply_dst, sizeof reply->dst);
+
+    orig->src_port = nx_flush->orig_src_port;
+    reply->src_port = nx_flush->reply_src_port;
+
+    if (match->ip_proto == IPPROTO_ICMP || match->ip_proto == IPPROTO_ICMPV6) {
+        uint16_t icmp = ntohs(nx_flush->orig_dst_port);
+        orig->icmp_type = icmp >> 8 & 0xff;
+        orig->icmp_code = icmp & 0xff;
+
+        icmp = ntohs(nx_flush->reply_dst_port);
+        reply->icmp_type = icmp >> 8 & 0xff;
+        reply->icmp_code = icmp & 0xff;
+    } else {
+        orig->dst_port = nx_flush->orig_dst_port;
+        reply->dst_port = nx_flush->reply_dst_port;
+    }
+}
+
diff --git a/lib/ofp-ct-util.h b/lib/ofp-ct-util.h
index 6e8f0f68a..4c6e61e2d 100644
--- a/lib/ofp-ct-util.h
+++ b/lib/ofp-ct-util.h
@@ -31,4 +31,7 @@  void ofputil_ct_match_format(struct ds *ds,
 bool ofputil_ct_match_parse(struct ofputil_ct_match *match, const char *s,
                             struct ds *ds);
 
+void ofputil_ct_match_decode(struct ofputil_ct_match *match, uint16_t *zone_id,
+                             const struct ofp_header *oh);
+
 #endif // lib/ofp-ct-util.h
diff --git a/lib/ofp-print.c b/lib/ofp-print.c
index bd37fa17a..fd4e982b6 100644
--- a/lib/ofp-print.c
+++ b/lib/ofp-print.c
@@ -36,6 +36,7 @@ 
 #include "learn.h"
 #include "multipath.h"
 #include "netdev.h"
+#include "ofp-ct-util.h"
 #include "nx-match.h"
 #include "odp-util.h"
 #include "openflow/nicira-ext.h"
@@ -949,6 +950,19 @@  ofp_print_nxt_ct_flush_zone(struct ds *string, const struct nx_zone_id *nzi)
     return 0;
 }
 
+static enum ofperr
+ofp_print_nxt_ct_flush(struct ds *string, const struct ofp_header *oh)
+{
+    uint16_t zone_id;
+    struct ofputil_ct_match match = {0};
+
+    ofputil_ct_match_decode(&match, &zone_id, oh);
+    ofputil_ct_match_format(string, &match);
+    ds_put_format(string, ",zone_id=%"PRIu16, zone_id);
+
+    return 0;
+}
+
 static enum ofperr
 ofp_to_string__(const struct ofp_header *oh,
                 const struct ofputil_port_map *port_map,
@@ -1184,6 +1198,8 @@  ofp_to_string__(const struct ofp_header *oh,
 
     case OFPTYPE_CT_FLUSH_ZONE:
         return ofp_print_nxt_ct_flush_zone(string, ofpmsg_body(oh));
+    case OFPTYPE_CT_FLUSH:
+        return ofp_print_nxt_ct_flush(string, oh);
     }
 
     return 0;
diff --git a/lib/ofp-util.c b/lib/ofp-util.c
index a324ceeea..51c42357f 100644
--- a/lib/ofp-util.c
+++ b/lib/ofp-util.c
@@ -237,3 +237,39 @@  ofputil_encode_barrier_request(enum ofp_version ofp_version)
 
     return ofpraw_alloc(type, ofp_version, 0);
 }
+
+struct ofpbuf *
+ofputil_ct_match_encode(const struct ofputil_ct_match *match, uint16_t zone_id,
+                        enum ofp_version version)
+{
+    struct ofpbuf *msg = ofpraw_alloc(OFPRAW_NXT_CT_FLUSH, version, 0);
+    struct nx_ct_flush *nx_flush = ofpbuf_put_zeros(msg, sizeof *nx_flush);
+    const struct ofputil_ct_tuple *orig = &match->tuple_orig;
+    const struct ofputil_ct_tuple *reply = &match->tuple_reply;
+
+    nx_flush->ip_proto = match->ip_proto;
+    nx_flush->family = match->l3_type;
+    nx_flush->zone_id = htons(zone_id);
+
+    memcpy(&nx_flush->orig_src, &orig->src, sizeof nx_flush->orig_src);
+    memcpy(&nx_flush->orig_dst, &orig->dst, sizeof nx_flush->orig_dst);
+    memcpy(&nx_flush->reply_src, &reply->src, sizeof nx_flush->reply_src);
+    memcpy(&nx_flush->reply_dst, &reply->dst, sizeof nx_flush->reply_dst);
+
+    nx_flush->orig_src_port = orig->src_port;
+    nx_flush->reply_src_port = reply->src_port;
+
+    if (nx_flush->ip_proto == IPPROTO_ICMP
+        || nx_flush->ip_proto == IPPROTO_ICMPV6) {
+
+        nx_flush->orig_dst_port =
+            htons(orig->icmp_type << 8 | orig->icmp_code);
+        nx_flush->reply_dst_port =
+            htons(reply->icmp_type << 8 | reply->icmp_code);
+    } else {
+        nx_flush->orig_dst_port = orig->dst_port;
+        nx_flush->reply_dst_port = reply->dst_port;
+    }
+
+    return msg;
+}
diff --git a/lib/rconn.c b/lib/rconn.c
index a96b2eb8b..4afa21515 100644
--- a/lib/rconn.c
+++ b/lib/rconn.c
@@ -1426,6 +1426,7 @@  is_admitted_msg(const struct ofpbuf *b)
     case OFPTYPE_IPFIX_FLOW_STATS_REQUEST:
     case OFPTYPE_IPFIX_FLOW_STATS_REPLY:
     case OFPTYPE_CT_FLUSH_ZONE:
+    case OFPTYPE_CT_FLUSH:
     default:
         return true;
     }
diff --git a/ofproto/ofproto-dpif.c b/ofproto/ofproto-dpif.c
index f9562dee8..29174a585 100644
--- a/ofproto/ofproto-dpif.c
+++ b/ofproto/ofproto-dpif.c
@@ -5358,11 +5358,12 @@  type_set_config(const char *type, const struct smap *other_config)
 }
 
 static void
-ct_flush(const struct ofproto *ofproto_, const uint16_t *zone)
+ct_flush(const struct ofproto *ofproto_, const uint16_t *zone,
+         const struct ofputil_ct_match *match)
 {
     struct ofproto_dpif *ofproto = ofproto_dpif_cast(ofproto_);
 
-    ct_dpif_flush(ofproto->backer->dpif, zone, NULL);
+    ct_dpif_flush(ofproto->backer->dpif, zone, match);
 }
 
 static struct ct_timeout_policy *
@@ -5674,6 +5675,9 @@  get_datapath_cap(const char *datapath_type, struct smap *cap)
     smap_add(cap, "lb_output_action", s.lb_output_action ? "true" : "false");
     smap_add(cap, "ct_zero_snat", s.ct_zero_snat ? "true" : "false");
     smap_add(cap, "add_mpls", s.add_mpls ? "true" : "false");
+    /* The ct_tuple_flush is implemented on dpif level, so it is supported
+     * for all backers. */
+    smap_add(cap, "ct_flush", "true");
 }
 
 /* Gets timeout policy name in 'backer' based on 'zone', 'dl_type' and
diff --git a/ofproto/ofproto-provider.h b/ofproto/ofproto-provider.h
index 7e3fb6698..5e39234f9 100644
--- a/ofproto/ofproto-provider.h
+++ b/ofproto/ofproto-provider.h
@@ -49,6 +49,7 @@ 
 #include "openvswitch/ofp-port.h"
 #include "openvswitch/ofp-switch.h"
 #include "openvswitch/ofp-table.h"
+#include "openvswitch/ofp-util.h"
 #include "ovs-atomic.h"
 #include "ovs-rcu.h"
 #include "ovs-thread.h"
@@ -1902,8 +1903,10 @@  struct ofproto_class {
 /* ## Connection tracking ## */
 /* ## ------------------- ## */
     /* Flushes the connection tracking tables. If 'zone' is not NULL,
-     * only deletes connections in '*zone'. */
-    void (*ct_flush)(const struct ofproto *, const uint16_t *zone);
+     * only deletes connections in '*zone'. If 'match' is not NULL,
+     * deletes connections specified by the match. */
+    void (*ct_flush)(const struct ofproto *, const uint16_t *zone,
+                     const struct ofputil_ct_match *match);
 
     /* Sets conntrack timeout policy specified by 'timeout_policy' to 'zone'
      * in datapath type 'dp_type'. */
diff --git a/ofproto/ofproto.c b/ofproto/ofproto.c
index 3a527683c..1aee6b327 100644
--- a/ofproto/ofproto.c
+++ b/ofproto/ofproto.c
@@ -34,6 +34,7 @@ 
 #include "openvswitch/hmap.h"
 #include "netdev.h"
 #include "nx-match.h"
+#include "ofp-ct-util.h"
 #include "ofproto.h"
 #include "ofproto-provider.h"
 #include "openflow/nicira-ext.h"
@@ -934,7 +935,25 @@  handle_nxt_ct_flush_zone(struct ofconn *ofconn, const struct ofp_header *oh)
 
     uint16_t zone = ntohs(nzi->zone_id);
     if (ofproto->ofproto_class->ct_flush) {
-        ofproto->ofproto_class->ct_flush(ofproto, &zone);
+        ofproto->ofproto_class->ct_flush(ofproto, &zone, NULL);
+    } else {
+        return EOPNOTSUPP;
+    }
+
+    return 0;
+}
+
+static enum ofperr
+handle_nxt_ct_flush(struct ofconn *ofconn, const struct ofp_header *oh)
+{
+    struct ofproto *ofproto = ofconn_get_ofproto(ofconn);
+    struct ofputil_ct_match match = {0};
+    uint16_t zone_id;
+
+    ofputil_ct_match_decode(&match, &zone_id, oh);
+
+    if (ofproto->ofproto_class->ct_flush) {
+        ofproto->ofproto_class->ct_flush(ofproto, &zone_id, &match);
     } else {
         return EOPNOTSUPP;
     }
@@ -8787,6 +8806,9 @@  handle_single_part_openflow(struct ofconn *ofconn, const struct ofp_header *oh,
     case OFPTYPE_CT_FLUSH_ZONE:
         return handle_nxt_ct_flush_zone(ofconn, oh);
 
+    case OFPTYPE_CT_FLUSH:
+        return handle_nxt_ct_flush(ofconn, oh);
+
     case OFPTYPE_HELLO:
     case OFPTYPE_ERROR:
     case OFPTYPE_FEATURES_REPLY:
diff --git a/tests/ofp-print.at b/tests/ofp-print.at
index fe41cc42c..418e98559 100644
--- a/tests/ofp-print.at
+++ b/tests/ofp-print.at
@@ -4073,3 +4073,74 @@  AT_CHECK([ovs-ofctl ofp-print "\
 NXT_CT_FLUSH_ZONE (xid=0x3): zone_id=13
 ])
 AT_CLEANUP
+
+AT_SETUP([NXT_CT_FLUSH])
+AT_KEYWORDS([ofp-print])
+AT_CHECK([ovs-ofctl ofp-print "\
+01 04 00 5c 00 00 00 03 00 00 23 20 00 00 00 20 \
+06 \
+02 \
+00 0d \
+00 00 00 00 00 00 00 00 00 00 ff ff 0a 0a 00 01 \
+00 00 00 00 00 00 00 00 00 00 ff ff 0a 0a 00 02 \
+00 50 \
+1f 90 \
+00 00 00 00 00 00 00 00 00 00 ff ff 0a 0a 00 02 \
+00 00 00 00 00 00 00 00 00 00 ff ff 0a 0a 00 01 \
+1f 90 \
+00 50 \
+"], [0], [dnl
+NXT_CT_FLUSH (xid=0x3): l3_type=2,ip_proto=6,orig=(src=10.10.0.1,dst=10.10.0.2,src_port=80,dst_port=8080),reply=(src=10.10.0.2,dst=10.10.0.1,src_port=8080,dst_port=80),zone_id=13
+])
+
+AT_CHECK([ovs-ofctl ofp-print "\
+01 04 00 5c 00 00 00 03 00 00 23 20 00 00 00 20 \
+06 \
+0a \
+00 0d \
+fd 18 00 00 00 00 00 00 00 00 ff ff ab cd 00 01 \
+fd 18 00 00 00 00 00 00 00 00 ff ff ab cd 00 02 \
+00 50 \
+1f 90 \
+fd 18 00 00 00 00 00 00 00 00 ff ff ab cd 00 02 \
+fd 18 00 00 00 00 00 00 00 00 ff ff ab cd 00 01 \
+1f 90 \
+00 50 \
+"], [0], [dnl
+NXT_CT_FLUSH (xid=0x3): l3_type=10,ip_proto=6,orig=(src=fd18::ffff:abcd:1,dst=fd18::ffff:abcd:2,src_port=80,dst_port=8080),reply=(src=fd18::ffff:abcd:2,dst=fd18::ffff:abcd:1,src_port=8080,dst_port=80),zone_id=13
+])
+
+AT_CHECK([ovs-ofctl ofp-print "\
+01 04 00 5c 00 00 00 03 00 00 23 20 00 00 00 20 \
+01 \
+02 \
+00 0d \
+00 00 00 00 00 00 00 00 00 00 ff ff 0a 0a 00 01 \
+00 00 00 00 00 00 00 00 00 00 ff ff 0a 0a 00 02 \
+00 01 \
+08 00 \
+00 00 00 00 00 00 00 00 00 00 ff ff 0a 0a 00 02 \
+00 00 00 00 00 00 00 00 00 00 ff ff 0a 0a 00 01 \
+00 01 \
+00 00 \
+"], [0], [dnl
+NXT_CT_FLUSH (xid=0x3): l3_type=2,ip_proto=1,orig=(src=10.10.0.1,dst=10.10.0.2,icmp_id=1,icmp_type=8,icmp_code=0),reply=(src=10.10.0.2,dst=10.10.0.1,icmp_id=1,icmp_type=0,icmp_code=0),zone_id=13
+])
+
+AT_CHECK([ovs-ofctl ofp-print "\
+01 04 00 5c 00 00 00 03 00 00 23 20 00 00 00 20 \
+01 \
+0a \
+00 0d \
+fd 18 00 00 00 00 00 00 00 00 ff ff ab cd 00 01 \
+fd 18 00 00 00 00 00 00 00 00 ff ff ab cd 00 02 \
+00 01 \
+08 00 \
+fd 18 00 00 00 00 00 00 00 00 ff ff ab cd 00 02 \
+fd 18 00 00 00 00 00 00 00 00 ff ff ab cd 00 01 \
+00 01 \
+00 00 \
+"], [0], [dnl
+NXT_CT_FLUSH (xid=0x3): l3_type=10,ip_proto=1,orig=(src=fd18::ffff:abcd:1,dst=fd18::ffff:abcd:2,icmp_id=1,icmp_type=8,icmp_code=0),reply=(src=fd18::ffff:abcd:2,dst=fd18::ffff:abcd:1,icmp_id=1,icmp_type=0,icmp_code=0),zone_id=13
+])
+AT_CLEANUP