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

From: Jana Iyengar <[email-masked]>
Date: Sun, 07 Apr 2019 22:29:25 -0700

I started with trying to introduce rationale for why only ack-eliciting packets are used for RTT computation, and ended up restructuring the section.

Closes #2567, #2568.
You can view, comment on, or merge this pull request online at:

  https://github.com/quicwg/base-drafts/pull/2592

-- Commit Summary --

  * rewrite of RTT estimate sectionbn

-- File Changes --

    M draft-ietf-quic-recovery.md (167)

-- Patch Links --

https://github.com/quicwg/base-drafts/pull/2592.patch
https://github.com/quicwg/base-drafts/pull/2592.diff

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

---

From: Jana Iyengar <[email-masked]>
Date: Mon, 08 Apr 2019 05:31:05 +0000 (UTC)

@janaiyengar pushed 1 commit.

59751b7067c17aafe401c1fb8d6ce608505d3f0f  fix lint

-- 
You are receiving this because you are subscribed to this thread.
View it on GitHub:
https://github.com/quicwg/base-drafts/pull/2592/files/524bc029ec70fc3b719c1bf0e60380fe3da6fc0b..59751b7067c17aafe401c1fb8d6ce608505d3f0f

---

From: Martin Thomson <[email-masked]>
Date: Sun, 07 Apr 2019 22:59:03 -0700

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

---

From: Jana Iyengar <[email-masked]>
Date: Mon, 08 Apr 2019 12:12:09 -0700

bf9f483b02be2526c60087473e75a49986f8fc05  update pseudocode

-- 
You are receiving this because you are subscribed to this thread.
View it on GitHub:
https://github.com/quicwg/base-drafts/pull/2592/files/59751b7067c17aafe401c1fb8d6ce608505d3f0f..bf9f483b02be2526c60087473e75a49986f8fc05

---

From: Jana Iyengar <[email-masked]>
Date: Mon, 08 Apr 2019 20:04:46 +0000 (UTC)

f4dddc5ad1f3f60f65a3faa14b133450b119f5ac  Update draft-ietf-quic-recovery.md

-- 
You are receiving this because you are subscribed to this thread.
View it on GitHub:
https://github.com/quicwg/base-drafts/pull/2592/files/bf9f483b02be2526c60087473e75a49986f8fc05..f4dddc5ad1f3f60f65a3faa14b133450b119f5ac

---

From: Jana Iyengar <[email-masked]>
Date: Mon, 08 Apr 2019 13:04:55 -0700

1ec3a952227f9179686b5445400307f9d90bde88  Update draft-ietf-quic-recovery.md

-- 
You are receiving this because you are subscribed to this thread.
View it on GitHub:
https://github.com/quicwg/base-drafts/pull/2592/files/f4dddc5ad1f3f60f65a3faa14b133450b119f5ac..1ec3a952227f9179686b5445400307f9d90bde88

---

From: Jana Iyengar <[email-masked]>
Date: Mon, 08 Apr 2019 13:05:09 -0700

6bfea4057db6df7786123989e51ea5ea46e6e64f  Update draft-ietf-quic-recovery.md

-- 
You are receiving this because you are subscribed to this thread.
View it on GitHub:
https://github.com/quicwg/base-drafts/pull/2592/files/1ec3a952227f9179686b5445400307f9d90bde88..6bfea4057db6df7786123989e51ea5ea46e6e64f

---

From: Jana Iyengar <[email-masked]>
Date: Mon, 08 Apr 2019 13:39:23 -0700

janaiyengar commented on this pull request.

Heh -- I used the word contract and then removed it. I'll use your words.

-- 
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#discussion_r273228879

---

From: Jana Iyengar <[email-masked]>
Date: Mon, 08 Apr 2019 18:17:05 -0700

21a0bda54390905d6db41b5e17a4c840b5731559  martin's comments

