diff --git a/src/bgp/bgp_peer.cc b/src/bgp/bgp_peer.cc index 8082ffd72da..e1b69584845 100644 --- a/src/bgp/bgp_peer.cc +++ b/src/bgp/bgp_peer.cc @@ -710,6 +710,7 @@ const IPeerDebugStats *BgpPeer::peer_stats() const { } void BgpPeer::Clear(int subcode) { + CHECK_CONCURRENCY("bgp::Config"); state_machine_->Shutdown(subcode); } diff --git a/src/bgp/test/bgp_peer_close_test.cc b/src/bgp/test/bgp_peer_close_test.cc index dd0f2a58362..19ce9afbc56 100644 --- a/src/bgp/test/bgp_peer_close_test.cc +++ b/src/bgp/test/bgp_peer_close_test.cc @@ -926,7 +926,7 @@ TEST_P(BgpPeerCloseTest, ClosePeers) { // Trigger ribin deletes BOOST_FOREACH(BgpNullPeer *npeer, peers_) { - npeer->peer()->Clear(BgpProto::Notification::AdminReset); + npeer->peer()->SetAdminState(true); } XmppPeerClose(); @@ -1027,7 +1027,7 @@ TEST_P(BgpPeerCloseTest, DISABLED_ClosePeersWithRouteStalingAndDelete) { // Trigger ribin deletes BOOST_FOREACH(BgpNullPeer *npeer, peers_) { - npeer->peer()->Clear(BgpProto::Notification::AdminReset); + npeer->peer()->SetAdminState(true); } XmppPeerClose(); diff --git a/src/bgp/test/bgp_server_test.cc b/src/bgp/test/bgp_server_test.cc index 614553a3a25..a6442ca43d1 100644 --- a/src/bgp/test/bgp_server_test.cc +++ b/src/bgp/test/bgp_server_test.cc @@ -1921,18 +1921,14 @@ TEST_F(BgpServerUnitTest, HoldTimeChange) { TASK_UTIL_EXPECT_EQ(10 * (idx - 1), sm_b->hold_time()); } - // Clear all the sessions. + // Clear all the sessions by setting peers to admin down on A. for (int j = 0; j < peer_count; j++) { string uuid = BgpConfigParser::session_uuid("A", "B", j + 1); BgpPeer *peer_a = a_->FindPeerByUuid(BgpConfigManager::kMasterInstance, uuid); - peer_a->Clear(BgpProto::Notification::AdminReset); - task_util::WaitForIdle(); - - BgpPeer *peer_b = - b_->FindPeerByUuid(BgpConfigManager::kMasterInstance, uuid); - peer_b->Clear(BgpProto::Notification::AdminReset); + peer_a->SetAdminState(true); task_util::WaitForIdle(); + peer_a->SetAdminState(false); } VerifyPeers(peer_count); @@ -2236,15 +2232,15 @@ TEST_F(BgpServerUnitTest, CloseInProgress) { PausePeerRibMembershipManager(a_.get()); // - // Trigger close of peers on A + // Trigger close of peers on A by making B send notifications // Peer close will be started but not completed since peer membership // manager has been paused // for (int j = 0; j < peer_count; j++) { string uuid = BgpConfigParser::session_uuid("A", "B", j + 1); - BgpPeer *peer_a = a_->FindPeerByUuid(BgpConfigManager::kMasterInstance, + BgpPeer *peer_b = b_->FindPeerByUuid(BgpConfigManager::kMasterInstance, uuid); - peer_a->Clear(BgpProto::Notification::AdminReset); + peer_b->SetAdminState(true); } // @@ -2254,6 +2250,7 @@ TEST_F(BgpServerUnitTest, CloseInProgress) { string uuid = BgpConfigParser::session_uuid("A", "B", j + 1); BgpPeer *peer_b = b_->FindPeerByUuid(BgpConfigManager::kMasterInstance, uuid); + peer_b->SetAdminState(false); TASK_UTIL_EXPECT_TRUE(peer_b->get_rx_notification() >= 3); } @@ -2333,23 +2330,27 @@ TEST_F(BgpServerUnitTest, CloseDeferred) { } // - // Trigger close of peers on A + // Trigger close of peers on A by making B send notifications // Peer close will be deferred since peer membership manager was paused // before the peers got established // for (int j = 0; j < peer_count; j++) { string uuid = BgpConfigParser::session_uuid("A", "B", j + 1); - BgpPeer *peer_a = a_->FindPeerByUuid(BgpConfigManager::kMasterInstance, + BgpPeer *peer_b = b_->FindPeerByUuid(BgpConfigManager::kMasterInstance, uuid); - peer_a->Clear(BgpProto::Notification::AdminReset); + peer_b->SetAdminState(true); } + // + // Note down notification counts and bring up peers on B + // vector b_rx_notification(peer_count); for (int j = 0; j < peer_count; j++) { string uuid = BgpConfigParser::session_uuid("A", "B", j + 1); BgpPeer *peer_b = b_->FindPeerByUuid(BgpConfigManager::kMasterInstance, uuid); b_rx_notification[j] = peer_b->get_rx_notification(); + peer_b->SetAdminState(false); } // diff --git a/src/bgp/test/bgp_stress_test.cc b/src/bgp/test/bgp_stress_test.cc index bf7a6b2c5eb..5f0d01c4723 100644 --- a/src/bgp/test/bgp_stress_test.cc +++ b/src/bgp/test/bgp_stress_test.cc @@ -2119,6 +2119,7 @@ void BgpStressTest::ClearBgpPeer(vector peer_ids) { map established; map flap_count; + // Remember flap counts and bring down peers. BOOST_FOREACH(int peer_id, peer_ids) { if (peer_id >= (int) peers_.size() || !peers_[peer_id]) continue; @@ -2126,23 +2127,30 @@ void BgpStressTest::ClearBgpPeer(vector peer_ids) { peers_[peer_id]->peer()->flap_count())); established.insert(make_pair(peer_id, peers_[peer_id]->peer()->GetState() == StateMachine::ESTABLISHED)); - peers_[peer_id]->peer()->Clear(BgpProto::Notification::AdminReset); + peers_[peer_id]->peer()->SetAdminState(true); } + // Verify that established peers did flap. BOOST_FOREACH(int peer_id, peer_ids) { - if (peer_id >= (int) peers_.size() || !peers_[peer_id]) continue; - - // - // If the peer was established, first make sure that it did flap - // + if (peer_id >= (int) peers_.size() || !peers_[peer_id]) + continue; if (established[peer_id]) { TASK_UTIL_EXPECT_TRUE(peers_[peer_id]->peer()->flap_count() > flap_count[peer_id]); } + } - // - // Wait for the peer to come back up - // + // Bring up peers. + BOOST_FOREACH(int peer_id, peer_ids) { + if (peer_id >= (int) peers_.size() || !peers_[peer_id]) + continue; + peers_[peer_id]->peer()->SetAdminState(false); + } + + // Wait for peers to come back up. + BOOST_FOREACH(int peer_id, peer_ids) { + if (peer_id >= (int) peers_.size() || !peers_[peer_id]) + continue; BGP_WAIT_FOR_PEER_STATE(peers_[peer_id]->peer(), StateMachine::ESTABLISHED); }