Subject: [quicwg/base-drafts] Clarify when to send acks (#907)

From: ianswett <[email-masked]>
Date: Wed, 08 Nov 2017 02:30:48 +0000 (UTC)

Clarify when to send ACK frames in the transport doc, as discussed in Seattle.
You can view, comment on, or merge this pull request online at:

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

-- Commit Summary --

  * Clarify when to send acks

-- File Changes --

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

-- Patch Links --

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

---

From: ianswett <[email-masked]>
Date: Wed, 08 Nov 2017 02:31:51 +0000 (UTC)

@ianswett pushed 1 commit.

d67b612  Update draft-ietf-quic-transport.md

-- 
You are receiving this because you are subscribed to this thread.
View it on GitHub:
https://github.com/quicwg/base-drafts/pull/907/files/957490141b4617aff8b5f1e505ebd98c07f7f285..d67b6127b9ab9a6f6c38d3c0c15c764af111d2dd

---

From: Martin Thomson <[email-masked]>
Date: Wed, 08 Nov 2017 06:00:18 +0000 (UTC)

martinthomson approved this pull request.

I like this structure.

I apologize for probably re-reviewing text that you probably just moved, but it's hard to track the actual changes and we could improve on things more.

Feel free to push back or open new issues if you disagree.

> @@ -2427,6 +2407,32 @@ ACK Block (repeated):
   packets preceding the largest packet number, as determined by the
   preceding Gap.
 
+### Sending ACK Frames
+
+Implementations MUST NOT generate packets that only contain ACK frames in
+response to packets which only contain ACK frames. However, they SHOULD

I think that we want to MUST here.  Requiring that all received packets be acknowledged is good for the protocol.

> @@ -2427,6 +2407,32 @@ ACK Block (repeated):
   packets preceding the largest packet number, as determined by the
   preceding Gap.
 
+### Sending ACK Frames
+
+Implementations MUST NOT generate packets that only contain ACK frames in
+response to packets which only contain ACK frames. However, they SHOULD
+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

This doesn't make sense to me:

I can receive a packet that contains other than ACK frames (which is the norm, I would expect).  But I might send more than one ACK frame over time.  I think that what you intend to say here is part of a different statement.

That is, every time a packet that contains more than just ACK frames is received, an implementation schedules an ACK frame to be sent in a new packet.  This might be immediately, but more often it will be delayed slightly.  Delaying acknowledgment in this way ensures that packets arriving in rapid succession only induce a single packet in response or that acknowledgments can be sent together with stream data.  Either way, fewer packets are sent, and fewer packets containing only ACK frames.  Packets containing only ACK frames that are generated as a result of incoming packets MUST NOT be generated at a rate higher than the rate of incoming packets.  Otherwise there would be in amplification in the number of packets, if not the number of bytes being exchanged.

> @@ -2427,6 +2407,32 @@ ACK Block (repeated):
   packets preceding the largest packet number, as determined by the
   preceding Gap.
 
+### Sending ACK Frames
+
+Implementations MUST NOT generate packets that only contain ACK frames in
+response to packets which only contain ACK frames. However, they SHOULD
+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.
+
+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

Could we also recommend or at least suggest tracking what acknowledgments are outstanding (sent but not yet acknowledged)?  There might be some value in not re-sending all unacknowledged acknowledgments (where ack_delay_timer << RTT especially).

> +
+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.
+
+To limit receiver state or the size of ACK frames, a receiver MAY limit the
+number of ACK blocks it sends.  A receiver can do this even without receiving
+acknowledgment of its ACK frames, with the knowledge this could cause the sender
+to unnecessarily retransmit some data.  When this is necessary, the receiver
+SHOULD acknowledge newly received packets and stop acknowledging packets

I think that s/SHOULD/can/  I don't think that we want to mandate anything here.

That is, unless your intent was to say that acknowledging the most recently received packets is preferable, but then I would say "if acknowledgments are removed rather than retransmitted, a receiver SHOULD discard acknowledgments with the lowest packet number first".  Or something along those lines.

-- 
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/907#pullrequestreview-74995267

---

From: janaiyengar <[email-masked]>
Date: Wed, 08 Nov 2017 07:14:57 +0000 (UTC)

janaiyengar commented on this pull request.

+### Sending ACK Frames
+

Yeah, agreed. I don't think there's a good reason to not ack these packets when an ACK frame is being sent anyways.

+### Sending ACK Frames
+

Perhaps a suggestion, but I think we should encourage receivers to re-ack as much as they can, for resilience and performance. How about something like: "In every ACK frame, a receiver SHOULD attempt to acknowledge all received packets whose acknowledgement is not yet known to have reached the sender. However, a receiver MAY avoid re-acknowledging all of them in subsequent ACK frames, and it may track outstanding acknowledgements to do so. Note that doing so reduces the connection's resilience to loss of acknowledgements." I think the wording could be improved, but something like this?

lowest packet number isn't necessarily the one that was the oldest. What you want really is to discard acks for the earliest received packets. I'm not concerned about the SHOULD here, I think that's fine... but I'm also fine with replacing that with a "can".

+### Sending ACK Frames
+

Hm. I think this is captured in the "to limit receiver state below". Do we need any more text here? I think it may still be valuable to state the first sentence in my suggestion above perhaps?

-- 
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/907#discussion_r149586679

---

From: Martin Thomson <[email-masked]>
Date: Wed, 08 Nov 2017 20:16:37 +0000 (UTC)

martinthomson commented on this pull request.

+### Sending ACK Frames
+

This isn't an example of limiting state, it's limiting bytes transmitted.  I think that your formulation here (MAY avoid sending an acknowledgment if an ACK frame containing the same acknowledgment is already in flight) is a good way to deal with this.

The limiting of state does end up having the same net effect (fewer ACK ranges), but it's because the receiver is discarding information.  This would require more effort.

-- 
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/907#discussion_r149784785

---

From: Martin Thomson <[email-masked]>
Date: Wed, 08 Nov 2017 20:18:25 +0000 (UTC)

It's oldest at the sender, so I am assuming that it will be marked as lost first (or might have already).

Separately, should we also note that dropping ACKs causes packets to be marked as lost causes congestion control to receive a warped signal?

-- 
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/907#discussion_r149785248

---

From: ianswett <[email-masked]>
Date: Fri, 10 Nov 2017 00:01:43 +0000 (UTC)

ianswett commented on this pull request.

+### Sending ACK Frames
+

This is new text, but I think it's correct.  There's no case where receiving one packet generates two ack frames.

-- 
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/907#discussion_r150123257

---

From: ianswett <[email-masked]>
Date: Fri, 10 Nov 2017 00:04:41 +0000 (UTC)

I think SHOULD is still good here, since this is what we recommend implementations do, and as Jana said, in theory it's not as simple as dropping smallest packet number.

I'll add a note that dropping ack blocks early increases the chance the peer never receives an acknowledgement and may be forced to spuriously retransmit the data it contains.

-- 
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/907#discussion_r150123691

---

From: ianswett <[email-masked]>
Date: Fri, 10 Nov 2017 00:07:50 +0000 (UTC)

7dfc1a8  Comments

-- 
You are receiving this because you are subscribed to this thread.
View it on GitHub:
https://github.com/quicwg/base-drafts/pull/907/files/d67b6127b9ab9a6f6c38d3c0c15c764af111d2dd..7dfc1a81a4d28650ccd76c1eb8e4b9adb438c149

---

From: ianswett <[email-masked]>
Date: Fri, 10 Nov 2017 00:09:16 +0000 (UTC)

+### Sending ACK Frames
+

I think this limiting of state is described a few paragraphs above.

-- 
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/907#discussion_r150124353

---

From: Martin Thomson <[email-masked]>
Date: Fri, 10 Nov 2017 23:29:33 +0000 (UTC)

+### Sending ACK Frames
+

limiting of state is below.  I'm making a different point.  We don't send stream data that we just sent, so it is possible that duplicating acknowledgments in the same way is inefficient.  Now, Jana rightly observes that the cost is low and the benefits high, which is an excellent point, but there might be cases where the cost is high enough (large BDP connections for example) to warrant trimming ACK frames down in size.

-- 
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/907#discussion_r150359731

---

From: ianswett <[email-masked]>
Date: Sat, 11 Nov 2017 05:52:33 +0000 (UTC)

+### Sending ACK Frames
+

Ok, understood.  In that case, I think the text below is sufficient for now?

-- 
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/907#discussion_r150375794

---

From: Mike Bishop <[email-masked]>
Date: Sat, 11 Nov 2017 06:21:13 +0000 (UTC)

MikeBishop approved this pull request.

> @@ -2427,6 +2407,34 @@ ACK Block (repeated):
 
+### Sending ACK Frames
+
+response to packets which only contain ACK frames. However, they MUST

"non-ACK", or just "other"

-- 
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/907#pullrequestreview-75924332

---

From: ianswett <[email-masked]>
Date: Sat, 11 Nov 2017 06:41:39 +0000 (UTC)

24690e1  Add '-'

-- 
You are receiving this because you are subscribed to this thread.
View it on GitHub:
https://github.com/quicwg/base-drafts/pull/907/files/7dfc1a81a4d28650ccd76c1eb8e4b9adb438c149..24690e11f7336fadaed62ea22462246d3f7cf4e7

---

From: ianswett <[email-masked]>
Date: Sat, 11 Nov 2017 06:43:07 +0000 (UTC)

Ok, I think this is enough of an improvement I'm going to merge it and we can follow up with more improvements later.

-- 
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/907#issuecomment-343645013

---

From: ianswett <[email-masked]>
Date: Sat, 11 Nov 2017 06:43:31 +0000 (UTC)

Merged #907.

-- 
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/907#event-1337135860