Subject: [quicwg/base-drafts] Flow control accounting uses the maximum data offset on each stream (#378)

From: Martin Thomson <[email-masked]>
Date: Wed, 08 Mar 2017 22:57:00 -0800

Also, scales the connection-level flow control window by 1024

Closes #370, #340.

Note: I was just going to fix these separately, but they both hit exactly the same text.  Seemed like I might as well kill two birds.
You can view, comment on, or merge this pull request online at:

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

-- Commit Summary --

  * Flow control accounting uses the maximum data offset on each stream

-- File Changes --

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

-- Patch Links --

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

---

From: Marten Seemann <[email-masked]>
Date: Wed, 08 Mar 2017 23:32:34 -0800

marten-seemann commented on this pull request.

> -Byte offset:
+Flow Control Offset:
+
+: A 64-bit unsigned integer indicating the flow control offset for the given
+  stream (for a stream ID other than 0) or the entire connection.
+
+The flow control offset is expressed in units of octets for individual streams
+(for stream identifiers other than 0).
+
+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.
+
+An endpoint accounts for the maximum offset of data that is sent or received on
+a stream.  Loss or reordering can mean that the size of STREAM frames can be
+less than the maximum data offset on a stream; similarly, STREAM frames might

What does the size of a STREAM frame have to do with the maximum offset?

-- 
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/378#pullrequestreview-25963452

---

From: Marten Seemann <[email-masked]>
Date: Wed, 08 Mar 2017 23:39:23 -0800

connection_fc_offset (0x0001):
 
 : The connection level flow control offset parameter contains the initial
-  connection flow control window encoded as an unsigned 32-bit integer.  The
-  sender of this parameter sets the byte offset for connection level flow
-  control to this value.  This is equivalent to sending a WINDOW_UPDATE
-  ({{frame-window-update}}) for stream 0 immediately after completing the
-  handshake.
+  connection flow control window encoded as an unsigned 32-bit integer in units
+  of 1024 octets.  That is, the value here is multiplied by 1024 to determine
+  the actual flow control offset.  The sender of this parameter sets the byte
+  offset for connection level flow control to this value.  This is equivalent to
+  sending a WINDOW_UPDATE ({{frame-window-update}}) for stream 0 immediately

Better: for the connection. There is no stream 0.

-- 
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/378#pullrequestreview-25964212

---

From: Mike Bishop <[email-masked]>
Date: Thu, 09 Mar 2017 14:30:40 -0800

MikeBishop approved this pull request.

Minor nits.  Looks fine.

+Flow Control Offset:
+

"...can mean that the maximum offset is greater than the total size of data received on a stream."

+
+not cause the maximum offset on a stream.  A STREAM frame with a FIN bit set or

"...might not cause the maximum offset on a stream **to increase**."

-- 
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/378#pullrequestreview-26161482

---

From: Martin Thomson <[email-masked]>
Date: Thu, 09 Mar 2017 16:03:23 -0800

@martinthomson pushed 1 commit.

8248c50  Review comments

-- 
You are receiving this because you are subscribed to this thread.
View it on GitHub:
https://github.com/quicwg/base-drafts/pull/378/files/1065da6fbf9e3730043b9887174b34d42c07dc1f..8248c503279c65376937d341f55b8c20b933f0be

---

From: ianswett <[email-masked]>
Date: Fri, 10 Mar 2017 10:09:52 -0800

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/378#pullrequestreview-26340212

---

From: janaiyengar <[email-masked]>
Date: Fri, 10 Mar 2017 12:16:23 -0800

janaiyengar approved this pull request.

One thought, but looks fine. I'll merge anyways.

> @@ -1530,12 +1532,31 @@ Stream ID:
 : ID of the stream whose flow control windows is being updated, or 0 to specify
   the connection-level flow control window.
 
-Byte offset:
+Flow Control Offset:
+

Why not use a 1K multiplier for both stream and connection-level?

-- 
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/378#pullrequestreview-26366929

---

From: janaiyengar <[email-masked]>
Date: Fri, 10 Mar 2017 12:16:36 -0800

Merged #378.

-- 
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/378#event-995492061

---

From: Martin Thomson <[email-masked]>
Date: Sun, 12 Mar 2017 17:25:27 -0700

martinthomson commented on this pull request.

-Byte offset:
+Flow Control Offset:
+

I considered it, but streams don't need as much space and they might benefit from a more granular expression of where they are up to.  We can refine this later.

-- 
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/378#discussion_r105574116