Hi Squid devs,
Below I include a patch to enable configurable SO_KEEPALIVE on outbound
connections from Squid. Amos helped me out with some advice on
squid-users@ and I've diverged a bit from the advice -- I'd like to
explain my reasoning:
1) This patch simply enables SO_KEEPALIVE for outbound connections if a
global "tcp_keepalive on" is configured. TCP keepalive settings are
distinct from protocol-level timeouts and I do not want to conflate them.
2) I did not use commSetTcpKeepalive() as I believe it may have an issue
in its handling of TCP_KEEPCNT. There are four relevant keepalive values:
a) SO_KEEPALIVE, boolean. The only portable value.
b) TCP_KEEPIDLE, idle time in seconds before the socket is placed in
keepalive mode. AKA tcp_keepalive_time on linux
c) TCP_KEEPINTVL, seconds between probes, once keepalive mode is
engaged. AKA tcp_keepalive_intvl on linux
d) TCP_KEEPCNT, the maximum number of keepalive probes sent before
dropping the connection.
commSetTcpKeepalive() appears to treat TCP_KEEPCNT as a time value, rather
than a count value:
> if (timeout && interval) {
> int count = (timeout + interval - 1) / interval;
> if (setsockopt(fd, IPPROTO_TCP, TCP_KEEPCNT, &count, sizeof(on)) < 0)
I would suggest a larger patch modifying commSetTcpKeepalive() to replace
"int timeout" with "int keep_count", and replace the above three lines
with only:
> if (setsockopt(fd, IPPROTO_TCP, TCP_KEEPCNT, &keep_count, sizeof(on))
> < 0)
If squid-dev is amenable to this change I will submit another patch for
this issue. It's a behavior change so I wanted to keep it separate from
the feature patch I include below. I would recommend:
* Introduce new keep_count= parameter
* Retain current behavior if keep_count= is not present and timeout= is
set to avoid changing existing behavior
* Document deprecation of timeout= parameter
3) I only include a boolean tcp_keepalive, rather than passing in args for
the remaining 3 parameters. I'm a bit confused as to how I should use
cf.data.pre to pass config parameters. If someone could point me at how to
supply something like this I'd appreciate it:
tcp_keepalive[=idle,interval,keep_count]
Patch:
diff -ur a/src/cf.data.pre b/src/cf.data.pre
--- a/src/cf.data.pre 2013-12-30 11:33:27.000000000 +0000
+++ b/src/cf.data.pre 2014-01-28 04:44:09.823041592 +0000
@@ -7093,6 +7093,14 @@
the server generating a directory listing.
DOC_END
+NAME: tcp_keepalive
+TYPE: onoff
+LOC: Config.onoff.tcp_keepalive
+DEFAULT: off
+DOC_START
+ This directive enables TCP keepalive for outbound connections.
+DOC_END
+
NAME: short_icon_urls
TYPE: onoff
LOC: Config.icons.use_short_names
diff -ur a/src/comm.cc b/src/comm.cc
--- a/src/comm.cc 2013-12-30 11:33:27.000000000 +0000
+++ b/src/comm.cc 2014-01-28 05:50:23.242620051 +0000
@@ -702,10 +702,14 @@
commSetTcpNoDelay(new_socket);
#endif
-
if (Config.tcpRcvBufsz > 0 && sock_type == SOCK_STREAM)
commSetTcpRcvbuf(new_socket, Config.tcpRcvBufsz);
+ if (Config.onoff.tcp_keepalive) {
+ int on = 1;
+ setsockopt(new_socket, SOL_SOCKET, SO_KEEPALIVE, (char *) &on,
sizeof(on));
+ }
+
return new_socket;
}
diff -ur a/src/SquidConfig.h b/src/SquidConfig.h
--- a/src/SquidConfig.h 2013-12-30 11:33:27.000000000 +0000
+++ b/src/SquidConfig.h 2014-01-28 05:06:59.588199383 +0000
@@ -341,6 +341,7 @@
int emailErrData;
int httpd_suppress_version_string;
int global_internal_static;
+ int tcp_keepalive;
#if FOLLOW_X_FORWARDED_FOR
int acl_uses_indirect_client;
