Subject: [quicwg/base-drafts] Reword Immediate ACK Recommendation (#2424)

From: martinduke <[email-masked]>
Date: Tue, 05 Feb 2019 16:04:54 -0800

This is a reword of #2025, as discussed with @ianswett previously.

It cleans up two problems:
1) Strictly speaking, the MAY in the original text is redundant, because receivers MAY do anything except exceed the delayed ack timeout. I think it reads better now.
2) @martinthomson mentioned that the spec was a little ambiguous about how to handle additional OOO packets during the interval. I put down some language, but I'm not sure it's what @ianswett would recommend.

You can view, comment on, or merge this pull request online at:

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

-- Commit Summary --

  * Reword Immediate ACK Recommendation

-- File Changes --

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

-- Patch Links --

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

---

From: Martin Thomson <[email-masked]>
Date: Tue, 05 Feb 2019 16:49:50 -0800

martinthomson commented on this pull request.

I think that this is better, though I'd like to confirm design intent.

> @@ -134,6 +134,11 @@ Crypto Packets:
 
 : Packets containing CRYPTO data sent in Initial or Handshake
   packets.
+  
+Out-of-order Packets:
+
+: Packets that do not increase the largest received packet number for its
+cryptographic context by exactly one.

Do you want to add: "Packets can arrive out of order as a result of packets ahead of them being lost or delayed."  I think that's accurate.

> @@ -244,11 +249,9 @@ ack-eliciting packet. QUIC recovery algorithms do not assume the peer sends
 an ACK immediately when receiving a second ack-eliciting packet.
 
 In order to accelerate loss recovery and reduce timeouts, the receiver SHOULD
-send an immediate ACK when it receives a new packet which is not one greater
-than the largest received packet number. A receiver MAY send immediate ACKs
-for the next few ack-eliciting packets that are received, but SHOULD NOT
-send an immediate ACK for more than 1/8 RTT after receiving an out-of-order
-packet.
+send immediate ACKs for an interval after it receives an out-of-order packet.
+This interval SHOULD NOT exceed 1/8 RTT unless more out-of-order packets arrive
+during the interval.

So the idea is that every out-of-order packet triggers a 1/8 RTT interval of immediate acknowledgments?

I thought that the idea was to disable the sending of immediate acknowledgments if out-of-order packets started becoming routine.  That is, if you have a highly unreliable path, then immediate acknowledgments aren't helping repair errors, they are just noise.

-- 
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/2424#pullrequestreview-200374387

---

From: Martin Thomson <[email-masked]>
Date: Tue, 05 Feb 2019 17:20:44 -0800

@martinthomson pushed 1 commit.

1a642b5306fae6352eadb2f972d50a9a443c0090  Trailing whitespace is annoying

-- 
You are receiving this because you are subscribed to this thread.
View it on GitHub:
https://github.com/quicwg/base-drafts/pull/2424/files/3755098358db59852e597c6a278ee8956ef6d5b7..1a642b5306fae6352eadb2f972d50a9a443c0090

---

From: ianswett <[email-masked]>
Date: Tue, 05 Feb 2019 17:59:14 -0800

ianswett commented on this pull request.

Thanks Martin, some suggestions

> @@ -135,6 +135,11 @@ Crypto Packets:
 
+Out-of-order Packets:
+

```suggestion
  packet number space by exactly one.
```

I'd prefer "Packets arrive out when later packets are lost or delayed."

The SHOULD is intended to apply to just the first immediate ACK, as with ECN marks.

-- 
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/2424#pullrequestreview-200388126

---

From: Martin Thomson <[email-masked]>
Date: Tue, 05 Feb 2019 18:40:26 -0800

I think that there are two cases we care about, I only caught the first

- an earlier packet was delayed - so there is a gap before this one
- this packet was delayed - and we already received a higher-numbered one

Maybe just: "Packets arrive out of order when packets are lost or delayed."  Which doesn't really capture all the reasons, but might suffice.

-- 
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/2424#discussion_r254122586

---

From: Martin Thomson <[email-masked]>
Date: Tue, 05 Feb 2019 20:50:06 -0800

