From ad46e52814672e41236439ee8523e668ecb24f4e Mon Sep 17 00:00:00 2001 From: Christopher Faulet Date: Fri, 14 Apr 2023 11:59:15 +0200 Subject: [PATCH] MINOR: tree-wide: Test SC_FL_ERROR with SE_FL_ERROR from upper layer From the stream, when SE_FL_ERROR flag is tested, we now also test the SC_FL_ERROR flag. Idea is to stop to rely on the SE descriptor to detect errors. --- src/backend.c | 12 ++++++------ src/cli.c | 2 +- src/http_ana.c | 8 ++++---- src/stconn.c | 13 ++++++------- src/stream.c | 26 +++++++++++++++----------- src/tcp_rules.c | 4 ++-- 6 files changed, 34 insertions(+), 31 deletions(-) diff --git a/src/backend.c b/src/backend.c index 324ac9b..9b7e9e1 100644 --- a/src/backend.c +++ b/src/backend.c @@ -1957,7 +1957,7 @@ int srv_redispatch_connect(struct stream *s) /* Check if the connection request is in such a state that it can be aborted. */ static int back_may_abort_req(struct channel *req, struct stream *s) { - return (sc_ep_test(s->scf, SE_FL_ERROR) || + return ((s->scf->flags & SC_FL_ERROR) || sc_ep_test(s->scf, SE_FL_ERROR) || ((s->scb->flags & (SC_FL_SHUT_WANTED|SC_FL_SHUT_DONE)) && /* empty and client aborted */ (channel_is_empty(req) || (s->be->options & PR_O_ABRT_CLOSE)))); } @@ -2266,9 +2266,9 @@ void back_handle_st_con(struct stream *s) done: /* retryable error ? */ - if ((s->flags & SF_CONN_EXP) || sc_ep_test(sc, SE_FL_ERROR)) { + if ((s->flags & SF_CONN_EXP) || sc_ep_test(sc, SE_FL_ERROR) || (sc->flags & SC_FL_ERROR)) { if (!s->conn_err_type) { - if (sc_ep_test(sc, SE_FL_ERROR)) + if ((sc->flags & SC_FL_ERROR) || sc_ep_test(sc, SE_FL_ERROR)) s->conn_err_type = STRM_ET_CONN_ERR; else s->conn_err_type = STRM_ET_CONN_TO; @@ -2294,7 +2294,7 @@ void back_handle_st_con(struct stream *s) void back_handle_st_cer(struct stream *s) { struct stconn *sc = s->scb; - int must_tar = sc_ep_test(sc, SE_FL_ERROR); + int must_tar = !!(sc->flags & SC_FL_ERROR) || sc_ep_test(sc, SE_FL_ERROR); DBG_TRACE_ENTER(STRM_EV_STRM_PROC|STRM_EV_CS_ST, s); @@ -2312,7 +2312,7 @@ void back_handle_st_cer(struct stream *s) _HA_ATOMIC_DEC(&__objt_server(s->target)->cur_sess); } - if (sc_ep_test(sc, SE_FL_ERROR) && + if (((sc->flags & SC_FL_ERROR) || sc_ep_test(sc, SE_FL_ERROR)) && conn && conn->err_code == CO_ER_SSL_MISMATCH_SNI) { /* We tried to connect to a server which is configured * with "verify required" and which doesn't have the @@ -2490,7 +2490,7 @@ void back_handle_st_rdy(struct stream *s) } /* retryable error ? */ - if (sc_ep_test(sc, SE_FL_ERROR)) { + if (sc->flags & SC_FL_ERROR || sc_ep_test(sc, SE_FL_ERROR)) { if (!s->conn_err_type) s->conn_err_type = STRM_ET_CONN_ERR; sc->state = SC_ST_CER; diff --git a/src/cli.c b/src/cli.c index 5f859b9..14b92a5 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) struct proxy *fe = strm_fe(s); struct proxy *be = s->be; - if (sc_ep_test(s->scb, SE_FL_ERROR) || (rep->flags & (CF_READ_TIMEOUT|CF_WRITE_TIMEOUT)) || + if ((s->scb->flags & SC_FL_ERROR) || sc_ep_test(s->scb, SE_FL_ERROR) || (rep->flags & (CF_READ_TIMEOUT|CF_WRITE_TIMEOUT)) || ((s->scf->flags & SC_FL_SHUT_DONE) && (rep->to_forward || co_data(rep)))) { pcli_reply_and_close(s, "Can't connect to the target CLI!\n"); s->req.analysers &= ~AN_REQ_WAIT_CLI; diff --git a/src/http_ana.c b/src/http_ana.c index bc9865a..087cfe1 100644 --- a/src/http_ana.c +++ b/src/http_ana.c @@ -784,7 +784,7 @@ int http_process_tarpit(struct stream *s, struct channel *req, int an_bit) */ s->logs.t_queue = tv_ms_elapsed(&s->logs.tv_accept, &now); - http_reply_and_close(s, txn->status, (!sc_ep_test(s->scf, SE_FL_ERROR) ? http_error_message(s) : NULL)); + http_reply_and_close(s, txn->status, (!(s->scf->flags & SC_FL_ERROR) && !sc_ep_test(s->scf, SE_FL_ERROR) ? http_error_message(s) : NULL)); http_set_term_flags(s); DBG_TRACE_LEAVE(STRM_EV_STRM_ANA|STRM_EV_HTTP_ANA, s, txn); @@ -1215,7 +1215,7 @@ int http_wait_for_response(struct stream *s, struct channel *rep, int an_bit) next_one: if (unlikely(htx_is_empty(htx) || htx->first == -1)) { /* 1: have we encountered a read error ? */ - if (sc_ep_test(s->scb, SE_FL_ERROR)) { + if ((s->scb->flags & SC_FL_ERROR) || sc_ep_test(s->scb, SE_FL_ERROR)) { struct connection *conn = sc_conn(s->scb); @@ -2672,7 +2672,7 @@ static enum rule_result http_req_get_intercept_rule(struct proxy *px, struct lis /* Always call the action function if defined */ if (rule->action_ptr) { - if (sc_ep_test(s->scf, SE_FL_ERROR) || + if ((s->scf->flags & SC_FL_ERROR) || sc_ep_test(s->scf, SE_FL_ERROR) || ((s->scf->flags & SC_FL_ABRT_DONE) && (px->options & PR_O_ABRT_CLOSE))) act_opts |= ACT_OPT_FINAL; @@ -2835,7 +2835,7 @@ resume_execution: /* Always call the action function if defined */ if (rule->action_ptr) { - if (sc_ep_test(s->scf, SE_FL_ERROR) || + if ((s->scf->flags & SC_FL_ERROR) || sc_ep_test(s->scf, SE_FL_ERROR) || ((s->scf->flags & SC_FL_ABRT_DONE) && (px->options & PR_O_ABRT_CLOSE))) act_opts |= ACT_OPT_FINAL; diff --git a/src/stconn.c b/src/stconn.c index c60afcc..6b2397c 100644 --- a/src/stconn.c +++ b/src/stconn.c @@ -579,8 +579,8 @@ static void sc_app_shut(struct stconn *sc) * However, if SC_FL_NOLINGER is explicitly set, we know there is * no risk so we close both sides immediately. */ - if (!sc_ep_test(sc, SE_FL_ERROR) && !(sc->flags & SC_FL_NOLINGER) && - !(sc->flags & SC_FL_ABRT_DONE) && !(ic->flags & CF_DONT_READ)) + if (!(sc->flags & (SC_FL_ERROR|SC_FL_NOLINGER|SC_FL_ABRT_DONE)) && !sc_ep_test(sc, SE_FL_ERROR) && + !(ic->flags & CF_DONT_READ)) return; __fallthrough; @@ -705,7 +705,7 @@ static void sc_app_shut_conn(struct stconn *sc) * no risk so we close both sides immediately. */ - if (sc_ep_test(sc, SE_FL_ERROR)) { + if ((sc->flags & SC_FL_ERROR) || sc_ep_test(sc, SE_FL_ERROR)) { /* quick close, the socket is already shut anyway */ } else if (sc->flags & SC_FL_NOLINGER) { @@ -903,8 +903,7 @@ static void sc_app_shut_applet(struct stconn *sc) * However, if SC_FL_NOLINGER is explicitly set, we know there is * no risk so we close both sides immediately. */ - if (!sc_ep_test(sc, SE_FL_ERROR) && !(sc->flags & SC_FL_NOLINGER) && - !(sc->flags & SC_FL_ABRT_DONE) && + if (!(sc->flags & (SC_FL_ERROR|SC_FL_NOLINGER|SC_FL_ABRT_DONE)) && !sc_ep_test(sc, SE_FL_ERROR) && !(ic->flags & CF_DONT_READ)) return; @@ -1095,14 +1094,14 @@ static void sc_notify(struct stconn *sc) /* wake the task up only when needed */ if (/* changes on the production side that must be handled: - * - An error on receipt: SE_FL_ERROR + * - An error on receipt: SC_FL_ERROR * - A read event: shutdown for reads (CF_READ_EVENT + ABRT_DONE) * end of input (CF_READ_EVENT + SC_FL_EOI) * data received and no fast-forwarding (CF_READ_EVENT + !to_forward) * read event while consumer side is not established (CF_READ_EVENT + sco->state != SC_ST_EST) */ ((ic->flags & CF_READ_EVENT) && ((sc->flags & SC_FL_EOI) || (sc->flags & SC_FL_ABRT_DONE) || !ic->to_forward || sco->state != SC_ST_EST)) || - sc_ep_test(sc, SE_FL_ERROR) || + (sc->flags & SC_FL_ERROR) || sc_ep_test(sc, SE_FL_ERROR) || /* changes on the consumption side */ sc_ep_test(sc, SE_FL_ERR_PENDING) || diff --git a/src/stream.c b/src/stream.c index 33a5a76..36af89f 100644 --- a/src/stream.c +++ b/src/stream.c @@ -907,7 +907,7 @@ static void back_establish(struct stream *s) s->flags &= ~SF_CONN_EXP; /* errors faced after sending data need to be reported */ - if (sc_ep_test(s->scb, SE_FL_ERROR) && req->flags & CF_WROTE_DATA) { + if (((s->scb->flags & SC_FL_ERROR) || sc_ep_test(s->scb, SE_FL_ERROR)) && req->flags & CF_WROTE_DATA) { s->req.flags |= CF_WRITE_EVENT; s->res.flags |= CF_READ_EVENT; s->conn_err_type = STRM_ET_DATA_ERR; @@ -1783,7 +1783,7 @@ struct task *process_stream(struct task *t, void *context, unsigned int state) * So let's not run a whole stream processing if only an expiration * timeout needs to be refreshed. */ - if (!((scf->flags | scb->flags) & (SC_FL_ABRT_DONE|SC_FL_SHUT_DONE)) && + if (!((scf->flags | scb->flags) & (SC_FL_ERROR|SC_FL_ABRT_DONE|SC_FL_SHUT_DONE)) && !((req->flags | res->flags) & (CF_READ_EVENT|CF_READ_TIMEOUT|CF_WRITE_EVENT|CF_WRITE_TIMEOUT)) && !(s->flags & SF_CONN_EXP) && !((sc_ep_get(scf) | sc_ep_get(scb)) & SE_FL_ERROR) && @@ -1826,7 +1826,7 @@ struct task *process_stream(struct task *t, void *context, unsigned int state) * connection setup code must be able to deal with any type of abort. */ srv = objt_server(s->target); - if (unlikely(sc_ep_test(scf, SE_FL_ERROR))) { + if (unlikely((scf->flags & SC_FL_ERROR) || sc_ep_test(scf, SE_FL_ERROR))) { if (sc_state_in(scf->state, SC_SB_EST|SC_SB_DIS)) { sc_abort(scf); sc_shutdown(scf); @@ -1846,7 +1846,7 @@ struct task *process_stream(struct task *t, void *context, unsigned int state) } } - if (unlikely(sc_ep_test(scb, SE_FL_ERROR))) { + if (unlikely((scb->flags & SC_FL_ERROR) || sc_ep_test(scb, SE_FL_ERROR))) { if (sc_state_in(scb->state, SC_SB_EST|SC_SB_DIS)) { sc_abort(scb); sc_shutdown(scb); @@ -2143,10 +2143,10 @@ struct task *process_stream(struct task *t, void *context, unsigned int state) */ srv = objt_server(s->target); if (unlikely(!(s->flags & SF_ERR_MASK))) { - if (sc_ep_test(s->scf, SE_FL_ERROR) || req->flags & (CF_READ_TIMEOUT|CF_WRITE_TIMEOUT)) { + if ((scf->flags & SC_FL_ERROR) || sc_ep_test(s->scf, SE_FL_ERROR) || req->flags & (CF_READ_TIMEOUT|CF_WRITE_TIMEOUT)) { /* Report it if the client got an error or a read timeout expired */ req->analysers &= AN_REQ_FLT_END; - if (sc_ep_test(s->scf, SE_FL_ERROR)) { + if ((scf->flags & SC_FL_ERROR) || sc_ep_test(s->scf, SE_FL_ERROR)) { _HA_ATOMIC_INC(&s->be->be_counters.cli_aborts); _HA_ATOMIC_INC(&sess->fe->fe_counters.cli_aborts); if (sess->listener && sess->listener->counters) @@ -2188,10 +2188,10 @@ struct task *process_stream(struct task *t, void *context, unsigned int state) channel_erase(req); } } - else if (sc_ep_test(s->scb, SE_FL_ERROR) || res->flags & (CF_READ_TIMEOUT|CF_WRITE_TIMEOUT)) { + else if ((scb->flags & SC_FL_ERROR) || sc_ep_test(s->scb, SE_FL_ERROR) || res->flags & (CF_READ_TIMEOUT|CF_WRITE_TIMEOUT)) { /* Report it if the server got an error or a read timeout expired */ res->analysers &= AN_RES_FLT_END; - if (sc_ep_test(s->scb, SE_FL_ERROR)) { + if ((scb->flags & SC_FL_ERROR) || sc_ep_test(s->scb, SE_FL_ERROR)) { _HA_ATOMIC_INC(&s->be->be_counters.srv_aborts); _HA_ATOMIC_INC(&sess->fe->fe_counters.srv_aborts); if (sess->listener && sess->listener->counters) @@ -2375,7 +2375,7 @@ struct task *process_stream(struct task *t, void *context, unsigned int state) /* shutdown(write) pending */ if (unlikely((scb->flags & (SC_FL_SHUT_DONE|SC_FL_SHUT_WANTED)) == SC_FL_SHUT_WANTED && channel_is_empty(req))) { - if (sc_ep_test(s->scf, SE_FL_ERROR)) + if ((scf->flags & SC_FL_ERROR) || sc_ep_test(s->scf, SE_FL_ERROR)) scb->flags |= SC_FL_NOLINGER; sc_shutdown(scb); } @@ -2396,7 +2396,9 @@ struct task *process_stream(struct task *t, void *context, unsigned int state) if (scf->state == SC_ST_DIS || sc_state_in(scb->state, SC_SB_RDY|SC_SB_DIS) || (sc_ep_test(scf, SE_FL_ERROR) && scf->state != SC_ST_CLO) || - (sc_ep_test(scb, SE_FL_ERROR) && scb->state != SC_ST_CLO)) + (sc_ep_test(scb, SE_FL_ERROR) && scb->state != SC_ST_CLO) || + ((scf->flags & SC_FL_ERROR) && scf->state != SC_ST_CLO) || + ((scb->flags & SC_FL_ERROR) && scb->state != SC_ST_CLO)) goto resync_stconns; /* otherwise we want to check if we need to resync the req buffer or not */ @@ -2515,7 +2517,9 @@ struct task *process_stream(struct task *t, void *context, unsigned int state) if (scf->state == SC_ST_DIS || sc_state_in(scb->state, SC_SB_RDY|SC_SB_DIS) || (sc_ep_test(scf, SE_FL_ERROR) && scf->state != SC_ST_CLO) || - (sc_ep_test(scb, SE_FL_ERROR) && scb->state != SC_ST_CLO)) + (sc_ep_test(scb, SE_FL_ERROR) && scb->state != SC_ST_CLO) || + ((scf->flags & SC_FL_ERROR) && scf->state != SC_ST_CLO) || + ((scb->flags & SC_FL_ERROR) && scb->state != SC_ST_CLO)) goto resync_stconns; if ((req->flags & ~rqf_last) & CF_MASK_ANALYSER) diff --git a/src/tcp_rules.c b/src/tcp_rules.c index d6ba9fb..991c16d 100644 --- a/src/tcp_rules.c +++ b/src/tcp_rules.c @@ -121,7 +121,7 @@ int tcp_inspect_request(struct stream *s, struct channel *req, int an_bit) !s->be->tcp_req.inspect_delay || tick_is_expired(s->rules_exp, now_ms)) { partial = SMP_OPT_FINAL; /* Action may yield while the inspect_delay is not expired and there is no read error */ - if (sc_ep_test(s->scf, SE_FL_ERROR) || !s->be->tcp_req.inspect_delay || tick_is_expired(s->rules_exp, now_ms)) + if ((s->scf->flags & SC_FL_ERROR) || sc_ep_test(s->scf, SE_FL_ERROR) || !s->be->tcp_req.inspect_delay || tick_is_expired(s->rules_exp, now_ms)) act_opts |= ACT_OPT_FINAL; } else @@ -303,7 +303,7 @@ int tcp_inspect_response(struct stream *s, struct channel *rep, int an_bit) !s->be->tcp_rep.inspect_delay || tick_is_expired(s->rules_exp, now_ms)) { partial = SMP_OPT_FINAL; /* Action may yield while the inspect_delay is not expired and there is no read error */ - if (sc_ep_test(s->scb, SE_FL_ERROR) || !s->be->tcp_rep.inspect_delay || tick_is_expired(s->rules_exp, now_ms)) + if ((s->scb->flags & SC_FL_ERROR) || sc_ep_test(s->scb, SE_FL_ERROR) || !s->be->tcp_rep.inspect_delay || tick_is_expired(s->rules_exp, now_ms)) act_opts |= ACT_OPT_FINAL; } else -- 1.7.10.4