Don't update RTT for ack-only packets #984
Conversation
|
I don't think ack-only packets and acks for non-retransmittable packets are necessarily the same. |
draft-ietf-quic-recovery.md
Outdated
| When an ack is received, it may acknowledge 0 or more packets. | ||
| When an acknowledgment is received, it may acknowledge 0 or more packets. | ||
|
|
||
| The first acknowledgment for the largest acknowledged packet is used to update |
There was a problem hiding this comment.
nit: "first acknowledgement received for", just to clarify that it's the first one we've seen, not the first one sent.
draft-ietf-quic-recovery.md
Outdated
| The first acknowledgment for the largest acknowledged packet is used to update | ||
| the estimate of minimum and smoothed RTT. However, if the largest acknowledged | ||
| packet would not have caused an acknowledgment to be sent, RTT estimates are not | ||
| updated, because the acknowledgment could be significantly delayed. |
There was a problem hiding this comment.
Use normative language: "... RTT estimates MUST NOT be updated..."
draft-ietf-quic-recovery.md
Outdated
| // If the largest acked is newly acked and is a packet | ||
| // that would normally cause an ack, update the RTT. | ||
| if (sent_packets[ack.largest_acked] and | ||
| not ack_only(sent_packets[ack.largest_acked])): |
There was a problem hiding this comment.
We use "ackable" as a term in this doc to refer to a packet that triggers an ack. I just realized that it's not yet defined in the doc, but I would use that term ("and is_ackable(sent_packets[ack.largest_acked])"). I'm happy to write up a definition for "ackable" in a PR.
Alternatively, you can leave this as you have it, and I can change it when I actually define the term. Up to you.
|
@ianswett, I was operating on the assumption that a long ACK delay would reduce the accuracy of the RTT estimate. I agree that this might be too blunt an instrument though. I'll let you sort it out. |
|
Closed by #991. |
No description provided.