MEDIUM: mux-quic: add BUG_ON if sending on locally closed QCS

Previously, if snd_buf operation was conducted despite QCS already
locally closed, the input buffer was silently dropped. This situation
could happen if a RESET_STREAM was emitted butemission not reported to
the stream layer. Resetting silently the buffer ensure QUIC MUX remain
compliant with RFC 9000 which forbid emission after RESET_STREAM.

Since previous commit, it is now ensured that RESET_STREAM sending will
always be reported to stream-layer. Thus, there is no need anymore to
silently reset the buffer. A BUG_ON() statement is added to ensure this
assumption will remain valid.

The new code is deemed cleaner as it does not hide a missing error
notification on the stconn-layer. Previously, if an error was missing,
sending would continue unnecessarily with a false success status
reported for the stream.

Note that the BUG_ON() statement was also added into nego_ff callback.
This is necessary to ensure both sending path remains consistent.

This patch is labelled as MEDIUM as issues were already encountered in
snd_buf/nego_ff implementation and it's not easy to cover all occurences
during test. If the BUG_ON() is triggered without any apparent
stream-layer issue, this commit should be reverted.
This commit is contained in:
Amaury Denoyelle 2023-12-19 11:25:18 +01:00
parent 0a69750a98
commit 235e8f1afd
3 changed files with 6 additions and 26 deletions

View File

@ -10,7 +10,6 @@ size_t qcs_http_rcv_buf(struct qcs *qcs, struct buffer *buf, size_t count,
char *fin);
size_t qcs_http_snd_buf(struct qcs *qcs, struct buffer *buf, size_t count,
char *fin);
size_t qcs_http_reset_buf(struct qcs *qcs, struct buffer *buf, size_t count);
#endif /* USE_QUIC */

View File

@ -2811,6 +2811,9 @@ static size_t qmux_strm_snd_buf(struct stconn *sc, struct buffer *buf,
TRACE_ENTER(QMUX_EV_STRM_SEND, qcs->qcc->conn, qcs);
/* Sending forbidden if QCS is locally closed (FIN or RESET_STREAM sent). */
BUG_ON(qcs_is_close_local(qcs) || (qcs->flags & QC_SF_TO_RESET));
/* stream layer has been detached so no transfer must occur after. */
BUG_ON_HOT(qcs->flags & QC_SF_DETACH);
@ -2821,11 +2824,6 @@ static size_t qmux_strm_snd_buf(struct stconn *sc, struct buffer *buf,
goto end;
}
if (qcs_is_close_local(qcs) || (qcs->flags & QC_SF_TO_RESET)) {
ret = qcs_http_reset_buf(qcs, buf, count);
goto end;
}
ret = qcs_http_snd_buf(qcs, buf, count, &fin);
if (fin) {
TRACE_STATE("reached stream fin", QMUX_EV_STRM_SEND, qcs->qcc->conn, qcs);
@ -2853,6 +2851,9 @@ static size_t qmux_strm_nego_ff(struct stconn *sc, struct buffer *input,
TRACE_ENTER(QMUX_EV_STRM_SEND, qcs->qcc->conn, qcs);
/* Sending forbidden if QCS is locally closed (FIN or RESET_STREAM sent). */
BUG_ON(qcs_is_close_local(qcs) || (qcs->flags & QC_SF_TO_RESET));
/* stream layer has been detached so no transfer must occur after. */
BUG_ON_HOT(qcs->flags & QC_SF_DETACH);

View File

@ -95,23 +95,3 @@ size_t qcs_http_snd_buf(struct qcs *qcs, struct buffer *buf, size_t count,
return ret;
}
/* QUIC MUX snd_buf reset. HTX data stored in <buf> of length <count> will be
* cleared. This can be used when data should not be transmitted any longer.
*
* Return the size in bytes of cleared data.
*/
size_t qcs_http_reset_buf(struct qcs *qcs, struct buffer *buf, size_t count)
{
struct htx *htx;
TRACE_ENTER(QMUX_EV_STRM_SEND, qcs->qcc->conn, qcs);
htx = htx_from_buf(buf);
htx_reset(htx);
htx_to_buf(htx, buf);
TRACE_LEAVE(QMUX_EV_STRM_SEND, qcs->qcc->conn, qcs);
return count;
}