-- 
You are receiving this because you are subscribed to this thread.
View it on GitHub:
https://github.com/quicwg/base-drafts/pull/2592/files/6bfea4057db6df7786123989e51ea5ea46e6e64f..21a0bda54390905d6db41b5e17a4c840b5731559

---

From: Jana Iyengar <[email-masked]>
Date: Mon, 08 Apr 2019 18:17:36 -0700

Thanks for your comments, @martinthomson 

-First RTT sample:

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.

+

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

+

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

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

---

From: Martin Thomson <[email-masked]>
Date: Mon, 08 Apr 2019 18:41:58 -0700

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

---

From: Jana Iyengar <[email-masked]>
Date: Mon, 08 Apr 2019 19:47:28 -0700

Yeah, I was thinking about this too, but it's text that we already had. FWIW, I think it's useful to separate out these delays, not because they shouldn't get used for PTO estimates, but because knowing component delays might be helpful. That said, I don't have a good use case for these component delays yet. OTOH, I've started that separation between stack-controlled delays and other delays in the text, it might be worth using that distinction here, and arguing that these delays ought to be part of the path RTT since they're not controlled delays. I'll file a separate issue.

-- 
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#discussion_r273307278

---

From: Jana Iyengar <[email-masked]>
Date: Mon, 08 Apr 2019 19:50:08 -0700

Filed #2596 

-- 
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#discussion_r273307653

---

From: Jana Iyengar <[email-masked]>
Date: Tue, 09 Apr 2019 02:52:23 +0000 (UTC)

c2b9da0a7cb90ea266489f2c1840bcd00cf2a19d  s/corrected/adjusted

-- 
You are receiving this because you are subscribed to this thread.
View it on GitHub:
https://github.com/quicwg/base-drafts/pull/2592/files/21a0bda54390905d6db41b5e17a4c840b5731559..c2b9da0a7cb90ea266489f2c1840bcd00cf2a19d

---

From: Jana Iyengar <[email-masked]>
Date: Tue, 09 Apr 2019 12:43:12 -0700

f0c860d55a936c37cabcb0e706db5b475d5ad07d  s/ack delay/host delay

-- 
You are receiving this because you are subscribed to this thread.
View it on GitHub:
https://github.com/quicwg/base-drafts/pull/2592/files/c2b9da0a7cb90ea266489f2c1840bcd00cf2a19d..f0c860d55a936c37cabcb0e706db5b475d5ad07d

---

From: Jana Iyengar <[email-masked]>
Date: Tue, 09 Apr 2019 12:48:43 -0700

cb683af63e45486fc1530f33f437f47beb84ff7e  moving host delay section to under Generating Acks

-- 
You are receiving this because you are subscribed to this thread.
View it on GitHub:
https://github.com/quicwg/base-drafts/pull/2592/files/f0c860d55a936c37cabcb0e706db5b475d5ad07d..cb683af63e45486fc1530f33f437f47beb84ff7e

---

From: Jana Iyengar <[email-masked]>
Date: Tue, 09 Apr 2019 19:49:55 +0000 (UTC)

0b38d2c8069c37e931f88c4f3e57ff1130c8c4aa  nit

-- 
You are receiving this because you are subscribed to this thread.
View it on GitHub:
https://github.com/quicwg/base-drafts/pull/2592/files/cb683af63e45486fc1530f33f437f47beb84ff7e..0b38d2c8069c37e931f88c4f3e57ff1130c8c4aa

---

From: ianswett <[email-masked]>
Date: Tue, 09 Apr 2019 15:03:03 -0700

ianswett commented on this pull request.

Thanks for the substantial rework, some comments.

Actually, ACKs are delayed by the receiver in this case and this whole section talks about the ACK frame, which makes sense to me.

-# 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}

Sidenote: Are we ever going to move this to transport?

> -RTT when multiple such ACK frames are received within an RTT.  When multiple
+## Measuring and Reporting Host Delay {#host-delay}

Would "receiver delay" be more accurate?  I know I've heard the term host delay before, but I thought it usually implied unintentional delays(ie: processing time, etc), not intentional delays like delayed ACKs?

