Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Independent states and STOP_SENDING #171

Merged
merged 29 commits into from Aug 3, 2017
Merged

Independent states and STOP_SENDING #171

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
29 commits
Select commit Hold shift + click to select a range
b1c79fb
First pass at REQUEST_RST
MikeBishop Jan 18, 2017
4c96bd2
Use REQUEST_RST in HTTP/QUIC
MikeBishop Jan 18, 2017
7f66526
Martin's feedback on REQUEST_RST
MikeBishop Jan 30, 2017
b2dc2c2
Wrong word form
MikeBishop Jan 30, 2017
dd17d05
Fix some merge stuff
MikeBishop Feb 23, 2017
c9d0d8f
Servers shouldn't freak out either
MikeBishop Mar 7, 2017
7f0a134
Rename to DISINTEREST
MikeBishop Mar 7, 2017
001c274
Suggest error code, loosen ordering
MikeBishop Mar 7, 2017
d4f77df
an => a
MikeBishop Mar 7, 2017
dccd3f3
Merge remote-tracking branch 'origin/master' into request_rst
MikeBishop Mar 9, 2017
4ce57c5
Without selecting?
MikeBishop Mar 9, 2017
5088ec5
Merge remote-tracking branch 'origin/master' into request_rst
MikeBishop Apr 13, 2017
df92cb1
Martin's less-MUSTy feedback on DISINTEREST
MikeBishop Apr 13, 2017
3ac8550
Merging with Martin's stream state text
MikeBishop May 2, 2017
5740393
Martin's feedback
MikeBishop Jun 7, 2017
299e72d
Merge remote-tracking branch 'origin/master' into request_rst
MikeBishop Jun 7, 2017
1536501
Merge remote-tracking branch 'origin/master' into request_rst
MikeBishop Jun 28, 2017
52f99c7
Update frame type value and table
MikeBishop Jun 28, 2017
15da4ce
Merge remote-tracking branch 'origin/master' into request_rst
MikeBishop Jul 24, 2017
35dfd02
Bring back the error code
MikeBishop Jul 27, 2017
e0be281
REQUEST_RST -> DISINTEREST -> STOP_SENDING
MikeBishop Jul 27, 2017
e91b428
DISINTEREST->STOP_SENDING in HTTP doc
MikeBishop Jul 27, 2017
1213176
Clarifying stream state
MikeBishop Jul 27, 2017
23c7dd2
delete 'up to and'
MikeBishop Jul 27, 2017
7a46965
STOP_SENDING in half-closed remote
MikeBishop Jul 31, 2017
5699b82
Remove implied ordering of STOP_SENDING and STREAM frames
MikeBishop Jul 31, 2017
106c165
STOP_SENDING is sorta retransmittable and MUSTy
MikeBishop Aug 1, 2017
0b027f3
Jana's nits
MikeBishop Aug 2, 2017
53d1621
Merge remote-tracking branch 'origin/master' into request_rst
MikeBishop Aug 2, 2017
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
16 changes: 10 additions & 6 deletions draft-ietf-quic-http.md
View file
Open in desktop
Expand Up @@ -244,11 +244,11 @@ A server can send a complete response prior to the client sending an entire
request if the response does not depend on any portion of the request that has
not been sent and received. When this is true, a server MAY request that the
client abort transmission of a request without error by triggering a QUIC
REQUEST_RST after sending a complete response and closing its stream. Clients
MUST NOT discard complete responses as a result of receiving a REQUEST_RST,
though clients can always discard responses at their discretion for other
reasons. Servers SHOULD NOT abort a response in progress as a result of
receiving a solicited RST_STREAM.
DISINTEREST with error code HTTP_EARLY_RESPONSE, sending a complete response,
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We need to reconcile this error code business, otherwise, I think we're good to go. (And I would like to resolve this soon.)

and cleanly closing its streams. Clients MUST NOT discard complete responses as
a result of receiving a DISINTEREST, though clients can always discard responses
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You're still using DISINTEREST here and a few more spots.

