Subject: [quicwg/base-drafts] Editorial suggestions on the Streams section (#2730)

From: ianswett <[email-masked]>
Date: Mon, 20 May 2019 14:50:00 -0700

You can view, comment on, or merge this pull request online at:

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

-- Commit Summary --

  * Editorial suggestions on the Streams section

-- File Changes --

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

-- Patch Links --

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

---

From: Martin Thomson <[email-masked]>
Date: Wed, 22 May 2019 06:00:32 -0700

martinthomson requested changes on this pull request.

A few comments, some of which are important enough to fix.

> @@ -187,8 +187,9 @@ QUIC:
 
 QUIC packet:
 
-: The smallest unit of QUIC that can be encapsulated in a UDP datagram. Multiple
-  QUIC packets can be encapsulated in a single UDP datagram.
+: A complete processable payload with a packet type, encryption level and

I don't understand this.  I agree that the existing definition isn't good, but "payload" means something else.

> @@ -244,8 +245,9 @@ x (*) ...:
 # Streams {#streams}
 
 Streams in QUIC provide a lightweight, ordered byte-stream abstraction to an
-application. An alternative view of QUIC streams is as an elastic "message"
-abstraction.
+application. Streams can be unidirectional or bidirecational.  An alternative
+view of QUIC unidirectional streams is a "message" abstraction of unlimited
+length.

Streams aren't unlimited length.

> @@ -543,9 +546,8 @@ MAX_STREAM_DATA frames, it only receives any retransmissions of stream data.
 
 Once all data for the stream has been received, the receiving part enters the
 "Data Recvd" state.  This might happen as a result of receiving the same STREAM
-frame that causes the transition to "Size Known".  In this state, the endpoint
-has all stream data.  Any STREAM or STREAM_DATA_BLOCKED frames it receives for
-the stream can be discarded.
+frame that causes the transition to "Size Known".  Any STREAM or
+STREAM_DATA_BLOCKED frames it receives for the stream can be discarded.

I realize here that the last sentence here, if taken out of context, might be misleading.

> @@ -643,7 +645,7 @@ receiving application is no longer reading data it receives from the stream, but
 it is not a guarantee that incoming data will be ignored.
 
 STREAM frames received after sending STOP_SENDING are still counted toward
-connection and stream flow control, even though these frames will be discarded
+connection and stream flow control, even though these frames may be discarded

```suggestion
connection and stream flow control, even though these frames can be discarded
```

I try to avoid lowercase 2119

> @@ -657,11 +659,6 @@ RESET_STREAM frame it sends, but MAY use any application error code.  The
 endpoint that sends a STOP_SENDING frame MAY ignore the error code carried in
 any RESET_STREAM frame it receives.
 
-If the STOP_SENDING frame is received on a stream that is already in the
-"Data Sent" state, an endpoint that wishes to cease retransmission of
-previously-sent STREAM frames on that stream MUST first send a RESET_STREAM
-frame.

Why?

> @@ -734,9 +731,8 @@ used to check for flow control violations. A receiver might use a sum of bytes
 consumed on all streams to determine the maximum data limit to be advertised.
 
 A receiver can advertise a larger offset by sending MAX_STREAM_DATA or MAX_DATA
-frames at any time during the connection.  A receiver cannot renege on an
-advertisement however.  That is, once a receiver advertises an offset, it MAY
-advertise a smaller offset, but this has no effect.
+frames.  That is, once a receiver advertises an offset, it MAY advertise a

```suggestion
frames.  Once a receiver advertises an offset, it MAY advertise a
```

> @@ -746,16 +742,15 @@ A sender MUST ignore any MAX_STREAM_DATA or MAX_DATA frames that do not increase
 flow control limits.
 
 If a sender runs out of flow control credit, it will be unable to send new data
-and is considered blocked.  A sender SHOULD send STREAM_DATA_BLOCKED or
-DATA_BLOCKED frames to indicate it has data to write but is blocked by flow
+and is considered blocked.  A sender SHOULD send a single STREAM_DATA_BLOCKED or

Are you attempting to imply that you don't retransmit the frames.

```suggestion
and is considered blocked.  A sender SHOULD send a STREAM_DATA_BLOCKED or
```

> @@ -797,10 +792,9 @@ been consumed, to avoid either exceeding flow control limits or deadlocking.
 On receipt of a RESET_STREAM frame, an endpoint will tear down state for the
 matching stream and ignore further data arriving on that stream.  If a
 RESET_STREAM frame is reordered with stream data for the same stream, the
-receiver's estimate of the number of bytes received on that stream can be lower
-than the sender's estimate of the number sent.  As a result, the two endpoints
-could disagree on the number of bytes that count towards connection flow
-control.
+receiver's maximum total size on that stream can be lower than the
+sender's total size.  As a result, the two endpoints could disagree on
+the number of bytes that count towards connection flow control.

Maybe "Without the offset included in RESET_STREAM, the two endpoints could disagree..."

> @@ -861,13 +855,12 @@ If either is received, the connection MUST be closed immediately with a
 connection error of type STREAM_LIMIT_ERROR (see {{immediate-close}}).
 
 Endpoints MUST NOT exceed the limit set by their peer.  An endpoint that
-receives a STREAM frame with a stream ID exceeding the limit it has sent MUST
+receives a frame with a stream ID exceeding the limit it has sent MUST

You are talking here about trying to include STREAM_BLOCKED and RESET_STREAM as well as STREAM?  But by making it generic you might accidentally capture STOP_SENDING.

-- 
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/2730#pullrequestreview-240607151