From f9eca51ca02ac7bbea1e592aa195f265e8fe7937 Mon Sep 17 00:00:00 2001 From: ashoksingh Date: Mon, 11 Jul 2016 10:54:12 +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. Change-Id: Ia3c11ad71558a6e9e215bbb6817ccd9f65253a5d Closes-Bug: #1596792 --- src/vnsw/agent/oper/vm_interface.cc | 19 ++- 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, 169 insertions(+), 12 deletions(-) diff --git a/src/vnsw/agent/oper/vm_interface.cc b/src/vnsw/agent/oper/vm_interface.cc index 8083c3de4ba..fa0b15a8c6c 100644 --- a/src/vnsw/agent/oper/vm_interface.cc +++ b/src/vnsw/agent/oper/vm_interface.cc @@ -1211,7 +1211,8 @@ void VmInterface::UpdateL3(bool old_ipv4_active, VrfEntry *old_vrf, policy_change, old_vrf, old_dhcp_addr); } - UpdateIpv4InstanceIp(force_update, policy_change, false, old_ethernet_tag); + UpdateIpv4InstanceIp(force_update, policy_change, false, + old_ethernet_tag, old_vrf); UpdateMetadataRoute(old_ipv4_active, old_vrf); UpdateFloatingIp(force_update, policy_change, false, old_ethernet_tag); UpdateServiceVlan(force_update, policy_change); @@ -1298,7 +1299,8 @@ void VmInterface::UpdateL2(bool old_l2_active, VrfEntry *old_vrf, old_layer3_forwarding, policy_change, Ip4Address(), Ip6Address(), MacAddress::FromString(vm_mac_)); - 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_l2_active, @@ -1365,7 +1367,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; } @@ -2999,6 +3001,10 @@ void VmInterface::UpdateL2InterfaceRoute(bool old_l2_active, 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, @@ -4472,8 +4478,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; } @@ -4487,6 +4493,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 742e9170ce1..20931840c4b 100644 --- a/src/vnsw/agent/oper/vm_interface.h +++ b/src/vnsw/agent/oper/vm_interface.h @@ -741,12 +741,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 273ddddf452..a2e4ad7c314 100644 --- a/src/vnsw/agent/uve/test/test_vn_uve.cc +++ b/src/vnsw/agent/uve/test/test_vn_uve.cc @@ -210,7 +210,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()); @@ -242,7 +242,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 @@ -1232,6 +1232,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 ee0935b4f0b..00ea5cd8562 100644 --- a/src/vnsw/agent/uve/vn_uve_table_base.cc +++ b/src/vnsw/agent/uve/vn_uve_table_base.cc @@ -277,8 +277,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;