OriginVn for aggregate route

Currently origin vn is not set for aggregate route.

Consider the following example:
  1. left-vn -- Service Chain -- Right-vn
  2. Internal routing instance created for service chain has route aggregate config
  3. Aggregate route for right-vn subnet is added to left-vn

Since the aggregate route doesn't have origin vn, it gets leaked backed to right
network as service chain route.

This can cause black-holing of traffic in right due to wrong next hop for right-vn
subnet route

Fix:
Add origin VN for aggregate route.
  a. If the routing instance has service chain config, origin vn is the
  dest-routing-instance
  b. Else origin-vn is routing instance's virtual network index
Re-order origin-vn calculation in service chain code to consider origin-vn
present in the BgpAttr(extCommunity)
Added test in route aggregate to validate service chain scenario and origin vn

Change-Id: I9f1ffb6fedb816eac7b5f070a965cf202b96cb49
Related-bug: #1500698

src/bgp/routing-instance/route_aggregator.cc

@@ -13,6 +13,7 @@
 #include "base/lifetime.h"
 #include "base/map_util.h"
 #include "base/task_annotations.h"
+#include "bgp/origin-vn/origin_vn.h"
 #include "bgp/routing-instance/path_resolver.h"
 #include "bgp/routing-instance/routing_instance.h"
 #include "bgp/routing-instance/route_aggregate_types.h"
