[reno][pico] do not increase CWND when the connection is in app-limit…

…ed state; ATM the state being referred to is that when `quicly_send` is called the last time. For Cubic, this is a TODO.

include/quicly.h

@@ -338,6 +338,10 @@ struct st_quicly_context_t {
      * if pacing should be used
      */
     unsigned use_pacing : 1;
+    /**
+     * if CC should take app-limited into consideration
+     */
+    unsigned cc_recognize_app_limited : 1;
     /**
      *
      */

include/quicly/cc.h

@@ -160,7 +160,7 @@ struct st_quicly_cc_type_t {
      * Called when a packet is newly acknowledged.
      */
     void (*cc_on_acked)(quicly_cc_t *cc, const quicly_loss_t *loss, uint32_t bytes, uint64_t largest_acked, uint32_t inflight,
-                        uint64_t next_pn, int64_t now, uint32_t max_udp_payload_size);
+                        int cc_limited, uint64_t next_pn, int64_t now, uint32_t max_udp_payload_size);
     /**
      * Called when a packet is detected as lost.
      * @param bytes    bytes declared lost, or zero iff ECN_CE is observed

lib/cc-cubic.c

@@ -62,13 +62,15 @@ static uint32_t calc_w_est(const quicly_cc_t *cc, cubic_float_t t_sec, cubic_flo
 
 /* TODO: Avoid increase if sender was application limited. */
 static void cubic_on_acked(quicly_cc_t *cc, const quicly_loss_t *loss, uint32_t bytes, uint64_t largest_acked, uint32_t inflight,
-                           uint64_t next_pn, int64_t now, uint32_t max_udp_payload_size)
+                           int cc_limited, uint64_t next_pn, int64_t now, uint32_t max_udp_payload_size)
 {
     assert(inflight >= bytes);
     /* Do not increase congestion window while in recovery. */
     if (largest_acked < cc->recovery_end)
         return;
 
+    /* TODO: respect cc_limited */
+
     /* Slow start. */
     if (cc->cwnd < cc->ssthresh) {
         cc->cwnd += bytes;

lib/cc-pico.c

@@ -62,15 +62,16 @@ static uint32_t calc_bytes_per_mtu_increase(uint32_t cwnd, uint32_t rtt, uint32_
 
 /* TODO: Avoid increase if sender was application limited. */
 static void pico_on_acked(quicly_cc_t *cc, const quicly_loss_t *loss, uint32_t bytes, uint64_t largest_acked, uint32_t inflight,
-                          uint64_t next_pn, int64_t now, uint32_t max_udp_payload_size)
+                          int cc_limited, uint64_t next_pn, int64_t now, uint32_t max_udp_payload_size)
 {
     assert(inflight >= bytes);
 
     /* Do not increase congestion window while in recovery. */
     if (largest_acked < cc->recovery_end)
         return;
 
-    cc->state.pico.stash += bytes;
+    if (!cc_limited)
+        return;
 
     /* Calculate the amount of bytes required to be acked for incrementing CWND by one MTU. */
     uint32_t bytes_per_mtu_increase;

lib/cc-reno.c

@@ -25,7 +25,7 @@
 
 /* TODO: Avoid increase if sender was application limited. */
 static void reno_on_acked(quicly_cc_t *cc, const quicly_loss_t *loss, uint32_t bytes, uint64_t largest_acked, uint32_t inflight,
-                          uint64_t next_pn, int64_t now, uint32_t max_udp_payload_size)
+                          int cc_limited, uint64_t next_pn, int64_t now, uint32_t max_udp_payload_size)
 {
     assert(inflight >= bytes);
     /* Do not increase congestion window while in recovery. */
@@ -34,13 +34,17 @@ static void reno_on_acked(quicly_cc_t *cc, const quicly_loss_t *loss, uint32_t b
 
     /* Slow start. */
     if (cc->cwnd < cc->ssthresh) {
-        cc->cwnd += bytes;
-        if (cc->cwnd_maximum < cc->cwnd)
-            cc->cwnd_maximum = cc->cwnd;
+        if (cc_limited) {
+            cc->cwnd += bytes;
+            if (cc->cwnd_maximum < cc->cwnd)
+                cc->cwnd_maximum = cc->cwnd;
+        }
         return;
     }
     /* Congestion avoidance. */
     cc->pacer_multiplier = QUICLY_PACER_CALC_MULTIPLIER(1.2);
+    if (!cc_limited)
+        return;
     cc->state.reno.stash += bytes;
     if (cc->state.reno.stash < cc->cwnd)
         return;

lib/defaults.c

@@ -52,6 +52,7 @@ const quicly_context_t quicly_spec_context = {NULL,
                                               0, /* enlarge_client_hello */
                                               1, /* enable_ecn */
                                               0, /* use_pacing */
+                                              1, /* cc_recognize_app_limited */
                                               NULL,
                                               NULL, /* on_stream_open */
                                               &quicly_default_stream_scheduler,
@@ -84,6 +85,7 @@ const quicly_context_t quicly_performant_context = {NULL,
                                                     0, /* enlarge_client_hello */
                                                     1, /* enable_ecn */
                                                     0, /* use_pacing */
+                                                    1, /* cc_recognize_app_limited */
                                                     NULL,
                                                     NULL, /* on_stream_open */
                                                     &quicly_default_stream_scheduler,

lib/quicly.c

@@ -350,6 +350,10 @@ struct st_quicly_conn_t {
  * have to be sent. There could be false positives; logic for sending each of these frames have the capability of detecting such
  * false positives. The purpose of this bit is to consolidate information as an optimization. */
 #define QUICLY_PENDING_FLOW_OTHERS_BIT (1 << 6)
+        /**
+         * if the most recent call to `quicly_send` ended in CC-limited state
+         */
+        uint8_t cc_limited : 1;
         /**
          * pending RETIRE_CONNECTION_ID frames to be sent
          */
@@ -1378,6 +1382,8 @@ static void update_send_alarm(quicly_conn_t *conn, int can_send_stream_data, int
 
 static void update_cc_limited(quicly_conn_t *conn, int is_cc_limited)
 {
+    conn->egress.cc_limited = is_cc_limited;
+
     if (quicly_ratemeter_is_cc_limited(&conn->egress.ratemeter) != is_cc_limited) {
         if (is_cc_limited) {
             quicly_ratemeter_enter_cc_limited(&conn->egress.ratemeter, conn->egress.packet_number);
@@ -4907,15 +4913,14 @@ static int do_send(quicly_conn_t *conn, quicly_send_context_t *s)
         commit_send_packet(conn, s, 0);
     }
     if (ret == 0) {
-        /* update timers, start / stop delivery rate estimator */
+        /* update timers, cc and delivery rate estimator states */
         if (conn->application == NULL || conn->application->super.unacked_count == 0)
             conn->egress.send_ack_at = INT64_MAX; /* we have sent ACKs for every epoch (or before address validation) */
         int can_send_stream_data = scheduler_can_send(conn);
         update_send_alarm(conn, can_send_stream_data, 1);
-        int is_cc_limited = can_send_stream_data && (s->num_datagrams == s->max_datagrams ||
-                                                     conn->egress.loss.sentmap.bytes_in_flight >= conn->egress.cc.cwnd ||
-                                                     pacer_can_send_at(conn) > conn->stash.now);
-        update_cc_limited(conn, is_cc_limited);
+        update_cc_limited(conn, can_send_stream_data && (s->num_datagrams == s->max_datagrams ||
+                                                         conn->egress.loss.sentmap.bytes_in_flight >= conn->egress.cc.cwnd ||
+                                                         pacer_can_send_at(conn) > conn->stash.now));
         if (s->num_datagrams != 0)
             update_idle_timeout(conn, 0);
     }
@@ -5411,9 +5416,16 @@ static int handle_ack_frame(quicly_conn_t *conn, struct st_quicly_handle_payload
 
     /* OnPacketAcked and OnPacketAckedCC */
     if (bytes_acked > 0) {
+        /* Here, we pass `conn->egress.cc_limited` - a boolean flag indicating if last the last call to `quicly_send` ended with
+         * data being throttled by CC or pacer - to CC to determine if CWND can be grown.
+         * This might not be the best way to do this, but would likely be sufficient, as the flag being passed would be true only if
+         * the connection was CC-limited for at least one RTT. Hopefully, itwould also be aggressive enough during the slow start
+         * phase. */
+        int cc_limited = conn->super.ctx->cc_recognize_app_limited ? conn->egress.cc_limited : 1;
         conn->egress.cc.type->cc_on_acked(&conn->egress.cc, &conn->egress.loss, (uint32_t)bytes_acked, frame.largest_acknowledged,
                                           (uint32_t)(conn->egress.loss.sentmap.bytes_in_flight + bytes_acked),
-                                          conn->egress.packet_number, conn->stash.now, conn->egress.max_udp_payload_size);
+                                          cc_limited, conn->egress.packet_number, conn->stash.now,
+                                          conn->egress.max_udp_payload_size);
         QUICLY_PROBE(QUICTRACE_CC_ACK, conn, conn->stash.now, &conn->egress.loss.rtt, conn->egress.cc.cwnd,
                      conn->egress.loss.sentmap.bytes_in_flight);
     }