So I'm still not clear.  If I get a continuous stream of reordered packets, a) when do I cease immediate ACKs, and b) if I do stop immediate ACKs, when can I start again?

-- 
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/2424#discussion_r254137269

---

From: ianswett <[email-masked]>
Date: Wed, 06 Feb 2019 01:45:34 -0800

If every packet is reordered(ie: every other packet is missing), then you keep sending immediate ACKs.

-- 
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/2424#discussion_r254197780

---

From: Martin Thomson <[email-masked]>
Date: Wed, 06 Feb 2019 18:14:32 -0800

So it isn't the first immediate ACK, it's every out-of-order packet that starts (or refreshes) a 1/8 RTT timer.  That helps.

-- 
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/2424#discussion_r254525949

---

From: martinduke <[email-masked]>
Date: Thu, 07 Feb 2019 11:43:43 -0800

@ianswett 

"I'd prefer "Packets arrive out when later packets are lost or delayed.""

earlier packets?

Do you want this text to replace the current definition, or in addition to the current 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/2424#issuecomment-461569068

---

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

My suggestion should have been "Packets arrive out of order when later packets are lost or delayed.", but Martin's suggestion is slightly better because it drops 'later', which is unnecessary.

The intent was as a replacement for Martin's suggestion, in addition to the first sentence 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/2424#discussion_r254875673

---

From: martinduke <[email-masked]>
Date: Mon, 11 Feb 2019 18:19:42 +0000 (UTC)

@martinduke pushed 1 commit.

12ecc93e2ea9d6283b3d798d88ee0bbe68e83d72  Address review issues.

-- 
You are receiving this because you are subscribed to this thread.
View it on GitHub:
https://github.com/quicwg/base-drafts/pull/2424/files/1a642b5306fae6352eadb2f972d50a9a443c0090..12ecc93e2ea9d6283b3d798d88ee0bbe68e83d72

---

From: ianswett <[email-masked]>
Date: Mon, 11 Feb 2019 11:49:27 -0800

Suggestions

> @@ -135,6 +135,12 @@ Crypto Packets:
 
+Out-of-order Packets:
+
+: Packets that do not increase the largest received packet number for its
+packet number space by exactly one. Packets usually arrive out of order

```suggestion
  packet number space by exactly one. Packets arrive out of order
```

> @@ -135,6 +135,12 @@ Crypto Packets:
 
+Out-of-order Packets:
+
+: Packets that do not increase the largest received packet number for its
+packet number space by exactly one. Packets usually arrive out of order
+because earlier packets are lost or delayed.

```suggestion
  when earlier packets are lost or delayed.
```

> @@ -244,11 +250,11 @@ ack-eliciting packet. QUIC recovery algorithms do not assume the peer sends

The intent of the text is you SHOULD send one immediate ACK, but you may send more than one immediate ACK.  This new text changes that to you SHOULD send an immediate ACK for an interval, and I don't want to make that a SHOULD, only a MAY.

> @@ -244,11 +250,11 @@ ack-eliciting packet. QUIC recovery algorithms do not assume the peer sends
 
+during the interval. Therefore, if out-of-order packets consistently arrive

Suggestion on an alternate clarification to Martin's question: "If every packet arrives out-of-order, then an immediate ACK will be sent for every received packet."

-- 
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/2424#pullrequestreview-202302569

---

From: martinduke <[email-masked]>
Date: Mon, 11 Feb 2019 12:11:18 -0800

martinduke commented on this pull request.

+Out-of-order Packets:
+

The exceptions I had in mind are packet duplication and misbehaving senders.

-- 
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/2424#discussion_r255674461

---

From: martinduke <[email-masked]>
Date: Mon, 11 Feb 2019 12:11:54 -0800

34b3a263f498a8ec0dd6f9e98840437ee4438328  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/2424/files/12ecc93e2ea9d6283b3d798d88ee0bbe68e83d72..34b3a263f498a8ec0dd6f9e98840437ee4438328

---

From: martinduke <[email-masked]>
Date: Mon, 11 Feb 2019 12:13:27 -0800

