]> git.ipfire.org Git - thirdparty/haproxy.git/commitdiff
MEDIUM: quic: use a global CID trees list
authorAmaury Denoyelle <adenoyelle@haproxy.com>
Tue, 18 Apr 2023 09:10:54 +0000 (11:10 +0200)
committerAmaury Denoyelle <adenoyelle@haproxy.com>
Tue, 18 Apr 2023 14:54:17 +0000 (16:54 +0200)
Previously, quic_connection_id were stored in a per-thread tree list.
Datagram were first dispatched to the correct thread using the encoded
TID before a tree lookup was done.

Remove these trees and replace it with a global trees list of 256
entries. A CID is using the list index corresponding to its first byte.
On datagram dispatch, CID is lookup on its tree and TID is retrieved
using new member quic_connection_id.tid. As such, a read-write lock
protects each list instances. With 256 entries, it is expected that
contention should be reduced.

A new structure quic_cid_tree served as a tree container associated with
its read-write lock. An API is implemented to ensure lock safety for
insert/lookup/delete operation.

This patch is a step forward to be able to break the affinity between a
CID and a TID encoded thread. This is required to be able to migrate a
quic_conn after accept to select thread based on their load.

This should be backported up to 2.7 after a period of observation.

include/haproxy/proto_quic.h
include/haproxy/quic_conn-t.h
include/haproxy/quic_conn.h
include/haproxy/thread.h
src/proto_quic.c
src/quic_conn.c
src/quic_sock.c
src/thread.c

index 8677cbfaedb0f9668cc5dc175271168be7a40d9d..a0e2b98160069b214a93f472bf11586235ca10c2 100644 (file)
 extern struct protocol proto_quic4;
 extern struct protocol proto_quic6;
 
+struct quic_cid_tree {
+       struct eb_root root;
+       __decl_thread(HA_RWLOCK_T lock);
+};
+
 extern struct quic_dghdlr *quic_dghdlrs;
+extern struct quic_cid_tree *quic_cid_trees;
 
 #endif /* _HAPROXY_PROTO_QUIC_H  */
index d366a0b4880a19f0b2d48d0c0d42d24b578d06d0..c8c6f48d33e42cb26a34e17ab9447dd1eb2b1e33 100644 (file)
@@ -303,6 +303,7 @@ struct quic_connection_id {
        struct quic_cid cid;   /* CID data */
 
        struct quic_conn *qc;  /* QUIC connection using this CID */
+       uint tid;              /* Attached Thread ID for the connection. */
 };
 
 /* Structure to hold a range of ACKs sent in ACK frames. */
