MINOR: mux-quic: split STREAM and RS/SS emission

This commit is a follow-up of the previous one which defines function
qcc_build_frms(). This function implements looping over qcc send_list,
to both encode and send individually any STOP_SENDING and RESET_STREAM,
but also encode STREAM frames as a preparator step. STREAM frames were
then sent as a list outside of qcc_build_frms() via qcc_send_frames().

Extract STOP_SENDING/RESET_STREAM encoding and emission step into a new
function qcc_emit_rs_ss(). The code is thus cleaner. In particular it
highlights that an error during STOP_SENDING/RESET_STREAM emission stage
is fatal and prevent any STREAM frames processing.

This should be backported up to 3.1.

diff --git a/src/mux_quic.c b/src/mux_quic.c
index b48def292b32454f1732b6fcb433bdb2c34cd380..26e21349a82d889e9d61adb906268ea486062c16 100644 (file)
--- a/src/mux_quic.c
+++ b/src/mux_quic.c
@@ -1414,6 +1414,9 @@ void qcc_send_stream(struct qcs *qcs, int urg, int count)
        BUG_ON(qcs_is_close_local(qcs));
 
        if (urg) {
+               /* qcc_emit_rs_ss() relies on resetted/aborted streams in send_list front. */
+               BUG_ON(!(qcs->flags & (QC_SF_TO_RESET|QC_SF_TO_STOP_SENDING|QC_SF_TXBUB_OOB)));
+
                LIST_DEL_INIT(&qcs->el_send);
                LIST_INSERT(&qcc->send_list, &qcs->el_send);
        }
@@ -2298,50 +2301,46 @@ static int qcs_send(struct qcs *qcs, struct list *frms, uint64_t window_conn)
        return -1;
 }
 
-/* Encode STREAM frames into <qcc> tx frms for streams registered into
- * send_list. On each error, related stream is removed from send_list and
- * inserted into <qcs_failed> list.
- *
- * This functions also serves to emit RESET_STREAM and STOP_SENDING frames. In
- * this case, frame is emitted immediately without using <qcc> tx frms. If an
- * error occured during this step, this is considered as fatal. Tx frms is
- * cleared and 0 is returned.
+/* Send RESET_STREAM/STOP_SENDING for streams in <qcc> send_list if requested.
+ * Each frame is encoded and emitted separately for now. If a frame cannot be
+ * sent, send_list looping is interrupted.
  *
- * Returns the sum of encoded STREAM frames length or 0 if no frame built.
+ * Returns 0 on success else non-zero.
  */
