]> git.ipfire.org Git - thirdparty/haproxy.git/commitdiff
MEDIUM: stream: No longer release backend conn-stream on connection retry
authorChristopher Faulet <cfaulet@haproxy.com>
Thu, 16 Dec 2021 13:44:31 +0000 (14:44 +0100)
committerChristopher Faulet <cfaulet@haproxy.com>
Thu, 24 Feb 2022 10:00:02 +0000 (11:00 +0100)
The backend conn-stream is no longer released on connection retry. This
means the conn-stream is detached from the underlying connection but not
released. Thus, during connection retries, the stream has always an
allocated conn-stream with no connection. All previous changes were made to
make this possible.

Note that .attach() mux callback function was changed to get the conn-stream
as argument. The muxes are no longer responsible to create the conn-stream
when a server connection is attached to a stream.

include/haproxy/connection-t.h
src/backend.c
src/cli.c
src/http_ana.c
src/mux_fcgi.c
src/mux_h1.c
src/mux_h2.c
src/mux_pt.c
src/stream.c

index 6a14cc090db66b9081daa3ecf5ae387808aae725..a5a596ced1ea586fc2cfb0415aa52f4dd235c5d5 100644 (file)
@@ -421,7 +421,7 @@ struct mux_ops {
        void (*shutr)(struct conn_stream *cs, enum cs_shr_mode);     /* shutr function */
        void (*shutw)(struct conn_stream *cs, enum cs_shw_mode);     /* shutw function */
 
-       struct conn_stream *(*attach)(struct connection *, struct session *sess); /* Create and attach a conn_stream to an outgoing connection */
+       int (*attach)(struct connection *conn, struct conn_stream *, struct session *sess); /* attach a conn_stream to an outgoing connection */
        const struct conn_stream *(*get_first_cs)(const struct connection *); /* retrieves any valid conn_stream from this connection */
        void (*detach)(struct conn_stream *); /* Detach a conn_stream from an outgoing connection, when the request is done */
        int (*show_fd)(struct buffer *, struct connection *); /* append some data about connection into chunk for "show fd"; returns non-zero if suspicious */
index 6a793a0c7ba77ac5658427b194601a8784f29ea8..f289337c9ec50af138daeebdf3ec9364b00f6720 100644 (file)
@@ -1496,9 +1496,13 @@ int connect_server(struct stream *s)
                        }
 
                        if (avail >= 1) {
-                               srv_cs = srv_conn->mux->attach(srv_conn, s->sess);
-                               if (srv_cs)
-                                       si_attach_cs(&s->si[1], srv_cs);
+                               srv_cs = si_attach_conn(&s->si[1], srv_conn);
+                               if (srv_cs) {
+                                       if (srv_conn->mux->attach(srv_conn, srv_cs, s->sess) == -1) {
+                                               srv_conn = NULL;
+                                               cs_init(srv_cs, NULL);
+                                       }
+                               }
                                else
                                        srv_conn = NULL;
                        }
@@ -1574,12 +1578,11 @@ skip_reuse:
                        return SF_ERR_INTERNAL;  /* how did we get there ? */
                }
 
-               srv_cs = si_alloc_cs(&s->si[1], srv_conn);
+               srv_cs = si_attach_conn(&s->si[1], srv_conn);
                if (!srv_cs) {
                        conn_free(srv_conn);
                        return SF_ERR_RESOURCE;
                }
