Skip to content

Commit

Permalink
Intrusive pointer back reference infra
Browse files Browse the repository at this point in the history
Changes to introduce intrusive pointer back reference
infra to manage back reference pointers of the objects
hold intrusive pointer reference.
incorporating use of this infra for VRF intrusive ptr
references to have better debug ability for VRF delete
timeout crash

Change-Id: I10d27af5a1f86067279ef788a8c99003dbc989f0
Closes-Bug: 1484796
  • Loading branch information
Prabhjot Singh Sethi committed Aug 21, 2015
1 parent 680cd2c commit a0716c7
Show file tree
Hide file tree
Showing 19 changed files with 156 additions and 41 deletions.
94 changes: 94 additions & 0 deletions src/base/intrusive_ptr_back_ref.h
Original file line number Diff line number Diff line change
@@ -0,0 +1,94 @@
/*
* Copyright (c) 2015 Juniper Networks, Inc. All rights reserved.
*/

#ifndef SRC_BASE_INTRUSIVE_PTR_BACK_REF_H_
#define SRC_BASE_INTRUSIVE_PTR_BACK_REF_H_

#include <boost/intrusive_ptr.hpp>

// IntrusivePtrRef Class is an extension to boost::intrusive_ptr
// to provide additional functionality of making back reference
// pointer available with callbacks intrusive_ptr_add_back_ref
// and intrusive_ptr_del_back_ref, using which application using
// IntrusivePtrRef can keep track of pointers who holds references
// to the object

// referrer, pair of base pointer and member pointer
typedef std::pair<void*, void*> IntrusiveReferrer;

template <class D>
class IntrusivePtrRef : public boost::intrusive_ptr<D> {
public:
IntrusivePtrRef() : boost::intrusive_ptr<D>(), referrer_(NULL) {
}

IntrusivePtrRef(D *data) : boost::intrusive_ptr<D>(data), referrer_(NULL) {
if (data) {
intrusive_ptr_add_back_ref(IntrusiveReferrer(referrer_, this), data);
}
}

IntrusivePtrRef(D *data, void *referrer) : boost::intrusive_ptr<D>(data),
referrer_(referrer) {
if (data) {
intrusive_ptr_add_back_ref(IntrusiveReferrer(referrer, this), data);
}
}

IntrusivePtrRef(IntrusivePtrRef const &rhs, void *referrer)
: boost::intrusive_ptr<D>(rhs), referrer_(referrer) {
if (this->get()) {
intrusive_ptr_add_back_ref(IntrusiveReferrer(referrer, this),
this->get());
}
}

virtual ~IntrusivePtrRef() {
if (this->get()) {
intrusive_ptr_del_back_ref(IntrusiveReferrer(referrer_, this), this->get());
}
}

IntrusivePtrRef & operator=(IntrusivePtrRef const &rhs) {
IntrusivePtrRef(rhs, this).swap(*this);
return *this;
}

IntrusivePtrRef & operator=(D *rhs) {
IntrusivePtrRef(rhs, this).swap(*this);
return *this;
}

void reset() {
IntrusivePtrRef(NULL, this).swap(*this);
}

void reset(D *rhs) {
IntrusivePtrRef(rhs, this).swap(*this);
}

void swap(IntrusivePtrRef &rhs) {
// Trigger Base class swap to swap values
boost::intrusive_ptr<D>::swap(rhs);

// swap the referrer for this and rhs
D *tmp = this->get();
if (tmp) {
// change referrer for new value of IntrusivePtrRef
intrusive_ptr_add_back_ref(IntrusiveReferrer(referrer_, this), tmp);
intrusive_ptr_del_back_ref(IntrusiveReferrer(rhs.referrer_, &rhs), tmp);
}
tmp = rhs.get();
if (tmp) {
// change referrer for new value of rhs
intrusive_ptr_add_back_ref(IntrusiveReferrer(rhs.referrer_, &rhs), tmp);
intrusive_ptr_del_back_ref(IntrusiveReferrer(referrer_, this), tmp);
}
}

private:
void *referrer_;
};

