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

Why is the crypto stream not subject to connection flow control #800

Closed
siyengar opened this issue Sep 27, 2017 · 3 comments
Closed

Why is the crypto stream not subject to connection flow control #800

siyengar opened this issue Sep 27, 2017 · 3 comments

Comments

@siyengar
Copy link

IIRC previously this was not done because flow control messages for the connection were sent on the stream 0. However with MAX_DATA frames they are not sent over stream id 0 any more. Having only crypto streams not be subject to connection level flow control creates a bunch of workaround code to check for if (stream == cryptoStream) before you increment the cumulativewritedataoffset for the connection which is used to track conn flow control.

@martinthomson
Copy link
Member

The problem is early data and the handshake. It is possible that early data will exhaust the connection flow control window. I fact, that's a feature: we use the connection flow control window to limit the amount of early data, just as there is an early data limit in TLS. If that happens, there is no room to send the client Finished (and Certificate[Verify]). That would block handshake completion, which is bad. Also, the server doesn't really have a good way to send authenticated MAX_DATA frames to the client at this point, so it doesn't easily resolve itself.

Yeah, it's a real pain. Stream 0 is too special for my liking also.

@siyengar
Copy link
Author

ya that sounds reasonable, I realized that this might be a reason just after posting :)

@janaiyengar
Copy link
Contributor

Necessary evil. Control channels are as old as the hills -- nothing new here :-)

@mnot mnot added the -transport label Mar 5, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants