Skip to content

Commit

Permalink
Merge "* Replace index based entry map with vector for better perform…
Browse files Browse the repository at this point in the history 
…ance * If a same flow with same key is evicted and reused just delete and readd the flow in ksync. * Delete the flow from index table when flow is delete marked. Closes-bug:#1512936" into R2.20
  • Loading branch information
@opencontrail-ci-admin
Zuul authored and opencontrail-ci-admin committed Nov 19, 2015
2 parents da32d09 + e3f0737 commit f117eec
Show file tree
Hide file tree
Showing 7 changed files with 63 additions and 42 deletions.
75 changes: 45 additions & 30 deletions src/vnsw/agent/pkt/flow_table.cc
View file
Expand Up @@ -883,7 +883,7 @@ uint32_t FlowEntry::acl_assigned_vrf_index() const {
return 0;
}

void FlowEntry::UpdateKSync(FlowTable* table) {
void FlowEntry::UpdateKSync(FlowTable* table, bool update) {
FlowInfo flow_info;
FillFlowInfo(flow_info);
if (stats_.last_modified_time != stats_.setup_time) {
Expand All @@ -898,6 +898,21 @@ void FlowEntry::UpdateKSync(FlowTable* table) {
FlowTableKSyncObject *ksync_obj =
Agent::GetInstance()->ksync()->flowtable_ksync_obj();

//In case of TCP flow eviction there is a chance
//that same flow with same flow key could be evicted
//and reused, in that case, we force programming of
//flow by deleting the entry in ksync and readding it
//Since flag vrouter flow evicted would be set
//Actual msg would not be sent to kernel
if (update == false) {
if (ksync_entry_ != NULL) {
data_.vrouter_evicted_flow_ = true;
ksync_obj->Delete(ksync_entry_);
data_.vrouter_evicted_flow_ = false;
ksync_entry_ = NULL;
}
}

if (ksync_entry_ == NULL) {
FLOW_TRACE(Trace, "Add", flow_info);
FlowTableKSyncEntry key(ksync_obj, this, flow_handle_);
Expand Down Expand Up @@ -989,7 +1004,7 @@ void FlowTable::DelLinkLocalFlowInfo(int fd) {
linklocal_flow_info_map_.erase(fd);
}

void FlowTable::Add(FlowEntry *flow, FlowEntry *rflow) {
void FlowTable::Add(FlowEntry *flow, FlowEntry *rflow, bool update) {
flow->reset_flags(FlowEntry::ReverseFlow);
/* reverse flow may not be aviable always, eg: Flow Audit */
if (rflow != NULL)
Expand All @@ -1012,13 +1027,14 @@ void FlowTable::Add(FlowEntry *flow, FlowEntry *rflow) {

if (rflow) {
rflow->GetPolicyInfo();
ResyncAFlow(rflow);
ResyncAFlow(rflow, update);
AddFlowInfo(rflow);
}


ResyncAFlow(flow);
ResyncAFlow(flow, update);
AddFlowInfo(flow);
AddIndexFlowInfo(flow, flow->flow_handle_);

agent_->flow_stats_manager()->AddEvent(flow);
agent_->flow_stats_manager()->AddEvent(rflow);
Expand Down Expand Up @@ -1117,7 +1133,7 @@ void FlowEntry::set_flow_handle(uint32_t flow_handle, FlowTable* table) {
/* trigger update KSync on flow handle change */
if (flow_handle_ != flow_handle) {
flow_handle_ = flow_handle;
UpdateKSync(table);
UpdateKSync(table, true);
}
}

Expand Down Expand Up @@ -1689,6 +1705,7 @@ void FlowTable::DeleteInternal(FlowEntryMap::iterator &it, uint64_t time)
fe->set_reverse_flow_entry(NULL);

DeleteFlowInfo(fe);
DeleteByIndex(fe->flow_handle_, fe);

FlowTableKSyncEntry *ksync_entry = fe->ksync_entry_;
KSyncEntry::KSyncEntryPtr ksync_ptr = ksync_entry;
Expand Down Expand Up @@ -1841,6 +1858,9 @@ void FlowTable::InitDone() {
max_vm_flows_ = (uint32_t)
(agent_->ksync()->flowtable_ksync_obj()->flow_table_entries_count() *
agent_->params()->max_vm_flows()) / 100;
flow_index_tree_.resize(
agent_->ksync()->flowtable_ksync_obj()->flow_table_entries_count(),
NULL);
}

void FlowTable::IntfNotify(DBTablePartBase *part, DBEntryBase *e) {
Expand Down Expand Up @@ -2052,29 +2072,26 @@ void FlowTable::DeleteVrouterEvictedFlow(FlowEntry *flow) {

void FlowTable::InsertByIndex(uint32_t flow_handle, FlowEntry *flow) {
if (flow_handle != FlowEntry::kInvalidFlowHandle &&
flow->deleted() == false) {
FlowEntry *old_flow = FindByIndex(flow_handle);
if (old_flow == NULL) {
flow_index_tree_.insert(std::pair<uint32_t,FlowEntryPtr>(flow_handle,
flow));
} else if (old_flow != flow) {
flow->deleted() == false) {
if (flow_index_tree_[flow_handle] &&
flow_index_tree_[flow_handle] != flow) {
assert(0);
}
flow_index_tree_[flow_handle] = flow;
}
}

void FlowTable::DeleteByIndex(uint32_t flow_handle, FlowEntry *fe) {
if (flow_handle != FlowEntry::kInvalidFlowHandle) {
if (FindByIndex(flow_handle) == fe) {
flow_index_tree_.erase(flow_handle);
if (flow_index_tree_[flow_handle].get() == fe) {
flow_index_tree_[flow_handle] = NULL;
}
}
}

FlowEntry* FlowTable::FindByIndex(uint32_t flow_handle) {
FlowIndexTree::iterator it = flow_index_tree_.find(flow_handle);
if (it != flow_index_tree_.end()) {
return it->second.get();
if (flow_handle <= flow_index_tree_.size()) {
return flow_index_tree_[flow_handle].get();
}
return NULL;
}
Expand Down Expand Up @@ -2317,7 +2334,7 @@ bool InetRouteFlowUpdate::SgUpdate(FlowEntry *fe, FlowTable *table,
if (fe->is_flags_set(FlowEntry::ReverseFlow)) {
FlowEntry *fwd_flow = fe->reverse_flow_entry();
if (fwd_flow) {
table->ResyncAFlow(fwd_flow);
table->ResyncAFlow(fwd_flow, true);
table->AddFlowInfo(fwd_flow);
}
}
Expand Down Expand Up @@ -2432,7 +2449,7 @@ bool BridgeEntryFlowUpdate::SgUpdate(FlowEntry *fe, FlowTable *table,
if (fe->is_flags_set(FlowEntry::ReverseFlow)) {
FlowEntry *fwd_flow = fe->reverse_flow_entry();
if (fwd_flow) {
table->ResyncAFlow(fwd_flow);
table->ResyncAFlow(fwd_flow, true);
table->AddFlowInfo(fwd_flow);
}
}
Expand Down Expand Up @@ -2559,11 +2576,11 @@ void FlowTable::ResyncVnFlows(const VnEntry *vn) {
fe->is_flags_set(FlowEntry::UnknownUnicastFlood)) {
fe->MakeShortFlow(FlowEntry::SHORT_NO_DST_ROUTE);
fe->GetPolicyInfo(vn);
ResyncAFlow(fe);
ResyncAFlow(fe, true);
continue;
}
fe->GetPolicyInfo(vn);
ResyncAFlow(fe);
ResyncAFlow(fe, true);
AddFlowInfo(fe);
FlowInfo flow_info;
fe->FillFlowInfo(flow_info);
Expand All @@ -2587,7 +2604,7 @@ void FlowTable::ResyncAclFlows(const AclDBEntry *acl)
FlowEntry *fe = (*fet_it).get();
DeleteFlowInfo(fe);
fe->GetPolicyInfo();
ResyncAFlow(fe);
ResyncAFlow(fe, true);
AddFlowInfo(fe);
FlowInfo flow_info;
fe->FillFlowInfo(flow_info);
Expand All @@ -2613,7 +2630,7 @@ void FlowTable::ResyncRpfNH(const RouteFlowKey &key, const AgentRoute *rt) {
}

if (flow->SetRpfNH(this, rt) == true) {
flow->UpdateKSync(this);
flow->UpdateKSync(this, true);
FlowInfo flow_info;
flow->FillFlowInfo(flow_info);
FLOW_TRACE(Trace, "Resync RPF NH", flow_info);
Expand Down Expand Up @@ -2726,7 +2743,7 @@ void FlowTable::IterateFlowInfoEntries(const RouteFlowKey &key, FlowEntryCb cb)
if (cb(fe) == false) {
fet.erase(fet_it);
} else {
ResyncAFlow(fe);
ResyncAFlow(fe, true);
AddFlowInfo(fe);
}
}
Expand All @@ -2752,7 +2769,7 @@ void FlowTable::ResyncVmPortFlows(const VmInterface *intf) {
if (fwd_flow) {
DeleteFlowInfo(fwd_flow);
fwd_flow->GetPolicyInfo();
ResyncAFlow(fwd_flow);
ResyncAFlow(fwd_flow, true);
AddFlowInfo(fwd_flow);
FlowInfo flow_info;
fwd_flow->FillFlowInfo(flow_info);
Expand All @@ -2761,7 +2778,7 @@ void FlowTable::ResyncVmPortFlows(const VmInterface *intf) {
}
DeleteFlowInfo(fe);
fe->GetPolicyInfo(intf->vn());
ResyncAFlow(fe);
ResyncAFlow(fe, true);
AddFlowInfo(fe);
FlowInfo flow_info;
fe->FillFlowInfo(flow_info);
Expand Down Expand Up @@ -2990,8 +3007,6 @@ void FlowTable::AddFlowInfo(FlowEntry *fe)
AddVmFlowInfo(fe);
// Add RouteFlowTree;
AddRouteFlowInfo(fe);

AddIndexFlowInfo(fe, fe->flow_handle_);
}

void FlowTable::AddAclFlowInfo (FlowEntry *fe)
Expand Down Expand Up @@ -3318,10 +3333,10 @@ void FlowTable::AddIndexFlowInfo(FlowEntry *fe, uint32_t flow_handle) {
fe->set_flow_handle(flow_handle, this);
}

void FlowTable::ResyncAFlow(FlowEntry *fe) {
void FlowTable::ResyncAFlow(FlowEntry *fe, bool update) {
fe->UpdateRpf();
fe->DoPolicy();
fe->UpdateKSync(this);
fe->UpdateKSync(this, update);

// If this is forward flow, update the SG action for reflexive entry
if (fe->is_flags_set(FlowEntry::ReverseFlow)) {
Expand All @@ -3337,7 +3352,7 @@ void FlowTable::ResyncAFlow(FlowEntry *fe) {
// Check if there is change in action for reverse flow
rflow->ActionRecompute();

rflow->UpdateKSync(this);
rflow->UpdateKSync(this, update);
}

void FlowTable::DeleteVnFlows(const VnEntry *vn)
Expand Down
9 changes: 4 additions & 5 deletions src/vnsw/agent/pkt/flow_table.h
View file
Expand Up @@ -337,7 +337,7 @@ class FlowEntry {
virtual ~FlowEntry();

bool ActionRecompute();
void UpdateKSync(FlowTable* table);
void UpdateKSync(FlowTable* table, bool update);
int GetRefCount() { return refcount_; }
void MakeShortFlow(FlowShortReason reason);
const FlowStats &stats() const { return stats_;}
Expand Down Expand Up @@ -611,8 +611,7 @@ class FlowTable {
typedef std::map<const VmEntry *, VmFlowInfo *> VmFlowTree;
typedef std::pair<const VmEntry *, VmFlowInfo *> VmFlowPair;

typedef std::map<uint32_t, FlowEntryPtr> FlowIndexTree;

typedef std::vector<FlowEntryPtr> FlowIndexTree;
typedef Patricia::Tree<RouteFlowInfo, &RouteFlowInfo::node, RouteFlowInfo::KeyCmp> RouteFlowTree;
typedef boost::function<bool(FlowEntry *flow)> FlowEntryCb;

Expand Down Expand Up @@ -678,7 +677,7 @@ class FlowTable {
void Shutdown();

FlowEntry *Allocate(const FlowKey &key);
void Add(FlowEntry *flow, FlowEntry *rflow);
void Add(FlowEntry *flow, FlowEntry *rflow, bool update);
FlowEntry *Find(const FlowKey &key);
bool Delete(const FlowKey &key, bool del_reverse_flow);

Expand Down Expand Up @@ -788,7 +787,7 @@ class FlowTable {
void IncrVnFlowCounter(VnFlowInfo *vn_flow_info, const FlowEntry *fe);
void DecrVnFlowCounter(VnFlowInfo *vn_flow_info, const FlowEntry *fe);
void ResyncVnFlows(const VnEntry *vn);
void ResyncAFlow(FlowEntry *fe);
void ResyncAFlow(FlowEntry *fe, bool update);
void ResyncVmPortFlows(const VmInterface *intf);
void ResyncRpfNH(const RouteFlowKey &key, const AgentRoute *rt);

Expand Down
10 changes: 8 additions & 2 deletions src/vnsw/agent/pkt/pkt_flow_info.cc
View file
Expand Up @@ -1447,14 +1447,20 @@ void PktFlowInfo::Add(const PktInfo *pkt, PktControlInfo *in,
flow->InitFwdFlow(this, pkt, in, out);
rflow->InitRevFlow(this, pkt, out, in);

bool update = true;
if (pkt->agent_hdr.cmd == AgentHdr::TRAP_FLOW_MISS) {
update = false;
}
/* Fip stats info in not updated in InitFwdFlow and InitRevFlow because
* both forward and reverse flows are not not linked to each other yet.
* We need both forward and reverse flows to update Fip stats info */
UpdateFipStatsInfo(flow.get(), rflow.get(), pkt, in, out);
if (swap_flows) {
Agent::GetInstance()->pkt()->flow_table()->Add(rflow.get(), flow.get());
Agent::GetInstance()->pkt()->flow_table()->Add(rflow.get(), flow.get(),
update);
} else {
Agent::GetInstance()->pkt()->flow_table()->Add(flow.get(), rflow.get());
Agent::GetInstance()->pkt()->flow_table()->Add(flow.get(), rflow.get(),
update);
}
}

Expand Down
4 changes: 2 additions & 2 deletions src/vnsw/agent/pkt/test/test_flowtable.cc
View file
Expand Up @@ -88,7 +88,7 @@ FlowEntry *FlowInit(TestFlowKey *t) {
}

static void FlowAdd(FlowEntry *fwd, FlowEntry *rev) {
Agent::GetInstance()->pkt()->flow_table()->Add(fwd, rev);
Agent::GetInstance()->pkt()->flow_table()->Add(fwd, rev, true);
}

class FlowTableTest : public ::testing::Test {
Expand Down Expand Up @@ -279,7 +279,7 @@ class FlowTableTest : public ::testing::Test {
}

static void FlowAdd(FlowEntry *fwd, FlowEntry *rev) {
Agent::GetInstance()->pkt()->flow_table()->Add(fwd, rev);
Agent::GetInstance()->pkt()->flow_table()->Add(fwd, rev, true);
client->WaitForIdle();
}

Expand Down
1 change: 0 additions & 1 deletion src/vnsw/agent/vrouter/flow_stats/flow_stats_collector.cc
View file
Expand Up @@ -118,7 +118,6 @@ bool FlowStatsCollector::ShouldBeAged(FlowStats *stats,
const vr_flow_entry *k_flow,
uint64_t curr_time,
const FlowEntry *flow) {

if (k_flow != NULL) {
uint64_t k_flow_bytes, bytes;

Expand Down
3 changes: 2 additions & 1 deletion src/vnsw/agent/vrouter/ksync/flowtable_ksync.cc
View file
Expand Up @@ -632,7 +632,8 @@ bool FlowTableKSyncObject::AuditProcess() {
flow->InitAuditFlow(flow_idx);
AGENT_ERROR(FlowLog, flow_idx, "FlowAudit : Converting HOLD "
"entry to short flow");
ksync_->agent()->pkt()->flow_table()->Add(flow.get(), NULL);
ksync_->agent()->pkt()->flow_table()->Add(flow.get(), NULL,
false);
}

}
Expand Down
3 changes: 2 additions & 1 deletion src/vnsw/agent/vrouter/ksync/sandesh_ksync.cc
View file
Expand Up @@ -127,7 +127,8 @@ void KSyncSandeshContext::FlowMsgHandler(vr_flow_req *r) {
//Tie forward flow and reverse flow
if (rev_flow && update_rev_flow) {
rev_flow->UpdateKSync(
flow_ksync_->ksync()->agent()->pkt()->flow_table());
flow_ksync_->ksync()->agent()->pkt()->flow_table(),
true);
}
}
} else {
Expand Down

0 comments on commit f117eec

Please sign in to comment.