Skip to content

Commit

Permalink
Fix flows stuck in vrouter for ever
Browse files Browse the repository at this point in the history
Issue:
------
Stats collector and flow processing run in parallel
so flow handle and gen id not being the key can change
while stats collector aging process, due to which it
can run eviction validation using old flow handle and
gen id, while enqueue an eviction request using new
flow handle and gen id, resulting in skipping delete
from vrouter and causing lingering flows

Fix:
----
Lock and fetch flow handle and gen idat the start of
eviction processing and use the same while enqueuing
eviction request

Closes-Bug: 1611881
Change-Id: I9c7aa4bfee0b82334e92d5a748d1b2045e3976df
(cherry picked from commit 52ffcdb)
  • Loading branch information
Prabhjot Singh Sethi committed Aug 26, 2016
1 parent 1040141 commit 3e2e001
Show file tree
Hide file tree
Showing 2 changed files with 22 additions and 6 deletions.
25 changes: 20 additions & 5 deletions src/vnsw/agent/vrouter/flow_stats/flow_stats_collector.cc
Expand Up @@ -337,10 +337,12 @@ void FlowStatsCollector::FlowDeleteEnqueue(FlowExportInfo *info, uint64_t t) {
}
}

void FlowStatsCollector::FlowEvictEnqueue(FlowExportInfo *info, uint64_t t) {
void FlowStatsCollector::FlowEvictEnqueue(FlowExportInfo *info, uint64_t t,
uint32_t flow_handle,
uint16_t gen_id) {
FlowEntry *fe = info->flow();
agent_uve_->agent()->pkt()->get_flow_proto()->EvictFlowRequest
(fe, fe->flow_handle(), fe->gen_id(), (fe->gen_id() + 1));
(fe, flow_handle, gen_id, (gen_id + 1));
info->set_evict_enqueue_time(t);
}

Expand Down Expand Up @@ -419,6 +421,19 @@ uint32_t FlowStatsCollector::RunAgeing(uint32_t max_count) {
info = &it->second;
FlowEntry *fe = info->flow();
FlowEntry *rfe = info->reverse_flow();
uint32_t flow_handle;
uint16_t gen_id;
{
FlowEntry *rflow = NULL;
FLOW_LOCK(fe, rflow, FlowEvent::FLOW_MESSAGE);
// since flow processing and stats collector can run in parallel
// flow handle and gen id not being the key for flow entry can
// change while processing, so flow handle and gen id should be
// fetched by holding an lock and should not be re-fetched again
// during the entry processing
flow_handle = fe->flow_handle();
gen_id = fe->gen_id();
}
it++;

// if we come across deleted entry, retry flow deletion after some time
Expand All @@ -434,18 +449,18 @@ uint32_t FlowStatsCollector::RunAgeing(uint32_t max_count) {

count++;
const vr_flow_entry *k_flow = ksync_obj->GetValidKFlowEntry
(fe->key(), fe->flow_handle(), fe->gen_id());
(fe->key(), flow_handle, gen_id);

if ((fe->key().protocol == IPPROTO_TCP) &&
ksync_obj->IsEvictionMarked(k_flow)) {
uint64_t evict_time = info->evict_enqueue_time();
if (evict_time) {
if ((curr_time - evict_time) > kFlowDeleteRetryTime) {
FlowEvictEnqueue(info, curr_time);
FlowEvictEnqueue(info, curr_time, flow_handle, gen_id);
}
continue;
}
FlowEvictEnqueue(info, curr_time);
FlowEvictEnqueue(info, curr_time, flow_handle, gen_id);
continue;
}

Expand Down
3 changes: 2 additions & 1 deletion src/vnsw/agent/vrouter/flow_stats/flow_stats_collector.h
Expand Up @@ -191,7 +191,8 @@ class FlowStatsCollector : public StatsCollector {
uint64_t *diff_bytes,
uint64_t *diff_pkts);
void FlowDeleteEnqueue(FlowExportInfo *info, uint64_t t);
void FlowEvictEnqueue(FlowExportInfo *info, uint64_t t);
void FlowEvictEnqueue(FlowExportInfo *info, uint64_t t,
uint32_t flow_handle, uint16_t gen_id);
void EnqueueFlowMsg();
void DispatchPendingFlowMsg();
void GetFlowSandeshActionParams(const FlowAction &action_info,
Expand Down

0 comments on commit 3e2e001

Please sign in to comment.