From: Christopher Faulet Date: Thu, 16 Dec 2021 13:44:31 +0000 (+0100) Subject: MEDIUM: stream: No longer release backend conn-stream on connection retry X-Git-Tag: v2.6-dev2~53 X-Git-Url: http://git.ipfire.org/?a=commitdiff_plain;h=e00ad358c914100d8784a8c349542d2379513d3b;p=thirdparty%2Fhaproxy.git MEDIUM: stream: No longer release backend conn-stream on connection retry 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. --- diff --git a/include/haproxy/connection-t.h b/include/haproxy/connection-t.h index 6a14cc090d..a5a596ced1 100644 --- a/include/haproxy/connection-t.h +++ b/include/haproxy/connection-t.h @@ -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 */ diff --git a/src/backend.c b/src/backend.c index 6a793a0c7b..f289337c9e 100644 --- a/src/backend.c +++ b/src/backend.c @@ -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); diff --git a/src/cli.c b/src/cli.c index ba14052c7b..541bad8f3a 100644 --- 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; } diff --git a/src/http_ana.c b/src/http_ana.c index 6cb248c415..848750c930 100644 --- a/src/http_ana.c +++ b/src/http_ana.c @@ -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 */ diff --git a/src/mux_fcgi.c b/src/mux_fcgi.c index fa9e3adb96..d5558a68be 100644 --- a/src/mux_fcgi.c +++ b/src/mux_fcgi.c @@ -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. diff --git a/src/mux_h1.c b/src/mux_h1.c index 4605c95772..db3483fb13 100644 --- a/src/mux_h1.c +++ b/src/mux_h1.c @@ -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 diff --git a/src/mux_h2.c b/src/mux_h2.c index b60e38b05b..0cc9eb3736 100644 --- a/src/mux_h2.c +++ b/src/mux_h2.c @@ -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. diff --git a/src/mux_pt.c b/src/mux_pt.c index 713f110781..4d6d2ba08a 100644 --- a/src/mux_pt.c +++ b/src/mux_pt.c @@ -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 diff --git a/src/stream.c b/src/stream.c index 1b554cb49a..f934ea0941 100644 --- a/src/stream.c +++ b/src/stream.c @@ -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);