Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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
[connect-tcp] Remove bare TCP payload mode #2943
[connect-tcp] Remove bare TCP payload mode #2943
Changes from 1 commit
98e6ef6
9da3ff7
2bf3e3b
57e8093
1f82e24
a89c5b5
5aa262b
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What's "incomplete DATA capsule"?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
A DATA capsule is incomplete if it is truncated by the end of the underlying data stream. This concept is defined in RFC 9297, so I've added a reference here.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What if there is no data in-flight (i.e. an idle connection)? Should intermediaries inject malformed
DATA
frame?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, the recommendation is to inject a truncated DATA capsule by sending a DATA capsule header indicating nonzero length, then close the stream without sending any payload.
An alternative would be to define a new capsule type that serves as an explicit error indication.
An explicit error indication is much more powerful, may support easier debugging of connection failures, and may be easier to implement on the sender side. However, it is more work to implement on the receiver side, and opens up a major design bikeshed. The parsimony of having fewer ways to fail also might increase the odds of those failures being handled correctly.
I'm open to either approach.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Since we're now always using capsules, I think there's value in making this an explicit signal. How about adding a DATA_END capsule of length_zero and requiring to send that? If the stream closes without receiving that, then it means it was aborted.
Alternate encoding: is to have two capsule types for DATA and a FIN bit, like we do for STREAM frames in QUIC.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We have to be able to emit those back to TCP endpoints, so the amount of details and bike-sheding that we can do here is limited.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
DID SOMEONE SAY BIKESHED??? I'M ENTHUSIASTIC
I never liked the fact that reading 0 on a BSD socket means "end of stream" for TCP. It's odd, and inconsistent with other uses (e.g., for UDP it means "I received a UDP packet with no payload").
We should definitely separate between "clean termination" (TCP FIN) and "abortive termination" (TCP RST, timeout, etc.) but I don't think we need to separate RST from timeout. That would match what classic CONNECT does.
So, +1 for DATA capsule with a FIN bit :)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
A FIN bit adds overhead to every DATA capsule, which seems unappealing to me. A zero-length DATA capsule seems fine to me (AFAIK we don't have any other use for length=0), but otherwise I would define a new capsule type ... probably a type meaning "clean shutdown", so that underlying transport failures naturally are treated as "unclean".
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sorry I meant putting the FIN bit in the capsule type. That can be described as two separate capsules (DATA = 0xTBD, DATA_END=0xTBD+1) or as a bit (DATA = 0xTBD..0xTBD+1, lowest bit of capsule type means FIN) but they're the same thing on the wire
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
OK, I've written this in as a new FINAL_DATA capsule type.