We replace the existing rculists used for conntrack sweeping with just iterating over all known connections. This allows us to get rid of the exp_lists in struct conntrack and thereby also of the usage of ct_lock in the connection insert and deletion path.
This means after this commit connection insertion and deletion across different zones has no longer any shared lock. Within the same zone there is still a single lock. The exp_lists where not ordered in any way, the only difference compared to iterating over all connections was that they only contained connections in the forward direction. This means that this patch will trade away some cleaning performance in order to gain connection insertion and deletion speeds. We will improve the ct cleaning in the following commits. Signed-off-by: Felix Huettner <[email protected]> --- Notes: v3->v4: split to 4 patches, this is 4/4 of the previous patch 5 lib/conntrack-private.h | 11 +----- lib/conntrack.c | 84 ++++++++++++++++------------------------- 2 files changed, 33 insertions(+), 62 deletions(-) diff --git a/lib/conntrack-private.h b/lib/conntrack-private.h index c0818a2fe..3062243b7 100644 --- a/lib/conntrack-private.h +++ b/lib/conntrack-private.h @@ -116,8 +116,6 @@ enum ct_timeout { N_CT_TM }; -#define N_EXP_LISTS 100 - struct conn_key_node { enum key_dir dir; struct conn_key key; @@ -134,9 +132,6 @@ struct conn { * True as soon as a thread has started freeing * its memory. */ - /* Inserted once by a PMD, then managed by the 'ct_clean' thread. */ - struct rculist node; - /* Mutable data. */ struct ovs_mutex lock; /* Guards all mutable fields. */ ovs_u128 label; @@ -212,17 +207,13 @@ struct conntrack_zone { struct conntrack { struct ovs_mutex ct_lock; /* Protects the following fields. */ struct conntrack_zone zones[UINT16_MAX + 1]; - struct rculist exp_lists[N_EXP_LISTS]; struct cmap timeout_policies; atomic_int64_t default_zone_limit; uint32_t hash_basis; /* Salt for hashing a connection key. */ pthread_t clean_thread; /* Periodically cleans up connection tracker. */ struct latch clean_thread_exit; /* To destroy the 'clean_thread'. */ - unsigned int next_list; /* Next list where the newly created connection - * gets inserted. */ - unsigned int next_sweep; /* List from which the gc thread will resume - * the sweeping. */ + unsigned int next_clean_zone; /* Next zone where the clean should run. */ /* Counting connections. */ atomic_count n_conn; /* Number of connections currently tracked. */ diff --git a/lib/conntrack.c b/lib/conntrack.c index 1c47bea84..b1ac0d9ca 100644 --- a/lib/conntrack.c +++ b/lib/conntrack.c @@ -41,7 +41,6 @@ #include "ovs-thread.h" #include "openvswitch/poll-loop.h" #include "random.h" -#include "rculist.h" #include "timeval.h" #include "unaligned.h" @@ -106,7 +105,6 @@ static enum ct_update_res conn_update(struct conntrack *ct, struct conn *conn, long long now); static long long int conn_expiration(const struct conn *); static bool conn_expired(const struct conn *, long long now); -static void conn_expire_push_front(struct conntrack *ct, struct conn *conn); static void set_mark(struct dp_packet *, struct conn *, uint32_t val, uint32_t mask); static void set_label(struct dp_packet *, struct conn *, @@ -279,9 +277,6 @@ conntrack_init(void) cz->limit = CONN_LIMIT_NONE; cz->requested_limit = CONN_LIMIT_USE_DEFAULT; } - for (unsigned i = 0; i < ARRAY_SIZE(ct->exp_lists); i++) { - rculist_init(&ct->exp_lists[i]); - } timeout_policy_init(ct); ovs_mutex_unlock(&ct->ct_lock); @@ -441,7 +436,6 @@ zone_limit_update(struct conntrack *ct, int32_t zone, uint32_t limit) static void conn_clean__(struct conntrack *ct, struct conn *conn) - OVS_REQUIRES(ct->ct_lock) { struct conntrack_zone *cz; uint16_t fwd_zone; @@ -468,7 +462,6 @@ conn_clean__(struct conntrack *ct, struct conn *conn) &conn->key_node[CT_DIR_REV].cm_node, hash); } - rculist_remove(&conn->node); ovs_mutex_unlock(&cz->zone_lock); } @@ -476,16 +469,14 @@ conn_clean__(struct conntrack *ct, struct conn *conn) datastructures. */ static void conn_clean(struct conntrack *ct, struct conn *conn) - OVS_EXCLUDED(conn->lock, ct->ct_lock) + OVS_EXCLUDED(conn->lock) { if (atomic_flag_test_and_set(&conn->reclaimed)) { return; } COVERAGE_INC(conntrack_remove); - ovs_mutex_lock(&ct->ct_lock); conn_clean__(ct, conn); - ovs_mutex_unlock(&ct->ct_lock); ovsrcu_postpone(delete_conn, conn); atomic_count_dec(&ct->n_conn); @@ -504,18 +495,11 @@ void conntrack_destroy(struct conntrack *ct) { struct conntrack_zone *cz; - struct conn *conn; latch_set(&ct->clean_thread_exit); pthread_join(ct->clean_thread, NULL); latch_destroy(&ct->clean_thread_exit); - for (unsigned i = 0; i < N_EXP_LISTS; i++) { - RCULIST_FOR_EACH (conn, node, &ct->exp_lists[i]) { - conn_clean(ct, conn); - } - } - struct timeout_policy *tp; CMAP_FOR_EACH (tp, node, &ct->timeout_policies) { uint32_t hash = hash_int(tp->policy.id, ct->hash_basis); @@ -524,7 +508,7 @@ conntrack_destroy(struct conntrack *ct) ovsrcu_postpone(free, tp); } - ovs_mutex_lock(&ct->ct_lock); + conntrack_flush(ct, NULL); for (unsigned i = 0; i < ARRAY_SIZE(ct->zones); i++) { cz = zone_lookup(ct, i); ovs_mutex_lock(&cz->zone_lock); @@ -534,7 +518,6 @@ conntrack_destroy(struct conntrack *ct) } cmap_destroy(&ct->timeout_policies); - ovs_mutex_unlock(&ct->ct_lock); ovs_mutex_destroy(&ct->ct_lock); ovs_mutex_lock(&ct->resources_lock); @@ -919,7 +902,7 @@ conn_insert(struct conntrack *ct, struct conntrack_zone *cz, const struct nat_action_info_t *nat_action_info, const char *helper, const struct alg_exp_node *alg_exp, enum ct_alg_ctl_type ct_alg_ctl, uint32_t tp_id) - OVS_REQUIRES(ct->ct_lock, cz->zone_lock) + OVS_REQUIRES(cz->zone_lock) { struct conn_key_node *fwd_key_node, *rev_key_node; struct conn *nc = NULL; @@ -992,7 +975,6 @@ conn_insert(struct conntrack *ct, struct conntrack_zone *cz, cmap_insert(&cz->conns, &fwd_key_node->cm_node, ctx->hash); - conn_expire_push_front(ct, nc); atomic_count_inc(&ct->n_conn); atomic_count_inc(&cz->count); @@ -1053,20 +1035,17 @@ conn_maybe_not_found(struct conntrack *ct, struct dp_packet *pkt, * We do not use multiple small if's as this confused clangs locking * analysis. */ if (commit) { - ovs_mutex_lock(&ct->ct_lock); ovs_mutex_lock(&cz->zone_lock); bool found = conn_lookup_zone(ct, cz, &ctx->key, now, NULL, NULL); if (!found) { if (!pkt_validate_and_set_new_ct_state(pkt, ctx, alg_exp)) { ovs_mutex_unlock(&cz->zone_lock); - ovs_mutex_unlock(&ct->ct_lock); return nc; } nc = conn_insert(ct, cz, pkt, ctx, now, nat_action_info, helper, alg_exp, ct_alg_ctl, tp_id); } ovs_mutex_unlock(&cz->zone_lock); - ovs_mutex_unlock(&ct->ct_lock); } else { bool found = conn_lookup_zone(ct, cz, &ctx->key, now, NULL, NULL); if (!found) { @@ -1513,29 +1492,35 @@ conntrack_get_sweep_interval(struct conntrack *ct) } static size_t -ct_sweep(struct conntrack *ct, struct rculist *list, long long now, - size_t *cleaned_count) +ct_sweep_zone(struct conntrack *ct, uint16_t zone, long long now, + size_t *cleaned_count) OVS_NO_THREAD_SAFETY_ANALYSIS { + struct conn_key_node *keyn; + struct conntrack_zone *cz; + unsigned int conn_count = 0; + unsigned int cleaned = 0; struct conn *conn; - size_t cleaned = 0; - size_t count = 0; + long long expiration; - RCULIST_FOR_EACH (conn, node, list) { - if (conn_expired(conn, now)) { + cz = zone_lookup(ct, zone); + CMAP_FOR_EACH (keyn, cm_node, &cz->conns) { + if (keyn->dir != CT_DIR_FWD) { + continue; + } + + conn = CONTAINER_OF(keyn, struct conn, key_node[keyn->dir]); + expiration = conn_expiration(conn); + if (now >= expiration) { conn_clean(ct, conn); cleaned++; } - count++; + conn_count++; } - - if (cleaned_count) { - *cleaned_count = cleaned; - } - - return count; -} + *cleaned_count = cleaned; + return conn_count; + } /* Cleans up old connection entries from 'ct'. Returns the time * when the next wake will happen. The return value might be zero, @@ -1547,23 +1532,28 @@ conntrack_clean(struct conntrack *ct, long long now) unsigned int n_conn_limit, i; size_t clean_end, count = 0; size_t total_cleaned = 0; + uint16_t current_zone = ct->next_clean_zone; atomic_read_relaxed(&ct->n_conn_limit, &n_conn_limit); clean_end = n_conn_limit / 64; - for (i = ct->next_sweep; i < N_EXP_LISTS; i++) { - size_t cleaned; + for (i = 0; i < ARRAY_SIZE(ct->zones); i++) { + size_t cleaned = 0; if (count > clean_end) { next_wakeup = 0; break; } - count += ct_sweep(ct, &ct->exp_lists[i], now, &cleaned); + count += ct_sweep_zone(ct, current_zone, now, &cleaned); total_cleaned += cleaned; + + /* This will overflow and thereby allow us to iterate through all + * zones. */ + current_zone++; } - ct->next_sweep = (i < N_EXP_LISTS) ? i : 0; + ct->next_clean_zone = current_zone + 1; VLOG_DBG("conntrack cleaned %"PRIuSIZE" entries out of %"PRIuSIZE " entries in %lld msec", total_cleaned, count, @@ -2600,16 +2590,6 @@ conn_update(struct conntrack *ct, struct conn *conn, struct dp_packet *pkt, return update_res; } -static void -conn_expire_push_front(struct conntrack *ct, struct conn *conn) - OVS_REQUIRES(ct->ct_lock) -{ - unsigned int curr = ct->next_list; - - ct->next_list = (ct->next_list + 1) % N_EXP_LISTS; - rculist_push_front(&ct->exp_lists[curr], &conn->node); -} - static long long int conn_expiration(const struct conn *conn) { -- 2.43.0 _______________________________________________ dev mailing list [email protected] https://mail.openvswitch.org/mailman/listinfo/ovs-dev
