Skip to content

Commit

Permalink
Prefer path with shorter cluster list length
Browse files Browse the repository at this point in the history
Highlights:

- Define ClusterListSpec, ClusterList and ClusterListDB
- Use cluster list length in path selection
- Add cluster list to route introspect
- Add unit tests for decode/encode and path selection

Change-Id: If7c15cc84d85fe31f8aafc48b54586457806e433
Closes-Bug: 1531679
  • Loading branch information
Nischal Sheth committed Jan 9, 2016
1 parent 2351540 commit f4cfa0b
Show file tree
Hide file tree
Showing 12 changed files with 338 additions and 1 deletion.
48 changes: 48 additions & 0 deletions src/bgp/bgp_attr.cc
Expand Up @@ -139,6 +139,44 @@ std::string BgpAttrOriginatorId::ToString() const {
return std::string(repr);
}

int ClusterListSpec::CompareTo(const BgpAttribute &rhs_attr) const {
int ret = BgpAttribute::CompareTo(rhs_attr);
if (ret != 0) return ret;
KEY_COMPARE(cluster_list,
static_cast<const ClusterListSpec &>(rhs_attr).cluster_list);
return 0;
}

void ClusterListSpec::ToCanonical(BgpAttr *attr) {
attr->set_cluster_list(this);
}

std::string ClusterListSpec::ToString() const {
std::stringstream repr;
repr << "CLUSTER_LIST <code: " << (int) code;
repr << ", flags: " << std::hex << (int) flags << "> :";
for (std::vector<uint32_t>::const_iterator iter = cluster_list.begin();
iter != cluster_list.end(); ++iter) {
repr << " " << Ip4Address(*iter).to_string();
}
repr << std::endl;
return repr.str();
}

ClusterList::ClusterList(ClusterListDB *cluster_list_db,
const ClusterListSpec &spec)
: cluster_list_db_(cluster_list_db),
spec_(spec) {
refcount_ = 0;
}

void ClusterList::Remove() {
cluster_list_db_->Delete(this);
}

ClusterListDB::ClusterListDB(BgpServer *server) {
}

int BgpMpNlri::CompareTo(const BgpAttribute &rhs_attr) const {
int ret = BgpAttribute::CompareTo(rhs_attr);
if (ret != 0) return ret;
Expand Down Expand Up @@ -786,6 +824,7 @@ BgpAttr::BgpAttr(const BgpAttr &rhs)
originator_id_(rhs.originator_id_),
source_rd_(rhs.source_rd_), esi_(rhs.esi_), params_(rhs.params_),
as_path_(rhs.as_path_),
cluster_list_(rhs.cluster_list_),
community_(rhs.community_),
ext_community_(rhs.ext_community_),
origin_vn_path_(rhs.origin_vn_path_),
Expand All @@ -810,6 +849,14 @@ void BgpAttr::set_as_path(const AsPathSpec *spec) {
}
}

void BgpAttr::set_cluster_list(const ClusterListSpec *spec) {
if (spec) {
cluster_list_ = attr_db_->server()->cluster_list_db()->Locate(*spec);
} else {
cluster_list_ = NULL;
}
}

