Skip to content

Commit

Permalink
Notify replicated routes
Browse files Browse the repository at this point in the history 
Scenario:

Consider a route with community c1 and c2 and routing instance with policy P1
Policy P1: { from community c1; then local-pref 9999 }
Due to the routing policy the route has local-pref set 9999. The route in
bgp.l3vpn.0 table has two paths (one replicated and other Bgp path) with
local-pref set to 9999.

Now attach routing instance to policy P2
Policy P2: { from community c2; then local-pref 8888 }

Since the route has community c1 and c2, it matches both policy and the end
result is path has local-pref set to 8888.

When the new route is replicated to bgp.l3vpn.0 table, new replicated route
will have lower local-pref(8888) compared to bgp-path from other control-node(9999).
So the replicated path will not be selected and is not inserted on top of the
path list. In this case if the replicated route in bgp.l3vpn.0 table is not
notified, then export code will not send the updated path with new local-pref

Fix:
Notify replicated even when inserted secondary path is not BestPath on the route.

Added test to verify that after the policy update on routing instance, the route
has local-pref as per the policy action update(i.e. in the above example 8888)

Change-Id: I762e001ba97d4739d12daf580109f2b042878ddf
Closes-bug: #1548115
  • Loading branch information
@bailkeri
bailkeri committed Feb 23, 2016
1 parent 927e287 commit c8a8805
Show file tree
Hide file tree
Showing 3 changed files with 151 additions and 7 deletions.
6 changes: 2 additions & 4 deletions src/bgp/inet6vpn/inet6vpn_table.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -117,10 +117,8 @@ BgpRoute *Inet6VpnTable::RouteReplicate(BgpServer *server, BgpTable *src_table,
replicated_path->SetReplicateInfo(src_table, source_rt);
dest_route->InsertPath(replicated_path);

// Trigger notification only if the inserted path is selected
if (replicated_path == dest_route->front()) {
partition->Notify(dest_route);
}
// Always trigger notification.
partition->Notify(dest_route);

return dest_route;
}
Expand Down
5 changes: 2 additions & 3 deletions src/bgp/l3vpn/inetvpn_table.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -121,9 +121,8 @@ BgpRoute *InetVpnTable::RouteReplicate(BgpServer *server,
replicated_path->SetReplicateInfo(src_table, src_rt);
dest_route->InsertPath(replicated_path);

// Trigger notification only if the inserted path is selected
if (replicated_path == dest_route->front())
rtp->Notify(dest_route);
// Always trigger notification.
rtp->Notify(dest_route);

return dest_route;
}
Expand Down
147 changes: 147 additions & 0 deletions src/bgp/test/bgp_xmpp_inetvpn_test.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -18,9 +18,13 @@
#include "control-node/test/network_agent_mock.h"
#include "io/test/event_manager_test.h"
#include "ifmap/test/ifmap_test_util.h"

#include "schema/bgp_schema_types.h"

#include "xmpp/xmpp_factory.h"

using namespace std;
using std::auto_ptr;
using boost::assign::list_of;

static const char *config_1_control_node_2_vns = "\
Expand Down Expand Up @@ -267,6 +271,66 @@ static const char *config_2_control_nodes_route_aggregate = "\
";


static const char *config_2_control_nodes_routing_policy = "\
<config>\
<bgp-router name=\'X\'>\
<identifier>192.168.0.1</identifier>\
<address>127.0.0.1</address>\
<port>%d</port>\
<session to=\'Y\'>\
<address-families>\
<family>route-target</family>\
<family>inet-vpn</family>\
</address-families>\
</session>\
</bgp-router>\
<bgp-router name=\'Y\'>\
<identifier>192.168.0.2</identifier>\
<address>127.0.0.2</address>\
<port>%d</port>\
<session to=\'X\'>\
<address-families>\
<family>route-target</family>\
<family>inet-vpn</family>\
</address-families>\
</session>\
</bgp-router>\
<virtual-network name='blue'>\
<network-id>1</network-id>\
</virtual-network>\
<routing-policy name='p1'>\
<term>\
<term-match-condition>\
<community>11:13</community>\
</term-match-condition>\
<term-action-list>\
<update>\
<local-pref>9999</local-pref>\
</update>\
</term-action-list>\
</term>\
</routing-policy>\
<routing-policy name='p2'>\
<term>\
<term-match-condition>\
<community>22:13</community>\
</term-match-condition>\
<term-action-list>\
<update>\
<local-pref>8888</local-pref>\
</update>\
</term-action-list>\
</term>\
</routing-policy>\
<routing-instance name='blue'>\
<virtual-network>blue</virtual-network>\
<routing-policy to='p1'>\
<sequence>1.0</sequence>\
</routing-policy>\
<vrf-target>target:1:1</vrf-target>\
</routing-instance>\
</config>\
";

