* Update SG-id in SG upon change

Test case for same. Closes-bug:#1558933 Change-Id: I2846433d6506ea13994e62608e16fde1dfe1be7f
Juniper · Mar 30, 2016 · fbcaa39 · fbcaa39
1 parent 959d2ad
commit fbcaa39
Show file tree

Hide file tree

Showing 4 changed files with 24 additions and 10 deletions.
diff --git a/src/vnsw/agent/oper/agent.sandesh b/src/vnsw/agent/oper/agent.sandesh
@@ -483,6 +483,16 @@ objectlog sandesh SgObjectLog {
     1: SgObjectLogInfo sg;
 }
 
+/**
+ * @description: Trace log for security group object in vrouter-agent oper DB
+ * @type: Trace
+ * @severity: DEBUG
+ * @object: SgEntry
+ */
+traceobject sandesh SgObjectTrace {
+    1: SgObjectLogInfo sg;
+}
+
 /**
  * Request message for security group request
  */

diff --git a/src/vnsw/agent/oper/sg.cc b/src/vnsw/agent/oper/sg.cc
@@ -53,18 +53,16 @@ std::auto_ptr<DBEntry> SgTable::AllocEntry(const DBRequestKey *k) const {
 
 DBEntry *SgTable::OperDBAdd(const DBRequest *req) {
     SgKey *key = static_cast<SgKey *>(req->key.get());
-    SgData *data = static_cast<SgData *>(req->data.get());
     SgEntry *sg = new SgEntry(key->sg_uuid_);
-    sg->sg_id_ = data->sg_id_;
     ChangeHandler(sg, req);
-    sg->SendObjectLog(AgentLogEvent::ADD);
+    sg->SendObjectLog(GetOperDBTraceBuf(), AgentLogEvent::ADD);
     return sg;
 }
 
 bool SgTable::OperDBOnChange(DBEntry *entry, const DBRequest *req) {
     bool ret = ChangeHandler(entry, req);
     SgEntry *sg = static_cast<SgEntry *>(entry);
-    sg->SendObjectLog(AgentLogEvent::CHANGE);
+    sg->SendObjectLog(GetOperDBTraceBuf(), AgentLogEvent::CHANGE);
     return ret;
 }
 
@@ -73,6 +71,11 @@ bool SgTable::ChangeHandler(DBEntry *entry, const DBRequest *req) {
     SgEntry *sg = static_cast<SgEntry *>(entry);
     SgData *data = static_cast<SgData *>(req->data.get());
 
+    if (sg->sg_id_ != data->sg_id_) {
+        sg->sg_id_ = data->sg_id_;
+        ret = true;
+    }
+
     AclKey key(data->egress_acl_id_);
     AclDBEntry *acl = static_cast<AclDBEntry *>(agent()->acl_table()->FindActiveEntry(&key));
     if (sg->egress_acl_ != acl) {
@@ -90,7 +93,7 @@ bool SgTable::ChangeHandler(DBEntry *entry, const DBRequest *req) {
 
 bool SgTable::OperDBDelete(DBEntry *entry, const DBRequest *req) {
     SgEntry *sg = static_cast<SgEntry *>(entry);
-    sg->SendObjectLog(AgentLogEvent::DELETE);
+    sg->SendObjectLog(GetOperDBTraceBuf(), AgentLogEvent::DELETE);
     return true;
 }
 
@@ -203,7 +206,8 @@ bool SgEntry::DBEntrySandesh(Sandesh *sresp, std::string &name)  const {
     return false;
 }
 
-void SgEntry::SendObjectLog(AgentLogEvent::type event) const {
+void SgEntry::SendObjectLog(SandeshTraceBufferPtr buf, 
+                            AgentLogEvent::type event) const {
     SgObjectLogInfo info;
 
     string str;
@@ -234,6 +238,7 @@ void SgEntry::SendObjectLog(AgentLogEvent::type event) const {
     }
     info.set_ref_count(GetRefCount());
     SG_OBJECT_LOG_LOG("AgentSg", SandeshLevel::SYS_INFO, info);
+    SG_OBJECT_TRACE_TRACE(buf, info);
 }
 
 void SgListReq::HandleRequest() const {

diff --git a/src/vnsw/agent/oper/sg.h b/src/vnsw/agent/oper/sg.h
@@ -59,7 +59,8 @@ class SgEntry : AgentRefCount<SgEntry>, public AgentOperDBEntry {
     }
 
     bool DBEntrySandesh(Sandesh *sresp, std::string &name) const;
-    void SendObjectLog(AgentLogEvent::type event) const;
+    void SendObjectLog(SandeshTraceBufferPtr ptr,
+                       AgentLogEvent::type event) const;
 private:
     friend class SgTable;
     uuid sg_uuid_;

diff --git a/src/vnsw/agent/test/test_vmport_cfg.cc b/src/vnsw/agent/test/test_vmport_cfg.cc
@@ -1509,15 +1509,13 @@ TEST_F(CfgTest, SecurityGroup_ignore_invalid_sgid_1) {
     EXPECT_TRUE(sg_entry != NULL);
     EXPECT_TRUE(sg_entry->GetSgId() == 2);
 
-    // Try modifying with another sg id for same uuid and it should not happen
-    // in oper. Old sgid i.e. 2 shud be retained.
     AddSg("sg1", 1, 3);
     client->WaitForIdle();
     sg_entry = static_cast<const SgEntry *>(Agent::GetInstance()->sg_table()->
                                             FindActiveEntry(key));
 
     EXPECT_TRUE(sg_entry != NULL);
-    EXPECT_TRUE(sg_entry->GetSgId() == 2);
+    EXPECT_TRUE(sg_entry->GetSgId() == 3);
 
     DelLink("virtual-network", "vn1", "access-control-list", "acl1");
     DelLink("virtual-machine-interface", "vnet1", "access-control-list", "acl1");