Subject: Re: [quicwg/base-drafts] Retransmitting an ACK frame warning (#3049)

From: Martin Thomson <[email-masked]>
Date: Wed, 23 Oct 2019 01:41:46 -0700

martinthomson commented on this pull request.

This is my preferred option, but I might reword it.

> @@ -3181,9 +3181,10 @@ containing that information is acknowledged.
   unless the endpoint has sent a RESET_STREAM for that stream.  Once an endpoint
   sends a RESET_STREAM frame, no further STREAM frames are needed.
 
-* The most recent set of acknowledgments are sent in ACK frames.  An ACK frame
-  SHOULD contain all unacknowledged acknowledgments, as described in
-  {{sending-ack-frames}}.
+* ACK frames are sent containing the most recent set of acknowledgements and
+  Ack Delay, as described in {{sending-ack-frames}}. Retransmitting an old ACK
+  frame with stale Ack Delay can cause the peer to generate an inflated RTT
+  sample.

Here's my alternative:

> ACK frames always contain an acknowledgement for the highest packet number that has been processed and an Ack Delay that is updated to the time that the packet is sent.  Note that delaying the transmission of packets containing ACK frames or sending copies of old ACK frames will cause problems for a peer, including inflated RTT estimates or unnecessary disabling of ECN.

-- 
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/3049#pullrequestreview-305721135

---

From: ianswett <[email-masked]>
Date: Wed, 23 Oct 2019 18:33:50 -0700

@ianswett pushed 1 commit.

d14b63ea5276bbb3cff4ebf727b71f7fc847ec50  Some of Martin's suggestions

-- 
You are receiving this because you are subscribed to this thread.
View it on GitHub:
https://github.com/quicwg/base-drafts/pull/3049/files/1d7abab6b77b9ea678143b12c0d308d89362cfa0..d14b63ea5276bbb3cff4ebf727b71f7fc847ec50

---

From: ianswett <[email-masked]>
Date: Wed, 23 Oct 2019 18:34:47 -0700

ianswett commented on this pull request.

I took part of this, but the first statement is incorrect: ACK frames do not have to contain the highest packet number that has been processed(there was an issue about this).

-- 
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/3049#discussion_r338347623

---

From: Martin Thomson <[email-masked]>
Date: Wed, 23 Oct 2019 18:35:23 -0700

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/3049#pullrequestreview-306276812

---

From: ianswett <[email-masked]>
Date: Wed, 23 Oct 2019 18:37:55 -0700

9bb859c4dc675c3327250e488fa0b0924d7de860  Reword the first sentence per 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/3049/files/d14b63ea5276bbb3cff4ebf727b71f7fc847ec50..9bb859c4dc675c3327250e488fa0b0924d7de860

---

From: ianswett <[email-masked]>
Date: Wed, 23 Oct 2019 18:38:47 -0700

> @@ -3183,7 +3183,8 @@ containing that information is acknowledged.
 
 * The most recent set of acknowledgments are sent in ACK frames.  An ACK frame
   SHOULD contain all unacknowledged acknowledgments, as described in
-  {{sending-ack-frames}}.
+  {{sending-ack-frames}}.  Retransmitting an ACK frame can result in an

I worked part of your first suggestion in.

The latter suggestion is now present(from a different 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/3049#discussion_r338348305

---

From: ianswett <[email-masked]>
Date: Wed, 23 Oct 2019 18:42:04 -0700

7a1d1352c87bf72cce779ef52d14e489c7afaaf4  Merge branch 'master' into ianswett-retransmitting-acks

-- 
You are receiving this because you are subscribed to this thread.
View it on GitHub:
https://github.com/quicwg/base-drafts/pull/3049/files/9bb859c4dc675c3327250e488fa0b0924d7de860..7a1d1352c87bf72cce779ef52d14e489c7afaaf4

---

From: Jana Iyengar <[email-masked]>
Date: Wed, 23 Oct 2019 20:17:00 -0700

janaiyengar 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/3049#pullrequestreview-306299453

---

From: Jana Iyengar <[email-masked]>
Date: Wed, 23 Oct 2019 20:17:38 -0700

Merged #3049 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/3049#event-2739203380