From 667b1c95a0c08043c4d176463b175aefd66feb5a Mon Sep 17 00:00:00 2001
From: Kanzhe Jiang <kanzhe.jiang@bigswitch.com>
Date: Mon, 8 Oct 2012 22:48:12 -0700
Subject: [PATCH] fix queuing of flows to be reconciled

---
 .../flowcache/FlowReconcileManager.java       | 14 ++++-
 .../flowcache/OFMatchReconcile.java           | 61 +++++++++++--------
 .../flowcache/FlowReconcileMgrTest.java       | 61 +++++++++++++++++++
 3 files changed, 109 insertions(+), 27 deletions(-)

diff --git a/src/main/java/net/floodlightcontroller/flowcache/FlowReconcileManager.java b/src/main/java/net/floodlightcontroller/flowcache/FlowReconcileManager.java
index 905468074..d5d323d75 100644
--- a/src/main/java/net/floodlightcontroller/flowcache/FlowReconcileManager.java
+++ b/src/main/java/net/floodlightcontroller/flowcache/FlowReconcileManager.java
@@ -109,8 +109,11 @@ public class FlowReconcileManager
      */
     public void reconcileFlow(OFMatchReconcile ofmRcIn) {
         if (ofmRcIn == null) return;
+        
+        // Make a copy before putting on the queue.
+        OFMatchReconcile myOfmRc = new OFMatchReconcile(ofmRcIn);
     
-        flowQueue.add(ofmRcIn);
+        flowQueue.add(myOfmRc);
     
         Date currTime = new Date();
         long delay = 0;
@@ -128,7 +131,7 @@ public class FlowReconcileManager
     
         if (logger.isTraceEnabled()) {
             logger.trace("Reconciling flow: {}, total: {}",
-                    ofmRcIn.toString(), flowQueue.size());
+                myOfmRc.toString(), flowQueue.size());
         }
     }
     
@@ -270,10 +273,15 @@ public class FlowReconcileManager
             logger.trace("Reconcile capacity {} flows", reconcileCapacity);
         }
         while (!flowQueue.isEmpty() && reconcileCapacity > 0) {
-            OFMatchReconcile ofmRc = flowQueue.remove();
+            OFMatchReconcile ofmRc = flowQueue.poll();
             reconcileCapacity--;
             if (ofmRc != null) {
                 ofmRcList.add(ofmRc);
+                if (logger.isTraceEnabled()) {
+                    logger.trace("Add flow {} to be the reconcileList", ofmRc.cookie);
+                }
+            } else {
+                break;
             }
         }
         
