From e05bf9e4137a7e61351cf0782abbe2dfd7ec7f04 Mon Sep 17 00:00:00 2001 From: Christopher Faulet Date: Tue, 29 Mar 2022 15:23:40 +0200 Subject: [PATCH] MINOR: stream-int/txn: Move buffer for L7 retries in the HTTP transaction The L7 retries only concerns the stream when a server connection is established. Thus instead of storing the L7 buffer into the stream-interface, it may be moved to the stream. And because it is only available for HTTP streams, it may be moved in the HTTP transaction. Associated flags are also moved into the HTTP transaction. --- dev/flags/flags.c | 4 ++-- include/haproxy/http_ana-t.h | 4 +++- include/haproxy/stream_interface-t.h | 4 +--- include/haproxy/stream_interface.h | 1 - src/http_act.c | 10 ++++------ src/http_ana.c | 23 +++++++++++++---------- src/stream.c | 4 ++-- src/stream_interface.c | 19 +++++++++---------- 8 files changed, 34 insertions(+), 35 deletions(-) diff --git a/dev/flags/flags.c b/dev/flags/flags.c index 50f48b1fb..02064b39e 100644 --- a/dev/flags/flags.c +++ b/dev/flags/flags.c @@ -279,8 +279,6 @@ void show_si_flags(unsigned int f) SHOW_FLAG(f, SI_FL_RXBLK_SHUT); SHOW_FLAG(f, SI_FL_RXBLK_CONN); SHOW_FLAG(f, SI_FL_RX_WAIT_EP); - SHOW_FLAG(f, SI_FL_L7_RETRY); - SHOW_FLAG(f, SI_FL_D_L7_RETRY); SHOW_FLAG(f, SI_FL_ADDR_FROM_SET); SHOW_FLAG(f, SI_FL_ADDR_TO_SET); @@ -332,6 +330,8 @@ void show_txn_flags(unsigned int f) return; } + SHOW_FLAG(f, TX_L7_RETRY); + SHOW_FLAG(f, TX_D_L7_RETRY); SHOW_FLAG(f, TX_NOT_FIRST); SHOW_FLAG(f, TX_USE_PX_CONN); SHOW_FLAG(f, TX_CACHE_HAS_SEC_KEY); diff --git a/include/haproxy/http_ana-t.h b/include/haproxy/http_ana-t.h index 96845a45b..b267ebebd 100644 --- a/include/haproxy/http_ana-t.h +++ b/include/haproxy/http_ana-t.h @@ -71,6 +71,8 @@ /* used only for keep-alive purposes, to indicate we're on a second transaction */ #define TX_NOT_FIRST 0x00040000 /* the transaction is not the first one */ +#define TX_L7_RETRY 0x000800000 /* The transaction may attempt L7 retries */ +#define TX_D_L7_RETRY 0x001000000 /* Disable L7 retries on this transaction, even if configured to do it */ /* * HTTP message status flags (msg->flags) */ @@ -184,7 +186,7 @@ struct http_txn { /* 1 unused byte here */ short status; /* HTTP status from the server, negative if from proxy */ struct http_reply *http_reply; /* The HTTP reply to use as reply */ - + struct buffer l7_buffer; /* To store the data, in case we have to retry */ char cache_hash[20]; /* Store the cache hash */ char cache_secondary_hash[HTTP_CACHE_SEC_KEY_LEN]; /* Optional cache secondary key. */ char *uri; /* first line if log needed, NULL otherwise */ diff --git a/include/haproxy/stream_interface-t.h b/include/haproxy/stream_interface-t.h index a6e798360..2e6ef3977 100644 --- a/include/haproxy/stream_interface-t.h +++ b/include/haproxy/stream_interface-t.h @@ -104,8 +104,7 @@ enum { SI_FL_RXBLK_CONN = 0x00100000, /* other side is not connected */ SI_FL_RXBLK_ANY = 0x001F0000, /* any of the RXBLK flags above */ SI_FL_RX_WAIT_EP = 0x00200000, /* stream-int waits for more data from the end point */ - SI_FL_L7_RETRY = 0x01000000, /* The stream interface may attempt L7 retries */ - SI_FL_D_L7_RETRY = 0x02000000, /* Disable L7 retries on this stream interface, even if configured to do it */ + /* unused: 0x01000000, 0x02000000 */ SI_FL_ADDR_FROM_SET = 0x04000000, /* source address is set */ SI_FL_ADDR_TO_SET = 0x08000000 /* destination address is set */ @@ -140,7 +139,6 @@ struct stream_interface { unsigned int hcto; /* half-closed timeout (0 = unset) */ struct wait_event wait_event; /* We're in a wait list */ - struct buffer l7_buffer; /* To store the data, in case we have to retry */ }; /* operations available on a stream-interface */ diff --git a/include/haproxy/stream_interface.h b/include/haproxy/stream_interface.h index 89ebcda38..60efe1de3 100644 --- a/include/haproxy/stream_interface.h +++ b/include/haproxy/stream_interface.h @@ -116,7 +116,6 @@ static inline int si_init(struct stream_interface *si) si->cs = NULL; si->state = si->prev_state = SI_ST_INI; si->ops = &si_embedded_ops; - si->l7_buffer = BUF_NULL; si->wait_event.tasklet = tasklet_new(); if (!si->wait_event.tasklet) return -1; diff --git a/src/http_act.c b/src/http_act.c index eebc1884f..f35fadfb5 100644 --- a/src/http_act.c +++ b/src/http_act.c @@ -752,19 +752,17 @@ static enum act_parse_ret parse_http_action_reject(const char **args, int *orig_ /* This function executes the "disable-l7-retry" HTTP action. * It disables L7 retries (all retry except for a connection failure). This * can be useful for example to avoid retrying on POST requests. - * It just removes the L7 retry flag on the stream_interface, and always + * It just removes the L7 retry flag on the HTTP transaction, and always * return ACT_RET_CONT; */ static enum act_return http_req_disable_l7_retry(struct act_rule *rule, struct proxy *px, struct session *sess, struct stream *s, int flags) { - struct stream_interface *si = cs_si(s->csb); - - /* In theory, the SI_FL_L7_RETRY flags isn't set at this point, but + /* In theory, the TX_L7_RETRY flags isn't set at this point, but * let's be future-proof and remove it anyway. */ - si->flags &= ~SI_FL_L7_RETRY; - si->flags |= SI_FL_D_L7_RETRY; + s->txn->flags &= ~TX_L7_RETRY; + s->txn->flags |= TX_D_L7_RETRY; return ACT_RET_CONT; } diff --git a/src/http_ana.c b/src/http_ana.c index 05b62fe02..4ce49bb67 100644 --- a/src/http_ana.c +++ b/src/http_ana.c @@ -987,7 +987,7 @@ int http_request_forward_body(struct stream *s, struct channel *req, int an_bit) * was a write error, we may recover. */ if (!(req->flags & (CF_READ_ERROR | CF_READ_TIMEOUT)) && - (cs_si(s->csb)->flags & SI_FL_L7_RETRY)) { + (txn->flags & TX_L7_RETRY)) { DBG_TRACE_DEVEL("leaving on L7 retry", STRM_EV_STRM_ANA|STRM_EV_HTTP_ANA|STRM_EV_HTTP_ERR, s, txn); return 0; @@ -1264,9 +1264,9 @@ static __inline int do_l7_retry(struct stream *s, struct stream_interface *si) 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_data = b_data(&s->txn->l7_buffer); + b_set_data(&s->txn->l7_buffer, b_size(&s->txn->l7_buffer)); + b_xfer(&req->buf, &s->txn->l7_buffer, b_data(&s->txn->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); @@ -1330,7 +1330,7 @@ int http_wait_for_response(struct stream *s, struct channel *rep, int an_bit) struct connection *conn = cs_conn(s->csb); /* Perform a L7 retry because server refuses the early data. */ - if ((si_b->flags & SI_FL_L7_RETRY) && + if ((txn->flags & TX_L7_RETRY) && (s->be->retry_type & PR_RE_EARLY_ERROR) && conn && conn->err_code == CO_ER_SSL_EARLY_FAILED && do_l7_retry(s, si_b) == 0) { @@ -1370,7 +1370,7 @@ int http_wait_for_response(struct stream *s, struct channel *rep, int an_bit) /* 2: read timeout : return a 504 to the client. */ else if (rep->flags & CF_READ_TIMEOUT) { - if ((si_b->flags & SI_FL_L7_RETRY) && + if ((txn->flags & TX_L7_RETRY) && (s->be->retry_type & PR_RE_TIMEOUT)) { if (co_data(rep) || do_l7_retry(s, si_b) == 0) { DBG_TRACE_DEVEL("leaving on L7 retry", @@ -1423,7 +1423,7 @@ int http_wait_for_response(struct stream *s, struct channel *rep, int an_bit) /* 4: close from server, capture the response if the server has started to respond */ else if (rep->flags & CF_SHUTR) { - if ((si_b->flags & SI_FL_L7_RETRY) && + if ((txn->flags & TX_L7_RETRY) && (s->be->retry_type & PR_RE_DISCONNECTED)) { if (co_data(rep) || do_l7_retry(s, si_b) == 0) { DBG_TRACE_DEVEL("leaving on L7 retry", @@ -1491,7 +1491,7 @@ int http_wait_for_response(struct stream *s, struct channel *rep, int an_bit) sl = http_get_stline(htx); /* Perform a L7 retry because of the status code */ - if ((si_b->flags & SI_FL_L7_RETRY) && + if ((txn->flags & TX_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); @@ -1499,7 +1499,7 @@ int http_wait_for_response(struct stream *s, struct channel *rep, int an_bit) } /* Now, L7 buffer is useless, it can be released */ - b_free(&(cs_si(s->csb)->l7_buffer)); + b_free(&txn->l7_buffer); msg->msg_state = HTTP_MSG_BODY; @@ -1720,7 +1720,7 @@ int http_wait_for_response(struct stream *s, struct channel *rep, int an_bit) health_adjust(__objt_server(s->target), HANA_STATUS_HTTP_HDRRSP); } if ((s->be->retry_type & PR_RE_JUNK_REQUEST) && - (si_b->flags & SI_FL_L7_RETRY) && + (txn->flags & TX_L7_RETRY) && do_l7_retry(s, si_b) == 0) { DBG_TRACE_DEVEL("leaving on L7 retry", STRM_EV_STRM_ANA|STRM_EV_HTTP_ANA, s, txn); @@ -5139,6 +5139,7 @@ struct http_txn *http_create_txn(struct stream *s) txn->flags = ((cs && cs->endp->flags & CS_EP_NOT_FIRST) ? TX_NOT_FIRST : 0); txn->status = -1; txn->http_reply = NULL; + txn->l7_buffer = BUF_NULL; write_u32(txn->cache_hash, 0); txn->cookie_first_date = 0; @@ -5183,6 +5184,8 @@ void http_destroy_txn(struct stream *s) if (!LIST_ISEMPTY(&s->vars_reqres.head)) vars_prune(&s->vars_reqres, s->sess, s); + b_free(&txn->l7_buffer); + pool_free(pool_head_http_txn, txn); s->txn = NULL; } diff --git a/src/stream.c b/src/stream.c index 833be0e1b..fe1e2c299 100644 --- a/src/stream.c +++ b/src/stream.c @@ -2163,8 +2163,8 @@ struct task *process_stream(struct task *t, void *context, unsigned int state) 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; + !(s->txn->flags & TX_D_L7_RETRY)) + s->txn->flags |= TX_L7_RETRY; } } else { diff --git a/src/stream_interface.c b/src/stream_interface.c index 194c5ff11..1e28a8fc0 100644 --- a/src/stream_interface.c +++ b/src/stream_interface.c @@ -124,7 +124,6 @@ void si_free(struct stream_interface *si) if (!si) return; - b_free(&si->l7_buffer); tasklet_free(si->wait_event.tasklet); sockaddr_free(&si->src); sockaddr_free(&si->dst); @@ -703,6 +702,7 @@ static int si_cs_send(struct conn_stream *cs) { struct connection *conn = __cs_conn(cs); struct stream_interface *si = cs_si(cs); + struct stream *s = si_strm(si); struct channel *oc = si_oc(si); int ret; int did_send = 0; @@ -778,8 +778,7 @@ static 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 (s->txn && s->txn->flags & TX_L7_RETRY && !b_data(&s->txn->l7_buffer)) { /* 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 @@ -789,17 +788,17 @@ static int si_cs_send(struct conn_stream *cs) * 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; + if (s->txn->req.msg_state != HTTP_MSG_DONE) + s->txn->flags &= ~TX_L7_RETRY; else { - if (b_alloc(&si->l7_buffer) == NULL) - si->flags &= ~SI_FL_L7_RETRY; + if (b_alloc(&s->txn->l7_buffer) == NULL) + s->txn->flags &= ~TX_L7_RETRY; else { - memcpy(b_orig(&si->l7_buffer), + memcpy(b_orig(&s->txn->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)); + s->txn->l7_buffer.head = co_data(oc); + b_add(&s->txn->l7_buffer, co_data(oc)); } }