-static int qcc_build_frms(struct qcc *qcc, struct list *qcs_failed)
+static int qcc_emit_rs_ss(struct qcc *qcc)
 {
-       struct list *frms = &qcc->tx.frms;
-       struct qcs *qcs, *qcs_tmp, *first_qcs = NULL;
-       uint64_t window_conn = qfctl_rcap(&qcc->tx.fc);
-       int ret = 0, total = 0;
+       struct qcs *qcs, *qcs_tmp;
 
        TRACE_ENTER(QMUX_EV_QCC_END, qcc->conn);
 
-       /* Frames list must first be cleared via qcc_clear_frms(). */
-       BUG_ON(!LIST_ISEMPTY(&qcc->tx.frms));
-
        list_for_each_entry_safe(qcs, qcs_tmp, &qcc->send_list, el_send) {
-               /* Check if all QCS were processed. */
-               if (qcs == first_qcs)
-                       break;
-
                /* Stream must not be present in send_list if it has nothing to send. */
                BUG_ON(!(qcs->flags & (QC_SF_FIN_STREAM|QC_SF_TO_STOP_SENDING|QC_SF_TO_RESET)) &&
                       (!qcs->stream || !qcs_prep_bytes(qcs)));
 
-               /* Each STOP_SENDING/RESET_STREAM frame is sent individually to
-                * guarantee its emission.
+               /* Interrupt looping for the first stream where no RS nor SS is
+                * necessary and is not use for "metadata" transfer. These
+                * streams are always in front of the send_list.
+                */
+               if (!(qcs->flags & (QC_SF_TO_STOP_SENDING|QC_SF_TO_RESET|QC_SF_TXBUB_OOB)))
+                       break;
+
+               TRACE_DATA("prepare for RS/SS transfer", QMUX_EV_QCC_SEND, qcc->conn, qcs);
+
+               /* Each RS and SS frame is sent individually. Necessary to
+                * ensure it has been emitted as there is no transport callback
+                * for now.
                 *
-                * TODO multiplex several frames in same datagram to optimize sending
+                * TODO multiplex frames to optimize sending. However, it may
+                * not be advisable to mix different streams in the same dgram
+                * to avoid interdependency in case of loss.
                 */
+
                if (qcs->flags & QC_SF_TO_STOP_SENDING) {
                        if (qcs_send_stop_sending(qcs))
                                goto err;
 
-                       /* Remove stream from send_list if it had only STOP_SENDING
-                        * to send.
-                        */
+                       /* Remove stream from send_list if only SS was necessary. */
                        if (!(qcs->flags & (QC_SF_FIN_STREAM|QC_SF_TO_RESET)) &&
                            (!qcs->stream || !qcs_prep_bytes(qcs))) {
                                LIST_DEL_INIT(&qcs->el_send);
@@ -2367,6 +2366,50 @@ static int qcc_build_frms(struct qcc *qcc, struct list *qcs_failed)
                        }
                        continue;
                }
+       }
+
+       TRACE_LEAVE(QMUX_EV_QCC_SEND, qcc->conn);
+       return 0;
+
+ err:
+       TRACE_DEVEL("leaving on error", QMUX_EV_QCC_SEND, qcc->conn);
+       return 1;
+}
+
+/* Encode STREAM frames into <qcc> tx frms for streams registered into
+ * send_list. On each error, related stream is removed from send_list and
+ * inserted into <qcs_failed> list.
+ *
+ * This functions also serves to emit RESET_STREAM and STOP_SENDING frames. In
+ * this case, frame is emitted immediately without using <qcc> tx frms. If an
+ * error occured during this step, this is considered as fatal. Tx frms is
+ * cleared and 0 is returned.
+ *
+ * Returns the sum of encoded STREAM frames length or 0 if no frame built.
+ */
+static int qcc_build_frms(struct qcc *qcc, struct list *qcs_failed)
+{
+       struct list *frms = &qcc->tx.frms;
+       struct qcs *qcs, *qcs_tmp, *first_qcs = NULL;
+       uint64_t window_conn = qfctl_rcap(&qcc->tx.fc);
+       int ret = 0, total = 0;
+
+       TRACE_ENTER(QMUX_EV_QCC_END, qcc->conn);
+
+       /* Frames list must first be cleared via qcc_clear_frms(). */
+       BUG_ON(!LIST_ISEMPTY(&qcc->tx.frms));
+
+       list_for_each_entry_safe(qcs, qcs_tmp, &qcc->send_list, el_send) {
+               /* Check if all QCS were processed. */
+               if (qcs == first_qcs)
+                       break;
+
+               TRACE_DATA("prepare for data transfer", QMUX_EV_QCC_SEND, qcc->conn, qcs);
+
+               /* Streams with RS/SS must be handled via qcc_emit_rs_ss(). */
+               BUG_ON(qcs->flags & (QC_SF_TO_STOP_SENDING|QC_SF_TO_RESET));
+               /* Stream must not be present in send_list if it has nothing to send. */
+               BUG_ON(!(qcs->flags & QC_SF_FIN_STREAM) && (!qcs->stream || !qcs_prep_bytes(qcs)));
 
                /* Total sent bytes must not exceed connection window. */
                BUG_ON(total > window_conn);
@@ -2457,7 +2500,12 @@ static int qcc_io_send(struct qcc *qcc)
                }
        }
 
-       /* Send STREAM/STOP_SENDING/RESET_STREAM data for registered streams. */
+       if (qcc_emit_rs_ss(qcc)) {
+               TRACE_DEVEL("emission interrupted on STOP_SENDING/RESET_STREAM send error", QMUX_EV_QCC_SEND, qcc->conn);
+               goto out;
+       }
+
+       /* Encode STREAM frames for registered streams. */
        total = qcc_build_frms(qcc, &qcs_failed);
        if (!total)
                goto sent_done;