From ea399c53054febc93edce271f579806d72588272 Mon Sep 17 00:00:00 2001 From: Prabhjot Singh Sethi Date: Mon, 18 May 2015 15:09:51 +0530 Subject: [PATCH] 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 c27071c4d10744ddc7fd5e4ae5248e2ce6c7e727) --- .../ovsdb_client/logical_switch_ovsdb.cc | 2 +- .../ovsdb_client/logical_switch_ovsdb.h | 2 +- .../ovsdb_client/ovsdb_object.cc | 19 +++++++++++++++++++ .../ovs_tor_agent/ovsdb_client/ovsdb_object.h | 3 +++ .../ovsdb_client/unicast_mac_remote_ovsdb.cc | 9 +-------- .../ovsdb_client/unicast_mac_remote_ovsdb.h | 2 +- .../ovsdb_client/vlan_port_binding_ovsdb.cc | 2 +- .../ovsdb_client/vlan_port_binding_ovsdb.h | 2 +- .../ovsdb_client/vm_interface_ksync.cc | 2 +- .../ovsdb_client/vm_interface_ksync.h | 2 +- .../ovs_tor_agent/ovsdb_client/vn_ovsdb.cc | 2 +- .../ovs_tor_agent/ovsdb_client/vn_ovsdb.h | 2 +- .../ovs_tor_agent/ovsdb_client/vrf_ovsdb.cc | 10 ++-------- .../ovs_tor_agent/ovsdb_client/vrf_ovsdb.h | 2 +- 14 files changed, 35 insertions(+), 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 index 7271255e83b..0c003739175 100644 --- 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(CreateStale(&key)); } -KSyncDBObject::DBFilterResp LogicalSwitchTable::DBEntryFilter( +KSyncDBObject::DBFilterResp LogicalSwitchTable::OvsdbDBEntryFilter( const DBEntry *db_entry) { const PhysicalDeviceVn *entry = static_cast(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 index 2e6d7dea27a..66c0023abcd 100644 --- 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 index 75eee6911a3..2e4d32b0549 100644 --- 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 index 4c9b1dc1fe2..7b656debf8c 100644 --- 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 index 451698fc1fb..9ac9f27f5b7 100644 --- 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(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(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 index 95d652b7605..76acaff1a68 100644 --- 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 index f948e2a2712..5221e166d95 100644 --- 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(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 index 48987210b8d..b7e0726b177 100644 --- 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 index 670567ad388..319c6195d7f 100644 --- 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(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 index a5df34782bb..c44888bc2f4 100644 --- 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 index a1876d579e0..e17fd033ebe 100644 --- 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(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 index fb0f8bcd266..8ed988b3343 100644 --- 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 index 9754ce076ce..21ab533cd01 100644 --- 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(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 index 2c0c125cd78..393f966b0a3 100644 --- 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);