From 6edffd00cad1cecec9d37e1ba03f6f029f13e1b6 Mon Sep 17 00:00:00 2001 From: Manish Date: Thu, 26 May 2016 15:04:58 +0530 Subject: [PATCH] VMI was not removed and it held on VRF. Problem: Sequence of events goes like this: - Delete is issued on VMI. This reset configurer in VMI and mark it for deletion. However because of some reference entry is still present. - Add is issued. As entry is present in DB this will convert to change operation. On seeing a change delete flag is cleared but VMI does not get its configurer back. Agent just resyncs the entry. - Delete is issued again. Since configurer was not set resync for entry is not called to flush off VMI parameters, hence VRF reference is held. Results in Vrf Delete timeout. Solution: Resync will always be done for non deleted entry. In above case on add, delete flags has been reset and entry is no more in deleted state. So whenever resync is done, set the configurer. Also OnDelete calls resync to flush VMI parameters. Reset of configurer should be done after resync call is done in OnDelete. Change-Id: I431cc3dcf264a23a0e2e1491c23ee2f8a257aba4 Closes-bug: #1550459 --- src/vnsw/agent/oper/test/test_logical_intf.cc | 59 +++++++++++++++++++ src/vnsw/agent/oper/vm_interface.cc | 5 +- 2 files changed, 62 insertions(+), 2 deletions(-) diff --git a/src/vnsw/agent/oper/test/test_logical_intf.cc b/src/vnsw/agent/oper/test/test_logical_intf.cc index 1240170c4e1..ee6e20b0ee8 100644 --- a/src/vnsw/agent/oper/test/test_logical_intf.cc +++ b/src/vnsw/agent/oper/test/test_logical_intf.cc @@ -154,6 +154,65 @@ TEST_F(IntfTest, basic_1) { client->WaitForIdle(); } +//https://bugs.launchpad.net/juniperopenstack/r3.0/+bug/1550459 +TEST_F(IntfTest, bug_1550459) { + AddVrf("vrf1"); + AddVn("vn1", 1); + AddPhysicalInterface("pi1", 1, "pi1"); + AddLogicalInterface("li1", 1, "li1", 100); + AddLink("physical-interface", "pi1", "logical-interface", "li1"); + + AddVmi(1, 1); + client->WaitForIdle(); + AddLink(1, 1); + client->WaitForIdle(); + AddLink("virtual-network", "vn1", "routing-instance", "vrf1"); + AddLink("virtual-machine-interface", "vmi1", + "virtual-network", "vn1"); + AddLink("virtual-machine-interface-routing-instance", "vmi1", + "routing-instance", "vrf1", + "virtual-machine-interface-routing-instance"); + AddLink("virtual-machine-interface-routing-instance", "vmi1", + "virtual-machine-interface", "vmi1", + "virtual-machine-interface-routing-instance"); + client->WaitForIdle(); + + WAIT_FOR(1000, 100, (LogicalInterfaceGet(1, "li1") != NULL)); + client->WaitForIdle(); + LogicalInterface *li = LogicalInterfaceGet(1, "li1"); + + WAIT_FOR(1000, 100, (li->vm_interface() != NULL)); + client->WaitForIdle(); + InterfaceRef vmi_ref = li->vm_interface(); + VmInterface *vmi = static_cast(vmi_ref.get()); + VmInterface::Delete(Agent::GetInstance()->interface_table(), + vmi->GetUuid(), VmInterface::CONFIG); + client->WaitForIdle(); + AddVmi(1, 1); + client->WaitForIdle(); + VmInterface::Delete(Agent::GetInstance()->interface_table(), + vmi->GetUuid(), VmInterface::CONFIG); + client->WaitForIdle(); + + DelLink(1, 1); + DelLink("virtual-network", "vn1", "routing-instance", "vrf1"); + DelLink("virtual-machine-interface", "vmi1", + "virtual-network", "vn1"); + DelLink("virtual-machine-interface-routing-instance", "vmi1", + "routing-instance", "vrf1"); + DelLink("virtual-machine-interface-routing-instance", "vmi1", + "virtual-machine-interface", "vmi1"); + client->WaitForIdle(); + DelLink("physical-interface", "pi1", "logical-interface", "li1"); + vmi_ref.reset(); + DelVmi(1); + DeleteLogicalInterface("li1"); + DeletePhysicalInterface("pi1"); + DelVrf("vrf1"); + DelVn("vn1"); + client->WaitForIdle(); +} + int main(int argc, char **argv) { GETUSERARGS(); diff --git a/src/vnsw/agent/oper/vm_interface.cc b/src/vnsw/agent/oper/vm_interface.cc index 958d88e2c74..7b96ab9f5fb 100644 --- a/src/vnsw/agent/oper/vm_interface.cc +++ b/src/vnsw/agent/oper/vm_interface.cc @@ -1687,9 +1687,9 @@ bool VmInterfaceConfigData::OnDelete(const InterfaceTable *table, if (vmi->IsConfigurerSet(VmInterface::CONFIG) == false) return true; - vmi->ResetConfigurer(VmInterface::CONFIG); VmInterfaceConfigData data(NULL, NULL); vmi->Resync(table, &data); + vmi->ResetConfigurer(VmInterface::CONFIG); return true; } @@ -1710,6 +1710,7 @@ bool VmInterfaceConfigData::OnResync(const InterfaceTable *table, ecmp_load_balance_changed || static_route_config_changed) *force_update = true; + vmi->SetConfigurer(VmInterface::CONFIG); return ret; } @@ -2093,9 +2094,9 @@ bool VmInterfaceNovaData::OnDelete(const InterfaceTable *table, if (vmi->IsConfigurerSet(VmInterface::INSTANCE_MSG) == false) return true; - vmi->ResetConfigurer(VmInterface::CONFIG); VmInterfaceConfigData data(NULL, NULL); vmi->Resync(table, &data); + vmi->ResetConfigurer(VmInterface::CONFIG); vmi->ResetConfigurer(VmInterface::INSTANCE_MSG); return true; }