diff --git a/src/vnsw/agent/oper/agent_route_walker.cc b/src/vnsw/agent/oper/agent_route_walker.cc index 4dd4b6ab313..d4827b1970a 100644 --- a/src/vnsw/agent/oper/agent_route_walker.cc +++ b/src/vnsw/agent/oper/agent_route_walker.cc @@ -167,7 +167,7 @@ void AgentRouteWalker::StartRouteWalk(VrfEntry *vrf) { void AgentRouteWalker::StartRouteWalkInternal(const VrfEntry *vrf) { DBTableWalker *walker = agent_->db()->GetWalker(); - DBTableWalker::WalkId walkid; + DBTableWalker::WalkId walkid = DBTableWalker::kInvalidWalkerId; uint32_t vrf_id = vrf->vrf_id(); AgentRouteTable *table = NULL; @@ -180,6 +180,13 @@ void AgentRouteWalker::StartRouteWalkInternal(const VrfEntry *vrf) { table_type++) { table = static_cast (vrf->GetRouteTable(table_type)); + if (table == NULL) { + AGENT_DBWALK_TRACE(AgentRouteWalkerTrace, + "Route table walk SKIPPED for vrf", walk_type_, + (vrf != NULL) ? vrf->GetName() : "Unknown", + vrf_walkid_, table_type, "", walkid); + continue; + } walkid = walker->WalkTable(table, NULL, boost::bind(&AgentRouteWalker::RouteWalkNotify, this, _1, _2), diff --git a/src/vnsw/agent/oper/nexthop.cc b/src/vnsw/agent/oper/nexthop.cc index 8218f6d8fe2..0f26b2cda62 100644 --- a/src/vnsw/agent/oper/nexthop.cc +++ b/src/vnsw/agent/oper/nexthop.cc @@ -836,7 +836,8 @@ bool TunnelNH::Change(const DBRequest *req) { void TunnelNH::Delete(const DBRequest *req) { InetUnicastAgentRouteTable *rt_table = (GetVrf()->GetInet4UnicastRouteTable()); - rt_table->RemoveUnresolvedNH(this); + if (rt_table) + rt_table->RemoveUnresolvedNH(this); } void TunnelNH::SendObjectLog(AgentLogEvent::type event) const { diff --git a/src/vnsw/agent/oper/vrf.cc b/src/vnsw/agent/oper/vrf.cc index 71b3532d56b..1bbd5cd9973 100644 --- a/src/vnsw/agent/oper/vrf.cc +++ b/src/vnsw/agent/oper/vrf.cc @@ -103,6 +103,33 @@ void VrfEntry::CreateTableLabel() { } } +void VrfEntry::CreateRouteTables() { + DB *db = get_table()->database(); + VrfTable *table = static_cast(get_table()); + + for (uint8_t type = (Agent::INVALID + 1); type < Agent::ROUTE_TABLE_MAX; type++) { + rt_table_db_[type] = static_cast + (db->CreateTable(name_ + + AgentRouteTable::GetSuffix(Agent::RouteTableType(type)))); + rt_table_db_[type]->SetVrf(this); + table->dbtree_[type].insert(VrfTable::VrfDbPair(name_, rt_table_db_[type])); + } +} + +void VrfEntry::DeleteRouteTables() { + for (int table_type = (Agent::INVALID + 1); + table_type < Agent::ROUTE_TABLE_MAX; table_type++) { + VrfTable *vrf_table = ((VrfTable *) get_table()); + AgentRouteTable *route_table = + vrf_table->GetRouteTable(name_, table_type); + if (route_table) { + vrf_table->DeleteFromDbTree(table_type, name_); + vrf_table->database()->RemoveTable(route_table); + delete route_table; + } + } +} + void VrfEntry::PostAdd() { VrfTable *table = static_cast(get_table()); Agent *agent = table->agent(); @@ -110,37 +137,7 @@ void VrfEntry::PostAdd() { // initialization to PostAdd deleter_.reset(new DeleteActor(this)); // Create the route-tables and insert them into dbtree_ - Agent::RouteTableType type = Agent::INET4_UNICAST; - DB *db = get_table()->database(); - rt_table_db_[type] = static_cast - (db->CreateTable(name_ + AgentRouteTable::GetSuffix(type))); - rt_table_db_[type]->SetVrf(this); - table->dbtree_[type].insert(VrfTable::VrfDbPair(name_, rt_table_db_[type])); - - type = Agent::INET4_MULTICAST; - rt_table_db_[type] = static_cast - (db->CreateTable(name_ + AgentRouteTable::GetSuffix(type))); - rt_table_db_[type]->SetVrf(this); - table->dbtree_[type].insert(VrfTable::VrfDbPair(name_, rt_table_db_[type])); - - type = Agent::EVPN; - rt_table_db_[type] = static_cast - (db->CreateTable(name_ + AgentRouteTable::GetSuffix(type))); - rt_table_db_[type]->SetVrf(this); - ((VrfTable *)get_table())->dbtree_[type].insert(VrfTable::VrfDbPair(name_, - rt_table_db_[type])); - - type = Agent::BRIDGE; - rt_table_db_[type] = static_cast - (db->CreateTable(name_ + AgentRouteTable::GetSuffix(type))); - rt_table_db_[type]->SetVrf(this); - table->dbtree_[type].insert(VrfTable::VrfDbPair(name_, rt_table_db_[type])); - - type = Agent::INET6_UNICAST; - rt_table_db_[type] = static_cast - (db->CreateTable(name_ + AgentRouteTable::GetSuffix(type))); - rt_table_db_[type]->SetVrf(this); - table->dbtree_[type].insert(VrfTable::VrfDbPair(name_, rt_table_db_[type])); + CreateRouteTables(); uint32_t vxlan_id = VxLanTable::kInvalidvxlan_id; if (vn_) { @@ -238,29 +235,27 @@ void VrfEntry::SetRouteTableDeleted(uint8_t table_type) { } AgentRouteTable *VrfEntry::GetRouteTable(uint8_t table_type) const { - return rt_table_db_[table_type]; + return (RouteTableDeleted(table_type) ? NULL : rt_table_db_[table_type]); } InetUnicastAgentRouteTable *VrfEntry::GetInet4UnicastRouteTable() const { - return static_cast - (rt_table_db_[Agent::INET4_UNICAST]); + return static_cast(GetRouteTable(Agent::INET4_UNICAST)); } AgentRouteTable *VrfEntry::GetInet4MulticastRouteTable() const { - return rt_table_db_[Agent::INET4_MULTICAST]; + return GetRouteTable(Agent::INET4_MULTICAST); } AgentRouteTable *VrfEntry::GetEvpnRouteTable() const { - return rt_table_db_[Agent::EVPN]; + return GetRouteTable(Agent::EVPN); } AgentRouteTable *VrfEntry::GetBridgeRouteTable() const { - return rt_table_db_[Agent::BRIDGE]; + return GetRouteTable(Agent::BRIDGE); } InetUnicastAgentRouteTable *VrfEntry::GetInet6UnicastRouteTable() const { - return static_cast - (rt_table_db_[Agent::INET6_UNICAST]); + return static_cast(GetRouteTable(Agent::INET6_UNICAST)); } bool VrfEntry::DBEntrySandesh(Sandesh *sresp, std::string &name) const { @@ -464,14 +459,7 @@ void VrfTable::VrfReuse(const std::string name) { void VrfTable::OnZeroRefcount(AgentDBEntry *e) { VrfEntry *vrf = static_cast(e); if (e->IsDeleted()) { - int table_type; - for (table_type = (Agent::INVALID + 1); - table_type < Agent::ROUTE_TABLE_MAX; table_type++) { - database()->RemoveTable(vrf->GetRouteTable(table_type)); - dbtree_[table_type].erase(vrf->GetName()); - delete vrf->GetRouteTable(table_type); - } - + vrf->DeleteRouteTables(); name_tree_.erase(vrf->GetName()); vrf->CancelDeleteTimer(); } @@ -589,6 +577,10 @@ void VrfTable::DeleteStaticVrf(const string &name) { DeleteVrfReq(name); } +void VrfTable::DeleteFromDbTree(int table_type, const std::string &vrf_name) { + dbtree_[table_type].erase(vrf_name); +} + void VrfTable::Input(DBTablePartition *partition, DBClient *client, DBRequest *req) { diff --git a/src/vnsw/agent/oper/vrf.h b/src/vnsw/agent/oper/vrf.h index b35f67569b2..99c351d5d74 100644 --- a/src/vnsw/agent/oper/vrf.h +++ b/src/vnsw/agent/oper/vrf.h @@ -111,12 +111,15 @@ class VrfEntry : AgentRefCount, public AgentDBEntry { InetUnicastAgentRouteTable *GetInet6UnicastRouteTable() const; AgentRouteTable *GetRouteTable(uint8_t table_type) const; void CreateTableLabel(); - bool AllRouteTableDeleted() const; bool RouteTableDeleted(uint8_t table_type) const; void SetRouteTableDeleted(uint8_t table_type); + void DeleteRouteTables(); + private: friend class VrfTable; + void CreateRouteTables(); + class DeleteActor; string name_; uint32_t id_; @@ -210,6 +213,7 @@ class VrfTable : public AgentDBTable { void DeleteRoutes(); void Shutdown(); + void DeleteFromDbTree(int table_type, const std::string &vrf_name); private: friend class VrfEntry; diff --git a/src/vnsw/agent/test/SConscript b/src/vnsw/agent/test/SConscript index 7f1238be6ab..2d69bcae767 100644 --- a/src/vnsw/agent/test/SConscript +++ b/src/vnsw/agent/test/SConscript @@ -114,7 +114,7 @@ test_global_vrouter_config = AgentEnv.MakeTestCmd(env, agent_suite) test_tunnel_encap = AgentEnv.MakeTestCmd(env, 'test_tunnel_encap', flaky_agent_suite) test_agent_route_walker = AgentEnv.MakeTestCmd(env, 'test_agent_route_walker', - flaky_agent_suite) + agent_suite) test_xmpp_hv = AgentEnv.MakeTestCmd(env, 'test_xmpp_hv', flaky_agent_suite) test_scale_walk = AgentEnv.MakeTestCmd(env, 'test_scale_walk', flaky_agent_suite) service_instance_test = AgentEnv.MakeTestCmd(env, 'service_instance_test', diff --git a/src/vnsw/agent/test/test_agent_route_walker.cc b/src/vnsw/agent/test/test_agent_route_walker.cc index f043db493ee..d2491c71936 100644 --- a/src/vnsw/agent/test/test_agent_route_walker.cc +++ b/src/vnsw/agent/test/test_agent_route_walker.cc @@ -189,6 +189,7 @@ class AgentRouteWalkerTest : public AgentRouteWalker, public ::testing::Test { AgentRouteWalker::VrfWalkDone(part); if (is_vrf_walk_done_ && !route_table_walk_started_ && + (queued_walk_done_count() == AgentRouteWalker::kInvalidWalkCount) && AreAllWalksDone()) assert(0); } @@ -250,6 +251,9 @@ class SetupTask : public Task { test_->WalkDoneCallback(boost::bind(&SetupTask::AllWalkDone, test_)); test_->StartVrfWalk(); + } else if (test_name_ == + "walk_on_deleted_vrf_with_deleted_route_table") { + test_->StartVrfWalk(); } return true; } @@ -272,7 +276,7 @@ TEST_F(AgentRouteWalkerTest, walk_all_routes_wih_1_vrf) { client->Reset(); SetupEnvironment(1); StartVrfWalk(); - VerifyNotifications(17, 2, 1, ((Agent::ROUTE_TABLE_MAX - 1) * 2)); + VerifyNotifications(19, 2, 1, ((Agent::ROUTE_TABLE_MAX - 1) * 2)); EXPECT_TRUE(walk_task_context_mismatch_ == false); walk_task_context_mismatch_ = true; DeleteEnvironment(1); @@ -282,7 +286,7 @@ TEST_F(AgentRouteWalkerTest, walk_all_routes_with_2_vrf) { client->Reset(); SetupEnvironment(2); StartVrfWalk(); - VerifyNotifications(26, 3, 1, ((Agent::ROUTE_TABLE_MAX - 1) * 3)); + VerifyNotifications(30, 3, 1, ((Agent::ROUTE_TABLE_MAX - 1) * 3)); EXPECT_TRUE(walk_task_context_mismatch_ == false); walk_task_context_mismatch_ = true; DeleteEnvironment(2); @@ -292,7 +296,7 @@ TEST_F(AgentRouteWalkerTest, walk_all_routes_with_3_vrf) { client->Reset(); SetupEnvironment(3); StartVrfWalk(); - VerifyNotifications(35, 4, 1, ((Agent::ROUTE_TABLE_MAX - 1) * 4)); + VerifyNotifications(41, 4, 1, ((Agent::ROUTE_TABLE_MAX - 1) * 4)); EXPECT_TRUE(walk_task_context_mismatch_ == false); walk_task_context_mismatch_ = true; DeleteEnvironment(3); @@ -333,6 +337,23 @@ TEST_F(AgentRouteWalkerTest, vrf_state_deleted) { DeleteEnvironment(1); } +TEST_F(AgentRouteWalkerTest, walk_on_deleted_vrf_with_deleted_route_table) { + client->Reset(); + SetupEnvironment(1); + SetupTask * task = new SetupTask(this, + "walk_on_deleted_vrf_with_deleted_route_table"); + VrfEntryRef vrf = VrfGet("vrf1"); + EXPECT_TRUE(vrf != NULL); + DeleteEnvironment(1); + EXPECT_TRUE(vrf->IsDeleted()); + Agent::GetInstance()->vrf_table()->OnZeroRefcount(vrf.get()); + TaskScheduler::GetInstance()->Enqueue(task); + WAIT_FOR(1000, 1000, IsWalkCompleted() == true); + SetupEnvironment(1); + vrf = NULL; + DeleteEnvironment(1); +} + //TODO REMAINING TESTS // - based on walktype - unicast/multicast/all //