[lustre-devel] [PATCH 17/24] lnet: LNet peer aliveness broken

[James Simmons]

From: Chris Horn <chris.horn at hpe.com>

The peer health feature used on LNet routers is intended to detect if
a peer is dead or alive by keeping track of the last time it received
a message from the peer. If the last alive value is outside of a
configurable interval then the peer is considered dead and the router
will drop messages to that peer rather than attempt to send to it.

This feature no longer works as intended because even if the
last alive value is outside the interval the router will still
consider the peer NI to be alive if the health value of the NI and
the cached status both indicate the peer NI is alive.

So even if a router has not received any messages from the client in
days, as long as the router thinks the peer's interfaces are healthy
then it will consider the peer alive. This doesn't make any sense as
peers are supposed to regularly ping the router, and if they don't do
so then they should not be considered alive.

Fix the issue by relying solely on the last alive value to determine
peer aliveness. Do not consider the health value or cached status
when determining whether to drop the message.

lnet_peer_alive_locked() has single caller that only checks whether
zero was returned. We can convert lnet_peer_alive_locked() to return
bool rather than int.

Rename lnet_peer_alive_locked() to lnet_check_message_drop() to
better reflect the purpose of the function. The return value is
inverted to reflect the name change.

WC-bug-id: https://jira.whamcloud.com/browse/LU-15595
Lustre-commit: caf6095ade66f70d4 ("LU-15595 lnet: LNet peer aliveness broken")
Signed-off-by: Chris Horn <chris.horn at hpe.com>
Reviewed-on: https://review.whamcloud.com/46623
Reviewed-by: Serguei Smirnov <ssmirnov at whamcloud.com>
Reviewed-by: Cyril Bordage <cbordage at whamcloud.com>
Reviewed-by: Oleg Drokin <green at whamcloud.com>
Signed-off-by: James Simmons <jsimmons at infradead.org>
---
 net/lnet/lnet/lib-move.c | 51 +++++++++++++++---------------------------------
 1 file changed, 16 insertions(+), 35 deletions(-)

diff --git a/net/lnet/lnet/lib-move.c b/net/lnet/lnet/lib-move.c
index 3b20a1b7..ec8be8f 100644
--- a/net/lnet/lnet/lib-move.c
+++ b/net/lnet/lnet/lib-move.c
@@ -572,55 +572,37 @@ void lnet_usr_translate_stats(struct lnet_ioctl_element_msg_stats *msg_stats,
 	return rc;
 }
 
+/* returns true if this message should be dropped */
 static bool
-lnet_is_peer_deadline_passed(struct lnet_peer_ni *lpni, time64_t now)
+lnet_check_message_drop(struct lnet_ni *ni, struct lnet_peer_ni *lpni,
+			struct lnet_msg *msg)
 {
-	time64_t deadline;
-
-	deadline = lpni->lpni_last_alive +
-		   lpni->lpni_net->net_tunables.lct_peer_timeout;
-
-	/* assume peer_ni is alive as long as we're within the configured
-	 * peer timeout
-	 */
-	if (deadline > now)
+	if (msg->msg_target.pid & LNET_PID_USERFLAG)
 		return false;
 
-	return true;
-}
-
-/*
- * NB: returns 1 when alive, 0 when dead, negative when error;
- *     may drop the lnet_net_lock
- */
-static int
-lnet_peer_alive_locked(struct lnet_ni *ni, struct lnet_peer_ni *lpni,
-		       struct lnet_msg *msg)
-{
-	time64_t now = ktime_get_seconds();
-
 	if (!lnet_peer_aliveness_enabled(lpni))
-		return -ENODEV;
+		return false;
 
-	/*
-	 * If we're resending a message, let's attempt to send it even if
+	/* If we're resending a message, let's attempt to send it even if
 	 * the peer is down to fulfill our resend quota on the message
 	 */
 	if (msg->msg_retry_count > 0)
-		return 1;
+		return false;
 
 	/* try and send recovery messages irregardless */
 	if (msg->msg_recovery)
-		return 1;
+		return false;
 
 	/* always send any responses */
 	if (lnet_msg_is_response(msg))
-		return 1;
-
-	if (!lnet_is_peer_deadline_passed(lpni, now))
-		return true;
+		return false;
 
-	return lnet_is_peer_ni_alive(lpni);
+	/* assume peer_ni is alive as long as we're within the configured
+	 * peer timeout
+	 */
+	return ktime_get_seconds() >=
+		(lpni->lpni_last_alive +
+		 lpni->lpni_net->net_tunables.lct_peer_timeout);
 }
 
 /**
@@ -653,8 +635,7 @@ void lnet_usr_translate_stats(struct lnet_ioctl_element_msg_stats *msg_stats,
 		LASSERT(!nid_same(&lp->lpni_nid, &the_lnet.ln_loni->ni_nid));
 
 	/* NB 'lp' is always the next hop */
-	if (!(msg->msg_target.pid & LNET_PID_USERFLAG) &&
-	    !lnet_peer_alive_locked(ni, lp, msg)) {
+	if (lnet_check_message_drop(ni, lp, msg)) {
 		the_lnet.ln_counters[cpt]->lct_common.lcc_drop_count++;
 		the_lnet.ln_counters[cpt]->lct_common.lcc_drop_length +=
 			msg->msg_len;
-- 
1.8.3.1