Subject: Re: [quicwg/base-drafts] Send a few immediate ACKs (#2025)

From: ianswett <[email-masked]>
Date: Fri, 11 Jan 2019 08:56:18 -0800

@ianswett pushed 1 commit.

31d1343358b9086b8ed92d83d6e278229f629501  Merge branch 'master' into ianswett-immediate-ack

-- 
You are receiving this because you are subscribed to this thread.
View it on GitHub:
https://github.com/quicwg/base-drafts/pull/2025/files/099dedd0f2cdaedf94ba34ced4f46b0edfce06ed..31d1343358b9086b8ed92d83d6e278229f629501

---

From: ianswett <[email-masked]>
Date: Fri, 11 Jan 2019 08:56:54 -0800

b343e2b684bd69a138bf15cbfa53a1419cb539c4  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/2025/files/31d1343358b9086b8ed92d83d6e278229f629501..b343e2b684bd69a138bf15cbfa53a1419cb539c4

---

From: ianswett <[email-masked]>
Date: Fri, 11 Jan 2019 08:57:27 -0800

Thanks Jana, I fixed the merge conflicts and took your suggestions.  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/2025#issuecomment-453584364

---

From: ianswett <[email-masked]>
Date: Tue, 15 Jan 2019 09:43:46 -0800

f070e8ff061c9cc366f7d56168f69dc33829b53c  Should immediately ACK one packet

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

---

From: martinduke <[email-masked]>
Date: Tue, 15 Jan 2019 10:18:01 -0800

martinduke commented on this pull request.

I am not sure where this stack of SHOULDs leaves us.

There is one MUST, which is that we MUST ack within max_ack_delay.

Beyond that, receivers:
SHOULD NOT ack every packet
SHOULD ack every other packet.
SHOULD immediately ack if OOO
MAY immediately ack subsequent packets, but
SHOULD NOT do it for a whole RTT.

The MAY seems a little redundant; it is clearly authorized by the lack of any MUSTs. I think you are trying to say "receivers SHOULD immediately ack an OOO packet and the next few packets after that to improve the chance of delivery". Maybe just say that?

 Out-of-order packets SHOULD be acknowledged more quickly, in order to accelerate
 loss recovery.  The receiver SHOULD send an immediate ACK when it receives a new
-packet which is not one greater than the largest received packet number.
+packet which is not the next expected one. That is, its packet number is not one
+greater than the largest received packet number.  A receiver MAY immediately
+ack subsequent packets after first receiving a packet out of order. A receiver
+SHOULD NOT send an immediate ACK any time there is a gap in the ACK frame,

the ACK frame it is sending, or the ACK it is receiving? I think you mean the ACK frame it's sending, but it took me a couple of readings to get 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/2025#pullrequestreview-192783368

---

From: ianswett <[email-masked]>
Date: Tue, 15 Jan 2019 10:47:56 -0800

60a6255b8de4a75f9e0d9bee4cb43036889d6d59  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/2025/files/f070e8ff061c9cc366f7d56168f69dc33829b53c..60a6255b8de4a75f9e0d9bee4cb43036889d6d59

---

From: ianswett <[email-masked]>
Date: Tue, 15 Jan 2019 10:48:42 -0800

cfb0ae882b4d80fc8f2311dd7245c6da86dd19ca  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/2025/files/60a6255b8de4a75f9e0d9bee4cb43036889d6d59..cfb0ae882b4d80fc8f2311dd7245c6da86dd19ca

---

From: ianswett <[email-masked]>
Date: Tue, 15 Jan 2019 10:48:59 -0800

183d6c301e5af64f795651eed7d73c20399321bf  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/2025/files/cfb0ae882b4d80fc8f2311dd7245c6da86dd19ca..183d6c301e5af64f795651eed7d73c20399321bf

---

From: ianswett <[email-masked]>
Date: Tue, 15 Jan 2019 18:50:07 +0000 (UTC)

87ac16a634eb4cf1e4cfd994039440c053536f12  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/2025/files/183d6c301e5af64f795651eed7d73c20399321bf..87ac16a634eb4cf1e4cfd994039440c053536f12

---

From: martinduke <[email-masked]>
Date: Tue, 15 Jan 2019 10:51:19 -0800

I think 'kPacketThreshold' is the ideal value of 'few' in my comment above. Maybe this should become an optional transport parameter?

-- 
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/2025#issuecomment-454506771

---

From: ianswett <[email-masked]>
Date: Tue, 15 Jan 2019 10:52:26 -0800

fba23ef4187a7f59b922ba0a8c0ea263627b447b  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/2025/files/87ac16a634eb4cf1e4cfd994039440c053536f12..fba23ef4187a7f59b922ba0a8c0ea263627b447b

---

From: ianswett <[email-masked]>
Date: Tue, 15 Jan 2019 18:53:36 +0000 (UTC)

ianswett commented on this pull request.

Clarified.

-- 
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/2025#discussion_r248018922

---

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

Previously this said immediately ack 3 packets, but that assumes kReorderingThreshold is always 3, which seems like a poor choice.

Given we now say you should declare a packet lost based on time or packet reordering threshold, I expect most people will implement an adaptive packet reordering threshold, so even if we added a transport param, I think that's a lower bound?

-- 
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/2025#issuecomment-454508341

---

From: ianswett <[email-masked]>
Date: Tue, 15 Jan 2019 10:58:36 -0800

ac2c595a33dfb08bb1dc5d0d9cf26f0fe4339b5f  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/2025/files/fba23ef4187a7f59b922ba0a8c0ea263627b447b..ac2c595a33dfb08bb1dc5d0d9cf26f0fe4339b5f

---

From: martinduke <[email-masked]>
Date: Tue, 15 Jan 2019 11:50:58 -0800

@ianswett good point; I return to the position of my comment before last.

-- 
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/2025#issuecomment-454527229

---

From: Jana Iyengar <[email-masked]>
Date: Mon, 28 Jan 2019 18:33:42 -0800

janaiyengar commented on this pull request.

 Out-of-order packets SHOULD be acknowledged more quickly, in order to accelerate
 loss recovery.  The receiver SHOULD send an immediate ACK when it receives a new
-packet which is not one greater than the largest received packet number.
+packet which is not the next expected one. That is, its packet number is not one
+greater than the largest received packet number.  A receiver MAY immediately
+acknowledge subsequent ack-eliciting packets after first receiving a packet out
+of order. A receiver SHOULD NOT send an immediate ACK any time there is a gap
+in the ACK frame being sent, because that will cause it to send an ACK in
+response to every packet for at least an RTT after a single packet loss.

I don't understand what the text here is trying to say.  I'll suggest rewording for text "A receiver MAY ... single packet loss", see if this matches what you're getting at.

"A receiver MAY send acknowledgements immediately for the next few packets that are received, but the receiver SHOULD revert back to delaying acknowledgements after sending a few immediate acknowledgements."

-- 
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/2025#pullrequestreview-197354553

---

From: ianswett <[email-masked]>
Date: Mon, 28 Jan 2019 18:45:08 -0800

741c29413014fd3541f930344e5b9e419f4b244c  Jana's suggestion

-- 
You are receiving this because you are subscribed to this thread.
View it on GitHub:
https://github.com/quicwg/base-drafts/pull/2025/files/ac2c595a33dfb08bb1dc5d0d9cf26f0fe4339b5f..741c29413014fd3541f930344e5b9e419f4b244c

---

From: martinduke <[email-masked]>
Date: Mon, 28 Jan 2019 23:15:24 -0800

Again, there are SHOULDs on top of SHOULDs here. How about this for a paragraph:

"Quicker acknowledgement of out-of-order packets may accelerate loss detection. Upon receipt of a packet number not one more than the previous maximum received, receivers SHOULD separately acknowledge it and the next few packets, but not more than five."

This is shorter, has only one SHOULD, and places a recommended limit that is less subjective than "a few".

-- 
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/2025#pullrequestreview-197404149

---

From: Martin Thomson <[email-masked]>
Date: Tue, 29 Jan 2019 00:23:38 -0800

martinthomson commented on this pull request.

Don't we have a reordering threshold constant we could use 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/2025#discussion_r251730189

---

From: ianswett <[email-masked]>
Date: Tue, 29 Jan 2019 10:33:58 +0000 (UTC)

7e473d989c13f7bb7e284641b0572fd6c71da6ef  Only send immediate acks for 1/8 RTT

-- 
You are receiving this because you are subscribed to this thread.
View it on GitHub:
https://github.com/quicwg/base-drafts/pull/2025/files/741c29413014fd3541f930344e5b9e419f4b244c..7e473d989c13f7bb7e284641b0572fd6c71da6ef

---

From: ianswett <[email-masked]>
Date: Tue, 29 Jan 2019 02:34:59 -0800

Thanks for the suggestions, I reworked the text to only have one SHOULD and mention the 1/8 RTT as the max amount of time to send 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/2025#discussion_r251776450

---

From: martinduke <[email-masked]>
Date: Tue, 29 Jan 2019 16:32:21 -0800

martinduke approved this pull request.

LGTM, modulo MT's point.

-- 
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/2025#pullrequestreview-197855104

---

From: Martin Thomson <[email-masked]>
Date: Tue, 29 Jan 2019 16:34:26 -0800

So when do I return to sending immediate ACKs?

Say that there is a period of reordering, so that I might otherwise be inclined to send immediate ACKs for 1-RTT or longer.  This text says that I return to sending delayed ACKs after 1/8 RTT.  That means that I mark the time when I first send an immediate ACK and as long as `now < mark + sRTT/8` I send immediate ACKs.  When that period elapses, then I will send delayed ACKs.  When do I return to 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/2025#pullrequestreview-197855551

---

From: ianswett <[email-masked]>
Date: Wed, 30 Jan 2019 00:27:59 -0800

You send immediate acks when there's an out of order packet or an ECN mark.

Honestly, I like the 'send one immediate ack per out of order packet' text I had earlier, because it parallels the ECN text nicely, but this is fine as well.

-- 
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/2025#issuecomment-458855118

---

From: ianswett <[email-masked]>
Date: Wed, 30 Jan 2019 00:30:41 -0800

8b7cb1ad06a48978de1d8dc8b6b21a32a99b1ee2  A bit clearer

-- 
You are receiving this because you are subscribed to this thread.
View it on GitHub:
https://github.com/quicwg/base-drafts/pull/2025/files/7e473d989c13f7bb7e284641b0572fd6c71da6ef..8b7cb1ad06a48978de1d8dc8b6b21a32a99b1ee2

---

From: ianswett <[email-masked]>
Date: Wed, 30 Jan 2019 23:14:24 +0000 (UTC)

2c9832bd48766216ac7c12f8c066c61dfe83e9c5  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/2025/files/8b7cb1ad06a48978de1d8dc8b6b21a32a99b1ee2..2c9832bd48766216ac7c12f8c066c61dfe83e9c5

---

From: ianswett <[email-masked]>
Date: Wed, 30 Jan 2019 15:17:36 -0800

d5cb783f471e74f6fd392f5e75a239ad7bf01415  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/2025/files/2c9832bd48766216ac7c12f8c066c61dfe83e9c5..d5cb783f471e74f6fd392f5e75a239ad7bf01415

---

From: martinduke <[email-masked]>
Date: Wed, 30 Jan 2019 16:13:59 -0800

I think the text is now clear, although technically the MAY is redundant. Please ship it, and I'll see if I can wordsmith something better.

-- 
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/2025#pullrequestreview-198372100