#endif // SRC_BASE_INTRUSIVE_PTR_BACK_REF_H_
7 changes: 5 additions & 2 deletions src/vnsw/agent/cmn/agent.h
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@
#include <stdint.h>
#include <net/ethernet.h>
#include <boost/intrusive_ptr.hpp>
#include <base/intrusive_ptr_back_ref.h>
#include <cmn/agent_cmn.h>
#include <base/connection_info.h>
#include "net/mac_address.h"
Expand Down Expand Up @@ -62,8 +63,10 @@ void intrusive_ptr_release(const SgEntry* p);
void intrusive_ptr_add_ref(const SgEntry* p);

class VrfEntry;
typedef boost::intrusive_ptr<VrfEntry> VrfEntryRef;
typedef boost::intrusive_ptr<const VrfEntry> VrfEntryConstRef;
typedef IntrusivePtrRef<VrfEntry> VrfEntryRef;
typedef IntrusivePtrRef<const VrfEntry> VrfEntryConstRef;
void intrusive_ptr_add_back_ref(const IntrusiveReferrer ref, const VrfEntry* p);
void intrusive_ptr_del_back_ref(const IntrusiveReferrer ref, const VrfEntry* p);
void intrusive_ptr_release(const VrfEntry* p);
void intrusive_ptr_add_ref(const VrfEntry* p);

Expand Down
16 changes: 16 additions & 0 deletions src/vnsw/agent/cmn/agent_db.h
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,20 @@ class AgentRefCount {
}
}

friend void intrusive_ptr_add_back_ref(const IntrusiveReferrer ref,
const Derived* p) {
const AgentRefCount *entry = (const AgentRefCount *) (p);
tbb::mutex::scoped_lock lock(entry->mutex_);
entry->back_ref_set_.insert(ref);
}

friend void intrusive_ptr_del_back_ref(const IntrusiveReferrer ref,
const Derived* p) {
const AgentRefCount *entry = (const AgentRefCount *) (p);
tbb::mutex::scoped_lock lock(entry->mutex_);
entry->back_ref_set_.erase(ref);
}

uint32_t GetRefCount() const {return refcount_;};
protected:
AgentRefCount() {refcount_ = 0;}
Expand All @@ -44,6 +58,8 @@ class AgentRefCount {
virtual ~AgentRefCount() {assert(refcount_ == 0);};
void swap(AgentRefCount&) {};
private:
mutable tbb::mutex mutex_;
mutable std::set<IntrusiveReferrer> back_ref_set_;
mutable tbb::atomic<uint32_t> refcount_;
};

Expand Down
2 changes: 1 addition & 1 deletion src/vnsw/agent/oper/agent_route.cc
Original file line number Diff line number Diff line change
Expand Up @@ -63,7 +63,7 @@ bool NHComparator::operator() (const NextHop *nh1, const NextHop *nh2) {
}

AgentRouteTable::AgentRouteTable(DB *db, const std::string &name):
RouteTable(db, name), agent_(NULL), deleter_(NULL),
RouteTable(db, name), agent_(NULL), vrf_entry_(NULL, this), deleter_(NULL),
vrf_delete_ref_(this, NULL) {
}

