MINOR: quic: remove access to CID global tree outside of quic_cid module

haproxy generates for each QUIC connection a set of CID. The peer must
reuse them as DCID for its emitted packet. On datagram reception, DCID
field serves as identifier to dispatch them on their correct thread.

These CIDs are stored in a global CID tree. Access to this data
structure must always be protected with CID_LOCK. This commit is a
refactoring to regroup all CID tree access in quic_cid module. Several
code parts are ajusted :

* quic_cid_insert() is extended to check for insertion race-condition.
  This is useful on quic_conn instantiation. Code where such race cannot
  happen can use unsafe _quic_cid_insert() instead.

* on RETIRE_CONNECTION_ID frame reception, existing quic_cid_delete()
  function is used.

* remove tree lookup from qc_check_dcid(), extracted in the new
  quic_cmp_cid_conn() function. Ultimately, the latter should be removed
  as CID lookup could be conducted on quic_conn owned tree without
  locking.

include/haproxy/quic_cid.h

@@ -18,9 +18,14 @@ struct quic_connection_id *new_quic_cid(struct eb_root *root,
                                         struct quic_conn *qc,
                                         const struct quic_cid *orig,
                                         const struct sockaddr_storage *addr);
+
+int quic_cid_insert(struct quic_connection_id *conn_id, int *new_tid);
+int quic_cmp_cid_conn(const unsigned char *cid, size_t cid_len,
+                      struct quic_conn *qc);
 int quic_get_cid_tid(const unsigned char *cid, size_t cid_len,
                      const struct sockaddr_storage *cli_addr,
                      unsigned char *pos, size_t len);
+
 struct quic_conn *retrieve_qc_conn_from_cid(struct quic_rx_packet *pkt,
                                             struct sockaddr_storage *saddr,
                                             int *new_tid);
@@ -67,8 +72,11 @@ 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)
+/* Insert <conn_id> into global CID tree. Do not check if value is already
+ * present in the tree. As such, it should not be used for the first DCID of a
+ * connection instance.
+ */
+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];

src/quic_cid.c

@@ -152,6 +152,63 @@ struct quic_connection_id *new_quic_cid(struct eb_root *root,
 	return NULL;
 }
 
