diff mbox series

[ovs-dev,v5,12/13] conntrack: Batch conn deletions in a single zone.

Message ID 88491fa0befafa7f82afde7e892ed6f340dda8a6.1788272618.git.felix.huettner@digits.schwarz
State New
Delegated to: aaron conole
Headers show
Series Multiple conntrack performance improvements. | expand

Checks

Context Check Description
ovsrobot/apply-robot success apply and check: success

Commit Message

Felix Huettner Sept. 1, 2026, 2:24 p.m. UTC 
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 <felix.huettner@digits.schwarz>
---

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 mbox series

Patch

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;