Expand Down
2 changes: 1 addition & 1 deletion src/vnsw/agent/oper/agent_route.h
Original file line number Diff line number Diff line change
Expand Up @@ -218,7 +218,7 @@ class AgentRoute : public Route {
typedef DependencyList<NextHop, AgentRoute> TunnelNhDependencyList;

AgentRoute(VrfEntry *vrf, bool is_multicast) :
Route(), vrf_(vrf), is_multicast_(is_multicast) { }
Route(), vrf_(vrf, this), is_multicast_(is_multicast) { }
virtual ~AgentRoute() { }

// Virtual functions from base DBEntry
Expand Down
2 changes: 1 addition & 1 deletion src/vnsw/agent/oper/agent_route_walker.h
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,7 @@ struct AgentRouteWalkerQueueEntry {

AgentRouteWalkerQueueEntry(VrfEntry *vrf, RequestType type,
bool all_walks_done) :
vrf_ref_(vrf), type_(type), all_walks_done_(all_walks_done) { }
vrf_ref_(vrf, this), type_(type), all_walks_done_(all_walks_done) { }
virtual ~AgentRouteWalkerQueueEntry() { }

VrfEntryRef vrf_ref_;
Expand Down
2 changes: 1 addition & 1 deletion src/vnsw/agent/oper/interface.cc
Original file line number Diff line number Diff line change
Expand Up @@ -320,7 +320,7 @@ void InterfaceTable::GlobalVrouterConfigChanged() {
Interface::Interface(Type type, const uuid &uuid, const string &name,
VrfEntry *vrf) :
type_(type), uuid_(uuid), name_(name),
vrf_(vrf), label_(MplsTable::kInvalidLabel),
vrf_(vrf, this), label_(MplsTable::kInvalidLabel),
l2_label_(MplsTable::kInvalidLabel), ipv4_active_(true),
ipv6_active_(false), l2_active_(true), id_(kInvalidIndex),
dhcp_enabled_(true), dns_enabled_(true), mac_(), os_index_(kInvalidIndex),
Expand Down
2 changes: 1 addition & 1 deletion src/vnsw/agent/oper/mirror_table.h
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ struct MirrorEntryData : public AgentData {
class MirrorEntry : AgentRefCount<MirrorEntry>, public AgentDBEntry {
public:
MirrorEntry(std::string analyzer_name) :
analyzer_name_(analyzer_name), vrf_(NULL), nh_(NULL) { };
analyzer_name_(analyzer_name), vrf_(NULL, this), nh_(NULL) { };
virtual ~MirrorEntry() { };

virtual bool IsLess(const DBEntry &rhs) const;
Expand Down
2 changes: 1 addition & 1 deletion src/vnsw/agent/oper/nexthop.cc
Original file line number Diff line number Diff line change
Expand Up @@ -693,7 +693,7 @@ void VrfNH::SendObjectLog(AgentLogEvent::type event) const {
/////////////////////////////////////////////////////////////////////////////
TunnelNH::TunnelNH(VrfEntry *vrf, const Ip4Address &sip, const Ip4Address &dip,
bool policy, TunnelType type) :
NextHop(NextHop::TUNNEL, false, policy), vrf_(vrf), sip_(sip),
NextHop(NextHop::TUNNEL, false, policy), vrf_(vrf, this), sip_(sip),
dip_(dip), tunnel_type_(type), arp_rt_(this), interface_(NULL), dmac_() {
}

Expand Down
12 changes: 6 additions & 6 deletions src/vnsw/agent/oper/nexthop.h
Original file line number Diff line number Diff line change
Expand Up @@ -708,7 +708,7 @@ class ArpNHData : public NextHopData {
class ArpNH : public NextHop {
public:
ArpNH(VrfEntry *vrf, const Ip4Address &ip) :
NextHop(ARP, false, false), vrf_(vrf), ip_(ip), interface_(), mac_() {};
NextHop(ARP, false, false), vrf_(vrf, this), ip_(ip), interface_(), mac_() {};
virtual ~ArpNH() { };

virtual std::string ToString() { return "ARP"; }
Expand Down Expand Up @@ -865,10 +865,10 @@ class InterfaceNH : public NextHop {
public:
InterfaceNH(Interface *intf, bool policy, uint8_t flags) :
NextHop(INTERFACE, true, policy), interface_(intf),
flags_(flags), dmac_() { };
flags_(flags), dmac_(), vrf_(NULL, this) { };
InterfaceNH(Interface *intf, bool policy) :
NextHop(INTERFACE, true, policy), interface_(intf),
flags_(InterfaceNHFlags::INET4), dmac_() { };
flags_(InterfaceNHFlags::INET4), dmac_(), vrf_(NULL, this) { };
virtual ~InterfaceNH() { };

virtual std::string ToString() const {
Expand Down Expand Up @@ -964,7 +964,7 @@ class VrfNHData : public NextHopData {
class VrfNH : public NextHop {
public:
VrfNH(VrfEntry *vrf, bool policy, bool vxlan_nh_):
NextHop(VRF, true, policy), vrf_(vrf), vxlan_nh_(vxlan_nh_),
NextHop(VRF, true, policy), vrf_(vrf, this), vxlan_nh_(vxlan_nh_),
flood_unknown_unicast_(false) {}
virtual ~VrfNH() { };

Expand Down Expand Up @@ -1049,7 +1049,7 @@ class VlanNH : public NextHop {
public:
VlanNH(Interface *intf, uint32_t vlan_tag):
NextHop(VLAN, true, false), interface_(intf), vlan_tag_(vlan_tag),
smac_(), dmac_(), vrf_(NULL) { };
smac_(), dmac_(), vrf_(NULL, this) { };
virtual ~VlanNH() { };

bool NextHopIsLess(const DBEntry &rhs) const;
Expand Down Expand Up @@ -1259,7 +1259,7 @@ class CompositeNH : public NextHop {
CompositeNH(COMPOSITETYPE type, bool policy,
const ComponentNHKeyList &component_nh_key_list, VrfEntry *vrf):
NextHop(COMPOSITE, policy), composite_nh_type_(type),
component_nh_key_list_(component_nh_key_list), vrf_(vrf) {
component_nh_key_list_(component_nh_key_list), vrf_(vrf, this) {
}

virtual ~CompositeNH() { };
Expand Down
2 changes: 1 addition & 1 deletion src/vnsw/agent/oper/tunnel_nh.h
Original file line number Diff line number Diff line change
Expand Up @@ -85,7 +85,7 @@ class MirrorNH : public NextHop {
public:
MirrorNH(VrfEntry *vrf, const Ip4Address &sip, uint16_t sport,
const Ip4Address &dip, uint16_t dport):
NextHop(NextHop::MIRROR, false, false), vrf_(vrf), sip_(sip),
NextHop(NextHop::MIRROR, false, false), vrf_(vrf, this), sip_(sip),
sport_(sport), dip_(dip), dport_(dport), arp_rt_(this),
interface_(NULL), dmac_(){ };
virtual ~MirrorNH() { };
Expand Down
26 changes: 13 additions & 13 deletions src/vnsw/agent/oper/vm_interface.cc
Original file line number Diff line number Diff line change
Expand Up @@ -3033,21 +3033,21 @@ bool VmInterface::GetIpamDhcpOptions(

VmInterface::FloatingIp::FloatingIp() :
ListEntry(), floating_ip_(), vn_(NULL),
vrf_(NULL), vrf_name_(""), vn_uuid_(), l2_installed_(false),
vrf_(NULL, this), vrf_name_(""), vn_uuid_(), l2_installed_(false),
ethernet_tag_(0) {
}

VmInterface::FloatingIp::FloatingIp(const FloatingIp &rhs) :
ListEntry(rhs.installed_, rhs.del_pending_),
floating_ip_(rhs.floating_ip_), vn_(rhs.vn_), vrf_(rhs.vrf_),
floating_ip_(rhs.floating_ip_), vn_(rhs.vn_), vrf_(rhs.vrf_, this),
vrf_name_(rhs.vrf_name_), vn_uuid_(rhs.vn_uuid_),
l2_installed_(rhs.l2_installed_), ethernet_tag_(rhs.ethernet_tag_) {
}

VmInterface::FloatingIp::FloatingIp(const IpAddress &addr,
const std::string &vrf,
const boost::uuids::uuid &vn_uuid) :
ListEntry(), floating_ip_(addr), vn_(NULL), vrf_(NULL), vrf_name_(vrf),
ListEntry(), floating_ip_(addr), vn_(NULL), vrf_(NULL, this), vrf_name_(vrf),
vn_uuid_(vn_uuid), l2_installed_(false), ethernet_tag_(0) {
}

Expand Down Expand Up @@ -3304,23 +3304,23 @@ void VmInterface::StaticRouteList::Remove(StaticRouteSet::iterator &it) {
///////////////////////////////////////////////////////////////////////////////
VmInterface::AllowedAddressPair::AllowedAddressPair() :
ListEntry(), vrf_(""), addr_(0), plen_(0), ecmp_(false), mac_(),
l2_entry_installed_(false), ethernet_tag_(0), vrf_ref_(NULL), gw_ip_(0) {
l2_entry_installed_(false), ethernet_tag_(0), vrf_ref_(NULL, this), gw_ip_(0) {
}

VmInterface::AllowedAddressPair::AllowedAddressPair(
const AllowedAddressPair &rhs) : ListEntry(rhs.installed_,
rhs.del_pending_), vrf_(rhs.vrf_), addr_(rhs.addr_), plen_(rhs.plen_),
ecmp_(rhs.ecmp_), mac_(rhs.mac_),
l2_entry_installed_(rhs.l2_entry_installed_), ethernet_tag_(rhs.ethernet_tag_),
vrf_ref_(rhs.vrf_ref_), gw_ip_(rhs.gw_ip_) {
vrf_ref_(rhs.vrf_ref_, this), gw_ip_(rhs.gw_ip_) {
}

VmInterface::AllowedAddressPair::AllowedAddressPair(const std::string &vrf,
const Ip4Address &addr,
uint32_t plen, bool ecmp,
const MacAddress &mac) :
ListEntry(), vrf_(vrf), addr_(addr), plen_(plen), ecmp_(ecmp), mac_(mac),
l2_entry_installed_(false), ethernet_tag_(0), vrf_ref_(NULL) {
l2_entry_installed_(false), ethernet_tag_(0), vrf_ref_(NULL, this) {
}

VmInterface::AllowedAddressPair::~AllowedAddressPair() {
Expand Down Expand Up @@ -3526,21 +3526,21 @@ void VmInterface::SecurityGroupEntryList::Remove
/////////////////////////////////////////////////////////////////////////////
VmInterface::ServiceVlan::ServiceVlan() :
ListEntry(), tag_(0), vrf_name_(""), addr_(0), plen_(32), smac_(), dmac_(),
vrf_(NULL), label_(MplsTable::kInvalidLabel) {
vrf_(NULL, this), label_(MplsTable::kInvalidLabel) {
}

VmInterface::ServiceVlan::ServiceVlan(const ServiceVlan &rhs) :
ListEntry(rhs.installed_, rhs.del_pending_), tag_(rhs.tag_),
vrf_name_(rhs.vrf_name_), addr_(rhs.addr_), plen_(rhs.plen_),
smac_(rhs.smac_), dmac_(rhs.dmac_), vrf_(rhs.vrf_), label_(rhs.label_) {
smac_(rhs.smac_), dmac_(rhs.dmac_), vrf_(rhs.vrf_, this), label_(rhs.label_) {
}

VmInterface::ServiceVlan::ServiceVlan(uint16_t tag, const std::string &vrf_name,
const Ip4Address &addr, uint8_t plen,
const MacAddress &smac,
const MacAddress &dmac) :
ListEntry(), tag_(tag), vrf_name_(vrf_name), addr_(addr), plen_(plen),
smac_(smac), dmac_(dmac), vrf_(NULL), label_(MplsTable::kInvalidLabel)
smac_(smac), dmac_(dmac), vrf_(NULL, this), label_(MplsTable::kInvalidLabel)
{
}

Expand Down Expand Up @@ -3722,21 +3722,21 @@ bool VmInterface::IsFloatingIp(const IpAddress &ip) const {
// VRF assign rule routines
////////////////////////////////////////////////////////////////////////////
VmInterface::VrfAssignRule::VrfAssignRule():
ListEntry(), id_(0), vrf_name_(" "), vrf_(NULL), ignore_acl_(false) {
ListEntry(), id_(0), vrf_name_(" "), vrf_(NULL, this), ignore_acl_(false) {
}

VmInterface::VrfAssignRule::VrfAssignRule(const VrfAssignRule &rhs):
ListEntry(rhs.installed_, rhs.del_pending_), id_(rhs.id_),
vrf_name_(rhs.vrf_name_), vrf_(rhs.vrf_), ignore_acl_(rhs.ignore_acl_),
vrf_name_(rhs.vrf_name_), vrf_(rhs.vrf_, this), ignore_acl_(rhs.ignore_acl_),
match_condition_(rhs.match_condition_) {
}

VmInterface::VrfAssignRule::VrfAssignRule(uint32_t id,
const autogen::MatchConditionType &match_condition,
const std::string &vrf_name,
bool ignore_acl):
ListEntry(), id_(id), vrf_name_(vrf_name), ignore_acl_(ignore_acl),
match_condition_(match_condition) {
ListEntry(), id_(id), vrf_name_(vrf_name), vrf_(NULL, this),
ignore_acl_(ignore_acl), match_condition_(match_condition) {
}

VmInterface::VrfAssignRule::~VrfAssignRule() {
Expand Down
9 changes: 5 additions & 4 deletions src/vnsw/agent/oper/vn.cc
Original file line number Diff line number Diff line change
Expand Up @@ -88,10 +88,11 @@ bool VnIpam::IsSubnetMember(const IpAddress &ip) const {
}

VnEntry::VnEntry(Agent *agent, uuid id) :
AgentOperDBEntry(), agent_(agent), uuid_(id), vxlan_id_(0), vnid_(0),
bridging_(true), layer3_forwarding_(true), admin_state_(true),
table_label_(0), enable_rpf_(true), flood_unknown_unicast_(false),
old_vxlan_id_(0), forwarding_mode_(Agent::L2_L3) {
AgentOperDBEntry(), agent_(agent), uuid_(id), vrf_(NULL, this),
vxlan_id_(0), vnid_(0), bridging_(true), layer3_forwarding_(true),
admin_state_(true), table_label_(0), enable_rpf_(true),
flood_unknown_unicast_(false), old_vxlan_id_(0),
forwarding_mode_(Agent::L2_L3) {
}

VnEntry::~VnEntry() {
Expand Down
2 changes: 1 addition & 1 deletion src/vnsw/agent/oper/vrf.cc
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@ class VrfEntry::DeleteActor : public LifetimeActor {
public:
DeleteActor(VrfEntry *vrf) :
LifetimeActor((static_cast<VrfTable *>(vrf->get_table()))->
agent()->lifetime_manager()), table_(vrf) {
agent()->lifetime_manager()), table_(vrf, this) {
}
virtual ~DeleteActor() {
}
Expand Down
2 changes: 1 addition & 1 deletion src/vnsw/agent/oper/vrf_assign.h
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,7 @@ class VrfAssign : AgentRefCount<VrfAssign>, public AgentDBEntry {
};

VrfAssign(Type type, Interface *interface)
: type_(type), interface_(interface), vrf_(NULL) { };
: type_(type), interface_(interface), vrf_(NULL, this) { };
virtual ~VrfAssign() { };

uint32_t GetRefCount() const {
Expand Down

0 comments on commit a0716c7

Please sign in to comment.