On Wed, Apr 12, 2017 at 05:50:54PM +0200, Olivier Houchard wrote:
> On Wed, Apr 12, 2017 at 05:30:17PM +0200, Conrad Hoffmann wrote:
> > Hi again,
> > 
> > so I tried to get this to work, but didn't manage yet. I also don't quite
> > understand how this is supposed to work. The first haproxy process is
> > started _without_ the -x option, is that correct? Where does that instance
> > ever create the socket for transfer to later instances?
> > 
> > I have it working now insofar that on reload, subsequent instances are
> > spawned with the -x option, but they'll just complain that they can't get
> > anything from the unix socket (because, for all I can tell, it's not
> > there?). I also can't see the relevant code path where this socket gets
> > created, but I didn't have time to read all of it yet.
> > 
> > Am I doing something wrong? Did anyone get this to work with the
> > systemd-wrapper so far?
> > 
> > Also, but this might be a coincidence, my test setup takes a huge
> > performance penalty just by applying your patches (without any reloading
> > whatsoever). Did this happen to anybody else? I'll send some numbers and
> > more details tomorrow.
> > 
> 
> Ok I can confirm the performance issues, I'm investigating.
> 

Found it, I was messing with SO_LINGER when I shouldn't have been.

Can you try the new version of
0003-MINOR-tcp-When-binding-socket-attempt-to-reuse-one-f.patch
or replace, in src/proto_tcp.c :
        if (getsockopt(fd, SOL_SOCKET, SO_LINGER, &tmplinger, &len) == 0 &&
                    (tmplinger.l_onoff == 0 || tmplinger.l_linger == 0)) {
                        /* Attempt to activate SO_LINGER, not sure what a sane
                         * value is, using the default BSD value of 120s.
                         */
                        tmplinger.l_onoff = 1;
                        tmplinger.l_linger = 120;
                        setsockopt(fd, SOL_SOCKET, SO_LINGER, &tmplinger,
                            sizeof(tmplinger));
                }


by :
if (getsockopt(fd, SOL_SOCKET, SO_LINGER, &tmplinger, &len) == 0 &&
                    (tmplinger.l_onoff == 1 || tmplinger.l_linger == 0)) {
                        tmplinger.l_onoff = 0;
                        tmplinger.l_linger = 0;
                        setsockopt(fd, SOL_SOCKET, SO_LINGER, &tmplinger,
                            sizeof(tmplinger));
                }
        }


Thanks !

Olivier
>From 1bf2b9c550285b434c865adeb175277ce00c842b Mon Sep 17 00:00:00 2001
From: Olivier Houchard <ohouch...@haproxy.com>
Date: Wed, 5 Apr 2017 22:39:56 +0200
Subject: [PATCH 3/9] MINOR: tcp: When binding socket, attempt to reuse one
 from the old proc.

Try to reuse any socket from the old process, provided by the "-x" flag,
before binding a new one, assuming it is compatible.
"Compatible" here means same address and port, same namspace if any,
same interface if any, and that the following flags are the same :
LI_O_FOREIGN, LI_O_V6ONLY and LI_O_V4V6.
Also change tcp_bind_listener() to always enable/disable socket options,
instead of just doing so if it is in the configuration file, as the option
may have been removed, ie TCP_FASTOPEN may have been set in the old process,
and removed from the new configuration, so we have to disable it.
---
 src/proto_tcp.c | 119 +++++++++++++++++++++++++++++++++++++++++++++++++++++++-
 1 file changed, 117 insertions(+), 2 deletions(-)

diff --git a/src/proto_tcp.c b/src/proto_tcp.c
index 5e12b99..ea6b8f7 100644
--- a/src/proto_tcp.c
+++ b/src/proto_tcp.c
@@ -731,6 +731,83 @@ int tcp_connect_probe(struct connection *conn)
        return 0;
 }
 
