MEDIUM: conn-stream: Use endpoint error instead of conn-stream error

Instead of relying on the conn-stream error, via CS_FL_ERR flags, we now
directly use the error at the endpoint level with the flag CS_EP_ERROR. It
should be safe to do so. But we must be careful because it is still possible
that an error is processed too early. Anyway, a conn-stream has always a
valid endpoint, maybe detached from any endpoint, but valid.

dev/flags/flags.c

@@ -219,7 +219,6 @@ 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);

include/haproxy/conn_stream-t.h

@@ -82,8 +82,6 @@ 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 */

src/backend.c

@@ -1744,8 +1744,8 @@ static int connect_server(struct stream *s)
 	 * 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))
+	if (srv_conn->flags & CO_FL_ERROR)
-		s->csb->flags |= CS_FL_ERR;
+		s->csb->endp->flags |= CS_EP_ERROR;
 
 	/* 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->flags &= ~CS_FL_ERR;
+		cs->endp->flags &= ~CS_EP_ERROR;
 		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->flags & CS_FL_ERR)) {
+	if ((s->flags & SF_CONN_EXP) || (cs->endp->flags & CS_EP_ERROR)) {
 		if (!cs->si->err_type) {
-			if (cs->flags & CS_FL_ERR)
+			if (cs->endp->flags & CS_EP_ERROR)
 				cs->si->err_type = SI_ET_CONN_ERR;
 			else
 				cs->si->err_type = SI_ET_CONN_TO;
@@ -2215,6 +2215,7 @@ void back_handle_st_con(struct stream *s)
 void back_handle_st_cer(struct stream *s)
 {
 	struct conn_stream *cs = s->csb;
+	int must_tar = (cs->endp->flags & CS_EP_ERROR);
 
 	DBG_TRACE_ENTER(STRM_EV_STRM_PROC|STRM_EV_SI_ST, s);
 
@@ -2234,7 +2235,7 @@ void back_handle_st_cer(struct stream *s)
 			_HA_ATOMIC_DEC(&__objt_server(s->target)->cur_sess);
 		}
 
-		if ((cs->flags & CS_FL_ERR) &&
+		if ((cs->endp->flags & CS_EP_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
@@ -2291,7 +2292,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 CS_FL_ERR and SF_CONN_EXP flags will be unset.
+	 * ST_TAR and CS_EP_ERROR and SF_CONN_EXP flags will be unset.
 	 */
 	if (cs_reset_endp(cs) < 0) {
 		if (!cs->si->err_type)
@@ -2319,7 +2320,7 @@ void back_handle_st_cer(struct stream *s)
 
 	stream_choose_redispatch(s);
 
-	if (cs->flags & CS_FL_ERR) {
+	if (must_tar) {
 		/* 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 +2346,6 @@ 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->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->flags & CS_FL_ERR) {
+		if (cs->endp->flags & CS_EP_ERROR) {
 			if (!cs->si->err_type)
 				cs->si->err_type = SI_ET_CONN_ERR;
 			cs->si->state = SI_ST_CER;

src/cli.c

@@ -1084,7 +1084,7 @@ static void cli_io_handler(struct appctx *appctx)
 					}
 				break;
 			default: /* abnormal state */
-				cs->flags |= CS_FL_ERR;
+				cs->endp->flags |= CS_EP_ERROR;
 				break;
 			}
 

src/http_ana.c

@@ -1249,7 +1249,6 @@ static __inline int do_l7_retry(struct stream *s, struct stream_interface *si)
 	res->analysers &= AN_RES_FLT_END;
 	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);

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->cs->flags & CS_FL_ERR && req->flags & CF_WROTE_DATA) {
+	if (s->csb->endp->flags & CS_EP_ERROR && 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->cs->flags | si_b->cs->flags) & CS_FL_ERR) &&
+		    !((s->csf->endp->flags | s->csb->flags) & CS_EP_ERROR) &&
 		    ((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->cs->flags |= CS_FL_ERR;
+		s->csf->endp->flags |= CS_EP_ERROR;
 		si_f->err_type = SI_ET_CONN_RES;
 
-		si_b->cs->flags |= CS_FL_ERR;
+		s->csb->endp->flags |= CS_EP_ERROR;
 		si_b->err_type = SI_ET_CONN_RES;
 
 		if (!(s->flags & SF_ERR_MASK))
@@ -1711,7 +1711,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(si_f->cs->flags & CS_FL_ERR)) {
+	if (unlikely(s->csf->endp->flags & CS_EP_ERROR)) {
 		if (si_state_in(si_f->state, SI_SB_EST|SI_SB_DIS)) {
 			si_shutr(si_f);
 			si_shutw(si_f);
@@ -1731,7 +1731,7 @@ struct task *process_stream(struct task *t, void *context, unsigned int state)
 		}
 	}
 
-	if (unlikely(si_b->cs->flags & CS_FL_ERR)) {
+	if (unlikely(s->csb->endp->flags & CS_EP_ERROR)) {
 		if (si_state_in(si_b->state, SI_SB_EST|SI_SB_DIS)) {
 			si_shutr(si_b);
 			si_shutw(si_b);
@@ -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->cs->flags & CS_FL_ERR && si_f->state != SI_ST_CLO) ||
+	    (s->csf->endp->flags & CS_EP_ERROR && si_f->state != SI_ST_CLO) ||
-	    (si_b->cs->flags & CS_FL_ERR && si_b->state != SI_ST_CLO))
+	    (s->csb->endp->flags & CS_EP_ERROR && 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->cs->flags & CS_FL_ERR && si_f->state != SI_ST_CLO) ||
+	    (s->csf->endp->flags & CS_EP_ERROR && si_f->state != SI_ST_CLO) ||
-	    (si_b->cs->flags & CS_FL_ERR && si_b->state != SI_ST_CLO))
+	    (s->csb->endp->flags & CS_EP_ERROR && si_b->state != SI_ST_CLO))
 		goto resync_stream_interface;
 
 	if ((req->flags & ~rqf_last) & CF_MASK_ANALYSER)

src/stream_interface.c

@@ -225,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->cs->flags & CS_FL_ERR) && !(si->flags & SI_FL_NOLINGER) &&
+		if (!(si->cs->endp->flags & CS_EP_ERROR) && !(si->flags & SI_FL_NOLINGER) &&
 		    !(ic->flags & (CF_SHUTR|CF_DONT_READ)))
 			return;
 