@@ -444,7 +445,6 @@ struct quic_rx_packet {
 struct quic_dghdlr {
        struct mt_list dgrams;
        struct tasklet *task;
-       struct eb_root cids;
 };
 
 /* Structure to store enough information about the RX CRYPTO frames. */
index e1b333ec5a65edd7257c1214f1e79e4a3a871372..b52535ff3e194c6b4937d715fe5500085198f9d8 100644 (file)
@@ -38,6 +38,7 @@
 #include <haproxy/ticks.h>
 
 #include <haproxy/listener.h>
+#include <haproxy/proto_quic.h>
 #include <haproxy/quic_cc.h>
 #include <haproxy/quic_conn-t.h>
 #include <haproxy/quic_enc.h>
@@ -137,9 +138,41 @@ static inline void quic_cid_dump(struct buffer *buf,
        chunk_appendf(buf, ")");
 }
 
-/* Free the CIDs attached to <conn> QUIC connection. This must be called under
- * the CID lock.
- */
+/* Return tree index where <cid> is stored. */
+static inline uchar _quic_cid_tree_idx(const unsigned char *cid)
+{
+       return cid[0];
+}
+
+/* Return tree index where <cid> is stored. */
+static inline uchar quic_cid_tree_idx(const struct quic_cid *cid)
+{
+       return _quic_cid_tree_idx(cid->data);
+}
+
+/* Insert <conn_id> into global CID tree as a thread-safe operation. */
+static inline void quic_cid_insert(struct quic_connection_id *conn_id)
+{
+       const uchar idx = quic_cid_tree_idx(&conn_id->cid);
+       struct quic_cid_tree *tree = &quic_cid_trees[idx];
+
+       HA_RWLOCK_WRLOCK(QC_CID_LOCK, &tree->lock);
+       ebmb_insert(&tree->root, &conn_id->node, conn_id->cid.len);
+       HA_RWLOCK_WRUNLOCK(QC_CID_LOCK, &tree->lock);
+}
+
+/* Remove <conn_id> from global CID tree as a thread-safe operation. */
+static inline void quic_cid_delete(struct quic_connection_id *conn_id)
+{
+       const uchar idx = quic_cid_tree_idx(&conn_id->cid);
+       struct quic_cid_tree *tree = &quic_cid_trees[idx];
+
+       HA_RWLOCK_WRLOCK(QC_CID_LOCK, &tree->lock);
+       ebmb_delete(&conn_id->node);
+       HA_RWLOCK_WRUNLOCK(QC_CID_LOCK, &tree->lock);
+}
+
+/* Free the CIDs attached to <conn> QUIC connection. */
 static inline void free_quic_conn_cids(struct quic_conn *conn)
 {
        struct eb64_node *node;
@@ -151,7 +184,7 @@ static inline void free_quic_conn_cids(struct quic_conn *conn)
                conn_id = eb64_entry(node, struct quic_connection_id, seq_num);
 
                /* remove the CID from the receiver tree */
-               ebmb_delete(&conn_id->node);
+               quic_cid_delete(conn_id);
 
                /* remove the CID from the quic_conn tree */
                node = eb64_next(node);
@@ -175,39 +208,6 @@ static inline void quic_connection_id_to_frm_cpy(struct quic_frame *dst,
        to->stateless_reset_token = src->stateless_reset_token;
 }
 
-/* extract a TID from a CID for receiver <rx>, from 0 to global.nbthread-1 and
- * in any case no more than 4095. It takes into account the bind_conf's thread
- * group and the bind_conf's thread mask. The algorithm is the following: most
- * packets contain a valid thread ID for the bind_conf, which means that the
- * retrieved ID directly maps to a bound thread ID. If that's not the case,
- * then we have to remap it. The resulting thread ID will then differ but will
- * be correctly encoded and decoded.
- */
-static inline uint quic_get_cid_tid(const unsigned char *cid, const struct receiver *rx)
-{
-       uint id, grp;
-       uint base, count;
-
-       id = read_n16(cid) & 4095;
-       grp = rx->bind_tgroup;
-       base  = ha_tgroup_info[grp - 1].base;
-       count = ha_tgroup_info[grp - 1].count;
-
-       if (base <= id && id < base + count &&
-           rx->bind_thread & ha_thread_info[id].ltid_bit)
-               return id; // part of the group and bound: valid
-
-       /* The thread number isn't valid, it doesn't map to a thread bound on
-        * this receiver. Let's reduce it to one of the thread(s) valid for
-        * that receiver.
-        */
-       count = my_popcountl(rx->bind_thread);
-       id = count - 1 - id % count;
-       id = mask_find_rank_bit(id, rx->bind_thread);
-       id += base;
-       return id;
-}
-
 /* Modify <cid> to have a CID linked to the thread ID <target_tid> that
  * quic_get_cid_tid() will be able to extract return.
  */
@@ -676,6 +676,9 @@ int quic_get_dgram_dcid(unsigned char *buf, const unsigned char *end,
                         unsigned char **dcid, size_t *dcid_len);
 struct quic_cid quic_derive_cid(const struct quic_cid *orig,
                                 const struct sockaddr_storage *addr);
+int quic_get_cid_tid(const unsigned char *cid, size_t cid_len,
+                     const struct sockaddr_storage *cli_addr,
+                     unsigned char *buf, size_t buf_len);
 int qc_send_mux(struct quic_conn *qc, struct list *frms);
 
 void qc_notify_close(struct quic_conn *qc);
index b504ab675ebe7814dcc4ae44b18a1e9ce2d3aa1a..30ff7827a0c67e25bb6b4f9b54930e7f352e5a9c 100644 (file)
@@ -424,6 +424,7 @@ enum lock_label {
        SFT_LOCK, /* sink forward target */
        IDLE_CONNS_LOCK,
        OCSP_LOCK,
+       QC_CID_LOCK,
        OTHER_LOCK,
        /* WT: make sure never to use these ones outside of development,
         * we need them for lock profiling!
index b1708eb9f2a4b31c4dc2e89ff15fcc42b451b233..0961fc21046eaae4a09a2cebdd7176deb671d6e5 100644 (file)
 
 /* per-thread quic datagram handlers */
 struct quic_dghdlr *quic_dghdlrs;
+struct eb_root *quic_cid_tree;
+
+/* global CID trees */
+#define QUIC_CID_TREES_CNT 256
+struct quic_cid_tree *quic_cid_trees;
 
 /* Size of the internal buffer of QUIC RX buffer at the fd level */
 #define QUIC_RX_BUFSZ  (1UL << 18)
@@ -734,11 +739,20 @@ static int quic_alloc_dghdlrs(void)
                dghdlr->task->context = dghdlr;
                dghdlr->task->process = quic_lstnr_dghdlr;
 
-               dghdlr->cids = EB_ROOT_UNIQUE;
-
                MT_LIST_INIT(&dghdlr->dgrams);
        }
 
+       quic_cid_trees = calloc(QUIC_CID_TREES_CNT, sizeof(struct quic_cid_tree));
+       if (!quic_cid_trees) {
+               ha_alert("Failed to allocate global CIDs trees.\n");
+               return 0;
+       }
+
+       for (i = 0; i < QUIC_CID_TREES_CNT; ++i) {
+               HA_RWLOCK_INIT(&quic_cid_trees[i].lock);
+               quic_cid_trees[i].root = EB_ROOT_UNIQUE;
+       }
+
        return 1;
 }
 REGISTER_POST_CHECK(quic_alloc_dghdlrs);
@@ -753,6 +767,8 @@ static int quic_deallocate_dghdlrs(void)
                free(quic_dghdlrs);
        }
 
