From 131d048399a198554080c5f71535706ca53f9e9f Mon Sep 17 00:00:00 2001 From: Prabhjot Singh Sethi Date: Wed, 15 Jul 2015 22:39:43 +0530 Subject: [PATCH] Handle removing self reference on connection close Issue: ------ On Connection Close, ToR agent fails to remove self reference held in logical switch, causing OVSDB client IDL to be stuck in deleted state forever Fix: ---- On Connection Close trigger Process Delete Table Request to Logical Switch table which will manage removal of self reference due to local MACs first followed by triggering DeleteTable, to allow KSync delete state machine to move forward allowing the cleanup to go through. Closes-Bug: 1474926 Change-Id: Ib9fca3b48db4195f2095d00920a971bcb540e4d5 (cherry picked from commit 98a0e83c1521a2621c0db48e915d103de33378c5) --- .../ovsdb_client/logical_switch_ovsdb.cc | 44 +++++++++++++++++ .../ovsdb_client/logical_switch_ovsdb.h | 15 ++++++ .../ovsdb_client/ovsdb_client_idl.cc | 7 ++- .../test/test_ovs_unicast_local.cc | 47 +++++++++++++++++++ 4 files changed, 112 insertions(+), 1 deletion(-) diff --git a/src/vnsw/agent/ovs_tor_agent/ovsdb_client/logical_switch_ovsdb.cc b/src/vnsw/agent/ovs_tor_agent/ovsdb_client/logical_switch_ovsdb.cc index 94cbfddb3be..fba1bbde91b 100644 --- a/src/vnsw/agent/ovs_tor_agent/ovsdb_client/logical_switch_ovsdb.cc +++ b/src/vnsw/agent/ovs_tor_agent/ovsdb_client/logical_switch_ovsdb.cc @@ -471,6 +471,50 @@ KSyncDBObject::DBFilterResp LogicalSwitchTable::OvsdbDBEntryFilter( return DBFilterAccept; } +void LogicalSwitchTable::ProcessDeleteTableReq() { + ProcessDeleteTableReqTask *task = + new ProcessDeleteTableReqTask(this); + TaskScheduler *scheduler = TaskScheduler::GetInstance(); + scheduler->Enqueue(task); +} + +LogicalSwitchTable::ProcessDeleteTableReqTask::ProcessDeleteTableReqTask( + LogicalSwitchTable *table) : + Task((TaskScheduler::GetInstance()->GetTaskId("Agent::KSync")), 0), + table_(table), entry_(NULL) { +} + +LogicalSwitchTable::ProcessDeleteTableReqTask::~ProcessDeleteTableReqTask() { +} + +bool LogicalSwitchTable::ProcessDeleteTableReqTask::Run() { + KSyncEntry *kentry = entry_.get(); + if (kentry == NULL) { + kentry = table_->Next(kentry); + } + + int count = 0; + while (kentry != NULL) { + LogicalSwitchEntry *entry = static_cast(kentry); + count++; + kentry = table_->Next(kentry); + // while table is set for deletion reset the local_mac_ref + // since there will be no trigger from OVSDB database + entry->local_mac_ref_ = NULL; + + // check for yield + if (count == KEntriesPerIteration && kentry != NULL) { + entry_ = kentry; + return false; + } + } + + entry_ = NULL; + // Done processing delete request, schedule delete table + table_->DeleteTable(); + return true; +} + ///////////////////////////////////////////////////////////////////////////// // Sandesh routines ///////////////////////////////////////////////////////////////////////////// diff --git a/src/vnsw/agent/ovs_tor_agent/ovsdb_client/logical_switch_ovsdb.h b/src/vnsw/agent/ovs_tor_agent/ovsdb_client/logical_switch_ovsdb.h index fdb5462690f..d4cf6eabf54 100644 --- a/src/vnsw/agent/ovs_tor_agent/ovsdb_client/logical_switch_ovsdb.h +++ b/src/vnsw/agent/ovs_tor_agent/ovsdb_client/logical_switch_ovsdb.h @@ -31,8 +31,23 @@ class LogicalSwitchTable : public OvsdbDBObject { OvsdbDBEntry *AllocOvsEntry(struct ovsdb_idl_row *row); DBFilterResp OvsdbDBEntryFilter(const DBEntry *entry, const OvsdbDBEntry *ovsdb_entry); + void ProcessDeleteTableReq(); private: + class ProcessDeleteTableReqTask : public Task { + public: + static const int KEntriesPerIteration = 32; + ProcessDeleteTableReqTask(LogicalSwitchTable *table); + virtual ~ProcessDeleteTableReqTask(); + + bool Run(); + + private: + LogicalSwitchTable *table_; + KSyncEntry::KSyncEntryPtr entry_; + DISALLOW_COPY_AND_ASSIGN(ProcessDeleteTableReqTask); + }; + OvsdbIdlRowMap idl_row_map_; DISALLOW_COPY_AND_ASSIGN(LogicalSwitchTable); }; diff --git a/src/vnsw/agent/ovs_tor_agent/ovsdb_client/ovsdb_client_idl.cc b/src/vnsw/agent/ovs_tor_agent/ovsdb_client/ovsdb_client_idl.cc index 9fb88c27827..fd7e690d330 100644 --- a/src/vnsw/agent/ovs_tor_agent/ovsdb_client/ovsdb_client_idl.cc +++ b/src/vnsw/agent/ovs_tor_agent/ovsdb_client/ovsdb_client_idl.cc @@ -449,7 +449,12 @@ void OvsdbClientIdl::TriggerDeletion() { // trigger KSync Object delete for all objects. vm_interface_table_->DeleteTable(); physical_switch_table_->DeleteTable(); - logical_switch_table_->DeleteTable(); + + // trigger Process Delete, which will do internal processing to + // clear self reference from logical switch before triggering + // delete table + logical_switch_table_->ProcessDeleteTableReq(); + physical_port_table_->DeleteTable(); physical_locator_table_->DeleteTable(); vlan_port_table_->DeleteTable(); diff --git a/src/vnsw/agent/ovs_tor_agent/ovsdb_client/test/test_ovs_unicast_local.cc b/src/vnsw/agent/ovs_tor_agent/ovsdb_client/test/test_ovs_unicast_local.cc index 34ec1aca1c1..85d16abdd97 100644 --- a/src/vnsw/agent/ovs_tor_agent/ovsdb_client/test/test_ovs_unicast_local.cc +++ b/src/vnsw/agent/ovs_tor_agent/ovsdb_client/test/test_ovs_unicast_local.cc @@ -103,6 +103,7 @@ class UnicastLocalRouteTest : public ::testing::Test { LoadAndRun("controller/src/vnsw/agent/ovs_tor_agent/ovsdb_" "client/test/xml/ucast-local-test-setup.xml"); teardown_done_in_test_ = false; + client->WaitForIdle(); } virtual void TearDown() { @@ -235,6 +236,52 @@ TEST_F(UnicastLocalRouteTest, UnicastLocalDelayLogicalSwitchDeleteAndRenew) { } } +TEST_F(UnicastLocalRouteTest, ConnectionCloseWhileUnicastLocalPresent) { + LogicalSwitchTable *table = + tcp_session_->client_idl()->logical_switch_table(); + LogicalSwitchEntry key(table, UuidToString(MakeUuid(1))); + LogicalSwitchEntry *entry = static_cast + (table->Find(&key)); + EXPECT_TRUE((entry != NULL)); + if (entry != NULL) { + WAIT_FOR(10, 10000, + (true == add_ucast_mac_local(entry->name(), + "00:00:00:00:01:01", + "11.11.11.11"))); + // Wait for entry to add + WAIT_FOR(100, 10000, + (NULL != FindUcastLocal(entry->name(), "00:00:00:00:01:01"))); + + // Take reference to idl so that session object itself is not deleted. + OvsdbClientIdlPtr tcp_idl = tcp_session_->client_idl(); + tcp_session_->TriggerClose(); + client->WaitForIdle(); + + // validate refcount to be 2 one from session and one locally held + // to validate session closure, when we release refcount + WAIT_FOR(1000, 1000, (2 == tcp_idl->refcount())); + tcp_idl = NULL; + } + + WAIT_FOR(100, 10000, + (tcp_session_ = static_cast + (init_->ovsdb_client()->NextSession(NULL))) != NULL); + client->WaitForIdle(); + + table = tcp_session_->client_idl()->logical_switch_table(); + LogicalSwitchEntry key1(table, UuidToString(MakeUuid(1))); + entry = static_cast (table->Find(&key1)); + EXPECT_TRUE((entry != NULL)); + if (entry != NULL) { + WAIT_FOR(10, 10000, + (true == del_ucast_mac_local(entry->name(), + "00:00:00:00:01:01"))); + // Wait for entry to del + WAIT_FOR(100, 10000, + (NULL == FindUcastLocal(entry->name(), "00:00:00:00:01:01"))); + } +} + TEST_F(UnicastLocalRouteTest, tunnel_nh_1) { IpAddress server = Ip4Address::from_string("1.1.1.1"); OvsPeer *peer = peer_manager_->Allocate(server);