|
commit 7bd7a8d2b8889f604b807c21190d2e70328d6674
|
|
Author: Christopher Faulet <cfaulet@haproxy.com>
|
|
Date: Tue Apr 30 12:17:13 2019 +0200
|
|
|
|
BUG/MEDIUM: listener: Fix how unlimited number of consecutive accepts is handled
|
|
|
|
There is a bug when global.tune.maxaccept is set to -1 (no limit). It is pretty
|
|
visible with one process (nbproc sets to 1). The functions listener_accept() and
|
|
accept_queue_process() don't expect to handle negative maxaccept values. So
|
|
instead of accepting incoming connections without any limit, none are never
|
|
accepted and HAProxy loop infinitly in the scheduler.
|
|
|
|
When there are 2 or more processes, the bug is a bit more subtile. The limit for
|
|
a listener is set to 1. So only one connection is accepted at a time by a given
|
|
listener. This happens because the listener's maxaccept value is an unsigned
|
|
integer. In check_config_validity(), it is first set to UINT_MAX (-1 casted in
|
|
an unsigned integer), and then some calculations on it leads to an integer
|
|
overflow.
|
|
|
|
To fix the bug, the listener's maxaccept value is now a signed integer. So, if a
|
|
negative value is set for global.tune.maxaccept, we keep it untouched for the
|
|
listener and no calculation is made on it. Then, in the listener code, this
|
|
signed value is casted to a unsigned one. It simplifies all tests instead of
|
|
dealing with negative values. So, it limits the number of connections accepted
|
|
at a time to UINT_MAX at most. But, honestly, it not an issue.
|
|
|
|
This patch must be backported to 1.9 and 1.8.
|
|
|
|
(cherry picked from commit 102854cbbaa4d22466dddec9035d411db244082f)
|
|
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
|
|
(cherry picked from commit bca4fb2d9d7f2966d9f8270fa1796fdc0dfc866d)
|
|
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
|
|
|
|
diff --git a/include/types/listener.h b/include/types/listener.h
|
|
index ea2eadb5..16ef6d7a 100644
|
|
--- a/include/types/listener.h
|
|
+++ b/include/types/listener.h
|
|
@@ -196,7 +196,7 @@ struct listener {
|
|
int nbconn; /* current number of connections on this listener */
|
|
int maxconn; /* maximum connections allowed on this listener */
|
|
unsigned int backlog; /* if set, listen backlog */
|
|
- unsigned int maxaccept; /* if set, max number of connections accepted at once */
|
|
+ int maxaccept; /* if set, max number of connections accepted at once (-1 when disabled) */
|
|
struct list proto_list; /* list in the protocol header */
|
|
int (*accept)(struct listener *l, int fd, struct sockaddr_storage *addr); /* upper layer's accept() */
|
|
enum obj_type *default_target; /* default target to use for accepted sessions or NULL */
|
|
diff --git a/src/listener.c b/src/listener.c
|
|
index 821c931a..74990c45 100644
|
|
--- a/src/listener.c
|
|
+++ b/src/listener.c
|
|
@@ -406,7 +406,7 @@ void listener_accept(int fd)
|
|
{
|
|
struct listener *l = fdtab[fd].owner;
|
|
struct proxy *p;
|
|
- int max_accept;
|
|
+ unsigned int max_accept;
|
|
int next_conn = 0;
|
|
int next_feconn = 0;
|
|
int next_actconn = 0;
|
|
@@ -420,6 +420,10 @@ void listener_accept(int fd)
|
|
if (!l)
|
|
return;
|
|
p = l->bind_conf->frontend;
|
|
+
|
|
+ /* if l->maxaccept is -1, then max_accept is UINT_MAX. It is not really
|
|
+ * illimited, but it is probably enough.
|
|
+ */
|
|
max_accept = l->maxaccept ? l->maxaccept : 1;
|
|
|
|
if (!(l->options & LI_O_UNLIMITED) && global.sps_lim) {
|
|
@@ -480,7 +484,7 @@ void listener_accept(int fd)
|
|
* worst case. If we fail due to system limits or temporary resource
|
|
* shortage, we try again 100ms later in the worst case.
|
|
*/
|
|
- for (; max_accept-- > 0; next_conn = next_feconn = next_actconn = 0) {
|
|
+ for (; max_accept; next_conn = next_feconn = next_actconn = 0, max_accept--) {
|
|
struct sockaddr_storage addr;
|
|
socklen_t laddr = sizeof(addr);
|
|
unsigned int count;
|