The text allows the interval to be zero, but I agree that's weaselly. I'll clarify more.

-- 
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/2424#discussion_r255675140

---

From: martinduke <[email-masked]>
Date: Mon, 11 Feb 2019 12:17:26 -0800

7faad221d26629406e0098b1a959fb858e1b967e  Resolves all but one comment

-- 
You are receiving this because you are subscribed to this thread.
View it on GitHub:
https://github.com/quicwg/base-drafts/pull/2424/files/34b3a263f498a8ec0dd6f9e98840437ee4438328..7faad221d26629406e0098b1a959fb858e1b967e

---

From: ianswett <[email-masked]>
Date: Mon, 11 Feb 2019 12:27:57 -0800

I'm confused about what problem you're trying to fix with the immediate ACK text.  Can you clarify?

+Out-of-order Packets:
+

Duplication doesn't count as out-of-order because the packet is ignored.  Misbehaving senders are certainly possible, but I'm not sure how important they are to this text?

+This interval SHOULD be between zero (immediately acking only out-of-order

I'm finding this increasingly hard to read.  Can we pop the stack and you can explain what's unclear about the existing text?

-- 
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/2424#pullrequestreview-202320362

---

From: martinduke <[email-masked]>
Date: Mon, 11 Feb 2019 12:31:57 -0800

9bb2b61e42fbb6a77936140a93478f144d8251b7  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/2424/files/7faad221d26629406e0098b1a959fb858e1b967e..9bb2b61e42fbb6a77936140a93478f144d8251b7

---

From: martinduke <[email-masked]>
Date: Mon, 11 Feb 2019 13:05:25 -0800

we've touched base offline. We understand each other, I'll rewrite to reduce bloat

-- 
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/2424#issuecomment-462493846

---

From: martinduke <[email-masked]>
Date: Mon, 11 Feb 2019 13:14:48 -0800

55a400d469223a061cb70681185dca4de3d4c514  Simplified the text

-- 
You are receiving this because you are subscribed to this thread.
View it on GitHub:
https://github.com/quicwg/base-drafts/pull/2424/files/9bb2b61e42fbb6a77936140a93478f144d8251b7..55a400d469223a061cb70681185dca4de3d4c514

---

From: ianswett <[email-masked]>
Date: Mon, 11 Feb 2019 21:20:46 +0000 (UTC)

> @@ -244,11 +250,10 @@ ack-eliciting packet. QUIC recovery algorithms do not assume the peer sends
 
+send an immediate ACK after it receives an out-of-order packet. it could send

```suggestion
send an immediate ACK after it receives an out-of-order packet. It could 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/2424#pullrequestreview-202343412

---

From: ianswett <[email-masked]>
Date: Mon, 11 Feb 2019 13:21:21 -0800

+Out-of-order Packets:
+
+: Packets that do not increase the largest received packet number for its
+  packet number space by exactly one. Packets arrive out of order
+  when earlier packets are lost or delayed.

I'd be inclined to drop earlier because it's potentially confusing.
```suggestion
  when packets are lost or delayed.
```

-- 
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/2424#pullrequestreview-202343663

---

From: martinduke <[email-masked]>
Date: Mon, 11 Feb 2019 21:22:12 +0000 (UTC)

ddb5cf069c5df7ce1b0f502d0895d9f7add95c87  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/2424/files/55a400d469223a061cb70681185dca4de3d4c514..ddb5cf069c5df7ce1b0f502d0895d9f7add95c87

---

From: ianswett <[email-masked]>
Date: Mon, 11 Feb 2019 21:23:08 +0000 (UTC)

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/2424#pullrequestreview-202344430

---

From: Nick Banks <[email-masked]>
Date: Mon, 11 Feb 2019 21:25:40 +0000 (UTC)

nibanks approved this pull request.

LGTM

+Out-of-order Packets:
+

 I don't know... I think it might read better with 'earlier' included. I don't think it's confusing.

-- 
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/2424#pullrequestreview-202345314

---

From: ianswett <[email-masked]>
Date: Tue, 12 Feb 2019 15:38:57 -0800

Merged #2424 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/2424#event-2135269045