mirror of
https://git.freebsd.org/ports.git
synced 2025-07-18 09:49:18 -04:00
Enchance patch (abtained from haproxy ML).
This commit is contained in:
parent
2d0b9d97db
commit
efe6e2886c
Notes:
svn2git
2021-03-31 03:12:20 +00:00
svn path=/head/; revision=436416
2 changed files with 29 additions and 92 deletions
|
@ -3,7 +3,7 @@
|
||||||
|
|
||||||
PORTNAME= haproxy
|
PORTNAME= haproxy
|
||||||
PORTVERSION= 1.7.3
|
PORTVERSION= 1.7.3
|
||||||
PORTREVISION= 2
|
PORTREVISION= 3
|
||||||
CATEGORIES= net www
|
CATEGORIES= net www
|
||||||
MASTER_SITES= http://www.haproxy.org/download/1.7/src/
|
MASTER_SITES= http://www.haproxy.org/download/1.7/src/
|
||||||
DISTFILES= ${PORTNAME}-${DISTVERSION}${EXTRACT_SUFX}
|
DISTFILES= ${PORTNAME}-${DISTVERSION}${EXTRACT_SUFX}
|
||||||
|
|
|
@ -1,100 +1,37 @@
|
||||||
From afbf56b951967e8fa4d509e423fdcb11c27d40e2 Mon Sep 17 00:00:00 2001
|
--- src/connection.c.orig 2017-03-18 18:06:14.328891000 +0300
|
||||||
From: Willy Tarreau <w@1wt.eu>
|
+++ src/connection.c 2017-03-18 18:10:31.264762000 +0300
|
||||||
Date: Tue, 14 Mar 2017 20:19:29 +0100
|
@@ -136,13 +136,17 @@ void conn_fd_handler(int fd)
|
||||||
Subject: BUG/MAJOR: connection: update CO_FL_CONNECTED before calling the
|
* have been released and the FD destroyed.
|
||||||
data layer
|
*/
|
||||||
|
if ((conn->flags & CO_FL_WAKE_DATA) &&
|
||||||
Matthias Fechner reported a regression in 1.7.3 brought by the backport
|
- ((conn->flags ^ flags) & CO_FL_CONN_STATE) &&
|
||||||
of commit 819efbf ("BUG/MEDIUM: tcp: don't poll for write when connect()
|
- conn->data->wake(conn) < 0)
|
||||||
succeeds"), causing some connections to fail to establish once in a while.
|
+ (((conn->flags ^ flags) & CO_FL_CONN_STATE) ||
|
||||||
While this commit itself was a fix for a bad sequencing of connection
|
+ ((flags & CO_FL_HANDSHAKE) && !(conn->flags & CO_FL_HANDSHAKE))) &&
|
||||||
events, it in fact unveiled a much deeper bug going back to the connection
|
+ conn->data->wake(conn) < 0) {
|
||||||
rework era in v1.5-dev12 : 8f8c92f ("MAJOR: connection: add a new
|
return;
|
||||||
CO_FL_CONNECTED flag").
|
+ }
|
||||||
|
+
|
||||||
It's worth noting that in a lab reproducing a similar environment as
|
+ /* Now set the CO_FL_CONNECTED flag if the connection was just established. */
|
||||||
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))))
|
+ if (unlikely(!(conn->flags & (CO_FL_WAIT_L4_CONN | CO_FL_WAIT_L6_CONN | CO_FL_CONNECTED))))
|
||||||
+ conn->flags |= 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
|
|
||||||
* have been released and the FD destroyed.
|
|
||||||
@@ -140,10 +147,6 @@ void conn_fd_handler(int fd)
|
|
||||||
conn->data->wake(conn) < 0)
|
|
||||||
return;
|
|
||||||
|
|
||||||
- /* Last check, verify if the connection just established */
|
- /* 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))))
|
- if (unlikely(!(conn->flags & (CO_FL_WAIT_L4_CONN | CO_FL_WAIT_L6_CONN | CO_FL_CONNECTED))))
|
||||||
- conn->flags |= CO_FL_CONNECTED;
|
- conn->flags |= CO_FL_CONNECTED;
|
||||||
-
|
|
||||||
/* remove the events before leaving */
|
/* remove the events before leaving */
|
||||||
fdtab[fd].ev &= FD_POLL_STICKY;
|
fdtab[fd].ev &= FD_POLL_STICKY;
|
||||||
|
|
||||||
--
|
--- include/types/connection.h
|
||||||
1.7.12.1
|
+++ 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) */
|
||||||
|
|
||||||
|
|
Loading…
Add table
Reference in a new issue