]> git.ipfire.org Git - thirdparty/haproxy.git/commitdiff
BUG: http: disable TCP delayed ACKs when forwarding content-length data
authorWilly Tarreau <w@1wt.eu>
Mon, 5 Mar 2012 07:29:20 +0000 (08:29 +0100)
committerWilly Tarreau <w@1wt.eu>
Mon, 5 Mar 2012 07:46:34 +0000 (08:46 +0100)
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.

src/proto_http.c

index f36dc3c73c01e2023673a3f7578b003f6a71517d..c71b839550b26bc915d9f967130accfb799bf31c 100644 (file)
@@ -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);