Re: [quicwg/base-drafts] Rewrite of RTT estimation section (#2592)

[Martin Thomson <[e-mail]>]

martinthomson approved this pull request.

This is a big improvement over what we had.  Nice work.

> @@ -229,19 +229,23 @@ forward progress without relying on timeouts.
 ### Explicit Correction For Delayed Acknowledgements {#ack-delay}
 
 An endpoint measures the delay incurred between when a packet is received and
-when the corresponding acknowledgment is sent.  The endpoint encodes this host delay for
-the largest acknowledged packet in the Ack Delay field of an ACK frame (see
-Section 19.3 of {{QUIC-TRANSPORT}}).  This allows the receiver of the ACK to
-adjust for any host delays - importantly, for delayed acknowledgements - when
-estimating the path RTT.  Where possible, this delay can include the time that a
-packet was held in the OS kernel before being processed by a userspace QUIC
-stack.
+when the corresponding acknowledgment is sent.  The endpoint encodes this host
+delay for the largest acknowledged packet in the Ack Delay field of an ACK frame
+(see Section 19.3 of {{QUIC-TRANSPORT}}).  This allows the receiver of the ACK
+to adjust for any host delays - importantly, for delayed acknowledgements - when
+estimating the path RTT.  In certain deployments, a packet might be held in the
+OS kernel or elsewhere on the host before being processed by the QUIC
+stack. Where possible, an endpoint SHOULD include these delays when populating
+the Ack Delay field in an ACK frame.

Probably not for this change, but do we really want to encourage people to be estimating delays in the kernel?  Because it seems like the point of some of the other changes is to include that sort of delay in the RTT estimate.  What matters here is end-to-end delays - the time between when the packet was consumed and when the packet containing the ACK was sent out.  Yes, it's true that you get a bigger RTT estimate if you don't correct for delays in the kernel or NIC, but those are just another piece of the plumbing now.  If we fail to cover things like scheduling delays in the RTT estimate, we're just deluding ourselves about where the delays are.

>  
 An endpoint MUST NOT excessively delay acknowledgements of ack-eliciting
-packets. Specifically, an endpoint is expected to enforce a maximum delay to
+packets.  Specifically, an endpoint is expected to enforce a maximum delay to
 avoid causing spurious timeouts at the peer.  The maximum ack delay is

The text doesn't say what delay is being "enforced".  Maybe:

>[...] an endpoint is expected to send acknowledgments within its advertised delay to avoid causing spurious timeouts at its peer.

But maybe you can remove this sentence now that the "contract" text is here.


>  if (min_rtt + ack_delay < latest_rtt):
-  latest_rtt = latest_rtt - ack_delay
-smoothed_rtt = 7/8 * smoothed_rtt + 1/8 * latest_rtt
-rttvar_sample = abs(smoothed_rtt - latest_rtt)
+  corrected_rtt = latest_rtt - ack_delay

Maybe `adjusted_rtt` instead.  "Corrected" implies that there is some notion of "correctness" that applies.

-- 
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/quicwg/base-drafts/pull/2592#pullrequestreview-224158497