Subject: [quicwg/base-drafts] Define Min RTT and use it to check ack delay (#975)

From: ianswett <[email-masked]>
Date: Thu, 30 Nov 2017 16:58:10 +0000 (UTC)

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

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

-- Commit Summary --

  * Define Min RTT and use it to check ack delay

-- File Changes --

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

-- Patch Links --

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

---

From: ianswett <[email-masked]>
Date: Thu, 30 Nov 2017 10:23:36 -0800

@ianswett pushed 1 commit.

dd7d960  Add rationale

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

---

From: Marten Seemann <[email-masked]>
Date: Thu, 30 Nov 2017 19:31:50 +0000 (UTC)

marten-seemann commented on this pull request.

 Like TCP, QUIC calculates both smoothed RTT and RTT variance as specified in
 {{?RFC6298}}.
 
+Min RTT is the minimum RTT measured over the connection, prior to adjusting

A connection might be long lived, so it kind of feels weird to have this value calculated over the whole lifetime of the connection. Maybe this is ok, since we're only using Min RTT as a sanity check for the ACK delay sent by the peer.

> @@ -588,6 +597,11 @@ Pseudocode for OnAckReceived and UpdateRtt follow:

    UpdateRtt(latest_rtt):

Add the second parameter here.

-- 
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/975#pullrequestreview-80301685

---

From: ianswett <[email-masked]>
Date: Thu, 30 Nov 2017 11:38:14 -0800

84e2986  Fix

-- 
You are receiving this because you are subscribed to this thread.
View it on GitHub:
https://github.com/quicwg/base-drafts/pull/975/files/dd7d96091babddb1ef4d09ae3775d3fbce1114cf..84e2986c2c3fe538c4434e9d7ae7183f66a5af6f

---

From: ianswett <[email-masked]>
Date: Thu, 30 Nov 2017 19:39:03 +0000 (UTC)

0b59902  Fix2

-- 
You are receiving this because you are subscribed to this thread.
View it on GitHub:
https://github.com/quicwg/base-drafts/pull/975/files/84e2986c2c3fe538c4434e9d7ae7183f66a5af6f..0b59902375c99f6da841f68bd395236a6be3d281

---

From: ianswett <[email-masked]>
Date: Thu, 30 Nov 2017 19:39:58 +0000 (UTC)

ianswett commented on this pull request.

BBR does use windowed min, but I think this is a fairly standard definition of 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/975#discussion_r154179941

---

From: janaiyengar <[email-masked]>
Date: Thu, 30 Nov 2017 13:00:31 -0800

janaiyengar approved this pull request.

-- 
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/975#pullrequestreview-80327999

---

From: janaiyengar <[email-masked]>
Date: Thu, 30 Nov 2017 21:00:49 +0000 (UTC)

janaiyengar commented on this pull request.

I think it's ok... we can get fancy here (with windowed min), but I think this is good for a simple safeguard.

-- 
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/975#discussion_r154199740

---

From: janaiyengar <[email-masked]>
Date: Thu, 30 Nov 2017 13:01:13 -0800

Merged #975.

-- 
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/975#event-1366508377