Subject: [quicwg/base-drafts] Stop using initial timeout (#4263)

From: Martin Thomson <[email-masked]>
Date: Tue, 27 Oct 2020 16:30:55 -0700

Address validation can use 3*PTO, just taking the maximum of the old and
new path PTO.  The idea of initial timeout is easily replaced in
recovery by instead describing the effect on the PTO.  The net effect is
the same, except that new paths often have to include the maximum ACK
delay, which this doesn&#39;t incorporate.

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

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

-- Commit Summary --

  * Stop using initial timeout

-- File Changes --

    M draft-ietf-quic-recovery.md (5)
    M draft-ietf-quic-transport.md (6)

-- Patch Links --

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

---

From: ianswett <[email-masked]>
Date: Tue, 27 Oct 2020 17:33:43 -0700

@ianswett requested changes on this pull request.

> @@ -632,8 +632,9 @@ in most cases and is less likely to spuriously retransmit data.
 
 Resumed connections over the same network MAY use the previous connection's
 final smoothed RTT value as the resumed connection's initial RTT.  When no
-previous RTT is available, the initial RTT SHOULD be set to 333ms, resulting in
-a 1 second initial timeout, as recommended in {{?RFC6298}}.
+previous RTT is available, the initial RTT SHOULD be set to 333ms.  This

I prefer the existing text to the updated text.  Is there something you're adding that is important?

 ~~~
-   validation_timeout = max(3*PTO, 6*kInitialRtt)
+   validation_timeout = 3*max(PTO_current, PTO_new)

PTO_current and PTO_new aren't defined, so even thought I understand the intent, I'm no longer sure this pseudocode is helpful.  Possibly it'd be better to remove 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/4263#pullrequestreview-518242232

---

From: Martin Thomson <[email-masked]>
Date: Tue, 27 Oct 2020 17:38:54 -0700

@martinthomson commented on this pull request.

 ~~~

That is probably good.  I will do that.

-- 
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/4263#discussion_r513112521

---

From: Martin Thomson <[email-masked]>
Date: Tue, 27 Oct 2020 17:38:56 -0700

"initial timeout" seems to be a concept that doesn't exist in 6298, but it seems to be implied something.  And it is not defined.  So I wanted to remove 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/4263#discussion_r513112530

---

From: Martin Thomson <[email-masked]>
Date: Tue, 27 Oct 2020 17:41:15 -0700

Oh, I forgot... The PTO is 3*kInitialRtt only for the handshake.  A new path will be after the handshake is complete, so the value will have max_ack_delay added.

-- 
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/4263#discussion_r513113224

---

From: Martin Thomson <[email-masked]>
Date: Tue, 27 Oct 2020 17:50:39 -0700

@martinthomson pushed 1 commit.

ec9355809d26c1a62fcf6c6b565c0e477d69a49d  Remove formula with confusing variables

-- 
You are receiving this because you are subscribed to this thread.
View it on GitHub:
https://github.com/quicwg/base-drafts/pull/4263/files/2719b297f7f12368ec12c8fdbf70aba6a2c2eb93..ec9355809d26c1a62fcf6c6b565c0e477d69a49d

---

From: Jana Iyengar <[email-masked]>
Date: Thu, 29 Oct 2020 15:59:06 -0700

@janaiyengar commented on this pull request.

I prefer this, but let's take the corrected explicit computation from Ian's PR.

> @@ -2254,12 +2254,8 @@ abandons its attempt to validate the path.
 Endpoints SHOULD abandon path validation based on a timer. When setting this
 timer, implementations are cautioned that the new path could have a longer
 round-trip time than the original.  A value of three times the larger of the
-current Probe Timeout (PTO) or the initial timeout (that is, 2*kInitialRtt) as
-defined in {{QUIC-RECOVERY}} is RECOMMENDED.  That is:
-
-~~~
-   validation_timeout = max(3*PTO, 6*kInitialRtt)
-~~~
+current Probe Timeout (PTO) or the PTO for the new path (that is, using
+kInitialRtt as defined in {{QUIC-RECOVERY}}) is RECOMMENDED.

Can we add the computation back in? With the corrected values of course:

That is, `validation timeout = max(3*PTO, 9*kInitialRtt)`

> +results in handshakes starting with a PTO of 1 second, as recommended in
+{{?RFC6298}}.

```suggestion
results in handshakes starting with a PTO of 1 second, as recommended
for TCP's initial retransmission timeout; see Section 2 of {{?RFC6298}}.
```

-- 
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/4263#pullrequestreview-520144163

---

From: Martin Thomson <[email-masked]>
Date: Thu, 29 Oct 2020 17:00:22 -0700

But that is wrong.  Because the PTO on the new path is `9*kInitialRtt` on the handshake, but `9*kInitialRtt + 3*max_ack_delay` on migration.

-- 
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/4263#discussion_r514634026

---

From: Martin Thomson <[email-masked]>
Date: Thu, 29 Oct 2020 17:00:44 -0700

a9d9aeb75f339408c1481c0bb5e1e048f16957f6  initial RTO comparison

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

---

From: Kazuho Oku <[email-masked]>
Date: Thu, 29 Oct 2020 17:29:56 -0700

@kazuho 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/4263#pullrequestreview-520217113

---

From: Jana Iyengar <[email-masked]>
Date: Thu, 29 Oct 2020 18:02:26 -0700

Oh hm. It's actually more than that, since rttvar is also set to kInitialRtt/2 initially. Ok, let's leave it as you have it then.

-- 
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/4263#discussion_r514651358

---

From: Jana Iyengar <[email-masked]>
Date: Thu, 29 Oct 2020 18:03:29 -0700

@janaiyengar approved this pull request.

I think this is correct. LGTM, but waiting for @ianswett to take a look.

-- 
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/4263#pullrequestreview-520281714

---

From: ianswett <[email-masked]>
Date: Fri, 30 Oct 2020 19:20:56 -0700

@ianswett 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/4263#pullrequestreview-521105110