Date: Sat, 18 Mar 2017 15:15:01 +0000 (UTC) From: Dmitry Sivachenko <demon@FreeBSD.org> To: ports-committers@freebsd.org, svn-ports-all@freebsd.org, svn-ports-head@freebsd.org Subject: svn commit: r436416 - in head/net/haproxy: . files Message-ID: <201703181515.v2IFF1tC090631@repo.freebsd.org>
next in thread | raw e-mail | index | archive | help
Author: demon Date: Sat Mar 18 15:15:01 2017 New Revision: 436416 URL: https://svnweb.freebsd.org/changeset/ports/436416 Log: Enchance patch (abtained from haproxy ML). Modified: head/net/haproxy/Makefile head/net/haproxy/files/patch-src-proto_tcp.c Modified: head/net/haproxy/Makefile ============================================================================== --- head/net/haproxy/Makefile Sat Mar 18 15:14:09 2017 (r436415) +++ head/net/haproxy/Makefile Sat Mar 18 15:15:01 2017 (r436416) @@ -3,7 +3,7 @@ PORTNAME= haproxy PORTVERSION= 1.7.3 -PORTREVISION= 2 +PORTREVISION= 3 CATEGORIES= net www MASTER_SITES= http://www.haproxy.org/download/1.7/src/ DISTFILES= ${PORTNAME}-${DISTVERSION}${EXTRACT_SUFX} Modified: head/net/haproxy/files/patch-src-proto_tcp.c ============================================================================== --- head/net/haproxy/files/patch-src-proto_tcp.c Sat Mar 18 15:14:09 2017 (r436415) +++ head/net/haproxy/files/patch-src-proto_tcp.c Sat Mar 18 15:15:01 2017 (r436416) @@ -1,100 +1,37 @@ -From afbf56b951967e8fa4d509e423fdcb11c27d40e2 Mon Sep 17 00:00:00 2001 -From: Willy Tarreau <w@1wt.eu> -Date: Tue, 14 Mar 2017 20:19:29 +0100 -Subject: BUG/MAJOR: connection: update CO_FL_CONNECTED before calling the - data layer - -Matthias Fechner reported a regression in 1.7.3 brought by the backport -of commit 819efbf ("BUG/MEDIUM: tcp: don't poll for write when connect() -succeeds"), causing some connections to fail to establish once in a while. -While this commit itself was a fix for a bad sequencing of connection -events, it in fact unveiled a much deeper bug going back to the connection -rework era in v1.5-dev12 : 8f8c92f ("MAJOR: connection: add a new -CO_FL_CONNECTED flag"). - -It's worth noting that in a lab reproducing a similar environment as -Matthias' about only 1 every 19000 connections exhibit this behaviour, -making the issue not so easy to observe. A trick to make the problem -more observable consists in disabling non-blocking mode on the socket -before calling connect() and re-enabling it later, so that connect() -always succeeds. Then it becomes 100% reproducible. - -The problem is that this CO_FL_CONNECTED flag is tested after deciding to -call the data layer (typically the stream interface but might be a health -check as well), and that the decision to call the data layer relies on a -change of one of the flags covered by the CO_FL_CONN_STATE set, which is -made of CO_FL_CONNECTED among others. - -Before the fix above, this bug couldn't appear with TCP but it could -appear with Unix sockets. Indeed, connect() was always considered -blocking so the CO_FL_WAIT_L4_CONN connection flag was always set, and -polling for write events was always enabled. This used to guarantee that -the conn_fd_handler() could detect a change among the CO_FL_CONN_STATE -flags. - -Now with the fix above, if a connect() immediately succeeds for non-ssl -connection with send-proxy enabled, and no data in the buffer (thus TCP -mode only), the CO_FL_WAIT_L4_CONN flag is not set, the lack of data in -the buffer doesn't enable polling flags for the data layer, the -CO_FL_CONNECTED flag is not set due to send-proxy still being pending, -and once send-proxy is done, its completion doesn't cause the data layer -to be woken up due to the fact that CO_FL_CONNECT is still not present -and that the CO_FL_SEND_PROXY flag is not watched in CO_FL_CONN_STATE. - -Then no progress is made when data are received from the client (and -attempted to be forwarded), because a CF_WRITE_NULL (or CF_WRITE_PARTIAL) -flag is needed for the stream-interface state to turn from SI_ST_CON to -SI_ST_EST, allowing ->chk_snd() to be called when new data arrive. And -the only way to set this flag is to call the data layer of course. - -After the connect timeout, the connection gets killed and if in the mean -time some data have accumulated in the buffer, the retry will succeed. - -This patch fixes this situation by simply placing the update of -CO_FL_CONNECTED where it should have been, before the check for a flag -change needed to wake up the data layer and not after. - -This fix must be backported to 1.7, 1.6 and 1.5. Versions not having -the patch above are still affected for unix sockets. - -Special thanks to Matthias Fechner who provided a very detailed bug -report with a bisection designating the faulty patch, and to Olivier -Houchard for providing full access to a pretty similar environment where -the issue could first be reproduced. -(cherry picked from commit 7bf3fa3c23f6a1b7ed1212783507ac50f7e27544) ---- - src/connection.c | 11 +++++++---- - 1 file changed, 7 insertions(+), 4 deletions(-) - -diff --git a/src/connection.c b/src/connection.c -index 26fc5f6..1e4c9aa 100644 ---- src/connection.c -+++ src/connection.c -@@ -131,6 +131,13 @@ void conn_fd_handler(int fd) - } - - leave: -+ /* Verify if the connection just established. The CO_FL_CONNECTED flag -+ * being included in CO_FL_CONN_STATE, its change will be noticed by -+ * the next block and be used to wake up the data layer. -+ */ -+ if (unlikely(!(conn->flags & (CO_FL_WAIT_L4_CONN | CO_FL_WAIT_L6_CONN | CO_FL_CONNECTED)))) -+ conn->flags |= CO_FL_CONNECTED; -+ - /* The wake callback may be used to process a critical error and abort the - * connection. If so, we don't want to go further as the connection will +--- src/connection.c.orig 2017-03-18 18:06:14.328891000 +0300 ++++ src/connection.c 2017-03-18 18:10:31.264762000 +0300 +@@ -136,13 +136,17 @@ void conn_fd_handler(int fd) * have been released and the FD destroyed. -@@ -140,10 +147,6 @@ void conn_fd_handler(int fd) - conn->data->wake(conn) < 0) + */ + if ((conn->flags & CO_FL_WAKE_DATA) && +- ((conn->flags ^ flags) & CO_FL_CONN_STATE) && +- conn->data->wake(conn) < 0) ++ (((conn->flags ^ flags) & CO_FL_CONN_STATE) || ++ ((flags & CO_FL_HANDSHAKE) && !(conn->flags & CO_FL_HANDSHAKE))) && ++ conn->data->wake(conn) < 0) { return; ++ } ++ ++ /* Now set the CO_FL_CONNECTED flag if the connection was just established. */ ++ ++ if (unlikely(!(conn->flags & (CO_FL_WAIT_L4_CONN | CO_FL_WAIT_L6_CONN | CO_FL_CONNECTED)))) ++ conn->flags |= CO_FL_CONNECTED; - /* Last check, verify if the connection just established */ - if (unlikely(!(conn->flags & (CO_FL_WAIT_L4_CONN | CO_FL_WAIT_L6_CONN | CO_FL_CONNECTED)))) - conn->flags |= CO_FL_CONNECTED; -- + /* remove the events before leaving */ fdtab[fd].ev &= FD_POLL_STICKY; - --- -1.7.12.1 +--- include/types/connection.h ++++ include/types/connection.h +@@ -98,7 +98,7 @@ enum { + + /* flags used to report connection status and errors */ + CO_FL_ERROR = 0x00100000, /* a fatal error was reported */ +- CO_FL_CONNECTED = 0x00200000, /* the connection is now established */ ++ CO_FL_CONNECTED = 0x00200000, /* L4+L6 now ready ; extra handshakes may or may not exist */ + CO_FL_WAIT_L4_CONN = 0x00400000, /* waiting for L4 to be connected */ + CO_FL_WAIT_L6_CONN = 0x00800000, /* waiting for L6 to be connected (eg: SSL) */ +
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?201703181515.v2IFF1tC090631>