From 7b5de651d15eebd78f3109b4034f36627f2fb18e Mon Sep 17 00:00:00 2001 From: Lars Eggert Date: Wed, 10 Apr 2024 11:22:22 +0300 Subject: [PATCH 1/2] fix: Correctly manage `bytes_in_flight` during a rebinding event Fixes #1289 --- neqo-transport/src/cc/classic_cc.rs | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/neqo-transport/src/cc/classic_cc.rs b/neqo-transport/src/cc/classic_cc.rs index a63d6e0b38..d8b41156f9 100644 --- a/neqo-transport/src/cc/classic_cc.rs +++ b/neqo-transport/src/cc/classic_cc.rs @@ -179,8 +179,9 @@ impl CongestionControl for ClassicCongestionControl { if pkt.pn < self.first_app_limited { is_app_limited = false; } - assert!(self.bytes_in_flight >= pkt.size); - self.bytes_in_flight -= pkt.size; + // BIF is set to 0 on a path change, but in case that was because of a simple rebinding + // event, we may still get ACKs for packets sent before the rebinding. + self.bytes_in_flight = self.bytes_in_flight.saturating_sub(pkt.size); if !self.after_recovery_start(pkt) { // Do not increase congestion window for packets sent before @@ -271,8 +272,9 @@ impl CongestionControl for ClassicCongestionControl { pkt.pn, pkt.size ); - assert!(self.bytes_in_flight >= pkt.size); - self.bytes_in_flight -= pkt.size; + // BIF is set to 0 on a path change, but in case that was because of a simple rebinding + // event, we may still get ACKs for packets sent before the rebinding. + self.bytes_in_flight = self.bytes_in_flight.saturating_sub(pkt.size); } qlog::metrics_updated( &mut self.qlog, From 412dc226bac7daeba4fe7162b5e11a13ab8d75ce Mon Sep 17 00:00:00 2001 From: Lars Eggert Date: Wed, 10 Apr 2024 11:32:03 +0300 Subject: [PATCH 2/2] Update comment. Remove incorrect `clippy::unused_self` while I'm here. --- neqo-transport/src/cc/classic_cc.rs | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/neqo-transport/src/cc/classic_cc.rs b/neqo-transport/src/cc/classic_cc.rs index d8b41156f9..f8bcee6722 100644 --- a/neqo-transport/src/cc/classic_cc.rs +++ b/neqo-transport/src/cc/classic_cc.rs @@ -273,7 +273,7 @@ impl CongestionControl for ClassicCongestionControl { pkt.size ); // BIF is set to 0 on a path change, but in case that was because of a simple rebinding - // event, we may still get ACKs for packets sent before the rebinding. + // event, we may still declare packets lost that were sent before the rebinding. self.bytes_in_flight = self.bytes_in_flight.saturating_sub(pkt.size); } qlog::metrics_updated( @@ -518,7 +518,6 @@ impl ClassicCongestionControl { true } - #[allow(clippy::unused_self)] fn app_limited(&self) -> bool { if self.bytes_in_flight >= self.congestion_window { false