[ovs-dev] [patch v4 3/4] conntrack: Some style improvements.

Darrell Ball dlu998 at gmail.com
Tue Jan 9 23:44:56 UTC 2018


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 declarations could be
folded into the loop.

One function was missing some const qualifiers.

There were a few instances where a local variable for conn_key_hash
could be eliminated.

Signed-off-by: Darrell Ball <dlu998 at gmail.com>
---

v3->v4: Eliminate some instances of local variables used for
        conn_key_hash.

v2->v3: Incorporate review comments from Flavio.
        A separate following patch is split out for reordering
        of ip fragmentation checks.

v1->v2: Add a few more improvements.


 lib/conntrack.c | 171 +++++++++++++++++++++++---------------------------------
 1 file changed, 70 insertions(+), 101 deletions(-)

diff --git a/lib/conntrack.c b/lib/conntrack.c
index 99e8118..3a7667f 100644
--- a/lib/conntrack.c
+++ b/lib/conntrack.c
@@ -251,8 +251,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 {                                                                    \
@@ -310,7 +310,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);
@@ -321,13 +320,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);
@@ -347,12 +346,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;
 
@@ -417,10 +414,11 @@ write_ct_md(struct dp_packet *pkt, uint16_t zone, const struct conn *conn,
         pkt->md.ct_label = alg_exp->master_label;
         key = &alg_exp->master_key;
     }
+
     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,
@@ -652,7 +650,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;
 
@@ -663,6 +660,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);
@@ -761,8 +759,7 @@ static void
 conn_seq_skew_set(struct conntrack *ct, const struct conn_key *key,
                   long long now, int seq_skew, bool seq_skew_dir)
 {
-    uint32_t hash = conn_key_hash(key, ct->hash_basis);
-    unsigned bucket = hash_to_bucket(hash);
+    unsigned bucket = hash_to_bucket(conn_key_hash(key, ct->hash_basis));
     ct_lock_lock(&ct->buckets[bucket].lock);
     struct conn *conn = conn_lookup(ct, key, now);
     if (conn && seq_skew) {
@@ -777,20 +774,16 @@ 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);
     ct_lock_unlock(&ctb->lock);
-
-    uint32_t hash_rev_conn = conn_key_hash(&conn->rev_key, ct->hash_basis);
-    unsigned bucket_rev_conn = hash_to_bucket(hash_rev_conn);
-
+    unsigned bucket_rev_conn =
+        hash_to_bucket(conn_key_hash(&conn->rev_key, ct->hash_basis));
     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);
@@ -804,8 +797,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);
@@ -859,21 +852,21 @@ conn_not_found(struct conntrack *ct, struct dp_packet *pkt,
                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)) {
         pkt->md.ct_state = CS_INVALID;
         return nc;
     }
+
     pkt->md.ct_state = CS_NEW;
+
     if (alg_exp) {
         pkt->md.ct_state |= CS_RELATED;
     }
 
     if (commit) {
         unsigned int n_conn_limit;
-
         atomic_read_relaxed(&ct->n_conn_limit, &n_conn_limit);
 
         if (atomic_count_get(&ct->n_conn) >= n_conn_limit) {
@@ -881,6 +874,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;
@@ -924,8 +918,8 @@ 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;
@@ -984,6 +978,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);
 
@@ -1039,7 +1034,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 {
@@ -1130,13 +1124,11 @@ 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);
     conn_key_lookup(&ct->buckets[*bucket], &ctx, now);
     *conn = ctx.conn;
-
     return *conn ? true : false;
 }
 
@@ -1241,10 +1233,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
@@ -1256,6 +1247,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;
 