+/* XXX: Should probably be elsewhere */
+static int compare_sockaddr(struct sockaddr_storage *a, struct 
sockaddr_storage *b)
+{
+       if (a->ss_family != b->ss_family) {
+               return (-1);
+       }
+       switch (a->ss_family) {
+       case AF_INET:
+               {
+                       struct sockaddr_in *a4 = (void *)a, *b4 = (void *)b;
+                       if (a4->sin_port != b4->sin_port)
+                               return (-1);
+                       return (memcmp(&a4->sin_addr, &b4->sin_addr,
+                           sizeof(a4->sin_addr)));
+               }
+       case AF_INET6:
+               {
+                       struct sockaddr_in6 *a6 = (void *)a, *b6 = (void *)b;
+                       if (a6->sin6_port != b6->sin6_port)
+                               return (-1);
+                       return (memcmp(&a6->sin6_addr, &b6->sin6_addr,
+                           sizeof(a6->sin6_addr)));
+               }
+       default:
+               return (-1);
+       }
+
+}
+
+#define LI_MANDATORY_FLAGS     (LI_O_FOREIGN | LI_O_V6ONLY | LI_O_V4V6)
+/* When binding the listeners, check if a socket has been sent to us by the
+ * previous process that we could reuse, instead of creating a new one.
+ */
+static int tcp_find_compatible_fd(struct listener *l)
+{
+       struct xfer_sock_list *xfer_sock = xfer_sock_list;
+       int ret = -1;
+
+       while (xfer_sock) {
+               if (!compare_sockaddr(&xfer_sock->addr, &l->addr)) {
+                       if ((l->interface == NULL && xfer_sock->iface == NULL) 
||
+                           (l->interface != NULL && xfer_sock->iface != NULL &&
+                            !strcmp(l->interface, xfer_sock->iface))) {
+                               if ((l->options & LI_MANDATORY_FLAGS) ==
+                                   (xfer_sock->options & LI_MANDATORY_FLAGS)) {
+                                       if ((xfer_sock->namespace == NULL &&
+                                           l->netns == NULL)
+#ifdef CONFIG_HAP_NS
+                                           || (xfer_sock->namespace != NULL &&
+                                           l->netns != NULL &&
+                                           !strcmp(xfer_sock->namespace,
+                                           l->netns->node.key))
+#endif
+                                          ) {
+                                               break;
+                                       }
+
+                               }
+                       }
+               }
+               xfer_sock = xfer_sock->next;
+       }
+       if (xfer_sock != NULL) {
+               ret = xfer_sock->fd;
+               if (xfer_sock == xfer_sock_list)
+                       xfer_sock_list = xfer_sock->next;
+               if (xfer_sock->prev)
+                       xfer_sock->prev->next = xfer_sock->next;
+               if (xfer_sock->next)
+                       xfer_sock->next->prev = xfer_sock->next->prev;
+               free(xfer_sock->iface);
+               free(xfer_sock->namespace);
+               free(xfer_sock);
+       }
+       return ret;
+}
+#undef L1_MANDATORY_FLAGS
 
 /* This function tries to bind a TCPv4/v6 listener. It may return a warning or
  * an error message in <errmsg> if the message is at most <errlen> bytes long
@@ -762,6 +839,9 @@ int tcp_bind_listener(struct listener *listener, char 
*errmsg, int errlen)
 
        err = ERR_NONE;
 
+       if (listener->fd == -1)
+               listener->fd = tcp_find_compatible_fd(listener);
+
        /* if the listener already has an fd assigned, then we were offered the
         * fd by an external process (most likely the parent), and we don't want
         * to create a new socket. However we still want to set a few flags on
@@ -800,6 +880,17 @@ int tcp_bind_listener(struct listener *listener, char 
*errmsg, int errlen)
 
        if (listener->options & LI_O_NOLINGER)
                setsockopt(fd, SOL_SOCKET, SO_LINGER, &nolinger, sizeof(struct 
linger));
+       else {
+               struct linger tmplinger;
+               socklen_t len = sizeof(tmplinger);
+               if (getsockopt(fd, SOL_SOCKET, SO_LINGER, &tmplinger, &len) == 
0 &&
+                   (tmplinger.l_onoff == 1 || tmplinger.l_linger == 0)) {
+                       tmplinger.l_onoff = 0;
+                       tmplinger.l_linger = 0;
+                       setsockopt(fd, SOL_SOCKET, SO_LINGER, &tmplinger,
+                           sizeof(tmplinger));
+               }
+       }
 
 #ifdef SO_REUSEPORT
        /* OpenBSD and Linux 3.9 support this. As it's present in old libc 
versions of
@@ -870,6 +961,9 @@ int tcp_bind_listener(struct listener *listener, char 
*errmsg, int errlen)
                        err |= ERR_WARN;
                }
        }
+       /* XXX: No else, the way to get the default MSS will vary from system
+        * to system.
+        */
 #endif
 #if defined(TCP_USER_TIMEOUT)
        if (listener->tcp_ut) {
@@ -878,7 +972,9 @@ int tcp_bind_listener(struct listener *listener, char 
*errmsg, int errlen)
                        msg = "cannot set TCP User Timeout";
                        err |= ERR_WARN;
                }
-       }
+       } else
+               setsockopt(fd, IPPROTO_TCP, TCP_USER_TIMEOUT, &zero,
+                   sizeof(zero));
 #endif
 #if defined(TCP_DEFER_ACCEPT)
        if (listener->options & LI_O_DEF_ACCEPT) {
@@ -888,7 +984,9 @@ int tcp_bind_listener(struct listener *listener, char 
*errmsg, int errlen)
                        msg = "cannot enable DEFER_ACCEPT";
                        err |= ERR_WARN;
                }
-       }
+       } else
+               setsockopt(fd, IPPROTO_TCP, TCP_DEFER_ACCEPT, &zero,
+                   sizeof(zero));
 #endif
 #if defined(TCP_FASTOPEN)
        if (listener->options & LI_O_TCP_FO) {
@@ -898,6 +996,21 @@ int tcp_bind_listener(struct listener *listener, char 
*errmsg, int errlen)
                        msg = "cannot enable TCP_FASTOPEN";
                        err |= ERR_WARN;
                }
+       } else {
+               socklen_t len;
+               int qlen;
+               len = sizeof(qlen);
+               /* Only disable fast open if it was enabled, we don't want
+                * the kernel to create a fast open queue if there's none.
+                */
+               if (getsockopt(fd, IPPROTO_TCP, TCP_FASTOPEN, &qlen, &len) == 0 
&&
+                   qlen != 0) {
+                       if (setsockopt(fd, IPPROTO_TCP, TCP_FASTOPEN, &zero,
+                           sizeof(zero)) == -1) {
+                               msg = "cannot disable TCP_FASTOPEN";
+                               err |= ERR_WARN;
+                       }
+               }
        }
 #endif
 #if defined(IPV6_V6ONLY)
@@ -928,6 +1041,8 @@ int tcp_bind_listener(struct listener *listener, char 
*errmsg, int errlen)
 #if defined(TCP_QUICKACK)
        if (listener->options & LI_O_NOQUICKACK)
                setsockopt(fd, IPPROTO_TCP, TCP_QUICKACK, &zero, sizeof(zero));
+       else
+               setsockopt(fd, IPPROTO_TCP, TCP_QUICKACK, &one, sizeof(one));
 #endif
 
        /* the socket is ready */
-- 
2.9.3

Reply via email to