From 9a42bd6998537c99162af4010ada69ba1c655ea7 Mon Sep 17 00:00:00 2001 From: Dan Goodliffe Date: Mon, 21 Feb 2022 09:39:43 +0000 Subject: Revert "Upstream HA Proxy segfault fix" This reverts commit 113fb5b22b06ff455d3ad842d517aadf1566fa89. --- .../ecc473b5299ccd0007a73ec4b4af099ac65e9c43.patch | 132 --------------------- 1 file changed, 132 deletions(-) delete mode 100644 net-proxy/haproxy/ecc473b5299ccd0007a73ec4b4af099ac65e9c43.patch (limited to 'net-proxy/haproxy') diff --git a/net-proxy/haproxy/ecc473b5299ccd0007a73ec4b4af099ac65e9c43.patch b/net-proxy/haproxy/ecc473b5299ccd0007a73ec4b4af099ac65e9c43.patch deleted file mode 100644 index 19acb57..0000000 --- a/net-proxy/haproxy/ecc473b5299ccd0007a73ec4b4af099ac65e9c43.patch +++ /dev/null @@ -1,132 +0,0 @@ -From ecc473b5299ccd0007a73ec4b4af099ac65e9c43 Mon Sep 17 00:00:00 2001 -From: Willy Tarreau -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 */ -- cgit v1.2.3