reconnect: Fix repeated RECONNECT_CONNECT that was confusing JSON-RPC.

reconnect_run() returns RECONNECT_CONNECT to tell the client that it should
start a new connection.  The client is then supposed to call
reconnect_connecting() to tell the FSM that it has begun a connection
attempt.  However, even after reconnect_connecting() was called,
reconnect_run() continued to return RECONNECT_CONNECT on each call until
the connection succeeded or failed.  This confused the jsonrpc_session
client, which expected that it would get a 0 return value from
reconnect_run() while the connection attempt was in progress.  Connections
that required multiple trips through the main poll loop, e.g. for SSL
negotiation, would often get cut off to start a second connection attempt.

This commit change reconnect_run() to return RECONNECT_CONNECT only until
the client tells it that a connection is in progress, which fixes the
problem.  This change entails a change to the internal details of the
reconnect FSM, so this commit also updates the reconnect tests to match.

Reported by Jeremy Stribling.

diff --git a/lib/reconnect.c b/lib/reconnect.c
index 2ae65c6..f159f01 100644 (file)
--- a/lib/reconnect.c
+++ b/lib/reconnect.c
@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2008, 2009 Nicira Networks.
+ * Copyright (c) 2008, 2009, 2010 Nicira Networks.
  *
  * Licensed under the Apache License, Version 2.0 (the "License");
  * you may not use this file except in compliance with the License.
@@ -28,10 +28,11 @@
 #define STATES                                  \
     STATE(VOID, 1 << 0)                         \
     STATE(BACKOFF, 1 << 1)                      \
