Fix Tor-Agent crash for VRF Delete timeout

Issue:
------
when a logical interface is associated with multiple VMIs
and administrator removes one VMI from the list associated
it results in deletion of existing VRF and re-addition
again. By this time if there are any routes imported from
TOR, they hold the reference to VRF and doesnot allow VRF
to clean up, which results in a delete timeout.
In usual scenarios VRF is deleted along with VN, so we
didnot observe this issue earlier.

Fix:
----
Maintain a VRF dependency list in UnicastMacLocalOvsdb
table and trigger re-eval of entries on VRF delete.
which removes the OvsdbEntry for unicast mac local and
re-adds resulting in removing the VRF reference and move
the entry to add-defer state to wait for new VRF object.

This trigger of re-eval is triggered in a work queue to
ensure the order of events such that re-eval kicks in after
the vn_ovsdb_entry becomes in-active to hold the
re-addition of route in Add defer state.

Also adding fix for marking a logical entry as incomplete
if logical interface to vmi association is removed to allow
completion of logical switch delete.

Change-Id: Ifdcd06266b1eeefb36a4fcd90f3f0d0e2e471527
Closes-Bug: 1420903

src/vnsw/agent/ovs_tor_agent/ovsdb_client/ovsdb_client_idl.cc

@@ -219,6 +219,19 @@ void OvsdbClientIdl::DeleteTxn(struct ovsdb_idl_txn *txn) {
     ovsdb_wrapper_idl_txn_destroy(txn);
 }
 
+// API to trigger ovs row del followed by add
+// used by OvsdbEntry on catastrophic change event, which
+// results in emulating a delete followed by add
+void OvsdbClientIdl::NotifyDelAdd(struct ovsdb_idl_row *row) {
+    int i = ovsdb_wrapper_row_type(row);
+    if (i >= OvsdbClientIdl::OVSDB_TYPE_COUNT)
+        return;
+    if (callback_[i] != NULL) {
+        callback_[i](OvsdbClientIdl::OVSDB_DEL, row);
+        callback_[i](OvsdbClientIdl::OVSDB_ADD, row);
+    }
+}
+
 Ip4Address OvsdbClientIdl::tsn_ip() {
     return session_->tsn_ip();
 }

src/vnsw/agent/ovs_tor_agent/ovsdb_client/ovsdb_client_idl.h

