Hi, Committed this one too, but after a number of changes. See the final results on syslog-ng 3.4 master. Comments that triggered the changes are below for your reference: On Thu, 2012-08-16 at 17:31 +0200, Gergely Nagy wrote:

With the new tcp_keep_alive_time() option one can control on a per-source basis the time after which a TCP connection will start sending TCP keepalive probes (assuming so-keepalive(yes)).

While it is accepted for all stream sources, it only makes sense for TCP-based streams.

By default, it's value is 0, which means using the kernel default (2 hours).

Signed-off-by: Gergely Nagy <algernon@balabit.hu> --- modules/afsocket/afsocket-grammar.ym | 2 ++ modules/afsocket/afsocket-parser.c | 1 + modules/afsocket/afsocket.c | 11 +++++++++++ modules/afsocket/afsocket.h | 2 ++ 4 files changed, 16 insertions(+), 0 deletions(-)

diff --git a/modules/afsocket/afsocket-grammar.ym b/modules/afsocket/afsocket-grammar.ym index 7970ca9..48b0526 100644 --- a/modules/afsocket/afsocket-grammar.ym +++ b/modules/afsocket/afsocket-grammar.ym @@ -83,6 +83,7 @@ TLSContext *last_tls_context; %token KW_SO_SNDBUF %token KW_SO_RCVBUF %token KW_SO_KEEPALIVE +%token KW_TCP_KEEP_ALIVE_TIME %token KW_SPOOF_SOURCE

%token KW_KEEP_ALIVE

I preferred to use the sysctl name for this option, which is tcp-keepalive-time, so I removed the extra underscore. Also since the parameters are closely related, I've also introduce the other two keepalive related parameters: tcp_keepalive_intvl (integer; default: 75; since Linux 2.4) The number of seconds between TCP keep-alive probes. tcp_keepalive_probes (integer; default: 9; since Linux 2.2) The maximum number of TCP keep-alive probes to send before giving up and killing the connection if no response is obtained from the other end. tcp_keepalive_time (integer; default: 7200; since Linux 2.2) The number of seconds a connection needs to be idle before TCP begins sending out keep-alive probes. Keep-alives are only sent when the SO_KEEPALIVE socket option is enabled. The default value is 7200 seconds (2 hours). An idle connection is terminated after approximately an additional 11 minutes (9 probes an interval of 75 seconds apart) when keep-alive is enabled. Note that underlying connection tracking mechanisms and application timeouts may be much shorter.

@@ -252,6 +253,7 @@ source_afinet_tcp_option

source_afsocket_stream_params : KW_KEEP_ALIVE '(' yesno ')' { afsocket_sd_set_keep_alive(last_driver, $3); } + | KW_TCP_KEEP_ALIVE_TIME '(' LL_NUMBER ')' { afsocket_sd_set_keep_alive_time(last_driver, $3); } | KW_MAX_CONNECTIONS '(' LL_NUMBER ')' { afsocket_sd_set_max_connections(last_driver, $3); } ;

this should have gone into parsing socket options, and not a separate driver specific option, so instead of this hunk, I've added a similar rule to inet_socket_option.

