From c8a8805e9236789c8341f7f20f1db5c8eb954cf5 Mon Sep 17 00:00:00 2001 From: Prakash Bailkeri Date: Wed, 24 Feb 2016 00:19:46 +0530 Subject: [PATCH] Notify replicated routes 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 --- src/bgp/inet6vpn/inet6vpn_table.cc | 6 +- src/bgp/l3vpn/inetvpn_table.cc | 5 +- src/bgp/test/bgp_xmpp_inetvpn_test.cc | 147 ++++++++++++++++++++++++++ 3 files changed, 151 insertions(+), 7 deletions(-) diff --git a/src/bgp/inet6vpn/inet6vpn_table.cc b/src/bgp/inet6vpn/inet6vpn_table.cc index a69a9357ceb..ad5c0ab35fe 100644 --- a/src/bgp/inet6vpn/inet6vpn_table.cc +++ b/src/bgp/inet6vpn/inet6vpn_table.cc @@ -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; } diff --git a/src/bgp/l3vpn/inetvpn_table.cc b/src/bgp/l3vpn/inetvpn_table.cc index fff9504bec1..ac07c11c637 100644 --- a/src/bgp/l3vpn/inetvpn_table.cc +++ b/src/bgp/l3vpn/inetvpn_table.cc @@ -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; } diff --git a/src/bgp/test/bgp_xmpp_inetvpn_test.cc b/src/bgp/test/bgp_xmpp_inetvpn_test.cc index 196cd122d9d..41f66ffbb2c 100644 --- a/src/bgp/test/bgp_xmpp_inetvpn_test.cc +++ b/src/bgp/test/bgp_xmpp_inetvpn_test.cc @@ -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 = "\ @@ -267,6 +271,66 @@ static const char *config_2_control_nodes_route_aggregate = "\ "; +static const char *config_2_control_nodes_routing_policy = "\ +\ + \ + 192.168.0.1\ +
127.0.0.1
\ + %d\ + \ + \ + route-target\ + inet-vpn\ + \ + \ + \ + \ + 192.168.0.2\ +
127.0.0.2
\ + %d\ + \ + \ + route-target\ + inet-vpn\ + \ + \ + \ + \ + 1\ + \ + \ + \ + \ + 11:13\ + \ + \ + \ + 9999\ + \ + \ + \ + \ + \ + \ + \ + 22:13\ + \ + \ + \ + 8888\ + \ + \ + \ + \ + \ + blue\ + \ + 1.0\ + \ + target:1:1\ + \ +\ +"; // // Control Nodes X and Y. @@ -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 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 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 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 {