void BgpAttr::set_community(CommunityPtr comm) {
community_ = comm;
}
Expand Down Expand Up @@ -933,6 +980,7 @@ int BgpAttr::CompareTo(const BgpAttr &rhs) const {
KEY_COMPARE(olist_.get(), rhs.olist_.get());
KEY_COMPARE(leaf_olist_.get(), rhs.leaf_olist_.get());
KEY_COMPARE(as_path_.get(), rhs.as_path_.get());
KEY_COMPARE(cluster_list_.get(), rhs.cluster_list_.get());
KEY_COMPARE(community_.get(), rhs.community_.get());
KEY_COMPARE(ext_community_.get(), rhs.ext_community_.get());
KEY_COMPARE(origin_vn_path_.get(), rhs.origin_vn_path_.get());
Expand Down
84 changes: 83 additions & 1 deletion src/bgp/bgp_attr.h
Expand Up @@ -29,7 +29,7 @@
// all information in the UPDATE except: NLRI (prefix) and label.

class BgpAttr;
class AsPathDB;
class ClusterListDB;

struct BgpAttrOrigin : public BgpAttribute {
static const int kSize = 1;
Expand Down Expand Up @@ -150,6 +150,82 @@ struct BgpAttrOriginatorId : public BgpAttribute {
virtual std::string ToString() const;
};

struct ClusterListSpec : public BgpAttribute {
static const int kSize = -1;
static const uint8_t kFlags = Optional;
ClusterListSpec() : BgpAttribute(ClusterList, kFlags) { }
explicit ClusterListSpec(const BgpAttribute &rhs) : BgpAttribute(rhs) { }
explicit ClusterListSpec(const ClusterListSpec &rhs)
: BgpAttribute(BgpAttribute::ClusterList, kFlags) {
cluster_list = rhs.cluster_list;
}
virtual int CompareTo(const BgpAttribute &rhs) const;
virtual void ToCanonical(BgpAttr *attr);
virtual std::string ToString() const;
std::vector<uint32_t> cluster_list;
};

class ClusterList {
public:
ClusterList(ClusterListDB *cluster_list_db, const ClusterListSpec &spec);
~ClusterList() { }
void Remove();
int CompareTo(const ClusterList &rhs) const {
return spec_.CompareTo(rhs.cluster_list());
}

const ClusterListSpec &cluster_list() const { return spec_; }
size_t size() const { return spec_.cluster_list.size(); }

friend std::size_t hash_value(const ClusterList &cluster_list) {
size_t hash = 0;
return hash;
}

private:
friend int intrusive_ptr_add_ref(const ClusterList *ccluster_list);
friend int intrusive_ptr_del_ref(const ClusterList *ccluster_list);
friend void intrusive_ptr_release(const ClusterList *ccluster_list);

mutable tbb::atomic<int> refcount_;
ClusterListDB *cluster_list_db_;
ClusterListSpec spec_;
};

inline int intrusive_ptr_add_ref(const ClusterList *ccluster_list) {
return ccluster_list->refcount_.fetch_and_increment();
}

inline int intrusive_ptr_del_ref(const ClusterList *ccluster_list) {
return ccluster_list->refcount_.fetch_and_decrement();
}

inline void intrusive_ptr_release(const ClusterList *ccluster_list) {
int prev = ccluster_list->refcount_.fetch_and_decrement();
if (prev == 1) {
ClusterList *cluster_list = const_cast<ClusterList *>(ccluster_list);
cluster_list->Remove();
assert(cluster_list->refcount_ == 0);
delete cluster_list;
}
}

typedef boost::intrusive_ptr<ClusterList> ClusterListPtr;

struct ClusterListCompare {
bool operator()(const ClusterList *lhs, const ClusterList *rhs) {
return lhs->CompareTo(*rhs) < 0;
}
};

class ClusterListDB : public BgpPathAttributeDB<ClusterList, ClusterListPtr,
ClusterListSpec,
ClusterListCompare,
ClusterListDB> {
public:
explicit ClusterListDB(BgpServer *server);
};

struct BgpMpNlri : public BgpAttribute {
static const int kSize = -1;
static const uint8_t kFlags = Optional;
Expand Down Expand Up @@ -703,6 +779,7 @@ class BgpAttr {
void set_params(uint64_t params) { params_ = params; }
void set_as_path(AsPathPtr aspath);
void set_as_path(const AsPathSpec *spec);
void set_cluster_list(const ClusterListSpec *spec);
void set_community(CommunityPtr comm);
void set_community(const CommunitySpec *comm);
void set_ext_community(ExtCommunityPtr extcomm);
Expand Down Expand Up @@ -731,6 +808,10 @@ class BgpAttr {
uint64_t params() const { return params_; }
const AsPath *as_path() const { return as_path_.get(); }
int as_path_count() const { return as_path_ ? as_path_->AsCount() : 0; }
const ClusterList *cluster_list() const { return cluster_list_.get(); }
size_t cluster_list_length() const {
return cluster_list_ ? cluster_list_->size() : 0;
}
const Community *community() const { return community_.get(); }
const ExtCommunity *ext_community() const { return ext_community_.get(); }
const OriginVnPath *origin_vn_path() const { return origin_vn_path_.get(); }
Expand Down Expand Up @@ -767,6 +848,7 @@ class BgpAttr {
EthernetSegmentId esi_;
uint64_t params_;
AsPathPtr as_path_;
ClusterListPtr cluster_list_;
CommunityPtr community_;
ExtCommunityPtr ext_community_;
OriginVnPathPtr origin_vn_path_;
Expand Down
6 changes: 6 additions & 0 deletions src/bgp/bgp_message_builder.cc
Expand Up @@ -59,6 +59,12 @@ bool BgpMessage::StartReach(const RibOutAttr *roattr, const BgpRoute *route) {
update.path_attributes.push_back(originator_id);
}

if (attr->cluster_list()) {
ClusterListSpec *clist =
new ClusterListSpec(attr->cluster_list()->cluster_list());
update.path_attributes.push_back(clist);
}

if (attr->as_path()) {
AsPathSpec *path = new AsPathSpec(attr->as_path()->path());
update.path_attributes.push_back(path);
Expand Down
2 changes: 2 additions & 0 deletions src/bgp/bgp_path.cc
Expand Up @@ -119,6 +119,8 @@ int BgpPath::PathCompare(const BgpPath &rhs, bool allow_ecmp) const {
uint32_t rid = rorig_id ? rorig_id : rhs.peer_->bgp_identifier();
KEY_COMPARE(id, rid);

KEY_COMPARE(attr_->cluster_list_length(), rattr->cluster_list_length());

const BgpPeer *lpeer = dynamic_cast<const BgpPeer *>(peer_);
const BgpPeer *rpeer = dynamic_cast<const BgpPeer *>(rhs.peer_);
if (lpeer != NULL && rpeer != NULL) {
Expand Down
1 change: 1 addition & 0 deletions src/bgp/bgp_peer.sandesh
Expand Up @@ -152,6 +152,7 @@ struct ShowRoutePath {
20: list<string> origin_vn_path;
21: ShowPmsiTunnel pmsi_tunnel;
22: ShowLoadBalance load_balance;
23: list<string> cluster_list;
}

struct ShowRoute {
Expand Down
19 changes: 19 additions & 0 deletions src/bgp/bgp_proto.cc
Expand Up @@ -876,6 +876,23 @@ class BgpPathAttributeExtendedCommunities :
BgpPathAttributeExtendedCommunityList> Sequence;
};

class BgpPathAttributeClusterListData :
public ProtoElement<BgpPathAttributeClusterListData> {
public:
static const int kSize = -1;
typedef VectorAccessor<ClusterListSpec, uint32_t,
&ClusterListSpec::cluster_list> Setter;
};

class BgpPathAttributeClusterList :
public ProtoSequence<BgpPathAttributeClusterList> {
public:
typedef ClusterListSpec ContextType;
typedef BgpContextSwap<ClusterListSpec> ContextSwap;
typedef mpl::list<BgpPathAttrLength,
BgpPathAttributeClusterListData> Sequence;
};

class BgpPathAttributeOriginVnList :
public ProtoElement<BgpPathAttributeOriginVnList> {
public:
Expand Down Expand Up @@ -1455,6 +1472,8 @@ class BgpPathAttribute : public ProtoChoice<BgpPathAttribute> {
mpl::pair<mpl::int_<BgpAttribute::OriginatorId>,
BgpAttrTemplate<BgpAttrOriginatorId, 4, uint32_t,
&BgpAttrOriginatorId::originator_id> >,
mpl::pair<mpl::int_<BgpAttribute::ClusterList>,
BgpPathAttributeClusterList>,
mpl::pair<mpl::int_<BgpAttribute::MPReachNlri>,
BgpPathAttributeMpReachNlriSequence>,
mpl::pair<mpl::int_<BgpAttribute::MPUnreachNlri>,
Expand Down
12 changes: 12 additions & 0 deletions src/bgp/bgp_route.cc
Expand Up @@ -310,6 +310,15 @@ void BgpRoute::FillRouteInfo(const BgpTable *table,
show_route->set_paths(show_route_paths);
}

static void FillRoutePathClusterListInfo(const ClusterList *clist,
ShowRoutePath *show_path) {
const vector<uint32_t> &list = clist->cluster_list().cluster_list;
for (vector<uint32_t>::const_iterator it = list.begin(); it != list.end();
++it) {
show_path->cluster_list.push_back(Ip4Address(*it).to_string());
}
}

static void FillRoutePathCommunityInfo(const Community *comm,
ShowRoutePath *show_path) {
comm->BuildStringList(&show_path->communities);
Expand Down Expand Up @@ -450,6 +459,9 @@ void BgpRoute::FillRouteInfo(const BgpTable *table,
} else {
srp.set_replicated(false);
}
if (attr->cluster_list()) {
FillRoutePathClusterListInfo(attr->cluster_list(), &srp);
}
if (attr->community()) {
FillRoutePathCommunityInfo(attr->community(), &srp);
}
Expand Down
1 change: 1 addition & 0 deletions src/bgp/bgp_server.cc
Expand Up @@ -293,6 +293,7 @@ BgpServer::BgpServer(EventManager *evm)
destroyed_(false),
aspath_db_(new AsPathDB(this)),
olist_db_(new BgpOListDB(this)),
cluster_list_db_(new ClusterListDB(this)),
comm_db_(new CommunityDB(this)),
edge_discovery_db_(new EdgeDiscoveryDB(this)),
edge_forwarding_db_(new EdgeForwardingDB(this)),
Expand Down
3 changes: 3 additions & 0 deletions src/bgp/bgp_server.h
Expand Up @@ -28,6 +28,7 @@ class BgpConfigManager;
class BgpOListDB;
class BgpPeer;
class BgpSessionManager;
class ClusterListDB;
class CommunityDB;
class EdgeDiscoveryDB;
class EdgeForwardingDB;
Expand Down Expand Up @@ -130,6 +131,7 @@ class BgpServer {
AsPathDB *aspath_db() { return aspath_db_.get(); }
BgpAttrDB *attr_db() { return attr_db_.get(); }
BgpOListDB *olist_db() { return olist_db_.get(); }
ClusterListDB *cluster_list_db() { return cluster_list_db_.get(); }
CommunityDB *comm_db() { return comm_db_.get(); }
EdgeDiscoveryDB *edge_discovery_db() { return edge_discovery_db_.get(); }
EdgeForwardingDB *edge_forwarding_db() { return edge_forwarding_db_.get(); }
Expand Down Expand Up @@ -248,6 +250,7 @@ class BgpServer {
// databases
boost::scoped_ptr<AsPathDB> aspath_db_;
boost::scoped_ptr<BgpOListDB> olist_db_;
boost::scoped_ptr<ClusterListDB> cluster_list_db_;
boost::scoped_ptr<CommunityDB> comm_db_;
boost::scoped_ptr<EdgeDiscoveryDB> edge_discovery_db_;
boost::scoped_ptr<EdgeForwardingDB> edge_forwarding_db_;
Expand Down
48 changes: 48 additions & 0 deletions src/bgp/test/bgp_attr_test.cc
Expand Up @@ -26,6 +26,7 @@ class BgpAttrTest : public ::testing::Test {
: server_(&evm_),
attr_db_(server_.attr_db()),
aspath_db_(server_.aspath_db()),
cluster_list_db_(server_.cluster_list_db()),
comm_db_(server_.comm_db()),
edge_discovery_db_(server_.edge_discovery_db()),
edge_forwarding_db_(server_.edge_forwarding_db()),
Expand Down Expand Up @@ -53,6 +54,7 @@ class BgpAttrTest : public ::testing::Test {
BgpServer server_;
BgpAttrDB *attr_db_;
AsPathDB *aspath_db_;
ClusterListDB *cluster_list_db_;
CommunityDB *comm_db_;
EdgeDiscoveryDB *edge_discovery_db_;
EdgeForwardingDB *edge_forwarding_db_;
Expand Down Expand Up @@ -388,6 +390,52 @@ TEST_F(BgpAttrTest, AsPathFormat2) {
EXPECT_EQ("100 200 {300 400} 600 500", path.path().ToString());
}

TEST_F(BgpAttrTest, ClusterList) {
BgpAttrSpec attr_spec;
ClusterListSpec spec;
for (int idx = 1; idx < 5; idx++)
spec.cluster_list.push_back(100 * idx);
attr_spec.push_back(&spec);
BgpAttrPtr attr_ptr = attr_db_->Locate(attr_spec);
EXPECT_EQ(4, attr_ptr->cluster_list_length());
BgpAttr attr(*attr_ptr);
EXPECT_EQ(attr_ptr->cluster_list(), attr.cluster_list());
}

TEST_F(BgpAttrTest, ClusterListCompare1) {
ClusterListSpec spec1;
for (int idx = 1; idx < 5; idx++)
spec1.cluster_list.push_back(100 * idx);
ClusterList clist1(cluster_list_db_, spec1);
EXPECT_EQ(4, clist1.size());

ClusterListSpec spec2;
for (int idx = 4; idx >= 1; idx--)
spec2.cluster_list.push_back(100 * idx);
ClusterList clist2(cluster_list_db_, spec2);
EXPECT_EQ(4, clist2.size());

EXPECT_EQ(-1, clist1.CompareTo(clist2));
EXPECT_EQ(1, clist2.CompareTo(clist1));
}

TEST_F(BgpAttrTest, ClusterListCompare2) {
ClusterListSpec spec1;
for (int idx = 1; idx < 3; idx++)
spec1.cluster_list.push_back(100 * idx);
ClusterList clist1(cluster_list_db_, spec1);
EXPECT_EQ(2, clist1.size());

ClusterListSpec spec2;
for (int idx = 1; idx < 5; idx++)
spec2.cluster_list.push_back(100 * idx);
ClusterList clist2(cluster_list_db_, spec2);
EXPECT_EQ(4, clist2.size());

EXPECT_EQ(-1, clist1.CompareTo(clist2));
EXPECT_EQ(1, clist2.CompareTo(clist1));
}

TEST_F(BgpAttrTest, CommunityCompare1) {
CommunitySpec spec1;
for (int idx = 1; idx < 5; idx++)
Expand Down

0 comments on commit f4cfa0b

Please sign in to comment.