+## Measuring and Reporting Host Delay {#host-delay}

endpoint -> receiver?

+## Measuring and Reporting Host Delay {#host-delay}

Can you avoid ' - ' with ", which is particularly important for delayed acknowledgements,"

+## Measuring and Reporting Host Delay {#host-delay}

Based on my experience, I might be inclined towards a MAY.  Using receive timestamps creates as many(or more) complexities as it solves.

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

```suggestion
parameter, see Section 18.1 of {{QUIC-TRANSPORT}}.  max_ack_delay specifies an
```

+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

```suggestion
An endpoint measures the time from when a packet was sent to
```

-First RTT sample:

I'm confused about where this is going, since we're going to adjust later on?

-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

I'm unclear what the latter half of this sentence means?  Maybe remove it?

-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

You say this two different ways, which are subtly different.  Above, you say the largest acked needs to be larger.  Here you say it needs to be newly acknowledged.  I think the latter is more correct, but we should be consistent.

-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

peer -> receiver?

Also, this paragraph could be a bit clearer, since I think there's an important point here and it may not be clear to all readers.

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

I'd suggest moving the adjusted_rtt calculation into the else and using latest_rtt here?  If not, I'd ensure smoothed_rtt and rttvar are set as a function of the same field, not one based on adjusted_rtt and the other latest_rtt.

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

---

From: Jana Iyengar <[email-masked]>
Date: Tue, 09 Apr 2019 16:51:46 -0700

9be9f1aaa1d7b89d87535c4b8a4748c89db8fea9  ian's comments

-- 
You are receiving this because you are subscribed to this thread.
View it on GitHub:
https://github.com/quicwg/base-drafts/pull/2592/files/0b38d2c8069c37e931f88c4f3e57ff1130c8c4aa..9be9f1aaa1d7b89d87535c4b8a4748c89db8fea9

---

From: Jana Iyengar <[email-masked]>
Date: Tue, 09 Apr 2019 16:52:13 -0700

Thanks, Ian -- updated.

next PR in progress

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.

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

I think implies works better here

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. 

-First RTT sample:

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

-First RTT sample:

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

-First RTT sample:

good call -- fixed the condition above.

-First RTT sample:

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.

   if (smoothed_rtt == 0):

*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

---

From: ianswett <[email-masked]>
Date: Tue, 09 Apr 2019 18:24:41 -0700

Bump?

-First RTT sample:

Bump, I'm still unclear on what the latter half of the sentence is trying to communicate.

-- 
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#discussion_r273759939

---

From: ianswett <[email-masked]>
Date: Tue, 09 Apr 2019 18:25:07 -0700

Ping

-- 
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#discussion_r273760586

---

From: ianswett <[email-masked]>
Date: Tue, 09 Apr 2019 18:26:28 -0700

Ping

-- 
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#discussion_r273760757

---

From: ianswett <[email-masked]>
Date: Tue, 09 Apr 2019 18:27:44 -0700

I would disagree, since I do not like the word implies in IETF documents.

-- 
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#discussion_r273760955

---

From: ianswett <[email-masked]>
Date: Tue, 09 Apr 2019 18:28:56 -0700

-First RTT sample:

I would just remove this paragraph, because it's potentially confusing and I'm not sure what it's adding?

-- 
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#discussion_r273761187

---

From: Jana Iyengar <[email-masked]>
Date: Wed, 10 Apr 2019 13:22:31 -0700

Thanks, Ian, responses below.

See my response above.

I'm not sure I follow. It's fine to use commas and dashes together. Can you suggest a rephrasing?

https://www.merriam-webster.com/dictionary/imply --> "imply" is commonly used to indicate necessary consequence, not just inference (I suspect your dislike is to this meaning of imply).

The way I read it, max_ack_delay specifies a transport parameter value which implies the contract.

-First RTT sample:

Fair, it's not necessary yet, so removed.

-First RTT sample:

I've rewritten the para, so hopefully it reads better.

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

---

