Include ORIGIN and MED in ECMP decision

Note that this changes ecmp behavior from older releases, but this is
deemed to be fine since the new behavior is what most users will expect
by default.

Change-Id: Iff5834710153e1315269f58662d58e647be6c9ce
Closes-Bug: 1586873

src/bgp/bgp_path.cc

@@ -63,16 +63,16 @@ int BgpPath::PathCompare(const BgpPath &rhs, bool allow_ecmp) const {
 
     KEY_COMPARE(attr_->as_path_count(), rattr->as_path_count());
 
-    // For ECMP paths, above checks should suffice.
-    if (allow_ecmp)
-        return 0;
-
     KEY_COMPARE(attr_->origin(), rattr->origin());
 
     // Compare med if both paths are learnt from the same neighbor as.
     if (attr_->neighbor_as() && attr_->neighbor_as() == rattr->neighbor_as())
         KEY_COMPARE(attr_->med(), rattr->med());
 
+    // For ECMP paths, above checks should suffice.
+    if (allow_ecmp)
+        return 0;
+
     // Prefer locally generated routes over bgp and xmpp routes.
     BOOL_COMPARE(peer_ == NULL, rhs.peer_ == NULL);
 

src/bgp/test/bgp_route_test.cc

@@ -387,6 +387,7 @@ TEST_F(BgpRouteTest, PathCompareClusterList2) {
 
 //
 // Path with lower med is better.
+// Paths with different MEDs are not considered ECMP.
 //
 TEST_F(BgpRouteTest, PathCompareMed1) {
     boost::system::error_code ec;
@@ -422,12 +423,15 @@ TEST_F(BgpRouteTest, PathCompareMed1) {
 
     EXPECT_EQ(-1, path1.PathCompare(path2, false));
     EXPECT_EQ(1, path2.PathCompare(path1, false));
+    EXPECT_EQ(-1, path1.PathCompare(path2, true));
+    EXPECT_EQ(1, path2.PathCompare(path1, true));
 }
 
 //
 // Paths with different neighbor as are not compared w.r.t med.
 // Path with higher med happens to win since it has lower router id.
 // Both paths have as paths, but the leftmost as is different.
+// Paths with different MEDs are considered ECMP as leftmost AS is different.
 //
 TEST_F(BgpRouteTest, PathCompareMed2) {
     boost::system::error_code ec;
@@ -463,12 +467,15 @@ TEST_F(BgpRouteTest, PathCompareMed2) {
 
     EXPECT_EQ(1, path1.PathCompare(path2, false));
     EXPECT_EQ(-1, path2.PathCompare(path1, false));
+    EXPECT_EQ(0, path1.PathCompare(path2, true));
+    EXPECT_EQ(0, path2.PathCompare(path1, true));
 }
 
 //
 // Paths with different neighbor as are not compared w.r.t med.
 // Path with higher med happens to win since it has lower router id.
 // Both paths have nil as path.
+// Paths with different MEDs are considered ECMP as leftmost AS is 0.
 //
 TEST_F(BgpRouteTest, PathCompareMed3) {
     boost::system::error_code ec;
@@ -494,12 +501,15 @@ TEST_F(BgpRouteTest, PathCompareMed3) {
 
     EXPECT_EQ(1, path1.PathCompare(path2, false));
     EXPECT_EQ(-1, path2.PathCompare(path1, false));
+    EXPECT_EQ(0, path1.PathCompare(path2, true));
+    EXPECT_EQ(0, path2.PathCompare(path1, true));
 }
 
 //
 // Paths with 0 neighbor as are not compared w.r.t med.
 // Path with higher med happens to win since it has lower router id.
 // First segment in both paths is an AS_SET.
+// Paths with different MEDs are considered ECMP as leftmost AS is 0.
 //
 TEST_F(BgpRouteTest, PathCompareMed4) {
     boost::system::error_code ec;
@@ -535,6 +545,8 @@ TEST_F(BgpRouteTest, PathCompareMed4) {
 
     EXPECT_EQ(1, path1.PathCompare(path2, false));
     EXPECT_EQ(-1, path2.PathCompare(path1, false));
+    EXPECT_EQ(0, path1.PathCompare(path2, true));
+    EXPECT_EQ(0, path2.PathCompare(path1, true));
 }
 
 static void SetUp() {