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 64aa9af..59b5c15 100644
--- a/include/proto/stream_interface.h
+++ b/include/proto/stream_interface.h
@@ -184,7 +184,7 @@
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 @@
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_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 @@
{
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 51d4f27..764e9ba 100644
--- a/src/flt_spoe.c
+++ b/src/flt_spoe.c
@@ -1341,15 +1341,18 @@
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 bf1dd12..a5997e5 100644
--- a/src/stream.c
+++ b/src/stream.c
@@ -1138,8 +1138,8 @@
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 @@
}
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 @@
*/
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 @@
}
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 @@
/* 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 @@
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 @@
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 @@
*/
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 @@
/* 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 @@
/* 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 @@
*/
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 @@
}
}
- 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 bdc9b00..0a00b16 100644
--- a/src/stream_interface.c
+++ b/src/stream_interface.c
@@ -175,7 +175,7 @@
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 @@
/* 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 @@
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 @@
/* 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 @@
}
/* 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 @@
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_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 @@
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 @@
*/
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 @@
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 @@
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 @@
/* 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) {