net/haproxy/patches/0001-BUG-MEDIUM-connection-remove-useless-flag-CO_FL_DATA.patch

   1 From 912e8f18ef274fdda0a522b2aa8255bddd00fb7b Mon Sep 17 00:00:00 2001
   2 From: Willy Tarreau <w@1wt.eu>
   3 Date: Wed, 30 Aug 2017 07:35:35 +0200
   4 Subject: [PATCH] BUG/MEDIUM: connection: remove useless flag CO_FL_DATA_RD_SH
   5
   6 This flag is both confusing and wrong. It is supposed to report the
   7 fact that the data layer has received a shutdown, but in fact this is
   8 reported by CO_FL_SOCK_RD_SH which is set by the transport layer after
   9 this condition is detected. The only case where the flag above is set
  10 is in the stream interface where CF_SHUTR is also set on the receiving
  11 channel.
  12
  13 In addition, it was checked in the health checks code (while never set)
  14 and was always test jointly with CO_FL_SOCK_RD_SH everywhere, except in
  15 conn_data_read0_pending() which incorrectly doesn't match the second
  16 time it's called and is fortunately protected by an extra check on
  17 (ic->flags & CF_SHUTR).
  18
  19 This patch gets rid of the flag completely. Now conn_data_read0_pending()
  20 accurately reports the fact that the transport layer has detected the end
  21 of the stream, regardless of the fact that this state was already consumed,
  22 and the stream interface watches ic->flags&CF_SHUTR to know if the channel
  23 was already closed by the upper layer (which it already used to do).
  24
  25 The now unused conn_data_read0() function was removed.
  26 (cherry picked from commit 54e917cfa1e7b0539550ae32c48c76da2f169041)
  27
  28 [wt: this happens to fix a real bug which occasionally strikes when
  29      using http-reuse in the rare case where a server shuts down after
  30      providing its response but before the connection is put back into
  31      the idle pool, and it gets immediately recycled for another request,
  32      without first passing through the idle handler, and the already
  33      reported shutdown is never reported to the second transaction,
  34      causing a loop to last for as long as the server timeout]
  35 ---
  36  contrib/debug/flags.c      |  1 -
  37  include/proto/connection.h |  8 +-------
  38  include/types/connection.h |  2 +-
  39  src/checks.c               |  4 ++--
  40  src/stream_interface.c     | 11 +++++------
  41  5 files changed, 9 insertions(+), 17 deletions(-)
  42
  43 diff --git a/contrib/debug/flags.c b/contrib/debug/flags.c
  44 index bc71bde9..19327f34 100644
  45 --- a/contrib/debug/flags.c
  46 +++ b/contrib/debug/flags.c
  47 @@ -117,7 +117,6 @@ void show_conn_flags(unsigned int f)
  48         SHOW_FLAG(f, CO_FL_SOCK_WR_SH);
  49         SHOW_FLAG(f, CO_FL_SOCK_RD_SH);
  50         SHOW_FLAG(f, CO_FL_DATA_WR_SH);
  51 -       SHOW_FLAG(f, CO_FL_DATA_RD_SH);
  52         SHOW_FLAG(f, CO_FL_WAKE_DATA);
  53         SHOW_FLAG(f, CO_FL_INIT_DATA);
  54         SHOW_FLAG(f, CO_FL_ADDR_TO_SET);
  55 diff --git a/include/proto/connection.h b/include/proto/connection.h
  56 index fce60259..eb68322a 100644
  57 --- a/include/proto/connection.h
  58 +++ b/include/proto/connection.h
  59 @@ -413,12 +413,6 @@ static inline void conn_sock_read0(struct connection *c)
  60                 fdtab[c->t.sock.fd].linger_risk = 0;
  61  }
  62
  63 -static inline void conn_data_read0(struct connection *c)
  64 -{
  65 -       c->flags |= CO_FL_DATA_RD_SH;
  66 -       __conn_data_stop_recv(c);
  67 -}
  68 -
  69  static inline void conn_sock_shutw(struct connection *c)
  70  {
  71         c->flags |= CO_FL_SOCK_WR_SH;
  72 @@ -450,7 +444,7 @@ static inline void conn_data_shutw_hard(struct connection *c)
  73  /* detect sock->data read0 transition */
  74  static inline int conn_data_read0_pending(struct connection *c)
  75  {
  76 -       return (c->flags & (CO_FL_DATA_RD_SH | CO_FL_SOCK_RD_SH)) == CO_FL_SOCK_RD_SH;
  77 +       return (c->flags & CO_FL_SOCK_RD_SH) != 0;
  78  }
  79
  80  /* detect data->sock shutw transition */
  81 diff --git a/include/types/connection.h b/include/types/connection.h
  82 index 02eac932..90e8e073 100644
  83 --- a/include/types/connection.h
  84 +++ b/include/types/connection.h
  85 @@ -90,7 +90,7 @@ enum {
  86         CO_FL_WAKE_DATA     = 0x00008000,  /* wake-up data layer upon activity at the transport layer */
  87
  88         /* flags used to remember what shutdown have been performed/reported */
  89 -       CO_FL_DATA_RD_SH    = 0x00010000,  /* DATA layer was notified about shutr/read0 */
  90 +       /* unused : 0x00010000 */
  91         CO_FL_DATA_WR_SH    = 0x00020000,  /* DATA layer asked for shutw */
  92         CO_FL_SOCK_RD_SH    = 0x00040000,  /* SOCK layer was notified about shutr/read0 */
  93         CO_FL_SOCK_WR_SH    = 0x00080000,  /* SOCK layer asked for shutw */
  94 diff --git a/src/checks.c b/src/checks.c
  95 index ca3881a5..6c5e3cbc 100644
  96 --- a/src/checks.c
  97 +++ b/src/checks.c
  98 @@ -839,7 +839,7 @@ static void event_srv_chk_r(struct connection *conn)
  99         done = 0;
 100
 101         conn->xprt->rcv_buf(conn, check->bi, check->bi->size);
 102 -       if (conn->flags & (CO_FL_ERROR | CO_FL_SOCK_RD_SH | CO_FL_DATA_RD_SH)) {
 103 +       if (conn->flags & (CO_FL_ERROR | CO_FL_SOCK_RD_SH)) {
 104                 done = 1;
 105                 if ((conn->flags & CO_FL_ERROR) && !check->bi->i) {
 106                         /* Report network errors only if we got no other data. Otherwise
 107 @@ -2892,7 +2892,7 @@ static void tcpcheck_main(struct connection *conn)
 108                                 goto out_end_tcpcheck;
 109
 110                         if (conn->xprt->rcv_buf(conn, check->bi, check->bi->size) <= 0) {
 111 -                               if (conn->flags & (CO_FL_ERROR | CO_FL_SOCK_RD_SH | CO_FL_DATA_RD_SH)) {
 112 +                               if (conn->flags & (CO_FL_ERROR | CO_FL_SOCK_RD_SH)) {
 113                                         done = 1;
 114                                         if ((conn->flags & CO_FL_ERROR) && !check->bi->i) {
 115                                                 /* Report network errors only if we got no other data. Otherwise
 116 diff --git a/src/stream_interface.c b/src/stream_interface.c
 117 index 836487bd..aba49c94 100644
 118 --- a/src/stream_interface.c
 119 +++ b/src/stream_interface.c
 120 @@ -1060,14 +1060,14 @@ static void si_conn_recv_cb(struct connection *conn)
 121         if (conn->flags & CO_FL_ERROR)
 122                 return;
 123
 124 -       /* stop here if we reached the end of data */
 125 -       if (conn_data_read0_pending(conn))
 126 -               goto out_shutdown_r;
 127 -
 128         /* maybe we were called immediately after an asynchronous shutr */
 129         if (ic->flags & CF_SHUTR)
 130                 return;
 131
 132 +       /* stop here if we reached the end of data */
 133 +       if (conn_data_read0_pending(conn))
 134 +               goto out_shutdown_r;
 135 +
 136         cur_read = 0;
 137
 138         if ((ic->flags & (CF_STREAMER | CF_STREAMER_FAST)) && !ic->buf->o &&
 139 @@ -1153,7 +1153,7 @@ static void si_conn_recv_cb(struct connection *conn)
 140          * that if such an event is not handled above in splice, it will be handled here by
 141          * recv().
 142          */
 143 -       while (!(conn->flags & (CO_FL_ERROR | CO_FL_SOCK_RD_SH | CO_FL_DATA_RD_SH | CO_FL_WAIT_ROOM | CO_FL_HANDSHAKE))) {
 144 +       while (!(conn->flags & (CO_FL_ERROR | CO_FL_SOCK_RD_SH | CO_FL_WAIT_ROOM | CO_FL_HANDSHAKE)) && !(ic->flags & CF_SHUTR)) {
 145                 max = channel_recv_max(ic);
 146
 147                 if (!max) {
 148 @@ -1267,7 +1267,6 @@ static void si_conn_recv_cb(struct connection *conn)
 149         if (ic->flags & CF_AUTO_CLOSE)
 150                 channel_shutw_now(ic);
 151         stream_sock_read0(si);
 152 -       conn_data_read0(conn);
 153         return;
 154  }
 155
 156 --
 157 2.13.5
 158