LU-17838 kfilnd: Prevent simultaneous hellos

There is a race condition with checking, setting and clearing the
kp_hello_pending flag that can result in multiple hello requests being
sent for the same peer. If no hello response is received after the
LND timeout then multiple threads can race with each other in
clearing the kp_hello_pending flag and posting a new hello request
message.

Thread 1: sets kp_hello_pending and posts hello request message
<No hello response received after LND timeout>
Thread 2: Clears kp_hello_pending, then sets kp_hello_sending
Thread 3: Clears kp_hello_pending, then sets kp_hello_sending
Thread 2/3: Both post hello request message

To resolve this issue we change kp_hello_pending from a simple binary
to instead track three states of a hello request: KP_HELLO_NONE,
KP_HELLO_INIT, and KP_HELLO_SENT. State is NONE when there is no
hello in the process of being sent. State is INIT when a thread is
allocating a HELLO request in preparation for sending. State is SENT
when the HELLO request is being posted. Now, when some threads detect
that we have not received hello response after LND timeout seconds
then only one of them will be able to transition to the hello state
from SENT -> NONE.

Add CFS_KFI_REPLAY_IDLE_EVENT fail_loc that can be used to delay
processing of TNs in the idle state depending on the TN event
value specified in fail_val.

HPE-bug-id: LUS-11974
Test-Parameters: trivial
Fixes: 11a32d886b ("LU-16213 kfilnd: Allow one HELLO in-flight per peer")
Signed-off-by: Chris Horn <chris.horn@hpe.com>
Change-Id: I4dddf57971848a80a550df7523d55ad03f4a083e
Reviewed-on: https://review.whamcloud.com/c/fs/lustre-release/+/55069
Tested-by: jenkins <devops@whamcloud.com>
Tested-by: Maloo <maloo@whamcloud.com>
Reviewed-by: Ian Ziemba <ian.ziemba@hpe.com>
Reviewed-by: Ron Gredvig <ron.gredvig@hpe.com>
Reviewed-by: Caleb Carlson <caleb.carlson@hpe.com>
Reviewed-by: Oleg Drokin <green@whamcloud.com>

