Skip to content

Commit

Permalink
Support pagination of output for bgp neighbor introspect
Browse files Browse the repository at this point in the history
Limit the maximum number of entries displayed on a single page for
both regular and summary requests.  A next_batch link is generated
if there are more entries to be displayed.

Also limit maximum number of entries examined in one invocation of
the callback routine.  This comes into play when there is a search
string specified and many entries don't match it.  A partial page
is saved in user-allocated data and the next invocation of callback
appends to it.  This is repeated till there's a full page or there
are no more entries in the table.

Following changes are implemented:

- Move code from bgp_sandesh.cc to bgp_show_neighbor.cc
- Use class template BgpShowHandler to avoid code duplication
- Implement iteration limit to avoid hogging CPU from introspect
- Add unit tests to cover combinations of page and iteration limits
- Remove older unit tests which were limited in scope
- Sprinkle const as required

Change-Id: I4ef1848fb315ebbf4738e21f313cf60c255b7912
Closes-Bug: 1479427
  • Loading branch information
Nischal Sheth committed Aug 10, 2015
1 parent bd9d9db commit 2992f17
Show file tree
Hide file tree
Showing 22 changed files with 1,596 additions and 425 deletions.
1 change: 1 addition & 0 deletions src/bgp/SConscript
Original file line number Diff line number Diff line change
Expand Up @@ -59,6 +59,7 @@ libbgp = env.Library('bgp',
'bgp_session_manager.cc',
'bgp_session.cc',
'bgp_show_config.cc',
'bgp_show_neighbor.cc',
'bgp_show_route_summary.cc',
'bgp_show_routing_instance.cc',
'bgp_table.cc',
Expand Down
2 changes: 1 addition & 1 deletion src/bgp/bgp_peer.cc
Original file line number Diff line number Diff line change
Expand Up @@ -1713,7 +1713,7 @@ void BgpPeer::FillBgpNeighborDebugState(BgpNeighborResp &resp,
resp.set_tx_socket_stats(peer_socket_stats);
}

void BgpPeer::FillNeighborInfo(BgpSandeshContext *bsc,
void BgpPeer::FillNeighborInfo(const BgpSandeshContext *bsc,
vector<BgpNeighborResp> *nbr_list, bool summary) const {
BgpNeighborResp nbr;
nbr.set_peer(peer_basename_);
Expand Down
2 changes: 1 addition & 1 deletion src/bgp/bgp_peer.h
Original file line number Diff line number Diff line change
Expand Up @@ -154,7 +154,7 @@ class BgpPeer : public IPeer {
const BgpNeighborConfig *config() const { return config_; }

virtual void SetDataCollectionKey(BgpPeerInfo *peer_info) const;
void FillNeighborInfo(BgpSandeshContext *bsc,
void FillNeighborInfo(const BgpSandeshContext *bsc,
std::vector<BgpNeighborResp> *nbr_list, bool summary) const;

// thread-safe
Expand Down
7 changes: 5 additions & 2 deletions src/bgp/bgp_peer.sandesh
Original file line number Diff line number Diff line change
Expand Up @@ -22,8 +22,7 @@ traceobject sandesh BgpPeerObjectTrace {
}

request sandesh BgpNeighborReq {
1: string neighbor;
2: string domain;
1: string search_string;
}

struct BgpNeighborRoutingInstance {
Expand Down Expand Up @@ -79,10 +78,14 @@ struct BgpNeighborResp {

response sandesh BgpNeighborListResp {
1: list<BgpNeighborResp> neighbors;
2: optional string next_batch (link="BgpNeighborReqIterate",
link_title="next_batch");
}

response sandesh ShowBgpNeighborSummaryResp {
1: list<BgpNeighborResp> neighbors;
2: optional string next_batch (link="ShowBgpNeighborSummaryReqIterate",
link_title="next_batch");
}

request sandesh ShowBgpNeighborSummaryReq {
Expand Down
8 changes: 8 additions & 0 deletions src/bgp/bgp_peer_internal.sandesh
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,14 @@

include "bgp/bgp_peer.sandesh"

request sandesh BgpNeighborReqIterate {
1: string iterate_info;
}

request sandesh ShowBgpNeighborSummaryReqIterate {
1: string iterate_info;
}

request sandesh ShowNeighborStatisticsReq {
1: string bgp_or_xmpp; // BGP or XMPP
2: string up_or_down; // "UP" for Established, "DOWN" for not-Established
Expand Down
51 changes: 36 additions & 15 deletions src/bgp/bgp_peer_membership.cc
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,8 @@
#include "bgp/scheduling_group.h"
#include "db/db.h"

using std::vector;

int PeerRibMembershipManager::membership_task_id_ = -1;
const int PeerRibMembershipManager::kMembershipTaskInstanceId;

Expand Down Expand Up @@ -811,6 +813,17 @@ IPeerRib *PeerRibMembershipManager::IPeerRibFind(IPeer *ipeer,
return (iter != peer_rib_set_.end() ? *iter : NULL);
}

//
// Find the IPeerRib corresponding to the given IPeer and BgpTable.
// Const version.
//
const IPeerRib *PeerRibMembershipManager::IPeerRibFind(IPeer *ipeer,
BgpTable *table) const {
IPeerRib peer_rib(ipeer, table, NULL);
PeerRibSet::const_iterator iter = peer_rib_set_.find(&peer_rib);
return (iter != peer_rib_set_.end() ? *iter : NULL);
}

//
// Find or create the IPeerRib corresponding to the given IPeer and BgpTable.
//
Expand All @@ -824,34 +837,42 @@ IPeerRib *PeerRibMembershipManager::IPeerRibLocate(IPeer *ipeer,
// Return the instance-id of the IPeer for the BgpTable.
//
int PeerRibMembershipManager::GetRegistrationId(const IPeer *ipeer,
BgpTable *table) {
const BgpTable *table) const {
assert(ipeer->IsXmppPeer());
IPeerRib *peer_rib = IPeerRibFind(const_cast<IPeer *>(ipeer), table);
const IPeerRib *peer_rib = IPeerRibFind(
const_cast<IPeer *>(ipeer), const_cast<BgpTable *>(table));
return (peer_rib ? peer_rib->instance_id() : -1);
}

void PeerRibMembershipManager::FillPeerMembershipInfo(const IPeer *peer,
BgpNeighborResp *resp) {
BgpNeighborResp *resp) const {
assert(resp->get_routing_tables().size() == 0);
IPeer *nc_peer = const_cast<IPeer *>(peer);

SchedulingGroupManager *sg_mgr = server_->scheduling_group_manager();
SchedulingGroup *sg = sg_mgr->PeerGroup(nc_peer);
resp->set_send_state(sg ?
(sg->PeerInSync(nc_peer) ? "in sync" : "not in sync") :
"not advertising");
IPeerRib peer_rib(nc_peer, NULL, this);
PeerRibMembershipManager::PeerRibSet::iterator it =
peer_rib_set_.lower_bound(&peer_rib);
std::vector<BgpNeighborRoutingTable> table_list;
for (; it != peer_rib_set_.end(); it++) {
if ((*it)->ipeer() != peer) break;
const SchedulingGroupManager *sg_mgr = server_->scheduling_group_manager();
const SchedulingGroup *sg = sg_mgr->PeerGroup(nc_peer);
if (sg) {
resp->set_send_state(
sg->PeerInSync(nc_peer) ? "in sync" : "not in sync");
} else {
resp->set_send_state("not advertising");
}

IPeerRib peer_rib(
nc_peer, NULL, const_cast<PeerRibMembershipManager *>(this));
vector<BgpNeighborRoutingTable> table_list;
for (PeerRibMembershipManager::PeerRibSet::const_iterator it =
peer_rib_set_.lower_bound(&peer_rib);
it != peer_rib_set_.end(); ++it) {
if ((*it)->ipeer() != peer)
break;
BgpNeighborRoutingTable table;
table.set_name((*it)->table()->name());
table.set_current_state("subscribed");
table_list.push_back(table);
}
if (table_list.size()) resp->set_routing_tables(table_list);
if (table_list.size())
resp->set_routing_tables(table_list);
}

void PeerRibMembershipManager::FillRegisteredTable(const IPeer *peer,
Expand Down
7 changes: 4 additions & 3 deletions src/bgp/bgp_peer_membership.h
Original file line number Diff line number Diff line change
Expand Up @@ -168,7 +168,7 @@ class IPeerRib {
// PeerMembershipMgr.
//
struct IPeerRibCompare {
bool operator()(const IPeerRib *lhs, const IPeerRib *rhs) {
bool operator()(const IPeerRib *lhs, const IPeerRib *rhs) const {
return lhs->operator<(*rhs);
}
};
Expand Down Expand Up @@ -226,16 +226,17 @@ class PeerRibMembershipManager {
}
return false;
}
int GetRegistrationId(const IPeer *ipeer, BgpTable *table);
int GetRegistrationId(const IPeer *ipeer, const BgpTable *table) const;

void Enqueue(IPeerRibEvent *event) { event_queue_->Enqueue(event); }

BgpServer *server() { return server_; }
void FillRoutingInstanceTableInfo(ShowRoutingInstanceTable *srit,
const BgpTable *table) const;

void FillPeerMembershipInfo(const IPeer *peer, BgpNeighborResp *resp);
void FillPeerMembershipInfo(const IPeer *peer, BgpNeighborResp *resp) const;
IPeerRib *IPeerRibFind(IPeer *ipeer, BgpTable *table);
const IPeerRib *IPeerRibFind(IPeer *ipeer, BgpTable *table) const;
bool IsQueueEmpty() const { return event_queue_->IsQueueEmpty(); }
void FillRegisteredTable(const IPeer *peer, std::vector<std::string> *list);
size_t GetMembershipCount() const { return peer_rib_set_.size(); }
Expand Down
130 changes: 13 additions & 117 deletions src/bgp/bgp_sandesh.cc
Original file line number Diff line number Diff line change
Expand Up @@ -638,105 +638,6 @@ void ShowRouteReqIterate::HandleRequest() const {
RequestPipeline rp(ps);
}

class ShowNeighborHandler {
public:
static bool CallbackS1(const Sandesh *sr,
const RequestPipeline::PipeSpec ps, int stage,
int instNum, RequestPipeline::InstData *data);
};

bool ShowNeighborHandler::CallbackS1(const Sandesh *sr,
const RequestPipeline::PipeSpec ps,
int stage, int instNum,
RequestPipeline::InstData * data) {
vector<BgpNeighborResp> nbr_list;
const BgpNeighborReq *req =
static_cast<const BgpNeighborReq *>(ps.snhRequest_.get());
BgpSandeshContext *bsc =
static_cast<BgpSandeshContext *>(req->client_context());
RoutingInstanceMgr *rim = bsc->bgp_server->routing_instance_mgr();
if (req->get_domain() != "") {
RoutingInstance *ri = rim->GetRoutingInstance(req->get_domain());
if (ri)
ri->peer_manager()->FillBgpNeighborInfo(
bsc, &nbr_list, req->get_neighbor(), false);
} else {
for (RoutingInstanceMgr::RoutingInstanceIterator it = rim->begin();
it != rim->end(); ++it) {
it->peer_manager()->FillBgpNeighborInfo(
bsc, &nbr_list, req->get_neighbor(), false);
}
}

bsc->ShowNeighborExtension(&nbr_list, req);

BgpNeighborListResp *resp = new BgpNeighborListResp;
resp->set_neighbors(nbr_list);
resp->set_context(req->context());
resp->Response();
return true;
}

// handler for 'show bgp neighbor'
void BgpNeighborReq::HandleRequest() const {
RequestPipeline::PipeSpec ps(this);
RequestPipeline::StageSpec s1;
TaskScheduler *scheduler = TaskScheduler::GetInstance();
s1.taskId_ = scheduler->GetTaskId("bgp::PeerMembership");
s1.cbFn_ = ShowNeighborHandler::CallbackS1;
s1.instances_.push_back(0);
ps.stages_= list_of(s1);
RequestPipeline rp(ps);

}

class ShowNeighborSummaryHandler {
public:
static bool CallbackS1(const Sandesh *sr,
const RequestPipeline::PipeSpec ps,
int stage, int instNum,
RequestPipeline::InstData *data);
};

bool ShowNeighborSummaryHandler::CallbackS1(const Sandesh *sr,
const RequestPipeline::PipeSpec ps,
int stage, int instNum,
RequestPipeline::InstData * data) {
vector<BgpNeighborResp> nbr_list;
const ShowBgpNeighborSummaryReq *req =
static_cast<const ShowBgpNeighborSummaryReq *>(ps.snhRequest_.get());
const string &search_string = req->get_search_string();
BgpSandeshContext *bsc =
static_cast<BgpSandeshContext *>(req->client_context());
RoutingInstanceMgr *rim = bsc->bgp_server->routing_instance_mgr();
for (RoutingInstanceMgr::RoutingInstanceIterator it = rim->begin();
it != rim->end(); ++it) {
it->peer_manager()->FillBgpNeighborInfo(
bsc, &nbr_list, search_string, true);
}

bsc->ShowNeighborSummaryExtension(&nbr_list, req);

ShowBgpNeighborSummaryResp *resp = new ShowBgpNeighborSummaryResp;
resp->set_neighbors(nbr_list);
resp->set_context(req->context());
resp->Response();
return true;
}

// handler for 'show bgp neighbor summary'
void ShowBgpNeighborSummaryReq::HandleRequest() const {
RequestPipeline::PipeSpec ps(this);
RequestPipeline::StageSpec s1;
TaskScheduler *scheduler = TaskScheduler::GetInstance();
s1.taskId_ = scheduler->GetTaskId("bgp::PeerMembership");
s1.cbFn_ = ShowNeighborSummaryHandler::CallbackS1;
s1.instances_.push_back(0);
ps.stages_= list_of(s1);
RequestPipeline rp(ps);

}

class ShowNeighborStatisticsHandler {
public:
static bool CallbackS1(const Sandesh *sr,
Expand Down Expand Up @@ -1403,30 +1304,25 @@ BgpSandeshContext::BgpSandeshContext()

void BgpSandeshContext::SetNeighborShowExtensions(
const NeighborListExtension &show_neighbor,
const NeighborSummaryListExtension &show_neighbor_summary,
const NeighborStatisticsExtension &show_neighbor_statistics) {
show_neighbor_ext_ = show_neighbor;
show_neighbor_summary_ext_ = show_neighbor_summary;
show_neighbor_statistics_ext_ = show_neighbor_statistics;
}

void BgpSandeshContext::ShowNeighborExtension(
std::vector<BgpNeighborResp> *list, const BgpNeighborReq *req) {
if (show_neighbor_ext_) {
show_neighbor_ext_(list, this, req);
}
}

void BgpSandeshContext::ShowNeighborSummaryExtension(
std::vector<BgpNeighborResp> *list, const ShowBgpNeighborSummaryReq *req) {
if (show_neighbor_summary_ext_) {
show_neighbor_summary_ext_(list, this, req);
}
bool BgpSandeshContext::ShowNeighborExtension(const BgpSandeshContext *bsc,
bool summary, uint32_t page_limit, uint32_t iter_limit,
const string &start_neighbor, const string &search_string,
vector<BgpNeighborResp> *list, string *next_neighbor) const {
if (!show_neighbor_ext_)
return true;
bool done = show_neighbor_ext_(bsc, summary, page_limit, iter_limit,
start_neighbor, search_string, list, next_neighbor);
return done;
}

void BgpSandeshContext::ShowNeighborStatisticsExtension(
size_t *count, const ShowNeighborStatisticsReq *req) {
if (show_neighbor_statistics_ext_) {
show_neighbor_statistics_ext_(count, this, req);
}
size_t *count, const ShowNeighborStatisticsReq *req) const {
if (!show_neighbor_statistics_ext_)
return;
show_neighbor_statistics_ext_(count, this, req);
}
30 changes: 11 additions & 19 deletions src/bgp/bgp_sandesh.h
Original file line number Diff line number Diff line change
Expand Up @@ -16,35 +16,28 @@ class ShowBgpNeighborSummaryReq;
class ShowNeighborStatisticsReq;

struct BgpSandeshContext : public SandeshContext {
typedef boost::function<void(std::vector<BgpNeighborResp> *,
BgpSandeshContext *,
const BgpNeighborReq *)>
NeighborListExtension;
typedef boost::function<void(std::vector<BgpNeighborResp> *,
BgpSandeshContext *,
const ShowBgpNeighborSummaryReq *)>
NeighborSummaryListExtension;
typedef boost::function<void(size_t *,
BgpSandeshContext *,
const ShowNeighborStatisticsReq *)>
NeighborStatisticsExtension;
typedef boost::function<bool(const BgpSandeshContext *, bool,
uint32_t, uint32_t, const std::string &, const std::string &,
std::vector<BgpNeighborResp> *, std::string *)> NeighborListExtension;

typedef boost::function<void(size_t *, const BgpSandeshContext *,
const ShowNeighborStatisticsReq *)> NeighborStatisticsExtension;

BgpSandeshContext();

void SetNeighborShowExtensions(
const NeighborListExtension &show_neighbor,
const NeighborSummaryListExtension &show_neighbor_summary,
const NeighborStatisticsExtension &show_neighbor_statistics);

BgpServer *bgp_server;
BgpXmppChannelManager *xmpp_peer_manager;

void ShowNeighborExtension(std::vector<BgpNeighborResp> *list,
const BgpNeighborReq *req);
void ShowNeighborSummaryExtension(std::vector<BgpNeighborResp> *list,
const ShowBgpNeighborSummaryReq *req);
bool ShowNeighborExtension(const BgpSandeshContext *bsc, bool summary,
uint32_t page_limit, uint32_t iter_limit,
const std::string &start_neighbor, const std::string &search_string,
std::vector<BgpNeighborResp> *list, std::string *next_neighbor) const;
void ShowNeighborStatisticsExtension(size_t *count,
const ShowNeighborStatisticsReq *req);
const ShowNeighborStatisticsReq *req) const;

// For testing.
bool test_mode() const { return test_mode_; }
Expand All @@ -59,7 +52,6 @@ struct BgpSandeshContext : public SandeshContext {
uint32_t page_limit_;
uint32_t iter_limit_;
NeighborListExtension show_neighbor_ext_;
NeighborSummaryListExtension show_neighbor_summary_ext_;
NeighborStatisticsExtension show_neighbor_statistics_ext_;
};

Expand Down

0 comments on commit 2992f17

Please sign in to comment.