From 5aae8266255766966515529b8d07faa81a6ccd35 Mon Sep 17 00:00:00 2001
From: Lars Eggert <lars@eggert.org>
Date: Sun, 21 Jul 2024 14:26:34 -0700
Subject: [PATCH 01/52] tmp

---
 .../src/connection/tests/zerortt.rs           | 59 +++++++++++++++++--
 neqo-transport/src/crypto.rs                  |  4 +-
 neqo-transport/src/rtt.rs                     |  6 +-
 qns/interop.sh                                |  2 +-
 test/test.sh                                  |  6 +-
 5 files changed, 67 insertions(+), 10 deletions(-)

diff --git a/neqo-transport/src/connection/tests/zerortt.rs b/neqo-transport/src/connection/tests/zerortt.rs
index a34e5acdb2..561e983c7e 100644
--- a/neqo-transport/src/connection/tests/zerortt.rs
+++ b/neqo-transport/src/connection/tests/zerortt.rs
@@ -4,9 +4,9 @@
 // option. This file may not be copied, modified, or distributed
 // except according to those terms.
 
-use std::{cell::RefCell, rc::Rc};
+use std::{cell::RefCell, rc::Rc, time::Duration};
 
-use neqo_common::event::Provider;
+use neqo_common::{event::Provider, qinfo};
 use neqo_crypto::{AllowZeroRtt, AntiReplay};
 use test_fixture::{assertions, now};
 
@@ -15,8 +15,11 @@ use super::{
     resumed_server, CountingConnectionIdGenerator,
 };
 use crate::{
-    events::ConnectionEvent, ConnectionParameters, Error, StreamType, Version,
-    MIN_INITIAL_PACKET_SIZE,
+    addr_valid::AddressValidation,
+    connection::tests::{connect_with_rtt, get_tokens},
+    events::ConnectionEvent,
+    server::ValidateAddress,
+    ConnectionParameters, Error, StreamType, Version, MIN_INITIAL_PACKET_SIZE,
 };
 
 #[test]
@@ -258,3 +261,51 @@ fn zero_rtt_update_flow_control() {
     assert!(client.stream_send_atomic(uni_stream, MESSAGE).unwrap());
     assert!(client.stream_send_atomic(bidi_stream, MESSAGE).unwrap());
 }