diff --git a/lnet/klnds/kfilnd/kfilnd.c b/lnet/klnds/kfilnd/kfilnd.c
index 53bf5ca..2066d41 100644 (file)
--- a/lnet/klnds/kfilnd/kfilnd.c
+++ b/lnet/klnds/kfilnd/kfilnd.c
@@ -63,7 +63,9 @@ int kfilnd_send_hello_request(struct kfilnd_dev *dev, int cpt,
        struct kfilnd_transaction *tn;
        int rc;
 
-       if (kfilnd_peer_set_check_hello_pending(kp)) {
+       /* Only one thread may progress state from NONE -> INIT */
+       if (atomic_cmpxchg(&kp->kp_hello_state, KP_HELLO_NONE, KP_HELLO_INIT) !=
+           KP_HELLO_NONE) {
                CDEBUG(D_NET, "Hello already pending to peer %s(%px)\n",
                       libcfs_nid2str(kp->kp_nid), kp);
                return 0;
@@ -73,7 +75,7 @@ int kfilnd_send_hello_request(struct kfilnd_dev *dev, int cpt,
        if (IS_ERR(tn)) {
                rc = PTR_ERR(tn);
                CERROR("Failed to allocate transaction struct: rc=%d\n", rc);
-               kfilnd_peer_clear_hello_pending(kp);
+               atomic_set(&kp->kp_hello_state, KP_HELLO_NONE);
                return rc;
        }
 
@@ -86,6 +88,8 @@ int kfilnd_send_hello_request(struct kfilnd_dev *dev, int cpt,
 
        kp->kp_hello_ts = ktime_get_seconds();
 
+       atomic_set(&kp->kp_hello_state, KP_HELLO_SENDING);
+
        kfilnd_tn_event_handler(tn, TN_EVENT_TX_HELLO, 0);
 
        return 0;

diff --git a/lnet/klnds/kfilnd/kfilnd.h b/lnet/klnds/kfilnd/kfilnd.h
index 93c7ad2..92b1a75 100644 (file)
--- a/lnet/klnds/kfilnd/kfilnd.h
+++ b/lnet/klnds/kfilnd/kfilnd.h
@@ -104,6 +104,7 @@
 #define CFS_KFI_FAIL_WAIT_SEND_COMP1 0xF115
 #define CFS_KFI_FAIL_WAIT_SEND_COMP2 0xF116
 #define CFS_KFI_FAIL_WAIT_SEND_COMP3 0xF117
+#define CFS_KFI_REPLAY_IDLE_EVENT 0xF118
 
 /* Maximum number of transaction keys supported. */
 #define KFILND_EP_KEY_BITS 16U
@@ -254,7 +255,7 @@ struct kfilnd_peer {
        u16 kp_version;
        u32 kp_local_session_key;
        u32 kp_remote_session_key;
-       atomic_t kp_hello_pending;
+       atomic_t kp_hello_state;
        time64_t kp_hello_ts;
        atomic_t kp_state;
 };
@@ -264,18 +265,20 @@ static inline bool kfilnd_peer_deleted(struct kfilnd_peer *kp)
        return atomic_read(&kp->kp_remove_peer) > 0;
 }
 
-/* Sets kp_hello_sending
- * Returns true if it was already set
- * Returns false otherwise
+/* Values for kp_hello_state. Valid transitions:
+ * NONE -> INIT
+ * INIT -> NONE (only when fail to allocate kfilnd_tn for hello req)
+ * INIT -> SENDING
+ * SENDING -> NONE
  */
-static inline bool kfilnd_peer_set_check_hello_pending(struct kfilnd_peer *kp)
-{
-       return (atomic_cmpxchg(&kp->kp_hello_pending, 0, 1) == 1);
-}
+#define KP_HELLO_NONE 0 /* There is no hello request being sent */
+#define KP_HELLO_INIT 1 /* Hello request is initializing */
+#define KP_HELLO_SENDING 2 /* Hello request TN is in the state machine */
 
-static inline void kfilnd_peer_clear_hello_pending(struct kfilnd_peer *kp)
+/* If kp_hello_state is SENDING then set to NONE */
+static inline void kfilnd_peer_clear_hello_state(struct kfilnd_peer *kp)
 {
-       atomic_set(&kp->kp_hello_pending, 0);
+       atomic_cmpxchg(&kp->kp_hello_state, KP_HELLO_SENDING, KP_HELLO_NONE);
 }
 
 static inline bool kfilnd_peer_is_new_peer(struct kfilnd_peer *kp)
@@ -303,7 +306,9 @@ static inline bool kfilnd_peer_needs_throttle(struct kfilnd_peer *kp)
 static inline bool kfilnd_peer_needs_hello(struct kfilnd_peer *kp,
                                           bool proactive_handshake)
 {
-       if (atomic_read(&kp->kp_hello_pending) == 0) {
+       int hello_state = atomic_read(&kp->kp_hello_state);
+
+       if (hello_state == KP_HELLO_NONE) {
                if (atomic_read(&kp->kp_state) != KP_STATE_UPTODATE)
                        return true;
                else if (proactive_handshake &&
@@ -311,7 +316,8 @@ static inline bool kfilnd_peer_needs_hello(struct kfilnd_peer *kp,
                                      lnet_get_lnd_timeout() * 2,
                                      ktime_get_seconds()))
                        return true;
-       } else if (ktime_before(kp->kp_hello_ts + lnet_get_lnd_timeout(),
+       } else if (hello_state == KP_HELLO_SENDING &&
+                  ktime_before(kp->kp_hello_ts + lnet_get_lnd_timeout(),
                                ktime_get_seconds())) {
                /* Sent hello but never received reply */
                CDEBUG(D_NET,
@@ -319,7 +325,7 @@ static inline bool kfilnd_peer_needs_hello(struct kfilnd_peer *kp,
                       libcfs_nid2str(kp->kp_nid), kp, kp->kp_addr,
                       ktime_sub(ktime_get_seconds(), kp->kp_hello_ts));
 
-               kfilnd_peer_clear_hello_pending(kp);
+               kfilnd_peer_clear_hello_state(kp);
                return true;
        }
 

diff --git a/lnet/klnds/kfilnd/kfilnd_peer.c b/lnet/klnds/kfilnd/kfilnd_peer.c
index 7288208..2c8ab95 100644 (file)
--- a/lnet/klnds/kfilnd/kfilnd_peer.c
+++ b/lnet/klnds/kfilnd/kfilnd_peer.c
@@ -251,7 +251,7 @@ again:
        kp->kp_nid = nid;
        atomic_set(&kp->kp_rx_base, 0);
        atomic_set(&kp->kp_remove_peer, 0);
-       atomic_set(&kp->kp_hello_pending, 0);
+       atomic_set(&kp->kp_hello_state, KP_HELLO_NONE);
        atomic_set(&kp->kp_state, KP_STATE_NEW);
        kp->kp_local_session_key = kfilnd_dev_get_session_key(dev);
        kp->kp_hello_ts = ktime_get_seconds();
@@ -375,10 +375,10 @@ void kfilnd_peer_process_hello(struct kfilnd_peer *kp, struct kfilnd_msg *msg)
        CDEBUG(D_NET, "kp %s(%p):0x%llx is up-to-date\n",
               libcfs_nid2str(kp->kp_nid), kp, kp->kp_addr);
 
-       /* Clear kp_hello_pending if we've received the hello response,
+       /* Clear kp_hello_state if we've received the hello response,
         * otherwise this is an incoming hello request and we may have our
         * own hello request to this peer still outstanding
         */
        if (msg->type == KFILND_MSG_HELLO_RSP)
-               kfilnd_peer_clear_hello_pending(kp);
+               kfilnd_peer_clear_hello_state(kp);
 }

diff --git a/lnet/klnds/kfilnd/kfilnd_tn.c b/lnet/klnds/kfilnd/kfilnd_tn.c
index cb9fbbe..eba1077 100644 (file)
--- a/lnet/klnds/kfilnd/kfilnd_tn.c
+++ b/lnet/klnds/kfilnd/kfilnd_tn.c
@@ -700,6 +700,9 @@ static int kfilnd_tn_state_idle(struct kfilnd_transaction *tn,
                goto out;
        }
 
+       if (CFS_FAIL_CHECK_VALUE(CFS_KFI_REPLAY_IDLE_EVENT, event))
+               return -EAGAIN;
+
        switch (event) {
        case TN_EVENT_INIT_IMMEDIATE:
        case TN_EVENT_TX_HELLO:
@@ -736,7 +739,7 @@ static int kfilnd_tn_state_idle(struct kfilnd_transaction *tn,
                                        libcfs_nid2str(tn->tn_kp->kp_nid),
                                        tn->tn_target_addr, rc);
                        if (event == TN_EVENT_TX_HELLO)
-                               kfilnd_peer_clear_hello_pending(tn->tn_kp);
+                               kfilnd_peer_clear_hello_state(tn->tn_kp);
                        kfilnd_tn_status_update(tn, rc,
                                                LNET_MSG_STATUS_LOCAL_ERROR);
                }
@@ -924,7 +927,7 @@ static int kfilnd_tn_state_imm_send(struct kfilnd_transaction *tn,
                 */
                kfilnd_peer_tn_failed(tn->tn_kp, status, false);
                if (tn->msg_type == KFILND_MSG_HELLO_REQ)
-                       kfilnd_peer_clear_hello_pending(tn->tn_kp);
+                       kfilnd_peer_clear_hello_state(tn->tn_kp);
                break;
 
        case TN_EVENT_TX_OK: