From 8d2dbb7a96ed41885dfdb90d672168fd41a31713 Mon Sep 17 00:00:00 2001 From: Andrew Deason Date: Fri, 8 Oct 2010 11:51:30 -0500 Subject: [PATCH] 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 Reviewed-by: Jeffrey Altman Tested-by: Jeffrey Altman (cherry picked from commit 7d6080a841ff8c91052fa708d5be3b582f8a971d) Reviewed-on: http://gerrit.openafs.org/3065 Reviewed-by: Derrick Brashear Tested-by: Derrick Brashear --- src/libafsrpc/afsrpc.def | 5 ++++ src/rx/rx.c | 50 ++++++++++++++++++++++++++++++++++- src/rx/rx.h | 3 --- src/rx/rx_prototypes.h | 2 ++ src/shlibafsrpc/libafsrpc.map | 2 ++ 5 files changed, 58 insertions(+), 4 deletions(-) diff --git a/src/libafsrpc/afsrpc.def b/src/libafsrpc/afsrpc.def index 4ed8c7c91..2bf61c045 100755 --- 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 529353cc1..fd6215c27 100644 --- 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 e7c10e88d..b92dc2249 100644 --- 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 2d8adc85a..a58ed2087 100644 --- 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 6a45ce276..846ef7025 100755 --- 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; -- 2.39.5