Consider the flow as CC-limited if the packet was sent while `infligh…

…t >= 1/2 * CNWD` or acked under the same condition.

1/2 of CWND is adopted for fairness with RFC 7661, and also provides correct increase; i.e., if an idle connection goes into CC-limited for X round-trips then becomes idle again, all packets sent during that X round-trips will be considered as CC-limited.

include/quicly/sentmap.h

@@ -56,6 +56,10 @@ typedef struct st_quicly_sent_packet_t {
      * if the frames being contained are considered inflight (becomes zero when deemed lost or when PTO fires)
      */
     uint8_t frames_in_flight : 1;
+    /**
+     * 
+     */
+    uint8_t cc_limited : 1;
     /**
      * number of bytes in-flight for the packet, from the context of CC (becomes zero when deemed lost, but not when PTO fires)
      */
@@ -252,7 +256,7 @@ int quicly_sentmap_prepare(quicly_sentmap_t *map, uint64_t packet_number, int64_
 /**
  * commits a write
  */
-static void quicly_sentmap_commit(quicly_sentmap_t *map, uint16_t bytes_in_flight);
+static void quicly_sentmap_commit(quicly_sentmap_t *map, uint16_t bytes_in_flight, int cc_limited);
 /**
  * Allocates a slot to contain a callback for a frame.  The function MUST be called after _prepare but before _commit.
  */
@@ -290,13 +294,14 @@ inline int quicly_sentmap_is_open(quicly_sentmap_t *map)
     return map->_pending_packet != NULL;
 }
 
-inline void quicly_sentmap_commit(quicly_sentmap_t *map, uint16_t bytes_in_flight)
+inline void quicly_sentmap_commit(quicly_sentmap_t *map, uint16_t bytes_in_flight, int cc_limited)
 {
     assert(quicly_sentmap_is_open(map));
 
     if (bytes_in_flight != 0) {
         map->_pending_packet->data.packet.ack_eliciting = 1;
         map->_pending_packet->data.packet.cc_bytes_in_flight = bytes_in_flight;
+        map->_pending_packet->data.packet.cc_limited = cc_limited;
         map->bytes_in_flight += bytes_in_flight;
     }
     map->_pending_packet->data.packet.frames_in_flight = 1;

lib/quicly.c

@@ -346,10 +346,6 @@ 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;
         /**
          *
          */
@@ -1366,10 +1362,8 @@ static void update_send_alarm(quicly_conn_t *conn, int can_send_stream_data, int
                              can_send_stream_data, handshake_is_in_progress, conn->egress.max_data.sent, is_after_send);
 }
 
-static void update_cc_limited(quicly_conn_t *conn, int is_cc_limited)
+static void update_ratemeter(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);
@@ -1395,7 +1389,7 @@ static void setup_next_send(quicly_conn_t *conn)
 
     /* When the flow becomes application-limited due to receiving some information, stop collecting delivery rate samples. */
     if (!can_send_stream_data)
-        update_cc_limited(conn, 0);
+        update_ratemeter(conn, 0);
 }
 
 static int create_handshake_flow(quicly_conn_t *conn, size_t epoch)
@@ -3314,8 +3308,11 @@ static int commit_send_packet(quicly_conn_t *conn, quicly_send_context_t *s, int
     } else {
         packet_bytes_in_flight = 0;
     }
-    if (quicly_sentmap_is_open(&conn->egress.loss.sentmap))
-        quicly_sentmap_commit(&conn->egress.loss.sentmap, (uint16_t)packet_bytes_in_flight);
+    if (quicly_sentmap_is_open(&conn->egress.loss.sentmap)) {
+        int cc_limited = conn->egress.loss.sentmap.bytes_in_flight + packet_bytes_in_flight >=
+                         conn->egress.cc.cwnd / 2; /* for the rationale behind this formula, see handle_ack_frame */
+        quicly_sentmap_commit(&conn->egress.loss.sentmap, (uint16_t)packet_bytes_in_flight, cc_limited);
+    }
 
     conn->egress.cc.type->cc_on_sent(&conn->egress.cc, &conn->egress.loss, (uint32_t)packet_bytes_in_flight, conn->stash.now);
     if (conn->egress.pacer != NULL)