+/* Insert <conn_id> in global CID tree. It may fail if an identical value is
+ * already stored. In this case, <new_tid> will be filled with the thread ID of
+ * the already stored CID.
+ *
+ * Returns 0 on insert success else non-zero.
+ */
+int quic_cid_insert(struct quic_connection_id *conn_id, int *new_tid)
+{
+	struct ebmb_node *node;
+	struct quic_cid_tree *tree;
+	int ret;
+
+	*new_tid = -1;
+	tree = &quic_cid_trees[quic_cid_tree_idx(&conn_id->cid)];
+
+	HA_RWLOCK_WRLOCK(QC_CID_LOCK, &tree->lock);
+	node = ebmb_insert(&tree->root, &conn_id->node, conn_id->cid.len);
+	if (node != &conn_id->node) {
+		/* Node already inserted, may happen on thread contention. */
+		conn_id = ebmb_entry(node, struct quic_connection_id, node);
+		*new_tid = HA_ATOMIC_LOAD(&conn_id->tid);
+		ret = -1;
+	}
+	else {
+		ret = 0;
+	}
+	HA_RWLOCK_WRUNLOCK(QC_CID_LOCK, &tree->lock);
+
+	return ret;
+}
+
+/* Lookup CID in global CID tree equal to <cid> data with <cid_len> length. If
+ * found, ensure CID instance is linked to <qc> connection.
+ *
+ * Returns a boolean value.
+ */
+int quic_cmp_cid_conn(const unsigned char *cid, size_t cid_len,
+                      struct quic_conn *qc)
+{
+	struct quic_cid_tree *tree;
+	struct quic_connection_id *conn_id;
+	struct ebmb_node *node;
+	int ret = 0;
+
+	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);
+	if (node) {
+		conn_id = ebmb_entry(node, struct quic_connection_id, node);
+		if (qc == conn_id->qc)
+			ret = 1;
+	}
+	HA_RWLOCK_RDUNLOCK(QC_CID_LOCK, &tree->lock);
+
+	return ret;
+}
+
 /* 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

src/quic_conn.c

@@ -47,6 +47,7 @@
 #include <haproxy/proxy.h>
 #include <haproxy/quic_ack.h>
 #include <haproxy/quic_cc.h>
+#include <haproxy/quic_cid.h>
 #include <haproxy/quic_cli-t.h>
 #include <haproxy/quic_frame.h>
 #include <haproxy/quic_enc.h>
@@ -507,7 +508,7 @@ int quic_build_post_handshake_frames(struct quic_conn *qc)
 		/* 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_cid_insert(conn_id);
 
 		quic_connection_id_to_frm_cpy(frm, conn_id);
 		LIST_APPEND(&frm_list, &frm->list);
@@ -1648,12 +1649,6 @@ const struct quic_version *qc_supported_version(uint32_t version)
  */
 int qc_check_dcid(struct quic_conn *qc, unsigned char *dcid, size_t dcid_len)
 {
-	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];
-	int ret;
-
 	/* Test against our default CID or client ODCID. */
 	if ((qc->scid.len == dcid_len &&
 	     memcmp(qc->scid.data, dcid, dcid_len) == 0) ||
@@ -1669,17 +1664,7 @@ int qc_check_dcid(struct quic_conn *qc, unsigned char *dcid, size_t dcid_len)
 	 *
 	 * TODO set it to our default CID to avoid this operation next time.
 	 */
-	ret = 0;
-	HA_RWLOCK_RDLOCK(QC_CID_LOCK, &tree->lock);
-	node = ebmb_lookup(&tree->root, dcid, dcid_len);
-	if (node) {
-		conn_id = ebmb_entry(node, struct quic_connection_id, node);
-		if (qc == conn_id->qc)
-			ret = 1;
-	}
-	HA_RWLOCK_RDUNLOCK(QC_CID_LOCK, &tree->lock);
-
-	return ret;
+	return quic_cmp_cid_conn(dcid, dcid_len, qc);
 }
 
 /* Wake-up upper layer for sending if all conditions are met :

src/quic_rx.c

@@ -982,7 +982,6 @@ static int qc_parse_pkt_frms(struct quic_conn *qc, struct quic_rx_packet *pkt,
 			break;
 		case QUIC_FT_RETIRE_CONNECTION_ID:
 		{
-			struct quic_cid_tree *tree __maybe_unused;
 			struct quic_connection_id *conn_id = NULL;
 
 			if (!qc_handle_retire_connection_id_frm(qc, &frm, &pkt->dcid, &conn_id))
@@ -991,10 +990,7 @@ static int qc_parse_pkt_frms(struct quic_conn *qc, struct quic_rx_packet *pkt,
 			if (!conn_id)
 				break;
 
-			tree = &quic_cid_trees[quic_cid_tree_idx(&conn_id->cid)];
-			HA_RWLOCK_WRLOCK(QC_CID_LOCK, &tree->lock);
-			ebmb_delete(&conn_id->node);
-			HA_RWLOCK_WRUNLOCK(QC_CID_LOCK, &tree->lock);
+			quic_cid_delete(conn_id);
 			eb64_delete(&conn_id->seq_num);
 			pool_free(pool_head_quic_connection_id, conn_id);
 			TRACE_PROTO("CID retired", QUIC_EV_CONN_PSTRM, qc);
@@ -1004,7 +1000,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 {
-				quic_cid_insert(conn_id);
+				_quic_cid_insert(conn_id);
 				qc_build_new_connection_id_frm(qc, conn_id);
 			}
 			break;
@@ -1583,8 +1579,6 @@ static struct quic_conn *quic_rx_pkt_retrieve_conn(struct quic_rx_packet *pkt,
 		BUG_ON(!pkt->version); /* This must not happen. */
 
 		if (!qc) {
-			struct quic_cid_tree *tree;
-			struct ebmb_node *node;
 			struct quic_connection_id *conn_id;
 			int ipv4;
 
@@ -1660,14 +1654,8 @@ static struct quic_conn *quic_rx_pkt_retrieve_conn(struct quic_rx_packet *pkt,
 				qc->hash64 = quic_hash64_from_cid(conn_id->cid.data, conn_id->cid.len,
 								  global.cluster_secret, sizeof(global.cluster_secret));
 
-			tree = &quic_cid_trees[quic_cid_tree_idx(&conn_id->cid)];
-			HA_RWLOCK_WRLOCK(QC_CID_LOCK, &tree->lock);
-			node = ebmb_insert(&tree->root, &conn_id->node, conn_id->cid.len);
-			if (node != &conn_id->node) {
+			if (quic_cid_insert(conn_id, new_tid)) {
 				pool_free(pool_head_quic_connection_id, conn_id);
-
-				conn_id = ebmb_entry(node, struct quic_connection_id, node);
-				*new_tid = HA_ATOMIC_LOAD(&conn_id->tid);
 				quic_conn_release(qc);
 				qc = NULL;
 			}
@@ -1680,7 +1668,6 @@ static struct quic_conn *quic_rx_pkt_retrieve_conn(struct quic_rx_packet *pkt,
 				/* Initialize the next CID sequence number to be used for this connection. */
 				qc->next_cid_seq_num = 1;
 			}
-			HA_RWLOCK_WRUNLOCK(QC_CID_LOCK, &tree->lock);
 
 			if (*new_tid != -1)
 				goto out;