Skip to content

Commit

Permalink
Merge "Fix access to DBEntry after its deleted"
Browse files Browse the repository at this point in the history
  • Loading branch information
@opencontrail-ci-admin
Zuul authored and opencontrail-ci-admin committed Mar 14, 2016
2 parents 0c46e09 + b9305b4 commit 133100e
Show file tree
Hide file tree
Showing 8 changed files with 165 additions and 24 deletions.
17 changes: 17 additions & 0 deletions src/vnsw/agent/pkt/flow_mgmt.cc
View file
Expand Up @@ -162,10 +162,24 @@ void FlowMgmtManager::RetryVrfDeleteEvent(const VrfEntry *vrf) {
request_queue_.Enqueue(req);
}

void FlowMgmtManager::DummyEvent() {
boost::shared_ptr<FlowMgmtRequest>req
(new FlowMgmtRequest(FlowMgmtRequest::DUMMY));
request_queue_.Enqueue(req);
}

void FlowMgmtManager::EnqueueFlowEvent(FlowEvent *event) {
agent_->pkt()->get_flow_proto()->EnqueueFlowEvent(event);
}

void FlowMgmtManager::FlowUpdateQueueDisable(bool disabled) {
request_queue_.set_disable(disabled);
}

size_t FlowMgmtManager::FlowUpdateQueueLength() {
return request_queue_.Length();
}

/////////////////////////////////////////////////////////////////////////////
// Handlers for events from the work-queue
/////////////////////////////////////////////////////////////////////////////
Expand Down Expand Up @@ -377,6 +391,9 @@ bool FlowMgmtManager::RequestHandler(boost::shared_ptr<FlowMgmtRequest> req) {
break;
}

case FlowMgmtRequest::DUMMY:
break;

default:
assert(0);

