From: Willy Tarreau Date: Mon, 5 Mar 2012 07:29:20 +0000 (+0100) Subject: BUG: http: disable TCP delayed ACKs when forwarding content-length data X-Git-Tag: v1.5-dev8~19 X-Git-Url: http://git.ipfire.org/?a=commitdiff_plain;h=869fc1edc2acfa8ff88de2f4e638ad48dca5d246;p=thirdparty%2Fhaproxy.git BUG: http: disable TCP delayed ACKs when forwarding content-length data 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. --- diff --git a/src/proto_http.c b/src/proto_http.c index f36dc3c73c..c71b839550 100644 --- a/src/proto_http.c +++ b/src/proto_http.c @@ -4684,9 +4684,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; @@ -5731,9 +5735,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);