fix(s2n-quic-transport): arm PTO timer after loss timer expires

[WesleyRosenblum]

Description of changes:

QUIC-RECOVERY does not explicitly include a requirement that the Probe Timeout (PTO) timer is re-armed after the loss timer expires, but the pseudocode in QUIC-RECOVERY§A.9 does include this behavior:

OnLossDetectionTimeout():
  earliest_loss_time, pn_space = GetLossTimeAndSpace()
  if (earliest_loss_time != 0):
    // Time threshold loss Detection
    lost_packets = DetectAndRemoveLostPackets(pn_space)
    assert(!lost_packets.empty())
    OnPacketsLost(lost_packets)
    SetLossDetectionTimer()    <---------- re-arms the PTO timer
    return

Arming the PTO timer in this case is necessary, as the loss timer expiring will not necessarily cause any additional transmissions. There may still be tail packets pending; the PTO timer is needed to ensure those packets are accounted for and retransmitted as necessary. An example interop failure caused by this can be seen here

In addition, QUIC-RECOVERY§6.2.1 specifies that the acknowledgement of ack-eliciting packets should trigger a PTO update:

A sender SHOULD restart its PTO timer every time an ack-eliciting packet is sent or acknowledged,

The pseudocode in QUIC-RECOVERY§A.7 does not make that distinction, however. This behavior is preferred, as detect_and_remove_lost_packets() will cancel the loss timer, and there may still be ack eliciting packets pending that require a PTO timer for recovery. I've removed the requirement that an acknowledged packet be ack-eliciting to more closely match the pseudocode.

Call-outs:

I refactored the max pto backoff a bit to make it easier to validate the consistency of the PTO timer within the recovery manager.

Testing:

New and updated unit tests

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

[camshaft]

Ideally we'd be checking consistency in any of these return points. Maybe move the whole thing into a closure?

(|| {
  // do logic here
})();
self.check_consistency();

especially where we're cancelling the timer it's good to have a sanity check that the invariants still hold

[camshaft]

nit: might be easier to debug if you use assert_ne since that'll print the actual value

Suggested change

	assert!(self.armed_timer_count() > 0);
	assert_ne!(self.armed_timer_count(), 0);