Expand Down
5 changes: 5 additions & 0 deletions src/vnsw/agent/pkt/flow_mgmt.h
View file
Expand Up @@ -1029,6 +1029,8 @@ class FlowMgmtManager {
void DeleteEvent(const DBEntry *entry, uint32_t gen_id);
void RetryVrfDeleteEvent(const VrfEntry *vrf);
void RetryVrfDelete(uint32_t vrf_id);
// Dummy event used for testing
void DummyEvent();

void VnFlowCounters(const VnEntry *vn,
uint32_t *ingress_flow_count,
Expand All @@ -1044,6 +1046,9 @@ class FlowMgmtManager {
uint32_t source_port);
void EnqueueUveAddEvent(const FlowEntry *flow) const;
void EnqueueUveDeleteEvent(const FlowEntry *flow) const;

void FlowUpdateQueueDisable(bool val);
size_t FlowUpdateQueueLength();
private:
// Handle Add/Change of a flow. Builds FlowMgmtKeyTree for all objects
void AddFlow(FlowEntryPtr &flow);
Expand Down
40 changes: 25 additions & 15 deletions src/vnsw/agent/pkt/flow_mgmt_dbclient.cc
View file
Expand Up @@ -48,6 +48,7 @@ void FlowMgmtDbClient::AddEvent(const DBEntry *entry, FlowMgmtState *state) {

void FlowMgmtDbClient::DeleteEvent(const DBEntry *entry, FlowMgmtState *state) {
state->gen_id_++;
state->deleted_ = true;
mgr_->DeleteEvent(entry, state->gen_id_);
}

Expand All @@ -66,6 +67,10 @@ static DBState *ValidateGenId(DBTableBase *table, DBEntry *entry,
if (state == NULL)
return NULL;

// If DBEntry is re-added in meanwhile, we do not want to free DBState
if (state->deleted_ == false)
return NULL;

if (state->gen_id_ > gen_id)
return NULL;

Expand All @@ -77,9 +82,6 @@ void FlowMgmtDbClient::FreeInterfaceState(Interface *intf, uint32_t gen_id) {
if (vm_port == NULL)
return;

if ((intf->IsDeleted() == false) && (vm_port->vn() != NULL))
return;

DBState *state = ValidateGenId(intf->get_table(), intf,
interface_listener_id_, gen_id);
if (state == NULL)
Expand All @@ -100,7 +102,7 @@ void FlowMgmtDbClient::InterfaceNotify(DBTablePartBase *part, DBEntryBase *e) {

VmIntfFlowHandlerState *state = static_cast<VmIntfFlowHandlerState *>
(e->GetState(part->parent(), interface_listener_id_));
if (intf->IsDeleted() || new_vn == NULL) {
if (intf->IsDeleted()) {
if (state) {
DeleteEvent(vm_port, state);
}
Expand All @@ -120,6 +122,11 @@ void FlowMgmtDbClient::InterfaceNotify(DBTablePartBase *part, DBEntryBase *e) {
state->vrf_assign_acl_ = vm_port->vrf_assign_acl();
changed = true;
} else {
if (state->deleted_) {
state->deleted_ = false;
changed = true;
}

if (state->vn_.get() != new_vn) {
changed = true;
state->vn_ = new_vn;
Expand Down Expand Up @@ -147,10 +154,6 @@ void FlowMgmtDbClient::InterfaceNotify(DBTablePartBase *part, DBEntryBase *e) {
// VN notification handler
////////////////////////////////////////////////////////////////////////////
void FlowMgmtDbClient::FreeVnState(VnEntry *vn, uint32_t gen_id) {
if (vn->IsDeleted() == false) {
return;
}

DBState *state = ValidateGenId(vn->get_table(), vn, vn_listener_id_,
gen_id);
if (state == NULL)
Expand Down Expand Up @@ -213,6 +216,11 @@ void FlowMgmtDbClient::VnNotify(DBTablePartBase *part, DBEntryBase *e) {
changed = true;
}

if (state->deleted_) {
state->deleted_ = false;
changed = true;
}

if (changed) {
AddEvent(vn, state);
}
Expand All @@ -222,9 +230,6 @@ void FlowMgmtDbClient::VnNotify(DBTablePartBase *part, DBEntryBase *e) {
// ACL notification handler
////////////////////////////////////////////////////////////////////////////
void FlowMgmtDbClient::FreeAclState(AclDBEntry *acl, uint32_t gen_id) {
if (acl->IsDeleted() == false)
return;

DBState *state = ValidateGenId(acl->get_table(), acl, acl_listener_id_,
gen_id);
if (state == NULL)
Expand All @@ -250,16 +255,14 @@ void FlowMgmtDbClient::AclNotify(DBTablePartBase *part, DBEntryBase *e) {
state = new AclFlowHandlerState();
e->SetState(part->parent(), acl_listener_id_, state);
}
state->deleted_ = false;
AddEvent(acl, state);
}

////////////////////////////////////////////////////////////////////////////
// NH notification handler
////////////////////////////////////////////////////////////////////////////
void FlowMgmtDbClient::FreeNhState(NextHop *nh, uint32_t gen_id) {
if (nh->IsDeleted() == false)
return;

DBState *state = ValidateGenId(nh->get_table(), nh, nh_listener_id_,
gen_id);
if (state == NULL)
Expand All @@ -286,6 +289,7 @@ void FlowMgmtDbClient::NhNotify(DBTablePartBase *part, DBEntryBase *e) {
state = new NhFlowHandlerState();
nh->SetState(part->parent(), nh_listener_id_, state);
}
state->deleted_ = false;
AddEvent(nh, state);
return;
}
Expand Down Expand Up @@ -378,6 +382,7 @@ void FlowMgmtDbClient::VrfNotify(DBTablePartBase *part, DBEntryBase *e) {
vrf->SetState(part->parent(), vrf_listener_id_, state);
AddEvent(vrf, state);
}
state->deleted_ = false;
return;
}

Expand Down Expand Up @@ -537,14 +542,19 @@ void FlowMgmtDbClient::RouteNotify(VrfFlowHandlerState *vrf_state,
}

bool new_route = false;
bool changed = false;
if (state == NULL) {
state = new RouteFlowHandlerState();
route->SetState(partition->parent(), id, state);
AddEvent(route, state);
new_route = true;
} else {
if (state->deleted_) {
state->deleted_ = false;
changed = true;
}
}

bool changed = false;
// Handle SG change
if (state->sg_l_ != new_sg_l) {
state->sg_l_ = new_sg_l;
Expand Down
4 changes: 3 additions & 1 deletion src/vnsw/agent/pkt/flow_mgmt_dbclient.h
View file
Expand Up @@ -13,11 +13,12 @@ class EcmpLoadBalance;
class FlowMgmtDbClient {
public:
struct FlowMgmtState : public DBState {
FlowMgmtState() : gen_id_(0) { }
FlowMgmtState() : gen_id_(0), deleted_(false) { }
virtual ~FlowMgmtState() { }

void IncrementGenId() { gen_id_++; }
uint32_t gen_id_;
bool deleted_;
};

struct VnFlowHandlerState : public FlowMgmtState {
Expand Down Expand Up @@ -104,6 +105,7 @@ class FlowMgmtDbClient {
bool FreeDBState(const DBEntry *entry, uint32_t gen_id);

private:
friend class FlowMgmtRouteTest;
void AddEvent(const DBEntry *entry, FlowMgmtState *state);
void DeleteEvent(const DBEntry *entry, FlowMgmtState *state);
void ChangeEvent(const DBEntry *entry, FlowMgmtState *state);
Expand Down
22 changes: 15 additions & 7 deletions src/vnsw/agent/pkt/flow_mgmt_request.h
View file
Expand Up @@ -22,32 +22,40 @@ class FlowMgmtRequest {
RETRY_DELETE_VRF,
UPDATE_FLOW_INDEX,
DELETE_BGP_AAS_FLOWS,
UPDATE_FLOW_STATS
UPDATE_FLOW_STATS,
DUMMY

};

FlowMgmtRequest(Event event, FlowEntryPtr &flow) :
event_(event), flow_(flow), db_entry_(NULL), vrf_id_(0) {
event_(event), flow_(flow), db_entry_(NULL), vrf_id_(0), gen_id_(),
bytes_(), packets_(), oflow_bytes_() {
if (event == RETRY_DELETE_VRF)
assert(vrf_id_);
}
}

FlowMgmtRequest(Event event, FlowEntryPtr &flow, uint32_t bytes,
uint32_t packets, uint32_t oflow_bytes) :
event_(event), flow_(flow), db_entry_(NULL), vrf_id_(0),
event_(event), flow_(flow), db_entry_(NULL), vrf_id_(0), gen_id_(),
bytes_(bytes), packets_(packets), oflow_bytes_(oflow_bytes) {
if (event == RETRY_DELETE_VRF)
assert(vrf_id_);
}
}

FlowMgmtRequest(Event event, const DBEntry *db_entry, uint32_t gen_id) :
event_(event), flow_(NULL), db_entry_(db_entry), vrf_id_(0),
gen_id_(gen_id) {
gen_id_(gen_id), bytes_(), packets_(), oflow_bytes_() {
if (event == RETRY_DELETE_VRF) {
const VrfEntry *vrf = dynamic_cast<const VrfEntry *>(db_entry);
assert(vrf);
vrf_id_ = vrf->vrf_id();
}
}
}

FlowMgmtRequest(Event event) :
event_(event), flow_(NULL), db_entry_(NULL), vrf_id_(),
gen_id_(), bytes_(), packets_(), oflow_bytes_() {
}

virtual ~FlowMgmtRequest() { }

Expand Down
4 changes: 4 additions & 0 deletions src/vnsw/agent/pkt/flow_proto.cc
View file
Expand Up @@ -136,6 +136,10 @@ void FlowProto::DisableFlowMgmtQueue(bool disabled) {
flow_update_queue_.set_disable(disabled);
}

size_t FlowProto::FlowMgmtQueueLength() {
return flow_update_queue_.Length();
}

/////////////////////////////////////////////////////////////////////////////
// FlowTable related routines
/////////////////////////////////////////////////////////////////////////////
Expand Down
2 changes: 2 additions & 0 deletions src/vnsw/agent/pkt/flow_proto.h
View file
Expand Up @@ -79,6 +79,8 @@ class FlowProto : public Proto {

void DisableFlowEventQueue(uint32_t index, bool disabled);
void DisableFlowMgmtQueue(bool disabled);
size_t FlowMgmtQueueLength();

const FlowStats *flow_stats() const { return &stats_; }

void SetProfileData(ProfileData *data);
Expand Down

0 comments on commit 133100e

Please sign in to comment.