From 08b91bbc31587a341c1c5e0724f65cfe33dce6c2 Mon Sep 17 00:00:00 2001 From: Manish Singh Date: Wed, 27 Jan 2016 15:37:55 +0530 Subject: [PATCH] EVPN route for floating IP was present even after VMI is gone. Problem: VMI creates evpn route for floating IP using its own peer. On change of encap(vxlan<->mpls), ethernet_tag gets changed for VMI. This change should withdraw all evpn routes using old ethernet tag and move on to use new ethernet tag. Because this was not handled in floating-ip old tag based route was not removed. This resulted in invalid path peer because VMI gets deleted after encap change and old tag based evpn route is never removed. Peer is freed on interface going off and path has invalid peer. Solution: Use ethernet tag from VMI and withdraw floating-ip evpn route when tag changes. Closes-bug: 1527427 (cherry picked from commit 276e70a715b0a3ad753943ba9249f72d6af70880) Conflicts: src/vnsw/agent/oper/vm_interface.cc src/vnsw/agent/oper/vm_interface.h Change-Id: I5cad082457ad05f32a80f5b520c7f2ecb80e9143 --- src/vnsw/agent/oper/test/test_intf.cc | 59 +++++++++++++++++++++++++++ src/vnsw/agent/oper/vm_interface.cc | 51 +++++++++++++---------- src/vnsw/agent/oper/vm_interface.h | 16 +++++--- 3 files changed, 99 insertions(+), 27 deletions(-) diff --git a/src/vnsw/agent/oper/test/test_intf.cc b/src/vnsw/agent/oper/test/test_intf.cc index 7d2df4cf107..c00d62cb84c 100644 --- a/src/vnsw/agent/oper/test/test_intf.cc +++ b/src/vnsw/agent/oper/test/test_intf.cc @@ -1212,6 +1212,65 @@ TEST_F(IntfTest, VmPortFloatingIpResync_1) { client->WaitForIdle(); } +TEST_F(IntfTest, VmPortFloatingIpEvpn_1) { + struct PortInfo input[] = { + {"vnet1", 1, "1.1.1.10", "00:00:00:01:01:01", 1, 1}, + }; + + client->Reset(); + CreateVmportEnv(input, 1); + client->WaitForIdle(); + EXPECT_TRUE(VmPortActive(input, 0)); + + AddVn("default-project:vn2", 2); + AddVrf("default-project:vn2:vn2", 2); + AddLink("virtual-network", "default-project:vn2", "routing-instance", + "default-project:vn2:vn2"); + //Add floating IP for vnet1 + AddFloatingIpPool("fip-pool1", 1); + AddFloatingIp("fip1", 1, "2.1.1.100"); + AddLink("floating-ip", "fip1", "floating-ip-pool", "fip-pool1"); + AddLink("floating-ip-pool", "fip-pool1", "virtual-network", + "default-project:vn2"); + client->WaitForIdle(); + AddLink("virtual-machine-interface", "vnet1", "floating-ip", "fip1"); + client->WaitForIdle(); + Ip4Address floating_ip = Ip4Address::from_string("2.1.1.100"); + EXPECT_TRUE(RouteFind("default-project:vn2:vn2", floating_ip, 32)); + EXPECT_TRUE(VmPortFloatingIpCount(1, 1)); + + //Change Encap + AddEncapList("VXLAN", "MPLSoUDP", "MPLSoGRE"); + client->WaitForIdle(); + + //Delete config for vnet1, forcing interface to deactivate + //verify that route and floating ip map gets cleaned up + DelNode("virtual-machine-interface", input[0].name); + client->WaitForIdle(); + EXPECT_FALSE(RouteFind("default-project:vn2:vn2", floating_ip, 32)); + // Interface not deleted till config is deleted + EXPECT_TRUE(VmPortFind(1)); + + //Clean up + DelLink("virtual-network", "default-project:vn2", "routing-instance", + "default-project:vn2:vn2"); + DelLink("floating-ip", "fip1", "floating-ip-pool", "fip-pool1"); + DelLink("floating-ip-pool", "fip-pool1", + "virtual-network", "default-project:vn2"); + DelLink("virtual-machine-interface", "vnet1", "floating-ip", "fip1"); + DelFloatingIp("fip1"); + DelFloatingIpPool("fip-pool1"); + client->WaitForIdle(); + DelLink("virtual-network", "vn2", "routing-instance", + "default-project:vn2:vn2"); + DelVrf("default-project:vn2:vn2"); + DelVn("default-project:vn2"); + DeleteVmportEnv(input, 1, true); + client->WaitForIdle(); + EXPECT_FALSE(VrfFind("vrf1")); + EXPECT_FALSE(VrfFind("default-project:vn2:vn2", true)); +} + TEST_F(IntfTest, VmPortFloatingIpDelete_1) { struct PortInfo input[] = { {"vnet1", 1, "1.1.1.10", "00:00:00:01:01:01", 1, 1}, diff --git a/src/vnsw/agent/oper/vm_interface.cc b/src/vnsw/agent/oper/vm_interface.cc index f7c9349ffc9..8b2acf5a6ee 100644 --- a/src/vnsw/agent/oper/vm_interface.cc +++ b/src/vnsw/agent/oper/vm_interface.cc @@ -1213,7 +1213,7 @@ void VmInterface::UpdateL3(bool old_ipv4_active, VrfEntry *old_vrf, } UpdateIpv4InstanceIp(force_update, policy_change, false, old_ethernet_tag); UpdateMetadataRoute(old_ipv4_active, old_vrf); - UpdateFloatingIp(force_update, policy_change, false); + UpdateFloatingIp(force_update, policy_change, false, old_ethernet_tag); UpdateServiceVlan(force_update, policy_change); UpdateAllowedAddressPair(force_update, policy_change, false, false, false); @@ -1247,7 +1247,7 @@ void VmInterface::DeleteL3(bool old_ipv4_active, VrfEntry *old_vrf, DeleteIpv6InstanceIp(true, old_ethernet_tag, old_vrf); } DeleteMetadataRoute(old_ipv4_active, old_vrf, old_need_linklocal_ip); - DeleteFloatingIp(false, 0); + DeleteFloatingIp(false, old_ethernet_tag); DeleteServiceVlan(); DeleteStaticRoute(); DeleteAllowedAddressPair(false); @@ -1300,7 +1300,7 @@ void VmInterface::UpdateL2(bool old_l2_active, VrfEntry *old_vrf, MacAddress::FromString(vm_mac_)); UpdateIpv4InstanceIp(force_update, policy_change, true, old_ethernet_tag); UpdateIpv6InstanceIp(force_update, policy_change, true, old_ethernet_tag); - UpdateFloatingIp(force_update, policy_change, true); + UpdateFloatingIp(force_update, policy_change, true, old_ethernet_tag); UpdateAllowedAddressPair(force_update, policy_change, true, old_l2_active, old_layer3_forwarding); //If the interface is Gateway we need to add a receive route, @@ -2679,14 +2679,15 @@ void VmInterface::CleanupFloatingIpList() { } void VmInterface::UpdateFloatingIp(bool force_update, bool policy_change, - bool l2) { + bool l2, uint32_t old_ethernet_tag) { FloatingIpSet::iterator it = floating_ip_list_.list_.begin(); while (it != floating_ip_list_.list_.end()) { FloatingIpSet::iterator prev = it++; if (prev->del_pending_) { - prev->DeActivate(this, l2); + prev->DeActivate(this, l2, old_ethernet_tag); } else { - prev->Activate(this, force_update||policy_change, l2); + prev->Activate(this, force_update||policy_change, l2, + old_ethernet_tag); } } } @@ -2695,7 +2696,7 @@ void VmInterface::DeleteFloatingIp(bool l2, uint32_t old_ethernet_tag) { FloatingIpSet::iterator it = floating_ip_list_.list_.begin(); while (it != floating_ip_list_.list_.end()) { FloatingIpSet::iterator prev = it++; - prev->DeActivate(this, l2); + prev->DeActivate(this, l2, old_ethernet_tag); } } @@ -3369,22 +3370,21 @@ void VmInterface::FatFlowList::Remove(FatFlowEntrySet::iterator &it) { VmInterface::FloatingIp::FloatingIp() : ListEntry(), floating_ip_(), vn_(NULL), - vrf_(NULL), vrf_name_(""), vn_uuid_(), l2_installed_(false), - ethernet_tag_(0) { + vrf_(NULL), vrf_name_(""), vn_uuid_(), l2_installed_(false) { } VmInterface::FloatingIp::FloatingIp(const FloatingIp &rhs) : ListEntry(rhs.installed_, rhs.del_pending_), floating_ip_(rhs.floating_ip_), vn_(rhs.vn_), vrf_(rhs.vrf_), vrf_name_(rhs.vrf_name_), vn_uuid_(rhs.vn_uuid_), - l2_installed_(rhs.l2_installed_), ethernet_tag_(rhs.ethernet_tag_) { + l2_installed_(rhs.l2_installed_) { } VmInterface::FloatingIp::FloatingIp(const IpAddress &addr, const std::string &vrf, const boost::uuids::uuid &vn_uuid) : ListEntry(), floating_ip_(addr), vn_(NULL), vrf_(NULL), vrf_name_(vrf), - vn_uuid_(vn_uuid), l2_installed_(false), ethernet_tag_(0) { + vn_uuid_(vn_uuid), l2_installed_(false) { } VmInterface::FloatingIp::~FloatingIp() { @@ -3449,8 +3449,12 @@ void VmInterface::FloatingIp::L3DeActivate(VmInterface *interface) const { } void VmInterface::FloatingIp::L2Activate(VmInterface *interface, - bool force_update) const { + bool force_update, + uint32_t old_ethernet_tag) const { // Add route if not installed or if force requested + if (interface->ethernet_tag() != old_ethernet_tag) + force_update = true; + if (l2_installed_ && force_update == false) return; @@ -3463,15 +3467,19 @@ void VmInterface::FloatingIp::L2Activate(VmInterface *interface, EvpnAgentRouteTable *evpn_table = static_cast (vrf_->GetEvpnRouteTable()); //Agent *agent = evpn_table->agent(); - ethernet_tag_ = vn_->ComputeEthernetTag(); + if (old_ethernet_tag != interface->ethernet_tag()) { + L2DeActivate(interface, old_ethernet_tag); + } evpn_table->AddReceiveRoute(interface->peer_.get(), vrf_->GetName(), interface->l2_label(), MacAddress::FromString(interface->vm_mac()), - floating_ip_, ethernet_tag_, vn_->GetName()); + floating_ip_, interface->ethernet_tag(), + vn_->GetName()); l2_installed_ = true; } -void VmInterface::FloatingIp::L2DeActivate(VmInterface *interface) const { +void VmInterface::FloatingIp::L2DeActivate(VmInterface *interface, + uint32_t ethernet_tag) const { if (l2_installed_ == false) return; @@ -3480,14 +3488,14 @@ void VmInterface::FloatingIp::L2DeActivate(VmInterface *interface) const { if (evpn_table) { evpn_table->DelLocalVmRoute(interface->peer_.get(), vrf_->GetName(), MacAddress::FromString(interface->vm_mac()), - interface, floating_ip_, ethernet_tag_); + interface, floating_ip_, ethernet_tag); } - ethernet_tag_ = 0; l2_installed_ = false; } void VmInterface::FloatingIp::Activate(VmInterface *interface, - bool force_update, bool l2) const { + bool force_update, bool l2, + uint32_t old_ethernet_tag) const { InterfaceTable *table = static_cast(interface->get_table()); @@ -3502,14 +3510,15 @@ void VmInterface::FloatingIp::Activate(VmInterface *interface, } if (l2) - L2Activate(interface, force_update); + L2Activate(interface, force_update, old_ethernet_tag); else L3Activate(interface, force_update); } -void VmInterface::FloatingIp::DeActivate(VmInterface *interface, bool l2) const{ +void VmInterface::FloatingIp::DeActivate(VmInterface *interface, bool l2, + uint32_t old_ethernet_tag) const{ if (l2) - L2DeActivate(interface); + L2DeActivate(interface, old_ethernet_tag); else L3DeActivate(interface); diff --git a/src/vnsw/agent/oper/vm_interface.h b/src/vnsw/agent/oper/vm_interface.h index 74dae4ec01e..742e9170ce1 100644 --- a/src/vnsw/agent/oper/vm_interface.h +++ b/src/vnsw/agent/oper/vm_interface.h @@ -112,10 +112,14 @@ class VmInterface : public Interface { bool IsLess(const FloatingIp *rhs) const; void L3Activate(VmInterface *interface, bool force_update) const; void L3DeActivate(VmInterface *interface) const; - void L2Activate(VmInterface *interface, bool force_update) const; - void L2DeActivate(VmInterface *interface) const; - void DeActivate(VmInterface *interface, bool l2) const; - void Activate(VmInterface *interface, bool force_update, bool l2) const; + void L2Activate(VmInterface *interface, bool force_update, + uint32_t old_ethernet_tag) const; + void L2DeActivate(VmInterface *interface, + uint32_t ethernet_tag) const; + void DeActivate(VmInterface *interface, bool l2, + uint32_t old_ethernet_tag) const; + void Activate(VmInterface *interface, bool force_update, bool l2, + uint32_t old_ethernet_tag) const; IpAddress floating_ip_; mutable VnEntryRef vn_; @@ -123,7 +127,6 @@ class VmInterface : public Interface { std::string vrf_name_; boost::uuids::uuid vn_uuid_; mutable bool l2_installed_; - mutable int ethernet_tag_; }; typedef std::set FloatingIpSet; @@ -715,7 +718,8 @@ class VmInterface : public Interface { void DeleteMetadataRoute(bool old_ipv4_active, VrfEntry *old_vrf, bool old_need_linklocal_ip); void CleanupFloatingIpList(); - void UpdateFloatingIp(bool force_update, bool policy_change, bool l2); + void UpdateFloatingIp(bool force_update, bool policy_change, bool l2, + uint32_t old_ethernet_tag); void DeleteFloatingIp(bool l2, uint32_t old_ethernet_tag); void UpdateServiceVlan(bool force_update, bool policy_change); void DeleteServiceVlan();