MINOR: quic: refactor frame encoding

This patch is a direct follow-up of the previous one. This time,
refactoring is performed on qc_build_frm() which is used for frame
encoding.

Function prototype has changed as now packet argument is removed. To be
able to check frame validity with a packet, one can use the new parent
function qc_build_frm_pkt() which relies on qc_build_frm().

As with the previous patch, there is no function change expected. The
objective is to facilitate a future QMux implementation.

diff --git a/include/haproxy/quic_frame.h b/include/haproxy/quic_frame.h
index 0c5fe70ca99661c81b3b2b84dee0e57c3dcf6252..34555850dbf37c83aeebcbc897f41c8192c9e01a 100644 (file)
--- a/include/haproxy/quic_frame.h
+++ b/include/haproxy/quic_frame.h
@@ -34,10 +34,14 @@
 
 const char *quic_frame_type_string(enum quic_frame_type ft);
 
-int qc_build_frm(unsigned char **pos, const unsigned char *end,
-                 struct quic_frame *frm, struct quic_tx_packet *pkt,
+int qc_build_frm(struct quic_frame *frm,
+                 unsigned char **pos, const unsigned char *end,
                  struct quic_conn *conn);
 
+int qc_build_frm_pkt(struct quic_frame *frm, struct quic_tx_packet *pkt,
+                     unsigned char **pos, const unsigned char *end,
+                     struct quic_conn *qc);
+
 int qc_parse_frm_type(struct quic_frame *frm,
                       const unsigned char **pos, const unsigned char *end,
                       struct quic_conn *conn);

diff --git a/src/quic_frame.c b/src/quic_frame.c
index cbff5277ad08d239a6e3366cf90bb18052a2bdfa..a7bb95ddb9f52403fca132f9a9b3fca0a8eb8b99 100644 (file)
--- a/src/quic_frame.c
+++ b/src/quic_frame.c
@@ -1227,26 +1227,21 @@ int qc_parse_frm_payload(struct quic_frame *frm,
        return ret;
 }
 
-/* Encode <frm> QUIC frame at <pos> buffer position.
- * Returns 1 if succeeded (enough room at <pos> buffer position to encode the frame), 0 if not.
- * The buffer is updated to point to one byte past the end of the built frame
- * only if succeeded.
+/* Encodes a <frm> QUIC frame in buffer starting at <pos> and ending
+ * at <end>. On success, <pos> is advanced up to the end of the newly encoded
+ * data.
+ *
+ * Returns 1 on success else 0.
  */
-int qc_build_frm(unsigned char **pos, const unsigned char *end,
-                 struct quic_frame *frm, struct quic_tx_packet *pkt,
+int qc_build_frm(struct quic_frame *frm,
+                 unsigned char **pos, const unsigned char *end,
                  struct quic_conn *qc)
 {
        int ret = 0;
-       const struct quic_frame_builder *builder;
+       const struct quic_frame_builder *builder = qf_builder(frm->type);
        unsigned char *p = *pos;
 
        TRACE_ENTER(QUIC_EV_CONN_BFRM, qc);
-       builder = qf_builder(frm->type);
-       if (!(builder->mask & (1U << pkt->type))) {
-               /* XXX This it a bug to send an unauthorized frame with such a packet type XXX */
-               TRACE_ERROR("unauthorized frame", QUIC_EV_CONN_BFRM, qc, frm);
-               BUG_ON(!(builder->mask & (1U << pkt->type)));
-       }
 
        if (!quic_enc_int(&p, end, frm->type)) {
                TRACE_DEVEL("not enough room", QUIC_EV_CONN_BFRM, qc, frm);
@@ -1259,9 +1254,39 @@ int qc_build_frm(unsigned char **pos, const unsigned char *end,
                goto leave;
        }
 
-       pkt->flags |= builder->flags;
        *pos = p;
+       ret = 1;
+ leave:
+       TRACE_LEAVE(QUIC_EV_CONN_BFRM, qc);
+       return ret;
+}
+
+/* Encodes a <frm> QUIC frame in <pkt> packet buffer via qc_build_frm() after
+ * checking packet type compatibility. Packet <pkt> flags are updated if the
+ * frame characteristics impacts it.
+ *
+ * Returns 1 on success else 0.
+ */
+int qc_build_frm_pkt(struct quic_frame *frm, struct quic_tx_packet *pkt,
+                     unsigned char **pos, const unsigned char *end,
+                     struct quic_conn *qc)
+{
+       const struct quic_frame_builder *builder = qf_builder(frm->type);
+       int ret = 0;
 
+       TRACE_ENTER(QUIC_EV_CONN_BFRM, qc);
+
+       if (pkt && !(builder->mask & (1U << pkt->type))) {
+               /* XXX This it a bug to send an unauthorized frame with such a packet type XXX */
+               TRACE_ERROR("unauthorized frame", QUIC_EV_CONN_BFRM, qc, frm);
+               BUG_ON(!(builder->mask & (1U << pkt->type)));
+       }
+
+       ret = qc_build_frm(frm, pos, end, qc);
+       if (!ret)
+               goto leave;
+
+       pkt->flags |= builder->flags;
        ret = 1;
  leave:
        TRACE_LEAVE(QUIC_EV_CONN_BFRM, qc);

diff --git a/src/quic_tx.c b/src/quic_tx.c
index c9a7c20946ff9a8c78814856925d9951b15eb11e..8cba69cf0f9dcb8d85172ce98c89a26ce8044d36 100644 (file)
--- a/src/quic_tx.c
+++ b/src/quic_tx.c
@@ -2087,7 +2087,7 @@ static int qc_do_build_pkt(unsigned char *pos, const unsigned char *end,
        /* payload building (ack-eliciting or not frames) */
        payload = pos;
        if (ack_frm_len) {
-               if (!qc_build_frm(&pos, end, &ack_frm, pkt, qc))
+               if (!qc_build_frm_pkt(&ack_frm, pkt, &pos, end, qc))
                        goto no_room;
 
                pkt->largest_acked_pn = quic_pktns_get_largest_acked_pn(qel->pktns);
@@ -2098,7 +2098,7 @@ static int qc_do_build_pkt(unsigned char *pos, const unsigned char *end,
        if (!LIST_ISEMPTY(&frm_list)) {
                struct quic_frame *tmp_cf;
                list_for_each_entry_safe(cf, tmp_cf, &frm_list, list) {
-                       if (!qc_build_frm(&pos, end, cf, pkt, qc)) {
+                       if (!qc_build_frm_pkt(cf, pkt, &pos, end, qc)) {
                                ssize_t room = end - pos;
                                TRACE_PROTO("Not enough room", QUIC_EV_CONN_TXPKT,
                                            qc, NULL, NULL, &room);
@@ -2118,13 +2118,13 @@ static int qc_do_build_pkt(unsigned char *pos, const unsigned char *end,
        /* Build a PING frame if needed. */
        if (add_ping_frm) {
                frm.type = QUIC_FT_PING;
-               if (!qc_build_frm(&pos, end, &frm, pkt, qc))
+               if (!qc_build_frm_pkt(&frm, pkt, &pos, end, qc))
                        goto no_room;
        }
 
        /* Build a CONNECTION_CLOSE frame if needed. */
        if (cc) {
-               if (!qc_build_frm(&pos, end, &cc_frm, pkt, qc))
+               if (!qc_build_frm_pkt(&cc_frm, pkt, &pos, end, qc))
                        goto no_room;
 
                pkt->flags |= QUIC_FL_TX_PACKET_CC;
@@ -2134,7 +2134,7 @@ static int qc_do_build_pkt(unsigned char *pos, const unsigned char *end,
        if (padding_len) {
                frm.type = QUIC_FT_PADDING;
                frm.padding.len = padding_len;
-               if (!qc_build_frm(&pos, end, &frm, pkt, qc))
+               if (!qc_build_frm_pkt(&frm, pkt, &pos, end, qc))
                        goto no_room;
        }