@@ -3351,7 +3348,7 @@ static int commit_send_packet(quicly_conn_t *conn, quicly_send_context_t *s, int
             return ret;
         if (quicly_sentmap_allocate(&conn->egress.loss.sentmap, on_invalid_ack) == NULL)
             return PTLS_ERROR_NO_MEMORY;
-        quicly_sentmap_commit(&conn->egress.loss.sentmap, 0);
+        quicly_sentmap_commit(&conn->egress.loss.sentmap, 0, 0);
         ++conn->egress.packet_number;
         conn->egress.next_pn_to_skip = calc_next_pn_to_skip(conn->super.ctx->tls, conn->egress.packet_number, conn->egress.cc.cwnd,
                                                             conn->egress.max_udp_payload_size);
@@ -5054,10 +5051,10 @@ static int do_send(quicly_conn_t *conn, quicly_send_context_t *s)
             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);
-        update_cc_limited(conn, can_send_stream_data && conn->super.remote.address_validation.validated &&
-                                    (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_ratemeter(conn, can_send_stream_data && conn->super.remote.address_validation.validated &&
+                                   (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);
     }
@@ -5262,7 +5259,7 @@ static int enter_close(quicly_conn_t *conn, int local_is_initiating, int wait_dr
         return ret;
     if (quicly_sentmap_allocate(&conn->egress.loss.sentmap, on_end_closing) == NULL)
         return PTLS_ERROR_NO_MEMORY;
-    quicly_sentmap_commit(&conn->egress.loss.sentmap, 0);
+    quicly_sentmap_commit(&conn->egress.loss.sentmap, 0, 0);
     ++conn->egress.packet_number;
 
     if (local_is_initiating) {
@@ -5445,6 +5442,11 @@ static int handle_ack_frame(quicly_conn_t *conn, struct st_quicly_handle_payload
     size_t bytes_acked = 0;
     int includes_ack_eliciting = 0, includes_late_ack = 0, ret;
 
+    /* The flow is considered CC-limited if the packet was sent while `inflight >= 1/2 * CNWD` or acked under the same condition.
+     * 1/2 of CWND is adopted from RFC 7661, and also provides correct increase; i.e., if an idle connection goes into CC-limited
+     * state for X round-trips then becomes idle again, all packets sent during that X round-trips will considered as CC-limited. */
+    int cc_limited = conn->super.ctx->respect_app_limited || conn->egress.loss.sentmap.bytes_in_flight >= conn->egress.cc.cwnd * 2;
+
     if ((ret = quicly_decode_ack_frame(&state->src, state->end, &frame, state->frame_type == QUICLY_FRAME_TYPE_ACK_ECN)) != 0)
         return ret;
 
@@ -5514,6 +5516,8 @@ static int handle_ack_frame(quicly_conn_t *conn, struct st_quicly_handle_payload
             if (sent->cc_bytes_in_flight != 0) {
                 bytes_acked += sent->cc_bytes_in_flight;
                 conn->super.stats.num_bytes.ack_received += sent->cc_bytes_in_flight;
+                if (sent->cc_limited)
+                    cc_limited = 1;
             }
             if ((ret = quicly_sentmap_update(&conn->egress.loss.sentmap, &iter, QUICLY_SENTMAP_EVENT_ACKED)) != 0)
                 return ret;
@@ -5555,12 +5559,6 @@ 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->respect_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), cc_limited,
                                           conn->egress.packet_number, conn->stash.now, conn->egress.max_udp_payload_size);

t/loss.c

@@ -60,11 +60,11 @@ static void test_time_detection(void)
 
     /* commit 3 packets (pn=0..2); check that loss timer is not active */
     ok(quicly_sentmap_prepare(&loss.sentmap, 0, now, QUICLY_EPOCH_INITIAL) == 0);
-    quicly_sentmap_commit(&loss.sentmap, 10);
+    quicly_sentmap_commit(&loss.sentmap, 10, 0);
     ok(quicly_sentmap_prepare(&loss.sentmap, 1, now, QUICLY_EPOCH_INITIAL) == 0);
-    quicly_sentmap_commit(&loss.sentmap, 10);
+    quicly_sentmap_commit(&loss.sentmap, 10, 0);
     ok(quicly_sentmap_prepare(&loss.sentmap, 2, now, QUICLY_EPOCH_INITIAL) == 0);
-    quicly_sentmap_commit(&loss.sentmap, 10);
+    quicly_sentmap_commit(&loss.sentmap, 10, 0);
     ok(quicly_loss_detect_loss(&loss, now, quicly_spec_context.transport_params.max_ack_delay, 0, on_loss_detected) == 0);
     ok(loss.loss_time == INT64_MAX);
 
@@ -104,13 +104,13 @@ static void test_pn_detection(void)
 
     /* commit 4 packets (pn=0..3); check that loss timer is not active */
     ok(quicly_sentmap_prepare(&loss.sentmap, 0, now, QUICLY_EPOCH_INITIAL) == 0);
-    quicly_sentmap_commit(&loss.sentmap, 10);
+    quicly_sentmap_commit(&loss.sentmap, 10, 0);
     ok(quicly_sentmap_prepare(&loss.sentmap, 1, now, QUICLY_EPOCH_INITIAL) == 0);
-    quicly_sentmap_commit(&loss.sentmap, 10);
+    quicly_sentmap_commit(&loss.sentmap, 10, 0);
     ok(quicly_sentmap_prepare(&loss.sentmap, 2, now, QUICLY_EPOCH_INITIAL) == 0);
-    quicly_sentmap_commit(&loss.sentmap, 10);
+    quicly_sentmap_commit(&loss.sentmap, 10, 0);
     ok(quicly_sentmap_prepare(&loss.sentmap, 3, now, QUICLY_EPOCH_INITIAL) == 0);
-    quicly_sentmap_commit(&loss.sentmap, 10);
+    quicly_sentmap_commit(&loss.sentmap, 10, 0);
     ok(quicly_loss_detect_loss(&loss, now, quicly_spec_context.transport_params.max_ack_delay, 0, on_loss_detected) == 0);
     ok(loss.loss_time == INT64_MAX);
 
@@ -145,9 +145,9 @@ static void test_slow_cert_verify(void)
 
     /* sent Handshake+1RTT packet */
     ok(quicly_sentmap_prepare(&loss.sentmap, 1, now, QUICLY_EPOCH_HANDSHAKE) == 0);
-    quicly_sentmap_commit(&loss.sentmap, 10);
+    quicly_sentmap_commit(&loss.sentmap, 10, 0);
     ok(quicly_sentmap_prepare(&loss.sentmap, 2, now, QUICLY_EPOCH_1RTT) == 0);
-    quicly_sentmap_commit(&loss.sentmap, 10);
+    quicly_sentmap_commit(&loss.sentmap, 10, 0);
     last_retransmittable_sent_at = now;
     quicly_loss_update_alarm(&loss, now, last_retransmittable_sent_at, 1, 0, 1, 0, 1);
 
@@ -169,9 +169,9 @@ static void test_slow_cert_verify(void)
 
     /* therefore send probes */
     ok(quicly_sentmap_prepare(&loss.sentmap, 3, now, QUICLY_EPOCH_HANDSHAKE) == 0);
-    quicly_sentmap_commit(&loss.sentmap, 10);
+    quicly_sentmap_commit(&loss.sentmap, 10, 0);
     ok(quicly_sentmap_prepare(&loss.sentmap, 4, now, QUICLY_EPOCH_1RTT) == 0);
-    quicly_sentmap_commit(&loss.sentmap, 10);
+    quicly_sentmap_commit(&loss.sentmap, 10, 0);
 
     now += 10;
 

t/sentmap.c

@@ -59,7 +59,7 @@ static void test_basic(void)
             quicly_sentmap_prepare(&map, at * 5 + i, at, QUICLY_EPOCH_INITIAL);
             quicly_sentmap_allocate(&map, on_acked);
             quicly_sentmap_allocate(&map, on_acked);
-            quicly_sentmap_commit(&map, 1);
+            quicly_sentmap_commit(&map, 1, 0);
         }
     }
 
@@ -115,10 +115,10 @@ static void test_late_ack(void)
     /* commit pn 1, 2 */
     quicly_sentmap_prepare(&map, 1, 0, QUICLY_EPOCH_INITIAL);
     quicly_sentmap_allocate(&map, on_acked);
-    quicly_sentmap_commit(&map, 10);
+    quicly_sentmap_commit(&map, 10, 0);
     quicly_sentmap_prepare(&map, 2, 0, QUICLY_EPOCH_INITIAL);
     quicly_sentmap_allocate(&map, on_acked);
-    quicly_sentmap_commit(&map, 20);
+    quicly_sentmap_commit(&map, 20, 0);
     ok(map.bytes_in_flight == 30);
 
     /* mark pn 1 as lost */
@@ -159,10 +159,10 @@ static void test_pto(void)
     /* commit pn 1, 2 */
     quicly_sentmap_prepare(&map, 1, 0, QUICLY_EPOCH_INITIAL);
     quicly_sentmap_allocate(&map, on_acked);
-    quicly_sentmap_commit(&map, 10);
+    quicly_sentmap_commit(&map, 10, 0);
     quicly_sentmap_prepare(&map, 2, 0, QUICLY_EPOCH_INITIAL);
     quicly_sentmap_allocate(&map, on_acked);
-    quicly_sentmap_commit(&map, 20);
+    quicly_sentmap_commit(&map, 20, 0);
     ok(map.bytes_in_flight == 30);
 
     /* mark pn 1 for PTO */