diff --git a/src/bgp/test/bgp_xmpp_inetvpn_test.cc b/src/bgp/test/bgp_xmpp_inetvpn_test.cc index 37ac3b33d21..968c1754496 100644 --- a/src/bgp/test/bgp_xmpp_inetvpn_test.cc +++ b/src/bgp/test/bgp_xmpp_inetvpn_test.cc @@ -531,7 +531,7 @@ class BgpXmppInetvpn2ControlNodeTest : public ::testing::Test { } bool CheckRoute(test::NetworkAgentMockPtr agent, string net, - string prefix, string nexthop, int local_pref, int med, + string prefix, string nexthop, int local_pref, int med, int seq, const string &encap, const string &origin_vn, const vector sgids, const LoadBalance &loadBalance, const vector communities) { const autogen::ItemType *rt = agent->RouteLookup(net, prefix); @@ -543,6 +543,8 @@ class BgpXmppInetvpn2ControlNodeTest : public ::testing::Test { return false; if (med && rt->entry.med != med) return false; + if (seq && rt->entry.sequence_number != seq) + return false; if (!origin_vn.empty() && rt->entry.virtual_network != origin_vn) return false; if (!sgids.empty() && @@ -565,8 +567,8 @@ class BgpXmppInetvpn2ControlNodeTest : public ::testing::Test { void VerifyRouteExists(test::NetworkAgentMockPtr agent, string net, string prefix, string nexthop, int local_pref, int med) { TASK_UTIL_EXPECT_TRUE(CheckRoute(agent, net, prefix, nexthop, - local_pref, med, string(), string(), vector(), LoadBalance(), - vector())); + local_pref, med, 0, string(), string(), vector(), + LoadBalance(), vector())); } void VerifyRouteExists(test::NetworkAgentMockPtr agent, string net, @@ -575,30 +577,38 @@ class BgpXmppInetvpn2ControlNodeTest : public ::testing::Test { const vector sgids = vector(), const LoadBalance lb = LoadBalance()) { TASK_UTIL_EXPECT_TRUE(CheckRoute( - agent, net, prefix, nexthop, local_pref, 0, encap, origin_vn, + agent, net, prefix, nexthop, local_pref, 0, 0, encap, origin_vn, sgids, lb, vector())); } void VerifyRouteExists(test::NetworkAgentMockPtr agent, string net, string prefix, string nexthop, const vector sgids) { TASK_UTIL_EXPECT_TRUE(CheckRoute( - agent, net, prefix, nexthop, 0, 0, string(), string(), sgids, + agent, net, prefix, nexthop, 0, 0, 0, string(), string(), sgids, LoadBalance(), vector())); } + void VerifyRouteExists(test::NetworkAgentMockPtr agent, string net, + string prefix, string nexthop, int local_pref, int seq, + const vector sgids) { + TASK_UTIL_EXPECT_TRUE(CheckRoute( + agent, net, prefix, nexthop, local_pref, 0, seq, string(), string(), + sgids, LoadBalance(), vector())); + } + void VerifyRouteExists(test::NetworkAgentMockPtr agent, string net, string prefix, string nexthop, const LoadBalance &lb) { TASK_UTIL_EXPECT_TRUE(CheckRoute( - agent, net, prefix, nexthop, 0, 0, string(), string(), vector(), - lb, vector())); + agent, net, prefix, nexthop, 0, 0, 0, string(), string(), + vector(), lb, vector())); } void VerifyRouteExists(test::NetworkAgentMockPtr agent, string net, string prefix, string nexthop, const vector &communities) { TASK_UTIL_EXPECT_TRUE(CheckRoute( - agent, net, prefix, nexthop, 0, 0, string(), string(), vector(), - LoadBalance(), communities)); + agent, net, prefix, nexthop, 0, 0, 0, string(), string(), + vector(), LoadBalance(), communities)); } void VerifyRouteNoExists(test::NetworkAgentMockPtr agent, string net, @@ -1537,6 +1547,7 @@ TEST_F(BgpXmppInetvpn2ControlNodeTest, MultipleRouteAddDelete3) { // Large xmpp update messages even though attributes for each route are // different. // Small bgp update messages since attributes for each route are different. +// Each route has a different local pref and med. // TEST_F(BgpXmppInetvpn2ControlNodeTest, MultipleRouteAddDelete4) { Configure(); @@ -1878,6 +1889,224 @@ TEST_F(BgpXmppInetvpn2ControlNodeTest, MultipleRouteAddDelete5) { agent_b_->SessionDown(); } +// +// Multiple routes are exchanged correctly. +// Large xmpp update messages even though attributes for each route are +// different. +// Small bgp update messages since attributes for each route are different. +// Each route has a different local pref and sequence number. +// Odd and even routes have different security groups. +// +TEST_F(BgpXmppInetvpn2ControlNodeTest, MultipleRouteAddDelete6) { + Configure(); + 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()); + + // Verify table walk count for blue.inet.0. + BgpTable *blue_x = VerifyTableExists(bs_x_, "blue.inet.0"); + BgpTable *blue_y = VerifyTableExists(bs_y_, "blue.inet.0"); + TASK_UTIL_EXPECT_EQ(0, blue_x->walk_complete_count()); + TASK_UTIL_EXPECT_EQ(0, blue_y->walk_complete_count()); + task_util::WaitForIdle(); + + // Register to blue instance + agent_a_->Subscribe("blue", 1); + agent_b_->Subscribe("blue", 1); + task_util::WaitForIdle(); + + // Verify that subscribe completed. + TASK_UTIL_EXPECT_EQ(1, blue_x->walk_complete_count()); + TASK_UTIL_EXPECT_EQ(1, blue_y->walk_complete_count()); + task_util::WaitForIdle(); + + // Pause update generation on X. + bs_x_->scheduling_group_manager()->DisableGroups(); + + // Add routes from agent A. + test::NextHops next_hops; + test::NextHop next_hop("192.168.1.1"); + next_hops.push_back(next_hop); + vector sgids1 = list_of(8000001)(8000003); + vector sgids2 = list_of(8000002)(8000004); + for (int idx = 0; idx < kRouteCount; ++idx) { + int lpref = idx + 1; + int seq = idx + 1; + if (idx % 2 == 0) { + test::RouteAttributes attributes(lpref, seq, sgids2); + agent_a_->AddRoute("blue", BuildPrefix(idx), next_hops, attributes); + } else { + test::RouteAttributes attributes(lpref, seq, sgids1); + agent_a_->AddRoute("blue", BuildPrefix(idx), next_hops, attributes); + } + } + task_util::WaitForIdle(); + + // Verify that blue.inet.0 and bgp.l3vpn.0 output queues have built up on X. + VerifyOutputQueueDepth(bs_x_, 2 * kRouteCount); + task_util::WaitForIdle(); + + // Pause update generation on Y. + bs_y_->scheduling_group_manager()->DisableGroups(); + + // Resume update generation on X. + bs_x_->scheduling_group_manager()->EnableGroups(); + task_util::WaitForIdle(); + + // Verify that blue.inet.0 and bgp.l3vpn.0 output queues are drained on X. + VerifyOutputQueueDepth(bs_x_, 0); + task_util::WaitForIdle(); + + // Verify that blue.inet.0 output queues have built up on Y. + VerifyOutputQueueDepth(bs_y_, kRouteCount); + task_util::WaitForIdle(); + + // Resume update generation on Y. + bs_y_->scheduling_group_manager()->EnableGroups(); + task_util::WaitForIdle(); + + // Verify that blue.inet.0 output queues are drained on Y. + VerifyOutputQueueDepth(bs_y_, 0); + task_util::WaitForIdle(); + + // Verify that routes showed up on agents A and B. + for (int idx = 0; idx < kRouteCount; ++idx) { + int lpref = idx + 1; + int seq = idx + 1; + if (idx % 2 == 0) { + VerifyRouteExists(agent_a_, "blue", BuildPrefix(idx), "192.168.1.1", + lpref, seq, sgids2); + VerifyRouteExists(agent_b_, "blue", BuildPrefix(idx), "192.168.1.1", + lpref, seq, sgids2); + } else { + VerifyRouteExists(agent_a_, "blue", BuildPrefix(idx), "192.168.1.1", + lpref, seq, sgids1); + VerifyRouteExists(agent_b_, "blue", BuildPrefix(idx), "192.168.1.1", + lpref, seq, sgids1); + } + } + + // Pause update generation on server X. + bs_x_->scheduling_group_manager()->DisableGroups(); + + // Delete routes from agent A. + for (int idx = 0; idx < kRouteCount; ++idx) { + agent_a_->DeleteRoute("blue", BuildPrefix(idx)); + } + task_util::WaitForIdle(); + + // Verify that blue.inet.0 and bgp.l3vpn.0 output queues have built up on X. + VerifyOutputQueueDepth(bs_x_, 2 * kRouteCount); + task_util::WaitForIdle(); + + // Pause update generation on Y. + bs_y_->scheduling_group_manager()->DisableGroups(); + + // Resume update generation on server X. + bs_x_->scheduling_group_manager()->EnableGroups(); + task_util::WaitForIdle(); + + // Verify that blue.inet.0 and bgp.l3vpn.0 output queues are drained on X. + VerifyOutputQueueDepth(bs_x_, 0); + + // Verify that blue.inet.0 output queues have built up on Y. + VerifyOutputQueueDepth(bs_y_, kRouteCount); + task_util::WaitForIdle(); + + // Resume update generation on Y. + bs_y_->scheduling_group_manager()->EnableGroups(); + task_util::WaitForIdle(); + + // Verify that blue.inet.0 output queues are drained on Y. + VerifyOutputQueueDepth(bs_y_, 0); + + // Verify that routes are deleted at agents A and B. + for (int idx = 0; idx < kRouteCount; ++idx) { + VerifyRouteNoExists(agent_a_, "blue", BuildPrefix(idx)); + VerifyRouteNoExists(agent_b_, "blue", BuildPrefix(idx)); + } + + // Unregister to blue instance + agent_a_->Unsubscribe("blue"); + agent_b_->Unsubscribe("blue"); + + // Verify bgp update counters. + const BgpPeer *peer_xy = VerifyPeerExists(bs_x_, bs_y_); + const BgpPeer *peer_yx = VerifyPeerExists(bs_y_, bs_x_); + TASK_UTIL_EXPECT_EQ( + peer_xy->get_rx_route_reach(), peer_yx->get_tx_route_reach()); + TASK_UTIL_EXPECT_EQ( + peer_xy->get_tx_route_reach(), peer_yx->get_rx_route_reach()); + TASK_UTIL_EXPECT_EQ( + peer_xy->get_rx_route_unreach(), peer_yx->get_tx_route_unreach()); + TASK_UTIL_EXPECT_EQ( + peer_xy->get_tx_route_unreach(), peer_yx->get_rx_route_unreach()); + TASK_UTIL_EXPECT_EQ( + peer_xy->get_rx_end_of_rib(), peer_yx->get_tx_end_of_rib()); + TASK_UTIL_EXPECT_EQ( + peer_xy->get_tx_end_of_rib(), peer_yx->get_rx_end_of_rib()); + TASK_UTIL_EXPECT_EQ( + peer_xy->get_rx_route_total(), peer_yx->get_tx_route_total()); + TASK_UTIL_EXPECT_EQ( + peer_xy->get_tx_route_total(), peer_yx->get_rx_route_total()); + + // Verify bgp reach/unreach, end-of-rib and total counts. + // 1 route-target and kRouteCount inet-vpn routes. + // 2 end-of-ribs - one for route-target and one for inet-vpn. + TASK_UTIL_EXPECT_EQ(1 + kRouteCount, peer_xy->get_tx_route_reach()); + TASK_UTIL_EXPECT_EQ(1 + kRouteCount, peer_xy->get_tx_route_unreach()); + TASK_UTIL_EXPECT_EQ(2, peer_xy->get_tx_end_of_rib()); + TASK_UTIL_EXPECT_EQ( + 2 * (1 + kRouteCount) + 2, peer_xy->get_tx_route_total()); + + // Verify bgp update message counters. + // X->Y : 2 route-target (1 advertise, 1 withdraw) + + // 515 inet-vpn (512 advertise, 3 withdraw) + + // 2 end-of-rib (1 route-target, 1 inet-vpn) + // Y->X : 2 route-target (1 advertise, 1 withdraw) + + // 2 end-of-rib (1 route-target, 1 inet-vpn) + TASK_UTIL_EXPECT_EQ(peer_xy->get_rx_update(), peer_yx->get_tx_update()); + TASK_UTIL_EXPECT_EQ(peer_xy->get_tx_update(), peer_yx->get_rx_update()); + TASK_UTIL_EXPECT_EQ(4 + 3 + kRouteCount, peer_xy->get_tx_update()); + TASK_UTIL_EXPECT_EQ(4, peer_yx->get_tx_update()); + + // Verify xmpp update counters. + const BgpXmppChannel *xc_a = + VerifyXmppChannelExists(cm_x_.get(), "agent-a"); + const BgpXmppChannel *xc_b = + VerifyXmppChannelExists(cm_y_.get(), "agent-b"); + TASK_UTIL_EXPECT_EQ(0 + kRouteCount, xc_a->get_rx_route_reach()); + TASK_UTIL_EXPECT_EQ(0 + kRouteCount, xc_a->get_tx_route_reach()); + TASK_UTIL_EXPECT_EQ(0 + kRouteCount, xc_a->get_rx_route_unreach()); + TASK_UTIL_EXPECT_EQ(0 + kRouteCount, xc_a->get_tx_route_unreach()); + TASK_UTIL_EXPECT_EQ(0, xc_b->get_rx_route_reach()); + TASK_UTIL_EXPECT_EQ(0 + kRouteCount, xc_b->get_tx_route_reach()); + TASK_UTIL_EXPECT_EQ(0, xc_b->get_rx_route_unreach()); + TASK_UTIL_EXPECT_EQ(0 + kRouteCount, xc_b->get_tx_route_unreach()); + + // Verify xmpp update message counters. + // agent-a: 16 (kRouteCount/BgpXmppMessage::kMaxReachCount) advertise + + // 2 (kRouteCount/BgpXmppMessage::kMaxUnreachCount) withdraw + // agent-b: 16 (kRouteCount/BgpXmppMessage::kMaxReachCount) advertise + + // 2 (kRouteCount/BgpXmppMessage::kMaxUnreachCount) withdraw + TASK_UTIL_EXPECT_EQ(18, xc_a->get_tx_update()); + TASK_UTIL_EXPECT_EQ(18, xc_b->get_tx_update()); + + // Close the sessions. + agent_a_->SessionDown(); + agent_b_->SessionDown(); +} + // // Generate big update message. // Each route has a very large number of attributes such that only a single diff --git a/src/bgp/xmpp_message_builder.cc b/src/bgp/xmpp_message_builder.cc index 2ab1a53429e..e08aa78e1ef 100644 --- a/src/bgp/xmpp_message_builder.cc +++ b/src/bgp/xmpp_message_builder.cc @@ -87,6 +87,7 @@ class BgpXmppMessage : public Message { bool AddMcastRoute(const BgpRoute *route, const RibOutAttr *roattr); void ProcessCommunity(const Community *community) { + community_list_.clear(); if (community == NULL) return; BOOST_FOREACH(uint32_t value, community->communities()) { @@ -95,6 +96,9 @@ class BgpXmppMessage : public Message { } void ProcessExtCommunity(const ExtCommunity *ext_community) { + sequence_number_ = 0; + security_group_list_.clear(); + load_balance_attribute_ = LoadBalance::LoadBalanceAttribute(); if (ext_community == NULL) return; @@ -176,6 +180,12 @@ bool BgpXmppMessage::AddRoute(const BgpRoute *route, const RibOutAttr *roattr) { if (!is_reachable_ && num_unreach_route_ >= kMaxUnreachCount) return false; + if (is_reachable_) { + const BgpAttr *attr = roattr->attr(); + ProcessCommunity(attr->community()); + ProcessExtCommunity(attr->ext_community()); + } + if (table_->family() == Address::ERMVPN) { return AddMcastRoute(route, roattr); } else if (table_->family() == Address::EVPN) { diff --git a/src/control-node/test/network_agent_mock.h b/src/control-node/test/network_agent_mock.h index 80be3129470..da5b458d01e 100644 --- a/src/control-node/test/network_agent_mock.h +++ b/src/control-node/test/network_agent_mock.h @@ -106,7 +106,7 @@ struct RouteAttributes { communities(community) { } RouteAttributes(const LoadBalance::LoadBalanceAttribute &lba) - : local_pref(kDefaultLocalPref), sequence(kDefaultSequence), + : local_pref(kDefaultLocalPref), med(0), sequence(kDefaultSequence), loadBalanceAttribute(lba) { } RouteAttributes(const RouteParams ¶ms)