Subject: Re: [quicwg/base-drafts] Idle timeout indicates you will timeout (#3099)

From: ianswett <[email-masked]>
Date: Fri, 01 Nov 2019 10:24:11 -0700

@ianswett pushed 1 commit.

22346234eeb5e2578c44cab1ef5c3e6f85c4138e  Update draft-ietf-quic-transport.md

-- 
You are receiving this because you are subscribed to this thread.
View it on GitHub:
https://github.com/quicwg/base-drafts/pull/3099/files/e4dd819fb3939e82c668b52076f7d89d500b8e3f..22346234eeb5e2578c44cab1ef5c3e6f85c4138e

---

From: ianswett <[email-masked]>
Date: Fri, 01 Nov 2019 10:24:41 -0700

11d8da8732728fe0c02546fa3cb594049cc1f653  Update draft-ietf-quic-transport.md

-- 
You are receiving this because you are subscribed to this thread.
View it on GitHub:
https://github.com/quicwg/base-drafts/pull/3099/files/22346234eeb5e2578c44cab1ef5c3e6f85c4138e..11d8da8732728fe0c02546fa3cb594049cc1f653

---

From: ianswett <[email-masked]>
Date: Fri, 01 Nov 2019 10:24:51 -0700

508413d41bb40573232797b12c1eb2f1e5d03278  Update draft-ietf-quic-transport.md

-- 
You are receiving this because you are subscribed to this thread.
View it on GitHub:
https://github.com/quicwg/base-drafts/pull/3099/files/11d8da8732728fe0c02546fa3cb594049cc1f653..508413d41bb40573232797b12c1eb2f1e5d03278

---

From: ianswett <[email-masked]>
Date: Fri, 01 Nov 2019 10:25:06 -0700

e0d76ce4795f1ee20ee16964534a3732ac8eba50  Update draft-ietf-quic-transport.md

-- 
You are receiving this because you are subscribed to this thread.
View it on GitHub:
https://github.com/quicwg/base-drafts/pull/3099/files/508413d41bb40573232797b12c1eb2f1e5d03278..e0d76ce4795f1ee20ee16964534a3732ac8eba50

---

From: ianswett <[email-masked]>
Date: Fri, 01 Nov 2019 10:25:35 -0700

48f04d6cb6f2b080dc75c00c88242d05d3d77c36  Update draft-ietf-quic-transport.md

-- 
You are receiving this because you are subscribed to this thread.
View it on GitHub:
https://github.com/quicwg/base-drafts/pull/3099/files/e0d76ce4795f1ee20ee16964534a3732ac8eba50..48f04d6cb6f2b080dc75c00c88242d05d3d77c36

---

From: Jana Iyengar <[email-masked]>
Date: Fri, 01 Nov 2019 14:21:02 -0700

janaiyengar commented on this pull request.

A few comments. I think it would help readability immensely if you could articulate a new timeout period, Idle Timeout (ITO) explicitly.

Also, can you change the title of the PR? It's not particularly clarifying.

> @@ -2344,28 +2344,30 @@ source address.
 
 ## Idle Timeout {#idle-timeout}
 
-If the idle timeout is enabled, a connection is silently closed and the state is
-discarded when it remains idle for longer than both the advertised
-idle timeout (see {{transport-parameter-definitions}}) and three times the
+If the idle timeout is enabled by either peer, a connection is silently closed
+and the state is discarded when it remains idle for longer than the

```suggestion
and its state is discarded when it remains idle for longer than the
```

> -containing frames other than ACK or PADDING (an ack-eliciting packet; see
-{{QUIC-RECOVERY}}), but only if no other ack-eliciting packets have been sent
-since last receiving a packet.  Restarting when sending packets ensures that
-connections do not prematurely time out when initiating new activity.
-
-The value for an idle timeout can be asymmetric.  The value advertised by an
-endpoint is only used to determine whether the connection is live at that
-endpoint.  An endpoint that sends packets near the end of the idle timeout
-period of a peer risks having those packets discarded if its peer enters the
-draining state before the packets arrive.  If a peer could timeout within a
-Probe Timeout (PTO; see Section 6.3 of {{QUIC-RECOVERY}}), it is advisable to
-test for liveness before sending any data that cannot be retried safely.  Note
-that it is likely that only applications or application protocols will
-know what information can be retried.
+Each endpoint advertises a different max_idle_timeout, but the effective value
+is determined by taking the minimum of the two advertised values. By announcing

```suggestion
at an endpoint is computed as the minimum of the two advertised values. By announcing
```

> -{{QUIC-RECOVERY}}), but only if no other ack-eliciting packets have been sent
-since last receiving a packet.  Restarting when sending packets ensures that
-connections do not prematurely time out when initiating new activity.
-
-The value for an idle timeout can be asymmetric.  The value advertised by an
-endpoint is only used to determine whether the connection is live at that
-endpoint.  An endpoint that sends packets near the end of the idle timeout
-period of a peer risks having those packets discarded if its peer enters the
-draining state before the packets arrive.  If a peer could timeout within a
-Probe Timeout (PTO; see Section 6.3 of {{QUIC-RECOVERY}}), it is advisable to
-test for liveness before sending any data that cannot be retried safely.  Note
-that it is likely that only applications or application protocols will
-know what information can be retried.
+Each endpoint advertises a different max_idle_timeout, but the effective value
+is determined by taking the minimum of the two advertised values. By announcing
+a max_idle_timeout, endpoints commit to initiating an immediate close

