Skip to content

Commit

Permalink
Merge "Do not apply routing policy to replicated route"
Browse files Browse the repository at this point in the history
  • Loading branch information
@opencontrail-ci-admin
Zuul authored and opencontrail-ci-admin committed Nov 10, 2016
2 parents 298b7ea + 16377d8 commit dce1698
Show file tree
Hide file tree
Showing 4 changed files with 304 additions and 16 deletions.
3 changes: 3 additions & 0 deletions src/bgp/routing-instance/routing_instance.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1516,6 +1516,9 @@ bool RoutingInstance::HasExportTarget(const ExtCommunity *extcomm) const {

bool RoutingInstance::ProcessRoutingPolicy(const BgpRoute *route,
BgpPath *path) const {
// Don't apply routing policy on secondary path
if (path->IsReplicated())
return true;
const RoutingPolicyMgr *policy_mgr = server()->routing_policy_mgr();
// Take snapshot of original attribute
BgpAttr *out_attr = new BgpAttr(*(path->GetOriginalAttr()));
Expand Down
280 changes: 264 additions & 16 deletions src/bgp/test/routing_policy_test.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2507,7 +2507,8 @@ TEST_F(RoutingPolicyTest, ProtocolMatchStaticRoute) {
// 1. two routing instance test_0 and test_1
// 2. Routing instnaces are connected with each other,
// 3. two routing policies to edit local pref of matching route.
// With above configuration, validate that route notify doesn't trigger
// Verify that routing policy is not applied to replicated route.
// Also, with above configuration, validate that route notify doesn't trigger
// re-addition of replicated route paths.
//
TEST_F(RoutingPolicyTest, Policy_RouteReplicate) {
Expand Down Expand Up @@ -2535,7 +2536,7 @@ TEST_F(RoutingPolicyTest, Policy_RouteReplicate) {
BgpRoute *rt = RouteLookup<Inet6Definition>("test_0.inet6.0",
"2001:db8:85a3::8a2e:370:7335/128");
ASSERT_TRUE(rt != NULL);
ASSERT_TRUE(rt->count() == 2);
ASSERT_TRUE(rt->count() == 1);
VERIFY_EQ(peers_[0], rt->BestPath()->GetPeer());
ASSERT_TRUE(rt->BestPath()->IsFeasible() == true);
const BgpAttr *attr = rt->BestPath()->GetAttr();
Expand All @@ -2545,15 +2546,14 @@ TEST_F(RoutingPolicyTest, Policy_RouteReplicate) {
ASSERT_TRUE(policy_local_pref == 200);
ASSERT_TRUE(original_local_pref == 100);

// Take the time stamp of the replicated path on the route.
// This will be compared with replicated path after triggering dummy
// notification of the primary route.
uint64_t rt1_replicated_path_time_stamp;
// Since the routing policy is not applied on the replicated route,
// the replicated route in test_1.inet6.0 table retains its
// local-pref of 200 and remains the best path.
// So it is no longer replicated to connected VRF test_0.inet6.0 table.
for (Route::PathList::iterator it = rt->GetPathList().begin();
it != rt->GetPathList().end(); it++) {
BgpPath *path = static_cast<BgpPath *>(it.operator->());
if (path->IsReplicated())
rt1_replicated_path_time_stamp = path->time_stamp_usecs();
ASSERT_FALSE(path->IsReplicated());
}

rt = RouteLookup<Inet6Definition>("test_1.inet6.0",
Expand All @@ -2562,13 +2562,17 @@ TEST_F(RoutingPolicyTest, Policy_RouteReplicate) {
ASSERT_TRUE(rt->count() == 2);
VERIFY_EQ(peers_[0], rt->BestPath()->GetPeer());
ASSERT_TRUE(rt->BestPath()->IsFeasible() == true);
ASSERT_TRUE(rt->BestPath()->IsReplicated() == true);
attr = rt->BestPath()->GetAttr();
orig_attr = rt->BestPath()->GetOriginalAttr();
original_local_pref = orig_attr->local_pref();
policy_local_pref = attr->local_pref();
ASSERT_TRUE(policy_local_pref == 50);
ASSERT_TRUE(policy_local_pref == 200);
ASSERT_TRUE(original_local_pref == 200);

// Take the time stamp of the replicated path on the route.
// This will be compared with replicated path after triggering dummy
// notification of the primary route.
uint64_t rt2_replicated_path_time_stamp;
for (Route::PathList::iterator it = rt->GetPathList().begin();
it != rt->GetPathList().end(); it++) {
Expand All @@ -2590,18 +2594,17 @@ TEST_F(RoutingPolicyTest, Policy_RouteReplicate) {
rt = RouteLookup<Inet6Definition>("test_0.inet6.0",
"2001:db8:85a3::8a2e:370:7335/128");
ASSERT_TRUE(rt != NULL);
ASSERT_TRUE(rt->count() == 2);
ASSERT_TRUE(rt->count() == 1);

uint64_t rt1_replicated_path_time_stamp_after_change;
// Since the routing policy is not applied on the replicated route,
// the replicated route in test_1.inet6.0 table retains its
// local-pref of 200 and remains the best path.
// So it is no longer replicated to connected VRF test_0.inet6.0 table.
for (Route::PathList::iterator it = rt->GetPathList().begin();
it != rt->GetPathList().end(); it++) {
BgpPath *path = static_cast<BgpPath *>(it.operator->());
if (path->IsReplicated())
rt1_replicated_path_time_stamp_after_change
= path->time_stamp_usecs();
ASSERT_FALSE(path->IsReplicated());
}
ASSERT_TRUE(rt1_replicated_path_time_stamp_after_change
== rt1_replicated_path_time_stamp);

rt = RouteLookup<Inet6Definition>("test_1.inet6.0",
"2001:db8:85a3::8a2e:370:7335/128");
Expand All @@ -2628,6 +2631,251 @@ TEST_F(RoutingPolicyTest, Policy_RouteReplicate) {
"routing-instance", "test_1", "connection");
}

//
// 1. two routing instance test_0 and test_1
// 2. Routing instnaces are connected with each other,
// 3. two routing policies to edit local pref of matching route.
// Verify that routing policy is not applied to replicated route.
// Now modify the rules of the policy applied on the routing instance and
// verify that update of policy doesn't trigger route change.
//
TEST_F(RoutingPolicyTest, Policy_RouteReplicate_PolicyUpdate_0) {
string content =
FileRead("controller/src/bgp/testdata/routing_policy_7i.xml");
EXPECT_TRUE(parser_.Parse(content));
task_util::WaitForIdle();

ifmap_test_util::IFMapMsgLink(&config_db_, "routing-instance", "test_0",
"routing-instance", "test_1", "connection");
boost::system::error_code ec;
peers_.push_back(
new BgpPeerMock(Ip4Address::from_string("192.168.0.1", ec)));
peers_.push_back(
new BgpPeerMock(Ip4Address::from_string("192.168.0.2", ec)));

AddRoute<Inet6Definition>(peers_[0], "test_0.inet6.0",
"2001:db8:85a3::8a2e:370:7335/128", 100, list_of("23:13"));
AddRoute<Inet6Definition>(peers_[1], "test_1.inet6.0",
"2001:db8:85a3::8a2e:370:7335/128", 100, list_of("23:13"));
task_util::WaitForIdle();

VERIFY_EQ(1, RouteCount("test_1.inet6.0"));
VERIFY_EQ(1, RouteCount("test_0.inet6.0"));
BgpRoute *rt = RouteLookup<Inet6Definition>("test_0.inet6.0",
"2001:db8:85a3::8a2e:370:7335/128");
ASSERT_TRUE(rt != NULL);
ASSERT_TRUE(rt->count() == 1);
VERIFY_EQ(peers_[0], rt->BestPath()->GetPeer());
ASSERT_TRUE(rt->BestPath()->IsFeasible() == true);
const BgpAttr *attr = rt->BestPath()->GetAttr();
const BgpAttr *orig_attr = rt->BestPath()->GetOriginalAttr();
uint32_t original_local_pref = orig_attr->local_pref();
uint32_t policy_local_pref = attr->local_pref();
ASSERT_TRUE(policy_local_pref == 200);
ASSERT_TRUE(original_local_pref == 100);

// Since the routing policy is not applied on the replicated route,
// the replicated route in test_1.inet6.0 table retains its
// local-pref of 200 and remains the best path.
// So it is no longer replicated to connected VRF test_0.inet6.0 table.
for (Route::PathList::iterator it = rt->GetPathList().begin();
it != rt->GetPathList().end(); it++) {
BgpPath *path = static_cast<BgpPath *>(it.operator->());
ASSERT_FALSE(path->IsReplicated());
}

rt = RouteLookup<Inet6Definition>("test_1.inet6.0",
"2001:db8:85a3::8a2e:370:7335/128");
ASSERT_TRUE(rt != NULL);
ASSERT_TRUE(rt->count() == 2);
VERIFY_EQ(peers_[0], rt->BestPath()->GetPeer());
ASSERT_TRUE(rt->BestPath()->IsFeasible() == true);
ASSERT_TRUE(rt->BestPath()->IsReplicated() == true);
attr = rt->BestPath()->GetAttr();
orig_attr = rt->BestPath()->GetOriginalAttr();
original_local_pref = orig_attr->local_pref();
policy_local_pref = attr->local_pref();
ASSERT_TRUE(policy_local_pref == 200);
ASSERT_TRUE(original_local_pref == 200);


content = FileRead("controller/src/bgp/testdata/routing_policy_0k.xml");
EXPECT_TRUE(parser_.Parse(content));
task_util::WaitForIdle();

// Verify that replicated routes are not updated after policy change
VERIFY_EQ(1, RouteCount("test_1.inet6.0"));
VERIFY_EQ(1, RouteCount("test_0.inet6.0"));
rt = RouteLookup<Inet6Definition>("test_0.inet6.0",
"2001:db8:85a3::8a2e:370:7335/128");
ASSERT_TRUE(rt != NULL);
ASSERT_TRUE(rt->count() == 1);
VERIFY_EQ(peers_[0], rt->BestPath()->GetPeer());
ASSERT_TRUE(rt->BestPath()->IsFeasible() == true);
attr = rt->BestPath()->GetAttr();
orig_attr = rt->BestPath()->GetOriginalAttr();
original_local_pref = orig_attr->local_pref();
policy_local_pref = attr->local_pref();
ASSERT_TRUE(policy_local_pref == 200);
ASSERT_TRUE(original_local_pref == 100);

// Since the routing policy is not applied on the replicated route,
// the replicated route in test_1.inet6.0 table retains its
// local-pref of 200 and remains the best path.
// So it is no longer replicated to connected VRF test_0.inet6.0 table.
for (Route::PathList::iterator it = rt->GetPathList().begin();
it != rt->GetPathList().end(); it++) {
BgpPath *path = static_cast<BgpPath *>(it.operator->());
ASSERT_FALSE(path->IsReplicated());
}

rt = RouteLookup<Inet6Definition>("test_1.inet6.0",
"2001:db8:85a3::8a2e:370:7335/128");
ASSERT_TRUE(rt != NULL);
ASSERT_TRUE(rt->count() == 2);
VERIFY_EQ(peers_[0], rt->BestPath()->GetPeer());
ASSERT_TRUE(rt->BestPath()->IsFeasible() == true);
ASSERT_TRUE(rt->BestPath()->IsReplicated() == true);
attr = rt->BestPath()->GetAttr();
orig_attr = rt->BestPath()->GetOriginalAttr();
original_local_pref = orig_attr->local_pref();
policy_local_pref = attr->local_pref();
ASSERT_TRUE(policy_local_pref == 200);
ASSERT_TRUE(original_local_pref == 200);

DeleteRoute<Inet6Definition>(peers_[0], "test_0.inet6.0",
"2001:db8:85a3::8a2e:370:7335/128");
DeleteRoute<Inet6Definition>(peers_[1], "test_1.inet6.0",
"2001:db8:85a3::8a2e:370:7335/128");
task_util::WaitForIdle();
ifmap_test_util::IFMapMsgUnlink(&config_db_, "routing-instance", "test_0",
"routing-instance", "test_1", "connection");
}

//
// 1. two routing instance test_0 and test_1
// 2. Routing instnaces are connected with each other,
// 3. two routing policies(basic_0 & basic_1) to edit local pref is applied on
// test_0 & test_1 routing instances respectively
// Verify that routing policy is not applied to replicated route.
// Now apply new policy(basic_2) on the routing instance(test_0) and
// verify that update of policy doesn't trigger route change.
//
TEST_F(RoutingPolicyTest, Policy_RouteReplicate_PolicyUpdate_1) {
string content =
FileRead("controller/src/bgp/testdata/routing_policy_7i.xml");
EXPECT_TRUE(parser_.Parse(content));
task_util::WaitForIdle();

ifmap_test_util::IFMapMsgLink(&config_db_, "routing-instance", "test_0",
"routing-instance", "test_1", "connection");
boost::system::error_code ec;
peers_.push_back(
new BgpPeerMock(Ip4Address::from_string("192.168.0.1", ec)));
peers_.push_back(
new BgpPeerMock(Ip4Address::from_string("192.168.0.2", ec)));

AddRoute<Inet6Definition>(peers_[0], "test_0.inet6.0",
"2001:db8:85a3::8a2e:370:7335/128", 100, list_of("23:13"));
AddRoute<Inet6Definition>(peers_[1], "test_1.inet6.0",
"2001:db8:85a3::8a2e:370:7335/128", 100, list_of("23:13"));
task_util::WaitForIdle();

VERIFY_EQ(1, RouteCount("test_1.inet6.0"));
VERIFY_EQ(1, RouteCount("test_0.inet6.0"));
BgpRoute *rt = RouteLookup<Inet6Definition>("test_0.inet6.0",
"2001:db8:85a3::8a2e:370:7335/128");
ASSERT_TRUE(rt != NULL);
ASSERT_TRUE(rt->count() == 1);
VERIFY_EQ(peers_[0], rt->BestPath()->GetPeer());
ASSERT_TRUE(rt->BestPath()->IsFeasible() == true);
const BgpAttr *attr = rt->BestPath()->GetAttr();
const BgpAttr *orig_attr = rt->BestPath()->GetOriginalAttr();
uint32_t original_local_pref = orig_attr->local_pref();
uint32_t policy_local_pref = attr->local_pref();
ASSERT_TRUE(policy_local_pref == 200);
ASSERT_TRUE(original_local_pref == 100);

// Since the routing policy is not applied on the replicated route,
// the replicated route in test_1.inet6.0 table retains its
// local-pref of 200 and remains the best path.
// So it is no longer replicated to connected VRF test_0.inet6.0 table.
for (Route::PathList::iterator it = rt->GetPathList().begin();
it != rt->GetPathList().end(); it++) {
BgpPath *path = static_cast<BgpPath *>(it.operator->());
ASSERT_FALSE(path->IsReplicated());
}

rt = RouteLookup<Inet6Definition>("test_1.inet6.0",
"2001:db8:85a3::8a2e:370:7335/128");
ASSERT_TRUE(rt != NULL);
ASSERT_TRUE(rt->count() == 2);
VERIFY_EQ(peers_[0], rt->BestPath()->GetPeer());
ASSERT_TRUE(rt->BestPath()->IsFeasible() == true);
ASSERT_TRUE(rt->BestPath()->IsReplicated() == true);
attr = rt->BestPath()->GetAttr();
orig_attr = rt->BestPath()->GetOriginalAttr();
original_local_pref = orig_attr->local_pref();
policy_local_pref = attr->local_pref();
ASSERT_TRUE(policy_local_pref == 200);
ASSERT_TRUE(original_local_pref == 200);


content = FileRead("controller/src/bgp/testdata/routing_policy_0l.xml");
EXPECT_TRUE(parser_.Parse(content));
task_util::WaitForIdle();

// Verify that replicated routes are not updated after policy change
VERIFY_EQ(1, RouteCount("test_1.inet6.0"));
VERIFY_EQ(1, RouteCount("test_0.inet6.0"));
rt = RouteLookup<Inet6Definition>("test_0.inet6.0",
"2001:db8:85a3::8a2e:370:7335/128");
ASSERT_TRUE(rt != NULL);
ASSERT_TRUE(rt->count() == 1);
VERIFY_EQ(peers_[0], rt->BestPath()->GetPeer());
ASSERT_TRUE(rt->BestPath()->IsFeasible() == true);
attr = rt->BestPath()->GetAttr();
orig_attr = rt->BestPath()->GetOriginalAttr();
original_local_pref = orig_attr->local_pref();
policy_local_pref = attr->local_pref();
ASSERT_TRUE(policy_local_pref == 200);
ASSERT_TRUE(original_local_pref == 100);

// Since the routing policy is not applied on the replicated route,
// the replicated route in test_1.inet6.0 table retains its
// local-pref of 200 and remains the best path.
// So it is no longer replicated to connected VRF test_0.inet6.0 table.
for (Route::PathList::iterator it = rt->GetPathList().begin();
it != rt->GetPathList().end(); it++) {
BgpPath *path = static_cast<BgpPath *>(it.operator->());
ASSERT_FALSE(path->IsReplicated());
}

rt = RouteLookup<Inet6Definition>("test_1.inet6.0",
"2001:db8:85a3::8a2e:370:7335/128");
ASSERT_TRUE(rt != NULL);
ASSERT_TRUE(rt->count() == 2);
VERIFY_EQ(peers_[0], rt->BestPath()->GetPeer());
ASSERT_TRUE(rt->BestPath()->IsFeasible() == true);
ASSERT_TRUE(rt->BestPath()->IsReplicated() == true);
attr = rt->BestPath()->GetAttr();
orig_attr = rt->BestPath()->GetOriginalAttr();
original_local_pref = orig_attr->local_pref();
policy_local_pref = attr->local_pref();
ASSERT_TRUE(policy_local_pref == 200);
ASSERT_TRUE(original_local_pref == 200);

DeleteRoute<Inet6Definition>(peers_[0], "test_0.inet6.0",
"2001:db8:85a3::8a2e:370:7335/128");
DeleteRoute<Inet6Definition>(peers_[1], "test_1.inet6.0",
"2001:db8:85a3::8a2e:370:7335/128");
task_util::WaitForIdle();
ifmap_test_util::IFMapMsgUnlink(&config_db_, "routing-instance", "test_0",
"routing-instance", "test_1", "connection");
}



static void ValidateShowRoutingPolicyResponse(
Sandesh *sandesh, bool *done,
const vector<ShowRoutingPolicyInfo> &policy_list) {
Expand Down
16 changes: 16 additions & 0 deletions src/bgp/testdata/routing_policy_0k.xml
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
<?xml version="1.0" encoding="utf-8"?>
<config>
<routing-policy name='basic_2'>
<term>
<term-match-condition>
<community>23:13</community>
</term-match-condition>
<term-action-list>
<update>
<local-pref>75</local-pref>
</update>
<action>accept</action>
</term-action-list>
</term>
</routing-policy>
</config>
21 changes: 21 additions & 0 deletions src/bgp/testdata/routing_policy_0l.xml
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
<?xml version="1.0" encoding="utf-8"?>
<config>
<routing-policy name='basic_3'>
<term>
<term-match-condition>
<community>23:13</community>
</term-match-condition>
<term-action-list>
<update>
<local-pref>99</local-pref>
</update>
<action>accept</action>
</term-action-list>
</term>
</routing-policy>
<routing-instance name="test_1">
<routing-policy to="basic_3">
<sequence>1.0</sequence>
</routing-policy>
</routing-instance>
</config>

0 comments on commit dce1698

Please sign in to comment.