BUG/MEDIUM: mux-quic: Don't unblock zero-copy fwding if blocked during 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.

(cherry picked from commit 9748df2)
Signed-off-by: Amaury Denoyelle <[email protected]>

Author: capflam

include/haproxy/stconn-t.h

@@ -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 */

include/haproxy/stconn.h

@@ -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)) {

src/applet.c

@@ -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);
 	}
 
@@ -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);

src/mux_quic.c

@@ -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;
 	}