From 93f5b4c8ae0e91acb890afced89fc0fcb9fabe26 Mon Sep 17 00:00:00 2001 From: Amaury Denoyelle Date: Fri, 5 Apr 2024 17:23:54 +0200 Subject: [PATCH] MINOR: quic: uniformize sending methods for handshake Emission of packets during handshakes was implemented via an API which uses two alternative ways to specify the list of frames. The first one uses a NULL list of quic_enc_level as argument for qc_prep_hpkts(). This was an implicit method to iterate on all qels stored in quic_conn instance, with frames already inserted in their corresponding quic_pktns. The second method was used for retransmission. It uses a custom local quic_enc_level list specified by the caller as input to qc_prep_hpkts(). Frames were accessible through list pointers of each quic_enc_level used in an implicit mechanism. This commit clarifies the API by using a single common method. Now quic_enc_level list must always be specified by the caller. As for frames list, each qels must set its new field pointer to the list of frames to send. Callers of qc_prep_hpkts() are responsible to always clear qels send list. This prevent a single instance of quic_enc_level to be inserted while being attached to another list. This allows notably to clean up some unnecessary code. First, list of quic_enc_level is removed as it is replaced by new . Also, it's now possible to use proper list_for_each_entry() inside qc_prep_hpkts() to loop over each qels. Internal functions for quic_enc_level selection is now removed. --- include/haproxy/quic_tls-t.h | 10 +-- src/quic_conn.c | 18 ++++- src/quic_tls.c | 5 +- src/quic_tx.c | 127 ++++++++++++----------------------- 4 files changed, 70 insertions(+), 90 deletions(-) diff --git a/include/haproxy/quic_tls-t.h b/include/haproxy/quic_tls-t.h index 2cb65a7b8..7f90d9a54 100644 --- a/include/haproxy/quic_tls-t.h +++ b/include/haproxy/quic_tls-t.h @@ -238,10 +238,12 @@ struct quic_cstream { struct quic_enc_level { struct list list; - /* Attach point to enqueue this encryption level during retransmissions */ - struct list retrans; - /* pointer to list used only during retransmissions */ - struct list *retrans_frms; + + /* Attach point to register encryption level before sending. */ + struct list el_send; + /* Pointer to the frames used by sending functions */ + struct list *send_frms; + /* Encryption level, as defined by the TLS stack. */ enum ssl_encryption_level_t level; /* TLS encryption context (AEAD only) */ diff --git a/src/quic_conn.c b/src/quic_conn.c index 538715b40..6989de6a8 100644 --- a/src/quic_conn.c +++ b/src/quic_conn.c @@ -744,6 +744,8 @@ struct task *quic_conn_io_cb(struct task *t, void *context, unsigned int state) int ret; struct quic_conn *qc = context; struct buffer *buf = NULL; + struct list send_list = LIST_HEAD_INIT(send_list); + struct quic_enc_level *qel, *tmp_qel; int st; struct tasklet *tl = (struct tasklet *)t; @@ -792,6 +794,13 @@ struct task *quic_conn_io_cb(struct task *t, void *context, unsigned int state) } } + /* Insert each QEL into sending list. */ + list_for_each_entry(qel, &qc->qel_list, list) { + BUG_ON(LIST_INLIST(&qel->el_send)); + LIST_APPEND(&send_list, &qel->el_send); + qel->send_frms = &qel->pktns->tx.frms; + } + buf = qc_get_txb(qc); if (!buf) goto out; @@ -806,7 +815,14 @@ struct task *quic_conn_io_cb(struct task *t, void *context, unsigned int state) BUG_ON_HOT(b_data(buf)); b_reset(buf); - ret = qc_prep_hpkts(qc, buf, NULL); + ret = qc_prep_hpkts(qc, buf, &send_list); + + /* Always reset QEL sending list. */ + list_for_each_entry_safe(qel, tmp_qel, &send_list, el_send) { + LIST_DEL_INIT(&qel->el_send); + qel->send_frms = NULL; + } + if (ret == -1) { qc_txb_release(qc); goto out; diff --git a/src/quic_tls.c b/src/quic_tls.c index aa7283125..885df6f68 100644 --- a/src/quic_tls.c +++ b/src/quic_tls.c @@ -206,8 +206,9 @@ static int quic_conn_enc_level_init(struct quic_conn *qc, if (!qel) goto leave; - LIST_INIT(&qel->retrans); - qel->retrans_frms = NULL; + LIST_INIT(&qel->el_send); + qel->send_frms = NULL; + qel->tx.crypto.bufs = NULL; qel->tx.crypto.nb_buf = 0; qel->cstream = NULL; diff --git a/src/quic_tx.c b/src/quic_tx.c index 356bc4a81..86cb63d17 100644 --- a/src/quic_tx.c +++ b/src/quic_tx.c @@ -628,44 +628,13 @@ int qc_send_mux(struct quic_conn *qc, struct list *frms) return ret; } -/* Return the encryption level following the one which contains list head - * depending on TX mode (retranmission or not). +/* Select <*tls_ctx> and <*ver> for the encryption level of QUIC + * connection, depending on its state, especially the negotiated version. */ -static inline struct quic_enc_level *qc_list_next_qel(struct list *el, int retrans) -{ - return !retrans ? LIST_NEXT(el, struct quic_enc_level *, list) : - LIST_NEXT(el, struct quic_enc_level *, retrans); -} - -/* Return the encryption level following depending on TX mode - * (retranmission or not). - */ -static inline struct quic_enc_level *qc_next_qel(struct quic_enc_level *qel, int retrans) -{ - struct list *el = !retrans ? &qel->list : &qel->retrans; - - return qc_list_next_qel(el, retrans); -} - -/* Return 1 if is at the head of its list, 0 if not. */ -static inline int qc_qel_is_head(struct quic_enc_level *qel, struct list *l, - int retrans) -{ - return !retrans ? &qel->list == l : &qel->retrans == l; -} - -/* Select <*tls_ctx>, <*frms> and <*ver> for the encryption level of QUIC - * connection, depending on its state, especially the negotiated version and if - * retransmissions are required. If this the case is the list of encryption - * levels to used, or NULL if no retransmissions are required. - * Never fails. - */ -static inline void qc_select_tls_frms_ver(struct quic_conn *qc, - struct quic_enc_level *qel, - struct quic_tls_ctx **tls_ctx, - struct list **frms, - const struct quic_version **ver, - struct list *qels) +static inline void qc_select_tls_ver(struct quic_conn *qc, + struct quic_enc_level *qel, + struct quic_tls_ctx **tls_ctx, + const struct quic_version **ver) { if (qc->negotiated_version) { *ver = qc->negotiated_version; @@ -678,18 +647,11 @@ static inline void qc_select_tls_frms_ver(struct quic_conn *qc, *ver = qc->original_version; *tls_ctx = &qel->tls_ctx; } - - if (!qels) - *frms = &qel->pktns->tx.frms; - else - *frms = qel->retrans_frms; } /* Prepare as much as possible QUIC datagrams/packets for sending from * list of encryption levels. Several packets can be coalesced into a single - * datagram. The result is written into . Note that if is NULL, - * the encryption levels which will be used are those currently allocated - * and attached to the connection. + * datagram. The result is written into . * * Each datagram is prepended by a two fields header : the datagram length and * the address of first packet in the datagram. @@ -699,12 +661,11 @@ static inline void qc_select_tls_frms_ver(struct quic_conn *qc, */ int qc_prep_hpkts(struct quic_conn *qc, struct buffer *buf, struct list *qels) { - int ret, cc, retrans, padding; + int ret, cc, padding; struct quic_tx_packet *first_pkt, *prv_pkt; unsigned char *end, *pos; uint16_t dglen; size_t total; - struct list *qel_list; struct quic_enc_level *qel; TRACE_ENTER(QUIC_EV_CONN_IO_CB, qc); @@ -715,32 +676,34 @@ int qc_prep_hpkts(struct quic_conn *qc, struct buffer *buf, struct list *qels) ret = -1; cc = qc->flags & QUIC_FL_CONN_IMMEDIATE_CLOSE; - retrans = !!qels; padding = 0; first_pkt = prv_pkt = NULL; end = pos = (unsigned char *)b_head(buf); dglen = 0; total = 0; - qel_list = qels ? qels : &qc->qel_list; - qel = qc_list_next_qel(qel_list, retrans); - while (!qc_qel_is_head(qel, qel_list, retrans)) { + list_for_each_entry(qel, qels, el_send) { struct quic_tls_ctx *tls_ctx; const struct quic_version *ver; - struct list *frms, *next_frms; + struct list *frms = qel->send_frms, *next_frms; struct quic_enc_level *next_qel; if (qel == qc->eel) { /* Next encryption level */ - qel = qc_next_qel(qel, retrans); continue; } - qc_select_tls_frms_ver(qc, qel, &tls_ctx, &frms, &ver, qels); + qc_select_tls_ver(qc, qel, &tls_ctx, &ver); - next_qel = qc_next_qel(qel, retrans); - next_frms = qc_qel_is_head(next_qel, qel_list, retrans) ? NULL : - !qels ? &next_qel->pktns->tx.frms : next_qel->retrans_frms; + /* Retrieve next QEL. Set it to NULL if on qels last element. */ + if (qel->el_send.n != qels) { + next_qel = LIST_ELEM(qel->el_send.n, struct quic_enc_level *, el_send); + next_frms = next_qel->send_frms; + } + else { + next_qel = NULL; + next_frms = NULL; + } /* Build as much as datagrams at encryption level. * Each datagram is prepended with its length followed by the address @@ -759,7 +722,7 @@ int qc_prep_hpkts(struct quic_conn *qc, struct buffer *buf, struct list *qels) probe = qel->pktns->tx.pto_probe; if (!qc_may_build_pkt(qc, frms, qel, cc, probe, &must_ack)) { - if (prv_pkt && qc_qel_is_head(next_qel, qel_list, retrans)) { + if (prv_pkt && !next_qel) { qc_txb_store(buf, dglen, first_pkt); /* Build only one datagram when an immediate close is required. */ if (cc) @@ -855,8 +818,7 @@ int qc_prep_hpkts(struct quic_conn *qc, struct buffer *buf, struct list *qels) * the same datagram, except if is the Application data * encryption level which cannot be selected to do that. */ - if (LIST_ISEMPTY(frms) && qel != qc->ael && - !qc_qel_is_head(next_qel, qel_list, retrans)) { + if (LIST_ISEMPTY(frms) && qel != qc->ael && next_qel) { if (qel == qc->iel && (!qc_is_listener(qc) || cur_pkt->flags & QUIC_FL_TX_PACKET_ACK_ELICITING)) @@ -876,9 +838,6 @@ int qc_prep_hpkts(struct quic_conn *qc, struct buffer *buf, struct list *qels) prv_pkt = NULL; } } - - /* Next encryption level */ - qel = next_qel; } out: @@ -903,9 +862,10 @@ int qc_prep_hpkts(struct quic_conn *qc, struct buffer *buf, struct list *qels) int qc_send_hdshk_pkts(struct quic_conn *qc, int old_data, struct quic_enc_level *qel1, struct quic_enc_level *qel2) { + struct quic_enc_level *qel, *tmp_qel; int ret, status = 0; struct buffer *buf = qc_get_txb(qc); - struct list qels = LIST_HEAD_INIT(qels); + struct list send_list = LIST_HEAD_INIT(send_list); TRACE_ENTER(QUIC_EV_CONN_TXPKT, qc); @@ -931,17 +891,22 @@ int qc_send_hdshk_pkts(struct quic_conn *qc, int old_data, qc->flags |= QUIC_FL_CONN_RETRANS_OLD_DATA; } + /* At least one QEL must be set or sending is unnecessary. */ + BUG_ON(!qel1 && !qel2); + if (qel1) { - BUG_ON(LIST_INLIST(&qel1->retrans)); - LIST_APPEND(&qels, &qel1->retrans); + /* Ensure QEL is not already registered for sending. */ + BUG_ON(LIST_INLIST(&qel1->el_send)); + LIST_APPEND(&send_list, &qel1->el_send); } if (qel2) { - BUG_ON(LIST_INLIST(&qel2->retrans)); - LIST_APPEND(&qels, &qel2->retrans); + /* Ensure QEL is not already registered for sending. */ + BUG_ON(LIST_INLIST(&qel2->el_send)); + LIST_APPEND(&send_list, &qel2->el_send); } - ret = qc_prep_hpkts(qc, buf, &qels); + ret = qc_prep_hpkts(qc, buf, &send_list); if (ret == -1) { qc_txb_release(qc); TRACE_ERROR("Could not build some packets", QUIC_EV_CONN_TXPKT, qc); @@ -959,21 +924,17 @@ int qc_send_hdshk_pkts(struct quic_conn *qc, int old_data, status = 1; out: - if (qel1) { - LIST_DEL_INIT(&qel1->retrans); - qel1->retrans_frms = NULL; - } - - if (qel2) { - LIST_DEL_INIT(&qel2->retrans); - qel2->retrans_frms = NULL; - } - if (old_data) { TRACE_STATE("no more need old data for probing", QUIC_EV_CONN_TXPKT, qc); qc->flags &= ~QUIC_FL_CONN_RETRANS_OLD_DATA; } + /* Always reset QEL sending list. */ + list_for_each_entry_safe(qel, tmp_qel, &send_list, el_send) { + LIST_DEL_INIT(&qel->el_send); + qel->send_frms = NULL; + } + TRACE_DEVEL((status ? "leaving" : "leaving in error"), QUIC_EV_CONN_TXPKT, qc); return status; } @@ -1009,9 +970,9 @@ int qc_dgrams_retransmit(struct quic_conn *qc) ipktns->tx.pto_probe = 1; if (!LIST_ISEMPTY(&hfrms)) hpktns->tx.pto_probe = 1; - qc->iel->retrans_frms = &ifrms; + qc->iel->send_frms = &ifrms; if (qc->hel) - qc->hel->retrans_frms = &hfrms; + qc->hel->send_frms = &hfrms; sret = qc_send_hdshk_pkts(qc, 1, qc->iel, qc->hel); qc_free_frm_list(qc, &ifrms); qc_free_frm_list(qc, &hfrms); @@ -1025,7 +986,7 @@ int qc_dgrams_retransmit(struct quic_conn *qc) * datagram. */ ipktns->tx.pto_probe = 1; - qc->iel->retrans_frms = &ifrms; + qc->iel->send_frms = &ifrms; sret = qc_send_hdshk_pkts(qc, 0, qc->iel, NULL); qc_free_frm_list(qc, &ifrms); qc_free_frm_list(qc, &hfrms); @@ -1053,7 +1014,7 @@ int qc_dgrams_retransmit(struct quic_conn *qc) TRACE_DEVEL("Avail. ack eliciting frames", QUIC_EV_CONN_FRMLIST, qc, &frms1); if (!LIST_ISEMPTY(&frms1)) { hpktns->tx.pto_probe = 1; - qc->hel->retrans_frms = &frms1; + qc->hel->send_frms = &frms1; sret = qc_send_hdshk_pkts(qc, 1, qc->hel, NULL); qc_free_frm_list(qc, &frms1); if (!sret)