Subject: Re: [quicwg/base-drafts] Add LIMIT_UPDATE and replaces concurrent stream limit (#421)

From: Mike Bishop <[email-masked]>
Date: Thu, 13 Apr 2017 14:52:10 -0700

Well, I'd typically say you add a flag, but QUIC embeds flags into the frame type, so a frame with one flag is equivalent to two frames.  We could pick another sentinel value for Stream ID (0xFFFFFFFF?), but that wastes quite a few bytes (since the window is 64 bits and you'll never need more than 32 to state a stream limit.

We could state the stream limit in each WINDOW_UPDATE frame, perhaps.  I don't have data on how quickly one changes versus the other.  Given that we have two levels of flow control and stream flow control seems likely to update more quickly than open streams, that seems wasteful as well.

What we _could_ perhaps do is remove the Stream 0 sentinel value from WINDOW_UPDATE and have a separate frame for connection flow control that gave you both a new max byte count and max stream ID in one shot.  That's a larger change, but if it's appealing to folks...?

What gets hairy there is that we say to ignore a WINDOW/LIMIT_UPDATE that doesn't increase what you're allowed; by having two values in one frame, you make it possible to send invalid combinations.

-- 
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/421#issuecomment-294031346

---

From: Mike Bishop <[email-masked]>
Date: Thu, 13 Apr 2017 14:59:55 -0700

@MikeBishop pushed 1 commit.

2101e69  Merge remote-tracking branch 'origin/master' into stream_limit

-- 
You are receiving this because you are subscribed to this thread.
View it on GitHub:
https://github.com/quicwg/base-drafts/pull/421/files/5f3be1a0eca9f26a01e9930aaf708c0f9e5911d7..2101e6938aa94837ceb702d17eac7e7906aa89f0

---

From: Martin Thomson <[email-masked]>
Date: Mon, 17 Apr 2017 23:10:30 -0700

martinthomson requested changes on this pull request.

How does this interact with GOAWAY?  I don't think that it does, but it might remove the need for GOAWAY in some cases.

Can an endpoint declare that they are blocked (with BLOCKED) on concurrent stream limits?

> @@ -788,6 +788,7 @@ explained in more detail as they are referenced later in the document.
 | 0x04             |  WINDOW_UPDATE     | {{frame-window-update}}    |
 | 0x05             |  BLOCKED           | {{frame-blocked}}          |
 | 0x07             |  PING              | {{frame-ping}}             |
+| 0x08             |  LIMIT_UPDATE      | {{frame-limit-update}}     |

I really don't like this name.  (I don't like WINDOW_UPDATE either).  How about `MAX_STREAM_ID`.

 Endpoints MUST NOT exceed the limit set by their peer.  An endpoint that
-receives a STREAM frame that causes its advertised concurrent stream limit to be
-exceeded MUST treat this as a stream error of type QUIC_TOO_MANY_OPEN_STREAMS
-({{error-handling}}).
-
+receives a STREAM frame with an ID greater than the limit it has sent MUST treat
+this as a stream error of type QUIC_TOO_MANY_OPEN_STREAMS ({{error-handling}}),
+unless this is a result of a change in the initial offsets (see
+{{zerortt-parameters}}).
+
+A Maximum Stream ID of 0 will cause a connection to be unusable.  Application

1 also makes the connection unusable because stream 1 counts.

-- 
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/421#pullrequestreview-33148527

---

From: Marten Seemann <[email-masked]>
Date: Tue, 18 Apr 2017 00:52:11 -0700

marten-seemann commented on this pull request.

1 is a valid value. It allows the handshake to complete. The peers can always advertise a higher value 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/421#discussion_r111890202

---

From: Marten Seemann <[email-masked]>
Date: Tue, 18 Apr 2017 00:57:11 -0700

Endpoints MUST NOT exceed the limit set by their peer.  An endpoint that
-receives a STREAM frame that causes its advertised concurrent stream limit to be
-exceeded MUST treat this as a stream error of type QUIC_TOO_MANY_OPEN_STREAMS
-({{error-handling}}).
-
+receives a STREAM frame with an ID greater than the limit it has sent MUST treat
+this as a stream error of type QUIC_TOO_MANY_OPEN_STREAMS ({{error-handling}}),
+unless this is a result of a change in the initial offsets (see
+{{zerortt-parameters}}).
+
+A Maximum Stream ID of 0 will cause a connection to be unusable.  Application
+protocols that use QUIC might require a certain minimum number of initial
+streams to function correctly.  If a peer advertises an maximum stream ID
+(initial_stream_limit) that is too small for the selected application protocol

