On Tue, Sep 29, 2015 at 02:58:04PM +0200, joris dedieu wrote:
> kevent(3,0x0,0,{},5,{1.000000000 }) = 0 (0x0)
> kevent(3,0x0,0,{0x4,EVFILT_READ,0x0,0,0x1,0x0},5,{1.000000000 }) = 1 (0x1)
> accept(4,{ AF_INET 80.247.233.242:48068 },0x7fffffffe804) = 5 (0x5)
> fcntl(5,F_SETFL,O_NONBLOCK) = 0 (0x0)
> recvfrom(5,0x801407000,16384,0x20080,0x0,0x0) ERR#35 'Resource
> temporarily unavailable'
> setsockopt(0x5,0x0,0x4,0x48668c,0x4,0x0) = 0 (0x0)
> setsockopt(0x5,0x6,0x1,0x48668c,0x4,0x0) = 0 (0x0)
> accept(4,0x7fffffffe808,0x7fffffffe804) ERR#35 'Resource
> temporarily unavailable'
> shutdown(5,SHUT_WR) = 0 (0x0)
> close(5) = 0 (0x0)
>
> Has you can see it doesn't looks great.
OK I found the reason, in my case the RST I was seeing was caused by pending
data otherwise haproxy didn't send it by itself since we're facing the client.
I've fixed it so that lingering is *really* disabled this time. You can retry
with the attached patch if you want. The second one will get rid of the
useless recvfrom() call if your system doesn't have TCP_QUICKACK. The third
patch addresses a build issue reported off-list on another FreeBSD machine
(SOL_IP not defined).
Thanks,
Willy
>From f50ec0fdbc7f5c8eff3fa91c09ce19c5df3cf8d6 Mon Sep 17 00:00:00 2001
From: Willy Tarreau <[email protected]>
Date: Tue, 29 Sep 2015 18:11:32 +0200
Subject: BUG/MINOR: tcp: make silent-drop always force a TCP reset
The silent-drop action is supposed to close with a TCP reset that is
either not sent or not too far. But since it's on the client-facing
side, the socket's lingering is enabled by default and the RST only
occurs if some pending unread data remain in the queue when closing.
This causes some clean shutdowns to occur with retransmits, which is
not good at all. Force linger_risk on the socket to flush all data
and destroy the socket.
No backport is needed, this was introduced in 1.6-dev6.
---
src/proto_tcp.c | 5 +++++
1 file changed, 5 insertions(+)
diff --git a/src/proto_tcp.c b/src/proto_tcp.c
index f698889..4c5005e 100644
--- a/src/proto_tcp.c
+++ b/src/proto_tcp.c
@@ -1445,6 +1445,11 @@ static enum act_return
tcp_exec_action_silent_drop(struct act_rule *rule, struct
if (strm)
strm->si[0].flags |= SI_FL_NOLINGER;
+ /* We're on the client-facing side, we must force to disable lingering
to
+ * ensure we will use an RST exclusively and kill any pending data.
+ */
+ fdtab[conn->t.sock.fd].linger_risk = 1;
+
#ifdef TCP_REPAIR
if (setsockopt(conn->t.sock.fd, SOL_TCP, TCP_REPAIR, &one, sizeof(one))
== 0) {
/* socket will be quiet now */
--
1.7.12.1
>From fc2a2d97d6855e9a00a22bc26623e4f2f2c1aeda Mon Sep 17 00:00:00 2001
From: Willy Tarreau <[email protected]>
Date: Tue, 29 Sep 2015 18:15:01 +0200
Subject: CLEANUP: tcp: silent-drop: only drain the connection when quick-ack
is disabled
The conn_sock_drain() call is only there to force the system to ACK
pending data in case of TCP_QUICKACK so that the client doesn't retransmit,
otherwise it leads to a real RST making the feature useless. There's no
point in draining the connection when quick ack cannot be disabled, so
let's move the call inside the ifdef part.
---
src/proto_tcp.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/src/proto_tcp.c b/src/proto_tcp.c
index 4c5005e..0655b0d 100644
--- a/src/proto_tcp.c
+++ b/src/proto_tcp.c
@@ -1431,8 +1431,10 @@ static enum act_return
tcp_exec_action_silent_drop(struct act_rule *rule, struct
if (!conn_ctrl_ready(conn))
goto out;
- conn_sock_drain(conn);
#ifdef TCP_QUICKACK
+ /* drain is needed only to send the quick ACK */
+ conn_sock_drain(conn);
+
/* re-enable quickack if it was disabled to ack all data and avoid
* retransmits from the client that might trigger a real reset.
*/
--
1.7.12.1
>From ae459f3b9f9810d18aaa090264398dee2d4de23b Mon Sep 17 00:00:00 2001
From: Willy Tarreau <[email protected]>
Date: Tue, 29 Sep 2015 18:19:32 +0200
Subject: BUILD: tcp: use IPPROTO_IP when SOL_IP is not available
Dmitry Sivachenko reported a build failure on FreeBSD due to SOL_IP not
being defined. IPPROTO_IP must be used there instead.
---
include/common/compat.h | 5 +++++
1 file changed, 5 insertions(+)
diff --git a/include/common/compat.h b/include/common/compat.h
index 07dd01d..b2e16af 100644
--- a/include/common/compat.h
+++ b/include/common/compat.h
@@ -129,6 +129,11 @@
#endif
#endif
+/* FreeBSD doesn't define SOL_IP and prefers IPPROTO_IP */
+#ifndef SOL_IP
+#define SOL_IP IPPROTO_IP
+#endif
+
/* If IPv6 is supported, define IN6_IS_ADDR_V4MAPPED() if missing. */
#if defined(IPV6_TCLASS) && !defined(IN6_IS_ADDR_V4MAPPED)
#define IN6_IS_ADDR_V4MAPPED(a) \
--
1.7.12.1
