From 3603f8da20f495611b077e3056db75d0cf8574fb Mon Sep 17 00:00:00 2001 From: Ben Pfaff Date: Mon, 11 Jan 2010 17:00:25 -0800 Subject: [PATCH] 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. --- lib/reconnect.c | 28 ++++++++++-------- tests/reconnect.at | 72 +++++++++++++++++++++------------------------- 2 files changed, 48 insertions(+), 52 deletions(-) diff --git a/lib/reconnect.c b/lib/reconnect.c index 2ae65c680..f159f01c0 100644 --- 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 225da0d49..95996ff3d 100644 --- 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 -- 2.20.1