Out of order packets cause spurious retransmit

[mb]

While debugging upload for Bug 1852924 I noticed that all the detected losses during my testing are spurious. An ack arrives shortly after. The loss detection seems to trigger after an out-of-order packet E with slightly shorter RTT. Probably because we update the last_rtt and use it to determine whether the packets before the packet E were lost. Packets before E were send earlier, are therefore in flight for longer and are all marked as lost.

I will investigate further, but wanted to note down this observation.

[mb]

This is the kind of packet loss I see: In this graph you can see the calls to cc_on_packet_acked on_packets_lost on_packet_sent graphed over time by each packet number. All the lost packets receive an ack slightly later (around 0.35ms). In this example we even sent out a recovery packet (=> still need to investigate what the conditions are when we don't sent out recovery packets #1473)

In this example it is probably the PACKET_THRESHOLD = 3[1][2] (from https://datatracker.ietf.org/doc/html/rfc9002#name-packet-threshold) causes the packets to be marked as lost.

[mb]

But Acks can arrive even later than $\frac{17}{8} \text{rtt}$: This is without packet number detection and with $\text{kTimeThreshold} = \frac{17}{8} \text{rtt}$: But the upload speed is fast for me (not quiet yet, but almost similar to http2) with both these parameters tuned.
Here you can see a few acks taking with a little bit longer than 2rtt.

[mb]

For this issue I see two ways forward:

1. simple: don't use packet reordering as a mean to detect loss and solely rely on time threshold https://datatracker.ietf.org/doc/html/rfc9002#name-time-threshold
2. probably better solution: use RACK, the recommended algorithm to increase the threshold on spuriously detected losses. Recommended in the Packet threshold section
   I think we want to go with (2), because it is a more general solution. We might also want to increase the kTimeThreshold on spurious detection. I'll have a read on the RFC and try to write a patch integrating the algorithm.

[mb]

Notes reading through RACK:

• 1.2 Motivation mentions

  If the reordering degree is beyond DupThresh, DupAck counting can cause a spurious fast recovery and unnecessary congestion window reduction. To mitigate the issue, Non-Congestion Robustness (NCR) for TCP [RFC4653] increases the DupThresh from the current fixed value of three duplicate ACKs [RFC5681] to approximate a congestion window of data having left the network.

  This could be an intermediate solution until we implement RACK. It looks like it would fix the spurious loss detection, but RACK might provide an even better algorithm.

• 3.3.1. Reordering Design Rationale clears up, why this approach is insufficient. Also shows how RACK differs:

  Specifically, RACK-TLP introduces a new dynamic reordering window parameter in time units, and the sender considers a data segment S lost if both of these conditions are met:
  (1) Another data segment sent later than S has been delivered.
  (2) S has not been delivered after the estimated round-trip time plus the reordering window.

  Note that condition (1) implies at least one round trip of time has elapsed since S has been sent.

  After reading this, I think RACK makes a good point, is easy enough to implement and will probably fix the issue. Writing tests for this on the other hand will probably take the longest time. I'd prefer if (1) was (1) Another data segment, sent after receiving the SACK, has been delivered. to cover the reordering observed in comment 3.

• 3.3.2. Reordering Window Adaptation One problem I have with this section:

  The RACK reordering window adapts to the measured duration of reordering events within reasonable and specific bounds to disincentivize excessive reordering.

  We do expect more reordering events to happen with QUIC than with TCP due to packet number being encrypted, therefore I think making slight adjustments that are only there to disincentivize excessive reordering can be loosened. The network can't fix reordering events with QUIC the way it does with TCP.

[mb]

@martinthomson What do you think about adopting RACK to fix this issue? As far as I can tell this has the highest impact to our upload speed problem.

With #1475 and #1478 being secondary important to increase the cwnd fast enough after a loss event.

[martinthomson]

I've always wanted to implement RACK, we just never really had time.