Skip to content

Commit

Permalink
Treat VN index >= 8000000 as global
Browse files Browse the repository at this point in the history 
This is required for correct operation in multi-region scenarios with
service chaining. If there are 2 or more regions re-originating routes
for service chaining, we do not want to derive the origin vn from the
route targets of routes received from other regions. We want the global
controller to manage allocation of vn ids and trust received values if
they have been allocated by global controller.

Values greater than 8000000 are treated as global and are allocated by
the global controller. Need to disregard the ASN and compare only the
vn ids in this case.

Change-Id: I2404517b5d0024ebbea04eb053273aea45ddf20d
Closes-Bug: 1612415
  • Loading branch information
Nischal Sheth committed Aug 11, 2016
1 parent b443e9d commit 115f958
Show file tree
Hide file tree
Showing 7 changed files with 84 additions and 4 deletions.
8 changes: 8 additions & 0 deletions src/bgp/bgp_origin_vn_path.cc
View file
Expand Up @@ -9,6 +9,7 @@
#include <string>

#include "bgp/bgp_proto.h"
#include "bgp/origin-vn/origin_vn.h"

using std::string;
using std::vector;
Expand Down Expand Up @@ -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;
}
Expand Down
4 changes: 4 additions & 0 deletions src/bgp/origin-vn/origin_vn.cc
View file
Expand Up @@ -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());
Expand Down
2 changes: 2 additions & 0 deletions src/bgp/origin-vn/origin_vn.h
View file
Expand Up @@ -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;

Expand All @@ -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;
Expand Down
30 changes: 30 additions & 0 deletions src/bgp/origin-vn/test/origin_vn_test.cc
View file
Expand Up @@ -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) {
Expand All @@ -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) {
Expand All @@ -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) {
Expand All @@ -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) {
Expand All @@ -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) {
Expand All @@ -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) {
Expand All @@ -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) {
Expand All @@ -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.
Expand Down
7 changes: 5 additions & 2 deletions src/bgp/routing-instance/routepath_replicator.cc
View file
Expand Up @@ -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);
}

Expand Down
36 changes: 35 additions & 1 deletion src/bgp/test/bgp_attr_test.cc
View file
Expand Up @@ -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);
Expand Down Expand Up @@ -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++) {
Expand Down
1 change: 0 additions & 1 deletion src/bgp/xmpp_message_builder.cc
View file
Expand Up @@ -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"
Expand Down

0 comments on commit 115f958

Please sign in to comment.