Merge "Fix HA Stale route withdraw from Active ToR Agent" into R2.20

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

@@ -121,6 +121,10 @@ KSyncEntry* HaStaleDevVnEntry::UnresolvedReference() {
         oper_bridge_table_ = entry->bridge_table();
     }
 
+    if (vn_name_.empty()) {
+        // update vn name from vn if available
+        vn_name_ = entry->vn_name();
+    }
     return NULL;
 }
 
@@ -165,7 +169,7 @@ HaStaleDevVnTable::HaStaleDevVnTable(Agent *agent,
         std::string &dev_name) :
     OvsdbDBObject(NULL, false), agent_(agent), manager_(manager),
     dev_name_(dev_name), state_(state),
-    vn_table_(new HaStaleVnTable(agent, this)), time_stamp_(0),
+    vn_table_(new HaStaleVnTable(agent, this)), time_stamp_(1),
     stale_clear_timer_(TimerManager::CreateTimer(
                 *(agent->event_manager())->io_service(),
                 "OVSDB Route Replicator cleanup timer",
@@ -177,8 +181,6 @@ HaStaleDevVnTable::HaStaleDevVnTable(Agent *agent,
     route_peer_.reset(manager->Allocate(zero_ip));
     route_peer_->set_ha_stale_export(true);
     OvsdbRegisterDBTable((DBTable *)agent->physical_device_vn_table());
-    stale_clear_timer_->Start(kStaleTimerJobInterval,
-            boost::bind(&HaStaleDevVnTable::StaleClearTimerCb, this));
 }
 
 HaStaleDevVnTable::~HaStaleDevVnTable() {
@@ -277,6 +279,11 @@ ConnectionStateEntry *HaStaleDevVnTable::state() const {
 void HaStaleDevVnTable::StaleClearAddEntry(uint64_t time_stamp,
                                            HaStaleL2RouteEntry *entry,
                                            StaleClearL2EntryCb cb) {
+    if (stale_l2_entry_map_.empty()) {
+        // start the timer while adding the first entry
+        stale_clear_timer_->Start(kStaleTimerJobInterval,
+                boost::bind(&HaStaleDevVnTable::StaleClearTimerCb, this));
+    }
     StaleL2Entry l2_entry(time_stamp, entry);
     stale_l2_entry_map_[l2_entry] = cb;
 }
@@ -285,6 +292,10 @@ void HaStaleDevVnTable::StaleClearDelEntry(uint64_t time_stamp,
                                            HaStaleL2RouteEntry *entry) {
     StaleL2Entry l2_entry(time_stamp, entry);
     stale_l2_entry_map_.erase(l2_entry);
+    if (stale_l2_entry_map_.empty()) {
+        // stop the timer on last entry removal
+        stale_clear_timer_->Cancel();
+    }
 }
 
 bool HaStaleDevVnTable::StaleClearTimerCb() {
@@ -303,6 +314,10 @@ bool HaStaleDevVnTable::StaleClearTimerCb() {
         count++;
     }
 
+    if (stale_l2_entry_map_.empty()) {
+        // do not restart the timer if all entries are removed
+        return false;
+    }
     return true;
 }
 

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

@@ -81,6 +81,8 @@ class HaStaleDevVnTable : public OvsdbDBObject {
     HaStaleVnTable *vn_table_;
     // entries for which the timer is running to clear stale entries
     CbMap stale_l2_entry_map_;
+    // time stamp represented by iteration count of interval
+    // kStaleTimerJobInterval should always be non-zero and start with 1
     uint64_t time_stamp_;
     Timer *stale_clear_timer_;
     // reeval queue used to trigger updates on change in VN-VRF link

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

@@ -52,9 +52,13 @@ void HaStaleL2RouteEntry::AddMsg(struct ovsdb_idl_txn *txn) {
     HaStaleDevVnEntry *dev_vn =
         static_cast<HaStaleDevVnEntry *>(table->dev_vn_ref_.get());
     vxlan_id_ = table->vxlan_id_;
+    // Add route path should be always be an async operation as this
+    // callback will be triggered as part of DB Notification, calling a
+    // DB operation inline may suppress change notification for  other
+    // DB Table clients.
     dev_vn->route_peer()->AddOvsRoute(table->vrf_.get(), table->vxlan_id_,
                                       table->vn_name_, MacAddress(mac_),
-                                      table->dev_ip_);
+                                      table->dev_ip_, true);
     OVSDB_TRACE(Trace, std::string("Adding Ha Stale route vrf ") +
                 table->vrf_->GetName() + ", VxlanId " +
                 integerToString(vxlan_id_) + ", MAC " + mac_ + ", dest ip " +
@@ -89,8 +93,12 @@ void HaStaleL2RouteEntry::DeleteMsg(struct ovsdb_idl_txn *txn) {
     OVSDB_TRACE(Trace, std::string("withdrawing Ha Stale route vrf ") +
                 table->vrf_->GetName() + ", VxlanId " +
                 integerToString(vxlan_id_) + ", MAC " + mac_);
+    // Delete route path should be always be an async operation as this
+    // callback will be triggered as part of DB Notification, calling a
+    // DB operation inline may suppress change/delete notification for
+    // other DB Table clients.
     dev_vn->route_peer()->DeleteOvsRoute(table->vrf_.get(), vxlan_id_,
-                                         MacAddress(mac_));
+                                         MacAddress(mac_), true);
 
     if (time_stamp_ != 0) {
         HaStaleDevVnTable *dev_vn_table =

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

@@ -17,13 +17,23 @@ extern "C" {
 using namespace OVSDB;
 
 HaStaleVnEntry::HaStaleVnEntry(HaStaleVnTable *table,
-        const boost::uuids::uuid &uuid) : OvsdbDBEntry(table), uuid_(uuid) {
+        const boost::uuids::uuid &uuid) : OvsdbDBEntry(table), uuid_(uuid),
+        vn_name_(""), bridge_table_(NULL) {
+}
+
+HaStaleVnEntry::~HaStaleVnEntry() {
 }
 
 void HaStaleVnEntry::AddMsg(struct ovsdb_idl_txn *txn) {
+    HaStaleVnTable *table = static_cast<HaStaleVnTable *>(table_);
+    if (table->dev_vn_table_ != NULL) {
+        // trigger VnReEval for Any Change
+        table->dev_vn_table_->VnReEvalEnqueue(uuid_);
+    }
 }
 
 void HaStaleVnEntry::ChangeMsg(struct ovsdb_idl_txn *txn) {
+    AddMsg(txn);
 }
 
 void HaStaleVnEntry::DeleteMsg(struct ovsdb_idl_txn *txn) {
@@ -35,17 +45,16 @@ void HaStaleVnEntry::DeleteMsg(struct ovsdb_idl_txn *txn) {
 
 bool HaStaleVnEntry::Sync(DBEntry *db_entry) {
     VnEntry *vn = static_cast<VnEntry *>(db_entry);
+    bool change = false;
+    if (vn_name_ != vn->GetName()) {
+        vn_name_ = vn->GetName();
+        change = true;
+    }
     if (bridge_table_ != vn->GetVrf()->GetBridgeRouteTable()) {
-        // trigger VnReEval for Change in Vrf pointer
-        // avoid reeval while initial add
-        HaStaleVnTable *table = static_cast<HaStaleVnTable *>(table_);
-        if (bridge_table_ == NULL && table->dev_vn_table_ != NULL) {
-            table->dev_vn_table_->VnReEvalEnqueue(uuid_);
-        }
         bridge_table_ = vn->GetVrf()->GetBridgeRouteTable();
-        return true;
+        change = true;
     }
-    return false;
+    return change;
 }
 
 bool HaStaleVnEntry::IsLess(const KSyncEntry &entry) const {
@@ -57,6 +66,10 @@ KSyncEntry *HaStaleVnEntry::UnresolvedReference() {
     return NULL;
 }
 
+const std::string &HaStaleVnEntry::vn_name() const {
+    return vn_name_;
+}
+
 AgentRouteTable *HaStaleVnEntry::bridge_table() const {
     return bridge_table_;
 }

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

@@ -40,6 +40,7 @@ class HaStaleVnTable : public OvsdbDBObject {
 class HaStaleVnEntry : public OvsdbDBEntry {
 public:
     HaStaleVnEntry(HaStaleVnTable *table, const boost::uuids::uuid &uuid);
+    virtual ~HaStaleVnEntry();
 
     // ovs_entry ref is not valid for VN, override IsDataResolved
     // to return always true
@@ -52,6 +53,7 @@ class HaStaleVnEntry : public OvsdbDBEntry {
     std::string ToString() const {return "Ha Stale VN entry";}
     KSyncEntry* UnresolvedReference();
 
+    const std::string &vn_name() const;
     AgentRouteTable *bridge_table() const;
 
 protected:
@@ -60,6 +62,7 @@ class HaStaleVnEntry : public OvsdbDBEntry {
 private:
     friend class HaStaleVnTable;
     boost::uuids::uuid uuid_;
+    std::string vn_name_;
     AgentRouteTable *bridge_table_;
     DISALLOW_COPY_AND_ASSIGN(HaStaleVnEntry);
 };

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

@@ -34,7 +34,7 @@ bool ConnectionStateEntry::IsConnectionActive() {
         // if there is any non deleted IDL available
         // return true
         if (!(*it)->IsDeleted()) {
-            return false;
+            return true;
         }
     }