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

Stream operations after RESET has been sent #101

Closed
vasilvv opened this issue Jan 3, 2024 · 4 comments · Fixed by #111
Closed

Stream operations after RESET has been sent #101

vasilvv opened this issue Jan 3, 2024 · 4 comments · Fixed by #111
Assignees
Labels
ready for PR Issues which are ready for a pull request

Comments

@vasilvv
Copy link
Collaborator

vasilvv commented Jan 3, 2024

  • Is it valid to send WT_STREAM for streams that already had a WT_RESET_STREAM sent?
  • Is it valid to send WT_RESET_STREAM for a stream that already had a WT_RESET_STREAM sent?
  • Is it valid to send WT_STOP_SENDING for a stream that already had a WT_STOP_SENDING sent?
  • Is it valid to send WT_MAX_STREAM_DATA for a stream that already had a WT_STOP_SENDING sent?

In QUIC, I believe we answered "yes" to all of the above since the protocol is inherently unordered. As far as I can tell, this does not apply to WT over HTTP/2, since it's strictly ordered, at least for an individual peer.

@vasilvv
Copy link
Collaborator Author

vasilvv commented Jan 3, 2024

Oh, forgot to add: we should decide the same for FIN, too.

@martinthomson
Copy link
Collaborator

It should be possible to do this, if you consider HTTP/2 uses in isolation, but are there cases where there could be a hop that is HTTP/3 and it would be easier to intermediate if the ordering could be loose?

@ekinnear
Copy link
Collaborator

ekinnear commented Jan 9, 2024

Seems like we have two options:

  • Most of these operations are inherently observable, for example if you go to forward a WT_STREAM frame and you think that the stream is already closed because you already had a WT_RESET_STREAM come through, you could see an error and drop that WT_STREAM frame.

  • Allow being a direct passthrough, so HTTP/2 is just preserving the inherent reordering that happened on the HTTP/3 side.

It's a little bit annoying that we'd need someone running pure HTTP/2 to accept obviously wrong things, just in case somewhere someone had HTTP/3 in existence.

However, it also seems nice to not make the intermediary keep track of the state of streams if possible -- but you have to have a mapping already because the stream IDs don't match across HTTP/2 and HTTP/3.

@ekinnear
Copy link
Collaborator

Discussed in editor's meeting:

  • Prohibit and error because you already have to write code that handles this case where you cannot find the mapping in your table. Preference towards it being cleaner.

@ekinnear ekinnear added the ready for PR Issues which are ready for a pull request label Jan 23, 2024
martinthomson added a commit to martinthomson/webtransport that referenced this issue Jan 23, 2024
As discussed in ietf-wg-webtrans/draft-ietf-webtrans-http2#101

This is NOT the only place where this state is used.  That one needs a
few more grey matter cycles before I have a resolution.  I'll open an
issue instead.
@martinthomson martinthomson linked a pull request May 13, 2024 that will close this issue
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ready for PR Issues which are ready for a pull request
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants