diff --git a/src/bgp/bgp_origin_vn_path.cc b/src/bgp/bgp_origin_vn_path.cc index 453475865e4..88990b87b36 100644 --- a/src/bgp/bgp_origin_vn_path.cc +++ b/src/bgp/bgp_origin_vn_path.cc @@ -9,6 +9,7 @@ #include #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; } diff --git a/src/bgp/origin-vn/origin_vn.cc b/src/bgp/origin-vn/origin_vn.cc index 9516f230947..1e3acd276f4 100644 --- a/src/bgp/origin-vn/origin_vn.cc +++ b/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()); diff --git a/src/bgp/origin-vn/origin_vn.h b/src/bgp/origin-vn/origin_vn.h index 51786524275..5697b2c1dec 100644 --- a/src/bgp/origin-vn/origin_vn.h +++ b/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 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; diff --git a/src/bgp/origin-vn/test/origin_vn_test.cc b/src/bgp/origin-vn/test/origin_vn_test.cc index 09643efcf73..de256669ddc 100644 --- a/src/bgp/origin-vn/test/origin_vn_test.cc +++ b/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. diff --git a/src/bgp/routing-instance/routepath_replicator.cc b/src/bgp/routing-instance/routepath_replicator.cc index 5ea37e83a1f..9cce204a473 100644 --- a/src/bgp/routing-instance/routepath_replicator.cc +++ b/src/bgp/routing-instance/routepath_replicator.cc @@ -399,14 +399,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); } diff --git a/src/bgp/test/bgp_attr_test.cc b/src/bgp/test/bgp_attr_test.cc index f2eccc8b863..1b0e591ba02 100644 --- a/src/bgp/test/bgp_attr_test.cc +++ b/src/bgp/test/bgp_attr_test.cc @@ -713,7 +713,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); @@ -741,6 +745,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++) { diff --git a/src/bgp/xmpp_message_builder.cc b/src/bgp/xmpp_message_builder.cc index 14501421124..39f9447c776 100644 --- a/src/bgp/xmpp_message_builder.cc +++ b/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"