MINOR: stream-int/conn-stream: Report error to the CS instead of the SI

SI_FL_ERR is removed and replaced by CS_FL_ERROR. It is a transient patch
because the idea is to rely on the endpoint to handle errors at this
level. But if for any reason it is not possible, the stream-interface flags
will still be replaced.

diff --git a/dev/flags/flags.c b/dev/flags/flags.c
index f43fe3a6c975825128cf105636a20cfcaa4a7948..c38d90efec5bcde87a04cae144cb519f60e83ce0 100644 (file)
--- a/dev/flags/flags.c
+++ b/dev/flags/flags.c
@@ -219,6 +219,7 @@ void show_cs_flags(unsigned int f)
                printf("0\n");
                return;
        }
+       SHOW_FLAG(f, CS_FL_ERR);
        SHOW_FLAG(f, CS_FL_ADDR_FROM_SET);
        SHOW_FLAG(f, CS_FL_ADDR_TO_SET);
        SHOW_FLAG(f, CS_FL_ISBACK);
@@ -263,7 +264,6 @@ void show_si_flags(unsigned int f)
                return;
        }
 
-       SHOW_FLAG(f, SI_FL_ERR);
        SHOW_FLAG(f, SI_FL_KILL_CONN);
        SHOW_FLAG(f, SI_FL_WAIT_DATA);
        SHOW_FLAG(f, SI_FL_ISBACK);

diff --git a/include/haproxy/conn_stream-t.h b/include/haproxy/conn_stream-t.h
index 55f338b095b1a33f2a59a58e61cfdf45bfabf85a..8530586cd06be86dd7cf80d330083c04a6ca4423 100644 (file)
--- a/include/haproxy/conn_stream-t.h
+++ b/include/haproxy/conn_stream-t.h
@@ -82,6 +82,8 @@ enum {
 
        CS_FL_ADDR_FROM_SET = 0x00000002, /* source address is set */
        CS_FL_ADDR_TO_SET   = 0x00000004, /* destination address is set */
+
+       CS_FL_ERR           = 0x00000008,  /* a non-recoverable error has occurred */
 };
 
 /* cs_shutr() modes */

diff --git a/include/haproxy/cs_utils.h b/include/haproxy/cs_utils.h
index 7381499dc125b327e05931d3202fedcd62630294..2346a8577b7cec32533418d4cb50682a74b86302 100644 (file)
--- a/include/haproxy/cs_utils.h
+++ b/include/haproxy/cs_utils.h
@@ -76,6 +76,16 @@ static inline struct conn_stream *cs_opposite(struct conn_stream *cs)
 }
 
 
