Skip to content

Commit

Permalink
Merge "Update IFNode seq number even for Delete operation"
Browse files Browse the repository at this point in the history
  • Loading branch information
@opencontrail-ci-admin
Zuul authored and opencontrail-ci-admin committed Jun 20, 2015
2 parents 845bc2e + 55debc2 commit 9527d75
Show file tree
Hide file tree
Showing 2 changed files with 203 additions and 13 deletions.
51 changes: 39 additions & 12 deletions src/ifmap/ifmap_agent_table.cc
View file
Expand Up @@ -78,7 +78,11 @@ IFMapNode *IFMapAgentTable::EntryLocate(IFMapNode *node, RequestKey *req) {

obj = node->GetObject();
assert(obj);
//We dont accept lesser sequence number updates
assert(obj->sequence_number() <= req->id_seq_num);

node->Remove(obj);

} else {
auto_ptr<DBEntry> key(AllocEntry(req));
node = const_cast<IFMapNode *>(
Expand All @@ -103,23 +107,27 @@ void IFMapAgentTable::HandlePendingLinks(IFMapNode *node) {
assert(ltable != NULL);

DBGraphVertex::adjacency_iterator iter;
bool origin_exists;
uint64_t seq;
for (iter = node->begin(graph_); iter != node->end(graph_);) {

right = static_cast<IFMapNode *>(iter.operator->());
iter++;
edge = graph_->GetEdge(node, right);
assert(edge);
IFMapLink *l = static_cast<IFMapLink *>(edge);
seq = l->sequence_number(IFMapOrigin::UNKNOWN, &origin_exists);
assert(origin_exists);

// Create both the request keys
auto_ptr <IFMapAgentLinkTable::RequestKey> req_key (new IFMapAgentLinkTable::RequestKey);
req_key->left_key.id_name = node->name();
req_key->left_key.id_type = node->table()->Typename();
req_key->left_key.id_seq_num = node->GetObject()->sequence_number();
req_key->left_key.id_seq_num = seq;

req_key->right_key.id_name = right->name();
req_key->right_key.id_type = right->table()->Typename();
req_key->right_key.id_seq_num = right->GetObject()->sequence_number();
req_key->right_key.id_seq_num = seq;
req_key->metadata = l->metadata();

DBRequest req;
Expand Down Expand Up @@ -158,6 +166,7 @@ void IFMapAgentTable::NotifyNode(IFMapNode *node) {
// Process link-defer list based for the request.
// If request is add, create left->right and right-left defer nodes
// If request is delete, remove left->right and right->left defer nodes
// The sequence number is valid only in the DeferrendNode entry
void IFMapAgentLinkTable::LinkDefAdd(DBRequest *request) {
RequestKey *key = static_cast<RequestKey *>(request->key.get());

Expand All @@ -174,6 +183,7 @@ void IFMapAgentLinkTable::LinkDefAdd(DBRequest *request) {
right = right_it->second;

if (request->oper == DBRequest::DB_ENTRY_DELETE) {
//We need to delete the old sequence links as well
// remove left->right entry
if (left) {
for(it = left->begin(); it != left->end(); it++) {
Expand Down Expand Up @@ -295,6 +305,12 @@ void IFMapAgentTable::Input(DBTablePartition *partition, DBClient *client,
return;
}

obj = node->GetObject();
//We dont accept lesser sequence number updates
assert(obj->sequence_number() <= key->id_seq_num);

//Upate the sequence number even for deletion of node
obj->set_sequence_number(key->id_seq_num);
DeleteNode(node);
return;
}
Expand All @@ -308,8 +324,10 @@ void IFMapAgentTable::Input(DBTablePartition *partition, DBClient *client,

//Set the sequence number of the object
obj->set_sequence_number(key->id_seq_num);

node->Insert(obj);
NotifyNode(node);

IFMapAgentLinkTable *link_table = static_cast<IFMapAgentLinkTable *>(
database()->FindTable(IFMAP_AGENT_LINK_DB_NAME));
link_table->EvalDefLink(key);
Expand Down Expand Up @@ -495,11 +513,17 @@ void IFMapAgentLinkTable::EvalDefLink(IFMapTable::RequestKey *key) {
continue;

// Skip if right-node is not yet present
if (IFMapAgentTable::TableEntryLookup(database(),
&((*left_list_entry).node_key))
== NULL) {
IFMapNode *node = IFMapAgentTable::TableEntryLookup(database(),
&((*left_list_entry).node_key));
if (!node)
continue;

//If the other end of the node is not from active control node,
//dont consider the link
IFMapObject *obj = node->GetObject();
if (obj->sequence_number() < key->id_seq_num)
continue;
}


// left->right entry found defer-list. Find the right->left entry
LinkDefMap::iterator right_defmap_it =
Expand All @@ -508,21 +532,24 @@ void IFMapAgentLinkTable::EvalDefLink(IFMapTable::RequestKey *key) {

std::list<DeferredNode> *right_list = right_defmap_it->second;
std::list<DeferredNode>::iterator right_it, right_list_entry;
for(right_it = right_list->begin(); right_it != right_list->end();) {
right_list_entry = right_it++;
for(right_it = right_list->begin(); right_it !=
right_list->end(); right_it++) {

// If link seq is older, dont consider the link.
if ((*right_list_entry).node_key.id_seq_num < key->id_seq_num)
if ((*right_it).node_key.id_seq_num < key->id_seq_num)
continue;

if ((*right_list_entry).node_key.id_type == key->id_type &&
(*right_list_entry).node_key.id_name == key->id_name) {
if ((*right_it).node_key.id_type == key->id_type &&
(*right_it).node_key.id_name == key->id_name) {
RemoveDefListEntry(&link_def_map_, right_defmap_it,
&right_list_entry);
&right_it);
break;
}
}

//We should have removed something in the above iteration
assert(right_it != right_list->end());

//Remove from deferred list before enqueing
auto_ptr <RequestKey> req_key (new RequestKey);
req_key->left_key = *key;
Expand Down
165 changes: 164 additions & 1 deletion src/vnsw/agent/test/test_cfg.cc
View file
Expand Up @@ -503,7 +503,6 @@ TEST_F(CfgTest, LinkReorderTest) {
" <name>testtest</name>\n"
" </node>\n"
"</update>");

IFMapTable *ftable = IFMapTable::FindTable(&db_, "foo");
ASSERT_TRUE(ftable!=NULL);

Expand Down Expand Up @@ -2190,6 +2189,170 @@ TEST_F(CfgTest, NodeDelLinkMetadata) {
EXPECT_EQ(link->metadata(), "foo-bar");
}

TEST_F(CfgTest, AssymetricLinkSeqTest) {
char buff[1500];
sprintf(buff,
"<update>\n"
" <link>\n"
" <node type=\"foo\">\n"
" <name>testfoo</name>\n"
" </node>\n"
" <node type=\"bar\">\n"
" <name>testbar</name>\n"
" </node>\n"
" </link>\n"
" <node type=\"foo\">\n"
" <name>testfoo</name>\n"
" </node>\n"
" <node type=\"bar\">\n"
" <name>testbar</name>\n"
" </node>\n"
"</update>");

IFMapTable *ftable = IFMapTable::FindTable(&db_, "foo");
ASSERT_TRUE(ftable!=NULL);

IFMapTable *btable = IFMapTable::FindTable(&db_, "bar");
ASSERT_TRUE(btable!=NULL);

IFMapAgentLinkTable *ltable = static_cast<IFMapAgentLinkTable *>(
db_.FindTable(IFMAP_AGENT_LINK_DB_NAME));
ASSERT_TRUE(ltable!=NULL);

//Add foo, bar and link between them with seq of 1
pugi::xml_parse_result result = xdoc_.load(buff);
EXPECT_TRUE(result);
parser_->ConfigParse(xdoc_, 1);
WaitForIdle();

IFMapNode *TestFoo = ftable->FindNode("testfoo");
ASSERT_TRUE(TestFoo !=NULL);
EXPECT_EQ("testfoo", TestFoo->name());

IFMapNode *TestBar = btable->FindNode("testbar");
ASSERT_TRUE(TestBar !=NULL);
EXPECT_EQ("testbar", TestBar->name());

//Ensure that there is Link from Foo to Bar
for (DBGraphVertex::adjacency_iterator iter = TestFoo->begin(&graph_);
iter != TestFoo->end(&graph_); ++iter) {
TestBar = static_cast<IFMapNode *>(iter.operator->());
EXPECT_EQ("testbar", TestBar->name());
}

//Update only bar with seq 2
sprintf(buff,
"<update>\n"
" <node type=\"bar\">\n"
" <name>testbar</name>\n"
" </node>\n"
"</update>\n");
result = xdoc_.load(buff);
EXPECT_TRUE(result);
parser_->ConfigParse(xdoc_, 2);
WaitForIdle();

TestFoo = ftable->FindNode("testfoo");
ASSERT_TRUE(TestFoo !=NULL);
EXPECT_EQ("testfoo", TestFoo->name());

TestBar = btable->FindNode("testbar");
ASSERT_TRUE(TestBar !=NULL);
EXPECT_EQ("testbar", TestBar->name());

//Ensure that there is Link from Foo to Bar even after seq update
for (DBGraphVertex::adjacency_iterator iter = TestFoo->begin(&graph_);
iter != TestFoo->end(&graph_); ++iter) {
TestBar = static_cast<IFMapNode *>(iter.operator->());
EXPECT_EQ("testbar", TestBar->name());
}

//Delete foo and ensure that bidifrectional link
//is moved to defer list
sprintf(buff,
"<delete>\n"
" <node type=\"foo\">\n"
" <name>testfoo</name>\n"
" </node>\n"
"</delete>\n");
result = xdoc_.load(buff);
EXPECT_TRUE(result);
parser_->ConfigParse(xdoc_, 2);
WaitForIdle();


const IFMapAgentLinkTable::LinkDefMap &def_list =
ltable->GetLinkDefMap();
EXPECT_EQ(def_list.size(), 2);

TestFoo = ftable->FindNode("testfoo");
ASSERT_TRUE(TestFoo == NULL);

//Ensure that there is Link from Foo to Bar even after seq update
int cnt = 0;
for (DBGraphVertex::adjacency_iterator iter = TestBar->begin(&graph_);
iter != TestBar->end(&graph_); ++iter) {
cnt++;
}
EXPECT_EQ(cnt, 0);

//Add the foo again but still link should not be added as
//link is of seq 1
sprintf(buff,
"<update>\n"
" <node type=\"foo\">\n"
" <name>testfoo</name>\n"
" </node>\n"
"</update>\n");
result = xdoc_.load(buff);
EXPECT_TRUE(result);
parser_->ConfigParse(xdoc_, 2);
WaitForIdle();
EXPECT_EQ(def_list.size(), 2);

TestFoo = ftable->FindNode("testfoo");
ASSERT_TRUE(TestFoo !=NULL);
EXPECT_EQ("testfoo", TestFoo->name());

cnt = 0;
for (DBGraphVertex::adjacency_iterator iter = TestBar->begin(&graph_);
iter != TestBar->end(&graph_); ++iter) {
cnt++;
}
EXPECT_EQ(cnt, 0);

//Update the link with seq of 2 and ensure link is added
sprintf(buff,
"<update>\n"
" <link>\n"
" <node type=\"foo\">\n"
" <name>testfoo</name>\n"
" </node>\n"
" <node type=\"bar\">\n"
" <name>testbar</name>\n"
" </node>\n"
" </link>\n"
"</update>");
result = xdoc_.load(buff);
EXPECT_TRUE(result);
parser_->ConfigParse(xdoc_, 2);
WaitForIdle();

for (DBGraphVertex::adjacency_iterator iter = TestFoo->begin(&graph_);
iter != TestFoo->end(&graph_); ++iter) {
TestBar = static_cast<IFMapNode *>(iter.operator->());
EXPECT_EQ("testbar", TestBar->name());
}
//Defer list should still exist, as above link is directly added
EXPECT_EQ(def_list.size(), 2);

//Run the stale timer and ensure defer list is cleaned
IFMapAgentStaleCleaner *cl = new IFMapAgentStaleCleaner(&db_, &graph_);
cl->StaleTimeout(2);
WaitForIdle();
EXPECT_EQ(def_list.size(), 0);
}

int main(int argc, char **argv) {
LoggingInit();
::testing::InitGoogleTest(&argc, argv);
Expand Down

0 comments on commit 9527d75

Please sign in to comment.