On Mon, Jul 04, 2011 at 12:32:51PM -0400, David Hill wrote:
> On Mon, Jul 04, 2011 at 09:47:36AM +0100, Stuart Henderson wrote:
> :let's try again with this. regenerated diffs because the only
> :complaints were about whitespace issues, no real changes compared
> :to the diffs I sent out previously.
> :
> :- add sysctl net.inet.tcp.always_keepalive to act as if
> :SO_KEEPALIVE was set on all TCP connections.
>
> I see good/bad with it. Fixes using apps behind crappy apps, but
> doesn't fix crappy apps? I have no opinion on this.
>
Same here. Sometimes I would like such a knob when using crappy NAT boxes.
On the other hand this only matters for ssh an there you could use the ssh
specific knob.
But if you add the knob then please also update sysctl(3) too.
> :
> :- fix old bug where net.inet.tcp.keepintvl was ignored,
> :keepalives were instead sent at keepidle/slow_hz seconds.
>
> I believe this is correct. Tested it. OK, but get more OKs :)
>
I belive it is wrong.
* The TCPT_KEEP timer is used to keep connections alive. If an
* connection is idle (no segments received) for TCPTV_KEEP_INIT amount of time,
* but not yet established, then we drop the connection. Once the connection
* is established, if the connection is idle for TCPTV_KEEP_IDLE time
* (and keepalives have been enabled on the socket), we begin to probe
* the connection.
So when a TCP packet is received we arm the timer with tcp_keepidle and
only when that timer fires we switch to tcp_keepintvl. From my quick look
this is what the code does and you would change that to always use
tcp_keepintvl when keepalive probes are enabled.
> :
> :OK?
> :
> :Index: sys/netinet/tcp_timer.c
> :===================================================================
> :RCS file: /cvs/src/sys/netinet/tcp_timer.c,v
> :retrieving revision 1.45
> :diff -u -p -r1.45 tcp_timer.c
> :--- sys/netinet/tcp_timer.c 3 Jul 2010 04:44:51 -0000 1.45
> :+++ sys/netinet/tcp_timer.c 4 Jul 2011 08:40:04 -0000
> :@@ -55,6 +55,7 @@
> : #include <netinet/ip_icmp.h>
> : #include <netinet/tcp_seq.h>
> :
> :+int tcp_always_keepalive;
> : int tcp_keepidle;
> : int tcp_keepintvl;
> : int tcp_maxpersistidle; /* max idle time in persist */
> :@@ -435,7 +436,8 @@ tcp_timer_keep(void *arg)
> : tcpstat.tcps_keeptimeo++;
> : if (TCPS_HAVEESTABLISHED(tp->t_state) == 0)
> : goto dropit;
> :- if (tp->t_inpcb->inp_socket->so_options & SO_KEEPALIVE &&
> :+ if ((tcp_always_keepalive ||
> :+ tp->t_inpcb->inp_socket->so_options & SO_KEEPALIVE) &&
> : tp->t_state <= TCPS_CLOSING) {
> : if ((tcp_maxidle > 0) &&
> : ((tcp_now - tp->t_rcvtime) >= tcp_keepidle + tcp_maxidle))
> :Index: sys/netinet/tcp_timer.h
> :===================================================================
> :RCS file: /cvs/src/sys/netinet/tcp_timer.h,v
> :retrieving revision 1.12
> :diff -u -p -r1.12 tcp_timer.h
> :--- sys/netinet/tcp_timer.h 8 Nov 2008 12:54:58 -0000 1.12
> :+++ sys/netinet/tcp_timer.h 4 Jul 2011 08:40:04 -0000
> :@@ -145,6 +145,7 @@ typedef void (*tcp_timer_func_t)(void *)
> : extern const tcp_timer_func_t tcp_timer_funcs[TCPT_NTIMERS];
> :
> : extern int tcptv_keep_init;
> :+extern int tcp_always_keepalive; /* assume SO_KEEPALIVE is always set */
> : extern int tcp_keepidle; /* time before keepalive probes begin */
> : extern int tcp_keepintvl; /* time between keepalive probes */
> : extern int tcp_maxidle; /* time to drop after starting
> probes */
> :Index: sys/netinet/tcp_var.h
> :===================================================================
> :RCS file: /cvs/src/sys/netinet/tcp_var.h,v
> :retrieving revision 1.98
> :diff -u -p -r1.98 tcp_var.h
> :--- sys/netinet/tcp_var.h 7 Jan 2011 17:50:42 -0000 1.98
> :+++ sys/netinet/tcp_var.h 4 Jul 2011 08:40:04 -0000
> :@@ -473,7 +473,8 @@ struct tcpstat {
> : #define TCPCTL_DROP 19 /* drop tcp connection */
> : #define TCPCTL_SACKHOLE_LIMIT 20 /* max entries for tcp sack queues */
> : #define TCPCTL_STATS 21 /* TCP statistics */
> :-#define TCPCTL_MAXID 22
> :+#define TCPCTL_ALWAYS_KEEPALIVE 22 /* assume SO_KEEPALIVE is always set
> */
> :+#define TCPCTL_MAXID 23
> :
> : #define TCPCTL_NAMES { \
> : { 0, 0 }, \
> :@@ -497,7 +498,8 @@ struct tcpstat {
> : { "reasslimit", CTLTYPE_INT }, \
> : { "drop", CTLTYPE_STRUCT }, \
> : { "sackholelimit", CTLTYPE_INT }, \
> :- { "stats", CTLTYPE_STRUCT } \
> :+ { "stats", CTLTYPE_STRUCT }, \
> :+ { "always_keepalive", CTLTYPE_INT } \
> : }
> :
> : #define TCPCTL_VARS { \
> :Index: sys/netinet/tcp_input.c
> :===================================================================
> :RCS file: /cvs/src/sys/netinet/tcp_input.c,v
> :retrieving revision 1.250
> :diff -u -p -r1.250 tcp_input.c
> :--- sys/netinet/tcp_input.c 13 May 2011 14:31:16 -0000 1.250
> :+++ sys/netinet/tcp_input.c 4 Jul 2011 08:40:04 -0000
> :@@ -961,8 +961,13 @@ findpcb:
> : * Reset idle time and keep-alive timer.
> : */
> : tp->t_rcvtime = tcp_now;
> :- if (TCPS_HAVEESTABLISHED(tp->t_state))
> :- TCP_TIMER_ARM(tp, TCPT_KEEP, tcp_keepidle);
> :+ if (TCPS_HAVEESTABLISHED(tp->t_state)) {
> :+ if (tcp_always_keepalive ||
> :+ tp->t_inpcb->inp_socket->so_options & SO_KEEPALIVE)
> :+ TCP_TIMER_ARM(tp, TCPT_KEEP, tcp_keepintvl);
> :+ else
> :+ TCP_TIMER_ARM(tp, TCPT_KEEP, tcp_keepidle);
> :+ }
> :
> : #ifdef TCP_SACK
> : if (tp->sack_enable)
> :Index: sbin/sysctl/sysctl.8
> :===================================================================
> :RCS file: /cvs/src/sbin/sysctl/sysctl.8,v
> :retrieving revision 1.159
> :diff -u -p -r1.159 sysctl.8
> :--- sbin/sysctl/sysctl.8 24 Jun 2011 19:47:48 -0000 1.159
> :+++ sbin/sysctl/sysctl.8 4 Jul 2011 08:40:04 -0000
> :@@ -257,6 +257,7 @@ and a few require a kernel compiled with
> : .It net.inet.tcp.keepinittime integer yes
> : .It net.inet.tcp.keepidle integer yes
> : .It net.inet.tcp.keepintvl integer yes
> :+.It net.inet.tcp.always_keepalive integer yes
> : .It net.inet.tcp.slowhz integer no
> : .It net.inet.tcp.baddynamic array yes
> : .It net.inet.tcp.sack integer yes
> :
>
> --
> Think big. Pollute the Mississippi.
>
--
:wq Claudio
