From: Amaury Denoyelle Date: Mon, 29 Jul 2024 15:01:38 +0000 (+0200) Subject: MINOR: mux-quic: retry after small buf alloc failure X-Git-Tag: v3.1-dev6~18 X-Git-Url: http://git.ipfire.org/?a=commitdiff_plain;h=0d6112b40b20f9eea0cf38dc304bc1155b673588;p=thirdparty%2Fhaproxy.git MINOR: mux-quic: retry after small buf alloc failure Previous commit switch to small buffers for HTTP/3 HEADERS emission. This ensures that several parallel streams can allocate their own buffer without hitting the connection buffer limit based now on the congestion window size. However, this prevents the transmission of responses with uncommonly large headers. Indeed, if all headers cannot be encoded in a single buffer, an error is reported which cause the whole connection closure. Adjust this by implementing a realloc API exposed by QUIC MUX. This allows application layer to switch from a small to a default buffer and restart its processing. This guarantees that again headers not longer than bufsize can be properly transferred. --- diff --git a/include/haproxy/mux_quic.h b/include/haproxy/mux_quic.h index 83f54f5074..fd6c09024b 100644 --- a/include/haproxy/mux_quic.h +++ b/include/haproxy/mux_quic.h @@ -27,6 +27,7 @@ void qcc_notify_buf(struct qcc *qcc, uint64_t free_size); struct buffer *qcc_get_stream_rxbuf(struct qcs *qcs); struct buffer *qcc_get_stream_txbuf(struct qcs *qcs, int *err, int small); +struct buffer *qcc_realloc_stream_txbuf(struct qcs *qcs); int qcc_realign_stream_txbuf(const struct qcs *qcs, struct buffer *out); int qcc_release_stream_txbuf(struct qcs *qcs); int qcc_stream_can_send(const struct qcs *qcs); diff --git a/include/haproxy/quic_stream.h b/include/haproxy/quic_stream.h index 4417ee2c92..11c4878365 100644 --- a/include/haproxy/quic_stream.h +++ b/include/haproxy/quic_stream.h @@ -17,6 +17,7 @@ void qc_stream_desc_free(struct qc_stream_desc *stream, int closing); struct buffer *qc_stream_buf_get(struct qc_stream_desc *stream); struct buffer *qc_stream_buf_alloc(struct qc_stream_desc *stream, uint64_t offset, int small); +struct buffer *qc_stream_buf_realloc(struct qc_stream_desc *stream); void qc_stream_buf_release(struct qc_stream_desc *stream); #endif /* USE_QUIC */ diff --git a/src/h3.c b/src/h3.c index 17afa1edbd..9cc7aa274e 100644 --- a/src/h3.c +++ b/src/h3.c @@ -1562,6 +1562,7 @@ static int h3_resp_headers_send(struct qcs *qcs, struct htx *htx) struct htx_blk *blk; enum htx_blk_type type; int frame_length_size; /* size in bytes of frame length varint field */ + int smallbuf = 1; int ret = 0; int hdr; int status = 0; @@ -1606,7 +1607,10 @@ static int h3_resp_headers_send(struct qcs *qcs, struct htx *htx) list[hdr].n = ist(""); - if (!(res = qcc_get_stream_txbuf(qcs, &err, 1))) { + retry: + res = smallbuf ? qcc_get_stream_txbuf(qcs, &err, 1) : + qcc_realloc_stream_txbuf(qcs); + if (!res) { if (err) { TRACE_ERROR("cannot allocate Tx buffer", H3_EV_TX_FRAME|H3_EV_TX_HDR, qcs->qcc->conn, qcs); goto err; @@ -1627,11 +1631,11 @@ static int h3_resp_headers_send(struct qcs *qcs, struct htx *htx) TRACE_DATA("encoding HEADERS frame", H3_EV_TX_FRAME|H3_EV_TX_HDR, qcs->qcc->conn, qcs); if (qpack_encode_field_section_line(&headers_buf)) - goto err; + goto err_full; if (qpack_encode_int_status(&headers_buf, status)) { /* TODO handle invalid status code VS no buf space left */ TRACE_ERROR("error during status code encoding", H3_EV_TX_FRAME|H3_EV_TX_HDR, qcs->qcc->conn, qcs); - goto err; + goto err_full; } for (hdr = 0; hdr < sizeof(list) / sizeof(list[0]); ++hdr) { @@ -1662,7 +1666,7 @@ static int h3_resp_headers_send(struct qcs *qcs, struct htx *htx) } if (qpack_encode_header(&headers_buf, list[hdr].n, list[hdr].v)) - goto err; + goto err_full; } /* Now that all headers are encoded, we are certain that res buffer is @@ -1688,6 +1692,12 @@ static int h3_resp_headers_send(struct qcs *qcs, struct htx *htx) TRACE_LEAVE(H3_EV_TX_FRAME|H3_EV_TX_HDR, qcs->qcc->conn, qcs); return ret; + err_full: + if (smallbuf) { + TRACE_DEVEL("retry with a full buffer", H3_EV_TX_FRAME|H3_EV_TX_HDR, qcs->qcc->conn, qcs); + smallbuf = 0; + goto retry; + } err: TRACE_DEVEL("leaving on error", H3_EV_TX_FRAME|H3_EV_TX_HDR, qcs->qcc->conn, qcs); return -1; diff --git a/src/mux_quic.c b/src/mux_quic.c index 7b25bba6c5..a89431b89d 100644 --- a/src/mux_quic.c +++ b/src/mux_quic.c @@ -1080,6 +1080,43 @@ struct buffer *qcc_get_stream_txbuf(struct qcs *qcs, int *err, int small) return out; } +/* Reallocate stream buffer to convert a small buffer to a bigger one. + * Contrary to standard allocation, this function will never stop due to a full + * buffer window. The smaller buffer is released first which guarantee that the + * buffer window has room left. + * + * Returns buffer pointer or NULL on allocation failure. + */ +struct buffer *qcc_realloc_stream_txbuf(struct qcs *qcs) +{ + struct qcc *qcc = qcs->qcc; + struct buffer *out = qc_stream_buf_get(qcs->stream); + const int unlimited = qcs->stream->flags & QC_SD_FL_OOB_BUF; + + /* Stream must not try to reallocate a buffer if currently waiting for one. */ + BUG_ON(LIST_INLIST(&qcs->el_buf)); + + if (likely(!unlimited)) { + /* Reduce buffer window. As such there is always some space + * left for a new buffer allocation. + */ + BUG_ON(qcc->tx.buf_in_flight < b_size(out)); + qcc->tx.buf_in_flight -= b_size(out); + } + + out = qc_stream_buf_realloc(qcs->stream); + if (!out) { + TRACE_ERROR("buffer alloc failure", QMUX_EV_QCS_SEND, qcc->conn, qcs); + goto out; + } + + if (likely(!unlimited)) + qcc->tx.buf_in_flight += b_size(out); + + out: + return out && b_size(out) ? out : NULL; +} + /* Returns total number of bytes not already sent to quic-conn layer. */ static uint64_t qcs_prep_bytes(const struct qcs *qcs) { diff --git a/src/quic_stream.c b/src/quic_stream.c index 5f1f1f4c53..bf9a54e620 100644 --- a/src/quic_stream.c +++ b/src/quic_stream.c @@ -315,6 +315,31 @@ struct buffer *qc_stream_buf_alloc(struct qc_stream_desc *stream, return &stream->buf->buf; } +/* Free current buffer and allocate a new one. This function is reserved + * to convert a small buffer to a standard one. + * + * Returns the buffer or NULL on error. + */ +struct buffer *qc_stream_buf_realloc(struct qc_stream_desc *stream) +{ + /* This function is reserved to convert a big buffer to a smaller one. */ + BUG_ON(!stream->buf || !stream->buf->sbuf); + + /* Release buffer */ + pool_free(pool_head_sbuf, stream->buf->buf.area); + stream->buf->buf = BUF_NULL; + stream->buf->sbuf = 0; + + if (!b_alloc(&stream->buf->buf, DB_MUX_TX)) { + LIST_DEL_INIT(&stream->buf->list); + pool_free(pool_head_quic_stream_buf, stream->buf); + stream->buf = NULL; + return NULL; + } + + return &stream->buf->buf; +} + /* Release the current buffer of . It will be kept internally by * the . The current buffer cannot be NULL. */