drbd: fix race between disconnect and receive_state

If the asender thread, or request_timer_fn(), or some other part of
the code, decided to drop the connection (because of timeout or other),
but the receiver just now was processing a P_STATE packet, there was a
chance that receive_state() would do a hard state change
"re-establishing" an already failed connection without additional handshake.

Log excerpt:
  Remote failed to finish a request within ko-count * timeout
  peer( Secondary -> Unknown ) conn( Connected -> Timeout ) pdsk( UpToDate -> DUnknown )
  asender terminated
  ...
  peer( Unknown -> Secondary ) conn( Timeout -> Connected ) pdsk( DUnknown -> UpToDate ) peer_isp( 0 -> 1 )
  ...
  Connection closed
  peer( Secondary -> Unknown ) conn( Connected -> Unconnected ) pdsk( UpToDate -> DUnknown ) peer_isp( 1 -> 0 )
  receiver terminated

Impact:
while the connection state is erroneously "Connected",
requests may be queued and even sent,
which would never be acknowledged,
and may have been missed by the cleanup.
These requests would never be completed.

The next drbd_suspend_io() will then lock up,
waiting forever for these requests to complete.

Fixed in several code paths:
  Make sure the connection state is NetworkFailure or worse
  before starting the cleanup in drbd_disconnect().
  This should make sure the cleanup won't miss any requests.

  Disallow receive_state() to "upgrade" the connection state
  from an error state. This will make sure the "illegal" state
  transition won't happen.

  For all connection failure states,
  relax the safe-guard in sanitize_state() again
  to silently mask out those state changes
  (e.g. Timeout -> Connected becomes Timeout -> Timeout).

Signed-off-by: Philipp Reisner <philipp.reisner@linbit.com>
Signed-off-by: Lars Ellenberg <lars.ellenberg@linbit.com>

diff --git a/drivers/block/drbd/drbd_main.c b/drivers/block/drbd/drbd_main.c
index 3a5b4dec529fe8f0dc946032210404101d5b9e03..f71a667f5f321c42b06b263afc61f22a693409cc 100644 (file)
--- a/drivers/block/drbd/drbd_main.c
+++ b/drivers/block/drbd/drbd_main.c
@@ -909,7 +909,7 @@ static union drbd_state sanitize_state(struct drbd_conf *mdev, union drbd_state
        /* After a network error (+C_TEAR_DOWN) only C_UNCONNECTED or C_DISCONNECTING can follow.
         * If you try to go into some Sync* state, that shall fail (elsewhere). */
        if (os.conn >= C_TIMEOUT && os.conn <= C_TEAR_DOWN &&
-           ns.conn != C_UNCONNECTED && ns.conn != C_DISCONNECTING && ns.conn <= C_TEAR_DOWN)
+           ns.conn != C_UNCONNECTED && ns.conn != C_DISCONNECTING && ns.conn <= C_CONNECTED)
                ns.conn = os.conn;
 
        /* we cannot fail (again) if we already detached */

diff --git a/drivers/block/drbd/drbd_receiver.c b/drivers/block/drbd/drbd_receiver.c
index 2e9dfc69828fe8e5f334247790b6f4296b3cc009..08e694ef5ed90ba57738287f29fa191046b46d9f 100644 (file)
--- a/drivers/block/drbd/drbd_receiver.c
+++ b/drivers/block/drbd/drbd_receiver.c
@@ -3169,6 +3169,12 @@ static int receive_state(struct drbd_conf *mdev, enum drbd_packets cmd, unsigned
        os = ns = mdev->state;
        spin_unlock_irq(&mdev->req_lock);
 
+       /* If some other part of the code (asender thread, timeout)
+        * already decided to close the connection again,
+        * we must not "re-establish" it here. */
+       if (os.conn <= C_TEAR_DOWN)
+               return false;
+
        /* If this is the "end of sync" confirmation, usually the peer disk
         * transitions from D_INCONSISTENT to D_UP_TO_DATE. For empty (0 bits
         * set) resync started in PausedSyncT, or if the timing of pause-/
@@ -3782,6 +3788,13 @@ static void drbd_disconnect(struct drbd_conf *mdev)
        if (mdev->state.conn == C_STANDALONE)
                return;
 
+       /* We are about to start the cleanup after connection loss.
+        * Make sure drbd_make_request knows about that.
+        * Usually we should be in some network failure state already,
+        * but just in case we are not, we fix it up here.
+        */
+       drbd_force_state(mdev, NS(conn, C_NETWORK_FAILURE));
+
        /* asender does not clean up anything. it must not interfere, either */
        drbd_thread_stop(&mdev->asender);
        drbd_free_sock(mdev);