+/* to be called only when in SI_ST_DIS with SI_FL_ERR */
+static inline void cs_report_error(struct conn_stream *cs)
+{
+       if (!cs->si->err_type)
+               cs->si->err_type = SI_ET_DATA_ERR;
+
+       cs_oc(cs)->flags |= CF_WRITE_ERROR;
+       cs_ic(cs)->flags |= CF_READ_ERROR;
+}
+
 /* Returns the source address of the conn-stream and, if not set, fallbacks on
  * the session for frontend CS and the server connection for the backend CS. It
  * returns a const address on success or NULL on failure.

diff --git a/include/haproxy/stream_interface-t.h b/include/haproxy/stream_interface-t.h
index 50b75d7d5643cbe80bc277d66048275047b497a1..5fe805ed1046280e31a467f1a8127f56be339ed5 100644 (file)
--- a/include/haproxy/stream_interface-t.h
+++ b/include/haproxy/stream_interface-t.h
@@ -83,8 +83,7 @@ enum {
 /* flags set after I/O (32 bit) */
 enum {
        SI_FL_NONE       = 0x00000000,  /* nothing */
-       /* unused: 0x00000001  */
-       SI_FL_ERR        = 0x00000002,  /* a non-recoverable error has occurred */
+       /* unused: 0x00000001, 0x00000002 */
        SI_FL_KILL_CONN  = 0x00000004,  /* next shutw must kill the whole conn, not just the stream */
        SI_FL_WAIT_DATA  = 0x00000008,  /* stream-int waits for more outgoing data to send */
        SI_FL_ISBACK     = 0x00000010,  /* 0 for front-side SI, 1 for back-side */

diff --git a/include/haproxy/stream_interface.h b/include/haproxy/stream_interface.h
index 6372c1a30ac36fa86539ee5e8fd113cffe28cf9b..4424a7e7a623249c7ffafe5241356b690e4ec3a4 100644 (file)
--- a/include/haproxy/stream_interface.h
+++ b/include/haproxy/stream_interface.h
@@ -39,7 +39,7 @@ struct stream_interface *si_new(struct conn_stream *cs);
 void si_free(struct stream_interface *si);
 
 /* main event functions used to move data between sockets and buffers */
-void si_report_error(struct stream_interface *si);
+
 void si_retnclose(struct stream_interface *si, const struct buffer *msg);
 int conn_si_send_proxy(struct connection *conn, unsigned int flag);
 struct appctx *si_register_handler(struct stream_interface *si, struct applet *app);

diff --git a/src/backend.c b/src/backend.c
index 2d7d91d8e15f6f51afa6b956747203faea79cb3e..7250aa9a81f2e8d60da0c22564feba760e075ed3 100644 (file)
--- a/src/backend.c
+++ b/src/backend.c
@@ -1741,11 +1741,11 @@ skip_reuse:
 
        /* Now handle synchronously connected sockets. We know the stream-int
         * is at least in state SI_ST_CON. These ones typically are UNIX
-        * sockets, socket pairs, and occasionally TCP connections on the
+        * sockets, socket pairs, andoccasionally TCP connections on the
         * loopback on a heavily loaded system.
         */
        if ((srv_conn->flags & CO_FL_ERROR || s->csb->endp->flags & CS_EP_ERROR))
-               cs_si(s->csb)->flags |= SI_FL_ERR;
+               s->csb->flags |= CS_FL_ERR;
 
        /* If we had early data, and the handshake ended, then
         * we can remove the flag, and attempt to wake the task up,
@@ -1964,7 +1964,7 @@ void back_try_conn_req(struct stream *s)
                 * allocation problem, so we want to retry now.
                 */
                cs->si->state = SI_ST_CER;
-               cs->si->flags &= ~SI_FL_ERR;
+               cs->flags &= ~CS_FL_ERR;
                back_handle_st_cer(s);
 
                DBG_TRACE_STATE("connection error, retry", STRM_EV_STRM_PROC|STRM_EV_SI_ST|STRM_EV_STRM_ERR, s);
@@ -2187,9 +2187,9 @@ void back_handle_st_con(struct stream *s)
 
  done:
        /* retryable error ? */
-       if ((s->flags & SF_CONN_EXP) || (cs->si->flags & SI_FL_ERR)) {
+       if ((s->flags & SF_CONN_EXP) || (cs->flags & CS_FL_ERR)) {
                if (!cs->si->err_type) {
-                       if (cs->si->flags & SI_FL_ERR)
+                       if (cs->flags & CS_FL_ERR)
                                cs->si->err_type = SI_ET_CONN_ERR;
                        else
                                cs->si->err_type = SI_ET_CONN_TO;
@@ -2234,7 +2234,7 @@ void back_handle_st_cer(struct stream *s)
                        _HA_ATOMIC_DEC(&__objt_server(s->target)->cur_sess);
                }
 
-               if ((cs->si->flags & SI_FL_ERR) &&
+               if ((cs->flags & CS_FL_ERR) &&
                    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
@@ -2291,7 +2291,7 @@ void back_handle_st_cer(struct stream *s)
         * layers in an unexpected state (i.e < ST_CONN).
         *
         * Note: the stream-interface will be switched to ST_REQ, ST_ASS or
-        * ST_TAR and SI_FL_ERR and SF_CONN_EXP flags will be unset.
+        * ST_TAR and CS_FL_ERR and SF_CONN_EXP flags will be unset.
         */
        if (cs_reset_endp(cs) < 0) {
                if (!cs->si->err_type)
@@ -2319,7 +2319,7 @@ void back_handle_st_cer(struct stream *s)
 
        stream_choose_redispatch(s);
 
-       if (cs->si->flags & SI_FL_ERR) {
+       if (cs->flags & CS_FL_ERR) {
                /* The error was an asynchronous connection error, and we will
                 * likely have to retry connecting to the same server, most
                 * likely leading to the same result. To avoid this, we wait
@@ -2345,7 +2345,7 @@ void back_handle_st_cer(struct stream *s)
                        cs->si->state = SI_ST_TAR;
                        s->conn_exp = tick_add(now_ms, MS_TO_TICKS(delay));
                }
-               cs->si->flags &= ~SI_FL_ERR;
+               cs->flags &= ~CS_FL_ERR;
                DBG_TRACE_STATE("retry a new connection", STRM_EV_STRM_PROC|STRM_EV_SI_ST, s);
        }
 
@@ -2401,7 +2401,7 @@ void back_handle_st_rdy(struct stream *s)
                }
 
                /* retryable error ? */
-               if (cs->si->flags & SI_FL_ERR) {
+               if (cs->flags & CS_FL_ERR) {
                        if (!cs->si->err_type)
                                cs->si->err_type = SI_ET_CONN_ERR;
                        cs->si->state = SI_ST_CER;

diff --git a/src/cli.c b/src/cli.c
index b5330db1991ca66b550f94ba661ce91f3e09b31f..35d2d36dcd86e6b714c3353b2a890e61a92f23c5 100644 (file)
--- a/src/cli.c
+++ b/src/cli.c
@@ -1084,7 +1084,7 @@ static void cli_io_handler(struct appctx *appctx)
                                        }
                                break;
                        default: /* abnormal state */
-                               cs->si->flags |= SI_FL_ERR;
+                               cs->flags |= CS_FL_ERR;
                                break;
                        }
 

diff --git a/src/http_ana.c b/src/http_ana.c
index f87db34c6a257266c3ef77238a6bf6bc13e99437..927f0b96c3083b03de00c34055062ab217be8009 100644 (file)
--- a/src/http_ana.c
+++ b/src/http_ana.c
@@ -1247,8 +1247,9 @@ static __inline int do_l7_retry(struct stream *s, struct stream_interface *si)
        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);
        res->analysers &= AN_RES_FLT_END;
-       si->flags &= ~(SI_FL_ERR | SI_FL_RXBLK_SHUT);
+       si->flags &= ~SI_FL_RXBLK_SHUT;
        si->err_type = SI_ET_NONE;
+       si->cs->flags &= ~CS_FL_ERR;
        s->flags &= ~(SF_CONN_EXP | SF_ERR_MASK | SF_FINST_MASK);
        s->conn_exp = TICK_ETERNITY;
        stream_choose_redispatch(s);

diff --git a/src/stream.c b/src/stream.c
index c80f63c92e1e04fad48a3843fc72a17766caee2a..e441b6ed225f7575d3419ce71949ea0d34f129cf 100644 (file)
--- a/src/stream.c
+++ b/src/stream.c
@@ -880,7 +880,7 @@ static void back_establish(struct stream *s)
        s->flags &= ~SF_CONN_EXP;
 
        /* errors faced after sending data need to be reported */
-       if (si->flags & SI_FL_ERR && req->flags & CF_WROTE_DATA) {
+       if (si->cs->flags & CS_FL_ERR && req->flags & CF_WROTE_DATA) {
                /* Don't add CF_WRITE_ERROR if we're here because
                 * early data were rejected by the server, or
                 * http_wait_for_response() will never be called
@@ -1673,7 +1673,7 @@ struct task *process_stream(struct task *t, void *context, unsigned int state)
                      (CF_SHUTR|CF_READ_ACTIVITY|CF_READ_TIMEOUT|CF_SHUTW|
                       CF_WRITE_ACTIVITY|CF_WRITE_TIMEOUT|CF_ANA_TIMEOUT)) &&
                    !(s->flags & SF_CONN_EXP) &&
-                   !((si_f->flags | si_b->flags) & SI_FL_ERR) &&
+                   !((si_f->cs->flags | si_b->cs->flags) & CS_FL_ERR) &&
                    ((s->pending_events & TASK_WOKEN_ANY) == TASK_WOKEN_TIMER)) {
                        si_f->flags &= ~SI_FL_DONT_WAKE;
                        si_b->flags &= ~SI_FL_DONT_WAKE;
@@ -1692,10 +1692,10 @@ struct task *process_stream(struct task *t, void *context, unsigned int state)
         *       must be be reviewed too.
         */
        if (!stream_alloc_work_buffer(s)) {
-               si_f->flags |= SI_FL_ERR;
+               si_f->cs->flags |= CS_FL_ERR;
                si_f->err_type = SI_ET_CONN_RES;
 
-               si_b->flags |= SI_FL_ERR;
+               si_b->cs->flags |= CS_FL_ERR;
                si_b->err_type = SI_ET_CONN_RES;
 
                if (!(s->flags & SF_ERR_MASK))
@@ -1711,11 +1711,11 @@ 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(si_f->flags & SI_FL_ERR)) {
+       if (unlikely(si_f->cs->flags & CS_FL_ERR)) {
                if (si_state_in(si_f->state, SI_SB_EST|SI_SB_DIS)) {
                        si_shutr(si_f);
                        si_shutw(si_f);
-                       si_report_error(si_f);
+                       cs_report_error(si_f->cs);
                        if (!(req->analysers) && !(res->analysers)) {
                                _HA_ATOMIC_INC(&s->be->be_counters.cli_aborts);
                                _HA_ATOMIC_INC(&sess->fe->fe_counters.cli_aborts);
@@ -1731,11 +1731,11 @@ struct task *process_stream(struct task *t, void *context, unsigned int state)
                }
        }
 
-       if (unlikely(si_b->flags & SI_FL_ERR)) {
+       if (unlikely(si_b->cs->flags & CS_FL_ERR)) {
                if (si_state_in(si_b->state, SI_SB_EST|SI_SB_DIS)) {
                        si_shutr(si_b);
                        si_shutw(si_b);
-                       si_report_error(si_b);
+                       cs_report_error(si_b->cs);
                        _HA_ATOMIC_INC(&s->be->be_counters.failed_resp);
                        if (srv)
                                _HA_ATOMIC_INC(&srv->counters.failed_resp);
@@ -2255,8 +2255,8 @@ struct task *process_stream(struct task *t, void *context, unsigned int state)
        /* Benchmarks have shown that it's optimal to do a full resync now */
        if (si_f->state == SI_ST_DIS ||
            si_state_in(si_b->state, SI_SB_RDY|SI_SB_DIS) ||
-           (si_f->flags & SI_FL_ERR && si_f->state != SI_ST_CLO) ||
-           (si_b->flags & SI_FL_ERR && si_b->state != SI_ST_CLO))
+           (si_f->cs->flags & CS_FL_ERR && si_f->state != SI_ST_CLO) ||
+           (si_b->cs->flags & CS_FL_ERR && si_b->state != SI_ST_CLO))
                goto resync_stream_interface;
 
        /* otherwise we want to check if we need to resync the req buffer or not */
@@ -2379,8 +2379,8 @@ struct task *process_stream(struct task *t, void *context, unsigned int state)
 
        if (si_f->state == SI_ST_DIS ||
            si_state_in(si_b->state, SI_SB_RDY|SI_SB_DIS) ||
-           (si_f->flags & SI_FL_ERR && si_f->state != SI_ST_CLO) ||
-           (si_b->flags & SI_FL_ERR && si_b->state != SI_ST_CLO))
+           (si_f->cs->flags & CS_FL_ERR && si_f->state != SI_ST_CLO) ||
+           (si_b->cs->flags & CS_FL_ERR && si_b->state != SI_ST_CLO))
                goto resync_stream_interface;
 
        if ((req->flags & ~rqf_last) & CF_MASK_ANALYSER)

diff --git a/src/stream_interface.c b/src/stream_interface.c
index 001a34d08146c077f03bdaece1e5ae6f910695ed..395fcf818877285fce95698bf6acf234ee58b589 100644 (file)
--- a/src/stream_interface.c
+++ b/src/stream_interface.c
@@ -128,16 +128,6 @@ void si_free(struct stream_interface *si)
        pool_free(pool_head_streaminterface, si);
 }
 
-/* to be called only when in SI_ST_DIS with SI_FL_ERR */
-void si_report_error(struct stream_interface *si)
-{
-       if (!si->err_type)
-               si->err_type = SI_ET_DATA_ERR;
-
-       si_oc(si)->flags |= CF_WRITE_ERROR;
-       si_ic(si)->flags |= CF_READ_ERROR;
-}
-
 /*
  * Returns a message to the client ; the connection is shut down for read,
  * and the request is cleared so that no server connection can be initiated.
@@ -235,7 +225,7 @@ static void stream_int_shutw(struct stream_interface *si)
                 * However, if SI_FL_NOLINGER is explicitly set, we know there is
                 * no risk so we close both sides immediately.
                 */
-               if (!(si->flags & (SI_FL_ERR | SI_FL_NOLINGER)) &&
+               if (!(si->cs->flags & CS_FL_ERR) && !(si->flags & SI_FL_NOLINGER) &&
                    !(ic->flags & (CF_SHUTR|CF_DONT_READ)))
                        return;
 
@@ -531,7 +521,7 @@ static void stream_int_notify(struct stream_interface *si)
        if (/* changes on the production side */
            (ic->flags & (CF_READ_NULL|CF_READ_ERROR)) ||
            !si_state_in(si->state, SI_SB_CON|SI_SB_RDY|SI_SB_EST) ||
-           (si->flags & SI_FL_ERR) ||
+           (si->cs->flags & CS_FL_ERR) ||
            ((ic->flags & CF_READ_PARTIAL) &&
             ((ic->flags & CF_EOI) || !ic->to_forward || sio->state != SI_ST_EST)) ||
 
@@ -598,13 +588,13 @@ static int si_cs_process(struct conn_stream *cs)
        if (!channel_is_empty(oc) && !(si->wait_event.events & SUB_RETRY_SEND))
                si_cs_send(cs);
 
-       /* First step, report to the stream-int what was detected at the
+       /* First step, report to the conn-stream what was detected at the
         * connection layer : errors and connection establishment.
-        * Only add SI_FL_ERR if we're connected, or we're attempting to
+        * Only add CS_FL_ERR if we're connected, or we're attempting to
         * connect, we may get there because we got woken up, but only run
         * after process_stream() noticed there were an error, and decided
         * to retry to connect, the connection may still have CO_FL_ERROR,
-        * and we don't want to add SI_FL_ERR back
+        * and we don't want to add CS_FL_ERR back
         *
         * Note: This test is only required because si_cs_process is also the SI
         *       wake callback. Otherwise si_cs_recv()/si_cs_send() already take
@@ -613,7 +603,7 @@ static int si_cs_process(struct conn_stream *cs)
 
        if (si->state >= SI_ST_CON) {
                if ((cs->endp->flags & CS_EP_ERROR) || si_is_conn_error(si))
-                       si->flags |= SI_FL_ERR;
+                       si->cs->flags |= CS_FL_ERR;
        }
 
        /* If we had early data, and the handshake ended, then
@@ -689,11 +679,11 @@ static int si_cs_send(struct conn_stream *cs)
                 * but process_stream() ran before, detected there were an
                 * error and put the si back to SI_ST_TAR. There's still
                 * CO_FL_ERROR on the connection but we don't want to add
-                * SI_FL_ERR back, so give up
+                * CS_FL_ERR back, so give up
                 */
                if (si->state < SI_ST_CON)
                        return 0;
-               si->flags |= SI_FL_ERR;
+               si->cs->flags |= CS_FL_ERR;
                return 1;
        }
 
@@ -807,7 +797,7 @@ static int si_cs_send(struct conn_stream *cs)
        }
 
        if (cs->endp->flags & (CS_EP_ERROR|CS_EP_ERR_PENDING)) {
-               si->flags |= SI_FL_ERR;
+               si->cs->flags |= CS_FL_ERR;
                return 1;
        }
 
@@ -1102,7 +1092,7 @@ static void stream_int_shutw_conn(struct stream_interface *si)
                if (si->flags & SI_FL_KILL_CONN)
                        cs->endp->flags |= CS_EP_KILL_CONN;
 
-               if (si->flags & SI_FL_ERR) {
+               if (si->cs->flags & CS_FL_ERR) {
                        /* quick close, the socket is already shut anyway */
                }
                else if (si->flags & SI_FL_NOLINGER) {
@@ -1192,7 +1182,7 @@ static void stream_int_chk_snd_conn(struct stream_interface *si)
        if (cs->endp->flags & (CS_EP_ERROR|CS_EP_ERR_PENDING) || si_is_conn_error(si)) {
                /* Write error on the file descriptor */
                if (si->state >= SI_ST_CON)
-                       si->flags |= SI_FL_ERR;
+                       si->cs->flags |= CS_FL_ERR;
                goto out_wakeup;
        }
 
@@ -1569,7 +1559,7 @@ static int si_cs_recv(struct conn_stream *cs)
        }
 
        if (cs->endp->flags & CS_EP_ERROR) {
-               si->flags |= SI_FL_ERR;
+               si->cs->flags |= CS_FL_ERR;
                ret = 1;
        }
        else if (cs->endp->flags & CS_EP_EOS) {
@@ -1656,7 +1646,7 @@ void si_applet_wake_cb(struct stream_interface *si)
         * broken pipe and it must be reported.
         */
        if (!(si->flags & SI_FL_RX_WAIT_EP) && (ic->flags & CF_SHUTR))
-               si->flags |= SI_FL_ERR;
+               si->cs->flags |= CS_FL_ERR;
 
        /* automatically mark the applet having data available if it reported
         * begin blocked by the channel.
@@ -1752,7 +1742,7 @@ static void stream_int_shutw_applet(struct stream_interface *si)
                 * However, if SI_FL_NOLINGER is explicitly set, we know there is
                 * no risk so we close both sides immediately.
                 */
-               if (!(si->flags & (SI_FL_ERR | SI_FL_NOLINGER)) &&
+               if (!(si->cs->flags & CS_FL_ERR) && !(si->flags & SI_FL_NOLINGER) &&
                    !(ic->flags & (CF_SHUTR|CF_DONT_READ)))
                        return;