I don't it's necessary to define an error condition here.
If the server sets a too small initial_stream_limit, this just means that the client can't immediately open the streams requested by the application, but it has to wait until the server sends a LIMIT_UPDATE.

-- 
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/421#pullrequestreview-33163629

---

From: Martin Thomson <[email-masked]>
Date: Tue, 18 Apr 2017 00:59:32 -0700

martinthomson commented on this pull request.

Of course, that is the advantage of this design.

-- 
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/421#discussion_r111891424

---

From: Mike Bishop <[email-masked]>
Date: Tue, 18 Apr 2017 09:17:28 -0700

MikeBishop commented on this pull request.

1 will likely make any application protocol unusable, either because it requires a control stream to open immediately (HTTP) or because the connection will be unable to serve any requests.  However, for something like the "connect and PING" test you proposed for first-interop (maybe ALPN token of "none"?), "1" would be an appropriate value to advertise.

-- 
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/421#discussion_r111998118

---

From: Mike Bishop <[email-masked]>
Date: Tue, 18 Apr 2017 09:18:09 -0700

If we have a BLOCKED analogue, that seems reasonable.  It effectively becomes a warning rather than a fatal error.

-- 
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/421#discussion_r111998305

---

From: Martin Thomson <[email-masked]>
Date: Tue, 18 Apr 2017 22:27:05 -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/421#pullrequestreview-33407491

---

From: Martin Thomson <[email-masked]>
Date: Tue, 18 Apr 2017 22:27:37 -0700

Approving this; I have a plan to rename the frame type now and the other comments were addressed already.

-- 
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/421#issuecomment-295099957

---

From: Mike Bishop <[email-masked]>
Date: Wed, 19 Apr 2017 07:24:00 -0700

Maybe MAX_STREAM_ID and MAX_OFFSET?  The names aren't really folded into the wire, so they're technically editorial.

-- 
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/421#issuecomment-295287908

---

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

Pretty close to what I had in mind.  With #443 I think we can have: STREAM_LIMIT, DATA_LIMIT and STREAM_DATA_LIMIT.  Or as you suggest MAX_STREAM_ID, MAX_STREAM_DATA_OFFSET, and MAX_DATA_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/421#issuecomment-295509433

---

From: Martin Thomson <[email-masked]>
Date: Wed, 19 Apr 2017 20:03:49 -0700

Closed #421.

-- 
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/421#event-1049769086

---

From: Martin Thomson <[email-masked]>
Date: Wed, 19 Apr 2017 20:03:49 -0700

Merged manually.

-- 
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/421#issuecomment-295561454

---

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

janaiyengar commented on this pull request.

Agreed that this surely doesn't seem worthy of a MUST terminate. I also don't think TOO_MANY_OPEN_STREAMS is the right error to use here.

-- 
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/421#discussion_r112366250

---

From: MikkelFJ <[email-masked]>
Date: Thu, 20 Apr 2017 02:41:19 -0700

mikkelfj commented on this pull request.

If the connection is not closed no a MAX_STREAM_ID violation, both sides need to update state to treat the violating stream as closed/reset and not to be used again, or at least some rules should clarify what should happen. Also, if such a violation happens, there would likely come many more because the sender is not keeping track. Alternatively, the sender could use a RST with error as control signal to back off, but that is just more complex state handling. So the simplest solution seems to be to close the entire connection and simply require sender to respect the limit.

-- 
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/421#discussion_r112410704

---

From: Martin Thomson <[email-masked]>
Date: Thu, 20 Apr 2017 02:57:08 -0700

We require that exceeding the limit is punished with an error closure.  But even if we didn't (see the 0-RTT exception), the connection still works when there are too many streams.  The specific error code that Marten is talking about here is the case where you don't have enough streams to operate: a condition that is easily rectified by increasing the maximum stream ID.

-- 
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/421#discussion_r112414178

---

From: MikkelFJ <[email-masked]>
Date: Thu, 20 Apr 2017 03:05:45 -0700

My comment was aimed at @janaiyengar 's comment. I think I agree with Marten - I have not read up on 0-RTT.

-- 
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/421#discussion_r112415824