QUIC CC: Major revisions to CC abstract interface

Reviewed-by: Tomas Mraz <tomas@openssl.org>
Reviewed-by: Matt Caswell <matt@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/20423)

4 files changed, 99 insertions, 92 deletions

diff --git a/ssl/quic/cc_dummy.c b/ssl/quic/cc_dummy.c
index 195c4324bf..af0d548379 100644
--- a/ssl/quic/cc_dummy.c
+++ b/ssl/quic/cc_dummy.c
@@ -8,15 +8,22 @@
  */
 
 #include "internal/quic_cc.h"
+#include "internal/quic_types.h"
 
 typedef struct ossl_cc_dummy_st {
-    char dummy;
+    size_t max_dgram_len;
 } OSSL_CC_DUMMY;
 
-static OSSL_CC_DATA *dummy_new(OSSL_PARAM *settings, OSSL_PARAM *options,
-                               OSSL_PARAM *changeables)
+static OSSL_CC_DATA *dummy_new(OSSL_TIME (*now_cb)(void *arg),
+                               void *now_cb_arg)
 {
-    return OPENSSL_zalloc(sizeof(OSSL_CC_DUMMY));
+    OSSL_CC_DUMMY *d = OPENSSL_zalloc(sizeof(*d));
+
+    if (d == NULL)
+        return NULL;
+
+    d->max_dgram_len = QUIC_MIN_INITIAL_DGRAM_LEN;
+    return (OSSL_CC_DATA *)d;
 }
 
 static void dummy_free(OSSL_CC_DATA *cc)
@@ -24,90 +31,97 @@ static void dummy_free(OSSL_CC_DATA *cc)
     OPENSSL_free(cc);
 }
 
-static void dummy_reset(OSSL_CC_DATA *cc, int flags)
+static void dummy_reset(OSSL_CC_DATA *cc)
 {
 
 }
 
-static int dummy_set_exemption(OSSL_CC_DATA *cc, int numpackets)
+static int dummy_set_option_uint(OSSL_CC_DATA *cc,
+                                 uint32_t option_id,
+                                 uint64_t value)
 {
-    return 1;
-}
+    OSSL_CC_DUMMY *d = (OSSL_CC_DUMMY *)cc;
 
-static int dummy_get_exemption(OSSL_CC_DATA *cc)
-{
-    return 0;
-}
+    switch (option_id) {
+    case OSSL_CC_OPTION_MAX_DGRAM_PAYLOAD_LEN:
+        if (value > SIZE_MAX)
+            return 0;
 
-static int dummy_can_send(OSSL_CC_DATA *cc)
-{
-    return 1;
+        d->max_dgram_len = (size_t)value;
+        return 1;
+
+    default:
+        return 0;
+    }
 }
 
-static uint64_t dummy_get_send_allowance(OSSL_CC_DATA *cc,
-                                       OSSL_TIME time_since_last_send,
-                                       int time_valid)
+static int dummy_get_option_uint(OSSL_CC_DATA *cc,
+                                 uint32_t option_id,
+                                 uint64_t *value)
 {
-    return SIZE_MAX;
+    OSSL_CC_DUMMY *d = (OSSL_CC_DUMMY *)cc;
+
+    switch (option_id) {
+    case OSSL_CC_OPTION_MAX_DGRAM_PAYLOAD_LEN:
+        *value = (uint64_t)d->max_dgram_len;
+        return 1;
+
+    default:
+        return 0;
+    }
 }
 
-static uint64_t dummy_get_bytes_in_flight_max(OSSL_CC_DATA *cc)
+static uint64_t dummy_get_tx_allowance(OSSL_CC_DATA *cc)
 {
     return SIZE_MAX;
 }
 
-static OSSL_TIME dummy_get_next_credit_time(OSSL_CC_DATA *cc_data)
+static OSSL_TIME dummy_get_wakeup_deadline(OSSL_CC_DATA *cc)
 {
     return ossl_time_infinite();
 }
 
 static int dummy_on_data_sent(OSSL_CC_DATA *cc,
-                              uint64_t num_retransmittable_bytes)
+                              uint64_t num_bytes)
 {
     return 1;
 }
 
