haproxy: Update HAProxy to v2.0.5lilik-openwrt-22.03
@ -1,302 +0,0 @@ | |||
commit 937604b4cfccddd607b8d4883815c4e3f9ab70d0 | |||
Author: Willy Tarreau <w@1wt.eu> | |||
Date: Wed Jul 24 16:45:02 2019 +0200 | |||
BUG/MEDIUM: protocols: add a global lock for the init/deinit stuff | |||
Dragan Dosen found that the listeners lock is not sufficient to protect | |||
the listeners list when proxies are stopping because the listeners are | |||
also unlinked from the protocol list, and under certain situations like | |||
bombing with soft-stop signals or shutting down many frontends in parallel | |||
from multiple CLI connections, it could be possible to provoke multiple | |||
instances of delete_listener() to be called in parallel for different | |||
listeners, thus corrupting the protocol lists. | |||
Such operations are pretty rare, they are performed once per proxy upon | |||
startup and once per proxy on shut down. Thus there is no point trying | |||
to optimize anything and we can use a global lock to protect the protocol | |||
lists during these manipulations. | |||
This fix (or a variant) will have to be backported as far as 1.8. | |||
(cherry picked from commit daacf3664506d56a1f3b050ccba504886a18b12a) | |||
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com> | |||
diff --git a/include/proto/protocol.h b/include/proto/protocol.h | |||
index 7bbebb8e..f25f77f0 100644 | |||
--- a/include/proto/protocol.h | |||
+++ b/include/proto/protocol.h | |||
@@ -23,9 +23,11 @@ | |||
#define _PROTO_PROTOCOL_H | |||
#include <sys/socket.h> | |||
+#include <common/hathreads.h> | |||
#include <types/protocol.h> | |||
extern struct protocol *__protocol_by_family[AF_CUST_MAX]; | |||
+__decl_hathreads(extern HA_SPINLOCK_T proto_lock); | |||
/* Registers the protocol <proto> */ | |||
void protocol_register(struct protocol *proto); | |||
diff --git a/include/types/protocol.h b/include/types/protocol.h | |||
index 1d3404b9..f38baeb9 100644 | |||
--- a/include/types/protocol.h | |||
+++ b/include/types/protocol.h | |||
@@ -80,9 +80,9 @@ struct protocol { | |||
int (*pause)(struct listener *l); /* temporarily pause this listener for a soft restart */ | |||
void (*add)(struct listener *l, int port); /* add a listener for this protocol and port */ | |||
- struct list listeners; /* list of listeners using this protocol */ | |||
- int nb_listeners; /* number of listeners */ | |||
- struct list list; /* list of registered protocols */ | |||
+ struct list listeners; /* list of listeners using this protocol (under proto_lock) */ | |||
+ int nb_listeners; /* number of listeners (under proto_lock) */ | |||
+ struct list list; /* list of registered protocols (under proto_lock) */ | |||
}; | |||
#define CONNECT_HAS_DATA 0x00000001 /* There's data available to be sent */ | |||
diff --git a/src/listener.c b/src/listener.c | |||
index 40a774ed..b5fe2ac2 100644 | |||
--- a/src/listener.c | |||
+++ b/src/listener.c | |||
@@ -433,6 +433,9 @@ static void limit_listener(struct listener *l, struct list *list) | |||
* used as a protocol's generic enable_all() primitive, for use after the | |||
* fork(). It puts the listeners into LI_READY or LI_FULL states depending on | |||
* their number of connections. It always returns ERR_NONE. | |||
+ * | |||
+ * Must be called with proto_lock held. | |||
+ * | |||
*/ | |||
int enable_all_listeners(struct protocol *proto) | |||
{ | |||
@@ -447,6 +450,9 @@ int enable_all_listeners(struct protocol *proto) | |||
* the polling lists when they are in the LI_READY or LI_FULL states. It is | |||
* intended to be used as a protocol's generic disable_all() primitive. It puts | |||
* the listeners into LI_LISTEN, and always returns ERR_NONE. | |||
+ * | |||
+ * Must be called with proto_lock held. | |||
+ * | |||
*/ | |||
int disable_all_listeners(struct protocol *proto) | |||
{ | |||
@@ -516,6 +522,9 @@ void unbind_listener_no_close(struct listener *listener) | |||
/* This function closes all listening sockets bound to the protocol <proto>, | |||
* and the listeners end in LI_ASSIGNED state if they were higher. It does not | |||
* detach them from the protocol. It always returns ERR_NONE. | |||
+ * | |||
+ * Must be called with proto_lock held. | |||
+ * | |||
*/ | |||
int unbind_all_listeners(struct protocol *proto) | |||
{ | |||
@@ -580,14 +589,19 @@ int create_listeners(struct bind_conf *bc, const struct sockaddr_storage *ss, | |||
* number of listeners is updated, as well as the global number of listeners | |||
* and jobs. Note that the listener must have previously been unbound. This | |||
* is the generic function to use to remove a listener. | |||
+ * | |||
+ * Will grab the proto_lock. | |||
+ * | |||
*/ | |||
void delete_listener(struct listener *listener) | |||
{ | |||
HA_SPIN_LOCK(LISTENER_LOCK, &listener->lock); | |||
if (listener->state == LI_ASSIGNED) { | |||
listener->state = LI_INIT; | |||
+ HA_SPIN_LOCK(PROTO_LOCK, &proto_lock); | |||
LIST_DEL(&listener->proto_list); | |||
listener->proto->nb_listeners--; | |||
+ HA_SPIN_UNLOCK(PROTO_LOCK, &proto_lock); | |||
_HA_ATOMIC_SUB(&jobs, 1); | |||
_HA_ATOMIC_SUB(&listeners, 1); | |||
} | |||
diff --git a/src/proto_sockpair.c b/src/proto_sockpair.c | |||
index a4faa370..e7dd670d 100644 | |||
--- a/src/proto_sockpair.c | |||
+++ b/src/proto_sockpair.c | |||
@@ -80,6 +80,9 @@ INITCALL1(STG_REGISTER, protocol_register, &proto_sockpair); | |||
/* Add <listener> to the list of sockpair listeners (port is ignored). The | |||
* listener's state is automatically updated from LI_INIT to LI_ASSIGNED. | |||
* The number of listeners for the protocol is updated. | |||
+ * | |||
+ * Must be called with proto_lock held. | |||
+ * | |||
*/ | |||
static void sockpair_add_listener(struct listener *listener, int port) | |||
{ | |||
@@ -97,6 +100,8 @@ static void sockpair_add_listener(struct listener *listener, int port) | |||
* loose them across the fork(). A call to uxst_enable_listeners() is needed | |||
* to complete initialization. | |||
* | |||
+ * Must be called with proto_lock held. | |||
+ * | |||
* The return value is composed from ERR_NONE, ERR_RETRYABLE and ERR_FATAL. | |||
*/ | |||
static int sockpair_bind_listeners(struct protocol *proto, char *errmsg, int errlen) | |||
diff --git a/src/proto_tcp.c b/src/proto_tcp.c | |||
index 64ffb83c..bcbe27a7 100644 | |||
--- a/src/proto_tcp.c | |||
+++ b/src/proto_tcp.c | |||
@@ -1103,6 +1103,9 @@ int tcp_bind_listener(struct listener *listener, char *errmsg, int errlen) | |||
* The sockets will be registered but not added to any fd_set, in order not to | |||
* loose them across the fork(). A call to enable_all_listeners() is needed | |||
* to complete initialization. The return value is composed from ERR_*. | |||
+ * | |||
+ * Must be called with proto_lock held. | |||
+ * | |||
*/ | |||
static int tcp_bind_listeners(struct protocol *proto, char *errmsg, int errlen) | |||
{ | |||
@@ -1121,6 +1124,9 @@ static int tcp_bind_listeners(struct protocol *proto, char *errmsg, int errlen) | |||
/* Add <listener> to the list of tcpv4 listeners, on port <port>. The | |||
* listener's state is automatically updated from LI_INIT to LI_ASSIGNED. | |||
* The number of listeners for the protocol is updated. | |||
+ * | |||
+ * Must be called with proto_lock held. | |||
+ * | |||
*/ | |||
static void tcpv4_add_listener(struct listener *listener, int port) | |||
{ | |||
@@ -1136,6 +1142,9 @@ static void tcpv4_add_listener(struct listener *listener, int port) | |||
/* Add <listener> to the list of tcpv6 listeners, on port <port>. The | |||
* listener's state is automatically updated from LI_INIT to LI_ASSIGNED. | |||
* The number of listeners for the protocol is updated. | |||
+ * | |||
+ * Must be called with proto_lock held. | |||
+ * | |||
*/ | |||
static void tcpv6_add_listener(struct listener *listener, int port) | |||
{ | |||
diff --git a/src/proto_uxst.c b/src/proto_uxst.c | |||
index 66093af6..7263240f 100644 | |||
--- a/src/proto_uxst.c | |||
+++ b/src/proto_uxst.c | |||
@@ -379,6 +379,9 @@ static int uxst_unbind_listener(struct listener *listener) | |||
/* Add <listener> to the list of unix stream listeners (port is ignored). The | |||
* listener's state is automatically updated from LI_INIT to LI_ASSIGNED. | |||
* The number of listeners for the protocol is updated. | |||
+ * | |||
+ * Must be called with proto_lock held. | |||
+ * | |||
*/ | |||
static void uxst_add_listener(struct listener *listener, int port) | |||
{ | |||
@@ -594,6 +597,8 @@ static int uxst_connect_server(struct connection *conn, int flags) | |||
* loose them across the fork(). A call to uxst_enable_listeners() is needed | |||
* to complete initialization. | |||
* | |||
+ * Must be called with proto_lock held. | |||
+ * | |||
* The return value is composed from ERR_NONE, ERR_RETRYABLE and ERR_FATAL. | |||
*/ | |||
static int uxst_bind_listeners(struct protocol *proto, char *errmsg, int errlen) | |||
@@ -613,6 +618,9 @@ static int uxst_bind_listeners(struct protocol *proto, char *errmsg, int errlen) | |||
/* This function stops all listening UNIX sockets bound to the protocol | |||
* <proto>. It does not detaches them from the protocol. | |||
* It always returns ERR_NONE. | |||
+ * | |||
+ * Must be called with proto_lock held. | |||
+ * | |||
*/ | |||
static int uxst_unbind_listeners(struct protocol *proto) | |||
{ | |||
diff --git a/src/protocol.c b/src/protocol.c | |||
index 96e01c82..ac45cf2e 100644 | |||
--- a/src/protocol.c | |||
+++ b/src/protocol.c | |||
@@ -18,18 +18,26 @@ | |||
#include <common/mini-clist.h> | |||
#include <common/standard.h> | |||
-#include <types/protocol.h> | |||
+#include <proto/protocol.h> | |||
/* List head of all registered protocols */ | |||
static struct list protocols = LIST_HEAD_INIT(protocols); | |||
struct protocol *__protocol_by_family[AF_CUST_MAX] = { }; | |||
+/* This is the global spinlock we may need to register/unregister listeners or | |||
+ * protocols. Its main purpose is in fact to serialize the rare stop/deinit() | |||
+ * phases. | |||
+ */ | |||
+__decl_spinlock(proto_lock); | |||
+ | |||
/* Registers the protocol <proto> */ | |||
void protocol_register(struct protocol *proto) | |||
{ | |||
+ HA_SPIN_LOCK(PROTO_LOCK, &proto_lock); | |||
LIST_ADDQ(&protocols, &proto->list); | |||
if (proto->sock_domain >= 0 && proto->sock_domain < AF_CUST_MAX) | |||
__protocol_by_family[proto->sock_domain] = proto; | |||
+ HA_SPIN_UNLOCK(PROTO_LOCK, &proto_lock); | |||
} | |||
/* Unregisters the protocol <proto>. Note that all listeners must have | |||
@@ -37,8 +45,10 @@ void protocol_register(struct protocol *proto) | |||
*/ | |||
void protocol_unregister(struct protocol *proto) | |||
{ | |||
+ HA_SPIN_LOCK(PROTO_LOCK, &proto_lock); | |||
LIST_DEL(&proto->list); | |||
LIST_INIT(&proto->list); | |||
+ HA_SPIN_UNLOCK(PROTO_LOCK, &proto_lock); | |||
} | |||
/* binds all listeners of all registered protocols. Returns a composition | |||
@@ -50,6 +60,7 @@ int protocol_bind_all(char *errmsg, int errlen) | |||
int err; | |||
err = 0; | |||
+ HA_SPIN_LOCK(PROTO_LOCK, &proto_lock); | |||
list_for_each_entry(proto, &protocols, list) { | |||
if (proto->bind_all) { | |||
err |= proto->bind_all(proto, errmsg, errlen); | |||
@@ -57,6 +68,7 @@ int protocol_bind_all(char *errmsg, int errlen) | |||
break; | |||
} | |||
} | |||
+ HA_SPIN_UNLOCK(PROTO_LOCK, &proto_lock); | |||
return err; | |||
} | |||
@@ -71,11 +83,13 @@ int protocol_unbind_all(void) | |||
int err; | |||
err = 0; | |||
+ HA_SPIN_LOCK(PROTO_LOCK, &proto_lock); | |||
list_for_each_entry(proto, &protocols, list) { | |||
if (proto->unbind_all) { | |||
err |= proto->unbind_all(proto); | |||
} | |||
} | |||
+ HA_SPIN_UNLOCK(PROTO_LOCK, &proto_lock); | |||
return err; | |||
} | |||
@@ -89,11 +103,13 @@ int protocol_enable_all(void) | |||
int err; | |||
err = 0; | |||
+ HA_SPIN_LOCK(PROTO_LOCK, &proto_lock); | |||
list_for_each_entry(proto, &protocols, list) { | |||
if (proto->enable_all) { | |||
err |= proto->enable_all(proto); | |||
} | |||
} | |||
+ HA_SPIN_UNLOCK(PROTO_LOCK, &proto_lock); | |||
return err; | |||
} | |||
@@ -107,11 +123,13 @@ int protocol_disable_all(void) | |||
int err; | |||
err = 0; | |||
+ HA_SPIN_LOCK(PROTO_LOCK, &proto_lock); | |||
list_for_each_entry(proto, &protocols, list) { | |||
if (proto->disable_all) { | |||
err |= proto->disable_all(proto); | |||
} | |||
} | |||
+ HA_SPIN_UNLOCK(PROTO_LOCK, &proto_lock); | |||
return err; | |||
} | |||
@ -0,0 +1,46 @@ | |||
commit 3a761682a65e7e7f7baf172f58b15e567a685387 | |||
Author: Willy Tarreau <w@1wt.eu> | |||
Date: Wed Aug 21 14:12:19 2019 +0200 | |||
MINOR: debug: indicate the applet name when the task is task_run_applet() | |||
This allows to figure what applet is currently being executed (and likely | |||
hung). | |||
(cherry picked from commit a512b02f67a30ab5519d04f8c8b1263415321c85) | |||
[wt: backported to improve troubleshooting when the watchdog fires] | |||
Signed-off-by: Willy Tarreau <w@1wt.eu> | |||
diff --git a/src/debug.c b/src/debug.c | |||
index 3077e97c..36cc9e71 100644 | |||
--- a/src/debug.c | |||
+++ b/src/debug.c | |||
@@ -90,6 +90,7 @@ void ha_thread_dump(struct buffer *buf, int thr, int calling_tid) | |||
void ha_task_dump(struct buffer *buf, const struct task *task, const char *pfx) | |||
{ | |||
const struct stream *s = NULL; | |||
+ const struct appctx __maybe_unused *appctx = NULL; | |||
if (!task) { | |||
chunk_appendf(buf, "0\n"); | |||
@@ -110,7 +111,7 @@ void ha_task_dump(struct buffer *buf, const struct task *task, const char *pfx) | |||
task->call_date ? " ns ago" : ""); | |||
chunk_appendf(buf, "%s" | |||
- " fct=%p (%s) ctx=%p\n", | |||
+ " fct=%p (%s) ctx=%p", | |||
pfx, | |||
task->process, | |||
task->process == process_stream ? "process_stream" : | |||
@@ -119,6 +120,11 @@ void ha_task_dump(struct buffer *buf, const struct task *task, const char *pfx) | |||
"?", | |||
task->context); | |||
+ if (task->process == task_run_applet && (appctx = task->context)) | |||
+ chunk_appendf(buf, "(%s)\n", appctx->applet->name); | |||
+ else | |||
+ chunk_appendf(buf, "\n"); | |||
+ | |||
if (task->process == process_stream && task->context) | |||
s = (struct stream *)task->context; | |||
else if (task->process == task_run_applet && task->context) |
@ -1,64 +0,0 @@ | |||
commit 6d79cedaaa4a16b2f42d2bf2bc25772a51354e91 | |||
Author: Willy Tarreau <w@1wt.eu> | |||
Date: Wed Jul 24 17:42:44 2019 +0200 | |||
BUG/MINOR: proxy: always lock stop_proxy() | |||
There is one unprotected call to stop_proxy() from the manage_proxy() | |||
task, so there is a single caller by definition, but there is also | |||
another such call from the CLI's "shutdown frontend" parser. This | |||
one does it under the proxy's lock but the first one doesn't use it. | |||
Thus it is theorically possible to corrupt the list of listeners in a | |||
proxy by issuing "shutdown frontend" and SIGUSR1 exactly at the same | |||
time. While it sounds particularly contrived or stupid, it could | |||
possibly happen with automated tools that would send actions via | |||
various channels. This could cause the process to loop forever or | |||
to crash and thus stop faster than expected. | |||
This might be backported as far as 1.8. | |||
(cherry picked from commit 3de3cd4d9761324b31d23eb2c4a9434ed33801b8) | |||
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com> | |||
diff --git a/src/proxy.c b/src/proxy.c | |||
index f669ebf1..ae761ead 100644 | |||
--- a/src/proxy.c | |||
+++ b/src/proxy.c | |||
@@ -1258,13 +1258,16 @@ void zombify_proxy(struct proxy *p) | |||
* to be called when going down in order to release the ports so that another | |||
* process may bind to them. It must also be called on disabled proxies at the | |||
* end of start-up. If all listeners are closed, the proxy is set to the | |||
- * PR_STSTOPPED state. | |||
+ * PR_STSTOPPED state. The function takes the proxy's lock so it's safe to | |||
+ * call from multiple places. | |||
*/ | |||
void stop_proxy(struct proxy *p) | |||
{ | |||
struct listener *l; | |||
int nostop = 0; | |||
+ HA_SPIN_LOCK(PROXY_LOCK, &p->lock); | |||
+ | |||
list_for_each_entry(l, &p->conf.listeners, by_fe) { | |||
if (l->options & LI_O_NOSTOP) { | |||
HA_ATOMIC_ADD(&unstoppable_jobs, 1); | |||
@@ -1278,6 +1281,8 @@ void stop_proxy(struct proxy *p) | |||
} | |||
if (!nostop) | |||
p->state = PR_STSTOPPED; | |||
+ | |||
+ HA_SPIN_UNLOCK(PROXY_LOCK, &p->lock); | |||
} | |||
/* This function resumes listening on the specified proxy. It scans all of its | |||
@@ -2110,10 +2115,7 @@ static int cli_parse_shutdown_frontend(char **args, char *payload, struct appctx | |||
send_log(px, LOG_WARNING, "Proxy %s stopped (FE: %lld conns, BE: %lld conns).\n", | |||
px->id, px->fe_counters.cum_conn, px->be_counters.cum_conn); | |||
- HA_SPIN_LOCK(PROXY_LOCK, &px->lock); | |||
stop_proxy(px); | |||
- HA_SPIN_UNLOCK(PROXY_LOCK, &px->lock); | |||
- | |||
return 1; | |||
} | |||
@ -0,0 +1,75 @@ | |||
commit fe575b5ca645d6751fba56efa907952eda200b09 | |||
Author: Willy Tarreau <w@1wt.eu> | |||
Date: Wed Aug 21 13:17:37 2019 +0200 | |||
MINOR: tools: add append_prefixed_str() | |||
This is somewhat related to indent_msg() except that this one places a | |||
known prefix at the beginning of each line, allows to replace the EOL | |||
character, and not to insert a prefix on the first line if not desired. | |||
It works with a normal output buffer/chunk so it doesn't need to allocate | |||
anything nor to modify the input string. It is suitable for use in multi- | |||
line backtraces. | |||
(cherry picked from commit a2c9911ace8537e0a350daf8d981170a001b6c7a) | |||
[wt: backported to improve troubleshooting when the watchdog fires] | |||
Signed-off-by: Willy Tarreau <w@1wt.eu> | |||
diff --git a/include/common/standard.h b/include/common/standard.h | |||
index 0f4b1870..cdefc9f5 100644 | |||
--- a/include/common/standard.h | |||
+++ b/include/common/standard.h | |||
@@ -1238,6 +1238,7 @@ char *memprintf(char **out, const char *format, ...) | |||
* free(err); | |||
*/ | |||
char *indent_msg(char **out, int level); | |||
+int append_prefixed_str(struct buffer *out, const char *in, const char *pfx, char eol, int first); | |||
/* removes environment variable <name> from the environment as found in | |||
* environ. This is only provided as an alternative for systems without | |||
diff --git a/src/standard.c b/src/standard.c | |||
index 2f205f74..717c14a9 100644 | |||
--- a/src/standard.c | |||
+++ b/src/standard.c | |||
@@ -3709,6 +3709,41 @@ char *indent_msg(char **out, int level) | |||
return ret; | |||
} | |||
+/* makes a copy of message <in> into <out>, with each line prefixed with <pfx> | |||
+ * and end of lines replaced with <eol> if not 0. The first line to indent has | |||
+ * to be indicated in <first> (starts at zero), so that it is possible to skip | |||
+ * indenting the first line if it has to be appended after an existing message. | |||
+ * Empty strings are never indented, and NULL strings are considered empty both | |||
+ * for <in> and <pfx>. It returns non-zero if an EOL was appended as the last | |||
+ * character, non-zero otherwise. | |||
+ */ | |||
+int append_prefixed_str(struct buffer *out, const char *in, const char *pfx, char eol, int first) | |||
+{ | |||
+ int bol, lf; | |||
+ int pfxlen = pfx ? strlen(pfx) : 0; | |||
+ | |||
+ if (!in) | |||
+ return 0; | |||
+ | |||
+ bol = 1; | |||
+ lf = 0; | |||
+ while (*in) { | |||
+ if (bol && pfxlen) { | |||
+ if (first > 0) | |||
+ first--; | |||
+ else | |||
+ b_putblk(out, pfx, pfxlen); | |||
+ bol = 0; | |||
+ } | |||
+ | |||
+ lf = (*in == '\n'); | |||
+ bol |= lf; | |||
+ b_putchr(out, (lf && eol) ? eol : *in); | |||
+ in++; | |||
+ } | |||
+ return lf; | |||
+} | |||
+ | |||
/* removes environment variable <name> from the environment as found in | |||
* environ. This is only provided as an alternative for systems without | |||
* unsetenv() (old Solaris and AIX versions). THIS IS NOT THREAD SAFE. |
@ -1,33 +0,0 @@ | |||
commit a4ca26661f95a60974fb13a78b1a0c89f9c09ea9 | |||
Author: Willy Tarreau <w@1wt.eu> | |||
Date: Thu Jul 25 07:53:56 2019 +0200 | |||
BUILD: threads: add the definition of PROTO_LOCK | |||
This one was added by commit daacf3664 ("BUG/MEDIUM: protocols: add a | |||
global lock for the init/deinit stuff") but I forgot to add it to the | |||
include file, breaking DEBUG_THREAD. | |||
(cherry picked from commit d6e0c03384cab2c72fb6ab841420045108ea4e6f) | |||
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com> | |||
diff --git a/include/common/hathreads.h b/include/common/hathreads.h | |||
index a7c8dc93..b05215bd 100644 | |||
--- a/include/common/hathreads.h | |||
+++ b/include/common/hathreads.h | |||
@@ -562,6 +562,7 @@ enum lock_label { | |||
AUTH_LOCK, | |||
LOGSRV_LOCK, | |||
DICT_LOCK, | |||
+ PROTO_LOCK, | |||
OTHER_LOCK, | |||
LOCK_LABELS | |||
}; | |||
@@ -679,6 +680,7 @@ static inline const char *lock_label(enum lock_label label) | |||
case AUTH_LOCK: return "AUTH"; | |||
case LOGSRV_LOCK: return "LOGSRV"; | |||
case DICT_LOCK: return "DICT"; | |||
+ case PROTO_LOCK: return "PROTO"; | |||
case OTHER_LOCK: return "OTHER"; | |||
case LOCK_LABELS: break; /* keep compiler happy */ | |||
}; |
@ -0,0 +1,66 @@ | |||
commit 83a5ff403a2cd625832f01032c0feb8bf9c2a89e | |||
Author: Willy Tarreau <w@1wt.eu> | |||
Date: Wed Aug 21 14:14:50 2019 +0200 | |||
MINOR: lua: export applet and task handlers | |||
The current functions are seen outside from the debugging code and are | |||
convenient to export so that we can improve the thread dump output : | |||
void hlua_applet_tcp_fct(struct appctx *ctx); | |||
void hlua_applet_http_fct(struct appctx *ctx); | |||
struct task *hlua_process_task(struct task *task, void *context, unsigned short state); | |||
Of course they are only available when USE_LUA is defined. | |||
(cherry picked from commit 60409db0b1743d670e54244425f6e08c389b7dde) | |||
[wt: backported to improve troubleshooting when the watchdog fires; | |||
while in 2.0 we also have hlua_applet_htx_fct(), it's not | |||
visible outside hlua_applet_http_fct() so we don't care] | |||
Signed-off-by: Willy Tarreau <w@1wt.eu> | |||
diff --git a/include/proto/hlua.h b/include/proto/hlua.h | |||
index 7ad5a99e..32468b77 100644 | |||
--- a/include/proto/hlua.h | |||
+++ b/include/proto/hlua.h | |||
@@ -27,6 +27,9 @@ | |||
void hlua_ctx_destroy(struct hlua *lua); | |||
void hlua_init(); | |||
int hlua_post_init(); | |||
+void hlua_applet_tcp_fct(struct appctx *ctx); | |||
+void hlua_applet_http_fct(struct appctx *ctx); | |||
+struct task *hlua_process_task(struct task *task, void *context, unsigned short state); | |||
#else /* USE_LUA */ | |||
diff --git a/src/hlua.c b/src/hlua.c | |||
index d2708f87..813aa724 100644 | |||
--- a/src/hlua.c | |||
+++ b/src/hlua.c | |||
@@ -6237,7 +6237,7 @@ __LJMP static int hlua_set_nice(lua_State *L) | |||
* Task wrapper are longjmp safe because the only one Lua code | |||
* executed is the safe hlua_ctx_resume(); | |||
*/ | |||
-static struct task *hlua_process_task(struct task *task, void *context, unsigned short state) | |||
+struct task *hlua_process_task(struct task *task, void *context, unsigned short state) | |||
{ | |||
struct hlua *hlua = context; | |||
enum hlua_exec status; | |||
@@ -7045,7 +7045,7 @@ static int hlua_applet_tcp_init(struct appctx *ctx, struct proxy *px, struct str | |||
return 1; | |||
} | |||
-static void hlua_applet_tcp_fct(struct appctx *ctx) | |||
+void hlua_applet_tcp_fct(struct appctx *ctx) | |||
{ | |||
struct stream_interface *si = ctx->owner; | |||
struct stream *strm = si_strm(si); | |||
@@ -7417,7 +7417,7 @@ static void hlua_applet_htx_fct(struct appctx *ctx) | |||
goto done; | |||
} | |||
-static void hlua_applet_http_fct(struct appctx *ctx) | |||
+void hlua_applet_http_fct(struct appctx *ctx) | |||
{ | |||
struct stream_interface *si = ctx->owner; | |||
struct stream *strm = si_strm(si); |
@ -1,32 +0,0 @@ | |||
commit 974c6916ba2f7efc83193bb8c04e95294ca21112 | |||
Author: Christopher Faulet <cfaulet@haproxy.com> | |||
Date: Fri Jul 26 13:52:13 2019 +0200 | |||
BUG/MEDIUM: lb-chash: Fix the realloc() when the number of nodes is increased | |||
When the number of nodes is increased because the server weight is changed, the | |||
nodes array must be realloc. But its new size is not correctly set. Only the | |||
total number of nodes is used to set the new size. But it must also depends on | |||
the size of a node. It must be the total nomber of nodes times the size of a | |||
node. | |||
This issue was reported on Github (#189). | |||
This patch must be backported to all versions since the 1.6. | |||
(cherry picked from commit 366ad86af72c455cc958943913cb2de20eefee71) | |||
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com> | |||
diff --git a/src/lb_chash.c b/src/lb_chash.c | |||
index a35351e9..0bf4e81a 100644 | |||
--- a/src/lb_chash.c | |||
+++ b/src/lb_chash.c | |||
@@ -84,7 +84,7 @@ static inline void chash_queue_dequeue_srv(struct server *s) | |||
* increased the weight beyond the original weight | |||
*/ | |||
if (s->lb_nodes_tot < s->next_eweight) { | |||
- struct tree_occ *new_nodes = realloc(s->lb_nodes, s->next_eweight); | |||
+ struct tree_occ *new_nodes = realloc(s->lb_nodes, s->next_eweight * sizeof(*new_nodes)); | |||
if (new_nodes) { | |||
unsigned int j; |
@ -0,0 +1,85 @@ | |||
commit 4856b36cba80a259a78645753520323caca78d0f | |||
Author: Willy Tarreau <w@1wt.eu> | |||
Date: Wed Aug 21 14:16:02 2019 +0200 | |||
MEDIUM: debug: make the thread dump code show Lua backtraces | |||
When we dump a thread's state (show thread, panic) we don't know if | |||
anything is happening in Lua, which can be problematic especially when | |||
calling external functions. With this patch, the thread dump code can | |||
now detect if we're running in a global Lua task (hlua_process_task), | |||
or in a TCP or HTTP Lua service (task_run_applet and applet.fct == | |||
hlua_applet_tcp_fct or http_applet_http_fct), or a fetch/converter | |||
from an analyser (s->hlua != NULL). In such situations, it's able to | |||
append a formatted Lua backtrace of the Lua execution path with | |||
function names, file names and line numbers. | |||
Note that a shorter alternative could be to call "luaL_where(hlua->T,0)" | |||
which only prints the current location, but it's not necessarily sufficient | |||
for complex code. | |||
(cherry picked from commit 78a7cb648ca33823c06430cedc6859ea7e7cd5df) | |||
[wt: backported to improve troubleshooting when the watchdog fires] | |||
Signed-off-by: Willy Tarreau <w@1wt.eu> | |||
diff --git a/src/debug.c b/src/debug.c | |||
index 36cc9e71..79bea884 100644 | |||
--- a/src/debug.c | |||
+++ b/src/debug.c | |||
@@ -26,6 +26,7 @@ | |||
#include <proto/cli.h> | |||
#include <proto/fd.h> | |||
+#include <proto/hlua.h> | |||
#include <proto/stream_interface.h> | |||
#include <proto/task.h> | |||
@@ -91,6 +92,7 @@ void ha_task_dump(struct buffer *buf, const struct task *task, const char *pfx) | |||
{ | |||
const struct stream *s = NULL; | |||
const struct appctx __maybe_unused *appctx = NULL; | |||
+ struct hlua __maybe_unused *hlua = NULL; | |||
if (!task) { | |||
chunk_appendf(buf, "0\n"); | |||
@@ -117,6 +119,9 @@ void ha_task_dump(struct buffer *buf, const struct task *task, const char *pfx) | |||
task->process == process_stream ? "process_stream" : | |||
task->process == task_run_applet ? "task_run_applet" : | |||
task->process == si_cs_io_cb ? "si_cs_io_cb" : | |||
+#ifdef USE_LUA | |||
+ task->process == hlua_process_task ? "hlua_process_task" : | |||
+#endif | |||
"?", | |||
task->context); | |||
@@ -134,6 +139,30 @@ void ha_task_dump(struct buffer *buf, const struct task *task, const char *pfx) | |||
if (s) | |||
stream_dump(buf, s, pfx, '\n'); | |||
+ | |||
+#ifdef USE_LUA | |||
+ hlua = NULL; | |||
+ if (s && (hlua = s->hlua)) { | |||
+ chunk_appendf(buf, "%sCurrent executing Lua from a stream analyser -- ", pfx); | |||
+ } | |||
+ else if (task->process == hlua_process_task && (hlua = task->context)) { | |||
+ chunk_appendf(buf, "%sCurrent executing a Lua task -- ", pfx); | |||
+ } | |||
+ else if (task->process == task_run_applet && (appctx = task->context) && | |||
+ (appctx->applet->fct == hlua_applet_tcp_fct && (hlua = appctx->ctx.hlua_apptcp.hlua))) { | |||
+ chunk_appendf(buf, "%sCurrent executing a Lua TCP service -- ", pfx); | |||
+ } | |||
+ else if (task->process == task_run_applet && (appctx = task->context) && | |||
+ (appctx->applet->fct == hlua_applet_http_fct && (hlua = appctx->ctx.hlua_apphttp.hlua))) { | |||
+ chunk_appendf(buf, "%sCurrent executing a Lua HTTP service -- ", pfx); | |||
+ } | |||
+ | |||
+ if (hlua) { | |||
+ luaL_traceback(hlua->T, hlua->T, NULL, 0); | |||
+ if (!append_prefixed_str(buf, lua_tostring(hlua->T, -1), pfx, '\n', 1)) | |||
+ b_putchr(buf, '\n'); | |||
+ } | |||
+#endif | |||
} | |||
/* This function dumps all profiling settings. It returns 0 if the output |
@ -0,0 +1,79 @@ | |||
commit 9a408abbb8559df5718bc696bd9c3934c6500d63 | |||
Author: Willy Tarreau <w@1wt.eu> | |||
Date: Fri Aug 23 08:11:36 2019 +0200 | |||
BUG/MEDIUM: mux-h1: do not truncate trailing 0CRLF on buffer boundary | |||
The H1 message parser calls the various message block parsers with an | |||
offset indicating where in the buffer to start from, and only consumes | |||
the data at the end of the parsing. The headers and trailers parsers | |||
have a condition detecting if a headers or trailers block is too large | |||
to fit into the buffer. This is detected by an incomplete block while | |||
the buffer is full. Unfortunately it doesn't take into account the fact | |||
that the block may be parsed after other blocks that are still present | |||
in the buffer, resulting in aborting some transfers early as reported | |||
in issue #231. This typically happens if a trailers block is incomplete | |||
at the end of a buffer full of data, which typically happens with data | |||
sizes multiple of the buffer size minus less than the trailers block | |||
size. It also happens with the CRLF that follows the 0-sized chunk of | |||
any transfer-encoded contents is itself on such a boundary since this | |||
CRLF is technically part of the trailers block. This can be reproduced | |||
by asking a server to retrieve exactly 31532 or 31533 bytes of static | |||
data using chunked encoding with curl, which reports: | |||
transfer closed with outstanding read data remaining | |||
This issue was revealed in 2.0 and does not affect 1.9 because in 1.9 | |||
the trailers block was processed at once as part of the data block | |||
processing, and would simply give up and wait for the rest of the data | |||
to arrive. | |||
It's interesting to note that the headers block parsing is also affected | |||
by this issue but in practice it has a much more limited impact since a | |||
headers block is normally only parsed at the beginning of a buffer. The | |||
only case where it seems to matter is when dealing with a response buffer | |||
full of 100-continue header blocks followed by a regular header block, | |||
which will then be rejected for the same reason. | |||
This fix must be backported to 2.0 and partially to 1.9 (the headers | |||
block part). | |||
(cherry picked from commit 347f464d4e5a8a2bf3acd2411a6c8228e605e7f6) | |||
Signed-off-by: Willy Tarreau <w@1wt.eu> | |||
diff --git a/src/mux_h1.c b/src/mux_h1.c | |||
index fa694c41..01f225a2 100644 | |||
--- a/src/mux_h1.c | |||
+++ b/src/mux_h1.c | |||
@@ -995,10 +995,11 @@ static size_t h1_process_headers(struct h1s *h1s, struct h1m *h1m, struct htx *h | |||
ret = h1_headers_to_hdr_list(b_peek(buf, *ofs), b_tail(buf), | |||
hdrs, sizeof(hdrs)/sizeof(hdrs[0]), h1m, &h1sl); | |||
if (ret <= 0) { | |||
- /* Incomplete or invalid message. If the buffer is full, it's an | |||
- * error because headers are too large to be handled by the | |||
- * parser. */ | |||
- if (ret < 0 || (!ret && !buf_room_for_htx_data(buf))) | |||
+ /* Incomplete or invalid message. If the input buffer only | |||
+ * contains headers and is full, which is detected by it being | |||
+ * full and the offset to be zero, it's an error because | |||
+ * headers are too large to be handled by the parser. */ | |||
+ if (ret < 0 || (!ret && !*ofs && !buf_room_for_htx_data(buf))) | |||
goto error; | |||
goto end; | |||
} | |||
@@ -1339,10 +1340,11 @@ static size_t h1_process_trailers(struct h1s *h1s, struct h1m *h1m, struct htx * | |||
ret = h1_headers_to_hdr_list(b_peek(buf, *ofs), b_tail(buf), | |||
hdrs, sizeof(hdrs)/sizeof(hdrs[0]), &tlr_h1m, NULL); | |||
if (ret <= 0) { | |||
- /* Incomplete or invalid trailers. If the buffer is full, it's | |||
- * an error because traliers are too large to be handled by the | |||
- * parser. */ | |||
- if (ret < 0 || (!ret && !buf_room_for_htx_data(buf))) | |||
+ /* Incomplete or invalid trailers. If the input buffer only | |||
+ * contains trailers and is full, which is detected by it being | |||
+ * full and the offset to be zero, it's an error because | |||
+ * trailers are too large to be handled by the parser. */ | |||
+ if (ret < 0 || (!ret && !*ofs && !buf_room_for_htx_data(buf))) | |||
goto error; | |||
goto end; | |||
} |
@ -1,32 +0,0 @@ | |||
commit 21a796cb83c29ee276feb04649a1b18214bbdee0 | |||
Author: Olivier Houchard <ohouchard@haproxy.com> | |||
Date: Fri Jul 26 14:54:34 2019 +0200 | |||
BUG/MEDIUM: streams: Don't switch the SI to SI_ST_DIS if we have data to send. | |||
In sess_established(), don't immediately switch the backend stream_interface | |||
to SI_ST_DIS if we only got a SHUTR. We may still have something to send, | |||
ie if the request is a POST, and we should be switched to SI_ST8DIS later | |||
when the shutw will happen. | |||
This should be backported to 2.0 and 1.9. | |||
(cherry picked from commit 7859526fd6ce7ea33e20b7e532b21aa2465cb11d) | |||
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com> | |||
diff --git a/src/stream.c b/src/stream.c | |||
index a5c5f45c..64875c80 100644 | |||
--- a/src/stream.c | |||
+++ b/src/stream.c | |||
@@ -954,8 +954,9 @@ static void sess_establish(struct stream *s) | |||
si_chk_rcv(si); | |||
} | |||
req->wex = TICK_ETERNITY; | |||
- /* If we managed to get the whole response, switch to SI_ST_DIS now. */ | |||
- if (rep->flags & CF_SHUTR) | |||
+ /* If we managed to get the whole response, and we don't have anything | |||
+ * left to send, or can't, switch to SI_ST_DIS now. */ | |||
+ if (rep->flags & (CF_SHUTR | CF_SHUTW)) | |||
si->state = SI_ST_DIS; | |||
} | |||
@ -0,0 +1,68 @@ | |||
commit 620381599324e15403002270637a3b677c3fe7e5 | |||
Author: Willy Tarreau <w@1wt.eu> | |||
Date: Fri Aug 23 09:29:29 2019 +0200 | |||
BUG/MEDIUM: mux-h1: do not report errors on transfers ending on buffer full | |||
If a receipt ends with the HTX buffer full and everything is completed except | |||
appending the HTX EOM block, we end up detecting an error because the H1 | |||
parser did not switch to H1_MSG_DONE yet while all conditions for an end of | |||
stream and end of buffer are met. This can be detected by retrieving 31532 | |||
or 31533 chunk-encoded bytes over H1 and seeing haproxy log "SD--" at the | |||
end of a successful transfer. | |||
Ideally the EOM part should be totally independent on the H1 message state | |||
since the block was really parsed and finished. So we should switch to a | |||
last state requiring to send only EOM. However this needs a few risky | |||
changes. This patch aims for simplicity and backport safety, thus it only | |||
adds a flag to the H1 stream indicating that an EOM is still needed, and | |||
excludes this condition from the ones used to detect end of processing. A | |||
cleaner approach needs to be studied, either by adding a state before DONE | |||
or by setting DONE once the various blocks are parsed and before trying to | |||
send EOM. | |||
This fix must be backported to 2.0. The issue does not seem to affect 1.9 | |||
though it is not yet known why, probably that it is related to the different | |||
encoding of trailers which always leaves a bit of room to let EOM be stored. | |||
(cherry picked from commit 0bb5a5c4b5ad375b1254c2e8bec2dd5ea85d6ebb) | |||
Signed-off-by: Willy Tarreau <w@1wt.eu> | |||
diff --git a/src/mux_h1.c b/src/mux_h1.c | |||
index 01f225a2..b9a37ce5 100644 | |||
--- a/src/mux_h1.c | |||
+++ b/src/mux_h1.c | |||
@@ -67,7 +67,8 @@ | |||
#define H1S_F_BUF_FLUSH 0x00000100 /* Flush input buffer and don't read more data */ | |||
#define H1S_F_SPLICED_DATA 0x00000200 /* Set when the kernel splicing is in used */ | |||
#define H1S_F_HAVE_I_TLR 0x00000800 /* Set during input process to know the trailers were processed */ | |||
-/* 0x00001000 .. 0x00002000 unused */ | |||
+#define H1S_F_APPEND_EOM 0x00001000 /* Send EOM to the HTX buffer */ | |||
+/* 0x00002000 .. 0x00002000 unused */ | |||
#define H1S_F_HAVE_O_CONN 0x00004000 /* Set during output process to know connection mode was processed */ | |||
/* H1 connection descriptor */ | |||
@@ -954,9 +955,12 @@ static size_t h1_eval_htx_res_size(struct h1m *h1m, union h1_sl *h1sl, struct ht | |||
*/ | |||
static size_t h1_process_eom(struct h1s *h1s, struct h1m *h1m, struct htx *htx, size_t max) | |||
{ | |||
- if (max < sizeof(struct htx_blk) + 1 || !htx_add_endof(htx, HTX_BLK_EOM)) | |||
+ if (max < sizeof(struct htx_blk) + 1 || !htx_add_endof(htx, HTX_BLK_EOM)) { | |||
+ h1s->flags |= H1S_F_APPEND_EOM; | |||
return 0; | |||
+ } | |||
+ h1s->flags &= ~H1S_F_APPEND_EOM; | |||
h1m->state = H1_MSG_DONE; | |||
h1s->cs->flags |= CS_FL_EOI; | |||
return (sizeof(struct htx_blk) + 1); | |||
@@ -1472,7 +1476,8 @@ static size_t h1_process_input(struct h1c *h1c, struct buffer *buf, size_t count | |||
else if (h1s_data_pending(h1s) && !htx_is_empty(htx)) | |||
h1s->cs->flags |= CS_FL_RCV_MORE | CS_FL_WANT_ROOM; | |||
- if ((h1s->flags & H1S_F_REOS) && (!h1s_data_pending(h1s) || htx_is_empty(htx))) { | |||
+ if (((h1s->flags & (H1S_F_REOS|H1S_F_APPEND_EOM)) == H1S_F_REOS) && | |||
+ (!h1s_data_pending(h1s) || htx_is_empty(htx))) { | |||
h1s->cs->flags |= CS_FL_EOS; | |||
if (h1m->state > H1_MSG_LAST_LF && h1m->state < H1_MSG_DONE) | |||
h1s->cs->flags |= CS_FL_ERROR; |
@ -1,42 +0,0 @@ | |||
commit 487b38e86c08431bc5f48aac72c8d753ee23cb03 | |||
Author: Willy Tarreau <w@1wt.eu> | |||
Date: Fri Jul 26 15:10:39 2019 +0200 | |||
BUG/MINOR: log: make sure writev() is not interrupted on a file output | |||
Since 1.9 we support sending logs to various non-blocking outputs like | |||
stdou/stderr or flies, by using writev() which guarantees that it only | |||
returns after having written everything or nothing. However the syscall | |||
may be interrupted while doing so, and this is visible when writing to | |||
a tty during debug sessions, as some logs occasionally appear interleaved | |||
if an xterm or SSH connection is not very fast. Performance here is not a | |||
critical concern, log correctness is. Let's simply take the logger's lock | |||
around the writev() call to prevent multiple senders from stepping onto | |||
each other's toes. | |||
This may be backported to 2.0 and 1.9. | |||
(cherry picked from commit 9fbcb7e2e9c32659ab11927394fec2e160be2d0b) | |||
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com> | |||
diff --git a/src/log.c b/src/log.c | |||
index ef999d13..99f185e4 100644 | |||
--- a/src/log.c | |||
+++ b/src/log.c | |||
@@ -1672,8 +1672,15 @@ send: | |||
iovec[7].iov_len = 1; | |||
if (logsrv->addr.ss_family == AF_UNSPEC) { | |||
- /* the target is a direct file descriptor */ | |||
+ /* the target is a direct file descriptor. While writev() guarantees | |||
+ * to write everything, it doesn't guarantee that it will not be | |||
+ * interrupted while doing so. This occasionally results in interleaved | |||
+ * messages when the output is a tty, hence the lock. There's no real | |||
+ * performance concern here for such type of output. | |||
+ */ | |||
+ HA_SPIN_LOCK(LOGSRV_LOCK, &logsrv->lock); | |||
sent = writev(*plogfd, iovec, 8); | |||
+ HA_SPIN_UNLOCK(LOGSRV_LOCK, &logsrv->lock); | |||
} | |||
else { | |||
msghdr.msg_name = (struct sockaddr *)&logsrv->addr; |
@ -0,0 +1,27 @@ | |||
commit 7c80af0fb53f2a1d93a597f7d97cc67996e36be2 | |||
Author: n9@users.noreply.github.com <n9@users.noreply.github.com> | |||
Date: Fri Aug 23 11:21:05 2019 +0200 | |||
DOC: fixed typo in management.txt | |||
replaced fot -> for | |||
added two periods | |||
(cherry picked from commit 25a1c8e4539c12c19a3fe04aabe563cdac5e36db) | |||
Signed-off-by: Willy Tarreau <w@1wt.eu> | |||
diff --git a/doc/management.txt b/doc/management.txt | |||
index 616a040b..ad6011e5 100644 | |||
--- a/doc/management.txt | |||
+++ b/doc/management.txt | |||
@@ -1549,8 +1549,8 @@ enable agent <backend>/<server> | |||
level "admin". | |||
enable dynamic-cookie backend <backend> | |||
- Enable the generation of dynamic cookies fot the backend <backend> | |||
- A secret key must also be provided | |||
+ Enable the generation of dynamic cookies for the backend <backend>. | |||
+ A secret key must also be provided. | |||
enable frontend <frontend> | |||
Resume a frontend which was temporarily stopped. It is possible that some of |
@ -1,101 +0,0 @@ | |||
commit 8de6badd32fb584d60733a6236113edba00f8701 | |||
Author: Willy Tarreau <w@1wt.eu> | |||
Date: Fri Jul 26 15:21:54 2019 +0200 | |||
DOC: improve the wording in CONTRIBUTING about how to document a bug fix | |||
Insufficiently described bug fixes are still too frequent. It's a real | |||
pain to create each new maintenance release, as 3/4 of the time is spent | |||
trying to guess what problem a patch fixes, which is already important | |||
in order to decide whether to pick the fix or not, but is even more | |||
capital in order to write understandable release notes. | |||
Christopher rightfully demands that a patch tagged "BUG" MUST ABSOLUTELY | |||
describe the problem and why this problem is a bug. Describing the fix | |||
is one thing but if the bug is unknown, why would there be a fix ? How | |||
can a stable maintainer be convinced to take a fix if its author didn't | |||
care about checking whether it was a real bug ? This patch tries to | |||
explain a bit better what really needs to appear in the commit message | |||
and how to describe a bug. | |||
To be backported to all relevant stable versions. | |||
(cherry picked from commit 41f638c1eb8167bb473a6c8811d7fd70d7c06e07) | |||
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com> | |||
diff --git a/CONTRIBUTING b/CONTRIBUTING | |||
index 0fcd921e..201e122d 100644 | |||
--- a/CONTRIBUTING | |||
+++ b/CONTRIBUTING | |||
@@ -454,7 +454,18 @@ do not think about them anymore after a few patches. | |||
11) Real commit messages please! | |||
- Please properly format your commit messages. To get an idea, just run | |||
+ The commit message is how you're trying to convince a maintainer to adopt | |||
+ your work and maintain it as long as possible. A dirty commit message almost | |||
+ always comes with dirty code. Too short a commit message indicates that too | |||
+ short an analysis was done and that side effects are extremely likely to be | |||
+ encountered. It's the maintainer's job to decide to accept this work in its | |||
+ current form or not, with the known constraints. Some patches which rework | |||
+ architectural parts or fix sensitive bugs come with 20-30 lines of design | |||
+ explanations, limitations, hypothesis or even doubts, and despite this it | |||
+ happens when reading them 6 months later while trying to identify a bug that | |||
+ developers still miss some information about corner cases. | |||
+ | |||
+ So please properly format your commit messages. To get an idea, just run | |||
"git log" on the file you've just modified. Patches always have the format | |||
of an e-mail made of a subject, a description and the actual patch. If you | |||
are sending a patch as an e-mail formatted this way, it can quickly be | |||
@@ -506,9 +517,17 @@ do not think about them anymore after a few patches. | |||
But in any case, it is important that there is a clean description of what | |||
the patch does, the motivation for what it does, why it's the best way to do | |||
- it, its impacts, and what it does not yet cover. Also, in HAProxy, like many | |||
- projects which take a great care of maintaining stable branches, patches are | |||
- reviewed later so that some of them can be backported to stable releases. | |||
+ it, its impacts, and what it does not yet cover. And this is particularly | |||
+ important for bugs. A patch tagged "BUG" must absolutely explain what the | |||
+ problem is, why it is considered as a bug. Anybody, even non-developers, | |||
+ should be able to tell whether or not a patch is likely to address an issue | |||
+ they are facing. Indicating what the code will do after the fix doesn't help | |||
+ if it does not say what problem is encountered without the patch. Note that | |||
+ in some cases the bug is purely theorical and observed by reading the code. | |||
+ In this case it's perfectly fine to provide an estimate about possible | |||
+ effects. Also, in HAProxy, like many projects which take a great care of | |||
+ maintaining stable branches, patches are reviewed later so that some of them | |||
+ can be backported to stable releases. | |||
While reviewing hundreds of patches can seem cumbersome, with a proper | |||
formatting of the subject line it actually becomes very easy. For example, | |||
@@ -630,13 +649,23 @@ patch types include : | |||
- BUG fix for a bug. The severity of the bug should also be indicated | |||
when known. Similarly, if a backport is needed to older versions, | |||
- it should be indicated on the last line of the commit message. If | |||
- the bug has been identified as a regression brought by a specific | |||
- patch or version, this indication will be appreciated too. New | |||
- maintenance releases are generally emitted when a few of these | |||
- patches are merged. If the bug is a vulnerability for which a CVE | |||
- identifier was assigned before you publish the fix, you can mention | |||
- it in the commit message, it will help distro maintainers. | |||
+ it should be indicated on the last line of the commit message. The | |||
+ commit message MUST ABSOLUTELY describe the problem and its impact | |||
+ to non-developers. Any user must be able to guess if this patch is | |||
+ likely to fix a problem they are facing. Even if the bug was | |||
+ discovered by accident while reading the code or running an | |||
+ automated tool, it is mandatory to try to estimate what potential | |||
+ issue it might cause and under what circumstances. There may even | |||
+ be security implications sometimes so a minimum analysis is really | |||
+ required. Also please think about stable maintainers who have to | |||
+ build the release notes, they need to have enough input about the | |||
+ bug's impact to explain it. If the bug has been identified as a | |||
+ regression brought by a specific patch or version, this indication | |||
+ will be appreciated too. New maintenance releases are generally | |||
+ emitted when a few of these patches are merged. If the bug is a | |||
+ vulnerability for which a CVE identifier was assigned before you | |||
+ publish the fix, you can mention it in the commit message, it will | |||
+ help distro maintainers. | |||
- CLEANUP code cleanup, silence of warnings, etc... theoretically no impact. | |||
These patches will rarely be seen in stable branches, though they |
@ -1,49 +0,0 @@ | |||
commit 72c692701ab4197f1f8ec7594b7e8ef5082b9d9e | |||
Author: Christopher Faulet <cfaulet@haproxy.com> | |||
Date: Fri Jul 26 16:40:24 2019 +0200 | |||
BUG/MINOR: hlua/htx: Reset channels analyzers when txn:done() is called | |||
For HTX streams, when txn:done() is called, the work is delegated to the | |||
function http_reply_and_close(). But it is not enough. The channel's analyzers | |||
must also be reset. Otherwise, some analyzers may still be called while | |||
processing should be aborted. | |||
For instance, if the function is called from an http-request rules on the | |||
frontend, request analyzers on the backend side are still called. So we may try | |||
to add an header to the request, while this one was already reset. | |||
This patch must be backported to 2.0 and 1.9. | |||
(cherry picked from commit fe6a71b8e08234dbe03fbd2fa3017590681479df) | |||
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com> | |||
diff --git a/src/hlua.c b/src/hlua.c | |||
index 23d2aa04..f9d1d699 100644 | |||
--- a/src/hlua.c | |||
+++ b/src/hlua.c | |||
@@ -5996,8 +5996,12 @@ __LJMP static int hlua_txn_done(lua_State *L) | |||
ic = &htxn->s->req; | |||
oc = &htxn->s->res; | |||
- if (IS_HTX_STRM(htxn->s)) | |||
- htx_reply_and_close(htxn->s, 0, NULL); | |||
+ if (IS_HTX_STRM(htxn->s)) { | |||
+ htxn->s->txn->status = 0; | |||
+ http_reply_and_close(htxn->s, 0, NULL); | |||
+ ic->analysers &= AN_REQ_FLT_END; | |||
+ oc->analysers &= AN_RES_FLT_END; | |||
+ } | |||
else { | |||
if (htxn->s->txn) { | |||
/* HTTP mode, let's stay in sync with the stream */ | |||
@@ -6031,6 +6035,9 @@ __LJMP static int hlua_txn_done(lua_State *L) | |||
ic->analysers = 0; | |||
} | |||
+ if (!(htxn->s->flags & SF_ERR_MASK)) // this is not really an error but it is | |||
+ htxn->s->flags |= SF_ERR_LOCAL; // to mark that it comes from the proxy | |||
+ | |||
hlua->flags |= HLUA_STOP; | |||
WILL_LJMP(hlua_done(L)); | |||
return 0; |
@ -0,0 +1,35 @@ | |||
commit f259fcc00a04e633a7a64f894a719f78f3644867 | |||
Author: Willy Tarreau <w@1wt.eu> | |||
Date: Mon Aug 26 10:37:39 2019 +0200 | |||
BUG/MINOR: mworker: disable SIGPROF on re-exec | |||
If haproxy is built with profiling enabled with -pg, it is possible to | |||
see the master quit during a reload while it's re-executing itself with | |||
error code 155 (signal 27) saying "Profile timer expired)". This happens | |||
if the SIGPROF signal is delivered during the execve() call while the | |||
handler was already unregistered. The issue itself is not directly inside | |||
haproxy but it's easy to address. This patch disables this signal before | |||
calling execvp() during a master reload. A simple test for this consists | |||
in running this little script with haproxy started in master-worker mode : | |||
$ while usleep 50000; do killall -USR2 haproxy; done | |||
This fix should be backported to all versions using the master-worker | |||
model. | |||
(cherry picked from commit e0d86e2c1caaaa2141118e3309d479de5f67e855) | |||
Signed-off-by: Willy Tarreau <w@1wt.eu> | |||
diff --git a/src/haproxy.c b/src/haproxy.c | |||
index f6f00fc1..c93b0d13 100644 | |||
--- a/src/haproxy.c | |||
+++ b/src/haproxy.c | |||
@@ -695,6 +695,7 @@ void mworker_reload() | |||
} | |||
ha_warning("Reexecuting Master process\n"); | |||
+ signal(SIGPROF, SIG_IGN); | |||
execvp(next_argv[0], next_argv); | |||
ha_warning("Failed to reexecute the master process [%d]: %s\n", pid, strerror(errno)); |
@ -1,201 +0,0 @@ | |||
commit dc2ee27c7a1908ca3157a10ad131f13644bcaea3 | |||
Author: Christopher Faulet <cfaulet@haproxy.com> | |||
Date: Fri Jul 26 16:17:01 2019 +0200 | |||
BUG/MEDIUM: hlua: Check the calling direction in lua functions of the HTTP class | |||
It is invalid to manipulate responses from http-request rules or to manipulate | |||
requests from http-response rules. When http-request rules are evaluated, the | |||
connection to server is not yet established, so there is no response at all. And | |||
when http-response rules are evaluated, the request has already been sent to the | |||
server. | |||
Now, the calling direction is checked. So functions "txn.http:req_*" can now | |||
only be called from http-request rules and the functions "txn.http:res_*" can | |||
only be called from http-response rules. | |||
This issue was reported on Github (#190). | |||
This patch must be backported to all versions since the 1.6. | |||
(cherry picked from commit 84a6d5bc217a418db8efc4e76a0a32860db2c608) | |||
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com> | |||
diff --git a/src/hlua.c b/src/hlua.c | |||
index f9d1d699..21351cd6 100644 | |||
--- a/src/hlua.c | |||
+++ b/src/hlua.c | |||
@@ -5346,6 +5346,9 @@ __LJMP static int hlua_http_req_get_headers(lua_State *L) | |||
MAY_LJMP(check_args(L, 1, "req_get_headers")); | |||
htxn = MAY_LJMP(hlua_checkhttp(L, 1)); | |||
+ if (htxn->dir != SMP_OPT_DIR_REQ) | |||
+ WILL_LJMP(lua_error(L)); | |||
+ | |||
return hlua_http_get_headers(L, htxn, &htxn->s->txn->req); | |||
} | |||
@@ -5356,6 +5359,9 @@ __LJMP static int hlua_http_res_get_headers(lua_State *L) | |||
MAY_LJMP(check_args(L, 1, "res_get_headers")); | |||
htxn = MAY_LJMP(hlua_checkhttp(L, 1)); | |||
+ if (htxn->dir != SMP_OPT_DIR_RES) | |||
+ WILL_LJMP(lua_error(L)); | |||
+ | |||
return hlua_http_get_headers(L, htxn, &htxn->s->txn->rsp); | |||
} | |||
@@ -5393,6 +5399,9 @@ __LJMP static int hlua_http_req_rep_hdr(lua_State *L) | |||
MAY_LJMP(check_args(L, 4, "req_rep_hdr")); | |||
htxn = MAY_LJMP(hlua_checkhttp(L, 1)); | |||
+ if (htxn->dir != SMP_OPT_DIR_REQ) | |||
+ WILL_LJMP(lua_error(L)); | |||
+ | |||
return MAY_LJMP(hlua_http_rep_hdr(L, htxn, &htxn->s->txn->req, ACT_HTTP_REPLACE_HDR)); | |||
} | |||
@@ -5403,6 +5412,9 @@ __LJMP static int hlua_http_res_rep_hdr(lua_State *L) | |||
MAY_LJMP(check_args(L, 4, "res_rep_hdr")); | |||
htxn = MAY_LJMP(hlua_checkhttp(L, 1)); | |||
+ if (htxn->dir != SMP_OPT_DIR_RES) | |||
+ WILL_LJMP(lua_error(L)); | |||
+ | |||
return MAY_LJMP(hlua_http_rep_hdr(L, htxn, &htxn->s->txn->rsp, ACT_HTTP_REPLACE_HDR)); | |||
} | |||
@@ -5413,6 +5425,9 @@ __LJMP static int hlua_http_req_rep_val(lua_State *L) | |||
MAY_LJMP(check_args(L, 4, "req_rep_hdr")); | |||
htxn = MAY_LJMP(hlua_checkhttp(L, 1)); | |||
+ if (htxn->dir != SMP_OPT_DIR_REQ) | |||
+ WILL_LJMP(lua_error(L)); | |||
+ | |||
return MAY_LJMP(hlua_http_rep_hdr(L, htxn, &htxn->s->txn->req, ACT_HTTP_REPLACE_VAL)); | |||
} | |||
@@ -5423,6 +5438,9 @@ __LJMP static int hlua_http_res_rep_val(lua_State *L) | |||
MAY_LJMP(check_args(L, 4, "res_rep_val")); | |||
htxn = MAY_LJMP(hlua_checkhttp(L, 1)); | |||
+ if (htxn->dir != SMP_OPT_DIR_RES) | |||
+ WILL_LJMP(lua_error(L)); | |||
+ | |||
return MAY_LJMP(hlua_http_rep_hdr(L, htxn, &htxn->s->txn->rsp, ACT_HTTP_REPLACE_VAL)); | |||
} | |||
@@ -5462,6 +5480,9 @@ __LJMP static int hlua_http_req_del_hdr(lua_State *L) | |||
MAY_LJMP(check_args(L, 2, "req_del_hdr")); | |||
htxn = MAY_LJMP(hlua_checkhttp(L, 1)); | |||
+ if (htxn->dir != SMP_OPT_DIR_REQ) | |||
+ WILL_LJMP(lua_error(L)); | |||
+ | |||
return hlua_http_del_hdr(L, htxn, &htxn->s->txn->req); | |||
} | |||
@@ -5469,9 +5490,12 @@ __LJMP static int hlua_http_res_del_hdr(lua_State *L) | |||
{ | |||
struct hlua_txn *htxn; | |||
- MAY_LJMP(check_args(L, 2, "req_del_hdr")); | |||
+ MAY_LJMP(check_args(L, 2, "res_del_hdr")); | |||
htxn = MAY_LJMP(hlua_checkhttp(L, 1)); | |||
+ if (htxn->dir != SMP_OPT_DIR_RES) | |||
+ WILL_LJMP(lua_error(L)); | |||
+ | |||
return hlua_http_del_hdr(L, htxn, &htxn->s->txn->rsp); | |||
} | |||
@@ -5523,6 +5547,9 @@ __LJMP static int hlua_http_req_add_hdr(lua_State *L) | |||
MAY_LJMP(check_args(L, 3, "req_add_hdr")); | |||
htxn = MAY_LJMP(hlua_checkhttp(L, 1)); | |||
+ if (htxn->dir != SMP_OPT_DIR_REQ) | |||
+ WILL_LJMP(lua_error(L)); | |||
+ | |||
return hlua_http_add_hdr(L, htxn, &htxn->s->txn->req); | |||
} | |||
@@ -5533,6 +5560,9 @@ __LJMP static int hlua_http_res_add_hdr(lua_State *L) | |||
MAY_LJMP(check_args(L, 3, "res_add_hdr")); | |||
htxn = MAY_LJMP(hlua_checkhttp(L, 1)); | |||
+ if (htxn->dir != SMP_OPT_DIR_RES) | |||
+ WILL_LJMP(lua_error(L)); | |||
+ | |||
return hlua_http_add_hdr(L, htxn, &htxn->s->txn->rsp); | |||
} | |||
@@ -5543,6 +5573,9 @@ static int hlua_http_req_set_hdr(lua_State *L) | |||
MAY_LJMP(check_args(L, 3, "req_set_hdr")); | |||
htxn = MAY_LJMP(hlua_checkhttp(L, 1)); | |||
+ if (htxn->dir != SMP_OPT_DIR_REQ) | |||
+ WILL_LJMP(lua_error(L)); | |||
+ | |||
hlua_http_del_hdr(L, htxn, &htxn->s->txn->req); | |||
return hlua_http_add_hdr(L, htxn, &htxn->s->txn->req); | |||
} | |||
@@ -5554,6 +5587,9 @@ static int hlua_http_res_set_hdr(lua_State *L) | |||
MAY_LJMP(check_args(L, 3, "res_set_hdr")); | |||
htxn = MAY_LJMP(hlua_checkhttp(L, 1)); | |||
+ if (htxn->dir != SMP_OPT_DIR_RES) | |||
+ WILL_LJMP(lua_error(L)); | |||
+ | |||
hlua_http_del_hdr(L, htxn, &htxn->s->txn->rsp); | |||
return hlua_http_add_hdr(L, htxn, &htxn->s->txn->rsp); | |||
} | |||
@@ -5565,6 +5601,9 @@ static int hlua_http_req_set_meth(lua_State *L) | |||
size_t name_len; | |||
const char *name = MAY_LJMP(luaL_checklstring(L, 2, &name_len)); | |||
+ if (htxn->dir != SMP_OPT_DIR_REQ) | |||
+ WILL_LJMP(lua_error(L)); | |||
+ | |||
lua_pushboolean(L, http_replace_req_line(0, name, name_len, htxn->p, htxn->s) != -1); | |||
return 1; | |||
} | |||
@@ -5576,6 +5615,9 @@ static int hlua_http_req_set_path(lua_State *L) | |||
size_t name_len; | |||
const char *name = MAY_LJMP(luaL_checklstring(L, 2, &name_len)); | |||
+ if (htxn->dir != SMP_OPT_DIR_REQ) | |||
+ WILL_LJMP(lua_error(L)); | |||
+ | |||
lua_pushboolean(L, http_replace_req_line(1, name, name_len, htxn->p, htxn->s) != -1); | |||
return 1; | |||
} | |||
@@ -5587,6 +5629,9 @@ static int hlua_http_req_set_query(lua_State *L) | |||
size_t name_len; | |||
const char *name = MAY_LJMP(luaL_checklstring(L, 2, &name_len)); | |||
+ if (htxn->dir != SMP_OPT_DIR_REQ) | |||
+ WILL_LJMP(lua_error(L)); | |||
+ | |||
/* Check length. */ | |||
if (name_len > trash.size - 1) { | |||
lua_pushboolean(L, 0); | |||
@@ -5611,6 +5656,9 @@ static int hlua_http_req_set_uri(lua_State *L) | |||
size_t name_len; | |||
const char *name = MAY_LJMP(luaL_checklstring(L, 2, &name_len)); | |||
+ if (htxn->dir != SMP_OPT_DIR_REQ) | |||
+ WILL_LJMP(lua_error(L)); | |||
+ | |||
lua_pushboolean(L, http_replace_req_line(3, name, name_len, htxn->p, htxn->s) != -1); | |||
return 1; | |||
} | |||
@@ -5622,6 +5670,9 @@ static int hlua_http_res_set_status(lua_State *L) | |||
unsigned int code = MAY_LJMP(luaL_checkinteger(L, 2)); | |||
const char *reason = MAY_LJMP(luaL_optlstring(L, 3, NULL, NULL)); | |||
+ if (htxn->dir != SMP_OPT_DIR_RES) | |||
+ WILL_LJMP(lua_error(L)); | |||
+ | |||
http_set_status(code, reason, htxn->s); | |||
return 0; | |||
} |
@ -0,0 +1,52 @@ | |||
commit b10c8d7641cc8ceae6fba4506b7f987d66109bd9 | |||
Author: Willy Tarreau <w@1wt.eu> | |||
Date: Mon Aug 26 10:55:52 2019 +0200 | |||
BUG/MEDIUM: listener/threads: fix an AB/BA locking issue in delete_listener() | |||
The delete_listener() function takes the listener's lock before taking | |||
the proto_lock, which is contrary to what other functions do, possibly | |||
causing an AB/BA deadlock. In practice the two only places where both | |||
are taken are during protocol_enable_all() and delete_listener(), the | |||
former being used during startup and the latter during stop. In practice | |||
during reload floods, it is technically possible for a thread to be | |||
initializing the listeners while another one is stopping. While this | |||
is too hard to trigger on 2.0 and above due to the synchronization of | |||
all threads during startup, it's reasonably easy to do in 1.9 by having | |||
hundreds of listeners, starting 64 threads and flooding them with reloads | |||
like this : | |||
$ while usleep 50000; do killall -USR2 haproxy; done | |||
Usually in less than a minute, all threads will be deadlocked. The fix | |||
consists in always taking the proto_lock before the listener lock. It | |||
seems to be the only place where these two locks were reversed. This | |||
fix needs to be backported to 2.0, 1.9, and 1.8. | |||
(cherry picked from commit 6ee9f8df3bfbb811526cff3313da5758b1277bc6) | |||
Signed-off-by: Willy Tarreau <w@1wt.eu> | |||
diff --git a/src/listener.c b/src/listener.c | |||
index b5fe2ac2..54c09960 100644 | |||
--- a/src/listener.c | |||
+++ b/src/listener.c | |||
@@ -595,17 +595,17 @@ int create_listeners(struct bind_conf *bc, const struct sockaddr_storage *ss, | |||
*/ | |||
void delete_listener(struct listener *listener) | |||
{ | |||
+ HA_SPIN_LOCK(PROTO_LOCK, &proto_lock); | |||
HA_SPIN_LOCK(LISTENER_LOCK, &listener->lock); | |||
if (listener->state == LI_ASSIGNED) { | |||
listener->state = LI_INIT; | |||
- HA_SPIN_LOCK(PROTO_LOCK, &proto_lock); | |||
LIST_DEL(&listener->proto_list); | |||
listener->proto->nb_listeners--; | |||
- HA_SPIN_UNLOCK(PROTO_LOCK, &proto_lock); | |||
_HA_ATOMIC_SUB(&jobs, 1); | |||
_HA_ATOMIC_SUB(&listeners, 1); | |||
} | |||
HA_SPIN_UNLOCK(LISTENER_LOCK, &listener->lock); | |||
+ HA_SPIN_UNLOCK(PROTO_LOCK, &proto_lock); | |||
} | |||
/* Returns a suitable value for a listener's backlog. It uses the listener's, |
@ -0,0 +1,34 @@ | |||
commit 4db294bc0b7988607f2dfdb9d57974b2ba47cbc3 | |||
Author: Jerome Magnin <jmagnin@haproxy.com> | |||
Date: Mon Aug 26 11:44:21 2019 +0200 | |||
BUG/MEDIUM: url32 does not take the path part into account in the returned hash. | |||
The url32 sample fetch does not take the path part of the URL into | |||
account. This is because in smp_fetch_url32() we erroneously modify | |||
path.len and path.ptr before testing their value and building the | |||
path based part of the hash. | |||
This fixes issue #235 | |||
This must be backported as far as 1.9, when HTX was introduced. | |||
(cherry picked from commit 2dd26ca9ff8e642611b8b012d6aee45ea45196bc) | |||
[wt: adjusted context, we still have legacy in 2.0] | |||
Signed-off-by: Willy Tarreau <w@1wt.eu> | |||
diff --git a/src/http_fetch.c b/src/http_fetch.c | |||
index e372a122..6448bde9 100644 | |||
--- a/src/http_fetch.c | |||
+++ b/src/http_fetch.c | |||
@@ -2735,10 +2735,6 @@ static int smp_fetch_url32(const struct arg *args, struct sample *smp, const cha | |||
/* now retrieve the path */ | |||
sl = http_get_stline(htx); | |||
path = http_get_path(htx_sl_req_uri(sl)); | |||
- while (path.len > 0 && *(path.ptr) != '?') { | |||
- path.ptr++; | |||
- path.len--; | |||
- } | |||
if (path.len && *(path.ptr) == '/') { | |||
while (path.len--) | |||
hash = *(path.ptr++) + (hash << 6) + (hash << 16) - hash; |
@ -1,34 +0,0 @@ | |||
commit b22f6501bc9838061472128360e0e55d08cb0bd9 | |||
Author: Christopher Faulet <cfaulet@haproxy.com> | |||
Date: Fri Jul 26 14:54:52 2019 +0200 | |||
MINOR: hlua: Don't set request analyzers on response channel for lua actions | |||
Setting some requests analyzers on the response channel was an old trick to be | |||
sure to re-evaluate the request's analyers after the response's ones have been | |||
called. It is no more necessary. In fact, this trick was removed in the version | |||
1.8 and backported up to the version 1.6. | |||
This patch must be backported to all versions since 1.6 to ease the backports of | |||
fixes on the lua code. | |||
(cherry picked from commit 51fa358432247fe5d7259d9d8a0e08d49d429c73) | |||
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com> | |||
diff --git a/src/hlua.c b/src/hlua.c | |||
index 21351cd6..36454cdc 100644 | |||
--- a/src/hlua.c | |||
+++ b/src/hlua.c | |||
@@ -6873,11 +6873,8 @@ static enum act_return hlua_action(struct act_rule *rule, struct proxy *px, | |||
* is detected on a response channel. This is useful | |||
* only for actions targeted on the requests. | |||
*/ | |||
- if (HLUA_IS_WAKERESWR(s->hlua)) { | |||
+ if (HLUA_IS_WAKERESWR(s->hlua)) | |||
s->res.flags |= CF_WAKE_WRITE; | |||
- if ((analyzer & (AN_REQ_INSPECT_FE|AN_REQ_HTTP_PROCESS_FE))) | |||
- s->res.analysers |= analyzer; | |||
- } | |||
if (HLUA_IS_WAKEREQWR(s->hlua)) | |||
s->req.flags |= CF_WAKE_WRITE; | |||
/* We can quit the function without consistency check |
@ -1,110 +0,0 @@ | |||
commit ff96b8bd3f85155f65b2b9c9f046fe3e40f630a4 | |||
Author: Christopher Faulet <cfaulet@haproxy.com> | |||
Date: Fri Jul 26 15:09:53 2019 +0200 | |||
MINOR: hlua: Add a flag on the lua txn to know in which context it can be used | |||
When a lua action or a lua sample fetch is called, a lua transaction is | |||
created. It is an entry in the stack containing the class TXN. Thanks to it, we | |||
can know the direction (request or response) of the call. But, for some | |||
functions, it is also necessary to know if the buffer is "HTTP ready" for the | |||
given direction. "HTTP ready" means there is a valid HTTP message in the | |||
channel's buffer. So, when a lua action or a lua sample fetch is called, the | |||
flag HLUA_TXN_HTTP_RDY is set if it is appropriate. | |||
(cherry picked from commit bfab2dddad3ded87617d1e2db54761943d1eb32d) | |||
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com> | |||
diff --git a/include/types/hlua.h b/include/types/hlua.h | |||
index 70c76852..2f4e38be 100644 | |||
--- a/include/types/hlua.h | |||
+++ b/include/types/hlua.h | |||
@@ -43,7 +43,8 @@ struct stream; | |||
#define HLUA_F_AS_STRING 0x01 | |||
#define HLUA_F_MAY_USE_HTTP 0x02 | |||
-#define HLUA_TXN_NOTERM 0x00000001 | |||
+#define HLUA_TXN_NOTERM 0x00000001 | |||
+#define HLUA_TXN_HTTP_RDY 0x00000002 /* Set if the txn is HTTP ready for the defined direction */ | |||
#define HLUA_CONCAT_BLOCSZ 2048 | |||
diff --git a/src/hlua.c b/src/hlua.c | |||
index 36454cdc..d37e3c61 100644 | |||
--- a/src/hlua.c | |||
+++ b/src/hlua.c | |||
@@ -6494,6 +6494,7 @@ static int hlua_sample_fetch_wrapper(const struct arg *arg_p, struct sample *smp | |||
struct stream *stream = smp->strm; | |||
const char *error; | |||
const struct buffer msg = { }; | |||
+ unsigned int hflags = HLUA_TXN_NOTERM; | |||
if (!stream) | |||
return 0; | |||
@@ -6517,6 +6518,13 @@ static int hlua_sample_fetch_wrapper(const struct arg *arg_p, struct sample *smp | |||
consistency_set(stream, smp->opt, &stream->hlua->cons); | |||
+ if (stream->be->mode == PR_MODE_HTTP) { | |||
+ if ((smp->opt & SMP_OPT_DIR) == SMP_OPT_DIR_REQ) | |||
+ hflags |= ((stream->txn->req.msg_state < HTTP_MSG_BODY) ? 0 : HLUA_TXN_HTTP_RDY); | |||
+ else | |||
+ hflags |= ((stream->txn->rsp.msg_state < HTTP_MSG_BODY) ? 0 : HLUA_TXN_HTTP_RDY); | |||
+ } | |||
+ | |||
/* If it is the first run, initialize the data for the call. */ | |||
if (!HLUA_IS_RUNNING(stream->hlua)) { | |||
@@ -6541,8 +6549,7 @@ static int hlua_sample_fetch_wrapper(const struct arg *arg_p, struct sample *smp | |||
lua_rawgeti(stream->hlua->T, LUA_REGISTRYINDEX, fcn->function_ref); | |||
/* push arguments in the stack. */ | |||
- if (!hlua_txn_new(stream->hlua->T, stream, smp->px, smp->opt & SMP_OPT_DIR, | |||
- HLUA_TXN_NOTERM)) { | |||
+ if (!hlua_txn_new(stream->hlua->T, stream, smp->px, smp->opt & SMP_OPT_DIR, hflags)) { | |||
SEND_ERR(smp->px, "Lua sample-fetch '%s': full stack.\n", fcn->name); | |||
RESET_SAFE_LJMP(stream->hlua->T); | |||
return 0; | |||
@@ -6759,16 +6766,16 @@ static enum act_return hlua_action(struct act_rule *rule, struct proxy *px, | |||
struct session *sess, struct stream *s, int flags) | |||
{ | |||
char **arg; | |||
- unsigned int analyzer; | |||
+ unsigned int hflags = 0; | |||
int dir; | |||
const char *error; | |||
const struct buffer msg = { }; | |||
switch (rule->from) { | |||
- case ACT_F_TCP_REQ_CNT: analyzer = AN_REQ_INSPECT_FE ; dir = SMP_OPT_DIR_REQ; break; | |||
- case ACT_F_TCP_RES_CNT: analyzer = AN_RES_INSPECT ; dir = SMP_OPT_DIR_RES; break; | |||
- case ACT_F_HTTP_REQ: analyzer = AN_REQ_HTTP_PROCESS_FE; dir = SMP_OPT_DIR_REQ; break; | |||
- case ACT_F_HTTP_RES: analyzer = AN_RES_HTTP_PROCESS_BE; dir = SMP_OPT_DIR_RES; break; | |||
+ case ACT_F_TCP_REQ_CNT: ; dir = SMP_OPT_DIR_REQ; break; | |||
+ case ACT_F_TCP_RES_CNT: ; dir = SMP_OPT_DIR_RES; break; | |||
+ case ACT_F_HTTP_REQ: hflags = HLUA_TXN_HTTP_RDY ; dir = SMP_OPT_DIR_REQ; break; | |||
+ case ACT_F_HTTP_RES: hflags = HLUA_TXN_HTTP_RDY ; dir = SMP_OPT_DIR_RES; break; | |||
default: | |||
SEND_ERR(px, "Lua: internal error while execute action.\n"); | |||
return ACT_RET_CONT; | |||
@@ -6821,7 +6828,7 @@ static enum act_return hlua_action(struct act_rule *rule, struct proxy *px, | |||
lua_rawgeti(s->hlua->T, LUA_REGISTRYINDEX, rule->arg.hlua_rule->fcn.function_ref); | |||
/* Create and and push object stream in the stack. */ | |||
- if (!hlua_txn_new(s->hlua->T, s, px, dir, 0)) { | |||
+ if (!hlua_txn_new(s->hlua->T, s, px, dir, hflags)) { | |||
SEND_ERR(px, "Lua function '%s': full stack.\n", | |||
rule->arg.hlua_rule->fcn.name); | |||
RESET_SAFE_LJMP(s->hlua->T); | |||
@@ -6864,9 +6871,9 @@ static enum act_return hlua_action(struct act_rule *rule, struct proxy *px, | |||
case HLUA_E_AGAIN: | |||
/* Set timeout in the required channel. */ | |||
if (s->hlua->wake_time != TICK_ETERNITY) { | |||
- if (analyzer & (AN_REQ_INSPECT_FE|AN_REQ_HTTP_PROCESS_FE)) | |||
+ if (dir & SMP_OPT_DIR_REQ) | |||
s->req.analyse_exp = s->hlua->wake_time; | |||
- else if (analyzer & (AN_RES_INSPECT|AN_RES_HTTP_PROCESS_BE)) | |||
+ else | |||
s->res.analyse_exp = s->hlua->wake_time; | |||
} | |||
/* Some actions can be wake up when a "write" event |
@ -1,180 +0,0 @@ | |||
commit 2351ca211d655c1be9ef6d62880899102134266d | |||
Author: Christopher Faulet <cfaulet@haproxy.com> | |||
Date: Fri Jul 26 16:31:34 2019 +0200 | |||
BUG/MINOR: hlua: Only execute functions of HTTP class if the txn is HTTP ready | |||
The flag HLUA_TXN_HTTP_RDY was added in the previous commit to know when a | |||
function is called for a channel with a valid HTTP message or not. Of course it | |||
also depends on the calling direction. In this commit, we allow the execution of | |||
functions of the HTTP class only if this flag is set. | |||
Nobody seems to use them from an unsupported context (for instance, trying to | |||
set an HTTP header from a tcp-request rule). But it remains a bug leading to | |||
undefined behaviors or crashes. | |||
This patch may be backported to all versions since the 1.6. It depends on the | |||
commits "MINOR: hlua: Add a flag on the lua txn to know in which context it can | |||
be used" and "MINOR: hlua: Don't set request analyzers on response channel for | |||
lua actions". | |||
(cherry picked from commit 301eff8e215d5dc7130e1ebacd7cf8da09a4f643) | |||
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com> | |||
diff --git a/src/hlua.c b/src/hlua.c | |||
index d37e3c61..4d92fa44 100644 | |||
--- a/src/hlua.c | |||
+++ b/src/hlua.c | |||
@@ -5346,7 +5346,7 @@ __LJMP static int hlua_http_req_get_headers(lua_State *L) | |||
MAY_LJMP(check_args(L, 1, "req_get_headers")); | |||
htxn = MAY_LJMP(hlua_checkhttp(L, 1)); | |||
- if (htxn->dir != SMP_OPT_DIR_REQ) | |||
+ if (htxn->dir != SMP_OPT_DIR_REQ || !(htxn->flags & HLUA_TXN_HTTP_RDY)) | |||
WILL_LJMP(lua_error(L)); | |||
return hlua_http_get_headers(L, htxn, &htxn->s->txn->req); | |||
@@ -5359,7 +5359,7 @@ __LJMP static int hlua_http_res_get_headers(lua_State *L) | |||
MAY_LJMP(check_args(L, 1, "res_get_headers")); | |||
htxn = MAY_LJMP(hlua_checkhttp(L, 1)); | |||
- if (htxn->dir != SMP_OPT_DIR_RES) | |||
+ if (htxn->dir != SMP_OPT_DIR_RES || !(htxn->flags & HLUA_TXN_HTTP_RDY)) | |||
WILL_LJMP(lua_error(L)); | |||
return hlua_http_get_headers(L, htxn, &htxn->s->txn->rsp); | |||
@@ -5399,7 +5399,7 @@ __LJMP static int hlua_http_req_rep_hdr(lua_State *L) | |||
MAY_LJMP(check_args(L, 4, "req_rep_hdr")); | |||
htxn = MAY_LJMP(hlua_checkhttp(L, 1)); | |||
- if (htxn->dir != SMP_OPT_DIR_REQ) | |||
+ if (htxn->dir != SMP_OPT_DIR_REQ || !(htxn->flags & HLUA_TXN_HTTP_RDY)) | |||
WILL_LJMP(lua_error(L)); | |||
return MAY_LJMP(hlua_http_rep_hdr(L, htxn, &htxn->s->txn->req, ACT_HTTP_REPLACE_HDR)); | |||
@@ -5412,7 +5412,7 @@ __LJMP static int hlua_http_res_rep_hdr(lua_State *L) | |||
MAY_LJMP(check_args(L, 4, "res_rep_hdr")); | |||
htxn = MAY_LJMP(hlua_checkhttp(L, 1)); | |||
- if (htxn->dir != SMP_OPT_DIR_RES) | |||
+ if (htxn->dir != SMP_OPT_DIR_RES || !(htxn->flags & HLUA_TXN_HTTP_RDY)) | |||
WILL_LJMP(lua_error(L)); | |||
return MAY_LJMP(hlua_http_rep_hdr(L, htxn, &htxn->s->txn->rsp, ACT_HTTP_REPLACE_HDR)); | |||
@@ -5425,7 +5425,7 @@ __LJMP static int hlua_http_req_rep_val(lua_State *L) | |||
MAY_LJMP(check_args(L, 4, "req_rep_hdr")); | |||
htxn = MAY_LJMP(hlua_checkhttp(L, 1)); | |||
- if (htxn->dir != SMP_OPT_DIR_REQ) | |||
+ if (htxn->dir != SMP_OPT_DIR_REQ || !(htxn->flags & HLUA_TXN_HTTP_RDY)) | |||
WILL_LJMP(lua_error(L)); | |||
return MAY_LJMP(hlua_http_rep_hdr(L, htxn, &htxn->s->txn->req, ACT_HTTP_REPLACE_VAL)); | |||
@@ -5438,7 +5438,7 @@ __LJMP static int hlua_http_res_rep_val(lua_State *L) | |||
MAY_LJMP(check_args(L, 4, "res_rep_val")); | |||
htxn = MAY_LJMP(hlua_checkhttp(L, 1)); | |||
- if (htxn->dir != SMP_OPT_DIR_RES) | |||
+ if (htxn->dir != SMP_OPT_DIR_RES || !(htxn->flags & HLUA_TXN_HTTP_RDY)) | |||
WILL_LJMP(lua_error(L)); | |||
return MAY_LJMP(hlua_http_rep_hdr(L, htxn, &htxn->s->txn->rsp, ACT_HTTP_REPLACE_VAL)); | |||
@@ -5480,7 +5480,7 @@ __LJMP static int hlua_http_req_del_hdr(lua_State *L) | |||
MAY_LJMP(check_args(L, 2, "req_del_hdr")); | |||
htxn = MAY_LJMP(hlua_checkhttp(L, 1)); | |||
- if (htxn->dir != SMP_OPT_DIR_REQ) | |||
+ if (htxn->dir != SMP_OPT_DIR_REQ || !(htxn->flags & HLUA_TXN_HTTP_RDY)) | |||
WILL_LJMP(lua_error(L)); | |||
return hlua_http_del_hdr(L, htxn, &htxn->s->txn->req); | |||
@@ -5493,7 +5493,7 @@ __LJMP static int hlua_http_res_del_hdr(lua_State *L) | |||
MAY_LJMP(check_args(L, 2, "res_del_hdr")); | |||
htxn = MAY_LJMP(hlua_checkhttp(L, 1)); | |||
- if (htxn->dir != SMP_OPT_DIR_RES) | |||
+ if (htxn->dir != SMP_OPT_DIR_RES || !(htxn->flags & HLUA_TXN_HTTP_RDY)) | |||
WILL_LJMP(lua_error(L)); | |||
return hlua_http_del_hdr(L, htxn, &htxn->s->txn->rsp); | |||
@@ -5547,7 +5547,7 @@ __LJMP static int hlua_http_req_add_hdr(lua_State *L) | |||
MAY_LJMP(check_args(L, 3, "req_add_hdr")); | |||
htxn = MAY_LJMP(hlua_checkhttp(L, 1)); | |||
- if (htxn->dir != SMP_OPT_DIR_REQ) | |||
+ if (htxn->dir != SMP_OPT_DIR_REQ || !(htxn->flags & HLUA_TXN_HTTP_RDY)) | |||
WILL_LJMP(lua_error(L)); | |||
return hlua_http_add_hdr(L, htxn, &htxn->s->txn->req); | |||
@@ -5560,7 +5560,7 @@ __LJMP static int hlua_http_res_add_hdr(lua_State *L) | |||
MAY_LJMP(check_args(L, 3, "res_add_hdr")); | |||
htxn = MAY_LJMP(hlua_checkhttp(L, 1)); | |||
- if (htxn->dir != SMP_OPT_DIR_RES) | |||
+ if (htxn->dir != SMP_OPT_DIR_RES || !(htxn->flags & HLUA_TXN_HTTP_RDY)) | |||
WILL_LJMP(lua_error(L)); | |||
return hlua_http_add_hdr(L, htxn, &htxn->s->txn->rsp); | |||
@@ -5573,7 +5573,7 @@ static int hlua_http_req_set_hdr(lua_State *L) | |||
MAY_LJMP(check_args(L, 3, "req_set_hdr")); | |||
htxn = MAY_LJMP(hlua_checkhttp(L, 1)); | |||
- if (htxn->dir != SMP_OPT_DIR_REQ) | |||
+ if (htxn->dir != SMP_OPT_DIR_REQ || !(htxn->flags & HLUA_TXN_HTTP_RDY)) | |||
WILL_LJMP(lua_error(L)); | |||
hlua_http_del_hdr(L, htxn, &htxn->s->txn->req); | |||
@@ -5587,7 +5587,7 @@ static int hlua_http_res_set_hdr(lua_State *L) | |||
MAY_LJMP(check_args(L, 3, "res_set_hdr")); | |||
htxn = MAY_LJMP(hlua_checkhttp(L, 1)); | |||
- if (htxn->dir != SMP_OPT_DIR_RES) | |||
+ if (htxn->dir != SMP_OPT_DIR_RES || !(htxn->flags & HLUA_TXN_HTTP_RDY)) | |||
WILL_LJMP(lua_error(L)); | |||
hlua_http_del_hdr(L, htxn, &htxn->s->txn->rsp); | |||
@@ -5601,7 +5601,7 @@ static int hlua_http_req_set_meth(lua_State *L) | |||
size_t name_len; | |||
const char *name = MAY_LJMP(luaL_checklstring(L, 2, &name_len)); | |||
- if (htxn->dir != SMP_OPT_DIR_REQ) | |||
+ if (htxn->dir != SMP_OPT_DIR_REQ || !(htxn->flags & HLUA_TXN_HTTP_RDY)) | |||
WILL_LJMP(lua_error(L)); | |||
lua_pushboolean(L, http_replace_req_line(0, name, name_len, htxn->p, htxn->s) != -1); | |||
@@ -5615,7 +5615,7 @@ static int hlua_http_req_set_path(lua_State *L) | |||
size_t name_len; | |||
const char *name = MAY_LJMP(luaL_checklstring(L, 2, &name_len)); | |||
- if (htxn->dir != SMP_OPT_DIR_REQ) | |||
+ if (htxn->dir != SMP_OPT_DIR_REQ || !(htxn->flags & HLUA_TXN_HTTP_RDY)) | |||
WILL_LJMP(lua_error(L)); | |||
lua_pushboolean(L, http_replace_req_line(1, name, name_len, htxn->p, htxn->s) != -1); | |||
@@ -5629,7 +5629,7 @@ static int hlua_http_req_set_query(lua_State *L) | |||
size_t name_len; | |||
const char *name = MAY_LJMP(luaL_checklstring(L, 2, &name_len)); | |||
- if (htxn->dir != SMP_OPT_DIR_REQ) | |||
+ if (htxn->dir != SMP_OPT_DIR_REQ || !(htxn->flags & HLUA_TXN_HTTP_RDY)) | |||
WILL_LJMP(lua_error(L)); | |||
/* Check length. */ | |||
@@ -5656,7 +5656,7 @@ static int hlua_http_req_set_uri(lua_State *L) | |||
size_t name_len; | |||
const char *name = MAY_LJMP(luaL_checklstring(L, 2, &name_len)); | |||
- if (htxn->dir != SMP_OPT_DIR_REQ) | |||
+ if (htxn->dir != SMP_OPT_DIR_REQ || !(htxn->flags & HLUA_TXN_HTTP_RDY)) | |||
WILL_LJMP(lua_error(L)); | |||
lua_pushboolean(L, http_replace_req_line(3, name, name_len, htxn->p, htxn->s) != -1); | |||
@@ -5670,7 +5670,7 @@ static int hlua_http_res_set_status(lua_State *L) | |||
unsigned int code = MAY_LJMP(luaL_checkinteger(L, 2)); | |||
const char *reason = MAY_LJMP(luaL_optlstring(L, 3, NULL, NULL)); | |||
- if (htxn->dir != SMP_OPT_DIR_RES) | |||
+ if (htxn->dir != SMP_OPT_DIR_RES || !(htxn->flags & HLUA_TXN_HTTP_RDY)) | |||
WILL_LJMP(lua_error(L)); | |||
http_set_status(code, reason, htxn->s); |
@ -1,37 +0,0 @@ | |||
commit 3cd7a1ea5110fc6a92627aaad06553a49723ac92 | |||
Author: Christopher Faulet <cfaulet@haproxy.com> | |||
Date: Mon Jul 29 10:50:28 2019 +0200 | |||
BUG/MINOR: htx: Fix free space addresses calculation during a block expansion | |||
When the payload of a block is shrinked or enlarged, addresses of the free | |||
spaces must be updated. There are many possible cases. One of them is | |||
buggy. When there is only one block in the HTX message and its payload is just | |||
before the tail room and it needs to be moved in the head room to be enlarged, | |||
addresses are not correctly updated. This bug may be hit by the compression | |||
filter. | |||
This patch must be backported to 2.0. | |||
(cherry picked from commit 61ed7797f6440ee1102576365553650b1982a233) | |||
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com> | |||
diff --git a/src/htx.c b/src/htx.c | |||
index c29a66d7..cd21050c 100644 | |||
--- a/src/htx.c | |||
+++ b/src/htx.c | |||
@@ -252,11 +252,13 @@ static int htx_prepare_blk_expansion(struct htx *htx, struct htx_blk *blk, int32 | |||
ret = 1; | |||
} | |||
else if ((sz + delta) < headroom) { | |||
+ uint32_t oldaddr = blk->addr; | |||
+ | |||
/* Move the block's payload into the headroom */ | |||
blk->addr = htx->head_addr; | |||
htx->tail_addr -= sz; | |||
htx->head_addr += sz + delta; | |||
- if (blk->addr == htx->end_addr) { | |||
+ if (oldaddr == htx->end_addr) { | |||
if (htx->end_addr == htx->tail_addr) { | |||
htx->tail_addr = htx->head_addr; | |||
htx->head_addr = htx->end_addr = 0; |
@ -1,225 +0,0 @@ | |||
commit 0ff395c154ad827c0c30eefc9371ba7f7c171027 | |||
Author: Willy Tarreau <w@1wt.eu> | |||
Date: Tue Jul 30 11:59:34 2019 +0200 | |||
BUG/MAJOR: queue/threads: avoid an AB/BA locking issue in process_srv_queue() | |||
A problem involving server slowstart was reported by @max2k1 in issue #197. | |||
The problem is that pendconn_grab_from_px() takes the proxy lock while | |||
already under the server's lock while process_srv_queue() first takes the | |||
proxy's lock then the server's lock. | |||
While the latter seems more natural, it is fundamentally incompatible with | |||
mayn other operations performed on servers, namely state change propagation, | |||
where the proxy is only known after the server and cannot be locked around | |||
the servers. Howwever reversing the lock in process_srv_queue() is trivial | |||
and only the few functions related to dynamic cookies need to be adjusted | |||
for this so that the proxy's lock is taken for each server operation. This | |||
is possible because the proxy's server list is built once at boot time and | |||
remains stable. So this is what this patch does. | |||
The comments in the proxy and server structs were updated to mention this | |||
rule that the server's lock may not be taken under the proxy's lock but | |||
may enclose it. | |||
Another approach could consist in using a second lock for the proxy's queue | |||
which would be different from the regular proxy's lock, but given that the | |||
operations above are rare and operate on small servers list, there is no | |||
reason for overdesigning a solution. | |||
This fix was successfully tested with 10000 servers in a backend where | |||
adjusting the dyncookies in loops over the CLI didn't have a measurable | |||
impact on the traffic. | |||
The only workaround without the fix is to disable any occurrence of | |||
"slowstart" on server lines, or to disable threads using "nbthread 1". | |||
This must be backported as far as 1.8. | |||
(cherry picked from commit 5e83d996cf965ee5ac625f702a446f4d8c80a220) | |||
Signed-off-by: Willy Tarreau <w@1wt.eu> | |||
diff --git a/include/types/proxy.h b/include/types/proxy.h | |||
index ca24dbfe..2518f88d 100644 | |||
--- a/include/types/proxy.h | |||
+++ b/include/types/proxy.h | |||
@@ -487,7 +487,7 @@ struct proxy { | |||
* name is used | |||
*/ | |||
struct list filter_configs; /* list of the filters that are declared on this proxy */ | |||
- __decl_hathreads(HA_SPINLOCK_T lock); | |||
+ __decl_hathreads(HA_SPINLOCK_T lock); /* may be taken under the server's lock */ | |||
}; | |||
struct switching_rule { | |||
diff --git a/include/types/server.h b/include/types/server.h | |||
index 4a077268..e0534162 100644 | |||
--- a/include/types/server.h | |||
+++ b/include/types/server.h | |||
@@ -319,7 +319,7 @@ struct server { | |||
} ssl_ctx; | |||
#endif | |||
struct dns_srvrq *srvrq; /* Pointer representing the DNS SRV requeest, if any */ | |||
- __decl_hathreads(HA_SPINLOCK_T lock); | |||
+ __decl_hathreads(HA_SPINLOCK_T lock); /* may enclose the proxy's lock, must not be taken under */ | |||
struct { | |||
const char *file; /* file where the section appears */ | |||
struct eb32_node id; /* place in the tree of used IDs */ | |||
diff --git a/src/proxy.c b/src/proxy.c | |||
index ae761ead..a537e0b1 100644 | |||
--- a/src/proxy.c | |||
+++ b/src/proxy.c | |||
@@ -1940,9 +1940,12 @@ static int cli_parse_enable_dyncookie_backend(char **args, char *payload, struct | |||
if (!px) | |||
return 1; | |||
+ /* Note: this lock is to make sure this doesn't change while another | |||
+ * thread is in srv_set_dyncookie(). | |||
+ */ | |||
HA_SPIN_LOCK(PROXY_LOCK, &px->lock); | |||
- | |||
px->ck_opts |= PR_CK_DYNAMIC; | |||
+ HA_SPIN_UNLOCK(PROXY_LOCK, &px->lock); | |||
for (s = px->srv; s != NULL; s = s->next) { | |||
HA_SPIN_LOCK(SERVER_LOCK, &s->lock); | |||
@@ -1950,8 +1953,6 @@ static int cli_parse_enable_dyncookie_backend(char **args, char *payload, struct | |||
HA_SPIN_UNLOCK(SERVER_LOCK, &s->lock); | |||
} | |||
- HA_SPIN_UNLOCK(PROXY_LOCK, &px->lock); | |||
- | |||
return 1; | |||
} | |||
@@ -1971,9 +1972,12 @@ static int cli_parse_disable_dyncookie_backend(char **args, char *payload, struc | |||
if (!px) | |||
return 1; | |||
+ /* Note: this lock is to make sure this doesn't change while another | |||
+ * thread is in srv_set_dyncookie(). | |||
+ */ | |||
HA_SPIN_LOCK(PROXY_LOCK, &px->lock); | |||
- | |||
px->ck_opts &= ~PR_CK_DYNAMIC; | |||
+ HA_SPIN_UNLOCK(PROXY_LOCK, &px->lock); | |||
for (s = px->srv; s != NULL; s = s->next) { | |||
HA_SPIN_LOCK(SERVER_LOCK, &s->lock); | |||
@@ -1984,8 +1988,6 @@ static int cli_parse_disable_dyncookie_backend(char **args, char *payload, struc | |||
HA_SPIN_UNLOCK(SERVER_LOCK, &s->lock); | |||
} | |||
- HA_SPIN_UNLOCK(PROXY_LOCK, &px->lock); | |||
- | |||
return 1; | |||
} | |||
@@ -2021,10 +2023,13 @@ static int cli_parse_set_dyncookie_key_backend(char **args, char *payload, struc | |||
return 1; | |||
} | |||
+ /* Note: this lock is to make sure this doesn't change while another | |||
+ * thread is in srv_set_dyncookie(). | |||
+ */ | |||
HA_SPIN_LOCK(PROXY_LOCK, &px->lock); | |||
- | |||
free(px->dyncookie_key); | |||
px->dyncookie_key = newkey; | |||
+ HA_SPIN_UNLOCK(PROXY_LOCK, &px->lock); | |||
for (s = px->srv; s != NULL; s = s->next) { | |||
HA_SPIN_LOCK(SERVER_LOCK, &s->lock); | |||
@@ -2032,8 +2037,6 @@ static int cli_parse_set_dyncookie_key_backend(char **args, char *payload, struc | |||
HA_SPIN_UNLOCK(SERVER_LOCK, &s->lock); | |||
} | |||
- HA_SPIN_UNLOCK(PROXY_LOCK, &px->lock); | |||
- | |||
return 1; | |||
} | |||
diff --git a/src/queue.c b/src/queue.c | |||
index f4a94530..6aa54170 100644 | |||
--- a/src/queue.c | |||
+++ b/src/queue.c | |||
@@ -312,16 +312,16 @@ void process_srv_queue(struct server *s) | |||
struct proxy *p = s->proxy; | |||
int maxconn; | |||
- HA_SPIN_LOCK(PROXY_LOCK, &p->lock); | |||
HA_SPIN_LOCK(SERVER_LOCK, &s->lock); | |||
+ HA_SPIN_LOCK(PROXY_LOCK, &p->lock); | |||
maxconn = srv_dynamic_maxconn(s); | |||
while (s->served < maxconn) { | |||
int ret = pendconn_process_next_strm(s, p); | |||
if (!ret) | |||
break; | |||
} | |||
- HA_SPIN_UNLOCK(SERVER_LOCK, &s->lock); | |||
HA_SPIN_UNLOCK(PROXY_LOCK, &p->lock); | |||
+ HA_SPIN_UNLOCK(SERVER_LOCK, &s->lock); | |||
} | |||
/* Adds the stream <strm> to the pending connection queue of server <strm>->srv | |||
@@ -424,7 +424,8 @@ int pendconn_redistribute(struct server *s) | |||
/* Check for pending connections at the backend, and assign some of them to | |||
* the server coming up. The server's weight is checked before being assigned | |||
* connections it may not be able to handle. The total number of transferred | |||
- * connections is returned. | |||
+ * connections is returned. It must be called with the server lock held, and | |||
+ * will take the proxy's lock. | |||
*/ | |||
int pendconn_grab_from_px(struct server *s) | |||
{ | |||
diff --git a/src/server.c b/src/server.c | |||
index a96f1ef6..236d6bae 100644 | |||
--- a/src/server.c | |||
+++ b/src/server.c | |||
@@ -125,7 +125,7 @@ static inline void srv_check_for_dup_dyncookie(struct server *s) | |||
} | |||
/* | |||
- * Must be called with the server lock held. | |||
+ * Must be called with the server lock held, and will grab the proxy lock. | |||
*/ | |||
void srv_set_dyncookie(struct server *s) | |||
{ | |||
@@ -137,15 +137,17 @@ void srv_set_dyncookie(struct server *s) | |||
int addr_len; | |||
int port; | |||
+ HA_SPIN_LOCK(PROXY_LOCK, &p->lock); | |||
+ | |||
if ((s->flags & SRV_F_COOKIESET) || | |||
!(s->proxy->ck_opts & PR_CK_DYNAMIC) || | |||
s->proxy->dyncookie_key == NULL) | |||
- return; | |||
+ goto out; | |||
key_len = strlen(p->dyncookie_key); | |||
if (s->addr.ss_family != AF_INET && | |||
s->addr.ss_family != AF_INET6) | |||
- return; | |||
+ goto out; | |||
/* | |||
* Buffer to calculate the cookie value. | |||
* The buffer contains the secret key + the server IP address | |||
@@ -174,7 +176,7 @@ void srv_set_dyncookie(struct server *s) | |||
hash_value = XXH64(tmpbuf, buffer_len, 0); | |||
memprintf(&s->cookie, "%016llx", hash_value); | |||
if (!s->cookie) | |||
- return; | |||
+ goto out; | |||
s->cklen = 16; | |||
/* Don't bother checking if the dyncookie is duplicated if | |||
@@ -183,6 +185,8 @@ void srv_set_dyncookie(struct server *s) | |||
*/ | |||
if (!(s->next_admin & SRV_ADMF_FMAINT)) | |||
srv_check_for_dup_dyncookie(s); | |||
+ out: | |||
+ HA_SPIN_UNLOCK(PROXY_LOCK, &p->lock); | |||
} | |||
/* |
@ -1,71 +0,0 @@ | |||
commit da767eaaf6128eccd349a54ec6eac2a68dcacacb | |||
Author: Willy Tarreau <w@1wt.eu> | |||
Date: Wed Jul 31 19:15:45 2019 +0200 | |||
BUG/MINOR: debug: fix a small race in the thread dumping code | |||
If a thread dump is requested from a signal handler, it may interrupt | |||
a thread already waiting for a dump to complete, and may see the | |||
threads_to_dump variable go to zero while others are waiting, steal | |||
the lock and prevent other threads from ever completing. This tends | |||
to happen when dumping many threads upon a watchdog timeout, to threads | |||
waiting for their turn. | |||
Instead now we proceed in two steps : | |||
1) the last dumped thread sets all bits again | |||
2) all threads only wait for their own bit to appear, then clear it | |||
and quit | |||
This way there's no risk that a bit performs a double flip in the same | |||
loop and threads cannot get stuck here anymore. | |||
This should be backported to 2.0 as it clarifies stack traces. | |||
(cherry picked from commit c07736209db764fb2aef6f18ed3687a504c35771) | |||
Signed-off-by: Willy Tarreau <w@1wt.eu> | |||
diff --git a/src/debug.c b/src/debug.c | |||
index 059bc6b9..07624ca5 100644 | |||
--- a/src/debug.c | |||
+++ b/src/debug.c | |||
@@ -440,8 +440,8 @@ void debug_handler(int sig, siginfo_t *si, void *arg) | |||
* 1- wait for our turn, i.e. when all lower bits are gone. | |||
* 2- perform the action if our bit is set | |||
* 3- remove our bit to let the next one go, unless we're | |||
- * the last one and have to put them all but ours | |||
- * 4- wait for zero and clear our bit if it's set | |||
+ * the last one and have to put them all as a signal | |||
+ * 4- wait out bit to re-appear, then clear it and quit. | |||
*/ | |||
/* wait for all previous threads to finish first */ | |||
@@ -454,7 +454,7 @@ void debug_handler(int sig, siginfo_t *si, void *arg) | |||
ha_thread_dump(thread_dump_buffer, tid, thread_dump_tid); | |||
if ((threads_to_dump & all_threads_mask) == tid_bit) { | |||
/* last one */ | |||
- HA_ATOMIC_STORE(&threads_to_dump, all_threads_mask & ~tid_bit); | |||
+ HA_ATOMIC_STORE(&threads_to_dump, all_threads_mask); | |||
thread_dump_buffer = NULL; | |||
} | |||
else | |||
@@ -462,14 +462,13 @@ void debug_handler(int sig, siginfo_t *si, void *arg) | |||
} | |||
/* now wait for all others to finish dumping. The last one will set all | |||
- * bits again to broadcast the leaving condition. | |||
+ * bits again to broadcast the leaving condition so we'll see ourselves | |||
+ * present again. This way the threads_to_dump variable never passes to | |||
+ * zero until all visitors have stopped waiting. | |||
*/ | |||
- while (threads_to_dump & all_threads_mask) { | |||
- if (threads_to_dump & tid_bit) | |||
- HA_ATOMIC_AND(&threads_to_dump, ~tid_bit); | |||
- else | |||
- ha_thread_relax(); | |||
- } | |||
+ while (!(threads_to_dump & tid_bit)) | |||
+ ha_thread_relax(); | |||
+ HA_ATOMIC_AND(&threads_to_dump, ~tid_bit); | |||
/* mark the current thread as stuck to detect it upon next invocation | |||
* if it didn't move. |
@ -1,70 +0,0 @@ | |||
commit 445b2b7c52a13678241a190c4ff52e77a09ef0a6 | |||
Author: Willy Tarreau <w@1wt.eu> | |||
Date: Wed Jul 31 19:20:39 2019 +0200 | |||
MINOR: wdt: also consider that waiting in the thread dumper is normal | |||
It happens that upon looping threads the watchdog fires, starts a dump, | |||
and other threads expire their budget while waiting for the other threads | |||
to get dumped and trigger a watchdog event again, adding some confusion | |||
to the traces. With this patch the situation becomes clearer as we export | |||
the list of threads being dumped so that the watchdog can check it before | |||
deciding to trigger. This way such threads in queue for being dumped are | |||
not attempted to be reported in turn. | |||
This should be backported to 2.0 as it helps understand stack traces. | |||
(cherry picked from commit a37cb1880c81b1f038e575d88ba7210aea0b7b8f) | |||
Signed-off-by: Willy Tarreau <w@1wt.eu> | |||
diff --git a/include/common/debug.h b/include/common/debug.h | |||
index 333203dd..f43258e9 100644 | |||
--- a/include/common/debug.h | |||
+++ b/include/common/debug.h | |||
@@ -70,6 +70,7 @@ | |||
struct task; | |||
struct buffer; | |||
+extern volatile unsigned long threads_to_dump; | |||
void ha_task_dump(struct buffer *buf, const struct task *task, const char *pfx); | |||
void ha_thread_dump(struct buffer *buf, int thr, int calling_tid); | |||
void ha_thread_dump_all_to_trash(); | |||
diff --git a/src/debug.c b/src/debug.c | |||
index 07624ca5..3077e97c 100644 | |||
--- a/src/debug.c | |||
+++ b/src/debug.c | |||
@@ -29,6 +29,11 @@ | |||
#include <proto/stream_interface.h> | |||
#include <proto/task.h> | |||
+/* mask of threads still having to dump, used to respect ordering. Only used | |||
+ * when USE_THREAD_DUMP is set. | |||
+ */ | |||
+volatile unsigned long threads_to_dump = 0; | |||
+ | |||
/* Dumps to the buffer some known information for the desired thread, and | |||
* optionally extra info for the current thread. The dump will be appended to | |||
* the buffer, so the caller is responsible for preliminary initializing it. | |||
@@ -405,9 +410,6 @@ void ha_thread_dump_all_to_trash() | |||
*/ | |||
#define DEBUGSIG SIGURG | |||
-/* mask of threads still having to dump, used to respect ordering */ | |||
-static volatile unsigned long threads_to_dump; | |||
- | |||
/* ID of the thread requesting the dump */ | |||
static unsigned int thread_dump_tid; | |||
diff --git a/src/wdt.c b/src/wdt.c | |||
index 19d36c34..aa89fd44 100644 | |||
--- a/src/wdt.c | |||
+++ b/src/wdt.c | |||
@@ -75,7 +75,7 @@ void wdt_handler(int sig, siginfo_t *si, void *arg) | |||
if (n - p < 1000000000UL) | |||
goto update_and_leave; | |||
- if ((threads_harmless_mask|sleeping_thread_mask) & (1UL << thr)) { | |||
+ if ((threads_harmless_mask|sleeping_thread_mask|threads_to_dump) & (1UL << thr)) { | |||
/* This thread is currently doing exactly nothing | |||
* waiting in the poll loop (unlikely but possible), | |||
* waiting for all other threads to join the rendez-vous |
@ -1,56 +0,0 @@ | |||
commit 0fc2d46fabb2b9317daf7030162e828c7e1684d5 | |||
Author: Christopher Faulet <cfaulet@haproxy.com> | |||
Date: Thu Aug 1 10:09:29 2019 +0200 | |||
BUG/MEDIUM: lb-chash: Ensure the tree integrity when server weight is increased | |||
When the server weight is increased in consistant hash, extra nodes have to be | |||
allocated. So a realloc() is performed on the nodes array of the server. the | |||
previous commit 962ea7732 ("BUG/MEDIUM: lb-chash: Remove all server's entries | |||
before realloc() to re-insert them after") have fixed the size used during the | |||
realloc() to avoid segfaults. But another bug remains. After the realloc(), the | |||
memory area allocated for the nodes array may change, invalidating all node | |||
addresses in the chash tree. | |||
So, to fix the bug, we must remove all server's entries from the chash tree | |||
before the realloc to insert all of them after, old nodes and new ones. The | |||
insert will be automatically handled by the loop at the end of the function | |||
chash_queue_dequeue_srv(). | |||
Note that if the call to realloc() failed, no new entries will be created for | |||
the server, so the effective server weight will be unchanged. | |||
This issue was reported on Github (#189). | |||
This patch must be backported to all versions since the 1.6. | |||
(cherry picked from commit 0a52c17f819a5b0a17718b605bdd990b9e2b58e6) | |||
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com> | |||
diff --git a/src/lb_chash.c b/src/lb_chash.c | |||
index 0bf4e81a..23448df8 100644 | |||
--- a/src/lb_chash.c | |||
+++ b/src/lb_chash.c | |||
@@ -84,8 +84,13 @@ static inline void chash_queue_dequeue_srv(struct server *s) | |||
* increased the weight beyond the original weight | |||
*/ | |||
if (s->lb_nodes_tot < s->next_eweight) { | |||
- struct tree_occ *new_nodes = realloc(s->lb_nodes, s->next_eweight * sizeof(*new_nodes)); | |||
+ struct tree_occ *new_nodes; | |||
+ /* First we need to remove all server's entries from its tree | |||
+ * because the realloc will change all nodes pointers */ | |||
+ chash_dequeue_srv(s); | |||
+ | |||
+ new_nodes = realloc(s->lb_nodes, s->next_eweight * sizeof(*new_nodes)); | |||
if (new_nodes) { | |||
unsigned int j; | |||
@@ -494,7 +499,6 @@ void chash_init_server_tree(struct proxy *p) | |||
srv->lb_nodes_tot = srv->uweight * BE_WEIGHT_SCALE; | |||
srv->lb_nodes_now = 0; | |||
srv->lb_nodes = calloc(srv->lb_nodes_tot, sizeof(struct tree_occ)); | |||
- | |||
for (node = 0; node < srv->lb_nodes_tot; node++) { | |||
srv->lb_nodes[node].server = srv; | |||
srv->lb_nodes[node].node.key = full_hash(srv->puid * SRV_EWGHT_RANGE + node); |
@ -1,71 +0,0 @@ | |||
commit c0968f59b723dfa9effa63ac28b59642b11c6b8b | |||
Author: Richard Russo <russor@whatsapp.com> | |||
Date: Wed Jul 31 11:45:56 2019 -0700 | |||
BUG/MAJOR: http/sample: use a static buffer for raw -> htx conversion | |||
Multiple calls to smp_fetch_fhdr use the header context to keep track of | |||
header parsing position; however, when using header sampling on a raw | |||
connection, the raw buffer is converted into an HTX structure each time, and | |||
this was done in the trash areas; so the block reference would be invalid on | |||
subsequent calls. | |||
This patch must be backported to 2.0 and 1.9. | |||
(cherry picked from commit 458eafb36df88932a02d1ce7ca31832abf11b8b3) | |||
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com> | |||
diff --git a/src/http_fetch.c b/src/http_fetch.c | |||
index 67ea2094..e372a122 100644 | |||
--- a/src/http_fetch.c | |||
+++ b/src/http_fetch.c | |||
@@ -46,10 +46,40 @@ | |||
/* this struct is used between calls to smp_fetch_hdr() or smp_fetch_cookie() */ | |||
static THREAD_LOCAL struct hdr_ctx static_hdr_ctx; | |||
static THREAD_LOCAL struct http_hdr_ctx static_http_hdr_ctx; | |||
+/* this is used to convert raw connection buffers to htx */ | |||
+static THREAD_LOCAL struct buffer static_raw_htx_chunk; | |||
+static THREAD_LOCAL char *static_raw_htx_buf; | |||
#define SMP_REQ_CHN(smp) (smp->strm ? &smp->strm->req : NULL) | |||
#define SMP_RES_CHN(smp) (smp->strm ? &smp->strm->res : NULL) | |||
+/* This function returns the static htx chunk, where raw connections get | |||
+ * converted to HTX as needed for samplxsing. | |||
+ */ | |||
+struct buffer *get_raw_htx_chunk(void) | |||
+{ | |||
+ chunk_reset(&static_raw_htx_chunk); | |||
+ return &static_raw_htx_chunk; | |||
+} | |||
+ | |||
+static int alloc_raw_htx_chunk_per_thread() | |||
+{ | |||
+ static_raw_htx_buf = malloc(global.tune.bufsize); | |||
+ if (!static_raw_htx_buf) | |||
+ return 0; | |||
+ chunk_init(&static_raw_htx_chunk, static_raw_htx_buf, global.tune.bufsize); | |||
+ return 1; | |||
+} | |||
+ | |||
+static void free_raw_htx_chunk_per_thread() | |||
+{ | |||
+ free(static_raw_htx_buf); | |||
+ static_raw_htx_buf = NULL; | |||
+} | |||
+ | |||
+REGISTER_PER_THREAD_ALLOC(alloc_raw_htx_chunk_per_thread); | |||
+REGISTER_PER_THREAD_FREE(free_raw_htx_chunk_per_thread); | |||
+ | |||
/* | |||
* Returns the data from Authorization header. Function may be called more | |||
* than once so data is stored in txn->auth_data. When no header is found | |||
@@ -265,7 +295,7 @@ struct htx *smp_prefetch_htx(struct sample *smp, struct channel *chn, int vol) | |||
else if (h1m.flags & H1_MF_CLEN) | |||
flags |= HTX_SL_F_CLEN; | |||
- htx = htx_from_buf(get_trash_chunk()); | |||
+ htx = htx_from_buf(get_raw_htx_chunk()); | |||
sl = htx_add_stline(htx, HTX_BLK_REQ_SL, flags, h1sl.rq.m, h1sl.rq.u, h1sl.rq.v); | |||
if (!sl || !htx_add_all_headers(htx, hdrs)) | |||
return NULL; |
@ -1,46 +0,0 @@ | |||
commit 7343c710152c586a232a194ef37a56af636d6a56 | |||
Author: Willy Tarreau <w@1wt.eu> | |||
Date: Thu Aug 1 18:51:38 2019 +0200 | |||
BUG/MINOR: stream-int: also update analysers timeouts on activity | |||
Between 1.6 and 1.7, some parts of the stream forwarding process were | |||
moved into lower layers and the stream-interface had to keep the | |||
stream's task up to date regarding the timeouts. The analyser timeouts | |||
were not updated there as it was believed this was not needed during | |||
forwarding, but actually there is a case for this which is "option | |||
contstats" which periodically triggers the analyser timeout, and this | |||
change broke the option in case of sustained traffic (if there is some | |||
I/O activity during the same millisecond as the timeout expires, then | |||
the update will be missed). | |||
This patch simply brings back the analyser expiration updates from | |||
process_stream() to stream_int_notify(). | |||
It may be backported as far as 1.7, taking care to adjust the fields | |||
names if needed. | |||
(cherry picked from commit 45bcb37f0f8fa1e16dd9358a59dc280a38834dcd) | |||
Signed-off-by: Willy Tarreau <w@1wt.eu> | |||
diff --git a/src/stream_interface.c b/src/stream_interface.c | |||
index 9b9a8e9f..7d89cc90 100644 | |||
--- a/src/stream_interface.c | |||
+++ b/src/stream_interface.c | |||
@@ -558,6 +558,16 @@ static void stream_int_notify(struct stream_interface *si) | |||
task->expire = tick_first((tick_is_expired(task->expire, now_ms) ? 0 : task->expire), | |||
tick_first(tick_first(ic->rex, ic->wex), | |||
tick_first(oc->rex, oc->wex))); | |||
+ | |||
+ task->expire = tick_first(task->expire, ic->analyse_exp); | |||
+ task->expire = tick_first(task->expire, oc->analyse_exp); | |||
+ | |||
+ if (si->exp) | |||
+ task->expire = tick_first(task->expire, si->exp); | |||
+ | |||
+ if (sio->exp) | |||
+ task->expire = tick_first(task->expire, sio->exp); | |||
+ | |||
task_queue(task); | |||
} | |||
if (ic->flags & CF_READ_ACTIVITY) |
@ -1,37 +0,0 @@ | |||
commit a8fcdacb8cc0dddec72b1ddc4d9afc92d3684acd | |||
Author: Willy Tarreau <w@1wt.eu> | |||
Date: Fri Aug 2 07:48:47 2019 +0200 | |||
BUG/MEDIUM: mux-h2: unbreak receipt of large DATA frames | |||
Recent optimization in commit 4d7a88482 ("MEDIUM: mux-h2: don't try to | |||
read more than needed") broke the receipt of large DATA frames because | |||
it would unconditionally subscribe if there was some room left, thus | |||
preventing any new rx from being done since subscription may only be | |||
done once the end was reached, as indicated by ret == 0. | |||
However, fixing this uncovered that in HTX mode previous versions might | |||
occasionally be affected as well, when an available frame is the same | |||
size as the maximum data that may fit into an HTX buffer, we may end | |||
up reading that whole frame and still subscribe since it's still allowed | |||
to receive, thus causing issues to read the next frame. | |||
This patch will only work for 2.1-dev but a minor adaptation will be | |||
needed for earlier versions (down to 1.9, where subscribe() was added). | |||
(cherry picked from commit 9bc1c95855b9c6300de5ecf3720cbe4b2558c5a1) | |||
Signed-off-by: Willy Tarreau <w@1wt.eu> | |||
diff --git a/src/mux_h2.c b/src/mux_h2.c | |||
index 5bb85181..d605fe94 100644 | |||
--- a/src/mux_h2.c | |||
+++ b/src/mux_h2.c | |||
@@ -2766,7 +2766,7 @@ static int h2_recv(struct h2c *h2c) | |||
ret = 0; | |||
} while (ret > 0); | |||
- if (h2_recv_allowed(h2c) && (b_data(buf) < buf->size)) | |||
+ if (max && !ret && h2_recv_allowed(h2c)) | |||
conn->xprt->subscribe(conn, conn->xprt_ctx, SUB_RETRY_RECV, &h2c->wait_event); | |||
if (!b_data(buf)) { |
@ -1,227 +0,0 @@ | |||
commit 5a9c875f0f1ee83bd5889dd1ad53e9da43e6c34e | |||
Author: Willy Tarreau <w@1wt.eu> | |||
Date: Fri Aug 2 07:52:08 2019 +0200 | |||
BUG/MEDIUM: mux-h2: split the stream's and connection's window sizes | |||
The SETTINGS frame parser updates all streams' window for each | |||
INITIAL_WINDOW_SIZE setting received on the connection (like h2spec | |||
does in test 6.5.3), which can start to be expensive if repeated when | |||
there are many streams (up to 100 by default). A quick test shows that | |||
it's possible to parse only 35000 settings per second on a 3 GHz core | |||
for 100 streams, which is rather small. | |||
Given that window sizes are relative and may be negative, there's no | |||
point in pre-initializing them for each stream and update them from | |||
the settings. Instead, let's make them relative to the connection's | |||
initial window size so that any change immediately affects all streams. | |||
The only thing that remains needed is to wake up the streams that were | |||
unblocked by the update, which is now done once at the end of | |||
h2_process_demux() instead of once per setting. This now results in | |||
5.7 million settings being processed per second, which is way better. | |||
In order to keep the change small, the h2s' mws field was renamed to | |||
"sws" for "stream window size", and an h2s_mws() function was added | |||
to add it to the connection's initial window setting and determine the | |||
window size to use when muxing. The h2c_update_all_ws() function was | |||
renamed to h2c_unblock_sfctl() since it's now only used to unblock | |||
previously blocked streams. | |||
This needs to be backported to all versions till 1.8. | |||
(cherry picked from commit 1d4a0f88100daeb17dd0c9470c659b1ec288bc07) | |||
[wt: context adjustment, port to legacy parts] | |||
Signed-off-by: Willy Tarreau <w@1wt.eu> | |||
diff --git a/src/mux_h2.c b/src/mux_h2.c | |||
index d605fe94..f90e9435 100644 | |||
--- a/src/mux_h2.c | |||
+++ b/src/mux_h2.c | |||
@@ -208,7 +208,7 @@ struct h2s { | |||
struct eb32_node by_id; /* place in h2c's streams_by_id */ | |||
int32_t id; /* stream ID */ | |||
uint32_t flags; /* H2_SF_* */ | |||
- int mws; /* mux window size for this stream */ | |||
+ int sws; /* stream window size, to be added to the mux's initial window size */ | |||
enum h2_err errcode; /* H2 err code (H2_ERR_*) */ | |||
enum h2_ss st; | |||
uint16_t status; /* HTTP response status */ | |||
@@ -707,6 +707,14 @@ static inline __maybe_unused int h2s_id(const struct h2s *h2s) | |||
return h2s ? h2s->id : 0; | |||
} | |||
+/* returns the sum of the stream's own window size and the mux's initial | |||
+ * window, which together form the stream's effective window size. | |||
+ */ | |||
+static inline int h2s_mws(const struct h2s *h2s) | |||
+{ | |||
+ return h2s->sws + h2s->h2c->miw; | |||
+} | |||
+ | |||
/* returns true of the mux is currently busy as seen from stream <h2s> */ | |||
static inline __maybe_unused int h2c_mux_busy(const struct h2c *h2c, const struct h2s *h2s) | |||
{ | |||
@@ -945,7 +953,7 @@ static struct h2s *h2s_new(struct h2c *h2c, int id) | |||
LIST_INIT(&h2s->sending_list); | |||
h2s->h2c = h2c; | |||
h2s->cs = NULL; | |||
- h2s->mws = h2c->miw; | |||
+ h2s->sws = 0; | |||
h2s->flags = H2_SF_NONE; | |||
h2s->errcode = H2_ERR_NO_ERROR; | |||
h2s->st = H2_SS_IDLE; | |||
@@ -1543,30 +1551,23 @@ static void h2_wake_some_streams(struct h2c *h2c, int last) | |||
} | |||
} | |||
-/* Increase all streams' outgoing window size by the difference passed in | |||
- * argument. This is needed upon receipt of the settings frame if the initial | |||
- * window size is different. The difference may be negative and the resulting | |||
- * window size as well, for the time it takes to receive some window updates. | |||
+/* Wake up all blocked streams whose window size has become positive after the | |||
+ * mux's initial window was adjusted. This should be done after having processed | |||
+ * SETTINGS frames which have updated the mux's initial window size. | |||
*/ | |||
-static void h2c_update_all_ws(struct h2c *h2c, int diff) | |||
+static void h2c_unblock_sfctl(struct h2c *h2c) | |||
{ | |||
struct h2s *h2s; | |||
struct eb32_node *node; | |||
- if (!diff) | |||
- return; | |||
- | |||
node = eb32_first(&h2c->streams_by_id); | |||
while (node) { | |||
h2s = container_of(node, struct h2s, by_id); | |||
- h2s->mws += diff; | |||
- | |||
- if (h2s->mws > 0 && (h2s->flags & H2_SF_BLK_SFCTL)) { | |||
+ if (h2s->flags & H2_SF_BLK_SFCTL && h2s_mws(h2s) > 0) { | |||
h2s->flags &= ~H2_SF_BLK_SFCTL; | |||
if (h2s->send_wait && !LIST_ADDED(&h2s->list)) | |||
LIST_ADDQ(&h2c->send_list, &h2s->list); | |||
} | |||
- | |||
node = eb32_next(node); | |||
} | |||
} | |||
@@ -1607,7 +1608,6 @@ static int h2c_handle_settings(struct h2c *h2c) | |||
error = H2_ERR_FLOW_CONTROL_ERROR; | |||
goto fail; | |||
} | |||
- h2c_update_all_ws(h2c, arg - h2c->miw); | |||
h2c->miw = arg; | |||
break; | |||
case H2_SETTINGS_MAX_FRAME_SIZE: | |||
@@ -1869,13 +1869,13 @@ static int h2c_handle_window_update(struct h2c *h2c, struct h2s *h2s) | |||
goto strm_err; | |||
} | |||
- if (h2s->mws >= 0 && h2s->mws + inc < 0) { | |||
+ if (h2s_mws(h2s) >= 0 && h2s_mws(h2s) + inc < 0) { | |||
error = H2_ERR_FLOW_CONTROL_ERROR; | |||
goto strm_err; | |||
} | |||
- h2s->mws += inc; | |||
- if (h2s->mws > 0 && (h2s->flags & H2_SF_BLK_SFCTL)) { | |||
+ h2s->sws += inc; | |||
+ if (h2s_mws(h2s) > 0 && (h2s->flags & H2_SF_BLK_SFCTL)) { | |||
h2s->flags &= ~H2_SF_BLK_SFCTL; | |||
if (h2s->send_wait && !LIST_ADDED(&h2s->list)) | |||
LIST_ADDQ(&h2c->send_list, &h2s->list); | |||
@@ -2237,6 +2237,7 @@ static void h2_process_demux(struct h2c *h2c) | |||
struct h2s *h2s = NULL, *tmp_h2s; | |||
struct h2_fh hdr; | |||
unsigned int padlen = 0; | |||
+ int32_t old_iw = h2c->miw; | |||
if (h2c->st0 >= H2_CS_ERROR) | |||
return; | |||
@@ -2625,6 +2626,9 @@ static void h2_process_demux(struct h2c *h2c) | |||
h2s_notify_recv(h2s); | |||
} | |||
+ if (old_iw != h2c->miw) | |||
+ h2c_unblock_sfctl(h2c); | |||
+ | |||
h2c_restart_reading(h2c, 0); | |||
} | |||
@@ -4259,8 +4263,8 @@ static size_t h2s_frt_make_resp_data(struct h2s *h2s, const struct buffer *buf, | |||
if (size > max) | |||
size = max; | |||
- if (size > h2s->mws) | |||
- size = h2s->mws; | |||
+ if (size > h2s_mws(h2s)) | |||
+ size = h2s_mws(h2s); | |||
if (size <= 0) { | |||
h2s->flags |= H2_SF_BLK_SFCTL; | |||
@@ -4362,7 +4366,7 @@ static size_t h2s_frt_make_resp_data(struct h2s *h2s, const struct buffer *buf, | |||
ofs += size; | |||
total += size; | |||
h1m->curr_len -= size; | |||
- h2s->mws -= size; | |||
+ h2s->sws -= size; | |||
h2c->mws -= size; | |||
if (size && !h1m->curr_len && (h1m->flags & H1_MF_CHNK)) { | |||
@@ -4390,7 +4394,7 @@ static size_t h2s_frt_make_resp_data(struct h2s *h2s, const struct buffer *buf, | |||
} | |||
end: | |||
- trace("[%d] sent simple H2 DATA response (sid=%d) = %d bytes out (%u in, st=%s, ep=%u, es=%s, h2cws=%d h2sws=%d) data=%u", h2c->st0, h2s->id, size+9, (unsigned int)total, h1m_state_str(h1m->state), h1m->err_pos, h1m_state_str(h1m->err_state), h2c->mws, h2s->mws, (unsigned int)b_data(buf)); | |||
+ trace("[%d] sent simple H2 DATA response (sid=%d) = %d bytes out (%u in, st=%s, ep=%u, es=%s, h2cws=%d h2sws=%d) data=%u", h2c->st0, h2s->id, size+9, (unsigned int)total, h1m_state_str(h1m->state), h1m->err_pos, h1m_state_str(h1m->err_state), h2c->mws, h2s_mws(h2s), (unsigned int)b_data(buf)); | |||
return total; | |||
} | |||
@@ -4937,7 +4941,7 @@ static size_t h2s_htx_frt_make_resp_data(struct h2s *h2s, struct buffer *buf, si | |||
*/ | |||
if (unlikely(fsize == count && | |||
htx->used == 1 && type == HTX_BLK_DATA && | |||
- fsize <= h2s->mws && fsize <= h2c->mws && fsize <= h2c->mfs)) { | |||
+ fsize <= h2s_mws(h2s) && fsize <= h2c->mws && fsize <= h2c->mfs)) { | |||
void *old_area = mbuf->area; | |||
if (b_data(mbuf)) { | |||
@@ -4972,7 +4976,7 @@ static size_t h2s_htx_frt_make_resp_data(struct h2s *h2s, struct buffer *buf, si | |||
h2_set_frame_size(outbuf.area, fsize); | |||
/* update windows */ | |||
- h2s->mws -= fsize; | |||
+ h2s->sws -= fsize; | |||
h2c->mws -= fsize; | |||
/* and exchange with our old area */ | |||
@@ -5024,7 +5028,7 @@ static size_t h2s_htx_frt_make_resp_data(struct h2s *h2s, struct buffer *buf, si | |||
if (!fsize) | |||
goto send_empty; | |||
- if (h2s->mws <= 0) { | |||
+ if (h2s_mws(h2s) <= 0) { | |||
h2s->flags |= H2_SF_BLK_SFCTL; | |||
if (LIST_ADDED(&h2s->list)) | |||
LIST_DEL_INIT(&h2s->list); | |||
@@ -5034,8 +5038,8 @@ static size_t h2s_htx_frt_make_resp_data(struct h2s *h2s, struct buffer *buf, si | |||
if (fsize > count) | |||
fsize = count; | |||
- if (fsize > h2s->mws) | |||
- fsize = h2s->mws; // >0 | |||
+ if (fsize > h2s_mws(h2s)) | |||
+ fsize = h2s_mws(h2s); // >0 | |||
if (h2c->mfs && fsize > h2c->mfs) | |||
fsize = h2c->mfs; // >0 | |||
@@ -5071,7 +5075,7 @@ static size_t h2s_htx_frt_make_resp_data(struct h2s *h2s, struct buffer *buf, si | |||
/* now let's copy this this into the output buffer */ | |||
memcpy(outbuf.area + 9, htx_get_blk_ptr(htx, blk), fsize); | |||
- h2s->mws -= fsize; | |||
+ h2s->sws -= fsize; | |||
h2c->mws -= fsize; | |||
count -= fsize; | |||