Skip to content

Commit 0a360df

Browse files
a-denoyellecapflam
authored andcommitted
BUG/MINOR: quic: ensure Tx buf is always purged
quic_conn API for sending was recently refactored. The main objective was to regroup the different functions present for both handshake and application emission. After this refactoring, an optimization was introduced to avoid calling qc_send() if there was nothing new to emit. However, this prevent the Tx buffer to be purged if previous sending was interrupted, until new frames are finally available. To fix this, simply remove the optimization. qc_send() is thus now always called in quic_conn IO handlers. The impact of this bug should be minimal as it happens only on sending temporary error. However in this case, this could cause extra latency or even a complete sending freeze in the worst scenario. This must be backported up to 3.0. (cherry picked from commit 0ef94e2) Signed-off-by: Christopher Faulet <[email protected]>
1 parent 8728d68 commit 0a360df

File tree

2 files changed

+11
-14
lines changed

2 files changed

+11
-14
lines changed

src/quic_conn.c

Lines changed: 3 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -551,11 +551,8 @@ struct task *quic_conn_app_io_cb(struct task *t, void *context, unsigned int sta
551551
{
552552
struct list send_list = LIST_HEAD_INIT(send_list);
553553
struct quic_conn *qc = context;
554-
struct quic_enc_level *qel;
555554

556555
TRACE_ENTER(QUIC_EV_CONN_IO_CB, qc);
557-
558-
qel = qc->ael;
559556
TRACE_STATE("connection handshake state", QUIC_EV_CONN_IO_CB, qc, &qc->state);
560557

561558
if (qc_test_fd(qc))
@@ -594,11 +591,10 @@ struct task *quic_conn_app_io_cb(struct task *t, void *context, unsigned int sta
594591
goto out;
595592
}
596593

597-
if (!qel_need_sending(qel, qc))
598-
goto out;
599-
600594
/* XXX TODO: how to limit the list frames to send */
601-
qel_register_send(&send_list, qel, &qel->pktns->tx.frms);
595+
if (qel_need_sending(qc->ael, qc))
596+
qel_register_send(&send_list, qc->ael, &qc->ael->pktns->tx.frms);
597+
602598
if (!qc_send(qc, 0, &send_list)) {
603599
TRACE_DEVEL("qc_send() failed", QUIC_EV_CONN_IO_CB, qc);
604600
goto out;
@@ -804,10 +800,6 @@ struct task *quic_conn_io_cb(struct task *t, void *context, unsigned int state)
804800
qel_register_send(&send_list, qel, &qel->pktns->tx.frms);
805801
}
806802

807-
/* Skip sending if no QEL with frames to sent. */
808-
if (LIST_ISEMPTY(&send_list))
809-
goto out;
810-
811803
if (!qc_send(qc, 0, &send_list)) {
812804
TRACE_DEVEL("qc_send() failed", QUIC_EV_CONN_IO_CB, qc);
813805
goto out;

src/quic_tx.c

Lines changed: 8 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -691,7 +691,7 @@ static int qc_prep_pkts(struct quic_conn *qc, struct buffer *buf,
691691
int qc_send(struct quic_conn *qc, int old_data, struct list *send_list)
692692
{
693693
struct quic_enc_level *qel, *tmp_qel;
694-
int ret, status = 0;
694+
int ret = 0, status = 0;
695695
struct buffer *buf;
696696

697697
TRACE_ENTER(QUIC_EV_CONN_TXPKT, qc);
@@ -713,7 +713,7 @@ int qc_send(struct quic_conn *qc, int old_data, struct list *send_list)
713713
}
714714

715715
/* Prepare and send packets until we could not further prepare packets. */
716-
do {
716+
while (!LIST_ISEMPTY(send_list)) {
717717
/* Buffer must always be empty before qc_prep_pkts() usage.
718718
* qc_send_ppkts() ensures it is cleared on success.
719719
*/
@@ -727,7 +727,12 @@ int qc_send(struct quic_conn *qc, int old_data, struct list *send_list)
727727
qc_txb_release(qc);
728728
goto out;
729729
}
730-
} while (ret > 0 && !LIST_ISEMPTY(send_list));
730+
731+
if (ret <= 0) {
732+
TRACE_DEVEL("stopping on qc_prep_pkts() return", QUIC_EV_CONN_TXPKT, qc);
733+
break;
734+
}
735+
}
731736

732737
qc_txb_release(qc);
733738
if (ret < 0)

0 commit comments

Comments
 (0)