-static int dummy_on_data_invalidated(OSSL_CC_DATA *cc,
-                                     uint64_t num_retransmittable_bytes)
+static int dummy_on_data_acked(OSSL_CC_DATA *cc,
+                               const OSSL_CC_ACK_INFO *info)
 {
     return 1;
 }
 
-static int dummy_on_data_acked(OSSL_CC_DATA *cc, OSSL_TIME time_now,
-                               uint64_t last_pn_acked,
-                               uint64_t num_retransmittable_bytes)
+static int dummy_on_data_lost(OSSL_CC_DATA *cc,
+                              const OSSL_CC_LOSS_INFO *info)
 {
     return 1;
 }
 
-static void dummy_on_data_lost(OSSL_CC_DATA *cc,
-                              uint64_t largest_pn_lost,
-                              uint64_t largest_pn_sent,
-                              uint64_t num_retransmittable_bytes,
-                              int persistent_congestion)
+static int dummy_on_data_lost_finished(OSSL_CC_DATA *cc,
+                                       uint32_t flags)
 {
-
+    return 1;
 }
 
-static int dummy_on_spurious_congestion_event(OSSL_CC_DATA *cc)
+static int dummy_on_data_invalidated(OSSL_CC_DATA *cc,
+                                     uint64_t num_bytes)
 {
     return 1;
 }
 
 const OSSL_CC_METHOD ossl_cc_dummy_method = {
-    NULL,
     dummy_new,
     dummy_free,
     dummy_reset,
-    dummy_set_exemption,
-    dummy_get_exemption,
-    dummy_can_send,
-    dummy_get_send_allowance,
-    dummy_get_bytes_in_flight_max,
-    dummy_get_next_credit_time,
+    dummy_set_option_uint,
+    dummy_get_option_uint,
+    dummy_get_tx_allowance,
+    dummy_get_wakeup_deadline,
     dummy_on_data_sent,
-    dummy_on_data_invalidated,
     dummy_on_data_acked,
     dummy_on_data_lost,
-    dummy_on_spurious_congestion_event,
+    dummy_on_data_lost_finished,
+    dummy_on_data_invalidated,
 };
diff --git a/ssl/quic/quic_ackm.c b/ssl/quic/quic_ackm.c
index 6061fb29f5..b798636571 100644
--- a/ssl/quic/quic_ackm.c
+++ b/ssl/quic/quic_ackm.c
@@ -911,7 +911,7 @@ static int ackm_set_loss_detection_timer(OSSL_ACKM *ackm)
 static int ackm_in_persistent_congestion(OSSL_ACKM *ackm,
                                          const OSSL_ACKM_TX_PKT *lpkt)
 {
-    /* Persistent congestion not currently implemented. */
+    /* TODO(QUIC): Persistent congestion not currently implemented. */
     return 0;
 }
 
