Skip to content

Commit

Permalink
Use subset of rtarget eBGP paths when advertising vpn routes
Browse files Browse the repository at this point in the history
Only consider paths that are learnt from same neighbor AS as
best eBGP path.

Note that different RTC routes for the same RT can and will
result in VPN routes being advertised to more than 1 AS. This
is correct behavior.

Change-Id: I7a08c452c9ee5788fdd615e58a096796752070e0
Closes-Bug: 1582933
  • Loading branch information
Nischal Sheth committed May 17, 2016
1 parent 7b17cee commit 355b4db
Show file tree
Hide file tree
Showing 4 changed files with 49 additions and 0 deletions.
9 changes: 9 additions & 0 deletions src/bgp/bgp_path.cc
Expand Up @@ -125,6 +125,15 @@ int BgpPath::PathCompare(const BgpPath &rhs, bool allow_ecmp) const {
return 0;
}

bool BgpPath::PathSameNeighborAs(const BgpPath &rhs) const {
const BgpAttr *rattr = rhs.GetAttr();
if (!peer_ || peer_->PeerType() != BgpProto::EBGP)
return false;
if (!rhs.peer_ || rhs.peer_->PeerType() != BgpProto::EBGP)
return false;
return (attr_->neighbor_as() == rattr->neighbor_as());
}

BgpSecondaryPath::BgpSecondaryPath(const IPeer *peer, uint32_t path_id,
PathSource src, const BgpAttrPtr ptr, uint32_t flags, uint32_t label)
: BgpPath(peer, path_id, src, ptr, flags, label) {
Expand Down
1 change: 1 addition & 0 deletions src/bgp/bgp_path.h
Expand Up @@ -123,6 +123,7 @@ class BgpPath : public Path {

// Select one path over other
int PathCompare(const BgpPath &rhs, bool allow_ecmp) const;
bool PathSameNeighborAs(const BgpPath &rhs) const;

private:
const IPeer *peer_;
Expand Down
9 changes: 9 additions & 0 deletions src/bgp/routing-instance/rtarget_group_mgr.cc
Expand Up @@ -118,6 +118,7 @@ void RTargetGroupMgr::BuildRTargetDistributionGraph(BgpTable *table,
return;
}

const BgpPath *best_ebgp_path = NULL;
for (Route::PathList::iterator it = rt->GetPathList().begin();
it != rt->GetPathList().end(); it++) {
BgpPath *path = static_cast<BgpPath *>(it.operator->());
Expand All @@ -127,6 +128,14 @@ void RTargetGroupMgr::BuildRTargetDistributionGraph(BgpTable *table,
continue;

const BgpPeer *peer = static_cast<const BgpPeer *>(path->GetPeer());
if (peer->PeerType() == BgpProto::EBGP) {
if (!best_ebgp_path) {
best_ebgp_path = path;
} else if (!best_ebgp_path->PathSameNeighborAs(*path)) {
continue;
}
}

std::pair<RtGroup::InterestedPeerList::iterator, bool> ret =
peer_list.insert(std::pair<const BgpPeer *,
RtGroup::RTargetRouteList>(peer, RtGroup::RTargetRouteList()));
Expand Down
30 changes: 30 additions & 0 deletions src/bgp/test/bgp_xmpp_rtarget_test.cc
Expand Up @@ -3366,6 +3366,36 @@ TEST_F(BgpXmppRTargetTest, PathSelection5) {
DeleteInetRoute(mx_.get(), NULL, "blue", BuildPrefix());
}

//
// If RTargetRoute is received from EBGP peers in different ASes, VPN routes
// should be sent to peers in AS of the best EBGP path.
//
TEST_F(BgpXmppRTargetTest, PathSelection6) {
Unconfigure();
task_util::WaitForIdle();
Configure(64496, 64497, 64498);
agent_a_1_->Subscribe("blue", 1);

AddRouteTarget(cn1_.get(), "blue", "target:1:1001");
AddRouteTarget(cn2_.get(), "blue", "target:1:1001");
task_util::WaitForIdle();

RTargetRoute *rtgt_rt =
VerifyRTargetRouteExists(mx_.get(), "64496:target:1:1001");
TASK_UTIL_EXPECT_EQ(2, rtgt_rt->count());
VerifyRTargetRouteNoExists(mx_.get(), "64497:target:1:1001");

AddInetRoute(mx_.get(), NULL, "blue", BuildPrefix());

BgpRoute *inet_rt =
VerifyInetRouteExists(cn1_.get(), "blue", BuildPrefix());
usleep(50000);
TASK_UTIL_EXPECT_EQ(1, inet_rt->count());
VerifyInetRouteNoExists(cn2_.get(), "blue", BuildPrefix());

DeleteInetRoute(mx_.get(), NULL, "blue", BuildPrefix());
}

//
// Local AS is different than the global AS.
// Verify that RTarget route prefixes use local AS while the route target
Expand Down

0 comments on commit 355b4db

Please sign in to comment.