Subject: [quicwg/base-drafts] Only pace retransmittable packets (#1222)

From: ianswett <[email-masked]>
Date: Sat, 17 Mar 2018 15:54:33 +0000 (UTC)

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

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

-- Commit Summary --

  * Only pace retransmittable packets

-- File Changes --

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

-- Patch Links --

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

---

From: Marten Seemann <[email-masked]>
Date: Sat, 17 Mar 2018 08:58:21 -0700

marten-seemann commented on this pull request.

This is correct, however, you won't send ACK-only packets in a situation where you're pacing packets, right?

-- 
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/1222#pullrequestreview-104770247

---

From: MikkelFJ <[email-masked]>
Date: Sat, 17 Mar 2018 10:02:47 -0700

You might occasionally if you think things could  be clogging up and that small ack packets has a greater chance of survival.

-- 
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/1222#issuecomment-373935969

---

From: ianswett <[email-masked]>
Date: Sat, 17 Mar 2018 23:12:27 +0000 (UTC)

@ianswett pushed 1 commit.

905e4fd  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/1222/files/e1ec9fbdbc37a3286b49ae477aaad7405ade93fd..905e4fd6ca9eab5566592be106f30385bc99c580

---

From: ianswett <[email-masked]>
Date: Sat, 17 Mar 2018 16:18:49 -0700

ACK-only packets shouldn't be limited by the pacer, otherwise the max ack delay may be unintentionally exceeded without measuring 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/1222#issuecomment-373960078

---

From: janaiyengar <[email-masked]>
Date: Tue, 20 Mar 2018 00:42:36 +0000 (UTC)

janaiyengar commented on this pull request.

 An implementation should take care to architect its congestion controller to
 work well with a pacer.  For instance, a pacer might wrap the congestion
 controller and control the availability of the congestion window, or a pacer
-might pace out packets handed to it by the congestion controller.
+might pace out packets handed to it by the congestion controller.  In either
+arrangement, packets only containing ACK frames should not be paced, to prevent
+spurious timeout based retransmissions and to ensure the intended max ack delay
+is honored.

This sentence is a bit unwieldy. How about:
"Timely delivery of ACK frames is important for efficient loss recovery.  Packets containing only ACK frames should therefore not be paced, to avoid delaying their delivery to the peer."

-- 
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/1222#pullrequestreview-105187835

---

From: Mike Bishop <[email-masked]>
Date: Tue, 20 Mar 2018 11:47:06 +0000 (UTC)

Didn't we talk yesterday about pacing ACK-only packets as well, to ensure that the recipient isn't flooding the network with an "every other incoming packet" ACK strategy?

-- 
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/1222#issuecomment-374568774

---

From: ianswett <[email-masked]>
Date: Tue, 20 Mar 2018 16:08:48 +0000 (UTC)

Mike, I think we should add some text about rate limiting based on Monday's discussion, but actually pacing acks risks exceeding the intended delayed ack timer, which could have unintended consequences I'd rather avoid.

-- 
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/1222#issuecomment-374656285

---

From: ianswett <[email-masked]>
Date: Tue, 20 Mar 2018 16:19:23 +0000 (UTC)

acba89a  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/1222/files/905e4fd6ca9eab5566592be106f30385bc99c580..acba89a1c02384473da69851fcb8828dda45f379

---

From: ianswett <[email-masked]>
Date: Tue, 20 Mar 2018 16:19:39 +0000 (UTC)

ianswett commented on this pull request.

Thanks, changed.

-- 
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/1222#discussion_r175830877

---

From: janaiyengar <[email-masked]>
Date: Thu, 22 Mar 2018 09:43:41 +0000 (UTC)

Merged #1222.

-- 
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/1222#event-1535134873

---

From: janaiyengar <[email-masked]>
Date: Thu, 22 Mar 2018 02:43:32 -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/1222#pullrequestreview-106043206