diff --git a/modules/afsocket/afsocket-parser.c b/modules/afsocket/afsocket-parser.c index 44671ca..bd8f5d6 100644 --- a/modules/afsocket/afsocket-parser.c +++ b/modules/afsocket/afsocket-parser.c @@ -64,6 +64,7 @@ static CfgLexerKeyword afsocket_keywords[] = { { "so_sndbuf", KW_SO_SNDBUF }, { "so_keepalive", KW_SO_KEEPALIVE }, { "tcp_keep_alive", KW_SO_KEEPALIVE, 0, KWS_OBSOLETE, "so_keepalive" }, + { "tcp_keep_alive_time",KW_TCP_KEEP_ALIVE_TIME }, { "spoof_source", KW_SPOOF_SOURCE }, { "transport", KW_TRANSPORT }, { "max_connections", KW_MAX_CONNECTIONS },

I added the other two options, and have decided to de-obsolete tcp-keep-alive (and added a tcp-keepalive alias), as so-keepalive() is clearly only TCP related.

diff --git a/modules/afsocket/afsocket.c b/modules/afsocket/afsocket.c index a7cc7bc..4a71b11 100644 --- a/modules/afsocket/afsocket.c +++ b/modules/afsocket/afsocket.c @@ -43,6 +43,7 @@ #include <arpa/inet.h> #include <sys/stat.h> #include <unistd.h> +#include <netinet/tcp.h>

#if ENABLE_TCP_WRAPPER #include <tcpd.h> @@ -79,6 +80,8 @@ afsocket_setup_socket(gint fd, SocketOptions *sock_options, AFSocketDirection di setsockopt(fd, SOL_SOCKET, SO_BROADCAST, &sock_options->broadcast, sizeof(sock_options->broadcast)); } setsockopt(fd, SOL_SOCKET, SO_KEEPALIVE, &sock_options->keepalive, sizeof(sock_options->keepalive)); + if (sock_options->keepalive_time > 0) + setsockopt(fd, SOL_TCP, TCP_KEEPIDLE, &sock_options->keepalive_time, sizeof(sock_options->keepalive_time)); return TRUE; }

this was moved to afinet_setup_socket(), also this was not functional since the timeout was stored in AFSocketSourceDriver->keepalive_timeout and not in sock_options.

@@ -386,6 +389,14 @@ afsocket_sd_set_keep_alive(LogDriver *s, gint enable) }

void +afsocket_sd_set_keep_alive_time(LogDriver *s, gint timeout) +{ + AFSocketSourceDriver *self = (AFSocketSourceDriver *) s; + + self->keepalive_timeout = timeout; +} +

dropped.

+void afsocket_sd_set_max_connections(LogDriver *s, gint max_connections) { AFSocketSourceDriver *self = (AFSocketSourceDriver *) s; diff --git a/modules/afsocket/afsocket.h b/modules/afsocket/afsocket.h index 392e249..fd11d28 100644 --- a/modules/afsocket/afsocket.h +++ b/modules/afsocket/afsocket.h @@ -58,6 +58,7 @@ typedef struct _SocketOptions gint rcvbuf; gint broadcast; gint keepalive; + gint keepalive_time;

this should've been introduced in InetSocketOptions, not here.

} SocketOptions;

gboolean afsocket_setup_socket(gint fd, SocketOptions *sock_options, AFSocketDirection dir); @@ -78,6 +79,7 @@ struct _AFSocketSourceDriver gint max_connections; gint num_connections; gint listen_backlog; + gint keepalive_timeout;

and definitely not here.

GList *connections; SocketOptions *sock_options_ptr;

This was the resulting commit: commit 2c99ebe31ff0f14b27dbf3b1262052534a2509f3 Author: Balazs Scheidler <bazsi@balabit.hu> Date: Sat Aug 18 13:19:51 2012 +0200 afsocket: Implement tcp_keepalive_time(), intvl() and probes() Three new options are introduced this patch to control various TCP keepalive related parameters tcp-keepalive-time() - equivalent to /proc/sys/net/ipv4/tcp_keepalive_time tcp-keepalive-probes() - equivalent to /proc/sys/net/ipv4/tcp_keepalive_probes tcp-keepalive-intvl() - equivalent to /proc/sys/net/ipv4/tcp_keepalive_intvl tcp-keepalive() - an alias to the earlier so-keepalive() option to match the keepalive related params With these new options one can control on a per-source basis the parameters governing TCP keepalives. While these options can be specified for all socket related sources, it only makes sense for TCP-based transports. By default, it's value is 0, which means using the kernel default (2 hours). These options only work on platforms which provide TCP_KEEPCNT, TCP_KEEPIDLE and TCP_KEEPINTVL setsockopts, which is only Linux as far as I know. Signed-off-by: Gergely Nagy <algernon@balabit.hu> Signed-off-by: Balazs Scheidler <bazsi@balabit.hu>