BUG/MINOR: quic: transform qc_set_timer() as a reentrant function
qc_set_timer() function is used to rearm the timer for loss detection
and probing. Previously, timer was always rearm when congestion window
was free due to a wrong interpretation of the RFC which mandates the
client to rearm the timer before handshake completion to avoid a
deadlock related to anti-amplification.
Fix this by removing this code from quic_pto_pktns(). This allows
qc_set_timer() to be reentrant and only activate the timer if needed.
The impact of this bug seems limited. It can probably caused the timer
task to be processed too frequently which could caused too frequent
probing.
This change will allow to reuse easily qc_set_timer() after quic_conn
thread migration. As such, the new timer task will be scheduled only if
needed.
This should be backported up to 2.6.
diff --git a/src/quic_loss.c b/src/quic_loss.c
index a8696e9..737119a 100644
--- a/src/quic_loss.c
+++ b/src/quic_loss.c
@@ -93,19 +93,18 @@
(ql->srtt >> 3) +
(QUIC_MAX(ql->rtt_var, QUIC_TIMER_GRANULARITY) << ql->pto_count);
- if (!qc->path->in_flight) {
- struct quic_enc_level *hel;
-
- hel = &qc->els[QUIC_TLS_ENC_LEVEL_HANDSHAKE];
- if (quic_tls_has_tx_sec(hel)) {
- pktns = &qc->pktns[QUIC_TLS_PKTNS_HANDSHAKE];
- }
- else {
- pktns = &qc->pktns[QUIC_TLS_PKTNS_INITIAL];
- }
- lpto = tick_add(now_ms, duration);
- goto out;
- }
+ /* RFC 9002 6.2.2.1. Before Address Validation
+ *
+ * the client MUST set the PTO timer if the client has not received an
+ * acknowledgment for any of its Handshake packets and the handshake is
+ * not confirmed (see Section 4.1.2 of [QUIC-TLS]), even if there are no
+ * packets in flight.
+ *
+ * TODO implement the above paragraph for QUIC on backend side. Note
+ * that if now_ms is used this function is not reentrant anymore and can
+ * not be used anytime without side-effect (for example after QUIC
+ * connection migration).
+ */
lpto = TICK_ETERNITY;
pktns = p = &qc->pktns[QUIC_TLS_PKTNS_INITIAL];