```suggestion
a max_idle_timeout, an endpoint commits to initiating an immediate close
```

> -processed successfully.  The timer is also restarted when sending a packet
-containing frames other than ACK or PADDING (an ack-eliciting packet; see
-{{QUIC-RECOVERY}}), but only if no other ack-eliciting packets have been sent
-since last receiving a packet.  Restarting when sending packets ensures that
-connections do not prematurely time out when initiating new activity.
-
-The value for an idle timeout can be asymmetric.  The value advertised by an
-endpoint is only used to determine whether the connection is live at that
-endpoint.  An endpoint that sends packets near the end of the idle timeout
-period of a peer risks having those packets discarded if its peer enters the
-draining state before the packets arrive.  If a peer could timeout within a
-Probe Timeout (PTO; see Section 6.3 of {{QUIC-RECOVERY}}), it is advisable to
-test for liveness before sending any data that cannot be retried safely.  Note
-that it is likely that only applications or application protocols will
-know what information can be retried.
+Each endpoint advertises a different max_idle_timeout, but the effective value

```suggestion
Each endpoint advertises a max_idle_timeout, but the effective value
```

> -since last receiving a packet.  Restarting when sending packets ensures that
-connections do not prematurely time out when initiating new activity.
-
-The value for an idle timeout can be asymmetric.  The value advertised by an
-endpoint is only used to determine whether the connection is live at that
-endpoint.  An endpoint that sends packets near the end of the idle timeout
-period of a peer risks having those packets discarded if its peer enters the
-draining state before the packets arrive.  If a peer could timeout within a
-Probe Timeout (PTO; see Section 6.3 of {{QUIC-RECOVERY}}), it is advisable to
-test for liveness before sending any data that cannot be retried safely.  Note
-that it is likely that only applications or application protocols will
-know what information can be retried.
+Each endpoint advertises a different max_idle_timeout, but the effective value
+is determined by taking the minimum of the two advertised values. By announcing
+a max_idle_timeout, endpoints commit to initiating an immediate close
+({{immediate-close}}) if it abandons a connection prior to the effective value.

```suggestion
({{immediate-close}}) if it abandons the connection prior to the effective value.
```

> -since last receiving a packet.  Restarting when sending packets ensures that
-connections do not prematurely time out when initiating new activity.
-
-The value for an idle timeout can be asymmetric.  The value advertised by an
-endpoint is only used to determine whether the connection is live at that
-endpoint.  An endpoint that sends packets near the end of the idle timeout
-period of a peer risks having those packets discarded if its peer enters the
-draining state before the packets arrive.  If a peer could timeout within a
-Probe Timeout (PTO; see Section 6.3 of {{QUIC-RECOVERY}}), it is advisable to
-test for liveness before sending any data that cannot be retried safely.  Note
-that it is likely that only applications or application protocols will
-know what information can be retried.
+Each endpoint advertises a different max_idle_timeout, but the effective value
+is determined by taking the minimum of the two advertised values. By announcing
+a max_idle_timeout, endpoints commit to initiating an immediate close
+({{immediate-close}}) if it abandons a connection prior to the effective value.

I don't understand this sentence... what does it mean for an endpoint to initiate an immediate close if it abandons the connection prior to the min? I thought that the endpoint was to initiate an immediate close _at_ the min. Why are we talking about abandoning?

