diff mbox series

[ovs-dev,v2,4/5] conntrack: Memory savings.

Message ID 1543422714-100901-5-git-send-email-dlu998@gmail.com
State Superseded
Headers show
Series conntrack: Optimize and refactor. | expand

Commit Message

Darrell Ball Nov. 28, 2018, 4:31 p.m. UTC
Allocate memory for nat and algs as needed.

Signed-off-by: Darrell Ball <dlu998@gmail.com>
---
 lib/conntrack-private.h |  29 +++++---
 lib/conntrack.c         | 189 ++++++++++++++++++++++++++++--------------------
 2 files changed, 127 insertions(+), 91 deletions(-)

Comments

Aaron Conole Nov. 28, 2018, 10:02 p.m. UTC | #1
Darrell Ball <dlu998@gmail.com> writes:

> Allocate memory for nat and algs as needed.
>
> Signed-off-by: Darrell Ball <dlu998@gmail.com>
> ---
>  lib/conntrack-private.h |  29 +++++---
>  lib/conntrack.c         | 189 ++++++++++++++++++++++++++++--------------------
>  2 files changed, 127 insertions(+), 91 deletions(-)
>
> diff --git a/lib/conntrack-private.h b/lib/conntrack-private.h
> index ac891cc..20d2d78 100644
> --- a/lib/conntrack-private.h
> +++ b/lib/conntrack-private.h
> @@ -81,19 +81,31 @@ struct alg_exp_node {
>      bool nat_rpl_dst;
>  };
>  
> +struct conn_ext_nat {
> +    struct nat_action_info_t *nat_info;
> +    struct conn *nat_conn;
> +};
> +
> +struct conn_ext_alg {
> +    char *alg;
> +    /* TCP sequence skew direction due to NATTing of FTP control messages;
> +     * true if reply direction. */
> +    int seq_skew;
> +    struct conn_key master_key;
> +    /* True if alg data connection. */
> +    bool alg_related;
> +    bool seq_skew_dir;
> +};
> +
>  struct conn {
>      struct conn_key key;
>      struct conn_key rev_key;
> -    /* Only used for orig_tuple support. */
> -    struct conn_key master_key;
>      long long expiration;
>      struct ovs_list exp_node;
>      struct cmap_node cm_node;
>      ovs_u128 label;
> -    struct nat_action_info_t *nat_info;
> -    char *alg;
> -    struct conn *nat_conn;
> -    int seq_skew;
> +    struct conn_ext_nat *ext_nat;
> +    struct conn_ext_alg *ext_alg;
>      uint32_t mark;
>      /* See ct_conn_type. */
>      uint8_t conn_type;
> @@ -101,11 +113,6 @@ struct conn {
>       * This field is used to signal an update to the specified list.  The
>       * value 'NO_UPD_EXP_LIST' is used to indicate no update to any list. */
>      uint8_t exp_list_id;
> -    /* TCP sequence skew direction due to NATTing of FTP control messages;
> -     * true if reply direction. */
> -    bool seq_skew_dir;
> -    /* True if alg data connection. */
> -    bool alg_related;
>      /* Inserted into the cmap; handle theoretical expiry list race; although
>       * such a race would probably mean a system meltdown. */
>      bool inserted;
> diff --git a/lib/conntrack.c b/lib/conntrack.c
> index c47a0b0..9d10b14 100644
> --- a/lib/conntrack.c
> +++ b/lib/conntrack.c
> @@ -399,16 +399,16 @@ conn_clean(struct conn *conn)
>  {
>      ovs_assert(conn->conn_type == CT_CONN_TYPE_DEFAULT);
>  
> -    if (conn->alg) {
> +    if (conn->ext_alg && conn->ext_alg->alg) {
>          expectation_clean(&conn->key);
>      }
>  
>      uint32_t hash = conn_key_hash(&conn->key, hash_basis);
>      cmap_remove(&cm_conns, &conn->cm_node, hash);
>      ovs_list_remove(&conn->exp_node);
> -    if (conn->nat_conn) {
> -        hash = conn_key_hash(&conn->nat_conn->key, hash_basis);
> -        cmap_remove(&cm_conns, &conn->nat_conn->cm_node, hash);
> +    if (conn->ext_nat && conn->ext_nat->nat_conn) {
> +        hash = conn_key_hash(&conn->ext_nat->nat_conn->key, hash_basis);
> +        cmap_remove(&cm_conns, &conn->ext_nat->nat_conn->cm_node, hash);
>      }
>      ovsrcu_postpone(delete_conn, conn);
>      atomic_count_dec(&n_conn);
> @@ -418,7 +418,7 @@ static void
>  conn_clean_one(struct conn *conn)
>      OVS_NO_THREAD_SAFETY_ANALYSIS
>  {
> -    if (conn->alg) {
> +    if (conn->ext_alg && conn->ext_alg->alg) {
>          expectation_clean(&conn->key);
>      }
>  
> @@ -473,8 +473,8 @@ write_ct_md(struct dp_packet *pkt, uint16_t zone, const struct conn *conn,
>  
>      /* Use the original direction tuple if we have it. */
>      if (conn) {
> -        if (conn->alg_related) {
> -            key = &conn->master_key;
> +        if (conn->ext_alg && conn->ext_alg->alg_related) {
> +            key = &conn->ext_alg->master_key;
>          } else {
>              key = &conn->key;
>          }
> @@ -615,7 +615,8 @@ handle_alg_ctl(const struct conn_lookup_ctx *ctx, struct dp_packet *pkt,
>                 long long now, bool nat)
>  {
>      /* ALG control packet handling with expectation creation. */
> -    if (OVS_UNLIKELY(alg_helpers[ct_alg_ctl] && conn && conn->alg)) {
> +    if (OVS_UNLIKELY(alg_helpers[ct_alg_ctl] && conn && conn->ext_alg &&
> +                     conn->ext_alg->alg)) {
>          conn_entry_lock(conn);
>          alg_helpers[ct_alg_ctl](ctx, pkt, conn, now, CT_FTP_CTL_INTEREST,
>                                  nat);
> @@ -626,7 +627,7 @@ handle_alg_ctl(const struct conn_lookup_ctx *ctx, struct dp_packet *pkt,
>  static void
>  pat_packet(struct dp_packet *pkt, const struct conn *conn)
>  {
> -    if (conn->nat_info->nat_action & NAT_ACTION_SRC) {
> +    if (conn->ext_nat->nat_info->nat_action & NAT_ACTION_SRC) {
>          if (conn->key.nw_proto == IPPROTO_TCP) {
>              struct tcp_header *th = dp_packet_l4(pkt);
>              packet_set_tcp_port(pkt, conn->rev_key.dst.port, th->tcp_dst);
> @@ -634,7 +635,7 @@ pat_packet(struct dp_packet *pkt, const struct conn *conn)
>              struct udp_header *uh = dp_packet_l4(pkt);
>              packet_set_udp_port(pkt, conn->rev_key.dst.port, uh->udp_dst);
>          }
> -    } else if (conn->nat_info->nat_action & NAT_ACTION_DST) {
> +    } else if (conn->ext_nat->nat_info->nat_action & NAT_ACTION_DST) {
>          if (conn->key.nw_proto == IPPROTO_TCP) {
>              struct tcp_header *th = dp_packet_l4(pkt);
>              packet_set_tcp_port(pkt, th->tcp_src, conn->rev_key.src.port);
> @@ -648,7 +649,7 @@ pat_packet(struct dp_packet *pkt, const struct conn *conn)
>  static void
>  nat_packet(struct dp_packet *pkt, const struct conn *conn, bool related)
>  {
> -    if (conn->nat_info->nat_action & NAT_ACTION_SRC) {
> +    if (conn->ext_nat->nat_info->nat_action & NAT_ACTION_SRC) {
>          pkt->md.ct_state |= CS_SRC_NAT;
>          if (conn->key.dl_type == htons(ETH_TYPE_IP)) {
>              struct ip_header *nh = dp_packet_l3(pkt);
> @@ -664,7 +665,7 @@ nat_packet(struct dp_packet *pkt, const struct conn *conn, bool related)
>          if (!related) {
>              pat_packet(pkt, conn);
>          }
> -    } else if (conn->nat_info->nat_action & NAT_ACTION_DST) {
> +    } else if (conn->ext_nat->nat_info->nat_action & NAT_ACTION_DST) {
>          pkt->md.ct_state |= CS_DST_NAT;
>          if (conn->key.dl_type == htons(ETH_TYPE_IP)) {
>              struct ip_header *nh = dp_packet_l3(pkt);
> @@ -686,7 +687,7 @@ nat_packet(struct dp_packet *pkt, const struct conn *conn, bool related)
>  static void
>  un_pat_packet(struct dp_packet *pkt, const struct conn *conn)
>  {
> -    if (conn->nat_info->nat_action & NAT_ACTION_SRC) {
> +    if (conn->ext_nat->nat_info->nat_action & NAT_ACTION_SRC) {
>          if (conn->key.nw_proto == IPPROTO_TCP) {
>              struct tcp_header *th = dp_packet_l4(pkt);
>              packet_set_tcp_port(pkt, th->tcp_src, conn->key.src.port);
> @@ -694,7 +695,7 @@ un_pat_packet(struct dp_packet *pkt, const struct conn *conn)
>              struct udp_header *uh = dp_packet_l4(pkt);
>              packet_set_udp_port(pkt, uh->udp_src, conn->key.src.port);
>          }
> -    } else if (conn->nat_info->nat_action & NAT_ACTION_DST) {
> +    } else if (conn->ext_nat->nat_info->nat_action & NAT_ACTION_DST) {
>          if (conn->key.nw_proto == IPPROTO_TCP) {
>              struct tcp_header *th = dp_packet_l4(pkt);
>              packet_set_tcp_port(pkt, conn->key.dst.port, th->tcp_dst);
> @@ -708,7 +709,7 @@ un_pat_packet(struct dp_packet *pkt, const struct conn *conn)
>  static void
>  reverse_pat_packet(struct dp_packet *pkt, const struct conn *conn)
>  {
> -    if (conn->nat_info->nat_action & NAT_ACTION_SRC) {
> +    if (conn->ext_nat->nat_info->nat_action & NAT_ACTION_SRC) {
>          if (conn->key.nw_proto == IPPROTO_TCP) {
>              struct tcp_header *th_in = dp_packet_l4(pkt);
>              packet_set_tcp_port(pkt, conn->key.src.port,
> @@ -718,7 +719,7 @@ reverse_pat_packet(struct dp_packet *pkt, const struct conn *conn)
>              packet_set_udp_port(pkt, conn->key.src.port,
>                                  uh_in->udp_dst);
>          }
> -    } else if (conn->nat_info->nat_action & NAT_ACTION_DST) {
> +    } else if (conn->ext_nat->nat_info->nat_action & NAT_ACTION_DST) {
>          if (conn->key.nw_proto == IPPROTO_TCP) {
>              struct tcp_header *th_in = dp_packet_l4(pkt);
>              packet_set_tcp_port(pkt, th_in->tcp_src,
> @@ -750,10 +751,10 @@ reverse_nat_packet(struct dp_packet *pkt, const struct conn *conn)
>          pkt->l3_ofs += (char *) inner_l3 - (char *) nh;
>          pkt->l4_ofs += inner_l4 - (char *) icmp;
>  
> -        if (conn->nat_info->nat_action & NAT_ACTION_SRC) {
> +        if (conn->ext_nat->nat_info->nat_action & NAT_ACTION_SRC) {
>              packet_set_ipv4_addr(pkt, &inner_l3->ip_src,
>                                   conn->key.src.addr.ipv4_aligned);
> -        } else if (conn->nat_info->nat_action & NAT_ACTION_DST) {
> +        } else if (conn->ext_nat->nat_info->nat_action & NAT_ACTION_DST) {
>              packet_set_ipv4_addr(pkt, &inner_l3->ip_dst,
>                                   conn->key.dst.addr.ipv4_aligned);
>          }
> @@ -772,12 +773,12 @@ reverse_nat_packet(struct dp_packet *pkt, const struct conn *conn)
>          pkt->l3_ofs += (char *) inner_l3_6 - (char *) nh6;
>          pkt->l4_ofs += inner_l4 - (char *) icmp6;
>  
> -        if (conn->nat_info->nat_action & NAT_ACTION_SRC) {
> +        if (conn->ext_nat->nat_info->nat_action & NAT_ACTION_SRC) {
>              packet_set_ipv6_addr(pkt, conn->key.nw_proto,
>                                   inner_l3_6->ip6_src.be32,
>                                   &conn->key.src.addr.ipv6_aligned,
>                                   true);
> -        } else if (conn->nat_info->nat_action & NAT_ACTION_DST) {
> +        } else if (conn->ext_nat->nat_info->nat_action & NAT_ACTION_DST) {
>              packet_set_ipv6_addr(pkt, conn->key.nw_proto,
>                                   inner_l3_6->ip6_dst.be32,
>                                   &conn->key.dst.addr.ipv6_aligned,
> @@ -797,7 +798,7 @@ static void
>  un_nat_packet(struct dp_packet *pkt, const struct conn *conn,
>                bool related)
>  {
> -    if (conn->nat_info->nat_action & NAT_ACTION_SRC) {
> +    if (conn->ext_nat->nat_info->nat_action & NAT_ACTION_SRC) {
>          pkt->md.ct_state |= CS_DST_NAT;
>          if (conn->key.dl_type == htons(ETH_TYPE_IP)) {
>              struct ip_header *nh = dp_packet_l3(pkt);
> @@ -815,7 +816,7 @@ un_nat_packet(struct dp_packet *pkt, const struct conn *conn,
>          } else {
>              un_pat_packet(pkt, conn);
>          }
> -    } else if (conn->nat_info->nat_action & NAT_ACTION_DST) {
> +    } else if (conn->ext_nat->nat_info->nat_action & NAT_ACTION_DST) {
>          pkt->md.ct_state |= CS_SRC_NAT;
>          if (conn->key.dl_type == htons(ETH_TYPE_IP)) {
>              struct ip_header *nh = dp_packet_l3(pkt);
> @@ -846,8 +847,8 @@ conn_seq_skew_set(const struct conn_key *key, long long now, int seq_skew,
>      conn_key_lookup(key, hash, now, &conn, &reply);
>  
>      if (conn && seq_skew) {
> -        conn->seq_skew = seq_skew;
> -        conn->seq_skew_dir = seq_skew_dir;
> +        conn->ext_alg->seq_skew = seq_skew;
> +        conn->ext_alg->seq_skew_dir = seq_skew_dir;
>      }
>  }
>  
> @@ -906,29 +907,38 @@ conn_not_found(struct dp_packet *pkt, struct conn_lookup_ctx *ctx,
>          nc->rev_key = nc->key;
>          conn_key_reverse(&nc->rev_key);
>  
> -        if (ct_verify_helper(helper, ct_alg_ctl)) {
> -            nc->alg = nullable_xstrdup(helper);
> +        if (alg_exp || helper || ct_alg_ctl != CT_ALG_CTL_NONE) {
> +            nc->ext_alg = xzalloc(sizeof *nc->ext_alg);
> +        }
> +
> +        if (nat_action_info) {
> +            nc->ext_nat = xzalloc(sizeof *nc->ext_nat);
> +        }
> +
> +        if (nc->ext_alg && ct_verify_helper(helper, ct_alg_ctl)) {
> +            nc->ext_alg->alg = nullable_xstrdup(helper);
>          }
>  
>          if (alg_exp) {
> -            nc->alg_related = true;
> +            nc->ext_alg->alg_related = true;
>              nc->mark = alg_exp->master_mark;
>              nc->label = alg_exp->master_label;
> -            nc->master_key = alg_exp->master_key;
> +            nc->ext_alg->master_key = alg_exp->master_key;
>          }
>  
>          if (nat_action_info) {
> -            nc->nat_info = xmemdup(nat_action_info, sizeof *nc->nat_info);
> +            nc->ext_nat->nat_info = xmemdup(nat_action_info,
> +                                        sizeof *nc->ext_nat->nat_info);
>  
>              nat_conn = xzalloc(sizeof *nat_conn);
>  
>              if (alg_exp) {
>                  if (alg_exp->nat_rpl_dst) {
>                      nc->rev_key.dst.addr = alg_exp->alg_nat_repl_addr;
> -                    nc->nat_info->nat_action = NAT_ACTION_SRC;
> +                    nc->ext_nat->nat_info->nat_action = NAT_ACTION_SRC;
>                  } else {
>                      nc->rev_key.src.addr = alg_exp->alg_nat_repl_addr;
> -                    nc->nat_info->nat_action = NAT_ACTION_DST;
> +                    nc->ext_nat->nat_info->nat_action = NAT_ACTION_DST;
>                  }
>                  *nat_conn = *nc;
>              } else {
> @@ -942,20 +952,19 @@ conn_not_found(struct dp_packet *pkt, struct conn_lookup_ctx *ctx,
>                  /* Update nc with nat adjustments. */
>                  *nc = *nat_conn;
>              }
> +            nc->ext_nat->nat_conn = nat_conn;
>              nat_packet(pkt, nc, ctx->icmp_related);
>  
>              nat_conn->key = nc->rev_key;
>              nat_conn->rev_key = nc->key;
>              nat_conn->conn_type = CT_CONN_TYPE_UN_NAT;
> -            nat_conn->nat_info = NULL;
> -            nat_conn->alg = NULL;
> -            nat_conn->nat_conn = NULL;
> +            nat_conn->ext_nat = NULL;
> +            nat_conn->ext_alg = NULL;
>              uint32_t nat_hash = conn_key_hash(&nat_conn->key,
>                                                hash_basis);
>              cmap_insert(&cm_conns, &nat_conn->cm_node, nat_hash);
>          }
>  
> -        nc->nat_conn = nat_conn;
>          nc->conn_type = CT_CONN_TYPE_DEFAULT;
>          cmap_insert(&cm_conns, &nc->cm_node, ctx->hash);
>          nc->inserted = true;
> @@ -993,7 +1002,7 @@ conn_update_state(struct dp_packet *pkt, struct conn_lookup_ctx *ctx,
>              pkt->md.ct_state |= CS_REPLY_DIR;
>          }
>      } else {
> -        if (conn->alg_related) {
> +        if (conn->ext_alg && conn->ext_alg->alg_related) {
>              pkt->md.ct_state |= CS_RELATED;
>          }
>  
> @@ -1027,7 +1036,7 @@ static void
>  handle_nat(struct dp_packet *pkt, struct conn *conn,
>             uint16_t zone, bool reply, bool related)
>  {
> -    if (conn->nat_info &&
> +    if (conn->ext_nat && conn->ext_nat->nat_info &&
>          (!(pkt->md.ct_state & (CS_SRC_NAT | CS_DST_NAT)) ||
>            (pkt->md.ct_state & (CS_SRC_NAT | CS_DST_NAT) &&
>             zone != pkt->md.ct_zone))) {
> @@ -1112,7 +1121,7 @@ conn_update_state_alg(struct dp_packet *pkt, struct conn_lookup_ctx *ctx,
>          /* Keep sequence tracking in sync with the source of the
>           * sequence skew. */
>          conn_entry_lock(conn);
> -        if (ctx->reply != conn->seq_skew_dir) {
> +        if (ctx->reply != conn->ext_alg->seq_skew_dir) {
>              handle_ftp_ctl(ctx, pkt, conn, now, CT_FTP_CTL_OTHER,
>                             !!nat_action_info);
>              /* conn_update_state locks for unrelated fields, so unlock. */
> @@ -1275,7 +1284,7 @@ conntrack_clear(struct dp_packet *packet)
>  static void
>  set_mark(struct dp_packet *pkt, struct conn *conn, uint32_t val, uint32_t mask)
>  {
> -    if (conn->alg_related) {
> +    if (conn->ext_alg && conn->ext_alg->alg_related) {
>          pkt->md.ct_mark = conn->mark;
>      } else {
>          pkt->md.ct_mark = val | (pkt->md.ct_mark & ~(mask));
> @@ -1288,7 +1297,7 @@ set_label(struct dp_packet *pkt, struct conn *conn,
>            const struct ovs_key_ct_labels *val,
>            const struct ovs_key_ct_labels *mask)
>  {
> -    if (conn->alg_related) {
> +    if (conn->ext_alg && conn->ext_alg->alg_related) {
>          pkt->md.ct_label = conn->label;
>      } else {
>          ovs_u128 v, m;
> @@ -2012,11 +2021,11 @@ nat_range_hash(const struct conn *conn, uint32_t basis)
>  {
>      uint32_t hash = basis;
>  
> -    hash = ct_addr_hash_add(hash, &conn->nat_info->min_addr);
> -    hash = ct_addr_hash_add(hash, &conn->nat_info->max_addr);
> +    hash = ct_addr_hash_add(hash, &conn->ext_nat->nat_info->min_addr);
> +    hash = ct_addr_hash_add(hash, &conn->ext_nat->nat_info->max_addr);
>      hash = hash_add(hash,
> -                    (conn->nat_info->max_port << 16)
> -                    | conn->nat_info->min_port);
> +                    (conn->ext_nat->nat_info->max_port << 16)
> +                    | conn->ext_nat->nat_info->min_port);
>      hash = ct_endpoint_hash_add(hash, &conn->key.src);
>      hash = ct_endpoint_hash_add(hash, &conn->key.dst);
>      hash = hash_add(hash, (OVS_FORCE uint32_t) conn->key.dl_type);
> @@ -2040,22 +2049,24 @@ nat_select_range_tuple(const struct conn *conn, struct conn *nat_conn)
>      uint16_t first_port;
>      uint32_t hash = nat_range_hash(conn, hash_basis);
>  
> -    if ((conn->nat_info->nat_action & NAT_ACTION_SRC) &&
> -        (!(conn->nat_info->nat_action & NAT_ACTION_SRC_PORT))) {
> +    if ((conn->ext_nat->nat_info->nat_action & NAT_ACTION_SRC) &&
> +        (!(conn->ext_nat->nat_info->nat_action & NAT_ACTION_SRC_PORT))) {
>          min_port = ntohs(conn->key.src.port);
>          max_port = ntohs(conn->key.src.port);
>          first_port = min_port;
> -    } else if ((conn->nat_info->nat_action & NAT_ACTION_DST) &&
> -               (!(conn->nat_info->nat_action & NAT_ACTION_DST_PORT))) {
> +    } else if ((conn->ext_nat->nat_info->nat_action & NAT_ACTION_DST) &&
> +               (!(conn->ext_nat->nat_info->nat_action
> +                  & NAT_ACTION_DST_PORT))) {
>          min_port = ntohs(conn->key.dst.port);
>          max_port = ntohs(conn->key.dst.port);
>          first_port = min_port;
>      } else {
> -        uint16_t deltap = conn->nat_info->max_port - conn->nat_info->min_port;
> +        uint16_t deltap = conn->ext_nat->nat_info->max_port -
> +            conn->ext_nat->nat_info->min_port;
>          uint32_t port_index = hash % (deltap + 1);
> -        first_port = conn->nat_info->min_port + port_index;
> -        min_port = conn->nat_info->min_port;
> -        max_port = conn->nat_info->max_port;
> +        first_port = conn->ext_nat->nat_info->min_port + port_index;
> +        min_port = conn->ext_nat->nat_info->min_port;
> +        max_port = conn->ext_nat->nat_info->max_port;
>      }
>  
>      uint32_t deltaa = 0;
> @@ -2064,37 +2075,39 @@ nat_select_range_tuple(const struct conn *conn, struct conn *nat_conn)
>      memset(&ct_addr, 0, sizeof ct_addr);
>      struct ct_addr max_ct_addr;
>      memset(&max_ct_addr, 0, sizeof max_ct_addr);
> -    max_ct_addr = conn->nat_info->max_addr;
> +    max_ct_addr = conn->ext_nat->nat_info->max_addr;
>  
>      if (conn->key.dl_type == htons(ETH_TYPE_IP)) {
> -        deltaa = ntohl(conn->nat_info->max_addr.ipv4_aligned) -
> -                 ntohl(conn->nat_info->min_addr.ipv4_aligned);
> +        deltaa = ntohl(conn->ext_nat->nat_info->max_addr.ipv4_aligned) -
> +                       ntohl(conn->ext_nat->nat_info->min_addr.ipv4_aligned);
>          address_index = hash % (deltaa + 1);
>          ct_addr.ipv4_aligned = htonl(
> -            ntohl(conn->nat_info->min_addr.ipv4_aligned) + address_index);
> +            ntohl(conn->ext_nat->nat_info->min_addr.ipv4_aligned) +
> +                  address_index);
>      } else {
> -        deltaa = nat_ipv6_addrs_delta(&conn->nat_info->min_addr.ipv6_aligned,
> -                                      &conn->nat_info->max_addr.ipv6_aligned);
> +        deltaa = nat_ipv6_addrs_delta(
> +                     &conn->ext_nat->nat_info->min_addr.ipv6_aligned,
> +                     &conn->ext_nat->nat_info->max_addr.ipv6_aligned);
>          /* deltaa must be within 32 bits for full hash coverage. A 64 or
>           * 128 bit hash is unnecessary and hence not used here. Most code
>           * is kept common with V4; nat_ipv6_addrs_delta() will do the
>           * enforcement via max_ct_addr. */
> -        max_ct_addr = conn->nat_info->min_addr;
> +        max_ct_addr = conn->ext_nat->nat_info->min_addr;
>          nat_ipv6_addr_increment(&max_ct_addr.ipv6_aligned, deltaa);
>          address_index = hash % (deltaa + 1);
> -        ct_addr.ipv6_aligned = conn->nat_info->min_addr.ipv6_aligned;
> +        ct_addr.ipv6_aligned = conn->ext_nat->nat_info->min_addr.ipv6_aligned;
>          nat_ipv6_addr_increment(&ct_addr.ipv6_aligned, address_index);
>      }
>  
>      uint16_t port = first_port;
>      bool all_ports_tried = false;
>      /* For DNAT, we don't use ephemeral ports. */
> -    bool ephemeral_ports_tried = conn->nat_info->nat_action & NAT_ACTION_DST
> -                                 ? true : false;
> +    bool ephemeral_ports_tried =
> +        conn->ext_nat->nat_info->nat_action & NAT_ACTION_DST ? true : false;
>      struct ct_addr first_addr = ct_addr;
>  
>      while (true) {
> -        if (conn->nat_info->nat_action & NAT_ACTION_SRC) {
> +        if (conn->ext_nat->nat_info->nat_action & NAT_ACTION_SRC) {
>              nat_conn->rev_key.dst.addr = ct_addr;
>          } else {
>              nat_conn->rev_key.src.addr = ct_addr;
> @@ -2103,7 +2116,7 @@ nat_select_range_tuple(const struct conn *conn, struct conn *nat_conn)
>          if ((conn->key.nw_proto == IPPROTO_ICMP) ||
>              (conn->key.nw_proto == IPPROTO_ICMPV6)) {
>              all_ports_tried = true;
> -        } else if (conn->nat_info->nat_action & NAT_ACTION_SRC) {
> +        } else if (conn->ext_nat->nat_info->nat_action & NAT_ACTION_SRC) {
>              nat_conn->rev_key.dst.port = htons(port);
>          } else {
>              nat_conn->rev_key.src.port = htons(port);
> @@ -2135,14 +2148,14 @@ nat_select_range_tuple(const struct conn *conn, struct conn *nat_conn)
>                      nat_ipv6_addr_increment(&ct_addr.ipv6_aligned, 1);
>                  }
>              } else {
> -                ct_addr = conn->nat_info->min_addr;
> +                ct_addr = conn->ext_nat->nat_info->min_addr;
>              }
>              if (!memcmp(&ct_addr, &first_addr, sizeof ct_addr)) {
>                  if (ephemeral_ports_tried) {
>                      break;
>                  } else {
>                      ephemeral_ports_tried = true;
> -                    ct_addr = conn->nat_info->min_addr;
> +                    ct_addr = conn->ext_nat->nat_info->min_addr;
>                      first_addr = ct_addr;
>                      min_port = MIN_NAT_EPHEMERAL_PORT;
>                      max_port = MAX_NAT_EPHEMERAL_PORT;
> @@ -2193,16 +2206,26 @@ conn_update(struct dp_packet *pkt, struct conn *conn,
>  }
>  
>  static void
> +delete_conn_cmn(struct conn *conn)
> +{
> +    free(conn->ext_alg);
> +    free(conn->ext_nat);
> +    free(conn);
> +}
> +
> +static void
>  delete_conn(struct conn *conn)
>  {
>      if (conn->conn_type == CT_CONN_TYPE_DEFAULT) {
> -        free(conn->nat_info);
> -        free(conn->alg);
> -        conn_entry_destroy(conn);
> -        if (conn->nat_conn) {
> -            free(conn->nat_conn);
> +        if (conn->ext_nat) {
> +            free(conn->ext_nat->nat_info);
> +            free(conn->ext_nat->nat_conn);

Free doesn't require tests.  It can handle NULL.  Goes for all
instances.

Plus I commented on condensing these entries earlier.  I think that
comment still holds true, but haven't re-read the calling places to make
sure it does.

>          }
> -        free(conn);
> +        if (conn->ext_alg) {
> +            free(conn->ext_alg->alg);
> +        }
> +        conn_entry_destroy(conn);
> +        delete_conn_cmn(conn);
>      }
>  }
>  
> @@ -2210,11 +2233,15 @@ static void
>  delete_conn_one(struct conn *conn)
>  {
>      if (conn->conn_type == CT_CONN_TYPE_DEFAULT) {
> -        free(conn->nat_info);
> -        free(conn->alg);
> +        if (conn->ext_nat) {
> +            free(conn->ext_nat->nat_info);
> +        }
> +        if (conn->ext_alg) {
> +            free(conn->ext_alg->alg);
> +        }
>          conn_entry_destroy(conn);
>      }
> -    free(conn);
> +    delete_conn_cmn(conn);
>  }
>  
>  /* Convert a conntrack address 'a' into an IP address 'b' based on 'dl_type'.
> @@ -2328,9 +2355,11 @@ conn_to_ct_dpif_entry(const struct conn *conn, struct ct_dpif_entry *entry,
>  
>      entry->bkt = bkt;
>  
> -    if (conn->alg) {
> +    if (conn->ext_alg && conn->ext_alg->alg) {
>          /* Caller is responsible for freeing. */
> -        entry->helper.name = xstrdup(conn->alg);
> +        entry->helper.name = xstrdup(conn->ext_alg->alg);
> +    } else {
> +        entry->helper.name = NULL;
>      }
>  }
>  
> @@ -3016,7 +3045,7 @@ handle_ftp_ctl(const struct conn_lookup_ctx *ctx, struct dp_packet *pkt,
>          return;
>      }
>  
> -    if (!nat || !conn_for_expectation->seq_skew) {
> +    if (!nat || !conn_for_expectation->ext_alg->seq_skew) {
>          do_seq_skew_adj = false;
>      }
>  
> @@ -3024,7 +3053,7 @@ handle_ftp_ctl(const struct conn_lookup_ctx *ctx, struct dp_packet *pkt,
>      int64_t seq_skew = 0;
>  
>      if (ftp_ctl == CT_FTP_CTL_OTHER) {
> -        seq_skew = conn_for_expectation->seq_skew;
> +        seq_skew = conn_for_expectation->ext_alg->seq_skew;
>      } else if (ftp_ctl == CT_FTP_CTL_INTEREST) {
>          enum ftp_ctl_pkt rc;
>          if (ctx->key.dl_type == htons(ETH_TYPE_IPV6)) {
> @@ -3079,7 +3108,7 @@ handle_ftp_ctl(const struct conn_lookup_ctx *ctx, struct dp_packet *pkt,
>      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 (ctx->reply != conn_for_expectation->ext_alg->seq_skew_dir) {
>  
>              uint32_t tcp_ack = ntohl(get_16aligned_be32(&th->tcp_ack));
Darrell Ball Nov. 29, 2018, 4:58 a.m. UTC | #2
On Wed, Nov 28, 2018 at 2:02 PM Aaron Conole <aconole@redhat.com> wrote:

> Darrell Ball <dlu998@gmail.com> writes:
>
> > Allocate memory for nat and algs as needed.
> >
> > Signed-off-by: Darrell Ball <dlu998@gmail.com>
> > ---
> >  lib/conntrack-private.h |  29 +++++---
> >  lib/conntrack.c         | 189
> ++++++++++++++++++++++++++++--------------------
> >  2 files changed, 127 insertions(+), 91 deletions(-)
> >
> > diff --git a/lib/conntrack-private.h b/lib/conntrack-private.h
> > index ac891cc..20d2d78 100644
> > --- a/lib/conntrack-private.h
> > +++ b/lib/conntrack-private.h
> > @@ -81,19 +81,31 @@ struct alg_exp_node {
> >      bool nat_rpl_dst;
> >  };
> >
> > +struct conn_ext_nat {
> > +    struct nat_action_info_t *nat_info;
> > +    struct conn *nat_conn;
> > +};
> > +
> > +struct conn_ext_alg {
> > +    char *alg;
> > +    /* TCP sequence skew direction due to NATTing of FTP control
> messages;
> > +     * true if reply direction. */
> > +    int seq_skew;
> > +    struct conn_key master_key;
> > +    /* True if alg data connection. */
> > +    bool alg_related;
> > +    bool seq_skew_dir;
> > +};
> > +
> >  struct conn {
> >      struct conn_key key;
> >      struct conn_key rev_key;
> > -    /* Only used for orig_tuple support. */
> > -    struct conn_key master_key;
> >      long long expiration;
> >      struct ovs_list exp_node;
> >      struct cmap_node cm_node;
> >      ovs_u128 label;
> > -    struct nat_action_info_t *nat_info;
> > -    char *alg;
> > -    struct conn *nat_conn;
> > -    int seq_skew;
> > +    struct conn_ext_nat *ext_nat;
> > +    struct conn_ext_alg *ext_alg;
> >      uint32_t mark;
> >      /* See ct_conn_type. */
> >      uint8_t conn_type;
> > @@ -101,11 +113,6 @@ struct conn {
> >       * This field is used to signal an update to the specified list.
> The
> >       * value 'NO_UPD_EXP_LIST' is used to indicate no update to any
> list. */
> >      uint8_t exp_list_id;
> > -    /* TCP sequence skew direction due to NATTing of FTP control
> messages;
> > -     * true if reply direction. */
> > -    bool seq_skew_dir;
> > -    /* True if alg data connection. */
> > -    bool alg_related;
> >      /* Inserted into the cmap; handle theoretical expiry list race;
> although
> >       * such a race would probably mean a system meltdown. */
> >      bool inserted;
> > diff --git a/lib/conntrack.c b/lib/conntrack.c
> > index c47a0b0..9d10b14 100644
> > --- a/lib/conntrack.c
> > +++ b/lib/conntrack.c
> > @@ -399,16 +399,16 @@ conn_clean(struct conn *conn)
> >  {
> >      ovs_assert(conn->conn_type == CT_CONN_TYPE_DEFAULT);
> >
> > -    if (conn->alg) {
> > +    if (conn->ext_alg && conn->ext_alg->alg) {
> >          expectation_clean(&conn->key);
> >      }
> >
> >      uint32_t hash = conn_key_hash(&conn->key, hash_basis);
> >      cmap_remove(&cm_conns, &conn->cm_node, hash);
> >      ovs_list_remove(&conn->exp_node);
> > -    if (conn->nat_conn) {
> > -        hash = conn_key_hash(&conn->nat_conn->key, hash_basis);
> > -        cmap_remove(&cm_conns, &conn->nat_conn->cm_node, hash);
> > +    if (conn->ext_nat && conn->ext_nat->nat_conn) {
> > +        hash = conn_key_hash(&conn->ext_nat->nat_conn->key, hash_basis);
> > +        cmap_remove(&cm_conns, &conn->ext_nat->nat_conn->cm_node, hash);
> >      }
> >      ovsrcu_postpone(delete_conn, conn);
> >      atomic_count_dec(&n_conn);
> > @@ -418,7 +418,7 @@ static void
> >  conn_clean_one(struct conn *conn)
> >      OVS_NO_THREAD_SAFETY_ANALYSIS
> >  {
> > -    if (conn->alg) {
> > +    if (conn->ext_alg && conn->ext_alg->alg) {
> >          expectation_clean(&conn->key);
> >      }
> >
> > @@ -473,8 +473,8 @@ write_ct_md(struct dp_packet *pkt, uint16_t zone,
> const struct conn *conn,
> >
> >      /* Use the original direction tuple if we have it. */
> >      if (conn) {
> > -        if (conn->alg_related) {
> > -            key = &conn->master_key;
> > +        if (conn->ext_alg && conn->ext_alg->alg_related) {
> > +            key = &conn->ext_alg->master_key;
> >          } else {
> >              key = &conn->key;
> >          }
> > @@ -615,7 +615,8 @@ handle_alg_ctl(const struct conn_lookup_ctx *ctx,
> struct dp_packet *pkt,
> >                 long long now, bool nat)
> >  {
> >      /* ALG control packet handling with expectation creation. */
> > -    if (OVS_UNLIKELY(alg_helpers[ct_alg_ctl] && conn && conn->alg)) {
> > +    if (OVS_UNLIKELY(alg_helpers[ct_alg_ctl] && conn && conn->ext_alg &&
> > +                     conn->ext_alg->alg)) {
> >          conn_entry_lock(conn);
> >          alg_helpers[ct_alg_ctl](ctx, pkt, conn, now,
> CT_FTP_CTL_INTEREST,
> >                                  nat);
> > @@ -626,7 +627,7 @@ handle_alg_ctl(const struct conn_lookup_ctx *ctx,
> struct dp_packet *pkt,
> >  static void
> >  pat_packet(struct dp_packet *pkt, const struct conn *conn)
> >  {
> > -    if (conn->nat_info->nat_action & NAT_ACTION_SRC) {
> > +    if (conn->ext_nat->nat_info->nat_action & NAT_ACTION_SRC) {
> >          if (conn->key.nw_proto == IPPROTO_TCP) {
> >              struct tcp_header *th = dp_packet_l4(pkt);
> >              packet_set_tcp_port(pkt, conn->rev_key.dst.port,
> th->tcp_dst);
> > @@ -634,7 +635,7 @@ pat_packet(struct dp_packet *pkt, const struct conn
> *conn)
> >              struct udp_header *uh = dp_packet_l4(pkt);
> >              packet_set_udp_port(pkt, conn->rev_key.dst.port,
> uh->udp_dst);
> >          }
> > -    } else if (conn->nat_info->nat_action & NAT_ACTION_DST) {
> > +    } else if (conn->ext_nat->nat_info->nat_action & NAT_ACTION_DST) {
> >          if (conn->key.nw_proto == IPPROTO_TCP) {
> >              struct tcp_header *th = dp_packet_l4(pkt);
> >              packet_set_tcp_port(pkt, th->tcp_src,
> conn->rev_key.src.port);
> > @@ -648,7 +649,7 @@ pat_packet(struct dp_packet *pkt, const struct conn
> *conn)
> >  static void
> >  nat_packet(struct dp_packet *pkt, const struct conn *conn, bool related)
> >  {
> > -    if (conn->nat_info->nat_action & NAT_ACTION_SRC) {
> > +    if (conn->ext_nat->nat_info->nat_action & NAT_ACTION_SRC) {
> >          pkt->md.ct_state |= CS_SRC_NAT;
> >          if (conn->key.dl_type == htons(ETH_TYPE_IP)) {
> >              struct ip_header *nh = dp_packet_l3(pkt);
> > @@ -664,7 +665,7 @@ nat_packet(struct dp_packet *pkt, const struct conn
> *conn, bool related)
> >          if (!related) {
> >              pat_packet(pkt, conn);
> >          }
> > -    } else if (conn->nat_info->nat_action & NAT_ACTION_DST) {
> > +    } else if (conn->ext_nat->nat_info->nat_action & NAT_ACTION_DST) {
> >          pkt->md.ct_state |= CS_DST_NAT;
> >          if (conn->key.dl_type == htons(ETH_TYPE_IP)) {
> >              struct ip_header *nh = dp_packet_l3(pkt);
> > @@ -686,7 +687,7 @@ nat_packet(struct dp_packet *pkt, const struct conn
> *conn, bool related)
> >  static void
> >  un_pat_packet(struct dp_packet *pkt, const struct conn *conn)
> >  {
> > -    if (conn->nat_info->nat_action & NAT_ACTION_SRC) {
> > +    if (conn->ext_nat->nat_info->nat_action & NAT_ACTION_SRC) {
> >          if (conn->key.nw_proto == IPPROTO_TCP) {
> >              struct tcp_header *th = dp_packet_l4(pkt);
> >              packet_set_tcp_port(pkt, th->tcp_src, conn->key.src.port);
> > @@ -694,7 +695,7 @@ un_pat_packet(struct dp_packet *pkt, const struct
> conn *conn)
> >              struct udp_header *uh = dp_packet_l4(pkt);
> >              packet_set_udp_port(pkt, uh->udp_src, conn->key.src.port);
> >          }
> > -    } else if (conn->nat_info->nat_action & NAT_ACTION_DST) {
> > +    } else if (conn->ext_nat->nat_info->nat_action & NAT_ACTION_DST) {
> >          if (conn->key.nw_proto == IPPROTO_TCP) {
> >              struct tcp_header *th = dp_packet_l4(pkt);
> >              packet_set_tcp_port(pkt, conn->key.dst.port, th->tcp_dst);
> > @@ -708,7 +709,7 @@ un_pat_packet(struct dp_packet *pkt, const struct
> conn *conn)
> >  static void
> >  reverse_pat_packet(struct dp_packet *pkt, const struct conn *conn)
> >  {
> > -    if (conn->nat_info->nat_action & NAT_ACTION_SRC) {
> > +    if (conn->ext_nat->nat_info->nat_action & NAT_ACTION_SRC) {
> >          if (conn->key.nw_proto == IPPROTO_TCP) {
> >              struct tcp_header *th_in = dp_packet_l4(pkt);
> >              packet_set_tcp_port(pkt, conn->key.src.port,
> > @@ -718,7 +719,7 @@ reverse_pat_packet(struct dp_packet *pkt, const
> struct conn *conn)
> >              packet_set_udp_port(pkt, conn->key.src.port,
> >                                  uh_in->udp_dst);
> >          }
> > -    } else if (conn->nat_info->nat_action & NAT_ACTION_DST) {
> > +    } else if (conn->ext_nat->nat_info->nat_action & NAT_ACTION_DST) {
> >          if (conn->key.nw_proto == IPPROTO_TCP) {
> >              struct tcp_header *th_in = dp_packet_l4(pkt);
> >              packet_set_tcp_port(pkt, th_in->tcp_src,
> > @@ -750,10 +751,10 @@ reverse_nat_packet(struct dp_packet *pkt, const
> struct conn *conn)
> >          pkt->l3_ofs += (char *) inner_l3 - (char *) nh;
> >          pkt->l4_ofs += inner_l4 - (char *) icmp;
> >
> > -        if (conn->nat_info->nat_action & NAT_ACTION_SRC) {
> > +        if (conn->ext_nat->nat_info->nat_action & NAT_ACTION_SRC) {
> >              packet_set_ipv4_addr(pkt, &inner_l3->ip_src,
> >                                   conn->key.src.addr.ipv4_aligned);
> > -        } else if (conn->nat_info->nat_action & NAT_ACTION_DST) {
> > +        } else if (conn->ext_nat->nat_info->nat_action &
> NAT_ACTION_DST) {
> >              packet_set_ipv4_addr(pkt, &inner_l3->ip_dst,
> >                                   conn->key.dst.addr.ipv4_aligned);
> >          }
> > @@ -772,12 +773,12 @@ reverse_nat_packet(struct dp_packet *pkt, const
> struct conn *conn)
> >          pkt->l3_ofs += (char *) inner_l3_6 - (char *) nh6;
> >          pkt->l4_ofs += inner_l4 - (char *) icmp6;
> >
> > -        if (conn->nat_info->nat_action & NAT_ACTION_SRC) {
> > +        if (conn->ext_nat->nat_info->nat_action & NAT_ACTION_SRC) {
> >              packet_set_ipv6_addr(pkt, conn->key.nw_proto,
> >                                   inner_l3_6->ip6_src.be32,
> >                                   &conn->key.src.addr.ipv6_aligned,
> >                                   true);
> > -        } else if (conn->nat_info->nat_action & NAT_ACTION_DST) {
> > +        } else if (conn->ext_nat->nat_info->nat_action &
> NAT_ACTION_DST) {
> >              packet_set_ipv6_addr(pkt, conn->key.nw_proto,
> >                                   inner_l3_6->ip6_dst.be32,
> >                                   &conn->key.dst.addr.ipv6_aligned,
> > @@ -797,7 +798,7 @@ static void
> >  un_nat_packet(struct dp_packet *pkt, const struct conn *conn,
> >                bool related)
> >  {
> > -    if (conn->nat_info->nat_action & NAT_ACTION_SRC) {
> > +    if (conn->ext_nat->nat_info->nat_action & NAT_ACTION_SRC) {
> >          pkt->md.ct_state |= CS_DST_NAT;
> >          if (conn->key.dl_type == htons(ETH_TYPE_IP)) {
> >              struct ip_header *nh = dp_packet_l3(pkt);
> > @@ -815,7 +816,7 @@ un_nat_packet(struct dp_packet *pkt, const struct
> conn *conn,
> >          } else {
> >              un_pat_packet(pkt, conn);
> >          }
> > -    } else if (conn->nat_info->nat_action & NAT_ACTION_DST) {
> > +    } else if (conn->ext_nat->nat_info->nat_action & NAT_ACTION_DST) {
> >          pkt->md.ct_state |= CS_SRC_NAT;
> >          if (conn->key.dl_type == htons(ETH_TYPE_IP)) {
> >              struct ip_header *nh = dp_packet_l3(pkt);
> > @@ -846,8 +847,8 @@ conn_seq_skew_set(const struct conn_key *key, long
> long now, int seq_skew,
> >      conn_key_lookup(key, hash, now, &conn, &reply);
> >
> >      if (conn && seq_skew) {
> > -        conn->seq_skew = seq_skew;
> > -        conn->seq_skew_dir = seq_skew_dir;
> > +        conn->ext_alg->seq_skew = seq_skew;
> > +        conn->ext_alg->seq_skew_dir = seq_skew_dir;
> >      }
> >  }
> >
> > @@ -906,29 +907,38 @@ conn_not_found(struct dp_packet *pkt, struct
> conn_lookup_ctx *ctx,
> >          nc->rev_key = nc->key;
> >          conn_key_reverse(&nc->rev_key);
> >
> > -        if (ct_verify_helper(helper, ct_alg_ctl)) {
> > -            nc->alg = nullable_xstrdup(helper);
> > +        if (alg_exp || helper || ct_alg_ctl != CT_ALG_CTL_NONE) {
> > +            nc->ext_alg = xzalloc(sizeof *nc->ext_alg);
> > +        }
> > +
> > +        if (nat_action_info) {
> > +            nc->ext_nat = xzalloc(sizeof *nc->ext_nat);
> > +        }
> > +
> > +        if (nc->ext_alg && ct_verify_helper(helper, ct_alg_ctl)) {
> > +            nc->ext_alg->alg = nullable_xstrdup(helper);
> >          }
> >
> >          if (alg_exp) {
> > -            nc->alg_related = true;
> > +            nc->ext_alg->alg_related = true;
> >              nc->mark = alg_exp->master_mark;
> >              nc->label = alg_exp->master_label;
> > -            nc->master_key = alg_exp->master_key;
> > +            nc->ext_alg->master_key = alg_exp->master_key;
> >          }
> >
> >          if (nat_action_info) {
> > -            nc->nat_info = xmemdup(nat_action_info, sizeof
> *nc->nat_info);
> > +            nc->ext_nat->nat_info = xmemdup(nat_action_info,
> > +                                        sizeof *nc->ext_nat->nat_info);
> >
> >              nat_conn = xzalloc(sizeof *nat_conn);
> >
> >              if (alg_exp) {
> >                  if (alg_exp->nat_rpl_dst) {
> >                      nc->rev_key.dst.addr = alg_exp->alg_nat_repl_addr;
> > -                    nc->nat_info->nat_action = NAT_ACTION_SRC;
> > +                    nc->ext_nat->nat_info->nat_action = NAT_ACTION_SRC;
> >                  } else {
> >                      nc->rev_key.src.addr = alg_exp->alg_nat_repl_addr;
> > -                    nc->nat_info->nat_action = NAT_ACTION_DST;
> > +                    nc->ext_nat->nat_info->nat_action = NAT_ACTION_DST;
> >                  }
> >                  *nat_conn = *nc;
> >              } else {
> > @@ -942,20 +952,19 @@ conn_not_found(struct dp_packet *pkt, struct
> conn_lookup_ctx *ctx,
> >                  /* Update nc with nat adjustments. */
> >                  *nc = *nat_conn;
> >              }
> > +            nc->ext_nat->nat_conn = nat_conn;
> >              nat_packet(pkt, nc, ctx->icmp_related);
> >
> >              nat_conn->key = nc->rev_key;
> >              nat_conn->rev_key = nc->key;
> >              nat_conn->conn_type = CT_CONN_TYPE_UN_NAT;
> > -            nat_conn->nat_info = NULL;
> > -            nat_conn->alg = NULL;
> > -            nat_conn->nat_conn = NULL;
> > +            nat_conn->ext_nat = NULL;
> > +            nat_conn->ext_alg = NULL;
> >              uint32_t nat_hash = conn_key_hash(&nat_conn->key,
> >                                                hash_basis);
> >              cmap_insert(&cm_conns, &nat_conn->cm_node, nat_hash);
> >          }
> >
> > -        nc->nat_conn = nat_conn;
> >          nc->conn_type = CT_CONN_TYPE_DEFAULT;
> >          cmap_insert(&cm_conns, &nc->cm_node, ctx->hash);
> >          nc->inserted = true;
> > @@ -993,7 +1002,7 @@ conn_update_state(struct dp_packet *pkt, struct
> conn_lookup_ctx *ctx,
> >              pkt->md.ct_state |= CS_REPLY_DIR;
> >          }
> >      } else {
> > -        if (conn->alg_related) {
> > +        if (conn->ext_alg && conn->ext_alg->alg_related) {
> >              pkt->md.ct_state |= CS_RELATED;
> >          }
> >
> > @@ -1027,7 +1036,7 @@ static void
> >  handle_nat(struct dp_packet *pkt, struct conn *conn,
> >             uint16_t zone, bool reply, bool related)
> >  {
> > -    if (conn->nat_info &&
> > +    if (conn->ext_nat && conn->ext_nat->nat_info &&
> >          (!(pkt->md.ct_state & (CS_SRC_NAT | CS_DST_NAT)) ||
> >            (pkt->md.ct_state & (CS_SRC_NAT | CS_DST_NAT) &&
> >             zone != pkt->md.ct_zone))) {
> > @@ -1112,7 +1121,7 @@ conn_update_state_alg(struct dp_packet *pkt,
> struct conn_lookup_ctx *ctx,
> >          /* Keep sequence tracking in sync with the source of the
> >           * sequence skew. */
> >          conn_entry_lock(conn);
> > -        if (ctx->reply != conn->seq_skew_dir) {
> > +        if (ctx->reply != conn->ext_alg->seq_skew_dir) {
> >              handle_ftp_ctl(ctx, pkt, conn, now, CT_FTP_CTL_OTHER,
> >                             !!nat_action_info);
> >              /* conn_update_state locks for unrelated fields, so unlock.
> */
> > @@ -1275,7 +1284,7 @@ conntrack_clear(struct dp_packet *packet)
> >  static void
> >  set_mark(struct dp_packet *pkt, struct conn *conn, uint32_t val,
> uint32_t mask)
> >  {
> > -    if (conn->alg_related) {
> > +    if (conn->ext_alg && conn->ext_alg->alg_related) {
> >          pkt->md.ct_mark = conn->mark;
> >      } else {
> >          pkt->md.ct_mark = val | (pkt->md.ct_mark & ~(mask));
> > @@ -1288,7 +1297,7 @@ set_label(struct dp_packet *pkt, struct conn *conn,
> >            const struct ovs_key_ct_labels *val,
> >            const struct ovs_key_ct_labels *mask)
> >  {
> > -    if (conn->alg_related) {
> > +    if (conn->ext_alg && conn->ext_alg->alg_related) {
> >          pkt->md.ct_label = conn->label;
> >      } else {
> >          ovs_u128 v, m;
> > @@ -2012,11 +2021,11 @@ nat_range_hash(const struct conn *conn, uint32_t
> basis)
> >  {
> >      uint32_t hash = basis;
> >
> > -    hash = ct_addr_hash_add(hash, &conn->nat_info->min_addr);
> > -    hash = ct_addr_hash_add(hash, &conn->nat_info->max_addr);
> > +    hash = ct_addr_hash_add(hash, &conn->ext_nat->nat_info->min_addr);
> > +    hash = ct_addr_hash_add(hash, &conn->ext_nat->nat_info->max_addr);
> >      hash = hash_add(hash,
> > -                    (conn->nat_info->max_port << 16)
> > -                    | conn->nat_info->min_port);
> > +                    (conn->ext_nat->nat_info->max_port << 16)
> > +                    | conn->ext_nat->nat_info->min_port);
> >      hash = ct_endpoint_hash_add(hash, &conn->key.src);
> >      hash = ct_endpoint_hash_add(hash, &conn->key.dst);
> >      hash = hash_add(hash, (OVS_FORCE uint32_t) conn->key.dl_type);
> > @@ -2040,22 +2049,24 @@ nat_select_range_tuple(const struct conn *conn,
> struct conn *nat_conn)
> >      uint16_t first_port;
> >      uint32_t hash = nat_range_hash(conn, hash_basis);
> >
> > -    if ((conn->nat_info->nat_action & NAT_ACTION_SRC) &&
> > -        (!(conn->nat_info->nat_action & NAT_ACTION_SRC_PORT))) {
> > +    if ((conn->ext_nat->nat_info->nat_action & NAT_ACTION_SRC) &&
> > +        (!(conn->ext_nat->nat_info->nat_action & NAT_ACTION_SRC_PORT)))
> {
> >          min_port = ntohs(conn->key.src.port);
> >          max_port = ntohs(conn->key.src.port);
> >          first_port = min_port;
> > -    } else if ((conn->nat_info->nat_action & NAT_ACTION_DST) &&
> > -               (!(conn->nat_info->nat_action & NAT_ACTION_DST_PORT))) {
> > +    } else if ((conn->ext_nat->nat_info->nat_action & NAT_ACTION_DST) &&
> > +               (!(conn->ext_nat->nat_info->nat_action
> > +                  & NAT_ACTION_DST_PORT))) {
> >          min_port = ntohs(conn->key.dst.port);
> >          max_port = ntohs(conn->key.dst.port);
> >          first_port = min_port;
> >      } else {
> > -        uint16_t deltap = conn->nat_info->max_port -
> conn->nat_info->min_port;
> > +        uint16_t deltap = conn->ext_nat->nat_info->max_port -
> > +            conn->ext_nat->nat_info->min_port;
> >          uint32_t port_index = hash % (deltap + 1);
> > -        first_port = conn->nat_info->min_port + port_index;
> > -        min_port = conn->nat_info->min_port;
> > -        max_port = conn->nat_info->max_port;
> > +        first_port = conn->ext_nat->nat_info->min_port + port_index;
> > +        min_port = conn->ext_nat->nat_info->min_port;
> > +        max_port = conn->ext_nat->nat_info->max_port;
> >      }
> >
> >      uint32_t deltaa = 0;
> > @@ -2064,37 +2075,39 @@ nat_select_range_tuple(const struct conn *conn,
> struct conn *nat_conn)
> >      memset(&ct_addr, 0, sizeof ct_addr);
> >      struct ct_addr max_ct_addr;
> >      memset(&max_ct_addr, 0, sizeof max_ct_addr);
> > -    max_ct_addr = conn->nat_info->max_addr;
> > +    max_ct_addr = conn->ext_nat->nat_info->max_addr;
> >
> >      if (conn->key.dl_type == htons(ETH_TYPE_IP)) {
> > -        deltaa = ntohl(conn->nat_info->max_addr.ipv4_aligned) -
> > -                 ntohl(conn->nat_info->min_addr.ipv4_aligned);
> > +        deltaa = ntohl(conn->ext_nat->nat_info->max_addr.ipv4_aligned) -
> > +
>  ntohl(conn->ext_nat->nat_info->min_addr.ipv4_aligned);
> >          address_index = hash % (deltaa + 1);
> >          ct_addr.ipv4_aligned = htonl(
> > -            ntohl(conn->nat_info->min_addr.ipv4_aligned) +
> address_index);
> > +            ntohl(conn->ext_nat->nat_info->min_addr.ipv4_aligned) +
> > +                  address_index);
> >      } else {
> > -        deltaa =
> nat_ipv6_addrs_delta(&conn->nat_info->min_addr.ipv6_aligned,
> > -
> &conn->nat_info->max_addr.ipv6_aligned);
> > +        deltaa = nat_ipv6_addrs_delta(
> > +                     &conn->ext_nat->nat_info->min_addr.ipv6_aligned,
> > +                     &conn->ext_nat->nat_info->max_addr.ipv6_aligned);
> >          /* deltaa must be within 32 bits for full hash coverage. A 64 or
> >           * 128 bit hash is unnecessary and hence not used here. Most
> code
> >           * is kept common with V4; nat_ipv6_addrs_delta() will do the
> >           * enforcement via max_ct_addr. */
> > -        max_ct_addr = conn->nat_info->min_addr;
> > +        max_ct_addr = conn->ext_nat->nat_info->min_addr;
> >          nat_ipv6_addr_increment(&max_ct_addr.ipv6_aligned, deltaa);
> >          address_index = hash % (deltaa + 1);
> > -        ct_addr.ipv6_aligned = conn->nat_info->min_addr.ipv6_aligned;
> > +        ct_addr.ipv6_aligned =
> conn->ext_nat->nat_info->min_addr.ipv6_aligned;
> >          nat_ipv6_addr_increment(&ct_addr.ipv6_aligned, address_index);
> >      }
> >
> >      uint16_t port = first_port;
> >      bool all_ports_tried = false;
> >      /* For DNAT, we don't use ephemeral ports. */
> > -    bool ephemeral_ports_tried = conn->nat_info->nat_action &
> NAT_ACTION_DST
> > -                                 ? true : false;
> > +    bool ephemeral_ports_tried =
> > +        conn->ext_nat->nat_info->nat_action & NAT_ACTION_DST ? true :
> false;
> >      struct ct_addr first_addr = ct_addr;
> >
> >      while (true) {
> > -        if (conn->nat_info->nat_action & NAT_ACTION_SRC) {
> > +        if (conn->ext_nat->nat_info->nat_action & NAT_ACTION_SRC) {
> >              nat_conn->rev_key.dst.addr = ct_addr;
> >          } else {
> >              nat_conn->rev_key.src.addr = ct_addr;
> > @@ -2103,7 +2116,7 @@ nat_select_range_tuple(const struct conn *conn,
> struct conn *nat_conn)
> >          if ((conn->key.nw_proto == IPPROTO_ICMP) ||
> >              (conn->key.nw_proto == IPPROTO_ICMPV6)) {
> >              all_ports_tried = true;
> > -        } else if (conn->nat_info->nat_action & NAT_ACTION_SRC) {
> > +        } else if (conn->ext_nat->nat_info->nat_action &
> NAT_ACTION_SRC) {
> >              nat_conn->rev_key.dst.port = htons(port);
> >          } else {
> >              nat_conn->rev_key.src.port = htons(port);
> > @@ -2135,14 +2148,14 @@ nat_select_range_tuple(const struct conn *conn,
> struct conn *nat_conn)
> >                      nat_ipv6_addr_increment(&ct_addr.ipv6_aligned, 1);
> >                  }
> >              } else {
> > -                ct_addr = conn->nat_info->min_addr;
> > +                ct_addr = conn->ext_nat->nat_info->min_addr;
> >              }
> >              if (!memcmp(&ct_addr, &first_addr, sizeof ct_addr)) {
> >                  if (ephemeral_ports_tried) {
> >                      break;
> >                  } else {
> >                      ephemeral_ports_tried = true;
> > -                    ct_addr = conn->nat_info->min_addr;
> > +                    ct_addr = conn->ext_nat->nat_info->min_addr;
> >                      first_addr = ct_addr;
> >                      min_port = MIN_NAT_EPHEMERAL_PORT;
> >                      max_port = MAX_NAT_EPHEMERAL_PORT;
> > @@ -2193,16 +2206,26 @@ conn_update(struct dp_packet *pkt, struct conn
> *conn,
> >  }
> >
> >  static void
> > +delete_conn_cmn(struct conn *conn)
> > +{
> > +    free(conn->ext_alg);
> > +    free(conn->ext_nat);
> > +    free(conn);
> > +}
> > +
> > +static void
> >  delete_conn(struct conn *conn)
> >  {
> >      if (conn->conn_type == CT_CONN_TYPE_DEFAULT) {
> > -        free(conn->nat_info);
> > -        free(conn->alg);
> > -        conn_entry_destroy(conn);
> > -        if (conn->nat_conn) {
> > -            free(conn->nat_conn);
> > +        if (conn->ext_nat) {
> > +            free(conn->ext_nat->nat_info);
> > +            free(conn->ext_nat->nat_conn);
>
> Free doesn't require tests.  It can handle NULL.  Goes for all
> instances.
>

The check is for 'conn->ext_nat' before dereferencing to
'conn->ext_nat->nat_info' and 'conn->ext_nat->nat_conn'


>
> Plus I commented on condensing these entries earlier.  I think that
> comment still holds true, but haven't re-read the calling places to make
> sure it does.
>

See Patch 2 for my response about the condensation.


>
> >          }
> > -        free(conn);
> > +        if (conn->ext_alg) {
> > +            free(conn->ext_alg->alg);
> > +        }
> > +        conn_entry_destroy(conn);
> > +        delete_conn_cmn(conn);
> >      }
> >  }
> >
> > @@ -2210,11 +2233,15 @@ static void
> >  delete_conn_one(struct conn *conn)
> >  {
> >      if (conn->conn_type == CT_CONN_TYPE_DEFAULT) {
> > -        free(conn->nat_info);
> > -        free(conn->alg);
> > +        if (conn->ext_nat) {
> > +            free(conn->ext_nat->nat_info);
> > +        }
> > +        if (conn->ext_alg) {
> > +            free(conn->ext_alg->alg);
> > +        }
> >          conn_entry_destroy(conn);
> >      }
> > -    free(conn);
> > +    delete_conn_cmn(conn);
> >  }
> >
> >  /* Convert a conntrack address 'a' into an IP address 'b' based on
> 'dl_type'.
> > @@ -2328,9 +2355,11 @@ conn_to_ct_dpif_entry(const struct conn *conn,
> struct ct_dpif_entry *entry,
> >
> >      entry->bkt = bkt;
> >
> > -    if (conn->alg) {
> > +    if (conn->ext_alg && conn->ext_alg->alg) {
> >          /* Caller is responsible for freeing. */
> > -        entry->helper.name = xstrdup(conn->alg);
> > +        entry->helper.name = xstrdup(conn->ext_alg->alg);
> > +    } else {
> > +        entry->helper.name = NULL;
> >      }
> >  }
> >
> > @@ -3016,7 +3045,7 @@ handle_ftp_ctl(const struct conn_lookup_ctx *ctx,
> struct dp_packet *pkt,
> >          return;
> >      }
> >
> > -    if (!nat || !conn_for_expectation->seq_skew) {
> > +    if (!nat || !conn_for_expectation->ext_alg->seq_skew) {
> >          do_seq_skew_adj = false;
> >      }
> >
> > @@ -3024,7 +3053,7 @@ handle_ftp_ctl(const struct conn_lookup_ctx *ctx,
> struct dp_packet *pkt,
> >      int64_t seq_skew = 0;
> >
> >      if (ftp_ctl == CT_FTP_CTL_OTHER) {
> > -        seq_skew = conn_for_expectation->seq_skew;
> > +        seq_skew = conn_for_expectation->ext_alg->seq_skew;
> >      } else if (ftp_ctl == CT_FTP_CTL_INTEREST) {
> >          enum ftp_ctl_pkt rc;
> >          if (ctx->key.dl_type == htons(ETH_TYPE_IPV6)) {
> > @@ -3079,7 +3108,7 @@ handle_ftp_ctl(const struct conn_lookup_ctx *ctx,
> struct dp_packet *pkt,
> >      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 (ctx->reply != conn_for_expectation->ext_alg->seq_skew_dir) {
> >
> >              uint32_t tcp_ack = ntohl(get_16aligned_be32(&th->tcp_ack));
>
diff mbox series

Patch

diff --git a/lib/conntrack-private.h b/lib/conntrack-private.h
index ac891cc..20d2d78 100644
--- a/lib/conntrack-private.h
+++ b/lib/conntrack-private.h
@@ -81,19 +81,31 @@  struct alg_exp_node {
     bool nat_rpl_dst;
 };
 
+struct conn_ext_nat {
+    struct nat_action_info_t *nat_info;
+    struct conn *nat_conn;
+};
+
+struct conn_ext_alg {
+    char *alg;
+    /* TCP sequence skew direction due to NATTing of FTP control messages;
+     * true if reply direction. */
+    int seq_skew;
+    struct conn_key master_key;
+    /* True if alg data connection. */
+    bool alg_related;
+    bool seq_skew_dir;
+};
+
 struct conn {
     struct conn_key key;
     struct conn_key rev_key;
-    /* Only used for orig_tuple support. */
-    struct conn_key master_key;
     long long expiration;
     struct ovs_list exp_node;
     struct cmap_node cm_node;
     ovs_u128 label;
-    struct nat_action_info_t *nat_info;
-    char *alg;
-    struct conn *nat_conn;
-    int seq_skew;
+    struct conn_ext_nat *ext_nat;
+    struct conn_ext_alg *ext_alg;
     uint32_t mark;
     /* See ct_conn_type. */
     uint8_t conn_type;
@@ -101,11 +113,6 @@  struct conn {
      * This field is used to signal an update to the specified list.  The
      * value 'NO_UPD_EXP_LIST' is used to indicate no update to any list. */
     uint8_t exp_list_id;
-    /* TCP sequence skew direction due to NATTing of FTP control messages;
-     * true if reply direction. */
-    bool seq_skew_dir;
-    /* True if alg data connection. */
-    bool alg_related;
     /* Inserted into the cmap; handle theoretical expiry list race; although
      * such a race would probably mean a system meltdown. */
     bool inserted;
diff --git a/lib/conntrack.c b/lib/conntrack.c
index c47a0b0..9d10b14 100644
--- a/lib/conntrack.c
+++ b/lib/conntrack.c
@@ -399,16 +399,16 @@  conn_clean(struct conn *conn)
 {
     ovs_assert(conn->conn_type == CT_CONN_TYPE_DEFAULT);
 
-    if (conn->alg) {
+    if (conn->ext_alg && conn->ext_alg->alg) {
         expectation_clean(&conn->key);
     }
 
     uint32_t hash = conn_key_hash(&conn->key, hash_basis);
     cmap_remove(&cm_conns, &conn->cm_node, hash);
     ovs_list_remove(&conn->exp_node);
-    if (conn->nat_conn) {
-        hash = conn_key_hash(&conn->nat_conn->key, hash_basis);
-        cmap_remove(&cm_conns, &conn->nat_conn->cm_node, hash);
+    if (conn->ext_nat && conn->ext_nat->nat_conn) {
+        hash = conn_key_hash(&conn->ext_nat->nat_conn->key, hash_basis);
+        cmap_remove(&cm_conns, &conn->ext_nat->nat_conn->cm_node, hash);
     }
     ovsrcu_postpone(delete_conn, conn);
     atomic_count_dec(&n_conn);
@@ -418,7 +418,7 @@  static void
 conn_clean_one(struct conn *conn)
     OVS_NO_THREAD_SAFETY_ANALYSIS
 {
-    if (conn->alg) {
+    if (conn->ext_alg && conn->ext_alg->alg) {
         expectation_clean(&conn->key);
     }
 
@@ -473,8 +473,8 @@  write_ct_md(struct dp_packet *pkt, uint16_t zone, const struct conn *conn,
 
     /* Use the original direction tuple if we have it. */
     if (conn) {
-        if (conn->alg_related) {
-            key = &conn->master_key;
+        if (conn->ext_alg && conn->ext_alg->alg_related) {
+            key = &conn->ext_alg->master_key;
         } else {
             key = &conn->key;
         }
@@ -615,7 +615,8 @@  handle_alg_ctl(const struct conn_lookup_ctx *ctx, struct dp_packet *pkt,
                long long now, bool nat)
 {
     /* ALG control packet handling with expectation creation. */
-    if (OVS_UNLIKELY(alg_helpers[ct_alg_ctl] && conn && conn->alg)) {
+    if (OVS_UNLIKELY(alg_helpers[ct_alg_ctl] && conn && conn->ext_alg &&
+                     conn->ext_alg->alg)) {
         conn_entry_lock(conn);
         alg_helpers[ct_alg_ctl](ctx, pkt, conn, now, CT_FTP_CTL_INTEREST,
                                 nat);
@@ -626,7 +627,7 @@  handle_alg_ctl(const struct conn_lookup_ctx *ctx, struct dp_packet *pkt,
 static void
 pat_packet(struct dp_packet *pkt, const struct conn *conn)
 {
-    if (conn->nat_info->nat_action & NAT_ACTION_SRC) {
+    if (conn->ext_nat->nat_info->nat_action & NAT_ACTION_SRC) {
         if (conn->key.nw_proto == IPPROTO_TCP) {
             struct tcp_header *th = dp_packet_l4(pkt);
             packet_set_tcp_port(pkt, conn->rev_key.dst.port, th->tcp_dst);
@@ -634,7 +635,7 @@  pat_packet(struct dp_packet *pkt, const struct conn *conn)
             struct udp_header *uh = dp_packet_l4(pkt);
             packet_set_udp_port(pkt, conn->rev_key.dst.port, uh->udp_dst);
         }
-    } else if (conn->nat_info->nat_action & NAT_ACTION_DST) {
+    } else if (conn->ext_nat->nat_info->nat_action & NAT_ACTION_DST) {
         if (conn->key.nw_proto == IPPROTO_TCP) {
             struct tcp_header *th = dp_packet_l4(pkt);
             packet_set_tcp_port(pkt, th->tcp_src, conn->rev_key.src.port);
@@ -648,7 +649,7 @@  pat_packet(struct dp_packet *pkt, const struct conn *conn)
 static void
 nat_packet(struct dp_packet *pkt, const struct conn *conn, bool related)
 {
-    if (conn->nat_info->nat_action & NAT_ACTION_SRC) {
+    if (conn->ext_nat->nat_info->nat_action & NAT_ACTION_SRC) {
         pkt->md.ct_state |= CS_SRC_NAT;
         if (conn->key.dl_type == htons(ETH_TYPE_IP)) {
             struct ip_header *nh = dp_packet_l3(pkt);
@@ -664,7 +665,7 @@  nat_packet(struct dp_packet *pkt, const struct conn *conn, bool related)
         if (!related) {
             pat_packet(pkt, conn);
         }
-    } else if (conn->nat_info->nat_action & NAT_ACTION_DST) {
+    } else if (conn->ext_nat->nat_info->nat_action & NAT_ACTION_DST) {
         pkt->md.ct_state |= CS_DST_NAT;
         if (conn->key.dl_type == htons(ETH_TYPE_IP)) {
             struct ip_header *nh = dp_packet_l3(pkt);
@@ -686,7 +687,7 @@  nat_packet(struct dp_packet *pkt, const struct conn *conn, bool related)
 static void
 un_pat_packet(struct dp_packet *pkt, const struct conn *conn)
 {
-    if (conn->nat_info->nat_action & NAT_ACTION_SRC) {
+    if (conn->ext_nat->nat_info->nat_action & NAT_ACTION_SRC) {
         if (conn->key.nw_proto == IPPROTO_TCP) {
             struct tcp_header *th = dp_packet_l4(pkt);
             packet_set_tcp_port(pkt, th->tcp_src, conn->key.src.port);
@@ -694,7 +695,7 @@  un_pat_packet(struct dp_packet *pkt, const struct conn *conn)
             struct udp_header *uh = dp_packet_l4(pkt);
             packet_set_udp_port(pkt, uh->udp_src, conn->key.src.port);
         }
-    } else if (conn->nat_info->nat_action & NAT_ACTION_DST) {
+    } else if (conn->ext_nat->nat_info->nat_action & NAT_ACTION_DST) {
         if (conn->key.nw_proto == IPPROTO_TCP) {
             struct tcp_header *th = dp_packet_l4(pkt);
             packet_set_tcp_port(pkt, conn->key.dst.port, th->tcp_dst);
@@ -708,7 +709,7 @@  un_pat_packet(struct dp_packet *pkt, const struct conn *conn)
 static void
 reverse_pat_packet(struct dp_packet *pkt, const struct conn *conn)
 {
-    if (conn->nat_info->nat_action & NAT_ACTION_SRC) {
+    if (conn->ext_nat->nat_info->nat_action & NAT_ACTION_SRC) {
         if (conn->key.nw_proto == IPPROTO_TCP) {
             struct tcp_header *th_in = dp_packet_l4(pkt);
             packet_set_tcp_port(pkt, conn->key.src.port,
@@ -718,7 +719,7 @@  reverse_pat_packet(struct dp_packet *pkt, const struct conn *conn)
             packet_set_udp_port(pkt, conn->key.src.port,
                                 uh_in->udp_dst);
         }
-    } else if (conn->nat_info->nat_action & NAT_ACTION_DST) {
+    } else if (conn->ext_nat->nat_info->nat_action & NAT_ACTION_DST) {
         if (conn->key.nw_proto == IPPROTO_TCP) {
             struct tcp_header *th_in = dp_packet_l4(pkt);
             packet_set_tcp_port(pkt, th_in->tcp_src,
@@ -750,10 +751,10 @@  reverse_nat_packet(struct dp_packet *pkt, const struct conn *conn)
         pkt->l3_ofs += (char *) inner_l3 - (char *) nh;
         pkt->l4_ofs += inner_l4 - (char *) icmp;
 
-        if (conn->nat_info->nat_action & NAT_ACTION_SRC) {
+        if (conn->ext_nat->nat_info->nat_action & NAT_ACTION_SRC) {
             packet_set_ipv4_addr(pkt, &inner_l3->ip_src,
                                  conn->key.src.addr.ipv4_aligned);
-        } else if (conn->nat_info->nat_action & NAT_ACTION_DST) {
+        } else if (conn->ext_nat->nat_info->nat_action & NAT_ACTION_DST) {
             packet_set_ipv4_addr(pkt, &inner_l3->ip_dst,
                                  conn->key.dst.addr.ipv4_aligned);
         }
@@ -772,12 +773,12 @@  reverse_nat_packet(struct dp_packet *pkt, const struct conn *conn)
         pkt->l3_ofs += (char *) inner_l3_6 - (char *) nh6;
         pkt->l4_ofs += inner_l4 - (char *) icmp6;
 
-        if (conn->nat_info->nat_action & NAT_ACTION_SRC) {
+        if (conn->ext_nat->nat_info->nat_action & NAT_ACTION_SRC) {
             packet_set_ipv6_addr(pkt, conn->key.nw_proto,
                                  inner_l3_6->ip6_src.be32,
                                  &conn->key.src.addr.ipv6_aligned,
                                  true);
-        } else if (conn->nat_info->nat_action & NAT_ACTION_DST) {
+        } else if (conn->ext_nat->nat_info->nat_action & NAT_ACTION_DST) {
             packet_set_ipv6_addr(pkt, conn->key.nw_proto,
                                  inner_l3_6->ip6_dst.be32,
                                  &conn->key.dst.addr.ipv6_aligned,
@@ -797,7 +798,7 @@  static void
 un_nat_packet(struct dp_packet *pkt, const struct conn *conn,
               bool related)
 {
-    if (conn->nat_info->nat_action & NAT_ACTION_SRC) {
+    if (conn->ext_nat->nat_info->nat_action & NAT_ACTION_SRC) {
         pkt->md.ct_state |= CS_DST_NAT;
         if (conn->key.dl_type == htons(ETH_TYPE_IP)) {
             struct ip_header *nh = dp_packet_l3(pkt);
@@ -815,7 +816,7 @@  un_nat_packet(struct dp_packet *pkt, const struct conn *conn,
         } else {
             un_pat_packet(pkt, conn);
         }
-    } else if (conn->nat_info->nat_action & NAT_ACTION_DST) {
+    } else if (conn->ext_nat->nat_info->nat_action & NAT_ACTION_DST) {
         pkt->md.ct_state |= CS_SRC_NAT;
         if (conn->key.dl_type == htons(ETH_TYPE_IP)) {
             struct ip_header *nh = dp_packet_l3(pkt);
@@ -846,8 +847,8 @@  conn_seq_skew_set(const struct conn_key *key, long long now, int seq_skew,
     conn_key_lookup(key, hash, now, &conn, &reply);
 
     if (conn && seq_skew) {
-        conn->seq_skew = seq_skew;
-        conn->seq_skew_dir = seq_skew_dir;
+        conn->ext_alg->seq_skew = seq_skew;
+        conn->ext_alg->seq_skew_dir = seq_skew_dir;
     }
 }
 
@@ -906,29 +907,38 @@  conn_not_found(struct dp_packet *pkt, struct conn_lookup_ctx *ctx,
         nc->rev_key = nc->key;
         conn_key_reverse(&nc->rev_key);
 
-        if (ct_verify_helper(helper, ct_alg_ctl)) {
-            nc->alg = nullable_xstrdup(helper);
+        if (alg_exp || helper || ct_alg_ctl != CT_ALG_CTL_NONE) {
+            nc->ext_alg = xzalloc(sizeof *nc->ext_alg);
+        }
+
+        if (nat_action_info) {
+            nc->ext_nat = xzalloc(sizeof *nc->ext_nat);
+        }
+
+        if (nc->ext_alg && ct_verify_helper(helper, ct_alg_ctl)) {
+            nc->ext_alg->alg = nullable_xstrdup(helper);
         }
 
         if (alg_exp) {
-            nc->alg_related = true;
+            nc->ext_alg->alg_related = true;
             nc->mark = alg_exp->master_mark;
             nc->label = alg_exp->master_label;
-            nc->master_key = alg_exp->master_key;
+            nc->ext_alg->master_key = alg_exp->master_key;
         }
 
         if (nat_action_info) {
-            nc->nat_info = xmemdup(nat_action_info, sizeof *nc->nat_info);
+            nc->ext_nat->nat_info = xmemdup(nat_action_info,
+                                        sizeof *nc->ext_nat->nat_info);
 
             nat_conn = xzalloc(sizeof *nat_conn);
 
             if (alg_exp) {
                 if (alg_exp->nat_rpl_dst) {
                     nc->rev_key.dst.addr = alg_exp->alg_nat_repl_addr;
-                    nc->nat_info->nat_action = NAT_ACTION_SRC;
+                    nc->ext_nat->nat_info->nat_action = NAT_ACTION_SRC;
                 } else {
                     nc->rev_key.src.addr = alg_exp->alg_nat_repl_addr;
-                    nc->nat_info->nat_action = NAT_ACTION_DST;
+                    nc->ext_nat->nat_info->nat_action = NAT_ACTION_DST;
                 }
                 *nat_conn = *nc;
             } else {
@@ -942,20 +952,19 @@  conn_not_found(struct dp_packet *pkt, struct conn_lookup_ctx *ctx,
                 /* Update nc with nat adjustments. */
                 *nc = *nat_conn;
             }
+            nc->ext_nat->nat_conn = nat_conn;
             nat_packet(pkt, nc, ctx->icmp_related);
 
             nat_conn->key = nc->rev_key;
             nat_conn->rev_key = nc->key;
             nat_conn->conn_type = CT_CONN_TYPE_UN_NAT;
-            nat_conn->nat_info = NULL;
-            nat_conn->alg = NULL;
-            nat_conn->nat_conn = NULL;
+            nat_conn->ext_nat = NULL;
+            nat_conn->ext_alg = NULL;
             uint32_t nat_hash = conn_key_hash(&nat_conn->key,
                                               hash_basis);
             cmap_insert(&cm_conns, &nat_conn->cm_node, nat_hash);
         }
 
-        nc->nat_conn = nat_conn;
         nc->conn_type = CT_CONN_TYPE_DEFAULT;
         cmap_insert(&cm_conns, &nc->cm_node, ctx->hash);
         nc->inserted = true;
@@ -993,7 +1002,7 @@  conn_update_state(struct dp_packet *pkt, struct conn_lookup_ctx *ctx,
             pkt->md.ct_state |= CS_REPLY_DIR;
         }
     } else {
-        if (conn->alg_related) {
+        if (conn->ext_alg && conn->ext_alg->alg_related) {
             pkt->md.ct_state |= CS_RELATED;
         }
 
@@ -1027,7 +1036,7 @@  static void
 handle_nat(struct dp_packet *pkt, struct conn *conn,
            uint16_t zone, bool reply, bool related)
 {
-    if (conn->nat_info &&
+    if (conn->ext_nat && conn->ext_nat->nat_info &&
         (!(pkt->md.ct_state & (CS_SRC_NAT | CS_DST_NAT)) ||
           (pkt->md.ct_state & (CS_SRC_NAT | CS_DST_NAT) &&
            zone != pkt->md.ct_zone))) {
@@ -1112,7 +1121,7 @@  conn_update_state_alg(struct dp_packet *pkt, struct conn_lookup_ctx *ctx,
         /* Keep sequence tracking in sync with the source of the
          * sequence skew. */
         conn_entry_lock(conn);
-        if (ctx->reply != conn->seq_skew_dir) {
+        if (ctx->reply != conn->ext_alg->seq_skew_dir) {
             handle_ftp_ctl(ctx, pkt, conn, now, CT_FTP_CTL_OTHER,
                            !!nat_action_info);
             /* conn_update_state locks for unrelated fields, so unlock. */
@@ -1275,7 +1284,7 @@  conntrack_clear(struct dp_packet *packet)
 static void
 set_mark(struct dp_packet *pkt, struct conn *conn, uint32_t val, uint32_t mask)
 {
-    if (conn->alg_related) {
+    if (conn->ext_alg && conn->ext_alg->alg_related) {
         pkt->md.ct_mark = conn->mark;
     } else {
         pkt->md.ct_mark = val | (pkt->md.ct_mark & ~(mask));
@@ -1288,7 +1297,7 @@  set_label(struct dp_packet *pkt, struct conn *conn,
           const struct ovs_key_ct_labels *val,
           const struct ovs_key_ct_labels *mask)
 {
-    if (conn->alg_related) {
+    if (conn->ext_alg && conn->ext_alg->alg_related) {
         pkt->md.ct_label = conn->label;
     } else {
         ovs_u128 v, m;
@@ -2012,11 +2021,11 @@  nat_range_hash(const struct conn *conn, uint32_t basis)
 {
     uint32_t hash = basis;
 
-    hash = ct_addr_hash_add(hash, &conn->nat_info->min_addr);
-    hash = ct_addr_hash_add(hash, &conn->nat_info->max_addr);
+    hash = ct_addr_hash_add(hash, &conn->ext_nat->nat_info->min_addr);
+    hash = ct_addr_hash_add(hash, &conn->ext_nat->nat_info->max_addr);
     hash = hash_add(hash,
-                    (conn->nat_info->max_port << 16)
-                    | conn->nat_info->min_port);
+                    (conn->ext_nat->nat_info->max_port << 16)
+                    | conn->ext_nat->nat_info->min_port);
     hash = ct_endpoint_hash_add(hash, &conn->key.src);
     hash = ct_endpoint_hash_add(hash, &conn->key.dst);
     hash = hash_add(hash, (OVS_FORCE uint32_t) conn->key.dl_type);
@@ -2040,22 +2049,24 @@  nat_select_range_tuple(const struct conn *conn, struct conn *nat_conn)
     uint16_t first_port;
     uint32_t hash = nat_range_hash(conn, hash_basis);
 
-    if ((conn->nat_info->nat_action & NAT_ACTION_SRC) &&
-        (!(conn->nat_info->nat_action & NAT_ACTION_SRC_PORT))) {
+    if ((conn->ext_nat->nat_info->nat_action & NAT_ACTION_SRC) &&
+        (!(conn->ext_nat->nat_info->nat_action & NAT_ACTION_SRC_PORT))) {
         min_port = ntohs(conn->key.src.port);
         max_port = ntohs(conn->key.src.port);
         first_port = min_port;
-    } else if ((conn->nat_info->nat_action & NAT_ACTION_DST) &&
-               (!(conn->nat_info->nat_action & NAT_ACTION_DST_PORT))) {
+    } else if ((conn->ext_nat->nat_info->nat_action & NAT_ACTION_DST) &&
+               (!(conn->ext_nat->nat_info->nat_action
+                  & NAT_ACTION_DST_PORT))) {
         min_port = ntohs(conn->key.dst.port);
         max_port = ntohs(conn->key.dst.port);
         first_port = min_port;
     } else {
-        uint16_t deltap = conn->nat_info->max_port - conn->nat_info->min_port;
+        uint16_t deltap = conn->ext_nat->nat_info->max_port -
+            conn->ext_nat->nat_info->min_port;
         uint32_t port_index = hash % (deltap + 1);
-        first_port = conn->nat_info->min_port + port_index;
-        min_port = conn->nat_info->min_port;
-        max_port = conn->nat_info->max_port;
+        first_port = conn->ext_nat->nat_info->min_port + port_index;
+        min_port = conn->ext_nat->nat_info->min_port;
+        max_port = conn->ext_nat->nat_info->max_port;
     }
 
     uint32_t deltaa = 0;
@@ -2064,37 +2075,39 @@  nat_select_range_tuple(const struct conn *conn, struct conn *nat_conn)
     memset(&ct_addr, 0, sizeof ct_addr);
     struct ct_addr max_ct_addr;
     memset(&max_ct_addr, 0, sizeof max_ct_addr);
-    max_ct_addr = conn->nat_info->max_addr;
+    max_ct_addr = conn->ext_nat->nat_info->max_addr;
 
     if (conn->key.dl_type == htons(ETH_TYPE_IP)) {
-        deltaa = ntohl(conn->nat_info->max_addr.ipv4_aligned) -
-                 ntohl(conn->nat_info->min_addr.ipv4_aligned);
+        deltaa = ntohl(conn->ext_nat->nat_info->max_addr.ipv4_aligned) -
+                       ntohl(conn->ext_nat->nat_info->min_addr.ipv4_aligned);
         address_index = hash % (deltaa + 1);
         ct_addr.ipv4_aligned = htonl(
-            ntohl(conn->nat_info->min_addr.ipv4_aligned) + address_index);
+            ntohl(conn->ext_nat->nat_info->min_addr.ipv4_aligned) +
+                  address_index);
     } else {
-        deltaa = nat_ipv6_addrs_delta(&conn->nat_info->min_addr.ipv6_aligned,
-                                      &conn->nat_info->max_addr.ipv6_aligned);
+        deltaa = nat_ipv6_addrs_delta(
+                     &conn->ext_nat->nat_info->min_addr.ipv6_aligned,
+                     &conn->ext_nat->nat_info->max_addr.ipv6_aligned);
         /* deltaa must be within 32 bits for full hash coverage. A 64 or
          * 128 bit hash is unnecessary and hence not used here. Most code
          * is kept common with V4; nat_ipv6_addrs_delta() will do the
          * enforcement via max_ct_addr. */
-        max_ct_addr = conn->nat_info->min_addr;
+        max_ct_addr = conn->ext_nat->nat_info->min_addr;
         nat_ipv6_addr_increment(&max_ct_addr.ipv6_aligned, deltaa);
         address_index = hash % (deltaa + 1);
-        ct_addr.ipv6_aligned = conn->nat_info->min_addr.ipv6_aligned;
+        ct_addr.ipv6_aligned = conn->ext_nat->nat_info->min_addr.ipv6_aligned;
         nat_ipv6_addr_increment(&ct_addr.ipv6_aligned, address_index);
     }
 
     uint16_t port = first_port;
     bool all_ports_tried = false;
     /* For DNAT, we don't use ephemeral ports. */
-    bool ephemeral_ports_tried = conn->nat_info->nat_action & NAT_ACTION_DST
-                                 ? true : false;
+    bool ephemeral_ports_tried =
+        conn->ext_nat->nat_info->nat_action & NAT_ACTION_DST ? true : false;
     struct ct_addr first_addr = ct_addr;
 
     while (true) {
-        if (conn->nat_info->nat_action & NAT_ACTION_SRC) {
+        if (conn->ext_nat->nat_info->nat_action & NAT_ACTION_SRC) {
             nat_conn->rev_key.dst.addr = ct_addr;
         } else {
             nat_conn->rev_key.src.addr = ct_addr;
@@ -2103,7 +2116,7 @@  nat_select_range_tuple(const struct conn *conn, struct conn *nat_conn)
         if ((conn->key.nw_proto == IPPROTO_ICMP) ||
             (conn->key.nw_proto == IPPROTO_ICMPV6)) {
             all_ports_tried = true;
-        } else if (conn->nat_info->nat_action & NAT_ACTION_SRC) {
+        } else if (conn->ext_nat->nat_info->nat_action & NAT_ACTION_SRC) {
             nat_conn->rev_key.dst.port = htons(port);
         } else {
             nat_conn->rev_key.src.port = htons(port);
@@ -2135,14 +2148,14 @@  nat_select_range_tuple(const struct conn *conn, struct conn *nat_conn)
                     nat_ipv6_addr_increment(&ct_addr.ipv6_aligned, 1);
                 }
             } else {
-                ct_addr = conn->nat_info->min_addr;
+                ct_addr = conn->ext_nat->nat_info->min_addr;
             }
             if (!memcmp(&ct_addr, &first_addr, sizeof ct_addr)) {
                 if (ephemeral_ports_tried) {
                     break;
                 } else {
                     ephemeral_ports_tried = true;
-                    ct_addr = conn->nat_info->min_addr;
+                    ct_addr = conn->ext_nat->nat_info->min_addr;
                     first_addr = ct_addr;
                     min_port = MIN_NAT_EPHEMERAL_PORT;
                     max_port = MAX_NAT_EPHEMERAL_PORT;
@@ -2193,16 +2206,26 @@  conn_update(struct dp_packet *pkt, struct conn *conn,
 }
 
 static void
+delete_conn_cmn(struct conn *conn)
+{
+    free(conn->ext_alg);
+    free(conn->ext_nat);
+    free(conn);
+}
+
+static void
 delete_conn(struct conn *conn)
 {
     if (conn->conn_type == CT_CONN_TYPE_DEFAULT) {
-        free(conn->nat_info);
-        free(conn->alg);
-        conn_entry_destroy(conn);
-        if (conn->nat_conn) {
-            free(conn->nat_conn);
+        if (conn->ext_nat) {
+            free(conn->ext_nat->nat_info);
+            free(conn->ext_nat->nat_conn);
         }
-        free(conn);
+        if (conn->ext_alg) {
+            free(conn->ext_alg->alg);
+        }
+        conn_entry_destroy(conn);
+        delete_conn_cmn(conn);
     }
 }
 
@@ -2210,11 +2233,15 @@  static void
 delete_conn_one(struct conn *conn)
 {
     if (conn->conn_type == CT_CONN_TYPE_DEFAULT) {
-        free(conn->nat_info);
-        free(conn->alg);
+        if (conn->ext_nat) {
+            free(conn->ext_nat->nat_info);
+        }
+        if (conn->ext_alg) {
+            free(conn->ext_alg->alg);
+        }
         conn_entry_destroy(conn);
     }
-    free(conn);
+    delete_conn_cmn(conn);
 }
 
 /* Convert a conntrack address 'a' into an IP address 'b' based on 'dl_type'.
@@ -2328,9 +2355,11 @@  conn_to_ct_dpif_entry(const struct conn *conn, struct ct_dpif_entry *entry,
 
     entry->bkt = bkt;
 
-    if (conn->alg) {
+    if (conn->ext_alg && conn->ext_alg->alg) {
         /* Caller is responsible for freeing. */
-        entry->helper.name = xstrdup(conn->alg);
+        entry->helper.name = xstrdup(conn->ext_alg->alg);
+    } else {
+        entry->helper.name = NULL;
     }
 }
 
@@ -3016,7 +3045,7 @@  handle_ftp_ctl(const struct conn_lookup_ctx *ctx, struct dp_packet *pkt,
         return;
     }
 
-    if (!nat || !conn_for_expectation->seq_skew) {
+    if (!nat || !conn_for_expectation->ext_alg->seq_skew) {
         do_seq_skew_adj = false;
     }
 
@@ -3024,7 +3053,7 @@  handle_ftp_ctl(const struct conn_lookup_ctx *ctx, struct dp_packet *pkt,
     int64_t seq_skew = 0;
 
     if (ftp_ctl == CT_FTP_CTL_OTHER) {
-        seq_skew = conn_for_expectation->seq_skew;
+        seq_skew = conn_for_expectation->ext_alg->seq_skew;
     } else if (ftp_ctl == CT_FTP_CTL_INTEREST) {
         enum ftp_ctl_pkt rc;
         if (ctx->key.dl_type == htons(ETH_TYPE_IPV6)) {
@@ -3079,7 +3108,7 @@  handle_ftp_ctl(const struct conn_lookup_ctx *ctx, struct dp_packet *pkt,
     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 (ctx->reply != conn_for_expectation->ext_alg->seq_skew_dir) {
 
             uint32_t tcp_ack = ntohl(get_16aligned_be32(&th->tcp_ack));