Skip to content

Commit

Permalink
More improvements to SchedulingGroupManager::Leave processing
Browse files Browse the repository at this point in the history 
Following changes are implemented:

- Make GetPeerRibList build a list of RibStates instead of RibOuts
  since building a RibStateList is much chepaer and the RibOutList
  is needed only if the group can be split (which is very rare).
- Tweak the code to check for overlap in the peers for advertised
  and not-advertised RibStateLists to improve performance.
- Add couple more tests to exercise Leave code with large number of
  RibOuts.

Change-Id: Iea5458b24c1f4e4be5acd70409b733bd141eda58
Partial-Bug: 1461322
  • Loading branch information
Nischal Sheth committed Jun 4, 2015
1 parent 45e29b5 commit d7524ab
Show file tree
Hide file tree
Showing 3 changed files with 179 additions and 44 deletions.
86 changes: 58 additions & 28 deletions src/bgp/scheduling_group.cc
View file
Expand Up @@ -534,25 +534,28 @@ bool SchedulingGroup::PeerInSync(IPeerUpdate *peer) const {
}

//
// Build two RibOutLists. The first is the list of RibOuts advertised by the
// IPeerUpdate and the second is the list of RibOuts that are not advertised
// by the IPeerUpdate.
// Build two RibStateLists.
// The first is the list of RibStates advertised by the IPeerUpdate and the
// second is the list of RibStates that are not advertised by the IPeerUpdate.
//
void SchedulingGroup::GetPeerRibList(
IPeerUpdate *peer, RibOutList *rlist, RibOutList *rcomplement) const {
void SchedulingGroup::GetPeerRibList(IPeerUpdate *peer,
RibStateList *rs_list, RibStateList *rsc_list) const {
CHECK_CONCURRENCY("bgp::PeerMembership");

PeerState *ps = peer_state_imap_.Find(peer);
rs_list->clear();
rsc_list->clear();

// Go through all possible indices for the RibOuts. Need to ignore the
// ones for which there's no RibState since those RibOuts don't exist.
PeerState *ps = peer_state_imap_.Find(peer);
for (size_t i = 0; i < rib_state_imap_.size(); i++) {
RibState *rs = rib_state_imap_.At(i);
if (rs == NULL) continue;
if (rs == NULL)
continue;
if (ps != NULL && ps->IsMember(i)) {
rlist->push_back(rs->ribout());
rs_list->push_back(rs);
} else {
rcomplement->push_back(rs->ribout());
rsc_list->push_back(rs);
}
}
}
Expand All @@ -574,16 +577,16 @@ void SchedulingGroup::GetRibPeerList(RibOut *ribout, PeerList *plist) const {
}

//
// Build the bitset of peers advertising at least one RibOut in the specified
// list.
// Build the bitset of peers advertising at least one RibOut corresponding to
// the RibState list.
//
void SchedulingGroup::GetSubsetPeers(const RibOutList &list, GroupPeerSet *pg) {
void SchedulingGroup::GetSubsetPeers(const RibStateList &list, GroupPeerSet *pg) {
CHECK_CONCURRENCY("bgp::PeerMembership");

pg->clear();
for (RibOutList::const_iterator iter = list.begin(); iter != list.end();
for (RibStateList::const_iterator iter = list.begin(); iter != list.end();
++iter) {
RibState *rs = rib_state_imap_.Find(*iter);
RibState *rs = *iter;
*pg |= rs->peer_set();
}
}
Expand Down Expand Up @@ -763,6 +766,20 @@ void SchedulingGroup::GetRibOutList(RibOutList *rlist) const {
}
}

//
// Populate the RibOutList with RibOuts corresponding to the RibStateList.
//
void SchedulingGroup::GetRibOutList(const RibStateList &rs_list,
RibOutList *ro_list) const {
CHECK_CONCURRENCY("bgp::PeerMembership");

ro_list->clear();
for (RibStateList::const_iterator iter = rs_list.begin();
iter != rs_list.end(); ++iter) {
ro_list->push_back((*iter)->ribout());
}
}

//
// Populate the PeerList with all the IPeers in this SchedulingGroup.
//
Expand Down Expand Up @@ -1329,24 +1346,37 @@ void SchedulingGroupManager::Leave(RibOut *ribout, IPeerUpdate *peer) {
// Retrieve the list of ribs that this peer is still advertising along with
// its complement. If either the advertised list or the complement list are
// empty, the group can't be split.
RibOutList rlist;
RibOutList rcomplement;
sg->GetPeerRibList(peer, &rlist, &rcomplement);
if (rlist.empty() || rcomplement.empty()) {
RibStateList rs_list, rsc_list;
sg->GetPeerRibList(peer, &rs_list, &rsc_list);
if (rs_list.empty() || rsc_list.empty()) {
return;
}

// Get the bitsets of peers advertising one or more of the ribouts in the
// advertised list and the complement list respectively. If the two bitsets
// don't have any peers in common, the group can be split based on the two
// ribout lists.
GroupPeerSet s1;
sg->GetSubsetPeers(rlist, &s1);
GroupPeerSet s2;
sg->GetSubsetPeers(rcomplement, &s2);
if (!s1.intersects(s2)) {
Split(sg, rlist, rcomplement);
// Get the bitset of peers advertising one or more of the ribs in the
// advertised list. If this bitset has any peers in common with bitset
// of peers for the ribs in complement list, the group cannot be split.
//
// Since the list of complement ribs is usually larger than the list of
// advertised ribs we build the bitset only for the advertised list and
// check for intersection with peer set for each rib in the complement
// list.
GroupPeerSet bitset;
sg->GetSubsetPeers(rs_list, &bitset);
for (RibStateList::const_iterator iter = rsc_list.begin();
iter != rsc_list.end(); ++iter) {
RibState *rs = *iter;
if (bitset.intersects(rs->peer_set())) {
return;
}
}

// Build the lists of advertised and complement RibOuts from the lists
// of advertised and complement RibStates and split the group.
RibOutList ro_list;
sg->GetRibOutList(rs_list, &ro_list);
RibOutList roc_list;
sg->GetRibOutList(rsc_list, &roc_list);
Split(sg, ro_list, roc_list);
}

//
Expand Down
31 changes: 16 additions & 15 deletions src/bgp/scheduling_group.h
View file
Expand Up @@ -57,8 +57,11 @@ class GroupPeerSet : public BitSet {
//
class SchedulingGroup {
public:
class RibState;

typedef std::vector<RibOut *> RibOutList;
typedef std::vector<IPeerUpdate *> PeerList;
typedef std::vector<RibState *> RibStateList;

SchedulingGroup();
~SchedulingGroup();
Expand All @@ -85,17 +88,15 @@ class SchedulingGroup {
// The index for a specific ribout.
size_t GetRibOutIndex(RibOut *ribout) const;

// Retrieve the list of all the ribs which this peer is advertising as
// well as its complement.
void GetPeerRibList(IPeerUpdate *peer, RibOutList *rlist,
RibOutList *rcomplement) const;

void GetPeerRibList(IPeerUpdate *peer,
RibStateList *rs_list, RibStateList *rsc_list) const;
void GetRibPeerList(RibOut *ribout, PeerList *plist) const;

// Retrieve the list of peers with this specific set of ribs.
void GetSubsetPeers(const RibOutList &list, GroupPeerSet *pg);
void GetSubsetPeers(const RibStateList &list, GroupPeerSet *pg);

void GetRibOutList(RibOutList *rlist) const;
void GetRibOutList(const RibStateList &rs_list, RibOutList *ro_list) const;
void GetPeerList(PeerList *plist) const;

bool CheckInvariants() const;
Expand All @@ -113,8 +114,9 @@ class SchedulingGroup {
friend class SGTest;
friend void IPeerSendReady(SchedulingGroup *sg, IPeerUpdate *peer);

class RibState;
class PeerState;
class PeerIterator;
class Worker;
struct PeerRibState;
struct WorkBase;
struct WorkRibOut;
Expand All @@ -123,9 +125,6 @@ class SchedulingGroup {
typedef boost::ptr_list<WorkBase> WorkQueue;
typedef IndexMap<IPeerUpdate *, PeerState, GroupPeerSet> PeerStateMap;
typedef IndexMap<RibOut *, RibState> RibStateMap;
class Worker;

class PeerIterator;

void MaybeStartWorker();
std::auto_ptr<WorkBase> WorkDequeue();
Expand Down Expand Up @@ -191,11 +190,6 @@ class SchedulingGroup {
//
class SchedulingGroupManager {
public:
typedef SchedulingGroup::RibOutList RibOutList;
typedef std::list<SchedulingGroup *> GroupList;
typedef std::map<IPeerUpdate *, SchedulingGroup *> PeerMap;
typedef std::map<RibOut *, SchedulingGroup *> RibOutMap;

SchedulingGroupManager();
~SchedulingGroupManager();

Expand All @@ -221,6 +215,13 @@ class SchedulingGroupManager {
void EnableGroups();

private:
typedef SchedulingGroup::RibOutList RibOutList;
typedef SchedulingGroup::RibState RibState;
typedef SchedulingGroup::RibStateList RibStateList;
typedef std::list<SchedulingGroup *> GroupList;
typedef std::map<IPeerUpdate *, SchedulingGroup *> PeerMap;
typedef std::map<RibOut *, SchedulingGroup *> RibOutMap;

// Merge two existing scheduling groups.
SchedulingGroup *Merge(SchedulingGroup *sg1, SchedulingGroup *sg2);

Expand Down
106 changes: 105 additions & 1 deletion src/bgp/test/scheduling_group_test.cc
View file
Expand Up @@ -455,7 +455,7 @@ TEST_F(SchedulingGroupManagerTest, TwoTablesMultiplePeers) {
//
// Exercise the Leave code with a large number of RibOuts.
// Test is constructed such that there's only a single SchedulingGroup.
// Intention is to test the performance of SchedulingGroup::GetPeerRibList.
// Intent is to test the performance of SchedulingGroup::GetPeerRibList.
//
TEST_F(SchedulingGroupManagerTest, LeaveScaling1) {
// Create 2 peers and join them to a common ribout.
Expand Down Expand Up @@ -497,6 +497,110 @@ TEST_F(SchedulingGroupManagerTest, LeaveScaling1) {
STLDeleteValues(&peers);
}

//
// Exercise the Leave code with a large number of RibOuts.
// Test is constructed such that there's only a single SchedulingGroup.
// Intent is to test the performance of the logic to determine if the
// SchedulingGroup can be split - focus is on the negative case.
// The common RibOut which prevents the split has the lowest index.
//
TEST_F(SchedulingGroupManagerTest, LeaveScaling2) {
// Create 2 peers and join them to a common ribout.
vector<BgpTestPeer *> peers;
BgpTestPeer *test_peer1(new BgpTestPeer());
BgpTestPeer *test_peer2(new BgpTestPeer());
peers.push_back(test_peer1);
peers.push_back(test_peer2);
RibOut ribout_common(inetvpn_table_, &sgman_, RibExportPolicy());
Join(&ribout_common, test_peer1);
Join(&ribout_common, test_peer2);
EXPECT_EQ(1, sgman_.size());

// Create large number of additional ribouts.
vector<RibOut *> ribouts;
for (int idx = 0; idx < 8192; ++idx) {
RibOut *ribout(new RibOut(inetvpn_table_, &sgman_,
RibExportPolicy(BgpProto::IBGP, RibExportPolicy::BGP, idx, 0)));
ribouts.push_back(ribout);
}

// Join the test peers to odd/even additional ribouts.
size_t idx = 0;
BOOST_FOREACH(RibOut *ribout, ribouts) {
Join(ribout, peers[idx % 2]);
idx++;
}
EXPECT_EQ(1, sgman_.size());

// Leave the test peer from odd/even additional ribouts.
idx = 0;
BOOST_FOREACH(RibOut *ribout, ribouts) {
Leave(ribout, peers[idx % 2]);
idx++;
}
EXPECT_EQ(1, sgman_.size());

// Cleanup.
Leave(&ribout_common, test_peer1);
Leave(&ribout_common, test_peer2);
EXPECT_EQ(0, sgman_.size());
STLDeleteValues(&ribouts);
STLDeleteValues(&peers);
}

//
// Exercise the Leave code with a large number of RibOuts.
// Test is constructed such that there's only a single SchedulingGroup.
// Intent is to test the performance of the logic to determine if the
// SchedulingGroup can be split - focus is on the negative case.
// The common RibOut which prevents the split has the highest index.
//
TEST_F(SchedulingGroupManagerTest, LeaveScaling3) {
// Create 2 peers.
vector<BgpTestPeer *> peers;
BgpTestPeer *test_peer1(new BgpTestPeer());
BgpTestPeer *test_peer2(new BgpTestPeer());
peers.push_back(test_peer1);
peers.push_back(test_peer2);

// Create large number of additional RibOuts.
vector<RibOut *> ribouts;
for (int idx = 0; idx < 8192; ++idx) {
RibOut *ribout(new RibOut(inetvpn_table_, &sgman_,
RibExportPolicy(BgpProto::IBGP, RibExportPolicy::BGP, idx, 0)));
ribouts.push_back(ribout);
}

// Join the test peers to odd/even additional RibOuts.
size_t idx = 0;
BOOST_FOREACH(RibOut *ribout, ribouts) {
Join(ribout, peers[idx % 2]);
idx++;
}
EXPECT_EQ(2, sgman_.size());

// Join the test peers to a common ribout so that the groups get merged.
RibOut ribout_common(inetvpn_table_, &sgman_, RibExportPolicy());
Join(&ribout_common, test_peer1);
Join(&ribout_common, test_peer2);
EXPECT_EQ(1, sgman_.size());

// Leave the test peers from odd/even additional RibOuts.
idx = 0;
BOOST_FOREACH(RibOut *ribout, ribouts) {
Leave(ribout, peers[idx % 2]);
idx++;
}
EXPECT_EQ(1, sgman_.size());

// Cleanup.
Leave(&ribout_common, test_peer1);
Leave(&ribout_common, test_peer2);
EXPECT_EQ(0, sgman_.size());
STLDeleteValues(&ribouts);
STLDeleteValues(&peers);
}

// Parameterize number of entries in the work queue and the order in which
// the entries are added.

Expand Down

0 comments on commit d7524ab

Please sign in to comment.