Subject: [quicwg/base-drafts] Update initial congestion window on MTU decrease (#3998)

From: ianswett <[email-masked]>
Date: Thu, 13 Aug 2020 13:39:52 -0700

Fixes #3997 by specifying that IW should be updated if the max packet size decreases in order to complete the handshake.
You can view, comment on, or merge this pull request online at:

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

-- Commit Summary --

  * Update initial congestion window on MTU decrease

-- File Changes --

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

-- Patch Links --

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

---

From: Martin Thomson <[email-masked]>
Date: Thu, 13 Aug 2020 18:50:07 -0700

@martinthomson commented on this pull request.

> +If the maximum datagram size is decreased in order to complete the handshake,
+the initial congestion window SHOULD be recalculated with the new smaller size.
+This avoids overly large initial congestion windows.  The initial window is
+not recalculated at any other time.

So if we are forced to reset to the initial window after going idle, we can't reset the congestion controller and calculate an initial using the current path MTU?  It seems to me like you might reasonably assume that an MTU is viable for longer than a congestion window.

-- 
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/3998#pullrequestreview-467259199

---

From: ianswett <[email-masked]>
Date: Sat, 15 Aug 2020 17:34:14 -0700

@ianswett commented on this pull request.

I agree with your second sentence, but I'm a bit confused about what you're asking.

The intent of the text to to say if you start with max datagram size X and drop down to X/2, because packets of size X aren't making it to the peer, then you should recalculate the initial window based on X/2.

-- 
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/3998#discussion_r471048210

---

From: ianswett <[email-masked]>
Date: Sat, 15 Aug 2020 17:35:20 -0700

@ianswett pushed 1 commit.

b01abe68367e571b6593ebb02fcd91cf674130d5  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/3998/files/1ae70bae19212489407d10e1878d27191182b93d..b01abe68367e571b6593ebb02fcd91cf674130d5

---

From: Martin Thomson <[email-masked]>
Date: Sun, 16 Aug 2020 18:41:55 -0700

So I'm concerned about other places we use the initial window.  One is where you go idle and decide to reset the congestion window.  Another is where you move to a new path.  The former is interesting in this case because the MTU might carry across.  The latter, less so.

-- 
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/3998#discussion_r471192053

---

From: Gorry Fairhurst <[email-masked]>
Date: Mon, 17 Aug 2020 02:15:51 -0700

@gorryfair approved this pull request.

Thanks: I'd be happy with the proposed text. This text doesn't go as far as RFC6928, but to me the need to reduce the cwnd/IW is clear.

-- 
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/3998#pullrequestreview-468298241

---

From: ianswett <[email-masked]>
Date: Mon, 17 Aug 2020 06:54:58 -0700

Is your specific concern that you pick a max datagram size during the handshake, and then do PMTUD and find the path supports a larger datagram size, but you never increase the initial congestion window?

-- 
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/3998#discussion_r471494928

---

From: Jana Iyengar <[email-masked]>
Date: Tue, 18 Aug 2020 14:13:34 -0700

@janaiyengar commented on this pull request.

I agree with @martinthomson. This can be made more general than just the handshake. Whenever there is a use of an IW, for bursts as an example, this recommendation applies. Is there a reason to make it specific to the handshake only?

-- 
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/3998#discussion_r472496415

---

From: Jana Iyengar <[email-masked]>
Date: Tue, 18 Aug 2020 14:13:58 -0700

See comment

-- 
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/3998#pullrequestreview-469835246

---

From: ianswett <[email-masked]>
Date: Tue, 18 Aug 2020 15:07:30 -0700

Not really, I was attempting to make the smallest change that fixed the issue.

-- 
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/3998#discussion_r472520483

---

From: ianswett <[email-masked]>
Date: Tue, 18 Aug 2020 15:16:36 -0700

5af85b5602b9562d13e2b94933eebdf4b9f6bf62  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/3998/files/b01abe68367e571b6593ebb02fcd91cf674130d5..5af85b5602b9562d13e2b94933eebdf4b9f6bf62

---

From: Martin Thomson <[email-masked]>
Date: Tue, 18 Aug 2020 15:16:59 -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/3998#pullrequestreview-469868244

---

From: Jana Iyengar <[email-masked]>
Date: Tue, 18 Aug 2020 16:05:03 -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/3998#pullrequestreview-469887654

---

From: Gorry Fairhurst <[email-masked]>
Date: Wed, 19 Aug 2020 01:31:31 -0700

@gorryfair commented on this pull request.

My comment related to the handshake because I saw that as most important. I agree though with Jana, a change of PMTU can require that the IW is updated, given that IW is also used after the handshake and might be cached for future connections. Could we change the final sentence in the PR to say something like: The initial window SHOULD also be recalculated when a change of the path MTU is detected.

-- 
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/3998#discussion_r472856788

---

From: ianswett <[email-masked]>
Date: Wed, 19 Aug 2020 09:49:04 -0700

@gorryfair I updated the PR to be more general and the first sentence now says
"If the maximum datagram size changes during the connection, the initial
congestion window SHOULD be recalculated with the new 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/3998#discussion_r473176383