Do not resolve paths with nexthop same as prefix

Change-Id: I10d8f51738eddcd8a62c31afddef19a9f41b1934 Closes-Bug: 1585832
Juniper · May 27, 2016 · 5aaca18 · 5aaca18
1 parent ca45ffe
commit 5aaca18
Show file tree

Hide file tree

Showing 2 changed files with 90 additions and 13 deletions.
diff --git a/src/bgp/routing-instance/path_resolver.cc b/src/bgp/routing-instance/path_resolver.cc
@@ -22,6 +22,27 @@ using std::make_pair;
 using std::string;
 using std::vector;
 
+//
+// Return true if the prefix for the BgpRoute is the same as given IpAddress.
+//
+static bool RoutePrefixIsAddress(Address::Family family, const BgpRoute *route,
+    const IpAddress &address) {
+    if (family == Address::INET) {
+        const InetRoute *inet_route = static_cast<const InetRoute *>(route);
+        if (inet_route->GetPrefix().addr() == address.to_v4() &&
+            inet_route->GetPrefix().prefixlen() == Address::kMaxV4PrefixLen) {
+            return true;
+        }
+    } else if (family == Address::INET6) {
+        const Inet6Route *inet6_route = static_cast<const Inet6Route *>(route);
+        if (inet6_route->GetPrefix().addr() == address.to_v6() &&
+            inet6_route->GetPrefix().prefixlen() == Address::kMaxV6PrefixLen) {
+            return true;
+        }
+    }
+    return false;
+}
+
 class PathResolver::DeleteActor : public LifetimeActor {
 public:
     explicit DeleteActor(PathResolver *resolver)
@@ -604,7 +625,12 @@ void PathResolverPartition::StartPathResolution(const BgpPath *path,
         return;
     if (table()->IsDeleted() || nh_table->IsDeleted())
         return;
+
+    Address::Family family = table()->family();
     IpAddress address = path->GetAttr()->nexthop();
+    if (table() == nh_table && RoutePrefixIsAddress(family, route, address))
+        return;
+
     ResolverNexthop *rnexthop =
         resolver_->LocateResolverNexthop(address, nh_table);
     assert(!FindResolverPath(path));
@@ -1053,19 +1079,8 @@ bool ResolverNexthop::Match(BgpServer *server, BgpTable *table,
     // Ignore if the route doesn't match the address.
     Address::Family family = table->family();
     assert(family == Address::INET || family == Address::INET6);
-    if (family == Address::INET) {
-        const InetRoute *inet_route = static_cast<InetRoute *>(route);
-        if (inet_route->GetPrefix().addr() != address_.to_v4() ||
-            inet_route->GetPrefix().prefixlen() != Address::kMaxV4PrefixLen) {
-            return false;
-        }
-    } else if (family == Address::INET6) {
-        const Inet6Route *inet6_route = static_cast<Inet6Route *>(route);
-        if (inet6_route->GetPrefix().addr() != address_.to_v6() ||
-            inet6_route->GetPrefix().prefixlen() != Address::kMaxV6PrefixLen) {
-            return false;
-        }
-    }
+    if (!RoutePrefixIsAddress(family, route, address_))
+        return false;
 
     // Set or remove MatchState as appropriate.
     BgpConditionListener *condition_listener =

diff --git a/src/bgp/test/path_resolver_test.cc b/src/bgp/test/path_resolver_test.cc
@@ -1841,6 +1841,68 @@ TYPED_TEST(PathResolverTest, SinglePrefixWithMultipathAndEcmp2) {
     this->DeleteBgpPath(bgp_peer2, "blue", this->BuildPrefix(1));
 }
 
+//
+// Do not resolve BGP path with prefix which is same as nexthop.
+// Add XMPP path before BGP path.
+//
+TYPED_TEST(PathResolverTest, Recursion1) {
+    if (this->GetFamily() == Address::INET && !nexthop_family_is_inet)
+        return;
+    if (this->GetFamily() == Address::INET6 && nexthop_family_is_inet)
+        return;
+
+    PeerMock *bgp_peer1 = this->bgp_peer1_;
+    PeerMock *xmpp_peer1 = this->xmpp_peer1_;
+
+    this->AddXmppPath(xmpp_peer1, "blue",
+        this->BuildPrefix(bgp_peer1->ToString(), 32),
+        this->BuildNextHopAddress("172.16.1.1"), 10000);
+    this->AddBgpPath(bgp_peer1, "blue",
+        this->BuildPrefix(bgp_peer1->ToString(), 32),
+        this->BuildHostAddress(bgp_peer1->ToString()));
+
+    task_util::WaitForIdle();
+    this->VerifyPathNoExists("blue",
+        this->BuildPrefix(bgp_peer1->ToString(), 32),
+        this->BuildNextHopAddress("172.16.1.1"));
+
+    this->DeleteBgpPath(bgp_peer1, "blue",
+        this->BuildPrefix(bgp_peer1->ToString(), 32));
+    this->DeleteXmppPath(xmpp_peer1, "blue",
+        this->BuildPrefix(bgp_peer1->ToString(), 32));
+}
+
+//
+// Do not resolve BGP path with prefix which is same as nexthop.
+// Add BGP path before XMPP path.
+//
+TYPED_TEST(PathResolverTest, Recursion2) {
+    if (this->GetFamily() == Address::INET && !nexthop_family_is_inet)
+        return;
+    if (this->GetFamily() == Address::INET6 && nexthop_family_is_inet)
+        return;
+
+    PeerMock *bgp_peer1 = this->bgp_peer1_;
+    PeerMock *xmpp_peer1 = this->xmpp_peer1_;
+
+    this->AddBgpPath(bgp_peer1, "blue",
+        this->BuildPrefix(bgp_peer1->ToString(), 32),
+        this->BuildHostAddress(bgp_peer1->ToString()));
+    this->AddXmppPath(xmpp_peer1, "blue",
+        this->BuildPrefix(bgp_peer1->ToString(), 32),
+        this->BuildNextHopAddress("172.16.1.1"), 10000);
+
+    task_util::WaitForIdle();
+    this->VerifyPathNoExists("blue",
+        this->BuildPrefix(bgp_peer1->ToString(), 32),
+        this->BuildNextHopAddress("172.16.1.1"));
+
+    this->DeleteXmppPath(xmpp_peer1, "blue",
+        this->BuildPrefix(bgp_peer1->ToString(), 32));
+    this->DeleteBgpPath(bgp_peer1, "blue",
+        this->BuildPrefix(bgp_peer1->ToString(), 32));
+}
+
 //
 // BGP has multiple prefixes, each with the same nexthop.
 //