-    STATE(CONNECTING, 1 << 2)                   \
-    STATE(ACTIVE, 1 << 3)                       \
-    STATE(IDLE, 1 << 4)                         \
-    STATE(RECONNECT, 1 << 5)
+    STATE(START_CONNECT, 1 << 2)                \
+    STATE(CONNECT_IN_PROGRESS, 1 << 3)          \
+    STATE(ACTIVE, 1 << 4)                       \
+    STATE(IDLE, 1 << 5)                         \
+    STATE(RECONNECT, 1 << 6)
 enum state {
 #define STATE(NAME, VALUE) S_##NAME = VALUE,
     STATES
@@ -259,7 +260,8 @@ reconnect_disable(struct reconnect *fsm, long long int now)
 void
 reconnect_force_reconnect(struct reconnect *fsm, long long int now)
 {
-    if (fsm->state & (S_CONNECTING | S_ACTIVE | S_IDLE)) {
+    if (fsm->state & (S_START_CONNECT | S_CONNECT_IN_PROGRESS
+                      | S_ACTIVE | S_IDLE)) {
         reconnect_transition__(fsm, now, S_RECONNECT);
     }
 }
@@ -321,9 +323,9 @@ reconnect_disconnected(struct reconnect *fsm, long long int now, int error)
 void
 reconnect_connecting(struct reconnect *fsm, long long int now)
 {
-    if (fsm->state != S_CONNECTING) {
+    if (fsm->state != S_CONNECT_IN_PROGRESS) {
         VLOG_INFO("%s: connecting...", fsm->name);
-        reconnect_transition__(fsm, now, S_CONNECTING);
+        reconnect_transition__(fsm, now, S_CONNECT_IN_PROGRESS);
     }
 }
 
@@ -371,7 +373,7 @@ static void
 reconnect_transition__(struct reconnect *fsm, long long int now,
                        enum state state)
 {
-    if (fsm->state == S_CONNECTING) {
+    if (fsm->state == S_CONNECT_IN_PROGRESS) {
         fsm->n_attempted_connections++;
         if (state == S_ACTIVE) {
             fsm->n_successful_connections++;
@@ -400,7 +402,8 @@ reconnect_deadline__(const struct reconnect *fsm)
     case S_BACKOFF:
         return fsm->state_entered + fsm->backoff;
 
-    case S_CONNECTING:
+    case S_START_CONNECT:
+    case S_CONNECT_IN_PROGRESS:
         return fsm->state_entered + MAX(1000, fsm->backoff);
 
     case S_ACTIVE:
@@ -457,7 +460,8 @@ reconnect_run(struct reconnect *fsm, long long int now)
         case S_BACKOFF:
             return RECONNECT_CONNECT;
 
-        case S_CONNECTING:
+        case S_START_CONNECT:
+        case S_CONNECT_IN_PROGRESS:
             return RECONNECT_DISCONNECT;
 
         case S_ACTIVE:
@@ -478,7 +482,7 @@ reconnect_run(struct reconnect *fsm, long long int now)
 
         NOT_REACHED();
     } else {
-        return fsm->state == S_CONNECTING ? RECONNECT_CONNECT : 0;
+        return fsm->state == S_START_CONNECT ? RECONNECT_CONNECT : 0;
     }
 }
 

diff --git a/tests/reconnect.at b/tests/reconnect.at
index 225da0d..95996ff 100644 (file)
--- a/tests/reconnect.at
+++ b/tests/reconnect.at
@@ -104,15 +104,14 @@ enable
 run
   should connect
 connecting
-  in CONNECTING for 0 ms (0 ms backoff)
+  in CONNECT_IN_PROGRESS for 0 ms (0 ms backoff)
 
 # Connect after 500 ms.
 advance 500
 
 ### t=1500 ###
-  in CONNECTING for 500 ms (0 ms backoff)
+  in CONNECT_IN_PROGRESS for 500 ms (0 ms backoff)
 run
-  should connect
 connected
   in ACTIVE for 0 ms (0 ms backoff)
   created 1000, last received 1000, last connected 1500
@@ -218,14 +217,13 @@ enable
 run
   should connect
 connecting
-  in CONNECTING for 0 ms (0 ms backoff)
+  in CONNECT_IN_PROGRESS for 0 ms (0 ms backoff)
 run
-  should connect
 timeout
   advance 1000 ms
 
 ### t=2000 ###
-  in CONNECTING for 1000 ms (0 ms backoff)
+  in CONNECT_IN_PROGRESS for 1000 ms (0 ms backoff)
 run
   should disconnect
 connect-failed
@@ -243,12 +241,12 @@ run
 
 # Second connection attempt fails after 1000 ms.
 connecting
-  in CONNECTING for 0 ms (1000 ms backoff)
+  in CONNECT_IN_PROGRESS for 0 ms (1000 ms backoff)
 timeout
   advance 1000 ms
 
 ### t=4000 ###
-  in CONNECTING for 1000 ms (1000 ms backoff)
+  in CONNECT_IN_PROGRESS for 1000 ms (1000 ms backoff)
 run
   should disconnect
 connect-failed
@@ -266,12 +264,12 @@ run
 
 # Third connection attempt fails after 2000 ms.
 connecting
-  in CONNECTING for 0 ms (2000 ms backoff)
+  in CONNECT_IN_PROGRESS for 0 ms (2000 ms backoff)
 timeout
   advance 2000 ms
 
 ### t=8000 ###
-  in CONNECTING for 2000 ms (2000 ms backoff)
+  in CONNECT_IN_PROGRESS for 2000 ms (2000 ms backoff)
 run
   should disconnect
 connect-failed
@@ -289,12 +287,12 @@ run
 
 # Third connection attempt fails after 4000 ms.
 connecting
-  in CONNECTING for 0 ms (4000 ms backoff)
+  in CONNECT_IN_PROGRESS for 0 ms (4000 ms backoff)
 timeout
   advance 4000 ms
 
 ### t=16000 ###
-  in CONNECTING for 4000 ms (4000 ms backoff)
+  in CONNECT_IN_PROGRESS for 4000 ms (4000 ms backoff)
 run
   should disconnect
 connect-failed
@@ -312,12 +310,12 @@ run
 
 # Third connection attempt fails after 8000 ms.
 connecting
-  in CONNECTING for 0 ms (8000 ms backoff)
+  in CONNECT_IN_PROGRESS for 0 ms (8000 ms backoff)
 timeout
   advance 8000 ms
 
 ### t=32000 ###
-  in CONNECTING for 8000 ms (8000 ms backoff)
+  in CONNECT_IN_PROGRESS for 8000 ms (8000 ms backoff)
 run
   should disconnect
 connect-failed
@@ -335,12 +333,12 @@ run
 
 # Fourth connection attempt fails after 8000 ms.
 connecting
-  in CONNECTING for 0 ms (8000 ms backoff)
+  in CONNECT_IN_PROGRESS for 0 ms (8000 ms backoff)
 timeout
   advance 8000 ms
 
 ### t=48000 ###
-  in CONNECTING for 8000 ms (8000 ms backoff)
+  in CONNECT_IN_PROGRESS for 8000 ms (8000 ms backoff)
 run
   should disconnect
 connect-failed
@@ -563,14 +561,13 @@ enable
 run
   should connect
 connecting
-  in CONNECTING for 0 ms (0 ms backoff)
+  in CONNECT_IN_PROGRESS for 0 ms (0 ms backoff)
 run
-  should connect
 timeout
   advance 1000 ms
 
 ### t=2000 ###
-  in CONNECTING for 1000 ms (0 ms backoff)
+  in CONNECT_IN_PROGRESS for 1000 ms (0 ms backoff)
 run
   should disconnect
 connect-failed
@@ -588,12 +585,12 @@ run
 
 # Second connection attempt fails after 1000 ms.
 connecting
-  in CONNECTING for 0 ms (1000 ms backoff)
+  in CONNECT_IN_PROGRESS for 0 ms (1000 ms backoff)
 timeout
   advance 1000 ms
 
 ### t=4000 ###
-  in CONNECTING for 1000 ms (1000 ms backoff)
+  in CONNECT_IN_PROGRESS for 1000 ms (1000 ms backoff)
 run
   should disconnect
 connect-failed
@@ -611,13 +608,12 @@ run
 
 # Third connection attempt succeeds after 500 ms.
 connecting
-  in CONNECTING for 0 ms (2000 ms backoff)
+  in CONNECT_IN_PROGRESS for 0 ms (2000 ms backoff)
 advance 500
 
 ### t=6500 ###
-  in CONNECTING for 500 ms (2000 ms backoff)
+  in CONNECT_IN_PROGRESS for 500 ms (2000 ms backoff)
 run
-  should connect
 connected
   in ACTIVE for 0 ms (2000 ms backoff)
   created 1000, last received 1000, last connected 6500
@@ -708,14 +704,13 @@ enable
 run
   should connect
 connecting
-  in CONNECTING for 0 ms (0 ms backoff)
+  in CONNECT_IN_PROGRESS for 0 ms (0 ms backoff)
 run
-  should connect
 timeout
   advance 1000 ms
 
 ### t=2000 ###
-  in CONNECTING for 1000 ms (0 ms backoff)
+  in CONNECT_IN_PROGRESS for 1000 ms (0 ms backoff)
 run
   should disconnect
 connect-failed
@@ -733,12 +728,12 @@ run
 
 # Second connection attempt fails after 1000 ms.
 connecting
-  in CONNECTING for 0 ms (1000 ms backoff)
+  in CONNECT_IN_PROGRESS for 0 ms (1000 ms backoff)
 timeout
   advance 1000 ms
 
 ### t=4000 ###
-  in CONNECTING for 1000 ms (1000 ms backoff)
+  in CONNECT_IN_PROGRESS for 1000 ms (1000 ms backoff)
 run
   should disconnect
 connect-failed
@@ -756,13 +751,12 @@ run
 
 # Third connection attempt succeeds after 500 ms.
 connecting
-  in CONNECTING for 0 ms (2000 ms backoff)
+  in CONNECT_IN_PROGRESS for 0 ms (2000 ms backoff)
 advance 500
 
 ### t=6500 ###
-  in CONNECTING for 500 ms (2000 ms backoff)
+  in CONNECT_IN_PROGRESS for 500 ms (2000 ms backoff)
 run
-  should connect
 connected
   in ACTIVE for 0 ms (2000 ms backoff)
   created 1000, last received 1000, last connected 6500
@@ -874,14 +868,13 @@ enable
 run
   should connect
 connecting
-  in CONNECTING for 0 ms (0 ms backoff)
+  in CONNECT_IN_PROGRESS for 0 ms (0 ms backoff)
 run
-  should connect
 timeout
   advance 1000 ms
 
 ### t=2000 ###
-  in CONNECTING for 1000 ms (0 ms backoff)
+  in CONNECT_IN_PROGRESS for 1000 ms (0 ms backoff)
 run
   should disconnect
 connect-failed
@@ -899,12 +892,12 @@ run
 
 # Second connection attempt fails after 1000 ms.
 connecting
-  in CONNECTING for 0 ms (1000 ms backoff)
+  in CONNECT_IN_PROGRESS for 0 ms (1000 ms backoff)
 timeout
   advance 1000 ms
 
 ### t=4000 ###
-  in CONNECTING for 1000 ms (1000 ms backoff)
+  in CONNECT_IN_PROGRESS for 1000 ms (1000 ms backoff)
 run
   should disconnect
 connect-failed
@@ -922,13 +915,12 @@ run
 
 # Third connection attempt succeeds after 500 ms.
 connecting
-  in CONNECTING for 0 ms (2000 ms backoff)
+  in CONNECT_IN_PROGRESS for 0 ms (2000 ms backoff)
 advance 500
 
 ### t=6500 ###
-  in CONNECTING for 500 ms (2000 ms backoff)
+  in CONNECT_IN_PROGRESS for 500 ms (2000 ms backoff)
 run
-  should connect
 connected
   in ACTIVE for 0 ms (2000 ms backoff)
   created 1000, last received 1000, last connected 6500