Subject: [quicwg/base-drafts] Add bidirectional streams on top of unidirectional (#720)

From: Martin Thomson <[email-masked]>
Date: Fri, 11 Aug 2017 01:55:15 +0000 (UTC)

This is based on the PR in #643.  It adds the concept of "related streams", which allows for pairing of streams.  It recommends against using the concept for anything else, but notes that any semantics are the responsibility of the application protocol.  This modifies the HTTP draft to prohibit use of this capability - that draft has a superior mechanism for request-response association.

I've kept this separate from #643 so that it can be assessed independently.
You can view, comment on, or merge this pull request online at:

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

-- Commit Summary --

  * Add related streams
  * Prohibit their use

-- File Changes --

    M draft-ietf-quic-http.md (4)
    M draft-ietf-quic-transport.md (60)

-- Patch Links --

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

---

From: MikkelFJ <[email-masked]>
Date: Fri, 11 Aug 2017 07:45:44 +0000 (UTC)

mikkelfj commented on this pull request.

> +This document doesn't proscribe the use of the Related Stream ID field, leaving
+decisions about the semantics of related streams to protocols that use QUIC.
+Protocols that use QUIC MUST either define the semantics of related streams or
+prohibit the use of the Related Stream ID field.  Protocols MAY define other
+uses for the Related Stream ID field, but are advised that complex relationships
+between streams could be poorly supported in generic QUIC implementations.  The
+INVALID_RELATED_STREAM error code can be used by protocols to signal receipt of
+invalid Related Stream ID field values, unless the protocol defines a more
+specific error code for the circumstances.
+
+The Related Stream ID for a stream only needs to be sent once for each (allowing
+for retransmissions due to loss).  Sending the field on all stream frames with
+an offset of 0 ensures that the value is received without duplicating the field
+unnecessarily.  If the value of the Related Stream ID changes for a stream, the
+endpoint that receives the conflicting value MUST treat this as a connection
+error of type INVALID_RELATED_STREAM.

First part of paragraph could be shortened a bit to just state the related stream id is only sent in the first frame with offset zero (allowing for retransmission).

Second part of the paragraph is unclear - what does it mean to receive different ID's? It can only happen during retransmission because offset 0 can only be sent once. If due to retransmission, the protocol logic should only see one version and discard duplicates without interpretation (general protocol logic, though this might be not be explicit).

The second part could also be interpreted as if the original referenced stream can only be associated with a single stream and using multiple would be an error. But I do not believe this is the intention, and if it is, I don't think it is reasonable to place any restrictions here. But - it might be good to add a note stating the a field may be referenced 0, 1, or more times depending on application semantics.

-- 
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/720#pullrequestreview-55732015

---

From: MikkelFJ <[email-masked]>
Date: Fri, 11 Aug 2017 00:48:00 -0700

+specific error code for the circumstances.
+
+The Related Stream ID for a stream only needs to be sent once for each (allowing
+for retransmissions due to loss).  Sending the field on all stream frames with
+an offset of 0 ensures that the value is received without duplicating the field
+unnecessarily.  If the value of the Related Stream ID changes for a stream, the
+endpoint that receives the conflicting value MUST treat this as a connection
+error of type INVALID_RELATED_STREAM.
+
+A related stream MUST NOT refer to a stream in the "idle" state
+({{stream-state-idle}}).  Receipt of a STREAM frame that includes a Related
+Stream ID field that identifies an "idle" stream MUST be treated as a connection
+error of type INVALID_RELATED_STREAM.
+
+
 ## Sending and Receiving Data

Isn't this redundant from earlier paragraph line 2229?

-- 
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/720#pullrequestreview-55732329

---

From: MikkelFJ <[email-masked]>
Date: Fri, 11 Aug 2017 01:05:08 -0700

My initial comment was thinking about packet duplicates which is not reasonable since frames are retransmitted independently of packets. However, each steam has its own retransmission filter and here  I believe the same applies: already received content should be ignored - it is not reasonable to check all received retransmission for inconsistencies, and related stream ID would just be a special case - unless I am missing something?

-- 
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/720#discussion_r132633408

---

From: Mike Bishop <[email-masked]>
Date: Tue, 15 Aug 2017 09:30:30 -0700

MikeBishop approved this pull request.

Looks nice and simple -- if applications want something fancier, they can freely build it, but simple applications can use this.  Mostly nits.

> @@ -2215,6 +2220,16 @@ Offset:
   on a stream - the sum of the re-constructed offset and data length - MUST be
   less than 2^64.
 
+Related Stream ID:
+
+: A 32-bit value containing the stream ID of a stream in the opposite direction.
+  This field is only present if the Offset field is zero length.  Using a

Well, it's actually only present if the `R` bit is set.  But what I think you mean is that the `R` bit MUST NOT be set if the `OO` bits aren't zero, which doesn't belong in the definition of this field.

(Side-note:  This means these three bits can encode eight states, of which only five are defined valid.)

> +assigning a related stream and the peer opens another stream that includes the
+stream ID of the first in the Related Stream ID of the first stream.  This
+enables straightforward use of QUIC for request-response protocols or protocols
+that rely on bidirectional channels.
+
+This document doesn't proscribe the use of the Related Stream ID field, leaving
+decisions about the semantics of related streams to protocols that use QUIC.
+Protocols that use QUIC MUST either define the semantics of related streams or
+prohibit the use of the Related Stream ID field.  Protocols MAY define other
+uses for the Related Stream ID field, but are advised that complex relationships
+between streams could be poorly supported in generic QUIC implementations.  The
+INVALID_RELATED_STREAM error code can be used by protocols to signal receipt of
+invalid Related Stream ID field values, unless the protocol defines a more
+specific error code for the circumstances.
+
+The Related Stream ID for a stream only needs to be sent once for each (allowing

Probably want a noun after "for each".

Actually, I read this part differently than I'd initially thought you meant.  It says you don't _need_ to include it on any but the first frame, but doesn't actually prohibit it and only prescribes an error condition when there's a mismatch.  I could envision an application choosing to stick this value on one frame per packet until something gets ACK'd to make sure it arrives ASAP -- but since the remote QUIC stack can't deliver data out of order (unless #252?), that's largely moot.

Not only redundant, conflicts on which error code to use.

-- 
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/720#pullrequestreview-56390887

---

From: MikkelFJ <[email-masked]>
Date: Tue, 15 Aug 2017 09:49:00 -0700

Ahh - I see. I then think the R bit MUST be set on offset 0 frames if the stream is related and then either only once or for all frames but not not anyhting else. I.e. leaving out the R bit is equivalent to changing the id which is an error. However, if the receiver is able to generate an error, it means it already knows the state, and then it is pointless to send it in other frames, or, it is helpful to include it in every frame but then an error cannot be generated. So overall, to reduce confusion, I would say the ID MUST only be be sent in the first frame if at all present.

-- 
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/720#discussion_r133240646

---

From: MikkelFJ <[email-masked]>
Date: Tue, 15 Aug 2017 16:53:38 +0000 (UTC)

OTOH - I can see the benefit of including the ID on every fram if you have a many to one relation because the recevier don't need to store a lot of context. But then, as I said before, it becomes impractical to generate an error. So alternatively a) don't set the R bit at all, b) set it only on the first frame (if app procotol says so), or c) set on all frames (if app protocol says so). In all cases the use of frame ids must be consistent and receiver SHOULD generate an error if it is able to do 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/720#discussion_r133241832

---

From: Martin Thomson <[email-masked]>
Date: Thu, 17 Aug 2017 04:53:13 +0000 (UTC)

@martinthomson pushed 1 commit.

c58b0eb  Editorial fixup

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

---

From: Martin Thomson <[email-masked]>
Date: Thu, 17 Aug 2017 05:37:28 +0000 (UTC)

martinthomson commented on this pull request.

+Related Stream ID:
+

That's actually an editing error on my part.  I initially started out with this being only at the start of a stream, but realized that that restriction doesn't necessarily help.  Providing related stream information in other STREAM frames isn't necessarily useful, but it can mean more timely provision of relevant metadata in the event of loss or reordering.  I decided against prohibiting the field on that basis (and also because it means avoiding a check or two).

Bah, nouns are overrated.

You both got the idea.  Knowing that a stream is paired on the first STREAM frame you receive can make some things easier.

I think that we might still end up doing #252 based on the feedback I've heard, it's just not that high a priority.

I will use the new code (that got added later in the process).

-- 
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/720#discussion_r133622704

---

From: Mike Bishop <[email-masked]>
Date: Thu, 17 Aug 2017 17:10:49 +0000 (UTC)

MikeBishop commented on this pull request.

I don't think the updated text explicitly says whether **NOT** setting the `R` bit on subsequent frames constitutes a change.  I think the simplest way to scope it is that if there's a related stream, `R` MUST be set on the zero-offset frame and MAY be set on other frames, but the value of the Related Stream ID MUST NOT change on subsequent frames if `R` is set.

If you receive frames out-of-order and `R` is set, you have the info early; if it's *not* set then you can't do anything with that yet because it might still have been set on some other frame.  You still have to wait for the zero-offset frame to conclude that it will never be set.

-- 
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/720#discussion_r133773301

---

From: Martin Thomson <[email-masked]>
Date: Fri, 18 Aug 2017 01:53:34 +0000 (UTC)

1279a5d  Make Related Stream ID mandatory with Offset 0

-- 
You are receiving this because you are subscribed to this thread.
View it on GitHub:
https://github.com/quicwg/base-drafts/pull/720/files/c58b0eb9315d9756d9cef18be00fb5839077783e..1279a5d6894d0e3574fc1ff0c21cdcf17983f9e8

---

From: Martin Thomson <[email-masked]>
Date: Thu, 17 Aug 2017 18:55:06 -0700

After thinking about the last comment, I think that it is prudent to mandate the inclusion of Related Stream ID on STREAM frames with an offset of 0.  That way, recipients have some certainty about where to look for the information.  I don't want to end up in a situation where you have a long stream that you start delivering to an application, only to find out right at the end that it was supposed to be related to something else.

-- 
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/720#issuecomment-323239008

---

From: MikkelFJ <[email-masked]>
Date: Fri, 18 Aug 2017 08:38:21 +0000 (UTC)

Yes, but I believe it should be 0, 1, or all frames that carry related stream id, starting at offset zero. Anything else is rather messy. I'm also a bit concerned about MUST treat this as an error, because it is either not enforceable or it implies implementation overhead that is not desirable.

-- 
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/720#issuecomment-323295441