diff --git a/src/bgp/test/bgp_server_test_util.cc b/src/bgp/test/bgp_server_test_util.cc index 3e393bc0ed4..c3ff2407f02 100644 --- a/src/bgp/test/bgp_server_test_util.cc +++ b/src/bgp/test/bgp_server_test_util.cc @@ -37,6 +37,7 @@ using namespace std; int StateMachineTest::hold_time_msecs_ = 0; int StateMachineTest::keepalive_time_msecs_ = 0; +int XmppStateMachineTest::hold_time_msecs_ = 0; // // This is a static data structure that maps client tcp end points to configured diff --git a/src/bgp/test/bgp_server_test_util.h b/src/bgp/test/bgp_server_test_util.h index 9f5b1b513e7..f68990a0266 100644 --- a/src/bgp/test/bgp_server_test_util.h +++ b/src/bgp/test/bgp_server_test_util.h @@ -306,6 +306,19 @@ class XmppStateMachineTest : public XmppStateMachine { boost::bind(&XmppStateMachine::OpenTimerExpired, this), boost::bind(&XmppStateMachine::TimerErrorHandler, this, _1, _2)); } + + virtual int hold_time_msecs() const { + if (hold_time_msecs_) + return hold_time_msecs_; + return XmppStateMachine::hold_time_msecs(); + } + + static void set_hold_time_msecs(int hold_time_msecs) { + hold_time_msecs_ = hold_time_msecs; + } + +private: + static int hold_time_msecs_; }; class XmppLifetimeManagerTest : public XmppLifetimeManager { diff --git a/src/bgp/test/bgp_xmpp_basic_test.cc b/src/bgp/test/bgp_xmpp_basic_test.cc index 00abbf53eb9..d27df3b318a 100644 --- a/src/bgp/test/bgp_xmpp_basic_test.cc +++ b/src/bgp/test/bgp_xmpp_basic_test.cc @@ -592,6 +592,57 @@ TEST_P(BgpXmppBasicParamTest, NoSelfBgpConfiguration2) { DestroyAgents(); } +TEST_P(BgpXmppBasicParamTest, ClientConnectionBackoff) { + XmppStateMachineTest::set_hold_time_msecs(300); + Unconfigure(); + task_util::WaitForIdle(); + TASK_UTIL_EXPECT_FALSE(bs_x_->HasSelfConfiguration()); + + CreateAgents(); + + uint32_t client_flap_a = agent_a_->flap_count(); + uint32_t client_flap_b = agent_b_->flap_count(); + uint32_t client_flap_c = agent_c_->flap_count(); + + size_t client_conn_attempts_a = agent_a_->get_sm_connect_attempts(); + size_t client_conn_attempts_b = agent_b_->get_sm_connect_attempts(); + size_t client_conn_attempts_c = agent_c_->get_sm_connect_attempts(); + + TASK_UTIL_EXPECT_TRUE(agent_a_->flap_count() > client_flap_a + 3); + TASK_UTIL_EXPECT_TRUE(agent_b_->flap_count() > client_flap_b + 3); + TASK_UTIL_EXPECT_TRUE(agent_c_->flap_count() > client_flap_c + 3); + + TASK_UTIL_EXPECT_TRUE( + agent_a_->get_sm_connect_attempts() > client_conn_attempts_a + 3); + TASK_UTIL_EXPECT_TRUE( + agent_b_->get_sm_connect_attempts() > client_conn_attempts_b + 3); + TASK_UTIL_EXPECT_TRUE( + agent_c_->get_sm_connect_attempts() > client_conn_attempts_c + 3); + + TASK_UTIL_EXPECT_TRUE(agent_a_->get_sm_keepalive_count() <= 1); + TASK_UTIL_EXPECT_TRUE(agent_b_->get_sm_keepalive_count() <= 1); + TASK_UTIL_EXPECT_TRUE(agent_c_->get_sm_keepalive_count() <= 1); + + Configure(bgp_config_template, 64512, 64512); + task_util::WaitForIdle(); + TASK_UTIL_EXPECT_TRUE(bs_x_->HasSelfConfiguration()); + + TASK_UTIL_EXPECT_TRUE(agent_a_->IsEstablished()); + TASK_UTIL_EXPECT_TRUE(agent_b_->IsEstablished()); + TASK_UTIL_EXPECT_TRUE(agent_c_->IsEstablished()); + + TASK_UTIL_EXPECT_TRUE(agent_a_->get_sm_keepalive_count() >= 3); + TASK_UTIL_EXPECT_TRUE(agent_b_->get_sm_keepalive_count() >= 3); + TASK_UTIL_EXPECT_TRUE(agent_c_->get_sm_keepalive_count() >= 3); + + TASK_UTIL_EXPECT_EQ(0, agent_a_->get_sm_connect_attempts()); + TASK_UTIL_EXPECT_EQ(0, agent_b_->get_sm_connect_attempts()); + TASK_UTIL_EXPECT_EQ(0, agent_c_->get_sm_connect_attempts()); + + DestroyAgents(); + XmppStateMachineTest::set_hold_time_msecs(0); +} + TEST_P(BgpXmppBasicParamTest, ShutdownServer1) { // Create agents and wait for them to come up. diff --git a/src/control-node/test/network_agent_mock.cc b/src/control-node/test/network_agent_mock.cc index cdbc7c5c474..a80b73350ec 100644 --- a/src/control-node/test/network_agent_mock.cc +++ b/src/control-node/test/network_agent_mock.cc @@ -878,6 +878,18 @@ void NetworkAgentMock::SessionUp() { connection->SetAdminState(false); } +size_t NetworkAgentMock::get_sm_connect_attempts() { + XmppConnection *connection = + client_->FindConnection("network-control@contrailsystems.com"); + return (connection ? connection->get_sm_connect_attempts() : 0); +} + +size_t NetworkAgentMock::get_sm_keepalive_count() { + XmppConnection *connection = + client_->FindConnection("network-control@contrailsystems.com"); + return (connection ? connection->get_sm_keepalive_count() : 0); +} + size_t NetworkAgentMock::get_connect_error() { XmppConnection *connection = client_->FindConnection("network-control@contrailsystems.com"); diff --git a/src/control-node/test/network_agent_mock.h b/src/control-node/test/network_agent_mock.h index deaadad4ee0..2b5f086d223 100644 --- a/src/control-node/test/network_agent_mock.h +++ b/src/control-node/test/network_agent_mock.h @@ -457,6 +457,8 @@ class NetworkAgentMock { bool ProcessRequest(Request *request); + size_t get_sm_connect_attempts(); + size_t get_sm_keepalive_count(); size_t get_connect_error(); size_t get_session_close(); uint32_t flap_count(); diff --git a/src/xmpp/test/xmpp_client_auth_sm_test.cc b/src/xmpp/test/xmpp_client_auth_sm_test.cc index 411c5131cea..ff5a1b586fa 100644 --- a/src/xmpp/test/xmpp_client_auth_sm_test.cc +++ b/src/xmpp/test/xmpp_client_auth_sm_test.cc @@ -168,7 +168,7 @@ class XmppStateMachineTest : public ::testing::Test { EXPECT_TRUE(HoldTimerRunning()); EXPECT_TRUE(sm_->session() != NULL); EXPECT_TRUE(connection_->session() != NULL); - EXPECT_TRUE(sm_->GetConnectTime() == 0); + EXPECT_TRUE(sm_->get_connect_attempts() != 0); break; default: ASSERT_TRUE(false); diff --git a/src/xmpp/test/xmpp_client_sm_test.cc b/src/xmpp/test/xmpp_client_sm_test.cc index 892e94953ab..496838975e8 100644 --- a/src/xmpp/test/xmpp_client_sm_test.cc +++ b/src/xmpp/test/xmpp_client_sm_test.cc @@ -135,7 +135,7 @@ class XmppStateMachineTest : public ::testing::Test { EXPECT_TRUE(HoldTimerRunning()); EXPECT_TRUE(sm_->session() != NULL); EXPECT_TRUE(connection_->session() != NULL); - EXPECT_TRUE(sm_->GetConnectTime() == 0); + EXPECT_TRUE(sm_->get_connect_attempts() != 0); break; default: ASSERT_TRUE(false); @@ -635,6 +635,40 @@ TEST_F(XmppStateMachineTest, Connect_EvTcpConnectFailed_EvTcpConnected) { VerifyState(xmsm::OPENSENT); } +TEST_F(XmppStateMachineTest, ConnectionBackoff) { + sm_->connect_attempts_inc(); // set attempts as we do not + // want connection timer to expire + sm_->connect_attempts_inc(); // set attempts as we do not + // want connection timer to expire + VerifyState(xmsm::IDLE); + EvStart(); + VerifyState(xmsm::ACTIVE); + + for (int idx = 0; idx <= 6; ++idx) { + EvConnectTimerExpired(); + VerifyState(xmsm::CONNECT); + + EvTcpConnected(); + VerifyState(xmsm::OPENSENT); + + EvXmppOpen(); + VerifyState(xmsm::ESTABLISHED); + + if (idx == 6) + break; + + EvTcpClose(); + VerifyState(xmsm::ACTIVE); + } + + TASK_UTIL_EXPECT_TRUE(sm_->get_connect_attempts() >= 6); + EvXmppKeepalive(); + EvXmppKeepalive(); + EvXmppKeepalive(); + TASK_UTIL_EXPECT_EQ(3, sm_->get_keepalive_count()); + TASK_UTIL_EXPECT_EQ(0, sm_->get_connect_attempts()); +} + } static void SetUp() { diff --git a/src/xmpp/xmpp_connection.cc b/src/xmpp/xmpp_connection.cc index d797ec16551..75ed39bad63 100644 --- a/src/xmpp/xmpp_connection.cc +++ b/src/xmpp/xmpp_connection.cc @@ -487,6 +487,14 @@ size_t XmppConnection::get_handshake_failure() { return error_stats_.handshake_fail; } +size_t XmppConnection::get_sm_connect_attempts() { + return state_machine_->get_connect_attempts(); +} + +size_t XmppConnection::get_sm_keepalive_count() { + return state_machine_->get_keepalive_count(); +} + void XmppConnection::ReceiveMsg(XmppSession *session, const string &msg) { XmppStanza::XmppMessage *minfo = XmppDecode(msg); diff --git a/src/xmpp/xmpp_connection.h b/src/xmpp/xmpp_connection.h index fe9ca64656f..135de092dc4 100644 --- a/src/xmpp/xmpp_connection.h +++ b/src/xmpp/xmpp_connection.h @@ -215,6 +215,8 @@ class XmppConnection { size_t get_open_fail(); size_t get_stream_feature_fail(); size_t get_handshake_failure(); + size_t get_sm_connect_attempts(); + size_t get_sm_keepalive_count(); static const char *kAuthTypeNil; static const char *kAuthTypeTls; diff --git a/src/xmpp/xmpp_state_machine.cc b/src/xmpp/xmpp_state_machine.cc index 9be89a8269d..acba4cfd1b8 100644 --- a/src/xmpp/xmpp_state_machine.cc +++ b/src/xmpp/xmpp_state_machine.cc @@ -264,6 +264,7 @@ struct Active : public sc::state { Active(my_context ctx) : my_base(ctx) { XmppStateMachine *state_machine = &context(); SM_LOG(state_machine, "(Xmpp Active State)"); + state_machine->keepalive_count_clear(); bool flap = (state_machine->get_state() == ESTABLISHED); state_machine->set_state(ACTIVE); if (flap) { @@ -951,7 +952,6 @@ struct XmppStreamEstablished : XmppStreamEstablished(my_context ctx) : my_base(ctx) { XmppStateMachine *state_machine = &context(); SM_LOG(state_machine, "(XMPP Established)"); - state_machine->connect_attempts_clear(); XmppConnection *connection = state_machine->connection(); state_machine->StartHoldTimer(); state_machine->set_state(ESTABLISHED); @@ -984,6 +984,10 @@ struct XmppStreamEstablished : if (event.session != state_machine->session()) { return discard_event(); } + state_machine->keepalive_count_inc(); + if (state_machine->get_keepalive_count() == 3) { + state_machine->connect_attempts_clear(); + } state_machine->StartHoldTimer(); return discard_event(); } @@ -1118,6 +1122,7 @@ XmppStateMachine::XmppStateMachine(XmppConnection *connection, bool active, TaskScheduler::GetInstance()->GetTaskId("xmpp::StateMachine"), 0)), hold_time_(GetConfiguredHoldTime()), attempts_(0), + keepalive_count_(0), deleted_(false), in_dequeue_(false), is_active_(active), @@ -1479,7 +1484,7 @@ void XmppStateMachine::AssignSession() { const int XmppStateMachine::kConnectInterval; int XmppStateMachine::GetConnectTime() const { - int backoff = min(attempts_, 6); + int backoff = attempts_ > 6 ? 6 : attempts_; return std::min(backoff ? 1 << (backoff - 1) : 0, kConnectInterval); } diff --git a/src/xmpp/xmpp_state_machine.h b/src/xmpp/xmpp_state_machine.h index 9b3fbbe92c5..c0f3c31d772 100644 --- a/src/xmpp/xmpp_state_machine.h +++ b/src/xmpp/xmpp_state_machine.h @@ -141,7 +141,11 @@ class XmppStateMachine : void connect_attempts_inc() { attempts_++; } void connect_attempts_clear() { attempts_ = 0; } - int get_connect_attempts() { return attempts_; } + int get_connect_attempts() const { return attempts_; } + + void keepalive_count_inc() { keepalive_count_++; } + void keepalive_count_clear() { keepalive_count_ = 0; } + int get_keepalive_count() const { return keepalive_count_; } int hold_time() const { return hold_time_; } virtual int hold_time_msecs() const { return hold_time_ * 1000; } @@ -180,7 +184,8 @@ class XmppStateMachine : Timer *open_timer_; Timer *hold_timer_; int hold_time_; - int attempts_; + uint32_t attempts_; + uint32_t keepalive_count_; bool deleted_; bool in_dequeue_; bool is_active_;