+       ha_free(&quic_cid_trees);
+
        return 1;
 }
 REGISTER_POST_DEINIT(quic_deallocate_dghdlrs);
index e1725e5c9b1b8b2f285f8d9d740e24af98994f8a..1a6d1176a03b023d8f8eb880297c2856dd8a0325 100644 (file)
@@ -3253,8 +3253,7 @@ static int qc_parse_pkt_frms(struct quic_conn *qc, struct quic_rx_packet *pkt,
                                TRACE_ERROR("CID allocation error", QUIC_EV_CONN_IO_CB, qc);
                        }
                        else {
-                               /* insert the allocated CID in the receiver datagram handler tree */
-                               ebmb_insert(&quic_dghdlrs[tid].cids, &conn_id->node, conn_id->cid.len);
+                               quic_cid_insert(conn_id);
                                qc_build_new_connection_id_frm(qc, conn_id);
                        }
                        break;
@@ -3969,6 +3968,59 @@ struct quic_cid quic_derive_cid(const struct quic_cid *orig,
        return cid;
 }
 
+/* Retrieve the thread ID associated to QUIC connection ID <cid> of length
+ * <cid_len>. CID may be not found on the CID tree because it is an ODCID. In
+ * this case, it will derived using client address <cli_addr> as hash
+ * parameter. However, this is done only if <buf> points to an INITIAL or 0RTT
+ * packet of length <len>.
+ *
+ * Returns the thread ID or a negative error code.
+ */
+int quic_get_cid_tid(const unsigned char *cid, size_t cid_len,
+                     const struct sockaddr_storage *cli_addr,
+                     unsigned char *buf, size_t buf_len)
+{
+       struct quic_cid_tree *tree;
+       struct quic_connection_id *conn_id;
+       struct ebmb_node *node;
+
+       tree = &quic_cid_trees[_quic_cid_tree_idx(cid)];
+       HA_RWLOCK_RDLOCK(QC_CID_LOCK, &tree->lock);
+       node = ebmb_lookup(&tree->root, cid, cid_len);
+       HA_RWLOCK_RDUNLOCK(QC_CID_LOCK, &tree->lock);
+
+       if (!node) {
+               struct quic_cid orig, derive_cid;
+               struct quic_rx_packet pkt;
+
+               if (!qc_parse_hd_form(&pkt, &buf, buf + buf_len))
+                       goto not_found;
+
+               if (pkt.type != QUIC_PACKET_TYPE_INITIAL &&
+                   pkt.type != QUIC_PACKET_TYPE_0RTT) {
+                       goto not_found;
+               }
+
+               memcpy(orig.data, cid, cid_len);
+               orig.len = cid_len;
+               derive_cid = quic_derive_cid(&orig, cli_addr);
+
+               tree = &quic_cid_trees[quic_cid_tree_idx(&derive_cid)];
+               HA_RWLOCK_RDLOCK(QC_CID_LOCK, &tree->lock);
+               node = ebmb_lookup(&tree->root, cid, cid_len);
+               HA_RWLOCK_RDUNLOCK(QC_CID_LOCK, &tree->lock);
+       }
+
+       if (!node)
+               goto not_found;
+
+       conn_id = ebmb_entry(node, struct quic_connection_id, node);
+       return HA_ATOMIC_LOAD(&conn_id->tid);
+
+ not_found:
+       return -1;
+}
+
 /* Allocate a new CID and attach it to <root> ebtree.
  *
  * If <orig> and <addr> params are non null, the new CID value is directly
@@ -4016,6 +4068,7 @@ static struct quic_connection_id *new_quic_cid(struct eb_root *root,
        }
 
        conn_id->qc = qc;
+       HA_ATOMIC_STORE(&conn_id->tid, tid);
 
        conn_id->seq_num.key = qc->next_cid_seq_num++;
        conn_id->retire_prior_to = 0;
@@ -4082,8 +4135,10 @@ static int quic_build_post_handshake_frames(struct quic_conn *qc)
                        goto err;
                }
 
-               /* insert the allocated CID in the receiver datagram handler tree */
-               ebmb_insert(&quic_dghdlrs[tid].cids, &conn_id->node, conn_id->cid.len);
+               /* TODO To prevent CID tree locking, all CIDs created here
+                * could be allocated at the same time as the first one.
+                */
+               quic_cid_insert(conn_id);
 
                quic_connection_id_to_frm_cpy(frm, conn_id);
                LIST_APPEND(&frm_list, &frm->list);