at their discretion for other reasons. Servers SHOULD NOT abort a response in
progress as a result of receiving a solicited RST_STREAM.

### Header Compression

Expand Down Expand Up @@ -367,7 +367,7 @@ frames sent on the control stream, and response body (if any) sent via the
corresponding data stream.

If a promised push stream is not needed by the client, the client SHOULD send a
QUIC REQUEST_RST on the promised stream with an appropriate error code (e.g.
QUIC DISINTEREST on the promised stream with an appropriate error code (e.g.
HTTP_PUSH_REFUSED, HTTP_PUSH_ALREADY_IN_CACHE; see {{errors}}). This asks the
server not to transfer the data and indicates that it will be discarded upon
receipt.
Expand Down Expand Up @@ -767,6 +767,10 @@ HTTP_SETTINGS_ON_WRONG_STREAM (0x0F):
HTTP_MULTIPLE_SETTINGS (0x10):
: More than one SETTINGS frame was received.

HTTP_EARLY_RESPONSE (0x11):
: Server is able to generate a response to the client request without selecting
the complete request body.

## Mapping HTTP/2 Error Codes

The HTTP/2 error codes defined in Section 7 of {{!RFC7540}} map to QUIC error
Expand Down
31 changes: 16 additions & 15 deletions draft-ietf-quic-transport.md
View file
Open in desktop
Expand Up @@ -574,7 +574,7 @@ document.
| 0x05 | BLOCKED | {{frame-blocked}} |
| 0x06 | STOP_WAITING | {{frame-stop-waiting}} |
| 0x07 | PING | {{frame-ping}} |
| 0x08 | REQUEST_RST | {{frame-request-rst}} |
| 0x08 | DISINTEREST | {{frame-disinterest}} |
| 0x40 - 0x7f | ACK | {{frame-ack}} |
| 0x80 - 0xff | STREAM | {{frame-stream}} |
|------------------|--------------------|----------------------------|
Expand Down Expand Up @@ -1471,10 +1471,11 @@ The fields are:



## REQUEST_RST Frame {#frame-request-rst}
## DISINTEREST Frame {#frame-disinterest}

An endpoint may use a REQUEST_RST frame (type=0x08) to request a peer to
abruptly terminate transmission on a stream. The frame is as follows:
An endpoint may use a DISINTEREST frame (type=0x08) to communicate that incoming
data is being discarded on receipt at application request. This signals a peer
to abruptly terminate transmission on a stream. The frame is as follows:

~~~
0 1 2 3
Expand All @@ -1488,10 +1489,10 @@ abruptly terminate transmission on a stream. The frame is as follows:

The fields are:

* Error code: A 32-bit error code which indicates why the stream should be
closed.
* Error code: A 32-bit error code which indicates why the data is being
discarded.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This field is shown in the figure.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@MikeBishop, you missed the error code. Do we even need an error code?


* Stream ID: The 32-bit Stream ID of the stream being terminated.
* Stream ID: The 32-bit Stream ID of the stream being ignored.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it would be simpler to have exactly the same setup as RST: Error code, stream ID, and amount consumed. At least the error code, with the reason for sending the DISINTEREST.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There originally was an error code, and @martinthomson suggested removing it (see his review in April). The point of this frame is that the sender's side isn't inherently closed, so any reasons can still be communicated at the application layer. That's certainly the case with HTTP, where the most common reason is that the server has already generated a response. I have a mild preference for leaving it out, since this isn't targeted at error cases, but I'm largely ambivalent. I'd suggest taking that design discussion to the issue or list.

Including the offset doesn't really matter -- the sender of DISINTEREST has received all the data they needed, which implies the receiver has already successfully sent at least that much. Why do they care what the offset of not-caring was? RST_STREAM includes the offset because that's necessary to synchronize flow control. Not so here.


## PADDING Frame {#frame-padding}
Expand Down Expand Up @@ -1824,7 +1825,7 @@ An endpoint receiving a RST_STREAM frame causes the stream state to become
### half-closed (local)

A stream that is in the "half-closed (local)" state MUST NOT be used for sending
STREAM frames; WINDOW_UPDATE, RST_STREAM, and REQUEST_RST MAY be sent in this
STREAM frames; WINDOW_UPDATE, RST_STREAM, and DISINTEREST MAY be sent in this
state.

A stream transitions from this state to "closed" when a STREAM frame that
Expand Down Expand Up @@ -1901,18 +1902,18 @@ permitted in the description of a state as a connection error
## Solicited State Transitions

If an endpoint is no longer interested in the data being received, it MAY send a
REQUEST_RST frame on a stream in the "open" or "half-closed (local)" state to
DISINTEREST frame on a stream in the "open" or "half-closed (local)" state to
request closure of the stream in the opposite direction. This typically
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Editorial nit: does it make sense to say "request", since it's more of a "demand"? I can't come up with better wording, but maybe "require"? Up to you.

indicates that the receiving application is no longer reading from the stream
and all future data will be discarded upon receipt.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

... "However, it doesn't imply a guarantee that the receiver will discard data, the sender still needs to either reset the stream or complete its transmission of data."


STREAM frames received after sending REQUEST_RST are still counted toward the
STREAM frames received after sending DISINTEREST are still counted toward the
connection and stream flow-control windows. Even though these frames will be
discarded, because they are sent before their sender receives the REQUEST_RST,
discarded, because they are sent before their sender receives the DISINTEREST,
the sender will consider the frames to count against its flow-control windows.

Upon receipt of a REQUEST_RST frame, an endpoint SHOULD send a RST_STREAM with
an error code of QUIC_RECEIVED_RST. If the REQUEST_RST frame is received on a
Upon receipt of a DISINTEREST frame, an endpoint SHOULD send a RST_STREAM with
an error code of QUIC_RECEIVED_RST. If the DISINTEREST frame is received on a
stream that is already in the "half-closed (local)" or "closed" states, a
RST_STREAM frame SHOULD still be sent and retransmission of previously-sent
STREAM frames SHOULD be cancelled.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We gots to get us some MUST here. This isn't squishy. The only real exemption to needing a RST_STREAM here is if the stream is long done. If the sender is still (re)transmitting anything, then it needs to either finish that or move over to (re)transmitting a RST_STREAM. Otherwise we have a wrinkle on the state machine and I don't want that, even if it could save a RST_STREAM.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It kind of is squishy, at least in my mind. DISINTEREST is a courtesy notification that your data is being ignored, so you might as well stop sending it. Converting that to a MUST is to put more semantics on it than I had intended. Notably, this is also hard behavior to verify in a peer -- you'd have to look for STREAM frames in a later packet than the ACK of the DISINTEREST, I suppose.

Expand Down Expand Up @@ -2122,7 +2123,7 @@ should be taken on the data already received is an application-specific issue,
but it will often be the case that upon receipt of a RST_STREAM an endpoint
will choose to stop sending data in its own direction. If the sender of a
RST_STREAM wishes to explicitly state that no future data will be processed,
that endpoint MAY send a REQUEST_RST frame at the same time.
that endpoint MAY send a DISINTEREST frame at the same time.

### Offset Increment

Expand Down Expand Up @@ -2199,7 +2200,7 @@ QUIC_MISSING_PAYLOAD (0x80000030):
: The packet contained no payload.

QUIC_RECEIVED_RST (0x80000035):
: Terminating stream because peer sent a RST_STREAM or REQUEST_RST.
: Terminating stream because peer sent a RST_STREAM or DISINTEREST.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Having just gone over the error codes, I think that NO_ERROR is perfectly appropriate for this. You have removed the existing one-way semantic from RST_STREAM+NO_ERROR and put that in a new frame, but the idea remains that there are cases for RST_STREAM where the stream just isn't wanted any more. NO_ERROR fits perfectly to my mind.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would probably call that a "CANCEL," but I agree we can merge them if NO_ERROR's special-case semantics are gone.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

"CANCEL" in h2 means "I didn't do anything with this stream", which might be nice to keep.


QUIC_INVALID_STREAM_DATA (0x8000002E):
: STREAM frame data is malformed.
Expand Down