Subject: [quicwg/base-drafts] Split WINDOW_UPDATE into MAX_DATA and MAX_STREAM_DATA (#450)

From: Martin Thomson <[email-masked]>
Date: Wed, 19 Apr 2017 21:27:45 -0700

This also changes the name of the LIMIT_UPDATE frame to MAX_STREAM_ID to match.

A lot of the text talked about stream offsets and had flow control affect
maximum stream offsets.  This turned out to be unnecessarily obtuse.  The text
in this changeset simply says that there is a limit to the amount of data that
can be sent.  This turns out to be a lot of changes, but I think that it is
easier to understand as a result.

Closes #443.
You can view, comment on, or merge this pull request online at:

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

-- Commit Summary --

  * Split WINDOW_UPDATE into MAX_DATA and MAX_STREAM_DATA

-- File Changes --

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

-- Patch Links --

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

---

From: Martin Thomson <[email-masked]>
Date: Wed, 19 Apr 2017 21:38:52 -0700

@martinthomson pushed 1 commit.

eba4f04  The hazards of renaming things automatically

-- 
You are receiving this because you are subscribed to this thread.
View it on GitHub:
https://github.com/quicwg/base-drafts/pull/450/files/84987e4770803e2d927872a0589496fec57c883b..eba4f043d8d55c86fe6db9d0e2e0c22ed66c44c2

---

From: Martin Thomson <[email-masked]>
Date: Wed, 19 Apr 2017 21:44:29 -0700

b825954  Fix IANA table

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

---

From: janaiyengar <[email-masked]>
Date: Wed, 19 Apr 2017 22:37:09 -0700

janaiyengar commented on this pull request.

I like this change -- a few comments inline

-The connection-level flow control offset is expressed in units of 1024 octets
-(for a stream identifier of 0).  That is, the connection-level flow control
-offset is determined by multiplying the encoded value by 1024.
+When counting data toward this limit, an endpoint accounts for the maximum
+offset of data that is sent or received on every stream.  Loss or reordering can
+mean that the maximum offset is greater than the total size of data received on
+a stream.  Receiving STREAM frames might not increase the maximum offset on a

"Loss or reordering can
 +mean that the maximum offset is greater than the total size of data received on
 +a stream." -- Not sure what this means.

-The connection-level flow control offset is expressed in units of 1024 octets
-(for a stream identifier of 0).  That is, the connection-level flow control
-offset is determined by multiplying the encoded value by 1024.
+When counting data toward this limit, an endpoint accounts for the maximum
+offset of data that is sent or received on every stream.  Loss or reordering can
+mean that the maximum offset is greater than the total size of data received on
+a stream.  Receiving STREAM frames might not increase the maximum offset on a

This sentence does not make sense: "Receiving STREAM frames might not increase the maximum offset on a stream if the maximum offset doesn't increase."

-The connection-level flow control offset is expressed in units of 1024 octets
-(for a stream identifier of 0).  That is, the connection-level flow control
-offset is determined by multiplying the encoded value by 1024.
+When counting data toward this limit, an endpoint accounts for the maximum
+offset of data that is sent or received on every stream.  Loss or reordering can
+mean that the maximum offset is greater than the total size of data received on
+a stream.  Receiving STREAM frames might not increase the maximum offset on a
+stream if the maximum offset doesn't increase.  A STREAM frame with a FIN bit
+set or RST_STREAM causes the final offset for a stream to be fixed.

I don't think you need to describe how RST_STREAM or FIN affects STREAM offsets here. MAX_DATA is for Connection flow control, and this information is useful only for STREAM flow control.

 A receiver MUST close the connection with a
 QUIC_FLOW_CONTROL_RECEIVED_TOO_MUCH_DATA error ({{error-handling}}) if the
-peer violates the advertised stream or connection flow control windows.
+peer violates the advertised limits on data on a connection or stream.

suggestion: "violates the advertised connection or stream data limits."

 ### Stream Limit Increment
 
 As with flow control, this document leaves when and how many streams to make
-available to a peer via LIMIT_UPDATE to the implementation, but offers a few
-considerations. LIMIT_UPDATE frames constitute minimal overhead, while
-withholding LIMIT_UPDATEs prevents the peer from fully utilizing the transport.
+available to a peer via MAX_STREAM_ID to implementations, but offers a few
+considerations.  MAX_STREAM_ID frames constitute minimal overhead, while
+withholding MAX_STREAM_ID frames can prevent the peer from using the available
+parallelism.
+
+Implementations will likely want to increase the maximum stream ID as
+peer-initiated streams close.  Sending a MAX_STREAM_ID frame along with ACK
+frames ensures a regular increase in the stream limit without generating excess

Sending a MAX_STREAM_ID with each ACK makes no sense. Perhaps you meant to say: "... along with acknowledgements of frames that close a stream"?

-- 
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/450#pullrequestreview-33675431

---

From: Martin Thomson <[email-masked]>
Date: Wed, 19 Apr 2017 22:39:39 -0700

martinthomson commented on this pull request.

Hmm, that's old text, it says that for any given stream max_offset >= sum([x.len for x in all_stream_frames]) (as opposed to ==).

-- 
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/450#discussion_r112368937

---

From: Martin Thomson <[email-masked]>
Date: Wed, 19 Apr 2017 22:40:07 -0700

It's true, just a little repetitive.

-- 
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/450#discussion_r112368973

---

From: Martin Thomson <[email-masked]>
Date: Wed, 19 Apr 2017 22:42:33 -0700

OK, dropping this (BTW, this is existing 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/450#discussion_r112369245

---

From: Martin Thomson <[email-masked]>
Date: Wed, 19 Apr 2017 22:44:41 -0700

Not exactly.  Implies a particular strategy for increasing the maximum stream ID, which I want to avoid.  The point is that you might send any MAX_STREAM_ID frames you might want to send on the same schedule as ACK.  I'll try to improve it some.

-- 
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/450#discussion_r112369601

---

From: Martin Thomson <[email-masked]>
Date: Wed, 19 Apr 2017 22:45:55 -0700

I realize that the context is bad for that, so I'll try to fix that too.

-- 
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/450#discussion_r112369867

---

From: Martin Thomson <[email-masked]>
Date: Wed, 19 Apr 2017 22:47:32 -0700

@martinthomson pushed 2 commits.

747885f  Jana's review comments
1f02a95  Fix frame type numbers

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

---

From: janaiyengar <[email-masked]>
Date: Thu, 20 Apr 2017 09:33:40 -0700

> @@ -1671,10 +1671,10 @@ Maximum Data:
 
 When counting data toward this limit, an endpoint accounts for the maximum
 offset of data that is sent or received on every stream.  Loss or reordering can
-mean that the maximum offset is greater than the total size of data received on
-a stream.  Receiving STREAM frames might not increase the maximum offset on a
-stream if the maximum offset doesn't increase.  A STREAM frame with a FIN bit
-set or RST_STREAM causes the final offset for a stream to be fixed.
+mean that the maximum offset on a stream can be greater than the total size of
+data received on that stream.  Conversely, receiving STREAM frames might not
+increase the maximum offset on a stream if the maximum offset in the frame is
+lower than the maximum received for that stream.

I understand now what this says, but it's confusing, since maximum offset and MAX_DATA both represent maxima. I would suggest using "largest received offset" instead of "maximum offset". 

> @@ -1671,10 +1671,10 @@ Maximum Data:
 
 When counting data toward this limit, an endpoint accounts for the maximum
 offset of data that is sent or received on every stream.  Loss or reordering can
-mean that the maximum offset is greater than the total size of data received on
-a stream.  Receiving STREAM frames might not increase the maximum offset on a
-stream if the maximum offset doesn't increase.  A STREAM frame with a FIN bit
-set or RST_STREAM causes the final offset for a stream to be fixed.
+mean that the maximum offset on a stream can be greater than the total size of
+data received on that stream.  Conversely, receiving STREAM frames might not
+increase the maximum offset on a stream if the maximum offset in the frame is

"might not" --> "will not". If the STREAM frame's largest offset is lower than the largest received so far, the largest received offset will not increase.

> @@ -2471,13 +2471,13 @@ considerations.  MAX_STREAM_ID frames constitute minimal overhead, while
 withholding MAX_STREAM_ID frames can prevent the peer from using the available
 parallelism.
 
-Implementations will likely want to increase the maximum stream ID as
-peer-initiated streams close.  Sending a MAX_STREAM_ID frame along with ACK
-frames ensures a regular increase in the stream limit without generating excess
+Sending any increase to the maximum stream ID in the same packet as an ACK frame
+ensures a regular increase in the stream limit without generating excess

This advice is true for all frames, including MAX_DATA and MAX_STREAM_DATA frames. I'd remove this sentence or say something about *when* to send the MAX_STREAM_ID frame rather than which frame to bundle it with. It might make sense to send it immediately after/along with a STREAM frame that carries a FIN or a RST_STREAM frame on a peer-initiated stream, for instance.

-- 
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/450#pullrequestreview-33813110

---

From: janaiyengar <[email-masked]>
Date: Thu, 20 Apr 2017 09:35:27 -0700

Can you move this text to the MAX_STREAM_DATA section instead? It was written for WINDOW_UPDATE, which covered both connection and stream flow control.

-- 
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/450#discussion_r112500549

---

From: janaiyengar <[email-masked]>
Date: Thu, 20 Apr 2017 09:37:23 -0700

An ACK is sent about every two packets received, which is not the schedule that a MAX_STREAM_ID frame would be sent. I've left a comment in my review of the new 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/450#discussion_r112500929

---

From: Martin Thomson <[email-masked]>
Date: Thu, 20 Apr 2017 16:37:56 -0700

Yep, I think that I will just remove the paragraph.  The advice on when to send is still present in the next paragraph, which is what you are looking for.

-- 
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/450#discussion_r112582950

---

From: Martin Thomson <[email-masked]>
Date: Thu, 20 Apr 2017 16:38:32 -0700

f76f760  More feedback from Jana

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

---

From: janaiyengar <[email-masked]>
Date: Tue, 25 Apr 2017 15:33:35 -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/450#pullrequestreview-34706704

---

From: janaiyengar <[email-masked]>
Date: Tue, 25 Apr 2017 15:33:41 -0700

Merged #450.

-- 
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/450#event-1057557727