Subject: [quicwg/base-drafts] the crypto timeout is based on the last crypto packet sent (#2456)

From: Marten Seemann <[email-masked]>
Date: Tue, 12 Feb 2019 01:42:00 +0000 (UTC)

>From reading the text, it was not clear that the timer is set based on the last (and not the first) crypto packet sent.
You can view, comment on, or merge this pull request online at:

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

-- Commit Summary --

  * the crypto timeout is based on the last crypto packet sent

-- File Changes --

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

-- Patch Links --

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

---

From: ianswett <[email-masked]>
Date: Tue, 12 Feb 2019 18:46:44 +0000 (UTC)

ianswett commented on this pull request.

> @@ -412,7 +412,7 @@ is available, or if the network changes, the initial RTT SHOULD be set to 100ms.
 When an acknowledgement is received, a new RTT is computed and the timer
 SHOULD be set for twice the newly computed smoothed RTT.
 
-When crypto packets are sent, the sender MUST set a timer for the crypto
+Every time a crypto packets is sent, the sender MUST set a timer for the crypto

I'm not sure this is significantly clearer, but I do understand what you're trying to address.

```suggestion
Every time a crypto packet is sent, the sender MUST set a timer for the crypto
```

-- 
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/2456#pullrequestreview-202842476

---

From: Marten Seemann <[email-masked]>
Date: Tue, 12 Feb 2019 17:30:03 -0800

@marten-seemann pushed 1 commit.

186160860e6d047a6e4ab2bea5001f4b220b16fa  Ian’s suggestion

-- 
You are receiving this because you are subscribed to this thread.
View it on GitHub:
https://github.com/quicwg/base-drafts/pull/2456/files/b8de486e078aa7a8d115fe056e2f2aadc0596246..186160860e6d047a6e4ab2bea5001f4b220b16fa

---

From: ianswett <[email-masked]>
Date: Wed, 13 Feb 2019 05:49:19 -0800

-When crypto packets are sent, the sender MUST set a timer for the crypto
+Every time a crypto packet is sent, the sender MUST set a timer for the crypto

How about "A timer MUST be updated to expire after the crypto timeout period after a new crypto packet is sent"?

-- 
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/2456#pullrequestreview-203217828

---

From: MikkelFJ <[email-masked]>
Date: Wed, 13 Feb 2019 09:41:19 -0800

mikkelfj commented on this pull request.

> If no previous RTT is available,
-->
If no previous RTT, or a-priori estimate, is available,

-- 
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/2456#discussion_r256509825

---

From: ianswett <[email-masked]>
Date: Wed, 13 Feb 2019 21:12:56 +0000 (UTC)

@mikkelfj is this suggestion about a different paragraph?

-- 
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/2456#discussion_r256590875

---

From: MikkelFJ <[email-masked]>
Date: Wed, 13 Feb 2019 13:17:28 -0800

yes, I should have been more clear. It is existing text in the preceding paragraph, or thereabout.

-- 
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/2456#discussion_r256592409

---

From: Jana Iyengar <[email-masked]>
Date: Tue, 19 Feb 2019 15:33:07 -0800

@janaiyengar pushed 1 commit.

4d20e6cef061b54a673987f1031c358d730a544d  editorial reword

-- 
You are receiving this because you are subscribed to this thread.
View it on GitHub:
https://github.com/quicwg/base-drafts/pull/2456/files/186160860e6d047a6e4ab2bea5001f4b220b16fa..4d20e6cef061b54a673987f1031c358d730a544d

---

From: Jana Iyengar <[email-masked]>
Date: Tue, 19 Feb 2019 15:33:31 -0800

janaiyengar approved this pull request.

LGTM. I've reworded a bit.

-- 
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/2456#pullrequestreview-205509287

---

From: ianswett <[email-masked]>
Date: Tue, 19 Feb 2019 15:49:55 -0800

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/2456#pullrequestreview-205513750

---

From: Jana Iyengar <[email-masked]>
Date: Tue, 19 Feb 2019 16:01:47 -0800

Merged #2456 into master.

-- 
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/2456#event-2150489581