BUG/MEDIUM: mux-quic: Don't unblock zero-copy fwding if blocked during nego

The previous fix (792a645ec2 ["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 9748df29ff655a9626d6e75ea9db79bb9afa2a50)
Signed-off-by: Amaury Denoyelle <adenoyelle@haproxy.com>
This commit is contained in:
Christopher Faulet 2024-06-04 18:10:51 +02:00 committed by Amaury Denoyelle
parent f2384ec8fa
commit 96aef291cb
4 changed files with 9 additions and 7 deletions

View File

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

View File

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

View File

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

View File

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