You are viewing a plain text version of this content. The canonical link for it is here.
Posted to github@trafficserver.apache.org by GitBox <gi...@apache.org> on 2022/10/01 21:37:51 UTC
[GitHub] [trafficserver] bneradt opened a new pull request, #9117: Fix HTTP/2 session receive window handling for small sizes
bneradt opened a new pull request, #9117:
URL: https://github.com/apache/trafficserver/pull/9117
This fixes the way we handle session window sizes if the user configures proxy.config.http2.initial_window_size_in to be smaller than the HTTP/2 protocol's default value of 65,535. For streams, we can communicate to the client a smaller stream size via a SETTINGS frame, but not so for smaller session window sizes. The way HTTP/2 hosts are expected to reduce their receive session window size is by receiving DATA frames without replying with WINDOW_UPDATE frames until the window falls to the desired value. ATS already handled this latter behavior correctly (letting the window size fall without sending WINDOW_UPDATE frames for the session until it reached the desired smaller value). However, it incorrectly reported an error and sent a GOAWAY frame when the user sent DATA content above the reduced window size before ATS gave the client a chance to reduce it via DATA frames. This fixes the receive session window check to allow for a shrinking session window.
Fixes #9115
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@trafficserver.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [trafficserver] maskit commented on a diff in pull request #9117: Fix HTTP/2 session receive window handling for small sizes
Posted by GitBox <gi...@apache.org>.
maskit commented on code in PR #9117:
URL: https://github.com/apache/trafficserver/pull/9117#discussion_r985328581
##########
proxy/http2/Http2ConnectionState.cc:
##########
@@ -1039,8 +1039,18 @@ Http2ConnectionState::Http2ConnectionState() : stream_list()
void
Http2ConnectionState::init(Http2CommonSession *ssn)
{
- session = ssn;
- this->_server_rwnd = Http2::initial_window_size;
+ session = ssn;
+
+ if (Http2::initial_window_size < HTTP2_INITIAL_WINDOW_SIZE) {
+ // There is no HTTP/2 specified way to shrink the connection window size
+ // other than to receive data and not send WINDOW_UPDATE frames for a
+ // while.
+ this->_server_rwnd = HTTP2_INITIAL_WINDOW_SIZE;
Review Comment:
Why do we have the lower limit here? The point of having `_server_rwnd_is_shrinking` is to allow setting small session window size, right?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@trafficserver.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [trafficserver] bneradt commented on pull request #9117: Fix HTTP/2 session receive window handling for small sizes
Posted by GitBox <gi...@apache.org>.
bneradt commented on PR #9117:
URL: https://github.com/apache/trafficserver/pull/9117#issuecomment-1265663919
[approve ci autest]
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@trafficserver.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [trafficserver] bneradt commented on a diff in pull request #9117: Fix HTTP/2 session receive window handling for small sizes
Posted by GitBox <gi...@apache.org>.
bneradt commented on code in PR #9117:
URL: https://github.com/apache/trafficserver/pull/9117#discussion_r985951880
##########
proxy/http2/Http2ConnectionState.cc:
##########
@@ -1039,8 +1039,18 @@ Http2ConnectionState::Http2ConnectionState() : stream_list()
void
Http2ConnectionState::init(Http2CommonSession *ssn)
{
- session = ssn;
- this->_server_rwnd = Http2::initial_window_size;
+ session = ssn;
+
+ if (Http2::initial_window_size < HTTP2_INITIAL_WINDOW_SIZE) {
+ // There is no HTTP/2 specified way to shrink the connection window size
+ // other than to receive data and not send WINDOW_UPDATE frames for a
+ // while.
+ this->_server_rwnd = HTTP2_INITIAL_WINDOW_SIZE;
Review Comment:
`HTTP2_INITIAL_WINDOW_SIZE` is actually the 65,535 value, so this isn't the lower but rather the higher value. So here's the function:
```
if (Http2::initial_window_size < HTTP2_INITIAL_WINDOW_SIZE) {
// There is no HTTP/2 specified way to shrink the connection window size
// other than to receive data and not send WINDOW_UPDATE frames for a
// while.
this->_server_rwnd = HTTP2_INITIAL_WINDOW_SIZE;
this->_server_rwnd_is_shrinking = true;
} else {
this->_server_rwnd = Http2::initial_window_size;
this->_server_rwnd_is_shrinking = false;
}
```
* If, per the first branch, the user configured a _smaller_ value than the spec's default session window size (`HTTP2_INITIAL_WINDOW_SIZE`, 65,535), then we initialize the window to that higher value and indicate that we are actively shrinking the window to the desired state. `restart_receiving` already handles this appropriately without any change in this patch...it sends `WINDOW_UPDATE` frames when the receive session window drops below the configured session window (not this default window).
* If the user configures a session window _larger_ than the default window size of 65,535 (the else case above), then we can simply set the `_server_rwnd` to the larger value and, later in the function, we send a `WINDOW_UPDATE` frame to tell the peer to increase the session window size as desired.
##########
proxy/http2/Http2ConnectionState.cc:
##########
@@ -1039,8 +1039,18 @@ Http2ConnectionState::Http2ConnectionState() : stream_list()
void
Http2ConnectionState::init(Http2CommonSession *ssn)
{
- session = ssn;
- this->_server_rwnd = Http2::initial_window_size;
+ session = ssn;
+
+ if (Http2::initial_window_size < HTTP2_INITIAL_WINDOW_SIZE) {
+ // There is no HTTP/2 specified way to shrink the connection window size
+ // other than to receive data and not send WINDOW_UPDATE frames for a
+ // while.
+ this->_server_rwnd = HTTP2_INITIAL_WINDOW_SIZE;
Review Comment:
Thank you for the review. Let me know whether I misunderstood your question.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@trafficserver.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org