]> git.ipfire.org Git - thirdparty/haproxy.git/commitdiff
MINOR: mux-quic: partially copy Rx frame if almost full buf
authorAmaury Denoyelle <adenoyelle@haproxy.com>
Tue, 26 Apr 2022 09:36:40 +0000 (11:36 +0200)
committerAmaury Denoyelle <adenoyelle@haproxy.com>
Thu, 28 Apr 2022 13:42:21 +0000 (15:42 +0200)
Improve the reception for STREAM frames. In qcc_recv(), if the frame is
bigger than the remaining space in rx buffer, do not reject it wholly.
Instead, copy as much data as possible. The rest of the data is
buffered.

This is necessary to handle H3 frames bigger than a buffer. The H3 code
does not demux until the frame is complete or the buffer is full.
Without this, the transfer on payload larger than the Rx buffer can
rapidly freeze.

include/haproxy/mux_quic.h
src/mux_quic.c
src/xprt_quic.c

index 233a210a6dcead4f1c300f83928b56f3a3fb54bb..b91ef57c7a7e43583d3e4ff55249a67aec447c47 100644 (file)
@@ -24,7 +24,7 @@ void qcs_notify_recv(struct qcs *qcs);
 void qcs_notify_send(struct qcs *qcs);
 
 int qcc_recv(struct qcc *qcc, uint64_t id, uint64_t len, uint64_t offset,
-             char fin, char *data, struct qcs **out_qcs);
+             char fin, char *data, struct qcs **out_qcs, size_t *done);
 int qcc_recv_max_data(struct qcc *qcc, uint64_t max);
 int qcc_recv_max_stream_data(struct qcc *qcc, uint64_t id, uint64_t max);
 int qcc_decode_qcs(struct qcc *qcc, struct qcs *qcs);
index 46681968e86b1ab4e7c5a70472d0897af855c7ac..17adf46611745f8a000a7a59db8bf1160644f99c 100644 (file)
@@ -333,19 +333,23 @@ struct qcs *qcc_get_qcs(struct qcc *qcc, uint64_t id)
  * <out_qcs>. In case of success, the caller can immediatly call qcc_decode_qcs
  * to process the frame content.
  *
