If a lot of conn entries expired in a single zone we previously took the zone_lock for each conn entry we deleted. This caused a lot of churn on the lock, especially if there are parallel inserts.
We fix this by batching the deletions on 100 entries and taking a lock for all of them at once. Signed-off-by: Felix Huettner <[email protected]> --- Notes: v4->v5: Fix missing cleaning if we reach the limit of entries to clean lib/conntrack.c | 72 +++++++++++++++++++++++++++++++++++++++---------- 1 file changed, 58 insertions(+), 14 deletions(-) diff --git a/lib/conntrack.c b/lib/conntrack.c index ea1a0f614..0c91d5dc1 100644 --- a/lib/conntrack.c +++ b/lib/conntrack.c @@ -438,34 +438,34 @@ zone_limit_update(struct conntrack *ct, int32_t zone, uint32_t limit) } static void -conn_clean__(struct conntrack *ct, struct conn *conn) +conn_clean_protected__(struct conntrack *ct, struct conntrack_zone *cz, + struct conn *conn) + OVS_REQUIRES(cz->zone_lock) { - struct conntrack_zone *cz; - uint16_t fwd_zone; uint32_t hash; + COVERAGE_INC(conntrack_remove); + if (conn->alg) { expectation_clean(ct, &conn->key_node[CT_DIR_FWD].key); } - fwd_zone = conn->key_node[CT_DIR_FWD].key.zone; - cz = zone_lookup(ct, fwd_zone); - hash = conn_key_hash(&conn->key_node[CT_DIR_FWD].key, ct->hash_basis); - ovs_mutex_lock(&cz->zone_lock); cmap_remove(&cz->conns, &conn->key_node[CT_DIR_FWD].cm_node, hash); atomic_count_dec(&cz->count); if (conn->nat_action) { - ovs_assert(fwd_zone == conn->key_node[CT_DIR_REV].key.zone); + ovs_assert(conn->key_node[CT_DIR_FWD].key.zone == + conn->key_node[CT_DIR_REV].key.zone); hash = conn_key_hash(&conn->key_node[CT_DIR_REV].key, ct->hash_basis); cmap_remove(&cz->conns, &conn->key_node[CT_DIR_REV].cm_node, hash); } - ovs_mutex_unlock(&cz->zone_lock); + ovsrcu_postpone(delete_conn, conn); + atomic_count_dec(&ct->n_conn); } /* Also removes the associated nat 'conn' from the lookup @@ -478,11 +478,27 @@ conn_clean(struct conntrack *ct, struct conn *conn) return; } - COVERAGE_INC(conntrack_remove); - conn_clean__(ct, conn); + struct conntrack_zone *cz = zone_lookup(ct, + conn->key_node[CT_DIR_FWD].key.zone); - ovsrcu_postpone(delete_conn, conn); - atomic_count_dec(&ct->n_conn); + ovs_mutex_lock(&cz->zone_lock); + conn_clean_protected__(ct, cz, conn); + ovs_mutex_unlock(&cz->zone_lock); +} + +/* Also removes the associated nat 'conn' from the lookup + datastructures. */ +static void +conn_clean_protected(struct conntrack *ct, struct conntrack_zone *cz, + struct conn *conn) + OVS_EXCLUDED(conn->lock) + OVS_REQUIRES(cz->zone_lock) +{ + if (atomic_flag_test_and_set(&conn->reclaimed)) { + return; + } + + conn_clean_protected__(ct, cz, conn); } static void @@ -1498,6 +1514,24 @@ conntrack_get_sweep_interval(struct conntrack *ct) return ms; } +#define CONN_SWEEP_BUF_SIZE 100 + +static void +ct_sweep_buf(struct conntrack *ct, uint16_t zone, + struct conn *conn_buf[CONN_SWEEP_BUF_SIZE], + unsigned int n_conn_buf) +{ + if (n_conn_buf == 0) { + return; + } + struct conntrack_zone *cz = zone_lookup(ct, zone); + ovs_mutex_lock(&cz->zone_lock); + for (unsigned i = 0; i < n_conn_buf; i++) { + conn_clean_protected(ct, cz, conn_buf[i]); + } + ovs_mutex_unlock(&cz->zone_lock); +} + static bool ct_sweep_zone(struct conntrack *ct, uint16_t zone, long long now, size_t *cleaned_count, size_t *conn_count, size_t limit, @@ -1511,6 +1545,9 @@ ct_sweep_zone(struct conntrack *ct, uint16_t zone, long long now, struct conn *conn; long long expiration; + struct conn *conn_buf[CONN_SWEEP_BUF_SIZE]; + unsigned int n_conn_buf = 0; + cz = zone_lookup(ct, zone); if (atomic_count_get(&cz->count) == 0) { *conn_count = 0; @@ -1533,6 +1570,7 @@ ct_sweep_zone(struct conntrack *ct, uint16_t zone, long long now, CMAP_CURSOR_FOR_EACH_CONTINUE (keyn, cm_node, &cursor) { if (conn_handled > limit) { + ct_sweep_buf(ct, zone, conn_buf, n_conn_buf); *current_position = xzalloc(sizeof(**current_position)); cmap_cursor_to_position(&cursor, *current_position); *conn_count = conn_handled; @@ -1546,12 +1584,18 @@ ct_sweep_zone(struct conntrack *ct, uint16_t zone, long long now, conn = CONTAINER_OF(keyn, struct conn, key_node[keyn->dir]); expiration = conn_expiration(conn); if (now >= expiration) { - conn_clean(ct, conn); + conn_buf[n_conn_buf++] = conn; (*cleaned_count)++; } + if (n_conn_buf == CONN_SWEEP_BUF_SIZE) { + ct_sweep_buf(ct, zone, conn_buf, n_conn_buf); + n_conn_buf = 0; + } + conn_handled++; } + ct_sweep_buf(ct, zone, conn_buf, n_conn_buf); *conn_count = conn_handled; return true; -- 2.43.0 _______________________________________________ dev mailing list [email protected] https://mail.openvswitch.org/mailman/listinfo/ovs-dev