@@ -521,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->cs->flags & CS_FL_ERR) ||
+	    (si->cs->endp->flags & CS_EP_ERROR) ||
 	    ((ic->flags & CF_READ_PARTIAL) &&
 	     ((ic->flags & CF_EOI) || !ic->to_forward || sio->state != SI_ST_EST)) ||
 
@@ -590,11 +590,11 @@ static int si_cs_process(struct conn_stream *cs)
 
 	/* First step, report to the conn-stream what was detected at the
 	 * connection layer : errors and connection establishment.
-	 * Only add CS_FL_ERR if we're connected, or we're attempting to
+	 * Only add CS_EP_ERROR 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 CS_FL_ERR back
+	 * and we don't want to add CS_EP_ERROR 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
@@ -602,8 +602,8 @@ 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))
+		if (si_is_conn_error(si))
-			si->cs->flags |= CS_FL_ERR;
+			cs->endp->flags |= CS_EP_ERROR;
 	}
 
 	/* If we had early data, and the handshake ended, then
@@ -679,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
-		 * CS_FL_ERR back, so give up
+		 * CS_EP_ERROR back, so give up
 		 */
 		if (si->state < SI_ST_CON)
 			return 0;
-		si->cs->flags |= CS_FL_ERR;
+		cs->endp->flags |= CS_EP_ERROR;
 		return 1;
 	}
 
@@ -797,7 +797,7 @@ static int si_cs_send(struct conn_stream *cs)
 	}
 
 	if (cs->endp->flags & (CS_EP_ERROR|CS_EP_ERR_PENDING)) {
-		si->cs->flags |= CS_FL_ERR;
+		cs->endp->flags |= CS_EP_ERROR;
 		return 1;
 	}
 
@@ -1092,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->cs->flags & CS_FL_ERR) {
+		if (cs->endp->flags & CS_EP_ERROR) {
 			/* quick close, the socket is already shut anyway */
 		}
 		else if (si->flags & SI_FL_NOLINGER) {
@@ -1182,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->cs->flags |= CS_FL_ERR;
+			cs->endp->flags |= CS_EP_ERROR;
 		goto out_wakeup;
 	}
 
@@ -1558,10 +1558,8 @@ static int si_cs_recv(struct conn_stream *cs)
 		ret = 1;
 	}
 
-	if (cs->endp->flags & CS_EP_ERROR) {
+	if (cs->endp->flags & CS_EP_ERROR)
-		si->cs->flags |= CS_FL_ERR;
 		ret = 1;
-	}
 	else if (cs->endp->flags & CS_EP_EOS) {
 		/* we received a shutdown */
 		ic->flags |= CF_READ_NULL;
@@ -1646,7 +1644,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->cs->flags |= CS_FL_ERR;
+		si->cs->endp->flags |= CS_EP_ERROR;
 
 	/* automatically mark the applet having data available if it reported
 	 * begin blocked by the channel.
@@ -1742,7 +1740,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->cs->flags & CS_FL_ERR) && !(si->flags & SI_FL_NOLINGER) &&
+		if (!(si->cs->endp->flags & CS_EP_ERROR) && !(si->flags & SI_FL_NOLINGER) &&
 		    !(ic->flags & (CF_SHUTR|CF_DONT_READ)))
 			return;