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 <retransmit> 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 <send_frms> 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,
<retransmit> list of quic_enc_level is removed as it is replaced by new
<send_frms>. 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.

diff --git a/include/haproxy/quic_tls-t.h b/include/haproxy/quic_tls-t.h
index 2cb65a7b802ce350c6d4df7c28b04bdef2801106..7f90d9a54ba35aa4591b5f7f408e53d76a97f379 100644 (file)
--- 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 538715b40772960120e1794a442a4af89a02c107..6989de6a822f1efa0e71094a1d60e42c4c95a60d 100644 (file)
--- 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 aa72831251ceadd11f9ec7b4f4aa9a0149195981..885df6f68cee31d02c7099d0470b66ee0ba50afb 100644 (file)
--- 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 356bc4a8187be80fb54a5bbede46a7e6556a4948..86cb63d1784cfdab202a6285c0bd81526450673c 100644 (file)
--- 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 <el> list head
- * depending on <retrans> TX mode (retranmission or not).
+/* Select <*tls_ctx> and <*ver> for the encryption level <qel> of <qc> 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 <qel> depending on <retrans> 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 <qel> 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 <qel> of <qc> QUIC
- * connection, depending on its state, especially the negotiated version and if
- * retransmissions are required. If this the case <qels> 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 <qels>
  * list of encryption levels. Several packets can be coalesced into a single
- * datagram. The result is written into <buf>. Note that if <qels> is NULL,
- * the encryption levels which will be used are those currently allocated
- * and attached to the connection.
+ * datagram. The result is written into <buf>.
  *
  * 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 <qel> 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 <qel> 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)