Replace recovery epoch with congestion event #2585
Conversation
Recovery epoch was undefined and congestion event makes more sense for QUIC.
ianswett
added
editorial
draft-ietf-quic-recovery.md
Outdated
| @@ -1319,7 +1319,7 @@ window. | |||
| ~~~ | |||
| CongestionEvent(sent_time): | |||
| // Start a new congestion event if the sent time is larger | |||
| // than the start time of the previous recovery epoch. | |||
| // than the start time of the previous congestion event. | |||
There was a problem hiding this comment.
Suggestion: "... if the packet was sent after the start of the last congestion event."
| // Start a new congestion event if the sent time is larger | ||
| // than the start time of the previous recovery epoch. | ||
| // Start a new congestion event if packet was sent after the | ||
| // start of the previous congestion event. |
There was a problem hiding this comment.
I understand what the start and the end of an epoch are, but what is the start and end of a congestion event.
In my understanding, a congestion event is when a packet is acknowledged or when a packet is declared lost, and there’s no start and end to it.
There was a problem hiding this comment.
Good point. We don't use epoch anywhere else in this doc except this pseudocode, and it's different from the definition in security contexts, so I'd like to avoid the term.
Do you have any other suggestions? Congestion Period, etc? I'm not even that happy about using InRecovery here, since in TCP that has a distinct meaning that doesn't map 1:1 to QUIC.
There was a problem hiding this comment.
Maybe keep congestion event as the trigger. Isn't the period referred to as the recovery period, if entered.
There was a problem hiding this comment.
Well, seems like you just made a commit doing exactly that ...
There was a problem hiding this comment.
I did, we keep cross-posting.
|
Events tend to be points in time, I agree. |
Addresses follow-up comments on #2585
* Use Congestion Recovery Period in psuedocode Addresses follow-up comments on #2585 * Update draft-ietf-quic-recovery.md * Remove duplicate comments
Recovery epoch was undefined and congestion event makes more sense for QUIC.
Fixes #2566