git.99rst.org Git - openwrt-packages.git/blob

   1 commit c98cdf7cc755c579a8b9cceee4809089267615ce
   2 Author: Willy Tarreau <w@1wt.eu>
   3 Date:   Wed Feb 27 19:32:32 2019 +0100
   4
   5     BUG/MEDIUM: listener: make sure the listener never accepts too many conns
   6
   7     We were not checking p->feconn nor the global actconn soon enough. In
   8     older versions this could result in a frontend accepting more connections
   9     than allowed by its maxconn or the global maxconn, exactly N-1 extra
  10     connections where N is the number of threads, provided each of these
  11     threads were running a different listener. But with the lock removal,
  12     it became worse, the excess could be the listener's maxconn multiplied
  13     by the number of threads. Among the nasty side effect was that LI_FULL
  14     could be removed while the limit was still over and in some cases the
  15     polling on the socket was no re-enabled.
  16
  17     This commit takes care of updating and checking p->feconn and the global
  18     actconn *before* processing the connection, so that the listener can be
  19     turned off before accepting the socket if needed. This requires to move
  20     some of the bookkeeping operations form session to listen, which totally
  21     makes sense in this context.
  22
  23     Now the limits are properly respected, even if a listener's maxconn is
  24     over a frontend's. This only applies on top of the listener lock removal
  25     series and doesn't have to be backported.
  26
  27     (cherry picked from commit 82c9789ac4f0cba9a74d16c1b730fc64e1f95a6e)
  28     [wt: backported since it fixes the previous patch set]
  29     Signed-off-by: Willy Tarreau <w@1wt.eu>
  30     (cherry picked from commit f54a86a229e1ee4b256d5614c0a65924f447df09)
  31     Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
  32
  33 diff --git a/src/listener.c b/src/listener.c
  34 index dab07a5e..68c84fbe 100644
  35 --- a/src/listener.c
  36 +++ b/src/listener.c
  37 @@ -408,6 +408,8 @@ void listener_accept(int fd)
  38         struct proxy *p;
  39         int max_accept;
  40         int next_conn = 0;
  41 +       int next_feconn = 0;
  42 +       int next_actconn = 0;
  43         int expire;
  44         int cfd;
  45         int ret;
  46 @@ -478,12 +480,15 @@ void listener_accept(int fd)
  47          * worst case. If we fail due to system limits or temporary resource
  48          * shortage, we try again 100ms later in the worst case.
  49          */
  50 -       for (; max_accept-- > 0; next_conn = 0) {
  51 +       for (; max_accept-- > 0; next_conn = next_feconn = next_actconn = 0) {
  52                 struct sockaddr_storage addr;
  53                 socklen_t laddr = sizeof(addr);
  54                 unsigned int count;
  55
  56 -               /* pre-increase the number of connections without going too far */
  57 +               /* pre-increase the number of connections without going too far.
  58 +                * We process the listener, then the proxy, then the process.
  59 +                * We know which ones to unroll based on the next_xxx value.
  60 +                */
  61                 do {
  62                         count = l->nbconn;
  63                         if (count >= l->maxconn) {
  64 @@ -501,15 +506,42 @@ void listener_accept(int fd)
  65                         listener_full(l);
  66                 }
  67
  68 -               if (unlikely(actconn >= global.maxconn) && !(l->options & LI_O_UNLIMITED)) {
  69 -                       limit_listener(l, &global_listener_queue);
  70 -                       task_schedule(global_listener_queue_task, tick_add(now_ms, 1000)); /* try again in 1 second */
  71 -                       goto end;
  72 +               if (p) {
  73 +                       do {
  74 +                               count = p->feconn;
  75 +                               if (count >= p->maxconn) {
  76 +                                       /* the frontend was marked full or another
  77 +                                        * thread is going to do it.
  78 +                                        */
  79 +                                       next_feconn = 0;
  80 +                                       goto end;
  81 +                               }
  82 +                               next_feconn = count + 1;
  83 +                       } while (!HA_ATOMIC_CAS(&p->feconn, &count, next_feconn));
  84 +
  85 +                       if (unlikely(next_feconn == p->maxconn)) {
  86 +                               /* we just filled it */
  87 +                               limit_listener(l, &p->listener_queue);
  88 +                       }
  89                 }
  90
  91 -               if (unlikely(p && p->feconn >= p->maxconn)) {
  92 -                       limit_listener(l, &p->listener_queue);
  93 -                       goto end;
  94 +               if (!(l->options & LI_O_UNLIMITED)) {
  95 +                       do {
  96 +                               count = actconn;
  97 +                               if (count >= global.maxconn) {
  98 +                                       /* the process was marked full or another
  99 +                                        * thread is going to do it.
 100 +                                        */
 101 +                                       next_actconn = 0;
 102 +                                       goto end;
 103 +                               }
 104 +                               next_actconn = count + 1;
 105 +                       } while (!HA_ATOMIC_CAS(&actconn, &count, next_actconn));
 106 +
 107 +                       if (unlikely(next_actconn == global.maxconn)) {
 108 +                               limit_listener(l, &global_listener_queue);
 109 +                               task_schedule(global_listener_queue_task, tick_add(now_ms, 1000)); /* try again in 1 second */
 110 +                       }
 111                 }
 112
 113  #ifdef USE_ACCEPT4
 114 @@ -544,6 +576,10 @@ void listener_accept(int fd)
 115                         case EINTR:
 116                         case ECONNABORTED:
 117                                 HA_ATOMIC_SUB(&l->nbconn, 1);
 118 +                               if (p)
 119 +                                       HA_ATOMIC_SUB(&p->feconn, 1);
 120 +                               if (!(l->options & LI_O_UNLIMITED))
 121 +                                       HA_ATOMIC_SUB(&actconn, 1);
 122                                 continue;
 123                         case ENFILE:
 124                                 if (p)
 125 @@ -574,18 +610,20 @@ void listener_accept(int fd)
 126                 if (l->counters)
 127                         HA_ATOMIC_UPDATE_MAX(&l->counters->conn_max, next_conn);
 128
 129 +               if (p)
 130 +                       HA_ATOMIC_UPDATE_MAX(&p->fe_counters.conn_max, next_feconn);
 131 +
 132 +               proxy_inc_fe_conn_ctr(l, p);
 133 +
 134                 if (!(l->options & LI_O_UNLIMITED)) {
 135                         count = update_freq_ctr(&global.conn_per_sec, 1);
 136                         HA_ATOMIC_UPDATE_MAX(&global.cps_max, count);
 137 -                       HA_ATOMIC_ADD(&actconn, 1);
 138                 }
 139
 140                 if (unlikely(cfd >= global.maxsock)) {
 141                         send_log(p, LOG_EMERG,
 142                                  "Proxy %s reached the configured maximum connection limit. Please check the global 'maxconn' value.\n",
 143                                  p->id);
 144 -                       if (!(l->options & LI_O_UNLIMITED))
 145 -                               HA_ATOMIC_SUB(&actconn, 1);
 146                         close(cfd);
 147                         limit_listener(l, &global_listener_queue);
 148                         task_schedule(global_listener_queue_task, tick_add(now_ms, 1000)); /* try again in 1 second */
 149 @@ -593,11 +631,13 @@ void listener_accept(int fd)
 150                 }
 151
 152                 /* past this point, l->accept() will automatically decrement
 153 -                * l->nbconn and actconn once done. Setting next_conn=0 allows
 154 -                * the error path not to rollback on nbconn. It's more convenient
 155 -                * than duplicating all exit labels.
 156 +                * l->nbconn, feconn and actconn once done. Setting next_*conn=0
 157 +                * allows the error path not to rollback on nbconn. It's more
 158 +                * convenient than duplicating all exit labels.
 159                  */
 160                 next_conn = 0;
 161 +               next_feconn = 0;
 162 +               next_actconn = 0;
 163
 164                 ret = l->accept(l, cfd, &addr);
 165                 if (unlikely(ret <= 0)) {
 166 @@ -644,7 +684,14 @@ void listener_accept(int fd)
 167         if (next_conn)
 168                 HA_ATOMIC_SUB(&l->nbconn, 1);
 169
 170 -       if (l->nbconn < l->maxconn && l->state == LI_FULL) {
 171 +       if (p && next_feconn)
 172 +               HA_ATOMIC_SUB(&p->feconn, 1);
 173 +
 174 +       if (next_actconn)
 175 +               HA_ATOMIC_SUB(&actconn, 1);
 176 +
 177 +       if ((l->state == LI_FULL && l->nbconn < l->maxconn) ||
 178 +           (l->state == LI_LIMITED && ((!p || p->feconn < p->maxconn) && (actconn < global.maxconn)))) {
 179                 /* at least one thread has to this when quitting */
 180                 resume_listener(l);
 181
 182 @@ -668,8 +715,11 @@ void listener_release(struct listener *l)
 183
 184         if (!(l->options & LI_O_UNLIMITED))
 185                 HA_ATOMIC_SUB(&actconn, 1);
 186 +       if (fe)
 187 +               HA_ATOMIC_SUB(&fe->feconn, 1);
 188         HA_ATOMIC_SUB(&l->nbconn, 1);
 189 -       if (l->state == LI_FULL)
 190 +
 191 +       if (l->state == LI_FULL || l->state == LI_LIMITED)
 192                 resume_listener(l);
 193
 194         /* Dequeues all of the listeners waiting for a resource */
 195 diff --git a/src/session.c b/src/session.c
 196 index b91d67ee..c1515261 100644
 197 --- a/src/session.c
 198 +++ b/src/session.c
 199 @@ -53,10 +53,6 @@ struct session *session_new(struct proxy *fe, struct listener *li, enum obj_type
 200                 vars_init(&sess->vars, SCOPE_SESS);
 201                 sess->task = NULL;
 202                 sess->t_handshake = -1; /* handshake not done yet */
 203 -               HA_ATOMIC_UPDATE_MAX(&fe->fe_counters.conn_max,
 204 -                                    HA_ATOMIC_ADD(&fe->feconn, 1));
 205 -               if (li)
 206 -                       proxy_inc_fe_conn_ctr(li, fe);
 207                 HA_ATOMIC_ADD(&totalconn, 1);
 208                 HA_ATOMIC_ADD(&jobs, 1);
 209         }
 210 @@ -65,7 +61,6 @@ struct session *session_new(struct proxy *fe, struct listener *li, enum obj_type
 211
 212  void session_free(struct session *sess)
 213  {
 214 -       HA_ATOMIC_SUB(&sess->fe->feconn, 1);
 215         if (sess->listener)
 216                 listener_release(sess->listener);
 217         session_store_counters(sess);