Merge "Speed up creation of RoutingInstances by using multiple Tasks"

src/bgp/bgp_condition_listener.cc

@@ -137,8 +137,9 @@ bool BgpConditionListener::PurgeTableState() {
 void BgpConditionListener::AddMatchCondition(BgpTable *table,
                                              ConditionMatch *obj,
                                              RequestDoneCb cb) {
-    CHECK_CONCURRENCY("bgp::Config");
+    CHECK_CONCURRENCY("bgp::Config", "bgp::ConfigHelper");
 
+    tbb::mutex::scoped_lock lock(mutex_);
     ConditionMatchTableState *ts = NULL;
     TableMap::iterator loc = map_.find(table);
     if (loc == map_.end()) {
@@ -167,7 +168,9 @@ void BgpConditionListener::AddMatchCondition(BgpTable *table,
 void BgpConditionListener::RemoveMatchCondition(BgpTable *table,
                                                 ConditionMatch *obj,
                                                 RequestDoneCb cb) {
-    CHECK_CONCURRENCY("bgp::Config");
+    CHECK_CONCURRENCY("bgp::Config", "bgp::ConfigHelper");
+
+    tbb::mutex::scoped_lock lock(mutex_);
     obj->SetDeleted();
 
     TableMap::iterator loc = map_.find(table);
@@ -292,7 +295,7 @@ void BgpConditionListener::RemoveMatchState(BgpTable *table, BgpRoute *route,
 
 void BgpConditionListener::TableWalk(ConditionMatchTableState *ts,
                  ConditionMatch *obj, BgpConditionListener::RequestDoneCb cb) {
-    CHECK_CONCURRENCY("bgp::Config");
+    CHECK_CONCURRENCY("bgp::Config", "bgp::ConfigHelper");
 
     if (ts->walk_ref() == NULL) {
         DBTable::DBTableWalkRef walk_ref = ts->table()->AllocWalker(

src/bgp/bgp_condition_listener.h

@@ -151,6 +151,8 @@ class ConditionMatchTableState;
 // to start applying the match condition on all BgpRoutes
 // Provides an interface to add/remove module specific data on each BgpRoute
 //
+// A mutex is used to serialize access from multiple bgp::ConfigHelper tasks.
+//
 class BgpConditionListener {
 public:
     typedef std::map<BgpTable *, ConditionMatchTableState *> TableMap;
@@ -200,12 +202,6 @@ class BgpConditionListener {
     template <typename U> friend class PathResolverTest;
     typedef std::set<ConditionMatchTableState *> PurgeTableStateList;
 
-    BgpServer *server_;
-
-    TableMap map_;
-
-    PurgeTableStateList purge_list_;
-
     // Table listener
     bool BgpRouteNotify(BgpServer *server, DBTablePartBase *root,
                         DBEntryBase *entry);
@@ -222,7 +218,12 @@ class BgpConditionListener {
     void DisableTableWalkProcessing();
     void EnableTableWalkProcessing();
 
+    BgpServer *server_;
+    tbb::mutex mutex_;
+    TableMap map_;
+    PurgeTableStateList purge_list_;
     boost::scoped_ptr<TaskTrigger> purge_trigger_;
+
     DISALLOW_COPY_AND_ASSIGN(BgpConditionListener);
 };
 

src/bgp/bgp_config.h

@@ -374,6 +374,7 @@ class BgpRoutingPolicyConfig {
 // Instance configuration.
 class BgpInstanceConfig {
 public:
+    typedef std::set<std::string> NeighborList;
     typedef std::set<std::string> RouteTargetList;
     typedef std::vector<StaticRouteConfig> StaticRouteList;
     typedef std::vector<ServiceChainConfig> ServiceChainList;
@@ -384,6 +385,14 @@ class BgpInstanceConfig {
 
     const std::string &name() const { return name_; }
 
+    const NeighborList &neighbor_list() const { return neighbor_list_; }
+    void add_neighbor(const std::string &neighbor) {
+        neighbor_list_.insert(neighbor);
+    }
+    void delete_neighbor(const std::string &neighbor) {
+        neighbor_list_.erase(neighbor);
+    }
+
     const RouteTargetList &import_list() const { return import_list_; }
     void set_import_list(const RouteTargetList &import_list) {
         import_list_ = import_list;
@@ -446,6 +455,7 @@ class BgpInstanceConfig {
     friend class BgpInstanceConfigTest;
 
     std::string name_;
+    NeighborList neighbor_list_;
     RouteTargetList import_list_;
     RouteTargetList export_list_;
     bool has_pnf_;

src/bgp/bgp_config_ifmap.cc

@@ -1137,6 +1137,7 @@ void BgpIfmapInstanceConfig::AddNeighbor(BgpConfigManager *manager,
         neighbor->GetAddressFamilies(), neighbor->AuthKeyTypeToString(),
         neighbor->AuthKeysToString());
     neighbors_.insert(make_pair(neighbor->name(), neighbor));
+    data_.add_neighbor(neighbor->name());
     manager->Notify(neighbor, BgpConfigManager::CFG_ADD);
 }
 
@@ -1175,6 +1176,7 @@ void BgpIfmapInstanceConfig::DeleteNeighbor(BgpConfigManager *manager,
         SandeshLevel::SYS_DEBUG, BGP_LOG_FLAG_ALL);
     manager->Notify(neighbor, BgpConfigManager::CFG_DELETE);
     neighbors_.erase(neighbor->name());
+    data_.delete_neighbor(neighbor->name());
 }
 
 //

src/bgp/bgp_config_parser.cc

@@ -360,6 +360,22 @@ static bool ParseStaticRoute(const string &instance, const xml_node &node,
     return true;
 }
 
+static bool ParseInstanceHasPnf(const string &instance, const xml_node &node,
+    bool add_change, BgpConfigParser::RequestList *requests) {
+    auto_ptr<autogen::RoutingInstance::OolProperty> property(
+        new autogen::RoutingInstance::OolProperty);
+    property->data = (string(node.child_value()) == "true");
+    if (add_change) {
+        MapObjectSetProperty("routing-instance", instance,
+            "routing-instance-has-pnf", property.release(), requests);
+    } else {
+        MapObjectClearProperty("routing-instance", instance,
+            "routing-instance-has-pnf", requests);
+    }
+
+    return true;
+}
+
 static bool ParseBgpRouter(const string &instance, const xml_node &node,
                            bool add_change, string *nodename,
                            SessionMap *sessions,
@@ -517,6 +533,8 @@ bool BgpConfigParser::ParseRoutingInstance(const xml_node &parent,
             ParseInstanceRoutingPolicy(instance, node, add_change, requests);
         } else if (strcmp(node.name(), "static-route-entries") == 0) {
             ParseStaticRoute(instance, node, add_change, requests);
+        } else if (strcmp(node.name(), "routing-instance-has-pnf") == 0) {
+            ParseInstanceHasPnf(instance, node, add_change, requests);
         }
     }
 

src/bgp/bgp_membership.cc

@@ -107,7 +107,8 @@ void BgpMembershipManager::NotifyPeerRegistration(IPeer *peer, BgpTable *table,
 //
 void BgpMembershipManager::Register(IPeer *peer, BgpTable *table,
     const RibExportPolicy &policy, int instance_id) {
-    CHECK_CONCURRENCY("bgp::Config", "bgp::StateMachine", "xmpp::StateMachine");
+    CHECK_CONCURRENCY("bgp::Config", "bgp::ConfigHelper",
+        "bgp::StateMachine", "xmpp::StateMachine");
 
     tbb::spin_rw_mutex::scoped_lock write_lock(rw_mutex_, true);
     current_jobs_count_++;

src/bgp/bgp_server.cc

@@ -63,8 +63,7 @@ class BgpServer::ConfigUpdater {
         server_->global_config()->set_eor_rx_time(system->eor_rx_time());
 
         RoutingInstanceMgr *ri_mgr = server_->routing_instance_mgr();
-        RoutingInstance *rti =
-            ri_mgr->GetRoutingInstance(BgpConfigManager::kMasterInstance);
+        RoutingInstance *rti = ri_mgr->GetDefaultRoutingInstance();
         assert(rti);
         PeerManager *peer_manager = rti->LocatePeerManager();
         peer_manager->ClearAllPeers();
@@ -195,9 +194,10 @@ class BgpServer::ConfigUpdater {
         string instance_name = neighbor_config->instance_name();
         RoutingInstanceMgr *ri_mgr = server_->routing_instance_mgr();
         RoutingInstance *rti = ri_mgr->GetRoutingInstance(instance_name);
-        assert(rti);
-        PeerManager *peer_manager = rti->LocatePeerManager();
+        if (!rti)
+            return;
 
+        PeerManager *peer_manager = rti->LocatePeerManager();
         if (event == BgpConfigManager::CFG_ADD ||
             event == BgpConfigManager::CFG_CHANGE) {
             BgpPeer *peer = peer_manager->PeerLocate(server_, neighbor_config);
@@ -229,10 +229,9 @@ class BgpServer::ConfigUpdater {
     void ProcessInstanceConfig(const BgpInstanceConfig *instance_config,
                                BgpConfigManager::EventType event) {
         RoutingInstanceMgr *mgr = server_->routing_instance_mgr();
-        if (event == BgpConfigManager::CFG_ADD) {
-            mgr->CreateRoutingInstance(instance_config);
-        } else if (event == BgpConfigManager::CFG_CHANGE) {
-            mgr->UpdateRoutingInstance(instance_config);
+        if (event == BgpConfigManager::CFG_ADD ||
+            event == BgpConfigManager::CFG_CHANGE) {
+            mgr->LocateRoutingInstance(instance_config->name());
         } else if (event == BgpConfigManager::CFG_DELETE) {
             mgr->DeleteRoutingInstance(instance_config->name());
         }
@@ -697,17 +696,20 @@ void BgpServer::NotifyIdentifierUpdate(Ip4Address old_identifier) {
 }
 
 void BgpServer::InsertStaticRouteMgr(IStaticRouteMgr *srt_manager) {
-    CHECK_CONCURRENCY("bgp::Config");
+    CHECK_CONCURRENCY("bgp::Config", "bgp::ConfigHelper");
+    tbb::spin_rw_mutex::scoped_lock write_lock(rw_mutex_, true);
     srt_manager_list_.insert(srt_manager);
 }
 
 void BgpServer::RemoveStaticRouteMgr(IStaticRouteMgr *srt_manager) {
-    CHECK_CONCURRENCY("bgp::Config");
+    CHECK_CONCURRENCY("bgp::Config", "bgp::ConfigHelper");
+    tbb::spin_rw_mutex::scoped_lock write_lock(rw_mutex_, true);
     srt_manager_list_.erase(srt_manager);
 }
 
 void BgpServer::NotifyAllStaticRoutes() {
-    CHECK_CONCURRENCY("bgp::Config");
+    CHECK_CONCURRENCY("bgp::Config", "bgp::ConfigHelper");
+    tbb::spin_rw_mutex::scoped_lock write_lock(rw_mutex_, true);
     for (StaticRouteMgrList::iterator it = srt_manager_list_.begin();
          it != srt_manager_list_.end(); ++it) {
         IStaticRouteMgr *srt_manager = *it;

src/bgp/bgp_session_manager.cc

@@ -105,7 +105,7 @@ BgpPeer *BgpSessionManager::FindPeer(Endpoint remote) {
     BgpPeer *peer = NULL;
     const RoutingInstance *instance =
         server_->routing_instance_mgr()->GetDefaultRoutingInstance();
-    if (instance) {
+    if (instance && !instance->deleted()) {
         peer = instance->peer_manager()->PeerLookup(remote);
     }
     if (!peer) {

src/bgp/bgp_xmpp_channel.cc

@@ -231,8 +231,7 @@ class BgpXmppChannel::PeerClose : public IPeerClose {
             return;
         BgpServer *server = parent_->bgp_server_;
         RoutingInstanceMgr *instance_mgr = server->routing_instance_mgr();
-        RoutingInstance *master =
-            instance_mgr->GetRoutingInstance(BgpConfigManager::kMasterInstance);
+        RoutingInstance *master = instance_mgr->GetDefaultRoutingInstance();
         assert(master);
         BgpTable *rtarget_table = master->GetTable(Address::RTARGET);
         assert(rtarget_table);
@@ -703,8 +702,7 @@ void BgpXmppChannel::ASNUpdateCallback(as_t old_asn, as_t old_local_asn) {
 
     RoutingInstanceMgr *instance_mgr = bgp_server_->routing_instance_mgr();
     assert(instance_mgr);
-    RoutingInstance *master =
-        instance_mgr->GetRoutingInstance(BgpConfigManager::kMasterInstance);
+    RoutingInstance *master = instance_mgr->GetDefaultRoutingInstance();
     assert(master);
     BgpTable *rtarget_table = master->GetTable(Address::RTARGET);
     assert(rtarget_table);
@@ -731,8 +729,7 @@ void BgpXmppChannel::IdentifierUpdateCallback(Ip4Address old_identifier) {
 
     RoutingInstanceMgr *instance_mgr = bgp_server_->routing_instance_mgr();
     assert(instance_mgr);
-    RoutingInstance *master =
-        instance_mgr->GetRoutingInstance(BgpConfigManager::kMasterInstance);
+    RoutingInstance *master = instance_mgr->GetDefaultRoutingInstance();
     assert(master);
     BgpTable *rtarget_table = master->GetTable(Address::RTARGET);
     assert(rtarget_table);
@@ -818,8 +815,7 @@ void BgpXmppChannel::RoutingInstanceCallback(string vrf_name, int op) {
 
         // Prepare for route add to rtarget table
         // get rtarget_table and locate the attr
-        RoutingInstance *master =
-            instance_mgr->GetRoutingInstance(BgpConfigManager::kMasterInstance);
+        RoutingInstance *master = instance_mgr->GetDefaultRoutingInstance();
         assert(master);
         BgpTable *rtarget_table = master->GetTable(Address::RTARGET);
         assert(rtarget_table);
@@ -2183,8 +2179,7 @@ void BgpXmppChannel::FlushDeferQ(string vrf_name) {
 void BgpXmppChannel::UpdateRouteTargetRouteFlag(
     const SubscriptionState *sub_state, bool llgr) {
     RoutingInstanceMgr *instance_mgr = bgp_server_->routing_instance_mgr();
-    RoutingInstance *master =
-        instance_mgr->GetRoutingInstance(BgpConfigManager::kMasterInstance);
+    RoutingInstance *master = instance_mgr->GetDefaultRoutingInstance();
     assert(master);
     BgpTable *rtarget_table = master->GetTable(Address::RTARGET);
     BOOST_FOREACH(RouteTarget rtarget, sub_state->targets) {
@@ -2280,8 +2275,7 @@ void BgpXmppChannel::PublishRTargetRoute(RoutingInstance *rt_instance,
     bool add_change, int index) {
     // Add rtarget route for import route target of the routing instance.
     RoutingInstanceMgr *instance_mgr = bgp_server_->routing_instance_mgr();
-    RoutingInstance *master =
-        instance_mgr->GetRoutingInstance(BgpConfigManager::kMasterInstance);
+    RoutingInstance *master = instance_mgr->GetDefaultRoutingInstance();
     assert(master);
     BgpTable *rtarget_table = master->GetTable(Address::RTARGET);
     assert(rtarget_table);
@@ -2704,7 +2698,7 @@ void BgpXmppChannelManager::IdentifierUpdateCallback(
 }
 
 void BgpXmppChannelManager::RoutingInstanceCallback(string vrf_name, int op) {
-    CHECK_CONCURRENCY("bgp::Config");
+    CHECK_CONCURRENCY("bgp::Config", "bgp::ConfigHelper");
     BOOST_FOREACH(XmppChannelMap::value_type &i, channel_map_) {
         i.second->RoutingInstanceCallback(vrf_name, op);
     }

src/bgp/routing-instance/iroute_aggregator.h

@@ -19,6 +19,7 @@ class IRouteAggregator {
     virtual void ProcessAggregateRouteConfig() = 0;
     virtual void UpdateAggregateRouteConfig() = 0;
     virtual void FlushAggregateRouteConfig() = 0;
+    virtual uint32_t GetAggregateRouteCount() const = 0;
 
     virtual bool IsAggregateRoute(const BgpRoute *route) const = 0;
     virtual bool IsContributingRoute(const BgpRoute *route) const = 0;

src/bgp/routing-instance/path_resolver.cc

@@ -125,7 +125,8 @@ Address::Family PathResolver::family() const {
 //
 void PathResolver::StartPathResolution(int part_id, const BgpPath *path,
     BgpRoute *route, BgpTable *nh_table) {
-    CHECK_CONCURRENCY("db::DBTable", "bgp::RouteAggregation", "bgp::Config");
+    CHECK_CONCURRENCY("db::DBTable", "bgp::RouteAggregation",
+        "bgp::Config", "bgp::ConfigHelper");
 
     if (!nh_table)
         nh_table = table_;
@@ -158,7 +159,8 @@ void PathResolver::UpdatePathResolution(int part_id, const BgpPath *path,
 // needed when the BgpPath changes nexthop.
 //
 void PathResolver::StopPathResolution(int part_id, const BgpPath *path) {
-    CHECK_CONCURRENCY("db::DBTable", "bgp::RouteAggregation", "bgp::Config");
+    CHECK_CONCURRENCY("db::DBTable", "bgp::RouteAggregation",
+        "bgp::Config", "bgp::ConfigHelper");
 
     partitions_[part_id]->StopPathResolution(path);
 }
@@ -211,7 +213,8 @@ PathResolverPartition *PathResolver::GetPartition(int part_id) {
 //
 ResolverNexthop *PathResolver::LocateResolverNexthop(IpAddress address,
     BgpTable *table) {
-    CHECK_CONCURRENCY("db::DBTable", "bgp::RouteAggregation", "bgp::Config");
+    CHECK_CONCURRENCY("db::DBTable", "bgp::RouteAggregation",
+        "bgp::Config", "bgp::ConfigHelper");
 
     tbb::mutex::scoped_lock lock(mutex_);
     ResolverNexthopKey key(address, table);
@@ -677,7 +680,7 @@ void PathResolverPartition::StopPathResolution(const BgpPath *path) {
 //
 void PathResolverPartition::TriggerPathResolution(ResolverPath *rpath) {
     CHECK_CONCURRENCY("db::DBTable", "bgp::ResolverNexthop",
-                      "bgp::Config", "bgp::RouteAggregation");
+        "bgp::Config", "bgp::ConfigHelper", "bgp::RouteAggregation");
 
     rpath_update_list_.insert(rpath);
     rpath_update_trigger_->Set();
@@ -1121,7 +1124,8 @@ bool ResolverNexthop::Match(BgpServer *server, BgpTable *table,
 // Do not attempt to access other partitions due to concurrency issues.
 //
 void ResolverNexthop::AddResolverPath(int part_id, ResolverPath *rpath) {
-    CHECK_CONCURRENCY("db::DBTable", "bgp::RouteAggregation", "bgp::Config");
+    CHECK_CONCURRENCY("db::DBTable", "bgp::RouteAggregation",
+        "bgp::Config", "bgp::ConfigHelper");
 
     if (rpath_lists_[part_id].empty())
         resolver_->RegisterUnregisterResolverNexthop(this);