diff --git a/src/main/java/net/floodlightcontroller/flowcache/OFMatchReconcile.java b/src/main/java/net/floodlightcontroller/flowcache/OFMatchReconcile.java
index f69ba12a5..68831f46e 100644
--- a/src/main/java/net/floodlightcontroller/flowcache/OFMatchReconcile.java
+++ b/src/main/java/net/floodlightcontroller/flowcache/OFMatchReconcile.java
@@ -7,31 +7,31 @@ import org.openflow.protocol.OFMatchWithSwDpid;
  * OFMatchReconcile class to indicate result of a flow-reconciliation.
  */
 public class OFMatchReconcile  {
-	
-	/**
-	 * The enum ReconcileAction. Specifies the result of reconciliation of a 
-	 * flow.
-	 */
-	public enum ReconcileAction {
+ 
+    /**
+     * The enum ReconcileAction. Specifies the result of reconciliation of a 
+     * flow.
+     */
+    public enum ReconcileAction {
+
+        /** Delete the flow-mod from the switch */
+        DROP,
+        /** Leave the flow-mod as-is. */
+        NO_CHANGE,
+        /** Program this new flow mod. */
+        NEW_ENTRY,
+        /** 
+         * Reprogram the flow mod as the path of the flow might have changed,
+         * for example when a host is moved or when a link goes down. */
+        UPDATE_PATH,
+        /* Flow is now in a different BVS */
+        APP_INSTANCE_CHANGED,
+        /* Delete the flow-mod - used to delete, for example, drop flow-mods
+         * when the source and destination are in the same BVS after a 
+         * configuration change */
+        DELETE
+    }
 
-	    /** Delete the flow-mod from the switch */
-	    DROP,
-	    /** Leave the flow-mod as-is. */
-	    NO_CHANGE,
-	    /** Program this new flow mod. */
-	    NEW_ENTRY,
-	    /** 
-	     * Reprogram the flow mod as the path of the flow might have changed,
-	     * for example when a host is moved or when a link goes down. */
-	    UPDATE_PATH,
-	    /* Flow is now in a different BVS */
-	    APP_INSTANCE_CHANGED,
-	    /* Delete the flow-mod - used to delete, for example, drop flow-mods
-	     * when the source and destination are in the same BVS after a 
-	     * configuration change */
-	    DELETE
-	}
-	
     /** The open flow match after reconciliation. */
     public OFMatchWithSwDpid ofmWithSwDpid;
     /** flow mod. priority */
@@ -62,6 +62,19 @@ public class OFMatchReconcile  {
         cntx = new FloodlightContext();
     }
     
+    public OFMatchReconcile(OFMatchReconcile copy) {
+        ofmWithSwDpid =
+            new OFMatchWithSwDpid(copy.ofmWithSwDpid.getOfMatch(),
+                    copy.ofmWithSwDpid.getSwitchDataPathId());
+        priority = copy.priority;
+        action = copy.action;
+        cookie = copy.cookie;
+        appInstName = copy.appInstName;
+        newAppInstName = copy.newAppInstName;
+        rcAction = copy.rcAction;
+        cntx = new FloodlightContext();
+    }
+    
     @Override
     public String toString() {
         return "OFMatchReconcile [" + ofmWithSwDpid + " priority=" + priority + " action=" + action + 
diff --git a/src/test/java/net/floodlightcontroller/flowcache/FlowReconcileMgrTest.java b/src/test/java/net/floodlightcontroller/flowcache/FlowReconcileMgrTest.java
index c37f6cc6a..042782804 100644
--- a/src/test/java/net/floodlightcontroller/flowcache/FlowReconcileMgrTest.java
+++ b/src/test/java/net/floodlightcontroller/flowcache/FlowReconcileMgrTest.java
@@ -4,6 +4,7 @@ import static org.easymock.EasyMock.*;
 
 import java.util.ArrayList;
 import java.util.Date;
+import java.util.ListIterator;
 
 import net.floodlightcontroller.core.IListener.Command;
 import net.floodlightcontroller.core.module.FloodlightModuleContext;
@@ -375,6 +376,66 @@ public class FlowReconcileMgrTest extends FloodlightTestCase {
         }
     }
     
+    /** Verify the flows are sent to the reconcile pipeline in order.
+     */
+    @SuppressWarnings("unchecked")
+    @Test
+    public void testQueueFlowsOrder() {
+        flowReconcileMgr.flowReconcileEnabled = false;
+        
+        IFlowReconcileListener r1 =
+            EasyMock.createNiceMock(IFlowReconcileListener.class);
+        
+        expect(r1.getName()).andReturn("r1").anyTimes();
+        
+        // Set the listeners' order: r1 -> r2 -> r3
+        expect(r1.isCallbackOrderingPrereq((OFType)anyObject(),
+            (String)anyObject())).andReturn(false).anyTimes();
+        expect(r1.isCallbackOrderingPostreq((OFType)anyObject(),
+            (String)anyObject())).andReturn(false).anyTimes();
+        
+        expect(r1.reconcileFlows((ArrayList<OFMatchReconcile>)anyObject()))
+        .andAnswer(new IAnswer<Command>() {
+            @Override
+            public Command answer() throws Throwable {
+                ArrayList<OFMatchReconcile> ofmList =
+                    (ArrayList<OFMatchReconcile>)EasyMock.
+                        getCurrentArguments()[0];
+                ListIterator<OFMatchReconcile> lit = ofmList.listIterator();
+                int index = 0;
+                while (lit.hasNext()) {
+                    OFMatchReconcile ofm = lit.next();
+                    assertEquals(index++, ofm.cookie);
+                }
+                return Command.STOP;
+            }
+        }).times(1);
+        
+        SimpleCounter cnt = (SimpleCounter)SimpleCounter.createCounter(
+                            new Date(),
+                            CounterType.LONG);
+        cnt.increment();
+        expect(counterStore.getCounter(
+                flowReconcileMgr.controllerPktInCounterName))
+                .andReturn(cnt)
+                .anyTimes();
+        
+        replay(r1, counterStore);
+        flowReconcileMgr.clearFlowReconcileListeners();
+        flowReconcileMgr.addFlowReconcileListener(r1);
+        
+        OFMatchReconcile ofmRcIn = new OFMatchReconcile();
+        int index = 0;
+        for (index = 0; index < 10; index++) {
+            ofmRcIn.cookie = index;
+            flowReconcileMgr.reconcileFlow(ofmRcIn);
+        }
+        flowReconcileMgr.flowReconcileEnabled = true;
+        flowReconcileMgr.doReconcile();
+        
+        verify(r1);
+    }
+    
     @SuppressWarnings("unchecked")
     @Test
     public void testQueueFlowsByManyThreads() {
-- 
GitLab