@@ -922,6 +922,8 @@ static void ackm_on_pkts_lost(OSSL_ACKM *ackm, int pkt_space,
     OSSL_RTT_INFO rtt;
     QUIC_PN largest_pn_lost = 0;
     uint64_t num_bytes = 0;
+    OSSL_CC_LOSS_INFO loss_info = {0};
+    uint32_t flags = 0;
 
     for (p = lpkt; p != NULL; p = pnext) {
         pnext = p->lnext;
@@ -938,41 +940,38 @@ static void ackm_on_pkts_lost(OSSL_ACKM *ackm, int pkt_space,
             num_bytes += p->num_bytes;
         }
 
+        if (!pseudo) {
+            /*
+             * If this is pseudo-loss (e.g. during connection retry) we do not
+             * inform the CC as it is not a real loss and not reflective of
+             * network conditions.
+             */
+            loss_info.tx_time = p->time;
+            loss_info.tx_size = p->num_bytes;
+
+            ackm->cc_method->on_data_lost(ackm->cc_data, &loss_info);
+        }
+
         p->on_lost(p->cb_arg);
     }
 
-    if (pseudo)
-        /*
-         * If this is pseudo-loss (e.g. during connection retry) we do not
-         * inform the CC as it is not a real loss and not reflective of network
-         * conditions.
-         */
-        return;
-
     /*
-     * Only consider lost packets with regards to congestion after getting an
-     * RTT sample.
+     * Persistent congestion can only be considered if we have gotten at least
+     * one RTT sample.
      */
     ossl_statm_get_rtt_info(ackm->statm, &rtt);
+    if (!ossl_time_is_zero(ackm->first_rtt_sample)
+        && ackm_in_persistent_congestion(ackm, lpkt))
+        flags |= OSSL_CC_LOST_FLAG_PERSISTENT_CONGESTION;
 
-    if (ossl_time_is_zero(ackm->first_rtt_sample))
-        return;
-
-    ackm->cc_method->on_data_lost(ackm->cc_data,
-        largest_pn_lost,
-        ackm->tx_history[pkt_space].highest_sent,
-        num_bytes,
-        ackm_in_persistent_congestion(ackm, lpkt));
+    ackm->cc_method->on_data_lost_finished(ackm->cc_data, flags);
 }
 
 static void ackm_on_pkts_acked(OSSL_ACKM *ackm, const OSSL_ACKM_TX_PKT *apkt)
 {
     const OSSL_ACKM_TX_PKT *anext;
-    OSSL_TIME now;
-    uint64_t num_retransmittable_bytes = 0;
     QUIC_PN last_pn_acked = 0;
-
-    now = ackm->now(ackm->now_arg);
+    OSSL_CC_ACK_INFO ainfo = {0};
 
     for (; apkt != NULL; apkt = anext) {
         if (apkt->is_inflight) {
@@ -981,7 +980,6 @@ static void ackm_on_pkts_acked(OSSL_ACKM *ackm, const OSSL_ACKM_TX_PKT *apkt)
                 ackm->ack_eliciting_bytes_in_flight[apkt->pkt_space]
                     -= apkt->num_bytes;
 
-            num_retransmittable_bytes += apkt->num_bytes;
             if (apkt->pkt_num > last_pn_acked)
                 last_pn_acked = apkt->pkt_num;
 
@@ -997,10 +995,11 @@ static void ackm_on_pkts_acked(OSSL_ACKM *ackm, const OSSL_ACKM_TX_PKT *apkt)
 
         anext = apkt->anext;
         apkt->on_acked(apkt->cb_arg); /* may free apkt */
-    }
 
-    ackm->cc_method->on_data_acked(ackm->cc_data, now,
-        last_pn_acked, num_retransmittable_bytes);
+        ainfo.tx_time = apkt->time;
+        ainfo.tx_size = apkt->num_bytes;
+        ackm->cc_method->on_data_acked(ackm->cc_data, &ainfo);
+    }
 }
 
 OSSL_ACKM *ossl_ackm_new(OSSL_TIME (*now)(void *arg),
@@ -1096,16 +1095,12 @@ int ossl_ackm_on_rx_datagram(OSSL_ACKM *ackm, size_t num_bytes)
     return 1;
 }
 
