MINOR: quic: define new functions for frame alloc

Define two utility functions for quic_frame allocation :
* qc_frm_alloc() is used to allocate a new frame
* qc_frm_dup() is used to allocate a new frame by duplicating an
  existing one

Theses functions are useful to centralize quic_frame initialization.
Note that pool_zalloc() is replaced by a proper pool_alloc() + explicit
initialization code.

This commit will simplify implementation of the per frame retransmission
limitation. Indeed, a new counter will be added in quic_frame structure
which must be initialized to 0.

This should be backported up to 2.7.

diff --git a/include/haproxy/quic_conn.h b/include/haproxy/quic_conn.h
index a9edd97d2284edda99969cc774605a8315543383..b14765ba55ce65b059ad49d2fe711b339ac6acff 100644 (file)
--- a/include/haproxy/quic_conn.h
+++ b/include/haproxy/quic_conn.h
@@ -197,7 +197,7 @@ static inline void free_quic_conn_cids(struct quic_conn *conn)
        }
 }
 
-/* Copy <src> new connection ID information to <to> NEW_CONNECTION_ID frame data.
+/* Copy <src> new connection ID information to <dst> NEW_CONNECTION_ID frame.
  * Always succeeds.
  */
 static inline void quic_connection_id_to_frm_cpy(struct quic_frame *dst,
@@ -205,7 +205,6 @@ static inline void quic_connection_id_to_frm_cpy(struct quic_frame *dst,
 {
        struct quic_new_connection_id *to = &dst->new_connection_id;
 
-       dst->type = QUIC_FT_NEW_CONNECTION_ID;
        to->seq_num = src->seq_num.key;
        to->retire_prior_to = src->retire_prior_to;
        to->cid.len = src->cid.len;

diff --git a/include/haproxy/quic_frame-t.h b/include/haproxy/quic_frame-t.h
index cae1ae88979acae43fb4584333174589d5abc518..59014e4dcef5b9f72cd20757a48f396f0e198f56 100644 (file)
--- a/include/haproxy/quic_frame-t.h
+++ b/include/haproxy/quic_frame-t.h
@@ -239,9 +239,9 @@ struct quic_connection_close_app {
 };
 
 struct quic_frame {
-       struct list list;
-       struct quic_tx_packet *pkt;
-       unsigned char type;
+       struct list list;           /* List elem from parent elem (typically a Tx packet instance, a PKTNS or a MUX element). */
+       struct quic_tx_packet *pkt; /* Last Tx packet used to send the frame. */
+       unsigned char type;         /* QUIC frame type. */
        union {
                struct quic_padding padding;
                struct quic_ack ack;
@@ -266,10 +266,10 @@ struct quic_frame {
                struct quic_connection_close connection_close;
                struct quic_connection_close_app connection_close_app;
        };
-       struct quic_frame *origin;
-       struct list reflist;
-       struct list ref;
-       unsigned int flags;
+       struct quic_frame *origin;  /* Parent frame. Set if frame is a duplicate (used for retransmission). */
+       struct list reflist;        /* List head containing duplicated children frames. */
+       struct list ref;            /* List elem from parent frame reflist. Set if frame is a duplicate (used for retransmission). */
+       unsigned int flags;         /* QUIC_FL_TX_FRAME_* */
 };
 
 

diff --git a/include/haproxy/quic_frame.h b/include/haproxy/quic_frame.h
index c893230761e2e52dcd73874ee84c3874a5d0516c..2b5286ff227b54135fc2fb5d152848fb026d30eb 100644 (file)
--- a/include/haproxy/quic_frame.h
+++ b/include/haproxy/quic_frame.h
@@ -176,5 +176,58 @@ static inline struct quic_err quic_err_app(uint64_t code)
        return (struct quic_err){ .code = code, .app = 1 };
 }
 
+/* Allocate a quic_frame with type <type>.
+ *
+ * Returns the allocated frame or NULL on failure.
+ */
+static inline struct quic_frame *qc_frm_alloc(int type)
+{
+       struct quic_frame *frm = NULL;
+
+       frm = pool_alloc(pool_head_quic_frame);
+       if (!frm)
+               return NULL;
+
+       frm->type = type;
+
+       LIST_INIT(&frm->list);
+       LIST_INIT(&frm->reflist);
+       LIST_INIT(&frm->ref);
+       frm->pkt = NULL;
+       frm->origin = NULL;
+       frm->flags = 0;
+
+       return frm;
+}
+
+/* Allocate a quic_frame by duplicating <origin> frame. This will create a new
+ * frame of the same type with the same content. Internal fields such as packet
+ * owner and flags are however resetted for the newly allocated frame.
+ *
+ * Returns the allocated frame or NULL on failure.
+ */
+static inline struct quic_frame *qc_frm_dup(struct quic_frame *origin)
+{
+       struct quic_frame *frm = NULL;
+
+       frm = pool_alloc(pool_head_quic_frame);
+       if (!frm)
+               return NULL;
+
+       *frm = *origin;
+
+       /* Reinit all internal members. */
+       LIST_INIT(&frm->list);
+       LIST_INIT(&frm->reflist);
+       frm->pkt = NULL;
+       frm->flags = 0;
+
+       /* Attach <frm> to <origin>. */
+       LIST_APPEND(&origin->reflist, &frm->ref);
+       frm->origin = origin;
+
+       return frm;
+}
+
 #endif /* USE_QUIC */
 #endif /* _HAPROXY_QUIC_FRAME_H */

diff --git a/src/mux_quic.c b/src/mux_quic.c
index fe33e4d51ad436f6d659559a5ad81e015840d501..1540c3bfde2fbd7acf08336f2cf04d7797783a32 100644 (file)
--- a/src/mux_quic.c
+++ b/src/mux_quic.c
@@ -11,6 +11,7 @@
 #include <haproxy/qmux_http.h>
 #include <haproxy/qmux_trace.h>
 #include <haproxy/quic_conn.h>
+#include <haproxy/quic_frame.h>
 #include <haproxy/quic_stream.h>
 #include <haproxy/quic_tp-t.h>
 #include <haproxy/ssl_sock-t.h>
@@ -704,13 +705,11 @@ static void qcs_consume(struct qcs *qcs, uint64_t bytes)
 
        if (qcs->rx.msd - qcs->rx.offset < qcs->rx.msd_init / 2) {
                TRACE_DATA("increase stream credit via MAX_STREAM_DATA", QMUX_EV_QCS_RECV, qcc->conn, qcs);
-               frm = pool_zalloc(pool_head_quic_frame);
+               frm = qc_frm_alloc(QUIC_FT_MAX_STREAM_DATA);
                BUG_ON(!frm); /* TODO handle this properly */
 
                qcs->rx.msd = qcs->rx.offset + qcs->rx.msd_init;
 
-               LIST_INIT(&frm->reflist);
-               frm->type = QUIC_FT_MAX_STREAM_DATA;
                frm->max_stream_data.id = qcs->id;
                frm->max_stream_data.max_stream_data = qcs->rx.msd;
 
@@ -722,13 +721,11 @@ static void qcs_consume(struct qcs *qcs, uint64_t bytes)
        qcc->lfctl.offsets_consume += bytes;
        if (qcc->lfctl.md - qcc->lfctl.offsets_consume < qcc->lfctl.md_init / 2) {
                TRACE_DATA("increase conn credit via MAX_DATA", QMUX_EV_QCS_RECV, qcc->conn, qcs);
-               frm = pool_zalloc(pool_head_quic_frame);
+               frm = qc_frm_alloc(QUIC_FT_MAX_DATA);
                BUG_ON(!frm); /* TODO handle this properly */
 
                qcc->lfctl.md = qcc->lfctl.offsets_consume + qcc->lfctl.md_init;
 
-               LIST_INIT(&frm->reflist);
-               frm->type = QUIC_FT_MAX_DATA;
                frm->max_data.max_data = qcc->lfctl.md;
 
                LIST_APPEND(&qcs->qcc->lfctl.frms, &frm->list);
@@ -1283,11 +1280,9 @@ static int qcc_release_remote_stream(struct qcc *qcc, uint64_t id)
                ++qcc->lfctl.cl_bidi_r;
                if (qcc->lfctl.cl_bidi_r > qcc->lfctl.ms_bidi_init / 2) {
                        TRACE_DATA("increase max stream limit with MAX_STREAMS_BIDI", QMUX_EV_QCC_SEND, qcc->conn);
-                       frm = pool_zalloc(pool_head_quic_frame);
+                       frm = qc_frm_alloc(QUIC_FT_MAX_STREAMS_BIDI);
                        BUG_ON(!frm); /* TODO handle this properly */
 
-                       LIST_INIT(&frm->reflist);
-                       frm->type = QUIC_FT_MAX_STREAMS_BIDI;
                        frm->max_streams_bidi.max_streams = qcc->lfctl.ms_bidi +
                                                            qcc->lfctl.cl_bidi_r;
                        LIST_APPEND(&qcc->lfctl.frms, &frm->list);
@@ -1424,14 +1419,12 @@ static int qcs_build_stream_frm(struct qcs *qcs, struct buffer *out, char fin,
        BUG_ON(qcc->tx.sent_offsets + total > qcc->rfctl.md);
 
        TRACE_PROTO("sending STREAM frame", QMUX_EV_QCS_SEND, qcc->conn, qcs);
-       frm = pool_zalloc(pool_head_quic_frame);
+       frm = qc_frm_alloc(QUIC_FT_STREAM_8);
        if (!frm) {
                TRACE_ERROR("frame alloc failure", QMUX_EV_QCS_SEND, qcc->conn, qcs);
                goto err;
        }
 
-       LIST_INIT(&frm->reflist);
-       frm->type = QUIC_FT_STREAM_8;
        frm->stream.stream = qcs->stream;
        frm->stream.id = qcs->id;
        frm->stream.buf = out;
@@ -1599,14 +1592,12 @@ static int qcs_send_reset(struct qcs *qcs)
 
        TRACE_ENTER(QMUX_EV_QCS_SEND, qcs->qcc->conn, qcs);
 
-       frm = pool_zalloc(pool_head_quic_frame);
+       frm = qc_frm_alloc(QUIC_FT_RESET_STREAM);
        if (!frm) {
                TRACE_LEAVE(QMUX_EV_QCS_SEND, qcs->qcc->conn, qcs);
                return 1;
        }
 
-       LIST_INIT(&frm->reflist);
-       frm->type = QUIC_FT_RESET_STREAM;
        frm->reset_stream.id = qcs->id;
        frm->reset_stream.app_error_code = qcs->err;
        frm->reset_stream.final_size = qcs->tx.sent_offset;
@@ -1657,14 +1648,12 @@ static int qcs_send_stop_sending(struct qcs *qcs)
                goto done;
        }
 
-       frm = pool_zalloc(pool_head_quic_frame);
+       frm = qc_frm_alloc(QUIC_FT_STOP_SENDING);
        if (!frm) {
                TRACE_LEAVE(QMUX_EV_QCS_SEND, qcs->qcc->conn, qcs);
                return 1;
        }
 
-       LIST_INIT(&frm->reflist);
-       frm->type = QUIC_FT_STOP_SENDING;
        frm->stop_sending.id = qcs->id;
        frm->stop_sending.app_error_code = qcs->err;
 

diff --git a/src/quic_conn.c b/src/quic_conn.c
index a64cd654d776860efc1223e76adc2340fbfca3d4..a42a07a2717153a1d381ea978e5590c342bc6202 100644 (file)
--- a/src/quic_conn.c
+++ b/src/quic_conn.c
@@ -1122,14 +1122,12 @@ static int quic_crypto_data_cpy(struct quic_conn *qc, struct quic_enc_level *qel
                        found->crypto.len += cf_len;
                }
                else {
-                       frm = pool_zalloc(pool_head_quic_frame);
+                       frm = qc_frm_alloc(QUIC_FT_CRYPTO);
                        if (!frm) {
                                TRACE_ERROR("Could not allocate quic frame", QUIC_EV_CONN_ADDDATA, qc);
                                goto leave;
                        }
 
-                       LIST_INIT(&frm->reflist);
-                       frm->type = QUIC_FT_CRYPTO;
                        frm->crypto.offset = cf_offset;
                        frm->crypto.len = cf_len;
                        frm->crypto.qel = qel;
@@ -2433,30 +2431,26 @@ static void qc_dup_pkt_frms(struct quic_conn *qc,
                        break;
                }
 
-               dup_frm = pool_alloc(pool_head_quic_frame);
+               /* If <frm> is already a copy of another frame, we must take
+                * its original frame as source for the copy.
+                */
+               origin = frm->origin ? frm->origin : frm;
+               dup_frm = qc_frm_dup(origin);
                if (!dup_frm) {
                        TRACE_ERROR("could not duplicate frame", QUIC_EV_CONN_PRSAFRM, qc, frm);
                        break;
                }
 
-               /* If <frm> is already a copy of another frame, we must take
-                * its original frame as source for the copy.
-                */
-               origin = frm->origin ? frm->origin : frm;
                TRACE_DEVEL("built probing frame", QUIC_EV_CONN_PRSAFRM, qc, origin);
-               if (origin->pkt)
+               if (origin->pkt) {
                        TRACE_DEVEL("duplicated from packet", QUIC_EV_CONN_PRSAFRM,
                                    qc, NULL, &origin->pkt->pn_node.key);
+               }
                else {
                        /* <origin> is a frame which was sent from a packet detected as lost. */
                        TRACE_DEVEL("duplicated from lost packet", QUIC_EV_CONN_PRSAFRM, qc);
                }
-               *dup_frm = *origin;
-               dup_frm->pkt = NULL;
-               dup_frm->origin = origin;
-               dup_frm->flags = 0;
-               LIST_INIT(&dup_frm->reflist);
-               LIST_APPEND(&origin->reflist, &dup_frm->ref);
+
                LIST_APPEND(&tmp, &dup_frm->list);
        }
 
@@ -2614,17 +2608,14 @@ static int qc_stop_sending_frm_enqueue(struct quic_conn *qc, uint64_t id)
         * at this time.
         */
        app_error_code = H3_REQUEST_REJECTED;
-       // fixme: zalloc
-       frm = pool_zalloc(pool_head_quic_frame);
+       frm = qc_frm_alloc(QUIC_FT_STOP_SENDING);
        if (!frm) {
                TRACE_ERROR("failed to allocate quic_frame", QUIC_EV_CONN_PRSHPKT, qc);
                goto out;
        }
 
-       frm->type = QUIC_FT_STOP_SENDING;
        frm->stop_sending.id = id;
        frm->stop_sending.app_error_code = app_error_code;
-       LIST_INIT(&frm->reflist);
        LIST_APPEND(&qel->pktns->tx.frms, &frm->list);
        ret = 1;
  out:
@@ -3548,14 +3539,12 @@ static int quic_build_post_handshake_frames(struct quic_conn *qc)
        qel = &qc->els[QUIC_TLS_ENC_LEVEL_APP];
        /* Only servers must send a HANDSHAKE_DONE frame. */
        if (qc_is_listener(qc)) {
-               frm = pool_zalloc(pool_head_quic_frame);
+               frm = qc_frm_alloc(QUIC_FT_HANDSHAKE_DONE);
                if (!frm) {
                        TRACE_ERROR("frame allocation error", QUIC_EV_CONN_IO_CB, qc);
                        goto leave;
                }
 
-               LIST_INIT(&frm->reflist);
-               frm->type = QUIC_FT_HANDSHAKE_DONE;
                LIST_APPEND(&frm_list, &frm->list);
        }
 
@@ -3569,13 +3558,12 @@ static int quic_build_post_handshake_frames(struct quic_conn *qc)
        for (i = first; i < max; i++) {
                struct quic_connection_id *cid;
 
-               frm = pool_zalloc(pool_head_quic_frame);
+               frm = qc_frm_alloc(QUIC_FT_NEW_CONNECTION_ID);
                if (!frm) {
                        TRACE_ERROR("frame allocation error", QUIC_EV_CONN_IO_CB, qc);
                        goto err;
                }
 
-               LIST_INIT(&frm->reflist);
                cid = new_quic_cid(&qc->cids, qc, i);
                if (!cid) {
                        pool_free(pool_head_quic_frame, frm);
@@ -6736,14 +6724,12 @@ static inline int qc_build_frms(struct list *outlist, struct list *inlist,
                        else {
                                struct quic_frame *new_cf;
 
-                               new_cf = pool_zalloc(pool_head_quic_frame);
+                               new_cf = qc_frm_alloc(QUIC_FT_CRYPTO);
                                if (!new_cf) {
                                        TRACE_ERROR("No memory for new crypto frame", QUIC_EV_CONN_BCFRMS, qc);
                                        continue;
                                }
 
-                               LIST_INIT(&new_cf->reflist);
-                               new_cf->type = QUIC_FT_CRYPTO;
                                new_cf->crypto.len = dlen;
                                new_cf->crypto.offset = cf->crypto.offset;
                                new_cf->crypto.qel = qel;
@@ -6846,14 +6832,12 @@ static inline int qc_build_frms(struct list *outlist, struct list *inlist,
                                struct quic_frame *new_cf;
                                struct buffer cf_buf;
 
-                               new_cf = pool_zalloc(pool_head_quic_frame);
+                               new_cf = qc_frm_alloc(cf->type);
                                if (!new_cf) {
                                        TRACE_ERROR("No memory for new STREAM frame", QUIC_EV_CONN_BCFRMS, qc);
                                        continue;
                                }
 
-                               LIST_INIT(&new_cf->reflist);
-                               new_cf->type = cf->type;
                                new_cf->stream.stream = cf->stream.stream;
                                new_cf->stream.buf = cf->stream.buf;
                                new_cf->stream.id = cf->stream.id;

diff --git a/src/quic_frame.c b/src/quic_frame.c
index 0d8bc87f5b892656c25d0626113db36fc4bd6a4e..38b524ec15b053b7ebe94c3aff4c669d112c7463 100644 (file)
--- a/src/quic_frame.c
+++ b/src/quic_frame.c
@@ -10,6 +10,9 @@
 #include <string.h>
 
 #include <import/eb64tree.h>
+#include <haproxy/buf-t.h>
+#include <haproxy/chunk.h>
+#include <haproxy/pool.h>
 #include <haproxy/quic_conn-t.h>
 #include <haproxy/quic_enc.h>
 #include <haproxy/quic_frame.h>