]> git.ipfire.org Git - thirdparty/haproxy.git/commitdiff
MEDIUM: quic: respect closing state even on soft-stop
authorAmaury Denoyelle <adenoyelle@haproxy.com>
Thu, 26 Oct 2023 12:15:41 +0000 (14:15 +0200)
committerAmaury Denoyelle <adenoyelle@haproxy.com>
Fri, 10 Nov 2023 14:26:03 +0000 (15:26 +0100)
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

index efd02cda876f064c7d6c07ba03ba90e5535d0a1f..d54d957f56eed16e00b0967d26530382f385ea0a 100644 (file)
@@ -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 <qc> QUIC connection depending on <read>