diff --git a/src/bgp/routing-instance/istatic_route_mgr.h b/src/bgp/routing-instance/istatic_route_mgr.h index be2dcafb66c..300876a1661 100644 --- a/src/bgp/routing-instance/istatic_route_mgr.h +++ b/src/bgp/routing-instance/istatic_route_mgr.h @@ -26,6 +26,9 @@ class IStaticRouteMgr { private: template friend class StaticRouteTest; + virtual void DisableResolveTrigger() = 0; + virtual void EnableResolveTrigger() = 0; + virtual void DisableQueue() = 0; virtual void EnableQueue() = 0; virtual bool IsQueueEmpty() = 0; diff --git a/src/bgp/routing-instance/static_route.cc b/src/bgp/routing-instance/static_route.cc index 87d9fefdf4b..dfe3034d886 100644 --- a/src/bgp/routing-instance/static_route.cc +++ b/src/bgp/routing-instance/static_route.cc @@ -738,6 +738,7 @@ void StaticRouteMgr::RemoveStaticRoutePrefix(const PrefixT &static_route) { template void StaticRouteMgr::ProcessStaticRouteConfig() { CHECK_CONCURRENCY("bgp::Config"); + if (routing_instance()->deleted() || !routing_instance()->config()) return; const BgpInstanceConfig::StaticRouteList &list = routing_instance()->config()->static_routes(GetFamily()); typedef BgpInstanceConfig::StaticRouteList::const_iterator iterator_t; @@ -754,6 +755,7 @@ bool StaticRouteMgr::ResolvePendingStaticRouteConfig() { template StaticRouteMgr::~StaticRouteMgr() { + resolve_trigger_->Reset(); if (static_route_queue_) delete static_route_queue_; } diff --git a/src/bgp/routing-instance/static_route.h b/src/bgp/routing-instance/static_route.h index 40433d8568c..0234fb68c2f 100644 --- a/src/bgp/routing-instance/static_route.h +++ b/src/bgp/routing-instance/static_route.h @@ -104,6 +104,9 @@ class StaticRouteMgr : public IStaticRouteMgr { bool ResolvePendingStaticRouteConfig(); bool StaticRouteEventCallback(StaticRouteRequest *req); + virtual void DisableResolveTrigger() { resolve_trigger_->set_disable(); } + virtual void EnableResolveTrigger() { resolve_trigger_->set_enable(); } + virtual void DisableQueue() { static_route_queue_->set_disable(true); } virtual void EnableQueue() { static_route_queue_->set_disable(false); } virtual bool IsQueueEmpty() { return static_route_queue_->IsQueueEmpty(); } diff --git a/src/bgp/test/static_route_test.cc b/src/bgp/test/static_route_test.cc index e26b52115bb..29eb3d34fc6 100644 --- a/src/bgp/test/static_route_test.cc +++ b/src/bgp/test/static_route_test.cc @@ -213,6 +213,19 @@ class StaticRouteTest : public ::testing::Test { parser->Receive(&config_db_, netconf.data(), netconf.length(), 0); } + void DisableResolveTrigger(const string &instance_name) { + RoutingInstance *rtinstance = + ri_mgr_->GetRoutingInstance(instance_name); + rtinstance->static_route_mgr(family_)->DisableResolveTrigger(); + } + + void EnableResolveTrigger(const string &instance_name) { + RoutingInstance *rtinstance = + ri_mgr_->GetRoutingInstance(instance_name); + if (rtinstance) + rtinstance->static_route_mgr(family_)->EnableResolveTrigger(); + } + void DisableStaticRouteQ(const string &instance_name) { RoutingInstance *rtinstance = ri_mgr_->GetRoutingInstance(instance_name); @@ -2003,6 +2016,139 @@ TYPED_TEST(StaticRouteTest, DeleteRoutingInstance) { task_util::WaitForIdle(); } +// +// Delete the static route config and instance with resolve_trigger disabled +// Allow the routing instance to get deleted with Resolve trigger +// +TYPED_TEST(StaticRouteTest, DeleteRoutingInstance_DisabledResolveTrigger) { + vector instance_names = list_of("blue")("nat"); + multimap connections; + this->NetworkConfig(instance_names, connections); + task_util::WaitForIdle(); + + std::auto_ptr params = + this->GetStaticRouteConfig( + "controller/src/bgp/testdata/static_route_1.xml"); + + ifmap_test_util::IFMapMsgPropertyAdd(&this->config_db_, "routing-instance", + "nat", "static-route-entries", params.release(), 0); + task_util::WaitForIdle(); + + // Add Nexthop Route + this->AddRoute(NULL, "nat", this->BuildPrefix("192.168.1.254", 32), + 100, this->BuildNextHopAddress("2.3.4.5")); + task_util::WaitForIdle(); + + // Check for Static route + TASK_UTIL_WAIT_NE_NO_MSG( + this->RouteLookup("blue", this->BuildPrefix("192.168.1.0", 24)), + NULL, 1000, 10000, "Wait for Static route in blue.."); + + // Disable resolve trigger + this->DisableResolveTrigger("nat"); + + // Delete the configuration for the nat instance. + ifmap_test_util::IFMapMsgPropertyDelete( + &this->config_db_, "routing-instance", + "nat", "static-route-entries"); + + // Delete nexthop route + this->DeleteRoute(NULL, "nat", this->BuildPrefix("192.168.1.254", 32)); + task_util::WaitForIdle(); + + // Check for Static route + TASK_UTIL_WAIT_EQ_NO_MSG( + this->RouteLookup("blue", this->BuildPrefix("192.168.1.0", 24)), + NULL, 1000, 10000, "Wait for Static route in blue.."); + + ifmap_test_util::IFMapMsgUnlink( + &this->config_db_, "routing-instance", "nat", + "virtual-network", "nat", "virtual-network-routing-instance"); + ifmap_test_util::IFMapMsgUnlink(&this->config_db_, "routing-instance", + "nat", "route-target", "target:64496:2", "instance-target"); + ifmap_test_util::IFMapMsgNodeDelete( + &this->config_db_, "virtual-network", "nat"); + ifmap_test_util::IFMapMsgNodeDelete( + &this->config_db_, "routing-instance", "nat"); + ifmap_test_util::IFMapMsgNodeDelete( + &this->config_db_, "route-target", "target:64496:2"); + task_util::WaitForIdle(); + + this->EnableResolveTrigger("nat"); +} + +// +// Delete the static route config and instance with resolve_trigger disabled +// Routing instance is not destroyed when the task trigger is enabled. +// Verify that enabling the task trigger ignores the deleted routing instance +// +TYPED_TEST(StaticRouteTest, DeleteRoutingInstance_DisabledResolveTrigger_1) { + vector instance_names = list_of("blue")("nat"); + multimap connections; + this->NetworkConfig(instance_names, connections); + task_util::WaitForIdle(); + + std::auto_ptr params = + this->GetStaticRouteConfig( + "controller/src/bgp/testdata/static_route_1.xml"); + + ifmap_test_util::IFMapMsgPropertyAdd(&this->config_db_, "routing-instance", + "nat", "static-route-entries", params.release(), 0); + task_util::WaitForIdle(); + + // Add Nexthop Route + this->AddRoute(NULL, "nat", this->BuildPrefix("192.168.1.254", 32), + 100, this->BuildNextHopAddress("2.3.4.5")); + task_util::WaitForIdle(); + + // Check for Static route + TASK_UTIL_WAIT_NE_NO_MSG( + this->RouteLookup("blue", this->BuildPrefix("192.168.1.0", 24)), + NULL, 1000, 10000, "Wait for Static route in blue.."); + + // Disable resolve trigger + this->DisableResolveTrigger("nat"); + + // Delete the configuration for the nat instance. + ifmap_test_util::IFMapMsgPropertyDelete( + &this->config_db_, "routing-instance", + "nat", "static-route-entries"); + + // Check for Static route + TASK_UTIL_WAIT_EQ_NO_MSG( + this->RouteLookup("blue", this->BuildPrefix("192.168.1.0", 24)), + NULL, 1000, 10000, "Wait for Static route in blue.."); + + // Delete the nat routing instance + ifmap_test_util::IFMapMsgUnlink( + &this->config_db_, "routing-instance", "nat", + "virtual-network", "nat", "virtual-network-routing-instance"); + ifmap_test_util::IFMapMsgUnlink(&this->config_db_, "routing-instance", + "nat", "route-target", "target:64496:2", "instance-target"); + ifmap_test_util::IFMapMsgNodeDelete( + &this->config_db_, "virtual-network", "nat"); + ifmap_test_util::IFMapMsgNodeDelete( + &this->config_db_, "routing-instance", "nat"); + ifmap_test_util::IFMapMsgNodeDelete( + &this->config_db_, "route-target", "target:64496:2"); + task_util::WaitForIdle(); + + RoutingInstance *nat_inst = this->ri_mgr_->GetRoutingInstance("nat"); + TASK_UTIL_WAIT_EQ_NO_MSG(nat_inst->deleted(), + true, 1000, 10000, "Wait for nat instance to be marked deleted"); + // + // Since the nexthop route is not yet deleted, routing instance is + // not destroyed + // + this->EnableResolveTrigger("nat"); + + // Delete nexthop route + this->DeleteRoute(NULL, "nat", this->BuildPrefix("192.168.1.254", 32)); + task_util::WaitForIdle(); + TASK_UTIL_WAIT_EQ_NO_MSG(this->ri_mgr_->GetRoutingInstance("nat"), + NULL, 1000, 10000, "Wait for nat instance to get destroyed"); +} + // // 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