> -
-The value for an idle timeout can be asymmetric.  The value advertised by an
-endpoint is only used to determine whether the connection is live at that
-endpoint.  An endpoint that sends packets near the end of the idle timeout
-period of a peer risks having those packets discarded if its peer enters the
-draining state before the packets arrive.  If a peer could timeout within a
-Probe Timeout (PTO; see Section 6.3 of {{QUIC-RECOVERY}}), it is advisable to
-test for liveness before sending any data that cannot be retried safely.  Note
-that it is likely that only applications or application protocols will
-know what information can be retried.
+Each endpoint advertises a different max_idle_timeout, but the effective value
+is determined by taking the minimum of the two advertised values. By announcing
+a max_idle_timeout, endpoints commit to initiating an immediate close
+({{immediate-close}}) if it abandons a connection prior to the effective value.
+
+An endpoint restarts any timer it maintains when a packet from its peer is

Do you mean an idle timeout timer? I would say that explicitly. Maybe even give it a name - ITO? 

> -period of a peer risks having those packets discarded if its peer enters the
-draining state before the packets arrive.  If a peer could timeout within a
-Probe Timeout (PTO; see Section 6.3 of {{QUIC-RECOVERY}}), it is advisable to
-test for liveness before sending any data that cannot be retried safely.  Note
-that it is likely that only applications or application protocols will
-know what information can be retried.
+Each endpoint advertises a different max_idle_timeout, but the effective value
+is determined by taking the minimum of the two advertised values. By announcing
+a max_idle_timeout, endpoints commit to initiating an immediate close
+({{immediate-close}}) if it abandons a connection prior to the effective value.
+
+An endpoint restarts any timer it maintains when a packet from its peer is
+received and processed successfully.  The timer is also restarted when sending
+a packet containing frames other than ACK or PADDING (an ack-eliciting packet;
+see {{QUIC-RECOVERY}}), but only if no other ack-eliciting packets have been
+sent since last receiving a packet.  Restarting when sending packets ensures

Can you write some rationale for these restarting rules?

> -know what information can be retried.
+Each endpoint advertises a different max_idle_timeout, but the effective value
+is determined by taking the minimum of the two advertised values. By announcing
+a max_idle_timeout, endpoints commit to initiating an immediate close
+({{immediate-close}}) if it abandons a connection prior to the effective value.
+
+An endpoint restarts any timer it maintains when a packet from its peer is
+received and processed successfully.  The timer is also restarted when sending
+a packet containing frames other than ACK or PADDING (an ack-eliciting packet;
+see {{QUIC-RECOVERY}}), but only if no other ack-eliciting packets have been
+sent since last receiving a packet.  Restarting when sending packets ensures
+that connections do not prematurely time out when initiating new activity.
+
+An endpoint that sends packets near the end of the idle timeout period
+risks having those packets discarded if its peer enters the draining state
+before the packets arrive.  If a peer could time out within an Probe Timeout

This is not a great construction: "If a peer could time out within a Probe Timeout".  It's also confusing ... maybe change this to "When an endpoint is about to send data that cannot be retried safely, it is possible that the peer may have reached the end of its idle timeout period before this data arrives at the peer. To avoid such situations, it is recommended that an endpoint test for liveness when it expects that the peer might be close, within a Probe Timeout (PTO; see Section ...) period for instance, to the end of its idle timeout period."

-: The idle timeout is a value in milliseconds that is encoded as an integer; see
-  ({{idle-timeout}}).  If this parameter is absent or zero then the idle
-  timeout is disabled.
+: The max idle timeout is a value in milliseconds that is encoded as an integer;
+  see ({{idle-timeout}}).  Idle timeout is disabled when this parameter is
+  absent or zero on both sides.

Rephrase -- not both sides, but when the parameter is advertized as zero by both endpoints.

-- 
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/3099#pullrequestreview-310693360

---

From: ianswett <[email-masked]>
Date: Sat, 02 Nov 2019 13:46:51 -0700

a7232f126f9ab0e54d8793a9e555d5fdd7a84b01  Update draft-ietf-quic-transport.md

-- 
You are receiving this because you are subscribed to this thread.
View it on GitHub:
https://github.com/quicwg/base-drafts/pull/3099/files/48f04d6cb6f2b080dc75c00c88242d05d3d77c36..a7232f126f9ab0e54d8793a9e555d5fdd7a84b01

---

From: ianswett <[email-masked]>
Date: Sat, 02 Nov 2019 13:47:48 -0700

ec80c86716185ddc5b2836afdf6ff6908180fa12  Update draft-ietf-quic-transport.md

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

