From a57ab0fabe7aae9a67d03f3a1e87052cdef32efd Mon Sep 17 00:00:00 2001 From: Amaury Denoyelle Date: Wed, 26 Apr 2023 11:38:11 +0200 Subject: [PATCH] MINOR: mux-quic: do not allocate Tx buf for empty STREAM frame Sometimes it may be necessary to send an empty STREAM frame to signal clean stream closure with FIN bit set. Prior to this change, a Tx buffer was allocated unconditionnally even if no data is transferred. Most of the times, allocation was not performed due to an older buffer reused. But if data were already acknowledge, a new buffer is allocated. No memory leak occurs as the buffer is properly released when the empty frame acknowledge is received. But this allocation is unnecessary and it consumes a connexion Tx buffer for nothing. Improve this by skipping buffer allocation if no data to transfer. qcs_build_stream_frm() is now able to deal with a NULL out argument. This should be backported up to 2.6. --- src/mux_quic.c | 41 +++++++++++++++++++++-------------------- 1 file changed, 21 insertions(+), 20 deletions(-) diff --git a/src/mux_quic.c b/src/mux_quic.c index f4306e9056..4019de5b92 100644 --- a/src/mux_quic.c +++ b/src/mux_quic.c @@ -1489,9 +1489,10 @@ static int qcs_xfer_data(struct qcs *qcs, struct buffer *out, struct buffer *in) /* Prepare a STREAM frame for instance using as payload. The frame * is appended in . Set if this is supposed to be the last - * stream frame. + * stream frame. If is NULL an empty STREAM frame is built : this may be + * useful if FIN needs to be sent without any data left. * - * Returns the length of the STREAM frame or a negative error code. + * Returns the payload length of the STREAM frame or a negative error code. */ static int qcs_build_stream_frm(struct qcs *qcs, struct buffer *out, char fin, struct list *frm_list) @@ -1508,7 +1509,7 @@ static int qcs_build_stream_frm(struct qcs *qcs, struct buffer *out, char fin, BUG_ON(qcs->tx.sent_offset < base_off); head = qcs->tx.sent_offset - base_off; - total = b_data(out) - head; + total = out ? b_data(out) - head : 0; BUG_ON(total < 0); if (!total && !fin) { @@ -1812,21 +1813,21 @@ static int _qc_send_qcs(struct qcs *qcs, struct list *frms) /* Cannot send STREAM on remote unidirectional streams. */ BUG_ON(quic_stream_is_uni(qcs->id) && quic_stream_is_remote(qcc, qcs->id)); - /* Allocate buffer if necessary. */ - if (!out) { - if (qcc->flags & QC_CF_CONN_FULL) - goto out; - - out = qc_stream_buf_alloc(qcs->stream, qcs->tx.offset); - if (!out) { - TRACE_STATE("cannot allocate stream desc buffer", QMUX_EV_QCS_SEND, qcc->conn, qcs); - qcc->flags |= QC_CF_CONN_FULL; - goto out; - } - } - - /* Transfer data from to . */ if (b_data(buf)) { + /* Allocate buffer if not already done. */ + if (!out) { + if (qcc->flags & QC_CF_CONN_FULL) + goto out; + + out = qc_stream_buf_alloc(qcs->stream, qcs->tx.offset); + if (!out) { + TRACE_STATE("cannot allocate stream desc buffer", QMUX_EV_QCS_SEND, qcc->conn, qcs); + qcc->flags |= QC_CF_CONN_FULL; + goto out; + } + } + + /* Transfer data from to . */ xfer = qcs_xfer_data(qcs, out, buf); if (xfer > 0) { qcs_notify_send(qcs); @@ -1837,10 +1838,10 @@ static int _qc_send_qcs(struct qcs *qcs, struct list *frms) BUG_ON_HOT(qcs->tx.offset > qcs->tx.msd); qcc->tx.offsets += xfer; BUG_ON_HOT(qcc->tx.offsets > qcc->rfctl.md); - } - /* out buffer cannot be emptied if qcs offsets differ. */ - BUG_ON(!b_data(out) && qcs->tx.sent_offset != qcs->tx.offset); + /* out buffer cannot be emptied if qcs offsets differ. */ + BUG_ON(!b_data(out) && qcs->tx.sent_offset != qcs->tx.offset); + } /* FIN is set if all incoming data were transferred. */ fin = qcs_stream_fin(qcs);