Skip to content

Commit

Permalink
Delete routes before unsubscribing from an instance
Browse files Browse the repository at this point in the history 
Production code agent does this and control-node expects this as well, from
any subscribed agent (mock or otherwise). Otherwise routes can remain undeleted
in the control-node's table (until the session flaps), or routes are explicitly
deleted.

But some tests still need routes not to be deleted during unsubscribe. Add an
argument to support this functionality to Unsubscribe() API of the mock agent
and use it in tests as necessary.

Also add security-group id to advertised routes. One can set --nsgids to a
desired number. route-id % nsgids is used as the actual sgid advertised. By
increasing nsgids, one can get different sgids across a given set of routes.

Mock-agent code also does not run eor timers (but uses BgpXmppChannel object)
Add necessary protection while cancelling the timer

Change-Id: Ic4f7ed2f0645151a7521130c09bb378418bc605a
Partial-Bug: #1464016
  • Loading branch information
@ananth-at-camphor-networks
ananth-at-camphor-networks committed Sep 7, 2016
1 parent fc259df commit 6b17e4f
Show file tree
Hide file tree
Showing 6 changed files with 190 additions and 38 deletions.
2 changes: 1 addition & 1 deletion src/bgp/bgp_xmpp_channel.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -575,7 +575,7 @@ bool BgpXmppChannel::XmppPeer::SendUpdate(const uint8_t *msg, size_t msgsize) {

// If EndOfRib Send timer is running, cancel it and reschedule it
// after socket gets unblocked.
if (parent_->eor_send_timer_->running())
if (parent_->eor_send_timer_ && parent_->eor_send_timer_->running())
parent_->eor_send_timer_->Cancel();
}
return send_ready_;
Expand Down
24 changes: 21 additions & 3 deletions src/bgp/test/bgp_stress_test.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -145,6 +145,7 @@ static vector<float> d_events_weight_ = boost::assign::list_of
static bool d_external_mode_ = false;
static int d_instances_ = 1;
static int d_routes_ = 1;
static int d_sgids_ = 1;
static int d_peers_ = 1;
static int d_agents_ = 1;
static int d_targets_ = 1;
Expand Down Expand Up @@ -187,6 +188,7 @@ static vector<bool> xmpp_close_from_control_node =
boost::assign::list_of(d_close_from_control_node_);
static vector<bool> xmpp_auth_enabled =
boost::assign::list_of(d_xmpp_auth_enabled_);
static int n_sgids = d_sgids_;

static int d_db_walker_wait_ = 0;
static int d_wait_for_idle_ = 30; // Seconds
Expand Down Expand Up @@ -1590,15 +1592,22 @@ void BgpStressTest::AddXmppRoute(int instance_id, int agent_id, int route_id) {
GetInstanceName(instance_id)))
return;

std::vector<int> sgids;
if (n_sgids)
sgids.push_back(route_id % n_sgids);
test::RouteAttributes attributes(100, sgids);

BGP_STRESS_TEST_EVENT_LOG(BgpStressTestEvent::ADD_XMPP_ROUTE);
Ip4Prefix prefix = GetAgentRoute(agent_id + 1, instance_id, route_id);
xmpp_agents_[agent_id]->AddRoute(GetInstanceName(instance_id),
prefix.ToString(),
GetAgentNexthop(agent_id, route_id));
GetAgentNexthop(agent_id, route_id),
attributes);

xmpp_agents_[agent_id]->AddEnetRoute(GetInstanceName(instance_id),
GetEnetPrefix(prefix.ToString()),
GetAgentNexthop(agent_id, route_id));
GetAgentNexthop(agent_id, route_id),
attributes);

if (instance_id == 0)
return;
Expand All @@ -1622,7 +1631,8 @@ void BgpStressTest::AddXmppRoute(int instance_id, int agent_id, int route_id) {
agent_nexthop.push_back(
test::NextHop(GetAgentNexthop(agent_id, route_id), 0));
xmpp_agents_[agent_id]->AddInet6Route(GetInstanceName(instance_id),
prefix6.ToString(), agent_nexthop);
prefix6.ToString(), agent_nexthop,
attributes);
}
}

