On eviction of a flow, dont delete reverse flow

When a flow is evicted, unlink the reverse flow and mark it as short flow. Change-Id: Ic468f3431c820cbd4d914519f57c76fc26152861 Partial-Bug: #1542656
Juniper · Feb 8, 2016 · 23d3519 · 23d3519
1 parent 8e2589e
commit 23d3519
Show file tree

Hide file tree

Showing 4 changed files with 23 additions and 1 deletion.
diff --git a/src/vnsw/agent/pkt/flow_proto.cc b/src/vnsw/agent/pkt/flow_proto.cc
@@ -276,7 +276,7 @@ bool FlowProto::FlowEventHandler(const FlowEvent &req, FlowTable *table) {
         FlowEntry *flow = req.flow();
         if (flow->flow_handle() != req.flow_handle())
             break;
-        flow->flow_table()->DeleteMessage(flow);
+        flow->flow_table()->EvictFlow(flow);
         break;
     }
 

diff --git a/src/vnsw/agent/pkt/flow_table.cc b/src/vnsw/agent/pkt/flow_table.cc
@@ -663,6 +663,18 @@ void FlowTable::DeleteMessage(FlowEntry *flow) {
     DeleteFlowInfo(flow);
 }
 
+void FlowTable::EvictFlow(FlowEntry *flow) {
+    FlowEntry *reverse_flow = flow->reverse_flow_entry();
+    Delete(flow->key(), false);
+    DeleteFlowInfo(flow);
+
+    // Reverse flow unlinked with forward flow. Make it short-flow
+    if (reverse_flow && reverse_flow->deleted() == false) {
+        reverse_flow->MakeShortFlow(FlowEntry::SHORT_NO_REVERSE_FLOW);
+        UpdateKSync(reverse_flow, true);
+    }
+}
+
 // Handle events from Flow Management module for a flow
 bool FlowTable::FlowResponseHandler(const FlowEvent *resp) {
     FlowEntry *flow = resp->flow();

diff --git a/src/vnsw/agent/pkt/flow_table.h b/src/vnsw/agent/pkt/flow_table.h
@@ -199,6 +199,7 @@ class FlowTable {
 
     void RevaluateFlow(FlowEntry *flow);
     void DeleteMessage(FlowEntry *flow);
+    void EvictFlow(FlowEntry *flow);
 
     void RevaluateInterface(FlowEntry *flow);
     void RevaluateVn(FlowEntry *flow);

diff --git a/src/vnsw/agent/pkt/test/test_flow_eviction.cc b/src/vnsw/agent/pkt/test/test_flow_eviction.cc
@@ -143,6 +143,15 @@ TEST_F(FlowEvictionTest, NewFlow_Evicted_Index_1) {
     EXPECT_TRUE(FlowGet(vrf_id, remote_vm1_ip, vm1_ip, 2, 0, 0,
                         vif0->flow_key_nh()->id()) == false);
 
+    // The reverse flow should not be deleted. Should be marked short flow
+    FlowEntry *rflow = FlowGet(vrf_id, vm1_ip, remote_vm1_ip, 2, 0, 0,
+                               vif0->flow_key_nh()->id());
+    EXPECT_TRUE(rflow != NULL);
+    if (rflow) {
+        EXPECT_TRUE(rflow->IsShortFlow());
+        EXPECT_TRUE(rflow->reverse_flow_entry() == NULL);
+    }
+
     // New flow should be present
     flow = FlowGet(vrf_id, remote_vm1_ip, vm1_ip, 1, 0, 0,
                    vif0->flow_key_nh()->id());