From: Jana Iyengar <[email-masked]>
Date: Wed, 10 Apr 2019 13:22:58 -0700

39b6ef61548221a83758522becd874445d881da4  Ian's comments

-- 
You are receiving this because you are subscribed to this thread.
View it on GitHub:
https://github.com/quicwg/base-drafts/pull/2592/files/9be9f1aaa1d7b89d87535c4b8a4748c89db8fea9..39b6ef61548221a83758522becd874445d881da4

---

From: ianswett <[email-masked]>
Date: Thu, 11 Apr 2019 12:26:23 -0700

```suggestion
to adjust for any host delays, which is particularly important for delayed acknowledgements, when
```

-- 
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#discussion_r274605377

---

From: ianswett <[email-masked]>
Date: Thu, 11 Apr 2019 12:28:01 -0700

Yeah, not in love with it here, but I'm failing to convince you, so let's leave it as is.

-- 
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#discussion_r274605893

---

From: ianswett <[email-masked]>
Date: Thu, 11 Apr 2019 12:28:40 -0700

-First RTT sample:
+An RTT sample is generated using only the largest acknowledged packet in the
+received ACK frame.  This is because a peer reports host delays for only the
+largest acknowledged packet in an ACK frame.  While the reported host delay is
+not used by the RTT sample measurement, it is used to adjust the RTT sample in
+subsequent computations of smoothed_rtt and rttvar {{smoothed-rtt}}.
+
+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
+ACK frame on receiving only non-ack-eliciting packets, and an ACK frame that is

```suggestion
ACK frame on receiving only non-ack-eliciting packets, so an ACK frame that is
```

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

---

From: ianswett <[email-masked]>
Date: Thu, 11 Apr 2019 12:29:03 -0700

-First RTT sample:

Suggestion below

-- 
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#discussion_r274606256

---

From: ianswett <[email-masked]>
Date: Thu, 11 Apr 2019 12:30:55 -0700

ianswett approved this pull request.

Two small suggestions, but looks good otherwise.

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

---

From: Jana Iyengar <[email-masked]>
Date: Thu, 11 Apr 2019 16:58:11 -0700

9b9973ce0ac06aecc8ea3969028e2384340f8e30  Update draft-ietf-quic-recovery.md

-- 
You are receiving this because you are subscribed to this thread.
View it on GitHub:
https://github.com/quicwg/base-drafts/pull/2592/files/39b6ef61548221a83758522becd874445d881da4..9b9973ce0ac06aecc8ea3969028e2384340f8e30

---

From: Jana Iyengar <[email-masked]>
Date: Thu, 11 Apr 2019 17:06:01 -0700

9cf7d921b5ab8f5ce6b61d5b86ded8bdc235ffcf  Update draft-ietf-quic-recovery.md

-- 
You are receiving this because you are subscribed to this thread.
View it on GitHub:
https://github.com/quicwg/base-drafts/pull/2592/files/9b9973ce0ac06aecc8ea3969028e2384340f8e30..9cf7d921b5ab8f5ce6b61d5b86ded8bdc235ffcf

---

From: Jana Iyengar <[email-masked]>
Date: Thu, 11 Apr 2019 17:06:51 -0700

77d6256bd08a1dc12f289bb3b7fa9ce3f21f3a70  reflow

-- 
You are receiving this because you are subscribed to this thread.
View it on GitHub:
https://github.com/quicwg/base-drafts/pull/2592/files/9cf7d921b5ab8f5ce6b61d5b86ded8bdc235ffcf..77d6256bd08a1dc12f289bb3b7fa9ce3f21f3a70

---

From: Jana Iyengar <[email-masked]>
Date: Thu, 11 Apr 2019 17:08:51 -0700

Thanks, @ianswett  -- I've taken in your suggestions. Merging.

-- 
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#issuecomment-482379916

---

From: Jana Iyengar <[email-masked]>
Date: Thu, 11 Apr 2019 17:09:13 -0700

Merged #2592 into master.

-- 
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#event-2271039037