summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorDan Goodliffe <dan@randomdan.homeip.net>2022-02-17 18:20:49 +0000
committerDan Goodliffe <dan@randomdan.homeip.net>2022-02-17 18:20:49 +0000
commit113fb5b22b06ff455d3ad842d517aadf1566fa89 (patch)
tree1d3e20a9a69b7ed694d8ed90f2a0feee4311023e
parentRemove some dead patches (diff)
downloadpatches-113fb5b22b06ff455d3ad842d517aadf1566fa89.tar.bz2
patches-113fb5b22b06ff455d3ad842d517aadf1566fa89.tar.xz
patches-113fb5b22b06ff455d3ad842d517aadf1566fa89.zip
Upstream HA Proxy segfault fix
https://github.com/haproxy/haproxy/issues/1517
-rw-r--r--net-proxy/haproxy/ecc473b5299ccd0007a73ec4b4af099ac65e9c43.patch132
1 files changed, 132 insertions, 0 deletions
diff --git a/net-proxy/haproxy/ecc473b5299ccd0007a73ec4b4af099ac65e9c43.patch b/net-proxy/haproxy/ecc473b5299ccd0007a73ec4b4af099ac65e9c43.patch
new file mode 100644
index 0000000..19acb57
--- /dev/null
+++ b/net-proxy/haproxy/ecc473b5299ccd0007a73ec4b4af099ac65e9c43.patch
@@ -0,0 +1,132 @@
+From ecc473b5299ccd0007a73ec4b4af099ac65e9c43 Mon Sep 17 00:00:00 2001
+From: Willy Tarreau <w@1wt.eu>
+Date: Thu, 27 Jan 2022 15:46:19 +0100
+Subject: [PATCH] BUG/MAJOR: compiler: relax alignment constraints on certain
+ structures
+
+In github bug #1517, Mike Lothian reported instant crashes on startup
+on RHEL8 + gcc-11 that appeared with 2.4 when allocating a proxy.
+
+The analysis brought us down to the THREAD_ALIGN() entries that were
+placed inside the "server" struct to avoid false sharing of cache lines.
+It turns out that some modern gcc make use of aligned vector operations
+to manipulate some fields (e.g. memset() etc) and that these structures
+allocated using malloc() are not necessarily aligned, hence the crash.
+
+The compiler is allowed to do that because the structure claims to be
+aligned. The problem is in fact that the alignment propagates to other
+structures that embed it. While most of these structures are used as
+statically allocated variables, some are dynamic and cannot use that.
+A deeper analysis showed that struct server does this, propagates to
+struct proxy, which propagates to struct spoe_config, all of which
+are allocated using malloc/calloc.
+
+A better approach would consist in usins posix_memalign(), but this one
+is not available everywhere and will either need to be reimplemented
+less efficiently (by always wasting 64 bytes before the area), or a
+few functions will have to be specifically written to deal with the
+few structures that are dynamically allocated.
+
+But the deeper problem that remains is that it is difficult to track
+structure alignment, as there's no available warning to check this.
+For the long term we'll probably have to create a macro such as
+"struct_malloc()" etc which takes a type and enforces an alignment
+based on the one of this type. This also means propagating that to
+pools as well, and it's not a tiny task.
+
+For now, let's get rid of the forced alignment in struct server, and
+replace it with extra padding. By punching 63-byte holes, we can keep
+areas on separate cache lines. Doing so moderately increases the size
+of the "server" structure (~+6%) but that's the best short-term option
+and it's easily backportable.
+
+This will have to be backported as far as 2.4.
+
+Thanks to Mike for the detailed report.
+---
+ include/haproxy/compiler.h | 25 +++++++++++++++++++++++++
+ include/haproxy/server-t.h | 8 ++++----
+ 2 files changed, 29 insertions(+), 4 deletions(-)
+
+diff --git a/include/haproxy/compiler.h b/include/haproxy/compiler.h
+index f18b5fa5cd..3943fe6459 100644
+--- a/include/haproxy/compiler.h
++++ b/include/haproxy/compiler.h
+@@ -220,6 +220,17 @@
+ #define HA_HAVE_CAS_DW
+ #endif
+
++/*********************** IMPORTANT NOTE ABOUT ALIGNMENT **********************\
++ * Alignment works fine for variables. It also works on types and struct *
++ * members by propagating the alignment to the container struct itself, *
++ * but this requires that variables of the affected type are properly *
++ * aligned themselves. While regular variables will always abide, those *
++ * allocated using malloc() will not! Most platforms provide posix_memalign()*
++ * for this, but it's not available everywhere. As such one ought not to use *
++ * these alignment declarations inside structures that are dynamically *
++ * allocated. If the purpose is only to avoid false sharing of cache lines *
++ * for multi_threading, see THREAD_PAD() below. *
++\*****************************************************************************/
+
+ /* sets alignment for current field or variable */
+ #ifndef ALIGNED
+@@ -294,6 +305,20 @@
+ #endif
+ #endif
+
++/* add optional padding of the specified size between fields in a structure,
++ * only when threads are enabled. This is used to avoid false sharing of cache
++ * lines for dynamically allocated structures which cannot guarantee alignment.
++ */
++#ifndef THREAD_PAD
++# ifdef USE_THREAD
++# define __THREAD_PAD(x,l) char __pad_##l[x]
++# define _THREAD_PAD(x,l) __THREAD_PAD(x, l)
++# define THREAD_PAD(x) _THREAD_PAD(x, __LINE__)
++# else
++# define THREAD_PAD(x)
++# endif
++#endif
++
+ /* The THREAD_LOCAL type attribute defines thread-local storage and is defined
+ * to __thread when threads are enabled or empty when disabled.
+ */
+diff --git a/include/haproxy/server-t.h b/include/haproxy/server-t.h
+index 09f39333d6..e53d6121b4 100644
+--- a/include/haproxy/server-t.h
++++ b/include/haproxy/server-t.h
+@@ -273,7 +273,7 @@ struct server {
+ /* The elements below may be changed on every single request by any
+ * thread, and generally at the same time.
+ */
+- ALWAYS_ALIGN(64);
++ THREAD_PAD(63);
+ struct eb32_node idle_node; /* When to next do cleanup in the idle connections */
+ unsigned int curr_idle_conns; /* Current number of orphan idling connections, both the idle and the safe lists */
+ unsigned int curr_idle_nb; /* Current number of connections in the idle list */
+@@ -288,14 +288,14 @@ struct server {
+ /* Element below are usd by LB algorithms and must be doable in
+ * parallel to other threads reusing connections above.
+ */
+- ALWAYS_ALIGN(64);
++ THREAD_PAD(63);
+ __decl_thread(HA_SPINLOCK_T lock); /* may enclose the proxy's lock, must not be taken under */
+ unsigned npos, lpos; /* next and last positions in the LB tree, protected by LB lock */
+ struct eb32_node lb_node; /* node used for tree-based load balancing */
+ struct server *next_full; /* next server in the temporary full list */
+
+ /* usually atomically updated by any thread during parsing or on end of request */
+- ALWAYS_ALIGN(64);
++ THREAD_PAD(63);
+ int cur_sess; /* number of currently active sessions (including syn_sent) */
+ int served; /* # of active sessions currently being served (ie not pending) */
+ int consecutive_errors; /* current number of consecutive errors */
+@@ -303,7 +303,7 @@ struct server {
+ struct be_counters counters; /* statistics counters */
+
+ /* Below are some relatively stable settings, only changed under the lock */
+- ALWAYS_ALIGN(64);
++ THREAD_PAD(63);
+
+ struct eb_root *lb_tree; /* we want to know in what tree the server is */
+ struct tree_occ *lb_nodes; /* lb_nodes_tot * struct tree_occ */