Skip to content

Commit

Permalink
Route aggregation code review comments
Browse files Browse the repository at this point in the history
Handle review comments from Nischal and fix issues reported by cpplint

Change-Id: I5cebd064f0832ecb870ff872e40065b347e49203
Related-bug: #1500698
  • Loading branch information
bailkeri committed Jan 21, 2016
1 parent 575f1c9 commit b024037
Show file tree
Hide file tree
Showing 9 changed files with 148 additions and 99 deletions.
3 changes: 2 additions & 1 deletion src/bgp/bgp_condition_listener.cc
Original file line number Diff line number Diff line change
Expand Up @@ -350,7 +350,8 @@ bool BgpConditionListener::BgpRouteNotify(BgpServer *server,
DBEntryBase *entry) {
BgpTable *bgptable = static_cast<BgpTable *>(root->parent());
BgpRoute *rt = static_cast<BgpRoute *> (entry);
bool del_rt = rt->IsDeleted();
// Either the route is deleted or no valid path exists
bool del_rt = !rt->IsUsable();

TableMap::iterator loc = map_.find(bgptable);
assert(loc != map_.end());
Expand Down
5 changes: 3 additions & 2 deletions src/bgp/bgp_path.h
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@ class BgpPath : public Path {
ResolvedRoute = 2,
ServiceChain = 3,
StaticRoute = 4,
Aggregation = 5,
Aggregate = 5,
Local = 6,
};

Expand All @@ -61,7 +61,8 @@ class BgpPath : public Path {
bool IsVrfOriginated() const {
if (IsReplicated())
return false;
if (source_ != ResolvedRoute && source_ != BGP_XMPP && source_ != Local)
if (source_ != ResolvedRoute && source_ != Aggregate &&
source_ != BGP_XMPP && source_ != Local)
return false;
return true;
}
Expand Down
4 changes: 4 additions & 0 deletions src/bgp/bgp_route.cc
Original file line number Diff line number Diff line change
Expand Up @@ -299,6 +299,10 @@ void BgpRoute::FillRouteInfo(const BgpTable *table,
srp.set_protocol("ServiceChain");
} else if (path->GetSource() == BgpPath::StaticRoute) {
srp.set_protocol("StaticRoute");
} else if (path->GetSource() == BgpPath::Aggregate) {
srp.set_protocol("Aggregate");
} else if (path->GetSource() == BgpPath::ResolvedRoute) {
srp.set_protocol("ResolvedRoute");
}

const BgpAttr *attr = path->GetAttr();
Expand Down
20 changes: 14 additions & 6 deletions src/bgp/bgp_table.cc
Original file line number Diff line number Diff line change
Expand Up @@ -134,12 +134,10 @@ UpdateInfo *BgpTable::GetUpdateInfo(RibOut *ribout, BgpRoute *route,
if (!path->IsFeasible())
return NULL;

if (IsRouteAggregationSupported()) {
// Check whether the route is contributing route
if (routing_instance()->IsContributingRoute(this, route)) {
return NULL;
}
}
// Check whether the route is contributing route
if (IsRouteAggregationSupported() && IsContributingRoute(route))
return NULL;

// Needs to be outside the if block so it's not destroyed prematurely.
BgpAttrPtr attr_ptr;
const BgpAttr *attr = path->GetAttr();
Expand Down Expand Up @@ -544,3 +542,13 @@ void BgpTable::UpdatePathCount(const BgpPath *path, int count) {
infeasible_path_count_ += count;
}
}

// Check whether the route is aggregate route
bool BgpTable::IsAggregateRoute(const BgpRoute *route) const {
return routing_instance()->IsAggregateRoute(this, route);
}

// Check whether the route is contributing route to aggregate route
bool BgpTable::IsContributingRoute(const BgpRoute *route) const {
return routing_instance()->IsContributingRoute(this, route);
}
6 changes: 6 additions & 0 deletions src/bgp/bgp_table.h
Original file line number Diff line number Diff line change
Expand Up @@ -132,6 +132,12 @@ class BgpTable : public RouteTable {
return infeasible_path_count_;
}

// Check whether the route is aggregate route
bool IsAggregateRoute(const BgpRoute *route) const;

// Check whether the route is contributing route to aggregate route
bool IsContributingRoute(const BgpRoute *route) const;

private:
class DeleteActor;
friend class BgpTableTest;
Expand Down

0 comments on commit b024037

Please sign in to comment.