From 011b085803d142514d7a432d020e0b1894b21052 Mon Sep 17 00:00:00 2001 From: Amaury Denoyelle Date: Mon, 30 Mar 2026 14:11:17 +0200 Subject: [PATCH] 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. --- include/haproxy/quic_frame.h | 8 ++++-- src/quic_frame.c | 53 ++++++++++++++++++++++++++---------- src/quic_tx.c | 10 +++---- 3 files changed, 50 insertions(+), 21 deletions(-) diff --git a/include/haproxy/quic_frame.h b/include/haproxy/quic_frame.h index 0c5fe70ca..34555850d 100644 --- 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 cbff5277a..a7bb95ddb 100644 --- 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 QUIC frame at buffer position. - * Returns 1 if succeeded (enough room at 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 QUIC frame in buffer starting at and ending + * at . On success, 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 QUIC frame in packet buffer via qc_build_frm() after + * checking packet type compatibility. Packet 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 c9a7c2094..8cba69cf0 100644 --- 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; } -- 2.47.3