@@ -380,6 +381,11 @@ void AggregateRoute<T>::AddAggregateRoute() {
     BgpAttrSpec attrs;
     BgpAttrNextHop attr_nexthop(this->GetAddress(nexthop()));
     attrs.push_back(&attr_nexthop);
+    ExtCommunitySpec extcomm_spec;
+    OriginVn origin_vn(routing_instance()->server()->autonomous_system(),
+        routing_instance()->GetOriginVnForAggregateRoute(GetFamily()));
+    extcomm_spec.communities.push_back(origin_vn.GetExtCommunityValue());
+    attrs.push_back(&extcomm_spec);
     BgpAttrPtr attr = routing_instance()->server()->attr_db()->Locate(attrs);
     BgpPath *new_path = new BgpPath(BgpPath::Aggregate,
                                     attr.get(), BgpPath::ResolveNexthop, 0);
@@ -411,6 +417,11 @@ void AggregateRoute<T>::UpdateAggregateRoute() {
     BgpAttrSpec attrs;
     BgpAttrNextHop attr_nexthop(this->GetAddress(nexthop()));
     attrs.push_back(&attr_nexthop);
+    ExtCommunitySpec extcomm_spec;
+    OriginVn origin_vn(routing_instance()->server()->autonomous_system(),
+        routing_instance()->GetOriginVnForAggregateRoute(GetFamily()));
+    extcomm_spec.communities.push_back(origin_vn.GetExtCommunityValue());
+    attrs.push_back(&extcomm_spec);
     BgpAttrPtr attr = routing_instance()->server()->attr_db()->Locate(attrs);
     BgpPath *new_path = new BgpPath(BgpPath::Aggregate,
                                     attr.get(), BgpPath::ResolveNexthop, 0);

src/bgp/routing-instance/routepath_replicator.cc

@@ -582,7 +582,7 @@ bool RoutePathReplicator::RouteListener(TableState *ts,
             continue;
 
         // Add OriginVn when replicating self-originated routes from a VRF.
-        if (!rtinstance->IsDefaultRoutingInstance() &&
+        if (!vn_index && !rtinstance->IsDefaultRoutingInstance() &&
             path->IsVrfOriginated() && rtinstance->virtual_network_index()) {
             vn_index = rtinstance->virtual_network_index();
             OriginVn origin_vn(server_->autonomous_system(), vn_index);

src/bgp/routing-instance/routing_instance.cc

@@ -1234,3 +1234,14 @@ bool RoutingInstance::IsContributingRoute(const BgpTable *table,
     if (!route_aggregator(table->family())) return false;
     return route_aggregator(table->family())->IsContributingRoute(route);
 }
+
+int RoutingInstance::GetOriginVnForAggregateRoute(Address::Family fmly) const {
+    const ServiceChainConfig *sc_config =
+        config_ ? config_->service_chain_info(fmly) : NULL;
+    if (sc_config && !sc_config->routing_instance.empty()) {
+        RoutingInstance *dest =
+            mgr_->GetRoutingInstance(sc_config->routing_instance);
+        if (dest) return dest->virtual_network_index();
+    }
+    return virtual_network_index_;
+}

src/bgp/routing-instance/routing_instance.h

@@ -168,6 +168,8 @@ class RoutingInstance {
     bool IsContributingRoute(const BgpTable *table,
                              const BgpRoute *route) const;
 
+    int GetOriginVnForAggregateRoute(Address::Family family) const;
+
 private:
     friend class RoutingInstanceMgr;
     class DeleteActor;

src/bgp/routing-instance/service_chaining.cc

@@ -37,22 +37,19 @@ static int GetOriginVnIndex(const BgpTable *table, const BgpRoute *route) {
     if (!path)
         return 0;
 
-    if (path->IsVrfOriginated())
-        return table->routing_instance()->virtual_network_index();
-
     const BgpAttr *attr = path->GetAttr();
     const ExtCommunity *ext_community = attr->ext_community();
-    if (!ext_community)
-        return 0;
-
-    BOOST_FOREACH(const ExtCommunity::ExtCommunityValue &comm,
-                  ext_community->communities()) {
-        if (!ExtCommunity::is_origin_vn(comm))
-            continue;
-        OriginVn origin_vn(comm);
-        return origin_vn.vn_index();
+    if (ext_community) {
+        BOOST_FOREACH(const ExtCommunity::ExtCommunityValue &comm,
+                      ext_community->communities()) {
+            if (!ExtCommunity::is_origin_vn(comm))
+                continue;
+            OriginVn origin_vn(comm);
+            return origin_vn.vn_index();
+        }
     }
-
+    if (path->IsVrfOriginated())
+        return table->routing_instance()->virtual_network_index();
     return 0;
 }
 
@@ -988,8 +985,13 @@ bool ServiceChainMgr<T>::LocateServiceChain(RoutingInstance *rtinstance,
 
     RoutingInstanceMgr *mgr = server_->routing_instance_mgr();
     RoutingInstance *dest = mgr->GetRoutingInstance(config.routing_instance);
+    //
     // Destination routing instance is not yet created.
-    if (dest == NULL || dest->deleted()) {
+    // Or Destination routing instance is deleted Or
+    // virtual network index is not yet calculated (due missing virtual network
+    // link)
+    //
+    if (dest == NULL || dest->deleted() || !dest->virtual_network_index()) {
         // Wait for the creation of RoutingInstance
         AddPendingServiceChain(rtinstance);
         return false;

src/bgp/test/route_aggregator_test.cc

@@ -314,6 +314,20 @@ class RouteAggregatorTest : public ::testing::Test {
         return list;
     }
 
+    string GetOriginVnFromRoute(const BgpPath *path) {
+        const ExtCommunity *ext_comm = path->GetAttr()->ext_community();
+        assert(ext_comm);
+        BOOST_FOREACH(const ExtCommunity::ExtCommunityValue &comm,
+                      ext_comm->communities()) {
+            if (!ExtCommunity::is_origin_vn(comm))
+                continue;
+            OriginVn origin_vn(comm);
+            RoutingInstanceMgr *ri_mgr = bgp_server_->routing_instance_mgr();
+            return ri_mgr->GetVirtualNetworkByVnIndex(origin_vn.vn_index());
+        }
+        return "unresolved";
+    }
+
     void DisableUnregResolveTask(const string &instance, Address::Family fmly) {
         RoutingInstance *rti =
             bgp_server_->routing_instance_mgr()->GetRoutingInstance(instance);
@@ -888,6 +902,64 @@ TEST_F(RouteAggregatorTest, Basic_LastMoreSpecificDelete) {
     task_util::WaitForIdle();
 }
 
+//
+// Add route aggregate config to routing instance with service chain config
+// Ensure that origin vn is set correctly on the aggregate route
+//
+TEST_F(RouteAggregatorTest, ServiceChain) {
+    string content =
+        FileRead("controller/src/bgp/testdata/route_aggregate_4.xml");
+    EXPECT_TRUE(parser_.Parse(content));
+    task_util::WaitForIdle();
+
+    boost::system::error_code ec;
+    peers_.push_back(
+        new BgpPeerMock(Ip4Address::from_string("192.168.0.1", ec)));
+
+    AddRoute<InetDefinition>(peers_[0], "red.inet.0", "1.1.1.3/32", 100);
+    AddRoute<InetDefinition>(peers_[0], "red.inet.0", "2.2.2.1/32", 100);
+    AddRoute<InetDefinition>(peers_[0], "blue.inet.0", "1.1.1.3/32", 100);
+    AddRoute<InetDefinition>(peers_[0], "blue.inet.0", "1.1.1.1/32", 100);
+    task_util::WaitForIdle();
+    VERIFY_EQ(4, RouteCount("red.inet.0"));
+    VERIFY_EQ(4, RouteCount("blue.inet.0"));
+    BgpRoute *rt = RouteLookup<InetDefinition>("blue.inet.0", "2.2.2.0/24");
+    ASSERT_TRUE(rt != NULL);
+    TASK_UTIL_EXPECT_EQ(rt->count(), 2);
+    TASK_UTIL_EXPECT_TRUE(rt->BestPath() != NULL);
+    TASK_UTIL_EXPECT_TRUE(rt->BestPath()->IsFeasible());
+    TASK_UTIL_EXPECT_TRUE(GetOriginVnFromRoute(rt->BestPath()) == "red-vn");
+    rt = RouteLookup<InetDefinition>("blue.inet.0", "2.2.2.1/32");
+    ASSERT_TRUE(rt != NULL);
+    TASK_UTIL_EXPECT_EQ(rt->count(), 1);
+    TASK_UTIL_EXPECT_TRUE(rt->BestPath() != NULL);
+    TASK_UTIL_EXPECT_TRUE(rt->BestPath()->IsFeasible());
+    TASK_UTIL_EXPECT_TRUE(rt->BestPath()->GetSource() == BgpPath::ServiceChain);
+    TASK_UTIL_EXPECT_TRUE(IsContributingRoute<InetDefinition>("blue",
+                                            "blue.inet.0", "2.2.2.1/32"));
+
+    rt = RouteLookup<InetDefinition>("red.inet.0", "1.1.1.0/24");
+    ASSERT_TRUE(rt != NULL);
+    TASK_UTIL_EXPECT_EQ(rt->count(), 2);
+    TASK_UTIL_EXPECT_TRUE(rt->BestPath() != NULL);
+    TASK_UTIL_EXPECT_TRUE(rt->BestPath()->IsFeasible());
+    TASK_UTIL_EXPECT_TRUE(GetOriginVnFromRoute(rt->BestPath()) == "blue-vn");
+    rt = RouteLookup<InetDefinition>("red.inet.0", "1.1.1.1/32");
+    ASSERT_TRUE(rt != NULL);
+    TASK_UTIL_EXPECT_EQ(rt->count(), 1);
+    TASK_UTIL_EXPECT_TRUE(rt->BestPath() != NULL);
+    TASK_UTIL_EXPECT_TRUE(rt->BestPath()->IsFeasible());
+    TASK_UTIL_EXPECT_TRUE(rt->BestPath()->GetSource() == BgpPath::ServiceChain);
+    TASK_UTIL_EXPECT_TRUE(IsContributingRoute<InetDefinition>("red",
+                                            "red.inet.0", "1.1.1.1/32"));
+
+    DeleteRoute<InetDefinition>(peers_[0], "red.inet.0", "1.1.1.3/32");
+    DeleteRoute<InetDefinition>(peers_[0], "red.inet.0", "2.2.2.1/32");
+    DeleteRoute<InetDefinition>(peers_[0], "blue.inet.0", "1.1.1.1/32");
+    DeleteRoute<InetDefinition>(peers_[0], "blue.inet.0", "1.1.1.3/32");
+    task_util::WaitForIdle();
+}
+
 //
 // Remove the route-aggregate object from routing instance and ensure that
 // aggregated route is deleted

src/bgp/testdata/route_aggregate_4.xml

@@ -0,0 +1,43 @@
+<?xml version="1.0" encoding="utf-8"?>
+<config>
+    <virtual-network name="blue-vn">
+        <network-id>2000</network-id>
+    </virtual-network>
+    <virtual-network name="red-vn">
+        <network-id>1000</network-id>
+    </virtual-network>
+    <route-aggregate name='red_subnet'>
+        <aggregate-route-entries>
+            <route>2.2.2.0/24</route>
+        </aggregate-route-entries>
+        <nexthop>1.1.1.3</nexthop>
+    </route-aggregate>
+    <route-aggregate name='blue_subnet'>
+        <aggregate-route-entries>
+            <route>1.1.1.0/24</route>
+        </aggregate-route-entries>
+        <nexthop>1.1.1.3</nexthop>
+    </route-aggregate>
+    <routing-instance name="red">
+        <virtual-network>red-vn</virtual-network>
+        <service-chain-info>
+            <routing-instance>blue</routing-instance>
+            <source-routing-instance>red</source-routing-instance>
+            <prefix>1.1.1.0/24</prefix>
+            <service-chain-address>1.1.1.3</service-chain-address>
+        </service-chain-info>
+        <vrf-target>target:1:103</vrf-target>
+        <route-aggregate to="blue_subnet"/>
+    </routing-instance>
+    <routing-instance name="blue">
+        <virtual-network>blue-vn</virtual-network>
+        <service-chain-info>
+            <routing-instance>red</routing-instance>
+            <source-routing-instance>blue</source-routing-instance>
+            <prefix>2.2.2.0/24</prefix>
+            <service-chain-address>1.1.1.3</service-chain-address>
+        </service-chain-info>
+        <vrf-target>target:1:104</vrf-target>
+        <route-aggregate to="red_subnet"/>
+    </routing-instance>
+</config>