-static void ackm_on_congestion(OSSL_ACKM *ackm, OSSL_TIME send_time)
-{
-    /* Not currently implemented. */
-}
-
 static void ackm_process_ecn(OSSL_ACKM *ackm, const OSSL_QUIC_FRAME_ACK *ack,
                              int pkt_space)
 {
     struct tx_pkt_history_st *h;
     OSSL_ACKM_TX_PKT *pkt;
+    OSSL_CC_ECN_INFO ecn_info = {0};
 
     /*
      * If the ECN-CE counter reported by the peer has increased, this could
@@ -1119,7 +1114,8 @@ static void ackm_process_ecn(OSSL_ACKM *ackm, const OSSL_QUIC_FRAME_ACK *ack,
         if (pkt == NULL)
             return;
 
-        ackm_on_congestion(ackm, pkt->time);
+        ecn_info.largest_acked_time = pkt->time;
+        ackm->cc_method->on_ecn(ackm->cc_data, &ecn_info);
     }
 }
 
@@ -1182,7 +1178,13 @@ int ossl_ackm_on_rx_ack_frame(OSSL_ACKM *ackm, const OSSL_QUIC_FRAME_ACK *ack,
                               ossl_time_subtract(now, na_pkts->time));
     }
 
-    /* Process ECN information if present. */
+    /*
+     * Process ECN information if present.
+     *
+     * We deliberately do most ECN processing in the ACKM rather than the
+     * congestion controller to avoid having to give the congestion controller
+     * access to ACKM internal state.
+     */
     if (ack->ecn_present)
         ackm_process_ecn(ackm, ack, pkt_space);
 
diff --git a/ssl/quic/quic_channel.c b/ssl/quic/quic_channel.c
index 460a7d77db..349e2f1055 100644
--- a/ssl/quic/quic_channel.c
+++ b/ssl/quic/quic_channel.c
@@ -150,7 +150,7 @@ static int ch_init(QUIC_CHANNEL *ch)
 
     ch->have_statm = 1;
     ch->cc_method = &ossl_cc_dummy_method;
-    if ((ch->cc_data = ch->cc_method->new(NULL, NULL, NULL)) == NULL)
+    if ((ch->cc_data = ch->cc_method->new(get_time, NULL)) == NULL)
         goto err;
 
     if ((ch->ackm = ossl_ackm_new(get_time, ch, &ch->statm,
@@ -1687,7 +1687,7 @@ static OSSL_TIME ch_determine_next_tick_deadline(QUIC_CHANNEL *ch)
     if (ossl_quic_tx_packetiser_has_pending(ch->txp, TX_PACKETISER_ARCHETYPE_NORMAL,
                                             TX_PACKETISER_BYPASS_CC))
         deadline = ossl_time_min(deadline,
-                                 ch->cc_method->get_next_credit_time(ch->cc_data));
+                                 ch->cc_method->get_wakeup_deadline(ch->cc_data));
 
     /* Is the terminating timer armed? */
     if (ossl_quic_channel_is_terminating(ch))
diff --git a/ssl/quic/quic_txp.c b/ssl/quic/quic_txp.c
index 2d49e64f7f..786d25ecf6 100644
--- a/ssl/quic/quic_txp.c
+++ b/ssl/quic/quic_txp.c
@@ -483,7 +483,8 @@ int ossl_quic_tx_packetiser_has_pending(OSSL_QUIC_TX_PACKETISER *txp,
     int cc_can_send;
 
     cc_can_send
-        = (bypass_cc || txp->args.cc_method->can_send(txp->args.cc_data));
+        = (bypass_cc
+           || txp->args.cc_method->get_tx_allowance(txp->args.cc_data) > 0);
 
     for (enc_level = QUIC_ENC_LEVEL_INITIAL;
          enc_level < QUIC_ENC_LEVEL_NUM;
@@ -512,7 +513,7 @@ int ossl_quic_tx_packetiser_generate(OSSL_QUIC_TX_PACKETISER *txp,
     /*
      * If CC says we cannot send we still may be able to send any queued probes.
      */
-    cc_can_send = txp->args.cc_method->can_send(txp->args.cc_data);
+    cc_can_send = (txp->args.cc_method->get_tx_allowance(txp->args.cc_data) > 0);
 
     for (enc_level = QUIC_ENC_LEVEL_INITIAL;
          enc_level < QUIC_ENC_LEVEL_NUM;
@@ -900,15 +901,8 @@ static int txp_generate_for_el(OSSL_QUIC_TX_PACKETISER *txp, uint32_t enc_level,
     size_t mdpl, hdr_len, pkt_overhead, cc_limit;
     uint64_t cc_limit_;
     QUIC_PKT_HDR phdr;
-    OSSL_TIME time_since_last;
 
     /* Determine the limit CC imposes on what we can send. */
-    if (ossl_time_is_zero(txp->last_tx_time))
-        time_since_last = ossl_time_zero();
-    else
-        time_since_last = ossl_time_subtract(txp->args.now(txp->args.now_arg),
-                                             txp->last_tx_time);
-
     if (!cc_can_send) {
         /*
          * If we are called when we cannot send, this must be because we want
@@ -917,10 +911,7 @@ static int txp_generate_for_el(OSSL_QUIC_TX_PACKETISER *txp, uint32_t enc_level,
         cc_limit = SIZE_MAX;
     } else {
         /* Allow CC to clamp how much we can send. */
-        cc_limit_ = txp->args.cc_method->get_send_allowance(txp->args.cc_data,
-                                                            time_since_last,
-                                                            ossl_time_is_zero(time_since_last));
-
+        cc_limit_ = txp->args.cc_method->get_tx_allowance(txp->args.cc_data);
         cc_limit = (cc_limit_ > SIZE_MAX ? SIZE_MAX : (size_t)cc_limit_);
     }