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: I681dce7f359bf71333e69ceab8186bdc1d54d8dc
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>

diff --git a/src/libafsrpc/afsrpc.def b/src/libafsrpc/afsrpc.def
index 37ff71c6ee368c9f11d0d1ac2d23fc6b37687700..0c9c2222e0486f91c495241687ee7c9a73b5f4b9 100755 (executable)
--- a/src/libafsrpc/afsrpc.def
+++ b/src/libafsrpc/afsrpc.def
@@ -264,6 +264,8 @@ EXPORTS
        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 47f540a55a44108a8421d78eec19ec199eacce4a..71b1a9987f52e4c8bb06805f8c4ed3eb60a62433 100644 (file)
--- a/src/rx/rx.c
+++ b/src/rx/rx.c
@@ -882,15 +882,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 e51c28c3c4d74209e6f970f0d7c92f421c444203..49614aa0a06bce18d12fee6a728146e10cebf505 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 afaa00f547099bcfa93de61286595b692de9e4b4..aee4478d5dd4f93efa700c2860ded3fcb38881ab 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 c795b1184338f2dee3ea3bc406ac7504cd8deb41..992e8d327064babeeb3b43869679a90e733d1ec2 100755 (executable)
--- a/src/shlibafsrpc/libafsrpc.map
+++ b/src/shlibafsrpc/libafsrpc.map
@@ -72,6 +72,8 @@
        rx_nWaiting;
        rx_SetNoJumbo;
        rx_SetConnDeadTime;
+       rx_SetConnHardDeadTime;
+       rx_SetConnIdleDeadTime;
        rx_FlushWrite;
        rx_thread_id_key;
        multi_Finalize;