From a975f6e8d4d6245388ac0ba3ec841754a80c7c6d Mon Sep 17 00:00:00 2001 From: Ananth Suryanarayana Date: Fri, 2 Dec 2016 17:02:08 -0800 Subject: [PATCH] Collect UVE information off bgp::ShowCommand task bgp::Uve task is not exclusive to peerMembership task which caused the crash reported in the bug Instead of having uve specific task and maintaining its policy correctly, use bgp::ShowCommand task. This task already has correctly policy configuration and runs exclusively from bgp::Config, bgp::ConfigHelper and bgp::PeerMembership task Change-Id: I11b543e4ceac279f886459f2d48280a7d1f88566 Closes-Bug: #1644424 --- src/bgp/bgp_server.cc | 6 +++--- src/bgp/bgp_table.cc | 2 +- src/bgp/bgp_xmpp_channel.cc | 2 +- src/bgp/routing-instance/static_route.cc | 4 ++-- src/bgp/test/static_route_test.cc | 4 ++-- src/control-node/control_node.cc | 1 - src/control-node/main.cc | 2 +- src/ifmap/ifmap_server.cc | 2 +- 8 files changed, 11 insertions(+), 12 deletions(-) diff --git a/src/bgp/bgp_server.cc b/src/bgp/bgp_server.cc index f14e15bf8f2..d5ba8fa1103 100644 --- a/src/bgp/bgp_server.cc +++ b/src/bgp/bgp_server.cc @@ -716,7 +716,7 @@ void BgpServer::NotifyAllStaticRoutes() { } uint32_t BgpServer::GetStaticRouteCount() const { - CHECK_CONCURRENCY("bgp::Uve"); + CHECK_CONCURRENCY("bgp::ShowCommand"); uint32_t count = 0; for (StaticRouteMgrList::iterator it = srt_manager_list_.begin(); it != srt_manager_list_.end(); ++it) { @@ -727,7 +727,7 @@ uint32_t BgpServer::GetStaticRouteCount() const { } uint32_t BgpServer::GetDownStaticRouteCount() const { - CHECK_CONCURRENCY("bgp::Uve"); + CHECK_CONCURRENCY("bgp::ShowCommand"); uint32_t count = 0; for (StaticRouteMgrList::iterator it = srt_manager_list_.begin(); it != srt_manager_list_.end(); ++it) { @@ -833,7 +833,7 @@ void BgpServer::FillPeerStats(const BgpPeer *peer) const { } bool BgpServer::CollectStats(BgpRouterState *state, bool first) const { - CHECK_CONCURRENCY("bgp::Uve"); + CHECK_CONCURRENCY("bgp::ShowCommand"); VisitBgpPeers(boost::bind(&BgpServer::FillPeerStats, this, _1)); bool change = false; diff --git a/src/bgp/bgp_table.cc b/src/bgp/bgp_table.cc index 323ac4a753b..fc1a911e0b1 100644 --- a/src/bgp/bgp_table.cc +++ b/src/bgp/bgp_table.cc @@ -642,7 +642,7 @@ void BgpTable::DestroyPathResolver() { } size_t BgpTable::GetPendingRiboutsCount(size_t *markers) const { - CHECK_CONCURRENCY("bgp::ShowCommand", "bgp::Config", "bgp::Uve"); + CHECK_CONCURRENCY("bgp::ShowCommand", "bgp::Config"); size_t count = 0; *markers = 0; diff --git a/src/bgp/bgp_xmpp_channel.cc b/src/bgp/bgp_xmpp_channel.cc index be1539f2c80..c7d31cc4263 100644 --- a/src/bgp/bgp_xmpp_channel.cc +++ b/src/bgp/bgp_xmpp_channel.cc @@ -2857,7 +2857,7 @@ void BgpXmppChannelManager::FillPeerInfo(const BgpXmppChannel *channel) const { bool BgpXmppChannelManager::CollectStats(BgpRouterState *state, bool first) const { - CHECK_CONCURRENCY("bgp::Uve"); + CHECK_CONCURRENCY("bgp::ShowCommand"); VisitChannels(boost::bind(&BgpXmppChannelManager::FillPeerInfo, this, _1)); bool change = false; diff --git a/src/bgp/routing-instance/static_route.cc b/src/bgp/routing-instance/static_route.cc index 3acfa47d7f4..7d8ab8c7c11 100644 --- a/src/bgp/routing-instance/static_route.cc +++ b/src/bgp/routing-instance/static_route.cc @@ -911,13 +911,13 @@ void StaticRouteMgr::EnableUnregisterTrigger() { template uint32_t StaticRouteMgr::GetRouteCount() const { - CHECK_CONCURRENCY("bgp::Uve"); + CHECK_CONCURRENCY("bgp::ShowCommand"); return static_route_map_.size(); } template uint32_t StaticRouteMgr::GetDownRouteCount() const { - CHECK_CONCURRENCY("bgp::Uve"); + CHECK_CONCURRENCY("bgp::ShowCommand"); uint32_t count = 0; for (typename StaticRouteMap::const_iterator it = static_route_map_.begin(); it != static_route_map_.end(); ++it) { diff --git a/src/bgp/test/static_route_test.cc b/src/bgp/test/static_route_test.cc index c1f93340110..8844f196fd0 100644 --- a/src/bgp/test/static_route_test.cc +++ b/src/bgp/test/static_route_test.cc @@ -775,12 +775,12 @@ class StaticRouteTest : public ::testing::Test { } void VerifyStaticRouteCount(uint32_t count) { - ConcurrencyScope scope("bgp::Uve"); + ConcurrencyScope scope("bgp::ShowCommand"); TASK_UTIL_EXPECT_EQ(count, bgp_server_->num_static_routes()); } void VerifyDownStaticRouteCount(uint32_t count) { - ConcurrencyScope scope("bgp::Uve"); + ConcurrencyScope scope("bgp::ShowCommand"); TASK_UTIL_EXPECT_EQ(count, bgp_server_->num_down_static_routes()); } diff --git a/src/control-node/control_node.cc b/src/control-node/control_node.cc index c4eafcfd438..2e18970037d 100644 --- a/src/control-node/control_node.cc +++ b/src/control-node/control_node.cc @@ -44,7 +44,6 @@ void ControlNode::SetDefaultSchedulingPolicy() { (TaskExclusion(scheduler->GetTaskId("bgp::ShowCommand"))) (TaskExclusion(scheduler->GetTaskId("bgp::SendReadyTask"))) (TaskExclusion(scheduler->GetTaskId("bgp::StaticRoute"))) - (TaskExclusion(scheduler->GetTaskId("bgp::Uve"))) (TaskExclusion(scheduler->GetTaskId("bgp::RouteAggregation"))) (TaskExclusion(scheduler->GetTaskId("bgp::ResolverPath"))) (TaskExclusion(scheduler->GetTaskId("bgp::ResolverNexthop"))); diff --git a/src/control-node/main.cc b/src/control-node/main.cc index 042655495a5..3570f5105de 100644 --- a/src/control-node/main.cc +++ b/src/control-node/main.cc @@ -552,7 +552,7 @@ int main(int argc, char *argv[]) { boost::bind(&ControlNodeInfoLogger, bgp_server.get(), bgp_peer_manager.get(), &ifmap_server, node_info_log_timer.get()), - TaskScheduler::GetInstance()->GetTaskId("bgp::Uve"), 0)); + TaskScheduler::GetInstance()->GetTaskId("bgp::ShowCommand"), 0)); // Start periodic timer to send BGPRouterInfo UVE. node_info_log_timer->Start( diff --git a/src/ifmap/ifmap_server.cc b/src/ifmap/ifmap_server.cc index 3117fe95de2..4dd4a4a508e 100644 --- a/src/ifmap/ifmap_server.cc +++ b/src/ifmap/ifmap_server.cc @@ -522,7 +522,7 @@ void IFMapServer::GetUIInfo(IFMapServerInfoUI *server_info) const { } bool IFMapServer::CollectStats(BgpRouterState *state, bool first) const { - CHECK_CONCURRENCY("bgp::Uve"); + CHECK_CONCURRENCY("bgp::ShowCommand"); IFMapPeerServerInfoUI peer_server_info; bool change = false;