Skip to content

Commit

Permalink
closes-bug:# 1390326
Browse files Browse the repository at this point in the history 
Holding the metadata of link while storing it in defer list. While
adding the link to Graph from defer list, the metadata also added back
to graph which makes the dependency tracker to work correctly

Change-Id: I5df26ba5f3ea66ee8d649e151106794c7f9ef387
  • Loading branch information
@divakardhar
divakardhar committed Feb 23, 2015
1 parent d4ba989 commit 43d7a71
Show file tree
Hide file tree
Showing 6 changed files with 327 additions and 44 deletions.
1 change: 1 addition & 0 deletions src/ifmap/ifmap_agent.sandesh
View file
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ struct IFMapAgentDefLink {
1: i64 seq_num;
2: string left_node;
3: string right_node;
4: string metadata;
}

response sandesh ShowIFMapAgentDefLinkResp {
Expand Down
13 changes: 8 additions & 5 deletions src/ifmap/ifmap_agent_sandesh.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -288,8 +288,8 @@ void ShowIFMapAgentDefLinkReq::HandleRequest() const {
ShowIFMapAgentTable::db_->FindTable(IFMAP_AGENT_LINK_DB_NAME));

IFMapAgentLinkTable::LinkDefMap::const_iterator dlist_it;
std::list<IFMapTable::RequestKey> *ent;
std::list<IFMapTable::RequestKey>::iterator it;
std::list<IFMapAgentLinkTable::DeferredNode> *ent;
std::list<IFMapAgentLinkTable::DeferredNode>::iterator it;

//Get linktables's deferred list
const IFMapAgentLinkTable::LinkDefMap &def_list = link_table->GetLinkDefMap();
Expand All @@ -306,9 +306,12 @@ void ShowIFMapAgentDefLinkReq::HandleRequest() const {
//Iterate the right nodes corresponding to above left node
for(it = ent->begin(); it != ent->end(); it++) {
IFMapAgentDefLink data;
data.set_seq_num((*it).id_seq_num);
data.set_left_node(temp.id_type + ":" + temp.id_name);
data.set_right_node((*it).id_type + ":" + (*it).id_name);
data.set_seq_num((*it).node_key.id_seq_num);
data.set_left_node(temp.id_type + ":" +
temp.id_name);
data.set_metadata((*it).link_metadata);
data.set_right_node((*it).node_key.id_type + ":" +
(*it).node_key.id_name);
list.push_back(data);
}
}
Expand Down
83 changes: 48 additions & 35 deletions src/ifmap/ifmap_agent_table.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -109,6 +109,7 @@ void IFMapAgentTable::HandlePendingLinks(IFMapNode *node) {
iter++;
edge = graph_->GetEdge(node, right);
assert(edge);
IFMapLink *l = static_cast<IFMapLink *>(edge);

// Create both the request keys
auto_ptr <IFMapAgentLinkTable::RequestKey> req_key (new IFMapAgentLinkTable::RequestKey);
Expand All @@ -119,6 +120,7 @@ void IFMapAgentTable::HandlePendingLinks(IFMapNode *node) {
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->metadata = l->metadata();

DBRequest req;
req.oper = DBRequest::DB_ENTRY_ADD_CHANGE;
Expand Down Expand Up @@ -159,14 +161,14 @@ void IFMapAgentTable::NotifyNode(IFMapNode *node) {
void IFMapAgentLinkTable::LinkDefAdd(DBRequest *request) {
RequestKey *key = static_cast<RequestKey *>(request->key.get());

std::list<IFMapTable::RequestKey>::iterator it;
std::list<DeferredNode>::iterator it;

std::list<IFMapTable::RequestKey> *left = NULL;
std::list<DeferredNode> *left = NULL;
LinkDefMap::iterator left_it = link_def_map_.find(key->left_key);
if (link_def_map_.end() != left_it)
left = left_it->second;

std::list<IFMapTable::RequestKey> *right = NULL;
std::list<DeferredNode> *right = NULL;
LinkDefMap::iterator right_it = link_def_map_.find(key->right_key);
if (link_def_map_.end() != right_it)
right = right_it->second;
Expand All @@ -175,8 +177,8 @@ void IFMapAgentLinkTable::LinkDefAdd(DBRequest *request) {
// remove left->right entry
if (left) {
for(it = left->begin(); it != left->end(); it++) {
if (((*it).id_type == key->right_key.id_type) &&
((*it).id_name == key->right_key.id_name)) {
if (((*it).node_key.id_type == key->right_key.id_type) &&
((*it).node_key.id_name == key->right_key.id_name)) {
left->erase(it);
break;
}
Expand All @@ -187,8 +189,8 @@ void IFMapAgentLinkTable::LinkDefAdd(DBRequest *request) {
// remove right->left entry
if (right) {
for(it = right->begin(); it != right->end(); it++) {
if (((*it).id_type == key->left_key.id_type) &&
((*it).id_name == key->left_key.id_name)) {
if (((*it).node_key.id_type == key->left_key.id_type) &&
((*it).node_key.id_name == key->left_key.id_name)) {
right->erase(it);
break;
}
Expand All @@ -205,15 +207,16 @@ void IFMapAgentLinkTable::LinkDefAdd(DBRequest *request) {
if (left) {
// If list already contains, just update the seq number
for(it = left->begin(); it != left->end(); it++) {
if (((*it).id_type == key->right_key.id_type) &&
((*it).id_name == key->right_key.id_name)) {
(*it).id_seq_num = key->right_key.id_seq_num;
if (((*it).node_key.id_type == key->right_key.id_type) &&
((*it).node_key.id_name == key->right_key.id_name)) {
(*it).node_key.id_seq_num = key->right_key.id_seq_num;
(*it).link_metadata = key->metadata;
push_left = false;
break;
}
}
} else {
left = new std::list<IFMapTable::RequestKey>();
left = new std::list<DeferredNode>();
link_def_map_[key->left_key] = left;
}

Expand All @@ -222,23 +225,30 @@ void IFMapAgentLinkTable::LinkDefAdd(DBRequest *request) {
if (right) {
// If list already contains, just update the seq number
for(it = right->begin(); it != right->end(); it++) {
if (((*it).id_type == key->left_key.id_type) &&
((*it).id_name == key->left_key.id_name)) {
(*it).id_seq_num = key->left_key.id_seq_num;
if (((*it).node_key.id_type == key->left_key.id_type) &&
((*it).node_key.id_name == key->left_key.id_name)) {
(*it).node_key.id_seq_num = key->left_key.id_seq_num;
(*it).link_metadata = key->metadata;
push_right = false;
break;
}
}
} else {
right = new std::list<IFMapTable::RequestKey>();
right = new std::list<DeferredNode>();
link_def_map_[key->right_key] = right;
}

// Add it to the end of the list
if (push_left)
left->push_back(key->right_key);
if (push_right)
right->push_back(key->left_key);
struct DeferredNode dn;
dn.link_metadata = key->metadata;
if (push_left) {
dn.node_key = key->right_key;
left->push_back(dn);
}
if (push_right) {
dn.node_key = key->left_key;
right->push_back(dn);
}
return;
}

Expand Down Expand Up @@ -449,9 +459,9 @@ void IFMapAgentLinkTable::Input(DBTablePartition *partition, DBClient *client,

bool IFMapAgentLinkTable::RemoveDefListEntry
(LinkDefMap *map, LinkDefMap::iterator &map_it,
std::list<IFMapTable::RequestKey>::iterator *list_it) {
std::list<DeferredNode>::iterator *list_it) {

std::list<IFMapTable::RequestKey> *list = map_it->second;
std::list<DeferredNode> *list = map_it->second;
if (list_it) {
list->erase(*list_it);
}
Expand All @@ -475,36 +485,38 @@ void IFMapAgentLinkTable::EvalDefLink(IFMapTable::RequestKey *key) {
if (link_def_map_.end() == link_defmap_it)
return;

std::list<IFMapTable::RequestKey> *left_list = link_defmap_it->second;
std::list<IFMapTable::RequestKey>::iterator left_it, left_list_entry;
std::list<DeferredNode> *left_list = link_defmap_it->second;
std::list<DeferredNode>::iterator left_it, left_list_entry;
for(left_it = left_list->begin(); left_it != left_list->end();) {
left_list_entry = left_it++;

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

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

// left->right entry found defer-list. Find the right->left entry
LinkDefMap::iterator right_defmap_it = link_def_map_.find(*left_list_entry);
LinkDefMap::iterator right_defmap_it =
link_def_map_.find((*left_list_entry).node_key);
assert(link_def_map_.end() != right_defmap_it);

std::list<IFMapTable::RequestKey> *right_list = right_defmap_it->second;
std::list<IFMapTable::RequestKey>::iterator right_it, right_list_entry;
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++;

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

if ((*right_list_entry).id_type == key->id_type &&
(*right_list_entry).id_name == key->id_name) {
if ((*right_list_entry).node_key.id_type == key->id_type &&
(*right_list_entry).node_key.id_name == key->id_name) {
RemoveDefListEntry(&link_def_map_, right_defmap_it,
&right_list_entry);
break;
Expand All @@ -514,7 +526,8 @@ void IFMapAgentLinkTable::EvalDefLink(IFMapTable::RequestKey *key) {
//Remove from deferred list before enqueing
auto_ptr <RequestKey> req_key (new RequestKey);
req_key->left_key = *key;
req_key->right_key = *left_list_entry;
req_key->right_key = (*left_list_entry).node_key;
req_key->metadata = (*left_list_entry).link_metadata;
// Dont delete left_list_entry. Its passed in req structure
left_list->erase(left_list_entry);

Expand All @@ -529,8 +542,8 @@ void IFMapAgentLinkTable::EvalDefLink(IFMapTable::RequestKey *key) {
}

void IFMapAgentLinkTable::DestroyDefLink(uint64_t seq) {
std::list<IFMapTable::RequestKey> *ent;
std::list<IFMapTable::RequestKey>::iterator it, list_entry;
std::list<DeferredNode> *ent;
std::list<DeferredNode>::iterator it, list_entry;
IFMapAgentLinkTable::LinkDefMap::iterator dlist_it, temp;

for(dlist_it = link_def_map_.begin(); dlist_it != link_def_map_.end(); ) {
Expand All @@ -540,7 +553,7 @@ void IFMapAgentLinkTable::DestroyDefLink(uint64_t seq) {
list_entry = it++;

//Delete the deferred link if it is old seq
if ((*list_entry).id_seq_num < seq) {
if ((*list_entry).node_key.id_seq_num < seq) {
if (RemoveDefListEntry(&link_def_map_, temp,
&list_entry) == true) {
//The list has been deleted. Move to the next map
Expand Down
10 changes: 8 additions & 2 deletions src/ifmap/ifmap_agent_table.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@ class IFMapNode;

class IFMapAgentTable : public IFMapTable {
public:

struct IFMapAgentData : DBRequestData {
std::auto_ptr<IFMapObject>content;
};
Expand Down Expand Up @@ -66,6 +67,11 @@ class IFMapAgentLinkTable : public IFMapLinkTable {
std::string metadata;
};

struct DeferredNode {
IFMapTable::RequestKey node_key;
std::string link_metadata;
};

class comp {
public:
bool operator()(const IFMapTable::RequestKey &left,
Expand All @@ -78,15 +84,15 @@ class IFMapAgentLinkTable : public IFMapLinkTable {
};

IFMapAgentLinkTable(DB *db, const std::string &name, DBGraph *graph);
typedef std::map<IFMapTable::RequestKey, std::list<IFMapTable::RequestKey> *, comp> LinkDefMap;
typedef std::map<IFMapTable::RequestKey, std::list<DeferredNode> *, comp> LinkDefMap;
virtual void Input(DBTablePartition *partition, DBClient *client,
DBRequest *req);
void IFMapAgentLinkTable_Init(DB *db, DBGraph *graph);
static DBTable *CreateTable(DB *db, const std::string &name,
DBGraph *graph);
void EvalDefLink(IFMapTable::RequestKey *key);
bool RemoveDefListEntry(LinkDefMap *map, LinkDefMap::iterator &map_it,
std::list<IFMapTable::RequestKey>::iterator *list_it);
std::list<DeferredNode>::iterator *list_it);
void DestroyDefLink(uint64_t);
const LinkDefMap &GetLinkDefMap() const {
return link_def_map_;
Expand Down
53 changes: 53 additions & 0 deletions src/vnsw/agent/oper/test/physical-device-vn.xml
View file
Original file line number Diff line number Diff line change
Expand Up @@ -153,6 +153,59 @@
uuid="1" name="tap1" mac="00:00:00:00:00:01" vn-name="vn1" vn-uuid="1"
vm-name="vm1" vm-uuid="1" vrf="vrf1" ip="1.1.1.1" del="1"
/>
</test>

<test name="Physical-Router-Physical-Intf-Link-Uuid-0">
<!-- Validate physical-router to physical-interface link propogation to
physical-device-vn when the UUID for physical-interface changes to 0 -->
<virtual-network uuid="1" name="vn-1"/>
<physical-interface uuid="1" name="intf-1" />
<!-- Add logical-interface with UUID 0 -->
<logical-interface uuid="0" name="l-intf-1"/>
<virtual-machine uuid="1" name="vm1"/>
<physical-router uuid="1" name="router-1"/>

<vmi nova="1"
uuid="1" name="tap1" mac="00:00:00:00:00:01" vn-name="vn-1" vn-uuid="1"
vm-name="vm1" vm-uuid="1" vrf="vrf1" ip="1.1.1.1"
/>

<link left="physical-router" left-name="router-1"
right="physical-interface" right-name="intf-1"/>
<link left="physical-interface" left-name="intf-1"
right="logical-interface" right-name="l-intf-1"/>
<link left="logical-interface" left-name="l-intf-1"
right="virtual-machine-interface" right-name="tap1"/>
<validate name="validate-4">
<physical-router-vn name="router-vn-1" uuid="1" device="1" vn="1"
present="no"/>
</validate>

<!-- Change UUID for logical-interface to 1 -->
<logical-interface uuid="1" name="l-intf-1"/>
<validate name="validate-5">
<physical-router-vn name="router-vn-1" uuid="1" device="1" vn="1"
present="yes"/>
</validate>

<!-- Cleanup configuration -->
<link left="virtual-machine-interface" left-name="tap1"
right="virtual-network" right-name="vn-1" del="1"/>
<link left="logical-interface" left-name="l-intf-1"
right="virtual-machine-interface" right-name="tap-1" del="1"/>
<link left="physical-router" left-name="router-1"
right="physical-interface" right-name="intf-1" del="1"/>
<link left="physical-interface" left-name="intf-1"
right="logical-interface" right-name="l-intf-1" del="1"/>
<physical-router uuid="1" name="router-1" del="1"/>
<virtual-network uuid="1" name="vn-1" del="1"/>
<virtual-network uuid="2" name="test-vn" del="1"/>
<physical-interface uuid="1" name="intf-1" device="router-1" del="1"/>
<logical-interface uuid="1" name="l-intf-1" port="p-intf-1" vmi="tap1" del="1"/>
<virtual-machine uuid="1" name="vm1" del="1"/>
<vmi nova="1"
uuid="1" name="tap1" mac="00:00:00:00:00:01" vn-name="vn1" vn-uuid="1"
vm-name="vm1" vm-uuid="1" vrf="vrf1" ip="1.1.1.1" del="1"
/>
</test>
</test_suite>

0 comments on commit 43d7a71

Please sign in to comment.