Subject: [quicwg/base-drafts] Editorial fixes and v1 of RTT calculation (#921)

From: ianswett <[email-masked]>
Date: Mon, 13 Nov 2017 02:47:09 +0000 (UTC)

Editorial fixes from my re-review of the recovery doc on the plane and add some text for the RTT calculation section, because it's important.
You can view, comment on, or merge this pull request online at:

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

-- Commit Summary --

  * Editorial fixes and v1 of RTT calculation

-- File Changes --

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

-- Patch Links --

https://github.com/quicwg/base-drafts/pull/921.patch
https://github.com/quicwg/base-drafts/pull/921.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/921

---

From: Marten Seemann <[email-masked]>
Date: Mon, 13 Nov 2017 02:55:14 +0000 (UTC)

marten-seemann commented on this pull request.

> @@ -194,8 +194,14 @@ round-trip time (RTT) is critical to these algorithms and is described first.
 
 ## Computing the RTT estimate
 
-(To be filled)
+RTT is calculated when an ACK frame arrives by computing the difference
+between the current time and the time the largest newly acked packet was sent.
+If no packets are newly acknowledged, RTT cannot be calculated. When RTT is
+calculated, the ack delay field from the ACK frame SHOULD be subtracted from
+the RTT as long as the result is positive.

Is this the right thing to do?
Sending an ack delay that would make the RTT negative sounds like a protocol violation. If we want to be generous with a peer that might just have adjusted its system time, the right thing to do would be ignoring that RTT sample completely, wouldn't it?

-- 
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/921#pullrequestreview-75988198

---

From: ianswett <[email-masked]>
Date: Mon, 13 Nov 2017 02:58:33 +0000 (UTC)

ianswett commented on this pull request.

 
-(To be filled)

It's a good question.  GQUIC does what I wrote, but I'm not sure it's the best thing.  

The only reason I wouldn't want to throw it out entirely is that it may cause you to get no RTT samples in the worst case.

-- 
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/921#discussion_r150439876

---

From: Marten Seemann <[email-masked]>
Date: Mon, 13 Nov 2017 02:59:30 +0000 (UTC)

-(To be filled)

Your text is consistent with the pseudocode. Do we need a new issue to track this?

-- 
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/921#discussion_r150439941

---

From: ianswett <[email-masked]>
Date: Mon, 13 Nov 2017 03:07:34 +0000 (UTC)

-(To be filled)

Possibly.  Alternately, I can add a sentence indicating that samples may be ignored if the ack delay is too large?  I'm not convinced we will or need to come to consensus on this, so it might be better to allow either until we have a strong reason to suggest one over the other.

-- 
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/921#discussion_r150440515

---

From: janaiyengar <[email-masked]>
Date: Tue, 28 Nov 2017 08:56:01 +0000 (UTC)

@janaiyengar pushed 1 commit.

36f057d  fix nits

-- 
You are receiving this because you are subscribed to this thread.
View it on GitHub:
https://github.com/quicwg/base-drafts/pull/921/files/8e7476f071868cd9ce321cbd6b927efb48a6a69e..36f057ddaa9d20835beb313b58f160c5a9d152d7

---

From: janaiyengar <[email-masked]>
Date: Tue, 28 Nov 2017 08:58:11 +0000 (UTC)

janaiyengar approved this pull request.

Ok, I've fixed the nits that I noted in my review, and I'm merging.

>  allow any acked packet to be reneged, greatly simplifying implementations on
 both sides and reducing memory pressure on the sender.
 
 ### More ACK Ranges
 
 QUIC supports up to 256 ACK ranges, opposed to TCP's 3 SACK ranges.  In high
-loss environments, this speeds recovery.
+loss environments, this speeds recovery, reduces spuriuos retransmits,

typo: "spurious"

 
-(To be filled)

Let's add this text in and open an issue. GQUIC currently subtracts this from RTT measurements, but if the peer's clock is off, this ack delay could be arbitrarily large, leading to a poor RTT estimate. Perhaps this should be protected by the measured minRTT as the min... but let's discuss that on an issue separately.

> @@ -292,13 +298,12 @@ conditions:
 * If there are more than one unacknowledged packets, PTO SHOULD be scheduled for
   max(2*SRTT, 10ms).
 
-* If RTO is earlier, schedule a TLP alarm in its place. That is, PTO SHOULD be
-  scheduled for min(RTO, PTO).
+* If RTO (RTO, {{rto}}) is earlier, schedule a TLP alarm in its place. That is,

Why the "RTO" inside the parenthesis? Did you mean to say "If RTO {{rto}} is earlier"?

> @@ -308,10 +313,8 @@ A PTO value of at least 2*SRTT ensures that the ACK is overdue. Using a PTO of
 exactly 1*SRTT may generate spurious probes, and 2*SRTT is simply the next
 integral value of RTT.
 
-(TODO: These values of 2 and 1.5 are a bit arbitrary. Reconsider these.)
-
-If the Retransmission Timeout (RTO, {{rto}}) period is smaller than the computed
-PTO, then a PTO is scheduled for the smaller RTO period.
+The values of 2 and 1.5 are based on the TLP draft, but implementations MAY

cite TLP draft

-- 
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/921#pullrequestreview-79424654

---

From: janaiyengar <[email-masked]>
Date: Tue, 28 Nov 2017 08:58:17 +0000 (UTC)

Merged #921.

-- 
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/921#event-1360890129