---

From: ianswett <[email-masked]>
Date: Sat, 02 Nov 2019 13:48:00 -0700

cd47d621b7c5de6c7ca504d034338023a117698d  Update draft-ietf-quic-transport.md

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

---

From: ianswett <[email-masked]>
Date: Sat, 02 Nov 2019 13:48:17 -0700

7a6d513d944900a423aa7558d92b5edc949c7a0c  Update draft-ietf-quic-transport.md

-- 
You are receiving this because you are subscribed to this thread.
View it on GitHub:
https://github.com/quicwg/base-drafts/pull/3099/files/cd47d621b7c5de6c7ca504d034338023a117698d..7a6d513d944900a423aa7558d92b5edc949c7a0c

---

From: ianswett <[email-masked]>
Date: Sat, 02 Nov 2019 13:48:29 -0700

0bfcf117ec2ea72d44e63513a0415475df48a2fe  Update draft-ietf-quic-transport.md

-- 
You are receiving this because you are subscribed to this thread.
View it on GitHub:
https://github.com/quicwg/base-drafts/pull/3099/files/7a6d513d944900a423aa7558d92b5edc949c7a0c..0bfcf117ec2ea72d44e63513a0415475df48a2fe

---

From: ianswett <[email-masked]>
Date: Sat, 02 Nov 2019 13:50:41 -0700

ianswett commented on this pull request.

The text from here down is pre-existing, so I'd prefer not to change it as part of this PR.

-- 
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/3099#discussion_r341825531

---

From: ianswett <[email-masked]>
Date: Sat, 02 Nov 2019 13:52:37 -0700

Also pre-existing text, so maybe in a different PR?

-- 
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/3099#discussion_r341825580

---

From: ianswett <[email-masked]>
Date: Sat, 02 Nov 2019 13:53:09 -0700

+Each endpoint advertises a different max idle timeout, but the effective value
+a max idle timeout, endpoints commit to initiating an immediate close

Also pre-existing.

-- 
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/3099#discussion_r341825589

---

From: ianswett <[email-masked]>
Date: Sat, 02 Nov 2019 13:53:38 -0700

Agreed, I'll file an issue to address your and Mike's comments.

-- 
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/3099#discussion_r341825603

---

From: ianswett <[email-masked]>
Date: Sat, 02 Nov 2019 15:49:19 -0700

