Le 30/03/2017 à 11:50, Christopher Faulet a écrit :
Le 29/03/2017 à 13:23, Cyril Bonté a écrit :
De: "Cyril Bonté" <[email protected]>
À: [email protected]
Cc: [email protected]
Envoyé: Mercredi 29 Mars 2017 12:36:01
Objet: Re: 100% cpu usage with compression in haproxy.cfg
Hi all,

De: [email protected]
À: [email protected]
Envoyé: Mercredi 29 Mars 2017 11:52:10
Objet: 100% cpu usage with compression in haproxy.cfg

i have upgraded to haproxy 1.7.4

when i add  the compression algo & type configs in frontend,
haproxy
raises up to 100%.
this happens also with 1.7.1.
when i comment those two lines out, everything is working like a
charm, even compression is working. so i expect i don't need this
config option at all - only when i want to disable or set some
differenzt algo/compression options....

I've just hit the issue today on a"public lab" server and haproxy
1.8-dev (1.8-dev0-827385-303).

I won't have time to investigate it more for the next hours, but I can 
reproduce it easily in HTTP/1.0 (in my logs, the issue occured when a bot came 
with HTTP/1.0 requests).

Let's take such a simple PHP script (bug.php) :
<?php
for ($i = 0; $i < 16384; $i++) {
        echo ("X");
}
?>
Requesting the web server without haproxy, the response is :
$echo -ne "GET /bug.php HTTP/1.0\r\nHost: localhost\r\n\r\n" | nc localhost 80
HTTP/1.1 200 OK
Date: Wed, 29 Mar 2017 11:15:32 GMT
Server: Apache/2.4.10 (Debian)
Vary: Accept-Encoding
Connection: close
Content-Type: text/html; charset=UTF-8

XXXXXXXXXXXXXXXXXX...

Here, we don't have any Content-Length, which will produce a 100% cpu loop when 
the request is made through haproxy.
I add Christopher Faulet to the thread, maybe those details will help.


Hi all,

Here is 2 patches that fix the bugs. I made some tests and it seems to
work without breaking other use cases. Could you confirm that it works
for you ?

Thanks


With patches this time ...

--
Christopher Faulet
>From 3372129a9c69c8dd37f1ebf84612c07e1c6c21e6 Mon Sep 17 00:00:00 2001
From: Christopher Faulet <[email protected]>
Date: Thu, 30 Mar 2017 10:54:35 +0200
Subject: [PATCH 1/2] BUG/MEDIUM: http: Fix blocked HTTP/1.0 responses when
 compression is enabled
X-Bogosity: Ham, tests=bogofilter, spamicity=0.000000, version=1.2.4

When the compression filter is enabled, if a HTTP/1.0 response is received (no
content-length and no transfer-encoding), no data are forwarded to the client
because of a bug and the transaction is blocked indefinitly.

The bug comes from the fact we need to synchronize the end of the request and
the response because of the compression filter. This inhibits the infinite
forwarding of data. But for these responses, the compression is not
activated. So the response body is not analyzed. This leads to a deadlock.

The solution is to enable the analyze of the response body in all cases and
handle this one to enable the infinite forwarding. All other cases should
already by handled.

This fix should be backported to 1.7.
---
 src/proto_http.c | 24 +++++++++++++++---------
 1 file changed, 15 insertions(+), 9 deletions(-)

diff --git a/src/proto_http.c b/src/proto_http.c
index 56480da..487c0fc 100644
--- a/src/proto_http.c
+++ b/src/proto_http.c
@@ -6856,11 +6856,9 @@ int http_process_res_common(struct stream *s, struct channel *rep, int an_bit, s
 	}
 
  skip_header_mangling:
