MEDIUM: stream-int: remove dangerous interval checks for stream-int states

The stream interface state checks involving ranges were replaced with
checks on a set of states, already revealing some issues. No issue was
fixed, all was replaced in a one-to-one mapping for easier control. Some
checks involving a strict difference were also replaced with fields to
be clearer. At this stage, the result must be strictly equivalent. A few
tests were also turned to their bit-field equivalent for better readability
or in preparation for upcoming changes.

The test performed in the SPOE filter was swapped so that the closed and
error states are evicted first and that the established vs conn state is
tested second.

diff --git a/include/proto/stream_interface.h b/include/proto/stream_interface.h
index 64aa9af949e4f36affdf2e3835d31658748aa6e0..59b5c15ade9288843eb9d408466e834cbf94ec93 100644 (file)
--- a/include/proto/stream_interface.h
+++ b/include/proto/stream_interface.h
@@ -184,7 +184,7 @@ static inline void si_release_endpoint(struct stream_interface *si)
                cs_destroy(cs);
        }
        else if ((appctx = objt_appctx(si->end))) {
-               if (appctx->applet->release && si->state < SI_ST_DIS)
+               if (appctx->applet->release && !si_state_in(si->state, SI_SB_DIS|SI_SB_CLO))
                        appctx->applet->release(appctx);
                appctx_free(appctx); /* we share the connection pool */
        } else if ((conn = objt_conn(si->end))) {
@@ -238,7 +238,7 @@ static inline void si_applet_release(struct stream_interface *si)
        struct appctx *appctx;
 
        appctx = objt_appctx(si->end);
-       if (appctx && appctx->applet->release && si->state < SI_ST_DIS)
+       if (appctx && appctx->applet->release && !si_state_in(si->state, SI_SB_DIS|SI_SB_CLO))
                appctx->applet->release(appctx);
 }
 
