[syslog-ng] [PATCH (3.4)] afsocket: Implement tcp_keep_alive_time()

Balazs Scheidler bazsi77 at gmail.com
Sat Aug 18 13:46:17 CEST 2012


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 at 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 at 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 at balabit.hu>
    Signed-off-by: Balazs Scheidler <bazsi at balabit.hu>




More information about the syslog-ng mailing list