Before initiating eviction, set M(odified) flag

Currently, before evicting the forward flow, we get to the reverse
flow and mark it for eviction. Then we go on to mark forward flow
for eviction and schedule the eviction at a later time. It is in
this deferred functionality that is scheduled that the forward and
the reverse flow gets evicted. We set the M(odified) flag at the
time of 'eviction-marking' in each of the flow entries so that
subsequent changes to the flow entry is not honoured.

It is possible that the forward flow can undergo a change in which
its reverse flow is unlinked (or changed) while the reverse flow
was in the process of getting marked for eviction. If that happens,
at the time of actual eviction, the deferred call will not be able
to find the reverse flow from the forward flow and the reverse flow
will remain in the 'Eviction-Candidate' state forever.

To fix this race, mark the forward flow as M(odified) before
initiating the eviction process, making sure that the mapping between
the forward and the reverse flow is untouched once eviction process
is initiated.

Change-Id: I088ac3abd5c3bfc7bf77efc697bbe1386254358f
Closes-BUG: 1521496

dp-core/vr_flow.c

@@ -709,22 +709,14 @@ __vr_flow_mark_evict(struct vrouter *router, struct vr_flow_entry *fe)
 
     flags = fe->fe_flags;
     if (flags & VR_FLOW_FLAG_ACTIVE) {
-        if (vr_flow_start_modify(router, fe)) {
-            flags = __sync_fetch_and_or(&fe->fe_flags,
-                    VR_FLOW_FLAG_EVICT_CANDIDATE);
-            if (flags & VR_FLOW_FLAG_EVICT_CANDIDATE) {
-                goto unset_modified;
-            } else {
-                return true;
-            }
+        flags = __sync_fetch_and_or(&fe->fe_flags,
+                VR_FLOW_FLAG_EVICT_CANDIDATE);
+        if (!(flags & VR_FLOW_FLAG_EVICT_CANDIDATE)) {
+            return true;
         }
     }
 
     return false;
-
-unset_modified:
-    vr_flow_stop_modify(router, fe);
-    return false;
 }
 
 static void
@@ -735,37 +727,56 @@ vr_flow_mark_evict(struct vrouter *router, struct vr_flow_entry *fe,
 
     struct vr_flow_entry *rfe = NULL;
 
+    /* start modifying the entry */
+    if (!vr_flow_start_modify(router, fe))
+        return;
+
     if (fe->fe_rflow >= 0) {
         rfe = vr_get_flow_entry(router, fe->fe_rflow);
-        if (rfe && (rfe->fe_rflow == index)) {
+        if (rfe) {
+            evict_forward_flow = false;
             if (rfe->fe_tcp_flags & VR_FLOW_TCP_DEAD) {
-                evict_forward_flow = __vr_flow_mark_evict(router, rfe);
+                if (!vr_flow_start_modify(router, rfe)) {
+                    /* no modification. hence...*/
+                    rfe = NULL;
+                } else {
+                    if (rfe->fe_rflow == index) {
+                        evict_forward_flow = __vr_flow_mark_evict(router, rfe);
+                    }
+                }
             } else {
-                evict_forward_flow = false;
+                /* no modification. hence...*/
+                rfe = NULL;
             }
-        } else {
-            rfe = NULL;
         }
     }
 
+    /*
+     * presence of rfe means that we might need to reset the evict bit
+     * or at the minimum reset the modified bit under failure conditions
+     */
     if (evict_forward_flow) {
         if (__vr_flow_mark_evict(router, fe)) {
             if (__vr_flow_schedule_transition(router, fe,
-                        index, fe->fe_flags) < 0) {
-                if (rfe)
-                    vr_flow_reset_evict(router, rfe);
-                vr_flow_reset_evict(router, fe);
+                        index, fe->fe_flags)) {
+                return;
+            } else {
+                goto reset_evict;
             }
-        } else {
-            goto unset_modified;
         }
     }
 
+    /* stop modifying the forward and the reverse */
+    if (rfe)
+        vr_flow_stop_modify(router, rfe);
+    vr_flow_stop_modify(router, fe);
+
     return;
 
-unset_modified:
+reset_evict:
     if (rfe)
-        vr_flow_stop_modify(router, rfe);
+        vr_flow_reset_evict(router, rfe);
+    vr_flow_reset_evict(router, fe);
 
     return;
 }