fix: Make neqo pass amplificationlimit QNS test (#1875)

* fix: Make neqo pass `amplificationlimit` QNS test

Fixes #1183

* Fix some tests

* Update neqo-transport/src/connection/tests/handshake.rs

Co-authored-by: Martin Thomson <mt@lowentropy.net>
Signed-off-by: Lars Eggert <lars@eggert.org>

* Address code review

* Fix idle_timeout_crazy_rtt

* Clarify test

* Restore

* Hopefully, a fix

* Nit

* Tweak

* Fix tests

* Fix

* Minimize diff

* Minimize more

* Fix?

* Fix?

* Fix?

* Deal with cancelled runs

* Try

* Roll back

* Again

* fmt

* Suggestion from @martinthomson

---------

Signed-off-by: Lars Eggert <lars@eggert.org>
Co-authored-by: Martin Thomson <mt@lowentropy.net>

neqo-transport/src/connection/mod.rs

@@ -1040,7 +1040,7 @@ impl Connection {
             qtrace!([self], "Idle/keepalive timer {:?}", idle_time);
             delays.push(idle_time);
 
-            if let Some(lr_time) = self.loss_recovery.next_timeout(rtt) {
+            if let Some(lr_time) = self.loss_recovery.next_timeout(&path) {
                 qtrace!([self], "Loss recovery timer {:?}", lr_time);
                 delays.push(lr_time);
             }
@@ -1664,7 +1664,9 @@ impl Connection {
                     .clone(),
             ),
         );
-        path.borrow_mut().set_valid(now);
+        if self.role == Role::Client {
+            path.borrow_mut().set_valid(now);
+        }
     }
 
     /// If the path isn't permanent, assign it a connection ID to make it so.
@@ -2375,7 +2377,7 @@ impl Connection {
         }
 
         if encoder.is_empty() {
-            qdebug!("TX blocked, profile={:?} ", profile);
+            qdebug!("TX blocked, profile={:?}", profile);
             Ok(SendOption::No(profile.paced()))
         } else {
             // Perform additional padding for Initial packets as necessary.

neqo-transport/src/connection/tests/handshake.rs

@@ -783,19 +783,15 @@ fn anti_amplification() {
     let c_init = client.process_output(now).dgram();
     now += DEFAULT_RTT / 2;
     let s_init1 = server.process(c_init.as_ref(), now).dgram().unwrap();
-    assert_eq!(s_init1.len(), client.plpmtu());
+    assert_eq!(s_init1.len(), server.plpmtu());
     let s_init2 = server.process_output(now).dgram().unwrap();
     assert_eq!(s_init2.len(), server.plpmtu());
-
-    // Skip the gap for pacing here.
-    let s_pacing = server.process_output(now).callback();
-    assert_ne!(s_pacing, Duration::new(0, 0));
-    now += s_pacing;
-
     let s_init3 = server.process_output(now).dgram().unwrap();
     assert_eq!(s_init3.len(), server.plpmtu());
     let cb = server.process_output(now).callback();
-    assert_ne!(cb, Duration::new(0, 0));
+
+    // We are blocked by the amplification limit now.
+    assert_eq!(cb, server.conn_params.get_idle_timeout());
 
     now += DEFAULT_RTT / 2;
     client.process_input(&s_init1, now);

neqo-transport/src/connection/tests/recovery.rs

@@ -584,6 +584,10 @@ fn loss_time_past_largest_acked() {
     let s_hs3 = server.process(None, now + s_pto).dgram();
     assert!(s_hs3.is_some());
 
+    // We are blocked by the amplification limit now.
+    let cb = server.process_output(now).callback();
+    assert_eq!(cb, server.conn_params.get_idle_timeout());
+
     // Get some Handshake packets from the client.
     // We need one to be left unacknowledged before one that is acknowledged.
     // So that the client engages the loss recovery timer.

neqo-transport/src/path.rs

@@ -1016,6 +1016,14 @@ impl Path {
         }
     }
 
+    /// 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).
+    pub fn pto_possible(&self) -> bool {
+        // See the implementation of `amplification_limit` for details.
+        self.amplification_limit() >= self.plpmtu()
+    }
+
     /// Get the number of bytes that can be written to this path.
     pub fn amplification_limit(&self) -> usize {
         if matches!(self.state, ProbeState::Failed) {

neqo-transport/src/recovery/mod.rs

@@ -735,9 +735,14 @@ impl LossRecovery {
 
     /// Calculate when the next timeout is likely to be.  This is the earlier of the loss timer
     /// and the PTO timer; either or both might be disabled, so this can return `None`.
-    pub fn next_timeout(&self, rtt: &RttEstimate) -> Option<Instant> {
+    pub fn next_timeout(&self, path: &Path) -> Option<Instant> {
+        let rtt = path.rtt();
         let loss_time = self.earliest_loss_time(rtt);
-        let pto_time = self.earliest_pto(rtt);
+        let pto_time = if path.pto_possible() {
+            self.earliest_pto(rtt)
+        } else {
+            None
+        };
         qtrace!(
             [self],
             "next_timeout loss={:?} pto={:?}",
@@ -1013,7 +1018,7 @@ mod tests {
         }
 
         pub fn next_timeout(&self) -> Option<Instant> {
-            self.lr.next_timeout(self.path.borrow().rtt())
+            self.lr.next_timeout(&self.path.borrow())
         }
 
         pub fn discard(&mut self, space: PacketNumberSpace, now: Instant) {