From a6f03e5250c75f21fa2e44000d40b122efffbd14 Mon Sep 17 00:00:00 2001 From: ashoksingh Date: Fri, 8 Jul 2016 11:57:56 +0530 Subject: [PATCH] Handle VN change in interface Notification Remove the assumption that VN of an interface cannot change without first getting a notification for NULL VN. Also fix the following issue. Also when VN changes for a VMI (clubbing of VMI getting NULL VRF and later new VRF events into a single event) we were not removing EVPN and Bridge routes for old VRF. Also added UT to verify this scenario. Closes-Bug: #1596792 (cherry picked from commit 79986fdf34e61f0e0fd82c5a19810450f98e985f) Change-Id: I2d8b28d166a53da9c166392eafb1ee8407e8ecc4 --- src/vnsw/agent/oper/vm_interface.cc | 18 ++- src/vnsw/agent/oper/vm_interface.h | 5 +- src/vnsw/agent/uve/test/test_vn_uve.cc | 147 +++++++++++++++++++++++- src/vnsw/agent/uve/vn_uve_table_base.cc | 10 +- 4 files changed, 168 insertions(+), 12 deletions(-) diff --git a/src/vnsw/agent/oper/vm_interface.cc b/src/vnsw/agent/oper/vm_interface.cc index 1916f93f0b4..d50eb6fe8e2 100644 --- a/src/vnsw/agent/oper/vm_interface.cc +++ b/src/vnsw/agent/oper/vm_interface.cc @@ -1341,7 +1341,7 @@ void VmInterface::UpdateL3(bool old_ipv4_active, VrfEntry *old_vrf, old_vrf, old_dhcp_addr); } UpdateIpv4InstanceIp(force_update, policy_change, false, - old_ethernet_tag); + old_ethernet_tag, old_vrf); UpdateFloatingIp(force_update, policy_change, false, old_ethernet_tag); UpdateResolveRoute(old_ipv4_active, force_update, policy_change, old_vrf, old_subnet, old_subnet_plen); @@ -1420,7 +1420,8 @@ void VmInterface::UpdateL2Bridging(bool old_bridging, VrfEntry *old_vrf, Ip4Address(), Ip6Address(), MacAddress::FromString(vm_mac_), Ip4Address(0)); - UpdateIpv4InstanceIp(force_update, policy_change, true, old_ethernet_tag); + UpdateIpv4InstanceIp(force_update, policy_change, true, old_ethernet_tag, + old_vrf); UpdateIpv6InstanceIp(force_update, policy_change, true, old_ethernet_tag); UpdateFloatingIp(force_update, policy_change, true, old_ethernet_tag); UpdateAllowedAddressPair(force_update, policy_change, true, old_bridging, @@ -1502,7 +1503,7 @@ void VmInterface::ApplyConfigCommon(const VrfEntry *old_vrf, void VmInterface::ApplyMacVmBindingConfig(const VrfEntry *old_vrf, bool old_active, bool old_dhcp_enable) { - if (!IsActive()) { + if (!IsActive() || old_vrf != vrf()) { DeleteMacVmBinding(old_vrf); return; } @@ -3275,6 +3276,10 @@ void VmInterface::UpdateL2InterfaceRoute(bool old_bridging, bool force_update, force_update = true; } + if (old_vrf && old_vrf != vrf()) { + force_update = true; + } + //Encap change will result in force update of l2 routes. if (force_update) { DeleteL2InterfaceRoute(true, old_vrf, old_v4_addr, @@ -4789,8 +4794,8 @@ bool VmInterface::CopyIp6Address(const Ip6Address &addr) { } void VmInterface::UpdateIpv4InstanceIp(bool force_update, bool policy_change, - bool l2, - uint32_t old_ethernet_tag) { + bool l2, uint32_t old_ethernet_tag, + VrfEntry *old_vrf) { if (l2 && old_ethernet_tag != ethernet_tag()) { force_update = true; } @@ -4804,6 +4809,9 @@ void VmInterface::UpdateIpv4InstanceIp(bool force_update, bool policy_change, instance_ipv4_list_.list_.erase(prev); } } else { + if (old_vrf && (old_vrf != vrf())) { + prev->DeActivate(this, l2, old_vrf, old_ethernet_tag); + } prev->Activate(this, force_update||policy_change, l2, old_ethernet_tag); } diff --git a/src/vnsw/agent/oper/vm_interface.h b/src/vnsw/agent/oper/vm_interface.h index 83d6a7f46a5..8dbbac0a0f8 100644 --- a/src/vnsw/agent/oper/vm_interface.h +++ b/src/vnsw/agent/oper/vm_interface.h @@ -778,12 +778,11 @@ class VmInterface : public Interface { int old_ethernet_tag, const MacAddress &mac) const; - void DeleteL2Route(const std::string &vrf_name, - const MacAddress &mac); void UpdateVrfAssignRule(); void DeleteVrfAssignRule(); void UpdateIpv4InstanceIp(bool force_update, bool policy_change, - bool l2, uint32_t old_ethernet_tag); + bool l2, uint32_t old_ethernet_tag, + VrfEntry *old_vrf); void DeleteIpv4InstanceIp(bool l2, uint32_t old_ethernet_tag, VrfEntry *old_vrf); void UpdateIpv6InstanceIp(bool force_update, bool policy_change, diff --git a/src/vnsw/agent/uve/test/test_vn_uve.cc b/src/vnsw/agent/uve/test/test_vn_uve.cc index f7db0fdd3b5..b8626ffff37 100644 --- a/src/vnsw/agent/uve/test/test_vn_uve.cc +++ b/src/vnsw/agent/uve/test/test_vn_uve.cc @@ -214,7 +214,7 @@ TEST_F(UveVnUveTest, VnIntfAddDel_1) { util_.EnqueueSendVnUveTask(); client->WaitForIdle(); - //Verify UVE + //Verify UVE WAIT_FOR(1000, 500, (vnut->send_count() >= 2U)); EXPECT_EQ(1U, uve1->get_virtualmachine_list().size()); EXPECT_EQ(1U, uve1->get_interface_list().size()); @@ -246,7 +246,7 @@ TEST_F(UveVnUveTest, VnIntfAddDel_1) { client->WaitForIdle(); WAIT_FOR(1000, 500, (vnut->VnUveCount() == 2U)); - //Verify UVE + //Verify UVE EXPECT_EQ(1U, vnut->delete_count()); //other cleanup @@ -1235,6 +1235,149 @@ TEST_F(UveVnUveTest, VnBandwidth) { EXPECT_EQ(0U, ksock->flow_map.size()); } +/* Change Vn for an Interface. To verify VN change is handled as part of + interface notification */ +TEST_F(UveVnUveTest, VnChangeForIntf_1) { + struct PortInfo input[] = { + {"vnet1", 1, "1.1.1.1", "00:00:00:01:01:01", 1, 1}, + }; + + VnUveTableTest *vnut = static_cast + (Agent::GetInstance()->uve()->vn_uve_table()); + int old_vn_uve_count = vnut->VnUveCount(); + //Add VN + util_.VnAdd(input[0].vn_id); + client->WaitForIdle(); + WAIT_FOR(1000, 500, (VnGet(1) != NULL)); + WAIT_FOR(1000, 500, (vnut->VnUveCount() == (old_vn_uve_count + 1))); + + util_.EnqueueSendVnUveTask(); + client->WaitForIdle(); + WAIT_FOR(1000, 500, (vnut->VnUveObject("vn1") != NULL)); + + UveVirtualNetworkAgent *uve1 = vnut->VnUveObject("vn1"); + + // Nova Port add message + util_.NovaPortAdd(input); + + // Config Port add + util_.ConfigPortAdd(input); + + //Verify that the port is inactive + EXPECT_TRUE(VmPortInactive(input, 0)); + + //Add necessary objects and links to make vm-intf active + util_.VmAdd(input[0].vm_id); + util_.VrfAdd(input[0].vn_id); + AddLink("virtual-network", "vn1", "routing-instance", "vrf1"); + client->WaitForIdle(); + AddLink("virtual-network", "vn1", "virtual-machine-interface", "vnet1"); + client->WaitForIdle(); + AddLink("virtual-machine", "vm1", "virtual-machine-interface", "vnet1"); + client->WaitForIdle(); + AddVmPortVrf("vnet1", "", 0); + client->WaitForIdle(); + AddInstanceIp("instance0", input[0].vm_id, input[0].addr); + AddLink("virtual-machine-interface", input[0].name, + "instance-ip", "instance0"); + client->WaitForIdle(); + AddLink("virtual-machine-interface-routing-instance", "vnet1", + "routing-instance", "vrf1"); + client->WaitForIdle(); + AddLink("virtual-machine-interface-routing-instance", "vnet1", + "virtual-machine-interface", "vnet1"); + client->WaitForIdle(); + EXPECT_TRUE(VmPortActive(input, 0)); + + /* Create a new VN and associate it with existing VMI. This should + * trigger change of VN for the VMI */ + util_.VnAdd(2); + client->WaitForIdle(); + WAIT_FOR(1000, 500, (VnGet(2) != NULL)); + util_.VrfAdd(2); + client->WaitForIdle(); + + agent_->db()->SetQueueDisable(true); + DelLink("virtual-network", "vn1", "virtual-machine-interface", "vnet1"); + DelLink("virtual-machine-interface-routing-instance", "vnet1", + "routing-instance", "vrf1"); + client->WaitForIdle(); + AddLink("virtual-network", "vn2", "routing-instance", "vrf2"); + AddLink("virtual-network", "vn2", "virtual-machine-interface", "vnet1"); + AddLink("virtual-machine-interface-routing-instance", "vnet1", + "routing-instance", "vrf2"); + agent_->db()->SetQueueDisable(false); + client->WaitForIdle(); + WAIT_FOR(1000, 5000, (vnut->GetVnUveEntry("vn2") != NULL)); + + VmInterface *vmi = VmInterfaceGet(input[0].intf_id); + EXPECT_TRUE(vmi != NULL); + VnEntry *vn = VnGet(2); + EXPECT_TRUE(vn != NULL); + WAIT_FOR(1000, 5000, (vmi->vn() == vn)); + + util_.EnqueueSendVnUveTask(); + client->WaitForIdle(); + //Verify UVE + WAIT_FOR(1000, 500, (vnut->send_count() >= 2U)); + EXPECT_EQ(0U, uve1->get_interface_list().size()); + + UveVirtualNetworkAgent *uve2 = vnut->VnUveObject("vn2"); + EXPECT_EQ(1U, uve2->get_interface_list().size()); + + // Delete virtual-machine-interface to vrf link attribute + DelLink("virtual-machine-interface-routing-instance", "vnet1", + "routing-instance", "vrf1"); + DelLink("virtual-machine-interface-routing-instance", "vnet1", + "virtual-machine-interface", "vnet1"); + client->WaitForIdle(); + + DelLink("virtual-network", "vn2", "virtual-machine-interface", "vnet1"); + DelLink("virtual-machine-interface-routing-instance", "vnet1", + "routing-instance", "vrf2"); + DelLink("virtual-network", "vn2", "routing-instance", "vrf2"); + DelNode("virtual-network", "vn2"); + DelNode("routing-instance", "vrf2"); + client->WaitForIdle(); + WAIT_FOR(1000, 5000, (VrfFind("vrf2") == false)); + + //Delete VN + util_.VnDelete(input[0].vn_id); + + util_.EnqueueSendVnUveTask(); + client->WaitForIdle(); + WAIT_FOR(1000, 500, (vnut->VnUveCount() == old_vn_uve_count)); + + //Verify UVE + EXPECT_EQ(2U, vnut->delete_count()); + + //other cleanup + DelLink("virtual-machine-interface", input[0].name, + "instance-ip", "instance0"); + DelLink("virtual-machine", "vm1", "virtual-machine-interface", "vnet1"); + DelLink("virtual-network", "vn1", "virtual-machine-interface", "vnet1"); + client->WaitForIdle(); + EXPECT_TRUE(VmPortInactive(input, 0)); + + DelLink("virtual-network", "vn1", "routing-instance", "vrf1"); + DelNode("virtual-machine-interface-routing-instance", "vnet1"); + DelNode("virtual-machine", "vm1"); + DelNode("routing-instance", "vrf1"); + DelNode("virtual-network", "vn1"); + DelNode("virtual-machine-interface", "vnet1"); + DelInstanceIp("instance0"); + client->WaitForIdle(); + IntfCfgDel(input, 0); + client->WaitForIdle(); + WAIT_FOR(1000, 5000, (VrfFind("vrf1") == false)); + WAIT_FOR(1000, 5000, (vnut->GetVnUveEntry("vn1") == NULL)); + WAIT_FOR(1000, 5000, (vnut->GetVnUveEntry("vn2") == NULL)); + + //clear counters at the end of test case + client->Reset(); + vnut->ClearCount(); +} + int main(int argc, char **argv) { GETUSERARGS(); /* Sent AgentStatsCollector and FlowStatsCollector timer intervals to 10 diff --git a/src/vnsw/agent/uve/vn_uve_table_base.cc b/src/vnsw/agent/uve/vn_uve_table_base.cc index de5235b4a40..eae1901a429 100644 --- a/src/vnsw/agent/uve/vn_uve_table_base.cc +++ b/src/vnsw/agent/uve/vn_uve_table_base.cc @@ -285,8 +285,14 @@ void VnUveTableBase::InterfaceNotify(DBTablePartBase *partition, DBEntryBase *e) e->SetState(partition->parent(), intf_listener_id_, state); InterfaceAddHandler(vn, vm_port, vm_name, state); } else { - /* Change in VN name is not supported now */ - assert(state->vn_name_.compare(vn->GetName()) == 0); + if (state->vn_name_.compare(vn->GetName()) != 0) { + InterfaceDeleteHandler(state->vm_name_, state->vn_name_, + vm_port); + state->vm_name_ = vm_name; + state->vn_name_ = vn->GetName(); + InterfaceAddHandler(vn, vm_port, vm_name, state); + return; + } if (state->vm_name_.compare(vm_name) != 0) { InterfaceAddHandler(vn, vm_port, vm_name, state); state->vm_name_ = vm_name;