MEDIUM: quic: release conn socket before using quic_cc_conn

After emission/reception of a CONNECTION_CLOSE, a connection enters the
CLOSING state. In this state, only minimal exchanges occurs as only the
packets which containted the CONNECTION_CLOSE frame can be reemitted. In
conformance with the RFC, most resources are released and quic_conn
instance is converted to the lighter quic_cc_conn.

Push further this optimization by closing quic_conn socket FD before
switching to a quic_cc_conn. This means that quic_cc_conn will rely on
listener socket for its send/recv operation. This should not impact
performance as as stated input/output are minimal on this state.

This patch should improve FD consumption as prior to this a socket FD
was kept during the closing delay which could cause maxsock to be
reached for other connections.

Note that fd member is kept in QUIC_CONN_COMMON and not removed from
quic_cc_conn. This is because quic_cc_conn relies on qc_snd_buf() which
access this field.

As a side-effect to this change, jobs accounting for quic_conn is also
updated. quic_cc_conn instances are now not counted as jobs. Indeed, the
main objective of jobs is to prevent haproxy process to be stopped with
data truncation. However, this relies on the connection to uses its
owned socket as the listener socket is shut down inconditionaly on
shutdown.

A consequence of the jobs handling change is that haproxy process will
be closed if only quic_cc_conn instances are present, thus preventing to
respect the closing state. In case of a reload, if a client missed a
CONNECTION_CLOSE frame just before process shutdown, it will probably
received a Stateless Reset on sending retry.

This change is considered safe as, for now, haproxy only emits
CONNECTION_CLOSE on error conditions (such as protocol violation or
timeout). It is considered as expected to suffer from data truncation
from this. However, if connection closing is reused by haproxy to
implement clean shutdown, it should be necessary to delay
CONNECTION_CLOSE frame emission to ensure no data truncation happens
here.

diff --git a/src/quic_conn.c b/src/quic_conn.c
index d54d957f56eed16e00b0967d26530382f385ea0a..4439d9d00d6bd301c7461f8e5864984ad0872e3c 100644 (file)
--- a/src/quic_conn.c
+++ b/src/quic_conn.c
@@ -748,12 +748,6 @@ static void quic_release_cc_conn(struct quic_cc_conn *cc_qc)
 
        TRACE_ENTER(QUIC_EV_CONN_IO_CB, cc_qc);
 
-       if (qc_test_fd(qc))
-               _HA_ATOMIC_DEC(&jobs);
-
-       /* Close quic-conn socket fd. */
-       qc_release_fd(qc, 0);
-
        task_destroy(cc_qc->idle_timer_task);
        cc_qc->idle_timer_task = NULL;
        tasklet_free(qc->wait_event.tasklet);
@@ -834,9 +828,6 @@ static struct quic_cc_conn *qc_new_cc_conn(struct quic_conn *qc)
 
        quic_conn_mv_cids_to_cc_conn(cc_qc, qc);
 
-       cc_qc->fd = qc->fd;
-       if (qc->fd >= 0)
-               fdtab[cc_qc->fd].owner = cc_qc;
        cc_qc->flags = qc->flags;
        cc_qc->err = qc->err;
 
@@ -1492,11 +1483,6 @@ void quic_conn_release(struct quic_conn *qc)
                cc_qc = qc_new_cc_conn(qc);
 
        if (!cc_qc) {
-               if (qc_test_fd(qc))
-                       _HA_ATOMIC_DEC(&jobs);
-
-               /* Close quic-conn socket fd. */
-               qc_release_fd(qc, 0);
                task_destroy(qc->idle_timer_task);
                qc->idle_timer_task = NULL;
                tasklet_free(qc->wait_event.tasklet);
@@ -1508,6 +1494,12 @@ void quic_conn_release(struct quic_conn *qc)
                qc->tx.cc_buf_area = NULL;
        }
 
+       if (qc_test_fd(qc))
+               _HA_ATOMIC_DEC(&jobs);
+
+       /* Close quic-conn socket fd. */
+       qc_release_fd(qc, 0);
+
        /* in the unlikely (but possible) case the connection was just added to
         * the accept_list we must delete it from there.
         */