Subject: Re: [quicwg/base-drafts] Remove unnecessary pacing text (#977)

From: ianswett <[email-masked]>
Date: Fri, 01 Dec 2017 06:56:44 -0800

ianswett commented on this pull request.

This approach does work fairly well, so it's a bit unfortunate to get rid of it.  The fact we take these constants from Linux doesn't seem like a major problem to me, given we do that in quite a few places.

I'd like to get a read from the WG about whether they'd like this included, but I'm also happy to chat later today.

-- 
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/977#pullrequestreview-80519159

---

From: janaiyengar <[email-masked]>
Date: Sun, 03 Dec 2017 22:33:57 +0000 (UTC)

Happy to chat, and happy to open an issue for WG consideration. The spec currently does not specify a full pacer, which is appropriately out of scope. Linux's fq is not defined anywhere except in Linux code, and these constants apply to that flavor of pacer. (In contrast, the other constants we use apply to documented mechanisms, but we are borrowing constants only from Linux.) Simply stating these constants is incomplete, which is why I wanted to remove this entirely. I could change the text to have some sort of more generic language about GQUIC and Linux fq implementations; let me give that a shot.

-- 
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/977#issuecomment-348819750

---

From: ianswett <[email-masked]>
Date: Wed, 06 Dec 2017 20:15:13 +0000 (UTC)

ianswett 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/977#pullrequestreview-81653783

---

From: janaiyengar <[email-masked]>
Date: Wed, 06 Dec 2017 16:21:49 -0800

@janaiyengar pushed 1 commit.

e92305c  add generic language about linux fq

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

---

From: ianswett <[email-masked]>
Date: Thu, 07 Dec 2017 01:19:50 +0000 (UTC)

One suggestion unrelated to your changes, LGTM.

> -## Pacing Rate
-
-The pacing rate is a function of the mode, the congestion window, and
-the smoothed rtt.  Specifically, the pacing rate is 2 times the
-congestion window divided by the smoothed RTT during slow start
-and 1.25 times the congestion window divided by the smoothed RTT during
-congestion avoidance.  In order to fairly compete with flows that are not
-pacing, it is recommended to not pace the first 10 sent packets when
-exiting quiescence.
+When retransmissions are sent due to a retransmission timeout alarm, no change
+is made to the congestion window until the next acknowledgement arrives.  When
+an ack arrives, if packets prior to the first retransmission timeout are
+acknowledged, then the congestion window remains the same.  If no packets prior
+to the first retransmission timeout are acknowledged, the retransmission timeout
+has been validated and the congestion window must be reduced to the minimum
+congestion window and slow start is begun.

nit: begun is weird in retrospect.  Maybe re-entered?

-- 
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/977#pullrequestreview-81722491

---

From: janaiyengar <[email-masked]>
Date: Thu, 07 Dec 2017 01:34:54 +0000 (UTC)

daa95fe  rto text

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

---

From: janaiyengar <[email-masked]>
Date: Thu, 07 Dec 2017 01:35:17 +0000 (UTC)

janaiyengar commented on this pull request.

-

reworded paragraph

-- 
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/977#discussion_r155410226

---

From: ianswett <[email-masked]>
Date: Thu, 07 Dec 2017 01:45:36 +0000 (UTC)

Suggestion on making the wording slightly more active

> @@ -856,12 +856,13 @@ Acknowledgement or loss of tail loss probes are treated like any other packet.
 ## Retransmission Timeout
 
 When retransmissions are sent due to a retransmission timeout alarm, no change
-is made to the congestion window until the next acknowledgement arrives.  When
-an ack arrives, if packets prior to the first retransmission timeout are
-acknowledged, then the congestion window remains the same.  If no packets prior
-to the first retransmission timeout are acknowledged, the retransmission timeout
-has been validated and the congestion window must be reduced to the minimum
-congestion window and slow start is begun.
+is made to the congestion window until the next acknowledgement arrives.  The
+retransmission timeout is considered spurious when an ack is received that

nit: when the next ack acknowleges?

> @@ -856,12 +856,13 @@ Acknowledgement or loss of tail loss probes are treated like any other packet.
 ## Retransmission Timeout
 
 When retransmissions are sent due to a retransmission timeout alarm, no change
-is made to the congestion window until the next acknowledgement arrives.  When
-an ack arrives, if packets prior to the first retransmission timeout are
-acknowledged, then the congestion window remains the same.  If no packets prior
-to the first retransmission timeout are acknowledged, the retransmission timeout
-has been validated and the congestion window must be reduced to the minimum
-congestion window and slow start is begun.
+is made to the congestion window until the next acknowledgement arrives.  The
+retransmission timeout is considered spurious when an ack is received that
+acknowledges packets sent prior to the first retransmission timeout.  The
+retransmission timeout is considered valid when an ack is received that

same nit

-- 
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/977#pullrequestreview-81726395

---

From: janaiyengar <[email-masked]>
Date: Thu, 07 Dec 2017 01:47:19 +0000 (UTC)

72c98c4  ian comments

-- 
You are receiving this because you are subscribed to this thread.
View it on GitHub:
https://github.com/quicwg/base-drafts/pull/977/files/daa95fe8abbac0b5ba5fd00bfd18605c82df1cf1..72c98c4afdc64d1e3dd147526b68dae42021c1ee

---

From: janaiyengar <[email-masked]>
Date: Thu, 07 Dec 2017 01:47:29 +0000 (UTC)

done

-- 
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/977#discussion_r155411908

---

From: janaiyengar <[email-masked]>
Date: Thu, 07 Dec 2017 01:47:37 +0000 (UTC)

done

-- 
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/977#discussion_r155411922

---

From: janaiyengar <[email-masked]>
Date: Wed, 06 Dec 2017 17:48:43 -0800

252c8ce  ian comments

-- 
You are receiving this because you are subscribed to this thread.
View it on GitHub:
https://github.com/quicwg/base-drafts/pull/977/files/72c98c4afdc64d1e3dd147526b68dae42021c1ee..252c8cee6c2fa39eb1291eb3ffc9a011e7be813a

---

From: janaiyengar <[email-masked]>
Date: Wed, 06 Dec 2017 18:00:58 -0800

Merging. Please add a comment if you want to change something post-merge, happy to do 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/977#issuecomment-349837866

---

From: janaiyengar <[email-masked]>
Date: Thu, 07 Dec 2017 02:01:25 +0000 (UTC)

Merged #977.

-- 
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/977#event-1376058494

---

From: ianswett <[email-masked]>
Date: Wed, 06 Dec 2017 18:54:29 -0800

Nope, LGTM, thanks for the improved 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/977#issuecomment-349847278