@@ -86,6 +86,9 @@ class OvsdbClientIdl {
     void DeleteTxn(struct ovsdb_idl_txn *txn);
     void Register(EntryType type, NotifyCB cb) {callback_[type] = cb;}
     void UnRegister(EntryType type) {callback_[type] = NULL;}
+
+    // Notify Delete followed by add for a given ovsdb_idl_row
+    void NotifyDelAdd(struct ovsdb_idl_row *row);
     // Get TOR Service Node IP
     Ip4Address tsn_ip();
 

src/vnsw/agent/ovs_tor_agent/ovsdb_client/unicast_mac_local_ovsdb.cc

@@ -7,6 +7,7 @@ extern "C" {
 };
 #include <cmn/agent.h>
 #include <oper/vn.h>
+#include <oper/vrf.h>
 #include <oper/vxlan.h>
 #include <oper/physical_device_vn.h>
 #include <ovs_tor_agent/tor_agent_init.h>
@@ -55,7 +56,10 @@ bool UnicastMacLocalEntry::Add() {
         static_cast<LogicalSwitchEntry *>(l_table->Find(&l_key));
     const PhysicalDeviceVn *dev_vn =
         static_cast<const PhysicalDeviceVn *>(ls_entry->GetDBEntry());
+    // Take vrf reference to genrate withdraw/delete route request
     vrf_ = dev_vn->vn()->GetVrf();
+    // Add vrf dep in UnicastMacLocalOvsdb
+    table->vrf_dep_list_.insert(UnicastMacLocalOvsdb::VrfDepEntry(vrf_.get(), this));
     vxlan_id_ = dev_vn->vxlan_id();
     boost::system::error_code err;
     Ip4Address dest = Ip4Address::from_string(dest_ip_, err);
@@ -67,7 +71,10 @@ bool UnicastMacLocalEntry::Delete() {
     UnicastMacLocalOvsdb *table = static_cast<UnicastMacLocalOvsdb *>(table_);
     OVSDB_TRACE(Trace, "Deleting Route " + mac_ + " VN uuid " +
             logical_switch_name_ + " destination IP " + dest_ip_);
-    table->peer()->DeleteOvsRoute(vrf_, vxlan_id_, MacAddress(mac_));
+    table->vrf_dep_list_.erase(UnicastMacLocalOvsdb::VrfDepEntry(vrf_.get(), this));
+    table->peer()->DeleteOvsRoute(vrf_.get(), vxlan_id_, MacAddress(mac_));
+    // remove vrf reference after deleting route
+    vrf_ = NULL;
     return true;
 }
 
@@ -118,11 +125,16 @@ const std::string &UnicastMacLocalEntry::dest_ip() const {
 
 UnicastMacLocalOvsdb::UnicastMacLocalOvsdb(OvsdbClientIdl *idl, OvsPeer *peer) :
     OvsdbObject(idl), peer_(peer) {
+    vrf_reeval_queue_ = new WorkQueue<VrfEntryRef>(
+            TaskScheduler::GetInstance()->GetTaskId("Agent::KSync"), 0,
+            boost::bind(&UnicastMacLocalOvsdb::VrfReEval, this, _1));
     idl->Register(OvsdbClientIdl::OVSDB_UCAST_MAC_LOCAL,
             boost::bind(&UnicastMacLocalOvsdb::Notify, this, _1, _2));
 }
 
 UnicastMacLocalOvsdb::~UnicastMacLocalOvsdb() {
+    vrf_reeval_queue_->Shutdown();
+    delete vrf_reeval_queue_;
 }
 
 OvsPeer *UnicastMacLocalOvsdb::peer() {
@@ -147,11 +159,13 @@ void UnicastMacLocalOvsdb::Notify(OvsdbClientIdl::Op op,
     /* trigger delete if dest ip is not available */
     if (op == OvsdbClientIdl::OVSDB_DEL || dest_ip == NULL) {
         if (entry != NULL) {
+            entry->ovs_entry_ = NULL;
             Delete(entry);
         }
     } else if (op == OvsdbClientIdl::OVSDB_ADD) {
         if (entry == NULL) {
             entry = static_cast<UnicastMacLocalEntry *>(Create(&key));
+            entry->ovs_entry_ = row;
         }
     } else {
         assert(0);
@@ -165,6 +179,29 @@ KSyncEntry *UnicastMacLocalOvsdb::Alloc(const KSyncEntry *key, uint32_t index) {
     return entry;
 }
 
+void UnicastMacLocalOvsdb::VrfReEvalEnqueue(VrfEntry *vrf) {
+    vrf_reeval_queue_->Enqueue(vrf);
+}
+
+bool UnicastMacLocalOvsdb::VrfReEval(VrfEntryRef vrf) {
+    // iterate through dependency list and trigger del and add
+    VrfDepList::iterator it =
+        vrf_dep_list_.upper_bound(VrfDepEntry(vrf.get(), NULL));
+    while (it != vrf_dep_list_.end()) {
+        if (it->first != vrf.get()) {
+            break;
+        }
+        UnicastMacLocalEntry *u_entry = it->second;
+        it++;
+        if (u_entry->ovs_entry() != NULL && client_idl() != NULL) {
+            // vrf re-eval for unicast mac local is a catastrophic change
+            // trigger NotifyAddDel on ovs row.
+            client_idl()->NotifyDelAdd(u_entry->ovs_entry());
+        }
+    }
+    return true;
+}
+
 /////////////////////////////////////////////////////////////////////////////
 // Sandesh routines
 /////////////////////////////////////////////////////////////////////////////

src/vnsw/agent/ovs_tor_agent/ovsdb_client/unicast_mac_local_ovsdb.h

@@ -10,17 +10,27 @@
 class OvsPeer;
 
 namespace OVSDB {
+class UnicastMacLocalEntry;
 class UnicastMacLocalOvsdb : public OvsdbObject {
 public:
+    typedef std::pair<VrfEntry *, UnicastMacLocalEntry *> VrfDepEntry;
+    typedef std::set<VrfDepEntry> VrfDepList;
+
     UnicastMacLocalOvsdb(OvsdbClientIdl *idl, OvsPeer *peer);
     ~UnicastMacLocalOvsdb();
 
     OvsPeer *peer();
     void Notify(OvsdbClientIdl::Op op, struct ovsdb_idl_row *row);
     KSyncEntry *Alloc(const KSyncEntry *key, uint32_t index);
 
+    void VrfReEvalEnqueue(VrfEntry *vrf);
+    bool VrfReEval(VrfEntryRef vrf);
+
 private:
+    friend class UnicastMacLocalEntry;
     OvsPeer *peer_;
+    VrfDepList vrf_dep_list_;
+    WorkQueue<VrfEntryRef> *vrf_reeval_queue_;
     DISALLOW_COPY_AND_ASSIGN(UnicastMacLocalOvsdb);
 };
 
@@ -47,9 +57,9 @@ class UnicastMacLocalEntry : public OvsdbEntry {
     std::string mac_;
     std::string logical_switch_name_;
     std::string dest_ip_;
-    // we don't need to hold reference to vrf since this pointer is valid,
-    // only once route is exported and will become invalid once its removed.
-    VrfEntry *vrf_;
+    // take reference to the vrf while exporting route, to assure sanity
+    // of vrf pointer even if Add route request fails, due to any reason
+    VrfEntryRef vrf_;
     uint32_t vxlan_id_;
     DISALLOW_COPY_AND_ASSIGN(UnicastMacLocalEntry);
 };

src/vnsw/agent/ovs_tor_agent/ovsdb_client/vlan_port_binding_ovsdb.cc

@@ -254,6 +254,15 @@ KSyncDBObject::DBFilterResp VlanPortBindingTable::DBEntryFilter(
         // Ignore entries other than VLanLogicalInterface.
         return DBFilterIgnore;
     }
+
+    // Logical interface without vm interface is incomplete entry
+    // for ovsdb, trigger delete.
+    if (l_port->vm_interface() == NULL) {
+        OVSDB_TRACE(Trace, "VM Interface Unavialable, Deleting Logical "
+                    "Port " + l_port->name());
+        return DBFilterDelete;
+    }
+
     // Since we need physical port name and device name as key, ignore entry
     // if physical port or device is not yet present.
     RemotePhysicalInterface *phy_intf = dynamic_cast<RemotePhysicalInterface *>

src/vnsw/agent/ovs_tor_agent/ovsdb_client/vn_ovsdb.cc

@@ -5,11 +5,14 @@
 extern "C" {
 #include <ovsdb_wrapper.h>
 };
-#include <vn_ovsdb.h>
 
 #include <oper/vn.h>
+#include <oper/vrf.h>
 #include <ovsdb_types.h>
 
+#include <unicast_mac_local_ovsdb.h>
+#include <vn_ovsdb.h>
+
 using OVSDB::OvsdbDBEntry;
 using OVSDB::VnOvsdbEntry;
 using OVSDB::VnOvsdbObject;
@@ -19,12 +22,21 @@ VnOvsdbEntry::VnOvsdbEntry(VnOvsdbObject *table,
 }
 
 void VnOvsdbEntry::AddMsg(struct ovsdb_idl_txn *txn) {
+    VnEntry *vn = static_cast<VnEntry *>(GetDBEntry());
+    vrf_ = vn->GetVrf();
 }
 
 void VnOvsdbEntry::ChangeMsg(struct ovsdb_idl_txn *txn) {
 }
 
 void VnOvsdbEntry::DeleteMsg(struct ovsdb_idl_txn *txn) {
+    UnicastMacLocalOvsdb *uc_obj =
+        table_->client_idl()->unicast_mac_local_ovsdb();
+    // Entries in Unicast Mac Local Table are dependent on vn/vrf
+    // on delete of this entry trigger Vrf re-eval for entries in
+    // Unicast Mac Local Table
+    uc_obj->VrfReEvalEnqueue(vrf_.get());
+    vrf_ = NULL;
 }
 
 bool VnOvsdbEntry::Sync(DBEntry *db_entry) {

src/vnsw/agent/ovs_tor_agent/ovsdb_client/vn_ovsdb.h

@@ -44,6 +44,7 @@ class VnOvsdbEntry : public OvsdbDBEntry {
 private:
     friend class VnOvsdbObject;
     boost::uuids::uuid uuid_;
+    VrfEntryRef vrf_;
     DISALLOW_COPY_AND_ASSIGN(VnOvsdbEntry);
 };
 };