@@ -4114,7 +4169,8 @@ static int quic_build_post_handshake_frames(struct quic_conn *qc)
                        break;
 
                node = eb64_next(node);
-               ebmb_delete(&conn_id->node);
+               quic_cid_delete(conn_id);
+
                eb64_delete(&conn_id->seq_num);
                pool_free(pool_head_quic_connection_id, conn_id);
        }
@@ -5504,7 +5560,7 @@ static struct quic_conn *qc_new_conn(const struct quic_version *qv, int ipv4,
 
        /* insert the allocated CID in the receiver datagram handler tree */
        if (server)
-               ebmb_insert(&quic_dghdlrs[tid].cids, &conn_id->node, conn_id->cid.len);
+               quic_cid_insert(conn_id);
 
        /* Select our SCID which is the first CID with 0 as sequence number. */
        qc->scid = conn_id->cid;
@@ -6511,11 +6567,15 @@ static struct quic_conn *retrieve_qc_conn_from_cid(struct quic_rx_packet *pkt,
        struct quic_conn *qc = NULL;
        struct ebmb_node *node;
        struct quic_connection_id *conn_id;
+       struct quic_cid_tree *tree;
 
        TRACE_ENTER(QUIC_EV_CONN_RXPKT);
 
        /* First look into DCID tree. */
-       node = ebmb_lookup(&quic_dghdlrs[tid].cids, pkt->dcid.data, pkt->dcid.len);
+       tree = &quic_cid_trees[_quic_cid_tree_idx(pkt->dcid.data)];
+       HA_RWLOCK_RDLOCK(QC_CID_LOCK, &tree->lock);
+       node = ebmb_lookup(&tree->root, pkt->dcid.data, pkt->dcid.len);
+       HA_RWLOCK_RDUNLOCK(QC_CID_LOCK, &tree->lock);
 
        /* If not found on an Initial/0-RTT packet, it could be because an
         * ODCID is reused by the client. Calculate the derived CID value to
@@ -6524,7 +6584,11 @@ static struct quic_conn *retrieve_qc_conn_from_cid(struct quic_rx_packet *pkt,
        if (!node && (pkt->type == QUIC_PACKET_TYPE_INITIAL ||
             pkt->type == QUIC_PACKET_TYPE_0RTT)) {
                const struct quic_cid derive_cid = quic_derive_cid(&pkt->dcid, saddr);
-               node = ebmb_lookup(&quic_dghdlrs[tid].cids, derive_cid.data, derive_cid.len);
+
+               tree = &quic_cid_trees[quic_cid_tree_idx(&derive_cid)];
+               HA_RWLOCK_RDLOCK(QC_CID_LOCK, &tree->lock);
+               node = ebmb_lookup(&tree->root, derive_cid.data, derive_cid.len);
+               HA_RWLOCK_RDUNLOCK(QC_CID_LOCK, &tree->lock);
        }
 
        if (!node)
@@ -8196,9 +8260,12 @@ int quic_dgram_parse(struct quic_dgram *dgram, struct quic_conn *from_qc,
  */
 int qc_check_dcid(struct quic_conn *qc, unsigned char *dcid, size_t dcid_len)
 {
-       struct ebmb_node *node;
+       const uchar idx = _quic_cid_tree_idx(dcid);
        struct quic_connection_id *conn_id;
+       struct ebmb_node *node = NULL;
+       struct quic_cid_tree *tree = &quic_cid_trees[idx];
 
+       /* Test against our default CID or client ODCID. */
        if ((qc->scid.len == dcid_len &&
             memcmp(qc->scid.data, dcid, dcid_len) == 0) ||
            (qc->odcid.len == dcid_len &&
@@ -8206,7 +8273,17 @@ int qc_check_dcid(struct quic_conn *qc, unsigned char *dcid, size_t dcid_len)
                return 1;
        }
 
-       node = ebmb_lookup(&quic_dghdlrs[tid].cids, dcid, dcid_len);
+       /* Test against our other CIDs. This can happen if the client has
+        * decided to switch to a new one.
+        *
+        * TODO to avoid locking, loop through qc.cids as an alternative.
+        *
+        * TODO set it to our default CID to avoid this operation next time.
+        */
+       HA_RWLOCK_RDLOCK(QC_CID_LOCK, &tree->lock);
+       node = ebmb_lookup(&tree->root, dcid, dcid_len);
+       HA_RWLOCK_RDUNLOCK(QC_CID_LOCK, &tree->lock);
+
        if (node) {
                conn_id = ebmb_entry(node, struct quic_connection_id, node);
                if (qc == conn_id->qc)
index 905ecab85f89c4d194e929bffe0c4f73a8163e56..3b13426f695eab97bf32c494f764355068b27694 100644 (file)
@@ -209,7 +209,6 @@ static int quic_lstnr_dgram_dispatch(unsigned char *buf, size_t len, void *owner
                                      struct quic_dgram *new_dgram, struct list *dgrams)
 {
        struct quic_dgram *dgram;
-       const struct listener *l = owner;
        unsigned char *dcid;
        size_t dcid_len;
        int cid_tid;
@@ -221,7 +220,18 @@ static int quic_lstnr_dgram_dispatch(unsigned char *buf, size_t len, void *owner
        if (!dgram)
                goto err;
 
-       cid_tid = quic_get_cid_tid(dcid, &l->rx);
+       if ((cid_tid = quic_get_cid_tid(dcid, dcid_len, saddr, buf, len)) < 0) {
+               /* CID not found. Ensure only one thread will handle this CID
+                * to avoid duplicating connection creation. This code may not
+                * work if using thread-mask on the listener but is only here
+                * for a short time.
+                *
+                * TODO this code implies the thread used for QUIC handshake is
+                * directly derived from client chosen CID. This association
+                * must be broken.
+                */
+               cid_tid = dcid[0] % global.nbthread;
+       }
 
        /* All the members must be initialized! */
        dgram->owner = owner;
index 6001f8c443b5890cfb717a0556b50fd7438f481b..d7128252ed0ea4f258369163280bcf409eb9c429 100644 (file)
@@ -443,6 +443,7 @@ static const char *lock_label(enum lock_label label)
        case SFT_LOCK:             return "SFT";
        case IDLE_CONNS_LOCK:      return "IDLE_CONNS";
        case OCSP_LOCK:            return "OCSP";
+       case QC_CID_LOCK:          return "QC_CID";
        case OTHER_LOCK:           return "OTHER";
        case DEBUG1_LOCK:          return "DEBUG1";
        case DEBUG2_LOCK:          return "DEBUG2";