From: Amaury Denoyelle Date: Thu, 26 Oct 2023 12:15:41 +0000 (+0200) Subject: MEDIUM: quic: respect closing state even on soft-stop X-Git-Tag: v2.9-dev10~103 X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=f549eb2b34597f59cc3a617276da0d9af6d00514;p=thirdparty%2Fhaproxy.git 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. --- diff --git a/src/quic_conn.c b/src/quic_conn.c index efd02cda87..d54d957f56 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]. - */ - - /* 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); - } + /* 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