From 6721c99eaa6c9d83d7469f243ec383046de467e6 Mon Sep 17 00:00:00 2001
From: Alessandro Boch <aboch@cisco.com>
Date: Wed, 11 Sep 2013 09:35:36 -0700
Subject: [PATCH] On container removal FRM does not uninstall flows

ISSUE:
If a container had container flow specified, when it is removed FRM fails to uninstall
the flows that were installed in that container context

ROOT CAUSE:
On FRM stop(), the uninstallAllFlowEntries() function is called.
uninstallAllFlowEntries() does 2 things:
 1) it stores the original FlowEntry elements for later re-install irrespectively
it is called because FRM is being shutdown (container removal) or because controller
is moving to container mode (and this is the default container context) which is a bug.
 2) It uninstall the flows using the removeEntry(FlowEntry fe, ...) function which recomputes
the hw flows to be removed merging fe with the container flows for this container
returned by IContainer service, which is unefficient anyway.

Problem is that when FRM is being stopped because of container removal, IContainer service does no
longer have the container flows for the given container when queried by FRM.

FIX:
- Have uninstallAllFlowEntries() take a parameter to specify if the back up of the original flow entries is needed.
- Have uninstallAllFlowEntries() use the removeEntryInternal() to directly remove the FlowEntryInstall elements, because
  besides of the inefficinecy of recomputing the container flow merged entries there is really no need for it.
- Updated the java docs

Change-Id: If33d80f9593ee81fa58d84dea05514921e143225
Signed-off-by: Alessandro Boch <aboch@cisco.com>
---
 .../internal/ForwardingRulesManager.java      | 38 ++++++++++++++-----
 1 file changed, 28 insertions(+), 10 deletions(-)

diff --git a/opendaylight/forwardingrulesmanager/implementation/src/main/java/org/opendaylight/controller/forwardingrulesmanager/internal/ForwardingRulesManager.java b/opendaylight/forwardingrulesmanager/implementation/src/main/java/org/opendaylight/controller/forwardingrulesmanager/internal/ForwardingRulesManager.java
index 0d0874990e..be139317dc 100644
--- a/opendaylight/forwardingrulesmanager/implementation/src/main/java/org/opendaylight/controller/forwardingrulesmanager/internal/ForwardingRulesManager.java
+++ b/opendaylight/forwardingrulesmanager/implementation/src/main/java/org/opendaylight/controller/forwardingrulesmanager/internal/ForwardingRulesManager.java
@@ -1849,27 +1849,38 @@ public class ForwardingRulesManager implements
 
     /**
      * Uninstall all the non-internal Flow Entries present in the software view.
-     * A copy of each entry is stored in the inactive list so that it can be
-     * re-applied when needed. This function is called on the global instance of
-     * FRM only, when the first container is created
+     * If requested, a copy of each original flow entry will be stored in the
+     * inactive list so that it can be re-applied when needed (This is typically
+     * the case when running in the default container and controller moved to
+     * container mode)
+     *
+     * @param preserveFlowEntries
+     *            if true, a copy of each original entry is stored in the
+     *            inactive list
      */
-    private void uninstallAllFlowEntries() {
+    private void uninstallAllFlowEntries(boolean preserveFlowEntries) {
         log.info("Uninstalling all non-internal flows");
 
+        List<FlowEntryInstall> toRemove = new ArrayList<FlowEntryInstall>();
+
         // Store entries / create target list
         for (ConcurrentMap.Entry<FlowEntryInstall, FlowEntryInstall> mapEntry : installedSwView.entrySet()) {
             FlowEntryInstall flowEntries = mapEntry.getValue();
             // Skip internal generated static flows
             if (!flowEntries.isInternal()) {
-                inactiveFlows.put(flowEntries.getOriginal(), flowEntries.getOriginal());
+                toRemove.add(flowEntries);
+                // Store the original entries if requested
+                if (preserveFlowEntries) {
+                    inactiveFlows.put(flowEntries.getOriginal(), flowEntries.getOriginal());
+                }
             }
         }
 
         // Now remove the entries
-        for (FlowEntry flowEntry : inactiveFlows.keySet()) {
-            Status status = this.removeEntry(flowEntry, false);
+        for (FlowEntryInstall flowEntryHw : toRemove) {
+            Status status = this.removeEntryInternal(flowEntryHw, false);
             if (!status.isSuccess()) {
-                log.warn("Failed to remove entry: {}. The failure is: {}", flowEntry, status.getDescription());
+                log.warn("Failed to remove entry: {}. The failure is: {}", flowEntryHw, status.getDescription());
             }
         }
     }
@@ -2472,7 +2483,7 @@ public class ForwardingRulesManager implements
      */
     void stop() {
         stopping = true;
-        uninstallAllFlowEntries();
+        uninstallAllFlowEntries(false);
     }
 
     public void setFlowProgrammerService(IFlowProgrammerService service) {
@@ -2588,8 +2599,15 @@ public class ForwardingRulesManager implements
         }
         switch (update) {
         case ADDED:
+            /*
+             * Controller is moving to container mode. We are in the default
+             * container context, we need to remove all our non-internal flows
+             * to prevent any container isolation breakage. We also need to
+             * preserve our flow so that they can be re-installed if we move
+             * back to non container mode (no containers).
+             */
             this.inContainerMode = true;
-            this.uninstallAllFlowEntries();
+            this.uninstallAllFlowEntries(true);
             break;
         case REMOVED:
             this.inContainerMode = false;
-- 
2.36.6