Merge "Trigger route aggregation on origin vn match"

Juniper · Feb 23, 2016 · 9850f2f · 9850f2f
2 parents 8529b29 + c5a18cf
commit 9850f2f
Show file tree

Hide file tree

Showing 4 changed files with 255 additions and 3 deletions.
diff --git a/src/bgp/routing-instance/route_aggregator.cc b/src/bgp/routing-instance/route_aggregator.cc
@@ -142,7 +142,8 @@ class AggregateRoute : public ConditionMatch {
         return false;
     }
 
-    bool IsBestMatch(BgpRoute *route);
+    bool IsOriginVnMatch(BgpRoute *route) const;
+    bool IsBestMatch(BgpRoute *route) const;
 
     virtual bool Match(BgpServer *server, BgpTable *table,
                        BgpRoute *route, bool deleted);
@@ -274,6 +275,31 @@ typename AggregateRoute<T>::CompareResult AggregateRoute<T>::CompareConfig(
     return NoChange;
 }
 
+template <typename T>
+bool AggregateRoute<T>::IsOriginVnMatch(BgpRoute *route) const {
+    const BgpPath *path = route->BestPath();
+    const BgpAttr *attr = path->GetAttr();
+    const ExtCommunity *ext_community = attr->ext_community();
+    int vni = 0;
+    if (ext_community) {
+        BOOST_FOREACH(const ExtCommunity::ExtCommunityValue &comm,
+                      ext_community->communities()) {
+            if (!ExtCommunity::is_origin_vn(comm)) continue;
+            OriginVn origin_vn(comm);
+            vni = origin_vn.vn_index();
+            break;
+        }
+    }
+
+    if (!vni && path->IsVrfOriginated())
+        vni = routing_instance()->virtual_network_index();
+
+    if (vni == routing_instance()->GetOriginVnForAggregateRoute(GetFamily()))
+        return true;
+
+    return false;
+}
+
 //
 // Calculate all aggregate prefixes to which the route can be contributing.
 // We need to calculate the longest prefix to which this route belongs.
