Skip to content

Commit

Permalink
BUG: http: disable TCP delayed ACKs when forwarding content-length data
Browse files Browse the repository at this point in the history
Commits 5c6209 and 072930 were aimed at avoiding undesirable PUSH flags
when forwarding chunked data, but had the undesired effect of causing
data advertised by content-length to be affected by the delayed ACK too.
This can happen when the data to be forwarded are small enough to fit into
a single send() call, otherwise the BF_EXPECT_MORE flag would be removed.

Content-length data don't need the BF_EXPECT_MORE flag since the low-level
forwarder already knows it can safely rely on bf->to_forward to set the
appropriate TCP flags.

Note that the issue is only observed in requests at the moment, though the
later introduction of server-side keep-alive could trigger the issue on the
response path too.

Special thanks to Randy Shults for reporting this issue with a lot of
details helping to reproduce it.

The fix must be backported to 1.4.
(cherry picked from commit 869fc1edc2acfa8ff88de2f4e638ad48dca5d246)
  • Loading branch information
wtarreau committed Mar 5, 2012
1 parent 9e78c99 commit 95e9a2b
Showing 1 changed file with 12 additions and 4 deletions.
16 changes: 12 additions & 4 deletions src/proto_http.c
Original file line number Diff line number Diff line change
Expand Up @@ -4550,9 +4550,13 @@ int http_request_forward_body(struct session *s, struct buffer *req, int an_bit)
/* We know that more data are expected, but we couldn't send more that
* what we did. So we always set the BF_EXPECT_MORE flag so that the
* system knows it must not set a PUSH on this first part. Interactive
* modes are already handled by the stream sock layer.
* modes are already handled by the stream sock layer. We must not do
* this in content-length mode because it could present the MSG_MORE
* flag with the last block of forwarded data, which would cause an
* additional delay to be observed by the receiver.
*/
req->flags |= BF_EXPECT_MORE;
if (txn->flags & TX_REQ_TE_CHNK)
req->flags |= BF_EXPECT_MORE;

http_silent_debug(__LINE__, s);
return 0;
Expand Down Expand Up @@ -5573,9 +5577,13 @@ int http_response_forward_body(struct session *s, struct buffer *res, int an_bit
/* We know that more data are expected, but we couldn't send more that
* what we did. So we always set the BF_EXPECT_MORE flag so that the
* system knows it must not set a PUSH on this first part. Interactive
* modes are already handled by the stream sock layer.
* modes are already handled by the stream sock layer. We must not do
* this in content-length mode because it could present the MSG_MORE
* flag with the last block of forwarded data, which would cause an
* additional delay to be observed by the receiver.
*/
res->flags |= BF_EXPECT_MORE;
if (txn->flags & TX_RES_TE_CHNK)
res->flags |= BF_EXPECT_MORE;

/* the session handler will take care of timeouts and errors */
http_silent_debug(__LINE__, s);
Expand Down

0 comments on commit 95e9a2b

Please sign in to comment.