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

[Martin Thomson <[e-mail]>]

martinthomson commented on this pull request.



>  
-QUIC ACKs explicitly encode the delay incurred at the receiver between when a
-packet is received and when the corresponding ACK is sent.  This allows the
-receiver of the ACK to adjust for receiver delays, specifically the delayed ack
-timer, when estimating the path RTT.  This mechanism also allows a receiver to
-measure and report the delay from when a packet was received by the OS kernel,
-which is useful in receivers which may incur delays such as context-switch
-latency before a userspace QUIC receiver processes a received packet.
+An endpoint measures the delay incurred between when a packet is received and
+when the corresponding ACK 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

"this delay" meaning which?  Delays introduced by the host that we want to include in the RTT, or delays introduced by the host that we **don't** want to include?

> -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:
+
+# Computing the round-trip time (RTT) estimate {#compute-rtt}

Use Title Case.  And drop the acronym from the title.

```suggestion
# Estimating the Round-Trip Time {#compute-rtt}
```

> @@ -226,24 +226,25 @@ QUIC supports many ACK ranges, opposed to TCP's 3 SACK ranges.  In high loss
 environments, this speeds recovery, reduces spurious retransmits, and ensures
 forward progress without relying on timeouts.
 
-### Explicit Correction For Delayed ACKs
+### Explicit Correction For Delayed ACKs {#ack-delay}

Use words.  ACKs are delayed in the network, because they are frames.  Acknowledgments are the things that you put in the frame (and the things where we need to compensate for delays).

```suggestion
### Explicit Correction For Delayed Acknowledgements {#ack-delay}
```

>  
-QUIC ACKs explicitly encode the delay incurred at the receiver between when a
-packet is received and when the corresponding ACK is sent.  This allows the
-receiver of the ACK to adjust for receiver delays, specifically the delayed ack
-timer, when estimating the path RTT.  This mechanism also allows a receiver to
-measure and report the delay from when a packet was received by the OS kernel,
-which is useful in receivers which may incur delays such as context-switch
-latency before a userspace QUIC receiver processes a received packet.
+An endpoint measures the delay incurred between when a packet is received and
+when the corresponding ACK is sent.  The endpoint encodes this host delay for

```suggestion
when the corresponding acknowledgment is sent.  The endpoint encodes this host delay for
```

>  
+An endpoint MUST NOT excessively delay acknowledgements of ack-eliciting

I think that the most important thing here is the contract that `max_ack_delay` implies: An endpoints promises to never delay acknowledgments of an ack-eliciting packet by more than the indicated value.  If they do, any excess accrues to the RTT estimate and could result in timeouts.

>  ~~~
 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.

Do we generate samples for packets?

```suggestion
An ACK frame that does not newly acknowledge any packets MUST NOT be used to update RTT estimates.
```

>  ~~~
 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.
+This avoids multiple RTT samples for the same packet.
+
+An RTT sample MUST NOT be generated for a packet that is not the largest
+acknowledged in the received ACK frame.  A peer reports host delays for only the

I don't think that you need a "MUST NOT" here.  Your definition of the sample covers this.

> +This avoids multiple RTT samples for the same packet.
+
+An RTT sample MUST NOT be generated for a packet that is not the largest
+acknowledged in the received ACK frame.  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 correcting for host delays.
+
+An RTT sample MUST NOT be generated if the ACK frame does not newly acknowledge
+an ack-eliciting packet. A peer does not send an ACK frame in response to
+receiving only non-ack-eliciting packets, and an ACK frame that is subsequently
+sent can include an arbitrarily large Ack Delay field.  Ignoring such ACK frames
+avoids complications in subsequent smoothed_rtt and rttvar computations.
+
+A sender might generate multiple RTT samples per RTT when multiple ACK frames
+are received within an RTT.  As suggested in {{?RFC6298}}, doing so might result
+in inadequate history in smoothed_rtt and rttvar.  This is an open question

The "This" here is not very clear.  "Ensuring that RTT estimates retain sufficient history is an open research question."

> +lesser of min_rtt and latest_rtt on subsequent samples.
+
+An endpoint uses only locally observed times in computing the min_rtt and does
+not correct for host delays reported by the peer ({{ack-delay}}).  Doing so
+allows the endpoint to set a lower bound for the smoothed_rtt based entirely on
+what it observes (see {{smoothed-rtt}}), and limits potential underestimation
+due to erroneously-reported delays by the peer.
+
+## Computing smoothed_rtt and rttvar {#smoothed-rtt}
+
+smoothed_rtt is an exponentially-weighted moving average of an endpoint's
+RTT samples, and rttvar is the endpoint's estimated variance in
+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

Check grammar.  "A peer indicates that the peer's delay" might be correct, but it's hard to read.

> +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}}).

Why SHOULD?  Why use 2119 language here at all?

> When correcting an RTT sample, endpoints first limit the reported Ack Delay to the value of the max_ack_delay transport parameter.  The RTT sample is then reduced by the resulting delay value.  If correcting for host delays would result in a sample that is less than min_rtt, the unmodified sample is used.  This guards against underestimation of RTT as a result of inflated reporting of host delays.

> +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.

This assertion isn't necessary.  Yes, the one sample you have for min_rtt is equal to latest_rtt, so of course any host delay will not be applied.  So this just duplicates the earlier requirement.  If anything, just note that there is no point in applying host delay corrections as they are never applied.

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

I think that you should use a different name for this result.  Otherwise, you are change the value of a variable that you rely on in prose.

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