From 674b7ed3f366cd1aa0e2d9e23091b327af9f5fcb Mon Sep 17 00:00:00 2001 From: Manish Date: Wed, 1 Jul 2015 15:27:58 +0530 Subject: [PATCH] Dangling EVPN allocated MPLS label. Problem: Multicast allocates mpls label for use in evpn. This label is deleted once all paths in the flood route goes off. Label is stored in multicast object created for flood route. In problematic scenario multicast object was deleted either via all VM going off(in regular agent) or via VN in TSN. Because of this unsubscribe is sent to control-node which in turns issues withdraw of route with peer as CN. However when this request is received at multicast handler, it tries to search for multicast object and if it is not found then request is ignored. Hence path keeps pending in route and label is also active. Now multicast object gets added again because of VM coming in or VN subscription is received in TSN. This will result in new evpn label allocation. Again this results in export of route to CN with new label. CN sends EVPN olist with new label. On receiving it multicast tries to search for object and finds the new one. It picks label(new) and sends it for path addition in route. It results in update of old path which was not deleted for peer CN. This updation results in label update in evpn path as well and the last context of old mpls label is lost. This results in dangling MPLS DB entry. Solution: If multicast object is null and delete request is received, use label as 0 as it does not matter what is passed. Change-Id: I75f5a3e12d312500259532fe95c543fa567eaeed Closes-bug: 1452362 --- src/vnsw/agent/oper/multicast.cc | 18 ++-- src/vnsw/agent/test/test_l2route.cc | 153 ++++++++++++++++++++++++++++ 2 files changed, 161 insertions(+), 10 deletions(-) diff --git a/src/vnsw/agent/oper/multicast.cc b/src/vnsw/agent/oper/multicast.cc index df0e5aae860..cf9e5d27e8f 100644 --- a/src/vnsw/agent/oper/multicast.cc +++ b/src/vnsw/agent/oper/multicast.cc @@ -661,18 +661,17 @@ void MulticastHandler::ModifyEvpnMembers(const Peer *peer, MCTRACE(Log, "XMPP call(EVPN) multicast handler ", vrf_name, grp.to_string(), 0); - if (obj == NULL) { - return; - } - bool delete_op = false; if (peer_identifier == ControllerPeerPath::kInvalidPeerIdentifier) { delete_op = true; + } else if (obj == NULL) { + return; } TriggerRemoteRouteChange(obj, peer, vrf_name, olist, peer_identifier, delete_op, Composite::EVPN, - obj->evpn_mpls_label(), false, ethernet_tag); + (obj ? obj->evpn_mpls_label() : 0), false, + ethernet_tag); MCTRACE(Log, "Add EVPN TOR Olist ", vrf_name, grp.to_string(), 0); } @@ -688,18 +687,17 @@ void MulticastHandler::ModifyTorMembers(const Peer *peer, MulticastGroupObject *obj = FindActiveGroupObject(vrf_name, grp); MCTRACE(Log, "TOR multicast handler ", vrf_name, grp.to_string(), 0); - if (obj == NULL) { - return; - } - bool delete_op = false; if (peer_identifier == ControllerPeerPath::kInvalidPeerIdentifier) { delete_op = true; + } else if (obj == NULL) { + return; } TriggerRemoteRouteChange(obj, peer, vrf_name, olist, peer_identifier, delete_op, Composite::TOR, - obj->evpn_mpls_label(), false, ethernet_tag); + (obj ? obj->evpn_mpls_label() : 0), false, + ethernet_tag); MCTRACE(Log, "Add external TOR Olist ", vrf_name, grp.to_string(), 0); } diff --git a/src/vnsw/agent/test/test_l2route.cc b/src/vnsw/agent/test/test_l2route.cc index 11810a6a2b1..881edf29f3b 100644 --- a/src/vnsw/agent/test/test_l2route.cc +++ b/src/vnsw/agent/test/test_l2route.cc @@ -737,6 +737,159 @@ TEST_F(RouteTest, add_deleted_peer_to_multicast_route) { bgp_peer.reset(); } +//Add local VM - flood route gets added +//Send explicitly EVPN path addition +//Delete local VM - removes local path +//Send explicit delete for evpn +//Result - route should get deleted +TEST_F(RouteTest, all_evpn_routes_deleted_when_local_vms_are_gone) { + struct PortInfo input[] = { + {"vnet1", 1, "1.1.1.10", "00:00:00:01:01:01", 1, 1}, + }; + + client->Reset(); + //Add local VM + CreateVmportEnv(input, 1); + client->WaitForIdle(); + + BridgeRouteEntry *rt = L2RouteGet("vrf1", + MacAddress::FromString("ff:ff:ff:ff:ff:ff"), + Ip4Address(0)); + EXPECT_TRUE(rt != NULL); + //Add a peer and enqueue path add in multicast route. + BgpPeer *bgp_peer_ptr = CreateBgpPeer(Ip4Address(1), "BGP Peer1"); + boost::shared_ptr bgp_peer = + bgp_peer_ptr->GetBgpXmppPeer()->bgp_peer_id_ref(); + MulticastHandler *mc_handler = static_cast(agent_-> + oper_db()->multicast()); + TunnelOlist olist; + olist.push_back(OlistTunnelEntry(nil_uuid(), 10, + IpAddress::from_string("8.8.8.8").to_v4(), + TunnelType::VxlanType())); + //Send explicit evpn olist + mc_handler->ModifyEvpnMembers(bgp_peer.get(), + "vrf1", + olist, + 0, + 1); + client->WaitForIdle(); + rt = L2RouteGet("vrf1", + MacAddress::FromString("ff:ff:ff:ff:ff:ff"), + Ip4Address(0)); + EXPECT_TRUE(rt != NULL); + + //Delete VM, local vm path goes off + DeleteVmportEnv(input, 1, false); + client->WaitForIdle(); + rt = L2RouteGet("vrf1", + MacAddress::FromString("ff:ff:ff:ff:ff:ff"), + Ip4Address(0)); + WAIT_FOR(1000, 1000, (rt->FindPath(agent_->local_vm_peer()) == NULL)); + EXPECT_TRUE(rt != NULL); + + //Send explicit delete of evpn olist + TunnelOlist del_olist; + mc_handler->ModifyEvpnMembers(bgp_peer.get(), + "vrf1", + del_olist, + 0, + ControllerPeerPath::kInvalidPeerIdentifier); + client->WaitForIdle(); + rt = L2RouteGet("vrf1", + MacAddress::FromString("ff:ff:ff:ff:ff:ff"), + Ip4Address(0)); + EXPECT_TRUE(rt == NULL); + + DeleteVmportEnv(input, 1, true); + client->WaitForIdle(); + DeleteBgpPeer(bgp_peer.get()); + client->WaitForIdle(); + bgp_peer.reset(); +} + +TEST_F(RouteTest, evpn_mcast_label_deleted) { + struct PortInfo input[] = { + {"vnet1", 1, "1.1.1.10", "00:00:00:01:01:01", 1, 1}, + }; + + client->Reset(); + //Add VM + CreateVmportEnv(input, 1); + client->WaitForIdle(); + + BridgeRouteEntry *rt = L2RouteGet("vrf1", + MacAddress::FromString("ff:ff:ff:ff:ff:ff"), + Ip4Address(0)); + EXPECT_TRUE(rt != NULL); + uint32_t evpn_mpls_label = rt->GetActivePath()->label(); + //Add a peer and enqueue path add in multicast route. + BgpPeer *bgp_peer_ptr = CreateBgpPeer(Ip4Address(1), "BGP Peer1"); + boost::shared_ptr bgp_peer = + bgp_peer_ptr->GetBgpXmppPeer()->bgp_peer_id_ref(); + MulticastHandler *mc_handler = static_cast(agent_-> + oper_db()->multicast()); + TunnelOlist olist; + olist.push_back(OlistTunnelEntry(nil_uuid(), 10, + IpAddress::from_string("8.8.8.8").to_v4(), + TunnelType::VxlanType())); + //Add EVPN olist + mc_handler->ModifyEvpnMembers(bgp_peer.get(), + "vrf1", + olist, + 0, + 1); + client->WaitForIdle(); + rt = L2RouteGet("vrf1", + MacAddress::FromString("ff:ff:ff:ff:ff:ff"), + Ip4Address(0)); + EXPECT_TRUE(rt != NULL); + //Label is retained + EXPECT_TRUE(rt->GetActivePath()->label() == evpn_mpls_label); + + //Delete VM + DeleteVmportEnv(input, 1, false); + client->WaitForIdle(); + rt = L2RouteGet("vrf1", + MacAddress::FromString("ff:ff:ff:ff:ff:ff"), + Ip4Address(0)); + WAIT_FOR(1000, 1000, (rt->FindPath(agent_->local_vm_peer()) == NULL)); + EXPECT_TRUE(rt != NULL); + //Label is retained as evpn still present + EXPECT_TRUE(rt->GetActivePath()->label() == evpn_mpls_label); + + //Delete EVPN + TunnelOlist del_olist; + mc_handler->ModifyEvpnMembers(bgp_peer.get(), + "vrf1", + del_olist, + 0, + ControllerPeerPath::kInvalidPeerIdentifier); + client->WaitForIdle(); + rt = L2RouteGet("vrf1", + MacAddress::FromString("ff:ff:ff:ff:ff:ff"), + Ip4Address(0)); + //Label should have been released + EXPECT_TRUE(agent_->mpls_table()->FindActiveEntry(new MplsLabelKey(evpn_mpls_label)) == NULL); + //route should get deleted + EXPECT_TRUE(rt == NULL); + + CreateVmportEnv(input, 1); + client->WaitForIdle(); + rt = L2RouteGet("vrf1", + MacAddress::FromString("ff:ff:ff:ff:ff:ff"), + Ip4Address(0)); + EXPECT_TRUE(rt != NULL); + WAIT_FOR(1000, 1000, (rt->FindPath(agent_->local_vm_peer()) != NULL)); + //New label taken + EXPECT_TRUE(rt->GetActivePath()->label() != evpn_mpls_label); + + DeleteVmportEnv(input, 1, true); + client->WaitForIdle(); + DeleteBgpPeer(bgp_peer.get()); + client->WaitForIdle(); + bgp_peer.reset(); +} + int main(int argc, char *argv[]) { ::testing::InitGoogleTest(&argc, argv); GETUSERARGS();