@@ -448,13 +448,13 @@ static inline void si_must_kill_conn(struct stream_interface *si)
  */
 static inline void si_chk_rcv(struct stream_interface *si)
 {
-       if (si->flags & SI_FL_RXBLK_CONN && (si_opposite(si)->state >= SI_ST_EST))
+       if (si->flags & SI_FL_RXBLK_CONN && si_state_in(si_opposite(si)->state, SI_SB_EST|SI_SB_DIS|SI_SB_CLO))
                si_rx_conn_rdy(si);
 
        if (si_rx_blocked(si) || !si_rx_endp_ready(si))
                return;
 
-       if (si->state != SI_ST_EST)
+       if (!si_state_in(si->state, SI_SB_EST))
                return;
 
        si->flags |= SI_FL_RX_WAIT_EP;
@@ -472,7 +472,7 @@ static inline int si_sync_recv(struct stream_interface *si)
 {
        struct conn_stream *cs;
 
-       if (si->state != SI_ST_EST)
+       if (!si_state_in(si->state, SI_SB_EST))
                return 0;
 
        cs = objt_cs(si->end);

diff --git a/src/flt_spoe.c b/src/flt_spoe.c
index 51d4f275a855b70ff5b8e17f52a1f8c1c1894c2c..764e9babfd9f619d7f672c9e7a801fa96ed641e9 100644 (file)
--- a/src/flt_spoe.c
+++ b/src/flt_spoe.c
@@ -1341,15 +1341,18 @@ spoe_handle_connect_appctx(struct appctx *appctx)
        char *frame, *buf;
        int   ret;
 
-       if (si->state <= SI_ST_CON) {
+       if (si_state_in(si->state, SI_SB_CER|SI_SB_DIS|SI_SB_CLO)) {
+               /* closed */
+               SPOE_APPCTX(appctx)->status_code = SPOE_FRM_ERR_IO;
+               goto exit;
+       }
+
+       if (!si_state_in(si->state, SI_SB_EST)) {
+               /* not connected yet */
                si_rx_endp_more(si);
                task_wakeup(si_strm(si)->task, TASK_WOKEN_MSG);
                goto stop;
        }
-       if (si->state != SI_ST_EST) {
-               SPOE_APPCTX(appctx)->status_code = SPOE_FRM_ERR_IO;
-               goto exit;
-       }
 
        if (appctx->st1 == SPOE_APPCTX_ERR_TOUT) {
                SPOE_PRINTF(stderr, "%d.%06d [SPOE/%-15s] %s: appctx=%p"

diff --git a/src/stream.c b/src/stream.c
index bf1dd123e95ff629afb06e423893d30ebaf23198..a5997e5466d6bc047691b09e215bdcd8a4225264 100644 (file)
--- a/src/stream.c
+++ b/src/stream.c
@@ -1138,8 +1138,8 @@ abort_connection:
 static void sess_set_term_flags(struct stream *s)
 {
        if (!(s->flags & SF_FINST_MASK)) {
-               if (s->si[1].state < SI_ST_REQ) {
-
+               if (s->si[1].state == SI_ST_INI) {
+                       /* anything before REQ in fact */
                        _HA_ATOMIC_ADD(&strm_fe(s)->fe_counters.failed_req, 1);
                        if (strm_li(s) && strm_li(s)->counters)
                                _HA_ATOMIC_ADD(&strm_li(s)->counters->failed_req, 1);
@@ -1148,7 +1148,7 @@ static void sess_set_term_flags(struct stream *s)
                }
                else if (s->si[1].state == SI_ST_QUE)
                        s->flags |= SF_FINST_Q;
-               else if (s->si[1].state < SI_ST_EST)
+               else if (si_state_in(s->si[1].state, SI_SB_REQ|SI_SB_TAR|SI_SB_ASS|SI_SB_CON|SI_SB_CER))
                        s->flags |= SF_FINST_C;
                else if (s->si[1].state == SI_ST_EST || s->si[1].prev_state == SI_ST_EST)
                        s->flags |= SF_FINST_D;
@@ -1906,7 +1906,7 @@ redo:
         */
        srv = objt_server(s->target);
        if (unlikely(si_f->flags & SI_FL_ERR)) {
-               if (si_f->state == SI_ST_EST || si_f->state == SI_ST_DIS) {
+               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);
@@ -1924,7 +1924,7 @@ redo:
        }
 
        if (unlikely(si_b->flags & SI_FL_ERR)) {
-               if (si_b->state == SI_ST_EST || si_b->state == SI_ST_DIS) {
+               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);
@@ -1945,7 +1945,7 @@ redo:
                /* note: maybe we should process connection errors here ? */
        }
 
-       if (si_b->state == SI_ST_CON) {
+       if (si_state_in(si_b->state, SI_SB_CON)) {
                /* we were trying to establish a connection on the server side,
                 * maybe it succeeded, maybe it failed, maybe we timed out, ...
                 */
@@ -2016,7 +2016,7 @@ redo:
            s->pending_events & TASK_WOKEN_MSG) {
                unsigned int flags = req->flags;
 
-               if (si_f->state >= SI_ST_EST) {
+               if (si_state_in(si_f->state, SI_SB_EST|SI_SB_DIS|SI_SB_CLO)) {
                        int max_loops = global.tune.maxpollevents;
                        unsigned int ana_list;
                        unsigned int ana_back;
@@ -2117,7 +2117,7 @@ redo:
                 s->pending_events & TASK_WOKEN_MSG) {
                unsigned int flags = res->flags;
 
-               if (si_b->state >= SI_ST_EST) {
+               if (si_state_in(si_b->state, SI_SB_EST|SI_SB_DIS|SI_SB_CLO)) {
                        int max_loops = global.tune.maxpollevents;
                        unsigned int ana_list;
                        unsigned int ana_back;
@@ -2282,7 +2282,7 @@ redo:
         */
        if (unlikely((!req->analysers || (req->analysers == AN_REQ_FLT_END && !(req->flags & CF_FLT_ANALYZE))) &&
            !(req->flags & (CF_SHUTW|CF_SHUTR_NOW)) &&
-           (si_f->state >= SI_ST_EST) &&
+           (si_state_in(si_f->state, SI_SB_EST|SI_SB_DIS|SI_SB_CLO)) &&
            (req->to_forward != CHN_INFINITE_FORWARD))) {
                /* This buffer is freewheeling, there's no analyser
                 * attached to it. If any data are left in, we'll permit them to
@@ -2401,8 +2401,7 @@ redo:
        /* we may have a pending connection request, or a connection waiting
         * for completion.
         */
-       if (si_b->state >= SI_ST_REQ && si_b->state < SI_ST_CON) {
-
+       if (si_state_in(si_b->state, SI_SB_REQ|SI_SB_QUE|SI_SB_TAR|SI_SB_ASS)) {
                /* prune the request variables and swap to the response variables. */
                if (s->vars_reqres.scope != SCOPE_RES) {
                        if (!LIST_ISEMPTY(&s->vars_reqres.head)) {
@@ -2429,7 +2428,7 @@ redo:
 
                        /* Now we can add the server name to a header (if requested) */
                        /* check for HTTP mode and proxy server_name_hdr_name != NULL */
-                       if ((si_b->state == SI_ST_CON || si_b->state == SI_ST_EST) &&
+                       if (si_state_in(si_b->state, SI_SB_CON|SI_SB_EST) &&
                            (s->be->server_id_hdr_name != NULL) &&
                            (s->be->mode == PR_MODE_HTTP) &&
                            objt_server(s->target)) {
@@ -2459,7 +2458,7 @@ redo:
         */
        if (unlikely((!res->analysers || (res->analysers == AN_RES_FLT_END && !(res->flags & CF_FLT_ANALYZE))) &&
            !(res->flags & (CF_SHUTW|CF_SHUTR_NOW)) &&
-           (si_b->state >= SI_ST_EST) &&
+           si_state_in(si_b->state, SI_SB_EST|SI_SB_DIS|SI_SB_CLO) &&
            (res->to_forward != CHN_INFINITE_FORWARD))) {
                /* This buffer is freewheeling, there's no analyser
                 * attached to it. If any data are left in, we'll permit them to
@@ -2607,8 +2606,7 @@ redo:
                }
        }
 
-       if (likely((si_f->state != SI_ST_CLO) ||
-                  (si_b->state > SI_ST_INI && si_b->state < SI_ST_CLO))) {
+       if (likely((si_f->state != SI_ST_CLO) || !si_state_in(si_b->state, SI_SB_INI|SI_SB_CLO))) {
                enum si_state si_b_prev_state, si_f_prev_state;
 
                si_f_prev_state = si_f->prev_state;

diff --git a/src/stream_interface.c b/src/stream_interface.c
index bdc9b0079d918dfd918e5067123e09cbb1b4e504..0a00b165ab32d6c0e69588a658be79af53757a46 100644 (file)
--- a/src/stream_interface.c
+++ b/src/stream_interface.c
@@ -175,7 +175,7 @@ static void stream_int_shutr(struct stream_interface *si)
        ic->flags |= CF_SHUTR;
        ic->rex = TICK_ETERNITY;
 
-       if (si->state != SI_ST_EST && si->state != SI_ST_CON)
+       if (!si_state_in(si->state, SI_SB_CON|SI_SB_EST))
                return;
 
        if (si_oc(si)->flags & CF_SHUTW) {
@@ -541,7 +541,7 @@ static void stream_int_notify(struct stream_interface *si)
        /* wake the task up only when needed */
        if (/* changes on the production side */
            (ic->flags & (CF_READ_NULL|CF_READ_ERROR)) ||
-           (si->state != SI_ST_EST && si->state != SI_ST_CON) ||
+           !si_state_in(si->state, SI_SB_CON|SI_SB_EST) ||
            (si->flags & SI_FL_ERR) ||
            ((ic->flags & CF_READ_PARTIAL) &&
             ((ic->flags & CF_EOI) || !ic->to_forward || sio->state != SI_ST_EST)) ||
@@ -602,7 +602,7 @@ static int si_cs_process(struct conn_stream *cs)
                task_wakeup(si_task(si), TASK_WOKEN_MSG);
        }
 
-       if ((si->state < SI_ST_EST) &&
+       if (!si_state_in(si->state, SI_SB_EST|SI_SB_DIS|SI_SB_CLO) &&
            (conn->flags & (CO_FL_CONNECTED | CO_FL_HANDSHAKE)) == CO_FL_CONNECTED) {
                si->exp = TICK_ETERNITY;
                oc->flags |= CF_WRITE_NULL;
@@ -893,7 +893,7 @@ void si_update_both(struct stream_interface *si_f, struct stream_interface *si_b
        /* back stream-int */
        cs = objt_cs(si_b->end);
        if (cs &&
-           (si_b->state == SI_ST_EST || si_b->state == SI_ST_CON) &&
+           si_state_in(si_b->state, SI_SB_CON|SI_SB_EST) &&
            !(req->flags & CF_SHUTW) && /* Write not closed */
            !channel_is_empty(req) &&
            !(cs->flags & (CS_FL_ERROR|CS_FL_ERR_PENDING)) &&
@@ -903,10 +903,10 @@ void si_update_both(struct stream_interface *si_f, struct stream_interface *si_b
        }
 
        /* let's recompute both sides states */
-       if (si_f->state == SI_ST_EST)
+       if (si_state_in(si_f->state, SI_SB_EST))
                si_update(si_f);
 
-       if (si_b->state == SI_ST_EST)
+       if (si_state_in(si_b->state, SI_SB_EST))
                si_update(si_b);
 
        /* stream ints are processed outside of process_stream() and must be
@@ -944,7 +944,7 @@ static void stream_int_shutr_conn(struct stream_interface *si)
        ic->flags |= CF_SHUTR;
        ic->rex = TICK_ETERNITY;
 
-       if (si->state != SI_ST_EST && si->state != SI_ST_CON)
+       if (!si_state_in(si->state, SI_SB_CON|SI_SB_EST))
                return;
 
        if (si->flags & SI_FL_KILL_CONN)
@@ -1060,7 +1060,7 @@ static void stream_int_shutw_conn(struct stream_interface *si)
 static void stream_int_chk_rcv_conn(struct stream_interface *si)
 {
        /* (re)start reading */
-       if (si->state == SI_ST_CON || si->state == SI_ST_EST)
+       if (si_state_in(si->state, SI_SB_CON|SI_SB_EST))
                tasklet_wakeup(si->wait_event.task);
 }
 
@@ -1075,7 +1075,7 @@ static void stream_int_chk_snd_conn(struct stream_interface *si)
        struct channel *oc = si_oc(si);
        struct conn_stream *cs = __objt_cs(si->end);
 
-       if (unlikely((si->state != SI_ST_CON && si->state != SI_ST_EST) ||
+       if (unlikely(!si_state_in(si->state, SI_SB_CON|SI_SB_EST) ||
            (oc->flags & CF_SHUTW)))
                return;
 
@@ -1106,7 +1106,7 @@ static void stream_int_chk_snd_conn(struct stream_interface *si)
                 */
                if (((oc->flags & (CF_SHUTW|CF_AUTO_CLOSE|CF_SHUTW_NOW)) ==
                     (CF_AUTO_CLOSE|CF_SHUTW_NOW)) &&
-                   (si->state == SI_ST_EST)) {
+                   si_state_in(si->state, SI_SB_EST)) {
                        si_shutw(si);
                        goto out_wakeup;
                }
@@ -1151,7 +1151,7 @@ static void stream_int_chk_snd_conn(struct stream_interface *si)
        if (likely((oc->flags & (CF_WRITE_NULL|CF_WRITE_ERROR|CF_SHUTW)) ||
                  ((oc->flags & CF_WAKE_WRITE) &&
                   ((channel_is_empty(oc) && !oc->to_forward) ||
-                   si->state != SI_ST_EST)))) {
+                   !si_state_in(si->state, SI_SB_EST))))) {
        out_wakeup:
                if (!(si->flags & SI_FL_DONT_WAKE))
                        task_wakeup(si_task(si), TASK_WOKEN_IO);
@@ -1471,7 +1471,7 @@ static void stream_int_read0(struct stream_interface *si)
        ic->flags |= CF_SHUTR;
        ic->rex = TICK_ETERNITY;
 
-       if (si->state != SI_ST_EST && si->state != SI_ST_CON)
+       if (!si_state_in(si->state, SI_SB_CON|SI_SB_EST))
                return;
 
        if (oc->flags & CF_SHUTW)
@@ -1561,7 +1561,7 @@ static void stream_int_shutr_applet(struct stream_interface *si)
 
        /* Note: on shutr, we don't call the applet */
 
-       if (si->state != SI_ST_EST && si->state != SI_ST_CON)
+       if (!si_state_in(si->state, SI_SB_CON|SI_SB_EST))
                return;
 
        if (si_oc(si)->flags & CF_SHUTW) {