diff --git a/src/bgp/routing-instance/static_route.cc b/src/bgp/routing-instance/static_route.cc index f777b9278a2..0b2d1416a7f 100644 --- a/src/bgp/routing-instance/static_route.cc +++ b/src/bgp/routing-instance/static_route.cc @@ -132,7 +132,8 @@ class StaticRoute : public ConditionMatch { it != rtargets.end(); ++it) { error_code ec; RouteTarget rtarget = RouteTarget::FromString(*it, &ec); - assert(ec == 0); + if (ec != 0) + continue; rtarget_list_.insert(rtarget); } @@ -208,7 +209,8 @@ StaticRoute::StaticRoute(RoutingInstance *rtinst, it != rtargets.end(); it++) { error_code ec; RouteTarget rtarget = RouteTarget::FromString(*it, &ec); - assert(ec == 0); + if (ec != 0) + continue; rtarget_list_.insert(rtarget); } } @@ -565,11 +567,9 @@ void StaticRouteMgr::LocateStaticRoutePrefix(const autogen::StaticRouteType &cfg) { CHECK_CONCURRENCY("bgp::Config"); error_code ec; - IpAddress nexthop = Ip4Address::from_string(cfg.next_hop, ec); - assert(ec == 0); - Ip4Prefix prefix = Ip4Prefix::FromString(cfg.prefix, &ec); - assert(ec == 0); + if (ec != 0) + return; BgpConditionListener *listener = routing_instance()->server()->condition_listener(); @@ -609,6 +609,10 @@ StaticRouteMgr::LocateStaticRoutePrefix(const autogen::StaticRouteType &cfg) { return; } + IpAddress nexthop = Ip4Address::from_string(cfg.next_hop, ec); + if (ec != 0) + return; + StaticRoute *match = new StaticRoute(routing_instance(), prefix, cfg.route_target, nexthop); StaticRoutePtr static_route_match = StaticRoutePtr(match); @@ -617,8 +621,6 @@ StaticRouteMgr::LocateStaticRoutePrefix(const autogen::StaticRouteType &cfg) { listener->AddMatchCondition(match->bgp_table(), static_route_match.get(), BgpConditionListener::RequestDoneCb()); - - return; } void StaticRouteMgr::StopStaticRouteDone(BgpTable *table, @@ -673,15 +675,9 @@ StaticRouteMgr::~StaticRouteMgr() { bool CompareStaticRouteConfig(autogen::StaticRouteType lhs, autogen::StaticRouteType rhs) { error_code ec; - Ip4Prefix lhs_static_route_prefix = - Ip4Prefix::FromString(lhs.prefix, &ec); - assert(*ec == 0); - - Ip4Prefix rhs_static_route_prefix = - Ip4Prefix::FromString(rhs.prefix, &ec); - assert(*ec == 0); - - return (lhs_static_route_prefix < rhs_static_route_prefix); + Ip4Prefix lhs_static_route_prefix = Ip4Prefix::FromString(lhs.prefix, &ec); + Ip4Prefix rhs_static_route_prefix = Ip4Prefix::FromString(rhs.prefix, &ec); + return (lhs_static_route_prefix < rhs_static_route_prefix); } @@ -702,7 +698,6 @@ void StaticRouteMgr::UpdateStaticRouteConfig() { error_code ec; Ip4Prefix static_route_prefix = Ip4Prefix::FromString(static_route_cfg_it->prefix, &ec); - assert(*ec == 0); if (static_route_prefix < oper_it->first) { LocateStaticRoutePrefix(*static_route_cfg_it); static_route_cfg_it++; diff --git a/src/bgp/test/static_route_test.cc b/src/bgp/test/static_route_test.cc index 8b4457dc5aa..2fed7952e4e 100644 --- a/src/bgp/test/static_route_test.cc +++ b/src/bgp/test/static_route_test.cc @@ -98,7 +98,7 @@ class BgpPeerMock : public IPeer { class StaticRouteTest : public ::testing::Test { protected: StaticRouteTest() - : bgp_server_(new BgpServer(&evm_)) { + : bgp_server_(new BgpServer(&evm_)), ri_mgr_(NULL) { IFMapLinkTable_Init(&config_db_, &config_graph_); vnc_cfg_Server_ModuleInit(&config_db_, &config_graph_); bgp_schema_Server_ModuleInit(&config_db_, &config_graph_); @@ -353,6 +353,172 @@ class StaticRouteTest : public ::testing::Test { }; int StaticRouteTest::validate_done_; + +TEST_F(StaticRouteTest, InvalidNextHop) { + vector instance_names = list_of("nat"); + multimap connections; + NetworkConfig(instance_names, connections); + task_util::WaitForIdle(); + + // Add Nexthop route. + AddInetRoute(NULL, "nat", "192.168.1.254/32", 100, "2.3.4.5"); + task_util::WaitForIdle(); + + std::auto_ptr params = + GetStaticRouteConfig("controller/src/bgp/testdata/static_route_9a.xml"); + ifmap_test_util::IFMapMsgPropertyAdd(&config_db_, "routing-instance", + "nat", "static-route-entries", params.release(), 0); + task_util::WaitForIdle(); + TASK_UTIL_WAIT_NE_NO_MSG(InetRouteLookup("nat", "192.168.1.0/24"), + NULL, 1000, 10000, + "Wait for Static route in nat instance.."); + TASK_UTIL_WAIT_NE_NO_MSG(InetRouteLookup("nat", "192.168.3.0/24"), + NULL, 1000, 10000, + "Wait for Static route in nat instance.."); + TASK_UTIL_WAIT_EQ_NO_MSG(InetRouteLookup("nat", "192.168.2.0/24"), + NULL, 1000, 10000, + "Wait for Static route in nat instance.."); + + params = + GetStaticRouteConfig("controller/src/bgp/testdata/static_route_9b.xml"); + ifmap_test_util::IFMapMsgPropertyAdd(&config_db_, "routing-instance", + "nat", "static-route-entries", params.release(), 0); + task_util::WaitForIdle(); + TASK_UTIL_WAIT_NE_NO_MSG(InetRouteLookup("nat", "192.168.2.0/24"), + NULL, 1000, 10000, + "Wait for Static route in nat instance.."); + TASK_UTIL_WAIT_NE_NO_MSG(InetRouteLookup("nat", "192.168.3.0/24"), + NULL, 1000, 10000, + "Wait for Static route in nat instance.."); + TASK_UTIL_WAIT_EQ_NO_MSG(InetRouteLookup("nat", "192.168.1.0/24"), + NULL, 1000, 10000, + "Wait for Static route in nat instance.."); + + params = + GetStaticRouteConfig("controller/src/bgp/testdata/static_route_9c.xml"); + ifmap_test_util::IFMapMsgPropertyAdd(&config_db_, "routing-instance", + "nat", "static-route-entries", params.release(), 0); + task_util::WaitForIdle(); + TASK_UTIL_WAIT_NE_NO_MSG(InetRouteLookup("nat", "192.168.1.0/24"), + NULL, 1000, 10000, + "Wait for Static route in nat instance.."); + TASK_UTIL_WAIT_NE_NO_MSG(InetRouteLookup("nat", "192.168.2.0/24"), + NULL, 1000, 10000, + "Wait for Static route in nat instance.."); + TASK_UTIL_WAIT_EQ_NO_MSG(InetRouteLookup("nat", "192.168.3.0/24"), + NULL, 1000, 10000, + "Wait for Static route in nat instance.."); + + // Delete nexthop route. + DeleteInetRoute(NULL, "nat", "192.168.1.254/32"); + task_util::WaitForIdle(); +} + +TEST_F(StaticRouteTest, InvalidPrefix) { + vector instance_names = list_of("nat"); + multimap connections; + NetworkConfig(instance_names, connections); + task_util::WaitForIdle(); + + // Add Nexthop route. + AddInetRoute(NULL, "nat", "192.168.1.254/32", 100, "2.3.4.5"); + task_util::WaitForIdle(); + + std::auto_ptr params = + GetStaticRouteConfig("controller/src/bgp/testdata/static_route_10a.xml"); + ifmap_test_util::IFMapMsgPropertyAdd(&config_db_, "routing-instance", + "nat", "static-route-entries", params.release(), 0); + task_util::WaitForIdle(); + TASK_UTIL_WAIT_NE_NO_MSG(InetRouteLookup("nat", "192.168.1.0/24"), + NULL, 1000, 10000, + "Wait for Static route in nat instance.."); + TASK_UTIL_WAIT_NE_NO_MSG(InetRouteLookup("nat", "192.168.3.0/24"), + NULL, 1000, 10000, + "Wait for Static route in nat instance.."); + + params = + GetStaticRouteConfig("controller/src/bgp/testdata/static_route_10b.xml"); + ifmap_test_util::IFMapMsgPropertyAdd(&config_db_, "routing-instance", + "nat", "static-route-entries", params.release(), 0); + task_util::WaitForIdle(); + TASK_UTIL_WAIT_NE_NO_MSG(InetRouteLookup("nat", "192.168.2.0/24"), + NULL, 1000, 10000, + "Wait for Static route in nat instance.."); + TASK_UTIL_WAIT_NE_NO_MSG(InetRouteLookup("nat", "192.168.3.0/24"), + NULL, 1000, 10000, + "Wait for Static route in nat instance.."); + + params = + GetStaticRouteConfig("controller/src/bgp/testdata/static_route_10c.xml"); + ifmap_test_util::IFMapMsgPropertyAdd(&config_db_, "routing-instance", + "nat", "static-route-entries", params.release(), 0); + task_util::WaitForIdle(); + TASK_UTIL_WAIT_NE_NO_MSG(InetRouteLookup("nat", "192.168.1.0/24"), + NULL, 1000, 10000, + "Wait for Static route in nat instance.."); + TASK_UTIL_WAIT_NE_NO_MSG(InetRouteLookup("nat", "192.168.2.0/24"), + NULL, 1000, 10000, + "Wait for Static route in nat instance.."); + + // Delete nexthop route. + DeleteInetRoute(NULL, "nat", "192.168.1.254/32"); + task_util::WaitForIdle(); +} + +TEST_F(StaticRouteTest, InvalidRouteTarget) { + vector instance_names = list_of("nat"); + multimap connections; + NetworkConfig(instance_names, connections); + task_util::WaitForIdle(); + + // Add Nexthop route. + AddInetRoute(NULL, "nat", "192.168.1.254/32", 100, "2.3.4.5"); + task_util::WaitForIdle(); + + std::auto_ptr params = + GetStaticRouteConfig("controller/src/bgp/testdata/static_route_11a.xml"); + ifmap_test_util::IFMapMsgPropertyAdd(&config_db_, "routing-instance", + "nat", "static-route-entries", params.release(), 0); + task_util::WaitForIdle(); + TASK_UTIL_WAIT_NE_NO_MSG(InetRouteLookup("nat", "192.168.1.0/24"), + NULL, 1000, 10000, + "Wait for Static route in nat instance.."); + BgpRoute *static_rt = InetRouteLookup("nat", "192.168.1.0/24"); + const BgpPath *static_path = static_rt->BestPath(); + set config_list = list_of("target:64496:1")("target:64496:3"); + TASK_UTIL_EXPECT_TRUE(GetRTargetFromPath(static_path) == config_list); + + params = + GetStaticRouteConfig("controller/src/bgp/testdata/static_route_11b.xml"); + ifmap_test_util::IFMapMsgPropertyAdd(&config_db_, "routing-instance", + "nat", "static-route-entries", params.release(), 0); + task_util::WaitForIdle(); + TASK_UTIL_WAIT_NE_NO_MSG(InetRouteLookup("nat", "192.168.1.0/24"), + NULL, 1000, 10000, + "Wait for Static route in nat instance.."); + static_rt = InetRouteLookup("nat", "192.168.1.0/24"); + static_path = static_rt->BestPath(); + config_list = list_of("target:64496:2")("target:64496:3"); + TASK_UTIL_EXPECT_TRUE(GetRTargetFromPath(static_path) == config_list); + + params = + GetStaticRouteConfig("controller/src/bgp/testdata/static_route_11c.xml"); + ifmap_test_util::IFMapMsgPropertyAdd(&config_db_, "routing-instance", + "nat", "static-route-entries", params.release(), 0); + task_util::WaitForIdle(); + TASK_UTIL_WAIT_NE_NO_MSG(InetRouteLookup("nat", "192.168.1.0/24"), + NULL, 1000, 10000, + "Wait for Static route in nat instance.."); + static_rt = InetRouteLookup("nat", "192.168.1.0/24"); + static_path = static_rt->BestPath(); + config_list = list_of("target:64496:1")("target:64496:2"); + TASK_UTIL_EXPECT_TRUE(GetRTargetFromPath(static_path) == config_list); + + // Delete nexthop route. + DeleteInetRoute(NULL, "nat", "192.168.1.254/32"); + task_util::WaitForIdle(); +} + // // Basic Test // 1. Configure routing instance with static route property diff --git a/src/bgp/testdata/static_route_10a.xml b/src/bgp/testdata/static_route_10a.xml new file mode 100644 index 00000000000..73405c42272 --- /dev/null +++ b/src/bgp/testdata/static_route_10a.xml @@ -0,0 +1,24 @@ + + + + 192.168.1.0/24 + 192.168.1.254 + target:64496:1 + target:64496:2 + target:64496:3 + + + + 192.168.1.254 + target:64496:1 + target:64496:2 + target:64496:3 + + + 192.168.3.0/24 + 192.168.1.254 + target:64496:1 + target:64496:2 + target:64496:3 + + diff --git a/src/bgp/testdata/static_route_10b.xml b/src/bgp/testdata/static_route_10b.xml new file mode 100644 index 00000000000..367697b4f82 --- /dev/null +++ b/src/bgp/testdata/static_route_10b.xml @@ -0,0 +1,24 @@ + + + + 192.168.1.0/33 + 192.168.1.254 + target:64496:1 + target:64496:2 + target:64496:3 + + + 192.168.2.0/24 + 192.168.1.254 + target:64496:1 + target:64496:2 + target:64496:3 + + + 192.168.3.0/24 + 192.168.1.254 + target:64496:1 + target:64496:2 + target:64496:3 + + diff --git a/src/bgp/testdata/static_route_10c.xml b/src/bgp/testdata/static_route_10c.xml new file mode 100644 index 00000000000..4f08f099d3d --- /dev/null +++ b/src/bgp/testdata/static_route_10c.xml @@ -0,0 +1,24 @@ + + + + 192.168.1.0/24 + 192.168.1.254 + target:64496:1 + target:64496:2 + target:64496:3 + + + 192.168.2.0/24 + 192.168.1.254 + target:64496:1 + target:64496:2 + target:64496:3 + + + 2001:abcd:abcd:abcd::/64 + 192.168.1.254 + target:64496:1 + target:64496:2 + target:64496:3 + + diff --git a/src/bgp/testdata/static_route_11a.xml b/src/bgp/testdata/static_route_11a.xml new file mode 100644 index 00000000000..b7400115f32 --- /dev/null +++ b/src/bgp/testdata/static_route_11a.xml @@ -0,0 +1,10 @@ + + + + 192.168.1.0/24 + 192.168.1.254 + target:64496:1 + xxxxxx:64496:2 + target:64496:3 + + diff --git a/src/bgp/testdata/static_route_11b.xml b/src/bgp/testdata/static_route_11b.xml new file mode 100644 index 00000000000..1ed27ebf288 --- /dev/null +++ b/src/bgp/testdata/static_route_11b.xml @@ -0,0 +1,10 @@ + + + + 192.168.1.0/24 + 192.168.1.254 + target:64496:5000000000 + target:64496:2 + target:64496:3 + + diff --git a/src/bgp/testdata/static_route_11c.xml b/src/bgp/testdata/static_route_11c.xml new file mode 100644 index 00000000000..07a5cf572ec --- /dev/null +++ b/src/bgp/testdata/static_route_11c.xml @@ -0,0 +1,10 @@ + + + + 192.168.1.0/24 + 192.168.1.254 + target:64496:1 + target:64496:2 + target:64496 + + diff --git a/src/bgp/testdata/static_route_9a.xml b/src/bgp/testdata/static_route_9a.xml new file mode 100644 index 00000000000..794e50debc2 --- /dev/null +++ b/src/bgp/testdata/static_route_9a.xml @@ -0,0 +1,24 @@ + + + + 192.168.1.0/24 + 192.168.1.254 + target:64496:1 + target:64496:2 + target:64496:3 + + + 192.168.2.0/24 + + target:64496:1 + target:64496:2 + target:64496:3 + + + 192.168.3.0/24 + 192.168.1.254 + target:64496:1 + target:64496:2 + target:64496:3 + + diff --git a/src/bgp/testdata/static_route_9b.xml b/src/bgp/testdata/static_route_9b.xml new file mode 100644 index 00000000000..2c0056d39da --- /dev/null +++ b/src/bgp/testdata/static_route_9b.xml @@ -0,0 +1,24 @@ + + + + 192.168.1.0/24 + 192.168.1.256 + target:64496:1 + target:64496:2 + target:64496:3 + + + 192.168.2.0/24 + 192.168.1.254 + target:64496:1 + target:64496:2 + target:64496:3 + + + 192.168.3.0/24 + 192.168.1.254 + target:64496:1 + target:64496:2 + target:64496:3 + + diff --git a/src/bgp/testdata/static_route_9c.xml b/src/bgp/testdata/static_route_9c.xml new file mode 100644 index 00000000000..68d8a4fa4d8 --- /dev/null +++ b/src/bgp/testdata/static_route_9c.xml @@ -0,0 +1,24 @@ + + + + 192.168.1.0/24 + 192.168.1.254 + target:64496:1 + target:64496:2 + target:64496:3 + + + 192.168.2.0/24 + 192.168.1.254 + target:64496:1 + target:64496:2 + target:64496:3 + + + 192.168.3.0/24 + 2001::2001 + target:64496:1 + target:64496:2 + target:64496:3 + +