From: Amaury Denoyelle Date: Wed, 25 Oct 2023 08:52:23 +0000 (+0200) Subject: MEDIUM: quic: count quic_conn instance for maxconn X-Git-Tag: v2.9-dev9~24 X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=7735cf3854eb155a50a5ea747406f2a25657e25c;p=thirdparty%2Fhaproxy.git MEDIUM: quic: count quic_conn instance for maxconn Increment actconn and check maxconn limit when a quic_conn is instantiated. This is necessary because prior to this patch, quic_conn instances where not counted. Global actconn was only incremented after the handshake has been completed and the connection structure is allocated. The increment is done using increment_actconn() on INITIAL packet parsing if a new connection is about to be created. If the limit is reached, the allocation is cancelled and the INITIAL packet is dropped. The decrement is done under quic_conn_release(). This means that quic_cc_conn instances are not taken into account. This seems safe enough because quic_cc_conn are only used for minimal usage. The counterpart of this change is that maxconn must not be checked a second time when listener_accept() is done over a QUIC connection. For this, a new bind_conf flag BC_O_XPRT_MAXCONN is set for listeners when maxconn is already counted by the lower layer. For the moment, it is positionned only for QUIC listeners. Without this patch, haproxy process could suffer from heavy memory/CPU load if the number of concurrent handshake is high. This patch is not considered a bug fix per-se. However, it has a major benefit to protect against too many QUIC handshakes. As such, it should be backported up to 2.6. For this, it relies on the following patch : "MINOR: frontend: implement a dedicated actconn increment function" --- diff --git a/include/haproxy/listener-t.h b/include/haproxy/listener-t.h index cdf64cbd6c..93856db55a 100644 --- a/include/haproxy/listener-t.h +++ b/include/haproxy/listener-t.h @@ -112,6 +112,7 @@ enum li_status { #define BC_O_UNLIMITED 0x00002000 /* listeners not subject to global limits (peers & stats socket) */ #define BC_O_NOSTOP 0x00004000 /* keep the listeners active even after a soft stop */ #define BC_O_REVERSE_HTTP 0x00008000 /* a reverse HTTP bind is used */ +#define BC_O_XPRT_MAXCONN 0x00010000 /* transport layer allocates its own resource prior to accept and is responsible to check maxconn limit */ /* flags used with bind_conf->ssl_options */ diff --git a/src/cfgparse.c b/src/cfgparse.c index ec5e7e4b6f..fc04fc479b 100644 --- a/src/cfgparse.c +++ b/src/cfgparse.c @@ -4234,6 +4234,9 @@ init_proxies_list_stage2: #ifdef USE_QUIC if (listener->bind_conf->xprt == xprt_get(XPRT_QUIC)) { + /* quic_conn are counted against maxconn. */ + listener->bind_conf->options |= BC_O_XPRT_MAXCONN; + # ifdef USE_QUIC_OPENSSL_COMPAT /* store the last checked bind_conf in bind_conf */ if (!(global.tune.options & GTUNE_NO_QUIC) && diff --git a/src/listener.c b/src/listener.c index bbbf180366..64d3aa038f 100644 --- a/src/listener.c +++ b/src/listener.c @@ -997,6 +997,12 @@ int listener_backlog(const struct listener *l) return 1024; } +/* Returns true if listener must check maxconn limit prior to accept. */ +static inline int listener_uses_maxconn(const struct listener *l) +{ + return !(l->bind_conf->options & (BC_O_UNLIMITED|BC_O_XPRT_MAXCONN)); +} + /* This function is called on a read event from a listening socket, corresponding * to an accept. It tries to accept as many connections as possible, and for each * calls the listener's accept handler (generally the frontend's accept handler). @@ -1114,7 +1120,7 @@ void listener_accept(struct listener *l) } while (!_HA_ATOMIC_CAS(&p->feconn, &count, next_feconn)); } - if (!(l->bind_conf->options & BC_O_UNLIMITED)) { + if (listener_uses_maxconn(l)) { next_actconn = increment_actconn(); if (!next_actconn) { /* the process was marked full or another @@ -1146,7 +1152,7 @@ void listener_accept(struct listener *l) _HA_ATOMIC_DEC(&l->nbconn); if (p) _HA_ATOMIC_DEC(&p->feconn); - if (!(l->bind_conf->options & BC_O_UNLIMITED)) + if (listener_uses_maxconn(l)) _HA_ATOMIC_DEC(&actconn); continue; @@ -1580,7 +1586,7 @@ void listener_release(struct listener *l) { struct proxy *fe = l->bind_conf->frontend; - if (!(l->bind_conf->options & BC_O_UNLIMITED)) + if (listener_uses_maxconn(l)) _HA_ATOMIC_DEC(&actconn); if (fe) _HA_ATOMIC_DEC(&fe->feconn); diff --git a/src/quic_conn.c b/src/quic_conn.c index 129780d708..750f9018e1 100644 --- a/src/quic_conn.c +++ b/src/quic_conn.c @@ -1435,6 +1435,12 @@ void quic_conn_release(struct quic_conn *qc) qc_free_ssl_sock_ctx(&qc->xprt_ctx); } + /* Decrement on quic_conn free. quic_cc_conn instances are not counted + * into global counters because they are designed to run for a limited + * time with a limited memory. + */ + _HA_ATOMIC_DEC(&actconn); + /* in the unlikely (but possible) case the connection was just added to * the accept_list we must delete it from there. */ diff --git a/src/quic_rx.c b/src/quic_rx.c index 4685d63149..b421c6cd97 100644 --- a/src/quic_rx.c +++ b/src/quic_rx.c @@ -14,6 +14,7 @@ #include +#include #include #include #include @@ -1901,6 +1902,7 @@ static struct quic_conn *quic_rx_pkt_retrieve_conn(struct quic_rx_packet *pkt, struct quic_conn *qc = NULL; struct proxy *prx; struct quic_counters *prx_counters; + unsigned int next_actconn = 0; TRACE_ENTER(QUIC_EV_CONN_LPKT); @@ -1958,6 +1960,14 @@ static struct quic_conn *quic_rx_pkt_retrieve_conn(struct quic_rx_packet *pkt, pkt->saddr = dgram->saddr; ipv4 = dgram->saddr.ss_family == AF_INET; + next_actconn = increment_actconn(); + if (!next_actconn) { + _HA_ATOMIC_INC(&maxconn_reached); + TRACE_STATE("drop packet on maxconn reached", + QUIC_EV_CONN_LPKT, NULL, NULL, NULL, pkt->version); + goto err; + } + /* Generate the first connection CID. This is derived from the client * ODCID and address. This allows to retrieve the connection from the * ODCID without storing it in the CID tree. This is an interesting @@ -1976,6 +1986,13 @@ static struct quic_conn *quic_rx_pkt_retrieve_conn(struct quic_rx_packet *pkt, goto err; } + /* Now quic_conn is allocated. If a future error + * occurred it will be freed with quic_conn_release() + * which also ensure actconn is decremented. + * Reset guard value to prevent a double decrement. + */ + next_actconn = 0; + /* Compute and store into the quic_conn the hash used to compute extra CIDs */ if (quic_hash64_from_cid) qc->hash64 = quic_hash64_from_cid(conn_id->cid.data, conn_id->cid.len, @@ -2025,6 +2042,11 @@ static struct quic_conn *quic_rx_pkt_retrieve_conn(struct quic_rx_packet *pkt, qc->cntrs.dropped_pkt++; else HA_ATOMIC_INC(&prx_counters->dropped_pkt); + + /* Reset active conn counter if needed. */ + if (next_actconn) + _HA_ATOMIC_DEC(&actconn); + TRACE_LEAVE(QUIC_EV_CONN_LPKT); return NULL; }