@@ -282,7 +308,7 @@ typename AggregateRoute<T>::CompareResult AggregateRoute<T>::CompareConfig(
 // as so on
 //
 template <typename T>
-bool AggregateRoute<T>::IsBestMatch(BgpRoute *route) {
+bool AggregateRoute<T>::IsBestMatch(BgpRoute *route) const {
     const RouteT *ip_route = static_cast<RouteT *>(route);
     const PrefixT &ip_prefix = ip_route->GetPrefix();
     typename RouteAggregator<T>::AggregateRouteMap::const_iterator it;
@@ -312,8 +338,14 @@ bool AggregateRoute<T>::Match(BgpServer *server, BgpTable *table,
                    BgpRoute *route, bool deleted) {
     CHECK_CONCURRENCY("db::DBTable");
 
+    //
     // Only interested routes
-    if (!IsMoreSpecific(route)) return false;
+    // Should satisfy following conditions
+    //   1. Origin VN should match origin VN of aggregated route
+    //   2. Route should be more specific
+    //
+    if ((!deleted && !IsOriginVnMatch(route)) || !IsMoreSpecific(route))
+        return false;
 
     if (!deleted) {
         //

diff --git a/src/bgp/test/route_aggregator_test.cc b/src/bgp/test/route_aggregator_test.cc
@@ -518,6 +518,7 @@ TEST_F(RouteAggregatorTest, Basic) {
     task_util::WaitForIdle();
 }
 
+
 //
 // The nexthop is also a more specific route
 // Verify that the aggregate route is not published when only nexthop route is
@@ -1013,6 +1014,158 @@ TEST_F(RouteAggregatorTest, ServiceChain) {
     task_util::WaitForIdle();
 }
 
+//
+// Add route aggregate config to aggregate to 0/0 to routing instance with
+// service chain config
+// 1. Ensure that origin vn is set correctly on the aggregate route
+// 2. Ensure that contributing routes are considered only if the origin vn
+// matches dest VN of service chain
+//
+TEST_F(RouteAggregatorTest, ServiceChain_0) {
+    string content =
+        FileRead("controller/src/bgp/testdata/route_aggregate_4a.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)));
+
+    // Add connected routes
+    AddRoute<InetDefinition>(peers_[0], "red.inet.0", "1.1.1.3/32", 100);
+    AddRoute<InetDefinition>(peers_[0], "blue.inet.0", "1.1.1.3/32", 100);
+    task_util::WaitForIdle();
+
+    VERIFY_EQ(1, RouteCount("red.inet.0"));
+    VERIFY_EQ(1, RouteCount("blue.inet.0"));
+
+    // No route aggregation
+    BgpRoute *rt = RouteLookup<InetDefinition>("blue.inet.0", "0/0");
+    ASSERT_TRUE(rt == NULL);
+    rt = RouteLookup<InetDefinition>("red.inet.0", "0/0");
+    ASSERT_TRUE(rt == NULL);
+
+    // Add more specific route to RED
+    AddRoute<InetDefinition>(peers_[0], "red.inet.0", "2.2.2.1/32", 100);
+    task_util::WaitForIdle();
+
+    // Route aggregation is triggered in blue
+    VERIFY_EQ(3, RouteCount("blue.inet.0"));
+
+    // Route aggregation is NOT triggered in red
+    VERIFY_EQ(2, RouteCount("red.inet.0"));
+
+    rt = RouteLookup<InetDefinition>("blue.inet.0", "0/0");
+    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"));
+
+    TASK_UTIL_EXPECT_FALSE(IsContributingRoute<InetDefinition>("blue",
+                                            "blue.inet.0", "1.1.1.3/32"));
+
+    // Verify the sandesh
+    VerifyRouteAggregateSandesh("blue");
+
+    // Add more specific route to blue
+    AddRoute<InetDefinition>(peers_[0], "blue.inet.0", "1.1.1.1/32", 100);
+    // Delete the more specific route in red
+    DeleteRoute<InetDefinition>(peers_[0], "red.inet.0", "2.2.2.1/32");
+    task_util::WaitForIdle();
+
+    // service chain vm-route + connected route + aggregate route
+    VERIFY_EQ(3, RouteCount("red.inet.0"));
+
+    // Aggregated route is removed and Route Agggregation is not triggered on
+    // VM route from blue vn
+    VERIFY_EQ(2, RouteCount("blue.inet.0"));
+
+    rt = RouteLookup<InetDefinition>("red.inet.0", "0/0");
+    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"));
+
+    TASK_UTIL_EXPECT_FALSE(IsContributingRoute<InetDefinition>("red",
+                                            "red.inet.0", "1.1.1.3/32"));
+
+    // Verify the sandesh
+    VerifyRouteAggregateSandesh("red");
+
+    DeleteRoute<InetDefinition>(peers_[0], "red.inet.0", "1.1.1.3/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();
+}
+
+//
+// Route aggregation is triggered only when route's origin VN matches
+//
+TEST_F(RouteAggregatorTest, OriginVnCheck) {
+    string content =
+        FileRead("controller/src/bgp/testdata/route_aggregate_4b.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.254/32", 100);
+    task_util::WaitForIdle();
+
+    AddRoute<InetDefinition>(peers_[0], "blue.inet.0", "2.2.1.1/32", 100);
+    task_util::WaitForIdle();
+
+    VERIFY_EQ(2, RouteCount("red.inet.0"));
+    BgpRoute *rt = RouteLookup<InetDefinition>("red.inet.0", "0/0");
+    ASSERT_TRUE(rt == NULL);
+
+    AddRoute<InetDefinition>(peers_[0], "red.inet.0", "1.1.1.1/32", 100);
+    task_util::WaitForIdle();
+
+    VERIFY_EQ(4, RouteCount("red.inet.0"));
+    rt = RouteLookup<InetDefinition>("red.inet.0", "0/0");
+    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_FALSE(IsContributingRoute<InetDefinition>("red",
+                                            "red.inet.0", "2.2.1.1/32"));
+    TASK_UTIL_EXPECT_TRUE(IsContributingRoute<InetDefinition>("red",
+                                            "red.inet.0", "1.1.1.1/32"));
+
+    // Verify the sandesh
+    VerifyRouteAggregateSandesh("red");
+
+    DeleteRoute<InetDefinition>(peers_[0], "blue.inet.0", "2.2.1.1/32");
+    DeleteRoute<InetDefinition>(peers_[0], "red.inet.0", "1.1.1.1/32");
+    DeleteRoute<InetDefinition>(peers_[0], "red.inet.0", "1.1.1.254/32");
+    task_util::WaitForIdle();
+}
+
+
 //
 // Remove the route-aggregate object from routing instance and ensure that
 // aggregated route is deleted

diff --git a/src/bgp/testdata/route_aggregate_4a.xml b/src/bgp/testdata/route_aggregate_4a.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>0/0</route>
+        </aggregate-route-entries>
+        <nexthop>1.1.1.3</nexthop>
+    </route-aggregate>
+    <route-aggregate name='blue_subnet'>
+        <aggregate-route-entries>
+            <route>0/0</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>
diff --git a/src/bgp/testdata/route_aggregate_4b.xml b/src/bgp/testdata/route_aggregate_4b.xml
@@ -0,0 +1,24 @@
+<?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='vn_subnet'>
+        <aggregate-route-entries>
+            <route>0/0</route>
+        </aggregate-route-entries>
+        <nexthop>1.1.1.254</nexthop>
+    </route-aggregate>
+    <routing-instance name="red">
+        <virtual-network>red-vn</virtual-network>
+        <route-aggregate to="vn_subnet"/>
+        <vrf-target>target:1:103</vrf-target>
+    </routing-instance>
+    <routing-instance name="blue">
+        <virtual-network>blue-vn</virtual-network>
+        <vrf-target>target:1:103</vrf-target>
+    </routing-instance>
+</config>