From 9f5382e45296da91d5c7f015469f0e743df1ffd0 Mon Sep 17 00:00:00 2001 From: Christopher Faulet Date: Fri, 21 May 2021 13:46:14 +0200 Subject: [PATCH] Revert "MEDIUM: http-ana: Deal with L7 retries in HTTP analysers" This reverts commit 5b82cc5b5c350c7cfa194cc6bc16ad9308784541. The purpose of this commit was to fully handle L7 retries in HTTP analysers and stop to deal with the L7 buffer in si_cs_send()/si_cs_recv(). It is of course cleaner this way. But there is a huge drawback. The L7 buffer is reserved from the time the request analysis is finished until the moment the response is received. For a small request, the analysis is finished before the connection to the server. Thus for the L7 buffer will be kept for queued sessions while it is not mandatory. So, for now, the commit is reverted to go back to the less expensive solution. This patch must be backported to 2.4. --- src/http_ana.c | 64 ++++++++++-------------------------------- src/stream.c | 4 +++ src/stream_interface.c | 51 +++++++++++++++++++++++++++++++++ 3 files changed, 70 insertions(+), 49 deletions(-) diff --git a/src/http_ana.c b/src/http_ana.c index a3618b9043..ef9ee65817 100644 --- a/src/http_ana.c +++ b/src/http_ana.c @@ -1119,27 +1119,6 @@ int http_request_forward_body(struct stream *s, struct channel *req, int an_bit) else { msg->msg_state = HTTP_MSG_DONE; req->to_forward = 0; - - if ((s->be->retry_type &~ PR_RE_CONN_FAILED) && !(s->si[1].flags & SI_FL_D_L7_RETRY)) { - struct stream_interface *si = &s->si[1]; - - /* If we want to be able to do L7 retries, copy the - * request, so that we are able to resend them if - * needed. - * - * Try to allocate a buffer if we had none. If it - * fails, the next test will just disable the l7 - * retries. - */ - DBG_TRACE_STATE("enable L7 retry, save the request", STRM_EV_STRM_ANA|STRM_EV_HTTP_ANA, s, txn); - si->flags |= SI_FL_L7_RETRY; - if (b_alloc(&si->l7_buffer) == NULL) - si->flags &= ~SI_FL_L7_RETRY; - else { - memcpy(b_orig(&si->l7_buffer), b_orig(&req->buf), b_size(&req->buf)); - b_add(&si->l7_buffer, co_data(req)); - } - } } done: @@ -1158,7 +1137,6 @@ int http_request_forward_body(struct stream *s, struct channel *req, int an_bit) } goto return_bad_req; } - DBG_TRACE_LEAVE(STRM_EV_STRM_ANA|STRM_EV_HTTP_ANA, s, txn); return 1; } @@ -1286,16 +1264,13 @@ int http_request_forward_body(struct stream *s, struct channel *req, int an_bit) /* Returns 0 if we can attempt to retry, -1 otherwise */ static __inline int do_l7_retry(struct stream *s, struct stream_interface *si) { - struct channel *req = &s->req; - struct channel *res = &s->res; + struct channel *req, *res; + int co_data; si->conn_retries--; if (si->conn_retries < 0) goto no_retry; - if (b_is_null(&req->buf) && !channel_alloc_buffer(req, &s->buffer_wait)) - goto no_retry; - if (objt_server(s->target)) { if (s->flags & SF_CURR_SESS) { s->flags &= ~SF_CURR_SESS; @@ -1305,6 +1280,8 @@ static __inline int do_l7_retry(struct stream *s, struct stream_interface *si) } _HA_ATOMIC_INC(&s->be->be_counters.retries); + req = &s->req; + res = &s->res; /* Remove any write error from the request, and read error from the response */ req->flags &= ~(CF_WRITE_ERROR | CF_WRITE_TIMEOUT | CF_SHUTW | CF_SHUTW_NOW); res->flags &= ~(CF_READ_ERROR | CF_READ_TIMEOUT | CF_SHUTR | CF_EOI | CF_READ_NULL | CF_SHUTR_NOW); @@ -1320,19 +1297,20 @@ static __inline int do_l7_retry(struct stream *s, struct stream_interface *si) res->total = 0; si_release_endpoint(&s->si[1]); - b_reset(&req->buf); - memcpy(b_orig(&req->buf), b_orig(&si->l7_buffer), b_size(&si->l7_buffer)); - b_set_data(&req->buf, b_size(&req->buf)); - co_set_data(req, b_data(&si->l7_buffer)); + b_free(&req->buf); + /* Swap the L7 buffer with the channel buffer */ + /* We know we stored the co_data as b_data, so get it there */ + co_data = b_data(&si->l7_buffer); + b_set_data(&si->l7_buffer, b_size(&si->l7_buffer)); + b_xfer(&req->buf, &si->l7_buffer, b_data(&si->l7_buffer)); + co_set_data(req, co_data); DBG_TRACE_DEVEL("perform a L7 retry", STRM_EV_STRM_ANA|STRM_EV_HTTP_ANA, s, s->txn); + + no_retry: b_reset(&res->buf); co_set_data(res, 0); return 0; - - no_retry: - b_free(&si->l7_buffer); - return -1; } /* This stream analyser waits for a complete HTTP response. It returns 1 if the @@ -1399,11 +1377,8 @@ int http_wait_for_response(struct stream *s, struct channel *rep, int an_bit) * the SI_FL_L7_RETRY flag, so it's ok not * to check s->be->retry_type. */ - if (co_data(rep) || do_l7_retry(s, si_b) == 0) { - DBG_TRACE_DEVEL("leaving on L7 retry", - STRM_EV_STRM_ANA|STRM_EV_HTTP_ANA, s, txn); + if (co_data(rep) || do_l7_retry(s, si_b) == 0) return 0; - } } if (txn->flags & TX_NOT_FIRST) @@ -1568,19 +1543,10 @@ int http_wait_for_response(struct stream *s, struct channel *rep, int an_bit) * response which at least looks like HTTP. We have an indicator * of each header's length, so we can parse them quickly. */ + msg->msg_state = HTTP_MSG_BODY; BUG_ON(htx_get_first_type(htx) != HTX_BLK_RES_SL); sl = http_get_stline(htx); - if ((si_b->flags & SI_FL_L7_RETRY) && - l7_status_match(s->be, sl->info.res.status) && - do_l7_retry(s, si_b) == 0) { - DBG_TRACE_DEVEL("leaving on L7 retry", STRM_EV_STRM_ANA|STRM_EV_HTTP_ANA, s, txn); - return 0; - } - b_free(&s->si[1].l7_buffer); - - msg->msg_state = HTTP_MSG_BODY; - /* 0: we might have to print this header in debug mode */ if (unlikely((global.mode & MODE_DEBUG) && (!(global.mode & MODE_QUIET) || (global.mode & MODE_VERBOSE)))) { diff --git a/src/stream.c b/src/stream.c index bff951ec5d..952c70f58d 100644 --- a/src/stream.c +++ b/src/stream.c @@ -2178,6 +2178,10 @@ struct task *process_stream(struct task *t, void *context, unsigned int state) */ si_b->state = SI_ST_REQ; /* new connection requested */ si_b->conn_retries = s->be->conn_retries; + if ((s->be->retry_type &~ PR_RE_CONN_FAILED) && + (s->be->mode == PR_MODE_HTTP) && + !(si_b->flags & SI_FL_D_L7_RETRY)) + si_b->flags |= SI_FL_L7_RETRY; } } else { diff --git a/src/stream_interface.c b/src/stream_interface.c index 43f1a88637..6d26919d8d 100644 --- a/src/stream_interface.c +++ b/src/stream_interface.c @@ -726,6 +726,35 @@ int si_cs_send(struct conn_stream *cs) if (oc->flags & CF_STREAMER) send_flag |= CO_SFL_STREAMER; + if ((si->flags & SI_FL_L7_RETRY) && !b_data(&si->l7_buffer)) { + struct stream *s = si_strm(si); + /* If we want to be able to do L7 retries, copy + * the data we're about to send, so that we are able + * to resend them if needed + */ + /* Try to allocate a buffer if we had none. + * If it fails, the next test will just + * disable the l7 retries by setting + * l7_conn_retries to 0. + */ + if (!s->txn || (s->txn->req.msg_state != HTTP_MSG_DONE)) + si->flags &= ~SI_FL_L7_RETRY; + else { + if (b_is_null(&si->l7_buffer)) + b_alloc(&si->l7_buffer); + if (b_is_null(&si->l7_buffer)) + si->flags &= ~SI_FL_L7_RETRY; + else { + memcpy(b_orig(&si->l7_buffer), + b_orig(&oc->buf), + b_size(&oc->buf)); + si->l7_buffer.head = co_data(oc); + b_add(&si->l7_buffer, co_data(oc)); + } + + } + } + ret = cs->conn->mux->snd_buf(cs, &oc->buf, co_data(oc), send_flag); if (ret > 0) { did_send = 1; @@ -1338,6 +1367,28 @@ int si_cs_recv(struct conn_stream *cs) break; } + /* L7 retries enabled and maximum connection retries not reached */ + if ((si->flags & SI_FL_L7_RETRY) && si->conn_retries) { + struct htx *htx; + struct htx_sl *sl; + + htx = htxbuf(&ic->buf); + if (htx) { + sl = http_get_stline(htx); + if (sl && l7_status_match(si_strm(si)->be, + sl->info.res.status)) { + /* If we got a status for which we would + * like to retry the request, empty + * the buffer and pretend there's an + * error on the channel. + */ + ic->flags |= CF_READ_ERROR; + htx_reset(htx); + return 1; + } + } + si->flags &= ~SI_FL_L7_RETRY; + } cur_read += ret; /* if we're allowed to directly forward data, we must update ->o */ -- 2.39.5