If a single zone creates a lot of churn we would have otherwise blocked the clean thread for quite a while.
The approach with cmap_position might have the drawback that we potentially skip some connections that have been inserted in the meantime, but cmap_cursor can not be used as we have a rcu quiesce period in between iterations. Signed-off-by: Felix Huettner <[email protected]> --- Notes: v4->v5: fix conntrack_clean jumping to next zone if the current one was only partially cleaned lib/conntrack-private.h | 11 +++++-- lib/conntrack.c | 64 +++++++++++++++++++++++++++++------------ 2 files changed, 53 insertions(+), 22 deletions(-) diff --git a/lib/conntrack-private.h b/lib/conntrack-private.h index 213b23fce..1a28ea680 100644 --- a/lib/conntrack-private.h +++ b/lib/conntrack-private.h @@ -211,9 +211,13 @@ struct conntrack { atomic_int64_t default_zone_limit; uint32_t hash_basis; /* Salt for hashing a connection key. */ + + /* Background cleanup thread. */ pthread_t clean_thread; /* Periodically cleans up connection tracker. */ struct latch clean_thread_exit; /* To destroy the 'clean_thread'. */ - unsigned int next_clean_zone; /* Next zone where the clean should run. */ + uint16_t current_clean_zone; /* Current zone where the clean should run. */ + /* The position in the cmap of the current zone. */ + struct cmap_position *current_clean_position; /* Counting connections. */ atomic_count n_conn; /* Number of connections currently tracked. */ @@ -235,8 +239,9 @@ struct conntrack { /* Lock acquisition order: * 1. 'conn->lock' - * 2. 'ct_lock' - * 3. 'resources_lock' + * 2. 'zone_lock' + * 3. 'ct_lock' + * 4. 'resources_lock' */ extern struct ct_l4_proto ct_proto_tcp; diff --git a/lib/conntrack.c b/lib/conntrack.c index 3e38e46ec..f2adb82a3 100644 --- a/lib/conntrack.c +++ b/lib/conntrack.c @@ -529,6 +529,10 @@ conntrack_destroy(struct conntrack *ct) ovs_mutex_unlock(&ct->resources_lock); ovs_mutex_destroy(&ct->resources_lock); + if (ct->current_clean_position) { + free(ct->current_clean_position); + } + ipf_destroy(ct->ipf); free(ct); } @@ -1493,20 +1497,36 @@ conntrack_get_sweep_interval(struct conntrack *ct) return ms; } -static size_t +static bool ct_sweep_zone(struct conntrack *ct, uint16_t zone, long long now, - size_t *cleaned_count) + size_t *cleaned_count, size_t *conn_count, size_t limit, + struct cmap_position **current_position) OVS_NO_THREAD_SAFETY_ANALYSIS { struct conn_key_node *keyn; struct conntrack_zone *cz; - unsigned int conn_count = 0; - unsigned int cleaned = 0; + unsigned int conn_handled = 0; struct conn *conn; + struct cmap_node *node; long long expiration; cz = zone_lookup(ct, zone); - CMAP_FOR_EACH (keyn, cm_node, &cz->conns) { + if (atomic_count_get(&cz->count) == 0) { + *conn_count = 0; + return true; + } + + if (!*current_position) { + *current_position = xzalloc(sizeof(**current_position)); + } + + while ((node = cmap_next_position(&cz->conns, *current_position))) { + keyn = OBJECT_CONTAINING(node, keyn, cm_node); + if (conn_handled > limit) { + *conn_count = conn_handled; + return false; + } + if (keyn->dir != CT_DIR_FWD) { continue; } @@ -1515,13 +1535,16 @@ ct_sweep_zone(struct conntrack *ct, uint16_t zone, long long now, expiration = conn_expiration(conn); if (now >= expiration) { conn_clean(ct, conn); - cleaned++; + (*cleaned_count)++; } - conn_count++; + conn_handled++; } - *cleaned_count = cleaned; - return conn_count; + + free(*current_position); + *current_position = NULL; + *conn_count = conn_handled; + return true; } /* Cleans up old connection entries from 'ct'. Returns the time @@ -1534,29 +1557,32 @@ 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; + bool zone_finished; atomic_read_relaxed(&ct->n_conn_limit, &n_conn_limit); clean_end = n_conn_limit / 64; for (i = 0; i < ARRAY_SIZE(ct->zones); i++) { + size_t zone_count = 0; size_t cleaned = 0; + zone_finished = ct_sweep_zone(ct, ct->current_clean_zone, now, &cleaned, + &zone_count, clean_end - count, + &ct->current_clean_position); + total_cleaned += cleaned; + + if (zone_finished) { + /* This will overflow and thereby allow us to iterate through all + * zones. */ + ct->current_clean_zone++; + } + if (count > clean_end) { next_wakeup = 0; break; } - - 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_clean_zone = current_zone + 1; - VLOG_DBG("conntrack cleaned %"PRIuSIZE" entries out of %"PRIuSIZE " entries in %lld msec", total_cleaned, count, time_msec() - now); -- 2.43.0 _______________________________________________ dev mailing list [email protected] https://mail.openvswitch.org/mailman/listinfo/ovs-dev
