diff --git a/src/bgp/bgp_server.cc b/src/bgp/bgp_server.cc index a55040000cc..1337f5b416a 100644 --- a/src/bgp/bgp_server.cc +++ b/src/bgp/bgp_server.cc @@ -23,6 +23,7 @@ #include "bgp/routing-instance/routepath_replicator.h" #include "bgp/routing-instance/rtarget_group_mgr.h" #include "bgp/routing-instance/service_chaining.h" +#include "bgp/routing-instance/static_route.h" #include "io/event_manager.h" using namespace std; @@ -246,6 +247,7 @@ BgpServer::BgpServer(EventManager *evm) } BgpServer::~BgpServer() { + assert(srt_manager_list_.empty()); } string BgpServer::ToString() const { @@ -438,3 +440,22 @@ void BgpServer::NotifyIdentifierUpdate(Ip4Address old_identifier) { } } } + +void BgpServer::InsertStaticRouteMgr(StaticRouteMgr *srt_manager) { + CHECK_CONCURRENCY("bgp::Config"); + srt_manager_list_.insert(srt_manager); +} + +void BgpServer::RemoveStaticRouteMgr(StaticRouteMgr *srt_manager) { + CHECK_CONCURRENCY("bgp::StaticRoute"); + srt_manager_list_.erase(srt_manager); +} + +void BgpServer::NotifyAllStaticRoutes() { + CHECK_CONCURRENCY("bgp::Config"); + for (StaticRouteMgrList::iterator it = srt_manager_list_.begin(); + it != srt_manager_list_.end(); ++it) { + StaticRouteMgr *srt_manager = *it; + srt_manager->NotifyAllRoutes(); + } +} diff --git a/src/bgp/bgp_server.h b/src/bgp/bgp_server.h index 4870b620978..2582d21d7e4 100644 --- a/src/bgp/bgp_server.h +++ b/src/bgp/bgp_server.h @@ -33,12 +33,14 @@ class RoutingInstanceMgr; class RTargetGroupMgr; class SchedulingGroupManager; class ServiceChainMgr; +class StaticRouteMgr; class BgpServer { public: typedef boost::function ASNUpdateCb; typedef boost::function IdentifierUpdateCb; typedef boost::function VisitorFn; + typedef std::set StaticRouteMgrList; explicit BgpServer(EventManager *evm); virtual ~BgpServer(); @@ -133,6 +135,10 @@ class BgpServer { void UnregisterIdentifierUpdateCallback(int listener); void NotifyIdentifierUpdate(Ip4Address old_identifier); + void InsertStaticRouteMgr(StaticRouteMgr *srt_manager); + void RemoveStaticRouteMgr(StaticRouteMgr *srt_manager); + void NotifyAllStaticRoutes(); + private: class ConfigUpdater; class DeleteActor; @@ -153,6 +159,7 @@ class BgpServer { IdentifierUpdateListenersList id_listeners_; boost::dynamic_bitset<> id_bmap_; // free list. uint16_t hold_time_; + StaticRouteMgrList srt_manager_list_; DB db_; boost::dynamic_bitset<> peer_bmap_; diff --git a/src/bgp/routing-instance/routepath_replicator.cc b/src/bgp/routing-instance/routepath_replicator.cc index a6de15a5cdf..a46651ea6d4 100644 --- a/src/bgp/routing-instance/routepath_replicator.cc +++ b/src/bgp/routing-instance/routepath_replicator.cc @@ -200,6 +200,8 @@ void RoutePathReplicator::Join(BgpTable *table, const RouteTarget &rt, RPR_TRACE(TableJoin, table->name(), rt.ToString(), import); if (import) { + if (family_ == Address::INETVPN) + server_->NotifyAllStaticRoutes(); BOOST_FOREACH(BgpTable *bgptable, group->GetExportTables(family())) { RequestWalk(bgptable); } @@ -236,6 +238,8 @@ void RoutePathReplicator::Leave(BgpTable *table, const RouteTarget &rt, if (import) { group->RemoveImportTable(family(), table); + if (family_ == Address::INETVPN) + server_->NotifyAllStaticRoutes(); BOOST_FOREACH(BgpTable *bgptable, group->GetExportTables(family())) { RequestWalk(bgptable); } diff --git a/src/bgp/routing-instance/static_route.cc b/src/bgp/routing-instance/static_route.cc index 468259dcbc4..3c4d6a79210 100644 --- a/src/bgp/routing-instance/static_route.cc +++ b/src/bgp/routing-instance/static_route.cc @@ -133,10 +133,9 @@ class StaticRoute : public ConditionMatch { } void AddStaticRoute(NexthopPathIdList *list); - void UpdateStaticRoute(); - void RemoveStaticRoute(); + void NotifyRoute(); virtual bool Match(BgpServer *server, BgpTable *table, BgpRoute *route, bool deleted); @@ -452,6 +451,16 @@ StaticRoute::AddStaticRoute(NexthopPathIdList *old_path_ids) { } } +void StaticRoute::NotifyRoute() { + InetRoute rt_key(static_route_prefix()); + DBTablePartition *partition = + static_cast(bgp_table()->GetTablePartition(&rt_key)); + BgpRoute *static_route = static_cast(partition->Find(&rt_key)); + if (!static_route) + return; + partition->Notify(static_route); +} + int StaticRouteMgr::static_route_task_id_ = -1; StaticRouteMgr::StaticRouteMgr(RoutingInstance *instance) @@ -520,7 +529,9 @@ bool StaticRouteMgr::StaticRouteEventCallback(StaticRouteRequest *req) { if (!info->num_matchstate()) { listener->UnregisterCondition(table, info); static_route_map_.erase(info->static_route_prefix()); - if (!routing_instance()->deleted() && + if (static_route_map_.empty()) + instance_->server()->RemoveStaticRouteMgr(this); + if (!routing_instance()->deleted() && routing_instance()->config() && routing_instance()->config()->instance_config()) resolve_trigger_->Set(); @@ -541,7 +552,9 @@ bool StaticRouteMgr::StaticRouteEventCallback(StaticRouteRequest *req) { if (!info->num_matchstate() && info->unregistered()) { listener->UnregisterCondition(table, info); static_route_map_.erase(info->static_route_prefix()); - if (!routing_instance()->deleted() && + if (static_route_map_.empty()) + instance_->server()->RemoveStaticRouteMgr(this); + if (!routing_instance()->deleted() && routing_instance()->config() && routing_instance()->config()->instance_config()) resolve_trigger_->Set(); @@ -608,6 +621,8 @@ StaticRouteMgr::LocateStaticRoutePrefix(const autogen::StaticRouteType &cfg) { new StaticRoute(routing_instance(), prefix, cfg.route_target, nexthop); StaticRoutePtr static_route_match = StaticRoutePtr(match); + if (static_route_map_.empty()) + instance_->server()->InsertStaticRouteMgr(this); static_route_map_.insert(std::make_pair(prefix, static_route_match)); listener->AddMatchCondition(match->bgp_table(), static_route_match.get(), @@ -719,6 +734,16 @@ void StaticRouteMgr::FlushStaticRouteConfig() { } } +void StaticRouteMgr::NotifyAllRoutes() { + CHECK_CONCURRENCY("bgp::Config"); + for (StaticRouteMap::iterator it = static_route_map_.begin(); + it != static_route_map_.end(); ++it) { + StaticRoute *static_route = + static_cast(it->second.get()); + static_route->NotifyRoute(); + } +} + class ShowStaticRouteHandler { public: static void FillStaticRoutesInfo(std::vector &list, diff --git a/src/bgp/routing-instance/static_route.h b/src/bgp/routing-instance/static_route.h index 07621576883..579c65bd9d0 100644 --- a/src/bgp/routing-instance/static_route.h +++ b/src/bgp/routing-instance/static_route.h @@ -57,6 +57,7 @@ class StaticRouteMgr { bool StaticRouteEventCallback(StaticRouteRequest *req); bool ResolvePendingStaticRouteConfig(); + void NotifyAllRoutes(); RoutingInstance *routing_instance() { return instance_; } diff --git a/src/bgp/test/static_route_test.cc b/src/bgp/test/static_route_test.cc index 684654709a8..16ae7ba70e6 100644 --- a/src/bgp/test/static_route_test.cc +++ b/src/bgp/test/static_route_test.cc @@ -340,6 +340,11 @@ class StaticRouteTest : public ::testing::Test { TASK_UTIL_EXPECT_EQ(1, validate_done_); } + void VerifyTableNoExists(const string &table_name) { + TASK_UTIL_EXPECT_TRUE( + bgp_server_->database()->FindTable(table_name) == NULL); + } + EventManager evm_; DB config_db_; DBGraph config_graph_; @@ -1501,6 +1506,99 @@ TEST_F(StaticRouteTest, EntryAfterStop) { "Wait for Static route in blue.."); } +// +// Delete the routing instance that imports the static route and make sure +// the inet table gets deleted. Objective is to check that the static route +// is removed from the table even though the static route config has not +// changed. +// +TEST_F(StaticRouteTest, DeleteRoutingInstance) { + vector instance_names = list_of("blue")("nat"); + multimap connections; + NetworkConfig(instance_names, connections); + task_util::WaitForIdle(); + + std::auto_ptr params = + GetStaticRouteConfig("controller/src/bgp/testdata/static_route_1.xml"); + + ifmap_test_util::IFMapMsgPropertyAdd(&config_db_, "routing-instance", + "nat", "static-route-entries", params.release(), 0); + task_util::WaitForIdle(); + + // Add Nexthop Route + AddInetRoute(NULL, "nat", "192.168.1.254/32", 100, "2.3.4.5"); + task_util::WaitForIdle(); + + // Check for Static route + TASK_UTIL_WAIT_NE_NO_MSG(InetRouteLookup("blue", "192.168.1.0/24"), + NULL, 1000, 10000, + "Wait for Static route in blue.."); + + // Delete the configuration for the blue instance. + ifmap_test_util::IFMapMsgUnlink(&config_db_, "routing-instance", "blue", + "virtual-network", "blue", "virtual-network-routing-instance"); + ifmap_test_util::IFMapMsgUnlink(&config_db_, "routing-instance", "blue", + "route-target", "target:64496:1", "instance-target"); + ifmap_test_util::IFMapMsgNodeDelete( + &config_db_, "virtual-network", "blue"); + ifmap_test_util::IFMapMsgNodeDelete( + &config_db_, "routing-instance", "blue"); + ifmap_test_util::IFMapMsgNodeDelete( + &config_db_, "route-target", "target:64496:1"); + task_util::WaitForIdle(); + + // Make sure that the blue inet table is gone. + VerifyTableNoExists("blue.inet.0"); + + // Delete nexthop route + DeleteInetRoute(NULL, "nat", "192.168.1.254/32"); + task_util::WaitForIdle(); +} + +// +// Add the routing instance that imports the static route after the static +// route has already been added. Objective is to check that the static route +// is replicated to the table in the new instance without any triggers to +// the static route module. +// +TEST_F(StaticRouteTest, AddRoutingInstance) { + vector instance_names = list_of("nat"); + multimap connections; + NetworkConfig(instance_names, connections); + task_util::WaitForIdle(); + + std::auto_ptr params = + GetStaticRouteConfig("controller/src/bgp/testdata/static_route_1.xml"); + + ifmap_test_util::IFMapMsgPropertyAdd(&config_db_, "routing-instance", + "nat", "static-route-entries", params.release(), 0); + task_util::WaitForIdle(); + + // Add Nexthop Route + AddInetRoute(NULL, "nat", "192.168.1.254/32", 100, "2.3.4.5"); + task_util::WaitForIdle(); + + // Check for Static route + TASK_UTIL_WAIT_NE_NO_MSG(InetRouteLookup("nat", "192.168.1.0/24"), + NULL, 1000, 10000, + "Wait for Static route in nat.."); + + // Add the blue instance. + // Make sure that the id and route target for nat instance don't change. + instance_names = list_of("nat")("blue"); + NetworkConfig(instance_names, connections); + task_util::WaitForIdle(); + + // Check for Static route + TASK_UTIL_WAIT_NE_NO_MSG(InetRouteLookup("blue", "192.168.1.0/24"), + NULL, 1000, 10000, + "Wait for Static route in blue.."); + + // Delete nexthop route + DeleteInetRoute(NULL, "nat", "192.168.1.254/32"); + task_util::WaitForIdle(); +} + // Sandesh introspect test // Verify http introspect output // 1. After creating the config and before nexthop route is published