-               srv_conn->ctx = srv_cs;
 #if defined(USE_OPENSSL) && defined(TLSEXT_TYPE_application_layer_protocol_negotiation)
                if (!srv ||
                    (srv->use_ssl != 1 || (!(srv->ssl_ctx.alpn_str) && !(srv->ssl_ctx.npn_str)) ||
@@ -2288,7 +2291,7 @@ void back_handle_st_cer(struct stream *s)
        }
 
        /* At this stage, we will trigger a connection retry (with or without
-        * redispatch). Thus we must release the SI endpoint on the server side
+        * redispatch). Thus we must reset the SI endpoint on the server side
         * an close the attached connection. It is especially important to do it
         * now if the retry is not immediately performed, to be sure to release
         * resources as soon as possible and to not catch errors from the lower
@@ -2297,7 +2300,7 @@ void back_handle_st_cer(struct stream *s)
         * Note: the stream-interface will be switched to ST_REQ, ST_ASS or
         * ST_TAR and SI_FL_ERR and SI_FL_EXP flags will be unset.
         */
-       si_release_endpoint(&s->si[1]);
+       si_reset_endpoint(&s->si[1]);
 
        stream_choose_redispatch(s);
 
index ba14052c7b42f5e6c14fa8628555ff7ebffd64c2..541bad8f3a330ba813ccc4423b7193fe7c1034ef 100644 (file)
--- a/src/cli.c
+++ b/src/cli.c
@@ -2715,7 +2715,7 @@ int pcli_wait_for_response(struct stream *s, struct channel *rep, int an_bit)
                 * connection.
                 */
                if (!si_conn_ready(&s->si[1])) {
-                       si_release_endpoint(&s->si[1]);
+                       si_reset_endpoint(&s->si[1]);
                        s->srv_conn = NULL;
                }
 
index 6cb248c415a5e7421d2f05c980f4dd5d0f6630af..848750c930c6183a52319c15550372c9ea361fd1 100644 (file)
@@ -1257,7 +1257,7 @@ static __inline int do_l7_retry(struct stream *s, struct stream_interface *si)
        res->to_forward = 0;
        res->analyse_exp = TICK_ETERNITY;
        res->total = 0;
-       si_release_endpoint(&s->si[1]);
+       si_reset_endpoint(&s->si[1]);
 
        b_free(&req->buf);
        /* Swap the L7 buffer with the channel buffer */
index fa9e3adb96757a1b5740cd3466cb84bbfb449565..d5558a68be52798f06fbff11a3be67af5aacc8e2 100644 (file)
@@ -3515,34 +3515,26 @@ static size_t fcgi_strm_parse_response(struct fcgi_strm *fstrm, struct buffer *b
  * Attach a new stream to a connection
  * (Used for outgoing connections)
  */
-static struct conn_stream *fcgi_attach(struct connection *conn, struct session *sess)
+static int fcgi_attach(struct connection *conn, struct conn_stream *cs, struct session *sess)
 {
-       struct conn_stream *cs;
        struct fcgi_strm *fstrm;
        struct fcgi_conn *fconn = conn->ctx;
 
        TRACE_ENTER(FCGI_EV_FSTRM_NEW, conn);
-       cs = cs_new(conn, conn->target);
-       if (!cs) {
-               TRACE_ERROR("CS allocation failure", FCGI_EV_FSTRM_NEW|FCGI_EV_FSTRM_ERR, conn);
-               goto err;
-       }
        fstrm = fcgi_conn_stream_new(fconn, cs, sess);
-       if (!fstrm) {
-               cs_free(cs);
+       if (!fstrm)
                goto err;
-       }
 
        /* the connection is not idle anymore, let's mark this */
        HA_ATOMIC_AND(&fconn->wait_event.tasklet->state, ~TASK_F_USR1);
        xprt_set_used(conn, conn->xprt, conn->xprt_ctx);
 
        TRACE_LEAVE(FCGI_EV_FSTRM_NEW, conn, fstrm);
-       return cs;
+       return 0;
 
   err:
        TRACE_DEVEL("leaving on error", FCGI_EV_FSTRM_NEW|FCGI_EV_FSTRM_ERR, conn);
-       return NULL;
+       return -1;
 }
 
 /* Retrieves the first valid conn_stream from this connection, or returns NULL.
index 4605c95772b61f504370904f9709b3c65da38966..db3483fb13482be42488ec3be261e9e0091b647a 100644 (file)
@@ -3219,10 +3219,9 @@ struct task *h1_timeout_task(struct task *t, void *context, unsigned int state)
  * Attach a new stream to a connection
  * (Used for outgoing connections)
  */
-static struct conn_stream *h1_attach(struct connection *conn, struct session *sess)
+static int h1_attach(struct connection *conn, struct conn_stream *cs, struct session *sess)
 {
        struct h1c *h1c = conn->ctx;
-       struct conn_stream *cs = NULL;
        struct h1s *h1s;
 
        TRACE_ENTER(H1_EV_STRM_NEW, conn);
@@ -3231,12 +3230,6 @@ static struct conn_stream *h1_attach(struct connection *conn, struct session *se
                goto err;
        }
 
-       cs = cs_new(h1c->conn, h1c->conn->target);
-       if (!cs) {
-               TRACE_ERROR("CS allocation failure", H1_EV_STRM_NEW|H1_EV_STRM_END|H1_EV_STRM_ERR, conn);
-               goto err;
-       }
-
        h1s = h1c_bck_stream_new(h1c, cs, sess);
        if (h1s == NULL) {
                TRACE_ERROR("h1s creation failure", H1_EV_STRM_NEW|H1_EV_STRM_END|H1_EV_STRM_ERR, conn);
@@ -3248,11 +3241,10 @@ static struct conn_stream *h1_attach(struct connection *conn, struct session *se
        xprt_set_used(conn, conn->xprt, conn->xprt_ctx);
 
        TRACE_LEAVE(H1_EV_STRM_NEW, conn, h1s);
-       return cs;
+       return 0;
   err:
-       cs_free(cs);
        TRACE_DEVEL("leaving on error", H1_EV_STRM_NEW|H1_EV_STRM_END|H1_EV_STRM_ERR, conn);
-       return NULL;
+       return -1;
 }
 
 /* Retrieves a valid conn_stream from this connection, or returns NULL. For
index b60e38b05b8ce0f73365cde44b3f81705d6c3540..0cc9eb373603ec302fc05b3fd156181bc951ddb8 100644 (file)
@@ -4180,23 +4180,16 @@ do_leave:
  * Attach a new stream to a connection
  * (Used for outgoing connections)
  */
-static struct conn_stream *h2_attach(struct connection *conn, struct session *sess)
+static int h2_attach(struct connection *conn, struct conn_stream *cs, struct session *sess)
 {
-       struct conn_stream *cs;
        struct h2s *h2s;
        struct h2c *h2c = conn->ctx;
 
        TRACE_ENTER(H2_EV_H2S_NEW, conn);
-       cs = cs_new(conn, conn->target);
-       if (!cs) {
-               TRACE_DEVEL("leaving on CS allocation failure", H2_EV_H2S_NEW|H2_EV_H2S_ERR, conn);
-               return NULL;
-       }
        h2s = h2c_bck_stream_new(h2c, cs, sess);
        if (!h2s) {
                TRACE_DEVEL("leaving on stream creation failure", H2_EV_H2S_NEW|H2_EV_H2S_ERR, conn);
-               cs_free(cs);
-               return NULL;
+               return -1;
        }
 
        /* the connection is not idle anymore, let's mark this */
@@ -4204,7 +4197,7 @@ static struct conn_stream *h2_attach(struct connection *conn, struct session *se
        xprt_set_used(h2c->conn, h2c->conn->xprt, h2c->conn->xprt_ctx);
 
        TRACE_LEAVE(H2_EV_H2S_NEW, conn, h2s);
-       return cs;
+       return 0;
 }
 
 /* Retrieves the first valid conn_stream from this connection, or returns NULL.
index 713f11078100a04ad48ec73eefabd0fbfc42e631..4d6d2ba08a167879da198577f75e3d66a4804881 100644 (file)
@@ -363,28 +363,18 @@ static int mux_pt_wake(struct connection *conn)
  * Attach a new stream to a connection
  * (Used for outgoing connections)
  */
-static struct conn_stream *mux_pt_attach(struct connection *conn, struct session *sess)
+static int mux_pt_attach(struct connection *conn, struct conn_stream *cs, struct session *sess)
 {
-       struct conn_stream *cs;
        struct mux_pt_ctx *ctx = conn->ctx;
 
        TRACE_ENTER(PT_EV_STRM_NEW, conn);
        if (ctx->wait_event.events)
                conn->xprt->unsubscribe(ctx->conn, conn->xprt_ctx, SUB_RETRY_RECV, &ctx->wait_event);
-       cs = cs_new(conn, conn->target);
-       if (!cs) {
-               TRACE_ERROR("CS allocation failure", PT_EV_STRM_NEW|PT_EV_STRM_END|PT_EV_STRM_ERR, conn);
-               goto fail;
-       }
-
        ctx->cs = cs;
        cs->flags |= CS_FL_RCV_MORE;
 
        TRACE_LEAVE(PT_EV_STRM_NEW, conn, cs);
-       return (cs);
-fail:
-       TRACE_DEVEL("leaving on error", PT_EV_STRM_NEW|PT_EV_STRM_END|PT_EV_STRM_ERR, conn);
-       return NULL;
+       return 0;
 }
 
 /* Retrieves a valid conn_stream from this connection, or returns NULL. For
index 1b554cb49aafdb1a2d287cef7ffcc3197d9f92d3..f934ea0941f2804d951465b1ce3914979de331a1 100644 (file)
@@ -2200,7 +2200,7 @@ struct task *process_stream(struct task *t, void *context, unsigned int state)
                        }
                }
                else {
-                       si_release_endpoint(si_b);
+                       si_reset_endpoint(si_b);
                        si_b->state = SI_ST_CLO; /* shutw+ini = abort */
                        channel_shutw_now(req);        /* fix buffer flags upon abort */
                        channel_shutr_now(res);