[MEDIUM] proxy: add a PAUSED state to listeners and move socket tricks out of proxy.c

Managing listeners state is difficult because they have their own state
and can at the same time have theirs dictated by their proxy. The pause
is not done properly, as the proxy code is fiddling with sockets. By
introducing new functions such as pause_listener()/resume_listener(), we
make it a bit more obvious how/when they're supposed to be used. The
listen_proxies() function was also renamed to resume_proxies() since
it's only used for pause/resume.

This patch is the first in a series aiming at getting rid of the maintain_proxies
mess. In the end, proxies should not call enable_listener()/disable_listener()
anymore.

diff --git a/include/proto/protocols.h b/include/proto/protocols.h
index d0364c047dec8df4f7ed0e04cd13769800084b56..e0d6ee432d49696c4a2a185802b6d3ea6334a8c2 100644 (file)
--- a/include/proto/protocols.h
+++ b/include/proto/protocols.h
@@ -36,6 +36,21 @@ void enable_listener(struct listener *listener);
  */
 void disable_listener(struct listener *listener);
 
+/* This function tries to temporarily disable a listener, depending on the OS
+ * capabilities. Linux unbinds the listen socket after a SHUT_RD, and ignores
+ * SHUT_WR. Solaris refuses either shutdown(). OpenBSD ignores SHUT_RD but
+ * closes upon SHUT_WR and refuses to rebind. So a common validation path
+ * involves SHUT_WR && listen && SHUT_RD. In case of success, the FD's polling
+ * is disabled. It normally returns non-zero, unless an error is reported.
+ */
+int pause_listener(struct listener *l);
+
+/* This function tries to resume a temporarily disabled listener.
+ * The resulting state will either be LI_READY or LI_FULL. 0 is returned
+ * in case of failure to resume (eg: dead socket).
+ */
+int resume_listener(struct listener *l);
+
 /* This function adds all of the protocol's listener's file descriptors to the
  * polling lists when they are in the LI_LISTEN state. It is intended to be
  * used as a protocol's generic enable_all() primitive, for use after the

diff --git a/include/proto/proxy.h b/include/proto/proxy.h
index ed29f145941218352cb48bc8f259b9611d0cef94..c9bfc027650ade2df833fd2e3ca4077c7b95e7e6 100644 (file)
--- a/include/proto/proxy.h
+++ b/include/proto/proxy.h
@@ -34,7 +34,7 @@ void soft_stop(void);
 void pause_proxy(struct proxy *p);
 void stop_proxy(struct proxy *p);
 void pause_proxies(void);
-void listen_proxies(void);
+void resume_proxies(void);
 int  session_set_backend(struct session *s, struct proxy *be);
 
 const char *proxy_cap_str(int cap);

diff --git a/include/types/protocols.h b/include/types/protocols.h
index 3dcb2e7400c5227ee5098cb24ca068d231a93984..a333ea2fc6b5aa17ebb681d1b29ca75eb3b91ca9 100644 (file)
--- a/include/types/protocols.h
+++ b/include/types/protocols.h
@@ -1,23 +1,23 @@
 /*
-  include/types/protocols.h
-  This file defines the structures used by generic network protocols.
-
-  Copyright (C) 2000-2009 Willy Tarreau - w@1wt.eu
-
-  This library is free software; you can redistribute it and/or
-  modify it under the terms of the GNU Lesser General Public
-  License as published by the Free Software Foundation, version 2.1
-  exclusively.
-
-  This library is distributed in the hope that it will be useful,
-  but WITHOUT ANY WARRANTY; without even the implied warranty of
-  MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
-  Lesser General Public License for more details.
-
-  You should have received a copy of the GNU Lesser General Public
-  License along with this library; if not, write to the Free Software
-  Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA  02110-1301  USA
-*/
+ * include/types/protocols.h
+ * This file defines the structures used by generic network protocols.
+ *
+ * Copyright (C) 2000-2011 Willy Tarreau - w@1wt.eu
+ *
+ * This library is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU Lesser General Public
+ * License as published by the Free Software Foundation, version 2.1
+ * exclusively.
+ *
+ * This library is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+ * Lesser General Public License for more details.
+ *
+ * You should have received a copy of the GNU Lesser General Public
+ * License along with this library; if not, write to the Free Software
+ * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA  02110-1301  USA
+ */
 
 #ifndef _TYPES_PROTOCOLS_H
 #define _TYPES_PROTOCOLS_H
@@ -38,12 +38,15 @@
 #define PROTO_NAME_LEN 16
 
 /* listener state */
