diff --git a/src/bgp/bgp_aspath.cc b/src/bgp/bgp_aspath.cc index 4e54a9e8385..5cab4825861 100644 --- a/src/bgp/bgp_aspath.cc +++ b/src/bgp/bgp_aspath.cc @@ -103,7 +103,9 @@ bool AsPathSpec::AsPathLoop(as_t as, uint8_t max_loop_count) const { return false; } -// Create a new AsPathSpec by prepending the given asn at the beginning +// +// Create a new AsPathSpec by prepending the given asn at the beginning. +// AsPathSpec *AsPathSpec::Add(as_t asn) const { AsPathSpec *new_spec = new AsPathSpec; PathSegment *ps = new PathSegment; @@ -122,7 +124,8 @@ AsPathSpec *AsPathSpec::Add(as_t asn) const { } else { new_spec->path_segments.push_back(ps); } - if (first == last) return new_spec; + if (first == last) + return new_spec; for (int i = first; i < last; i++) { PathSegment *ps = new PathSegment; *ps = *path_segments[i]; @@ -131,6 +134,21 @@ AsPathSpec *AsPathSpec::Add(as_t asn) const { return new_spec; } +// +// Create a new AsPathSpec by replacing the old asn with given asn. +// +AsPathSpec *AsPathSpec::Replace(as_t old_asn, as_t asn) const { + AsPathSpec *new_spec = new AsPathSpec(*this); + for (size_t i = 0; i < new_spec->path_segments.size(); ++i) { + PathSegment *ps = new_spec->path_segments[i]; + for (size_t j = 0; j < ps->path_segment.size(); ++j) { + if (ps->path_segment[j] == old_asn) + ps->path_segment[j] = asn; + } + } + return new_spec; +} + void AsPath::Remove() { aspath_db_->Delete(this); } diff --git a/src/bgp/bgp_aspath.h b/src/bgp/bgp_aspath.h index 5ff2a7124e0..11e809357e1 100644 --- a/src/bgp/bgp_aspath.h +++ b/src/bgp/bgp_aspath.h @@ -62,6 +62,7 @@ struct AsPathSpec : public BgpAttribute { virtual size_t EncodeLength() const; virtual std::string ToString() const; AsPathSpec *Add(as_t asn) const; + AsPathSpec *Replace(as_t old_asn, as_t asn) const; std::vector path_segments; }; diff --git a/src/bgp/bgp_config.cc b/src/bgp/bgp_config.cc index f0b18f8dbbf..f039b3670df 100644 --- a/src/bgp/bgp_config.cc +++ b/src/bgp/bgp_config.cc @@ -150,6 +150,7 @@ BgpNeighborConfig::BgpNeighborConfig() : type_(UNSPECIFIED), admin_down_(false), passive_(false), + as_override_(false), peer_as_(0), identifier_(0), port_(BgpConfigManager::kDefaultPort), @@ -167,6 +168,7 @@ void BgpNeighborConfig::CopyValues(const BgpNeighborConfig &rhs) { type_ = rhs.type_; admin_down_ = rhs.admin_down_; passive_ = rhs.passive_; + as_override_ = rhs.as_override_; peer_as_ = rhs.peer_as_; identifier_ = rhs.identifier_; address_ = rhs.address_; @@ -189,6 +191,7 @@ int BgpNeighborConfig::CompareTo(const BgpNeighborConfig &rhs) const { KEY_COMPARE(type_, rhs.type_); KEY_COMPARE(admin_down_, rhs.admin_down_); KEY_COMPARE(passive_, rhs.passive_); + KEY_COMPARE(as_override_, rhs.as_override_); KEY_COMPARE(peer_as_, rhs.peer_as_); KEY_COMPARE(identifier_, rhs.identifier_); KEY_COMPARE(address_, rhs.address_); diff --git a/src/bgp/bgp_config.h b/src/bgp/bgp_config.h index b580213cd62..0afbb7aa843 100644 --- a/src/bgp/bgp_config.h +++ b/src/bgp/bgp_config.h @@ -168,6 +168,9 @@ class BgpNeighborConfig { bool passive() const { return passive_; } void set_passive(bool passive) { passive_ = passive; } + bool as_override() const { return as_override_; } + void set_as_override(bool as_override) { as_override_ = as_override; } + uint32_t peer_as() const { return peer_as_; } void set_peer_as(uint32_t peer_as) { peer_as_ = peer_as; } @@ -252,6 +255,7 @@ class BgpNeighborConfig { std::string router_type_; bool admin_down_; bool passive_; + bool as_override_; uint32_t peer_as_; uint32_t identifier_; IpAddress address_; diff --git a/src/bgp/bgp_peer.cc b/src/bgp/bgp_peer.cc index bc2dbe7d54b..6802a46f8d4 100644 --- a/src/bgp/bgp_peer.cc +++ b/src/bgp/bgp_peer.cc @@ -263,12 +263,12 @@ RibExportPolicy BgpPeer::BuildRibExportPolicy(Address::Family family) const { family_attributes_list_[family]; if (!family_attributes || family_attributes->gateway_address.is_unspecified()) { - return RibExportPolicy( - peer_type_, RibExportPolicy::BGP, peer_as_, -1, 0); + return RibExportPolicy(peer_type_, RibExportPolicy::BGP, peer_as_, + as_override_, -1, 0); } else { IpAddress nexthop = family_attributes->gateway_address; - return RibExportPolicy( - peer_type_, RibExportPolicy::BGP, peer_as_, nexthop, -1, 0); + return RibExportPolicy(peer_type_, RibExportPolicy::BGP, peer_as_, + as_override_, nexthop, -1, 0); } } @@ -370,6 +370,7 @@ BgpPeer::BgpPeer(BgpServer *server, RoutingInstance *instance, admin_down_(config->admin_down()), passive_(config->passive()), resolve_paths_(config->router_type() == "bgpaas-client"), + as_override_(config->as_override()), membership_req_pending_(0), defer_close_(false), vpn_tables_registered_(false), @@ -407,6 +408,7 @@ BgpPeer::BgpPeer(BgpServer *server, RoutingInstance *instance, peer_info.set_name(ToUVEKey()); peer_info.set_admin_down(admin_down_); peer_info.set_passive(passive_); + peer_info.set_as_override(as_override_); peer_info.set_router_type(router_type_); peer_info.set_peer_type( PeerType() == BgpProto::IBGP ? "internal" : "external"); @@ -604,6 +606,12 @@ void BgpPeer::ConfigUpdate(const BgpNeighborConfig *config) { clear_session = true; } + if (as_override_ != config->as_override()) { + as_override_ = config->as_override(); + peer_info.set_as_override(as_override_); + clear_session = true; + } + if (router_type_ != config->router_type()) { router_type_ = config->router_type(); peer_info.set_router_type(router_type_); @@ -1743,6 +1751,7 @@ void BgpPeer::FillNeighborInfo(const BgpSandeshContext *bsc, bnr->set_closed_at(UTCUsecToString(deleter_->delete_time_stamp_usecs())); bnr->set_admin_down(admin_down_); bnr->set_passive(passive_); + bnr->set_as_override(as_override_); bnr->set_peer_address(peer_address_string()); bnr->set_peer_id(bgp_identifier_string()); bnr->set_peer_asn(peer_as()); diff --git a/src/bgp/bgp_peer.h b/src/bgp/bgp_peer.h index 3a51d7aa4b5..f10b93d2773 100644 --- a/src/bgp/bgp_peer.h +++ b/src/bgp/bgp_peer.h @@ -386,6 +386,7 @@ class BgpPeer : public IPeer { bool admin_down_; bool passive_; bool resolve_paths_; + bool as_override_; uint64_t membership_req_pending_; bool defer_close_; diff --git a/src/bgp/bgp_peer.sandesh b/src/bgp/bgp_peer.sandesh index 3957dcb8cf9..d8315635b39 100644 --- a/src/bgp/bgp_peer.sandesh +++ b/src/bgp/bgp_peer.sandesh @@ -61,18 +61,19 @@ struct BgpNeighborResp { 53: string instance_name; 1: string peer (link="BgpNeighborReq"); // Peer name 36: bool deleted; // Deletion in progress - 43: string closed_at; - 47: bool admin_down; - 48: bool passive; 2: string peer_address (link="BgpNeighborReq"); 25: string peer_id; 3: u32 peer_asn; - 50: u32 peer_port; - 51: string transport_address; 6: string encoding; // BGP/XMPP 7: string peer_type // internal/external - 52: string router_type; // bgp_schema.xsd:BgpRouterType 8: string state; + 43: string closed_at; + 52: string router_type; // bgp_schema.xsd:BgpRouterType + 47: bool admin_down; + 48: bool passive; + 55: bool as_override; + 50: u32 peer_port; + 51: string transport_address; 4: string local_address; // local ip address and port 26: string local_id; 5: u32 local_asn; @@ -550,6 +551,7 @@ struct ShowBgpNeighborConfig { 2: string name; 13: bool admin_down; 15: bool passive; + 20: bool as_override; 18: string router_type; 8: string local_identifier; 9: i32 local_as; @@ -602,6 +604,7 @@ struct BgpPeerInfoData { 2: optional bool deleted 23: optional bool admin_down; 24: optional bool passive; + 27: optional bool as_override; 25: optional string router_type; // bgp_schema.xsd:BgpRouterType 3: optional string peer_type; // internal/external 20: optional string peer_address; diff --git a/src/bgp/bgp_ribout.cc b/src/bgp/bgp_ribout.cc index ef5b1654a28..f0bd9de0d4e 100644 --- a/src/bgp/bgp_ribout.cc +++ b/src/bgp/bgp_ribout.cc @@ -41,6 +41,12 @@ bool RibExportPolicy::operator<(const RibExportPolicy &rhs) const { if (as_number > rhs.as_number) { return false; } + if (as_override < rhs.as_override) { + return true; + } + if (as_override > rhs.as_override) { + return false; + } if (nexthop < rhs.nexthop) { return true; } diff --git a/src/bgp/bgp_ribout.h b/src/bgp/bgp_ribout.h index e4adcea048b..221a265774f 100644 --- a/src/bgp/bgp_ribout.h +++ b/src/bgp/bgp_ribout.h @@ -218,6 +218,9 @@ class RouteState : public DBState { // practical deployment scenarios all eBGP peers will belong to the same // neighbor AS anyway. // +// Including AS override as part of the policy results in creation of a +// different RibOuts for neighbors that need and don't AS override. +// // Including the nexthop as part of the policy results in creation of a // different RibOut for each set of BGPaaS clients that belong to the same // subnet. This allows us to rewrite the nexthop to the specified value. @@ -234,12 +237,12 @@ struct RibExportPolicy { RibExportPolicy() : type(BgpProto::IBGP), encoding(BGP), - as_number(0), affinity(-1), cluster_id(0) { + as_number(0), as_override(false), affinity(-1), cluster_id(0) { } RibExportPolicy(BgpProto::BgpPeerType type, Encoding encoding, int affinity, u_int32_t cluster_id) - : type(type), encoding(encoding), as_number(0), + : type(type), encoding(encoding), as_number(0), as_override(false), affinity(affinity), cluster_id(cluster_id) { if (encoding == XMPP) assert(type == BgpProto::XMPP); @@ -248,9 +251,9 @@ struct RibExportPolicy { } RibExportPolicy(BgpProto::BgpPeerType type, Encoding encoding, - as_t as_number, int affinity, u_int32_t cluster_id) + as_t as_number, bool as_override, int affinity, u_int32_t cluster_id) : type(type), encoding(encoding), as_number(as_number), - affinity(affinity), cluster_id(cluster_id) { + as_override(as_override), affinity(affinity), cluster_id(cluster_id) { if (encoding == XMPP) assert(type == BgpProto::XMPP); if (encoding == BGP) @@ -258,9 +261,11 @@ struct RibExportPolicy { } RibExportPolicy(BgpProto::BgpPeerType type, Encoding encoding, - as_t as_number, IpAddress nexthop, int affinity, u_int32_t cluster_id) + as_t as_number, bool as_override, IpAddress nexthop, + int affinity, u_int32_t cluster_id) : type(type), encoding(BGP), as_number(as_number), - nexthop(nexthop), affinity(affinity), cluster_id(cluster_id) { + as_override(as_override), nexthop(nexthop), + affinity(affinity), cluster_id(cluster_id) { assert(type == BgpProto::IBGP || type == BgpProto::EBGP); assert(encoding == BGP); } @@ -270,6 +275,7 @@ struct RibExportPolicy { BgpProto::BgpPeerType type; Encoding encoding; as_t as_number; + bool as_override; IpAddress nexthop; int affinity; uint32_t cluster_id; @@ -342,6 +348,7 @@ class RibOut { BgpProto::BgpPeerType peer_type() const { return policy_.type; } as_t peer_as() const { return policy_.as_number; } + bool as_override() const { return policy_.as_override; } const IpAddress &nexthop() const { return policy_.nexthop; } bool IsEncodingXmpp() const { return (policy_.encoding == RibExportPolicy::XMPP); diff --git a/src/bgp/bgp_show_config.cc b/src/bgp/bgp_show_config.cc index 085f4786698..259be40558b 100644 --- a/src/bgp/bgp_show_config.cc +++ b/src/bgp/bgp_show_config.cc @@ -327,6 +327,7 @@ static void FillBgpNeighborConfigInfo(ShowBgpNeighborConfig *sbnc, sbnc->set_name(neighbor->name()); sbnc->set_admin_down(neighbor->admin_down()); sbnc->set_passive(neighbor->passive()); + sbnc->set_as_override(neighbor->as_override()); sbnc->set_router_type(neighbor->router_type()); sbnc->set_local_identifier(neighbor->local_identifier_string()); sbnc->set_local_as(neighbor->local_as()); diff --git a/src/bgp/bgp_table.cc b/src/bgp/bgp_table.cc index 935e5e9db67..9eb7361be08 100644 --- a/src/bgp/bgp_table.cc +++ b/src/bgp/bgp_table.cc @@ -181,19 +181,19 @@ UpdateInfo *BgpTable::GetUpdateInfo(RibOut *ribout, BgpRoute *route, BgpAttr *clone = new BgpAttr(*attr); // Retain LocalPref value if set, else set default to 100. - if (attr->local_pref() == 0) + if (clone->local_pref() == 0) clone->set_local_pref(100); // If the route is locally originated i.e. there's no AsPath, // then generate a Nil AsPath i.e. one with 0 length. No need // to modify the AsPath if it already exists since this is an // iBGP RibOut. - if (attr->as_path() == NULL) { + if (clone->as_path() == NULL) { AsPathSpec as_path; clone->set_as_path(&as_path); } - attr_ptr = attr->attr_db()->Locate(clone); + attr_ptr = clone->attr_db()->Locate(clone); attr = attr_ptr.get(); } else if (ribout->peer_type() == BgpProto::EBGP) { // Don't advertise routes from non-master instances if there's @@ -205,7 +205,7 @@ UpdateInfo *BgpTable::GetUpdateInfo(RibOut *ribout, BgpRoute *route, } // Sender side AS path loop check. - if (attr->as_path() && + if (!ribout->as_override() && attr->as_path() && attr->as_path()->path().AsPathLoop(ribout->peer_as())) { return NULL; } @@ -225,21 +225,31 @@ UpdateInfo *BgpTable::GetUpdateInfo(RibOut *ribout, BgpRoute *route, clone->set_nexthop(ribout->nexthop()); // Reset LocalPref. - if (attr->local_pref()) + if (clone->local_pref()) clone->set_local_pref(0); // Reset Med if the path did not originate from an xmpp peer. // The AS path is NULL if the originating xmpp peer is locally // connected. It's non-NULL but empty if the originating xmpp // peer is connected to another bgp speaker in the iBGP mesh. - if (attr->med() && attr->as_path() && !attr->as_path()->empty()) + if (clone->med() && clone->as_path() && !clone->as_path()->empty()) clone->set_med(0); - // Prepend the local AS to AsPath. as_t local_as = - attr->attr_db()->server()->local_autonomous_system(); - if (attr->as_path() != NULL) { - const AsPathSpec &as_path = attr->as_path()->path(); + clone->attr_db()->server()->local_autonomous_system(); + + // Override the peer AS with local AS in AsPath. + if (ribout->as_override() && clone->as_path() != NULL) { + const AsPathSpec &as_path = clone->as_path()->path(); + AsPathSpec *as_path_ptr = + as_path.Replace(ribout->peer_as(), local_as); + clone->set_as_path(as_path_ptr); + delete as_path_ptr; + } + + // Prepend the local AS to AsPath. + if (clone->as_path() != NULL) { + const AsPathSpec &as_path = clone->as_path()->path(); AsPathSpec *as_path_ptr = as_path.Add(local_as); clone->set_as_path(as_path_ptr); delete as_path_ptr; @@ -250,7 +260,7 @@ UpdateInfo *BgpTable::GetUpdateInfo(RibOut *ribout, BgpRoute *route, delete as_path_ptr; } - attr_ptr = attr->attr_db()->Locate(clone); + attr_ptr = clone->attr_db()->Locate(clone); attr = attr_ptr.get(); } } diff --git a/src/bgp/bgp_xmpp_channel.cc b/src/bgp/bgp_xmpp_channel.cc index 67840219276..8017c31d7ec 100644 --- a/src/bgp/bgp_xmpp_channel.cc +++ b/src/bgp/bgp_xmpp_channel.cc @@ -502,7 +502,7 @@ BgpXmppChannel::BgpXmppChannel(XmppChannel *channel, BgpServer *bgp_server, peer_(new XmppPeer(bgp_server, this)), peer_close_(new PeerClose(this)), peer_stats_(new PeerStats(this)), - bgp_policy_(peer_->PeerType(), RibExportPolicy::XMPP, 0, -1, 0), + bgp_policy_(peer_->PeerType(), RibExportPolicy::XMPP, -1, 0), manager_(manager), delete_in_progress_(false), deleted_(false), diff --git a/src/bgp/test/bgp_attr_test.cc b/src/bgp/test/bgp_attr_test.cc index d4eebb0f041..868f0e01366 100644 --- a/src/bgp/test/bgp_attr_test.cc +++ b/src/bgp/test/bgp_attr_test.cc @@ -5,6 +5,8 @@ #include #include +#include + #include #include "base/test/task_test_util.h" @@ -322,7 +324,7 @@ TEST_F(BgpAttrTest, AsPathAdd) { AsPathSpec::PathSegment *ps = new AsPathSpec::PathSegment; ps->path_segment_type = AsPathSpec::PathSegment::AS_SEQUENCE; for (int j = 0; j < 10; j++) { - ps->path_segment.push_back((i<<4) + j); + ps->path_segment.push_back((i << 4) + j); } path.path_segments.push_back(ps); } @@ -340,7 +342,8 @@ TEST_F(BgpAttrTest, AsPathAdd) { EXPECT_EQ(1000, p2->path_segments[0]->path_segment[0]); delete p2; - path.path_segments[0]->path_segment_type = AsPathSpec::PathSegment::AS_SEQUENCE; + path.path_segments[0]->path_segment_type = + AsPathSpec::PathSegment::AS_SEQUENCE; for (int j = 11; j < 256; j++) { path.path_segments[0]->path_segment.push_back(j); } @@ -350,7 +353,35 @@ TEST_F(BgpAttrTest, AsPathAdd) { EXPECT_EQ(1, p2->path_segments[0]->path_segment.size()); EXPECT_EQ(1000, p2->path_segments[0]->path_segment[0]); delete p2; +} + +TEST_F(BgpAttrTest, AsPathReplace) { + 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(64513); + ps2->path_segment.push_back(64514); + + boost::scoped_ptr new_spec1(spec.Replace(64513, 65000)); + EXPECT_FALSE(new_spec1->AsPathLoop(64513, 0)); + EXPECT_TRUE(new_spec1->AsPathLoop(65000, 1)); + EXPECT_FALSE(new_spec1->AsPathLoop(65000, 2)); + + boost::scoped_ptr new_spec2(new_spec1->Replace(65000, 64513)); + EXPECT_FALSE(new_spec2->AsPathLoop(65000, 0)); + EXPECT_TRUE(new_spec2->AsPathLoop(64513, 1)); + EXPECT_FALSE(new_spec2->AsPathLoop(64513, 2)); + EXPECT_EQ(0, spec.CompareTo(*new_spec2)); } TEST_F(BgpAttrTest, AsPathFormat1) { @@ -1175,7 +1206,8 @@ TEST_F(BgpAttrTest, PmsiTunnel3) { EXPECT_EQ(BgpAttribute::PmsiTunnel, pmsi_spec2.code); EXPECT_EQ(BgpAttribute::Optional | BgpAttribute::Transitive, pmsi_spec2.flags); - EXPECT_EQ(PmsiTunnelSpec::EdgeReplicationSupported, pmsi_spec2.tunnel_flags); + EXPECT_EQ(PmsiTunnelSpec::EdgeReplicationSupported, + pmsi_spec2.tunnel_flags); EXPECT_EQ(PmsiTunnelSpec::IngressReplication, pmsi_spec2.tunnel_type); EXPECT_EQ(10000, pmsi_spec2.GetLabel()); EXPECT_EQ("10.1.1.1", pmsi_spec2.GetIdentifier().to_string()); @@ -1410,7 +1442,8 @@ TEST_F(BgpAttrTest, EdgeDiscovery3) { EXPECT_EQ(2, edspec2.edge_list.size()); int idx = 1; for (EdgeDiscoverySpec::EdgeList::const_iterator it = - edspec2.edge_list.begin(); it != edspec2.edge_list.end(); ++it, ++idx) { + edspec2.edge_list.begin(); it != edspec2.edge_list.end(); + ++it, ++idx) { const EdgeDiscoverySpec::Edge *edge = *it; std::string addr_str = "10.1.1." + integerToString(idx); uint32_t first_label, last_label; @@ -1952,7 +1985,8 @@ TEST_F(BgpAttrTest, EdgeForwarding3) { EXPECT_EQ(2, efspec2.edge_list.size()); int idx = 1; for (EdgeForwardingSpec::EdgeList::const_iterator it = - efspec2.edge_list.begin(); it != efspec2.edge_list.end(); ++it, ++idx) { + efspec2.edge_list.begin(); it != efspec2.edge_list.end(); + ++it, ++idx) { const EdgeForwardingSpec::Edge *edge = *it; error_code ec; std::string addr_str = "10.1.1." + integerToString(idx); @@ -2322,7 +2356,6 @@ class AttributeMock : public Type { AttributeMock(TypeDB *db, const TypeSpec &spec) : Type(db, spec) { } virtual void Remove() { - // Inject artificial delay to detect concurrency issues, if any. usleep(10000); Type::Remove(); @@ -2361,18 +2394,6 @@ static void ConcurrencyTest(TypeDB *db) { TASK_UTIL_EXPECT_EQ(0, db->Size()); } -// Instantiate the template functions. -template void ConcurrencyTest(BgpAttrDB *); -template void ConcurrencyTest(AsPathDB *); -template void ConcurrencyTest(CommunityDB *); -template void ConcurrencyTest(ExtCommunityDB *); -template void ConcurrencyTest(OriginVnPathDB *); - TEST_F(BgpAttrTest, BgpAttrDBConcurrency) { ConcurrencyTest(attr_db_); } diff --git a/src/bgp/test/bgp_table_export_test.cc b/src/bgp/test/bgp_table_export_test.cc index 1b832cb900d..98722524dd9 100644 --- a/src/bgp/test/bgp_table_export_test.cc +++ b/src/bgp/test/bgp_table_export_test.cc @@ -13,7 +13,11 @@ #include "control-node/control_node.h" #include "net/community_type.h" -using namespace std; +using std::auto_ptr; +using std::cout; +using std::endl; +using std::ostringstream; +using std::string; // // OVERVIEW @@ -48,13 +52,13 @@ using namespace std; class BgpTestPeer : public IPeer { public: - BgpTestPeer(bool internal) : internal_(internal) { } + explicit BgpTestPeer(bool internal) : internal_(internal) { } virtual ~BgpTestPeer() { } - virtual std::string ToString() const { + virtual string ToString() const { return internal_ ? "TestPeerInt" : "TestPeerExt"; } - virtual std::string ToUVEKey() const { + virtual string ToUVEKey() const { return internal_ ? "TestPeerInt" : "TestPeerExt"; } virtual bool SendUpdate(const uint8_t *msg, size_t msgsize) { return true; } @@ -69,7 +73,7 @@ class BgpTestPeer : public IPeer { return internal_ ? BgpProto::IBGP : BgpProto::EBGP; } virtual uint32_t bgp_identifier() const { return 0; } - virtual const std::string GetStateName() const { return ""; } + virtual const string GetStateName() const { return ""; } virtual void UpdateRefCount(int count) const { } virtual tbb::atomic GetRefCount() const { tbb::atomic count; @@ -85,7 +89,7 @@ class BgpTestPeer : public IPeer { class BgpRouteMock : public BgpRoute { public: - virtual std::string ToString() const { return ""; } + virtual string ToString() const { return ""; } virtual int CompareTo(const Route &rhs) const { return 0; } virtual void SetKey(const DBRequestKey *key) { } virtual KeyPtr GetDBRequestKey() const { return KeyPtr(NULL); } @@ -97,7 +101,7 @@ class BgpRouteMock : public BgpRoute { class RTargetGroupMgrTest : public RTargetGroupMgr { public: - RTargetGroupMgrTest(BgpServer *server) : RTargetGroupMgr(server) { + explicit RTargetGroupMgrTest(BgpServer *server) : RTargetGroupMgr(server) { } virtual void GetRibOutInterestedPeers(RibOut *ribout, const ExtCommunity *ext_community, @@ -109,7 +113,6 @@ class RTargetGroupMgrTest : public RTargetGroupMgr { class BgpTableExportTest : public ::testing::Test { protected: - BgpTableExportTest() : server_(&evm_, "Local"), internal_(false), @@ -125,10 +128,10 @@ class BgpTableExportTest : public ::testing::Test { } virtual void SetUp() { - std::cout << "Table: " << table_name_ + cout << "Table: " << table_name_ << " Source: " << (internal_ ? "IBGP" : "EBGP") << " Local AS: " << (local_as_is_different_ ? 201 : 200) - << std::endl; + << endl; if (local_as_is_different_) { server_.set_local_autonomous_system(201); @@ -202,14 +205,15 @@ class BgpTableExportTest : public ::testing::Test { void CreateRibOut(BgpProto::BgpPeerType type, RibExportPolicy::Encoding encoding, as_t as_number = 0) { - RibExportPolicy policy(type, encoding, as_number, -1, 0); + RibExportPolicy policy(type, encoding, as_number, false, -1, 0); ribout_ = table_->RibOutLocate(&mgr_, policy); } void CreateRibOut(BgpProto::BgpPeerType type, RibExportPolicy::Encoding encoding, as_t as_number, - IpAddress nexthop) { - RibExportPolicy policy(type, encoding, as_number, nexthop, -1, 0); + bool as_override, IpAddress nexthop) { + RibExportPolicy policy( + type, encoding, as_number, as_override, nexthop, -1, 0); ribout_ = table_->RibOutLocate(&mgr_, policy); } @@ -307,6 +311,12 @@ class BgpTableExportTest : public ::testing::Test { EXPECT_EQ(med, attr->med()); } + void VerifyAttrAsPathCount(uint32_t count) { + const UpdateInfo &uinfo = uinfo_slist_->front(); + const BgpAttr *attr = uinfo.roattr.attr(); + EXPECT_EQ(count, attr->as_path_count()); + } + void VerifyAttrAsPrepend() { const UpdateInfo &uinfo = uinfo_slist_->front(); const BgpAttr *attr = uinfo.roattr.attr(); @@ -328,6 +338,13 @@ class BgpTableExportTest : public ::testing::Test { EXPECT_FALSE(as_path->path().AsLeftMostMatch(my_local_as)); } + void VerifyAttrNoAsPathLoop(as_t as_number) { + const UpdateInfo &uinfo = uinfo_slist_->front(); + const BgpAttr *attr = uinfo.roattr.attr(); + const AsPath *as_path = attr->as_path(); + EXPECT_FALSE(as_path->path().AsPathLoop(as_number, 0)); + } + void VerifyAttrExtCommunity(bool is_null) { const UpdateInfo &uinfo = uinfo_slist_->front(); const BgpAttr *attr = uinfo.roattr.attr(); @@ -350,14 +367,13 @@ class BgpTableExportTest : public ::testing::Test { BgpTable *table_; RibOut *ribout_; - std::auto_ptr peer_; + auto_ptr peer_; RibPeerSet active_peerset_; BgpRouteMock rt_; BgpAttrPtr attr_ptr_; UpdateInfoSList uinfo_slist_; bool result_; - }; // Parameterize table name, peer type and local AS. @@ -367,7 +383,6 @@ typedef std::tr1::tuple TestParams1; class BgpTableExportParamTest1 : public BgpTableExportTest, public ::testing::WithParamInterface { - virtual void SetUp() { table_name_ = std::tr1::get<0>(GetParam()); internal_ = std::tr1::get<1>(GetParam()); @@ -533,7 +548,6 @@ INSTANTIATE_TEST_CASE_P(Instance, BgpTableExportParamTest1, class BgpTableExportParamTest2 : public BgpTableExportTest, public ::testing::WithParamInterface { - virtual void SetUp() { table_name_ = GetParam(); internal_ = true; @@ -617,7 +631,6 @@ typedef std::tr1::tuple TestParams3; class BgpTableExportParamTest3 : public BgpTableExportTest, public ::testing::WithParamInterface { - virtual void SetUp() { table_name_ = std::tr1::get<0>(GetParam()); internal_ = false; @@ -765,7 +778,6 @@ typedef std::tr1::tuple TestParams4a; class BgpTableExportParamTest4a : public BgpTableExportTest, public ::testing::WithParamInterface { - virtual void SetUp() { table_name_ = "inet.0"; internal_ = std::tr1::get<0>(GetParam()); @@ -817,19 +829,54 @@ TEST_P(BgpTableExportParamTest4a, StripExtendedCommunity2) { // // Table : inet.0 // Source: eBGP, iBGP -// RibOut: eBGP with nexthop rewrite -// Intent: +// RibOut: eBGP with nexthop rewrite. +// Intent: Nexthop is rewritten on export. // TEST_P(BgpTableExportParamTest4a, RewriteNexthop) { boost::system::error_code ec; IpAddress nexthop = IpAddress::from_string("2.2.2.2", ec); - CreateRibOut(BgpProto::EBGP, RibExportPolicy::BGP, 300, nexthop); + CreateRibOut(BgpProto::EBGP, RibExportPolicy::BGP, 300, false, nexthop); + AddPath(); + RunExport(); + VerifyExportAccept(); + VerifyAttrNexthop("2.2.2.2"); +} + +// +// Table : inet.0 +// Source: eBGP, iBGP +// RibOut: eBGP with AS override. +// Intent: Ribout AS is overridden to local AS. +// +TEST_P(BgpTableExportParamTest4a, AsOverride) { + CreateRibOut(BgpProto::EBGP, RibExportPolicy::BGP, 100, true, IpAddress()); + AddPath(); + RunExport(); + VerifyExportAccept(); + VerifyAttrAsPrepend(); + VerifyAttrAsPathCount(PeerIsInternal() ? 1 : 2); + VerifyAttrNoAsPathLoop(100); +} + +// +// Table : inet.0 +// Source: eBGP, iBGP +// RibOut: eBGP with AS override and nexthop rewrite. +// Intent: Nexthop rewrite and AS override work together. +// +TEST_P(BgpTableExportParamTest4a, AsOverrideAndRewriteNexthop) { + boost::system::error_code ec; + IpAddress nexthop = IpAddress::from_string("2.2.2.2", ec); + CreateRibOut(BgpProto::EBGP, RibExportPolicy::BGP, 100, true, nexthop); SetAttrExtCommunity(12345); AddPath(); RunExport(); VerifyExportAccept(); VerifyAttrExtCommunity(true); VerifyAttrNexthop("2.2.2.2"); + VerifyAttrAsPrepend(); + VerifyAttrAsPathCount(PeerIsInternal() ? 1 : 2); + VerifyAttrNoAsPathLoop(100); } INSTANTIATE_TEST_CASE_P(Instance, BgpTableExportParamTest4a, @@ -843,7 +890,6 @@ INSTANTIATE_TEST_CASE_P(Instance, BgpTableExportParamTest4a, class BgpTableExportParamTest4b : public BgpTableExportTest, public ::testing::WithParamInterface { - virtual void SetUp() { table_name_ = "inet.0"; internal_ = GetParam(); @@ -921,7 +967,6 @@ typedef std::tr1::tuple TestParams5; class BgpTableExportParamTest5 : public BgpTableExportTest, public ::testing::WithParamInterface { - virtual void SetUp() { table_name_ = "bgp.l3vpn.0"; internal_ = std::tr1::get<0>(GetParam()); diff --git a/src/bgp/test/bgp_table_test.cc b/src/bgp/test/bgp_table_test.cc index d4556ef9b36..13f09edc394 100644 --- a/src/bgp/test/bgp_table_test.cc +++ b/src/bgp/test/bgp_table_test.cc @@ -11,8 +11,6 @@ #include "bgp/inet/inet_table.h" #include "db/db.h" -using namespace std; - class BgpTableTest : public ::testing::Test { protected: BgpTableTest() @@ -249,9 +247,12 @@ TEST_F(BgpTableTest, RiboutClusterId) { // Different AS numbers result in creation of different RibOuts. TEST_F(BgpTableTest, RiboutAS) { RibOut *ribout1 = NULL, *ribout2 = NULL, *ribout3 = NULL, *temp = NULL; - RibExportPolicy policy1(BgpProto::EBGP, RibExportPolicy::BGP, 101, -1, 0); - RibExportPolicy policy2(BgpProto::EBGP, RibExportPolicy::BGP, 102, -1, 0); - RibExportPolicy policy3(BgpProto::EBGP, RibExportPolicy::BGP, 103, -1, 0); + RibExportPolicy policy1( + BgpProto::EBGP, RibExportPolicy::BGP, 101, false, -1, 0); + RibExportPolicy policy2( + BgpProto::EBGP, RibExportPolicy::BGP, 102, false, -1, 0); + RibExportPolicy policy3( + BgpProto::EBGP, RibExportPolicy::BGP, 103, false, -1, 0); // Create 3 ribouts. ribout1 = rt_table_->RibOutLocate(&mgr_, policy1); @@ -285,6 +286,39 @@ TEST_F(BgpTableTest, RiboutAS) { ASSERT_TRUE(ribout3 == NULL); } +// AS Override enabled/disabled results in creation of different RibOuts. +TEST_F(BgpTableTest, RiboutASOverride) { + RibOut *ribout1 = NULL, *ribout2 = NULL, *temp = NULL; + RibExportPolicy policy1( + BgpProto::EBGP, RibExportPolicy::BGP, 100, true, -1, 0); + RibExportPolicy policy2( + BgpProto::EBGP, RibExportPolicy::BGP, 100, false, -1, 0); + + // Create 2 ribouts. + ribout1 = rt_table_->RibOutLocate(&mgr_, policy1); + ASSERT_TRUE(ribout1 != NULL); + ribout2 = rt_table_->RibOutLocate(&mgr_, policy2); + ASSERT_TRUE(ribout2 != NULL); + ASSERT_EQ(rt_table_->ribout_map().size(), 2); + + // Check if we can find them. + temp = rt_table_->RibOutFind(policy1); + ASSERT_EQ(temp, ribout1); + temp = rt_table_->RibOutFind(policy2); + ASSERT_EQ(temp, ribout2); + + // Delete all of them. + rt_table_->RibOutDelete(policy1); + rt_table_->RibOutDelete(policy2); + ASSERT_EQ(rt_table_->ribout_map().size(), 0); + + // Make sure they are all gone. + ribout1 = rt_table_->RibOutFind(policy1); + ASSERT_TRUE(ribout1 == NULL); + ribout2 = rt_table_->RibOutFind(policy2); + ASSERT_TRUE(ribout2 == NULL); +} + // Different nexthops result in creation of different RibOuts. TEST_F(BgpTableTest, RiboutNexthop) { boost::system::error_code ec; @@ -293,11 +327,11 @@ TEST_F(BgpTableTest, RiboutNexthop) { IpAddress nexthop3 = IpAddress::from_string("10.1.1.3", ec); RibOut *ribout1 = NULL, *ribout2 = NULL, *ribout3 = NULL, *temp = NULL; RibExportPolicy policy1( - BgpProto::EBGP, RibExportPolicy::BGP, 100, nexthop1, -1, 0); + BgpProto::EBGP, RibExportPolicy::BGP, 100, true, nexthop1, -1, 0); RibExportPolicy policy2( - BgpProto::EBGP, RibExportPolicy::BGP, 100, nexthop2, -1, 0); + BgpProto::EBGP, RibExportPolicy::BGP, 100, true, nexthop2, -1, 0); RibExportPolicy policy3( - BgpProto::EBGP, RibExportPolicy::BGP, 100, nexthop3, -1, 0); + BgpProto::EBGP, RibExportPolicy::BGP, 100, true, nexthop3, -1, 0); // Create 3 ribouts. ribout1 = rt_table_->RibOutLocate(&mgr_, policy1);