@MikeBishop and @janaiyengar I filed a new issue(#3184) for your comments on the existing idle timeout text.  I hope all of those comments can be addressed in an editorial PR.

-- 
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/3099#issuecomment-549088064

---

From: ianswett <[email-masked]>
Date: Sat, 02 Nov 2019 16:09:23 -0700

210821bf6cc4376b057d47bc5a2c2dc1fad46b11  Merge branch 'master' into ianswett-symmetric-timeout

-- 
You are receiving this because you are subscribed to this thread.
View it on GitHub:
https://github.com/quicwg/base-drafts/pull/3099/files/0bfcf117ec2ea72d44e63513a0415475df48a2fe..210821bf6cc4376b057d47bc5a2c2dc1fad46b11

---

From: ianswett <[email-masked]>
Date: Sun, 03 Nov 2019 10:44:02 -0800

07d63d916e4738f6b845da81235dbf3ddb2cf93b  Jana's comment

-- 
You are receiving this because you are subscribed to this thread.
View it on GitHub:
https://github.com/quicwg/base-drafts/pull/3099/files/210821bf6cc4376b057d47bc5a2c2dc1fad46b11..07d63d916e4738f6b845da81235dbf3ddb2cf93b

---

From: Martin Thomson <[email-masked]>
Date: Sun, 03 Nov 2019 15:56:07 -0800

martinthomson approved this pull request.

>  current Probe Timeout (PTO).
 
-Each endpoint advertises its own idle timeout to its peer.  An endpoint
-restarts any timer it maintains when a packet from its peer is received and
-processed successfully.  The timer is also restarted when sending an
+Each endpoint advertises a max_idle_timeout, but the effective value
+at an endpoint is computed as the minimum of the two advertised values. By announcing
+a max_idle_timeout, an endpoint commits to initiating an immediate close

> a max_idle_timeout, an endpoint declares that after being idle for longer than the advertised time it will not send any packets and it could generate a stateless reset ({{stateless-reset}}) in reaction to any received packet.  An endpoint commits to initiating an immediate close [...]

I don't think that we need new names.  New names are best when you need to refer to them in many places, but idle timeout logic is all constrained to this one section.

-- 
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/3099#pullrequestreview-310849894

---

From: Marten Seemann <[email-masked]>
Date: Mon, 04 Nov 2019 03:35:11 -0800

marten-seemann commented on this pull request.

> @@ -2343,28 +2343,30 @@ source address.

-If the idle timeout is enabled, a connection is silently closed and the state is
-discarded when it remains idle for longer than both the advertised
-idle timeout (see {{transport-parameter-definitions}}) and three times the
+If the idle timeout is enabled by either peer, a connection is silently closed
+and its state is discarded when it remains idle for longer than the
+max_idle_timeout (see {{transport-parameter-definitions}}) and three times the

This sentence seems to contradict the next paragraph: The connection is closed when it remains idle for longer than the minimum of the max_idle_timeouts.

-: The idle timeout is a value in milliseconds that is encoded as an integer; see
-  ({{idle-timeout}}).  If this parameter is absent or zero then the idle
-  timeout is disabled.
+: The max idle timeout is a value in milliseconds that is encoded as an integer;
+  see ({{idle-timeout}}).  Idle timeout is disabled when both endpoints omit this
+  transport parameteter or specify a value of 0.

In its current wording, this would mean that if one of the endpoints omits the parameter or sets it to 0, the resulting idle timeout would be 0 (since we're taking the minimum of both values).

-- 
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/3099#pullrequestreview-311011596

---

From: Martin Thomson <[email-masked]>
Date: Mon, 04 Nov 2019 15:46:19 -0800

martinthomson commented on this pull request.

I think that we should not interpret 0→∞.  I would prefer that we omit a value when there is none and assign the value an infinite value (or 2^62-1 if you prefer) as a default.

-- 
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/3099#discussion_r342325516

---

From: Jana Iyengar <[email-masked]>
Date: Mon, 04 Nov 2019 16:56:25 -0800

It's a new timeout. It's not covered by any existing timeouts, so calling it out explicitly avoids use of generic language 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/3099#discussion_r342341964

---

From: Jana Iyengar <[email-masked]>
Date: Mon, 04 Nov 2019 18:38:50 -0800

I think the text should be fixed here... why introduce text here and then fix it in a new PR?

-- 
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/3099#discussion_r342361117

---

From: ianswett <[email-masked]>
Date: Tue, 05 Nov 2019 15:30:20 -0800

SG, I'm going to no longer make the default 0.

-- 
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/3099#discussion_r342850882

---

From: ianswett <[email-masked]>
Date: Mon, 18 Nov 2019 05:29:49 -0800

e74856dad49d3a666d4a9b028d549cb48fe9187b  Update draft-ietf-quic-transport.md

-- 
You are receiving this because you are subscribed to this thread.
View it on GitHub:
https://github.com/quicwg/base-drafts/pull/3099/files/07d63d916e4738f6b845da81235dbf3ddb2cf93b..e74856dad49d3a666d4a9b028d549cb48fe9187b

---

From: ianswett <[email-masked]>
Date: Tue, 19 Nov 2019 17:57:51 -0800

581a368bb9fee7d3b72ae56d59492a0134ea4361  Marten's suggestion

-- 
You are receiving this because you are subscribed to this thread.
View it on GitHub:
https://github.com/quicwg/base-drafts/pull/3099/files/e74856dad49d3a666d4a9b028d549cb48fe9187b..581a368bb9fee7d3b72ae56d59492a0134ea4361

---

From: ianswett <[email-masked]>
Date: Tue, 19 Nov 2019 18:00:38 -0800

I'm not introducing this text, it just moved, hence the different PR, which is editorial.

-- 
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/3099#discussion_r348259533

---

From: ianswett <[email-masked]>
Date: Tue, 19 Nov 2019 18:02:22 -0800

875de3f37a1455793d715d481f895cfb32e33aa8  Rebuild

-- 
You are receiving this because you are subscribed to this thread.
View it on GitHub:
https://github.com/quicwg/base-drafts/pull/3099/files/581a368bb9fee7d3b72ae56d59492a0134ea4361..875de3f37a1455793d715d481f895cfb32e33aa8

---

From: ianswett <[email-masked]>
Date: Tue, 19 Nov 2019 18:07:09 -0800

This is failing due to a QPACK error, but I believe the PR is done.

As noted above, I opened #3184 for the many comments about the existing text.  I can take a stab at this, but I'd like to do that separately from this design change.

-- 
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/3099#issuecomment-555802002