* Read reverse flow entry after acquiring the lock

In case of short lived TCP session using same 5 tuple we might
end up accesssing a reverse flow entry which might be already deleted
and present in free list. Below set of events lead to this issue

1. F1 and R1 flow are added to flow table
2. R1 is written to vrouter
3. F1 is written to vrouter
4. R1 flow add response is received, triggering update of
   F1(not needed now as reverse flow index is not written to
   kernel?)
5. In the meantime flow is evicted in vrouter, hence flow
   update for F1 would result in error from vrouter resulting in short flow
6. Since F1 is shortflow Flow delete gets enqueued
7. Since R1 is evict marked, flow evict gets enqueued
8. Both event F1 and R1 delete and evict event can run in
   parallel,and hence reverse flow pointer obtained before FLOW
   lock could be invalid, hence read back the same

Was able to simulate the same in UT, UT is disabled as it need some
instrumentation to run it to hit the issue

Change-Id: I368f465f9d446b43dfe0e4baa547d8c8cbfd6840
Closes-bug: #1714371
(cherry picked from commit a0cb166)

src/vnsw/agent/pkt/flow_table.cc

@@ -757,6 +757,21 @@ bool FlowTable::ProcessFlowEvent(const FlowEvent *req, FlowEntry *flow,
     //Now process events.
     switch (req->event()) {
     case FlowEvent::DELETE_FLOW: {
+        //In case of continous stream of short lived TCP flows with same 5 tuple,
+        //flow eviction logic might cause below set of event
+        //1> F1 and R1 flow are added to flow table
+        //2> R1 is written to vrouter
+        //3> F1 is written to vrouter
+        //4> R1 flow add response is received, triggering update of
+        //   F1(not needed now as reverse flow index is not written to kernel?)
+        //5> In the meantime flow is evicted in vrouter, hence flow update for F1
+        //   would result in error from vrouter resulting in short flow
+        //6> Since F1 is shortflow Flow delete gets enqueued
+        //7> Since R1 is evict marked, flow evict gets enqueued
+        //8> Both event F1 and R1 delete and evict event can run in parallel,
+        //   and hence reverse flow pointer obtained before FLOW lock could
+        //   be invalid, hence read back the same
+        rflow = flow->reverse_flow_entry();
         DeleteUnLocked(true, flow, rflow);
         break;
     }

src/vnsw/agent/pkt/test/test_ecmp_mx.cc

@@ -6,6 +6,7 @@
 #include "test/test_cmn_util.h"
 #include "test_pkt_util.h"
 #include "pkt/flow_proto.h"
+#include "ksync/ksync_sock_user.h"
 
 #define AGE_TIME 10*1000
 
@@ -446,6 +447,55 @@ TEST_F(EcmpTest, EcmpTest_8) {
     WAIT_FOR(1000, 1000, (get_flow_proto()->FlowCount() == 0));
 }
 
+//Test to simulate continous stream of flow packets getting evicted
+//To run the test, please modify KSyncUserSockFlowContext::Process
+//to return same flow handle and incremental gen_id
+//
+//+  static uint8_t gen_id = 0;
+//+  fwd_flow_idx = 100;
+//   req_->set_fr_index(fwd_flow_idx);
+//-  req_->set_fr_gen_id((fwd_flow_idx % 255));
+//+  req_->set_fr_gen_id(gen_id++);
+//
+//and to pass ttl as gen id in below API
+#if 0
+TEST_F(EcmpTest, TEST_1) {
+    Ip4Address server_ip = Ip4Address::from_string("10.1.1.3");
+    Ip4Address zero = Ip4Address::from_string("0.0.0.0");
+    TunnelType::TypeBmap bmap = (1 << TunnelType::MPLS_GRE);
+    PathPreference path_preference(0, PathPreference::HIGH, false, false);
+    Inet4TunnelRouteAdd(bgp_peer, "vrf1", zero, 0, server_ip, bmap,
+                        16, "vn1", SecurityGroupList(), path_preference);
+    client->WaitForIdle();
+
+    uint32_t gen_id = 0;
+    for (uint32_t i = 0; i < 10000; i++) {
+        gen_id++;
+        TxTcpPacket(VmPortGetId(1), "1.1.1.1", "2.1.1.1",
+                    10, 15, false, 1, VrfGet("vrf1")->vrf_id(), gen_id);
+        client->WaitForIdle();
+
+        FlowEntry *entry = FlowGet(VrfGet("vrf1")->vrf_id(),
+                                   "1.1.1.1", "2.1.1.1", 6, 10, 15,
+                                   GetFlowKeyNH(1));
+        client->WaitForIdle();
+
+        if (entry) {
+            entry->MakeShortFlow(FlowEntry::SHORT_FAILED_VROUTER_INSTALL);
+            KSyncSockTypeMap::SetEvictedFlag(entry->reverse_flow_entry()->
+                                             flow_handle());
+        }
+        client->WaitForIdle();
+
+        if (gen_id % 10 == 0) {
+            WAIT_FOR(1000, 1000, (flow_proto_->FlowCount() == 0));
+        }
+    }
+
+    WAIT_FOR(1000, 1000, (flow_proto_->FlowCount() == 0));
+}
+#endif
+
 int main(int argc, char *argv[]) {
     GETUSERARGS();
     client = TestInit(init_file, ksync_init, true, true, true, 100*1000);