Merge "Fix corner case in bgp peer close processing"

Juniper · Apr 18, 2016 · 9102f88 · 9102f88
2 parents ccce787 + 8e41bf7
commit 9102f88
Show file tree

Hide file tree

Showing 5 changed files with 16 additions and 4 deletions.
diff --git a/src/bgp/state_machine.cc b/src/bgp/state_machine.cc
@@ -1137,11 +1137,12 @@ void StateMachine::SetAdminState(bool down) {
     if (down) {
         Enqueue(fsm::EvStop(BgpProto::Notification::AdminShutdown));
     } else {
+        // Reset all previous state.
         reset_idle_hold_time();
-        peer_->reset_flap_count();
-        // On fresh restart of state machine, all previous state should be reset
         reset_last_info();
-        Enqueue(fsm::EvStart());
+        peer_->reset_flap_count();
+        if (!peer_->IsCloseInProgress())
+            Enqueue(fsm::EvStart());
     }
 }
 

diff --git a/src/bgp/test/bgp_server_test.cc b/src/bgp/test/bgp_server_test.cc
@@ -2,7 +2,7 @@
  * Copyright (c) 2013 Juniper Networks, Inc. All rights reserved.
  */
 
-
+#include "base/task_annotations.h"
 #include "bgp/bgp_config_parser.h"
 #include "bgp/bgp_factory.h"
 #include "bgp/bgp_peer_membership.h"
@@ -1224,6 +1224,7 @@ TEST_F(BgpServerUnitTest, ChangePeerAddressFamilies) {
 }
 
 TEST_F(BgpServerUnitTest, AdminDown) {
+    ConcurrencyScope scope("bgp::Config");
     int peer_count = 3;
 
     BgpPeerTest::verbose_name(true);
@@ -1867,6 +1868,7 @@ TEST_F(BgpServerUnitTest, Passive2) {
 }
 
 TEST_F(BgpServerUnitTest, ResetStatsOnFlap) {
+    ConcurrencyScope scope("bgp::Config");
     int peer_count = 3;
 
     StateMachineTest::set_keepalive_time_msecs(10);
@@ -2650,6 +2652,7 @@ TEST_F(BgpServerUnitTest, DeleteInProgress) {
 }
 
 TEST_F(BgpServerUnitTest, CloseInProgress) {
+    ConcurrencyScope scope("bgp::Config");
     int peer_count = 3;
 
     vector<string> families_a;
@@ -2702,6 +2705,7 @@ TEST_F(BgpServerUnitTest, CloseInProgress) {
 }
 
 TEST_F(BgpServerUnitTest, CloseDeferred) {
+    ConcurrencyScope scope("bgp::Config");
     int peer_count = 3;
 
     vector<string> families_a;

diff --git a/src/bgp/test/bgp_server_test_util.cc b/src/bgp/test/bgp_server_test_util.cc
@@ -6,6 +6,7 @@
 
 #include <boost/foreach.hpp>
 
+#include "base/task_annotations.h"
 #include "bgp/bgp_config_ifmap.h"
 #include "bgp/bgp_config_parser.h"
 #include "bgp/bgp_factory.h"
@@ -138,13 +139,15 @@ BgpPeer *BgpServerTest::FindMatchingPeer(const string &routing_instance,
 }
 
 void BgpServerTest::DisableAllPeers() {
+    ConcurrencyScope scope("bgp::Config");
     for (BgpPeerList::iterator it = peer_list_.begin();
          it != peer_list_.end(); ++it) {
         it->second->SetAdminState(true);
     }
 }
 
 void BgpServerTest::EnableAllPeers() {
+    ConcurrencyScope scope("bgp::Config");
     for (BgpPeerList::iterator it = peer_list_.begin();
          it != peer_list_.end(); ++it) {
         it->second->SetAdminState(false);

diff --git a/src/bgp/test/bgp_stress_test.cc b/src/bgp/test/bgp_stress_test.cc
@@ -11,6 +11,7 @@
 #include <map>
 #include <string>
 
+#include "base/task_annotations.h"
 #include "base/test/addr_test_util.h"
 
 #include "bgp/bgp_config_parser.h"
@@ -2103,6 +2104,7 @@ void BgpStressTest::DeleteBgpPeers(int npeers) {
 }
 
 void BgpStressTest::ClearBgpPeer(vector<int> peer_ids) {
+    ConcurrencyScope scope("bgp::Config");
     map<int, bool> established;
     map<int, uint32_t> flap_count;
 

diff --git a/src/bgp/test/state_machine_test.cc b/src/bgp/test/state_machine_test.cc
@@ -394,10 +394,12 @@ class StateMachineUnitTest : public ::testing::Test {
         sm_->Shutdown(BgpProto::Notification::AdminShutdown);
     }
     void EvAdminUp() {
+        ConcurrencyScope scope("bgp::Config");
         peer_->SetAdminState(false);
         sm_->set_idle_hold_time(1);
     }
     void EvAdminDown() {
+        ConcurrencyScope scope("bgp::Config");
         peer_->SetAdminState(true);
         sm_->set_idle_hold_time(1);
     }