Subject: [quicwg/base-drafts] Omit already acknowledged ACK Blocks (#2245)

From: ianswett <[email-masked]>
Date: Fri, 21 Dec 2018 14:01:39 -0800

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

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

-- Commit Summary --

  * Omit already acknowledged ACK Blocks

-- File Changes --

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

-- Patch Links --

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

---

From: MikkelFJ <[email-masked]>
Date: Fri, 21 Dec 2018 14:36:54 -0800

mikkelfj commented on this pull request.

> @@ -2858,8 +2858,8 @@ discussed in more detail in {{QUIC-RECOVERY}}.
 
 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.
+an ACK frame has been acknowledged, an endpoint SHOULD omit all ACK Blocks that
+have not changed since the ACK frame was sent from future ACK frames.

Cant this be reworded somehow? I think I know what it means, but I don't understand the 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/2245#pullrequestreview-187557022

---

From: ianswett <[email-masked]>
Date: Fri, 21 Dec 2018 15:51:27 -0800

ianswett commented on this pull request.

Certainly, suggestions welcome.

-- 
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/2245#discussion_r243714948

---

From: Kazuho Oku <[email-masked]>
Date: Fri, 21 Dec 2018 16:03:40 -0800

kazuho commented on this pull request.

To me it seems that the goal stated here is inaccurate. The goal is to limit the size of ACK frames.

So maybe something like: _And endpoint SHOULD minimize the size of acknowledgements. It SHOULD track which ACK frames have been acknowledged by its peer, and once an ACK frame is acknowledged, refrain from re-acknowledging the packets._

-- 
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/2245#pullrequestreview-187568643

---

From: MikkelFJ <[email-masked]>
Date: Fri, 21 Dec 2018 16:33:40 -0800

"The receiver SHOULD track which ACK frames have been acknowledged by its
peer in order to exclude already acknowledged packets from future ACK frames
whenever these packets would unnecessarily contribute to the ACK frame size."

I mention packets and not ranges or blocks because sometimes joining two blocks can reduce the size.

I wonder if adding "have been acknowledged or presumed lost" would make any sense.

-- 
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/2245#discussion_r243718254

---

From: ianswett <[email-masked]>
Date: Sat, 22 Dec 2018 18:10:40 -0800

I'm happy with your suggested text.  It's less proscriptive, but fairly clear in intent.  @kazuho does that work for you?

-- 
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/2245#discussion_r243750553

---

From: Kazuho Oku <[email-masked]>
Date: Sat, 22 Dec 2018 18:33:00 -0800

@ianswett It does.

-- 
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/2245#discussion_r243750786

---

From: ianswett <[email-masked]>
Date: Sat, 22 Dec 2018 18:42:47 -0800

@ianswett pushed 1 commit.

fb6f04b5c1460f781844a4d8fff1dac4bc247272  A tweak of mikkelfj's suggestion

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

---

From: ianswett <[email-masked]>
Date: Sat, 22 Dec 2018 18:43:14 -0800

Updated, while keeping a bit of the existing text, 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/2245#discussion_r243750884

---

From: MikkelFJ <[email-masked]>
Date: Sun, 23 Dec 2018 02:41:02 -0800

My rewording was to avoid a double SHOULD, but this is also fine.

-- 
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/2245#discussion_r243759184

---

From: MikkelFJ <[email-masked]>
Date: Sun, 23 Dec 2018 02:51:22 -0800

Perhaps a clarification is needed that packets can be repeated until peers sees. For example by adding ", but can/MAY otherwise repeat packets for redundancy.".

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.  The
receiver SHOULD exclude already acknowledged packets from future ACK frames
whenever these packets would unnecessarily contribute to the ACK frame size, but can otherwise repeat packets for redundancy."

-- 
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/2245#discussion_r243759457

---

From: Martin Thomson <[email-masked]>
Date: Sun, 23 Dec 2018 20:09:12 -0800

Thanks everyone, this is much 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/2245#issuecomment-449683687

---

From: Martin Thomson <[email-masked]>
Date: Sun, 23 Dec 2018 20:09:17 -0800

Merged #2245 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/2245#event-2042723014