Skip to content

Commit

Permalink
Merge "Fix ToR-Agent crash on withdrawing HA stale route" into R2.21.x
Browse files Browse the repository at this point in the history
  • Loading branch information
@opencontrail-ci-admin
Zuul authored and opencontrail-ci-admin committed Nov 4, 2015
2 parents e502cec + a3bf35a commit 4773e6e
Show file tree
Hide file tree
Showing 3 changed files with 89 additions and 27 deletions.
82 changes: 67 additions & 15 deletions src/vnsw/agent/ovs_tor_agent/ovsdb_client/ha_stale_l2_route.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,39 @@ HaStaleL2RouteEntry::HaStaleL2RouteEntry(HaStaleL2RouteTable *table,
HaStaleL2RouteEntry::~HaStaleL2RouteEntry() {
}

void HaStaleL2RouteEntry::AddMsg(struct ovsdb_idl_txn *txn) {
// Add/Change/Delete of 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.
// Enqueue entry to export queue to process Asynchronously
// Additionaly hold the entry for Ack to assure KSync State
// Machine won't let the peer get deleted till all the route
// operations are complete
bool HaStaleL2RouteEntry::Add() {
HaStaleL2RouteTable *table =
static_cast<HaStaleL2RouteTable *>(table_);
ack_event_ = KSyncEntry::ADD_ACK;
table->EnqueueExportEntry(this);
return false;
}

bool HaStaleL2RouteEntry::Change() {
HaStaleL2RouteTable *table =
static_cast<HaStaleL2RouteTable *>(table_);
ack_event_ = KSyncEntry::CHANGE_ACK;
table->EnqueueExportEntry(this);
return false;
}

bool HaStaleL2RouteEntry::Delete() {
HaStaleL2RouteTable *table =
static_cast<HaStaleL2RouteTable *>(table_);
ack_event_ = KSyncEntry::DEL_ACK;
table->EnqueueExportEntry(this);
return false;
}

void HaStaleL2RouteEntry::AddEvent() {
HaStaleL2RouteTable *table =
static_cast<HaStaleL2RouteTable *>(table_);
if (table->vn_name_.empty()) {
Expand All @@ -52,13 +84,9 @@ 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_, true);
table->dev_ip_);
OVSDB_TRACE(Trace, std::string("Adding Ha Stale route vrf ") +
table->vrf_->GetName() + ", VxlanId " +
integerToString(vxlan_id_) + ", MAC " + mac_ + ", dest ip " +
Expand All @@ -73,32 +101,28 @@ void HaStaleL2RouteEntry::AddMsg(struct ovsdb_idl_txn *txn) {
}
}

void HaStaleL2RouteEntry::ChangeMsg(struct ovsdb_idl_txn *txn) {
void HaStaleL2RouteEntry::ChangeEvent() {
HaStaleL2RouteTable *table =
static_cast<HaStaleL2RouteTable *>(table_);
if (vxlan_id_ != 0 && vxlan_id_ != table->vxlan_id_) {
// for change in vxlan id delete the previously exported
// route before calling AddMsg
DeleteMsg(txn);
DeleteEvent();
vxlan_id_ = 0;
}
AddMsg(txn);
AddEvent();
}

void HaStaleL2RouteEntry::DeleteMsg(struct ovsdb_idl_txn *txn) {
void HaStaleL2RouteEntry::DeleteEvent() {
HaStaleL2RouteTable *table =
static_cast<HaStaleL2RouteTable *>(table_);
HaStaleDevVnEntry *dev_vn =
static_cast<HaStaleDevVnEntry *>(table->dev_vn_ref_.get());
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_), true);
MacAddress(mac_));

if (time_stamp_ != 0) {
HaStaleDevVnTable *dev_vn_table =
Expand Down Expand Up @@ -180,12 +204,17 @@ HaStaleL2RouteTable::HaStaleL2RouteTable(
static_cast<HaStaleDevVnTable*>(dev_vn->table());
HaStaleDevVnEntry dev_key(dev_table, boost::uuids::nil_uuid());
dev_vn_ref_ = dev_table->GetReference(&dev_key);
route_export_queue_ = new WorkQueue<HaStaleL2RouteEntry *>(
TaskScheduler::GetInstance()->GetTaskId("Agent::KSync"), 0,
boost::bind(&HaStaleL2RouteTable::ProcessExportEntry, this, _1));
OvsdbRegisterDBTable(table);
}

HaStaleL2RouteTable::~HaStaleL2RouteTable() {
// Table unregister will be done by Destructor of KSyncDBObject
table_delete_ref_.Reset(NULL);
route_export_queue_->Shutdown();
delete route_export_queue_;
}

KSyncEntry *HaStaleL2RouteTable::Alloc(const KSyncEntry *key, uint32_t index) {
Expand Down Expand Up @@ -310,6 +339,29 @@ void HaStaleL2RouteTable::UpdateParams(HaStaleDevVnEntry *dev_vn) {
}
}

void HaStaleL2RouteTable::EnqueueExportEntry(HaStaleL2RouteEntry *entry) {
route_export_queue_->Enqueue(entry);
}

bool HaStaleL2RouteTable::ProcessExportEntry(HaStaleL2RouteEntry *entry) {
KSyncEntry::KSyncEvent ack_event = entry->ack_event();
switch (ack_event) {
case KSyncEntry::ADD_ACK:
entry->AddEvent();
break;
case KSyncEntry::CHANGE_ACK:
entry->ChangeEvent();
break;
case KSyncEntry::DEL_ACK:
entry->DeleteEvent();
break;
default:
assert(false);
}
SafeNotifyEvent(entry, ack_event);
return true;
}

/////////////////////////////////////////////////////////////////////////////
// Sandesh routines
/////////////////////////////////////////////////////////////////////////////
Expand Down
20 changes: 14 additions & 6 deletions src/vnsw/agent/ovs_tor_agent/ovsdb_client/ha_stale_l2_route.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,12 @@ class HaStaleL2RouteTable : public OvsdbDBObject {
void OvsdbNotify(OvsdbClientIdl::Op, struct ovsdb_idl_row *) {}
OvsdbDBEntry *AllocOvsEntry(struct ovsdb_idl_row *row) { return NULL; }

// Enqueue entry to route export queue
void EnqueueExportEntry(HaStaleL2RouteEntry *entry);

// API to process route export queue entries
bool ProcessExportEntry(HaStaleL2RouteEntry *entry);

LifetimeRef<HaStaleL2RouteTable> table_delete_ref_;
// take reference to a dummy dev_vn entry to hold peer till
// route table is deleted. route object cannot hold reference
Expand All @@ -56,6 +62,7 @@ class HaStaleL2RouteTable : public OvsdbDBObject {
// of vrf pointer even if Add route request fails, due to any reason
VrfEntryRef vrf_;
std::string vn_name_;
WorkQueue<HaStaleL2RouteEntry *> *route_export_queue_;
DISALLOW_COPY_AND_ASSIGN(HaStaleL2RouteTable);
};

Expand All @@ -65,9 +72,9 @@ class HaStaleL2RouteEntry : public OvsdbDBEntry {
const std::string &mac);
~HaStaleL2RouteEntry();

void AddMsg(struct ovsdb_idl_txn *);
void ChangeMsg(struct ovsdb_idl_txn *);
void DeleteMsg(struct ovsdb_idl_txn *);
bool Add();
bool Change();
bool Delete();

bool Sync(DBEntry*);
bool IsLess(const KSyncEntry&) const;
Expand All @@ -78,15 +85,16 @@ class HaStaleL2RouteEntry : public OvsdbDBEntry {
uint32_t vxlan_id() const;
bool IsStale() const;

protected:
virtual bool IsNoTxnEntry() { return true; }

private:
friend class HaStaleL2RouteTable;
friend class VrfRouteReflectorTable;

void StaleClearCb();

void AddEvent();
void ChangeEvent();
void DeleteEvent();

std::string mac_;
uint32_t path_preference_;
uint32_t vxlan_id_;
Expand Down
14 changes: 8 additions & 6 deletions src/vnsw/agent/ovs_tor_agent/ovsdb_client/ovsdb_entry.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,8 @@ class OvsdbEntryBase {
public:
virtual void Ack(bool success) = 0;

KSyncEntry::KSyncEvent ack_event() {return ack_event_;}

protected:
friend class OvsdbClientIdl;
KSyncEntry::KSyncEvent ack_event_;
Expand Down Expand Up @@ -61,21 +63,21 @@ class OvsdbDBEntry : public KSyncDBEntry, public OvsdbEntryBase {
// after we are done with the current transaction
virtual void PostDelete() {}
// Encode add message for entry
virtual void AddMsg(struct ovsdb_idl_txn *) = 0;
virtual void AddMsg(struct ovsdb_idl_txn *) {}
// Encode change message for entry
virtual void ChangeMsg(struct ovsdb_idl_txn *) = 0;
virtual void ChangeMsg(struct ovsdb_idl_txn *) {}
// Encode delete message for entry
virtual void DeleteMsg(struct ovsdb_idl_txn *) = 0;
virtual void DeleteMsg(struct ovsdb_idl_txn *) {}

virtual void OvsdbChange() {}

bool AllowDeleteStateComp() {return false;}
virtual void NotifyAdd(struct ovsdb_idl_row *);
virtual void NotifyDelete(struct ovsdb_idl_row *);

bool Add();
bool Change();
bool Delete();
virtual bool Add();
virtual bool Change();
virtual bool Delete();

virtual bool IsDataResolved();
bool IsDelAckWaiting();
Expand Down

0 comments on commit 4773e6e

Please sign in to comment.