From 52ffcdbdde5b32a7c868c59dd370b13e827ea96f Mon Sep 17 00:00:00 2001
From: Prabhjot Singh Sethi <prabhjot@juniper.net>
Date: Thu, 11 Aug 2016 16:35:52 +0530
Subject: [PATCH] Fix flows stuck in vrouter for ever

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
---
 .../flow_stats/flow_stats_collector.cc        | 25 +++++++++++++++----
 .../vrouter/flow_stats/flow_stats_collector.h |  3 ++-
 2 files changed, 22 insertions(+), 6 deletions(-)

diff --git a/src/vnsw/agent/vrouter/flow_stats/flow_stats_collector.cc b/src/vnsw/agent/vrouter/flow_stats/flow_stats_collector.cc
index 013b9c5198e..0660bb02431 100644
--- a/src/vnsw/agent/vrouter/flow_stats/flow_stats_collector.cc
+++ b/src/vnsw/agent/vrouter/flow_stats/flow_stats_collector.cc
@@ -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);
 }
 
@@ -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
@@ -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;
         }
 
diff --git a/src/vnsw/agent/vrouter/flow_stats/flow_stats_collector.h b/src/vnsw/agent/vrouter/flow_stats/flow_stats_collector.h
index 21f99802692..1e379e391ed 100644
--- a/src/vnsw/agent/vrouter/flow_stats/flow_stats_collector.h
+++ b/src/vnsw/agent/vrouter/flow_stats/flow_stats_collector.h
@@ -194,7 +194,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,