]> git.ipfire.org Git - thirdparty/haproxy.git/commitdiff
MINOR: mux-quic: retry after small buf alloc failure
authorAmaury Denoyelle <adenoyelle@haproxy.com>
Mon, 29 Jul 2024 15:01:38 +0000 (17:01 +0200)
committerAmaury Denoyelle <adenoyelle@haproxy.com>
Tue, 20 Aug 2024 16:12:27 +0000 (18:12 +0200)
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.

include/haproxy/mux_quic.h
include/haproxy/quic_stream.h
src/h3.c
src/mux_quic.c
src/quic_stream.c

index 83f54f50748fc550016899cf581285a9e7745207..fd6c09024b2e7fd90a91523cde56b1e4588f6587 100644 (file)
@@ -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);
index 4417ee2c92256ad99fce71dda76b88b2788844d0..11c4878365e1534aa1a28d4733313f1238832713 100644 (file)
@@ -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 */
index 17afa1edbdf7b7df0bf90fcb35e5fddc42d0ca74..9cc7aa274e83f5caf2ad1128a1c3f14f2031c3a0 100644 (file)
--- 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;
index 7b25bba6c555f5700d867b96ec6e6ab79e7307cb..a89431b89dc92f94a6e81d79ca0699fc130daca9 100644 (file)
@@ -1080,6 +1080,43 @@ struct buffer *qcc_get_stream_txbuf(struct qcs *qcs, int *err, int small)
        return out;
 }
 
+/* Reallocate <qcs> 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)
 {
index 5f1f1f4c532a4070625492558484a18435693866..bf9a54e620c0a6b36a640e8ad379667a06621671 100644 (file)
@@ -315,6 +315,31 @@ struct buffer *qc_stream_buf_alloc(struct qc_stream_desc *stream,
        return &stream->buf->buf;
 }
 
+/* Free current <stream> 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 <stream>. It will be kept internally by
  * the <stream>. The current buffer cannot be NULL.
  */