From 42d755103fd07336f6e9cb32504fb577fd261452 Mon Sep 17 00:00:00 2001 From: Nischal Sheth Date: Thu, 12 Nov 2015 14:21:33 -0800 Subject: [PATCH] Add loops functionality for bgp peer Highlights: - A loop_count can be configured for peer or peer address family - Value configured under peer address family overrides per peer value - Default value of loop_count is 0 - Paths with upto loop count occurrences of asn are accepted by peer - Include address family attributes in introspect output - Add unit tests to cover new functionality Change-Id: I8e558473a2c467a1bb0fb42ea299919167f11560 Partial-Bug: 1513278 --- src/bgp/bgp_aspath.cc | 13 +- src/bgp/bgp_aspath.h | 2 +- src/bgp/bgp_config.cc | 3 + src/bgp/bgp_config.h | 4 + src/bgp/bgp_config_ifmap.cc | 1 + src/bgp/bgp_peer.cc | 75 +++++++--- src/bgp/bgp_peer.h | 6 +- src/bgp/bgp_peer.sandesh | 15 ++ src/bgp/bgp_sandesh.cc | 12 ++ src/bgp/test/bgp_attr_test.cc | 106 ++++++++++++++ src/bgp/test/bgp_config_test.cc | 59 ++++++++ src/bgp/test/bgp_ifmap_config_manager_test.cc | 53 +++++++ src/bgp/test/bgp_update_rx_test.cc | 134 +++++++++++++++--- src/bgp/testdata/config_test_35a.xml | 23 +++ src/bgp/testdata/config_test_35b.xml | 23 +++ src/bgp/testdata/config_test_35c.xml | 23 +++ src/schema/bgp_schema.xsd | 3 +- 17 files changed, 506 insertions(+), 49 deletions(-) create mode 100644 src/bgp/testdata/config_test_35a.xml create mode 100644 src/bgp/testdata/config_test_35b.xml create mode 100644 src/bgp/testdata/config_test_35c.xml diff --git a/src/bgp/bgp_aspath.cc b/src/bgp/bgp_aspath.cc index 3269ffe98bc..4e54a9e8385 100644 --- a/src/bgp/bgp_aspath.cc +++ b/src/bgp/bgp_aspath.cc @@ -90,11 +90,16 @@ size_t AsPathSpec::EncodeLength() const { return sz; } -bool AsPathSpec::AsPathLoop(as_t as) const { - for (size_t i = 0; i < path_segments.size(); i++) - for (size_t j = 0; j < path_segments[i]->path_segment.size(); j++) - if (path_segments[i]->path_segment[j] == as) +bool AsPathSpec::AsPathLoop(as_t as, uint8_t max_loop_count) const { + uint8_t loop_count = 0; + for (size_t i = 0; i < path_segments.size(); ++i) { + for (size_t j = 0; j < path_segments[i]->path_segment.size(); ++j) { + if (path_segments[i]->path_segment[j] == as && + ++loop_count > max_loop_count) { return true; + } + } + } return false; } diff --git a/src/bgp/bgp_aspath.h b/src/bgp/bgp_aspath.h index 308aab0fafe..5ff2a7124e0 100644 --- a/src/bgp/bgp_aspath.h +++ b/src/bgp/bgp_aspath.h @@ -55,7 +55,7 @@ struct AsPathSpec : public BgpAttribute { as_t AsLeftMost() const; bool AsLeftMostMatch(as_t as) const; - bool AsPathLoop(as_t as) const; + bool AsPathLoop(as_t as, uint8_t max_loop_count = 0) const; virtual int CompareTo(const BgpAttribute &rhs_attr) const; virtual void ToCanonical(BgpAttr *attr); diff --git a/src/bgp/bgp_config.cc b/src/bgp/bgp_config.cc index bd712f5bee5..d427bf48bbe 100644 --- a/src/bgp/bgp_config.cc +++ b/src/bgp/bgp_config.cc @@ -144,6 +144,7 @@ BgpNeighborConfig::BgpNeighborConfig() identifier_(0), port_(BgpConfigManager::kDefaultPort), hold_time_(0), + loop_count_(0), local_as_(0), local_identifier_(0), last_change_at_(UTCTimestampUsec()) { @@ -160,6 +161,7 @@ void BgpNeighborConfig::CopyValues(const BgpNeighborConfig &rhs) { address_ = rhs.address_; port_ = rhs.port_; hold_time_ = rhs.hold_time_; + loop_count_ = rhs.loop_count_; local_as_ = rhs.local_as_; local_identifier_ = rhs.local_identifier_; auth_data_ = rhs.auth_data_; @@ -178,6 +180,7 @@ int BgpNeighborConfig::CompareTo(const BgpNeighborConfig &rhs) const { KEY_COMPARE(address_, rhs.address_); KEY_COMPARE(port_, rhs.port_); KEY_COMPARE(hold_time_, rhs.hold_time_); + KEY_COMPARE(loop_count_, rhs.loop_count_); KEY_COMPARE(local_as_, rhs.local_as_); KEY_COMPARE(local_identifier_, rhs.local_identifier_); KEY_COMPARE(auth_data_, rhs.auth_data_); diff --git a/src/bgp/bgp_config.h b/src/bgp/bgp_config.h index 4fc3ee3ccff..427afe894d9 100644 --- a/src/bgp/bgp_config.h +++ b/src/bgp/bgp_config.h @@ -183,6 +183,9 @@ class BgpNeighborConfig { int hold_time() const { return hold_time_; } void set_hold_time(int hold_time) { hold_time_ = hold_time; } + uint8_t loop_count() const { return loop_count_; } + void set_loop_count(uint8_t loop_count) { loop_count_ = loop_count; } + uint32_t local_as() const { return local_as_; } void set_local_as(uint32_t local_as) { local_as_ = local_as; } @@ -233,6 +236,7 @@ class BgpNeighborConfig { IpAddress address_; int port_; int hold_time_; + uint8_t loop_count_; uint32_t local_as_; uint32_t local_identifier_; mutable uint64_t last_change_at_; diff --git a/src/bgp/bgp_config_ifmap.cc b/src/bgp/bgp_config_ifmap.cc index 4df0e749604..1ca6a71e000 100644 --- a/src/bgp/bgp_config_ifmap.cc +++ b/src/bgp/bgp_config_ifmap.cc @@ -169,6 +169,7 @@ static void NeighborSetSessionAttributes( } if (attributes != NULL) { neighbor->set_passive(attributes->passive); + neighbor->set_loop_count(attributes->loop_count); if (attributes->admin_down) { neighbor->set_admin_down(true); } diff --git a/src/bgp/bgp_peer.cc b/src/bgp/bgp_peer.cc index 7f4831636dd..b5eabe262a7 100644 --- a/src/bgp/bgp_peer.cc +++ b/src/bgp/bgp_peer.cc @@ -254,8 +254,13 @@ class BgpPeer::DeleteActor : public LifetimeActor { // Constructor for BgpPeerFamilyAttributes. // BgpPeerFamilyAttributes::BgpPeerFamilyAttributes( + const BgpNeighborConfig *config, const BgpFamilyAttributesConfig &family_config) { - loop_count = family_config.loop_count; + if (family_config.loop_count) { + loop_count = family_config.loop_count; + } else { + loop_count = config->loop_count(); + } prefix_limit = family_config.prefix_limit; } @@ -528,7 +533,7 @@ bool BgpPeer::ProcessFamilyAttributesConfig(const BgpNeighborConfig *config) { Address::FamilyFromString(family_config.family); assert(family != Address::UNSPEC); BgpPeerFamilyAttributes *family_attributes = - new BgpPeerFamilyAttributes(family_config); + new BgpPeerFamilyAttributes(config, family_config); family_attributes_list[family] = family_attributes; } @@ -1103,31 +1108,37 @@ void BgpPeer::ProcessNlri(Address::Family family, DBRequest::DBOperation oper, } } -void BgpPeer::ProcessUpdate(const BgpProto::Update *msg, size_t msgsize) { - BgpAttrPtr attr = server_->attr_db()->Locate(msg->path_attributes); - // Check as path loop and neighbor-as - const BgpAttr *path_attr = attr.get(); +uint32_t BgpPeer::GetPathFlags(Address::Family family, + const BgpAttr *attr) const { uint32_t flags = 0; - if (path_attr->as_path() != NULL) { - // Check whether neighbor has appended its AS to the AS_PATH - if ((PeerType() == BgpProto::EBGP) && - (!path_attr->as_path()->path().AsLeftMostMatch(peer_as()))) { - flags |= BgpPath::NoNeighborAs; - } - - // Check for AS_PATH loop - if (path_attr->as_path()->path().AsPathLoop(local_as_)) { - flags |= BgpPath::AsPathLooped; - } - } - // Check for OriginatorId loop in case we are an RR client. if (peer_type_ == BgpProto::IBGP && - path_attr->originator_id().to_ulong() == ntohl(local_bgp_id_)) { + attr->originator_id().to_ulong() == ntohl(local_bgp_id_)) { flags |= BgpPath::OriginatorIdLooped; } + if (!attr->as_path()) + return flags; + + // Check whether neighbor has appended its AS to the AS_PATH. + if ((PeerType() == BgpProto::EBGP) && + (!attr->as_path()->path().AsLeftMostMatch(peer_as()))) { + flags |= BgpPath::NoNeighborAs; + } + + // Check for AS_PATH loop. + uint8_t max_loop_count = family_attributes_list_[family]->loop_count; + if (attr->as_path()->path().AsPathLoop(local_as_, max_loop_count)) { + flags |= BgpPath::AsPathLooped; + } + + return flags; +} + +void BgpPeer::ProcessUpdate(const BgpProto::Update *msg, size_t msgsize) { + BgpAttrPtr attr = server_->attr_db()->Locate(msg->path_attributes); + uint32_t reach_count = 0, unreach_count = 0; RoutingInstance *instance = GetRoutingInstance(); if (msg->nlri.size() || msg->withdrawn_routes.size()) { @@ -1159,6 +1170,7 @@ void BgpPeer::ProcessUpdate(const BgpProto::Update *msg, size_t msgsize) { table->Enqueue(&req); } + uint32_t flags = GetPathFlags(Address::INET, attr.get()); reach_count += msg->nlri.size(); for (vector::const_iterator it = msg->nlri.begin(); it != msg->nlri.end(); ++it) { @@ -1214,8 +1226,11 @@ void BgpPeer::ProcessUpdate(const BgpProto::Update *msg, size_t msgsize) { return; } - if ((*ait)->code == BgpAttribute::MPReachNlri) + uint32_t flags = 0; + if ((*ait)->code == BgpAttribute::MPReachNlri) { + flags = GetPathFlags(family, attr.get()); attr = GetMpNlriNexthop(nlri, attr); + } switch (family) { case Address::INET: @@ -1605,6 +1620,23 @@ void BgpPeer::FillBgpNeighborDebugState(BgpNeighborResp &resp, resp.set_tx_socket_stats(peer_socket_stats); } +void BgpPeer::FillBgpNeighborFamilyAttributes(BgpNeighborResp *nbr) const { + vector show_family_attributes_list; + for (int idx = Address::UNSPEC; idx < Address::NUM_FAMILIES; ++idx) { + if (!family_attributes_list_[idx]) + continue; + ShowBgpNeighborFamily show_family_attributes; + show_family_attributes.family = + Address::FamilyToString(static_cast(idx)); + show_family_attributes.loop_count = + family_attributes_list_[idx]->loop_count; + show_family_attributes.prefix_limit = + family_attributes_list_[idx]->prefix_limit; + show_family_attributes_list.push_back(show_family_attributes); + } + nbr->set_family_attributes_list(show_family_attributes_list); +} + void BgpPeer::FillNeighborInfo(const BgpSandeshContext *bsc, vector *nbr_list, bool summary) const { BgpNeighborResp nbr; @@ -1637,6 +1669,7 @@ void BgpPeer::FillNeighborInfo(const BgpSandeshContext *bsc, nbr.set_configured_address_families(configured_families_); nbr.set_negotiated_address_families(negotiated_families_); nbr.set_configured_hold_time(state_machine_->GetConfiguredHoldTime()); + FillBgpNeighborFamilyAttributes(&nbr); FillBgpNeighborDebugState(nbr, peer_stats_.get()); PeerRibMembershipManager *mgr = server_->membership_mgr(); mgr->FillPeerMembershipInfo(this, &nbr); diff --git a/src/bgp/bgp_peer.h b/src/bgp/bgp_peer.h index 58149f15b97..2599e9f2f12 100644 --- a/src/bgp/bgp_peer.h +++ b/src/bgp/bgp_peer.h @@ -43,7 +43,8 @@ class BgpSandeshContext; // Address::Family value. // struct BgpPeerFamilyAttributes { - BgpPeerFamilyAttributes(const BgpFamilyAttributesConfig &family_config); + BgpPeerFamilyAttributes(const BgpNeighborConfig *config, + const BgpFamilyAttributesConfig &family_config); uint8_t loop_count; uint32_t prefix_limit; }; @@ -305,6 +306,7 @@ class BgpPeer : public IPeer { void UnregisterAllTables(); + uint32_t GetPathFlags(Address::Family family, const BgpAttr *attr) const; virtual bool MpNlriAllowed(uint16_t afi, uint8_t safi); BgpAttrPtr GetMpNlriNexthop(BgpMpNlri *nlri, BgpAttrPtr attr); template @@ -323,6 +325,8 @@ class BgpPeer : public IPeer { void PostCloseRelease(); void CustomClose(); + void FillBgpNeighborFamilyAttributes(BgpNeighborResp *nbr) const; + std::string BytesToHexString(const u_int8_t *msg, size_t size); BgpServer *server_; diff --git a/src/bgp/bgp_peer.sandesh b/src/bgp/bgp_peer.sandesh index a65da67a369..0fbe84d75f7 100644 --- a/src/bgp/bgp_peer.sandesh +++ b/src/bgp/bgp_peer.sandesh @@ -39,6 +39,12 @@ struct BgpNeighborRoutingTable { 4: string pending_request; } +struct ShowBgpNeighborFamily { + string family; + u32 loop_count; + u32 prefix_limit; +} + struct BgpNeighborResp { 1: string peer (link="BgpNeighborReq"); // Peer name 36: bool deleted; // Deletion in progress @@ -63,6 +69,7 @@ struct BgpNeighborResp { 45: optional list auth_keys; 38: optional list configured_address_families; 39: optional list negotiated_address_families; + 49: optional list family_attributes_list; 40: optional u32 configured_hold_time; 41: u32 negotiated_hold_time; 46: u32 primary_path_count; @@ -433,6 +440,12 @@ struct ShowBgpPeeringConfig { 4: list sessions; } +struct ShowBgpNeighborFamilyConfig { + 1: string family; + 2: i32 loop_count; + 3: i32 prefix_limit; +} + struct ShowBgpNeighborConfig { 1: string instance_name; 2: string name; @@ -445,10 +458,12 @@ struct ShowBgpNeighborConfig { 5: string identifier; 6: string address; 14: i32 hold_time; + 16: i32 loop_count; 10: string last_change_at; 11: optional string auth_type; 12: optional list auth_keys; 7: list address_families; + 17: list family_attributes_list; } response sandesh ShowBgpNeighborConfigResp { diff --git a/src/bgp/bgp_sandesh.cc b/src/bgp/bgp_sandesh.cc index d91f1785738..a5b34111245 100644 --- a/src/bgp/bgp_sandesh.cc +++ b/src/bgp/bgp_sandesh.cc @@ -466,6 +466,7 @@ class ShowBgpNeighborConfigHandler { nbr.set_address(neighbor->peer_address().to_string()); nbr.set_address_families(neighbor->GetAddressFamilies()); nbr.set_hold_time(neighbor->hold_time()); + nbr.set_loop_count(neighbor->loop_count()); nbr.set_last_change_at( UTCUsecToString(neighbor->last_change_at())); nbr.set_auth_type(neighbor->auth_data().KeyTypeToString()); @@ -473,6 +474,17 @@ class ShowBgpNeighborConfigHandler { nbr.set_auth_keys(neighbor->auth_data().KeysToStringDetail()); } + vector family_attributes_list; + BOOST_FOREACH(const BgpFamilyAttributesConfig family_config, + neighbor->family_attributes_list()) { + ShowBgpNeighborFamilyConfig family_attributes; + family_attributes.family = family_config.family; + family_attributes.loop_count = family_config.loop_count; + family_attributes.prefix_limit = family_config.prefix_limit; + family_attributes_list.push_back(family_attributes); + } + nbr.set_family_attributes_list(family_attributes_list); + nbr_list.push_back(nbr); } diff --git a/src/bgp/test/bgp_attr_test.cc b/src/bgp/test/bgp_attr_test.cc index 3db1a81e27c..13789cc1c33 100644 --- a/src/bgp/test/bgp_attr_test.cc +++ b/src/bgp/test/bgp_attr_test.cc @@ -158,6 +158,112 @@ TEST_F(BgpAttrTest, AsPathLeftMost4) { EXPECT_EQ(64512, spec.AsLeftMost()); } +TEST_F(BgpAttrTest, AsPathLoop1) { + AsPathSpec spec; + EXPECT_FALSE(spec.AsPathLoop(64512, 0)); +} + +TEST_F(BgpAttrTest, AsPathLoop2) { + AsPathSpec spec; + AsPathSpec::PathSegment *ps = new AsPathSpec::PathSegment; + spec.path_segments.push_back(ps); + ps->path_segment_type = AsPathSpec::PathSegment::AS_SET; + EXPECT_FALSE(spec.AsPathLoop(64512, 0)); + ps->path_segment_type = AsPathSpec::PathSegment::AS_SEQUENCE; + EXPECT_FALSE(spec.AsPathLoop(64512, 0)); +} + +TEST_F(BgpAttrTest, AsPathLoop3) { + AsPathSpec spec; + AsPathSpec::PathSegment *ps = new AsPathSpec::PathSegment; + spec.path_segments.push_back(ps); + ps->path_segment_type = AsPathSpec::PathSegment::AS_SEQUENCE; + ps->path_segment.push_back(64512); + ps->path_segment.push_back(64513); + ps->path_segment.push_back(64514); + EXPECT_TRUE(spec.AsPathLoop(64512, 0)); + EXPECT_TRUE(spec.AsPathLoop(64513, 0)); + EXPECT_TRUE(spec.AsPathLoop(64514, 0)); + + EXPECT_FALSE(spec.AsPathLoop(64511, 0)); + EXPECT_FALSE(spec.AsPathLoop(64515, 0)); + + EXPECT_FALSE(spec.AsPathLoop(64511, 1)); + EXPECT_FALSE(spec.AsPathLoop(64512, 1)); + EXPECT_FALSE(spec.AsPathLoop(64513, 1)); + EXPECT_FALSE(spec.AsPathLoop(64514, 1)); + EXPECT_FALSE(spec.AsPathLoop(64515, 1)); +} + +TEST_F(BgpAttrTest, AsPathLoop4) { + AsPathSpec spec; + AsPathSpec::PathSegment *ps = new AsPathSpec::PathSegment; + spec.path_segments.push_back(ps); + + vector segment_type_list = list_of + (AsPathSpec::PathSegment::AS_SET) + (AsPathSpec::PathSegment::AS_SEQUENCE); + BOOST_FOREACH(AsPathSpec::PathSegment::PathSegmentType segment_type, + segment_type_list) { + ps->path_segment_type = segment_type; + + ps->path_segment.clear(); + ps->path_segment.push_back(64512); + ps->path_segment.push_back(64513); + ps->path_segment.push_back(64512); + ps->path_segment.push_back(64514); + ps->path_segment.push_back(64512); + EXPECT_TRUE(spec.AsPathLoop(64512, 0)); + EXPECT_TRUE(spec.AsPathLoop(64512, 1)); + EXPECT_TRUE(spec.AsPathLoop(64512, 2)); + EXPECT_FALSE(spec.AsPathLoop(64512, 3)); + + ps->path_segment.clear(); + ps->path_segment.push_back(64512); + ps->path_segment.push_back(64512); + ps->path_segment.push_back(64512); + ps->path_segment.push_back(64513); + ps->path_segment.push_back(64513); + EXPECT_TRUE(spec.AsPathLoop(64512, 0)); + EXPECT_TRUE(spec.AsPathLoop(64512, 1)); + EXPECT_TRUE(spec.AsPathLoop(64512, 2)); + EXPECT_FALSE(spec.AsPathLoop(64512, 3)); + + ps->path_segment.clear(); + ps->path_segment.push_back(64513); + ps->path_segment.push_back(64513); + ps->path_segment.push_back(64512); + ps->path_segment.push_back(64512); + ps->path_segment.push_back(64512); + EXPECT_TRUE(spec.AsPathLoop(64512, 0)); + EXPECT_TRUE(spec.AsPathLoop(64512, 1)); + EXPECT_TRUE(spec.AsPathLoop(64512, 2)); + EXPECT_FALSE(spec.AsPathLoop(64512, 3)); + } +} + +TEST_F(BgpAttrTest, AsPathLoop5) { + AsPathSpec spec; + AsPathSpec::PathSegment *ps1 = new AsPathSpec::PathSegment; + spec.path_segments.push_back(ps1); + ps1->path_segment_type = AsPathSpec::PathSegment::AS_SET; + ps1->path_segment.push_back(64511); + ps1->path_segment.push_back(64513); + ps1->path_segment.push_back(64515); + + AsPathSpec::PathSegment *ps2 = new AsPathSpec::PathSegment; + spec.path_segments.push_back(ps2); + ps2->path_segment_type = AsPathSpec::PathSegment::AS_SEQUENCE; + ps2->path_segment.push_back(64512); + ps2->path_segment.push_back(64512); + ps2->path_segment.push_back(64512); + + EXPECT_TRUE(spec.AsPathLoop(64512, 0)); + EXPECT_TRUE(spec.AsPathLoop(64512, 1)); + EXPECT_TRUE(spec.AsPathLoop(64512, 2)); + EXPECT_FALSE(spec.AsPathLoop(64512, 3)); +} + TEST_F(BgpAttrTest, AsPathCompare_Sequence) { AsPathSpec spec1; AsPathSpec::PathSegment *ps1 = new AsPathSpec::PathSegment; diff --git a/src/bgp/test/bgp_config_test.cc b/src/bgp/test/bgp_config_test.cc index 2ce8f22ba76..63af4779ce7 100644 --- a/src/bgp/test/bgp_config_test.cc +++ b/src/bgp/test/bgp_config_test.cc @@ -60,6 +60,11 @@ class BgpConfigTest : public ::testing::Test { return peer->state_machine(); } + const BgpPeerFamilyAttributes *GetPeerFamilyAttributes(BgpPeer *peer, + Address::Family family) { + return (peer->family_attributes_list_[family]); + } + EventManager evm_; BgpServer server_; DB config_db_; @@ -310,6 +315,60 @@ TEST_F(BgpConfigTest, MasterNeighbors) { TASK_UTIL_EXPECT_EQ(2, rti->peer_manager()->size()); } +TEST_F(BgpConfigTest, MasterNeighborAttributes) { + string content_a = FileRead("controller/src/bgp/testdata/config_test_35a.xml"); + EXPECT_TRUE(parser_.Parse(content_a)); + task_util::WaitForIdle(); + + RoutingInstance *rti = server_.routing_instance_mgr()->GetRoutingInstance( + BgpConfigManager::kMasterInstance); + TASK_UTIL_EXPECT_TRUE(rti != NULL); + TASK_UTIL_EXPECT_EQ(1, rti->peer_manager()->size()); + string name = rti->name() + ":" + "remote:100"; + + TASK_UTIL_EXPECT_TRUE(rti->peer_manager()->PeerLookup(name) != NULL); + BgpPeer *peer = rti->peer_manager()->PeerLookup(name); + TASK_UTIL_EXPECT_TRUE( + GetPeerFamilyAttributes(peer, Address::INET) != NULL); + TASK_UTIL_EXPECT_EQ(2, + GetPeerFamilyAttributes(peer, Address::INET)->loop_count); + TASK_UTIL_EXPECT_TRUE( + GetPeerFamilyAttributes(peer, Address::INETVPN) != NULL); + TASK_UTIL_EXPECT_EQ(4, + GetPeerFamilyAttributes(peer, Address::INETVPN)->loop_count); + + string content_b = FileRead("controller/src/bgp/testdata/config_test_35b.xml"); + EXPECT_TRUE(parser_.Parse(content_b)); + task_util::WaitForIdle(); + TASK_UTIL_EXPECT_TRUE( + GetPeerFamilyAttributes(peer, Address::INET) != NULL); + TASK_UTIL_EXPECT_EQ(4, + GetPeerFamilyAttributes(peer, Address::INET)->loop_count); + TASK_UTIL_EXPECT_TRUE( + GetPeerFamilyAttributes(peer, Address::INETVPN) != NULL); + TASK_UTIL_EXPECT_EQ(2, + GetPeerFamilyAttributes(peer, Address::INETVPN)->loop_count); + + string content_c = FileRead("controller/src/bgp/testdata/config_test_35c.xml"); + EXPECT_TRUE(parser_.Parse(content_c)); + task_util::WaitForIdle(); + TASK_UTIL_EXPECT_TRUE( + GetPeerFamilyAttributes(peer, Address::INETVPN) == NULL); + TASK_UTIL_EXPECT_TRUE( + GetPeerFamilyAttributes(peer, Address::INET) != NULL); + TASK_UTIL_EXPECT_EQ(2, + GetPeerFamilyAttributes(peer, Address::INET)->loop_count); + TASK_UTIL_EXPECT_TRUE( + GetPeerFamilyAttributes(peer, Address::EVPN) != NULL); + TASK_UTIL_EXPECT_EQ(12, + GetPeerFamilyAttributes(peer, Address::EVPN)->loop_count); + + boost::replace_all(content_c, "", ""); + boost::replace_all(content_c, "", ""); + EXPECT_TRUE(parser_.Parse(content_b)); + task_util::WaitForIdle(); +} + // // Pause and resume neighbor deletion. The peer should get destroyed // after deletion is resumed. diff --git a/src/bgp/test/bgp_ifmap_config_manager_test.cc b/src/bgp/test/bgp_ifmap_config_manager_test.cc index 679df56dc8f..f07cbaffd60 100644 --- a/src/bgp/test/bgp_ifmap_config_manager_test.cc +++ b/src/bgp/test/bgp_ifmap_config_manager_test.cc @@ -218,6 +218,7 @@ static void ValidateShowNeighborResponse( LOG(DEBUG, " AS: " << resp_neighbor.get_autonomous_system()); LOG(DEBUG, " Identifier: " << resp_neighbor.get_identifier()); LOG(DEBUG, " Address: " << resp_neighbor.get_address()); + LOG(DEBUG, " Log: " << resp_neighbor.log()); } LOG(DEBUG, "************************************************************"); @@ -623,6 +624,58 @@ TEST_F(BgpIfmapConfigManagerTest, MasterNeighborsDelete) { TASK_UTIL_EXPECT_EQ(0, db_graph_.vertex_count()); } +TEST_F(BgpIfmapConfigManagerTest, MasterNeighborAttributes) { + string content_a = FileRead( + "controller/src/bgp/testdata/config_test_35a.xml"); + EXPECT_TRUE(parser_.Parse(content_a)); + task_util::WaitForIdle(); + + EXPECT_EQ( + 1, config_manager_->NeighborCount(BgpConfigManager::kMasterInstance)); + const BgpNeighborConfig *config1 = config_manager_->FindNeighbor( + BgpConfigManager::kMasterInstance, "remote:100"); + EXPECT_EQ(2, config1->family_attributes_list().size()); + BOOST_FOREACH(const BgpFamilyAttributesConfig &family_config, + config1->family_attributes_list()) { + if (family_config.family == "inet") { + EXPECT_EQ(2, family_config.loop_count); + } else if (family_config.family == "inet-vpn") { + EXPECT_EQ(4, family_config.loop_count); + } else { + EXPECT_TRUE(false); + } + } + + string content_b = FileRead( + "controller/src/bgp/testdata/config_test_35b.xml"); + EXPECT_TRUE(parser_.Parse(content_b)); + task_util::WaitForIdle(); + + const BgpNeighborConfig *config2 = config_manager_->FindNeighbor( + BgpConfigManager::kMasterInstance, "remote:100"); + EXPECT_EQ(2, config2->family_attributes_list().size()); + BOOST_FOREACH(const BgpFamilyAttributesConfig &family_config, + config2->family_attributes_list()) { + if (family_config.family == "inet") { + EXPECT_EQ(4, family_config.loop_count); + } else if (family_config.family == "inet-vpn") { + EXPECT_EQ(2, family_config.loop_count); + } else { + EXPECT_TRUE(false); + } + } + + boost::replace_all(content_b, "", ""); + boost::replace_all(content_b, "", ""); + EXPECT_TRUE(parser_.Parse(content_b)); + task_util::WaitForIdle(); + + TASK_UTIL_EXPECT_EQ(1, config_manager_->config().instances().size()); + + TASK_UTIL_EXPECT_EQ(0, db_graph_.edge_count()); + TASK_UTIL_EXPECT_EQ(0, db_graph_.vertex_count()); +} + // Add and delete new sessions between existing ones. TEST_F(BgpIfmapConfigManagerTest, MasterPeeringUpdate1) { const BgpIfmapPeeringConfig *peering; diff --git a/src/bgp/test/bgp_update_rx_test.cc b/src/bgp/test/bgp_update_rx_test.cc index 86425124c9e..a8cb73a19df 100644 --- a/src/bgp/test/bgp_update_rx_test.cc +++ b/src/bgp/test/bgp_update_rx_test.cc @@ -31,28 +31,18 @@ class BgpUpdateRxTest : public ::testing::Test { protected: BgpUpdateRxTest() : server_(&evm_), - instance_config_(BgpConfigManager::kMasterInstance), + master_(NULL), peer_(NULL), rib1_(NULL), rib2_(NULL), tid1_(DBTableBase::kInvalidId), tid2_(DBTableBase::kInvalidId) { ConcurrencyScope scope("bgp::Config"); + BgpInstanceConfig instance_config(BgpConfigManager::kMasterInstance); boost::system::error_code ec; local_identifier_ = Ip4Address::from_string("10.1.1.1", ec); - RoutingInstance *instance = - server_.routing_instance_mgr()->CreateRoutingInstance( - &instance_config_); - config_.set_name("test-peer"); - config_.set_instance_name(BgpConfigManager::kMasterInstance); - config_.set_local_identifier(htonl(local_identifier_.to_ulong())); - config_.set_local_as(64512); - config_.set_peer_as(64512); - BgpNeighborConfig::FamilyAttributesList family_attributes_list; - family_attributes_list.push_back(BgpFamilyAttributesConfig("inet")); - family_attributes_list.push_back(BgpFamilyAttributesConfig("inet-vpn")); - config_.set_family_attributes_list(family_attributes_list); - rib1_ = instance->GetTable(Address::INET); - rib2_ = instance->GetTable(Address::INETVPN); - peer_ = instance->peer_manager()->PeerLocate(&server_, &config_); + master_ = server_.routing_instance_mgr()->CreateRoutingInstance( + &instance_config); + rib1_ = master_->GetTable(Address::INET); + rib2_ = master_->GetTable(Address::INETVPN); adc_notification_ = 0; del_notification_ = 0; } @@ -86,14 +76,33 @@ class BgpUpdateRxTest : public ::testing::Test { adc_notification_++; } + void CreatePeer(uint32_t local_as = 64512, uint32_t peer_as = 64512, + uint8_t loop_count = 0) { + ConcurrencyScope scope("bgp::Config"); + BgpNeighborConfig nbr_config; + nbr_config.set_name("test-peer"); + nbr_config.set_instance_name(BgpConfigManager::kMasterInstance); + nbr_config.set_local_identifier(htonl(local_identifier_.to_ulong())); + nbr_config.set_local_as(local_as); + nbr_config.set_peer_as(peer_as); + nbr_config.set_loop_count(loop_count); + + BgpNeighborConfig::FamilyAttributesList family_attributes_list; + BgpFamilyAttributesConfig family_attributes1("inet"); + family_attributes_list.push_back(family_attributes1); + BgpFamilyAttributesConfig family_attributes2("inet-vpn"); + family_attributes_list.push_back(family_attributes2); + nbr_config.set_family_attributes_list(family_attributes_list); + peer_ = master_->peer_manager()->PeerLocate(&server_, &nbr_config); + } + tbb::atomic adc_notification_; tbb::atomic del_notification_; EventManager evm_; BgpServer server_; Ip4Address local_identifier_; - BgpInstanceConfig instance_config_; - BgpNeighborConfig config_; + RoutingInstance *master_; BgpPeer *peer_; BgpTable *rib1_; @@ -104,6 +113,7 @@ class BgpUpdateRxTest : public ::testing::Test { }; TEST_F(BgpUpdateRxTest, AdvertiseWithdraw) { + CreatePeer(); adc_notification_ = 0; del_notification_ = 0; @@ -248,12 +258,13 @@ TEST_F(BgpUpdateRxTest, AdvertiseWithdraw) { } // Parameterize originator id to be same vs. different. -class BgpUpdateRxParamTest: +class BgpUpdateRxParamTest1: public BgpUpdateRxTest, public ::testing::WithParamInterface { }; -TEST_P(BgpUpdateRxParamTest, OriginatorIdLoop) { +TEST_P(BgpUpdateRxParamTest1, OriginatorIdLoop) { + CreatePeer(); EXPECT_EQ(rib2_, server_.database()->FindTable("bgp.l3vpn.0")); BgpProto::OpenMessage open; @@ -325,7 +336,88 @@ TEST_P(BgpUpdateRxParamTest, OriginatorIdLoop) { peer_->ResetCapabilities(); } -INSTANTIATE_TEST_CASE_P(Instance, BgpUpdateRxParamTest, ::testing::Bool()); +// Parameterize loop count. +class BgpUpdateRxParamTest2: + public BgpUpdateRxTest, + public ::testing::WithParamInterface { +}; + +TEST_P(BgpUpdateRxParamTest2, AsPathLoop) { + CreatePeer(64512, 64513, GetParam()); + EXPECT_EQ(rib2_, server_.database()->FindTable("bgp.l3vpn.0")); + + BgpProto::OpenMessage open; + uint8_t capc[] = {0, 1, 0, 128}; + BgpProto::OpenMessage::Capability *cap = + new BgpProto::OpenMessage::Capability( + BgpProto::OpenMessage::Capability::MpExtension, capc, 4); + BgpProto::OpenMessage::OptParam *opt = new BgpProto::OpenMessage::OptParam; + opt->capabilities.push_back(cap); + open.opt_params.push_back(opt); + peer_->SetCapabilities(&open); + + // Advertise the prefix. + BgpProto::Update update; + InetVpnPrefix iv_prefix(InetVpnPrefix::FromString("2:20:192.168.24.0/24")); + BgpAttrOrigin *origin = new BgpAttrOrigin(BgpAttrOrigin::INCOMPLETE); + update.path_attributes.push_back(origin); + BgpAttrNextHop *nexthop = new BgpAttrNextHop(0xabcdef01); + update.path_attributes.push_back(nexthop); + AsPathSpec *path_spec = new AsPathSpec; + AsPathSpec::PathSegment *ps = new AsPathSpec::PathSegment; + ps->path_segment_type = AsPathSpec::PathSegment::AS_SEQUENCE; + ps->path_segment.push_back(64512); + ps->path_segment.push_back(64514); + ps->path_segment.push_back(64512); + path_spec->path_segments.push_back(ps); + update.path_attributes.push_back(path_spec); + uint32_t identifier = local_identifier_.to_ulong(); + + BgpMpNlri *mp_nlri = new BgpMpNlri; + BgpProtoPrefix *bpp = new BgpProtoPrefix; + iv_prefix.BuildProtoPrefix(12, bpp); + mp_nlri->code = BgpAttribute::MPReachNlri; + mp_nlri->afi = 1; + mp_nlri->safi = 128; + uint8_t nh[12] = {0,0,0,0,0,0,0,0,192,168,1,1}; + mp_nlri->nexthop.assign(&nh[0], &nh[12]); + mp_nlri->nlri.push_back(bpp); + update.path_attributes.push_back(mp_nlri); + + peer_->ProcessUpdate(&update); + task_util::WaitForIdle(); + InetVpnTable::RequestKey key(iv_prefix, peer_); + TASK_UTIL_EXPECT_TRUE(rib2_->Find(&key) != NULL); + BgpRoute *rt = static_cast(rib2_->Find(&key)); + TASK_UTIL_EXPECT_TRUE(rt->BestPath() != NULL); + const BgpPath *path = rt->BestPath(); + if (GetParam() < 2) { + EXPECT_TRUE((path->GetFlags() & BgpPath::AsPathLooped) != 0); + } else { + EXPECT_TRUE((path->GetFlags() & BgpPath::AsPathLooped) == 0); + } + + // Withdraw the prefix. + BgpProto::Update withdraw; + BgpMpNlri *mp_nlri2 = new BgpMpNlri; + BgpProtoPrefix *bpp2 = new BgpProtoPrefix; + iv_prefix.BuildProtoPrefix(12, bpp2); + mp_nlri2->code = BgpAttribute::MPUnreachNlri; + mp_nlri2->afi = 1; + mp_nlri2->safi = 128; + mp_nlri2->nlri.push_back(bpp2); + withdraw.path_attributes.push_back(mp_nlri2); + + peer_->ProcessUpdate(&withdraw); + task_util::WaitForIdle(); + TASK_UTIL_EXPECT_TRUE(rib2_->Find(&key) == NULL); + + peer_->ResetCapabilities(); +} + +INSTANTIATE_TEST_CASE_P(Instance, BgpUpdateRxParamTest1, ::testing::Bool()); +INSTANTIATE_TEST_CASE_P(Instance, BgpUpdateRxParamTest2, + ::testing::Values(0, 1, 2, 3)); static void SetUp() { ControlNode::SetDefaultSchedulingPolicy(); diff --git a/src/bgp/testdata/config_test_35a.xml b/src/bgp/testdata/config_test_35a.xml new file mode 100644 index 00000000000..6d4bbeee079 --- /dev/null +++ b/src/bgp/testdata/config_test_35a.xml @@ -0,0 +1,23 @@ + + + + +
127.0.0.1
+ 64512 + + + inet + 2 + + + inet-vpn + 4 + + + + +
10.1.1.1
+ 64512 + + + diff --git a/src/bgp/testdata/config_test_35b.xml b/src/bgp/testdata/config_test_35b.xml new file mode 100644 index 00000000000..02821a84d9b --- /dev/null +++ b/src/bgp/testdata/config_test_35b.xml @@ -0,0 +1,23 @@ + + + + +
127.0.0.1
+ 64512 + + + inet-vpn + 2 + + + inet + 4 + + + + +
10.1.1.1
+ 64512 + + + diff --git a/src/bgp/testdata/config_test_35c.xml b/src/bgp/testdata/config_test_35c.xml new file mode 100644 index 00000000000..f3b1e616f6d --- /dev/null +++ b/src/bgp/testdata/config_test_35c.xml @@ -0,0 +1,23 @@ + + + + +
127.0.0.1
+ 64512 + + + e-vpn + 12 + + + inet + 2 + + + + +
10.1.1.1
+ 64512 + + + diff --git a/src/schema/bgp_schema.xsd b/src/schema/bgp_schema.xsd index 063ec90c95c..fdd38f05b07 100644 --- a/src/schema/bgp_schema.xsd +++ b/src/schema/bgp_schema.xsd @@ -122,6 +122,7 @@ + @@ -139,7 +140,7 @@ - +