- * Returns 0 on success. On errors, two codes are present.
- * - 1 is returned if the frame cannot be decoded and must be discarded.
- * - 2 is returned if the stream cannot decode at the moment the frame. The
- *   frame should be buffered to be handled later.
+ * Returns a code indicating how the frame was handled.
+ * - 0: frame received completly and can be dropped.
+ * - 1: frame not received but can be dropped.
+ * - 2: frame cannot be handled, either partially or not at all. <done>
+ *   indicated the number of bytes handled. The rest should be buffered.
  */
 int qcc_recv(struct qcc *qcc, uint64_t id, uint64_t len, uint64_t offset,
-             char fin, char *data, struct qcs **out_qcs)
+             char fin, char *data, struct qcs **out_qcs, size_t *done)
 {
        struct qcs *qcs;
        size_t total, diff;
 
        TRACE_ENTER(QMUX_EV_QCC_RECV, qcc->conn);
 
+       *out_qcs = NULL;
+       *done = 0;
+
        qcs = qcc_get_qcs(qcc, id);
        if (!qcs) {
                TRACE_DEVEL("leaving on stream not found", QMUX_EV_QCC_RECV|QMUX_EV_QCC_NQCS, qcc->conn, NULL, &id);
@@ -367,7 +371,7 @@ int qcc_recv(struct qcc *qcc, uint64_t id, uint64_t len, uint64_t offset,
        /* TODO initial max-stream-data overflow. Implement FLOW_CONTROL_ERROR emission. */
        BUG_ON(offset + len > qcs->rx.msd);
 
-       if (!qc_get_buf(qcs, &qcs->rx.buf)) {
+       if (!qc_get_buf(qcs, &qcs->rx.buf) || b_full(&qcs->rx.buf)) {
                /* TODO should mark qcs as full */
                return 2;
        }
@@ -375,27 +379,24 @@ int qcc_recv(struct qcc *qcc, uint64_t id, uint64_t len, uint64_t offset,
        TRACE_DEVEL("newly received offset", QMUX_EV_QCC_RECV|QMUX_EV_QCS_RECV, qcc->conn, qcs);
        diff = qcs->rx.offset - offset;
 
-       /* TODO do not partially copy a frame if not enough size left. Maybe
-        * this can be optimized.
-        */
-       if (len > b_room(&qcs->rx.buf)) {
-               /* TODO handle STREAM frames larger than RX buffer. */
-               BUG_ON(len > b_size(&qcs->rx.buf));
-               return 2;
-       }
-
        len -= diff;
        data += diff;
 
-       total = b_putblk(&qcs->rx.buf, data, len);
-       /* TODO handle partial copy of a STREAM frame. */
-       BUG_ON(len != total);
+       /* TODO handle STREAM frames larger than RX buffer. */
+       BUG_ON(len > b_size(&qcs->rx.buf));
 
+       total = b_putblk(&qcs->rx.buf, data, len);
        qcs->rx.offset += total;
+       *done = total;
 
        /* TODO initial max-stream-data reached. Implement MAX_STREAM_DATA emission. */
        BUG_ON(qcs->rx.offset == qcs->rx.msd);
 
+       if (total < len) {
+               TRACE_DEVEL("leaving on partially received offset", QMUX_EV_QCC_RECV|QMUX_EV_QCS_RECV, qcc->conn, qcs);
+               return 2;
+       }
+
        if (fin)
                qcs->flags |= QC_SF_FIN_RECV;
 
index 5691f63d9f0de05a3e7e42ab184c72297581bc06..773b62b0d0ee889a9f48d279e3ac4481aa7a3ece 100644 (file)
@@ -2139,11 +2139,12 @@ static int qc_handle_bidi_strm_frm(struct quic_rx_packet *pkt,
        struct quic_rx_strm_frm *frm;
        struct eb64_node *frm_node;
        struct qcs *qcs = NULL;
+       size_t done;
        int ret;
 
        ret = qcc_recv(qc->qcc, strm_frm->id, strm_frm->len,
                       strm_frm->offset.key, strm_frm->fin,
-                      (char *)strm_frm->data, &qcs);
+                      (char *)strm_frm->data, &qcs, &done);
 
        /* invalid or already received frame */
        if (ret == 1)
@@ -2160,13 +2161,22 @@ static int qc_handle_bidi_strm_frm(struct quic_rx_packet *pkt,
                        return 0;
                }
 
+               if (done) {
+                       BUG_ON(done >= frm->len); /* must never happen */
+                       frm->len -= done;
+                       frm->data += done;
+                       frm->offset_node.key += done;
+               }
+
                eb64_insert(&qcs->rx.frms, &frm->offset_node);
                quic_rx_packet_refinc(pkt);
 
-               return 1;
+               /* interrupt only if frame was not received at all. */
+               if (!done)
+                       return 1;
        }
 
-       /* Frame correctly received by the mux.
+       /* Frame received (partially or not) by the mux.
         * If there is buffered frame for next offset, it may be possible to
         * receive them now.
         */
@@ -2177,13 +2187,23 @@ static int qc_handle_bidi_strm_frm(struct quic_rx_packet *pkt,
 
                ret = qcc_recv(qc->qcc, qcs->id, frm->len,
                               frm->offset_node.key, frm->fin,
-                              (char *)frm->data, &qcs);
+                              (char *)frm->data, &qcs, &done);
 
-               /* interrupt the parsing if the frame cannot be handled for the
-                * moment only by the MUX.
+               /* interrupt the parsing if the frame cannot be handled
+                * entirely for the moment only.
                 */
-               if (ret == 2)
+               if (ret == 2) {
+                       if (done) {
+                               BUG_ON(done >= frm->len); /* must never happen */
+                               frm->len -= done;
+                               frm->data += done;
+
+                               eb64_delete(&frm->offset_node);
+                               frm->offset_node.key += done;
+                               eb64_insert(&qcs->rx.frms, &frm->offset_node);
+                       }
                        break;
+               }
 
                /* Remove a newly received frame or an invalid one. */
                frm_node = eb64_next(frm_node);