Subject: [quicwg/base-drafts] Clarify time threshold vs crypto timer (#2620)

From: ianswett <[email-masked]>
Date: Mon, 15 Apr 2019 07:02:49 -0700

Adds some explanation about why it's better to arm/fire the time threshold alarm before the crypto retransmission timeout.

Hopefully clarifies comments from #2565
You can view, comment on, or merge this pull request online at:

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

-- Commit Summary --

  * Clarify time threshold vs crypto timer

-- File Changes --

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

-- Patch Links --

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

---

From: Marten Seemann <[email-masked]>
Date: Mon, 15 Apr 2019 07:32:23 -0700

marten-seemann 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/2620#pullrequestreview-226683273

---

From: Yang Chi <[email-masked]>
Date: Mon, 15 Apr 2019 09:01:24 -0700

Thanks for the explanation. It's much clearer.
Can i ask for one more clarification? In DetectLostPackets, shouldn't the for-loop have been something like this:
`foreach unacked in sent_packets[pn_space]:`

-- 
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/2620#issuecomment-483313908

---

From: ianswett <[email-masked]>
Date: Mon, 15 Apr 2019 16:47:14 +0000 (UTC)

@ianswett pushed 1 commit.

b3a333888dd11ddee03db628662f5ff9f7021ab2  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/2620/files/3a0c9b5bbd68d4fc234f9f0f7cae80955bf2b60a..b3a333888dd11ddee03db628662f5ff9f7021ab2

---

From: ianswett <[email-masked]>
Date: Mon, 15 Apr 2019 09:47:49 -0700

Thanks for catching the pseudocode error in DetectLostPackets.  PTAL.

-- 
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/2620#issuecomment-483330590

---

From: Yang Chi <[email-masked]>
Date: Mon, 15 Apr 2019 11:02:47 -0700

lgtm. thank you!

-- 
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/2620#issuecomment-483357160

---

From: Jana Iyengar <[email-masked]>
Date: Mon, 15 Apr 2019 14:03:55 -0700

janaiyengar commented on this pull request.

A couple of comments.

> @@ -537,8 +537,13 @@ and otherwise it MUST send an Initial packet in a UDP datagram of at least
 1200 bytes.
 
 The crypto retransmission timer is not set if the time threshold
-{{time-threshold}} loss detection timer is set.  When the crypto
-retransmission timer is active, the probe timer ({{pto}}) is not active.
+{{time-threshold}} loss detection timer is set.  The time threshold loss
+detection timer is both expected to expire earlier than the crypto

```suggestion
detection timer is expected to both expire earlier than the crypto
```

> @@ -537,8 +537,13 @@ and otherwise it MUST send an Initial packet in a UDP datagram of at least
 1200 bytes.
 
 The crypto retransmission timer is not set if the time threshold
-{{time-threshold}} loss detection timer is set.  When the crypto
-retransmission timer is active, the probe timer ({{pto}}) is not active.
+{{time-threshold}} loss detection timer is set.  The time threshold loss
+detection timer is both expected to expire earlier than the crypto

Can you explain what this expectation is based on?

> @@ -537,8 +537,13 @@ and otherwise it MUST send an Initial packet in a UDP datagram of at least
 1200 bytes.
 
 The crypto retransmission timer is not set if the time threshold
-{{time-threshold}} loss detection timer is set.  When the crypto
-retransmission timer is active, the probe timer ({{pto}}) is not active.
+{{time-threshold}} loss detection timer is set.  The time threshold loss
+detection timer is both expected to expire earlier than the crypto
+retransmission timeout and be much less likely to spuriously retransmit data.
+The Initial and Handshake packet number spaces are typically going to have a

I'm not sure what the number of packets in these spaces has to do with the description here about crypto timeout. Why is this sentence 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/2620#pullrequestreview-226863361

---

From: ianswett <[email-masked]>
Date: Mon, 15 Apr 2019 14:31:18 -0700

918fe38b0957befc00ff5f21b085b8da38b28d4f  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/2620/files/b3a333888dd11ddee03db628662f5ff9f7021ab2..918fe38b0957befc00ff5f21b085b8da38b28d4f

---

From: ianswett <[email-masked]>
Date: Mon, 15 Apr 2019 14:32:22 -0700

ianswett commented on this pull request.

 1200 bytes.

If the number of packets is small, it's unlikely you'll hit the packet threshold loss detection, making time threshold much more important.

-- 
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/2620#discussion_r275553163

---

From: ianswett <[email-masked]>
Date: Mon, 15 Apr 2019 14:34:37 -0700

3197adb9e62157472762259bb4af3949f0f41399  Simplify text

-- 
You are receiving this because you are subscribed to this thread.
View it on GitHub:
https://github.com/quicwg/base-drafts/pull/2620/files/918fe38b0957befc00ff5f21b085b8da38b28d4f..3197adb9e62157472762259bb4af3949f0f41399

---

From: ianswett <[email-masked]>
Date: Mon, 15 Apr 2019 14:35:10 -0700

d41f6810c2dd0d8be8d67ba34a5e0fbec4d9e9fc  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/2620/files/3197adb9e62157472762259bb4af3949f0f41399..d41f6810c2dd0d8be8d67ba34a5e0fbec4d9e9fc

---

From: ianswett <[email-masked]>
Date: Mon, 15 Apr 2019 21:36:00 +0000 (UTC)

1200 bytes.

I tightened up the text, but feel free to suggest further clarification.

-- 
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/2620#discussion_r275554274

---

From: Jana Iyengar <[email-masked]>
Date: Mon, 15 Apr 2019 19:08:02 -0700

> @@ -537,8 +537,15 @@ and otherwise it MUST send an Initial packet in a UDP datagram of at least
 1200 bytes.
 
 The crypto retransmission timer is not set if the time threshold
-{{time-threshold}} loss detection timer is set.  When the crypto
-retransmission timer is active, the probe timer ({{pto}}) is not active.
+{{time-threshold}} loss detection timer is set.  The time threshold loss
+detection timer is expected to both expire earlier than the crypto
+retransmission timeout and be less likely to spuriously retransmit data.
+The Initial and Handshake packet number spaces typically have a small number
+of packets in them, so time threshold loss detection will typically declare
+packets lost before packet threshold.

I think what you want to say is that time-threshold is more likely to be used for loss detection than packet-threshold, since there just might not be enough packets. So how about just this, given that you've just talked about time-threshold: "The Initial and Handshake packet number spaces will typically carry a small number of packets, so losses are less likely to be detected using packet-threshold detection."

That said, I still am not sure that it's worth mentioning the packet-threshold here at all.  But my opinion is not strong, so your call.

-- 
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/2620#pullrequestreview-226956695

---

From: ianswett <[email-masked]>
Date: Tue, 16 Apr 2019 10:41:39 +0000 (UTC)

1200 bytes.

Thanks, suggestion taken.  I think why you'd arm the loss-detection timer is non-obvious, so I thought more explanatory text was better than less, and I think the small number of packets argument is relevant.

-- 
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/2620#discussion_r275736025

---

From: ianswett <[email-masked]>
Date: Tue, 16 Apr 2019 03:42:24 -0700

f3364ca1bf45d42268c2f38e51f2bc97b82901ef  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/2620/files/d41f6810c2dd0d8be8d67ba34a5e0fbec4d9e9fc..f3364ca1bf45d42268c2f38e51f2bc97b82901ef

---

From: Jana Iyengar <[email-masked]>
Date: Tue, 16 Apr 2019 15:45:37 -0700

Merged #2620 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/2620#event-2281153770