Re: Help tracking "connection refused" under pressure on v2.9
Hi Willy, On Fri, 29 Mar 2024 07:17:56 +0100 Willy Tarreau wrote: > > These "connection refused" is from our watchdog; but the effects are as > > perceptible from the outside. When our watchdog hits this situation, > > it will forcefully restart HAProxy (we have 2 instances) because there > > will be a considerable service degradation. If you remember, there's > > https://github.com/haproxy/haproxy/issues/1895 and we talked briefly > > about this in person, at HAProxyConf. > > Thanks for the context. I didn't remember about the issue. I remembered > we discussed for a while but didn't remember about the issue in question > obviously, given the number of issues I'm dealing with :-/ > > In the issue above I'm seeing an element from Felipe saying that telnet > to port 80 can take between 3 seconds to accept. That really makes me > think about either the SYN queue being full, causing drops and retransmits, > or a lack of socket memory to accept packets. That one could possibly be > caused by tcp_mem not being large enough due to some transfers with high > latency fast clients taking a lot of RAM, but it should not affect the > local UNIX socket. Also, killing the process means killing all the > associated connections and will definitely result in freeing a huge > amount of network buffers, so it could fuel that direction. If you have > two instances, did you notice if the two start to behave badly at the > same time ? If that's the case, it would definitely indicate a possible > resource-based cause like socket memory etc. Of our 2 HAProxy instances, it is usually one (mostly the frontend one) that exhibits this behavior. And as it is imperative that the corrective action be as swift as possible, all instances are terminated (which can include older instances, from graceful reloads), and new instances are started. Very harsh, but at >50 Gbps, each full second of downtime adds up considerably to network pressure. So for context, our least capable machine has 256 GB RAM. We have not seen any spikes over the metrics we monitor, and this issue tends to happen at a very stable steady-state, albeit a loaded one. While it is currently outside of our range for detailed data, we didn't notice anything unusual, especially regarding memory usage, on these traps we reported. But of course, there could be a metric that we're not yet aware that correlates. Anyone from the dustier, darkest corners that you know of? :-) > > > But this is incredibly elusive to reproduce; it comes and goes. It > > might happen every few minutes, or not happen at all for months. Not > > tied to a specific setup: different versions, kernels, machines. In > > fact, we do not have better ways to detect the situation, at least not > > as fast, reactive, and resilient. > > It might be useful to take periodic snapshots of /proc/slabinfo and > see if something jumps during such incidents (grep for TCP, net, skbuff > there). I guess you have not noticed any "out of socket memory" nor such > indications in your kernel logs, of course :-/ We have no indications of memory pressure related to network. At the peak, we usually see something like 15~22% overall active memory (it fails me, but it might take >70% of active memory for these machines to actually degrade, maybe more). As for TCP stuff, around 16~30k active sockets, plus some 50~100k in timewait, and still not creating any problems. > > Another one that could make sense to monitor is "PoolFailed" in > "show info". It should always remain zero. We collect this (all available actually); I don't remember this one ever measuring more than zero. But we'll keep an eye on it. In time, could this be somewhat unrelated to HAProxy? I.e., maybe kernel? Cheers, -- Ricardo Nabinger Sanchez https://www.taghos.com.br/
[PATCH 2/4] CLEANUP: Reapply strcmp.cocci (2)
This reapplies strcmp.cocci across the whole src/ tree.
---
src/event_hdl.c | 2 +-
src/hlua_fcn.c | 8
src/sample.c| 2 +-
src/tcp_act.c | 4 ++--
4 files changed, 8 insertions(+), 8 deletions(-)
diff --git a/src/event_hdl.c b/src/event_hdl.c
index f5bb5b6e7e..f4f7b19e4d 100644
--- a/src/event_hdl.c
+++ b/src/event_hdl.c
@@ -138,7 +138,7 @@ struct event_hdl_sub_type
event_hdl_string_to_sub_type(const char *name)
int it;
for (it = 0; it < (int)(sizeof(event_hdl_sub_type_map) /
sizeof(event_hdl_sub_type_map[0])); it++) {
- if (!strcmp(name, event_hdl_sub_type_map[it].name))
+ if (strcmp(name, event_hdl_sub_type_map[it].name) == 0)
return event_hdl_sub_type_map[it].type;
}
return EVENT_HDL_SUB_NONE;
diff --git a/src/hlua_fcn.c b/src/hlua_fcn.c
index a13e0f5f41..7aaab3a381 100644
--- a/src/hlua_fcn.c
+++ b/src/hlua_fcn.c
@@ -1329,14 +1329,14 @@ static int hlua_server_index(struct lua_State *L)
{
const char *key = lua_tostring(L, 2);
- if (!strcmp(key, "name")) {
+ if (strcmp(key, "name") == 0) {
if (ONLY_ONCE())
ha_warning("hlua: use of server 'name' attribute is
deprecated and will eventually be removed, please use get_name() function
instead: %s\n", hlua_traceback(L, ", "));
lua_pushvalue(L, 1);
hlua_server_get_name(L);
return 1;
}
- if (!strcmp(key, "puid")) {
+ if (strcmp(key, "puid") == 0) {
if (ONLY_ONCE())
ha_warning("hlua: use of server 'puid' attribute is
deprecated and will eventually be removed, please use get_puid() function
instead: %s\n", hlua_traceback(L, ", "));
lua_pushvalue(L, 1);
@@ -1980,14 +1980,14 @@ static int hlua_proxy_index(struct lua_State *L)
{
const char *key = lua_tostring(L, 2);
- if (!strcmp(key, "name")) {
+ if (strcmp(key, "name") == 0) {
if (ONLY_ONCE())
ha_warning("hlua: use of proxy 'name' attribute is
deprecated and will eventually be removed, please use get_name() function
instead: %s\n", hlua_traceback(L, ", "));
lua_pushvalue(L, 1);
hlua_proxy_get_name(L);
return 1;
}
- if (!strcmp(key, "uuid")) {
+ if (strcmp(key, "uuid") == 0) {
if (ONLY_ONCE())
ha_warning("hlua: use of proxy 'uuid' attribute is
deprecated and will eventually be removed, please use get_uuid() function
instead: %s\n", hlua_traceback(L, ", "));
lua_pushvalue(L, 1);
diff --git a/src/sample.c b/src/sample.c
index cbb959161b..8f46d31b96 100644
--- a/src/sample.c
+++ b/src/sample.c
@@ -69,7 +69,7 @@ int type_to_smp(const char *type)
int it = 0;
while (it < SMP_TYPES) {
- if (!strcmp(type, smp_to_type[it]))
+ if (strcmp(type, smp_to_type[it]) == 0)
break; // found
it += 1;
}
diff --git a/src/tcp_act.c b/src/tcp_act.c
index 1cefc90dcd..a88fab4afe 100644
--- a/src/tcp_act.c
+++ b/src/tcp_act.c
@@ -678,7 +678,7 @@ static enum act_parse_ret tcp_parse_set_mark(const char
**args, int *orig_arg, s
}
/* Register processing function. */
- if (!strcmp("set-bc-mark", args[cur_arg - 1]))
+ if (strcmp("set-bc-mark", args[cur_arg - 1]) == 0)
rule->action_ptr = tcp_action_set_bc_mark;
else
rule->action_ptr = tcp_action_set_fc_mark; // fc mark
@@ -740,7 +740,7 @@ static enum act_parse_ret tcp_parse_set_tos(const char
**args, int *orig_arg, st
}
/* Register processing function. */
- if (!strcmp("set-bc-tos", args[cur_arg - 1]))
+ if (strcmp("set-bc-tos", args[cur_arg - 1]) == 0)
rule->action_ptr = tcp_action_set_bc_tos;
else
rule->action_ptr = tcp_action_set_fc_tos; // fc tos
--
2.43.2
[PATCH 3/4] CLEANUP: Reapply xalloc_cast.cocci
This reapplies xalloc_cast.cocci across the whole src/ tree.
---
src/cpuset.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/src/cpuset.c b/src/cpuset.c
index 82e350f132..a20b81a25d 100644
--- a/src/cpuset.c
+++ b/src/cpuset.c
@@ -280,7 +280,7 @@ int cpu_map_configured(void)
static int cpuset_alloc(void)
{
/* allocate the structures used to store CPU topology info */
- cpu_map = (struct cpu_map*)calloc(MAX_TGROUPS, sizeof(*cpu_map));
+ cpu_map = calloc(MAX_TGROUPS, sizeof(*cpu_map));
if (!cpu_map)
return 0;
--
2.43.2
[PATCH 1/4] CLEANUP: Reapply ist.cocci (3)
This reapplies ist.cocci across the whole src/ tree.
---
src/resolvers.c | 4 ++--
src/stick_table.c | 4 ++--
2 files changed, 4 insertions(+), 4 deletions(-)
diff --git a/src/resolvers.c b/src/resolvers.c
index f97fb29b01..d68208555f 100644
--- a/src/resolvers.c
+++ b/src/resolvers.c
@@ -3947,9 +3947,9 @@ static int rslv_promex_fill_ts(void *unused, void
*metric_ctx, unsigned int id,
int ret;
labels[0].name = ist("resolver");
- labels[0].value = ist2(resolver->id, strlen(resolver->id));
+ labels[0].value = ist(resolver->id);
labels[1].name = ist("nameserver");
- labels[1].value = ist2(ns->id, strlen(ns->id));
+ labels[1].value = ist(ns->id);
ret = resolv_fill_stats(ns->counters, stats, &id);
if (ret == 1)
diff --git a/src/stick_table.c b/src/stick_table.c
index 964542cdea..2f75359291 100644
--- a/src/stick_table.c
+++ b/src/stick_table.c
@@ -5947,9 +5947,9 @@ static int stk_promex_fill_ts(void *unused, void
*metric_ctx, unsigned int id, s
return 0;
labels[0].name = ist("name");
- labels[0].value = ist2(t->id, strlen(t->id));
+ labels[0].value = ist(t->id);
labels[1].name = ist("type");
- labels[1].value = ist2(stktable_types[t->type].kw,
strlen(stktable_types[t->type].kw));
+ labels[1].value = ist(stktable_types[t->type].kw);
switch (id) {
case STICKTABLE_SIZE:
--
2.43.2
[PATCH 2/2] REGTESTS: Remove REQUIRE_VERSION=1.9 from all tests (2)
see also: 2a5fb62ad REGTESTS: Remove REQUIRE_VERSION=1.9 from all tests --- reg-tests/http-messaging/truncated.vtc | 1 - 1 file changed, 1 deletion(-) diff --git a/reg-tests/http-messaging/truncated.vtc b/reg-tests/http-messaging/truncated.vtc index 7579f6d763..7f262d75dc 100644 --- a/reg-tests/http-messaging/truncated.vtc +++ b/reg-tests/http-messaging/truncated.vtc @@ -1,5 +1,4 @@ varnishtest "HTTP response size tests: H2->H1 (HTX and legacy mode)" -#REQUIRE_VERSION=1.9 feature ignore_unknown_macro -- 2.43.2
[PATCH 1/2] REGTESTS: Do not use REQUIRE_VERSION for HAProxy 2.5+ (4)
Introduced in:
dfb1cea69 REGTESTS: promex: Adapt script to be less verbose
36d936dd1 REGTESTS: write a full reverse regtest
b57f15158 REGTESTS: provide a reverse-server test with name argument
f0bff2947 REGTESTS: provide a reverse-server test
see also:
fbbbc33df REGTESTS: Do not use REQUIRE_VERSION for HAProxy 2.5+
---
reg-tests/connection/reverse_connect_full.vtc | 2 +-
reg-tests/connection/reverse_server.vtc | 2 +-
reg-tests/connection/reverse_server_name.vtc | 2 +-
reg-tests/contrib/prometheus.vtc | 2 +-
4 files changed, 4 insertions(+), 4 deletions(-)
diff --git a/reg-tests/connection/reverse_connect_full.vtc
b/reg-tests/connection/reverse_connect_full.vtc
index 238831fc38..cc88382ced 100644
--- a/reg-tests/connection/reverse_connect_full.vtc
+++ b/reg-tests/connection/reverse_connect_full.vtc
@@ -1,7 +1,7 @@
varnishtest "Reverse connect full test"
feature ignore_unknown_macro
-#REQUIRE_VERSION=2.9
+feature cmd "$HAPROXY_PROGRAM -cc 'version_atleast(2.9-dev0)'"
server s1 {
rxreq
diff --git a/reg-tests/connection/reverse_server.vtc
b/reg-tests/connection/reverse_server.vtc
index 50fe8ceb80..5cd77ca7bb 100644
--- a/reg-tests/connection/reverse_server.vtc
+++ b/reg-tests/connection/reverse_server.vtc
@@ -1,7 +1,7 @@
varnishtest "Reverse server test"
feature ignore_unknown_macro
-#REQUIRE_VERSION=2.9
+feature cmd "$HAPROXY_PROGRAM -cc 'version_atleast(2.9-dev0)'"
barrier b1 cond 2
diff --git a/reg-tests/connection/reverse_server_name.vtc
b/reg-tests/connection/reverse_server_name.vtc
index 0fd850fe8f..3a24601743 100644
--- a/reg-tests/connection/reverse_server_name.vtc
+++ b/reg-tests/connection/reverse_server_name.vtc
@@ -2,7 +2,7 @@ varnishtest "Reverse server with a name parameter test"
feature cmd "$HAPROXY_PROGRAM -cc 'feature(OPENSSL)'"
feature ignore_unknown_macro
-#REQUIRE_VERSION=2.9
+feature cmd "$HAPROXY_PROGRAM -cc 'version_atleast(2.9-dev0)'"
barrier b1 cond 2
diff --git a/reg-tests/contrib/prometheus.vtc b/reg-tests/contrib/prometheus.vtc
index 60217c2a0e..89d65d7b74 100644
--- a/reg-tests/contrib/prometheus.vtc
+++ b/reg-tests/contrib/prometheus.vtc
@@ -1,6 +1,6 @@
varnishtest "prometheus exporter test"
-#REQUIRE_VERSION=3.0
+feature cmd "$HAPROXY_PROGRAM -cc 'version_atleast(3.0-dev0)'"
#REQUIRE_SERVICES=prometheus-exporter
feature ignore_unknown_macro
--
2.43.2
[PATCH] PKCS#11 key support provider & engine (rev. 2)
Diff attached.
Regards
Richard
diff --git a/include/haproxy/ssl_ckch-t.h b/include/haproxy/ssl_ckch-t.h
index 0002b84d9..2470c45b2 100644
--- a/include/haproxy/ssl_ckch-t.h
+++ b/include/haproxy/ssl_ckch-t.h
@@ -56,6 +56,13 @@ struct ckch_data {
X509 *ocsp_issuer;
OCSP_CERTID *ocsp_cid;
int ocsp_update_mode;
+#if defined(USE_ENGINE) && !defined(OPENSSL_NO_ENGINE)
+ char *engine;
+ char *private_key;
+#endif
+ int len;
+ char *pem_data;
+ EVP_PKEY *worker_key;
};
/*
diff --git a/src/ssl_ckch.c b/src/ssl_ckch.c
index b69caacb3..0f6ab6a0a 100644
--- a/src/ssl_ckch.c
+++ b/src/ssl_ckch.c
@@ -516,11 +516,45 @@ int ssl_sock_load_files_into_ckch(const char *path, struct ckch_data *data, char
*
* Return 0 on success or != 0 on failure
*/
+
+/*
+ * This caches the BIO data in case the private key i
+ *
+ * to be instantiated in the child process (used for HSM keys)
+ *
+ * The key ckch_data* and the value is ckch_data_cache*
+ *
+ */
+static void ckch_data_put(struct ckch_data *cur, char *data, int len)
+{
+ cur->pem_data = malloc(len);
+ memcpy(cur->pem_data, data, len);
+ cur->len = len;
+}
+
+#if defined(USE_ENGINE) && !defined(OPENSSL_NO_ENGINE)
+static void engine_data_put(struct ckch_data *cur, char *engine, char *private_key)
+{
+ cur->engine = strdup(engine);
+ cur->private_key = strdup(private_key);
+ cur->len = -1;
+}
+#endif
+
int ssl_sock_load_key_into_ckch(const char *path, char *buf, struct ckch_data *data , char **err)
{
BIO *in = NULL;
int ret = 1;
EVP_PKEY *key = NULL;
+ static char src[16384];
+ char *src_temp;
+ int len;
+ int filetype = 0;
+#if defined(USE_ENGINE) && !defined(OPENSSL_NO_ENGINE)
+ CONF *conf = NULL;
+ BIO *conf_in = NULL;
+ ENGINE *eng;
+#endif
if (buf) {
/* reading from a buffer */
@@ -538,16 +572,67 @@ int ssl_sock_load_key_into_ckch(const char *path, char *buf, struct ckch_data *d
if (BIO_read_filename(in, path) <= 0)
goto end;
+ filetype = 1;
}
/* Read Private Key */
+ if(filetype) {
+ len = BIO_read(in, src, sizeof(src));
+ BIO_reset(in);
+ } else {
+ len = BIO_get_mem_data(in, &src_temp);
+ memcpy(src, src_temp, len);
+ }
+
key = PEM_read_bio_PrivateKey(in, NULL, NULL, NULL);
+#if OPENSSL_VERSION_NUMBER >= 0x03000UL
+ data->len = 0;
+ if(key && global.mode&MODE_MWORKER) {
+ ckch_data_put(data, src, len);
+ }
+#endif
+ if(key)
+ goto ok;
+#if defined(USE_ENGINE) && !defined(OPENSSL_NO_ENGINE)
+ /* ENGINE should be loaded in global config */
+ conf = NCONF_new(NCONF_default());
+ if (conf == NULL)
+ goto end;
+ conf_in = BIO_new_mem_buf(src, len);
+ if (conf_in == NULL)
+ goto end;
+ ret = NCONF_load_bio(conf, conf_in, NULL);
+ if (ret<=0)
+ goto end;
+ char *eng_id = NCONF_get_string(conf, NULL, "engine");
+ char *private_key = NCONF_get_string(conf, NULL, "private_key");
+
+ ha_notice("ENGINE private_key = %s\n", private_key);
+ ha_notice("ENGINE engine_id = %s\n", eng_id);
+
+ eng = ENGINE_by_id(eng_id);
+ if (eng == NULL)
+ goto end;
+ else
+ fprintf(stderr, "ENGINE %s is successfully loaded!\n", eng_id);
+ key = ENGINE_load_private_key(eng, private_key, NULL, NULL);
+ if (key)
+ fprintf(stderr, "private key %s is successfully loaded!\n", private_key);
+
+ data->len = 0;
+ if(key && global.mode&MODE_MWORKER) {
+ ha_notice("storing key = %p engine_id = %s private_key = %s\n",
+ data, eng_id, private_key);
+ engine_data_put(data, eng_id, private_key);
+ }
+#endif
if (key == NULL) {
memprintf(err, "%sunable to load private key from file '%s'.\n",
err && *err ? *err : "", path);
goto end;
}
+ok:
ret = 0;
SWAP(data->key, key);
@@ -560,6 +645,12 @@ int ssl_sock_load_key_into_ckch(const char *path, char *buf, struct ckch_data *d
if (key)
EVP_PKEY_free(key);
+#if defined(USE_ENGINE) && !defined(OPENSSL_NO_ENGINE)
+ if (conf)
+ NCONF_free(conf);
+ if (conf_in)
+ BIO_free(conf_in);
+#endif
return ret;
}
@@ -581,6 +672,10 @@ int ssl_sock_load_pem_into_ckch(const char *path, char *buf, struct ckch_data *d
EVP_PKEY *key = NULL;
HASSL_DH *dh = NULL;
STACK_OF(X509) *chain = NULL;
+ int filetype = 0;
+ int len;
+ static char src[16384];
+ char *src_temp;
if (buf) {
/* reading from a buffer */
@@ -603,11 +698,24 @@ int ssl_sock_load_pem_into_ckch(const char *path, char *buf, struct ckch_data *d
err && *err ? *err : "", path);
goto end;
}
+ filetype = 1;
}
/* Read Private Key */
+ if(filetype) {
+ len = BIO_read(in, src, sizeof(src));
+ BIO_reset(in);
+ } else {
+ len = BIO_get_mem_data(in, &src_temp);
+ memcpy(src, src_temp, len);
+ }
+
key = PEM_read_bio_PrivateKey(in, NULL, NULL, NULL);
/* no need to check for errors here, because the private key could be loaded later */
+ data->len = 0;
+ if(key && global.mode&MODE_MWORKER) {
+ ckch_data_put(data, src, len);
+ }
#ifndef OPENSSL_NO_DH
/* Seek back to beginning of file */
diff --git a/src/ssl_sock.c b/src/ssl_sock.c
index 824bdaa72..5034745c8 100644
--- a/src/ssl_sock
[RFC] PKCS#11 key support provider & engine (rev. 2) - GH#71
Hello list, [RFC] PKCS#11 key support provider & engine (rev. 2) Addresses: https://github.com/haproxy/haproxy/issues/71 This RFC now supports provider(OpenSSL 3) and engine(OpenSSL 1.1.1) The patch will follow in the next email. Background Here is the revised version of my original RFC[1] to support PKCS#11 keys in master-worker mode by recreating private keys in the worker. First let me address some previous questions: from @wlallemand[2] "Did you identify why the fork was causing an issue? We should probably try to understand this before, it could be something stupid in haproxy's code or in the pkcs11 provider." This is a known PKCS#11 issue and not really a haproxy problem. Several integrations, such as provider/engine, do try to mitigate this but in general it is a tricky problem. After fork some of these integrations will attempt to restore handles but as GnuTLS says[3] "Note that, PKCS #11 modules behave in a peculiar way after a fork; they require a reinitialization of all the used PKCS #11 resources. While GnuTLS automates that process, there are corner cases where it is not possible to handle it correctly in an automated way. For that, it is recommended not to mix fork() and PKCS #11 module usage. It is recommended to initialize and use any PKCS #11 resources in a single process." PKCS#11 v3 does have support for PKCS#11 modules with "fork tolerant semantics"[4], but such modules are rare at the moment. Revised version Changes from [1] - now support both provider(OpenSSL 3) and engine(OpenSSL 1.1.1); the exemplars are pkcs11-provider[5] and opensc pkcs11 engine[6] - remove use of maps (ckch_data* -> PEM, SSL_CTX* -> ckch_data*) What does this RFC do? In master-worker mode it recreates the private key in the worker and installs this private key in the SSL_CTX*, overriding the original private key created in master. How does it do this? - extend struct ckch_data to store a copy of the PEM data used to create the cert / key - (engine specific) extend struct ckch_data to store the engine name and key name - thus - hash map (struct ckch_data -> PEM/engine data) is not needed - when creating SSL_CTX* store a pointer to struct ckch_data with SSL_CTX_set_app_data(); thus - hash map (SSL_CTX* -> struct ckch_data) is no longer needed - in the worker process, just before SSL_use_SSL_CTX(), we use the stored PEM data to recreate the provider private key or (engine specific) use the stored engine/key name to recreate the engine private key - the worker key is cached for future use How are engine keys referenced? - use crt "extra files" and store engine "key" in a specially formatted ".crt.key" file - this is not a private key PEM file, it is simply a text file containing two lines: engine = pkcs11 private_key = pkcs11:token=SomePKCS11Token;object=SomePrivateKeyLabel;type=private - repeat: an engine ".crt.key" file is not a PEM-"PRIVATE KEY" file that we are familiar with; it is a special text file with the name of the engine and private key - engine keys must live in a separate .key file as they are not PEM How are pkcs11-provider keys referenced? - like TPM2 pkcs11-provider can store keys in PEM objects with label "PKCS#11 PROVIDER URI" - a provider key can be merged into the .crt file - current glitch in pkcs11-provider PEM parser: a key in a merged file must be the first PEM object References 1. https://www.mail-archive.com/haproxy@formilux.org/msg44736.html 2. https://www.mail-archive.com/haproxy@formilux.org/msg44741.html 3. https://www.gnutls.org/manual/html_node/PKCS11-Initialization.html 4. https://docs.oasis-open.org/pkcs11/pkcs11-base/v3.0/csprd01/pkcs11-base-v3.0-csprd01.html 5. https://github.com/latchset/pkcs11-provider 6. https://github.com/OpenSC/libp11 Regards Richard
