Merge "Route aggregation code review comments"

src/bgp/bgp_condition_listener.cc

@@ -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());

src/bgp/bgp_path.h

@@ -34,7 +34,7 @@ class BgpPath : public Path {
         ResolvedRoute = 2,
         ServiceChain = 3,
         StaticRoute = 4,
-        Aggregation = 5,
+        Aggregate = 5,
         Local = 6,
     };
 
@@ -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;
     }

src/bgp/bgp_route.cc

@@ -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();

src/bgp/bgp_table.cc

@@ -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();
@@ -548,3 +546,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);
+}

src/bgp/bgp_table.h

@@ -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;