//
// Control Nodes X and Y.
Expand Down Expand Up @@ -2465,6 +2529,89 @@ TEST_F(BgpXmppInetvpn2ControlNodeTest, RouteAggregation_NoExportContributing_2)
agent_b_->SessionDown();
}

//
// Verify local-pref update by routing policy
// Consider a route with community c1 and c2 and routing instance with policy p1
// Policy P1: { from community c1; then local-pref 9999 }
// Due to the routing policy the route has local-pref set 9999. The route in
// bgp.l3vpn.0 table has two paths (one replicated and other Bgp path) with
// local-pref set to 9999.
// Now attach routing instance to policy P2
// Policy P2: { from community c2; then local-pref 8888 }
// Since the route has community c1 and c2, it matches both policy and end
// result is path has local-pref set to 8888.
// When the new route is replicated to bgp-.l3vpn.0 table, new replicated route
// will have lower local-pref(8888) compared to bgp-path from other
// control-node(9999).
// Test is to verify that after the policy update on routing instance, the route
// has local-pref as per the policy action update(i.e. in this example 8888)
//
TEST_F(BgpXmppInetvpn2ControlNodeTest, RoutingPolicy_UpdateLocalPref) {
// Configure bgp server X.
Configure(config_2_control_nodes_routing_policy);
task_util::WaitForIdle();

// Create XMPP Agent A connected to XMPP server X.
agent_a_.reset(
new test::NetworkAgentMock(&evm_, "agent-a", xs_x_->GetPort(),
"127.0.0.1", "127.0.0.1"));
TASK_UTIL_EXPECT_TRUE(agent_a_->IsEstablished());

// Create XMPP Agent B connected to XMPP server Y.
agent_b_.reset(
new test::NetworkAgentMock(&evm_, "agent-b", xs_y_->GetPort(),
"127.0.0.2", "127.0.0.2"));
TASK_UTIL_EXPECT_TRUE(agent_b_->IsEstablished());

// Register agent A to blue instance.
// Register agent B to blue instance.
agent_a_->Subscribe("blue", 1);
agent_b_->Subscribe("blue", 1);

vector<std::string> community_a;
stringstream route_a;
route_a << "10.1.1.1/32";
community_a.push_back("11:13");
community_a.push_back("22:13");
test::RouteAttributes attr_a(community_a);
test::NextHops nexthops_a;
nexthops_a.push_back(test::NextHop("192.168.1.1", 0));
agent_a_->AddRoute("blue", route_a.str(), nexthops_a, attr_a);
agent_b_->AddRoute("blue", route_a.str(), nexthops_a, attr_a);
task_util::WaitForIdle();

VerifyRouteExists(agent_a_, "blue", route_a.str(), "192.168.1.1", 9999);
VerifyRouteExists(agent_b_, "blue", route_a.str(), "192.168.1.1", 9999);


auto_ptr<autogen::RoutingPolicyType> p_a(new autogen::RoutingPolicyType());
p_a->sequence = "2.0";
ifmap_test_util::IFMapMsgLink(bs_x_->config_db(), "routing-instance",
"blue", "routing-policy", "p2",
"routing-policy-routing-instance", 0, p_a.release());
task_util::WaitForIdle();

auto_ptr<autogen::RoutingPolicyType> p_b(new autogen::RoutingPolicyType());
p_b->sequence = "2.0";
ifmap_test_util::IFMapMsgLink(bs_y_->config_db(), "routing-instance",
"blue", "routing-policy", "p2",
"routing-policy-routing-instance", 0, p_b.release());
task_util::WaitForIdle();


VerifyRouteExists(agent_a_, "blue", route_a.str(), "192.168.1.1", 8888);
VerifyRouteExists(agent_b_, "blue", route_a.str(), "192.168.1.1", 8888);

// Delete blue path from agent A.
agent_a_->DeleteRoute("blue", route_a.str());
agent_b_->DeleteRoute("blue", route_a.str());
task_util::WaitForIdle();

// Close the sessions.
agent_a_->SessionDown();
agent_b_->SessionDown();
}

class BgpXmppInetvpnJoinLeaveTest :
public BgpXmppInetvpn2ControlNodeTest,
public ::testing::WithParamInterface<bool> {
Expand Down

0 comments on commit c8a8805

Please sign in to comment.