Skip to content

Commit

Permalink
Verify MvpnNeigbor correctly after BGP RouterID change
Browse files Browse the repository at this point in the history
MVPN Neighbor information is not verified correctly, after bgp router
id changes. It can so happen that after router-id is changed, before
the router-id change is fully processed, part of the verification over
old data is complete. Later the test fails when verifying against new
data, especially the new RouterId information from the neighbor.

Fix by ensuring that MvpnNeighbor info is checked correctly with retry
over entire type1 routes lookup

Change-Id: I836451c7e4f2487f19850fe94cfdcefe1c3b1aca
Closes-Bug: 1777488
  • Loading branch information
ananth-at-camphor-networks committed Jun 18, 2018
1 parent 62bbf9f commit 535f4fb
Show file tree
Hide file tree
Showing 2 changed files with 54 additions and 34 deletions.
50 changes: 50 additions & 0 deletions src/bgp/test/bgp_mvpn_test.cc
Expand Up @@ -634,6 +634,56 @@ class BgpMvpnTest : public ::testing::TestWithParam<TestParams> {
}
}

bool CheckMvpnNeighborRoute(size_t i) const {
if (red_[i-1]->Size() != 1)
return false;
if (!red_[i-1]->FindType1ADRoute())
return false;

if (blue_[i-1]->Size() != 1)
return false;
if (!blue_[i-1]->FindType1ADRoute())
return false;

if (green_[i-1]->Size() != 3) // 1 green1+1 red1+1 blue1
return false;
if (!green_[i-1]->FindType1ADRoute())
return false;

// Verify that only green1 has discovered a neighbor from red1.
if (red_[i-1]->manager()->neighbors_count())
return false;
if (blue_[i-1]->manager()->neighbors_count())
return false;
if (green_[i-1]->manager()->neighbors_count() != 2)
return false;

MvpnNeighbor neighbor;
if (!green_[i-1]->manager()->FindNeighbor(
*(red_[i-1]->routing_instance()->GetRD()), &neighbor)) {
return false;
}
if (!(*(red_[i-1]->routing_instance()->GetRD()) == neighbor.rd()))
return false;
if (neighbor.source_as())
return false;
error_code err;
if (IpAddress::from_string("127.0.0.2", err) != neighbor.originator())
return false;

if (!green_[i-1]->manager()->FindNeighbor(
*(blue_[i-1]->routing_instance()->GetRD()), &neighbor)) {
return false;
}
if (!(*(blue_[i-1]->routing_instance()->GetRD()) == neighbor.rd()))
return false;
if (neighbor.source_as())
return false;
if (IpAddress::from_string("127.0.0.2", err) != neighbor.originator())
return false;
return true;
}

void VerifyInitialState(bool pm_configure = true,
size_t red1c = 0, size_t blue1c = 0, size_t green1c = 0,
size_t masterc = 0, size_t red1_nopm_c = 0, size_t blue1_nopm_c = 0,
Expand Down
38 changes: 4 additions & 34 deletions src/bgp/test/bgp_mvpn_test2.cc
Expand Up @@ -17,41 +17,11 @@ TEST_P(BgpMvpnTest, Type1ADLocalWithIdentifierChanged) {
}
}
VerifyInitialState();
error_code err;
UpdateBgpIdentifier("127.0.0.2");
TASK_UTIL_EXPECT_EQ(4*instances_set_count_ + 1, master_->Size());

for (size_t i = 1; i <= instances_set_count_; i++) {
TASK_UTIL_EXPECT_EQ(1, red_[i-1]->Size());
TASK_UTIL_EXPECT_NE(static_cast<MvpnRoute *>(NULL),
red_[i-1]->FindType1ADRoute());

TASK_UTIL_EXPECT_EQ(1, blue_[i-1]->Size());
TASK_UTIL_EXPECT_NE(static_cast<MvpnRoute *>(NULL),
blue_[i-1]->FindType1ADRoute());

TASK_UTIL_EXPECT_EQ(3, green_[i-1]->Size()); // 1 green1+1 red1+1 blue1
TASK_UTIL_EXPECT_NE(static_cast<MvpnRoute *>(NULL),
green_[i-1]->FindType1ADRoute());

// Verify that only green1 has discovered a neighbor from red1.
TASK_UTIL_EXPECT_EQ(0, red_[i-1]->manager()->neighbors_count());
TASK_UTIL_EXPECT_EQ(0, blue_[i-1]->manager()->neighbors_count());
TASK_UTIL_EXPECT_EQ(2, green_[i-1]->manager()->neighbors_count());

MvpnNeighbor neighbor;
EXPECT_TRUE(green_[i-1]->manager()->FindNeighbor(
*(red_[i-1]->routing_instance()->GetRD()), &neighbor));
EXPECT_EQ(*(red_[i-1]->routing_instance()->GetRD()), neighbor.rd());
EXPECT_EQ(0, neighbor.source_as());
EXPECT_EQ(IpAddress::from_string("127.0.0.2", err),
neighbor.originator());

EXPECT_TRUE(green_[i-1]->manager()->FindNeighbor(
*(blue_[i-1]->routing_instance()->GetRD()), &neighbor));
EXPECT_EQ(*(blue_[i-1]->routing_instance()->GetRD()), neighbor.rd());
EXPECT_EQ(0, neighbor.source_as());
EXPECT_EQ(IpAddress::from_string("127.0.0.2", err),
neighbor.originator());
}
// Retry Type1 route checks as router-id update processing is executed
// completely asynchronously.
for (size_t i = 1; i <= instances_set_count_; i++)
TASK_UTIL_EXPECT_TRUE(CheckMvpnNeighborRoute(i));
}

0 comments on commit 535f4fb

Please sign in to comment.