-	if ((msg->flags & HTTP_MSGF_XFER_LEN) || HAS_DATA_FILTERS(s, rep) ||
-	    (txn->flags & TX_CON_WANT_MSK) == TX_CON_WANT_TUN) {
-		rep->analysers &= ~AN_RES_FLT_XFER_DATA;
-		rep->analysers |= AN_RES_HTTP_XFER_BODY;
-	}
+	/* Always enter in the body analyzer */
+	rep->analysers &= ~AN_RES_FLT_XFER_DATA;
+	rep->analysers |= AN_RES_HTTP_XFER_BODY;
 
 	/* if the user wants to log as soon as possible, without counting
 	 * bytes from the server, then this is the right moment. We have
@@ -7003,11 +7001,11 @@ int http_response_forward_body(struct stream *s, struct channel *res, int an_bit
 
 	/* When TE: chunked is used, we need to get there again to parse
 	 * remaining chunks even if the server has closed, so we don't want to
-	 * set CF_DONTCLOSE. Similarly, if the body length is undefined, if
-	 * keep-alive is set on the client side or if there are filters
-	 * registered on the stream, we don't want to forward a close
+	 * set CF_DONTCLOSE. Similarly, if keep-alive is set on the client side
+	 * or if there are filters registered on the stream, we don't want to
+	 * forward a close
 	 */
-	if ((msg->flags & HTTP_MSGF_TE_CHNK) || !(msg->flags & HTTP_MSGF_XFER_LEN) ||
+	if ((msg->flags & HTTP_MSGF_TE_CHNK) ||
 	    HAS_DATA_FILTERS(s, res) ||
 	    (txn->flags & TX_CON_WANT_MSK) == TX_CON_WANT_KAL ||
 	    (txn->flags & TX_CON_WANT_MSK) == TX_CON_WANT_SCL)
@@ -7024,6 +7022,14 @@ int http_response_forward_body(struct stream *s, struct channel *res, int an_bit
 	if ((msg->flags & HTTP_MSGF_TE_CHNK) || (msg->flags & HTTP_MSGF_COMPRESSING))
 		res->flags |= CF_EXPECT_MORE;
 
+	/* If there is neither content-length, nor transfer-encoding header
+	 * _AND_ there is no data filtering, we can safely forward all data
+	 * indefinitely. */
+	if (!(msg->flags & HTTP_MSGF_XFER_LEN) && !HAS_DATA_FILTERS(s, res)) {
+		buffer_flush(res->buf);
+		channel_forward_forever(res);
+	}
+
 	/* the stream handler will take care of timeouts and errors */
 	return 0;
 
-- 
2.9.3

>From 1005c2fdb26f97fcfb7c75c7b096e9e6350a09f4 Mon Sep 17 00:00:00 2001
From: Christopher Faulet <[email protected]>
Date: Thu, 30 Mar 2017 11:13:22 +0200
Subject: [PATCH 2/2] BUG/MINOR: filters: Don't force the stream's wakeup when
 we wait in flt_end_analyze
X-Bogosity: Ham, tests=bogofilter, spamicity=0.000000, version=1.2.4

In flt_end_analyze, we wait that the anlayze is finished for both the request
and the response. In this case, because of a task_wakeup, some streams can
consume too much CPU to do nothing. So now, this is the filter's responsibility
to know if this wakeup is needed.

This fix should be backported in 1.7.
---
 src/filters.c | 7 +------
 1 file changed, 1 insertion(+), 6 deletions(-)

diff --git a/src/filters.c b/src/filters.c
index 24e1d53..68bf7ee 100644
--- a/src/filters.c
+++ b/src/filters.c
@@ -861,12 +861,7 @@ flt_end_analyze(struct stream *s, struct channel *chn, unsigned int an_bit)
 		/* Remove backend filters from the list */
 		flt_stream_release(s, 1);
 	}
-	else {
-		/* This analyzer ends only for one channel. So wake up the
-		 * stream to be sure to process it for the other side as soon as
-		 * possible. */
-		task_wakeup(s->task, TASK_WOKEN_MSG);
-	}
+
 	return ret;
 }
 
-- 
2.9.3

Reply via email to