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

[Jana Iyengar <[e-mail]>]

janaiyengar commented on this pull request.

Thanks, Ian -- updated.

>  
 
-# Generating Acknowledgements
-
-QUIC SHOULD delay sending acknowledgements in response to packets, but MUST NOT
-excessively delay acknowledgements of ack-eliciting packets. Specifically,
-implementations MUST attempt to enforce a maximum ack delay to avoid causing
-the peer spurious timeouts.  The maximum ack delay is communicated in the
-`max_ack_delay` transport parameter and the default value is 25ms.
+# Generating Acknowledgements {#generating-acks}

next PR in progress

> -RTT when multiple such ACK frames are received within an RTT.  When multiple
-samples are generated within an RTT, the smoothed RTT and RTT variance could
-retain inadequate history, as suggested in {{?RFC6298}}. Changing these
-computations is currently an open research question.
-
-min_rtt is the minimum RTT measured over the connection, prior to adjusting by
-ack delay.  Ignoring ack delay for min RTT prevents intentional or unintentional
-underestimation of min RTT, which in turn prevents underestimating smoothed RTT.
-
-A sender calculates both smoothed RTT (SRTT) and RTT variance (RTTVAR) similar
-to those specified in {{?RFC6298}}.  Note that computing smoothed_rtt does not
-use ack_delay for the first RTT sample, because doing so would result in a
-smoothed_rtt that is smaller than the min_rtt.
-
-On every newly acknowledged ack-eliciting largest acked:
+## Measuring and Reporting Host Delay {#host-delay}

We don't _really_ use sender and receiver terminology, partly because a receiver of STREAM frames is a sender of MAX_STREAM_DATA (et al) frames, which are also ack-eliciting.

> -A sender calculates both smoothed RTT (SRTT) and RTT variance (RTTVAR) similar
-to those specified in {{?RFC6298}}.  Note that computing smoothed_rtt does not
-use ack_delay for the first RTT sample, because doing so would result in a
-smoothed_rtt that is smaller than the min_rtt.
-
-On every newly acknowledged ack-eliciting largest acked:
+## Measuring and Reporting Host Delay {#host-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.  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

Yeah, this is going to need some more work anyway (#2596), but I've changed it to a MAY. What complexities does it create?

> -On every newly acknowledged ack-eliciting largest acked:
+## Measuring and Reporting Host Delay {#host-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.  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.
+
+An endpoint MUST NOT excessively delay acknowledgements of ack-eliciting
+packets.  The maximum ack delay is communicated in the max_ack_delay transport
+parameter, see Section 18.1 of {{QUIC-TRANSPORT}}.  max_ack_delay implies an

I think implies works better here

> +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.
+
+An endpoint MUST NOT excessively delay acknowledgements of ack-eliciting
+packets.  The maximum ack delay is communicated in the max_ack_delay transport
+parameter, see Section 18.1 of {{QUIC-TRANSPORT}}.  max_ack_delay implies an
+explicit contract: an endpoint promises to never delay acknowledgments of an
+ack-eliciting packet by more than the indicated value. If it does, any excess
+accrues to the RTT estimate and could result in spurious retransmissions from
+the peer.
+
+
+# Estimating the Round-Trip Time {#compute-rtt}
+
+At a high level, an endpoint measures the time from when a packet was sent to

It's not quite accurate to say that this is an RTT sample because of all the conditions below, which is why I have the phrase. 

>  ~~~
 latest_rtt = ack_time - send_time_of_largest_acked
 ~~~
 
-First RTT sample:
+An endpoint uses only locally observed times in generating RTT samples and does

This subsection is talking about generating RTT samples, which do not adjust for host delays.

>  ~~~
 latest_rtt = ack_time - send_time_of_largest_acked
 ~~~
 
-First RTT sample:
+An endpoint uses only locally observed times in generating RTT samples and does
+not adjust for any host delays reported by the peer ({{host-delay}}).
+
+A peer reports host delays for only the largest acknowledged packet in an ACK
+frame, which is assumed by subsequent computations of smoothed_rtt and rttvar in

Same as above -- this section on generating RTT samples doesn't use host delay, but later subsections (srtt, rttvar) do.

>  ~~~
 latest_rtt = ack_time - send_time_of_largest_acked
 ~~~
 
-First RTT sample:
+An endpoint uses only locally observed times in generating RTT samples and does
+not adjust for any host delays reported by the peer ({{host-delay}}).
+
+A peer reports host delays for only the largest acknowledged packet in an ACK
+frame, which is assumed by subsequent computations of smoothed_rtt and rttvar in
+adjusting for host delays.  As a result, an RTT sample is only generated using
+the largest acknowledged packet in the received ACK frame.
+
+To avoid generating multiple RTT samples using the same packet, an ACK frame
+SHOULD NOT be used to update RTT estimates if it does not newly acknowledge the

good call -- fixed the condition above.

>  ~~~
 latest_rtt = ack_time - send_time_of_largest_acked
 ~~~
 
-First RTT sample:
+An endpoint uses only locally observed times in generating RTT samples and does
+not adjust for any host delays reported by the peer ({{host-delay}}).
+
+A peer reports host delays for only the largest acknowledged packet in an ACK
+frame, which is assumed by subsequent computations of smoothed_rtt and rttvar in
+adjusting for host delays.  As a result, an RTT sample is only generated using
+the largest acknowledged packet in the received ACK frame.
+
+To avoid generating multiple RTT samples using the same packet, an ACK frame
+SHOULD NOT be used to update RTT estimates if it does not newly acknowledge the
+largest acknowledged packet.
+
+An RTT sample MUST NOT be generated on receiving an ACK frame that does not
+newly acknowledge at least one ack-eliciting packet.  A peer does not send an

We are clear on the MUST NOT, and I don't want to dwell too long on the rationale... happy to take any text suggestions here, but I really don't think it's worth getting into the complications.

>    // Based on {{?RFC6298}}.
   if (smoothed_rtt == 0):
-    smoothed_rtt = latest_rtt
+    smoothed_rtt = adjusted_rtt

*facepalm* thanks for catching this. I believe this was a bug in the original pseudo-code and the ack_delay adjustment should have been in the else to begin with.

-- 
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-224690825