Merge "Treat VN index >= 8000000 as global"

src/bgp/bgp_origin_vn_path.cc

@@ -9,6 +9,7 @@
 #include <string>
 
 #include "bgp/bgp_proto.h"
+#include "bgp/origin-vn/origin_vn.h"
 
 using std::string;
 using std::vector;
@@ -59,10 +60,17 @@ void OriginVnPath::Prepend(const OriginVnValue &value) {
 }
 
 bool OriginVnPath::Contains(const OriginVnValue &val) const {
+    OriginVn in_origin_vn(val);
+    int in_vn_index = in_origin_vn.IsGlobal() ? in_origin_vn.vn_index() : 0;
     for (OriginVnList::const_iterator it = origin_vns_.begin();
          it != origin_vns_.end(); ++it) {
         if (*it == val)
             return true;
+        if (in_vn_index == 0)
+            continue;
+        OriginVn origin_vn(*it);
+        if (origin_vn.vn_index() == in_vn_index)
+            return true;
     }
     return false;
 }

src/bgp/origin-vn/origin_vn.cc

@@ -110,6 +110,10 @@ int OriginVn::vn_index() const {
     return 0;
 }
 
+bool OriginVn::IsGlobal() const {
+    return (vn_index() >= kMinGlobalId);
+}
+
 string OriginVn::ToString() {
     char temp[50];
     snprintf(temp, sizeof(temp), "originvn:%u:%u", as_number(), vn_index());

src/bgp/origin-vn/origin_vn.h

@@ -16,6 +16,7 @@
 class OriginVn {
 public:
     static const int kSize = 8;
+    static const int kMinGlobalId = 8000000;
     static OriginVn null_originvn;
     typedef boost::array<uint8_t, kSize> bytes_type;
 
@@ -24,6 +25,7 @@ class OriginVn {
     explicit OriginVn(const bytes_type &data);
 
     bool IsNull() { return operator==(OriginVn::null_originvn); }
+    bool IsGlobal() const;
 
     as_t as_number() const;
     int vn_index() const;

src/bgp/origin-vn/test/origin_vn_test.cc

@@ -20,6 +20,7 @@ TEST_F(OriginVnTest, ByteArray_1) {
     EXPECT_EQ(65412, origin_vn.as_number());
     EXPECT_EQ(16909060, origin_vn.vn_index());
     EXPECT_EQ("originvn:65412:16909060", origin_vn.ToString());
+    EXPECT_TRUE(origin_vn.IsGlobal());
 }
 
 TEST_F(OriginVnTest, ByteArray_2) {
@@ -31,6 +32,7 @@ TEST_F(OriginVnTest, ByteArray_2) {
     EXPECT_EQ(65412, origin_vn.as_number());
     EXPECT_EQ(67305985, origin_vn.vn_index());
     EXPECT_EQ("originvn:65412:67305985", origin_vn.ToString());
+    EXPECT_TRUE(origin_vn.IsGlobal());
 }
 
 TEST_F(OriginVnTest, ByteArray_3) {
@@ -42,6 +44,7 @@ TEST_F(OriginVnTest, ByteArray_3) {
     EXPECT_EQ(65412, origin_vn.as_number());
     EXPECT_EQ(0, origin_vn.vn_index());
     EXPECT_EQ("originvn:65412:0", origin_vn.ToString());
+    EXPECT_FALSE(origin_vn.IsGlobal());
 }
 
 TEST_F(OriginVnTest, ByteArray_4) {
@@ -53,6 +56,19 @@ TEST_F(OriginVnTest, ByteArray_4) {
     EXPECT_EQ(65412, origin_vn.as_number());
     EXPECT_EQ(2147483647, origin_vn.vn_index());
     EXPECT_EQ("originvn:65412:2147483647", origin_vn.ToString());
+    EXPECT_TRUE(origin_vn.IsGlobal());
+}
+
+TEST_F(OriginVnTest, ByteArray_5) {
+    OriginVn::bytes_type data = {
+        { 0x80, 0x71, 0xff, 0x84, 0x00, 0x00, 0xff, 0xff }
+    };
+    OriginVn origin_vn(data);
+    EXPECT_FALSE(origin_vn.IsNull());
+    EXPECT_EQ(65412, origin_vn.as_number());
+    EXPECT_EQ(65535, origin_vn.vn_index());
+    EXPECT_EQ("originvn:65412:65535", origin_vn.ToString());
+    EXPECT_FALSE(origin_vn.IsGlobal());
 }
 
 TEST_F(OriginVnTest, FromString_1) {
@@ -62,6 +78,7 @@ TEST_F(OriginVnTest, FromString_1) {
     EXPECT_EQ(65412, origin_vn.as_number());
     EXPECT_EQ(16909060, origin_vn.vn_index());
     EXPECT_EQ("originvn:65412:16909060", origin_vn.ToString());
+    EXPECT_TRUE(origin_vn.IsGlobal());
 }
 
 TEST_F(OriginVnTest, FromString_2) {
@@ -71,6 +88,7 @@ TEST_F(OriginVnTest, FromString_2) {
     EXPECT_EQ(65412, origin_vn.as_number());
     EXPECT_EQ(67305985, origin_vn.vn_index());
     EXPECT_EQ("originvn:65412:67305985", origin_vn.ToString());
+    EXPECT_TRUE(origin_vn.IsGlobal());
 }
 
 TEST_F(OriginVnTest, FromString_3) {
@@ -80,6 +98,7 @@ TEST_F(OriginVnTest, FromString_3) {
     EXPECT_EQ(65412, origin_vn.as_number());
     EXPECT_EQ(0, origin_vn.vn_index());
     EXPECT_EQ("originvn:65412:0", origin_vn.ToString());
+    EXPECT_FALSE(origin_vn.IsGlobal());
 }
 
 TEST_F(OriginVnTest, FromString_4) {
@@ -89,6 +108,17 @@ TEST_F(OriginVnTest, FromString_4) {
     EXPECT_EQ(65412, origin_vn.as_number());
     EXPECT_EQ(2147483647, origin_vn.vn_index());
     EXPECT_EQ("originvn:65412:2147483647", origin_vn.ToString());
+    EXPECT_TRUE(origin_vn.IsGlobal());
+}
+
+TEST_F(OriginVnTest, FromString_5) {
+    boost::system::error_code ec;
+    OriginVn origin_vn = OriginVn::FromString("originvn:65412:65535", &ec);
+    EXPECT_EQ(0, ec.value());
+    EXPECT_EQ(65412, origin_vn.as_number());
+    EXPECT_EQ(65535, origin_vn.vn_index());
+    EXPECT_EQ("originvn:65412:65535", origin_vn.ToString());
+    EXPECT_FALSE(origin_vn.IsGlobal());
 }
 
 // Does not contain a colon.

src/bgp/routing-instance/routepath_replicator.cc

@@ -401,14 +401,17 @@ static ExtCommunityPtr UpdateExtCommunity(BgpServer *server,
     if (!ext_community)
         return ExtCommunityPtr(NULL);
 
-    // Nothing to do if we already have the OriginVn community with our AS.
+    // Nothing to do if we already have the OriginVn community with our AS
+    // or with a vn index from the global range.
     BOOST_FOREACH(const ExtCommunity::ExtCommunityValue &comm,
                   ext_community->communities()) {
         if (!ExtCommunity::is_origin_vn(comm))
             continue;
         OriginVn origin_vn(comm);
-        if (origin_vn.as_number() != server->autonomous_system())
+        if (!origin_vn.IsGlobal() &&
+            origin_vn.as_number() != server->autonomous_system()) {
             continue;
+        }
         return ExtCommunityPtr(ext_community);
     }
 

src/bgp/test/bgp_attr_test.cc

@@ -1228,7 +1228,11 @@ TEST_F(BgpAttrTest, OriginVnPathPrepend) {
     EXPECT_EQ(0, ovnpath2.CompareTo(ovnpath1));
 }
 
-TEST_F(BgpAttrTest, OriginVnPathContains) {
+//
+// Both AS and VN index are compared for OriginVns with index from non-global
+// range.
+//
+TEST_F(BgpAttrTest, OriginVnPathContains1) {
     OriginVnPathSpec spec;
     for (int idx = 9; idx >= 1; idx -= 2) {
         OriginVn origin_vn(64512, 100 * idx);
@@ -1256,6 +1260,36 @@ TEST_F(BgpAttrTest, OriginVnPathContains) {
     }
 }
 
+//
+// OriginVns with matching VN index from global range are treated as equal
+// even if the AS numbers are different.
+// OriginVns with matching VN index from non-global range are treated as not
+// equal because AS numbers are different.
+//
+TEST_F(BgpAttrTest, OriginVnPathContains2) {
+    OriginVnPathSpec spec;
+    for (int idx = 9; idx >= 1; idx--) {
+        if (idx % 2 == 1) {
+            OriginVn origin_vn(64512, OriginVn::kMinGlobalId + 100 * idx);
+            spec.origin_vns.push_back(origin_vn.GetExtCommunityValue());
+        } else {
+            OriginVn origin_vn(64512, 100 * idx);
+            spec.origin_vns.push_back(origin_vn.GetExtCommunityValue());
+        }
+    }
+    OriginVnPath ovnpath(ovnpath_db_, spec);
+
+    for (int idx = 1; idx <= 9; idx++) {
+        if (idx % 2 == 1) {
+            OriginVn origin_vn(64513, OriginVn::kMinGlobalId + 100 * idx);
+            EXPECT_TRUE(ovnpath.Contains(origin_vn.GetExtCommunity()));
+        } else {
+            OriginVn origin_vn(64513, OriginVn::kMinGlobalId + 100 * idx);
+            EXPECT_FALSE(ovnpath.Contains(origin_vn.GetExtCommunity()));
+        }
+    }
+}
+
 TEST_F(BgpAttrTest, OriginVnPathLocate) {
     OriginVnPathSpec spec1;
     for (int idx = 1; idx < 5; idx++) {

src/bgp/xmpp_message_builder.cc

@@ -18,7 +18,6 @@
 #include "bgp/extended-community/mac_mobility.h"
 #include "bgp/ermvpn/ermvpn_route.h"
 #include "bgp/evpn/evpn_route.h"
-#include "bgp/origin-vn/origin_vn.h"
 #include "bgp/routing-instance/routing_instance.h"
 #include "bgp/security_group/security_group.h"
 #include "net/community_type.h"