From 54e917cfa1e7b0539550ae32c48c76da2f169041 Mon Sep 17 00:00:00 2001 From: Willy Tarreau Date: Wed, 30 Aug 2017 07:35:35 +0200 Subject: [PATCH] MEDIUM: connection: remove useless flag CO_FL_DATA_RD_SH This flag is both confusing and wrong. It is supposed to report the fact that the data layer has received a shutdown, but in fact this is reported by CO_FL_SOCK_RD_SH which is set by the transport layer after this condition is detected. The only case where the flag above is set is in the stream interface where CF_SHUTR is also set on the receiving channel. In addition, it was checked in the health checks code (while never set) and was always test jointly with CO_FL_SOCK_RD_SH everywhere, except in conn_data_read0_pending() which incorrectly doesn't match the second time it's called and is fortunately protected by an extra check on (ic->flags & CF_SHUTR). This patch gets rid of the flag completely. Now conn_data_read0_pending() accurately reports the fact that the transport layer has detected the end of the stream, regardless of the fact that this state was already consumed, and the stream interface watches ic->flags&CF_SHUTR to know if the channel was already closed by the upper layer (which it already used to do). The now unused conn_data_read0() function was removed. --- contrib/debug/flags.c | 1 - include/proto/connection.h | 8 +------- include/types/connection.h | 2 +- src/checks.c | 4 ++-- src/stream_interface.c | 11 +++++------ 5 files changed, 9 insertions(+), 17 deletions(-) diff --git a/contrib/debug/flags.c b/contrib/debug/flags.c index d45b9684f..7b938a078 100644 --- a/contrib/debug/flags.c +++ b/contrib/debug/flags.c @@ -117,7 +117,6 @@ void show_conn_flags(unsigned int f) SHOW_FLAG(f, CO_FL_SOCK_WR_SH); SHOW_FLAG(f, CO_FL_SOCK_RD_SH); SHOW_FLAG(f, CO_FL_DATA_WR_SH); - SHOW_FLAG(f, CO_FL_DATA_RD_SH); SHOW_FLAG(f, CO_FL_ADDR_TO_SET); SHOW_FLAG(f, CO_FL_ADDR_FROM_SET); SHOW_FLAG(f, CO_FL_WAIT_ROOM); diff --git a/include/proto/connection.h b/include/proto/connection.h index db712f58b..3a31fa385 100644 --- a/include/proto/connection.h +++ b/include/proto/connection.h @@ -428,12 +428,6 @@ static inline void conn_sock_read0(struct connection *c) fdtab[c->handle.fd].linger_risk = 0; } -static inline void conn_data_read0(struct connection *c) -{ - c->flags |= CO_FL_DATA_RD_SH; - __conn_data_stop_recv(c); -} - static inline void conn_sock_shutw(struct connection *c) { c->flags |= CO_FL_SOCK_WR_SH; @@ -465,7 +459,7 @@ static inline void conn_data_shutw_hard(struct connection *c) /* detect sock->data read0 transition */ static inline int conn_data_read0_pending(struct connection *c) { - return (c->flags & (CO_FL_DATA_RD_SH | CO_FL_SOCK_RD_SH)) == CO_FL_SOCK_RD_SH; + return (c->flags & CO_FL_SOCK_RD_SH) != 0; } /* detect data->sock shutw transition */ diff --git a/include/types/connection.h b/include/types/connection.h index ce43b734e..a24b8585c 100644 --- a/include/types/connection.h +++ b/include/types/connection.h @@ -99,7 +99,7 @@ enum { /* unused : 0x00008000 */ /* flags used to remember what shutdown have been performed/reported */ - CO_FL_DATA_RD_SH = 0x00010000, /* DATA layer was notified about shutr/read0 */ + /* unused : 0x00010000 */ CO_FL_DATA_WR_SH = 0x00020000, /* DATA layer asked for shutw */ CO_FL_SOCK_RD_SH = 0x00040000, /* SOCK layer was notified about shutr/read0 */ CO_FL_SOCK_WR_SH = 0x00080000, /* SOCK layer asked for shutw */ diff --git a/src/checks.c b/src/checks.c index ca18220dc..db805e8f9 100644 --- a/src/checks.c +++ b/src/checks.c @@ -833,7 +833,7 @@ static void event_srv_chk_r(struct connection *conn) done = 0; conn->xprt->rcv_buf(conn, check->bi, check->bi->size); - if (conn->flags & (CO_FL_ERROR | CO_FL_SOCK_RD_SH | CO_FL_DATA_RD_SH)) { + if (conn->flags & (CO_FL_ERROR | CO_FL_SOCK_RD_SH)) { done = 1; if ((conn->flags & CO_FL_ERROR) && !check->bi->i) { /* Report network errors only if we got no other data. Otherwise @@ -2793,7 +2793,7 @@ static void tcpcheck_main(struct connection *conn) goto out_end_tcpcheck; if (conn->xprt->rcv_buf(conn, check->bi, check->bi->size) <= 0) { - if (conn->flags & (CO_FL_ERROR | CO_FL_SOCK_RD_SH | CO_FL_DATA_RD_SH)) { + if (conn->flags & (CO_FL_ERROR | CO_FL_SOCK_RD_SH)) { done = 1; if ((conn->flags & CO_FL_ERROR) && !check->bi->i) { /* Report network errors only if we got no other data. Otherwise diff --git a/src/stream_interface.c b/src/stream_interface.c index a57828389..c19c14695 100644 --- a/src/stream_interface.c +++ b/src/stream_interface.c @@ -1060,14 +1060,14 @@ static void si_conn_recv_cb(struct connection *conn) if (conn->flags & CO_FL_ERROR) return; - /* stop here if we reached the end of data */ - if (conn_data_read0_pending(conn)) - goto out_shutdown_r; - /* maybe we were called immediately after an asynchronous shutr */ if (ic->flags & CF_SHUTR) return; + /* stop here if we reached the end of data */ + if (conn_data_read0_pending(conn)) + goto out_shutdown_r; + cur_read = 0; if ((ic->flags & (CF_STREAMER | CF_STREAMER_FAST)) && !ic->buf->o && @@ -1153,7 +1153,7 @@ static void si_conn_recv_cb(struct connection *conn) * that if such an event is not handled above in splice, it will be handled here by * recv(). */ - while (!(conn->flags & (CO_FL_ERROR | CO_FL_SOCK_RD_SH | CO_FL_DATA_RD_SH | CO_FL_WAIT_ROOM | CO_FL_HANDSHAKE))) { + while (!(conn->flags & (CO_FL_ERROR | CO_FL_SOCK_RD_SH | CO_FL_WAIT_ROOM | CO_FL_HANDSHAKE)) && !(ic->flags & CF_SHUTR)) { max = channel_recv_max(ic); if (!max) { @@ -1267,7 +1267,6 @@ static void si_conn_recv_cb(struct connection *conn) if (ic->flags & CF_AUTO_CLOSE) channel_shutw_now(ic); stream_sock_read0(si); - conn_data_read0(conn); return; }