search

quinn-rs / quinn

Async-friendly QUIC implementation in Rust
Apache License 2.0
3.59k stars 367 forks source link

Consider detecting spurious retransmits #447

Open Ralith opened 4 years ago

Ralith commented 4 years ago

A number of implementations are keeping track of packets deemed lost for some period after, so that an extremely delayed ACK can still be processed correctly, resulting in in-flight retransmissions of the newly acknowledged being abandoned. This wouldn't be trivial for us to support, since even if we keep track of what was in lost packets we don't currently have a very satisfying way to abandon an already in-flight retransmit of it (which may be mixed in with newer data). A better understanding of the benefits might help motivate this, along with some research into the data structures that make it easier to accomplish.

Ralith commented 3 years ago

This is specifically recommended as of draft 30.

Ralith commented 3 years ago

https://github.com/NTAP/rfc8312bis/issues/45#issuecomment-785515567 provides a nice summary of how to approach this. In short, we'd want to add a retransmitted_packets list to PacketSpace similar to the existing sent_packets list, and if any packet in retransmitted_packets is ACKed, notify the congestion controller of a spurious retransmit. The maximum length of retransmitted_packets should be bounded, with old entries getting dropped.

There's probably no need to bother with the complexity of dropping already in-flight retransmits; keeping the congestion controller updated is enough.