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

[Jana Iyengar <[e-mail]>]

janaiyengar commented on this pull request.

Thanks for your comments, @martinthomson 

>  ~~~
 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 correct for any host delays reported by the peer ({{ack-delay}}).
+
+An RTT sample MUST NOT be generated for a packet that is not newly acknowledged.

Samples are generated using both the packet and the ack, so I have used "for packet" a couple of times. I like your phrasing better though, so using it.

> +the RTT samples.
+
+smoothed_rtt estimates path latency by correcting RTT samples for peer-reported
+host delays ({{ack-delay}}).  A peer indicates that the peer's delay in sending
+an acknowledgement for an ack-eliciting packet will be no greater than the
+max_ack_delay transport parameter.  Consequently, when a peer reports an Ack
+Delay that is greater than max_ack_delay, the delay is attributed to reasons out
+of the peer's control, such as scheduler latency at the peer or loss of previous
+ACK frames.  Any delays beyond the peer's max_ack_delay are therefore considered
+effectively part of path delay and incorporated into the smoothed_rtt estimate.
+
+When correcting an RTT sample using peer-reported acknowledgement delays, an
+endpoint:
+
+- SHOULD use the lesser of the value reported in Ack Delay field of the ACK
+  frame and the peer's max_ack_delay transport parameter ({{ack-delay}}).

Fair point.  Ultimately, this is bounded by another value that the peer advertises (max_ack_delay), so there's no additional safety issue here.

> +effectively part of path delay and incorporated into the smoothed_rtt estimate.
+
+When correcting an RTT sample using peer-reported acknowledgement delays, an
+endpoint:
+
+- SHOULD use the lesser of the value reported in Ack Delay field of the ACK
+  frame and the peer's max_ack_delay transport parameter ({{ack-delay}}).
+
+- MUST not apply the correction if the resulting RTT sample is smaller than the
+  min_rtt.  This limits the underestimation that a misreporting peer can cause
+  to the smoothed_rtt.
+
+On the first RTT sample in a connection, the smoothed_rtt is set to the
+latest_rtt.  The peer-reported host delay MUST NOT be used on the first sample,
+since correcting for it would result in a smoothed_rtt that is smaller than the
+min_rtt.

I think the way the text flows now makes this statement moot. I'm removing it.

>  ~~~
-min_rtt = min(min_rtt, latest_rtt)
-ack_delay = min(ack_delay, max_ack_delay)
-if (ack_delay + min_rtt < latest_rtt):
+ack_delay = min(Ack Delay in ACK Frame, max_ack_delay)
+if (min_rtt + ack_delay < latest_rtt):
   latest_rtt = latest_rtt - ack_delay

Good point. Changed.

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