Subject: [quicwg/base-drafts] Identify streams unidirectional vs bidirectional based on stream ID (#872)

From: Ryan Hamilton <[email-masked]>
Date: Fri, 13 Oct 2017 18:10:57 +0000 (UTC)

Identify streams unidirectional vs bidirectional based on stream ID, much like how client vs server created is identified.
You can view, comment on, or merge this pull request online at:

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

-- Commit Summary --

  * Identify unidirectional streams by the 2nd bit in the stream ID
  * Address nits

-- File Changes --

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

-- Patch Links --

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

---

From: Ryan Hamilton <[email-masked]>
Date: Fri, 13 Oct 2017 18:20:52 +0000 (UTC)

This identifies unidirectional vs bidirectional streams by a bit in the stream ID, just like we do for client vs server. This avoids the need to pass around directionality meta-data in the various places we use stream id (STREAM frames, MAX_STREAM_ID, MAX_STREAM_DATA, etc) and means that stream id is always a unique identifier of a single 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/872#issuecomment-336530372

---

From: janaiyengar <[email-masked]>
Date: Fri, 13 Oct 2017 11:35:30 -0700

janaiyengar commented on this pull request.

A couple of comments inline. This PR needs discussion on the list, so I would post an email there to encourage folks to look at the PR. I also think this closes #175 so I would say that in the PR description.

> @@ -2525,6 +2542,7 @@ shown in the following figure and described below.
    recv:   endpoint receives this frame
 
    STREAM: a STREAM frame
+   UNI:    a unidirectional stream

nit: a STREAM frame with a unidirectional stream ID

> @@ -2544,6 +2562,12 @@ coordinate the creation of streams; they are created unilaterally by either
 endpoint.  Endpoints can use acknowledgments to understand the peer's subjective
 view of stream state at any given time.
 
+Only the initiator of a stream may send a STREAM frame without receiving one
+unless otherwise specified by the application.  All implicitly opened streams

This sentence is a bit confusing. I don't think we want to allow applications to override stream ID semantics, do we?

-- 
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/872#pullrequestreview-69313367

---

From: Ryan Hamilton <[email-masked]>
Date: Fri, 13 Oct 2017 18:55:20 +0000 (UTC)

@RyanAtGoogle pushed 1 commit.

a9b13de  invert

-- 
You are receiving this because you are subscribed to this thread.
View it on GitHub:
https://github.com/quicwg/base-drafts/pull/872/files/5061a15eb804b1c090fda3439fadbc12c58dfd3e..a9b13de5d57ea4ffb82ac507931109275336cd3f

---

From: Ryan Hamilton <[email-masked]>
Date: Fri, 13 Oct 2017 19:18:17 +0000 (UTC)

4137259  clarify

-- 
You are receiving this because you are subscribed to this thread.
View it on GitHub:
https://github.com/quicwg/base-drafts/pull/872/files/a9b13de5d57ea4ffb82ac507931109275336cd3f..41372597f7ee9b7bb0cbb5d11690cfc6b84a5f70

---

From: Ryan Hamilton <[email-masked]>
Date: Fri, 13 Oct 2017 19:20:32 +0000 (UTC)

0740371  more

-- 
You are receiving this because you are subscribed to this thread.
View it on GitHub:
https://github.com/quicwg/base-drafts/pull/872/files/41372597f7ee9b7bb0cbb5d11690cfc6b84a5f70..07403710273bd18f1f5962b924b75344b3d70722

---

From: Ryan Hamilton <[email-masked]>
Date: Fri, 13 Oct 2017 19:20:42 +0000 (UTC)

RyanAtGoogle commented on this pull request.

Not sure, I copied this language from Ian's PR on this topic. Perhaps I should just nuke the whole paragraph?

    STREAM: a STREAM frame

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/872#discussion_r144637342

---

From: ianswett <[email-masked]>
Date: Fri, 13 Oct 2017 19:22:22 +0000 (UTC)

ianswett commented on this pull request.

Yes, this was an effort to fix a problem with my design that you do not have, so I'd nuke 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/872#discussion_r144637769

---

From: Ryan Hamilton <[email-masked]>
Date: Fri, 13 Oct 2017 12:33:24 -0700

afabe8a  Remove unnecessary language

-- 
You are receiving this because you are subscribed to this thread.
View it on GitHub:
https://github.com/quicwg/base-drafts/pull/872/files/07403710273bd18f1f5962b924b75344b3d70722..afabe8a7596999ab4d49463cc144a618e4e184dc

---

From: Ryan Hamilton <[email-masked]>
Date: Fri, 13 Oct 2017 12:33:39 -0700

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/872#discussion_r144639919

---

From: Martin Thomson <[email-masked]>
Date: Mon, 16 Oct 2017 00:30:14 +0000 (UTC)

Firstly, thanks for this PR, I think that it is almost exactly what we need.  I have two concerns, the first of which we should resolve before merging this.

The encoding of type into the low bits of the identifier is problematic.  I would prefer that we encode an explicit type field into the stream ID encoding.  The number of bits doesn't change, but the way that we identify the streams could.  I suspect that this is going to generate a ton of angry emails, in the same way that suggesting that 0 being special did, but I find the encoding of the type into the low bits to make certain operations more difficult than is ideal.

If you look at the proposed change in #877, I would define a modified stream ID encoding that steals two bits before the length bits.  It's less uniform in terms of the integer encoding, but you have a predictable location to look for the stream type.

The state machine here needs some serious work, but that can be deferred.  I would prefer that we wait for someone to transcribe the state machine that Mike shared in the Seattle interim.

-- 
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/872#issuecomment-336753522

---

From: MikkelFJ <[email-masked]>
Date: Mon, 16 Oct 2017 00:47:11 +0000 (UTC)

While I am also concerned with the complexity of adding stream type into the ID (it adds a lot of special cases, but likely most alternatives would too), I don't think it is a good idea to have several ways of decoding varlen integers. It is best to have a single function to deal with endianness and unaligned read access and decoding, rather than several implementations.

If the bits are to be relocated, they could be placed in the frame type which can the be placed in the upper bits after decoding. But unfortunately that adds a lot of sub-frame types as Ian pointed out. Still, I think I prefer this. It makes it possible to immediately branch out on stream type.

-- 
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/872#issuecomment-336754603

---

From: Martin Thomson <[email-masked]>
Date: Mon, 16 Oct 2017 00:53:23 +0000 (UTC)

@mikkelfj, I originally wanted to move the bits to the frame type; but you would need three of each type of frame that mentions streams and there are 6 frame types to modify in that way.  On balance, moving the identification into the same slot that we encode the stream ID as @RyanAtGoogle proposes might be better. That said, the HTTP mapping only uses certain specific types of streams, so maybe it could work out.

I forgot one comment: this PR doesn't address the initial values for MAX_STREAM_ID and MAX_STREAM_DATA.  I think that we should have separate configuration for those.  That suggests that maybe a separate frame type isn't so bad an idea.

-- 
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/872#issuecomment-336755025

---

From: janaiyengar <[email-masked]>
Date: Mon, 16 Oct 2017 15:08:18 -0700

@martinthomson On using the high-order bits instead of the low-order ones, I don't have a strong opinion, but it's not complicated to use low-order ones. We've done that for client/server streams, this merely extends it.

I'm happy with the PR (modulo adding default MAX_STREAM_ID and MAX_STREAM_DATA separately for uni and bi streams), and I think we can make further changes after we get this one in.

-- 
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/872#issuecomment-337058116

---

From: MikkelFJ <[email-masked]>
Date: Mon, 16 Oct 2017 15:20:04 -0700

I also believe it is simpler to keep bits low rather than having a non-standard varlen integer format.

-- 
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/872#issuecomment-337060597

---

From: ekr <[email-masked]>
Date: Mon, 16 Oct 2017 23:55:08 +0000 (UTC)

I'm generally OK with this strategy, but I agree with MT that I'd rather use the high-order bits of the stream ID as control bits (for both the direction and for the bi/uni switch). This just gives you four spaces that you can talk about sensibly.

-- 
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/872#issuecomment-337077083

---

From: janaiyengar <[email-masked]>
Date: Mon, 16 Oct 2017 17:00:54 -0700

You get four spaces either way -- you can use the high-order bits or the low-order ones. ... and we'd lose the high-order bits to size if we went the direction of PR 877. In the context of that PR, I definitely prefer low-order bits.

-- 
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/872#issuecomment-337077956

---

From: Mike Bishop <[email-masked]>
Date: Tue, 17 Oct 2017 22:51:24 +0000 (UTC)

Yes, if we go with the direction of #877, the type bits need to be held separately from the Stream ID.  I agree that's challenging, but doesn't seem insurmountable, and feels cleaner than a mod-4 space.  I never liked the even-odd approach to begin with, I didn't like the mod-4 structure when it was in the HTTP/QUIC draft, and I really don't want to see mod-4 invading the QUIC transport.

That said, it's a bikeshed.  The bits exist, and exactly where they land in the frame doesn't ultimately change the state model of the protocol.

-- 
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/872#issuecomment-337399450

---

From: Martin Thomson <[email-masked]>
Date: Tue, 17 Oct 2017 16:33:49 -0700

So that I don't forget this... This PR omits two very important things:

1. It doesn't actually say what a unidirectional stream *is*.

2. It doesn't say anything about stream usage in HTTP.

As we discussed on the editor's call today, we are going to merge this, then fix it.  I'll prioritize fixing the first issue though.

Given the HTTP usage, I'm going to push harder for a separation of the type bits, because that will make the HTTP usage of the identifiers easier (it has implicit type identification).  That will have to wait until #877 resolves.  That also needs some HTTP 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/872#issuecomment-337409206

---

From: Martin Thomson <[email-masked]>
Date: Wed, 18 Oct 2017 03:42:30 +0000 (UTC)

martinthomson commented on this pull request.

> @@ -2472,6 +2476,18 @@ corresponds to a stream which is allocated to it (i.e., odd-numbered for
 the client or even-numbered for the server) but which it has not yet
 created, it MUST close the connection with error code STREAM_STATE_ERROR.
 
+The second least significant bit differentiates between unidirectional streams
+and bidirectional streams. Unidirectional streams always have this bit set to
+1 and bidirectional streams have this bit set to 0. As a result the initial
+stream ID for various stream types is listed below:
+
+| Stream ID   | Type                          |
+|:------------|:------------------------------|
+| 0x01        | Client Bidirectional          |
+| 0x02        | Server Bidirectional          |
+| 0x03        | Client Unidirectional         |
+| 0x04        | Server Unidirectional         |

Hmm, this table is wrong 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/872#pullrequestreview-70090652

---

From: Ryan Hamilton <[email-masked]>
Date: Wed, 18 Oct 2017 22:31:01 +0000 (UTC)

On Tue, Oct 17, 2017 at 4:33 PM, Martin Thomson <[email-masked]>
wrote:

>    1.
>    It doesn't actually say what a unidirectional stream *is*.
>
> ​Doh.​
​ I'm not awesome at spec language. I tried to define this implicitly by
saying that "

​"​
Unidirectional streams created by the peer are
​ ​
immediately half-closed (local) to the receiver and are subsequently treated
​ ​
identically to a half-closed bidirectional stream.
​"​
​

But that's probably not terribly useful. Have a suggestion on test?

>
>    1.
>    2.
>
>    It doesn't say anything about stream usage in HTTP.
>
> As we discussed on the editor's call today, we are going to merge this,
> then fix it. I'll prioritize fixing the first issue though.
>

​Are you planning to merge this as is, or shall I address some of the other
feedback (the table, for example).​

> Given the HTTP usage, I'm going to push harder for a separation of the
> type bits, because that will make the HTTP usage of the identifiers easier
> (it has implicit type identification). That will have to wait until #877
> <https://github.com/quicwg/base-drafts/pull/877> resolves. That also
> needs some HTTP text.
>
> —
> You are receiving this because you were mentioned.
> Reply to this email directly, view it on GitHub
> <https://github.com/quicwg/base-drafts/pull/872#issuecomment-337409206>,
> or mute the thread
> <https://github.com/notifications/unsubscribe-auth/ASp6yt7lvT26YAtK3F1nj3Fql4Tu3SDAks5stTldgaJpZM4P4257>
> .
>

-- 
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/872#issuecomment-337746754

---

From: Martin Thomson <[email-masked]>
Date: Wed, 18 Oct 2017 22:56:29 +0000 (UTC)

Hi Ryan, maybe you could provide feedback on #885.  If we can't agree on that, then maybe we need to defer the change until we do agree.

-- 
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/872#issuecomment-337751343

---

From: janaiyengar <[email-masked]>
Date: Thu, 26 Oct 2017 19:37:31 +0000 (UTC)

I think this is good to go. We have agreement on #885, so let's land this and then land that one. I'll wait 24 hours and then merge.

-- 
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/872#issuecomment-339777473

---

From: janaiyengar <[email-masked]>
Date: Thu, 26 Oct 2017 19:37:48 +0000 (UTC)

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/872#pullrequestreview-72315622

---

From: Martin Thomson <[email-masked]>
Date: Sun, 29 Oct 2017 21:26:57 -0700

Merged #872.

-- 
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/872#event-1315955608