]> git.ipfire.org Git - thirdparty/haproxy.git/commitdiff
BUG/MINOR: quic: fix race on quic_conns list during affinity rebind
authorAmaury Denoyelle <adenoyelle@haproxy.com>
Wed, 26 Apr 2023 14:12:12 +0000 (16:12 +0200)
committerAmaury Denoyelle <adenoyelle@haproxy.com>
Wed, 26 Apr 2023 15:50:22 +0000 (17:50 +0200)
Each quic_conn are attached in a global thread-local quic_conns list
used for "show quic" command. During thread rebinding, a connection is
detached from its local list instance and moved to its new thread list.
However this operation is not thread-safe and may cause a race
condition.

To fix this, only remove the connection from its list inside
qc_set_tid_affinity(). The connection is inserted only after in
qc_finalize_affinity_rebind() on the new thread instance thus prevented
a race condition. One impact of this is that a connection will be
invisible during rebinding for "show quic".

A connection must not transition to closing state in between this two
steps or else cleanup via quic_handle_stopping() may not miss it. To
ensure this, this patch relies on the previous commit :
  commit d6646dddccb1aae08f60717b5b6743c513c37299
  MINOR: quic: finalize affinity change as soon as possible

This should be backported up to 2.7.

src/quic_conn.c

index 254ce58d581521b9e9da79d4a83d657ee387510b..33ac15170b83c5646edc0246fe87aed27c0291d5 100644 (file)
@@ -8524,12 +8524,10 @@ int qc_set_tid_affinity(struct quic_conn *qc, uint new_tid, struct listener *new
                fd_migrate_on(qc->fd, new_tid);
        }
 
-       /* Remove conn from per-thread list instance. */
+       /* Remove conn from per-thread list instance. It will be hidden from
+        * "show quic" until rebinding is completed.
+        */
        qc_detach_th_ctx_list(qc, 0);
-       /* Connection must not be closing or else it must be inserted in quic_conns_clo list instance instead. */
-       BUG_ON(qc->flags & (QUIC_FL_CONN_CLOSING|QUIC_FL_CONN_DRAINING));
-       LIST_APPEND(&ha_thread_ctx[new_tid].quic_conns, &qc->el_th_ctx);
-       qc->qc_epoch = HA_ATOMIC_LOAD(&qc_epoch);
 
        node = eb64_first(&qc->cids);
        BUG_ON(!node || eb64_next(node)); /* One and only one CID must be present before affinity rebind. */
@@ -8569,6 +8567,16 @@ void qc_finalize_affinity_rebind(struct quic_conn *qc)
        BUG_ON(!(qc->flags & QUIC_FL_CONN_AFFINITY_CHANGED));
        qc->flags &= ~QUIC_FL_CONN_AFFINITY_CHANGED;
 
+       /* A connection must not pass to closing state until affinity rebind
+        * is completed. Else quic_handle_stopping() may miss it during process
+        * stopping cleanup.
+        */
+       BUG_ON(qc->flags & (QUIC_FL_CONN_CLOSING|QUIC_FL_CONN_DRAINING));
+
+       /* Reinsert connection in ha_thread_ctx global list. */
+       LIST_APPEND(&th_ctx->quic_conns, &qc->el_th_ctx);
+       qc->qc_epoch = HA_ATOMIC_LOAD(&qc_epoch);
+
        /* Reactivate FD polling if connection socket is active. */
        qc_want_recv(qc);