From 0a69750a98bbf4c8d9ab2c84e3890956116dbfa0 Mon Sep 17 00:00:00 2001 From: Amaury Denoyelle Date: Tue, 19 Dec 2023 11:22:28 +0100 Subject: [PATCH] BUG/MINOR: mux-quic: always report error to SC on RESET_STREAM emission On RESET_STREAM emission, the stream Tx channel is closed. This event must be reported to stream-conn layer to interrupt future send operations. Previously, se_fl_set_error() was manually invocated before/after qcc_reset_stream(). Change this by moving se_fl_set_error() invocation into the latter. This ensures that notification won't be forget, most notably in HTTP/3 layer. In most cases, behavior should be identical as both functions were called together unless not necessary. However, there is one exception which could cause a RESET_STREAM emission without error notification : this happens on H3 trailers parsing error. All other H3 errors happen before the stream-layer creation and thus the error is notified on stream creation. This regression has been caused by the following patch : 152beeec34baed98ad4c186454ddb25e4c496b50 MINOR: mux-quic: report error on stream-endpoint earlier Thus it should be backported up to 2.7. Note that the case described above did not cause any crash or protocol error. This is because currently MUX QUIC snd_buf operation silently reset buffer on transmission if QCS is already closed locally. This will however be removed in a future commit so the current patch is necessary to prevent an invalid behavior. --- src/mux_quic.c | 30 +++++++++++++++--------------- 1 file changed, 15 insertions(+), 15 deletions(-) diff --git a/src/mux_quic.c b/src/mux_quic.c index 98d407697..bf3616e0c 100644 --- a/src/mux_quic.c +++ b/src/mux_quic.c @@ -957,6 +957,12 @@ void qcc_reset_stream(struct qcs *qcs, int err) qcs->tx.offset = qcs->tx.sent_offset; } + /* Report send error to stream-endpoint layer. */ + if (qcs_sc(qcs)) { + se_fl_set_error(qcs->sd); + qcs_alert(qcs); + } + qcc_send_stream(qcs, 1); tasklet_wakeup(qcc->wait_event.tasklet); } @@ -1410,6 +1416,14 @@ int qcc_recv_stop_sending(struct qcc *qcc, uint64_t id, uint64_t err) } } + /* If FIN already reached, future RESET_STREAMS will be ignored. + * Manually set EOS in this case. + */ + if (qcs_sc(qcs) && se_fl_test(qcs->sd, SE_FL_EOI)) { + se_fl_set(qcs->sd, SE_FL_EOS); + qcs_alert(qcs); + } + /* RFC 9000 3.5. Solicited State Transitions * * An endpoint that receives a STOP_SENDING frame @@ -1426,18 +1440,6 @@ int qcc_recv_stop_sending(struct qcc *qcc, uint64_t id, uint64_t err) */ qcc_reset_stream(qcs, err); - /* Report send error to stream-endpoint layer. */ - if (qcs_sc(qcs)) { - /* If FIN already reached, future RESET_STREAMS will be ignored. - * Manually set EOS in this case. - */ - if (se_fl_test(qcs->sd, SE_FL_EOI)) - se_fl_set(qcs->sd, SE_FL_EOS); - - se_fl_set_error(qcs->sd); - qcs_alert(qcs); - } - if (qcc_may_expire(qcc) && !qcc->nb_hreq) qcc_refresh_timeout(qcc); @@ -3007,9 +3009,7 @@ static void qmux_strm_shutw(struct stconn *sc, enum co_shw_mode mode) } else { /* RESET_STREAM necessary. */ - if (!(qcc->flags & (QC_CF_ERR_CONN|QC_CF_ERRL))) - qcc_reset_stream(qcs, 0); - se_fl_set_error(qcs->sd); + qcc_reset_stream(qcs, 0); } tasklet_wakeup(qcc->wait_event.tasklet);