QUIC ACKM: Clean up max_ack_delay tracking and separate TX and RX values

Reviewed-by: Tomas Mraz <tomas@openssl.org>
Reviewed-by: Paul Dale <pauli@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/21349)

diff --git a/include/internal/quic_ackm.h b/include/internal/quic_ackm.h
index ca0c1aab1f0e7487941062384329dd3f94312a92..96673303bd2bb244c9b0cb8c5116cad4018f7387 100644 (file)
--- a/include/internal/quic_ackm.h
+++ b/include/internal/quic_ackm.h
@@ -38,6 +38,23 @@ void ossl_ackm_set_ack_deadline_callback(OSSL_ACKM *ackm,
                                                     void *arg),
                                          void *arg);
 
+/*
+ * Configures the RX-side maximum ACK delay. This is the maximum amount of time
+ * the peer is allowed to delay sending an ACK frame after receiving an
+ * ACK-eliciting packet. The peer communicates this value via a transport
+ * parameter and it must be provided to the ACKM.
+ */
+void ossl_ackm_set_rx_max_ack_delay(OSSL_ACKM *ackm, OSSL_TIME rx_max_ack_delay);
+
+/*
+ * Configures the TX-side maximum ACK delay. This is the maximum amount of time
+ * we are allowed to delay sending an ACK frame after receiving an ACK-eliciting
+ * packet. Note that this cannot be changed after a connection is established as
+ * it must be accurately reported in the transport parameters we send to our
+ * peer.
+ */
+void ossl_ackm_set_tx_max_ack_delay(OSSL_ACKM *ackm, OSSL_TIME tx_max_ack_delay);
+
 typedef struct ossl_ackm_tx_pkt_st OSSL_ACKM_TX_PKT;
 struct ossl_ackm_tx_pkt_st {
     /* The packet number of the transmitted packet. */

diff --git a/ssl/quic/quic_ackm.c b/ssl/quic/quic_ackm.c
index a6c8ebef1fc88eb9a7d0d5f1cf345fb07f73ebc5..025b7cc7bd30e022298879f14cda4b98a9f581a0 100644 (file)
--- a/ssl/quic/quic_ackm.c
+++ b/ssl/quic/quic_ackm.c
@@ -488,6 +488,9 @@ static int rx_pkt_history_bump_watermark(struct rx_pkt_history_st *h,
 /* The maximum number of times we allow PTO to be doubled. */
 #define MAX_PTO_COUNT          16
 
+/* Default maximum amount of time to leave an ACK-eliciting packet un-ACK'd. */
+#define DEFAULT_TX_MAX_ACK_DELAY       ossl_ms2time(QUIC_DEFAULT_MAX_ACK_DELAY)
+
 struct ossl_ackm_st {
     /* Our list of transmitted packets. Corresponds to RFC 9002 sent_packets. */
     struct tx_pkt_history_st tx_history[QUIC_PN_SPACE_NUM];
@@ -580,6 +583,19 @@ struct ossl_ackm_st {
      */
     OSSL_TIME       rx_ack_flush_deadline[QUIC_PN_SPACE_NUM];
 
+    /*
+     * The RX maximum ACK delay (the maximum amount of time our peer might
+     * wait to send us an ACK after receiving an ACK-eliciting packet).
+     */
+    OSSL_TIME       rx_max_ack_delay;
+
+    /*
+     * The TX maximum ACK delay (the maximum amount of time we allow ourselves
+     * to wait before generating an ACK after receiving an ACK-eliciting
+     * packet).
+     */
+    OSSL_TIME       tx_max_ack_delay;
+
     /* Callbacks for deadline updates. */
     void (*loss_detection_deadline_cb)(OSSL_TIME deadline, void *arg);
     void *loss_detection_deadline_cb_arg;
@@ -593,7 +609,7 @@ static ossl_inline uint32_t min_u32(uint32_t x, uint32_t y)
     return x < y ? x : y;
 }
 
-/* 
+/*
  * Get TX history for a given packet number space. Must not have been
  * discarded.
  */
@@ -848,13 +864,13 @@ static OSSL_TIME ackm_get_pto_time_and_space(OSSL_ACKM *ackm, int *space)
                 break;
 
             /* Include max_ack_delay and backoff for app data. */
-            if (!ossl_time_is_infinite(rtt.max_ack_delay)) {
+            if (!ossl_time_is_infinite(ackm->rx_max_ack_delay)) {
                 uint64_t factor
                     = (uint64_t)1 << min_u32(ackm->pto_count, MAX_PTO_COUNT);
 
                 duration
                     = ossl_time_add(duration,
-                                    ossl_time_multiply(rtt.max_ack_delay,
+                                    ossl_time_multiply(ackm->rx_max_ack_delay,
                                                        factor));
             }
         }
@@ -1028,6 +1044,10 @@ OSSL_ACKM *ossl_ackm_new(OSSL_TIME (*now)(void *arg),
     ackm->statm     = statm;
     ackm->cc_method = cc_method;
     ackm->cc_data   = cc_data;
+
+    ackm->rx_max_ack_delay = ossl_ms2time(QUIC_DEFAULT_MAX_ACK_DELAY);
+    ackm->tx_max_ack_delay = DEFAULT_TX_MAX_ACK_DELAY;
+
     return ackm;
 
 err:
@@ -1169,12 +1189,8 @@ int ossl_ackm_on_rx_ack_frame(OSSL_ACKM *ackm, const OSSL_QUIC_FRAME_ACK *ack,
 
         /* Enforce maximum ACK delay. */
         ack_delay = ack->delay_time;
-        if (ackm->handshake_confirmed) {
-            OSSL_RTT_INFO rtt;
-
-            ossl_statm_get_rtt_info(ackm->statm, &rtt);
-            ack_delay = ossl_time_min(ack_delay, rtt.max_ack_delay);
-        }
+        if (ackm->handshake_confirmed)
+            ack_delay = ossl_time_min(ack_delay, ackm->rx_max_ack_delay);
 
         ossl_statm_update_rtt(ackm->statm, ack_delay,
                               ossl_time_subtract(now, na_pkts->time));
@@ -1342,8 +1358,6 @@ int ossl_ackm_get_largest_unacked(OSSL_ACKM *ackm, int pkt_space, QUIC_PN *pn)
 
 /* Number of ACK-eliciting packets RX'd before we always emit an ACK. */
 #define PKTS_BEFORE_ACK     2
-/* Maximum amount of time to leave an ACK-eliciting packet un-ACK'd. */
-#define MAX_ACK_DELAY       ossl_ms2time(25)
 
 /*
  * Return 1 if emission of an ACK frame is currently desired.
@@ -1496,12 +1510,12 @@ static void ackm_on_rx_ack_eliciting(OSSL_ACKM *ackm,
      */
     if (ossl_time_is_infinite(ackm->rx_ack_flush_deadline[pkt_space]))
         ackm_set_flush_deadline(ackm, pkt_space,
-                                ossl_time_add(rx_time, MAX_ACK_DELAY));
+                                ossl_time_add(rx_time, ackm->tx_max_ack_delay));
     else
         ackm_set_flush_deadline(ackm, pkt_space,
                                 ossl_time_min(ackm->rx_ack_flush_deadline[pkt_space],
                                               ossl_time_add(rx_time,
-                                                            MAX_ACK_DELAY)));
+                                                            ackm->tx_max_ack_delay)));
 }
 
 int ossl_ackm_on_rx_packet(OSSL_ACKM *ackm, const OSSL_ACKM_RX_PKT *pkt)
@@ -1676,8 +1690,8 @@ OSSL_TIME ossl_ackm_get_pto_duration(OSSL_ACKM *ackm)
     duration = ossl_time_add(rtt.smoothed_rtt,
                              ossl_time_max(ossl_time_multiply(rtt.rtt_variance, 4),
                                            ossl_ticks2time(K_GRANULARITY)));
-    if (!ossl_time_is_infinite(rtt.max_ack_delay))
-        duration = ossl_time_add(duration, rtt.max_ack_delay);
+    if (!ossl_time_is_infinite(ackm->rx_max_ack_delay))
+        duration = ossl_time_add(duration, ackm->rx_max_ack_delay);
 
     return duration;
 }
@@ -1686,3 +1700,13 @@ QUIC_PN ossl_ackm_get_largest_acked(OSSL_ACKM *ackm, int pkt_space)
 {
     return ackm->largest_acked_pkt[pkt_space];
 }
+
+void ossl_ackm_set_rx_max_ack_delay(OSSL_ACKM *ackm, OSSL_TIME rx_max_ack_delay)
+{
+    ackm->rx_max_ack_delay = rx_max_ack_delay;
+}
+
+void ossl_ackm_set_tx_max_ack_delay(OSSL_ACKM *ackm, OSSL_TIME tx_max_ack_delay)
+{
+    ackm->tx_max_ack_delay = tx_max_ack_delay;
+}