Subject: [quicwg/base-drafts] Don't arm the handshake timer if there's no data (#2590)

From: ianswett <[email-masked]>
Date: Fri, 05 Apr 2019 06:41:13 -0700

Fixes the portion of #1964 not fixed in #2281

This could be added to #2281, but that PR already has a lot of comments.
You can view, comment on, or merge this pull request online at:

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

-- Commit Summary --

  * Don't arm the handshake timer if there's no data

-- File Changes --

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

-- Patch Links --

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

---

From: Marten Seemann <[email-masked]>
Date: Fri, 05 Apr 2019 13:47:59 +0000 (UTC)

marten-seemann commented on this pull request.

> @@ -458,11 +458,11 @@ sent, then no alarm should be armed until data has been received from the
 client.
 
 Because the server could be blocked until more packets are received, the client
-MUST start the crypto retransmission timer even if there is no unacknowledged
-CRYPTO data.  If the timer expires and the client has no CRYPTO data to
-retransmit and does not have Handshake keys, it SHOULD send an Initial packet in
-a UDP datagram of at least 1200 bytes.  If the client has Handshake keys, it
-SHOULD send a Handshake packet.
+MUST start the crypto retransmission timer until it has 1RTT keys, even if there

How would a lost CFIN be retransmitted?

-- 
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/2590#pullrequestreview-223284249

---

From: martinduke <[email-masked]>
Date: Fri, 05 Apr 2019 14:24:53 +0000 (UTC)

martinduke commented on this pull request.

My comments are a little broader than the issue, but it's all related.

> @@ -1151,7 +1152,8 @@ OnLossDetectionTimeout():
     DetectLostPackets(pn_space)
   // Retransmit crypto data if no packets were lost
   // and there are still crypto packets in flight.
-  else if (crypto packets are in flight):
+  else if (crypto packets are in flight &&
+           crypto data to send):
     // Crypto retransmission timeout.
     RetransmitUnackedCryptoData()
     crypto_count++

else if (client && (no 1-RTT keys)
    send a 1200B packet
or something to that effect

> @@ -1114,7 +1114,8 @@ SetLossDetectionTimer():
     loss_detection_timer.update(loss_time)
     return
 
-  if (crypto packets are in flight):
+  if (crypto packets are in flight &&
+      crypto data to send):

|| (client && (no 1-RTT keys yet))

 client.

MUST start the crypto retransmission timer upon timeout....

In line 448: ack-eliciting crypto packet?

LIne 451: "if possible, and restart the crypto retransmission timer if there is unacknowledged data"

 client.
 
 Because the server could be blocked until more packets are received, the client
-MUST start the crypto retransmission timer even if there is no unacknowledged
-CRYPTO data.  If the timer expires and the client has no CRYPTO data to
-retransmit and does not have Handshake keys, it SHOULD send an Initial packet in
-a UDP datagram of at least 1200 bytes.  If the client has Handshake keys, it
-SHOULD send a Handshake packet.
+MUST start the crypto retransmission timer until it has 1RTT keys, even if there
+is no unacknowledged CRYPTO data.  If the timer expires and the client has no
+CRYPTO data to retransmit and does not have Handshake keys, it SHOULD send an

SHOULD -> MUST? If we don't do this, the connection could hang in a fairly common case.

-- 
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/2590#pullrequestreview-223296742

---

From: martinduke <[email-masked]>
Date: Fri, 05 Apr 2019 14:29:17 +0000 (UTC)

client.

There would be outstanding CRYPTO data, so it would restart the timer and then send.

-- 
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/2590#discussion_r272613388

---

From: ianswett <[email-masked]>
Date: Fri, 05 Apr 2019 07:29:22 -0700

@ianswett pushed 1 commit.

8f015e3512baf444100a1c1bd7cacfb8ae657bf0  MUST send

-- 
You are receiving this because you are subscribed to this thread.
View it on GitHub:
https://github.com/quicwg/base-drafts/pull/2590/files/308a6137527715a16abe1278043ea053688bb110..8f015e3512baf444100a1c1bd7cacfb8ae657bf0

---

From: martinduke <[email-masked]>
Date: Fri, 05 Apr 2019 07:30:20 -0700

client.

ah you beat me to 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/2590#discussion_r272613827

---

From: Marten Seemann <[email-masked]>
Date: Fri, 05 Apr 2019 07:37:51 -0700

client.

You're right, but it's kind of hard to understand.
What about the following structure: "MUST start the crypto retransmission timer if there is outstanding CRYPTO data. It MUST also start the crypto retransmission timer until it has 1-RTT keys,..."

-- 
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/2590#discussion_r272616981

---

From: ianswett <[email-masked]>
Date: Fri, 05 Apr 2019 07:48:55 -0700

d9f257ba0ef829e0786cb154dd6e31ae04bc5383  Martin and Marten's comments

-- 
You are receiving this because you are subscribed to this thread.
View it on GitHub:
https://github.com/quicwg/base-drafts/pull/2590/files/8f015e3512baf444100a1c1bd7cacfb8ae657bf0..d9f257ba0ef829e0786cb154dd6e31ae04bc5383

---

From: ianswett <[email-masked]>
Date: Fri, 05 Apr 2019 07:49:24 -0700

48fa157a8eb2085dcd0f7b0f0efeb4d1e7297527  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/2590/files/d9f257ba0ef829e0786cb154dd6e31ae04bc5383..48fa157a8eb2085dcd0f7b0f0efeb4d1e7297527

---

From: ianswett <[email-masked]>
Date: Fri, 05 Apr 2019 07:50:16 -0700

24a55d424d20cc7fabd4ac846891689796218a43  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/2590/files/48fa157a8eb2085dcd0f7b0f0efeb4d1e7297527..24a55d424d20cc7fabd4ac846891689796218a43

---

From: ianswett <[email-masked]>
Date: Fri, 05 Apr 2019 07:50:53 -0700

ianswett commented on this pull request.

Thanks, I think this is closer.  I can also try to pull in all the changes from the other PR into this one if it makes this easier?

 client.

It's not just timeout(ie: it needs to be set after an ACK is received), but I'll try to clarify.

line 448: The definition of crypto packet is that it has a CRYPTO frame in it, so it's always ack-eliciting.

Retransmitting the crypto data would naturally update the timer(see previous sentence) and there would be unacknowledged crypto data in that case, so I'm not sure your extra clause is clarifying or potentially confusing?

That's in the the other PR: #2281 

Also in the other 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/2590#pullrequestreview-223286807

---

From: martinduke <[email-masked]>
Date: Fri, 05 Apr 2019 07:51:56 -0700

> @@ -435,7 +435,9 @@ Data in CRYPTO frames is critical to QUIC transport and crypto negotiation, so a
 more aggressive timeout is used to retransmit it.
 
 The initial crypto retransmission timeout SHOULD be set to twice the initial
-RTT.
+RTT.  On each consecutive expiration of the crypto timer without receiving an
+acknowledgement for a new packet, the sender SHOULD double the crypto
+retransmission timeout and set a timer for this period.

Not quite right. If we got some acknowledgments, we still need to restart the timer. And note the pseudocode comments 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/2590#pullrequestreview-223320317

---

From: ianswett <[email-masked]>
Date: Fri, 05 Apr 2019 07:58:45 -0700

This was just a reorg.  This text is about doubling the timer after expiry when no ACKs are received.

I think the pseudocode comments are all addressed in the other 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/2590#discussion_r272626054

---

From: martinduke <[email-masked]>
Date: Fri, 05 Apr 2019 15:00:26 +0000 (UTC)

client.

You're right, please disregard.

-- 
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/2590#discussion_r272626777

---

From: martinduke <[email-masked]>
Date: Fri, 05 Apr 2019 15:13:43 +0000 (UTC)

martinduke approved this pull request.

Alright, I'm down to two issues, one of which is entirely trivial.

> @@ -458,17 +460,15 @@ sent, then no alarm should be armed until data has been received from the
 client.
 
 Because the server could be blocked until more packets are received, the client
-MUST start the crypto retransmission timer even if there is no unacknowledged
-CRYPTO data.  If the timer expires and the client has no CRYPTO data to
-retransmit and does not have Handshake keys, it SHOULD send an Initial packet in
-a UDP datagram of at least 1200 bytes.  If the client has Handshake keys, it
-SHOULD send a Handshake packet.
-
-On each consecutive expiration of the crypto timer without receiving an
-acknowledgement for a new packet, the sender SHOULD double the crypto
-retransmission timeout and set a timer for this period.
+MUST ensure the crypto retransmission timer is set if there is unacknowledged
+crypto data and MUST ensure  the timer is set until it has 1RTT keys.

double space

Ah, OK. Line 448 probably should read "When an acknowledgment is received, a new RTT is computed. The endpoint MUST restart the timer and SHOULD set it for twice the newly computed smoothed 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/2590#pullrequestreview-223330867

---

From: ianswett <[email-masked]>
Date: Fri, 05 Apr 2019 08:32:33 -0700

15117ecb647d95014e8964b072c6eede69bc227f  Martin and Ian's comments

-- 
You are receiving this because you are subscribed to this thread.
View it on GitHub:
https://github.com/quicwg/base-drafts/pull/2590/files/24a55d424d20cc7fabd4ac846891689796218a43..15117ecb647d95014e8964b072c6eede69bc227f

---

From: ianswett <[email-masked]>
Date: Fri, 05 Apr 2019 15:35:32 +0000 (UTC)

1eb61596fa9f0fd66272d6007f9c664f90a53f6e  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/2590/files/15117ecb647d95014e8964b072c6eede69bc227f..1eb61596fa9f0fd66272d6007f9c664f90a53f6e

---

From: ianswett <[email-masked]>
Date: Fri, 05 Apr 2019 08:38:26 -0700

5cfa8cc5fbee5b0302ad027655a4d6a00e595391  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/2590/files/1eb61596fa9f0fd66272d6007f9c664f90a53f6e..5cfa8cc5fbee5b0302ad027655a4d6a00e595391

---

From: ianswett <[email-masked]>
Date: Fri, 05 Apr 2019 08:42:01 -0700

4d31a23b5847d7a9b8f8edbdee4f7ca7a9bacc3f  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/2590/files/5cfa8cc5fbee5b0302ad027655a4d6a00e595391..4d31a23b5847d7a9b8f8edbdee4f7ca7a9bacc3f

---

From: martinduke <[email-masked]>
Date: Fri, 05 Apr 2019 08:49:49 -0700

Ship 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/2590#pullrequestreview-223351069

---

From: Marten Seemann <[email-masked]>
Date: Sun, 07 Apr 2019 20:43:29 -0700

> @@ -458,17 +462,16 @@ sent, then no alarm should be armed until data has been received from the
 client.
 
 Because the server could be blocked until more packets are received, the client
-MUST start the crypto retransmission timer even if there is no unacknowledged
-CRYPTO data.  If the timer expires and the client has no CRYPTO data to
-retransmit and does not have Handshake keys, it SHOULD send an Initial packet in
-a UDP datagram of at least 1200 bytes.  If the client has Handshake keys, it
-SHOULD send a Handshake packet.
-
-On each consecutive expiration of the crypto timer without receiving an
-acknowledgement for a new packet, the sender SHOULD double the crypto
-retransmission timeout and set a timer for this period.
+MUST ensure the crypto retransmission timer is set if there is unacknowledged
+crypto data and MUST ensure the timer is set until it has 1-RTT keys.
+If the timer expires and the client has no CRYPTO data to retransmit and does
+not have Handshake keys, it MUST send an Initial packet in a UDP datagram of
+at least 1200 bytes.  If the client has Handshake keys, it MUST send a

Why would the client send a Handshake packet if there’s no CRYPTO data to retransmit? If there’s no Handshake CRYPTO data to retransmit, this means that the server already received the client‘s TLS messages sent with Handshake keys, and won’t be blocked by the 3x Limit any more, doesn’t 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/2590#pullrequestreview-223632964

---

From: martinduke <[email-masked]>
Date: Mon, 08 Apr 2019 00:41:41 -0700

If the server first flight does not include finished and the client
handshake ack is lost, the client has no unacked data but the server is
flow control limited.

On Sun, Apr 7, 2019, 20:43 Marten Seemann <[email-masked]> wrote:

> *@marten-seemann* commented on this pull request.
> ------------------------------
>
> In draft-ietf-quic-recovery.md
> <https://github.com/quicwg/base-drafts/pull/2590#discussion_r272877569>:
>
> Why would the client send a Handshake packet if there’s no CRYPTO data to
> retransmit? If there’s no Handshake CRYPTO data to retransmit, this means
> that the server already received the client‘s TLS messages sent with
> Handshake keys, and won’t be blocked by the 3x Limit any more, doesn’t it?
>
> —
> You are receiving this because your review was requested.
> Reply to this email directly, view it on GitHub
> <https://github.com/quicwg/base-drafts/pull/2590#pullrequestreview-223632964>,
> or mute the thread
> <https://github.com/notifications/unsubscribe-auth/AXRMEZLB_4dnuds0LhuvnVg5c60SndfPks5verrhgaJpZM4ce8zS>
> .
>

-- 
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/2590#issuecomment-480720320

---

From: ianswett <[email-masked]>
Date: Mon, 08 Apr 2019 13:37:13 +0000 (UTC)

client.

I believe the particular concern is that the client could have Handshake keys because they receive the server's Initial, but not receive the server's Handshake packet, so have nothing to send in Handshake encryption.

If the client ever had CRYPTO data to send in Handshake, then this would not be a concern, as you state.

-- 
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/2590#discussion_r273049190

---

From: Jana Iyengar <[email-masked]>
Date: Mon, 08 Apr 2019 14:44:26 -0700

janaiyengar commented on this pull request.

> @@ -442,13 +442,17 @@ connections over the same network SHOULD use the previous connection's final
 smoothed RTT value as the resumed connection's initial RTT.  If no previous RTT
 is available, or if the network changes, the initial RTT SHOULD be set to 500ms,
 resulting in a 1 second initial handshake timeout as recommended in
-{{?RFC6298}}. When an acknowledgement is received, a new RTT is computed and the
-timer SHOULD be set for twice the newly computed smoothed RTT.
+{{?RFC6298}}. When the first acknowledgement is received, an RTT is computed and
+the timer SHOULD be set for twice the newly computed RTT.

How about "When the first acknowledgement is received, the timer SHOULD be set for twice the value of the resulting RTT sample"? Now that I see further below, why is this necessary at all? I would remove this last sentence. The beginning of the next para says exactly this, since the smoothed_rtt is set to the RTT sample on the first ack.

-When a crypto packet is sent, the sender MUST set a timer for the crypto
-timeout period.  This timer MUST be updated when a new crypto packet is sent.
-Upon timeout, the sender MUST retransmit all unacknowledged CRYPTO data if
-possible.
+When a crypto packet is sent, the sender MUST set a timer for twice the smoothed
+RTT.  This timer MUST be updated when a new crypto packet is sent and when
+an acknowledgement is received which computes a new RTT sample. Upon timeout,
+the sender MUST retransmit all unacknowledged CRYPTO data if possible.

Why "if possible"?

-When a crypto packet is sent, the sender MUST set a timer for the crypto
-timeout period.  This timer MUST be updated when a new crypto packet is sent.
-Upon timeout, the sender MUST retransmit all unacknowledged CRYPTO data if
-possible.
+When a crypto packet is sent, the sender MUST set a timer for twice the smoothed
+RTT.  This timer MUST be updated when a new crypto packet is sent and when
+an acknowledgement is received which computes a new RTT sample. Upon timeout,
+the sender MUST retransmit all unacknowledged CRYPTO data if possible.
+
+On each consecutive expiration of the crypto timer without receiving an
+acknowledgement for a new packet, the sender SHOULD double the crypto
+retransmission timeout and set a timer for this period.

I'd make this a MUST.

 client.

This needs rephrasing. How about "MUST ensure that the crypto retransmission timer is set if there is unacknowledged crypto data or if the client does not yet have 1-RTT keys."

 client.
 
+Handshake packet.

Suggested reword: "If the crypto retransmission timer expires before the client has 1-RTT keys, it is possible that the client may not have any crypto data to retransmit. The client MUST still send a new packet to allow the server to continue sending data (see Section XX in {{QUIC-TRANSPORT}}).  If Handshake keys are available to the client, it MUST send a Handshake packet, and otherwise, an Initial packet in a UDP datagram of at least 1200 bytes."

> @@ -1114,7 +1117,8 @@ SetLossDetectionTimer():

what does this predicate mean? That there's new data to send?

-- 
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/2590#pullrequestreview-224097674

---

From: ianswett <[email-masked]>
Date: Tue, 09 Apr 2019 20:45:49 +0000 (UTC)

SG, removed.

-- 
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/2590#discussion_r273692311

---

From: ianswett <[email-masked]>
Date: Tue, 09 Apr 2019 20:46:20 +0000 (UTC)

It can't if the anti-amplification limit limits 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/2590#discussion_r273692496

---

From: ianswett <[email-masked]>
Date: Tue, 09 Apr 2019 13:49:57 -0700

It's supposed to mean that there's unacknowledged crypto data.  Should I say "unacknowledged crypto data" instead?

-- 
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/2590#discussion_r273693872

---

From: ianswett <[email-masked]>
Date: Tue, 09 Apr 2019 13:56:57 -0700

77a01dcc5cd8192cea242036315f2218d024a467  Jana's comments

-- 
You are receiving this because you are subscribed to this thread.
View it on GitHub:
https://github.com/quicwg/base-drafts/pull/2590/files/4d31a23b5847d7a9b8f8edbdee4f7ca7a9bacc3f..77a01dcc5cd8192cea242036315f2218d024a467

---

From: ianswett <[email-masked]>
Date: Wed, 10 Apr 2019 12:05:33 -0700

5a1e2480b7ef450531899b266e5a06b9e53d0a96  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/2590/files/77a01dcc5cd8192cea242036315f2218d024a467..5a1e2480b7ef450531899b266e5a06b9e53d0a96

---

From: ianswett <[email-masked]>
Date: Wed, 10 Apr 2019 12:10:51 -0700

9ddde76fb29baefc476b9f991014afc94a7ab4dd  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/2590/files/5a1e2480b7ef450531899b266e5a06b9e53d0a96..9ddde76fb29baefc476b9f991014afc94a7ab4dd

---

From: ianswett <[email-masked]>
Date: Wed, 10 Apr 2019 12:13:09 -0700

client.

Done, with some minor editing.

-- 
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/2590#discussion_r274116034

---

From: ianswett <[email-masked]>
Date: Wed, 10 Apr 2019 19:13:53 +0000 (UTC)

I removed this from here due to the other PR, but left it in below.  Feel free to suggest alternate phrasing.

-- 
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/2590#discussion_r274116296

---

From: Jana Iyengar <[email-masked]>
Date: Wed, 10 Apr 2019 19:40:16 -0700

> @@ -458,17 +463,18 @@ sent, then no alarm should be armed until data has been received from the
 client.
 
 Because the server could be blocked until more packets are received, the client
-MUST start the crypto retransmission timer even if there is no unacknowledged
-CRYPTO data.  If the timer expires and the client has no CRYPTO data to
-retransmit and does not have Handshake keys, it SHOULD send an Initial packet in
-a UDP datagram of at least 1200 bytes.  If the client has Handshake keys, it
-SHOULD send a Handshake packet.
-
-On each consecutive expiration of the crypto timer without receiving an
-acknowledgement for a new packet, the sender SHOULD double the crypto
-retransmission timeout and set a timer for this period.
+MUST ensure that the crypto retransmission timer is set if there is
+unacknowledged crypto data or if the client does not yet have 1-RTT keys.
+If the crypto retransmission timer expires before the client has 1-RTT keys,
+it is possible that the client may not have any crypto data to retransmit.
+The client MUST send a new packet to allow the server to continue sending

```suggestion
However, the client MUST send a new packet, containing only PING or PADDDING frames if necessary, to allow the server to continue sending
```

-When a crypto packet is sent, the sender MUST set a timer for the crypto
-timeout period.  This timer MUST be updated when a new crypto packet is sent.
-Upon timeout, the sender MUST retransmit all unacknowledged CRYPTO data if
-possible.
+When a crypto packet is sent, the sender MUST set a timer for twice the smoothed
+RTT.  This timer MUST be updated when a new crypto packet is sent and when
+an acknowledgement is received which computes a new RTT sample. Upon timeout,
+the sender MUST retransmit all unacknowledged CRYPTO data if possible.  The
+sender MUST NOT declare in flight crypto packets as lost when the crypto timer

```suggestion
sender MUST NOT declare in-flight crypto packets as lost when the crypto timer
```

> @@ -1151,7 +1158,8 @@ OnLossDetectionTimeout():

I still don't get this. If crypto packets are in flight, doesn't that mean there is unacked crypto data to send? Why is this second predicate useful?

-- 
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/2590#pullrequestreview-225302634

---

From: ianswett <[email-masked]>
Date: Thu, 11 Apr 2019 17:27:23 +0000 (UTC)

6e204a084b859025041dde2497e153ee781f2d70  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/2590/files/9ddde76fb29baefc476b9f991014afc94a7ab4dd..6e204a084b859025041dde2497e153ee781f2d70

---

From: ianswett <[email-masked]>
Date: Thu, 11 Apr 2019 17:28:07 +0000 (UTC)

0f9d4ecd0b6835294a000494a5ce1d3ef6093158  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/2590/files/6e204a084b859025041dde2497e153ee781f2d70..0f9d4ecd0b6835294a000494a5ce1d3ef6093158

---

From: ianswett <[email-masked]>
Date: Thu, 11 Apr 2019 17:28:46 +0000 (UTC)

Crypto packets could be in flight because of a timer based retransmission, but the original was ACked.

-- 
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/2590#discussion_r274543618

---

From: ianswett <[email-masked]>
Date: Thu, 11 Apr 2019 10:29:57 -0700

See issue #1964

-- 
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/2590#discussion_r274544063

---

From: ianswett <[email-masked]>
Date: Thu, 11 Apr 2019 12:36:33 -0700

014a0acc57053f150726f66f3deae74da6a83f61  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/2590/files/0f9d4ecd0b6835294a000494a5ce1d3ef6093158..014a0acc57053f150726f66f3deae74da6a83f61

---

From: Jana Iyengar <[email-masked]>
Date: Thu, 11 Apr 2019 16:55:07 -0700

Ah, ok. How about just "unacked crypto data exists"? Is that enough to capture 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/2590#discussion_r274715535

---

From: Jana Iyengar <[email-masked]>
Date: Thu, 11 Apr 2019 16:56:41 -0700

I meant using just this one condition instead of the two that you have.

-- 
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/2590#discussion_r274715805

---

From: Benjamin Saunders <[email-masked]>
Date: Thu, 11 Apr 2019 17:34:23 -0700

Ralith commented on this pull request.

I agree that this wording is confusing; in particular, it's not obvious that "to send" means "not yet acknowledged," except perhaps by process of elimination.

-- 
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/2590#discussion_r274726184

---

From: ianswett <[email-masked]>
Date: Fri, 12 Apr 2019 05:04:24 -0700

Sure, I took Jana's suggestion.

-- 
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/2590#discussion_r274876009

---

From: ianswett <[email-masked]>
Date: Fri, 12 Apr 2019 05:08:35 -0700

f00125a83c551b8cad39c2f3c887b60093a5abc3  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/2590/files/014a0acc57053f150726f66f3deae74da6a83f61..f00125a83c551b8cad39c2f3c887b60093a5abc3

---

From: ianswett <[email-masked]>
Date: Fri, 12 Apr 2019 07:24:24 -0700

569a48a0bd8722df3ddd08aaa0d09c22b2452ff1  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/2590/files/f00125a83c551b8cad39c2f3c887b60093a5abc3..569a48a0bd8722df3ddd08aaa0d09c22b2452ff1

---

From: ianswett <[email-masked]>
Date: Fri, 12 Apr 2019 14:27:36 +0000 (UTC)

402281d7f88a9796b894b0de3e8535f9b6ee5f41  Merge branch 'master' into ianswett-no-crypto-data

-- 
You are receiving this because you are subscribed to this thread.
View it on GitHub:
https://github.com/quicwg/base-drafts/pull/2590/files/569a48a0bd8722df3ddd08aaa0d09c22b2452ff1..402281d7f88a9796b894b0de3e8535f9b6ee5f41

---

From: ianswett <[email-masked]>
Date: Fri, 12 Apr 2019 14:34:08 +0000 (UTC)

7e2f58d7cde5c60d4173f98576d57ab131f97556  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/2590/files/402281d7f88a9796b894b0de3e8535f9b6ee5f41..7e2f58d7cde5c60d4173f98576d57ab131f97556

---

From: Jana Iyengar <[email-masked]>
Date: Fri, 12 Apr 2019 17:37:47 -0700

janaiyengar approved this pull request.

Thanks for working through this, @ianswett!

-- 
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/2590#pullrequestreview-226329722

---

From: Jana Iyengar <[email-masked]>
Date: Sat, 13 Apr 2019 00:37:57 +0000 (UTC)

Merged #2590 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/2590#event-2273837277