Subject: [quicwg/base-drafts] Restrict ACK-clocking to ACK-only frames. (#1231)

From: ekr <[email-masked]>
Date: Sun, 18 Mar 2018 21:42:50 +0000 (UTC)

As written, this would forbid the case where A sends a single stream
frame and then B responds with three stream frames and piggybacks
ACKs along with each. That seems silly and a lot of work. Rather,
this should just be about ACK-only frames.
You can view, comment on, or merge this pull request online at:

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

-- Commit Summary --

  * As written, this would forbid the case where A sends a single stream

-- File Changes --

    M draft-ietf-quic-transport.md (8)

-- Patch Links --

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

---

From: Martin Thomson <[email-masked]>
Date: Sun, 18 Mar 2018 23:19:54 +0000 (UTC)

martinthomson 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/1231#pullrequestreview-104816265

---

From: ianswett <[email-masked]>
Date: Mon, 19 Mar 2018 00:30:49 +0000 (UTC)

ianswett commented on this pull request.

> @@ -2538,10 +2538,10 @@ ACK Block (repeated):
 Implementations MUST NOT generate packets that only contain ACK frames in
 response to packets which only contain ACK frames. However, they MUST
 acknowledge packets containing only ACK frames when sending ACK frames in
-response to other packets.  Implementations MUST NOT send more than one ACK
-frame per received packet that contains frames other than ACK frames.  Packets
-containing non-ACK frames MUST be acknowledged immediately or when a delayed
-ack timer expires.
+response to other packets.  Implementations MUST NOT send more than one packet
+containing only ACK frames per received packet that contains frames other than

I have no objection, but is the motivation for this the coming CRYPTO_ACK/ACK split?

-- 
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/1231#pullrequestreview-104819282

---

From: ekr <[email-masked]>
Date: Mon, 19 Mar 2018 01:02:51 -0700

ekr commented on this pull request.

No. It was purely that the text didn't match my expectations

-- 
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/1231#discussion_r175352821

---

From: ekr <[email-masked]>
Date: Mon, 19 Mar 2018 09:42:50 +0000 (UTC)

@ekr pushed 1 commit.

3a7437d  Rephrase the text about one-way text

-- 
You are receiving this because you are subscribed to this thread.
View it on GitHub:
https://github.com/quicwg/base-drafts/pull/1231/files/503027d10b59d73808014672657e3b2bfb8cf595..3a7437d8ba744c8c76e0cf3f9bf0713d37ff54b9

---

From: Marten Seemann <[email-masked]>
Date: Mon, 19 Mar 2018 10:01:27 +0000 (UTC)

marten-seemann commented on this pull request.

I'm not sure I really I understand the problem you're trying to solve.
If you really want to send duplicate ACK frames (without receiving any new packets, and without any timer expiring), the change of the first sentence in your PR would be sufficient.

 To limit ACK blocks to those that have not yet been received by the sender, the
 receiver SHOULD track which ACK frames have been acknowledged by its peer.  Once
 an ACK frame has been acknowledged, the packets it acknowledges SHOULD NOT be
 acknowledged again.
 
-A receiver that is only sending ACK frames will not receive acknowledgments for
-its packets.  Sending an occasional MAX_DATA or MAX_STREAM_DATA frame as data is
-received will ensure that acknowledgements are generated by a peer.  Otherwise,
-an endpoint MAY send a PING frame once per RTT to solicit an acknowledgment.
+Because ACK-only packets are not acknowledged, a receiver that is only
+sending ACK frames will only receive acknowledgements for its packets
+if the sender piggybacks them on other packets. In order to enable

Why does it play a role how the sender bundles frames into packets?

 To limit ACK blocks to those that have not yet been received by the sender, the
 receiver SHOULD track which ACK frames have been acknowledged by its peer.  Once
 an ACK frame has been acknowledged, the packets it acknowledges SHOULD NOT be
 acknowledged again.
 
-A receiver that is only sending ACK frames will not receive acknowledgments for
-its packets.  Sending an occasional MAX_DATA or MAX_STREAM_DATA frame as data is
-received will ensure that acknowledgements are generated by a peer.  Otherwise,
-an endpoint MAY send a PING frame once per RTT to solicit an acknowledgment.
+Because ACK-only packets are not acknowledged, a receiver that is only
+sending ACK frames will only receive acknowledgements for its packets
+if the sender piggybacks them on other packets. In order to enable
+this, senders SHOULD prioritize sending ACKs along with their data.

I don't think you should prioritize ACKs, if the ACKs don't ack any new retransmittable packets.

 To limit ACK blocks to those that have not yet been received by the sender, the
 receiver SHOULD track which ACK frames have been acknowledged by its peer.  Once
 an ACK frame has been acknowledged, the packets it acknowledges SHOULD NOT be
 acknowledged again.
 
-A receiver that is only sending ACK frames will not receive acknowledgments for
-its packets.  Sending an occasional MAX_DATA or MAX_STREAM_DATA frame as data is
-received will ensure that acknowledgements are generated by a peer.  Otherwise,
-an endpoint MAY send a PING frame once per RTT to solicit an acknowledgment.
+Because ACK-only packets are not acknowledged, a receiver that is only
+sending ACK frames will only receive acknowledgements for its packets
+if the sender piggybacks them on other packets. In order to enable
+this, senders SHOULD prioritize sending ACKs along with their data.
+In addition, if a receiver regularly issues additional flow control
+credit rather than just issuing infinite flow control credit at the

Issuing infinite flow control credit is a really bad idea. I don't think we should mention this as an option 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/1231#pullrequestreview-104894712

---

From: ianswett <[email-masked]>
Date: Mon, 19 Mar 2018 10:48:43 +0000 (UTC)

I think prioritize is a loaded word, maybe "senders SHOULD bundle ACKs with non-ACK frames when possible."

-- 
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/1231#discussion_r175392605

---

From: ianswett <[email-masked]>
Date: Mon, 19 Mar 2018 03:57:14 -0700

ACK-only packets are acknowledged, but they do not cause an ACK frame to be sent.

Maybe: ""Because ACK frames are not sent in response to ACK-only packets, ..."

I would remove this sentence.

-- 
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/1231#pullrequestreview-104912166

---

From: ekr <[email-masked]>
Date: Mon, 19 Mar 2018 04:15:22 -0700

Consider the case where the receiver issued 1MB of flow credit and the sender sends 950K of data. In this case the receiver might just send ACK-only packets which the sender is forbidden to send its own ACK-only packets. Thus, if the sender never bundles ACKs, the receiver will never get any.

-- 
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/1231#discussion_r175399660

---

From: ekr <[email-masked]>
Date: Mon, 19 Mar 2018 04:16:34 -0700

@ekr pushed 1 commit.

0359cc7  Update per comments

-- 
You are receiving this because you are subscribed to this thread.
View it on GitHub:
https://github.com/quicwg/base-drafts/pull/1231/files/32f3bd9c4c5f7cbec074538350ebe52ead81e2d5..0359cc729efc4575fda5c918c12b011e2c4ac94b

---

From: Marten Seemann <[email-masked]>
Date: Mon, 19 Mar 2018 11:20:02 +0000 (UTC)

Where is the problem? You don't expect to get any ACK for ACK-only packets, they don't count towards bytes-in-flight, they are never 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/1231#discussion_r175400744

---

From: ekr <[email-masked]>
Date: Mon, 19 Mar 2018 04:29:51 -0700

Well, the text I am updating assumes it is a problem, I'm just fixing it up.

With that said, while the packets don't count towards bytes-in-flight, you generally end up storing the data about the to-be-ACKed packets (as implied by the following text):

```To limit ACK blocks to those that have not yet been received by the sender, the
receiver SHOULD track which ACK frames have been acknowledged by its peer.  Once
an ACK frame has been acknowledged, the packets it acknowledges SHOULD NOT be
acknowledged again.```

Thus, you can't garbage collect this state until those ACKs are 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/1231#discussion_r175403013

---

From: MikkelFJ <[email-masked]>
Date: Mon, 19 Mar 2018 11:36:00 +0000 (UTC)

mikkelfj commented on this pull request.

I think that is an unfortunate requirement (SHOULD NOT be ack'ed again):
You want to ack packets again because you collapse gaps into larger blocks. Only the oldest ACK'ed block should not be acked 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/1231#discussion_r175404347

---

From: Martin Thomson <[email-masked]>
Date: Mon, 19 Mar 2018 04:49:32 -0700

martinthomson commented on this pull request.

GitHub ate my homework, but let me take another look...

I agree with this.  The point of changing this text is to make it clear what the consequences of particular design choices are.  I would remove this sentence (starting with "In order to enable").

Again, I would remove "rather than just issuing infinite flow control credit at the beginning".  The point remains good though, so I'd keep the rest.

-- 
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/1231#pullrequestreview-104909628

---

From: ekr <[email-masked]>
Date: Mon, 19 Mar 2018 04:52:16 -0700

@mikkelfj: I suggest you file a separate issue.

-- 
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/1231#discussion_r175408850

---

From: MikkelFJ <[email-masked]>
Date: Mon, 19 Mar 2018 12:07:35 +0000 (UTC)

https://github.com/quicwg/base-drafts/issues/1234

-- 
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/1231#discussion_r175412321

---

From: Marten Seemann <[email-masked]>
Date: Mon, 19 Mar 2018 12:17:02 +0000 (UTC)

The text as it stands makes sense: **If** a peer wants an ACK for an ACK (e.g. to prune the lowest acked packet), it can include a retransmittable frame. This depends on which ack-pruning algorithm it uses, and it has nothing to do with how frames are bundled.

-- 
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/1231#discussion_r175414525

---

From: ekr <[email-masked]>
Date: Mon, 19 Mar 2018 05:32:30 -0700

That part of the text is correct. What's not correct is the claim that you will not receive ACKs if you do not do so.

-- 
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/1231#discussion_r175418177