Skip to content

Commit

Permalink
Handle removing self reference on connection close
Browse files Browse the repository at this point in the history
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 98a0e83)
  • Loading branch information
Prabhjot Singh Sethi committed Jul 16, 2015
1 parent 642f3f8 commit 131d048
Show file tree
Hide file tree
Showing 4 changed files with 112 additions and 1 deletion.
44 changes: 44 additions & 0 deletions src/vnsw/agent/ovs_tor_agent/ovsdb_client/logical_switch_ovsdb.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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<LogicalSwitchEntry *>(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
/////////////////////////////////////////////////////////////////////////////
Expand Down
15 changes: 15 additions & 0 deletions src/vnsw/agent/ovs_tor_agent/ovsdb_client/logical_switch_ovsdb.h
Original file line number Diff line number Diff line change
Expand Up @@ -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);
};
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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() {
Expand Down Expand Up @@ -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<LogicalSwitchEntry *>
(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<OvsdbClientTcpSession *>
(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<LogicalSwitchEntry *> (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);
Expand Down

0 comments on commit 131d048

Please sign in to comment.