RX: Force sane timeout values

Currently we do not check the specified timeout values when someone
changes a connection's dead, idle, or hard dead time. However, if the
conn's dead time is larger than the other two times, a loss of network
activity will result in one of the other timeouts getting triggered
first.

To prevent this and possibly other problems from happening, force a
connection's timeouts to always obey the relationship
secondsUntilDead <= idleDeadTime <= hardDeadTime, by checking these
values whenever they are changed.

Change-Id: Ib9a0c65d91b4dc61c8d00381c8266b88b1d89b81
Reviewed-on: http://gerrit.openafs.org/2947
Tested-by: BuildBot <buildbot@rampaginggeek.com>
Reviewed-by: Jeffrey Altman <jaltman@openafs.org>
Tested-by: Jeffrey Altman <jaltman@openafs.org>
(cherry picked from commit 7d6080a841ff8c91052fa708d5be3b582f8a971d)
Reviewed-on: http://gerrit.openafs.org/3065
Reviewed-by: Derrick Brashear <shadow@dementia.org>
Tested-by: Derrick Brashear <shadow@dementia.org>

diff --git a/src/libafsrpc/afsrpc.def b/src/libafsrpc/afsrpc.def
index 4ed8c7c91b3f68308091702c9e8691a7f249bc2c..2bf61c04580b545381fb9edc74904237bf7a672e 100755 (executable)
--- a/src/libafsrpc/afsrpc.def
+++ b/src/libafsrpc/afsrpc.def
@@ -261,6 +261,11 @@ EXPORTS
         rx_SetConnSecondsUntilNatPing           @266
         rx_GetServiceSpecific                   @267
         rx_SetServiceSpecific                   @268
+;      rx_NewThreadId                          @269
+;       rx_GetStatistics                        @270
+;       rx_FreeStatistics                       @271
+       rx_SetConnHardDeadTime                  @272
+       rx_SetConnIdleDeadTime                  @273
 
 ; for performance testing
         rx_TSFPQGlobSize                        @2001 DATA

diff --git a/src/rx/rx.c b/src/rx/rx.c
index 529353cc1ad689d3532f852799e2ada2096c045d..fd6215c27c3f708d2223eccc4accf055b47e2993 100644 (file)
--- a/src/rx/rx.c
+++ b/src/rx/rx.c
@@ -866,15 +866,63 @@ rx_NewConnection(afs_uint32 shost, u_short sport, u_short sservice,
     return conn;
 }
 
+/**
+ * Ensure a connection's timeout values are valid.
+ *
+ * @param[in] conn The connection to check
+ *
+ * @post conn->secondUntilDead <= conn->idleDeadTime <= conn->hardDeadTime,
+ *       unless idleDeadTime and/or hardDeadTime are not set
+ * @internal
+ */
+static void
+rxi_CheckConnTimeouts(struct rx_connection *conn)
+{
+    /* a connection's timeouts must have the relationship
+     * deadTime <= idleDeadTime <= hardDeadTime. Otherwise, for example, a
+     * total loss of network to a peer may cause an idle timeout instead of a
+     * dead timeout, simply because the idle timeout gets hit first. Also set
+     * a minimum deadTime of 6, just to ensure it doesn't get set too low. */
+    /* this logic is slightly complicated by the fact that
+     * idleDeadTime/hardDeadTime may not be set at all, but it's not too bad.
+     */
+    conn->secondsUntilDead = MAX(conn->secondsUntilDead, 6);
+    if (conn->idleDeadTime) {
+       conn->idleDeadTime = MAX(conn->idleDeadTime, conn->secondsUntilDead);
+    }
+    if (conn->hardDeadTime) {
+       if (conn->idleDeadTime) {
+           conn->hardDeadTime = MAX(conn->idleDeadTime, conn->hardDeadTime);
+       } else {
+           conn->hardDeadTime = MAX(conn->secondsUntilDead, conn->hardDeadTime);
+       }
+    }
+}
+
 void
 rx_SetConnDeadTime(struct rx_connection *conn, int seconds)
 {
     /* The idea is to set the dead time to a value that allows several
      * keepalives to be dropped without timing out the connection. */
-    conn->secondsUntilDead = MAX(seconds, 6);
+    conn->secondsUntilDead = seconds;
+    rxi_CheckConnTimeouts(conn);
     conn->secondsUntilPing = conn->secondsUntilDead / 6;
 }
 
+void
+rx_SetConnHardDeadTime(struct rx_connection *conn, int seconds)
+{
+    conn->hardDeadTime = seconds;
+    rxi_CheckConnTimeouts(conn);
+}
+
+void
+rx_SetConnIdleDeadTime(struct rx_connection *conn, int seconds)
+{
+    conn->idleDeadTime = seconds;
+    rxi_CheckConnTimeouts(conn);
+}
+
 int rxi_lowPeerRefCount = 0;
 int rxi_lowConnRefCount = 0;
 

diff --git a/src/rx/rx.h b/src/rx/rx.h
index e7c10e88d0c95d438d06eaeb03ff34c60fd21e94..b92dc2249e415f4dd0d9508d99bd69ce44f97f3b 100644 (file)
--- a/src/rx/rx.h
+++ b/src/rx/rx.h
@@ -173,9 +173,6 @@ rx_IsLoopbackAddr(afs_uint32 addr)
 /* Enable or disable asymmetric client checking for a service */
 #define rx_SetCheckReach(service, x) ((service)->checkReach = (x))
 
-/* Set connection hard and idle timeouts for a connection */
-#define rx_SetConnHardDeadTime(conn, seconds) ((conn)->hardDeadTime = (seconds))
-#define rx_SetConnIdleDeadTime(conn, seconds) ((conn)->idleDeadTime = (seconds))
 #define rx_SetServerConnIdleDeadErr(conn,err) ((conn)->idleDeadErr = (err))
 
 /* Set the overload threshold and the overload error */

diff --git a/src/rx/rx_prototypes.h b/src/rx/rx_prototypes.h
index 2d8adc85a17f365edcdeecb9770e8c7dbdb2a989..a58ed20879feb84a9f7ed7446472c9a0ac1f6eb0 100644 (file)
--- a/src/rx/rx_prototypes.h
+++ b/src/rx/rx_prototypes.h
@@ -39,6 +39,8 @@ extern struct rx_connection *rx_NewConnection(afs_uint32 shost,
                                              int serviceSecurityIndex);
 extern void rx_SetConnDeadTime(struct rx_connection *conn,
                               int seconds);
+extern void rx_SetConnHardDeadTime(struct rx_connection *conn, int seconds);
+extern void rx_SetConnIdleDeadTime(struct rx_connection *conn, int seconds);
 extern void rxi_CleanupConnection(struct rx_connection *conn);
 extern void rxi_DestroyConnection(struct rx_connection *conn);
 extern void rx_GetConnection(struct rx_connection *conn);

diff --git a/src/shlibafsrpc/libafsrpc.map b/src/shlibafsrpc/libafsrpc.map
index 6a45ce27614e2d9b8e358dbba6c8ce4c6f7118f9..846ef70250272b3969029efbb1a0bb7dac9f1bca 100755 (executable)
--- a/src/shlibafsrpc/libafsrpc.map
+++ b/src/shlibafsrpc/libafsrpc.map
@@ -73,6 +73,8 @@
        rx_stats;
        rx_SetNoJumbo;
        rx_SetConnDeadTime;
+       rx_SetConnHardDeadTime;
+       rx_SetConnIdleDeadTime;
        rx_FlushWrite;
        rx_thread_id_key;
        multi_Finalize;