diff --git a/src/bgp/routing-instance/route_aggregator.cc b/src/bgp/routing-instance/route_aggregator.cc index 83474ba975e..c76e3c5276b 100644 --- a/src/bgp/routing-instance/route_aggregator.cc +++ b/src/bgp/routing-instance/route_aggregator.cc @@ -289,7 +289,7 @@ bool AggregateRoute::IsBestMatch(BgpRoute *route) { std::set prefix_list; for (it = manager_->aggregate_route_map().begin(); it != manager_->aggregate_route_map().end(); ++it) { - if (ip_prefix != it->first && + if (!it->second->deleted() && ip_prefix != it->first && ip_prefix.IsMoreSpecific(it->first)) { prefix_list.insert(it->first); } diff --git a/src/bgp/test/route_aggregator_test.cc b/src/bgp/test/route_aggregator_test.cc index cc5fffbff55..851a6b33782 100644 --- a/src/bgp/test/route_aggregator_test.cc +++ b/src/bgp/test/route_aggregator_test.cc @@ -1545,6 +1545,103 @@ TEST_F(RouteAggregatorTest, ConfigUpdate_RemoveOverlappingPrefixes) { task_util::WaitForIdle(); } +// +// With multiple route aggregate config object linked to the routing instance, +// update the config to remove longer prefix +// RA-1: +// 9.0.1.0/24 +// RA-2: +// 9.0.0.0/8 +// 21.1.1.0/24 +// 21.1.0.0/16 +// 21.0.0.0/8 +// Step 1: Attach RA-1 to routing instance +// Step 2: Attach RA-2 to routing instance. Now routing instance has both 9.0/16 +// and 9/8 route aggregate prefix +// Step 3: Remove RA-1. Routing instance is only left with 9/8. +// Ensure valid route aggregate objects after config update +// +TEST_F(RouteAggregatorTest, ConfigUpdate_OverlappingPrefixes_1) { + string content = + FileRead("controller/src/bgp/testdata/route_aggregate_4c.xml"); + EXPECT_TRUE(parser_.Parse(content)); + task_util::WaitForIdle(); + + boost::system::error_code ec; + peers_.push_back( + new BgpPeerMock(Ip4Address::from_string("192.168.0.1", ec))); + + AddRoute(peers_[0], "test.inet.0", "2.2.2.1/32", 100); + AddRoute(peers_[0], "test.inet.0", "9.0.1.1/32", 100); + AddRoute(peers_[0], "test.inet.0", "9.0.1.10/32", 100); + AddRoute(peers_[0], "test.inet.0", "21.1.1.3/32", 100); + AddRoute(peers_[0], "test.inet.0", "21.1.1.4/32", 100); + AddRoute(peers_[0], "test.inet.0", "21.1.1.5/32", 100); + AddRoute(peers_[0], "test.inet.0", "21.1.1.0/24", 100); + task_util::WaitForIdle(); + + // Link the route aggregate config vn_subnet_1 to test + ifmap_test_util::IFMapMsgLink(&config_db_, "routing-instance", "test", + "route-aggregate", "vn_subnet_1", "route-aggregate-routing-instance"); + task_util::WaitForIdle(); + + VERIFY_EQ(8, RouteCount("test.inet.0")); + BgpRoute *rt = RouteLookup("test.inet.0", "9.0.1.0/24"); + ASSERT_TRUE(rt != NULL); + TASK_UTIL_EXPECT_EQ(rt->count(), 2); + TASK_UTIL_EXPECT_TRUE(rt->BestPath() != NULL); + TASK_UTIL_EXPECT_TRUE(rt->BestPath()->IsFeasible()); + + VerifyRouteAggregateSandesh("test"); + + // Link the route aggregate config vn_subnet_2 to test + ifmap_test_util::IFMapMsgLink(&config_db_, "routing-instance", "test", + "route-aggregate", "vn_subnet_2", "route-aggregate-routing-instance"); + task_util::WaitForIdle(); + + + VERIFY_EQ(11, RouteCount("test.inet.0")); + rt = RouteLookup("test.inet.0", "9.0.1.0/24"); + ASSERT_TRUE(rt != NULL); + TASK_UTIL_EXPECT_EQ(rt->count(), 2); + TASK_UTIL_EXPECT_TRUE(rt->BestPath() != NULL); + TASK_UTIL_EXPECT_TRUE(rt->BestPath()->IsFeasible()); + rt = RouteLookup("test.inet.0", "9.0.0.0/8"); + ASSERT_TRUE(rt != NULL); + TASK_UTIL_EXPECT_EQ(rt->count(), 2); + TASK_UTIL_EXPECT_TRUE(rt->BestPath() != NULL); + TASK_UTIL_EXPECT_TRUE(rt->BestPath()->IsFeasible()); + + // Verify the sandesh + VerifyRouteAggregateSandesh("test"); + + + // Unlink the route aggregate config vn_subnet_1 from test + ifmap_test_util::IFMapMsgUnlink(&config_db_, "routing-instance", "test", + "route-aggregate", "vn_subnet_1", "route-aggregate-routing-instance"); + task_util::WaitForIdle(); + + VERIFY_EQ(10, RouteCount("test.inet.0")); + rt = RouteLookup("test.inet.0", "9.0.1.0/24"); + ASSERT_TRUE(rt == NULL); + rt = RouteLookup("test.inet.0", "9.0.0.0/8"); + ASSERT_TRUE(rt != NULL); + TASK_UTIL_EXPECT_EQ(rt->count(), 2); + TASK_UTIL_EXPECT_TRUE(rt->BestPath() != NULL); + TASK_UTIL_EXPECT_TRUE(rt->BestPath()->IsFeasible()); + // Verify the sandesh + VerifyRouteAggregateSandesh("test"); + + DeleteRoute(peers_[0], "test.inet.0", "2.2.2.1/32"); + DeleteRoute(peers_[0], "test.inet.0", "9.0.1.1/32"); + DeleteRoute(peers_[0], "test.inet.0", "9.0.1.10/32"); + DeleteRoute(peers_[0], "test.inet.0", "21.1.1.3/32"); + DeleteRoute(peers_[0], "test.inet.0", "21.1.1.4/32"); + DeleteRoute(peers_[0], "test.inet.0", "21.1.1.5/32"); + DeleteRoute(peers_[0], "test.inet.0", "21.1.1.0/24"); + task_util::WaitForIdle(); +} + // // Update the config to update the prefix len of route-aggregate // Higher prefix len to lower diff --git a/src/bgp/testdata/route_aggregate_4c.xml b/src/bgp/testdata/route_aggregate_4c.xml new file mode 100644 index 00000000000..a948789120c --- /dev/null +++ b/src/bgp/testdata/route_aggregate_4c.xml @@ -0,0 +1,23 @@ + + + + + 9.0.1.0/24 + + 2.2.2.1 + + + + + 9.0.0.0/8 + 21.1.1.0/24 + 21.1.0.0/16 + 21.0.0.0/8 + + 2.2.2.1 + + + + target:1:103 + +