Expand Down Expand Up @@ -2790,6 +2800,8 @@ static void process_command_line_args(int argc, const char **argv) {
"set number of bgp peers")
("nroutes", value<int>()->default_value(d_routes_),
"set number of routes")
("nsgids", value<int>()->default_value(d_sgids_),
"set number of security-group ids")
("ntargets", value<int>()->default_value(d_targets_),
"set number of route targets (minium 1)")
("nvms", value<int>()->default_value(d_vms_count_),
Expand Down Expand Up @@ -2959,6 +2971,10 @@ static void process_command_line_args(int argc, const char **argv) {
d_routes_ = vm["nroutes"].as<int>();
cmd_line_arg_set = true;
}
if (vm.count("nsgids")) {
d_sgids_ = vm["nsgids"].as<int>();
cmd_line_arg_set = true;
}
if (vm.count("npeers")) {
d_peers_ = vm["npeers"].as<int>();
cmd_line_arg_set = true;
Expand Down Expand Up @@ -3221,6 +3237,8 @@ static void process_command_line_args(int argc, const char **argv) {
n_routes.clear();
n_routes.push_back(d_routes_);

n_sgids = d_sgids_;

n_peers.clear();
n_peers.push_back(d_peers_);

Expand Down
30 changes: 15 additions & 15 deletions src/bgp/test/bgp_xmpp_deferq_test.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1034,7 +1034,7 @@ TEST_F(BgpXmppUnitTest, UnregisterWithDeletedRoutingInstance) {

// Unsubscribe from the blue instance and make sure that the unsubscribe
// message has been processed on the bgp server.
agent_a_->Unsubscribe("blue");
agent_a_->Unsubscribe("blue", -1, true, false);
TASK_UTIL_EXPECT_EQ(3, bgp_channel_manager_->channel_->Count());
TASK_UTIL_EXPECT_TRUE(
PeerNotRegistered(bgp_channel_manager_->channel_, "blue"));
Expand Down Expand Up @@ -1091,7 +1091,7 @@ TEST_F(BgpXmppUnitTest, RegisterUnregisterWithDeletedRoutingInstance) {

// Unsubscribe from the blue instance and make sure that the unsubscribe
// message has been processed on the bgp server.
agent_a_->Unsubscribe("blue");
agent_a_->Unsubscribe("blue", -1, true, false);
TASK_UTIL_EXPECT_EQ(3, bgp_channel_manager_->channel_->Count());
TASK_UTIL_EXPECT_TRUE(
PeerNotRegistered(bgp_channel_manager_->channel_, "blue"));
Expand Down Expand Up @@ -1312,7 +1312,7 @@ TEST_F(BgpXmppUnitTest, DuplicateRegisterWithDeletedRoutingInstance2) {
TASK_UTIL_EXPECT_TRUE(blue->deleted());

// Send unsubscribe for the blue instance.
agent_a_->Unsubscribe("blue", -1);
agent_a_->Unsubscribe("blue", -1, -1, true, false);
TASK_UTIL_EXPECT_FALSE(
PeerRegistered(bgp_channel_manager_->channel_, "blue", 1));

Expand Down Expand Up @@ -2072,7 +2072,7 @@ TEST_F(BgpXmppUnitTest, RegisterUnregisterWithDeletedBgpTableThenRegisterAgain1)
PeerNotRegistered(bgp_channel_manager_->channel_, "blue"));

// Unsubscribe for the old incarnation.
agent_a_->Unsubscribe("blue", -1, false);
agent_a_->Unsubscribe("blue", -1, false, false);
TASK_UTIL_EXPECT_EQ(3, bgp_channel_manager_->channel_->Count());
TASK_UTIL_EXPECT_NE(0,
PeerTableUnsubscribeComplete(bgp_channel_manager_->channel_));
Expand Down Expand Up @@ -2188,7 +2188,7 @@ TEST_F(BgpXmppUnitTest, RegisterUnregisterWithDeletedBgpTableThenRegisterAgain2)

// Unsubscribe for the old incarnation. The unsubscribe request will get
// enqueued in the membership manager, but won't be processed.
agent_a_->Unsubscribe("blue", -1, false);
agent_a_->Unsubscribe("blue", -1, false, false);
TASK_UTIL_EXPECT_EQ(3, bgp_channel_manager_->channel_->Count());
TASK_UTIL_EXPECT_TRUE(
PeerHasPendingMembershipRequests(bgp_channel_manager_->channel_));
Expand Down Expand Up @@ -3171,11 +3171,11 @@ TEST_F(BgpXmppSerializeMembershipReqTest, FlushDeferQForVrfAndTable1) {
TASK_UTIL_EXPECT_EQ(6, bgp_channel_manager_->channel_->Count());
TASK_UTIL_EXPECT_EQ(4, PeerDeferQSize(bgp_channel_manager_->channel_));

agent_a_->Unsubscribe("blue", -1, false);
agent_a_->Unsubscribe("blue", -1, false, false);
TASK_UTIL_EXPECT_EQ(7, bgp_channel_manager_->channel_->Count());
TASK_UTIL_EXPECT_EQ(2, PeerDeferQSize(bgp_channel_manager_->channel_));

agent_a_->Unsubscribe("red", -1, false);
agent_a_->Unsubscribe("red", -1, false, false);
TASK_UTIL_EXPECT_EQ(8, bgp_channel_manager_->channel_->Count());
TASK_UTIL_EXPECT_EQ(0, PeerDeferQSize(bgp_channel_manager_->channel_));

Expand Down Expand Up @@ -3208,11 +3208,11 @@ TEST_F(BgpXmppSerializeMembershipReqTest, FlushDeferQForVrfAndTable2) {
TASK_UTIL_EXPECT_EQ(6, bgp_channel_manager_->channel_->Count());
TASK_UTIL_EXPECT_EQ(4, PeerDeferQSize(bgp_channel_manager_->channel_));

agent_a_->Unsubscribe("red", -1, false);
agent_a_->Unsubscribe("red", -1, false, false);
TASK_UTIL_EXPECT_EQ(7, bgp_channel_manager_->channel_->Count());
TASK_UTIL_EXPECT_EQ(2, PeerDeferQSize(bgp_channel_manager_->channel_));

agent_a_->Unsubscribe("blue", -1, false);
agent_a_->Unsubscribe("blue", -1, false, false);
TASK_UTIL_EXPECT_EQ(8, bgp_channel_manager_->channel_->Count());
TASK_UTIL_EXPECT_EQ(0, PeerDeferQSize(bgp_channel_manager_->channel_));

Expand Down Expand Up @@ -3246,15 +3246,15 @@ TEST_F(BgpXmppSerializeMembershipReqTest, FlushDeferQForVrf1) {
TASK_UTIL_EXPECT_EQ(9, bgp_channel_manager_->channel_->Count());
TASK_UTIL_EXPECT_EQ(6, PeerDeferQSize(bgp_channel_manager_->channel_));

agent_a_->Unsubscribe("red1", -1, false);
agent_a_->Unsubscribe("red1", -1, false, false);
TASK_UTIL_EXPECT_EQ(10, bgp_channel_manager_->channel_->Count());
TASK_UTIL_EXPECT_EQ(4, PeerDeferQSize(bgp_channel_manager_->channel_));

agent_a_->Unsubscribe("red2", -1, false);
agent_a_->Unsubscribe("red2", -1, false, false);
TASK_UTIL_EXPECT_EQ(11, bgp_channel_manager_->channel_->Count());
TASK_UTIL_EXPECT_EQ(2, PeerDeferQSize(bgp_channel_manager_->channel_));

agent_a_->Unsubscribe("red3", -1, false);
agent_a_->Unsubscribe("red3", -1, false, false);
TASK_UTIL_EXPECT_EQ(12, bgp_channel_manager_->channel_->Count());
TASK_UTIL_EXPECT_EQ(0, PeerDeferQSize(bgp_channel_manager_->channel_));
}
Expand Down Expand Up @@ -3282,15 +3282,15 @@ TEST_F(BgpXmppSerializeMembershipReqTest, FlushDeferQForVrf2) {
TASK_UTIL_EXPECT_EQ(9, bgp_channel_manager_->channel_->Count());
TASK_UTIL_EXPECT_EQ(6, PeerDeferQSize(bgp_channel_manager_->channel_));

agent_a_->Unsubscribe("red2", -1, false);
agent_a_->Unsubscribe("red2", -1, false, false);
TASK_UTIL_EXPECT_EQ(10, bgp_channel_manager_->channel_->Count());
TASK_UTIL_EXPECT_EQ(4, PeerDeferQSize(bgp_channel_manager_->channel_));

agent_a_->Unsubscribe("red1", -1, false);
agent_a_->Unsubscribe("red1", -1, false, false);
TASK_UTIL_EXPECT_EQ(11, bgp_channel_manager_->channel_->Count());
TASK_UTIL_EXPECT_EQ(2, PeerDeferQSize(bgp_channel_manager_->channel_));

agent_a_->Unsubscribe("red3", -1, false);
agent_a_->Unsubscribe("red3", -1, false, false);
TASK_UTIL_EXPECT_EQ(12, bgp_channel_manager_->channel_->Count());
TASK_UTIL_EXPECT_EQ(0, PeerDeferQSize(bgp_channel_manager_->channel_));
}
Expand Down
9 changes: 5 additions & 4 deletions src/bgp/test/graceful_restart_test.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -323,7 +323,7 @@ class GracefulRestartTest : public ::testing::TestWithParam<TestParams> {
void AddRoutes();
void CreateAgents();
void Subscribe();
void UnSubscribe();
void Unsubscribe();
test::NextHops GetNextHops(test::NetworkAgentMock *agent, int instance_id);
void AddOrDeleteXmppRoutes(bool add, int nroutes = -1,
int down_agents = -1);
Expand Down Expand Up @@ -861,7 +861,7 @@ void GracefulRestartTest::Subscribe() {
WaitForIdle();
}

void GracefulRestartTest::UnSubscribe() {
void GracefulRestartTest::Unsubscribe() {

BOOST_FOREACH(test::NetworkAgentMock *agent, xmpp_agents_) {
agent->Unsubscribe(BgpConfigManager::kMasterInstance);
Expand Down Expand Up @@ -1036,7 +1036,7 @@ void GracefulRestartTest::DeleteRoutingInstances(vector<int> instances,
continue;
if (std::find(dont_unsubscribe.begin(), dont_unsubscribe.end(),
agent) == dont_unsubscribe.end())
agent->Unsubscribe(instance_name);
agent->Unsubscribe(instance_name, -1, true, false);
}

BOOST_FOREACH(BgpPeerTest *peer, bgp_peers_) {
Expand Down Expand Up @@ -1460,6 +1460,7 @@ void GracefulRestartTest::GracefulRestartTestRun () {
BOOST_FOREACH(test::NetworkAgentMock *agent, n_down_from_agents_) {
WaitForAgentToBeEstablished(agent);
agent->SessionDown();
dont_unsubscribe.push_back(agent);
TASK_UTIL_EXPECT_FALSE(agent->IsEstablished());
total_routes -= remaining_instances * n_routes_;
}
Expand Down Expand Up @@ -1577,7 +1578,7 @@ void GracefulRestartTest::GracefulRestartTestRun () {
instances_to_delete_before_gr_.end())
continue;
if (std::find(instances_to_delete_during_gr_.begin(),
instances_to_delete_during_gr_.end(), instance_id) !=
instances_to_delete_during_gr_.end(), instance_id) !=
instances_to_delete_during_gr_.end())
continue;
string instance_name = "instance" +
Expand Down

0 comments on commit 6b17e4f

Please sign in to comment.