From f549eb2b34597f59cc3a617276da0d9af6d00514 Mon Sep 17 00:00:00 2001 From: Amaury Denoyelle Date: Thu, 26 Oct 2023 14:15:41 +0200 Subject: [PATCH] MEDIUM: quic: respect closing state even on soft-stop Prior to this patch, a special condition was set when idle timer was rearmed for closing connections during haproxy process stopping. In this case, the timeout was ditched and the idle task woken up immediatly. The objective was to release quickly closing connections to not prevent the process stopping to be too long. However, it is not conform with RFC 9000 recommandations and may cause some clients to miss a CONNECTION_CLOSE in case of a packet loss. A recent fix was set to use a shorter timeout for closing state. Now a connection should only be left in this state for one second or less. This reduces greatly the importance of stopping special condition. Thus, this patch removes it completely. --- src/quic_conn.c | 79 ++++++++++++++++++++++--------------------------- 1 file changed, 36 insertions(+), 43 deletions(-) diff --git a/src/quic_conn.c b/src/quic_conn.c index efd02cda8..d54d957f5 100644 --- a/src/quic_conn.c +++ b/src/quic_conn.c @@ -1633,55 +1633,48 @@ void qc_idle_timer_do_rearm(struct quic_conn *qc, int arm_ack) if (!qc->idle_timer_task) return; - if (stopping && qc->flags & (QUIC_FL_CONN_CLOSING|QUIC_FL_CONN_DRAINING)) { - TRACE_PROTO("executing idle timer immediately on stopping", QUIC_EV_CONN_IDLE_TIMER, qc); - qc->ack_expire = TICK_ETERNITY; - task_wakeup(qc->idle_timer_task, TASK_WOKEN_MSG); + if (qc->flags & (QUIC_FL_CONN_CLOSING|QUIC_FL_CONN_DRAINING)) { + /* RFC 9000 10.2. Immediate Close + * + * The closing and draining connection states exist to ensure that + * connections close cleanly and that delayed or reordered packets are + * properly discarded. These states SHOULD persist for at least three + * times the current PTO interval as defined in [QUIC-RECOVERY]. + */ + + /* Delay is limited to 1s which should cover most of network + * conditions. The process should not be impacted by a + * connection with a high RTT. + */ + expire = MIN(3 * quic_pto(qc), 1000); } else { - if (qc->flags & (QUIC_FL_CONN_CLOSING|QUIC_FL_CONN_DRAINING)) { - /* RFC 9000 10.2. Immediate Close - * - * The closing and draining connection states exist to ensure that - * connections close cleanly and that delayed or reordered packets are - * properly discarded. These states SHOULD persist for at least three - * times the current PTO interval as defined in [QUIC-RECOVERY]. - */ + /* RFC 9000 10.1. Idle Timeout + * + * To avoid excessively small idle timeout periods, endpoints MUST + * increase the idle timeout period to be at least three times the + * current Probe Timeout (PTO). This allows for multiple PTOs to expire, + * and therefore multiple probes to be sent and lost, prior to idle + * timeout. + */ + expire = QUIC_MAX(3 * quic_pto(qc), qc->max_idle_timeout); + } - /* Delay is limited to 1s which should cover most of - * network conditions. The process should not be - * impacted by a connection with a high RTT. - */ - expire = MIN(3 * quic_pto(qc), 1000); - } - else { - /* RFC 9000 10.1. Idle Timeout - * - * To avoid excessively small idle timeout periods, endpoints MUST - * increase the idle timeout period to be at least three times the - * current Probe Timeout (PTO). This allows for multiple PTOs to expire, - * and therefore multiple probes to be sent and lost, prior to idle - * timeout. - */ - expire = QUIC_MAX(3 * quic_pto(qc), qc->max_idle_timeout); - } - - qc->idle_expire = tick_add(now_ms, MS_TO_TICKS(expire)); - if (arm_ack) { - /* Arm the ack timer only if not already armed. */ - if (!tick_isset(qc->ack_expire)) { - qc->ack_expire = tick_add(now_ms, MS_TO_TICKS(QUIC_ACK_DELAY)); - qc->idle_timer_task->expire = qc->ack_expire; - task_queue(qc->idle_timer_task); - TRACE_PROTO("ack timer armed", QUIC_EV_CONN_IDLE_TIMER, qc); - } - } - else { - qc->idle_timer_task->expire = tick_first(qc->ack_expire, qc->idle_expire); + qc->idle_expire = tick_add(now_ms, MS_TO_TICKS(expire)); + if (arm_ack) { + /* Arm the ack timer only if not already armed. */ + if (!tick_isset(qc->ack_expire)) { + qc->ack_expire = tick_add(now_ms, MS_TO_TICKS(QUIC_ACK_DELAY)); + qc->idle_timer_task->expire = qc->ack_expire; task_queue(qc->idle_timer_task); - TRACE_PROTO("idle timer armed", QUIC_EV_CONN_IDLE_TIMER, qc); + TRACE_PROTO("ack timer armed", QUIC_EV_CONN_IDLE_TIMER, qc); } } + else { + qc->idle_timer_task->expire = tick_first(qc->ack_expire, qc->idle_expire); + task_queue(qc->idle_timer_task); + TRACE_PROTO("idle timer armed", QUIC_EV_CONN_IDLE_TIMER, qc); + } } /* Rearm the idle timer or ack timer for QUIC connection depending on