@@ -1377,10 +1369,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) {
@@ -1410,11 +1401,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;
@@ -1485,7 +1475,6 @@ clean_thread_main(void *f_)
     while (!latch_is_set(&ct->clean_thread_exit)) {
         long long next_wake;
         long long now = time_msec();
-
         next_wake = conntrack_clean(ct, now);
 
         if (next_wake < now) {
@@ -1512,16 +1501,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)) {
@@ -1566,11 +1553,10 @@ 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_proto = ip6->ip6_nxt;
+    uint8_t nw_frag = 0;
 
     if (!parse_ipv6_ext_hdrs(&data, &size, &nw_proto, &nw_frag)) {
         return false;
@@ -1662,12 +1648,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;
 
@@ -1678,12 +1663,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;
 
@@ -1727,12 +1711,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:
@@ -1759,7 +1743,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;
@@ -1767,7 +1750,7 @@ 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;
         }
@@ -1845,7 +1828,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;
@@ -1853,7 +1835,7 @@ 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;
         }
@@ -1922,8 +1904,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);
 
@@ -1965,13 +1945,16 @@ 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 {
-            bool  hwol_good_l3_csum = dp_packet_ip_checksum_valid(pkt);
+            bool hwol_good_l3_csum = dp_packet_ip_checksum_valid(pkt);
             /* Validate the checksum only when hwol is not supported. */
             ok = extract_l3_ipv4(&ctx->key, l3, tail - (char *) l3, NULL,
                                  !hwol_good_l3_csum);
@@ -1982,7 +1965,6 @@ conn_key_extract(struct conntrack *ct, struct dp_packet *pkt, ovs_be16 dl_type,
         ok = false;
     }
 
-
     if (ok) {
         bool hwol_bad_l4_csum = dp_packet_l4_checksum_bad(pkt);
         if (!hwol_bad_l4_csum) {
@@ -2018,7 +2000,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);
@@ -2037,9 +2018,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;
 }
@@ -2064,6 +2043,7 @@ nat_ipv6_addrs_delta(struct in6_addr *ipv6_aligned_min,
     memcpy(&addr6_64_max_lo, ipv6_max_lo, sizeof addr6_64_max_lo);
 
     uint64_t diff;
+
     if (addr6_64_min_hi == addr6_64_max_hi &&
         ntohll(addr6_64_min_lo) <= ntohll(addr6_64_max_lo)) {
         diff = ntohll(addr6_64_max_lo) - ntohll(addr6_64_min_lo);
@@ -2077,6 +2057,7 @@ nat_ipv6_addrs_delta(struct in6_addr *ipv6_aligned_min,
          * support check, however the practical impact is probably nil. */
         diff = 0xfffffffe;
     }
+
     if (diff > 0xfffffffe) {
         diff = 0xfffffffe;
     }
@@ -2121,10 +2102,8 @@ nat_range_hash(const struct conn *conn, uint32_t basis)
     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);
@@ -2145,7 +2124,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) &&
@@ -2189,7 +2167,6 @@ nat_select_range_tuple(struct conntrack *ct, const struct conn *conn,
          * enforcement via max_ct_addr. */
         max_ct_addr = conn->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;
         nat_ipv6_addr_increment(&ct_addr.ipv6_aligned, address_index);
@@ -2267,10 +2244,9 @@ nat_conn_keys_lookup(struct hmap *nat_conn_keys,
                      uint32_t basis)
 {
     struct nat_conn_key_node *nat_conn_key_node;
-    uint32_t nat_conn_key_hash = conn_key_hash(key, basis);
 
-    HMAP_FOR_EACH_WITH_HASH (nat_conn_key_node, node, nat_conn_key_hash,
-                             nat_conn_keys) {
+    HMAP_FOR_EACH_WITH_HASH (nat_conn_key_node, node,
+                             conn_key_hash(key, basis), nat_conn_keys) {
         if (!conn_key_cmp(&nat_conn_key_node->key, key)) {
             return nat_conn_key_node;
         }
@@ -2290,9 +2266,8 @@ nat_conn_keys_insert(struct hmap *nat_conn_keys, const struct conn *nat_conn,
         struct nat_conn_key_node *nat_conn_key = xzalloc(sizeof *nat_conn_key);
         nat_conn_key->key = nat_conn->rev_key;
         nat_conn_key->value = nat_conn->key;
-        uint32_t nat_conn_key_hash = conn_key_hash(&nat_conn_key->key,
-                                                   basis);
-        hmap_insert(nat_conn_keys, &nat_conn_key->node, nat_conn_key_hash);
+        hmap_insert(nat_conn_keys, &nat_conn_key->node,
+                    conn_key_hash(&nat_conn_key->key, basis));
         return true;
     }
     return false;
@@ -2305,10 +2280,9 @@ nat_conn_keys_remove(struct hmap *nat_conn_keys,
                      uint32_t basis)
 {
     struct nat_conn_key_node *nat_conn_key_node;
-    uint32_t nat_conn_key_hash = conn_key_hash(key, basis);
 
-    HMAP_FOR_EACH_WITH_HASH (nat_conn_key_node, node, nat_conn_key_hash,
-                             nat_conn_keys) {
+    HMAP_FOR_EACH_WITH_HASH (nat_conn_key_node, node,
+                             conn_key_hash(key, basis), nat_conn_keys) {
         if (!conn_key_cmp(&nat_conn_key_node->key, key)) {
             hmap_remove(nat_conn_keys, &nat_conn_key_node->node);
             free(nat_conn_key_node);
@@ -2370,10 +2344,7 @@ 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;
     }
@@ -2429,8 +2400,6 @@ 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);
@@ -2443,10 +2412,10 @@ conn_to_ct_dpif_entry(const struct conn *conn, struct ct_dpif_entry *entry,
     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);
     }
@@ -2464,14 +2433,14 @@ 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;
 }
 
@@ -2523,9 +2492,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);
@@ -2548,14 +2515,15 @@ expectation_lookup(struct hmap *alg_expectations, const struct conn_key *key,
 {
     struct conn_key check_key = *key;
     check_key.src.port = ALG_WC_SRC_PORT;
+
     if (src_ip_wc) {
         memset(&check_key.src.addr, 0, sizeof check_key.src.addr);
     }
+
     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,
+                             conn_key_hash(&check_key, basis),
                              alg_expectations) {
         if (!conn_key_cmp(&alg_exp_node->key, &check_key)) {
             return alg_exp_node;
@@ -2690,10 +2658,8 @@ expectation_create(struct conntrack *ct, ovs_be16 dst_port,
     }
 
     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);
     hmap_insert(&ct->alg_expectations, &alg_exp_node->node,
-                alg_exp_conn_key_hash);
+                conn_key_hash(&alg_exp_node->key, ct->hash_basis));
     expectation_ref_create(&ct->alg_expectation_refs, alg_exp_node,
                            ct->hash_basis);
     ct_rwlock_unlock(&ct->resources_lock);
@@ -2736,7 +2702,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;
 
@@ -2805,9 +2770,9 @@ 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)) {
         if (strncasecmp(ftp_msg, FTP_EPRT_CMD, strlen(FTP_EPRT_CMD)) &&
             !strcasestr(ftp_msg, FTP_EPSV_REPLY)) {
@@ -2838,9 +2803,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;
@@ -2863,8 +2828,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++;
@@ -2928,6 +2893,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;
@@ -2981,12 +2947,11 @@ process_ftp_ctl_v6(struct conntrack *ct,
     size_t tcp_hdr_len = TCP_OFFSET(th->tcp_ctl) * 4;
     char *tcp_hdr = (char *) th;
     char ftp_msg[LARGEST_FTP_MSG_OF_INTEREST + 1] = {0};
-
     get_ftp_ctl_msg(pkt, ftp_msg);
     *ftp_data_start = tcp_hdr + tcp_hdr_len;
-
     char *ftp = ftp_msg;
     struct in6_addr ip6_addr;
+
     if (!strncasecmp(ftp, FTP_EPRT_CMD, strlen(FTP_EPRT_CMD))) {
         ftp = ftp_msg + strlen(FTP_EPRT_CMD);
         ftp = skip_non_digits(ftp);
@@ -2996,8 +2961,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;
@@ -3027,6 +2992,7 @@ 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;
@@ -3131,6 +3097,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) {
@@ -3152,6 +3119,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,
@@ -3184,6 +3152,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) {
 
@@ -3214,8 +3183,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)) {
@@ -3223,6 +3190,8 @@ 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



More information about the dev mailing list