From: Amaury Denoyelle Date: Thu, 6 Jun 2024 09:59:34 +0000 (+0200) Subject: MINOR: quic: refactor qc_build_pkt() error handling X-Git-Tag: v3.1-dev1~34 X-Git-Url: http://git.ipfire.org/?a=commitdiff_plain;h=88681681cc9c7c6b46f11949a7aa4418b61ab192;p=thirdparty%2Fhaproxy.git MINOR: quic: refactor qc_build_pkt() error handling qc_build_pkt() error handling was difficult due to multiple error code possible. Improve this by defining a proper enum to describe the various error code. Also clean up ending labels inside qc_build_pkt(). --- diff --git a/include/haproxy/quic_tx-t.h b/include/haproxy/quic_tx-t.h index 6979204859..efbdfe6870 100644 --- a/include/haproxy/quic_tx-t.h +++ b/include/haproxy/quic_tx-t.h @@ -56,4 +56,12 @@ struct quic_tx_packet { unsigned char type; }; +/* Return value for qc_build_pkt(). */ +enum qc_build_pkt_err { + QC_BUILD_PKT_ERR_NONE = 0, + QC_BUILD_PKT_ERR_ALLOC, /* memory allocation failure */ + QC_BUILD_PKT_ERR_ENCRYPT, /* error during encryption operation */ + QC_BUILD_PKT_ERR_BUFROOM, /* no more room in input buf or congestion window */ +}; + #endif /* _HAPROXY_TX_T_H */ diff --git a/src/quic_tx.c b/src/quic_tx.c index a21e61d54f..5887386424 100644 --- a/src/quic_tx.c +++ b/src/quic_tx.c @@ -32,7 +32,8 @@ static struct quic_tx_packet *qc_build_pkt(unsigned char **pos, const unsigned c struct quic_enc_level *qel, struct quic_tls_ctx *ctx, struct list *frms, struct quic_conn *qc, const struct quic_version *ver, size_t dglen, int pkt_type, - int must_ack, int padding, int probe, int cc, int *err); + int must_ack, int padding, int probe, int cc, + enum qc_build_pkt_err *err); static void quic_packet_encrypt(unsigned char *payload, size_t payload_len, unsigned char *aad, size_t aad_len, uint64_t pn, @@ -533,9 +534,10 @@ static int qc_prep_pkts(struct quic_conn *qc, struct buffer *buf, * of the first packet in the datagram (QUIC_DGRAM_HEADLEN). */ while (b_contig_space(buf) > QUIC_DGRAM_HEADLEN || prv_pkt) { - int err, probe, must_ack; + int probe, must_ack; enum quic_pkt_type pkt_type; struct quic_tx_packet *cur_pkt; + enum qc_build_pkt_err err; TRACE_PROTO("TX prep pkts", QUIC_EV_CONN_PHPKTS, qc, qel); probe = 0; @@ -601,29 +603,34 @@ static int qc_prep_pkts(struct quic_conn *qc, struct buffer *buf, qc, ver, dglen, pkt_type, must_ack, padding && !next_qel, probe, cc, &err); - switch (err) { - case -3: + if (!cur_pkt) { + switch (err) { + case QC_BUILD_PKT_ERR_ALLOC: if (first_pkt) qc_txb_store(buf, dglen, first_pkt); qc_purge_tx_buf(qc, buf); - goto leave; - case -2: + break; + + case QC_BUILD_PKT_ERR_ENCRYPT: // trace already emitted by function above - goto leave; - case -1: - /* If there was already a correct packet present, set the - * current datagram as prepared into . - */ - if (prv_pkt) + break; + + case QC_BUILD_PKT_ERR_BUFROOM: + if (first_pkt) qc_txb_store(buf, dglen, first_pkt); TRACE_PROTO("could not prepare anymore packet", QUIC_EV_CONN_PHPKTS, qc, qel); - goto out; + break; + default: + ABORT_NOW(); /* error case not handled */ break; + } + + if (err == QC_BUILD_PKT_ERR_ALLOC || err == QC_BUILD_PKT_ERR_ENCRYPT) + goto leave; + goto out; } - /* This is to please to GCC. We cannot have (err >= 0 && !cur_pkt) */ - BUG_ON(!cur_pkt); total += cur_pkt->len; dglen += cur_pkt->len; @@ -2051,12 +2058,8 @@ static inline void quic_tx_packet_init(struct quic_tx_packet *pkt, int type) * the end of this buffer, with as packet type for QUIC connection * at encryption level with list of prebuilt frames. * - * Return -3 if the packet could not be allocated, -2 if could not be encrypted for - * any reason, -1 if there was not enough room to build a packet. - * XXX NOTE XXX - * If you provide provide qc_build_pkt() with a big enough buffer to build a packet as big as - * possible (to fill an MTU), the unique reason why this function may fail is the congestion - * control window limitation. + * Return built packet instance or NULL on error. will be set to the + * specific error encountered. */ static struct quic_tx_packet *qc_build_pkt(unsigned char **pos, const unsigned char *end, @@ -2064,9 +2067,9 @@ static struct quic_tx_packet *qc_build_pkt(unsigned char **pos, struct quic_tls_ctx *tls_ctx, struct list *frms, struct quic_conn *qc, const struct quic_version *ver, size_t dglen, int pkt_type, int must_ack, - int padding, int probe, int cc, int *err) + int padding, int probe, int cc, + enum qc_build_pkt_err *err) { - struct quic_tx_packet *ret_pkt = NULL; /* The pointer to the packet number field. */ unsigned char *buf_pn; unsigned char *first_byte, *last_byte, *payload; @@ -2077,11 +2080,11 @@ static struct quic_tx_packet *qc_build_pkt(unsigned char **pos, TRACE_ENTER(QUIC_EV_CONN_TXPKT, qc); TRACE_PROTO("TX pkt build", QUIC_EV_CONN_TXPKT, qc, NULL, qel); - *err = 0; + *err = QC_BUILD_PKT_ERR_NONE; pkt = pool_alloc(pool_head_quic_tx_packet); if (!pkt) { TRACE_DEVEL("Not enough memory for a new packet", QUIC_EV_CONN_TXPKT, qc); - *err = -3; + *err = QC_BUILD_PKT_ERR_ALLOC; goto err; } @@ -2094,7 +2097,7 @@ static struct quic_tx_packet *qc_build_pkt(unsigned char **pos, if (!qc_do_build_pkt(*pos, end, dglen, pkt, pn, &pn_len, &buf_pn, must_ack, padding, cc, probe, qel, qc, ver, frms)) { // trace already emitted by function above - *err = -1; + *err = QC_BUILD_PKT_ERR_BUFROOM; goto err; } @@ -2107,7 +2110,7 @@ static struct quic_tx_packet *qc_build_pkt(unsigned char **pos, if (encrypt_failure) { /* TODO Unrecoverable failure, unencrypted data should be returned to the caller. */ WARN_ON("quic_packet_encrypt failure"); - *err = -2; + *err = QC_BUILD_PKT_ERR_ENCRYPT; goto err; } @@ -2117,7 +2120,7 @@ static struct quic_tx_packet *qc_build_pkt(unsigned char **pos, if (encrypt_failure) { /* TODO Unrecoverable failure, unencrypted data should be returned to the caller. */ WARN_ON("quic_apply_header_protection failure"); - *err = -2; + *err = QC_BUILD_PKT_ERR_ENCRYPT; goto err; } @@ -2154,18 +2157,15 @@ static struct quic_tx_packet *qc_build_pkt(unsigned char **pos, pkt->pktns = qel->pktns; - ret_pkt = pkt; - leave: - TRACE_PROTO("TX pkt built", QUIC_EV_CONN_TXPKT, qc, ret_pkt); + TRACE_PROTO("TX pkt built", QUIC_EV_CONN_TXPKT, qc, pkt); TRACE_LEAVE(QUIC_EV_CONN_TXPKT, qc); - return ret_pkt; + return pkt; err: - /* TODO: what about the frames which have been built - * for this packet. - */ + /* TODO What about the frames which have been built for this packet? */ free_quic_tx_packet(qc, pkt); - goto leave; + TRACE_DEVEL("leaving on error", QUIC_EV_CONN_TXPKT, qc); + return NULL; } /* * Local variables: