fixed inflight, fixed several calculation errors, fixed pacing logic to not be affected by leaky bucket logic

Author: Tipuch

quinn-proto/src/congestion/bbr3/mod.rs

@@ -212,19 +212,22 @@ struct BbrPacket {
 struct BbrRateSample {
     /// equivalent to RS.delivery_rate: The delivery rate (aka bandwidth) sample obtained from the packet that has just been ACKed.
     delivery_rate: f64,
-    /// equivalent to RS.is_app_limited
+    /// equivalent to RS.is_app_limited: The P.is_app_limited from the most recent packet
+    ///    delivered; indicates whether the rate sample is application-limited.
     is_app_limited: bool,
-    /// equivalent to RS.interval
+    /// equivalent to RS.interval: The length of the sampling interval.
     interval: Duration,
     /// equivalent to RS.delivered: The volume of data delivered between the transmission of the packet that has just been ACKed and the current time.
     delivered: u64,
-    /// equivalent to RS.prior_delivered
+    /// equivalent to RS.prior_delivered: The P.delivered count from the most recent packet delivered.
     prior_delivered: u64,
-    /// equivalent to RS.prior_time
+    /// equivalent to RS.prior_time: The P.delivered_time from the most recent packet delivered.
     prior_time: Instant,
-    /// equivalent to RS.send_elapsed
+    /// equivalent to RS.send_elapsed: Send time interval calculated from the most recent
+    ///    packet delivered (see the "Send Rate" section above).
     send_elapsed: Duration,
-    /// equivalent to RS.ack_elapsed
+    /// equivalent to RS.ack_elapsed: ACK time interval calculated from the most recent
+    ///    packet delivered (see the "ACK Rate" section above).
     ack_elapsed: Duration,
     /// equivalent to RS.rtt: The RTT sample calculated based on the most recently-sent packet of the packets that have just been ACKed.
     rtt: Duration,
@@ -516,14 +519,14 @@ impl Bbr3 {
             bw_latest: 0.0,
             inflight_latest: 0,
             max_bw_filter: MaxFilter::new(MAX_BW_FILTER_LEN as u64),
-            extra_acked_interval_start: None,
+            extra_acked_interval_start: Some(Instant::now()),
             extra_acked_delivered: 0,
             extra_acked_filter: MaxFilter::new(EXTRA_ACKED_FILTER_LEN as u64),
             full_bw_reached: false,
             full_bw_now: false,
             full_bw: 0.0,
             full_bw_count: 0,
-            min_rtt_stamp: None,
+            min_rtt_stamp: Some(Instant::now()),
             min_rtt_filter_len: MIN_RTT_FILTER_LEN,
             probe_rtt_cwnd_gain,
             probe_rtt_duration: Duration::from_millis(PROBE_RTT_DURATION_MS),
@@ -580,13 +583,12 @@ impl Bbr3 {
     /// equivalent to BBRInflightAtLoss <https://www.ietf.org/archive/id/draft-ietf-ccwg-bbr-04.html#section-5.5.10.2-11>
     /// We check at what prefix of packet did losses exceed `loss_thresh`
     fn inflight_at_loss(&mut self, lost_bytes: u64) -> u64 {
-        let inflight_prev;
         if let Some(rate_sample) = self.rs {
-            inflight_prev = rate_sample.tx_in_flight.saturating_sub(lost_bytes);
+            let inflight_prev = rate_sample.tx_in_flight.saturating_sub(lost_bytes);
             let lost_prev = rate_sample.lost.saturating_sub(lost_bytes);
             let compared_loss = inflight_prev.saturating_sub(lost_prev);
             let lost_prefix = (self.loss_thresh * compared_loss as f64) / (1.0 - self.loss_thresh);
-            let inflight_at_loss = inflight_prev.saturating_sub(lost_prefix as u64);
+            let inflight_at_loss = inflight_prev + lost_prefix as u64;
             return inflight_at_loss;
         }
         0
@@ -657,14 +659,13 @@ impl Bbr3 {
         }
 
         if self.is_inflight_too_high() {
-            let mut new_inflight_hi = self.bdp;
+            let mut new_inflight_hi = self.bdp.max(self.inflight_latest);
             if let Some(rate_sample) = self.rs {
-                if new_inflight_hi < rate_sample.prior_delivered {
-                    new_inflight_hi = rate_sample.prior_delivered;
+                if new_inflight_hi < rate_sample.delivered {
+                    new_inflight_hi = rate_sample.delivered;
                 }
             }
-
-            self.inflight_latest = new_inflight_hi;
+            self.inflight_longterm = new_inflight_hi;
             self.full_bw_reached = true;
         }
     }
@@ -1087,6 +1088,7 @@ impl Bbr3 {
             {
                 self.max_bw_filter
                     .update_max(self.cycle_count, rate_sample.delivery_rate.round() as u64);
+
                 self.max_bw = self.max_bw_filter.get_max() as f64;
             }
         }
@@ -1253,8 +1255,8 @@ impl Bbr3 {
         if self.loss_round_start {
             if let Some(rate_sample) = self.rs {
                 self.bw_latest = rate_sample.delivery_rate;
+                self.inflight_latest = rate_sample.delivered;
             }
-            self.inflight_latest = self.delivered;
         }
     }
 
@@ -1376,10 +1378,8 @@ impl Controller for Bbr3 {
             self.first_send_time = Some(now);
             self.delivered_time = Some(now);
         }
-        if bytes > 0 {
-            let added_bytes = bytes as u64;
-            self.inflight += added_bytes;
-        }
+        let added_bytes = bytes as u64;
+        self.inflight += added_bytes;
         self.packets.push_back(BbrPacket {
             delivered: self.delivered,
             delivered_time: self.delivered_time.unwrap_or(now),
@@ -1407,7 +1407,6 @@ impl Controller for Bbr3 {
         let bytes64 = bytes as u64;
         if let Some(mut rate_sample) = self.rs {
             rate_sample.newly_acked += bytes64;
-            rate_sample.rtt = rtt.get();
             self.rs = Some(rate_sample);
             self.delivered += bytes64;
             self.delivered_time = Some(now);
@@ -1420,8 +1419,9 @@ impl Controller for Bbr3 {
             if let Some(p) = self.packets.get_mut(p_index) {
                 p.acknowledged = true;
                 if let Some(mut rate_sample) = self.rs {
+                    rate_sample.rtt = now - p.send_time;
                     if is_newest_packet {
-                        self.srtt = rate_sample.rtt;
+                        self.srtt = rtt.get();
                         rate_sample.prior_delivered = p.delivered;
                         rate_sample.prior_time = p.delivered_time;
                         rate_sample.is_app_limited = p.is_app_limited;
@@ -1471,9 +1471,9 @@ impl Controller for Bbr3 {
         app_limited: bool,
         largest_packet_num_acked: Option<u64>,
     ) {
-        self.inflight = in_flight.ack_eliciting;
+        self.inflight = in_flight.bytes;
         if let Some(largest_packet_num) = largest_packet_num_acked {
-            if app_limited {
+            if app_limited && self.delivered > self.app_limited {
                 self.app_limited = largest_packet_num;
             } else {
                 self.app_limited = 0;
@@ -1505,6 +1505,8 @@ impl Controller for Bbr3 {
                     }
                 }
                 rate_sample.newly_acked = 0;
+                rate_sample.lost = 0;
+                rate_sample.newly_lost = 0;
                 self.rs = Some(rate_sample);
             }
         } else if self.app_limited > 0 && self.delivered > self.app_limited {

quinn-proto/src/connection/pacing.rs

@@ -81,6 +81,15 @@ impl Pacer {
             self.tokens = self.capacity.min(self.tokens);
         }
 
+        if let Some(pacing_rate) = pacing_rate {
+            if bytes_to_send <= self.capacity {
+                return None;
+            }
+            let capped_bytes_to_send = bytes_to_send.max(self.capacity);
+            let delay = Duration::from_secs_f64(capped_bytes_to_send as f64 / pacing_rate as f64);
+            return Some(now + delay);
+        }
+
         // if we can already send a packet, there is no need for delay
         if self.tokens >= bytes_to_send {
             return None;
@@ -117,12 +126,6 @@ impl Pacer {
             return None;
         }
 
-        if let Some(pacing_rate) = pacing_rate {
-            let capped_bytes_to_send = bytes_to_send.max(self.capacity);
-            let delay = Duration::from_secs_f64(capped_bytes_to_send as f64 / pacing_rate as f64);
-            return Some(now + delay);
-        }
-
         let unscaled_delay = smoothed_rtt
             .checked_mul((bytes_to_send.max(self.capacity) - self.tokens) as _)
             .unwrap_or(Duration::MAX)