From b781a1bb09f868cc1ab38a45acf06ef398695718 Mon Sep 17 00:00:00 2001 From: Amaury Denoyelle Date: Wed, 23 Aug 2023 18:02:51 +0200 Subject: [PATCH] MINOR: connection: prepare init code paths for active reverse When an active reverse connection is initialized, it has no stream-conn attached to it contrary to other backend connections. This forces to add extra check on stream existence in conn_create_mux() and h2_init(). There is also extra checks required for session_accept_fd() after reverse and accept is done. This is because contrary to other frontend connections, reversed connections have already initialized their mux and transport layers. This forces us to skip the majority of session_accept_fd() initialization part. Finally, if session_accept_fd() is interrupted due to an early error, a reverse connection cannot be freed directly or else mux will remain alone. Instead, the mux destroy callback is used to free all connection elements properly. --- src/connection.c | 3 ++- src/mux_h2.c | 5 +++-- src/session.c | 57 ++++++++++++++++++++++++++++++------------------ 3 files changed, 41 insertions(+), 24 deletions(-) diff --git a/src/connection.c b/src/connection.c index 70c9e45c2..ae4ee386d 100644 --- a/src/connection.c +++ b/src/connection.c @@ -91,7 +91,8 @@ int conn_create_mux(struct connection *conn) return 0; fail: /* let the upper layer know the connection failed */ - sc->app_ops->wake(sc); + if (sc) + sc->app_ops->wake(sc); return -1; } else return conn_complete_session(conn); diff --git a/src/mux_h2.c b/src/mux_h2.c index 048aa3958..6de86f6be 100644 --- a/src/mux_h2.c +++ b/src/mux_h2.c @@ -1059,7 +1059,7 @@ static int h2_init(struct connection *conn, struct proxy *prx, struct session *s if (t) task_queue(t); - if (h2c->flags & H2_CF_IS_BACK) { + if (h2c->flags & H2_CF_IS_BACK && likely(!conn_is_reverse(h2c->conn))) { /* FIXME: this is temporary, for outgoing connections we need * to immediately allocate a stream until the code is modified * so that the caller calls ->attach(). For now the outgoing sc @@ -1072,7 +1072,8 @@ static int h2_init(struct connection *conn, struct proxy *prx, struct session *s goto fail_stream; } - proxy_inc_fe_cum_sess_ver_ctr(sess->listener, prx, 2); + if (sess) + proxy_inc_fe_cum_sess_ver_ctr(sess->listener, prx, 2); HA_ATOMIC_INC(&h2c->px_counters->open_conns); HA_ATOMIC_INC(&h2c->px_counters->total_conns); diff --git a/src/session.c b/src/session.c index d3b0c886d..68dd4bc87 100644 --- a/src/session.c +++ b/src/session.c @@ -165,24 +165,31 @@ int session_accept_fd(struct connection *cli_conn) cli_conn->proxy_netns = l->rx.settings->netns; - if (conn_prepare(cli_conn, l->rx.proto, l->bind_conf->xprt) < 0) - goto out_free_conn; - - conn_ctrl_init(cli_conn); - - /* wait for a PROXY protocol header */ - if (l->bind_conf->options & BC_O_ACC_PROXY) - cli_conn->flags |= CO_FL_ACCEPT_PROXY; - - /* wait for a NetScaler client IP insertion protocol header */ - if (l->bind_conf->options & BC_O_ACC_CIP) - cli_conn->flags |= CO_FL_ACCEPT_CIP; - - /* Add the handshake pseudo-XPRT */ - if (cli_conn->flags & (CO_FL_ACCEPT_PROXY | CO_FL_ACCEPT_CIP)) { - if (xprt_add_hs(cli_conn) != 0) + /* Active reversed connection has already been initialized before being + * accepted. It must not be resetted. + * TODO use a dedicated accept_fd callback for reverse protocol + */ + if (!cli_conn->xprt) { + if (conn_prepare(cli_conn, l->rx.proto, l->bind_conf->xprt) < 0) goto out_free_conn; + + conn_ctrl_init(cli_conn); + + /* wait for a PROXY protocol header */ + if (l->bind_conf->options & BC_O_ACC_PROXY) + cli_conn->flags |= CO_FL_ACCEPT_PROXY; + + /* wait for a NetScaler client IP insertion protocol header */ + if (l->bind_conf->options & BC_O_ACC_CIP) + cli_conn->flags |= CO_FL_ACCEPT_CIP; + + /* Add the handshake pseudo-XPRT */ + if (cli_conn->flags & (CO_FL_ACCEPT_PROXY | CO_FL_ACCEPT_CIP)) { + if (xprt_add_hs(cli_conn) != 0) + goto out_free_conn; + } } + sess = session_new(p, l, &cli_conn->obj_type); if (!sess) goto out_free_conn; @@ -309,9 +316,15 @@ int session_accept_fd(struct connection *cli_conn) MSG_DONTWAIT|MSG_NOSIGNAL); } - conn_stop_tracking(cli_conn); - conn_full_close(cli_conn); - conn_free(cli_conn); + if (cli_conn->mux) { + /* Mux is already initialized for active reversed connection. */ + cli_conn->mux->destroy(cli_conn->ctx); + } + else { + conn_stop_tracking(cli_conn); + conn_full_close(cli_conn); + conn_free(cli_conn); + } listener_release(l); return ret; } @@ -483,8 +496,10 @@ int conn_complete_session(struct connection *conn) goto fail; session_count_new(sess); - if (conn_install_mux_fe(conn, NULL) < 0) - goto fail; + if (!conn->mux) { + if (conn_install_mux_fe(conn, NULL) < 0) + goto fail; + } /* the embryonic session's task is not needed anymore */ task_destroy(sess->task);