diff --git a/src/bgp/bgp_config_ifmap.cc b/src/bgp/bgp_config_ifmap.cc index 69755f88f11..7418a2bdc6a 100644 --- a/src/bgp/bgp_config_ifmap.cc +++ b/src/bgp/bgp_config_ifmap.cc @@ -154,11 +154,22 @@ static bool AddressFamilyIsValid(BgpNeighborConfig *neighbor, // that represent each family with a simple string. // static void BuildFamilyAttributesList(BgpNeighborConfig *neighbor, - const BgpNeighborConfig::AddressFamilyList &family_list) { + const BgpNeighborConfig::AddressFamilyList &family_list, + const vector &remote_family_list) { BgpNeighborConfig::FamilyAttributesList family_attributes_list; BOOST_FOREACH(const string &family, family_list) { + // Skip families that are not valid/supported for the neighbor. if (!AddressFamilyIsValid(neighbor, family)) continue; + + // Skip families that are not configured on remote bgp-router. + if (!remote_family_list.empty()) { + vector::const_iterator it = find( + remote_family_list.begin(), remote_family_list.end(), family); + if (it == remote_family_list.end()) + continue; + } + BgpFamilyAttributesConfig family_attributes(family); family_attributes_list.push_back(family_attributes); } @@ -309,19 +320,20 @@ static BgpNeighborConfig *MakeBgpNeighborConfig( const BgpIfmapProtocolConfig *master_protocol = master_instance->protocol_config(); if (master_protocol && master_protocol->bgp_router()) { - const autogen::BgpRouterParams ¶ms = + const autogen::BgpRouterParams &master_params = master_protocol->router_params(); - if (params.admin_down) { + if (master_params.admin_down) { neighbor->set_admin_down(true); } - Ip4Address localid = Ip4Address::from_string(params.identifier, err); + Ip4Address localid = + Ip4Address::from_string(master_params.identifier, err); if (err == 0) { neighbor->set_local_identifier(IpAddressToBgpIdentifier(localid)); } - if (params.local_autonomous_system) { - neighbor->set_local_as(params.local_autonomous_system); + if (master_params.local_autonomous_system) { + neighbor->set_local_as(master_params.local_autonomous_system); } else { - neighbor->set_local_as(params.autonomous_system); + neighbor->set_local_as(master_params.autonomous_system); } if (instance != master_instance) { neighbor->set_passive(true); @@ -332,18 +344,23 @@ static BgpNeighborConfig *MakeBgpNeighborConfig( // Note that there's no instance protocol config for non-master instances. const BgpIfmapProtocolConfig *protocol = instance->protocol_config(); if (protocol && protocol->bgp_router()) { - const autogen::BgpRouterParams ¶ms = protocol->router_params(); + const autogen::BgpRouterParams &protocol_params = + protocol->router_params(); if (neighbor->family_attributes_list().empty()) { - BuildFamilyAttributesList(neighbor, params.address_families.family); + BuildFamilyAttributesList(neighbor, + protocol_params.address_families.family, + params.address_families.family); } if (neighbor->auth_data().Empty()) { - const autogen::BgpRouterParams &lp = local_router->parameters(); - BuildKeyChain(neighbor, lp.auth_data); + const autogen::BgpRouterParams &local_params = + local_router->parameters(); + BuildKeyChain(neighbor, local_params.auth_data); } } if (neighbor->family_attributes_list().empty()) { - BuildFamilyAttributesList(neighbor, default_addr_family_list); + BuildFamilyAttributesList(neighbor, default_addr_family_list, + params.address_families.family); } return neighbor; diff --git a/src/bgp/test/bgp_config_test.cc b/src/bgp/test/bgp_config_test.cc index 98206e3c8fc..1f0975244ed 100644 --- a/src/bgp/test/bgp_config_test.cc +++ b/src/bgp/test/bgp_config_test.cc @@ -1705,7 +1705,7 @@ TEST_F(BgpConfigTest, AddressFamilies1) { // The address-families config for the local bgp-router should be used when // there's no per session address-families configuration. // -TEST_F(BgpConfigTest, AddressFamilies2) { +TEST_F(BgpConfigTest, AddressFamilies2a) { string content = FileRead("controller/src/bgp/testdata/config_test_13.xml"); EXPECT_TRUE(parser_.Parse(content)); task_util::WaitForIdle(); @@ -1729,11 +1729,40 @@ TEST_F(BgpConfigTest, AddressFamilies2) { TASK_UTIL_EXPECT_EQ(0, db_graph_.vertex_count()); } +// +// The address-families config for the local bgp-router should be used when +// there's no per session address-families configuration. +// Families that are not configured on the remote bgp-router should not be +// included in the configured family list. +// +TEST_F(BgpConfigTest, AddressFamilies2b) { + string content = FileRead("controller/src/bgp/testdata/config_test_44.xml"); + EXPECT_TRUE(parser_.Parse(content)); + task_util::WaitForIdle(); + + RoutingInstance *rti = server_.routing_instance_mgr()->GetRoutingInstance( + BgpConfigManager::kMasterInstance); + TASK_UTIL_ASSERT_TRUE(rti != NULL); + BgpPeer *peer = rti->peer_manager()->PeerFind("10.1.1.1"); + TASK_UTIL_ASSERT_TRUE(peer != NULL); + + TASK_UTIL_EXPECT_EQ(2, peer->configured_families().size()); + TASK_UTIL_EXPECT_TRUE(peer->LookupFamily(Address::RTARGET)); + TASK_UTIL_EXPECT_TRUE(peer->LookupFamily(Address::EVPN)); + + boost::replace_all(content, "", ""); + boost::replace_all(content, "", ""); + EXPECT_TRUE(parser_.Parse(content)); + task_util::WaitForIdle(); + + TASK_UTIL_EXPECT_EQ(0, db_graph_.vertex_count()); +} + // // The default family should be used if there's no address-families config // for the session or the local bgp-router. // -TEST_F(BgpConfigTest, AddressFamilies3) { +TEST_F(BgpConfigTest, AddressFamilies3a) { string content = FileRead("controller/src/bgp/testdata/config_test_14.xml"); EXPECT_TRUE(parser_.Parse(content)); task_util::WaitForIdle(); @@ -1756,6 +1785,34 @@ TEST_F(BgpConfigTest, AddressFamilies3) { TASK_UTIL_EXPECT_EQ(0, db_graph_.vertex_count()); } +// +// The default family should be used if there's no address-families config +// for the session or the local bgp-router. +// Families that are not configured on the remote bgp-router should not be +// included in the configured family list. +// +TEST_F(BgpConfigTest, AddressFamilies3b) { + string content = FileRead("controller/src/bgp/testdata/config_test_45.xml"); + EXPECT_TRUE(parser_.Parse(content)); + task_util::WaitForIdle(); + + RoutingInstance *rti = server_.routing_instance_mgr()->GetRoutingInstance( + BgpConfigManager::kMasterInstance); + TASK_UTIL_ASSERT_TRUE(rti != NULL); + BgpPeer *peer = rti->peer_manager()->PeerFind("10.1.1.1"); + TASK_UTIL_ASSERT_TRUE(peer != NULL); + + TASK_UTIL_EXPECT_EQ(1, peer->configured_families().size()); + TASK_UTIL_EXPECT_TRUE(peer->LookupFamily(Address::INETVPN)); + + boost::replace_all(content, "", ""); + boost::replace_all(content, "", ""); + EXPECT_TRUE(parser_.Parse(content)); + task_util::WaitForIdle(); + + TASK_UTIL_EXPECT_EQ(0, db_graph_.vertex_count()); +} + TEST_F(BgpConfigTest, RoutePolicy_0) { string content = FileRead("controller/src/bgp/testdata/routing_policy_0.xml"); diff --git a/src/bgp/testdata/config_test_44.xml b/src/bgp/testdata/config_test_44.xml new file mode 100644 index 00000000000..f800fec685a --- /dev/null +++ b/src/bgp/testdata/config_test_44.xml @@ -0,0 +1,22 @@ + + + + +
127.0.0.1
+ 1 + + route-target + inet-vpn + e-vpn + + + +
10.1.1.1
+ 101 + + route-target + e-vpn + + + + diff --git a/src/bgp/testdata/config_test_45.xml b/src/bgp/testdata/config_test_45.xml new file mode 100644 index 00000000000..bf54fb0b5a9 --- /dev/null +++ b/src/bgp/testdata/config_test_45.xml @@ -0,0 +1,16 @@ + + + + +
127.0.0.1
+ 1 + + +
10.1.1.1
+ 101 + + inet-vpn + + + +