Subject: [quicwg/base-drafts] Clarify RTT calculations in recovery (#2506)

From: ianswett <[email-masked]>
Date: Fri, 08 Mar 2019 11:51:02 -0800

Fixes #2137
You can view, comment on, or merge this pull request online at:

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

-- Commit Summary --

  * Update draft-ietf-quic-recovery.md

-- File Changes --

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

-- Patch Links --

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

---

From: ianswett <[email-masked]>
Date: Fri, 08 Mar 2019 11:52:41 -0800

@ianswett pushed 1 commit.

da791704876a49d7344d49a7e9731256925705ba  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/2506/files/27b3b5173f4d7957f08e45fda100a819c3561307..da791704876a49d7344d49a7e9731256925705ba

---

From: ianswett <[email-masked]>
Date: Sun, 10 Mar 2019 07:27:51 -0700

64f7c8c09c68d04bfd15a7246698ee9ba45afdab  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/2506/files/da791704876a49d7344d49a7e9731256925705ba..64f7c8c09c68d04bfd15a7246698ee9ba45afdab

---

From: Martin Thomson <[email-masked]>
Date: Wed, 20 Mar 2019 21:24:24 +0000 (UTC)

martinthomson commented on this pull request.

Meta-level comments.  The code and text look fine.

> @@ -330,6 +330,31 @@ 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}}, see {{on-ack-received}}.
+
+On every newly acknowledged ack-eliciting largest acked:
+~~~
+latest_rtt = ack_time - send_time_of_largest_acked
+~~~
+
+First RTT sample:
+~~~
+min_rtt = latest_rtt
+smoothed_rtt = latest_rtt

So no ack delay here at all?  That probably needs explanation.

> @@ -330,6 +330,31 @@ 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.

I find the structure of this section to be a little hard to follow.  I think that you want to lay it out differently.

The baseline concepts here are:

1. the observed RTT (latest_rtt in the code), which includes any processing delays on the receiver end

2. minimum RTT, which is set to the minimum observed RTT for the path.  this doesn't include any processing delays on the receiver end, which might inflate the value a little, but we assume that receivers will immediately acknowledge a packet at some point (usually this is a good idea for a new path, and handshaking more or less depends on that)

3. ack delay, which captures the processing time and any forced delays on the receiver.  This is limited to the maximum value the receiver advertises.  This value is self-reported, so any error needs to result in using less network capacity, not more, or it could be used to increase network load inappropriately.  The goal here is to have an RTT estimate that is larger.  Thus, the design is such that if ACK delay is under-reported, or the maximum is set lower than the reality, this will result in an inflated RTT estimate.  If this is over-reported, the maximum will cap the value and again cause the RTT estimate to be driven up also.

4. The final RTT calculation.  This is calculated based on the observed RTT and the adjusted ack delay.  The value is smoothed so that some history is retained.

This structure matches the code, and I think 

-- 
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/2506#pullrequestreview-216985589

---

From: Jana Iyengar <[email-masked]>
Date: Sat, 30 Mar 2019 13:03:26 -0700

janaiyengar approved this pull request.

Small fix, but I'll file a new editorial issue with martin's restructure comment, and let's merge this.

For the first sample, using ack_delay will reduce the smoothed_rtt to below min_rtt. I think adding a sentence up in the para above might make sense. How about "Note that smoothed_rtt computation on the first RTT sample does not use ack_delay, because doing so will lead to a smoothed_rtt that is smaller than the min_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/2506#pullrequestreview-220835554