MINOR: quic: use dynamically allocated frame on parsing
authorAmaury Denoyelle <adenoyelle@haproxy.com>
Tue, 5 Nov 2024 15:33:27 +0000 (16:33 +0100)
committerChristopher Faulet <cfaulet@haproxy.com>
Fri, 8 Nov 2024 14:54:11 +0000 (15:54 +0100)
qc_parse_pkt_frms() is the function responsible to parse a received QUIC
packet. Payload is decoded and splitted into individual frames which are
then handled individually. Previously, frame was used as locally stack
allocated. Change this to work on a dynamically allocated frame.

This commit does bring any functional changes. However, it will be
useful to extend packet parsing. In particular, it will be necessary to
save some frames during parsing to reparse them after the others.

(cherry picked from commit 190fc97606560568bf4a611d92c1e70aed057843)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>

src/quic_rx.c

index 3c4139c..02b899e 100644 (file)
@@ -809,7 +809,7 @@ static inline unsigned int quic_ack_delay_ms(struct qf_ack *ack_frm,
 static int qc_parse_pkt_frms(struct quic_conn *qc, struct quic_rx_packet *pkt,
                              struct quic_enc_level *qel)
 {
-       struct quic_frame frm;
+       struct quic_frame *frm = NULL;
        const unsigned char *pos, *end;
        int fast_retrans = 0;
 
@@ -833,12 +833,17 @@ static int qc_parse_pkt_frms(struct quic_conn *qc, struct quic_rx_packet *pkt,
        }
 
        while (pos < end) {
-               if (!qc_parse_frm(&frm, pkt, &pos, end, qc)) {
+               if (!frm && !(frm = qc_frm_alloc(0))) {
+                       TRACE_ERROR("cannot allocate frame", QUIC_EV_CONN_PRSHPKT, qc);
+                       goto err;
+               }
+
+               if (!qc_parse_frm(frm, pkt, &pos, end, qc)) {
                        // trace already emitted by function above
                        goto err;
                }
 
-               switch (frm.type) {
+               switch (frm->type) {
                case QUIC_FT_PADDING:
                        break;
                case QUIC_FT_PING:
@@ -849,7 +854,7 @@ static int qc_parse_pkt_frms(struct quic_conn *qc, struct quic_rx_packet *pkt,
                        unsigned int rtt_sample;
                        rtt_sample = UINT_MAX;
 
-                       if (!qc_parse_ack_frm(qc, &frm, qel, &rtt_sample, &pos, end)) {
+                       if (!qc_parse_ack_frm(qc, frm, qel, &rtt_sample, &pos, end)) {
                                // trace already emitted by function above
                                goto err;
                        }
@@ -859,21 +864,21 @@ static int qc_parse_pkt_frms(struct quic_conn *qc, struct quic_rx_packet *pkt,
 
                                ack_delay = !quic_application_pktns(qel->pktns, qc) ? 0 :
                                        qc->state >= QUIC_HS_ST_CONFIRMED ?
-                                       MS_TO_TICKS(QUIC_MIN(quic_ack_delay_ms(&frm.ack, qc), qc->max_ack_delay)) :
-                                       MS_TO_TICKS(quic_ack_delay_ms(&frm.ack, qc));
+                                       MS_TO_TICKS(QUIC_MIN(quic_ack_delay_ms(&frm->ack, qc), qc->max_ack_delay)) :
+                                       MS_TO_TICKS(quic_ack_delay_ms(&frm->ack, qc));
                                quic_loss_srtt_update(&qc->path->loss, rtt_sample, ack_delay, qc);
                        }
                        break;
                }
                case QUIC_FT_RESET_STREAM:
                        if (qc->mux_state == QC_MUX_READY) {
-                               struct qf_reset_stream *rs_frm = &frm.reset_stream;
+                               struct qf_reset_stream *rs_frm = &frm->reset_stream;
                                qcc_recv_reset_stream(qc->qcc, rs_frm->id, rs_frm->app_error_code, rs_frm->final_size);
                        }
                        break;
                case QUIC_FT_STOP_SENDING:
                {
-                       struct qf_stop_sending *ss_frm = &frm.stop_sending;
+                       struct qf_stop_sending *ss_frm = &frm->stop_sending;
                        if (qc->mux_state == QC_MUX_READY) {
                                if (qcc_recv_stop_sending(qc->qcc, ss_frm->id,
                                                          ss_frm->app_error_code)) {
@@ -884,7 +889,7 @@ static int qc_parse_pkt_frms(struct quic_conn *qc, struct quic_rx_packet *pkt,
                        break;
                }
                case QUIC_FT_CRYPTO:
-                       if (!qc_handle_crypto_frm(qc, &frm.crypto, pkt, qel, &fast_retrans))
+                       if (!qc_handle_crypto_frm(qc, &frm->crypto, pkt, qel, &fast_retrans))
                                goto err;
                        break;
                case QUIC_FT_NEW_TOKEN:
@@ -892,9 +897,9 @@ static int qc_parse_pkt_frms(struct quic_conn *qc, struct quic_rx_packet *pkt,
                        break;
                case QUIC_FT_STREAM_8 ... QUIC_FT_STREAM_F:
                {
-                       struct qf_stream *strm_frm = &frm.stream;
+                       struct qf_stream *strm_frm = &frm->stream;
                        unsigned nb_streams = qc->rx.strms[qcs_id_type(strm_frm->id)].nb_streams;
-                       const char fin = frm.type & QUIC_STREAM_FRAME_TYPE_FIN_BIT;
+                       const char fin = frm->type & QUIC_STREAM_FRAME_TYPE_FIN_BIT;
 
                        /* The upper layer may not be allocated. */
                        if (qc->mux_state != QC_MUX_READY) {
@@ -928,13 +933,13 @@ static int qc_parse_pkt_frms(struct quic_conn *qc, struct quic_rx_packet *pkt,
                }
                case QUIC_FT_MAX_DATA:
                        if (qc->mux_state == QC_MUX_READY) {
-                               struct qf_max_data *md_frm = &frm.max_data;
+                               struct qf_max_data *md_frm = &frm->max_data;
                                qcc_recv_max_data(qc->qcc, md_frm->max_data);
                        }
                        break;
                case QUIC_FT_MAX_STREAM_DATA:
                        if (qc->mux_state == QC_MUX_READY) {
-                               struct qf_max_stream_data *msd_frm = &frm.max_stream_data;
+                               struct qf_max_stream_data *msd_frm = &frm->max_stream_data;
                                if (qcc_recv_max_stream_data(qc->qcc, msd_frm->id,
                                                              msd_frm->max_stream_data)) {
                                        TRACE_ERROR("qcc_recv_max_stream_data() failed", QUIC_EV_CONN_PRSHPKT, qc);
@@ -965,7 +970,7 @@ static int qc_parse_pkt_frms(struct quic_conn *qc, struct quic_rx_packet *pkt,
                        struct quic_cid_tree *tree __maybe_unused;
                        struct quic_connection_id *conn_id = NULL;
 
-                       if (!qc_handle_retire_connection_id_frm(qc, &frm, &pkt->dcid, &conn_id))
+                       if (!qc_handle_retire_connection_id_frm(qc, frm, &pkt->dcid, &conn_id))
                                goto err;
 
                        if (!conn_id)
@@ -996,7 +1001,7 @@ static int qc_parse_pkt_frms(struct quic_conn *qc, struct quic_rx_packet *pkt,
                case QUIC_FT_CONNECTION_CLOSE:
                case QUIC_FT_CONNECTION_CLOSE_APP:
                        /* Increment the error counters */
-                       quic_conn_closed_err_count_inc(qc, &frm);
+                       quic_conn_closed_err_count_inc(qc, frm);
                        if (!(qc->flags & QUIC_FL_CONN_DRAINING)) {
                                TRACE_STATE("Entering draining state", QUIC_EV_CONN_PRSHPKT, qc);
                                /* RFC 9000 10.2. Immediate Close:
@@ -1037,6 +1042,9 @@ static int qc_parse_pkt_frms(struct quic_conn *qc, struct quic_rx_packet *pkt,
                }
        }
 
+       if (frm)
+               qc_frm_free(qc, &frm);
+
        if (fast_retrans && qc->iel && qc->hel) {
                struct quic_enc_level *iqel = qc->iel;
                struct quic_enc_level *hqel = qc->hel;
@@ -1069,6 +1077,8 @@ static int qc_parse_pkt_frms(struct quic_conn *qc, struct quic_rx_packet *pkt,
        return 1;
 
  err:
+       if (frm)
+               qc_frm_free(qc, &frm);
        TRACE_DEVEL("leaving on error", QUIC_EV_CONN_PRSHPKT, qc);
        return 0;
 }