diff --git a/src/bgp/bgp_server.cc b/src/bgp/bgp_server.cc index 90521171507..3a2df7f3489 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 boost::system::error_code; @@ -271,6 +272,7 @@ BgpServer::BgpServer(EventManager *evm) } BgpServer::~BgpServer() { + assert(srt_manager_list_.empty()); } string BgpServer::ToString() const { @@ -460,3 +462,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 6139f23f51c..1d1f00416c2 100644 --- a/src/bgp/bgp_server.h +++ b/src/bgp/bgp_server.h @@ -11,6 +11,7 @@ #include #include +#include #include #include @@ -36,12 +37,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(); @@ -137,6 +140,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; @@ -158,6 +165,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 72d6540f556..2c4c1bd13a9 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 0b2d1416a7f..33aa174a611 100644 --- a/src/bgp/routing-instance/static_route.cc +++ b/src/bgp/routing-instance/static_route.cc @@ -142,10 +142,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); @@ -159,17 +158,6 @@ class StaticRoute : public ConditionMatch { } private: - RoutingInstance *routing_instance_; - - Ip4Prefix static_route_prefix_; - IpAddress nexthop_; - BgpRoute *nexthop_route_; - NexthopPathIdList nexthop_path_ids_; - - RouteTargetList rtarget_list_; - - bool unregistered_; - // Helper function to match bool is_nexthop_route(BgpRoute *route) { InetRoute *inet_route = dynamic_cast(route); @@ -196,15 +184,25 @@ class StaticRoute : public ConditionMatch { return new_ext_community; } + RoutingInstance *routing_instance_; + Ip4Prefix static_route_prefix_; + IpAddress nexthop_; + BgpRoute *nexthop_route_; + NexthopPathIdList nexthop_path_ids_; + RouteTargetList rtarget_list_; + bool unregistered_; + DISALLOW_COPY_AND_ASSIGN(StaticRoute); }; -StaticRoute::StaticRoute(RoutingInstance *rtinst, - const Ip4Prefix &static_route, const vector &rtargets, - IpAddress nexthop) - : routing_instance_(rtinst), static_route_prefix_(static_route), - nexthop_(nexthop), nexthop_route_(NULL), unregistered_(false) { +StaticRoute::StaticRoute(RoutingInstance *rtinst, const Ip4Prefix &static_route, + const vector &rtargets, IpAddress nexthop) + : routing_instance_(rtinst), + static_route_prefix_(static_route), + nexthop_(nexthop), + nexthop_route_(NULL), + unregistered_(false) { for (vector::const_iterator it = rtargets.begin(); it != rtargets.end(); it++) { error_code ec; @@ -462,6 +460,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) @@ -530,6 +538,8 @@ bool StaticRouteMgr::StaticRouteEventCallback(StaticRouteRequest *req) { if (!info->num_matchstate()) { listener->UnregisterCondition(table, info); static_route_map_.erase(info->static_route_prefix()); + if (static_route_map_.empty()) + instance_->server()->RemoveStaticRouteMgr(this); if (!routing_instance()->deleted() && routing_instance()->config() && routing_instance()->config()->instance_config()) @@ -551,6 +561,8 @@ bool StaticRouteMgr::StaticRouteEventCallback(StaticRouteRequest *req) { if (!info->num_matchstate() && info->unregistered()) { listener->UnregisterCondition(table, info); static_route_map_.erase(info->static_route_prefix()); + if (static_route_map_.empty()) + instance_->server()->RemoveStaticRouteMgr(this); if (!routing_instance()->deleted() && routing_instance()->config() && routing_instance()->config()->instance_config()) @@ -617,6 +629,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(make_pair(prefix, static_route_match)); listener->AddMatchCondition(match->bgp_table(), static_route_match.get(), @@ -728,6 +742,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(vector *list, diff --git a/src/bgp/routing-instance/static_route.h b/src/bgp/routing-instance/static_route.h index f41cfea733a..5393368fd65 100644 --- a/src/bgp/routing-instance/static_route.h +++ b/src/bgp/routing-instance/static_route.h @@ -61,6 +61,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 2fed7952e4e..56c765b2ec8 100644 --- a/src/bgp/test/static_route_test.cc +++ b/src/bgp/test/static_route_test.cc @@ -343,6 +343,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_; @@ -1504,6 +1509,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