Fix ToR Agent Crash - add on deleted IDL

Issue: ------ In Certain quick connect-disconnect of session may result in scheduling of only some KSyncObjects, where other might be pending in KSyncObjectManager workqueue, on further update it may result in Add operation on other table which is already scheduled for deletion resulting in this crash Fix: ---- Moving client idl is deleted check to common routine in base class to ignore Add/Change notifications for deleted client IDL. Closes-Bug: 1455949 Change-Id: Iefd50bee4594bcdef8fe70f4d61578b6b54ac9a9 (cherry picked from commit c27071c)
Juniper · May 18, 2015 · ea399c5 · ea399c5
1 parent 6c7edcc
commit ea399c5
Show file tree

Hide file tree

Showing 14 changed files with 35 additions and 26 deletions.
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
@@ -436,7 +436,7 @@ OvsdbDBEntry *LogicalSwitchTable::AllocOvsEntry(struct ovsdb_idl_row *row) {
     return static_cast<OvsdbDBEntry *>(CreateStale(&key));
 }
 
-KSyncDBObject::DBFilterResp LogicalSwitchTable::DBEntryFilter(
+KSyncDBObject::DBFilterResp LogicalSwitchTable::OvsdbDBEntryFilter(
         const DBEntry *db_entry) {
     const PhysicalDeviceVn *entry =
         static_cast<const PhysicalDeviceVn *>(db_entry);

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
@@ -29,7 +29,7 @@ class LogicalSwitchTable : public OvsdbDBObject {
     KSyncEntry *Alloc(const KSyncEntry *key, uint32_t index);
     KSyncEntry *DBToKSyncEntry(const DBEntry*);
     OvsdbDBEntry *AllocOvsEntry(struct ovsdb_idl_row *row);
-    KSyncDBObject::DBFilterResp DBEntryFilter(const DBEntry *entry);
+    DBFilterResp OvsdbDBEntryFilter(const DBEntry *entry);
 
 private:
     OvsdbIdlRowMap  idl_row_map_;

diff --git a/src/vnsw/agent/ovs_tor_agent/ovsdb_client/ovsdb_object.cc b/src/vnsw/agent/ovs_tor_agent/ovsdb_client/ovsdb_object.cc
@@ -140,3 +140,22 @@ void OvsdbDBObject::EmptyTable(void) {
     }
 }
 
+KSyncDBObject::DBFilterResp OvsdbDBObject::OvsdbDBEntryFilter(
+        const DBEntry *entry) {
+    // Accept by default, unless overriden by dereived class
+    return DBFilterAccept;
+}
+
+KSyncDBObject::DBFilterResp OvsdbDBObject::DBEntryFilter(const DBEntry *entry) {
+    // Ignore Add/Change notifications while client idl is deleted
+    // there can be cases that the current table might be pending
+    // for delete schedule in KSyncObjectManager Queue and in
+    // the mean while we get a notification, where we should not
+    // process it further.
+    if (client_idl_->deleted()) {
+        return DBFilterIgnore;
+    }
+
+    return OvsdbDBEntryFilter(entry);
+}
+
diff --git a/src/vnsw/agent/ovs_tor_agent/ovsdb_client/ovsdb_object.h b/src/vnsw/agent/ovs_tor_agent/ovsdb_client/ovsdb_object.h
@@ -63,9 +63,12 @@ class OvsdbDBObject : public KSyncDBObject {
     void DeleteTable(void);
 
     virtual void EmptyTable(void);
+    virtual DBFilterResp OvsdbDBEntryFilter(const DBEntry *entry);
 
     OvsdbClientIdl *client_idl() { return client_idl_.get();}
 protected:
+    DBFilterResp DBEntryFilter(const DBEntry *entry);
+
     OvsdbClientIdlPtr client_idl_;
 private:
     friend class OvsdbDBEntry;

diff --git a/src/vnsw/agent/ovs_tor_agent/ovsdb_client/unicast_mac_remote_ovsdb.cc b/src/vnsw/agent/ovs_tor_agent/ovsdb_client/unicast_mac_remote_ovsdb.cc
@@ -333,15 +333,8 @@ OvsdbDBEntry *UnicastMacRemoteTable::AllocOvsEntry(struct ovsdb_idl_row *row) {
     return static_cast<OvsdbDBEntry *>(CreateStale(&key));
 }
 
-KSyncDBObject::DBFilterResp UnicastMacRemoteTable::DBEntryFilter(
+KSyncDBObject::DBFilterResp UnicastMacRemoteTable::OvsdbDBEntryFilter(
         const DBEntry *db_entry) {
-    // Since Object delete for unicast remote table happens by db
-    // walk on vrf table, it needs to implement filter to ignore
-    // db Add/Change notifications if idl is marked deleted.
-    if (client_idl()->deleted()) {
-        return DBFilterIgnore;
-    }
-
     const BridgeRouteEntry *entry =
         static_cast<const BridgeRouteEntry *>(db_entry);
     //Locally programmed multicast route should not be added in

diff --git a/src/vnsw/agent/ovs_tor_agent/ovsdb_client/unicast_mac_remote_ovsdb.h b/src/vnsw/agent/ovs_tor_agent/ovsdb_client/unicast_mac_remote_ovsdb.h
@@ -31,7 +31,7 @@ class UnicastMacRemoteTable : public OvsdbDBObject {
     KSyncEntry *DBToKSyncEntry(const DBEntry*);
     OvsdbDBEntry *AllocOvsEntry(struct ovsdb_idl_row *row);
 
-    KSyncDBObject::DBFilterResp DBEntryFilter(const DBEntry *entry);
+    DBFilterResp OvsdbDBEntryFilter(const DBEntry *entry);
 
     void ManagedDelete();
     virtual void EmptyTable();

diff --git a/src/vnsw/agent/ovs_tor_agent/ovsdb_client/vlan_port_binding_ovsdb.cc b/src/vnsw/agent/ovs_tor_agent/ovsdb_client/vlan_port_binding_ovsdb.cc
@@ -263,7 +263,7 @@ OvsdbDBEntry *VlanPortBindingTable::AllocOvsEntry(struct ovsdb_idl_row *row) {
     return NULL;
 }
 
-KSyncDBObject::DBFilterResp VlanPortBindingTable::DBEntryFilter(
+KSyncDBObject::DBFilterResp VlanPortBindingTable::OvsdbDBEntryFilter(
         const DBEntry *entry) {
     const VlanLogicalInterface *l_port =
         dynamic_cast<const VlanLogicalInterface *>(entry);

diff --git a/src/vnsw/agent/ovs_tor_agent/ovsdb_client/vlan_port_binding_ovsdb.h b/src/vnsw/agent/ovs_tor_agent/ovsdb_client/vlan_port_binding_ovsdb.h
@@ -21,7 +21,7 @@ class VlanPortBindingTable : public OvsdbDBObject {
     KSyncEntry *Alloc(const KSyncEntry *key, uint32_t index);
     KSyncEntry *DBToKSyncEntry(const DBEntry*);
     OvsdbDBEntry* AllocOvsEntry(struct ovsdb_idl_row*);
-    DBFilterResp DBEntryFilter(const DBEntry *entry);
+    DBFilterResp OvsdbDBEntryFilter(const DBEntry *entry);
 
 private:
     DISALLOW_COPY_AND_ASSIGN(VlanPortBindingTable);

diff --git a/src/vnsw/agent/ovs_tor_agent/ovsdb_client/vm_interface_ksync.cc b/src/vnsw/agent/ovs_tor_agent/ovsdb_client/vm_interface_ksync.cc
@@ -129,7 +129,7 @@ OvsdbDBEntry *VMInterfaceKSyncObject::AllocOvsEntry(struct ovsdb_idl_row *row) {
     return NULL;
 }
 
-KSyncDBObject::DBFilterResp VMInterfaceKSyncObject::DBEntryFilter(
+KSyncDBObject::DBFilterResp VMInterfaceKSyncObject::OvsdbDBEntryFilter(
         const DBEntry *entry) {
     const Interface *intf = static_cast<const Interface *>(entry);
     // only accept vm interfaces

diff --git a/src/vnsw/agent/ovs_tor_agent/ovsdb_client/vm_interface_ksync.h b/src/vnsw/agent/ovs_tor_agent/ovsdb_client/vm_interface_ksync.h
@@ -20,7 +20,7 @@ class VMInterfaceKSyncObject : public OvsdbDBObject {
     KSyncEntry *Alloc(const KSyncEntry *key, uint32_t index);
     KSyncEntry *DBToKSyncEntry(const DBEntry*);
     OvsdbDBEntry* AllocOvsEntry(struct ovsdb_idl_row*);
-    DBFilterResp DBEntryFilter(const DBEntry *entry);
+    DBFilterResp OvsdbDBEntryFilter(const DBEntry *entry);
 
 private:
     DISALLOW_COPY_AND_ASSIGN(VMInterfaceKSyncObject);

diff --git a/src/vnsw/agent/ovs_tor_agent/ovsdb_client/vn_ovsdb.cc b/src/vnsw/agent/ovs_tor_agent/ovsdb_client/vn_ovsdb.cc
@@ -115,7 +115,7 @@ OvsdbDBEntry *VnOvsdbObject::AllocOvsEntry(struct ovsdb_idl_row *row) {
     return NULL;
 }
 
-KSyncDBObject::DBFilterResp VnOvsdbObject::DBEntryFilter(
+KSyncDBObject::DBFilterResp VnOvsdbObject::OvsdbDBEntryFilter(
         const DBEntry *entry) {
     const VnEntry *vn = static_cast<const VnEntry *>(entry);
     // only accept Virtual Networks with non-NULL vrf

diff --git a/src/vnsw/agent/ovs_tor_agent/ovsdb_client/vn_ovsdb.h b/src/vnsw/agent/ovs_tor_agent/ovsdb_client/vn_ovsdb.h
@@ -20,7 +20,7 @@ class VnOvsdbObject : public OvsdbDBObject {
     KSyncEntry *Alloc(const KSyncEntry *key, uint32_t index);
     KSyncEntry *DBToKSyncEntry(const DBEntry*);
     OvsdbDBEntry* AllocOvsEntry(struct ovsdb_idl_row*);
-    DBFilterResp DBEntryFilter(const DBEntry *entry);
+    DBFilterResp OvsdbDBEntryFilter(const DBEntry *entry);
 
 private:
     DISALLOW_COPY_AND_ASSIGN(VnOvsdbObject);

diff --git a/src/vnsw/agent/ovs_tor_agent/ovsdb_client/vrf_ovsdb.cc b/src/vnsw/agent/ovs_tor_agent/ovsdb_client/vrf_ovsdb.cc
@@ -155,14 +155,8 @@ OvsdbDBEntry *VrfOvsdbObject::AllocOvsEntry(struct ovsdb_idl_row *row) {
     return NULL;
 }
 
-KSyncDBObject::DBFilterResp VrfOvsdbObject::DBEntryFilter(const DBEntry *entry) {
-    if (client_idl_->deleted()) {
-        // To Handle a case when client idl is deleted and Delete table
-        // is still in KSyncObject Manager queue, ignore notification to
-        // avoid creating unicast remote table for deleted IDL
-        return DBFilterIgnore;
-    }
-
+KSyncDBObject::DBFilterResp VrfOvsdbObject::OvsdbDBEntryFilter(
+        const DBEntry *entry) {
     const VrfEntry *vrf = static_cast<const VrfEntry *>(entry);
     // Delete Vrf if vn goes NULL
     if (vrf->vn() == NULL) {

diff --git a/src/vnsw/agent/ovs_tor_agent/ovsdb_client/vrf_ovsdb.h b/src/vnsw/agent/ovs_tor_agent/ovsdb_client/vrf_ovsdb.h
@@ -25,7 +25,7 @@ class VrfOvsdbObject : public OvsdbDBObject {
     KSyncEntry *Alloc(const KSyncEntry *key, uint32_t index);
     KSyncEntry *DBToKSyncEntry(const DBEntry*);
     OvsdbDBEntry *AllocOvsEntry(struct ovsdb_idl_row *row);
-    KSyncDBObject::DBFilterResp DBEntryFilter(const DBEntry *entry);
+    DBFilterResp OvsdbDBEntryFilter(const DBEntry *entry);
 
 private:
     DISALLOW_COPY_AND_ASSIGN(VrfOvsdbObject);