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 cdf64cbd6cf114128ab129ee2feb212c552ac229..93856db55a81dbff87610d448a346aa00a77e8e8 100644 (file)
--- 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 ec5e7e4b6f6cf717ec0614d94643acd213d8785e..fc04fc479be932bc703b9b6ecf627cb865c70d2f 100644 (file)
--- 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 bbbf18036636fb2ac1bc72bedc4fca63d9224fc5..64d3aa038f7f8b94fdfe8d020bf4480b1643212e 100644 (file)
--- 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 <l> 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 129780d708215c5707f0809e18bbd3e4929daa54..750f9018e1bd1d4636919f66c8ad6d0e03e2c8ff 100644 (file)
--- 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 4685d631492e8dc9434d21eb5f8cc605563663f0..b421c6cd973747abd54a516e8162f28272a0a831 100644 (file)
--- a/src/quic_rx.c
+++ b/src/quic_rx.c
@@ -14,6 +14,7 @@
 
 #include <haproxy/quic_rx.h>
 
+#include <haproxy/frontend.h>
 #include <haproxy/h3.h>
 #include <haproxy/list.h>
 #include <haproxy/ncbuf.h>
@@ -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;
 }