tsvwg / careful-resume

Careful Resume
1 stars 5 forks source link

Qlog related changes #39

Closed gorryfair closed 2 months ago

ana-cc commented 2 months ago

This reads better, but I am now wondering about the case where flightsize < pipesize and CR transitions to Normal phase on exit from Unvalidated. We don't define a trigger for this phase change.

gorryfair commented 2 months ago

I think this is a case that we discovered in unit tests in our fork of Quiche. It's not likely to be super-common, because the sender ought to have had more data to send, but it can happen. If you'd like to propose a trigger and a little text, please go ahead. I'll plan to merge this sometime today.

ana-cc commented 2 months ago

Fix one last NiT:

-       Because CWND will now now be less than ssthresh, a sender in the Normal Phase is permitted to use
+       Because CWND will now be less than ssthresh, a sender in the Normal Phase is permitted to use

Text around the new trigger:

-         (Note 1: When flight_size is less than or equal to the PipeSize (the validated window), there is no need to validate the data in flight, CWND is reset to the PipeSize and the Normal phase is immediately entered).
+         (Note 1: When flight_size is less than or equal to the PipeSize (the validated window), there is no need to validate the data in flight, CWND is reset to the PipeSize and the Normal phase is immediately entered - see trigger application_limited in <xref target="sec-QLOG"></xref>).

And in the qlog definition:

+        ; for the Normal phase, when the application has fewer unvalidated packets to send compared to its previous cwnd
+        application_limited" /
        ; for the Safe Retreat phase, when loss detected
gorryfair commented 2 months ago

Applictaion_limited became rate_limited, but understand. I will resolve conflicts and merge.

ana-cc commented 2 months ago

Thanks! Looks great now. One minor thing, I am still seeing the duplicated word 'now' in: "Because CWND will now now be less than ssthresh, [...]"