+
+#[test]
+fn zero_rtt_loss_recovery() {
+    let rtt = Duration::from_millis(10);
+    let mut now = now();
+    let mut client = default_client();
+    let mut server = default_server();
+    connect_with_rtt(&mut client, &mut server, now, rtt);
+    assert_eq!(client.paths.rtt(), rtt);
+
+    // We can't use exchange_ticket here because it doesn't respect RTT.
+    // Also, connect_with_rtt() ends with the server receiving a packet it
+    // wants to acknowledge; so the ticket will include an ACK frame too.
+    let validation = AddressValidation::new(now, ValidateAddress::NoToken).unwrap();
+    let validation = Rc::new(RefCell::new(validation));
+    server.set_validation(&validation);
+    server.send_ticket(now, &[]).expect("can send ticket");
+    let ticket = server.process_output(now).dgram();
+    assert!(ticket.is_some());
+    now += rtt / 2;
+    client.process_input(&ticket.unwrap(), now);
+    let token = get_tokens(&mut client).pop().unwrap();
+
+    let mut client = default_client();
+    client
+        .enable_resumption(now, token)
+        .expect("should set token");
+    let mut server = resumed_server(&client);
+    qinfo!("XXXXXX");
+
+    // Send ClientHello.
+    let client_hs = client.process(None, now);
+    assert!(client_hs.as_dgram_ref().is_some());
+
+    // Now send a 0-RTT packet.
+    let client_stream_id = client.stream_create(StreamType::UniDi).unwrap();
+    client.stream_send(client_stream_id, &[1, 2, 3]).unwrap();
+    let client_0rtt = client.process(None, now);
+    assert!(client_0rtt.as_dgram_ref().is_some());
+
+    now += rtt / 2;
+
+    let server_hs = server.process(client_hs.as_dgram_ref(), now);
+    assert!(server_hs.as_dgram_ref().is_some());
+
+    let server_process_0rtt = server.process(client_0rtt.as_dgram_ref(), now);
+    assert!(server_process_0rtt.as_dgram_ref().is_some());
+}
diff --git a/neqo-transport/src/crypto.rs b/neqo-transport/src/crypto.rs
index 2123ed5ff1..581d776ac1 100644
--- a/neqo-transport/src/crypto.rs
+++ b/neqo-transport/src/crypto.rs
@@ -223,9 +223,11 @@ impl Crypto {
         // `info.early_data()` returns false for a server,
         // so use `early_data_cipher()` to tell if 0-RTT is enabled.
         let cipher = info.early_data_cipher();
+        qinfo!("Early data cipher {:?}", cipher);
         if cipher.is_none() {
             return Ok(false);
         }
+        qinfo!([self], "XXXX 0-RTT enabled");
         let (dir, secret) = match role {
             Role::Client => (
                 CryptoDxDirection::Write,
@@ -1335,7 +1337,7 @@ pub struct CryptoStream {
 }
 
 #[derive(Debug)]
-#[allow(dead_code)] // Suppress false positive: https://github.com/rust-lang/rust/issues/68408
+#[allow(dead_code)]// Suppress false positive: https://github.com/rust-lang/rust/issues/68408
 pub enum CryptoStreams {
     Initial {
         initial: CryptoStream,
diff --git a/neqo-transport/src/rtt.rs b/neqo-transport/src/rtt.rs
index 736a229bb4..6c01b48f82 100644
--- a/neqo-transport/src/rtt.rs
+++ b/neqo-transport/src/rtt.rs
@@ -157,7 +157,11 @@ impl RttEstimate {
         // loss_delay = kTimeThreshold * max(latest_rtt, smoothed_rtt)
         // loss_delay = max(loss_delay, kGranularity)
         let rtt = max(self.latest_rtt, self.smoothed_rtt);
-        max(rtt * 9 / 8, GRANULARITY)
+        // A fixed kTimeThreshold often leads to falsely declaring 0-RTT packets lost,
+        // probably because the peer needs to do a bunch of handshake processing that
+        // delays the ACK.  So, we use a dynamic threshold that use the max of kTimeThreshold or the
+        // peer's max ACK delay.
+        max(rtt + max(rtt / 8, self.ack_delay.max()), GRANULARITY)
     }
 
     pub const fn first_sample_time(&self) -> Option<Instant> {
diff --git a/qns/interop.sh b/qns/interop.sh
index c83dd6552d..d47bd072e9 100755
--- a/qns/interop.sh
+++ b/qns/interop.sh
@@ -13,7 +13,7 @@ case "$ROLE" in
 client)
   /wait-for-it.sh sim:57832 -s -t 30
   # shellcheck disable=SC2086
-  RUST_LOG=debug RUST_BACKTRACE=1 neqo-client --cc cubic --qns-test "$TESTCASE" \
+  RUST_LOG=trace RUST_BACKTRACE=1 neqo-client --cc cubic --qns-test "$TESTCASE" \
     --qlog-dir "$QLOGDIR" --output-dir /downloads $REQUESTS 2> >(tee -i -a "/logs/$ROLE.log" >&2)
   ;;
 
diff --git a/test/test.sh b/test/test.sh
index 99286a19d7..30d657313c 100755
--- a/test/test.sh
+++ b/test/test.sh
@@ -16,15 +16,15 @@ cargo build --bin neqo-client --bin neqo-server
 addr=127.0.0.1
 port=4433
 path=/20000
-flags="--verbose --verbose --verbose --qlog-dir $tmp --use-old-http --alpn hq-interop --quic-version 1"
+flags="--verbose --verbose --verbose --verbose --qlog-dir $tmp --use-old-http --alpn hq-interop --quic-version 1"
 if [ "$(uname -s)" != "Linux" ]; then
         iface=lo0
 else
         iface=lo
 fi
 
-client="./target/debug/neqo-client $flags --output-dir $tmp --stats https://$addr:$port$path"
-server="SSLKEYLOGFILE=$tmp/test.tlskey ./target/debug/neqo-server $flags $addr:$port"
+client="./target/debug/neqo-client $flags --output-dir $tmp --qns-test zerortt --idle 2 --stats https://$addr:$port$path https://$addr:$port$path"
+server="SSLKEYLOGFILE=$tmp/test.tlskey ./target/debug/neqo-server $flags --qns-test zerortt $addr:$port"
 
 tcpdump -U -i "$iface" -w "$tmp/test.pcap" host $addr and port $port >/dev/null 2>&1 &
 tcpdump_pid=$!

From 776fcf46df39a37144e8f86ac09615ba08d0e557 Mon Sep 17 00:00:00 2001
From: Lars Eggert <lars@eggert.org>
Date: Sun, 21 Jul 2024 15:28:53 -0700
Subject: [PATCH 02/52] Cleanup

---
 neqo-transport/src/crypto.rs       |  2 --
 neqo-transport/src/recovery/mod.rs | 11 +++++++----
 qns/interop.sh                     |  2 +-
 test-fixture/src/lib.rs            |  2 +-
 test/test.sh                       |  6 +++---
 5 files changed, 12 insertions(+), 11 deletions(-)

diff --git a/neqo-transport/src/crypto.rs b/neqo-transport/src/crypto.rs
index 581d776ac1..456a81cd41 100644
--- a/neqo-transport/src/crypto.rs
+++ b/neqo-transport/src/crypto.rs
@@ -223,11 +223,9 @@ impl Crypto {
         // `info.early_data()` returns false for a server,
         // so use `early_data_cipher()` to tell if 0-RTT is enabled.
         let cipher = info.early_data_cipher();
-        qinfo!("Early data cipher {:?}", cipher);
         if cipher.is_none() {
             return Ok(false);
         }
-        qinfo!([self], "XXXX 0-RTT enabled");
         let (dir, secret) = match role {
             Role::Client => (
                 CryptoDxDirection::Write,
diff --git a/neqo-transport/src/recovery/mod.rs b/neqo-transport/src/recovery/mod.rs
index e697e78695..9150afb8d5 100644
--- a/neqo-transport/src/recovery/mod.rs
+++ b/neqo-transport/src/recovery/mod.rs
@@ -944,6 +944,7 @@ impl ::std::fmt::Display for LossRecovery {
 mod tests {
     use std::{
         cell::RefCell,
+        cmp::max,
         convert::TryInto,
         ops::{Deref, DerefMut, RangeInclusive},
         rc::Rc,
@@ -957,12 +958,13 @@ mod tests {
         LossRecovery, LossRecoverySpace, PacketNumberSpace, SendProfile, SentPacket, FAST_PTO_SCALE,
     };
     use crate::{
+        ackrate::PeerAckDelay,
         cc::CongestionControlAlgorithm,
         cid::{ConnectionId, ConnectionIdEntry},
         ecn::EcnCount,
         packet::{PacketNumber, PacketType},
         path::{Path, PathRef},
-        rtt::RttEstimate,
+        rtt::{RttEstimate, INITIAL_RTT},
         stats::{Stats, StatsCell},
     };
 
@@ -973,8 +975,8 @@ mod tests {
 
     const ON_SENT_SIZE: usize = 100;
     /// An initial RTT for using with `setup_lr`.
-    const TEST_RTT: Duration = ms(80);
-    const TEST_RTTVAR: Duration = ms(40);
+    const TEST_RTT: Duration = INITIAL_RTT;
+    const TEST_RTTVAR: Duration = Duration::from_millis(INITIAL_RTT.as_millis() as u64 / 2);
 
     struct Fixture {
         lr: LossRecovery,
@@ -1325,7 +1327,8 @@ mod tests {
         // We want to declare PN 2 as acknowledged before we declare PN 1 as lost.
         // For this to work, we need PACING above to be less than 1/8 of an RTT.
         let pn1_sent_time = pn_time(1);
-        let pn1_loss_time = pn1_sent_time + (TEST_RTT * 9 / 8);
+        let pn1_loss_time =
+            pn1_sent_time + (TEST_RTT + max(TEST_RTT / 8, PeerAckDelay::default().max()));
         let pn2_ack_time = pn_time(2) + TEST_RTT;
         assert!(pn1_loss_time > pn2_ack_time);
 
diff --git a/qns/interop.sh b/qns/interop.sh
index d47bd072e9..c83dd6552d 100755
--- a/qns/interop.sh
+++ b/qns/interop.sh
@@ -13,7 +13,7 @@ case "$ROLE" in
 client)
   /wait-for-it.sh sim:57832 -s -t 30
   # shellcheck disable=SC2086
-  RUST_LOG=trace RUST_BACKTRACE=1 neqo-client --cc cubic --qns-test "$TESTCASE" \
+  RUST_LOG=debug RUST_BACKTRACE=1 neqo-client --cc cubic --qns-test "$TESTCASE" \
     --qlog-dir "$QLOGDIR" --output-dir /downloads $REQUESTS 2> >(tee -i -a "/logs/$ROLE.log" >&2)
   ;;
 
diff --git a/test-fixture/src/lib.rs b/test-fixture/src/lib.rs
index 54fdaa6ebd..a39e20c1b4 100644
--- a/test-fixture/src/lib.rs
+++ b/test-fixture/src/lib.rs
@@ -52,7 +52,7 @@ pub fn fixture_init() {
 
 // This needs to be > 2ms to avoid it being rounded to zero.
 // NSS operates in milliseconds and halves any value it is provided.
-pub const ANTI_REPLAY_WINDOW: Duration = Duration::from_millis(10);
+pub const ANTI_REPLAY_WINDOW: Duration = Duration::from_millis(1000);
 
 /// A baseline time for all tests.  This needs to be earlier than what `now()` produces
 /// because of the need to have a span of time elapse for anti-replay purposes.
diff --git a/test/test.sh b/test/test.sh
index 30d657313c..99286a19d7 100755
--- a/test/test.sh
+++ b/test/test.sh
@@ -16,15 +16,15 @@ cargo build --bin neqo-client --bin neqo-server
 addr=127.0.0.1
 port=4433
 path=/20000
-flags="--verbose --verbose --verbose --verbose --qlog-dir $tmp --use-old-http --alpn hq-interop --quic-version 1"
+flags="--verbose --verbose --verbose --qlog-dir $tmp --use-old-http --alpn hq-interop --quic-version 1"
 if [ "$(uname -s)" != "Linux" ]; then
         iface=lo0
 else
         iface=lo
 fi
 
-client="./target/debug/neqo-client $flags --output-dir $tmp --qns-test zerortt --idle 2 --stats https://$addr:$port$path https://$addr:$port$path"
-server="SSLKEYLOGFILE=$tmp/test.tlskey ./target/debug/neqo-server $flags --qns-test zerortt $addr:$port"
+client="./target/debug/neqo-client $flags --output-dir $tmp --stats https://$addr:$port$path"
+server="SSLKEYLOGFILE=$tmp/test.tlskey ./target/debug/neqo-server $flags $addr:$port"
 
 tcpdump -U -i "$iface" -w "$tmp/test.pcap" host $addr and port $port >/dev/null 2>&1 &
 tcpdump_pid=$!

From 7334fac6c77197a95d03d793f3863c9c48b55aef Mon Sep 17 00:00:00 2001
From: Lars Eggert <lars@eggert.org>
Date: Sun, 21 Jul 2024 15:53:15 -0700
Subject: [PATCH 03/52] Finalize test

---
 .../src/connection/tests/zerortt.rs           | 36 ++++++++-----------
 neqo-transport/src/crypto.rs                  |  2 +-
 neqo-transport/src/recovery/mod.rs            |  1 +
 3 files changed, 17 insertions(+), 22 deletions(-)

diff --git a/neqo-transport/src/connection/tests/zerortt.rs b/neqo-transport/src/connection/tests/zerortt.rs
index 561e983c7e..3a93ff340f 100644
--- a/neqo-transport/src/connection/tests/zerortt.rs
+++ b/neqo-transport/src/connection/tests/zerortt.rs
@@ -6,7 +6,7 @@
 
 use std::{cell::RefCell, rc::Rc, time::Duration};
 
-use neqo_common::{event::Provider, qinfo};
+use neqo_common::event::Provider;
 use neqo_crypto::{AllowZeroRtt, AntiReplay};
 use test_fixture::{assertions, now};
 
@@ -15,10 +15,7 @@ use super::{
     resumed_server, CountingConnectionIdGenerator,
 };
 use crate::{
-    addr_valid::AddressValidation,
-    connection::tests::{connect_with_rtt, get_tokens},
-    events::ConnectionEvent,
-    server::ValidateAddress,
+    ackrate::PeerAckDelay, connection::tests::connect_with_rtt, events::ConnectionEvent,
     ConnectionParameters, Error, StreamType, Version, MIN_INITIAL_PACKET_SIZE,
 };
 
@@ -271,25 +268,12 @@ fn zero_rtt_loss_recovery() {
     connect_with_rtt(&mut client, &mut server, now, rtt);
     assert_eq!(client.paths.rtt(), rtt);
 
-    // We can't use exchange_ticket here because it doesn't respect RTT.
-    // Also, connect_with_rtt() ends with the server receiving a packet it
-    // wants to acknowledge; so the ticket will include an ACK frame too.
-    let validation = AddressValidation::new(now, ValidateAddress::NoToken).unwrap();
-    let validation = Rc::new(RefCell::new(validation));
-    server.set_validation(&validation);
-    server.send_ticket(now, &[]).expect("can send ticket");
-    let ticket = server.process_output(now).dgram();
-    assert!(ticket.is_some());
-    now += rtt / 2;
-    client.process_input(&ticket.unwrap(), now);
-    let token = get_tokens(&mut client).pop().unwrap();
-
+    let token = exchange_ticket(&mut client, &mut server, now);
     let mut client = default_client();
     client
         .enable_resumption(now, token)
         .expect("should set token");
     let mut server = resumed_server(&client);
-    qinfo!("XXXXXX");
 
     // Send ClientHello.
     let client_hs = client.process(None, now);
@@ -301,11 +285,21 @@ fn zero_rtt_loss_recovery() {
     let client_0rtt = client.process(None, now);
     assert!(client_0rtt.as_dgram_ref().is_some());
 
-    now += rtt / 2;
-
     let server_hs = server.process(client_hs.as_dgram_ref(), now);
     assert!(server_hs.as_dgram_ref().is_some());
 
     let server_process_0rtt = server.process(client_0rtt.as_dgram_ref(), now);
     assert!(server_process_0rtt.as_dgram_ref().is_some());
+
+    // After RTT*9/8, the client should not have declared anything lost yet.
+    now += rtt * 9 / 8;
+    let pkt = client.process(None, now);
+    assert!(pkt.as_dgram_ref().is_none());
+    assert_eq!(client.stats().lost, 0);
+
+    // After RTT*9/8, the client should have declared the three packets as lost.
+    now += (rtt - rtt / 8) + PeerAckDelay::default().max();
+    let pkt = client.process(None, now);
+    assert!(pkt.as_dgram_ref().is_some());
+    assert_eq!(client.stats().lost, 3);
 }
diff --git a/neqo-transport/src/crypto.rs b/neqo-transport/src/crypto.rs
index 456a81cd41..2123ed5ff1 100644
--- a/neqo-transport/src/crypto.rs
+++ b/neqo-transport/src/crypto.rs
@@ -1335,7 +1335,7 @@ pub struct CryptoStream {
 }
 
 #[derive(Debug)]
-#[allow(dead_code)]// Suppress false positive: https://github.com/rust-lang/rust/issues/68408
+#[allow(dead_code)] // Suppress false positive: https://github.com/rust-lang/rust/issues/68408
 pub enum CryptoStreams {
     Initial {
         initial: CryptoStream,
diff --git a/neqo-transport/src/recovery/mod.rs b/neqo-transport/src/recovery/mod.rs
index 9150afb8d5..5024cbd940 100644
--- a/neqo-transport/src/recovery/mod.rs
+++ b/neqo-transport/src/recovery/mod.rs
@@ -976,6 +976,7 @@ mod tests {
     const ON_SENT_SIZE: usize = 100;
     /// An initial RTT for using with `setup_lr`.
     const TEST_RTT: Duration = INITIAL_RTT;
+    #[allow(clippy::cast_possible_truncation)]
     const TEST_RTTVAR: Duration = Duration::from_millis(INITIAL_RTT.as_millis() as u64 / 2);
 
     struct Fixture {

From b6e05e304b1ef582bc232710d3e1177fb5b371fe Mon Sep 17 00:00:00 2001
From: Lars Eggert <lars@eggert.org>
Date: Sun, 21 Jul 2024 18:09:55 -0700
Subject: [PATCH 04/52] Argh

---
 .../src/connection/tests/handshake.rs         |  6 +--
 .../src/connection/tests/recovery.rs          |  9 +---
 .../src/connection/tests/zerortt.rs           | 51 ++-----------------
 neqo-transport/src/recovery/mod.rs            |  7 +--
 neqo-transport/src/rtt.rs                     |  6 +--
 5 files changed, 10 insertions(+), 69 deletions(-)

diff --git a/neqo-transport/src/connection/tests/handshake.rs b/neqo-transport/src/connection/tests/handshake.rs
index fce6e0bf97..a9f2a27b12 100644
--- a/neqo-transport/src/connection/tests/handshake.rs
+++ b/neqo-transport/src/connection/tests/handshake.rs
@@ -18,9 +18,7 @@ use neqo_crypto::{
 };
 #[cfg(not(feature = "disable-encryption"))]
 use test_fixture::datagram;
-use test_fixture::{
-    assertions, assertions::assert_coalesced_0rtt, fixture_init, now, split_datagram, DEFAULT_ADDR,
-};
+use test_fixture::{assertions, fixture_init, now, split_datagram, DEFAULT_ADDR};
 
 use super::{
     super::{Connection, Output, State},
@@ -401,10 +399,10 @@ fn reorder_05rtt_with_0rtt() {
     // Now PTO at the client and cause the server to re-send handshake packets.
     now += AT_LEAST_PTO;
     let c3 = client.process(None, now).dgram();
-    assert_coalesced_0rtt(c3.as_ref().unwrap());
 
     now += RTT / 2;
     let s3 = server.process(c3.as_ref(), now).dgram().unwrap();
+    assertions::assert_no_1rtt(&s3[..]);
 
     // The client should be able to process the 0.5 RTT now.
     // This should contain an ACK, so we are processing an ACK from the past.
diff --git a/neqo-transport/src/connection/tests/recovery.rs b/neqo-transport/src/connection/tests/recovery.rs
index a94a3c0b5d..d4baecc326 100644
--- a/neqo-transport/src/connection/tests/recovery.rs
+++ b/neqo-transport/src/connection/tests/recovery.rs
@@ -294,21 +294,16 @@ fn pto_handshake_complete() {
     // Check that the other packets (pkt2, pkt3) are Handshake packets.
     // The server discarded the Handshake keys already, therefore they are dropped.
     // Note that these don't include 1-RTT packets, because 1-RTT isn't send on PTO.
-    let (pkt2_hs, pkt2_1rtt) = split_datagram(&pkt2.unwrap());
-    assert_handshake(&pkt2_hs);
-    assert!(pkt2_1rtt.is_some());
     let dropped_before1 = server.stats().dropped_rx;
     let server_frames = server.stats().frame_rx.all;
-    server.process_input(&pkt2_hs, now);
+    server.process_input(&pkt2.unwrap(), now);
     assert_eq!(1, server.stats().dropped_rx - dropped_before1);
     assert_eq!(server.stats().frame_rx.all, server_frames);
 
-    server.process_input(&pkt2_1rtt.unwrap(), now);
-    let server_frames2 = server.stats().frame_rx.all;
     let dropped_before2 = server.stats().dropped_rx;
     server.process_input(&pkt3_hs, now);
     assert_eq!(1, server.stats().dropped_rx - dropped_before2);
-    assert_eq!(server.stats().frame_rx.all, server_frames2);
+    assert_eq!(server.stats().frame_rx.all, server_frames);
 
     now += HALF_RTT;
 
diff --git a/neqo-transport/src/connection/tests/zerortt.rs b/neqo-transport/src/connection/tests/zerortt.rs
index 3a93ff340f..a34e5acdb2 100644
--- a/neqo-transport/src/connection/tests/zerortt.rs
+++ b/neqo-transport/src/connection/tests/zerortt.rs
@@ -4,7 +4,7 @@
 // option. This file may not be copied, modified, or distributed
 // except according to those terms.
 
-use std::{cell::RefCell, rc::Rc, time::Duration};
+use std::{cell::RefCell, rc::Rc};
 
 use neqo_common::event::Provider;
 use neqo_crypto::{AllowZeroRtt, AntiReplay};
@@ -15,8 +15,8 @@ use super::{
     resumed_server, CountingConnectionIdGenerator,
 };
 use crate::{
-    ackrate::PeerAckDelay, connection::tests::connect_with_rtt, events::ConnectionEvent,
-    ConnectionParameters, Error, StreamType, Version, MIN_INITIAL_PACKET_SIZE,
+    events::ConnectionEvent, ConnectionParameters, Error, StreamType, Version,
+    MIN_INITIAL_PACKET_SIZE,
 };
 
 #[test]
@@ -258,48 +258,3 @@ fn zero_rtt_update_flow_control() {
     assert!(client.stream_send_atomic(uni_stream, MESSAGE).unwrap());
     assert!(client.stream_send_atomic(bidi_stream, MESSAGE).unwrap());
 }
-
-#[test]
-fn zero_rtt_loss_recovery() {
-    let rtt = Duration::from_millis(10);
-    let mut now = now();
-    let mut client = default_client();
-    let mut server = default_server();
-    connect_with_rtt(&mut client, &mut server, now, rtt);
-    assert_eq!(client.paths.rtt(), rtt);
-
-    let token = exchange_ticket(&mut client, &mut server, now);
-    let mut client = default_client();
-    client
-        .enable_resumption(now, token)
-        .expect("should set token");
-    let mut server = resumed_server(&client);
-
-    // Send ClientHello.
-    let client_hs = client.process(None, now);
-    assert!(client_hs.as_dgram_ref().is_some());
-
-    // Now send a 0-RTT packet.
-    let client_stream_id = client.stream_create(StreamType::UniDi).unwrap();
-    client.stream_send(client_stream_id, &[1, 2, 3]).unwrap();
-    let client_0rtt = client.process(None, now);
-    assert!(client_0rtt.as_dgram_ref().is_some());
-
-    let server_hs = server.process(client_hs.as_dgram_ref(), now);
-    assert!(server_hs.as_dgram_ref().is_some());
-
-    let server_process_0rtt = server.process(client_0rtt.as_dgram_ref(), now);
-    assert!(server_process_0rtt.as_dgram_ref().is_some());
-
-    // After RTT*9/8, the client should not have declared anything lost yet.
-    now += rtt * 9 / 8;
-    let pkt = client.process(None, now);
-    assert!(pkt.as_dgram_ref().is_none());
-    assert_eq!(client.stats().lost, 0);
-
-    // After RTT*9/8, the client should have declared the three packets as lost.
-    now += (rtt - rtt / 8) + PeerAckDelay::default().max();
-    let pkt = client.process(None, now);
-    assert!(pkt.as_dgram_ref().is_some());
-    assert_eq!(client.stats().lost, 3);
-}
diff --git a/neqo-transport/src/recovery/mod.rs b/neqo-transport/src/recovery/mod.rs
index 5024cbd940..f772520518 100644
--- a/neqo-transport/src/recovery/mod.rs
+++ b/neqo-transport/src/recovery/mod.rs
@@ -327,7 +327,7 @@ impl LossRecoverySpace {
             .sent_packets
             .iter_mut()
             // BTreeMap iterates in order of ascending PN
-            .take_while(|p| p.pn() < largest_acked.unwrap_or(PacketNumber::MAX))
+            .take_while(|p| p.pn() < largest_acked.unwrap_or(0))
         {
             // Packets sent before now - loss_delay are deemed lost.
             if packet.time_sent() + loss_delay <= now {
@@ -944,7 +944,6 @@ impl ::std::fmt::Display for LossRecovery {
 mod tests {
     use std::{
         cell::RefCell,
-        cmp::max,
         convert::TryInto,
         ops::{Deref, DerefMut, RangeInclusive},
         rc::Rc,
@@ -958,7 +957,6 @@ mod tests {
         LossRecovery, LossRecoverySpace, PacketNumberSpace, SendProfile, SentPacket, FAST_PTO_SCALE,
     };
     use crate::{
-        ackrate::PeerAckDelay,
         cc::CongestionControlAlgorithm,
         cid::{ConnectionId, ConnectionIdEntry},
         ecn::EcnCount,
@@ -1328,8 +1326,7 @@ mod tests {
         // We want to declare PN 2 as acknowledged before we declare PN 1 as lost.
         // For this to work, we need PACING above to be less than 1/8 of an RTT.
         let pn1_sent_time = pn_time(1);
-        let pn1_loss_time =
-            pn1_sent_time + (TEST_RTT + max(TEST_RTT / 8, PeerAckDelay::default().max()));
+        let pn1_loss_time = pn1_sent_time + (TEST_RTT * 9 / 8);
         let pn2_ack_time = pn_time(2) + TEST_RTT;
         assert!(pn1_loss_time > pn2_ack_time);
 
diff --git a/neqo-transport/src/rtt.rs b/neqo-transport/src/rtt.rs
index 6c01b48f82..736a229bb4 100644
--- a/neqo-transport/src/rtt.rs
+++ b/neqo-transport/src/rtt.rs
@@ -157,11 +157,7 @@ impl RttEstimate {
         // loss_delay = kTimeThreshold * max(latest_rtt, smoothed_rtt)
         // loss_delay = max(loss_delay, kGranularity)
         let rtt = max(self.latest_rtt, self.smoothed_rtt);
-        // A fixed kTimeThreshold often leads to falsely declaring 0-RTT packets lost,
-        // probably because the peer needs to do a bunch of handshake processing that
-        // delays the ACK.  So, we use a dynamic threshold that use the max of kTimeThreshold or the
-        // peer's max ACK delay.
-        max(rtt + max(rtt / 8, self.ack_delay.max()), GRANULARITY)
+        max(rtt * 9 / 8, GRANULARITY)
     }
 
     pub const fn first_sample_time(&self) -> Option<Instant> {

From 2af5adddd81a15eef7d51131e0201205bac502c8 Mon Sep 17 00:00:00 2001
From: Lars Eggert <lars@eggert.org>
Date: Mon, 22 Jul 2024 13:50:07 -0700
Subject: [PATCH 05/52] Fix

---
 neqo-transport/src/cc/classic_cc.rs            |  3 +--
 neqo-transport/src/connection/tests/ackrate.rs |  3 ++-
 .../src/connection/tests/migration.rs          |  2 --
 .../src/connection/tests/recovery.rs           |  2 ++
 neqo-transport/src/recovery/mod.rs             | 18 ++++++++++++++----
 5 files changed, 19 insertions(+), 9 deletions(-)

diff --git a/neqo-transport/src/cc/classic_cc.rs b/neqo-transport/src/cc/classic_cc.rs
index f7beb3ad61..f0fe947008 100644
--- a/neqo-transport/src/cc/classic_cc.rs
+++ b/neqo-transport/src/cc/classic_cc.rs
@@ -347,8 +347,7 @@ impl<T: WindowAdjustment> CongestionControl for ClassicCongestionControl<T> {
 
     fn discard(&mut self, pkt: &SentPacket) {
         if pkt.cc_outstanding() {
-            assert!(self.bytes_in_flight >= pkt.len());
-            self.bytes_in_flight -= pkt.len();
+            self.bytes_in_flight = self.bytes_in_flight.saturating_sub(pkt.len());
             qlog::metrics_updated(
                 &self.qlog,
                 &[QlogMetric::BytesInFlight(self.bytes_in_flight)],
diff --git a/neqo-transport/src/connection/tests/ackrate.rs b/neqo-transport/src/connection/tests/ackrate.rs
index f0a1d17cd9..4630fe9541 100644
--- a/neqo-transport/src/connection/tests/ackrate.rs
+++ b/neqo-transport/src/connection/tests/ackrate.rs
@@ -99,9 +99,10 @@ fn ack_rate_persistent_congestion() {
     let now = induce_persistent_congestion(&mut client, &mut server, stream, now);
 
     // The client sends a second ACK_FREQUENCY frame with an increased rate.
+    // That one is declared lost, and a is hence retransmitted as a third.
     let af = client.process_output(now).dgram();
     assert!(af.is_some());
-    assert_eq!(client.stats().frame_tx.ack_frequency, 2);
+    assert_eq!(client.stats().frame_tx.ack_frequency, 3);
 }
 
 /// Validate that the configuration works for the client.
diff --git a/neqo-transport/src/connection/tests/migration.rs b/neqo-transport/src/connection/tests/migration.rs
index 3ee88943dd..81451a4550 100644
--- a/neqo-transport/src/connection/tests/migration.rs
+++ b/neqo-transport/src/connection/tests/migration.rs
@@ -256,14 +256,12 @@ fn migrate_immediate_fail() {
         let after = client.stats().frame_tx;
         assert_eq!(after.path_challenge, before.path_challenge + 1);
         assert_eq!(after.padding, before.padding + 1);
-        assert_eq!(after.all, before.all + 2);
 
         // This might be a PTO, which will result in sending a probe.
         if let Some(probe) = client.process_output(now).dgram() {
             assert_v4_path(&probe, false); // Contains PATH_CHALLENGE.
             let after = client.stats().frame_tx;
             assert_eq!(after.ping, before.ping + 1);
-            assert_eq!(after.all, before.all + 3);
         }
     }
 
diff --git a/neqo-transport/src/connection/tests/recovery.rs b/neqo-transport/src/connection/tests/recovery.rs
index d4baecc326..bd8cf5fe2b 100644
--- a/neqo-transport/src/connection/tests/recovery.rs
+++ b/neqo-transport/src/connection/tests/recovery.rs
@@ -174,10 +174,12 @@ fn pto_initial() {
     assert_eq!(delay, INITIAL_PTO);
 
     // Resend initial after PTO.
+    let cwnd_prior: usize = cwnd(&client);
     now += delay;
     let pkt2 = client.process(None, now).dgram();
     assert!(pkt2.is_some());
     assert_eq!(pkt2.unwrap().len(), client.plpmtu());
+    assert_eq!(cwnd_prior, 2 * cwnd(&client)); // cwnd has halved
 
     let delay = client.process(None, now).callback();
     // PTO has doubled.
diff --git a/neqo-transport/src/recovery/mod.rs b/neqo-transport/src/recovery/mod.rs
index f772520518..1e58e7f89e 100644
--- a/neqo-transport/src/recovery/mod.rs
+++ b/neqo-transport/src/recovery/mod.rs
@@ -835,14 +835,14 @@ impl LossRecovery {
     /// When it has, mark a few packets as "lost" for the purposes of having frames
     /// regenerated in subsequent packets.  The packets aren't truly lost, so
     /// we have to clone the `SentPacket` instance.
-    fn maybe_fire_pto(&mut self, rtt: &RttEstimate, now: Instant, lost: &mut Vec<SentPacket>) {
+    fn maybe_fire_pto(&mut self, path: &PathRef, now: Instant, lost: &mut Vec<SentPacket>) {
         let mut pto_space = None;
         // The spaces in which we will allow probing.
         let mut allow_probes = PacketNumberSpaceSet::default();
         for pn_space in PacketNumberSpace::iter() {
-            if let Some(t) = self.pto_time(rtt, *pn_space) {
-                allow_probes[*pn_space] = true;
+            if let Some(t) = self.pto_time(path.borrow().rtt(), *pn_space) {
                 if t <= now {
+                    allow_probes[*pn_space] = true;
                     qdebug!([self], "PTO timer fired for {}", pn_space);
                     let space = self.spaces.get_mut(*pn_space).unwrap();
                     lost.extend(
@@ -863,6 +863,16 @@ impl LossRecovery {
         // pto_time to increase which might cause PTO for later packet number spaces to not fire.
         if let Some(pn_space) = pto_space {
             qtrace!([self], "PTO {}, probing {:?}", pn_space, allow_probes);
+            // If we hit a PTO, also do a congestion control reaction.
+            if let Some(space) = self.spaces.get(pn_space) {
+                path.borrow_mut().on_packets_lost(
+                    space.largest_acked_sent_time,
+                    pn_space,
+                    lost,
+                    &mut self.stats.borrow_mut(),
+                    now,
+                );
+            }
             self.fire_pto(pn_space, allow_probes);
         }
     }
@@ -893,7 +903,7 @@ impl LossRecovery {
         }
         self.stats.borrow_mut().lost += lost_packets.len();
 
-        self.maybe_fire_pto(primary_path.borrow().rtt(), now, &mut lost_packets);
+        self.maybe_fire_pto(primary_path, now, &mut lost_packets);
         lost_packets
     }
 

From 49e2836b4fb9d9f8c0ebc7820af875ce8dcbdec7 Mon Sep 17 00:00:00 2001
From: Lars Eggert <lars@eggert.org>
Date: Mon, 22 Jul 2024 13:58:57 -0700
Subject: [PATCH 06/52] Suggestion from @mxinden

---
 neqo-transport/src/recovery/mod.rs | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/neqo-transport/src/recovery/mod.rs b/neqo-transport/src/recovery/mod.rs
index 1e58e7f89e..9efcb4d2db 100644
--- a/neqo-transport/src/recovery/mod.rs
+++ b/neqo-transport/src/recovery/mod.rs
@@ -327,7 +327,7 @@ impl LossRecoverySpace {
             .sent_packets
             .iter_mut()
             // BTreeMap iterates in order of ascending PN
-            .take_while(|p| p.pn() < largest_acked.unwrap_or(0))
+            .take_while(|p| Some(p.pn()) < largest_acked)
         {
             // Packets sent before now - loss_delay are deemed lost.
             if packet.time_sent() + loss_delay <= now {

From 150881f889f1fe2d7bcf3a6531a29e9694aa9e85 Mon Sep 17 00:00:00 2001
From: Lars Eggert <lars@eggert.org>
Date: Mon, 22 Jul 2024 14:48:27 -0700
Subject: [PATCH 07/52] Suggestion from @martinthomson

---
 neqo-transport/src/recovery/mod.rs | 15 +++++++++------
 1 file changed, 9 insertions(+), 6 deletions(-)

diff --git a/neqo-transport/src/recovery/mod.rs b/neqo-transport/src/recovery/mod.rs
index 9efcb4d2db..d4af5be06a 100644
--- a/neqo-transport/src/recovery/mod.rs
+++ b/neqo-transport/src/recovery/mod.rs
@@ -972,7 +972,6 @@ mod tests {
         ecn::EcnCount,
         packet::{PacketNumber, PacketType},
         path::{Path, PathRef},
-        rtt::{RttEstimate, INITIAL_RTT},
         stats::{Stats, StatsCell},
     };
 
@@ -983,9 +982,8 @@ mod tests {
 
     const ON_SENT_SIZE: usize = 100;
     /// An initial RTT for using with `setup_lr`.
-    const TEST_RTT: Duration = INITIAL_RTT;
-    #[allow(clippy::cast_possible_truncation)]
-    const TEST_RTTVAR: Duration = Duration::from_millis(INITIAL_RTT.as_millis() as u64 / 2);
+    const TEST_RTT: Duration = ms(7000);
+    const TEST_RTTVAR: Duration = ms(3500);
 
     struct Fixture {
         lr: LossRecovery,
@@ -1056,6 +1054,7 @@ mod tests {
                 ConnectionIdEntry::new(0, ConnectionId::from(&[1, 2, 3]), [0; 16]),
             );
             path.set_primary(true);
+            path.rtt_mut().set_initial(TEST_RTT);
             Self {
                 lr: LossRecovery::new(StatsCell::default(), FAST_PTO_SCALE),
                 path: Rc::new(RefCell::new(path)),
@@ -1546,7 +1545,11 @@ mod tests {
 
         // Expiring state after the PTO on the ApplicationData space has
         // expired should result in setting a PTO state.
-        let default_pto = RttEstimate::default().pto(PacketNumberSpace::ApplicationData);
+        let default_pto = lr
+            .path
+            .borrow()
+            .rtt()
+            .pto(PacketNumberSpace::ApplicationData);
         let expected_pto = pn_time(2) + default_pto;
         lr.discard(PacketNumberSpace::Handshake, expected_pto);
         let profile = lr.send_profile(now());
@@ -1578,7 +1581,7 @@ mod tests {
             ON_SENT_SIZE,
         ));
 
-        let handshake_pto = RttEstimate::default().pto(PacketNumberSpace::Handshake);
+        let handshake_pto = lr.path.borrow().rtt().pto(PacketNumberSpace::Handshake);
         let expected_pto = now() + handshake_pto;
         assert_eq!(lr.pto_time(PacketNumberSpace::Initial), Some(expected_pto));
         let profile = lr.send_profile(now());

From 4bf8205068010b99acd1c29acf48ef29ef19478e Mon Sep 17 00:00:00 2001
From: Lars Eggert <lars@eggert.org>
Date: Mon, 22 Jul 2024 14:49:34 -0700
Subject: [PATCH 08/52] Wording

---
 neqo-transport/src/connection/tests/ackrate.rs | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/neqo-transport/src/connection/tests/ackrate.rs b/neqo-transport/src/connection/tests/ackrate.rs
index 4630fe9541..272b23a732 100644
--- a/neqo-transport/src/connection/tests/ackrate.rs
+++ b/neqo-transport/src/connection/tests/ackrate.rs
@@ -99,7 +99,7 @@ fn ack_rate_persistent_congestion() {
     let now = induce_persistent_congestion(&mut client, &mut server, stream, now);
 
     // The client sends a second ACK_FREQUENCY frame with an increased rate.
-    // That one is declared lost, and a is hence retransmitted as a third.
+    // That one is declared lost, and is hence retransmitted as a third.
     let af = client.process_output(now).dgram();
     assert!(af.is_some());
     assert_eq!(client.stats().frame_tx.ack_frequency, 3);

From a0b7ddfa921d5360db18bfb25bf2c0e0bb61b800 Mon Sep 17 00:00:00 2001
From: Lars Eggert <lars@eggert.org>
Date: Mon, 22 Jul 2024 16:55:34 -0700
Subject: [PATCH 09/52] Only do this when we don't have a largest_acked

---
 .../src/connection/tests/ackrate.rs           |  3 +--
 neqo-transport/src/recovery/mod.rs            | 19 +++++++++++--------
 2 files changed, 12 insertions(+), 10 deletions(-)

diff --git a/neqo-transport/src/connection/tests/ackrate.rs b/neqo-transport/src/connection/tests/ackrate.rs
index 272b23a732..f0a1d17cd9 100644
--- a/neqo-transport/src/connection/tests/ackrate.rs
+++ b/neqo-transport/src/connection/tests/ackrate.rs
@@ -99,10 +99,9 @@ fn ack_rate_persistent_congestion() {
     let now = induce_persistent_congestion(&mut client, &mut server, stream, now);
 
     // The client sends a second ACK_FREQUENCY frame with an increased rate.
-    // That one is declared lost, and is hence retransmitted as a third.
     let af = client.process_output(now).dgram();
     assert!(af.is_some());
-    assert_eq!(client.stats().frame_tx.ack_frequency, 3);
+    assert_eq!(client.stats().frame_tx.ack_frequency, 2);
 }
 
 /// Validate that the configuration works for the client.
diff --git a/neqo-transport/src/recovery/mod.rs b/neqo-transport/src/recovery/mod.rs
index d4af5be06a..d886a2fb84 100644
--- a/neqo-transport/src/recovery/mod.rs
+++ b/neqo-transport/src/recovery/mod.rs
@@ -863,15 +863,18 @@ impl LossRecovery {
         // pto_time to increase which might cause PTO for later packet number spaces to not fire.
         if let Some(pn_space) = pto_space {
             qtrace!([self], "PTO {}, probing {:?}", pn_space, allow_probes);
-            // If we hit a PTO, also do a congestion control reaction.
+            // If we hit a PTO while we don't have a largest_acked yet, also do a congestion control
+            // reaction (because otherwise none would happen).
             if let Some(space) = self.spaces.get(pn_space) {
-                path.borrow_mut().on_packets_lost(
-                    space.largest_acked_sent_time,
-                    pn_space,
-                    lost,
-                    &mut self.stats.borrow_mut(),
-                    now,
-                );
+                if space.largest_acked.is_none() {
+                    path.borrow_mut().on_packets_lost(
+                        space.largest_acked_sent_time,
+                        pn_space,
+                        lost,
+                        &mut self.stats.borrow_mut(),
+                        now,
+                    );
+                }
             }
             self.fire_pto(pn_space, allow_probes);
         }

From 4d22083ccb96f9124188d8fe36f5788e0224dd1e Mon Sep 17 00:00:00 2001
From: Lars Eggert <lars@eggert.org>
Date: Mon, 22 Jul 2024 16:58:12 -0700
Subject: [PATCH 10/52] Minimize diff

---
 neqo-transport/src/connection/tests/migration.rs | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/neqo-transport/src/connection/tests/migration.rs b/neqo-transport/src/connection/tests/migration.rs
index 81451a4550..3ee88943dd 100644
--- a/neqo-transport/src/connection/tests/migration.rs
+++ b/neqo-transport/src/connection/tests/migration.rs
@@ -256,12 +256,14 @@ fn migrate_immediate_fail() {
         let after = client.stats().frame_tx;
         assert_eq!(after.path_challenge, before.path_challenge + 1);
         assert_eq!(after.padding, before.padding + 1);
+        assert_eq!(after.all, before.all + 2);
 
         // This might be a PTO, which will result in sending a probe.
         if let Some(probe) = client.process_output(now).dgram() {
             assert_v4_path(&probe, false); // Contains PATH_CHALLENGE.
             let after = client.stats().frame_tx;
             assert_eq!(after.ping, before.ping + 1);
+            assert_eq!(after.all, before.all + 3);
         }
     }
 

From 4f40deee73408fb40873649087dba0ede480623b Mon Sep 17 00:00:00 2001
From: Lars Eggert <lars@eggert.org>
Date: Mon, 22 Jul 2024 18:17:04 -0700
Subject: [PATCH 11/52] Expose `on_congestion_event`

---
 neqo-transport/src/cc/classic_cc.rs | 73 +++++++++++++++--------------
 neqo-transport/src/cc/mod.rs        |  2 +
 neqo-transport/src/path.rs          |  5 ++
 neqo-transport/src/recovery/mod.rs  | 10 ++--
 neqo-transport/src/sender.rs        |  5 ++
 5 files changed, 52 insertions(+), 43 deletions(-)

diff --git a/neqo-transport/src/cc/classic_cc.rs b/neqo-transport/src/cc/classic_cc.rs
index f0fe947008..a5ad7acc4d 100644
--- a/neqo-transport/src/cc/classic_cc.rs
+++ b/neqo-transport/src/cc/classic_cc.rs
@@ -337,6 +337,41 @@ impl<T: WindowAdjustment> CongestionControl for ClassicCongestionControl<T> {
         congestion || persistent_congestion
     }
 
+    /// Handle a congestion event.
+    /// Returns true if this was a true congestion event.
+    fn on_congestion_event(&mut self, last_packet: &SentPacket) -> bool {
+        // Start a new congestion event if lost or ECN CE marked packet was sent
+        // after the start of the previous congestion recovery period.
+        if !self.after_recovery_start(last_packet) {
+            return false;
+        }
+
+        let (cwnd, acked_bytes) = self.cc_algorithm.reduce_cwnd(
+            self.congestion_window,
+            self.acked_bytes,
+            self.max_datagram_size(),
+        );
+        self.congestion_window = max(cwnd, self.cwnd_min());
+        self.acked_bytes = acked_bytes;
+        self.ssthresh = self.congestion_window;
+        qdebug!(
+            [self],
+            "Cong event -> recovery; cwnd {}, ssthresh {}",
+            self.congestion_window,
+            self.ssthresh
+        );
+        qlog::metrics_updated(
+            &self.qlog,
+            &[
+                QlogMetric::CongestionWindow(self.congestion_window),
+                QlogMetric::SsThresh(self.ssthresh),
+                QlogMetric::InRecovery(true),
+            ],
+        );
+        self.set_state(State::RecoveryStart);
+        true
+    }
+
     /// Report received ECN CE mark(s) to the congestion controller as a
     /// congestion event.
     ///
@@ -347,7 +382,8 @@ impl<T: WindowAdjustment> CongestionControl for ClassicCongestionControl<T> {
 
     fn discard(&mut self, pkt: &SentPacket) {
         if pkt.cc_outstanding() {
-            self.bytes_in_flight = self.bytes_in_flight.saturating_sub(pkt.len());
+            assert!(self.bytes_in_flight >= pkt.len());
+            self.bytes_in_flight -= pkt.len();
             qlog::metrics_updated(
                 &self.qlog,
                 &[QlogMetric::BytesInFlight(self.bytes_in_flight)],
@@ -536,41 +572,6 @@ impl<T: WindowAdjustment> ClassicCongestionControl<T> {
         !self.state.transient() && self.recovery_start.map_or(true, |pn| packet.pn() >= pn)
     }
 
-    /// Handle a congestion event.
-    /// Returns true if this was a true congestion event.
-    fn on_congestion_event(&mut self, last_packet: &SentPacket) -> bool {
-        // Start a new congestion event if lost or ECN CE marked packet was sent
-        // after the start of the previous congestion recovery period.
-        if !self.after_recovery_start(last_packet) {
-            return false;
-        }
-
-        let (cwnd, acked_bytes) = self.cc_algorithm.reduce_cwnd(
-            self.congestion_window,
-            self.acked_bytes,
-            self.max_datagram_size(),
-        );
-        self.congestion_window = max(cwnd, self.cwnd_min());
-        self.acked_bytes = acked_bytes;
-        self.ssthresh = self.congestion_window;
-        qdebug!(
-            [self],
-            "Cong event -> recovery; cwnd {}, ssthresh {}",
-            self.congestion_window,
-            self.ssthresh
-        );
-        qlog::metrics_updated(
-            &self.qlog,
-            &[
-                QlogMetric::CongestionWindow(self.congestion_window),
-                QlogMetric::SsThresh(self.ssthresh),
-                QlogMetric::InRecovery(true),
-            ],
-        );
-        self.set_state(State::RecoveryStart);
-        true
-    }
-
     fn app_limited(&self) -> bool {
         if self.bytes_in_flight >= self.congestion_window {
             false
diff --git a/neqo-transport/src/cc/mod.rs b/neqo-transport/src/cc/mod.rs
index bbb47c4fd0..a78f1f9021 100644
--- a/neqo-transport/src/cc/mod.rs
+++ b/neqo-transport/src/cc/mod.rs
@@ -62,6 +62,8 @@ pub trait CongestionControl: Display + Debug {
         lost_packets: &[SentPacket],
     ) -> bool;
 
+    fn on_congestion_event(&mut self, last_packet: &SentPacket) -> bool;
+
     /// Returns true if the congestion window was reduced.
     fn on_ecn_ce_received(&mut self, largest_acked_pkt: &SentPacket) -> bool;
 
diff --git a/neqo-transport/src/path.rs b/neqo-transport/src/path.rs
index de4037b495..cd8b445d60 100644
--- a/neqo-transport/src/path.rs
+++ b/neqo-transport/src/path.rs
@@ -1016,6 +1016,11 @@ impl Path {
         }
     }
 
+    /// Initiate a congestion response.
+    pub fn on_congestion_event(&mut self, last_packet: &SentPacket) -> bool {
+        self.sender.on_congestion_event(last_packet)
+    }
+
     /// Get the number of bytes that can be written to this path.
     pub fn amplification_limit(&self) -> usize {
         if matches!(self.state, ProbeState::Failed) {
diff --git a/neqo-transport/src/recovery/mod.rs b/neqo-transport/src/recovery/mod.rs
index d886a2fb84..aa7490851e 100644
--- a/neqo-transport/src/recovery/mod.rs
+++ b/neqo-transport/src/recovery/mod.rs
@@ -867,13 +867,9 @@ impl LossRecovery {
             // reaction (because otherwise none would happen).
             if let Some(space) = self.spaces.get(pn_space) {
                 if space.largest_acked.is_none() {
-                    path.borrow_mut().on_packets_lost(
-                        space.largest_acked_sent_time,
-                        pn_space,
-                        lost,
-                        &mut self.stats.borrow_mut(),
-                        now,
-                    );
+                    if let Some(last) = lost.last() {
+                        path.borrow_mut().on_congestion_event(last);
+                    }
                 }
             }
             self.fire_pto(pn_space, allow_probes);
diff --git a/neqo-transport/src/sender.rs b/neqo-transport/src/sender.rs
index a9ead627aa..2370c118f0 100644
--- a/neqo-transport/src/sender.rs
+++ b/neqo-transport/src/sender.rs
@@ -113,6 +113,11 @@ impl PacketSender {
         self.maybe_update_pacer_mtu();
     }
 
+    /// Initiate a congestion response.
+    pub fn on_congestion_event(&mut self, last_packet: &SentPacket) -> bool {
+        self.cc.on_congestion_event(last_packet)
+    }
+
     /// Called when packets are lost.  Returns true if the congestion window was reduced.
     pub fn on_packets_lost(
         &mut self,

From d1bc820d5d57c1d49937116de7910b6c5c8dc96d Mon Sep 17 00:00:00 2001
From: Lars Eggert <lars@eggert.org>
Date: Mon, 22 Jul 2024 18:18:22 -0700
Subject: [PATCH 12/52] Add comment

---
 neqo-transport/src/cc/mod.rs | 1 +
 1 file changed, 1 insertion(+)

diff --git a/neqo-transport/src/cc/mod.rs b/neqo-transport/src/cc/mod.rs
index a78f1f9021..59a34ccf91 100644
--- a/neqo-transport/src/cc/mod.rs
+++ b/neqo-transport/src/cc/mod.rs
@@ -62,6 +62,7 @@ pub trait CongestionControl: Display + Debug {
         lost_packets: &[SentPacket],
     ) -> bool;
 
+    /// Initiate a congestion response.
     fn on_congestion_event(&mut self, last_packet: &SentPacket) -> bool;
 
     /// Returns true if the congestion window was reduced.

From 5e1ea678e62a869a7608e60815b097a5f4e19b6e Mon Sep 17 00:00:00 2001
From: Lars Eggert <lars@eggert.org>
Date: Tue, 23 Jul 2024 16:45:22 +0300
Subject: [PATCH 13/52] Apply suggestions from code review

Co-authored-by: Max Inden <mail@max-inden.de>
Signed-off-by: Lars Eggert <lars@eggert.org>
---
 neqo-transport/src/cc/mod.rs       | 2 ++
 neqo-transport/src/path.rs         | 2 ++
 neqo-transport/src/recovery/mod.rs | 6 ++++--
 neqo-transport/src/sender.rs       | 2 ++
 4 files changed, 10 insertions(+), 2 deletions(-)

diff --git a/neqo-transport/src/cc/mod.rs b/neqo-transport/src/cc/mod.rs
index 59a34ccf91..43eed5d26c 100644
--- a/neqo-transport/src/cc/mod.rs
+++ b/neqo-transport/src/cc/mod.rs
@@ -63,6 +63,8 @@ pub trait CongestionControl: Display + Debug {
     ) -> bool;
 
     /// Initiate a congestion response.
+    ///
+    /// Returns true if the congestion window was reduced.
     fn on_congestion_event(&mut self, last_packet: &SentPacket) -> bool;
 
     /// Returns true if the congestion window was reduced.
diff --git a/neqo-transport/src/path.rs b/neqo-transport/src/path.rs
index cd8b445d60..6c38c83cfd 100644
--- a/neqo-transport/src/path.rs
+++ b/neqo-transport/src/path.rs
@@ -1017,6 +1017,8 @@ impl Path {
     }
 
     /// Initiate a congestion response.
+    ///
+    /// Returns true if the congestion window was reduced.
     pub fn on_congestion_event(&mut self, last_packet: &SentPacket) -> bool {
         self.sender.on_congestion_event(last_packet)
     }
diff --git a/neqo-transport/src/recovery/mod.rs b/neqo-transport/src/recovery/mod.rs
index aa7490851e..196d8eb0d5 100644
--- a/neqo-transport/src/recovery/mod.rs
+++ b/neqo-transport/src/recovery/mod.rs
@@ -863,8 +863,10 @@ impl LossRecovery {
         // pto_time to increase which might cause PTO for later packet number spaces to not fire.
         if let Some(pn_space) = pto_space {
             qtrace!([self], "PTO {}, probing {:?}", pn_space, allow_probes);
-            // If we hit a PTO while we don't have a largest_acked yet, also do a congestion control
-            // reaction (because otherwise none would happen).
+            // Packets are only declared as lost, relative to the
+            // `largest_acked`. If we hit a PTO while we don't have a
+            // largest_acked yet, also do a congestion control reaction (because
+            // otherwise none would happen).
             if let Some(space) = self.spaces.get(pn_space) {
                 if space.largest_acked.is_none() {
                     if let Some(last) = lost.last() {
diff --git a/neqo-transport/src/sender.rs b/neqo-transport/src/sender.rs
index 2370c118f0..20213cc11e 100644
--- a/neqo-transport/src/sender.rs
+++ b/neqo-transport/src/sender.rs
@@ -114,6 +114,8 @@ impl PacketSender {
     }
 
     /// Initiate a congestion response.
+    ///
+    /// Returns true if the congestion window was reduced.
     pub fn on_congestion_event(&mut self, last_packet: &SentPacket) -> bool {
         self.cc.on_congestion_event(last_packet)
     }

From 0781c722e82642e10ac488b906b2380becb9dad8 Mon Sep 17 00:00:00 2001
From: Lars Eggert <lars@eggert.org>
Date: Tue, 23 Jul 2024 06:50:42 -0700
Subject: [PATCH 14/52] More suggestions from @mxinden; thanks

---
 neqo-transport/src/recovery/mod.rs | 58 ++++++++++++++++--------------
 1 file changed, 31 insertions(+), 27 deletions(-)

diff --git a/neqo-transport/src/recovery/mod.rs b/neqo-transport/src/recovery/mod.rs
index 196d8eb0d5..ffc332651d 100644
--- a/neqo-transport/src/recovery/mod.rs
+++ b/neqo-transport/src/recovery/mod.rs
@@ -321,13 +321,15 @@ impl LossRecoverySpace {
         );
         self.first_ooo_time = None;
 
-        let largest_acked = self.largest_acked;
+        let Some(largest_acked) = self.largest_acked else {
+            return;
+        };
 
         for packet in self
             .sent_packets
             .iter_mut()
             // BTreeMap iterates in order of ascending PN
-            .take_while(|p| Some(p.pn()) < largest_acked)
+            .take_while(|p| p.pn() < largest_acked)
         {
             // Packets sent before now - loss_delay are deemed lost.
             if packet.time_sent() + loss_delay <= now {
@@ -337,7 +339,7 @@ impl LossRecoverySpace {
                     packet.time_sent(),
                     loss_delay
                 );
-            } else if largest_acked >= Some(packet.pn() + PACKET_THRESHOLD) {
+            } else if largest_acked >= packet.pn() + PACKET_THRESHOLD {
                 qtrace!(
                     "lost={}, is >= {} from largest acked {:?}",
                     packet.pn(),
@@ -345,9 +347,7 @@ impl LossRecoverySpace {
                     largest_acked
                 );
             } else {
-                if largest_acked.is_some() {
-                    self.first_ooo_time = Some(packet.time_sent());
-                }
+                self.first_ooo_time = Some(packet.time_sent());
                 // No more packets can be declared lost after this one.
                 break;
             };
@@ -840,23 +840,25 @@ impl LossRecovery {
         // The spaces in which we will allow probing.
         let mut allow_probes = PacketNumberSpaceSet::default();
         for pn_space in PacketNumberSpace::iter() {
-            if let Some(t) = self.pto_time(path.borrow().rtt(), *pn_space) {
-                if t <= now {
-                    allow_probes[*pn_space] = true;
-                    qdebug!([self], "PTO timer fired for {}", pn_space);
-                    let space = self.spaces.get_mut(*pn_space).unwrap();
-                    lost.extend(
-                        space
-                            .pto_packets(PtoState::pto_packet_count(
-                                *pn_space,
-                                self.stats.borrow().packets_rx,
-                            ))
-                            .cloned(),
-                    );
-
-                    pto_space = pto_space.or(Some(*pn_space));
-                }
+            if self
+                .pto_time(path.borrow().rtt(), *pn_space)
+                .map_or(true, |t| now < t)
+            {
+                continue;
             }
+            allow_probes[*pn_space] = true;
+            qdebug!([self], "PTO timer fired for {}", pn_space);
+            let space = self.spaces.get_mut(*pn_space).unwrap();
+            lost.extend(
+                space
+                    .pto_packets(PtoState::pto_packet_count(
+                        *pn_space,
+                        self.stats.borrow().packets_rx,
+                    ))
+                    .cloned(),
+            );
+
+            pto_space = pto_space.or(Some(*pn_space));
         }
 
         // This has to happen outside the loop. Increasing the PTO count here causes the
@@ -867,11 +869,13 @@ impl LossRecovery {
             // `largest_acked`. If we hit a PTO while we don't have a
             // largest_acked yet, also do a congestion control reaction (because
             // otherwise none would happen).
-            if let Some(space) = self.spaces.get(pn_space) {
-                if space.largest_acked.is_none() {
-                    if let Some(last) = lost.last() {
-                        path.borrow_mut().on_congestion_event(last);
-                    }
+            if self
+                .spaces
+                .get(pn_space)
+                .map_or(false, |space| space.largest_acked.is_none())
+            {
+                if let Some(last) = lost.last() {
+                    path.borrow_mut().on_congestion_event(last);
                 }
             }
             self.fire_pto(pn_space, allow_probes);

From 76c32f3101e61b6c8864a44a70413321a15d1475 Mon Sep 17 00:00:00 2001
From: Lars Eggert <lars@eggert.org>
Date: Wed, 31 Jul 2024 15:27:10 +0300
Subject: [PATCH 15/52] Potential fix?

---
 neqo-transport/src/recovery/mod.rs | 14 +++++++-------
 1 file changed, 7 insertions(+), 7 deletions(-)

diff --git a/neqo-transport/src/recovery/mod.rs b/neqo-transport/src/recovery/mod.rs
index ffc332651d..c9234c1f40 100644
--- a/neqo-transport/src/recovery/mod.rs
+++ b/neqo-transport/src/recovery/mod.rs
@@ -321,25 +321,23 @@ impl LossRecoverySpace {
         );
         self.first_ooo_time = None;
 
-        let Some(largest_acked) = self.largest_acked else {
-            return;
-        };
+        let largest_acked = self.largest_acked;
 
         for packet in self
             .sent_packets
             .iter_mut()
             // BTreeMap iterates in order of ascending PN
-            .take_while(|p| p.pn() < largest_acked)
+            .take_while(|p| p.pn() < largest_acked.unwrap_or(PacketNumber::MAX))
         {
             // Packets sent before now - loss_delay are deemed lost.
-            if packet.time_sent() + loss_delay <= now {
+            if largest_acked.is_some() && packet.time_sent() + loss_delay <= now {
                 qtrace!(
                     "lost={}, time sent {:?} is before lost_delay {:?}",
                     packet.pn(),
                     packet.time_sent(),
                     loss_delay
                 );
-            } else if largest_acked >= packet.pn() + PACKET_THRESHOLD {
+            } else if largest_acked >= Some(packet.pn() + PACKET_THRESHOLD) {
                 qtrace!(
                     "lost={}, is >= {} from largest acked {:?}",
                     packet.pn(),
@@ -347,7 +345,9 @@ impl LossRecoverySpace {
                     largest_acked
                 );
             } else {
-                self.first_ooo_time = Some(packet.time_sent());
+                if largest_acked.is_some() {
+                    self.first_ooo_time = Some(packet.time_sent());
+                }
                 // No more packets can be declared lost after this one.
                 break;
             };

From bb5769e1d27acc73559ac5e1a61b4bf346c557be Mon Sep 17 00:00:00 2001
From: Lars Eggert <lars@eggert.org>
Date: Wed, 31 Jul 2024 17:28:05 +0300
Subject: [PATCH 16/52] Revert "Potential fix?"

This reverts commit 76c32f3101e61b6c8864a44a70413321a15d1475.
---
 neqo-transport/src/recovery/mod.rs | 14 +++++++-------
 1 file changed, 7 insertions(+), 7 deletions(-)

diff --git a/neqo-transport/src/recovery/mod.rs b/neqo-transport/src/recovery/mod.rs
index c9234c1f40..ffc332651d 100644
--- a/neqo-transport/src/recovery/mod.rs
+++ b/neqo-transport/src/recovery/mod.rs
@@ -321,23 +321,25 @@ impl LossRecoverySpace {
         );
         self.first_ooo_time = None;
 
-        let largest_acked = self.largest_acked;
+        let Some(largest_acked) = self.largest_acked else {
+            return;
+        };
 
         for packet in self
             .sent_packets
             .iter_mut()
             // BTreeMap iterates in order of ascending PN
-            .take_while(|p| p.pn() < largest_acked.unwrap_or(PacketNumber::MAX))
+            .take_while(|p| p.pn() < largest_acked)
         {
             // Packets sent before now - loss_delay are deemed lost.
-            if largest_acked.is_some() && packet.time_sent() + loss_delay <= now {
+            if packet.time_sent() + loss_delay <= now {
                 qtrace!(
                     "lost={}, time sent {:?} is before lost_delay {:?}",
                     packet.pn(),
                     packet.time_sent(),
                     loss_delay
                 );
-            } else if largest_acked >= Some(packet.pn() + PACKET_THRESHOLD) {
+            } else if largest_acked >= packet.pn() + PACKET_THRESHOLD {
                 qtrace!(
                     "lost={}, is >= {} from largest acked {:?}",
                     packet.pn(),
@@ -345,9 +347,7 @@ impl LossRecoverySpace {
                     largest_acked
                 );
             } else {
-                if largest_acked.is_some() {
-                    self.first_ooo_time = Some(packet.time_sent());
-                }
+                self.first_ooo_time = Some(packet.time_sent());
                 // No more packets can be declared lost after this one.
                 break;
             };

From cd84b611441a593fdfdd340cbd8b75ff0e1d1400 Mon Sep 17 00:00:00 2001
From: Lars Eggert <lars@eggert.org>
Date: Fri, 9 Aug 2024 16:31:28 +0300
Subject: [PATCH 17/52] fmt

---
 neqo-transport/src/path.rs | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/neqo-transport/src/path.rs b/neqo-transport/src/path.rs
index 3da334770b..7fce6917fe 100644
--- a/neqo-transport/src/path.rs
+++ b/neqo-transport/src/path.rs
@@ -1024,7 +1024,7 @@ impl Path {
     pub fn on_congestion_event(&mut self, last_packet: &SentPacket) -> bool {
         self.sender.on_congestion_event(last_packet)
     }
-      
+
     /// Determine whether we should be setting a PTO for this path. This is true when either the
     /// path is valid or when there is enough remaining in the amplification limit to fit a
     /// full-sized path (i.e., the path MTU).

From 1b6a81d16a1212c0cdbab08807cb50b4afadf40d Mon Sep 17 00:00:00 2001
From: Lars Eggert <lars@eggert.org>
Date: Fri, 9 Aug 2024 17:42:50 +0300
Subject: [PATCH 18/52] Fixes

---
 neqo-transport/src/connection/tests/ecn.rs | 14 ++++++++++----
 neqo-transport/src/path.rs                 |  9 +++++++--
 neqo-transport/src/recovery/mod.rs         |  4 +---
 3 files changed, 18 insertions(+), 9 deletions(-)

diff --git a/neqo-transport/src/connection/tests/ecn.rs b/neqo-transport/src/connection/tests/ecn.rs
index ff86a458f3..c3344286c7 100644
--- a/neqo-transport/src/connection/tests/ecn.rs
+++ b/neqo-transport/src/connection/tests/ecn.rs
@@ -6,7 +6,7 @@
 
 use std::time::Duration;
 
-use neqo_common::{Datagram, IpTos, IpTosEcn};
+use neqo_common::{qdebug, Datagram, IpTos, IpTosEcn};
 use test_fixture::{
     assertions::{assert_v4_path, assert_v6_path},
     fixture_init, now, DEFAULT_ADDR_V4,
@@ -84,10 +84,16 @@ fn handshake_delay_with_ecn_blackhole() {
         drop_ecn_marked_datagrams(),
     );
 
+    // This will simplify once we merge #2027
+    let pto = DEFAULT_RTT * 3;
     assert_eq!(
-        (finish - start).as_millis() / DEFAULT_RTT.as_millis(),
-        15,
-        "expected 6 RTT for client to detect blackhole, 6 RTT for server to detect blackhole and 3 RTT for handshake to be confirmed.",
+        (finish - start),
+        (1 + 2 + 4)
+            * pto + // client RTOs twice
+        (1 + 2)
+            * pto + // server RTOs once until amplification limit (2nd RTX has PING)
+        pto // delay until client sends next RTX to lift amplification limit
+        + DEFAULT_RTT + DEFAULT_RTT/2
     );
 }
 
diff --git a/neqo-transport/src/path.rs b/neqo-transport/src/path.rs
index 7fce6917fe..29dfe7e84c 100644
--- a/neqo-transport/src/path.rs
+++ b/neqo-transport/src/path.rs
@@ -1021,8 +1021,13 @@ impl Path {
     /// Initiate a congestion response.
     ///
     /// Returns true if the congestion window was reduced.
-    pub fn on_congestion_event(&mut self, last_packet: &SentPacket) -> bool {
-        self.sender.on_congestion_event(last_packet)
+    pub fn on_congestion_event(&mut self, lost_packets: &[SentPacket]) -> bool {
+        if let Some(last) = lost_packets.last() {
+            self.ecn_info.on_packets_lost(lost_packets);
+            self.sender.on_congestion_event(last)
+        } else {
+            false
+        }
     }
 
     /// Determine whether we should be setting a PTO for this path. This is true when either the
diff --git a/neqo-transport/src/recovery/mod.rs b/neqo-transport/src/recovery/mod.rs
index 0020c13d74..e0456c6fe8 100644
--- a/neqo-transport/src/recovery/mod.rs
+++ b/neqo-transport/src/recovery/mod.rs
@@ -879,9 +879,7 @@ impl LossRecovery {
                 .get(pn_space)
                 .map_or(false, |space| space.largest_acked.is_none())
             {
-                if let Some(last) = lost.last() {
-                    path.borrow_mut().on_congestion_event(last);
-                }
+                path.borrow_mut().on_congestion_event(lost);
             }
             self.fire_pto(pn_space, allow_probes);
         }

From 9156292d2974bede3aaef750a94ab3c71b80a36b Mon Sep 17 00:00:00 2001
From: Lars Eggert <lars@eggert.org>
Date: Fri, 16 Aug 2024 13:20:33 +0100
Subject: [PATCH 19/52] Robustify (?)

---
 neqo-transport/src/connection/mod.rs          | 14 ++--
 neqo-transport/src/connection/tests/ecn.rs    | 19 +++--
 .../src/connection/tests/handshake.rs         | 41 ++++++++-
 neqo-transport/src/connection/tests/idle.rs   |  2 +
 .../src/connection/tests/recovery.rs          | 25 +++---
 neqo-transport/src/path.rs                    |  5 +-
 neqo-transport/src/recovery/mod.rs            | 84 ++++++++-----------
 neqo-transport/src/rtt.rs                     |  5 +-
 neqo-transport/src/tracking.rs                |  7 +-
 9 files changed, 113 insertions(+), 89 deletions(-)

diff --git a/neqo-transport/src/connection/mod.rs b/neqo-transport/src/connection/mod.rs
index 4bc8d0f7fc..fa21b1993b 100644
--- a/neqo-transport/src/connection/mod.rs
+++ b/neqo-transport/src/connection/mod.rs
@@ -618,8 +618,8 @@ impl Connection {
     /// only use it where a more precise value is not important.
     fn pto(&self) -> Duration {
         self.paths.primary().map_or_else(
-            || RttEstimate::default().pto(PacketNumberSpace::ApplicationData),
-            |p| p.borrow().rtt().pto(PacketNumberSpace::ApplicationData),
+            || RttEstimate::default().pto(self.confirmed()),
+            |p| p.borrow().rtt().pto(self.confirmed()),
         )
     }
 
@@ -1034,7 +1034,7 @@ impl Connection {
         if let Some(p) = self.paths.primary() {
             let path = p.borrow();
             let rtt = path.rtt();
-            let pto = rtt.pto(PacketNumberSpace::ApplicationData);
+            let pto = rtt.pto(self.confirmed());
 
             let idle_time = self.idle_timeout.expiry(now, pto);
             qtrace!([self], "Idle/keepalive timer {:?}", idle_time);
@@ -1497,7 +1497,7 @@ impl Connection {
         let mut dcid = None;
 
         qtrace!([self], "{} input {}", path.borrow(), hex(&**d));
-        let pto = path.borrow().rtt().pto(PacketNumberSpace::ApplicationData);
+        let pto = path.borrow().rtt().pto(self.confirmed());
 
         // Handle each packet in the datagram.
         while !slc.is_empty() {
@@ -2106,7 +2106,7 @@ impl Connection {
             // or the PTO timer fired: probe.
             true
         } else {
-            let pto = path.borrow().rtt().pto(PacketNumberSpace::ApplicationData);
+            let pto = path.borrow().rtt().pto(self.confirmed());
             if !builder.packet_empty() {
                 // The packet only contains an ACK.  Check whether we want to
                 // force an ACK with a PING so we can stop tracking packets.
@@ -2756,6 +2756,10 @@ impl Connection {
         Ok(())
     }
 
+    fn confirmed(&self) -> bool {
+        self.state == State::Confirmed
+    }
+
     fn set_confirmed(&mut self) -> Res<()> {
         self.set_state(State::Confirmed);
         if self.conn_params.pmtud_enabled() {
diff --git a/neqo-transport/src/connection/tests/ecn.rs b/neqo-transport/src/connection/tests/ecn.rs
index c3344286c7..d034386a10 100644
--- a/neqo-transport/src/connection/tests/ecn.rs
+++ b/neqo-transport/src/connection/tests/ecn.rs
@@ -6,7 +6,7 @@
 
 use std::time::Duration;
 
-use neqo_common::{qdebug, Datagram, IpTos, IpTosEcn};
+use neqo_common::{Datagram, IpTos, IpTosEcn};
 use test_fixture::{
     assertions::{assert_v4_path, assert_v6_path},
     fixture_init, now, DEFAULT_ADDR_V4,
@@ -84,16 +84,19 @@ fn handshake_delay_with_ecn_blackhole() {
         drop_ecn_marked_datagrams(),
     );
 
-    // This will simplify once we merge #2027
     let pto = DEFAULT_RTT * 3;
+    let half_rtt = DEFAULT_RTT / 2;
     assert_eq!(
         (finish - start),
-        (1 + 2 + 4)
-            * pto + // client RTOs twice
-        (1 + 2)
-            * pto + // server RTOs once until amplification limit (2nd RTX has PING)
-        pto // delay until client sends next RTX to lift amplification limit
-        + DEFAULT_RTT + DEFAULT_RTT/2
+        (1 + 2 + 4) * pto + // Client RTOs its CI w/ECN twice (3x total)
+        half_rtt + // Fourth CI w/o ECN being delivered
+        (1 + 2 + 4) * pto + // Server RTOs its CI w/ECN twice (3x total)
+        half_rtt + // Fourth SI w/o ECN being delivered
+        half_rtt + // Client ACK
+        half_rtt + // Client Handshake/Short
+        half_rtt + // Server HandshakeDone
+        half_rtt + // Client ACK
+        half_rtt
     );
 }
 
diff --git a/neqo-transport/src/connection/tests/handshake.rs b/neqo-transport/src/connection/tests/handshake.rs
index 6c3fbfd4bb..57c5c554b7 100644
--- a/neqo-transport/src/connection/tests/handshake.rs
+++ b/neqo-transport/src/connection/tests/handshake.rs
@@ -18,7 +18,9 @@ use neqo_crypto::{
 };
 #[cfg(not(feature = "disable-encryption"))]
 use test_fixture::datagram;
-use test_fixture::{assertions, fixture_init, now, split_datagram, DEFAULT_ADDR};
+use test_fixture::{
+    assertions, assertions::assert_coalesced_0rtt, fixture_init, now, split_datagram, DEFAULT_ADDR,
+};
 
 use super::{
     super::{Connection, Output, State},
@@ -400,10 +402,10 @@ fn reorder_05rtt_with_0rtt() {
     // Now PTO at the client and cause the server to re-send handshake packets.
     now += AT_LEAST_PTO;
     let c3 = client.process(None, now).dgram();
+    assert_coalesced_0rtt(c3.as_ref().unwrap());
 
     now += RTT / 2;
     let s3 = server.process(c3.as_ref(), now).dgram().unwrap();
-    assertions::assert_no_1rtt(&s3[..]);
 
     // The client should be able to process the 0.5 RTT now.
     // This should contain an ACK, so we are processing an ACK from the past.
@@ -1252,3 +1254,38 @@ fn server_initial_retransmits_identical() {
         total_ptos += pto;
     }
 }
+
+#[test]
+fn client_handshake_retransmits_identical() {
+    let mut now = now();
+    let mut client = default_client();
+    let mut ci = client.process(None, now).dgram();
+    let mut server = default_server();
+    let mut si = server.process(ci.take().as_ref(), now).dgram();
+
+    now += DEFAULT_RTT;
+
+    _ = client.process(si.take().as_ref(), now).callback();
+    maybe_authenticate(&mut client);
+
+    // Force the client to retransmit its coalesced Handshake/Short packet a number of times and
+    // make sure the retranmissions are identical to the original. Also, verify the PTO
+    // durations.
+    for i in 1..=3 {
+        _ = client.process(None, now).dgram().unwrap();
+        let pto = client.process(None, now).callback();
+        assert_eq!(pto, DEFAULT_RTT * 3 * (1 << (i - 1)));
+        now += pto;
+
+        assert_eq!(
+            client.stats().frame_tx,
+            FrameStats {
+                crypto: i + 1,
+                ack: i,
+                new_connection_id: i * 7,
+                all: i * 9 + 1,
+                ..Default::default()
+            }
+        );
+    }
+}
diff --git a/neqo-transport/src/connection/tests/idle.rs b/neqo-transport/src/connection/tests/idle.rs
index 8677d7f5d5..6694e3bc19 100644
--- a/neqo-transport/src/connection/tests/idle.rs
+++ b/neqo-transport/src/connection/tests/idle.rs
@@ -721,6 +721,8 @@ fn keep_alive_with_ack_eliciting_packet_lost() {
     assert!(retransmit.is_some());
     let retransmit = client.process_output(now).dgram();
     assert!(retransmit.is_some());
+    let retransmit = client.process_output(now).dgram();
+    assert!(retransmit.is_some());
 
     // The next callback will be an idle timeout.
     assert_eq!(
diff --git a/neqo-transport/src/connection/tests/recovery.rs b/neqo-transport/src/connection/tests/recovery.rs
index 5d58769a18..cec39931c2 100644
--- a/neqo-transport/src/connection/tests/recovery.rs
+++ b/neqo-transport/src/connection/tests/recovery.rs
@@ -252,7 +252,6 @@ fn pto_handshake_complete() {
     assert_eq!(client.stats.borrow().pto_counts, pto_counts);
 
     // Wait for PTO to expire and resend a handshake packet.
-    // Wait long enough that the 1-RTT PTO also fires.
     qdebug!("---- client: PTO");
     now += HALF_RTT * 6;
     let pkt2 = client.process(None, now).dgram();
@@ -261,17 +260,11 @@ fn pto_handshake_complete() {
     pto_counts[0] = 1;
     assert_eq!(client.stats.borrow().pto_counts, pto_counts);
 
-    // Get a second PTO packet.
-    // Add some application data to this datagram, then split the 1-RTT off.
+    // Split the 1-RTT off.
     // We'll use that packet to force the server to acknowledge 1-RTT.
-    let stream_id = client.stream_create(StreamType::UniDi).unwrap();
-    client.stream_close_send(stream_id).unwrap();
-    now += HALF_RTT * 6;
-    let pkt3 = client.process(None, now).dgram();
-    assert_handshake(pkt3.as_ref().unwrap());
-    let (pkt3_hs, pkt3_1rtt) = split_datagram(&pkt3.unwrap());
-    assert_handshake(&pkt3_hs);
-    assert!(pkt3_1rtt.is_some());
+    let (pkt2_hs, pkt2_1rtt) = split_datagram(&pkt2.unwrap());
+    assert_handshake(&pkt2_hs);
+    assert!(pkt2_1rtt.is_some());
 
     // PTO has been doubled.
     let cb = client.process(None, now).callback();
@@ -288,7 +281,7 @@ fn pto_handshake_complete() {
     // This should remove the 1-RTT PTO from messing this test up.
     let server_acks = server.stats().frame_tx.ack;
     let server_done = server.stats().frame_tx.handshake_done;
-    server.process_input(&pkt3_1rtt.unwrap(), now);
+    server.process_input(&pkt2_1rtt.unwrap(), now);
     let ack = server.process(pkt1.as_ref(), now).dgram();
     assert!(ack.is_some());
     assert_eq!(server.stats().frame_tx.ack, server_acks + 2);
@@ -299,14 +292,16 @@ fn pto_handshake_complete() {
     // Note that these don't include 1-RTT packets, because 1-RTT isn't send on PTO.
     let dropped_before1 = server.stats().dropped_rx;
     let server_frames = server.stats().frame_rx.all;
-    server.process_input(&pkt2.unwrap(), now);
+    server.process_input(&pkt2_hs, now);
     assert_eq!(1, server.stats().dropped_rx - dropped_before1);
     assert_eq!(server.stats().frame_rx.all, server_frames);
 
+    // server.process_input(&pkt2_1rtt.unwrap(), now);
+    let server_frames2 = server.stats().frame_rx.all;
     let dropped_before2 = server.stats().dropped_rx;
-    server.process_input(&pkt3_hs, now);
+    server.process_input(&pkt2_hs, now);
     assert_eq!(1, server.stats().dropped_rx - dropped_before2);
-    assert_eq!(server.stats().frame_rx.all, server_frames);
+    assert_eq!(server.stats().frame_rx.all, server_frames2);
 
     now += HALF_RTT;
 
diff --git a/neqo-transport/src/path.rs b/neqo-transport/src/path.rs
index 29dfe7e84c..b10d1cbb1b 100644
--- a/neqo-transport/src/path.rs
+++ b/neqo-transport/src/path.rs
@@ -30,7 +30,6 @@ use crate::{
     rtt::RttEstimate,
     sender::PacketSender,
     stats::FrameStats,
-    tracking::PacketNumberSpace,
     Stats,
 };
 
@@ -998,7 +997,7 @@ impl Path {
     pub fn on_packets_lost(
         &mut self,
         prev_largest_acked_sent: Option<Instant>,
-        space: PacketNumberSpace,
+        confirmed: bool,
         lost_packets: &[SentPacket],
         stats: &mut Stats,
         now: Instant,
@@ -1008,7 +1007,7 @@ impl Path {
         let cwnd_reduced = self.sender.on_packets_lost(
             self.rtt.first_sample_time(),
             prev_largest_acked_sent,
-            self.rtt.pto(space), // Important: the base PTO, not adjusted.
+            self.rtt.pto(confirmed), // Important: the base PTO, not adjusted.
             lost_packets,
             stats,
             now,
diff --git a/neqo-transport/src/recovery/mod.rs b/neqo-transport/src/recovery/mod.rs
index c259eeef36..5b195800ca 100644
--- a/neqo-transport/src/recovery/mod.rs
+++ b/neqo-transport/src/recovery/mod.rs
@@ -153,11 +153,6 @@ impl LossRecoverySpace {
         }
     }
 
-    #[must_use]
-    pub const fn space(&self) -> PacketNumberSpace {
-        self.space
-    }
-
     /// Find the time we sent the first packet that is lower than the
     /// largest acknowledged and that isn't yet declared lost.
     /// Use the value we prepared earlier in `detect_lost_packets`.
@@ -170,18 +165,15 @@ impl LossRecoverySpace {
         self.in_flight_outstanding > 0
     }
 
-    pub fn pto_packets(&mut self, count: usize) -> impl Iterator<Item = &SentPacket> {
-        self.sent_packets
-            .iter_mut()
-            .filter_map(|sent| {
-                if sent.pto() {
-                    qtrace!("PTO: marking packet {} lost ", sent.pn());
-                    Some(&*sent)
-                } else {
-                    None
-                }
-            })
-            .take(count)
+    pub fn pto_packets(&mut self) -> impl Iterator<Item = &SentPacket> {
+        self.sent_packets.iter_mut().filter_map(|sent| {
+            if sent.pto() {
+                qtrace!("PTO: marking packet {} lost ", sent.pn());
+                Some(&*sent)
+            } else {
+                None
+            }
+        })
     }
 
     pub fn pto_base_time(&self) -> Option<Instant> {
@@ -628,7 +620,7 @@ impl LossRecovery {
         // as we rely on the count of in-flight packets to determine whether to send
         // another probe.  Removing them too soon would result in not sending on PTO.
         let loss_delay = primary_path.borrow().rtt().loss_delay();
-        let cleanup_delay = self.pto_period(primary_path.borrow().rtt(), pn_space);
+        let cleanup_delay = self.pto_period(primary_path.borrow().rtt());
         let mut lost = Vec::new();
         self.spaces.get_mut(pn_space).unwrap().detect_lost_packets(
             now,
@@ -643,7 +635,7 @@ impl LossRecovery {
         // backoff, so that we can determine persistent congestion.
         primary_path.borrow_mut().on_packets_lost(
             prev_largest_acked,
-            pn_space,
+            self.is_confirmed(),
             &lost,
             &mut self.stats.borrow_mut(),
             now,
@@ -680,6 +672,10 @@ impl LossRecovery {
         dropped
     }
 
+    const fn is_confirmed(&self) -> bool {
+        self.confirmed_time.is_some()
+    }
+
     fn confirmed(&mut self, rtt: &RttEstimate, now: Instant) {
         debug_assert!(self.confirmed_time.is_none());
         self.confirmed_time = Some(now);
@@ -758,41 +754,42 @@ impl LossRecovery {
     fn pto_period_inner(
         rtt: &RttEstimate,
         pto_state: Option<&PtoState>,
-        pn_space: PacketNumberSpace,
+        confirmed: bool,
         fast_pto: u8,
     ) -> Duration {
         // This is a complicated (but safe) way of calculating:
         //   base_pto * F * 2^pto_count
         // where F = fast_pto / FAST_PTO_SCALE (== 1 by default)
         let pto_count = pto_state.map_or(0, |p| u32::try_from(p.count).unwrap_or(0));
-        rtt.pto(pn_space)
+        rtt.pto(confirmed)
             .checked_mul(u32::from(fast_pto) << min(pto_count, u32::BITS - u8::BITS))
             .map_or(Duration::from_secs(3600), |p| p / u32::from(FAST_PTO_SCALE))
     }
 
     /// Get the current PTO period for the given packet number space.
     /// Unlike calling `RttEstimate::pto` directly, this includes exponential backoff.
-    fn pto_period(&self, rtt: &RttEstimate, pn_space: PacketNumberSpace) -> Duration {
-        Self::pto_period_inner(rtt, self.pto_state.as_ref(), pn_space, self.fast_pto)
+    fn pto_period(&self, rtt: &RttEstimate) -> Duration {
+        Self::pto_period_inner(
+            rtt,
+            self.pto_state.as_ref(),
+            self.is_confirmed(),
+            self.fast_pto,
+        )
     }
 
     // Calculate PTO time for the given space.
     fn pto_time(&self, rtt: &RttEstimate, pn_space: PacketNumberSpace) -> Option<Instant> {
-        if self.confirmed_time.is_none() && pn_space == PacketNumberSpace::ApplicationData {
-            None
-        } else {
-            self.spaces.get(pn_space).and_then(|space| {
-                space
-                    .pto_base_time()
-                    .map(|t| t + self.pto_period(rtt, pn_space))
+        self.spaces
+            .get(pn_space)
+            .and_then(|space: &LossRecoverySpace| {
+                space.pto_base_time().map(|t| t + self.pto_period(rtt))
             })
-        }
     }
 
     /// Find the earliest PTO time for all active packet number spaces.
     /// Ignore Application if either Initial or Handshake have an active PTO.
     fn earliest_pto(&self, rtt: &RttEstimate) -> Option<Instant> {
-        if self.confirmed_time.is_some() {
+        if self.is_confirmed() {
             self.pto_time(rtt, PacketNumberSpace::ApplicationData)
         } else {
             self.pto_time(rtt, PacketNumberSpace::Initial)
@@ -841,11 +838,7 @@ impl LossRecovery {
             allow_probes[*pn_space] = true;
             qdebug!([self], "PTO timer fired for {}", pn_space);
             let space = self.spaces.get_mut(*pn_space).unwrap();
-            lost.extend(
-                space
-                    .pto_packets(PtoState::pto_packet_count(*pn_space))
-                    .cloned(),
-            );
+            lost.extend(space.pto_packets().cloned());
 
             pto_space = pto_space.or(Some(*pn_space));
         }
@@ -875,19 +868,20 @@ impl LossRecovery {
         let loss_delay = primary_path.borrow().rtt().loss_delay();
 
         let mut lost_packets = Vec::new();
+        let confirmed = self.is_confirmed();
         for space in self.spaces.iter_mut() {
             let first = lost_packets.len(); // The first packet lost in this space.
             let pto = Self::pto_period_inner(
                 primary_path.borrow().rtt(),
                 self.pto_state.as_ref(),
-                space.space(),
+                confirmed,
                 self.fast_pto,
             );
             space.detect_lost_packets(now, loss_delay, pto, &mut lost_packets);
 
             primary_path.borrow_mut().on_packets_lost(
                 space.largest_acked_sent_time,
-                space.space(),
+                confirmed,
                 &lost_packets[first..],
                 &mut self.stats.borrow_mut(),
                 now,
@@ -1524,17 +1518,13 @@ mod tests {
             ON_SENT_SIZE,
         ));
 
-        assert_eq!(lr.pto_time(PacketNumberSpace::ApplicationData), None);
+        assert!(lr.pto_time(PacketNumberSpace::ApplicationData).is_some());
         lr.discard(PacketNumberSpace::Initial, pn_time(1));
-        assert_eq!(lr.pto_time(PacketNumberSpace::ApplicationData), None);
+        assert!(lr.pto_time(PacketNumberSpace::ApplicationData).is_some());
 
         // Expiring state after the PTO on the ApplicationData space has
         // expired should result in setting a PTO state.
-        let default_pto = lr
-            .path
-            .borrow()
-            .rtt()
-            .pto(PacketNumberSpace::ApplicationData);
+        let default_pto = lr.path.borrow().rtt().pto(true);
         let expected_pto = pn_time(2) + default_pto;
         lr.discard(PacketNumberSpace::Handshake, expected_pto);
         let profile = lr.send_profile(now());
@@ -1566,7 +1556,7 @@ mod tests {
             ON_SENT_SIZE,
         ));
 
-        let handshake_pto = lr.path.borrow().rtt().pto(PacketNumberSpace::Handshake);
+        let handshake_pto = lr.path.borrow().rtt().pto(false);
         let expected_pto = now() + handshake_pto;
         assert_eq!(lr.pto_time(PacketNumberSpace::Initial), Some(expected_pto));
         let profile = lr.send_profile(now());
diff --git a/neqo-transport/src/rtt.rs b/neqo-transport/src/rtt.rs
index 7bfbbe4aaa..d3c5d79e09 100644
--- a/neqo-transport/src/rtt.rs
+++ b/neqo-transport/src/rtt.rs
@@ -19,7 +19,6 @@ use crate::{
     qlog::{self, QlogMetric},
     recovery::RecoveryToken,
     stats::FrameStats,
-    tracking::PacketNumberSpace,
 };
 
 /// The smallest time that the system timer (via `sleep()`, `nanosleep()`,
@@ -143,9 +142,9 @@ impl RttEstimate {
         self.smoothed_rtt
     }
 
-    pub fn pto(&self, pn_space: PacketNumberSpace) -> Duration {
+    pub fn pto(&self, hs_confirmed: bool) -> Duration {
         let mut t = self.estimate() + max(4 * self.rttvar, GRANULARITY);
-        if pn_space == PacketNumberSpace::ApplicationData {
+        if hs_confirmed {
             t += self.ack_delay.max();
         }
         t
diff --git a/neqo-transport/src/tracking.rs b/neqo-transport/src/tracking.rs
index 9eb23d70a6..669ee82cb6 100644
--- a/neqo-transport/src/tracking.rs
+++ b/neqo-transport/src/tracking.rs
@@ -287,12 +287,7 @@ impl RecvdPackets {
             ack_frequency_seqno: 0,
             ack_delay: DEFAULT_ACK_DELAY,
             unacknowledged_count: 0,
-            unacknowledged_tolerance: if space == PacketNumberSpace::ApplicationData {
-                DEFAULT_ACK_PACKET_TOLERANCE
-            } else {
-                // ACK more aggressively
-                0
-            },
+            unacknowledged_tolerance: DEFAULT_ACK_PACKET_TOLERANCE,
             ignore_order: false,
             ecn_count: EcnCount::default(),
         }

From fca99afd2851b0b8b92e6afe97d8087bb46dace7 Mon Sep 17 00:00:00 2001
From: Lars Eggert <lars@eggert.org>
Date: Fri, 16 Aug 2024 13:24:47 +0100
Subject: [PATCH 20/52] Nits

---
 neqo-transport/src/connection/tests/recovery.rs | 1 -
 neqo-transport/src/recovery/mod.rs              | 2 +-
 2 files changed, 1 insertion(+), 2 deletions(-)

diff --git a/neqo-transport/src/connection/tests/recovery.rs b/neqo-transport/src/connection/tests/recovery.rs
index cec39931c2..28327b0503 100644
--- a/neqo-transport/src/connection/tests/recovery.rs
+++ b/neqo-transport/src/connection/tests/recovery.rs
@@ -296,7 +296,6 @@ fn pto_handshake_complete() {
     assert_eq!(1, server.stats().dropped_rx - dropped_before1);
     assert_eq!(server.stats().frame_rx.all, server_frames);
 
-    // server.process_input(&pkt2_1rtt.unwrap(), now);
     let server_frames2 = server.stats().frame_rx.all;
     let dropped_before2 = server.stats().dropped_rx;
     server.process_input(&pkt2_hs, now);
diff --git a/neqo-transport/src/recovery/mod.rs b/neqo-transport/src/recovery/mod.rs
index 5b195800ca..e48160ecbb 100644
--- a/neqo-transport/src/recovery/mod.rs
+++ b/neqo-transport/src/recovery/mod.rs
@@ -847,7 +847,7 @@ impl LossRecovery {
         // pto_time to increase which might cause PTO for later packet number spaces to not fire.
         if let Some(pn_space) = pto_space {
             qtrace!([self], "PTO {}, probing {:?}", pn_space, allow_probes);
-            // Packets are only declared as lost, relative to the
+            // Packets are only declared as lost relative to
             // `largest_acked`. If we hit a PTO while we don't have a
             // largest_acked yet, also do a congestion control reaction (because
             // otherwise none would happen).

From 53c7bc7d7ddd3ad15bca6bc65b7b27a4a91e1faa Mon Sep 17 00:00:00 2001
From: Lars Eggert <lars@eggert.org>
Date: Thu, 22 Aug 2024 12:01:23 +0300
Subject: [PATCH 21/52] Use my patched simulator image (for now)

---
 .github/actions/quic-interop-runner/action.yml | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/.github/actions/quic-interop-runner/action.yml b/.github/actions/quic-interop-runner/action.yml
index fa22d76dc7..f2c7e7ea46 100644
--- a/.github/actions/quic-interop-runner/action.yml
+++ b/.github/actions/quic-interop-runner/action.yml
@@ -29,6 +29,13 @@ runs:
         repository: 'quic-interop/quic-interop-runner'
         path: 'quic-interop-runner'
 
+    # Until https://github.com/quic-interop/quic-network-simulator/pull/133 is merged
+    - name: Use patched simulator image
+      run: |
+        docker pull larseggert/quic-network-simulator:tcpdump
+        docker tag larseggert/quic-network-simulator:tcpdump martenseemann/quic-network-simulator:latest
+      shell: bash
+
     - name: Enable IPv6 support
       run: sudo modprobe ip6table_filter
       shell: bash

From 97b6be86ee1cd7a2ab00f76b4618aa30fdaafb2d Mon Sep 17 00:00:00 2001
From: Lars Eggert <lars@eggert.org>
Date: Thu, 5 Sep 2024 12:53:27 +0300
Subject: [PATCH 22/52] Revert QNS image

---
 .github/actions/quic-interop-runner/action.yml | 7 -------
 1 file changed, 7 deletions(-)

diff --git a/.github/actions/quic-interop-runner/action.yml b/.github/actions/quic-interop-runner/action.yml
index 6f9b354e0c..30a74c926f 100644
--- a/.github/actions/quic-interop-runner/action.yml
+++ b/.github/actions/quic-interop-runner/action.yml
@@ -29,13 +29,6 @@ runs:
         repository: 'quic-interop/quic-interop-runner'
         path: 'quic-interop-runner'
 
-    # Until https://github.com/quic-interop/quic-network-simulator/pull/133 is merged
-    - name: Use patched simulator image
-      run: |
-        docker pull larseggert/quic-network-simulator:tcpdump
-        docker tag larseggert/quic-network-simulator:tcpdump martenseemann/quic-network-simulator:latest
-      shell: bash
-
     - name: Enable IPv6 support
       run: sudo modprobe ip6table_filter
       shell: bash

From b29a83256b22c76d5eec578a66df270b63c898a9 Mon Sep 17 00:00:00 2001
From: Lars Eggert <lars@eggert.org>
Date: Thu, 5 Sep 2024 13:00:00 +0300
Subject: [PATCH 23/52] Fixes

---
 neqo-transport/src/path.rs         | 4 ++--
 neqo-transport/src/recovery/mod.rs | 3 ++-
 2 files changed, 4 insertions(+), 3 deletions(-)

diff --git a/neqo-transport/src/path.rs b/neqo-transport/src/path.rs
index 5940d38e2c..0de7fac6e1 100644
--- a/neqo-transport/src/path.rs
+++ b/neqo-transport/src/path.rs
@@ -1045,9 +1045,9 @@ impl Path {
     /// Initiate a congestion response.
     ///
     /// Returns true if the congestion window was reduced.
-    pub fn on_congestion_event(&mut self, lost_packets: &[SentPacket]) -> bool {
+    pub fn on_congestion_event(&mut self, lost_packets: &[SentPacket], stats: &mut Stats) -> bool {
         if let Some(last) = lost_packets.last() {
-            self.ecn_info.on_packets_lost(lost_packets);
+            self.ecn_info.on_packets_lost(lost_packets, stats);
             self.sender.on_congestion_event(last)
         } else {
             false
diff --git a/neqo-transport/src/recovery/mod.rs b/neqo-transport/src/recovery/mod.rs
index 7ea4e87d97..6be088e6e8 100644
--- a/neqo-transport/src/recovery/mod.rs
+++ b/neqo-transport/src/recovery/mod.rs
@@ -856,7 +856,8 @@ impl LossRecovery {
                 .get(pn_space)
                 .map_or(false, |space| space.largest_acked.is_none())
             {
-                path.borrow_mut().on_congestion_event(lost);
+                path.borrow_mut()
+                    .on_congestion_event(lost, &mut self.stats.borrow_mut());
             }
             self.fire_pto(pn_space, allow_probes);
         }

From 34fa79c84642bbd8ad65b3f9b186786df86091aa Mon Sep 17 00:00:00 2001
From: Lars Eggert <lars@eggert.org>
Date: Fri, 6 Sep 2024 11:38:36 +0300
Subject: [PATCH 24/52] Add debugs suggested by @mxinden

---
 neqo-transport/src/connection/mod.rs | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/neqo-transport/src/connection/mod.rs b/neqo-transport/src/connection/mod.rs
index 914a4ef6aa..58372ea0e5 100644
--- a/neqo-transport/src/connection/mod.rs
+++ b/neqo-transport/src/connection/mod.rs
@@ -2314,6 +2314,7 @@ impl Connection {
             );
             // The builder will set the limit to 0 if there isn't enough space for the header.
             if builder.is_full() {
+                qdebug!("Builder is full 1");
                 encoder = builder.abort();
                 break;
             }
@@ -2331,6 +2332,7 @@ impl Connection {
             );
             builder.enable_padding(needs_padding);
             if builder.is_full() {
+                qdebug!("Builder is full 2");
                 encoder = builder.abort();
                 break;
             }
@@ -2346,6 +2348,7 @@ impl Connection {
             }
             if builder.packet_empty() {
                 // Nothing to include in this packet.
+                qdebug!("Builder packet empty");
                 encoder = builder.abort();
                 continue;
             }

From 3e329458b9ab799c9457392be2ff3d72092e4988 Mon Sep 17 00:00:00 2001
From: Lars Eggert <lars@eggert.org>
Date: Fri, 6 Sep 2024 12:39:46 +0300
Subject: [PATCH 25/52] POssible fix

---
 neqo-transport/src/connection/mod.rs          | 11 +++++++++-
 .../src/connection/tests/handshake.rs         | 20 +++++++++++++++++++
 neqo-transport/src/crypto.rs                  |  5 ++++-
 neqo-transport/src/path.rs                    |  4 ++++
 neqo-transport/src/recovery/mod.rs            |  2 ++
 neqo-transport/src/recv_stream.rs             |  4 +++-
 6 files changed, 43 insertions(+), 3 deletions(-)

diff --git a/neqo-transport/src/connection/mod.rs b/neqo-transport/src/connection/mod.rs
index 58372ea0e5..17fa64c4a8 100644
--- a/neqo-transport/src/connection/mod.rs
+++ b/neqo-transport/src/connection/mod.rs
@@ -978,6 +978,7 @@ impl Connection {
 
         if let Some(path) = self.paths.primary() {
             let lost = self.loss_recovery.timeout(&path, now);
+            qdebug!("Handle lost packets: {:?}", lost);
             self.handle_lost_packets(&lost);
             qlog::packets_lost(&self.qlog, &lost);
         }
@@ -1919,7 +1920,7 @@ impl Connection {
     }
 
     fn output(&mut self, now: Instant) -> SendOption {
-        qtrace!([self], "output {:?}", now);
+        qtrace!([self], "output {:?} {:?}", now, self.state);
         let res = match &self.state {
             State::Init
             | State::WaitInitial
@@ -2156,6 +2157,7 @@ impl Connection {
         coalesced: bool, // Whether this packet is coalesced behind another one.
         now: Instant,
     ) -> (Vec<RecoveryToken>, bool, bool) {
+        qdebug!("write_frames {:?}", space);
         let mut tokens = Vec::new();
         let primary = path.borrow().is_primary();
         let mut ack_eliciting = false;
@@ -2191,6 +2193,7 @@ impl Connection {
 
         if profile.ack_only(space) {
             // If we are CC limited we can only send acks!
+            qdebug!("Only sending ACKs");
             return (tokens, false, false);
         }
 
@@ -2292,8 +2295,10 @@ impl Connection {
         // packets can go in a single datagram
         let mut encoder = Encoder::with_capacity(profile.limit());
         for space in PacketNumberSpace::iter() {
+            qdebug!("output_path {:?}", space);
             // Ensure we have tx crypto state for this epoch, or skip it.
             let Some((cspace, tx)) = self.crypto.states.select_tx_mut(self.version, *space) else {
+                qdebug!([self], "No tx state in {:?}", self.crypto.states);
                 continue;
             };
 
@@ -2862,6 +2867,10 @@ impl Connection {
                 } else {
                     // If we get a useless CRYPTO frame send outstanding CRYPTO frames again.
                     self.crypto.resend_unacked(space);
+                    // If this happens in the Initial space, resend also resend any Handshake data.
+                    if space == PacketNumberSpace::Initial {
+                        self.crypto.resend_unacked(PacketNumberSpace::Handshake);
+                    }
                 }
             }
             Frame::NewToken { token } => {
diff --git a/neqo-transport/src/connection/tests/handshake.rs b/neqo-transport/src/connection/tests/handshake.rs
index 57c5c554b7..4238a19e6d 100644
--- a/neqo-transport/src/connection/tests/handshake.rs
+++ b/neqo-transport/src/connection/tests/handshake.rs
@@ -1289,3 +1289,23 @@ fn client_handshake_retransmits_identical() {
         );
     }
 }
+
+#[test]
+fn quicgo() {
+    let mut now = now();
+    let mut client = default_client();
+    let mut server = default_server();
+
+    let ci = client.process(None, now).dgram();
+    now += DEFAULT_RTT / 2;
+    _ = server.process(ci.as_ref(), now);
+
+    // Drop si and wait for client to retransmit.
+    let pto = client.process_output(now).callback();
+    now += pto;
+    let ci = client.process_output(now).dgram();
+
+    // Server processes the retransmitted Initial packet.
+    // now += DEFAULT_RTT / 2;
+    _ = server.process(ci.as_ref(), now);
+}
diff --git a/neqo-transport/src/crypto.rs b/neqo-transport/src/crypto.rs
index 3a6d890cde..caefaf112d 100644
--- a/neqo-transport/src/crypto.rs
+++ b/neqo-transport/src/crypto.rs
@@ -326,6 +326,7 @@ impl Crypto {
         tokens: &mut Vec<RecoveryToken>,
         stats: &mut FrameStats,
     ) {
+        qdebug!("Writing crypto frames for space {:?}", space);
         self.streams.write_frame(space, builder, tokens, stats);
     }
 
@@ -1410,7 +1411,9 @@ impl CryptoStreams {
     }
 
     pub fn data_ready(&self, space: PacketNumberSpace) -> bool {
-        self.get(space).map_or(false, |cs| cs.rx.data_ready())
+        let x = self.get(space).map_or(false, |cs| cs.rx.data_ready());
+        qdebug!("Data ready for {}? {}", space, x);
+        x
     }
 
     pub fn read_to_end(&mut self, space: PacketNumberSpace, buf: &mut Vec<u8>) -> Res<usize> {
diff --git a/neqo-transport/src/path.rs b/neqo-transport/src/path.rs
index 0de7fac6e1..b1dc67a459 100644
--- a/neqo-transport/src/path.rs
+++ b/neqo-transport/src/path.rs
@@ -257,6 +257,7 @@ impl Paths {
     /// TODO(mt) - the paths should own the RTT estimator, so they can find the PTO
     /// for themselves.
     pub fn process_timeout(&mut self, now: Instant, pto: Duration, stats: &mut Stats) -> bool {
+        qdebug!("Processing path timeouts");
         let to_retire = &mut self.to_retire;
         let mut primary_failed = false;
         self.paths.retain(|p| {
@@ -286,8 +287,10 @@ impl Paths {
                 let path = Rc::clone(fallback);
                 qinfo!([path.borrow()], "Failing over after primary path failed");
                 mem::drop(self.select_primary(&path));
+                qdebug!("Path timeout processing complete 1");
                 true
             } else {
+                qdebug!("Path timeout processing complete 2");
                 false
             }
         } else {
@@ -295,6 +298,7 @@ impl Paths {
             if let Some(path) = self.primary() {
                 path.borrow_mut().pmtud_mut().maybe_fire_raise_timer(now);
             }
+            qdebug!("Path timeout processing complete 3");
             true
         }
     }
diff --git a/neqo-transport/src/recovery/mod.rs b/neqo-transport/src/recovery/mod.rs
index 6be088e6e8..eb0ff698f5 100644
--- a/neqo-transport/src/recovery/mod.rs
+++ b/neqo-transport/src/recovery/mod.rs
@@ -622,6 +622,7 @@ impl LossRecovery {
         let loss_delay = primary_path.borrow().rtt().loss_delay();
         let cleanup_delay = self.pto_period(primary_path.borrow().rtt());
         let mut lost = Vec::new();
+        qdebug!("detect 1");
         self.spaces.get_mut(pn_space).unwrap().detect_lost_packets(
             now,
             loss_delay,
@@ -878,6 +879,7 @@ impl LossRecovery {
                 confirmed,
                 self.fast_pto,
             );
+            qdebug!("detect 2");
             space.detect_lost_packets(now, loss_delay, pto, &mut lost_packets);
 
             primary_path.borrow_mut().on_packets_lost(
diff --git a/neqo-transport/src/recv_stream.rs b/neqo-transport/src/recv_stream.rs
index 7b46a386bc..22375f6c32 100644
--- a/neqo-transport/src/recv_stream.rs
+++ b/neqo-transport/src/recv_stream.rs
@@ -15,7 +15,7 @@ use std::{
     rc::{Rc, Weak},
 };
 
-use neqo_common::{qtrace, Role};
+use neqo_common::{qdebug, qtrace, Role};
 use smallvec::SmallVec;
 
 use crate::{
@@ -153,6 +153,7 @@ impl RxStreamOrderer {
         let new_end = new_start + u64::try_from(new_data.len()).unwrap();
 
         if new_end <= self.retired {
+            qdebug!("Dropping frame with already-retired range {}-{}", new_start, new_end);
             // Range already read by application, this frame is very late and unneeded.
             return;
         }
@@ -164,6 +165,7 @@ impl RxStreamOrderer {
 
         if new_data.is_empty() {
             // No data to insert
+            qdebug!("Dropping empty frame at {}", new_start);
             return;
         }
 

From 17190e95370fc1751e63e53d62aa56d028257056 Mon Sep 17 00:00:00 2001
From: Lars Eggert <lars@eggert.org>
Date: Fri, 6 Sep 2024 12:42:01 +0300
Subject: [PATCH 26/52] More drastic fix

---
 neqo-transport/src/connection/mod.rs | 4 ----
 neqo-transport/src/crypto.rs         | 4 ++++
 2 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/neqo-transport/src/connection/mod.rs b/neqo-transport/src/connection/mod.rs
index 17fa64c4a8..55a4460cb5 100644
--- a/neqo-transport/src/connection/mod.rs
+++ b/neqo-transport/src/connection/mod.rs
@@ -2867,10 +2867,6 @@ impl Connection {
                 } else {
                     // If we get a useless CRYPTO frame send outstanding CRYPTO frames again.
                     self.crypto.resend_unacked(space);
-                    // If this happens in the Initial space, resend also resend any Handshake data.
-                    if space == PacketNumberSpace::Initial {
-                        self.crypto.resend_unacked(PacketNumberSpace::Handshake);
-                    }
                 }
             }
             Frame::NewToken { token } => {
diff --git a/neqo-transport/src/crypto.rs b/neqo-transport/src/crypto.rs
index caefaf112d..4fd26e1694 100644
--- a/neqo-transport/src/crypto.rs
+++ b/neqo-transport/src/crypto.rs
@@ -354,6 +354,10 @@ impl Crypto {
     /// that they can be sent again.
     pub fn resend_unacked(&mut self, space: PacketNumberSpace) {
         self.streams.resend_unacked(space);
+        // If this happens in the Initial space, resend also resend any Handshake data.
+        if space == PacketNumberSpace::Initial {
+            self.resend_unacked(PacketNumberSpace::Handshake);
+        }
     }
 
     /// Discard state for a packet number space and return true

From bfde87f0e637fca8e30a872203364dc9feb74105 Mon Sep 17 00:00:00 2001
From: Lars Eggert <lars@eggert.org>
Date: Fri, 6 Sep 2024 13:05:17 +0300
Subject: [PATCH 27/52] fmt

---
 neqo-transport/src/recv_stream.rs | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/neqo-transport/src/recv_stream.rs b/neqo-transport/src/recv_stream.rs
index 22375f6c32..9245c17004 100644
--- a/neqo-transport/src/recv_stream.rs
+++ b/neqo-transport/src/recv_stream.rs
@@ -153,7 +153,11 @@ impl RxStreamOrderer {
         let new_end = new_start + u64::try_from(new_data.len()).unwrap();
 
         if new_end <= self.retired {
-            qdebug!("Dropping frame with already-retired range {}-{}", new_start, new_end);
+            qdebug!(
+                "Dropping frame with already-retired range {}-{}",
+                new_start,
+                new_end
+            );
             // Range already read by application, this frame is very late and unneeded.
             return;
         }

From e1c18c5e68683c857829a6414605dacc475f2fed Mon Sep 17 00:00:00 2001
From: Lars Eggert <lars@eggert.org>
Date: Fri, 6 Sep 2024 13:48:14 +0300
Subject: [PATCH 28/52] Remove debug logging

---
 neqo-transport/src/connection/mod.rs | 10 +---------
 neqo-transport/src/crypto.rs         |  7 ++-----
 neqo-transport/src/path.rs           |  4 ----
 neqo-transport/src/recovery/mod.rs   |  2 --
 neqo-transport/src/recv_stream.rs    |  8 +-------
 5 files changed, 4 insertions(+), 27 deletions(-)

diff --git a/neqo-transport/src/connection/mod.rs b/neqo-transport/src/connection/mod.rs
index 55a4460cb5..914a4ef6aa 100644
--- a/neqo-transport/src/connection/mod.rs
+++ b/neqo-transport/src/connection/mod.rs
@@ -978,7 +978,6 @@ impl Connection {
 
         if let Some(path) = self.paths.primary() {
             let lost = self.loss_recovery.timeout(&path, now);
-            qdebug!("Handle lost packets: {:?}", lost);
             self.handle_lost_packets(&lost);
             qlog::packets_lost(&self.qlog, &lost);
         }
@@ -1920,7 +1919,7 @@ impl Connection {
     }
 
     fn output(&mut self, now: Instant) -> SendOption {
-        qtrace!([self], "output {:?} {:?}", now, self.state);
+        qtrace!([self], "output {:?}", now);
         let res = match &self.state {
             State::Init
             | State::WaitInitial
@@ -2157,7 +2156,6 @@ impl Connection {
         coalesced: bool, // Whether this packet is coalesced behind another one.
         now: Instant,
     ) -> (Vec<RecoveryToken>, bool, bool) {
-        qdebug!("write_frames {:?}", space);
         let mut tokens = Vec::new();
         let primary = path.borrow().is_primary();
         let mut ack_eliciting = false;
@@ -2193,7 +2191,6 @@ impl Connection {
 
         if profile.ack_only(space) {
             // If we are CC limited we can only send acks!
-            qdebug!("Only sending ACKs");
             return (tokens, false, false);
         }
 
@@ -2295,10 +2292,8 @@ impl Connection {
         // packets can go in a single datagram
         let mut encoder = Encoder::with_capacity(profile.limit());
         for space in PacketNumberSpace::iter() {
-            qdebug!("output_path {:?}", space);
             // Ensure we have tx crypto state for this epoch, or skip it.
             let Some((cspace, tx)) = self.crypto.states.select_tx_mut(self.version, *space) else {
-                qdebug!([self], "No tx state in {:?}", self.crypto.states);
                 continue;
             };
 
@@ -2319,7 +2314,6 @@ impl Connection {
             );
             // The builder will set the limit to 0 if there isn't enough space for the header.
             if builder.is_full() {
-                qdebug!("Builder is full 1");
                 encoder = builder.abort();
                 break;
             }
@@ -2337,7 +2331,6 @@ impl Connection {
             );
             builder.enable_padding(needs_padding);
             if builder.is_full() {
-                qdebug!("Builder is full 2");
                 encoder = builder.abort();
                 break;
             }
@@ -2353,7 +2346,6 @@ impl Connection {
             }
             if builder.packet_empty() {
                 // Nothing to include in this packet.
-                qdebug!("Builder packet empty");
                 encoder = builder.abort();
                 continue;
             }
diff --git a/neqo-transport/src/crypto.rs b/neqo-transport/src/crypto.rs
index 4fd26e1694..454a7679d3 100644
--- a/neqo-transport/src/crypto.rs
+++ b/neqo-transport/src/crypto.rs
@@ -326,7 +326,6 @@ impl Crypto {
         tokens: &mut Vec<RecoveryToken>,
         stats: &mut FrameStats,
     ) {
-        qdebug!("Writing crypto frames for space {:?}", space);
         self.streams.write_frame(space, builder, tokens, stats);
     }
 
@@ -354,7 +353,7 @@ impl Crypto {
     /// that they can be sent again.
     pub fn resend_unacked(&mut self, space: PacketNumberSpace) {
         self.streams.resend_unacked(space);
-        // If this happens in the Initial space, resend also resend any Handshake data.
+        // If this happens in the Initial space, also resend any Handshake data.
         if space == PacketNumberSpace::Initial {
             self.resend_unacked(PacketNumberSpace::Handshake);
         }
@@ -1415,9 +1414,7 @@ impl CryptoStreams {
     }
 
     pub fn data_ready(&self, space: PacketNumberSpace) -> bool {
-        let x = self.get(space).map_or(false, |cs| cs.rx.data_ready());
-        qdebug!("Data ready for {}? {}", space, x);
-        x
+        self.get(space).map_or(false, |cs| cs.rx.data_ready())
     }
 
     pub fn read_to_end(&mut self, space: PacketNumberSpace, buf: &mut Vec<u8>) -> Res<usize> {
diff --git a/neqo-transport/src/path.rs b/neqo-transport/src/path.rs
index b1dc67a459..0de7fac6e1 100644
--- a/neqo-transport/src/path.rs
+++ b/neqo-transport/src/path.rs
@@ -257,7 +257,6 @@ impl Paths {
     /// TODO(mt) - the paths should own the RTT estimator, so they can find the PTO
     /// for themselves.
     pub fn process_timeout(&mut self, now: Instant, pto: Duration, stats: &mut Stats) -> bool {
-        qdebug!("Processing path timeouts");
         let to_retire = &mut self.to_retire;
         let mut primary_failed = false;
         self.paths.retain(|p| {
@@ -287,10 +286,8 @@ impl Paths {
                 let path = Rc::clone(fallback);
                 qinfo!([path.borrow()], "Failing over after primary path failed");
                 mem::drop(self.select_primary(&path));
-                qdebug!("Path timeout processing complete 1");
                 true
             } else {
-                qdebug!("Path timeout processing complete 2");
                 false
             }
         } else {
@@ -298,7 +295,6 @@ impl Paths {
             if let Some(path) = self.primary() {
                 path.borrow_mut().pmtud_mut().maybe_fire_raise_timer(now);
             }
-            qdebug!("Path timeout processing complete 3");
             true
         }
     }
diff --git a/neqo-transport/src/recovery/mod.rs b/neqo-transport/src/recovery/mod.rs
index eb0ff698f5..6be088e6e8 100644
--- a/neqo-transport/src/recovery/mod.rs
+++ b/neqo-transport/src/recovery/mod.rs
@@ -622,7 +622,6 @@ impl LossRecovery {
         let loss_delay = primary_path.borrow().rtt().loss_delay();
         let cleanup_delay = self.pto_period(primary_path.borrow().rtt());
         let mut lost = Vec::new();
-        qdebug!("detect 1");
         self.spaces.get_mut(pn_space).unwrap().detect_lost_packets(
             now,
             loss_delay,
@@ -879,7 +878,6 @@ impl LossRecovery {
                 confirmed,
                 self.fast_pto,
             );
-            qdebug!("detect 2");
             space.detect_lost_packets(now, loss_delay, pto, &mut lost_packets);
 
             primary_path.borrow_mut().on_packets_lost(
diff --git a/neqo-transport/src/recv_stream.rs b/neqo-transport/src/recv_stream.rs
index 9245c17004..7b46a386bc 100644
--- a/neqo-transport/src/recv_stream.rs
+++ b/neqo-transport/src/recv_stream.rs
@@ -15,7 +15,7 @@ use std::{
     rc::{Rc, Weak},
 };
 
-use neqo_common::{qdebug, qtrace, Role};
+use neqo_common::{qtrace, Role};
 use smallvec::SmallVec;
 
 use crate::{
@@ -153,11 +153,6 @@ impl RxStreamOrderer {
         let new_end = new_start + u64::try_from(new_data.len()).unwrap();
 
         if new_end <= self.retired {
-            qdebug!(
-                "Dropping frame with already-retired range {}-{}",
-                new_start,
-                new_end
-            );
             // Range already read by application, this frame is very late and unneeded.
             return;
         }
@@ -169,7 +164,6 @@ impl RxStreamOrderer {
 
         if new_data.is_empty() {
             // No data to insert
-            qdebug!("Dropping empty frame at {}", new_start);
             return;
         }
 

From 02bede7750a0113b671845a443da4aee99e0d279 Mon Sep 17 00:00:00 2001
From: Lars Eggert <lars@eggert.org>
Date: Mon, 9 Sep 2024 09:13:46 +0300
Subject: [PATCH 29/52] Update test

---
 .../src/connection/tests/handshake.rs         | 47 +++++++++++--------
 1 file changed, 27 insertions(+), 20 deletions(-)

diff --git a/neqo-transport/src/connection/tests/handshake.rs b/neqo-transport/src/connection/tests/handshake.rs
index 4238a19e6d..5691693ce9 100644
--- a/neqo-transport/src/connection/tests/handshake.rs
+++ b/neqo-transport/src/connection/tests/handshake.rs
@@ -1255,6 +1255,33 @@ fn server_initial_retransmits_identical() {
     }
 }
 
+#[test]
+fn server_triggered_initial_retransmits_identical() {
+    let mut now = now();
+    let mut client = default_client();
+    let mut server = default_server();
+
+    let ci = client.process(None, now).dgram();
+    now += DEFAULT_RTT / 2;
+    let si1 = server.process(ci.as_ref(), now);
+    let stats1 = server.stats().frame_tx;
+
+    // Drop si and wait for client to retransmit.
+    let pto = client.process_output(now).callback();
+    now += pto;
+    let ci = client.process_output(now).dgram();
+
+    // Feed the RTX'ed ci into the server before its PTO fires. The server
+    // should process it and then retransmit its Initial packet including
+    // any coalesced Handshake data.
+    let si2 = server.process(ci.as_ref(), now);
+    let stats2 = server.stats().frame_tx;
+    assert_eq!(si1.dgram().unwrap().len(), si2.dgram().unwrap().len());
+    assert_eq!(stats2.all, stats1.all * 2);
+    assert_eq!(stats2.crypto, stats1.crypto * 2);
+    assert_eq!(stats2.ack, stats1.ack * 2);
+}
+
 #[test]
 fn client_handshake_retransmits_identical() {
     let mut now = now();
@@ -1289,23 +1316,3 @@ fn client_handshake_retransmits_identical() {
         );
     }
 }
-
-#[test]
-fn quicgo() {
-    let mut now = now();
-    let mut client = default_client();
-    let mut server = default_server();
-
-    let ci = client.process(None, now).dgram();
-    now += DEFAULT_RTT / 2;
-    _ = server.process(ci.as_ref(), now);
-
-    // Drop si and wait for client to retransmit.
-    let pto = client.process_output(now).callback();
-    now += pto;
-    let ci = client.process_output(now).dgram();
-
-    // Server processes the retransmitted Initial packet.
-    // now += DEFAULT_RTT / 2;
-    _ = server.process(ci.as_ref(), now);
-}

From 9356346d97bef587ae9489317fcd641725a45b08 Mon Sep 17 00:00:00 2001
From: Lars Eggert <lars@eggert.org>
Date: Mon, 9 Sep 2024 11:17:55 +0300
Subject: [PATCH 30/52] Do not encode long RTT guesses in resumption tokens

---
 neqo-transport/src/connection/mod.rs | 22 +++++++++++++++++++---
 1 file changed, 19 insertions(+), 3 deletions(-)

diff --git a/neqo-transport/src/connection/mod.rs b/neqo-transport/src/connection/mod.rs
index 914a4ef6aa..a21e5b1f9b 100644
--- a/neqo-transport/src/connection/mod.rs
+++ b/neqo-transport/src/connection/mod.rs
@@ -48,7 +48,7 @@ use crate::{
     quic_datagrams::{DatagramTracking, QuicDatagrams},
     recovery::{LossRecovery, RecoveryToken, SendProfile, SentPacket},
     recv_stream::RecvStreamStats,
-    rtt::{RttEstimate, GRANULARITY},
+    rtt::{RttEstimate, GRANULARITY, INITIAL_RTT},
     send_stream::SendStream,
     stats::{Stats, StatsCell},
     stream_id::StreamType,
@@ -594,9 +594,25 @@ impl Connection {
     fn make_resumption_token(&mut self) -> ResumptionToken {
         debug_assert_eq!(self.role, Role::Client);
         debug_assert!(self.crypto.has_resumption_token());
+        // Values less than GRANULARITY are ignored when using the token, so use 0 where needed.
+        let zero = Duration::from_millis(0);
         let rtt = self.paths.primary().map_or_else(
-            || RttEstimate::default().estimate(),
-            |p| p.borrow().rtt().estimate(),
+            || zero,
+            |p| {
+                let rtt = p.borrow().rtt().estimate();
+                // Do not encode a guestimated RTT if we have no actual samples and the guess is
+                // larger than the default initial RTT. (The guess can be very large under lossy
+                // conditions.)
+                if p.borrow().rtt().first_sample_time().is_none() {
+                    if rtt < INITIAL_RTT {
+                        rtt
+                    } else {
+                        zero
+                    }
+                } else {
+                    rtt
+                }
+            },
         );
 
         self.crypto

From de91e3286fa9eaab0cf4ea52b66559b13a89ba4f Mon Sep 17 00:00:00 2001
From: Lars Eggert <lars@eggert.org>
Date: Mon, 9 Sep 2024 14:46:18 +0300
Subject: [PATCH 31/52] trace

---
 qns/interop.sh | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/qns/interop.sh b/qns/interop.sh
index c83dd6552d..3cc448694c 100755
--- a/qns/interop.sh
+++ b/qns/interop.sh
@@ -13,7 +13,7 @@ case "$ROLE" in
 client)
   /wait-for-it.sh sim:57832 -s -t 30
   # shellcheck disable=SC2086
-  RUST_LOG=debug RUST_BACKTRACE=1 neqo-client --cc cubic --qns-test "$TESTCASE" \
+  RUST_LOG=trace RUST_BACKTRACE=1 neqo-client --cc cubic --qns-test "$TESTCASE" \
     --qlog-dir "$QLOGDIR" --output-dir /downloads $REQUESTS 2> >(tee -i -a "/logs/$ROLE.log" >&2)
   ;;
 
@@ -27,7 +27,7 @@ server)
     -name "$CERT" -passout pass: -out "$P12CERT"
   pk12util -d "sql:$DB" -i "$P12CERT" -W ''
   certutil -L -d "sql:$DB" -n "$CERT"
-  RUST_LOG=debug RUST_BACKTRACE=1 neqo-server --cc cubic --qns-test "$TESTCASE" \
+  RUST_LOG=trace RUST_BACKTRACE=1 neqo-server --cc cubic --qns-test "$TESTCASE" \
     --qlog-dir "$QLOGDIR" -d "$DB" -k "$CERT" '[::]:443' 2> >(tee -i -a "/logs/$ROLE.log" >&2)
   ;;
 

From 91a3ae6692fcbcf7c439d90278b1e45820784e9d Mon Sep 17 00:00:00 2001
From: Lars Eggert <lars@eggert.org>
Date: Mon, 9 Sep 2024 15:48:03 +0300
Subject: [PATCH 32/52] Also RTX 0-RTT

---
 neqo-transport/src/connection/mod.rs          | 28 ++++++++-----
 .../src/connection/tests/handshake.rs         | 40 ++++++++++++++++++-
 neqo-transport/src/crypto.rs                  |  4 --
 neqo-transport/src/send_stream.rs             |  6 +++
 4 files changed, 64 insertions(+), 14 deletions(-)

diff --git a/neqo-transport/src/connection/mod.rs b/neqo-transport/src/connection/mod.rs
index a21e5b1f9b..967607305e 100644
--- a/neqo-transport/src/connection/mod.rs
+++ b/neqo-transport/src/connection/mod.rs
@@ -1412,12 +1412,14 @@ impl Connection {
                 // that Initial packets were lost.
                 // Resend Initial CRYPTO frames immediately a few times just
                 // in case.  As we don't have an RTT estimate yet, this helps
-                // when there is a short RTT and losses.
+                // when there is a short RTT and losses. Also mark all 0-RTT
+                // data as lost.
                 if dcid.is_none()
                     && self.cid_manager.is_valid(packet.dcid())
                     && self.stats.borrow().saved_datagrams <= EXTRA_INITIALS
                 {
                     self.crypto.resend_unacked(PacketNumberSpace::Initial);
+                    self.drop_0rtt(now);
                 }
             }
             (PacketType::VersionNegotiation | PacketType::Retry | PacketType::OtherVersion, ..) => {
@@ -2873,8 +2875,13 @@ impl Connection {
                     self.handshake(now, packet_version, space, Some(&buf))?;
                     self.create_resumption_token(now);
                 } else {
-                    // If we get a useless CRYPTO frame send outstanding CRYPTO frames again.
+                    // If we get a useless CRYPTO frame send outstanding CRYPTO frames and 0-RTT
+                    // data again.
                     self.crypto.resend_unacked(space);
+                    if space == PacketNumberSpace::Initial {
+                        self.crypto.resend_unacked(PacketNumberSpace::Handshake);
+                        self.drop_0rtt(now);
+                    }
                 }
             }
             Frame::NewToken { token } => {
@@ -3086,19 +3093,22 @@ impl Connection {
         stats.largest_acknowledged = max(stats.largest_acknowledged, largest_acknowledged);
     }
 
-    /// When the server rejects 0-RTT we need to drop a bunch of stuff.
-    fn client_0rtt_rejected(&mut self, now: Instant) {
-        if !matches!(self.zero_rtt_state, ZeroRttState::Sending) {
-            return;
-        }
-        qdebug!([self], "0-RTT rejected");
-
+        // Tell 0-RTT packets that they were "lost".
+    fn drop_0rtt(&mut self, now: Instant) {
         // Tell 0-RTT packets that they were "lost".
         if let Some(path) = self.paths.primary() {
             let dropped = self.loss_recovery.drop_0rtt(&path, now);
             self.handle_lost_packets(&dropped);
         }
+    }
 
+    /// When the server rejects 0-RTT we need to drop a bunch of stuff.
+    fn client_0rtt_rejected(&mut self, now: Instant) {
+        if !matches!(self.zero_rtt_state, ZeroRttState::Sending) {
+            return;
+        }
+        qdebug!([self], "0-RTT rejected");
+        self.drop_0rtt(now);
         self.streams.zero_rtt_rejected();
 
         self.crypto.states.discard_0rtt_keys();
diff --git a/neqo-transport/src/connection/tests/handshake.rs b/neqo-transport/src/connection/tests/handshake.rs
index 5691693ce9..05c42de41d 100644
--- a/neqo-transport/src/connection/tests/handshake.rs
+++ b/neqo-transport/src/connection/tests/handshake.rs
@@ -30,7 +30,7 @@ use super::{
 };
 use crate::{
     connection::{
-        tests::{new_client, new_server},
+        tests::{exchange_ticket, new_client, new_server},
         AddressValidation,
     },
     events::ConnectionEvent,
@@ -1220,6 +1220,44 @@ fn client_initial_retransmits_identical() {
     }
 }
 
+#[test]
+fn client_triggered_zerortt_retransmits_identical() {
+    let mut client = default_client();
+    let mut server = default_server();
+    connect(&mut client, &mut server);
+
+    let token = exchange_ticket(&mut client, &mut server, now());
+    let mut client = default_client();
+    client
+        .enable_resumption(now(), token)
+        .expect("should set token");
+    let mut server = resumed_server(&client);
+
+    // Write 0-RTT before generating any packets.
+    // This should result in a datagram that coalesces Initial and 0-RTT.
+    let client_stream_id = client.stream_create(StreamType::UniDi).unwrap();
+    client.stream_send(client_stream_id, &[1, 2, 3]).unwrap();
+    let client_0rtt = client.process(None, now());
+    assert!(client_0rtt.as_dgram_ref().is_some());
+    let stats1 = client.stats().frame_tx;
+
+    assertions::assert_coalesced_0rtt(&client_0rtt.as_dgram_ref().unwrap()[..]);
+
+    let s1 = server.process(client_0rtt.as_dgram_ref(), now());
+    assert!(s1.as_dgram_ref().is_some()); // Should produce ServerHello etc...
+
+    // Drop the Initial packet from this.
+    let (_, s_hs) = split_datagram(s1.as_dgram_ref().unwrap());
+    assert!(s_hs.is_some());
+
+    // Passing only the server handshake packet to the client should trigger a retransmit.
+    _ = client.process(s_hs.as_ref(), now()).dgram();
+    let stats2 = client.stats().frame_tx;
+    assert_eq!(stats2.all, stats1.all * 2);
+    assert_eq!(stats2.crypto, stats1.crypto * 2);
+    assert_eq!(stats2.stream, stats1.stream * 2);
+}
+
 #[test]
 fn server_initial_retransmits_identical() {
     let mut now = now();
diff --git a/neqo-transport/src/crypto.rs b/neqo-transport/src/crypto.rs
index 454a7679d3..3a6d890cde 100644
--- a/neqo-transport/src/crypto.rs
+++ b/neqo-transport/src/crypto.rs
@@ -353,10 +353,6 @@ impl Crypto {
     /// that they can be sent again.
     pub fn resend_unacked(&mut self, space: PacketNumberSpace) {
         self.streams.resend_unacked(space);
-        // If this happens in the Initial space, also resend any Handshake data.
-        if space == PacketNumberSpace::Initial {
-            self.resend_unacked(PacketNumberSpace::Handshake);
-        }
     }
 
     /// Discard state for a packet number space and return true
diff --git a/neqo-transport/src/send_stream.rs b/neqo-transport/src/send_stream.rs
index 3f0002da13..de7ba78ec4 100644
--- a/neqo-transport/src/send_stream.rs
+++ b/neqo-transport/src/send_stream.rs
@@ -1659,6 +1659,12 @@ impl SendStreams {
         }
     }
 
+    pub fn mark_all_as_lost(&mut self) {
+        for stream in self.map.values_mut() {
+            stream.mark_as_lost(0, usize::MAX, true);
+        }
+    }
+
     pub fn clear(&mut self) {
         self.map.clear();
         self.sendordered.clear();

From 8d88c2411f27c498917c0117c18886df110cbd4f Mon Sep 17 00:00:00 2001
From: Lars Eggert <lars@eggert.org>
Date: Mon, 9 Sep 2024 16:06:15 +0300
Subject: [PATCH 33/52] fmt

---
 neqo-transport/src/connection/mod.rs | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/neqo-transport/src/connection/mod.rs b/neqo-transport/src/connection/mod.rs
index 967607305e..5fbd652d46 100644
--- a/neqo-transport/src/connection/mod.rs
+++ b/neqo-transport/src/connection/mod.rs
@@ -3093,7 +3093,7 @@ impl Connection {
         stats.largest_acknowledged = max(stats.largest_acknowledged, largest_acknowledged);
     }
 
-        // Tell 0-RTT packets that they were "lost".
+    // Tell 0-RTT packets that they were "lost".
     fn drop_0rtt(&mut self, now: Instant) {
         // Tell 0-RTT packets that they were "lost".
         if let Some(path) = self.paths.primary() {

From 63aa0bd21c04abecf82d0f91f8e48db19fb93b41 Mon Sep 17 00:00:00 2001
From: Lars Eggert <lars@eggert.org>
Date: Mon, 9 Sep 2024 16:52:11 +0300
Subject: [PATCH 34/52] debug

---
 qns/interop.sh | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/qns/interop.sh b/qns/interop.sh
index 3cc448694c..c83dd6552d 100755
--- a/qns/interop.sh
+++ b/qns/interop.sh
@@ -13,7 +13,7 @@ case "$ROLE" in
 client)
   /wait-for-it.sh sim:57832 -s -t 30
   # shellcheck disable=SC2086
-  RUST_LOG=trace RUST_BACKTRACE=1 neqo-client --cc cubic --qns-test "$TESTCASE" \
+  RUST_LOG=debug RUST_BACKTRACE=1 neqo-client --cc cubic --qns-test "$TESTCASE" \
     --qlog-dir "$QLOGDIR" --output-dir /downloads $REQUESTS 2> >(tee -i -a "/logs/$ROLE.log" >&2)
   ;;
 
@@ -27,7 +27,7 @@ server)
     -name "$CERT" -passout pass: -out "$P12CERT"
   pk12util -d "sql:$DB" -i "$P12CERT" -W ''
   certutil -L -d "sql:$DB" -n "$CERT"
-  RUST_LOG=trace RUST_BACKTRACE=1 neqo-server --cc cubic --qns-test "$TESTCASE" \
+  RUST_LOG=debug RUST_BACKTRACE=1 neqo-server --cc cubic --qns-test "$TESTCASE" \
     --qlog-dir "$QLOGDIR" -d "$DB" -k "$CERT" '[::]:443' 2> >(tee -i -a "/logs/$ROLE.log" >&2)
   ;;
 

From 4fa5b10cddac0e542e794d68a191e9f9ff6ae9bf Mon Sep 17 00:00:00 2001
From: Lars Eggert <lars@eggert.org>
Date: Mon, 9 Sep 2024 17:00:04 +0300
Subject: [PATCH 35/52] No pacing for zerortt test

---
 neqo-bin/src/client/mod.rs | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/neqo-bin/src/client/mod.rs b/neqo-bin/src/client/mod.rs
index ebbab98286..923be18fc1 100644
--- a/neqo-bin/src/client/mod.rs
+++ b/neqo-bin/src/client/mod.rs
@@ -254,6 +254,9 @@ impl Args {
                 self.resume = true;
                 // PMTUD probes inflate what we sent in 1-RTT, causing QNS to fail the test.
                 self.shared.quic_parameters.no_pmtud = true;
+                // If we pace, we might get the initial server flight before sending sufficient 0-RTT
+                // data to pass the QNS check.
+                self.shared.quic_parameters.no_pacing = true;
             }
             "multiconnect" => {
                 self.shared.use_old_http = true;

From 1c83522ba1dbd69933626cb0951a267934362ae6 Mon Sep 17 00:00:00 2001
From: Lars Eggert <lars@eggert.org>
Date: Mon, 9 Sep 2024 17:54:09 +0300
Subject: [PATCH 36/52] log resumption

---
 neqo-transport/src/connection/mod.rs | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/neqo-transport/src/connection/mod.rs b/neqo-transport/src/connection/mod.rs
index 5fbd652d46..4e52f737ca 100644
--- a/neqo-transport/src/connection/mod.rs
+++ b/neqo-transport/src/connection/mod.rs
@@ -730,25 +730,25 @@ impl Connection {
         let version = Version::try_from(u32::try_from(
             dec.decode_uint(4).ok_or(Error::InvalidResumptionToken)?,
         )?)?;
-        qtrace!([self], "  version {:?}", version);
+        qdebug!([self], "  version {:?}", version);
         if !self.conn_params.get_versions().all().contains(&version) {
             return Err(Error::DisabledVersion);
         }
 
         let rtt = Duration::from_millis(dec.decode_varint().ok_or(Error::InvalidResumptionToken)?);
-        qtrace!([self], "  RTT {:?}", rtt);
+        qdebug!([self], "  RTT {:?}", rtt);
 
         let tp_slice = dec.decode_vvec().ok_or(Error::InvalidResumptionToken)?;
-        qtrace!([self], "  transport parameters {}", hex(tp_slice));
+        qdebug!([self], "  transport parameters {}", hex(tp_slice));
         let mut dec_tp = Decoder::from(tp_slice);
         let tp =
             TransportParameters::decode(&mut dec_tp).map_err(|_| Error::InvalidResumptionToken)?;
 
         let init_token = dec.decode_vvec().ok_or(Error::InvalidResumptionToken)?;
-        qtrace!([self], "  Initial token {}", hex(init_token));
+        qdebug!([self], "  Initial token {}", hex(init_token));
 
         let tok = dec.decode_remainder();
-        qtrace!([self], "  TLS token {}", hex(tok));
+        qdebug!([self], "  TLS token {}", hex(tok));
 
         match self.crypto.tls {
             Agent::Client(ref mut c) => {

From d1d3898250cfee39d8526db909d21e02c17d5a0c Mon Sep 17 00:00:00 2001
From: Lars Eggert <lars@eggert.org>
Date: Tue, 10 Sep 2024 10:11:50 +0300
Subject: [PATCH 37/52] Log type of saved datagram

---
 neqo-transport/src/connection/saved.rs | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/neqo-transport/src/connection/saved.rs b/neqo-transport/src/connection/saved.rs
index d5e0313e52..fb8cba2227 100644
--- a/neqo-transport/src/connection/saved.rs
+++ b/neqo-transport/src/connection/saved.rs
@@ -41,10 +41,10 @@ impl SavedDatagrams {
         let store = self.store(cspace);
 
         if store.len() < MAX_SAVED_DATAGRAMS {
-            qdebug!("saving datagram of {} bytes", d.len());
+            qdebug!("saving {:?} datagram of {} bytes", cspace, d.len());
             store.push(SavedDatagram { d, t });
         } else {
-            qinfo!("not saving datagram of {} bytes", d.len());
+            qinfo!("not saving {:?} datagram of {} bytes", cspace, d.len());
         }
     }
 

From 15b7521f4a4e36b55aa85277ad3a7ae4ef9aaa57 Mon Sep 17 00:00:00 2001
From: Lars Eggert <lars@eggert.org>
Date: Tue, 10 Sep 2024 12:30:55 +0300
Subject: [PATCH 38/52] Separate out the `resumption` test

---
 neqo-bin/src/client/mod.rs | 10 +++++++++-
 1 file changed, 9 insertions(+), 1 deletion(-)

diff --git a/neqo-bin/src/client/mod.rs b/neqo-bin/src/client/mod.rs
index 923be18fc1..0c0f3d4321 100644
--- a/neqo-bin/src/client/mod.rs
+++ b/neqo-bin/src/client/mod.rs
@@ -245,7 +245,15 @@ impl Args {
             "handshake" | "transfer" | "retry" | "ecn" => {
                 self.shared.use_old_http = true;
             }
-            "zerortt" | "resumption" => {
+            "resumption" => {
+                if self.urls.len() < 2 {
+                    qerror!("Warning: resumption tests won't work without >1 URL");
+                    exit(127);
+                }
+                self.shared.use_old_http = true;
+                self.resume = true;
+            }
+            "zerortt" => {
                 if self.urls.len() < 2 {
                     qerror!("Warning: resumption tests won't work without >1 URL");
                     exit(127);

From 838942e412819c14e313e0c972624420747a3065 Mon Sep 17 00:00:00 2001
From: Lars Eggert <lars@eggert.org>
Date: Tue, 10 Sep 2024 15:31:30 +0200
Subject: [PATCH 39/52] fmt

---
 neqo-bin/src/client/mod.rs | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/neqo-bin/src/client/mod.rs b/neqo-bin/src/client/mod.rs
index 0c0f3d4321..08e0abeb46 100644
--- a/neqo-bin/src/client/mod.rs
+++ b/neqo-bin/src/client/mod.rs
@@ -262,8 +262,8 @@ impl Args {
                 self.resume = true;
                 // PMTUD probes inflate what we sent in 1-RTT, causing QNS to fail the test.
                 self.shared.quic_parameters.no_pmtud = true;
-                // If we pace, we might get the initial server flight before sending sufficient 0-RTT
-                // data to pass the QNS check.
+                // If we pace, we might get the initial server flight before sending sufficient
+                // 0-RTT data to pass the QNS check.
                 self.shared.quic_parameters.no_pacing = true;
             }
             "multiconnect" => {

From 3b7fe10859297a448d954176e878df17a5a497db Mon Sep 17 00:00:00 2001
From: Lars Eggert <lars@eggert.org>
Date: Tue, 10 Sep 2024 16:39:50 +0200
Subject: [PATCH 40/52] Discard Initial keys after first Handshake packet is
 sent

---
 neqo-transport/src/connection/mod.rs | 21 +++++++++------------
 1 file changed, 9 insertions(+), 12 deletions(-)

diff --git a/neqo-transport/src/connection/mod.rs b/neqo-transport/src/connection/mod.rs
index 4e52f737ca..16cc6371ae 100644
--- a/neqo-transport/src/connection/mod.rs
+++ b/neqo-transport/src/connection/mod.rs
@@ -2422,13 +2422,15 @@ impl Connection {
                 self.loss_recovery.on_packet_sent(path, sent);
             }
 
-            if *space == PacketNumberSpace::Handshake
-                && self.role == Role::Server
-                && self.state == State::Confirmed
-            {
-                // We could discard handshake keys in set_state,
-                // but wait until after sending an ACK.
-                self.discard_keys(PacketNumberSpace::Handshake, now);
+            if *space == PacketNumberSpace::Handshake {
+                if self.role == Role::Server && self.state == State::Confirmed {
+                    // We could discard handshake keys in set_state,
+                    // but wait until after sending an ACK.
+                    self.discard_keys(PacketNumberSpace::Handshake, now);
+                } else if self.role == Role::Client {
+                    // We just sent a Handshake packet, so we can discard the Initial keys.
+                    self.discard_keys(PacketNumberSpace::Initial, now);
+                }
             }
         }
 
@@ -2779,11 +2781,6 @@ impl Connection {
                 self.set_initial_limits();
             }
             if self.crypto.install_keys(self.role)? {
-                if self.role == Role::Client {
-                    // We won't acknowledge Initial packets as a result of this, but the
-                    // server can rely on implicit acknowledgment.
-                    self.discard_keys(PacketNumberSpace::Initial, now);
-                }
                 self.saved_datagrams.make_available(CryptoSpace::Handshake);
             }
         }

From c2e20ffe18817028c7b9fc0cc7dd2ef392a2a207 Mon Sep 17 00:00:00 2001
From: Lars Eggert <lars@eggert.org>
Date: Wed, 11 Sep 2024 10:01:34 +0200
Subject: [PATCH 41/52] Revert

---
 neqo-transport/src/connection/mod.rs | 21 ++++++++++++---------
 1 file changed, 12 insertions(+), 9 deletions(-)

diff --git a/neqo-transport/src/connection/mod.rs b/neqo-transport/src/connection/mod.rs
index 16cc6371ae..4e52f737ca 100644
--- a/neqo-transport/src/connection/mod.rs
+++ b/neqo-transport/src/connection/mod.rs
@@ -2422,15 +2422,13 @@ impl Connection {
                 self.loss_recovery.on_packet_sent(path, sent);
             }
 
-            if *space == PacketNumberSpace::Handshake {
-                if self.role == Role::Server && self.state == State::Confirmed {
-                    // We could discard handshake keys in set_state,
-                    // but wait until after sending an ACK.
-                    self.discard_keys(PacketNumberSpace::Handshake, now);
-                } else if self.role == Role::Client {
-                    // We just sent a Handshake packet, so we can discard the Initial keys.
-                    self.discard_keys(PacketNumberSpace::Initial, now);
-                }
+            if *space == PacketNumberSpace::Handshake
+                && self.role == Role::Server
+                && self.state == State::Confirmed
+            {
+                // We could discard handshake keys in set_state,
+                // but wait until after sending an ACK.
+                self.discard_keys(PacketNumberSpace::Handshake, now);
             }
         }
 
@@ -2781,6 +2779,11 @@ impl Connection {
                 self.set_initial_limits();
             }
             if self.crypto.install_keys(self.role)? {
+                if self.role == Role::Client {
+                    // We won't acknowledge Initial packets as a result of this, but the
+                    // server can rely on implicit acknowledgment.
+                    self.discard_keys(PacketNumberSpace::Initial, now);
+                }
                 self.saved_datagrams.make_available(CryptoSpace::Handshake);
             }
         }

From ddabf5756c59dcb14448433b0d3a2c2df20b3844 Mon Sep 17 00:00:00 2001
From: Lars Eggert <lars@eggert.org>
Date: Wed, 11 Sep 2024 10:09:42 +0200
Subject: [PATCH 42/52] Fix?

---
 neqo-transport/src/recovery/mod.rs | 20 +++++++++-----------
 1 file changed, 9 insertions(+), 11 deletions(-)

diff --git a/neqo-transport/src/recovery/mod.rs b/neqo-transport/src/recovery/mod.rs
index 6be088e6e8..4c0e3c6753 100644
--- a/neqo-transport/src/recovery/mod.rs
+++ b/neqo-transport/src/recovery/mod.rs
@@ -829,18 +829,16 @@ impl LossRecovery {
         // The spaces in which we will allow probing.
         let mut allow_probes = PacketNumberSpaceSet::default();
         for pn_space in PacketNumberSpace::iter() {
-            if self
-                .pto_time(path.borrow().rtt(), *pn_space)
-                .map_or(true, |t| now < t)
-            {
-                continue;
+            if let Some(t) = self.pto_time(path.borrow().rtt(), *pn_space) {
+                allow_probes[*pn_space] = true;
+                if t <= now {
+                    qdebug!([self], "PTO timer fired for {}", pn_space);
+                    let space = self.spaces.get_mut(*pn_space).unwrap();
+                    lost.extend(space.pto_packets().cloned());
+
+                    pto_space = pto_space.or(Some(*pn_space));
+                }
             }
-            allow_probes[*pn_space] = true;
-            qdebug!([self], "PTO timer fired for {}", pn_space);
-            let space = self.spaces.get_mut(*pn_space).unwrap();
-            lost.extend(space.pto_packets().cloned());
-
-            pto_space = pto_space.or(Some(*pn_space));
         }
 
         // This has to happen outside the loop. Increasing the PTO count here causes the

From 67498389d7d8295aecd21380fd06cdf1c6dced72 Mon Sep 17 00:00:00 2001
From: Lars Eggert <lars@eggert.org>
Date: Wed, 11 Sep 2024 14:24:34 +0200
Subject: [PATCH 43/52] Remove unused fn

---
 neqo-transport/src/send_stream.rs | 6 ------
 1 file changed, 6 deletions(-)

diff --git a/neqo-transport/src/send_stream.rs b/neqo-transport/src/send_stream.rs
index de7ba78ec4..3f0002da13 100644
--- a/neqo-transport/src/send_stream.rs
+++ b/neqo-transport/src/send_stream.rs
@@ -1659,12 +1659,6 @@ impl SendStreams {
         }
     }
 
-    pub fn mark_all_as_lost(&mut self) {
-        for stream in self.map.values_mut() {
-            stream.mark_as_lost(0, usize::MAX, true);
-        }
-    }
-
     pub fn clear(&mut self) {
         self.map.clear();
         self.sendordered.clear();

From 8f4c566638ce408e6f8796f1cbd098d654518663 Mon Sep 17 00:00:00 2001
From: Lars Eggert <lars@eggert.org>
Date: Wed, 11 Sep 2024 15:23:20 +0200
Subject: [PATCH 44/52] ACK

---
 neqo-transport/src/tracking.rs | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/neqo-transport/src/tracking.rs b/neqo-transport/src/tracking.rs
index 1c6c777830..dfd4c0d737 100644
--- a/neqo-transport/src/tracking.rs
+++ b/neqo-transport/src/tracking.rs
@@ -287,7 +287,12 @@ impl RecvdPackets {
             ack_frequency_seqno: 0,
             ack_delay: DEFAULT_ACK_DELAY,
             unacknowledged_count: 0,
-            unacknowledged_tolerance: DEFAULT_ACK_PACKET_TOLERANCE,
+            unacknowledged_tolerance: if space == PacketNumberSpace::ApplicationData {
+                DEFAULT_ACK_PACKET_TOLERANCE
+            } else {
+                // ACK more aggressively
+                0
+            },
             ignore_order: false,
             ecn_count: EcnCount::default(),
         }

From 73f2929b9512e637753d02e8b77292f0584c1e02 Mon Sep 17 00:00:00 2001
From: Lars Eggert <lars@eggert.org>
Date: Wed, 11 Sep 2024 16:55:14 +0200
Subject: [PATCH 45/52] Maybe Handshake ping

---
 neqo-transport/src/connection/mod.rs | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/neqo-transport/src/connection/mod.rs b/neqo-transport/src/connection/mod.rs
index 4e52f737ca..63568e9bea 100644
--- a/neqo-transport/src/connection/mod.rs
+++ b/neqo-transport/src/connection/mod.rs
@@ -2783,6 +2783,9 @@ impl Connection {
                     // We won't acknowledge Initial packets as a result of this, but the
                     // server can rely on implicit acknowledgment.
                     self.discard_keys(PacketNumberSpace::Initial, now);
+                    // If the PTO was armed for the Initial space, also arm it for the Handshake
+                    // space, so we send probes there if needed.
+                    self.received_untracked = true;
                 }
                 self.saved_datagrams.make_available(CryptoSpace::Handshake);
             }

From af765b676a387d5aa0530a4572aad88195da9fd0 Mon Sep 17 00:00:00 2001
From: Lars Eggert <lars@eggert.org>
Date: Thu, 12 Sep 2024 16:38:55 +0200
Subject: [PATCH 46/52] picoquic bug

---
 .../src/connection/tests/zerortt.rs           | 160 +++++++++++++++++-
 1 file changed, 156 insertions(+), 4 deletions(-)

diff --git a/neqo-transport/src/connection/tests/zerortt.rs b/neqo-transport/src/connection/tests/zerortt.rs
index 0b7aa0eb41..5dfa49c32d 100644
--- a/neqo-transport/src/connection/tests/zerortt.rs
+++ b/neqo-transport/src/connection/tests/zerortt.rs
@@ -6,17 +6,25 @@
 
 use std::{cell::RefCell, rc::Rc, time::Duration};
 
-use neqo_common::{event::Provider, qdebug};
+use neqo_common::{event::Provider, qdebug, Datagram, Decoder, Role};
 use neqo_crypto::{AllowZeroRtt, AntiReplay};
-use test_fixture::{assertions, now};
+use test_fixture::{
+    assertions,
+    header_protection::{
+        apply_header_protection, decode_initial_header, initial_aead_and_hp,
+        remove_header_protection,
+    },
+    now, split_datagram,
+};
 
 use super::{
     super::Connection, connect, default_client, default_server, exchange_ticket, new_server,
     resumed_server, CountingConnectionIdGenerator,
 };
 use crate::{
-    events::ConnectionEvent, ConnectionParameters, Error, StreamType, Version,
-    MIN_INITIAL_PACKET_SIZE,
+    connection::tests::{new_client, DEFAULT_RTT},
+    events::ConnectionEvent,
+    ConnectionParameters, Error, StreamType, Version, MIN_INITIAL_PACKET_SIZE,
 };
 
 #[test]
@@ -320,3 +328,147 @@ fn zero_rtt_loss_accepted() {
         );
     }
 }
+
+#[test]
+#[allow(clippy::too_many_lines)]
+fn picoquic() {
+    const ACK_FRAME: &[u8] = &[3, 0, 0, 0, 0, 1, 0, 0];
+    const ACK_FRAME_2: &[u8] = &[3, 1, 0, 0, 1, 2, 0, 0];
+
+    let mut client = new_client(
+        ConnectionParameters::default()
+            .versions(Version::Version1, vec![Version::Version1])
+            .grease(false),
+    );
+    let mut server = new_server(ConnectionParameters::default().grease(false));
+    let mut now = now();
+    connect(&mut client, &mut server);
+
+    let token = exchange_ticket(&mut client, &mut server, now);
+    let mut client = new_client(
+        ConnectionParameters::default()
+            .versions(Version::Version1, vec![Version::Version1])
+            .grease(false),
+    );
+    client
+        .enable_resumption(now, token)
+        .expect("should set token");
+    let mut server = resumed_server(&client);
+
+    // Write 0-RTT before generating any packets.
+    // This should result in a datagram that coalesces Initial and 0-RTT.
+    let client_stream_id = client.stream_create(StreamType::UniDi).unwrap();
+    client.stream_send(client_stream_id, &[1, 2, 3]).unwrap();
+    let client_0rtt = client.process(None, now);
+    assert!(client_0rtt.as_dgram_ref().is_some());
+    assertions::assert_coalesced_0rtt(&client_0rtt.as_dgram_ref().unwrap()[..]);
+
+    let (_, client_dcid, _, _) =
+        decode_initial_header(client_0rtt.as_dgram_ref().unwrap(), Role::Client).unwrap();
+    let client_dcid = client_dcid.to_owned();
+
+    now += DEFAULT_RTT / 2;
+    let server_hs = server.process(client_0rtt.as_dgram_ref(), now);
+    assert!(server_hs.as_dgram_ref().is_some()); // Should produce ServerHello etc...
+
+    let server_stream_id = server
+        .events()
+        .find_map(|evt| match evt {
+            ConnectionEvent::NewStream { stream_id } => Some(stream_id),
+            _ => None,
+        })
+        .expect("should have received a new stream event");
+    assert_eq!(client_stream_id, server_stream_id.as_u64());
+
+    // Now, only deliver the ACK from the server's Intial packet.
+    let (server_initial, _server_hs) = split_datagram(server_hs.as_dgram_ref().unwrap());
+    let (protected_header, _, _, payload) =
+        decode_initial_header(&server_initial, Role::Server).unwrap();
+
+    // Now decrypt the packet.
+    let (aead, hp) = initial_aead_and_hp(&client_dcid, Role::Server);
+    let (header, pn) = remove_header_protection(&hp, protected_header, payload);
+    assert_eq!(pn, 0);
+    let pn_len = header.len() - protected_header.len();
+    let mut buf = vec![0; payload.len()];
+    let mut plaintext = aead
+        .decrypt(pn, &header, &payload[pn_len..], &mut buf)
+        .unwrap()
+        .to_owned();
+
+    // Now we need to find the frames.  Make some really strong assumptions.
+    let mut dec = Decoder::new(&plaintext[..]);
+    assert_eq!(dec.decode(ACK_FRAME.len()), Some(ACK_FRAME));
+    let end = dec.offset();
+
+    // Remove the CRYPTO frame.
+    plaintext[end..].fill(0);
+
+    // And rebuild a packet.
+    let mut packet = header.clone();
+    packet.resize(MIN_INITIAL_PACKET_SIZE, 0);
+    aead.encrypt(pn, &header, &plaintext, &mut packet[header.len()..])
+        .unwrap();
+    apply_header_protection(&hp, &mut packet, protected_header.len()..header.len());
+    let modified = Datagram::new(
+        server_initial.source(),
+        server_initial.destination(),
+        server_initial.tos(),
+        packet,
+    );
+
+    // Deliver the ACK and make the client RTX.
+    now += DEFAULT_RTT / 2;
+    now += client.process(Some(&modified), now).callback();
+    let client_out = client.process(None, now);
+
+    // The server should get the retransmission.
+    now += DEFAULT_RTT / 2;
+    let server_initial = server.process(client_out.as_dgram_ref(), now);
+    let (server_initial, _) = split_datagram(server_initial.as_dgram_ref().unwrap());
+
+    // Reorder the ACK and CRYPTO frames in the server's Initial packet.
+    let (protected_header, _, _, payload) =
+        decode_initial_header(&server_initial, Role::Server).unwrap();
+
+    // Now decrypt the packet.
+    let (aead, hp) = initial_aead_and_hp(&client_dcid, Role::Server);
+    let (header, pn) = remove_header_protection(&hp, protected_header, payload);
+    assert_eq!(pn, 1);
+    let pn_len = header.len() - protected_header.len();
+    let mut buf = vec![0; payload.len()];
+    let mut plaintext = aead
+        .decrypt(pn, &header, &payload[pn_len..], &mut buf)
+        .unwrap()
+        .to_owned();
+
+    // Now we need to find the frames.  Make some really strong assumptions.
+    let mut dec = Decoder::new(&plaintext[..]);
+    assert_eq!(dec.decode(ACK_FRAME_2.len()), Some(ACK_FRAME_2));
+    assert_eq!(dec.decode_varint(), Some(0x06)); // CRYPTO
+    assert_eq!(dec.decode_varint(), Some(0x00)); // offset
+    dec.skip_vvec(); // Skip over the payload.
+    let end = dec.offset();
+
+    // Move the ACK frame after the CRYPTO frame.
+    plaintext[..end].rotate_left(ACK_FRAME_2.len());
+
+    // And rebuild a packet.
+    let mut packet = header.clone();
+    packet.resize(MIN_INITIAL_PACKET_SIZE, 0);
+    aead.encrypt(pn, &header, &plaintext, &mut packet[header.len()..])
+        .unwrap();
+    apply_header_protection(&hp, &mut packet, protected_header.len()..header.len());
+    let modified = Datagram::new(
+        server_initial.source(),
+        server_initial.destination(),
+        server_initial.tos(),
+        packet,
+    );
+
+    // Deliver the server's Initial (ACK + CRYPTO) after a delay long enough to trigger the
+    // application space PTO.
+    now += DEFAULT_RTT * 5;
+    let probe = client.process(Some(&modified), now).dgram().unwrap();
+    assertions::assert_handshake(&probe[..]);
+}

From 8b3663d8ceb5ae677f9d0770480846702dda5825 Mon Sep 17 00:00:00 2001
From: Lars Eggert <lars@eggert.org>
Date: Fri, 13 Sep 2024 16:18:33 +0300
Subject: [PATCH 47/52] Merge

---
 neqo-transport/src/connection/mod.rs          | 32 ++++++++-------
 .../src/connection/tests/handshake.rs         | 39 ++++++++++---------
 .../src/connection/tests/recovery.rs          |  4 +-
 .../src/connection/tests/zerortt.rs           |  9 ++++-
 4 files changed, 47 insertions(+), 37 deletions(-)

diff --git a/neqo-transport/src/connection/mod.rs b/neqo-transport/src/connection/mod.rs
index 3f82ff4cc7..faafe88def 100644
--- a/neqo-transport/src/connection/mod.rs
+++ b/neqo-transport/src/connection/mod.rs
@@ -2419,13 +2419,15 @@ impl Connection {
                 self.loss_recovery.on_packet_sent(path, sent);
             }
 
-            if *space == PacketNumberSpace::Handshake
-                && self.role == Role::Server
-                && self.state == State::Confirmed
-            {
-                // We could discard handshake keys in set_state,
-                // but wait until after sending an ACK.
-                self.discard_keys(PacketNumberSpace::Handshake, now);
+            if *space == PacketNumberSpace::Handshake {
+                if self.role == Role::Client {
+                    // We're sending a Handshake packet, so we can discard Initial keys.
+                    self.discard_keys(PacketNumberSpace::Initial, now);
+                } else if self.role == Role::Server && self.state == State::Confirmed {
+                    // We could discard handshake keys in set_state,
+                    // but wait until after sending an ACK.
+                    self.discard_keys(PacketNumberSpace::Handshake, now);
+                }
             }
         }
 
@@ -2776,14 +2778,14 @@ impl Connection {
                 self.set_initial_limits();
             }
             if self.crypto.install_keys(self.role)? {
-                if self.role == Role::Client {
-                    // We won't acknowledge Initial packets as a result of this, but the
-                    // server can rely on implicit acknowledgment.
-                    self.discard_keys(PacketNumberSpace::Initial, now);
-                    // If the PTO was armed for the Initial space, also arm it for the Handshake
-                    // space, so we send probes there if needed.
-                    self.received_untracked = true;
-                }
+                // if self.role == Role::Client {
+                //     // We won't acknowledge Initial packets as a result of this, but the
+                //     // server can rely on implicit acknowledgment.
+                //     self.discard_keys(PacketNumberSpace::Initial, now);
+                //     // If the PTO was armed for the Initial space, also arm it for the Handshake
+                //     // space, so we send probes there if needed.
+                //     self.received_untracked = true;
+                // }
                 self.saved_datagrams.make_available(CryptoSpace::Handshake);
             }
         }
diff --git a/neqo-transport/src/connection/tests/handshake.rs b/neqo-transport/src/connection/tests/handshake.rs
index cf3fdf3fd2..026d77cc6f 100644
--- a/neqo-transport/src/connection/tests/handshake.rs
+++ b/neqo-transport/src/connection/tests/handshake.rs
@@ -591,8 +591,8 @@ fn reorder_1rtt() {
     now += RTT / 2;
     let s2 = server.process(c2.as_ref(), now).dgram();
     // The server has now received those packets, and saved them.
-    // The two additional are a Handshake and a 1-RTT (w/ NEW_CONNECTION_ID).
-    assert_eq!(server.stats().packets_rx, PACKETS * 2 + 4);
+    // The two additional are an Initial w/ACK, a Handshake w/ACK and a 1-RTT (w/ NEW_CONNECTION_ID).
+    assert_eq!(server.stats().packets_rx, PACKETS * 2 + 5);
     assert_eq!(server.stats().saved_datagrams, PACKETS);
     assert_eq!(server.stats().dropped_rx, 1);
     assert_eq!(*server.state(), State::Confirmed);
@@ -802,9 +802,9 @@ fn anti_amplification() {
     let ack = client.process(Some(&s_init3), now).dgram().unwrap();
     assert!(!maybe_authenticate(&mut client)); // No need yet.
 
-    // The client sends a padded datagram, with just ACK for Handshake.
-    assert_eq!(client.stats().frame_tx.ack, ack_count + 1);
-    assert_eq!(client.stats().frame_tx.all(), frame_count + 1);
+    // The client sends a padded datagram, with just ACKs for Initial and Handshake.
+    assert_eq!(client.stats().frame_tx.ack, ack_count + 2);
+    assert_eq!(client.stats().frame_tx.all(), frame_count + 2);
     assert_ne!(ack.len(), client.plpmtu()); // Not padded (it includes Handshake).
 
     now += DEFAULT_RTT / 2;
@@ -1047,29 +1047,28 @@ fn only_server_initial() {
     let server_dgram1 = server.process(client_dgram.as_ref(), now).dgram();
     let server_dgram2 = server.process_output(now + AT_LEAST_PTO).dgram();
 
-    // Only pass on the Initial from the first.  We should get a Handshake in return.
+    // Only pass on the Initial from the first.  We should get an ACK in return.
     let (initial, handshake) = split_datagram(&server_dgram1.unwrap());
     assert!(handshake.is_some());
 
-    // The client will not acknowledge the Initial as it discards keys.
-    // It sends a Handshake probe instead, containing just a PING frame.
-    assert_eq!(client.stats().frame_tx.ping, 0);
+    // The client sends an Initial ACK.
+    assert_eq!(client.stats().frame_tx.ack, 0);
     let probe = client.process(Some(&initial), now).dgram();
-    assertions::assert_handshake(&probe.unwrap());
+    assertions::assert_initial(&probe.unwrap(), false);
     assert_eq!(client.stats().dropped_rx, 0);
-    assert_eq!(client.stats().frame_tx.ping, 1);
+    assert_eq!(client.stats().frame_tx.ack, 1);
 
     let (initial, handshake) = split_datagram(&server_dgram2.unwrap());
     assert!(handshake.is_some());
 
-    // The same happens after a PTO, even though the client will discard the Initial packet.
+    // The same happens after a PTO.
     now += AT_LEAST_PTO;
-    assert_eq!(client.stats().frame_tx.ping, 1);
+    assert_eq!(client.stats().frame_tx.ack, 1);
     let discarded = client.stats().dropped_rx;
     let probe = client.process(Some(&initial), now).dgram();
-    assertions::assert_handshake(&probe.unwrap());
-    assert_eq!(client.stats().frame_tx.ping, 2);
-    assert_eq!(client.stats().dropped_rx, discarded + 1);
+    assertions::assert_initial(&probe.unwrap(), false);
+    assert_eq!(client.stats().frame_tx.ack, 2);
+    assert_eq!(client.stats().dropped_rx, discarded);
 
     // Pass the Handshake packet and complete the handshake.
     client.process_input(&handshake.unwrap(), now);
@@ -1136,7 +1135,9 @@ fn implicit_rtt_server() {
     let dgram = server.process(dgram.as_ref(), now).dgram();
     now += RTT / 2;
     let dgram = client.process(dgram.as_ref(), now).dgram();
-    assertions::assert_handshake(dgram.as_ref().unwrap());
+    let (initial, handshake) = split_datagram(dgram.as_ref().unwrap());
+    assertions::assert_initial(&initial, false);
+    assertions::assert_handshake(handshake.as_ref().unwrap());
     now += RTT / 2;
     server.process_input(&dgram.unwrap(), now);
 
@@ -1344,9 +1345,9 @@ fn client_handshake_retransmits_identical() {
             client.stats().frame_tx,
             FrameStats {
                 crypto: i + 1,
-                ack: i,
+                ack: i + 1,
                 new_connection_id: i * 7,
-                all: i * 9 + 1,
+                all: i * 9 + 2,
                 ..Default::default()
             }
         );
diff --git a/neqo-transport/src/connection/tests/recovery.rs b/neqo-transport/src/connection/tests/recovery.rs
index 134d120521..f19e9a2573 100644
--- a/neqo-transport/src/connection/tests/recovery.rs
+++ b/neqo-transport/src/connection/tests/recovery.rs
@@ -226,7 +226,9 @@ fn pto_handshake_complete() {
 
     now += HALF_RTT;
     let pkt = client.process(pkt.as_ref(), now).dgram();
-    assert_handshake(pkt.as_ref().unwrap());
+    let (initial, handshake) = split_datagram(&pkt.clone().unwrap());
+    assert_initial(&initial, false);
+    assert_handshake(handshake.as_ref().unwrap());
 
     let cb = client.process(None, now).callback();
     // The client now has a single RTT estimate (20ms), so
diff --git a/neqo-transport/src/connection/tests/zerortt.rs b/neqo-transport/src/connection/tests/zerortt.rs
index ece8c76d4c..d9edd6c514 100644
--- a/neqo-transport/src/connection/tests/zerortt.rs
+++ b/neqo-transport/src/connection/tests/zerortt.rs
@@ -340,7 +340,7 @@ fn picoquic() {
             .versions(Version::Version1, vec![Version::Version1])
             .grease(false),
     );
-    let mut server = new_server(ConnectionParameters::default().grease(false));
+    let mut server = default_server();
     let mut now = now();
     connect(&mut client, &mut server);
 
@@ -470,5 +470,10 @@ fn picoquic() {
     // application space PTO.
     now += DEFAULT_RTT * 5;
     let probe = client.process(Some(&modified), now).dgram().unwrap();
-    assertions::assert_handshake(&probe[..]);
+    assertions::assert_initial(&probe[..], true);
+
+    now += client.process(None, now).callback();
+    let probe = client.process(None, now).dgram().unwrap();
+    assertions::assert_initial(&probe[..], true);
+    assertions::assert_coalesced_0rtt(&probe[..]);
 }

From 230714d5864583c85e028ad0345d4370b0a0a36e Mon Sep 17 00:00:00 2001
From: Lars Eggert <lars@eggert.org>
Date: Fri, 13 Sep 2024 16:20:38 +0300
Subject: [PATCH 48/52] Merge

---
 neqo-transport/src/connection/tests/handshake.rs | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/neqo-transport/src/connection/tests/handshake.rs b/neqo-transport/src/connection/tests/handshake.rs
index 026d77cc6f..3296aa6218 100644
--- a/neqo-transport/src/connection/tests/handshake.rs
+++ b/neqo-transport/src/connection/tests/handshake.rs
@@ -591,7 +591,8 @@ fn reorder_1rtt() {
     now += RTT / 2;
     let s2 = server.process(c2.as_ref(), now).dgram();
     // The server has now received those packets, and saved them.
-    // The two additional are an Initial w/ACK, a Handshake w/ACK and a 1-RTT (w/ NEW_CONNECTION_ID).
+    // The two additional are an Initial w/ACK, a Handshake w/ACK and a 1-RTT (w/
+    // NEW_CONNECTION_ID).
     assert_eq!(server.stats().packets_rx, PACKETS * 2 + 5);
     assert_eq!(server.stats().saved_datagrams, PACKETS);
     assert_eq!(server.stats().dropped_rx, 1);
@@ -1253,7 +1254,7 @@ fn client_triggered_zerortt_retransmits_identical() {
     // Passing only the server handshake packet to the client should trigger a retransmit.
     _ = client.process(s_hs.as_ref(), now()).dgram();
     let stats2 = client.stats().frame_tx;
-    assert_eq!(stats2.all, stats1.all * 2);
+    assert_eq!(stats2.all(), stats1.all() * 2);
     assert_eq!(stats2.crypto, stats1.crypto * 2);
     assert_eq!(stats2.stream, stats1.stream * 2);
 }
@@ -1314,7 +1315,7 @@ fn server_triggered_initial_retransmits_identical() {
     let si2 = server.process(ci.as_ref(), now);
     let stats2 = server.stats().frame_tx;
     assert_eq!(si1.dgram().unwrap().len(), si2.dgram().unwrap().len());
-    assert_eq!(stats2.all, stats1.all * 2);
+    assert_eq!(stats2.all(), stats1.all() * 2);
     assert_eq!(stats2.crypto, stats1.crypto * 2);
     assert_eq!(stats2.ack, stats1.ack * 2);
 }
@@ -1347,7 +1348,6 @@ fn client_handshake_retransmits_identical() {
                 crypto: i + 1,
                 ack: i + 1,
                 new_connection_id: i * 7,
-                all: i * 9 + 2,
                 ..Default::default()
             }
         );

From 8fd763e6fd577aff080c6a3f0d43c1b21d2d530e Mon Sep 17 00:00:00 2001
From: Lars Eggert <lars@eggert.org>
Date: Fri, 13 Sep 2024 16:29:51 +0300
Subject: [PATCH 49/52] Rename test and turn off grease

---
 neqo-transport/src/connection/tests/zerortt.rs | 8 ++++++--
 1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/neqo-transport/src/connection/tests/zerortt.rs b/neqo-transport/src/connection/tests/zerortt.rs
index d9edd6c514..c34e1e673b 100644
--- a/neqo-transport/src/connection/tests/zerortt.rs
+++ b/neqo-transport/src/connection/tests/zerortt.rs
@@ -331,7 +331,7 @@ fn zero_rtt_loss_accepted() {
 
 #[test]
 #[allow(clippy::too_many_lines)]
-fn picoquic() {
+fn zerortt_reorder_frames() {
     const ACK_FRAME: &[u8] = &[3, 0, 0, 0, 0, 1, 0, 0];
     const ACK_FRAME_2: &[u8] = &[3, 1, 0, 0, 1, 2, 0, 0];
 
@@ -340,7 +340,11 @@ fn picoquic() {
             .versions(Version::Version1, vec![Version::Version1])
             .grease(false),
     );
-    let mut server = default_server();
+    let mut server = new_server(
+        ConnectionParameters::default()
+            .versions(Version::Version1, vec![Version::Version1])
+            .grease(false),
+    );
     let mut now = now();
     connect(&mut client, &mut server);
 

From 0385874897d56d1016cc4251e33eee1f20751a88 Mon Sep 17 00:00:00 2001
From: Lars Eggert <lars@eggert.org>
Date: Fri, 13 Sep 2024 17:06:27 +0300
Subject: [PATCH 50/52] Cleanups

---
 neqo-transport/src/connection/mod.rs   | 8 --------
 neqo-transport/src/connection/saved.rs | 4 ++--
 2 files changed, 2 insertions(+), 10 deletions(-)

diff --git a/neqo-transport/src/connection/mod.rs b/neqo-transport/src/connection/mod.rs
index faafe88def..8d8110a4e5 100644
--- a/neqo-transport/src/connection/mod.rs
+++ b/neqo-transport/src/connection/mod.rs
@@ -2778,14 +2778,6 @@ impl Connection {
                 self.set_initial_limits();
             }
             if self.crypto.install_keys(self.role)? {
-                // if self.role == Role::Client {
-                //     // We won't acknowledge Initial packets as a result of this, but the
-                //     // server can rely on implicit acknowledgment.
-                //     self.discard_keys(PacketNumberSpace::Initial, now);
-                //     // If the PTO was armed for the Initial space, also arm it for the Handshake
-                //     // space, so we send probes there if needed.
-                //     self.received_untracked = true;
-                // }
                 self.saved_datagrams.make_available(CryptoSpace::Handshake);
             }
         }
diff --git a/neqo-transport/src/connection/saved.rs b/neqo-transport/src/connection/saved.rs
index fb8cba2227..d5e0313e52 100644
--- a/neqo-transport/src/connection/saved.rs
+++ b/neqo-transport/src/connection/saved.rs
@@ -41,10 +41,10 @@ impl SavedDatagrams {
         let store = self.store(cspace);
 
         if store.len() < MAX_SAVED_DATAGRAMS {
-            qdebug!("saving {:?} datagram of {} bytes", cspace, d.len());
+            qdebug!("saving datagram of {} bytes", d.len());
             store.push(SavedDatagram { d, t });
         } else {
-            qinfo!("not saving {:?} datagram of {} bytes", cspace, d.len());
+            qinfo!("not saving datagram of {} bytes", d.len());
         }
     }
 

From 70049064cf52cbc0d012c21c809a4b7533140776 Mon Sep 17 00:00:00 2001
From: Lars Eggert <lars@eggert.org>
Date: Mon, 16 Sep 2024 12:39:43 +0300
Subject: [PATCH 51/52] Minimize diff

---
 neqo-transport/src/connection/mod.rs | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/neqo-transport/src/connection/mod.rs b/neqo-transport/src/connection/mod.rs
index 8d8110a4e5..a0d9977369 100644
--- a/neqo-transport/src/connection/mod.rs
+++ b/neqo-transport/src/connection/mod.rs
@@ -730,25 +730,25 @@ impl Connection {
         let version = Version::try_from(u32::try_from(
             dec.decode_uint(4).ok_or(Error::InvalidResumptionToken)?,
         )?)?;
-        qdebug!([self], "  version {:?}", version);
+        qtrace!([self], "  version {:?}", version);
         if !self.conn_params.get_versions().all().contains(&version) {
             return Err(Error::DisabledVersion);
         }
 
         let rtt = Duration::from_millis(dec.decode_varint().ok_or(Error::InvalidResumptionToken)?);
-        qdebug!([self], "  RTT {:?}", rtt);
+        qtrace!([self], "  RTT {:?}", rtt);
 
         let tp_slice = dec.decode_vvec().ok_or(Error::InvalidResumptionToken)?;
-        qdebug!([self], "  transport parameters {}", hex(tp_slice));
+        qtrace!([self], "  transport parameters {}", hex(tp_slice));
         let mut dec_tp = Decoder::from(tp_slice);
         let tp =
             TransportParameters::decode(&mut dec_tp).map_err(|_| Error::InvalidResumptionToken)?;
 
         let init_token = dec.decode_vvec().ok_or(Error::InvalidResumptionToken)?;
-        qdebug!([self], "  Initial token {}", hex(init_token));
+        qtrace!([self], "  Initial token {}", hex(init_token));
 
         let tok = dec.decode_remainder();
-        qdebug!([self], "  TLS token {}", hex(tok));
+        qtrace!([self], "  TLS token {}", hex(tok));
 
         match self.crypto.tls {
             Agent::Client(ref mut c) => {

From d94af34bba52ef707bdbd97846024a395331669d Mon Sep 17 00:00:00 2001
From: Lars Eggert <lars@eggert.org>
Date: Wed, 18 Sep 2024 16:49:55 +0300
Subject: [PATCH 52/52] Merge fixes

---
 neqo-transport/src/recovery/mod.rs | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/neqo-transport/src/recovery/mod.rs b/neqo-transport/src/recovery/mod.rs
index 83eb3f80d0..4a41ccb983 100644
--- a/neqo-transport/src/recovery/mod.rs
+++ b/neqo-transport/src/recovery/mod.rs
@@ -877,14 +877,14 @@ impl LossRecovery {
             let pto = Self::pto_period_inner(
                 primary_path.borrow().rtt(),
                 self.pto_state.as_ref(),
-                space.space,
+                confirmed,
                 self.fast_pto,
             );
             space.detect_lost_packets(now, loss_delay, pto, &mut lost_packets);
 
             primary_path.borrow_mut().on_packets_lost(
                 space.largest_acked_sent_time,
-                space.space,
+                confirmed,
                 &lost_packets[first..],
                 &mut self.stats.borrow_mut(),
                 now,