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.

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

Make RST_STREAM a bit on STREAM frame #775

Closed
ekr opened this issue Sep 18, 2017 · 12 comments
Closed

Make RST_STREAM a bit on STREAM frame #775

ekr opened this issue Sep 18, 2017 · 12 comments
Labels
-transport design An issue that affects the design of the protocol; resolution requires consensus. has-consensus An issue that the Chairs have determined has consensus, by canvassing the mailing list.

Comments

@ekr
Copy link
Collaborator

ekr commented Sep 18, 2017

It seems to me that it would be easier if RST_STREAM were instead a bit on the STREAM frame. This is one step towards having a standalone STREAM processor that doesn't have to know about connections.

@RyanTheOptimist
Copy link
Contributor

Moving this into the STREAM frame sounds like it could be made to work, but I'm not sure that I understand the motivation.

Can you say more about what a standalone stream processor would be? (I'm not sure I understand how this change makes such a beast any more or less aware of a connection. It still needs to handle abnormal termination, it's just a question of the API used to express that, I think)

@ekr
Copy link
Collaborator Author

ekr commented Sep 18, 2017

I'm not sure how much detail I can provide, because I've just started coding this up, but at the moment, I have a "stream" object at: https://github.com/ekr/minq/blob/master/stream.go.

So, the question, as you say, is what the APIs are. Currently, FIN is handled by delivering the STREAM frame to the stream, but RST is handled by a separate API used by the connection:
https://github.com/ekr/minq/blob/master/connection.go#L1036.

If RST were a stream frame, then I wouldn't need this API point, which needs to look up the stream, etc. The obvious generalization here would be that everything that affected a stream would come in a frame which first indicated the stream ID and then you could pass all that stuff to the stream handler, which wouldn't even need to know about the connection, except for a few narrow abstractions like conn-level flow control, making self-testing and the like easier.

@RyanTheOptimist
Copy link
Contributor

When you receive a STREAM frame, don't you have to look up the stream in order to deliver the stream data? In other words, I'm surprised that it's any less work to handle receipt of a RST_STREAM frame than it is to handle receipt of a STREAM frame with a build-in RST_STREAM semantic. Would you like to have only one place in the code which does the stream id to stream lookup? (That place being receipt of a STREAM frame).

Keep in mind that depending on how you implement things, receipt of a WINDOW_UPDATE frame needs to frob the stream as does receipt of an ACK frame for a packet containing data sent by that stream.

@ekr
Copy link
Collaborator Author

ekr commented Sep 19, 2017 via email

@ianswett
Copy link
Contributor

To be clear, you're talking about a very different 'stream frame' from the one that exists today. In particular, you're talking about a single frame type that performs all stream state modifications?

This seems possible, but routing 3 types to a stream isn't that much harder than a single type.

@ekr
Copy link
Collaborator Author

ekr commented Sep 19, 2017

Yes, that's what I'm talking about. I agree that it's not dramatically easier my way, but IMO it would make for a cleaner layering boundary

@janaiyengar
Copy link
Contributor

I don't see the value in making it a bit on the STREAM frame, and I prefer the semantic that currently all STREAM frames are processed in sequence. RST_STREAM, no matter what you call it, will get processed immediately on receipt, which is quite different from a STREAM frame carrying, say, a FIN bit. Since RST_STREAM applies to specific streams, why would the logic be different to pass a RST_STREAM to your stream processor than it is to pass a STREAM frame with RST bit set?

@janaiyengar
Copy link
Contributor

BTW, this doesn't perform all stream state modifications. STOP_SENDING is another one that may modify stream state.

If we add MAX_STREAM_DATA information in the STREAM frame, that makes it just a bulkier frame to schlep all the time, where we should expect to send new window update info at most once every RTT.

@ekr
Copy link
Collaborator Author

ekr commented Sep 19, 2017

Well, I'm not sure how to respond to "I don't see the value". As I said, it would be convenient for certain implementation strategies to have everything that affects stream state grouped together so that you can just do:

1. Is this a stream-type frame?
2. Find stream
3. Pass to stream

And having the check in one require ranging across multiple types and then having the step in 3 require passing a type (and needing to keep the type lists in sync) is clumsy. I'm prepared to believe that it wouldn't be useful in your implementation, but it would be in Minq. Note that this would be somewhat easier if the ranges for all the stream-affecting frames were contiguous, but they are currently not.

And yes, you would need to go through and find everything state affecting.

WRT the question of the stream frames being bulkier, that seems like an artifact of how you construct things. You don't need to have MAX_STREAM_DATA in any frame, but rather you need a way to carry it, but presence or absence can be a bit or some other encoding. It need not chew up the full size.

@RyanTheOptimist
Copy link
Contributor

This seems like it makes stream frames larger and more complex. Larger stream frames means a less efficient transport. And more complex frames means that instead of making the implementation simpler we just end up moving the complexity from on part of the code to another. This doesn't seem like a good tradeoff to me.

@mnot mnot added -transport design An issue that affects the design of the protocol; resolution requires consensus. labels Sep 25, 2017
@martinthomson
Copy link
Member

FWIW, I have been considering splitting STREAM further into discrete types. Not all the combinations of bits make sense; especially as we get to the two proposals for unidirectional-ism. I think that splitting the space of frame types into two easily recognizable groups (stream frames, other frames) is what is being requested here. It need not be a bit, though it might be right to allocate have the space to stream-related frames.

Seems like a fairly minor improvement either way, though it might allow us to consolidate the definition of things like stream numbers.

@martinthomson
Copy link
Member

Discussed with editors. This isn't phrased as a problem. If there is a problem to address, please write it that way.

@mnot mnot added the has-consensus An issue that the Chairs have determined has consensus, by canvassing the mailing list. label Mar 5, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
-transport design An issue that affects the design of the protocol; resolution requires consensus. has-consensus An issue that the Chairs have determined has consensus, by canvassing the mailing list.
Projects
None yet
Development

No branches or pull requests

6 participants