-#define LI_NEW         0       /* not initialized yet */
-#define LI_INIT                1       /* all parameters filled in, but not assigned yet */
-#define LI_ASSIGNED    2       /* assigned to the protocol, but not listening yet */
-#define LI_LISTEN      3       /* started, listening but not enabled */
-#define LI_READY       4       /* started, listening and enabled */
-#define LI_FULL                5       /* reached its connection limit */
+enum {
+       LI_NEW  = 0,    /* not initialized yet */
+       LI_INIT,        /* all parameters filled in, but not assigned yet */
+       LI_ASSIGNED,    /* assigned to the protocol, but not listening yet */
+       LI_PAUSED,      /* listener was paused, it's bound but not listening  */
+       LI_LISTEN,      /* started, listening but not enabled */
+       LI_READY,       /* started, listening and enabled */
+       LI_FULL,        /* reached its connection limit */
+};
 
 /* Listener transitions
  * calloc()     set()      add_listener()       bind()

diff --git a/src/haproxy.c b/src/haproxy.c
index 4c20331bb81f549bfa5f32eb5409403c12511d2c..8da7aa9458235bd321454d519edfb4901dd42180 100644 (file)
--- a/src/haproxy.c
+++ b/src/haproxy.c
@@ -281,7 +281,7 @@ void sig_pause(struct sig_handler *sh)
  */
 void sig_listen(struct sig_handler *sh)
 {
-       listen_proxies();
+       resume_proxies();
 }
 
 /*

diff --git a/src/proto_uxst.c b/src/proto_uxst.c
index 37abb4cc5148b1b3c1679b5897c7b939abf98b0c..6fb7264d52c626f636114e8347f51c39445853fc 100644 (file)
--- a/src/proto_uxst.c
+++ b/src/proto_uxst.c
@@ -261,12 +261,8 @@ static int uxst_bind_listener(struct listener *listener, char *errmsg, int errle
  */
 static int uxst_unbind_listener(struct listener *listener)
 {
-       if (listener->state == LI_READY)
-               EV_FD_CLR(listener->fd, DIR_RD);
-
-       if (listener->state >= LI_LISTEN) {
-               fd_delete(listener->fd);
-               listener->state = LI_ASSIGNED;
+       if (listener->state > LI_ASSIGNED) {
+               unbind_listener(listener);
                destroy_uxst_socket(((struct sockaddr_un *)&listener->addr)->sun_path);
        }
        return ERR_NONE;

diff --git a/src/protocols.c b/src/protocols.c
index 5eddb2c593c7e5a350f4d6881613a1cf30d0cfd3..49e7eaaa2e465cac5680c45e4e8a527a852f1f51 100644 (file)
--- a/src/protocols.c
+++ b/src/protocols.c
@@ -53,6 +53,59 @@ void disable_listener(struct listener *listener)
        listener->state = LI_LISTEN;
 }
 
+/* This function tries to temporarily disable a listener, depending on the OS
+ * capabilities. Linux unbinds the listen socket after a SHUT_RD, and ignores
+ * SHUT_WR. Solaris refuses either shutdown(). OpenBSD ignores SHUT_RD but
+ * closes upon SHUT_WR and refuses to rebind. So a common validation path
+ * involves SHUT_WR && listen && SHUT_RD. In case of success, the FD's polling
+ * is disabled. It normally returns non-zero, unless an error is reported.
+ */
+int pause_listener(struct listener *l)
+{
+       if (l->state <= LI_PAUSED)
+               return 1;
+
+       if (shutdown(l->fd, SHUT_WR) != 0)
+               return 0; /* Solaris dies here */
+
+       if (listen(l->fd, l->backlog ? l->backlog : l->maxconn) != 0)
+               return 0; /* OpenBSD dies here */
+
+       if (shutdown(l->fd, SHUT_RD) != 0)
+               return 0; /* should always be OK */
+
+       EV_FD_CLR(l->fd, DIR_RD);
+       l->state = LI_PAUSED;
+       return 1;
+}
+
+/* This function tries to resume a temporarily disabled listener. Paused, full
+ * and disabled listeners are handled, which means that this function may
+ * replace enable_listener(). The resulting state will either be LI_READY or
+ * LI_FULL. 0 is returned in case of failure to resume (eg: dead socket).
+ */
+int resume_listener(struct listener *l)
+{
+       if (l->state < LI_PAUSED)
+               return 0;
+
+       if (l->state == LI_PAUSED &&
+           listen(l->fd, l->backlog ? l->backlog : l->maxconn) != 0)
+               return 0;
+
+       if (l->state == LI_READY)
+               return 1;
+
+       if (l->nbconn >= l->maxconn) {
+               l->state = LI_FULL;
+               return 1;
+       }
+
+       EV_FD_SET(l->fd, DIR_RD);
+       l->state = LI_READY;
+       return 1;
+}
+
 /* This function adds all of the protocol's listener's file descriptors to the
  * polling lists when they are in the LI_LISTEN state. It is intended to be
  * used as a protocol's generic enable_all() primitive, for use after the
@@ -92,7 +145,7 @@ int unbind_listener(struct listener *listener)
        if (listener->state == LI_READY)
                EV_FD_CLR(listener->fd, DIR_RD);
 
-       if (listener->state >= LI_LISTEN) {
+       if (listener->state >= LI_PAUSED) {
                fd_delete(listener->fd);
                listener->state = LI_ASSIGNED;
        }

diff --git a/src/proxy.c b/src/proxy.c
index 02e1ee50c47ea42053f74da657122c5e800fd1cb..2184a1bcb0b9ae87fc9809014f3f817563f62f7d 100644 (file)
--- a/src/proxy.c
+++ b/src/proxy.c
@@ -589,28 +589,20 @@ void soft_stop(void)
 }
 
 
-/*
- * Linux unbinds the listen socket after a SHUT_RD, and ignores SHUT_WR.
- * Solaris refuses either shutdown().
- * OpenBSD ignores SHUT_RD but closes upon SHUT_WR and refuses to rebind.
- * So a common validation path involves SHUT_WR && listen && SHUT_RD.
- * If disabling at least one listener returns an error, then the proxy
- * state is set to PR_STERROR because we don't know how to resume from this.
+/* Temporarily disables listening on all of the proxy's listeners. Upon
+ * success, the proxy enters the PR_PAUSED state. If disabling at least one
+ * listener returns an error, then the proxy state is set to PR_STERROR
+ * because we don't know how to resume from this.
  */
 void pause_proxy(struct proxy *p)
 {
        struct listener *l;
        for (l = p->listen; l != NULL; l = l->next) {
-               if (shutdown(l->fd, SHUT_WR) == 0 &&
-                   listen(l->fd, p->backlog ? p->backlog : p->maxconn) == 0 &&
-                   shutdown(l->fd, SHUT_RD) == 0) {
-                       EV_FD_CLR(l->fd, DIR_RD);
-                       if (p->state != PR_STERROR)
-                               p->state = PR_STPAUSED;
-               }
-               else
+               if (!pause_listener(l))
                        p->state = PR_STERROR;
        }
+       if (p->state != PR_STERROR)
+               p->state = PR_STPAUSED;
 }
 
 
@@ -700,10 +692,11 @@ void pause_proxies(void)
  * sig_pause(), for example when a new instance has failed starting up.
  * It is designed to be called upon reception of a SIGTTIN.
  */
-void listen_proxies(void)
+void resume_proxies(void)
 {
        struct proxy *p;
        struct listener *l;
+       int fail;
 
        p = proxy;
        tv_update_date(0,1); /* else, the old time before select will be used */
@@ -712,15 +705,9 @@ void listen_proxies(void)
                        Warning("Enabling %s %s.\n", proxy_cap_str(p->cap), p->id);
                        send_log(p, LOG_WARNING, "Enabling %s %s.\n", proxy_cap_str(p->cap), p->id);
 
+                       fail = 0;
                        for (l = p->listen; l != NULL; l = l->next) {
-                               if (listen(l->fd, p->backlog ? p->backlog : p->maxconn) == 0) {
-                                       if (actconn < global.maxconn && p->feconn < p->maxconn) {
-                                               EV_FD_SET(l->fd, DIR_RD);
-                                               p->state = PR_STRUN;
-                                       }
-                                       else
-                                               p->state = PR_STIDLE;
-                               } else {
+                               if (!resume_listener(l)) {
                                        int port;
 
                                        if (l->addr.ss_family == AF_INET6) {
@@ -745,10 +732,15 @@ void listen_proxies(void)
                                        }
 
                                        /* Another port might have been enabled. Let's stop everything. */
-                                       pause_proxy(p);
+                                       fail = 1;
                                        break;
                                }
                        }
+
+                       /* maintain_proxies() will check if the proxy may remain enabled or not */
+                       p->state = PR_STRUN;
+                       if (fail)
+                               pause_proxy(p);
                }
                p = p->next;
        }