MINOR: connection: make conn_stream users also check for per-stream error flag

In a 1:1 connection:stream there's no problem relying on the connection
flags alone to check for errors. But in a mux, it will be possible to mark
certain streams in error without having to mark all of them. An example is
an H2 client sending RST_STREAM frames to abort a long download, or a parse
error requiring to abort only this specific stream.

This commit ensures that stream-interface and checks properly check for
CS_FL_ERROR in cs->flags wherever CO_FL_ERROR was in use. Most likely over
the long term, any check for CO_FL_ERROR will have to disappear.
This commit is contained in:
Willy Tarreau 2017-10-16 15:17:17 +02:00
parent 9aaf778129
commit 4ff3b89643
3 changed files with 35 additions and 24 deletions

View File

@ -596,7 +596,8 @@ static void chk_report_conn_err(struct check *check, int errno_bck, int expired)
if (conn && (!errno || errno == EAGAIN)) if (conn && (!errno || errno == EAGAIN))
retrieve_errno_from_socket(conn); retrieve_errno_from_socket(conn);
if (conn && !(conn->flags & CO_FL_ERROR) && !expired) if (conn && !(conn->flags & CO_FL_ERROR) &&
!(cs->flags & CS_FL_ERROR) && !expired)
return; return;
/* we'll try to build a meaningful error message depending on the /* we'll try to build a meaningful error message depending on the
@ -665,7 +666,7 @@ static void chk_report_conn_err(struct check *check, int errno_bck, int expired)
} }
else if ((conn->flags & (CO_FL_CONNECTED|CO_FL_WAIT_L4_CONN)) == CO_FL_WAIT_L4_CONN) { else if ((conn->flags & (CO_FL_CONNECTED|CO_FL_WAIT_L4_CONN)) == CO_FL_WAIT_L4_CONN) {
/* L4 not established (yet) */ /* L4 not established (yet) */
if (conn->flags & CO_FL_ERROR) if (conn->flags & CO_FL_ERROR || cs->flags & CS_FL_ERROR)
set_server_check_status(check, HCHK_STATUS_L4CON, err_msg); set_server_check_status(check, HCHK_STATUS_L4CON, err_msg);
else if (expired) else if (expired)
set_server_check_status(check, HCHK_STATUS_L4TOUT, err_msg); set_server_check_status(check, HCHK_STATUS_L4TOUT, err_msg);
@ -679,12 +680,12 @@ static void chk_report_conn_err(struct check *check, int errno_bck, int expired)
} }
else if ((conn->flags & (CO_FL_CONNECTED|CO_FL_WAIT_L6_CONN)) == CO_FL_WAIT_L6_CONN) { else if ((conn->flags & (CO_FL_CONNECTED|CO_FL_WAIT_L6_CONN)) == CO_FL_WAIT_L6_CONN) {
/* L6 not established (yet) */ /* L6 not established (yet) */
if (conn->flags & CO_FL_ERROR) if (conn->flags & CO_FL_ERROR || cs->flags & CS_FL_ERROR)
set_server_check_status(check, HCHK_STATUS_L6RSP, err_msg); set_server_check_status(check, HCHK_STATUS_L6RSP, err_msg);
else if (expired) else if (expired)
set_server_check_status(check, HCHK_STATUS_L6TOUT, err_msg); set_server_check_status(check, HCHK_STATUS_L6TOUT, err_msg);
} }
else if (conn->flags & CO_FL_ERROR) { else if (conn->flags & CO_FL_ERROR || cs->flags & CS_FL_ERROR) {
/* I/O error after connection was established and before we could diagnose */ /* I/O error after connection was established and before we could diagnose */
set_server_check_status(check, HCHK_STATUS_SOCKERR, err_msg); set_server_check_status(check, HCHK_STATUS_SOCKERR, err_msg);
} }
@ -744,7 +745,7 @@ static void event_srv_chk_w(struct conn_stream *cs)
if (check->bo->o) { if (check->bo->o) {
conn->mux->snd_buf(cs, check->bo, 0); conn->mux->snd_buf(cs, check->bo, 0);
if (conn->flags & CO_FL_ERROR) { if (conn->flags & CO_FL_ERROR || cs->flags & CS_FL_ERROR) {
chk_report_conn_err(check, errno, 0); chk_report_conn_err(check, errno, 0);
__cs_stop_both(cs); __cs_stop_both(cs);
goto out_wakeup; goto out_wakeup;
@ -819,9 +820,9 @@ static void event_srv_chk_r(struct conn_stream *cs)
done = 0; done = 0;
conn->mux->rcv_buf(cs, check->bi, check->bi->size); conn->mux->rcv_buf(cs, check->bi, check->bi->size);
if (conn->flags & (CO_FL_ERROR | CO_FL_SOCK_RD_SH)) { if (conn->flags & (CO_FL_ERROR | CO_FL_SOCK_RD_SH) || cs->flags & CS_FL_ERROR) {
done = 1; done = 1;
if ((conn->flags & CO_FL_ERROR) && !check->bi->i) { if ((conn->flags & CO_FL_ERROR || cs->flags & CS_FL_ERROR) && !check->bi->i) {
/* Report network errors only if we got no other data. Otherwise /* Report network errors only if we got no other data. Otherwise
* we'll let the upper layers decide whether the response is OK * we'll let the upper layers decide whether the response is OK
* or not. It is very common that an RST sent by the server is * or not. It is very common that an RST sent by the server is
@ -1328,7 +1329,7 @@ static void event_srv_chk_r(struct conn_stream *cs)
SPIN_UNLOCK(SERVER_LOCK, &check->server->lock); SPIN_UNLOCK(SERVER_LOCK, &check->server->lock);
out_wakeup: out_wakeup:
/* collect possible new errors */ /* collect possible new errors */
if (conn->flags & CO_FL_ERROR) if (conn->flags & CO_FL_ERROR || cs->flags & CS_FL_ERROR)
chk_report_conn_err(check, 0, 0); chk_report_conn_err(check, 0, 0);
/* Reset the check buffer... */ /* Reset the check buffer... */
@ -1375,7 +1376,7 @@ static int wake_srv_chk(struct conn_stream *cs)
conn = cs_conn(cs); conn = cs_conn(cs);
} }
if (unlikely(conn->flags & CO_FL_ERROR)) { if (unlikely(conn->flags & CO_FL_ERROR || cs->flags & CS_FL_ERROR)) {
/* We may get error reports bypassing the I/O handlers, typically /* We may get error reports bypassing the I/O handlers, typically
* the case when sending a pure TCP check which fails, then the I/O * the case when sending a pure TCP check which fails, then the I/O
* handlers above are not called. This is completely handled by the * handlers above are not called. This is completely handled by the
@ -2191,7 +2192,7 @@ static struct task *process_chk_conn(struct task *t)
else else
set_server_check_status(check, HCHK_STATUS_L4OK, NULL); set_server_check_status(check, HCHK_STATUS_L4OK, NULL);
} }
else if ((conn->flags & CO_FL_ERROR) || expired) { else if ((conn->flags & CO_FL_ERROR) || cs->flags & CS_FL_ERROR || expired) {
chk_report_conn_err(check, 0, expired); chk_report_conn_err(check, 0, expired);
} }
else else
@ -2646,7 +2647,7 @@ static int tcpcheck_main(struct check *check)
__cs_want_send(cs); __cs_want_send(cs);
if (conn->mux->snd_buf(cs, check->bo, 0) <= 0) { if (conn->mux->snd_buf(cs, check->bo, 0) <= 0) {
if (conn->flags & CO_FL_ERROR) { if (conn->flags & CO_FL_ERROR || cs->flags & CS_FL_ERROR) {
chk_report_conn_err(check, errno, 0); chk_report_conn_err(check, errno, 0);
__cs_stop_both(cs); __cs_stop_both(cs);
goto out_end_tcpcheck; goto out_end_tcpcheck;
@ -2878,9 +2879,9 @@ static int tcpcheck_main(struct check *check)
__cs_want_recv(cs); __cs_want_recv(cs);
if (conn->mux->rcv_buf(cs, check->bi, check->bi->size) <= 0) { if (conn->mux->rcv_buf(cs, check->bi, check->bi->size) <= 0) {
if (conn->flags & (CO_FL_ERROR | CO_FL_SOCK_RD_SH)) { if (conn->flags & (CO_FL_ERROR | CO_FL_SOCK_RD_SH) || cs->flags & CS_FL_ERROR) {
done = 1; done = 1;
if ((conn->flags & CO_FL_ERROR) && !check->bi->i) { if ((conn->flags & CO_FL_ERROR || cs->flags & CS_FL_ERROR) && !check->bi->i) {
/* Report network errors only if we got no other data. Otherwise /* Report network errors only if we got no other data. Otherwise
* we'll let the upper layers decide whether the response is OK * we'll let the upper layers decide whether the response is OK
* or not. It is very common that an RST sent by the server is * or not. It is very common that an RST sent by the server is
@ -3037,7 +3038,7 @@ static int tcpcheck_main(struct check *check)
out_end_tcpcheck: out_end_tcpcheck:
/* collect possible new errors */ /* collect possible new errors */
if (conn->flags & CO_FL_ERROR) if (conn->flags & CO_FL_ERROR || cs->flags & CS_FL_ERROR)
chk_report_conn_err(check, 0, 0); chk_report_conn_err(check, 0, 0);
/* cleanup before leaving */ /* cleanup before leaving */

View File

@ -82,6 +82,8 @@ static void mux_pt_recv(struct connection *conn)
{ {
struct conn_stream *cs = conn->mux_ctx; struct conn_stream *cs = conn->mux_ctx;
if (conn->flags & CO_FL_ERROR)
cs->flags |= CS_FL_ERROR;
if (conn_xprt_read0_pending(conn)) if (conn_xprt_read0_pending(conn))
cs->flags |= CS_FL_EOS; cs->flags |= CS_FL_EOS;
cs->data_cb->recv(cs); cs->data_cb->recv(cs);
@ -95,6 +97,8 @@ static void mux_pt_send(struct connection *conn)
{ {
struct conn_stream *cs = conn->mux_ctx; struct conn_stream *cs = conn->mux_ctx;
if (conn->flags & CO_FL_ERROR)
cs->flags |= CS_FL_ERROR;
cs->data_cb->send(cs); cs->data_cb->send(cs);
cs_update_mux_polling(cs); cs_update_mux_polling(cs);
} }
@ -138,6 +142,8 @@ static int mux_pt_rcv_buf(struct conn_stream *cs, struct buffer *buf, int count)
ret = cs->conn->xprt->rcv_buf(cs->conn, buf, count); ret = cs->conn->xprt->rcv_buf(cs->conn, buf, count);
if (conn_xprt_read0_pending(cs->conn)) if (conn_xprt_read0_pending(cs->conn))
cs->flags |= CS_FL_EOS; cs->flags |= CS_FL_EOS;
if (cs->conn->flags & CO_FL_ERROR)
cs->flags |= CS_FL_ERROR;
return (ret); return (ret);
} }
@ -155,6 +161,8 @@ static int mux_pt_rcv_pipe(struct conn_stream *cs, struct pipe *pipe, unsigned i
ret = cs->conn->xprt->rcv_pipe(cs->conn, pipe, count); ret = cs->conn->xprt->rcv_pipe(cs->conn, pipe, count);
if (conn_xprt_read0_pending(cs->conn)) if (conn_xprt_read0_pending(cs->conn))
cs->flags |= CS_FL_EOS; cs->flags |= CS_FL_EOS;
if (cs->conn->flags & CO_FL_ERROR)
cs->flags |= CS_FL_ERROR;
return (ret); return (ret);
} }

View File

@ -435,7 +435,7 @@ static int si_idle_conn_wake_cb(struct conn_stream *cs)
if (!conn_ctrl_ready(conn)) if (!conn_ctrl_ready(conn))
return 0; return 0;
if (conn->flags & (CO_FL_ERROR | CO_FL_SOCK_RD_SH)) { if (conn->flags & (CO_FL_ERROR | CO_FL_SOCK_RD_SH) || cs->flags & CS_FL_ERROR) {
/* warning, we can't do anything on <conn> after this call ! */ /* warning, we can't do anything on <conn> after this call ! */
si_release_endpoint(si); si_release_endpoint(si);
return -1; return -1;
@ -574,7 +574,7 @@ static int si_cs_wake_cb(struct conn_stream *cs)
/* First step, report to the stream-int what was detected at the /* First step, report to the stream-int what was detected at the
* connection layer : errors and connection establishment. * connection layer : errors and connection establishment.
*/ */
if (conn->flags & CO_FL_ERROR) if (conn->flags & CO_FL_ERROR || cs->flags & CS_FL_ERROR)
si->flags |= SI_FL_ERR; si->flags |= SI_FL_ERR;
/* If we had early data, and the handshake ended, then /* If we had early data, and the handshake ended, then
@ -643,7 +643,7 @@ static void si_cs_send(struct conn_stream *cs)
oc->pipe = NULL; oc->pipe = NULL;
} }
if (conn->flags & CO_FL_ERROR) if (conn->flags & CO_FL_ERROR || cs->flags & CS_FL_ERROR)
return; return;
} }
@ -656,7 +656,8 @@ static void si_cs_send(struct conn_stream *cs)
/* when we're here, we already know that there is no spliced /* when we're here, we already know that there is no spliced
* data left, and that there are sendable buffered data. * data left, and that there are sendable buffered data.
*/ */
if (!(conn->flags & (CO_FL_ERROR | CO_FL_SOCK_WR_SH | CO_FL_HANDSHAKE)) && !(oc->flags & CF_SHUTW)) { if (!(conn->flags & (CO_FL_ERROR | CO_FL_SOCK_WR_SH | CO_FL_HANDSHAKE)) &&
!(cs->flags & CS_FL_ERROR) && !(oc->flags & CF_SHUTW)) {
/* check if we want to inform the kernel that we're interested in /* check if we want to inform the kernel that we're interested in
* sending more data after this call. We want this if : * sending more data after this call. We want this if :
* - we're about to close after this last send and want to merge * - we're about to close after this last send and want to merge
@ -991,7 +992,7 @@ static void stream_int_chk_snd_conn(struct stream_interface *si)
__cs_want_send(cs); __cs_want_send(cs);
si_cs_send(cs); si_cs_send(cs);
if (cs->conn->flags & CO_FL_ERROR) { if (cs->flags & CS_FL_ERROR || cs->conn->flags & CO_FL_ERROR) {
/* Write error on the file descriptor */ /* Write error on the file descriptor */
__cs_stop_both(cs); __cs_stop_both(cs);
si->flags |= SI_FL_ERR; si->flags |= SI_FL_ERR;
@ -1085,7 +1086,7 @@ static void si_cs_recv_cb(struct conn_stream *cs)
* happens when we send too large a request to a backend server * happens when we send too large a request to a backend server
* which rejects it before reading it all. * which rejects it before reading it all.
*/ */
if (conn->flags & CO_FL_ERROR) if (conn->flags & CO_FL_ERROR || cs->flags & CS_FL_ERROR)
return; return;
/* maybe we were called immediately after an asynchronous shutr */ /* maybe we were called immediately after an asynchronous shutr */
@ -1150,7 +1151,7 @@ static void si_cs_recv_cb(struct conn_stream *cs)
if (cs->flags & CS_FL_EOS) if (cs->flags & CS_FL_EOS)
goto out_shutdown_r; goto out_shutdown_r;
if (conn->flags & CO_FL_ERROR) if (conn->flags & CO_FL_ERROR || cs->flags & CS_FL_ERROR)
return; return;
if (conn->flags & CO_FL_WAIT_ROOM) { if (conn->flags & CO_FL_WAIT_ROOM) {
@ -1181,7 +1182,8 @@ static void si_cs_recv_cb(struct conn_stream *cs)
* that if such an event is not handled above in splice, it will be handled here by * that if such an event is not handled above in splice, it will be handled here by
* recv(). * recv().
*/ */
while (!(conn->flags & (CO_FL_ERROR | CO_FL_SOCK_RD_SH | CO_FL_WAIT_ROOM | CO_FL_HANDSHAKE)) && !(ic->flags & CF_SHUTR)) { while (!(conn->flags & (CO_FL_ERROR | CO_FL_SOCK_RD_SH | CO_FL_WAIT_ROOM | CO_FL_HANDSHAKE)) &&
!(cs->flags & CS_FL_ERROR) && !(ic->flags & CF_SHUTR)) {
max = channel_recv_max(ic); max = channel_recv_max(ic);
if (!max) { if (!max) {
@ -1283,7 +1285,7 @@ static void si_cs_recv_cb(struct conn_stream *cs)
} }
end_recv: end_recv:
if (conn->flags & CO_FL_ERROR) if (conn->flags & CO_FL_ERROR || cs->flags & CS_FL_ERROR)
return; return;
if (cs->flags & CS_FL_EOS) if (cs->flags & CS_FL_EOS)
@ -1311,7 +1313,7 @@ static void si_cs_send_cb(struct conn_stream *cs)
struct connection *conn = cs->conn; struct connection *conn = cs->conn;
struct stream_interface *si = cs->data; struct stream_interface *si = cs->data;
if (conn->flags & CO_FL_ERROR) if (conn->flags & CO_FL_ERROR || cs->flags & CS_FL_ERROR)
return; return;
if (conn->flags & CO_FL_HANDSHAKE) if (conn->flags & CO_FL_HANDSHAKE)