Skip to content

Commit

Permalink
BUG/MEDIUM: mux-quic: Don't unblock zero-copy fwding if blocked durin…
Browse files Browse the repository at this point in the history
…g nego

The previous fix (792a645 ["BUG/MEDIUM: mux-quic: Unblock zero-copy
forwarding if the txbuf can be released"]) introduced a regression. The
zero-copy data forwarding must only be unblocked if it was blocked by the
producer, after a successful negotiation.

It is important because during a negotiation, the consumer may be blocked
for another reason. Because of the flow control for instance. In that case,
there is not necessarily a TX buffer. And it unexpected to try to release an
unallocated TX buf.

In addition, the same may happen while a TX buf is still in-use. In that
case, it must also not be released. So testing the TX buffer is not the
right solution.

To fix the issue, a new IOBUF flag was added (IOBUF_FL_FF_WANT_ROOM). It
must be set by the producer if it is blocked after a sucessful negotiation
because it needs more room. In that case, we know a buffer was provided by
the consummer. In done_fastfwd() callback function, it is then possible to
safely unblock the zero-copy data forwarding if this flag is set.

This patch must be backported to 3.0 with the commit above.
  • Loading branch information
capflam committed Jun 5, 2024
1 parent 2bde0d6 commit 9748df2
Show file tree
Hide file tree
Showing 4 changed files with 9 additions and 7 deletions.
1 change: 1 addition & 0 deletions include/haproxy/stconn-t.h
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,7 @@ enum iobuf_flags {
* .done_fastfwd() on consumer side must take care of this flag
*/
IOBUF_FL_EOI = 0x00000010, /* A EOI was encountered on producer side */
IOBUF_FL_FF_WANT_ROOM = 0x00000020, /* Producer need more room in the IOBUF to forward data */
};

/* Flags used */
Expand Down
2 changes: 1 addition & 1 deletion include/haproxy/stconn.h
Original file line number Diff line number Diff line change
Expand Up @@ -474,7 +474,7 @@ static inline size_t se_nego_ff(struct sedesc *se, struct buffer *input, size_t
if (se_fl_test(se, SE_FL_T_MUX)) {
const struct mux_ops *mux = se->conn->mux;

se->iobuf.flags &= ~IOBUF_FL_FF_BLOCKED;
se->iobuf.flags &= ~(IOBUF_FL_FF_BLOCKED|IOBUF_FL_FF_WANT_ROOM);
if (mux->nego_fastfwd && mux->done_fastfwd) {
/* Disable zero-copy forwarding if EOS or an error was reported. */
if (se_fl_test(se, SE_FL_EOS|SE_FL_ERROR|SE_FL_ERR_PENDING)) {
Expand Down
4 changes: 2 additions & 2 deletions src/applet.c
Original file line number Diff line number Diff line change
Expand Up @@ -694,7 +694,7 @@ int appctx_fastfwd(struct stconn *sc, unsigned int count, unsigned int flags)

if (se_fl_test(appctx->sedesc, SE_FL_WANT_ROOM)) {
/* The applet request more room, report the info at the iobuf level */
sdo->iobuf.flags |= IOBUF_FL_FF_BLOCKED;
sdo->iobuf.flags |= (IOBUF_FL_FF_BLOCKED|IOBUF_FL_FF_WANT_ROOM);
TRACE_STATE("waiting for more room", APPLET_EV_RECV|APPLET_EV_BLK, appctx);
}

Expand All @@ -716,7 +716,7 @@ int appctx_fastfwd(struct stconn *sc, unsigned int count, unsigned int flags)
/* else */
/* applet_have_more_data(appctx); */

if (se_done_ff(sdo) != 0 || !(sdo->iobuf.flags & IOBUF_FL_FF_BLOCKED)) {
if (se_done_ff(sdo) != 0 || !(sdo->iobuf.flags & (IOBUF_FL_FF_BLOCKED|IOBUF_FL_FF_WANT_ROOM))) {
/* Something was forwarding or the consumer states it is not
* blocked anyore, don't reclaim more room */
se_fl_clr(appctx->sedesc, SE_FL_WANT_ROOM);
Expand Down
9 changes: 5 additions & 4 deletions src/mux_quic.c
Original file line number Diff line number Diff line change
Expand Up @@ -3049,11 +3049,12 @@ static size_t qmux_strm_done_ff(struct stconn *sc)
if (!(qcs->flags & QC_SF_FIN_STREAM) && !sd->iobuf.data) {
TRACE_STATE("no data sent", QMUX_EV_STRM_SEND, qcs->qcc->conn, qcs);

/* There is nothing to forward and the SD is blocked. Try to
* release the TXBUF to retry.
/* There is nothing to forward and the SD was blocked after a
* successful nego by the producer. We can try to release the
* TXBUF to retry. In this case, the TX buf MUST exist.
*/
if ((qcs->sd->iobuf.flags & IOBUF_FL_FF_BLOCKED) && !qcc_release_stream_txbuf(qcs))
qcs->sd->iobuf.flags &= ~IOBUF_FL_FF_BLOCKED;
if ((qcs->sd->iobuf.flags & IOBUF_FL_FF_WANT_ROOM) && !qcc_release_stream_txbuf(qcs))
qcs->sd->iobuf.flags &= ~(IOBUF_FL_FF_BLOCKED|IOBUF_FL_FF_WANT_ROOM);
goto end;
}

Expand Down

0 comments on commit 9748df2

Please sign in to comment.