I missed this one when I reviewed V1, but looks like some comments
still applies.

fbl


On Mon, Dec 11, 2017 at 08:00:08AM -0800, Darrell Ball wrote:
> Fix up some instances where variable declarations were not close
> enough to their use, as these were missed before.  This is the
> preferred art in OVS code and flagged heavily in code reviews.
> This is highly desirable due to code clarity reasons.
> 
> There are also some cases where newlines were not needed by prior art
> and some cases where they were needed but missed.
> 
> There was one case where there was a missing space after "}".
> 
> There were a few cases where for loop index decalrations could be
> folded into the loop.
> 
> One function was missing some const qualifiers.
> 
> Signed-off-by: Darrell Ball <dlu...@gmail.com>
> ---
> 
> This patch depends on the series here:
> https://patchwork.ozlabs.org/project/openvswitch/list/?series=16626
> 
> and the folowing patch.
> 
> https://patchwork.ozlabs.org/patch/846490/
> 
>  lib/conntrack.c | 145 
> +++++++++++++++++++++++---------------------------------
>  1 file changed, 59 insertions(+), 86 deletions(-)
> 
> diff --git a/lib/conntrack.c b/lib/conntrack.c
> index be0752d..03f6fce 100644
> --- a/lib/conntrack.c
> +++ b/lib/conntrack.c
> @@ -248,8 +248,8 @@ conn_key_cmp(const struct conn_key *key1, const struct 
> conn_key *key2)
>  }
>  
>  static void
> -ct_print_conn_info(struct conn *c, char *log_msg, enum vlog_level vll,
> -                   bool force, bool rl_on)
> +ct_print_conn_info(const struct conn *c, const char *log_msg,
> +                   enum vlog_level vll, bool force, bool rl_on)
>  {
>  #define CT_VLOG(RL_ON, LEVEL, ...)                                          \
>      do {                                                                    \
> @@ -307,7 +307,6 @@ ct_print_conn_info(struct conn *c, char *log_msg, enum 
> vlog_level vll,
>  void
>  conntrack_init(struct conntrack *ct)
>  {
> -    unsigned i, j;
>      long long now = time_msec();
>  
>      ct_rwlock_init(&ct->resources_lock);
> @@ -318,13 +317,13 @@ conntrack_init(struct conntrack *ct)
>      ovs_list_init(&ct->alg_exp_list);
>      ct_rwlock_unlock(&ct->resources_lock);
>  
> -    for (i = 0; i < CONNTRACK_BUCKETS; i++) {
> +    for (unsigned i = 0; i < CONNTRACK_BUCKETS; i++) {
>          struct conntrack_bucket *ctb = &ct->buckets[i];
> -
>          ct_lock_init(&ctb->lock);
>          ct_lock_lock(&ctb->lock);
>          hmap_init(&ctb->connections);
> -        for (j = 0; j < ARRAY_SIZE(ctb->exp_lists); j++) {
> +
> +        for (unsigned j = 0; j < ARRAY_SIZE(ctb->exp_lists); j++) {
>              ovs_list_init(&ctb->exp_lists[j]);
>          }
>          ct_lock_unlock(&ctb->lock);
> @@ -344,12 +343,10 @@ conntrack_init(struct conntrack *ct)
>  void
>  conntrack_destroy(struct conntrack *ct)
>  {
> -    unsigned i;
> -
>      latch_set(&ct->clean_thread_exit);
>      pthread_join(ct->clean_thread, NULL);
>      latch_destroy(&ct->clean_thread_exit);
> -    for (i = 0; i < CONNTRACK_BUCKETS; i++) {
> +    for (unsigned i = 0; i < CONNTRACK_BUCKETS; i++) {
>          struct conntrack_bucket *ctb = &ct->buckets[i];
>          struct conn *conn;
>  
> @@ -422,7 +419,6 @@ write_ct_md(struct dp_packet *pkt, uint16_t zone, const 
> struct conn *conn,
>      pkt->md.ct_orig_tuple_ipv6 = false;
>      if (key) {
>          if (key->dl_type == htons(ETH_TYPE_IP)) {
> -
>              pkt->md.ct_orig_tuple.ipv4 = (struct ovs_key_ct_tuple_ipv4) {
>                  key->src.addr.ipv4_aligned,
>                  key->dst.addr.ipv4_aligned,
> @@ -645,7 +641,6 @@ reverse_nat_packet(struct dp_packet *pkt, const struct 
> conn *conn)
>          struct ip_header *inner_l3 = (struct ip_header *) (icmp + 1);
>          extract_l3_ipv4(&inner_key, inner_l3, tail - ((char *)inner_l3) - 
> pad,
>                          &inner_l4, false);
> -
>          pkt->l3_ofs += (char *) inner_l3 - (char *) nh;
>          pkt->l4_ofs += inner_l4 - (char *) icmp;
>  
> @@ -656,6 +651,7 @@ reverse_nat_packet(struct dp_packet *pkt, const struct 
> conn *conn)
>              packet_set_ipv4_addr(pkt, &inner_l3->ip_dst,
>                                   conn->key.dst.addr.ipv4_aligned);
>          }
> +
>          reverse_pat_packet(pkt, conn);
>          icmp->icmp_csum = 0;
>          icmp->icmp_csum = csum(icmp, tail - (char *) icmp - pad);
> @@ -770,7 +766,6 @@ nat_clean(struct conntrack *ct, struct conn *conn,
>            struct conntrack_bucket *ctb)
>      OVS_REQUIRES(ctb->lock)
>  {
> -    long long now = time_msec();
>      ct_rwlock_wrlock(&ct->resources_lock);
>      nat_conn_keys_remove(&ct->nat_conn_keys, &conn->rev_key, ct->hash_basis);
>      ct_rwlock_unlock(&ct->resources_lock);
> @@ -782,8 +777,8 @@ nat_clean(struct conntrack *ct, struct conn *conn,
>      ct_lock_lock(&ct->buckets[bucket_rev_conn].lock);
>      ct_rwlock_wrlock(&ct->resources_lock);
>  
> +    long long now = time_msec();
>      struct conn *rev_conn = conn_lookup(ct, &conn->rev_key, now);
> -
>      struct nat_conn_key_node *nat_conn_key_node =
>          nat_conn_keys_lookup(&ct->nat_conn_keys, &conn->rev_key,
>                               ct->hash_basis);
> @@ -797,8 +792,8 @@ nat_clean(struct conntrack *ct, struct conn *conn,
>                      &rev_conn->node);
>          free(rev_conn);
>      }
> -    delete_conn(conn);
>  
> +    delete_conn(conn);
>      ct_rwlock_unlock(&ct->resources_lock);
>      ct_lock_unlock(&ct->buckets[bucket_rev_conn].lock);
>      ct_lock_lock(&ctb->lock);
> @@ -848,11 +843,9 @@ conn_not_found(struct conntrack *ct, struct dp_packet 
> *pkt,
>                 struct conn_lookup_ctx *ctx, bool commit, long long now,
>                 const struct nat_action_info_t *nat_action_info,
>                 struct conn *conn_for_un_nat_copy,
> -               const char *helper,
> -               const struct alg_exp_node *alg_exp,
> +               const char *helper, const struct alg_exp_node *alg_exp,
>                 enum ct_alg_ctl_type ct_alg_ctl)
>  {
> -    unsigned bucket = hash_to_bucket(ctx->hash);
>      struct conn *nc = NULL;
>  
>      if (!valid_new(pkt, &ctx->key)) {
> @@ -874,6 +867,7 @@ conn_not_found(struct conntrack *ct, struct dp_packet 
> *pkt,
>              return nc;
>          }
>  
> +        unsigned bucket = hash_to_bucket(ctx->hash);
>          nc = new_conn(&ct->buckets[bucket], pkt, &ctx->key, now);
>          ctx->conn = nc;
>          nc->rev_key = nc->key;
> @@ -917,9 +911,9 @@ conn_not_found(struct conntrack *ct, struct dp_packet 
> *pkt,
>              } else {
>                  *conn_for_un_nat_copy = *nc;
>                  ct_rwlock_wrlock(&ct->resources_lock);
> -                bool nat_res = nat_select_range_tuple(
> -                                   ct, nc, conn_for_un_nat_copy);
>  
> +                bool nat_res = nat_select_range_tuple(ct, nc,
> +                                                      conn_for_un_nat_copy);
>                  if (!nat_res) {
>                      goto nat_res_exhaustion;
>                  }
> @@ -977,6 +971,7 @@ conn_update_state(struct conntrack *ct, struct dp_packet 
> *pkt,
>          if ((*conn)->alg_related) {
>              pkt->md.ct_state |= CS_RELATED;
>          }
> +
>          enum ct_update_res res = conn_update(*conn, &ct->buckets[bucket],
>                                               pkt, ctx->reply, now);
>  
> @@ -1032,7 +1027,6 @@ create_un_nat_conn(struct conntrack *ct, struct conn 
> *conn_for_un_nat_copy,
>              nat_conn_keys_lookup(&ct->nat_conn_keys, &nc->key, 
> ct->hash_basis);
>          if (nat_conn_key_node && !conn_key_cmp(&nat_conn_key_node->value,
>              &nc->rev_key) && !rev_conn) {
> -
>              hmap_insert(&ct->buckets[un_nat_conn_bucket].connections,
>                          &nc->node, un_nat_hash);
>          } else {
> @@ -1123,7 +1117,6 @@ check_orig_tuple(struct conntrack *ct, struct dp_packet 
> *pkt,
>  
>      ctx.key.dl_type = ctx_in->key.dl_type;
>      ctx.key.zone = pkt->md.ct_zone;
> -
>      ctx.hash = conn_key_hash(&ctx.key, ct->hash_basis);
>      *bucket = hash_to_bucket(ctx.hash);
>      ct_lock_lock(&ct->buckets[*bucket].lock);
> @@ -1190,15 +1183,12 @@ process_one(struct conntrack *ct, struct dp_packet 
> *pkt,
>          if (conn->conn_type == CT_CONN_TYPE_UN_NAT) {
>  
>              ctx->reply = true;
> -
>              struct conn_lookup_ctx ctx2;
>              ctx2.conn = NULL;
>              ctx2.key = conn->rev_key;
>              ctx2.hash = conn_key_hash(&conn->rev_key, ct->hash_basis);
> -
>              ct_lock_unlock(&ct->buckets[bucket].lock);
>              bucket = hash_to_bucket(ctx2.hash);
> -
>              ct_lock_lock(&ct->buckets[bucket].lock);
>              conn_key_lookup(&ct->buckets[bucket], &ctx2, now);
>  
> @@ -1234,10 +1224,9 @@ process_one(struct conntrack *ct, struct dp_packet 
> *pkt,
>              handle_nat(pkt, conn, zone, ctx->reply, ctx->icmp_related);
>          }
>  
> -    }else if (check_orig_tuple(ct, pkt, ctx, now, &bucket, &conn,
> +    } else if (check_orig_tuple(ct, pkt, ctx, now, &bucket, &conn,
>                                 nat_action_info)) {
> -        create_new_conn = conn_update_state(ct, pkt, ctx, &conn, now,
> -                                            bucket);
> +        create_new_conn = conn_update_state(ct, pkt, ctx, &conn, now, 
> bucket);
>      } else {
>          if (ctx->icmp_related) {
>              /* An icmp related conn should always be found; no new
> @@ -1249,6 +1238,7 @@ process_one(struct conntrack *ct, struct dp_packet *pkt,
>      }
>  
>      const struct alg_exp_node *alg_exp = NULL;
> +
>      if (OVS_UNLIKELY(create_new_conn)) {
>          struct alg_exp_node alg_exp_entry;
>  
> @@ -1369,10 +1359,9 @@ sweep_bucket(struct conntrack *ct, struct 
> conntrack_bucket *ctb,
>  {
>      struct conn *conn, *next;
>      long long min_expiration = LLONG_MAX;
> -    unsigned i;
>      size_t count = 0;
>  
> -    for (i = 0; i < N_CT_TM; i++) {
> +    for (unsigned i = 0; i < N_CT_TM; i++) {
>          LIST_FOR_EACH_SAFE (conn, next, exp_node, &ctb->exp_lists[i]) {
>              if (conn->conn_type == CT_CONN_TYPE_DEFAULT) {
>                  if (!conn_expired(conn, now) || count >= limit) {
> @@ -1402,11 +1391,10 @@ conntrack_clean(struct conntrack *ct, long long now)
>      long long next_wakeup = now + CT_TM_MIN;
>      unsigned int n_conn_limit;
>      size_t clean_count = 0;
> -    unsigned i;
>  
>      atomic_read_relaxed(&ct->n_conn_limit, &n_conn_limit);
>  
> -    for (i = 0; i < CONNTRACK_BUCKETS; i++) {
> +    for (unsigned i = 0; i < CONNTRACK_BUCKETS; i++) {
>          struct conntrack_bucket *ctb = &ct->buckets[i];
>          size_t prev_count;
>          long long min_exp;
> @@ -1504,16 +1492,14 @@ static inline bool
>  extract_l3_ipv4(struct conn_key *key, const void *data, size_t size,
>                  const char **new_data, bool validate_checksum)
>  {
> -    const struct ip_header *ip = data;
> -    size_t ip_len;
> -
>      if (new_data) {
>          if (OVS_UNLIKELY(size < IP_HEADER_LEN)) {
>              return false;
>          }
>      }
>  
> -    ip_len = IP_IHL(ip->ip_ihl_ver) * 4;
> +    const struct ip_header *ip = data;
> +    size_t ip_len = IP_IHL(ip->ip_ihl_ver) * 4;
>  
>      if (new_data) {
>          if (OVS_UNLIKELY(ip_len < IP_HEADER_LEN)) {
> @@ -1559,23 +1545,23 @@ extract_l3_ipv6(struct conn_key *key, const void 
> *data, size_t size,
>      }
>  
>      uint8_t nw_proto = ip6->ip6_nxt;
> -    uint8_t nw_frag = 0;
>  
>      data = ip6 + 1;
>      size -=  sizeof *ip6;
>  
> +    uint8_t nw_frag = 0;
>      if (!parse_ipv6_ext_hdrs(&data, &size, &nw_proto, &nw_frag)) {
>          return false;
>      }
>  
> -    if (new_data) {
> -        *new_data = data;
> -    }
> -
>      if (nw_frag) {
>          return false;
>      }
>  
> +    if (new_data) {
> +        *new_data = data;
> +    }
> +
>      key->src.addr.ipv6 = ip6->ip6_src;
>      key->dst.addr.ipv6 = ip6->ip6_dst;
>      key->nw_proto = nw_proto;
> @@ -1654,12 +1640,11 @@ check_l4_icmp6(const struct conn_key *key, const void 
> *data, size_t size,
>  static inline bool
>  extract_l4_tcp(struct conn_key *key, const void *data, size_t size)
>  {
> -    const struct tcp_header *tcp = data;
> -
>      if (OVS_UNLIKELY(size < TCP_HEADER_LEN)) {
>          return false;
>      }
>  
> +    const struct tcp_header *tcp = data;
>      key->src.port = tcp->tcp_src;
>      key->dst.port = tcp->tcp_dst;
>  
> @@ -1670,12 +1655,11 @@ extract_l4_tcp(struct conn_key *key, const void 
> *data, size_t size)
>  static inline bool
>  extract_l4_udp(struct conn_key *key, const void *data, size_t size)
>  {
> -    const struct udp_header *udp = data;
> -
>      if (OVS_UNLIKELY(size < UDP_HEADER_LEN)) {
>          return false;
>      }
>  
> +    const struct udp_header *udp = data;
>      key->src.port = udp->udp_src;
>      key->dst.port = udp->udp_dst;
>  
> @@ -1719,12 +1703,12 @@ static inline int
>  extract_l4_icmp(struct conn_key *key, const void *data, size_t size,
>                  bool *related)
>  {
> -    const struct icmp_header *icmp = data;
> -
>      if (OVS_UNLIKELY(size < ICMP_HEADER_LEN)) {
>          return false;
>      }
>  
> +    const struct icmp_header *icmp = data;
> +
>      switch (icmp->icmp_type) {
>      case ICMP4_ECHO_REQUEST:
>      case ICMP4_ECHO_REPLY:
> @@ -1751,7 +1735,6 @@ extract_l4_icmp(struct conn_key *key, const void *data, 
> size_t size,
>          const char *l3 = (const char *) (icmp + 1);
>          const char *tail = (const char *) data + size;
>          const char *l4;
> -        bool ok;
>  
>          if (!related) {
>              return false;
> @@ -1759,7 +1742,8 @@ extract_l4_icmp(struct conn_key *key, const void *data, 
> size_t size,
>  
>          memset(&inner_key, 0, sizeof inner_key);
>          inner_key.dl_type = htons(ETH_TYPE_IP);
> -        ok = extract_l3_ipv4(&inner_key, l3, tail - l3, &l4, false);
> +
> +        bool ok = extract_l3_ipv4(&inner_key, l3, tail - l3, &l4, false);
>          if (!ok) {
>              return false;
>          }
> @@ -1837,7 +1821,6 @@ extract_l4_icmp6(struct conn_key *key, const void 
> *data, size_t size,
>          const char *l3 = (const char *) icmp6 + 8;
>          const char *tail = (const char *) data + size;
>          const char *l4 = NULL;
> -        bool ok;
>  
>          if (!related) {
>              return false;
> @@ -1845,7 +1828,8 @@ extract_l4_icmp6(struct conn_key *key, const void 
> *data, size_t size,
>  
>          memset(&inner_key, 0, sizeof inner_key);
>          inner_key.dl_type = htons(ETH_TYPE_IPV6);
> -        ok = extract_l3_ipv6(&inner_key, l3, tail - l3, &l4);
> +
> +        bool ok = extract_l3_ipv6(&inner_key, l3, tail - l3, &l4);
>          if (!ok) {
>              return false;
>          }
> @@ -1914,8 +1898,6 @@ conn_key_extract(struct conntrack *ct, struct dp_packet 
> *pkt, ovs_be16 dl_type,
>      const struct eth_header *l2 = dp_packet_eth(pkt);
>      const struct ip_header *l3 = dp_packet_l3(pkt);
>      const char *l4 = dp_packet_l4(pkt);
> -    const char *tail = dp_packet_tail(pkt);
> -    bool ok;
>  
>      memset(ctx, 0, sizeof *ctx);
>  
> @@ -1957,9 +1939,11 @@ conn_key_extract(struct conntrack *ct, struct 
> dp_packet *pkt, ovs_be16 dl_type,
>       *     we use a sparse representation (miniflow).
>       *
>       */
> +    const char *tail = dp_packet_tail(pkt);
> +    bool ok;
>      ctx->key.dl_type = dl_type;
>      if (ctx->key.dl_type == htons(ETH_TYPE_IP)) {
> -        bool  hwol_bad_l3_csum = dp_packet_ip_checksum_bad(pkt);
> +        bool hwol_bad_l3_csum = dp_packet_ip_checksum_bad(pkt);
>          if (hwol_bad_l3_csum) {
>              ok = false;
>          } else {
> @@ -2010,7 +1994,6 @@ static uint32_t
>  conn_key_hash(const struct conn_key *key, uint32_t basis)
>  {
>      uint32_t hsrc, hdst, hash;
> -
>      hsrc = hdst = basis;
>      hsrc = ct_endpoint_hash_add(hsrc, &key->src);
>      hdst = ct_endpoint_hash_add(hdst, &key->dst);
> @@ -2029,9 +2012,7 @@ conn_key_hash(const struct conn_key *key, uint32_t 
> basis)
>  static void
>  conn_key_reverse(struct conn_key *key)
>  {
> -    struct ct_endpoint tmp;
> -
> -    tmp = key->src;
> +    struct ct_endpoint tmp = key->src;
>      key->src = key->dst;
>      key->dst = tmp;
>  }
> @@ -2107,16 +2088,13 @@ static uint32_t
>  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 = hash_add(hash,
>                      (conn->nat_info->max_port << 16)
>                      | conn->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);
>      hash = hash_add(hash, conn->key.nw_proto);
>      hash = hash_add(hash, conn->key.zone);
> @@ -2137,7 +2115,6 @@ nat_select_range_tuple(struct conntrack *ct, const 
> struct conn *conn,
>      uint16_t min_port;
>      uint16_t max_port;
>      uint16_t first_port;
> -
>      uint32_t hash = nat_range_hash(conn, ct->hash_basis);
>  
>      if ((conn->nat_info->nat_action & NAT_ACTION_SRC) &&
> @@ -2362,14 +2339,10 @@ static struct conn *
>  new_conn(struct conntrack_bucket *ctb, struct dp_packet *pkt,
>           struct conn_key *key, long long now)
>  {
> -    struct conn *newconn;
> -
> -    newconn = l4_protos[key->nw_proto]->new_conn(ctb, pkt, now);
> -
> +    struct conn *newconn = l4_protos[key->nw_proto]->new_conn(ctb, pkt, now);
>      if (newconn) {
>          newconn->key = *key;
>      }
> -
>      return newconn;
>  }
>  
> @@ -2421,24 +2394,20 @@ static void
>  conn_to_ct_dpif_entry(const struct conn *conn, struct ct_dpif_entry *entry,
>                        long long now, int bkt)
>  {
> -    struct ct_l4_proto *class;
> -    long long expiration;
>      memset(entry, 0, sizeof *entry);
>      conn_key_to_tuple(&conn->key, &entry->tuple_orig);
>      conn_key_to_tuple(&conn->rev_key, &entry->tuple_reply);
> -
>      entry->zone = conn->key.zone;
>      entry->mark = conn->mark;
> -
>      memcpy(&entry->labels, &conn->label, sizeof entry->labels);
>      /* Not implemented yet */
>      entry->timestamp.start = 0;
>      entry->timestamp.stop = 0;
>  
> -    expiration = conn->expiration - now;
> +    long long expiration = conn->expiration - now;
>      entry->timeout = (expiration > 0) ? expiration / 1000 : 0;
>  
> -    class = l4_protos[conn->key.nw_proto];
> +    struct ct_l4_proto *class = l4_protos[conn->key.nw_proto];
>      if (class->conn_get_protoinfo) {
>          class->conn_get_protoinfo(conn, &entry->protoinfo);
>      }
> @@ -2456,12 +2425,13 @@ conntrack_dump_start(struct conntrack *ct, struct 
> conntrack_dump *dump,
>                       const uint16_t *pzone, int *ptot_bkts)
>  {
>      memset(dump, 0, sizeof(*dump));
> +
>      if (pzone) {
>          dump->zone = *pzone;
>          dump->filter_zone = true;
>      }
> -    dump->ct = ct;
>  
> +    dump->ct = ct;
>      *ptot_bkts = CONNTRACK_BUCKETS;
>  
>      return 0;
> @@ -2515,9 +2485,7 @@ conntrack_dump_done(struct conntrack_dump *dump 
> OVS_UNUSED)
>  int
>  conntrack_flush(struct conntrack *ct, const uint16_t *zone)
>  {
> -    unsigned i;
> -
> -    for (i = 0; i < CONNTRACK_BUCKETS; i++) {
> +    for (unsigned i = 0; i < CONNTRACK_BUCKETS; i++) {
>          struct conn *conn, *next;
>  
>          ct_lock_lock(&ct->buckets[i].lock);
> @@ -2541,8 +2509,8 @@ expectation_lookup(struct hmap *alg_expectations,
>      struct conn_key check_key = *key;
>      check_key.src.port = ALG_WC_SRC_PORT;
>      struct alg_exp_node *alg_exp_node;
> -
>      uint32_t alg_exp_conn_key_hash = conn_key_hash(&check_key, basis);
> +
>      HMAP_FOR_EACH_WITH_HASH (alg_exp_node, node,
>                               alg_exp_conn_key_hash,
>                               alg_expectations) {
> @@ -2722,8 +2690,8 @@ expectation_create(struct conntrack *ct,
>      }
>  
>      alg_exp_node->alg_nat_repl_addr = alg_nat_repl_addr;
> -    uint32_t alg_exp_conn_key_hash =
> -        conn_key_hash(&alg_exp_node->key, ct->hash_basis);
> +    uint32_t alg_exp_conn_key_hash = conn_key_hash(&alg_exp_node->key,
> +                                                   ct->hash_basis);
>      hmap_insert(&ct->alg_expectations, &alg_exp_node->node,
>                  alg_exp_conn_key_hash);
>      expectation_ref_create(&ct->alg_expectation_refs, &master_conn->key,
> @@ -2768,7 +2736,6 @@ repl_ftp_v4_addr(struct dp_packet *pkt, ovs_be32 
> v4_addr_rep,
>  
>      size_t remain_size = tcp_payload_length(pkt) -
>                               addr_offset_from_ftp_data_start;
> -
>      int overall_delta = 0;
>      char *byte_str = ftp_data_start + addr_offset_from_ftp_data_start;
>  
> @@ -2837,7 +2804,6 @@ static enum ftp_ctl_pkt
>  detect_ftp_ctl_type(const struct conn_lookup_ctx *ctx,
>                      struct dp_packet *pkt)
>  {
> -
>      char ftp_msg[LARGEST_FTP_MSG_OF_INTEREST + 1] = {0};
>      get_ftp_ctl_msg(pkt, ftp_msg);
>      if (ctx->key.dl_type == htons(ETH_TYPE_IPV6)) {
> @@ -2870,9 +2836,9 @@ process_ftp_ctl_v4(struct conntrack *ct,
>      *ftp_data_v4_start = tcp_hdr + tcp_hdr_len;
>      char ftp_msg[LARGEST_FTP_MSG_OF_INTEREST + 1] = {0};
>      get_ftp_ctl_msg(pkt, ftp_msg);
> -
>      char *ftp = ftp_msg;
>      enum ct_alg_mode mode;
> +
>      if (!strncasecmp(ftp, FTP_PORT_CMD, strlen(FTP_PORT_CMD))) {
>          ftp = ftp_msg + strlen(FTP_PORT_CMD);
>          mode = CT_FTP_MODE_ACTIVE;
> @@ -2895,8 +2861,8 @@ process_ftp_ctl_v4(struct conntrack *ct,
>  
>      char *ip_addr_start = ftp;
>      *addr_offset_from_ftp_data_start = ip_addr_start - ftp_msg;
> -    uint8_t comma_count = 0;
>  
> +    uint8_t comma_count = 0;
>      while (comma_count < 4 && *ftp) {
>          if (*ftp == ',') {
>              comma_count++;
> @@ -2960,6 +2926,7 @@ process_ftp_ctl_v4(struct conntrack *ct,
>      if (65535 - port_hs < port_lo_hs) {
>          return CT_FTP_CTL_INVALID;
>      }
> +
>      port_hs |= port_lo_hs;
>      ovs_be16 port = htons(port_hs);
>      ovs_be32 conn_ipv4_addr;
> @@ -3027,8 +2994,8 @@ process_ftp_ctl_v6(struct conntrack *ct,
>          /* Jump over delimiter. */
>          ftp += 2;
>  
> -        char *ip_addr_start = ftp;
>          memset(&ip6_addr, 0, sizeof ip6_addr);
> +        char *ip_addr_start = ftp;
>          *addr_offset_from_ftp_data_start = ip_addr_start - ftp_msg;
>          ftp = skip_ipv6_digits(ftp);
>          *ftp = 0;
> @@ -3058,10 +3025,12 @@ process_ftp_ctl_v6(struct conntrack *ct,
>      if (!ftp) {
>          return CT_FTP_CTL_INVALID;
>      }
> +
>      int value;
>      if (!str_to_int(save_ftp, 10, &value)) {
>          return CT_FTP_CTL_INVALID;
>      }
> +
>      if (value > CT_MAX_L4_PORT) {
>          return CT_FTP_CTL_INVALID;
>      }
> @@ -3161,6 +3130,7 @@ handle_ftp_ctl(struct conntrack *ct, const struct 
> conn_lookup_ctx *ctx,
>  
>      struct ovs_16aligned_ip6_hdr *nh6 = dp_packet_l3(pkt);
>      int64_t seq_skew = 0;
> +
>      if (ftp_ctl == CT_FTP_CTL_OTHER) {
>          seq_skew = conn_for_expectation->seq_skew;
>      } else if (ftp_ctl == CT_FTP_CTL_INTEREST) {
> @@ -3182,6 +3152,7 @@ handle_ftp_ctl(struct conntrack *ct, const struct 
> conn_lookup_ctx *ctx,
>              return;
>          } else if (rc == CT_FTP_CTL_INTEREST) {
>              uint16_t ip_len;
> +
>              if (ctx->key.dl_type == htons(ETH_TYPE_IPV6)) {
>                  seq_skew = repl_ftp_v6_addr(pkt, v6_addr_rep, ftp_data_start,
>                                              addr_offset_from_ftp_data_start,
> @@ -3214,6 +3185,7 @@ handle_ftp_ctl(struct conntrack *ct, const struct 
> conn_lookup_ctx *ctx,
>      }
>  
>      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) {
>  
> @@ -3244,8 +3216,6 @@ handle_ftp_ctl(struct conntrack *ct, const struct 
> conn_lookup_ctx *ctx,
>          }
>      }
>  
> -    const char *tail = dp_packet_tail(pkt);
> -    uint8_t pad = dp_packet_l2_pad_size(pkt);
>      th->tcp_csum = 0;
>      uint32_t tcp_csum;
>      if (ctx->key.dl_type == htons(ETH_TYPE_IPV6)) {
> @@ -3253,6 +3223,9 @@ handle_ftp_ctl(struct conntrack *ct, const struct 
> conn_lookup_ctx *ctx,
>      } else {
>          tcp_csum = packet_csum_pseudoheader(l3_hdr);
>      }
> +
> +    const char *tail = dp_packet_tail(pkt);
> +    uint8_t pad = dp_packet_l2_pad_size(pkt);
>      th->tcp_csum = csum_finish(
>          csum_continue(tcp_csum, th, tail - (char *) th - pad));
>      return;
> -- 
> 1.9.1
> 
> _______________________________________________
> dev mailing list
> d...@openvswitch.org
> https://mail.openvswitch.org/mailman/listinfo/ovs-dev

-- 
Flavio

_______________________________________________
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Reply via email to