Merge "In case of an error event, cleanup the config too"
[controller.git] / opendaylight / forwardingrulesmanager / implementation / src / main / java / org / opendaylight / controller / forwardingrulesmanager / internal / ForwardingRulesManager.java
index 0ac375668b9126caf7b2cfce26da5296706e4bb6..c2d1f46737683b4a0eb8a7339c7677ee7f0ca961 100644 (file)
@@ -28,7 +28,6 @@ import java.util.concurrent.ConcurrentMap;
 import java.util.concurrent.ExecutionException;
 import java.util.concurrent.ExecutorService;
 import java.util.concurrent.Executors;
-import java.util.concurrent.Future;
 import java.util.concurrent.LinkedBlockingQueue;
 
 import org.eclipse.osgi.framework.console.CommandInterpreter;
@@ -39,6 +38,7 @@ import org.opendaylight.controller.clustering.services.ICacheUpdateAware;
 import org.opendaylight.controller.clustering.services.IClusterContainerServices;
 import org.opendaylight.controller.clustering.services.IClusterServices;
 import org.opendaylight.controller.configuration.IConfigurationContainerAware;
+import org.opendaylight.controller.sal.connection.ConnectionLocality;
 import org.opendaylight.controller.connectionmanager.IConnectionManager;
 import org.opendaylight.controller.forwardingrulesmanager.FlowConfig;
 import org.opendaylight.controller.forwardingrulesmanager.FlowEntry;
@@ -56,6 +56,7 @@ import org.opendaylight.controller.sal.action.Controller;
 import org.opendaylight.controller.sal.action.Flood;
 import org.opendaylight.controller.sal.action.Output;
 import org.opendaylight.controller.sal.action.PopVlan;
+import org.opendaylight.controller.sal.core.Config;
 import org.opendaylight.controller.sal.core.ContainerFlow;
 import org.opendaylight.controller.sal.core.IContainer;
 import org.opendaylight.controller.sal.core.IContainerListener;
@@ -101,7 +102,7 @@ public class ForwardingRulesManager implements
         IConfigurationContainerAware,
         IInventoryListener,
         IObjectReader,
-        ICacheUpdateAware,
+        ICacheUpdateAware<Object,Object>,
         CommandProvider,
         IFlowProgrammerListener {
     private static final String NODEDOWN = "Node is Down";
@@ -201,59 +202,6 @@ public class ForwardingRulesManager implements
     private ConcurrentMap<FlowEntryDistributionOrder, FlowEntryDistributionOrderFutureTask> workMonitor =
             new ConcurrentHashMap<FlowEntryDistributionOrder, FlowEntryDistributionOrderFutureTask>();
 
-    /*
-     * Create an executor pool to create the distributionOrder, this is a stop
-     * gap solution caused by an issue with non-transactional caches in the
-     * implementation we use, being currently worked on. It has been noticed in
-     * fact that when non-transactional caches are being used sometime the key
-     * are no distributed to all the nodes properly. To workaround the issue
-     * transactional caches are being used, but there was a reason for using
-     * non-transactional caches to start with, in fact we needed to be able in
-     * the context of a northbound transaction to program the FRM entries
-     * irrespective of the fact that transaction would commit or no else we
-     * would not be able to achieve the entry programming and implement the
-     * scheme for recovery from network element failures. Bottom line, now in
-     * order to make sure an update on a transactional cache goes out while in a
-     * transaction that need to be initiated by a different thread.
-     */
-    private ExecutorService executor;
-
-    class DistributeOrderCallable implements Callable<Future<Status>> {
-        private FlowEntryInstall e;
-        private FlowEntryInstall u;
-        private UpdateType t;
-        DistributeOrderCallable(FlowEntryInstall e, FlowEntryInstall u, UpdateType t) {
-            this.e = e;
-            this.u = u;
-            this.t = t;
-        }
-
-        @Override
-        public Future<Status> call() throws Exception {
-            if (e == null || t == null) {
-                logsync.error("Unexpected null Entry up update type");
-                return null;
-            }
-            // Create the work order and distribute it
-            FlowEntryDistributionOrder fe =
-                    new FlowEntryDistributionOrder(e, t, clusterContainerService.getMyAddress());
-            // First create the monitor job
-            FlowEntryDistributionOrderFutureTask ret = new FlowEntryDistributionOrderFutureTask(fe);
-            logsync.trace("Node {} not local so sending fe {}", e.getNode(), fe);
-            workMonitor.put(fe, ret);
-            if (t.equals(UpdateType.CHANGED)) {
-                // Then distribute the work
-                workOrder.put(fe, u);
-            } else {
-                // Then distribute the work
-                workOrder.put(fe, e);
-            }
-            logsync.trace("WorkOrder requested");
-            // Now create an Handle to monitor the execution of the operation
-            return ret;
-        }
-    }
-
     /**
      * @param e
      *            Entry being installed/updated/removed
@@ -265,7 +213,8 @@ public class ForwardingRulesManager implements
      * @return a Future object for monitoring the progress of the result, or
      *         null in case the processing should take place locally
      */
-    private Future<Status> distributeWorkOrder(FlowEntryInstall e, FlowEntryInstall u, UpdateType t) {
+    private FlowEntryDistributionOrderFutureTask distributeWorkOrder(FlowEntryInstall e, FlowEntryInstall u,
+            UpdateType t) {
         // A null entry it's an unexpected condition, anyway it's safe to keep
         // the handling local
         if (e == null) {
@@ -273,25 +222,27 @@ public class ForwardingRulesManager implements
         }
 
         Node n = e.getNode();
-        if (!connectionManager.isLocal(n)) {
-            Callable<Future<Status>> worker = new DistributeOrderCallable(e, u, t);
-            if (worker != null) {
-                Future<Future<Status>> workerRes = this.executor.submit(worker);
-                try {
-                    return workerRes.get();
-                } catch (InterruptedException e1) {
-                    // we where interrupted, not a big deal.
-                    return null;
-                } catch (ExecutionException e1) {
-                    logsync.error(
-                            "We got an execution exception {} we cannot much, so returning we don't have nothing to wait for",
-                            e);
-                    return null;
-                }
+        if (connectionManager.getLocalityStatus(n) == ConnectionLocality.NOT_LOCAL) {
+            // Create the work order and distribute it
+            FlowEntryDistributionOrder fe =
+                    new FlowEntryDistributionOrder(e, t, clusterContainerService.getMyAddress());
+            // First create the monitor job
+            FlowEntryDistributionOrderFutureTask ret = new FlowEntryDistributionOrderFutureTask(fe);
+            logsync.trace("Node {} not local so sending fe {}", n, fe);
+            workMonitor.put(fe, ret);
+            if (t.equals(UpdateType.CHANGED)) {
+                // Then distribute the work
+                workOrder.put(fe, u);
+            } else {
+                // Then distribute the work
+                workOrder.put(fe, e);
             }
+            logsync.trace("WorkOrder requested");
+            // Now create an Handle to monitor the execution of the operation
+            return ret;
         }
 
-        logsync.trace("LOCAL Node {} so processing Entry:{} UpdateType:{}", n, e, t);
+        logsync.trace("Node {} could be local. so processing Entry:{} UpdateType:{}", n, e, t);
         return null;
     }
 
@@ -593,11 +544,17 @@ public class ForwardingRulesManager implements
      *         contain the unique id assigned to this request
      */
     private Status modifyEntryInternal(FlowEntryInstall currentEntries, FlowEntryInstall newEntries, boolean async) {
-        Future<Status> futureStatus = distributeWorkOrder(currentEntries, newEntries, UpdateType.CHANGED);
+        FlowEntryDistributionOrderFutureTask futureStatus =
+                distributeWorkOrder(currentEntries, newEntries, UpdateType.CHANGED);
         if (futureStatus != null) {
             Status retStatus = new Status(StatusCode.UNDEFINED);
             try {
                 retStatus = futureStatus.get();
+                if (retStatus.getCode()
+                        .equals(StatusCode.TIMEOUT)) {
+                    // A timeout happened, lets cleanup the workMonitor
+                    workMonitor.remove(futureStatus.getOrder());
+                }
             } catch (InterruptedException e) {
                 log.error("", e);
             } catch (ExecutionException e) {
@@ -705,11 +662,16 @@ public class ForwardingRulesManager implements
      *         contain the unique id assigned to this request
      */
     private Status removeEntryInternal(FlowEntryInstall entry, boolean async) {
-        Future<Status> futureStatus = distributeWorkOrder(entry, null, UpdateType.REMOVED);
+        FlowEntryDistributionOrderFutureTask futureStatus = distributeWorkOrder(entry, null, UpdateType.REMOVED);
         if (futureStatus != null) {
             Status retStatus = new Status(StatusCode.UNDEFINED);
             try {
                 retStatus = futureStatus.get();
+                if (retStatus.getCode()
+                        .equals(StatusCode.TIMEOUT)) {
+                    // A timeout happened, lets cleanup the workMonitor
+                    workMonitor.remove(futureStatus.getOrder());
+                }
             } catch (InterruptedException e) {
                 log.error("", e);
             } catch (ExecutionException e) {
@@ -753,11 +715,16 @@ public class ForwardingRulesManager implements
      *         contain the unique id assigned to this request
      */
     private Status addEntriesInternal(FlowEntryInstall entry, boolean async) {
-        Future<Status> futureStatus = distributeWorkOrder(entry, null, UpdateType.ADDED);
+        FlowEntryDistributionOrderFutureTask futureStatus = distributeWorkOrder(entry, null, UpdateType.ADDED);
         if (futureStatus != null) {
             Status retStatus = new Status(StatusCode.UNDEFINED);
             try {
                 retStatus = futureStatus.get();
+                if (retStatus.getCode()
+                        .equals(StatusCode.TIMEOUT)) {
+                    // A timeout happened, lets cleanup the workMonitor
+                    workMonitor.remove(futureStatus.getOrder());
+                }
             } catch (InterruptedException e) {
                 log.error("", e);
             } catch (ExecutionException e) {
@@ -815,6 +782,16 @@ public class ForwardingRulesManager implements
         return true;
     }
 
+    private ConcurrentMap.Entry<Integer, FlowConfig> getStaticFlowEntry(String name, Node node) {
+        for (ConcurrentMap.Entry<Integer, FlowConfig> flowEntry : staticFlows.entrySet()) {
+            FlowConfig flowConfig = flowEntry.getValue();
+            if (flowConfig.isByNameAndNodeIdEqual(name, node)) {
+                return flowEntry;
+            }
+        }
+        return null;
+    }
+
     private void updateLocalDatabase(FlowEntryInstall entry, boolean add) {
         // Update the software view
         updateSwViewes(entry, add);
@@ -1362,7 +1339,6 @@ public class ForwardingRulesManager implements
         retrieveCaches();
     }
 
-    @SuppressWarnings("deprecation")
     private void allocateCaches() {
         if (this.clusterContainerService == null) {
             log.warn("Un-initialized clusterContainerService, can't create cache");
@@ -1406,10 +1382,10 @@ public class ForwardingRulesManager implements
                     EnumSet.of(IClusterServices.cacheMode.TRANSACTIONAL));
 
             clusterContainerService.createCache(WORKSTATUSCACHE,
- EnumSet.of(IClusterServices.cacheMode.TRANSACTIONAL));
+ EnumSet.of(IClusterServices.cacheMode.NON_TRANSACTIONAL));
 
             clusterContainerService.createCache(WORKORDERCACHE,
- EnumSet.of(IClusterServices.cacheMode.TRANSACTIONAL));
+ EnumSet.of(IClusterServices.cacheMode.NON_TRANSACTIONAL));
 
         } catch (CacheConfigException cce) {
             log.error("CacheConfigException");
@@ -1418,7 +1394,7 @@ public class ForwardingRulesManager implements
         }
     }
 
-    @SuppressWarnings({ "unchecked", "deprecation" })
+    @SuppressWarnings({ "unchecked" })
     private void retrieveCaches() {
         ConcurrentMap<?, ?> map;
 
@@ -1867,9 +1843,13 @@ public class ForwardingRulesManager implements
             }
         }
         if (target != null) {
-            // Program the network node
-            Status status = (target.installInHw()) ? this.uninstallFlowEntry(target.getFlowEntry()) : this
-                    .installFlowEntry(target.getFlowEntry());
+            Status status = target.validate(container);
+            if (!status.isSuccess()) {
+                log.warn(status.getDescription());
+                return status;
+            }
+            status = (target.installInHw()) ? this.uninstallFlowEntry(target.getFlowEntry()) : this
+                                    .installFlowEntry(target.getFlowEntry());
             if (status.isSuccess()) {
                 // Update Configuration database
                 target.setStatus(SUCCESS);
@@ -1976,10 +1956,9 @@ public class ForwardingRulesManager implements
 
     @Override
     public FlowConfig getStaticFlow(String name, Node node) {
-        for (ConcurrentMap.Entry<Integer, FlowConfig> entry : staticFlows.entrySet()) {
-            if (entry.getValue().isByNameAndNodeIdEqual(name, node)) {
-                return entry.getValue();
-            }
+        ConcurrentMap.Entry<Integer, FlowConfig> entry = getStaticFlowEntry(name, node);
+        if(entry != null) {
+            return entry.getValue();
         }
         return null;
     }
@@ -2096,51 +2075,79 @@ public class ForwardingRulesManager implements
         addStaticFlowInternal(allowARP, true); // skip validation on internal static flow name
     }
 
+    /**
+     * (non-Javadoc)
+     *
+     * @see org.opendaylight.controller.switchmanager.ISwitchManagerAware#modeChangeNotify(org.opendaylight.controller.sal.core.Node,
+     *      boolean)
+     *
+     *      This method can be called from within the OSGi framework context,
+     *      given the programming operation can take sometime, it not good
+     *      pratice to have in it's context operations that can take time,
+     *      hence moving off to a different thread for async processing.
+     */
+    private ExecutorService executor;
     @Override
-    public void modeChangeNotify(Node node, boolean proactive) {
-        List<FlowConfig> defaultConfigs = new ArrayList<FlowConfig>();
-
-        List<String> puntAction = new ArrayList<String>();
-        puntAction.add(ActionType.CONTROLLER.toString());
-
-        FlowConfig allowARP = new FlowConfig();
-        allowARP.setInstallInHw(true);
-        allowARP.setName(FlowConfig.INTERNALSTATICFLOWBEGIN + "Punt ARP" + FlowConfig.INTERNALSTATICFLOWEND);
-        allowARP.setPriority("1");
-        allowARP.setNode(node);
-        allowARP.setEtherType("0x" + Integer.toHexString(EtherTypes.ARP.intValue()).toUpperCase());
-        allowARP.setActions(puntAction);
-        defaultConfigs.add(allowARP);
-
-        FlowConfig allowLLDP = new FlowConfig();
-        allowLLDP.setInstallInHw(true);
-        allowLLDP.setName(FlowConfig.INTERNALSTATICFLOWBEGIN + "Punt LLDP" + FlowConfig.INTERNALSTATICFLOWEND);
-        allowLLDP.setPriority("1");
-        allowLLDP.setNode(node);
-        allowLLDP.setEtherType("0x" + Integer.toHexString(EtherTypes.LLDP.intValue()).toUpperCase());
-        allowLLDP.setActions(puntAction);
-        defaultConfigs.add(allowLLDP);
-
-        List<String> dropAction = new ArrayList<String>();
-        dropAction.add(ActionType.DROP.toString());
-
-        FlowConfig dropAllConfig = new FlowConfig();
-        dropAllConfig.setInstallInHw(true);
-        dropAllConfig.setName(FlowConfig.INTERNALSTATICFLOWBEGIN + "Catch-All Drop" + FlowConfig.INTERNALSTATICFLOWEND);
-        dropAllConfig.setPriority("0");
-        dropAllConfig.setNode(node);
-        dropAllConfig.setActions(dropAction);
-        defaultConfigs.add(dropAllConfig);
-
-        log.info("Forwarding mode for node {} set to {}", node, (proactive ? "proactive" : "reactive"));
-        for (FlowConfig fc : defaultConfigs) {
-            Status status = (proactive) ? addStaticFlowInternal(fc, false) : removeStaticFlow(fc);
-            if (status.isSuccess()) {
-                log.info("{} Proactive Static flow: {}", (proactive ? "Installed" : "Removed"), fc.getName());
-            } else {
-                log.warn("Failed to {} Proactive Static flow: {}", (proactive ? "install" : "remove"), fc.getName());
+    public void modeChangeNotify(final Node node, final boolean proactive) {
+        Callable<Status> modeChangeCallable = new Callable<Status>() {
+            @Override
+            public Status call() throws Exception {
+                List<FlowConfig> defaultConfigs = new ArrayList<FlowConfig>();
+
+                List<String> puntAction = new ArrayList<String>();
+                puntAction.add(ActionType.CONTROLLER.toString());
+
+                FlowConfig allowARP = new FlowConfig();
+                allowARP.setInstallInHw(true);
+                allowARP.setName(FlowConfig.INTERNALSTATICFLOWBEGIN + "Punt ARP" + FlowConfig.INTERNALSTATICFLOWEND);
+                allowARP.setPriority("1");
+                allowARP.setNode(node);
+                allowARP.setEtherType("0x" + Integer.toHexString(EtherTypes.ARP.intValue())
+                        .toUpperCase());
+                allowARP.setActions(puntAction);
+                defaultConfigs.add(allowARP);
+
+                FlowConfig allowLLDP = new FlowConfig();
+                allowLLDP.setInstallInHw(true);
+                allowLLDP.setName(FlowConfig.INTERNALSTATICFLOWBEGIN + "Punt LLDP" + FlowConfig.INTERNALSTATICFLOWEND);
+                allowLLDP.setPriority("1");
+                allowLLDP.setNode(node);
+                allowLLDP.setEtherType("0x" + Integer.toHexString(EtherTypes.LLDP.intValue())
+                        .toUpperCase());
+                allowLLDP.setActions(puntAction);
+                defaultConfigs.add(allowLLDP);
+
+                List<String> dropAction = new ArrayList<String>();
+                dropAction.add(ActionType.DROP.toString());
+
+                FlowConfig dropAllConfig = new FlowConfig();
+                dropAllConfig.setInstallInHw(true);
+                dropAllConfig.setName(FlowConfig.INTERNALSTATICFLOWBEGIN + "Catch-All Drop"
+                        + FlowConfig.INTERNALSTATICFLOWEND);
+                dropAllConfig.setPriority("0");
+                dropAllConfig.setNode(node);
+                dropAllConfig.setActions(dropAction);
+                defaultConfigs.add(dropAllConfig);
+
+                log.info("Forwarding mode for node {} set to {}", node, (proactive ? "proactive" : "reactive"));
+                for (FlowConfig fc : defaultConfigs) {
+                    Status status = (proactive) ? addStaticFlowInternal(fc, false) : removeStaticFlow(fc);
+                    if (status.isSuccess()) {
+                        log.info("{} Proactive Static flow: {}", (proactive ? "Installed" : "Removed"), fc.getName());
+                    } else {
+                        log.warn("Failed to {} Proactive Static flow: {}", (proactive ? "install" : "remove"),
+                                fc.getName());
+                    }
+                }
+                return new Status(StatusCode.SUCCESS);
             }
-        }
+        };
+
+        /*
+         * Execute the work outside the caller context, this could be an
+         * expensive operation and we don't want to block the caller for it.
+         */
+        this.executor.submit(modeChangeCallable);
     }
 
     /**
@@ -2192,7 +2199,99 @@ public class ForwardingRulesManager implements
 
     @Override
     public void notifyNodeConnector(NodeConnector nodeConnector, UpdateType type, Map<String, Property> propMap) {
+        boolean updateStaticFlowCluster = false;
+
+        switch (type) {
+        case ADDED:
+            break;
+        case CHANGED:
+            Config config = (propMap == null) ? null : (Config) propMap.get(Config.ConfigPropName);
+            if (config != null) {
+                switch (config.getValue()) {
+                case Config.ADMIN_DOWN:
+                    log.trace("Port {} is administratively down: uninstalling interested flows", nodeConnector);
+                    updateStaticFlowCluster = removeFlowsOnNodeConnectorDown(nodeConnector);
+                    break;
+                case Config.ADMIN_UP:
+                    log.trace("Port {} is administratively up: installing interested flows", nodeConnector);
+                    updateStaticFlowCluster = installFlowsOnNodeConnectorUp(nodeConnector);
+                    break;
+                case Config.ADMIN_UNDEF:
+                    break;
+                default:
+                }
+            }
+            break;
+        case REMOVED:
+            // This is the case where a switch port is removed from the SDN agent space
+            log.trace("Port {} was removed from our control: uninstalling interested flows", nodeConnector);
+            updateStaticFlowCluster = removeFlowsOnNodeConnectorDown(nodeConnector);
+            break;
+        default:
+
+        }
+
+        if (updateStaticFlowCluster) {
+            refreshClusterStaticFlowsStatus(nodeConnector.getNode());
+        }
+    }
 
+    /*
+     * It goes through the static flows configuration, it identifies the ones
+     * which have the specified node connector as input or output port and
+     * install them on the network node if they are marked to be installed in
+     * hardware and their status shows they were not installed yet
+     */
+    private boolean installFlowsOnNodeConnectorUp(NodeConnector nodeConnector) {
+        boolean updated = false;
+        List<FlowConfig> flowConfigForNode = getStaticFlows(nodeConnector.getNode());
+        for (FlowConfig flowConfig : flowConfigForNode) {
+            if (doesFlowContainNodeConnector(flowConfig.getFlow(), nodeConnector)) {
+                if (flowConfig.installInHw() && !flowConfig.getStatus().equals(SUCCESS)) {
+                    Status status = this.installFlowEntry(flowConfig.getFlowEntry());
+                    if (!status.isSuccess()) {
+                        flowConfig.setStatus(status.getDescription());
+                    } else {
+                        flowConfig.setStatus(SUCCESS);
+                    }
+                    updated = true;
+                }
+            }
+        }
+        return updated;
+    }
+
+    /*
+     * Remove from the network node all the flows which have the specified node
+     * connector as input or output port. If any of the flow entry is a static
+     * flow, it updates the correspondent configuration.
+     */
+    private boolean removeFlowsOnNodeConnectorDown(NodeConnector nodeConnector) {
+        boolean updated = false;
+        List<FlowEntryInstall> nodeFlowEntries = nodeFlows.get(nodeConnector.getNode());
+        if (nodeFlowEntries == null) {
+            return updated;
+        }
+        for (FlowEntryInstall fei : new ArrayList<FlowEntryInstall>(nodeFlowEntries)) {
+            if (doesFlowContainNodeConnector(fei.getInstall().getFlow(), nodeConnector)) {
+                Status status = this.removeEntryInternal(fei, true);
+                if (!status.isSuccess()) {
+                    continue;
+                }
+                /*
+                 * If the flow entry is a static flow, then update its
+                 * configuration
+                 */
+                if (fei.getGroupName().equals(FlowConfig.STATICFLOWGROUP)) {
+                    FlowConfig flowConfig = getStaticFlow(fei.getFlowName(), fei.getNode());
+                    if (flowConfig != null) {
+                        flowConfig.setStatus(PORTREMOVED);
+                        updated = true;
+                    }
+                }
+            }
+        }
+        return updated;
     }
 
     private FlowConfig getDerivedFlowConfig(FlowConfig original, String configName, Short port) {
@@ -2480,6 +2579,14 @@ public class ForwardingRulesManager implements
                             } else {
                                 log.warn("Not expected null WorkStatus", work);
                             }
+                        }  else if (event instanceof ContainerFlowChangeEvent) {
+                            /*
+                             * Whether it is an addition or removal, we have to
+                             * recompute the merged flows entries taking into
+                             * account all the current container flows because
+                             * flow merging is not an injective function
+                             */
+                            updateFlowsContainerFlow();
                         } else {
                             log.warn("Dequeued unknown event {}", event.getClass()
                                     .getSimpleName());
@@ -2517,6 +2624,9 @@ public class ForwardingRulesManager implements
         // Initialize graceful stop flag
         stopping = false;
 
+        // Allocate the executor service
+        this.executor = Executors.newSingleThreadExecutor();
+
         // Start event handler thread
         frmEventHandler.start();
 
@@ -2527,9 +2637,6 @@ public class ForwardingRulesManager implements
         if (staticFlows.isEmpty()) {
             loadFlowConfiguration();
         }
-
-        // Allocate the executor service
-        this.executor = Executors.newSingleThreadExecutor();
     }
 
     /**
@@ -2578,12 +2685,8 @@ public class ForwardingRulesManager implements
         }
         log.trace("Container {}: Updating installed flows because of container flow change: {} {}",
                 container.getName(), t, current);
-        /*
-         * Whether it is an addition or removal, we have to recompute the merged
-         * flows entries taking into account all the current container flows
-         * because flow merging is not an injective function
-         */
-        updateFlowsContainerFlow();
+        ContainerFlowChangeEvent ev = new ContainerFlowChangeEvent(previous, current, t);
+        pendingEvents.offer(ev);
     }
 
     @Override
@@ -2596,57 +2699,22 @@ public class ForwardingRulesManager implements
 
         switch (t) {
         case REMOVED:
-
-            List<FlowEntryInstall> nodeFlowEntries = nodeFlows.get(nc.getNode());
-            if (nodeFlowEntries == null) {
-                return;
-            }
-            for (FlowEntryInstall fei : new ArrayList<FlowEntryInstall>(nodeFlowEntries)) {
-                if (doesFlowContainNodeConnector(fei.getInstall().getFlow(), nc)) {
-                    Status status = this.removeEntryInternal(fei, true);
-                    if (!status.isSuccess()) {
-                        continue;
-                    }
-                    /*
-                     * If the flow entry is a static flow, then update its
-                     * configuration
-                     */
-                    if (fei.getGroupName().equals(FlowConfig.STATICFLOWGROUP)) {
-                        FlowConfig flowConfig = getStaticFlow(fei.getFlowName(), fei.getNode());
-                        if (flowConfig != null) {
-                            flowConfig.setStatus(PORTREMOVED);
-                            updateStaticFlowCluster = true;
-                        }
-                    }
-                }
-            }
-            if (updateStaticFlowCluster) {
-                refreshClusterStaticFlowsStatus(nc.getNode());
-            }
+            log.trace("Port {} was removed from container: uninstalling interested flows", nc);
+            updateStaticFlowCluster = removeFlowsOnNodeConnectorDown(nc);
             break;
         case ADDED:
-            List<FlowConfig> flowConfigForNode = getStaticFlows(nc.getNode());
-            for (FlowConfig flowConfig : flowConfigForNode) {
-                if (doesFlowContainNodeConnector(flowConfig.getFlow(), nc)) {
-                    if (flowConfig.installInHw()) {
-                        Status status = this.installFlowEntry(flowConfig.getFlowEntry());
-                        if (!status.isSuccess()) {
-                            flowConfig.setStatus(status.getDescription());
-                        } else {
-                            flowConfig.setStatus(SUCCESS);
-                        }
-                        updateStaticFlowCluster = true;
-                    }
-                }
-            }
-            if (updateStaticFlowCluster) {
-                refreshClusterStaticFlowsStatus(nc.getNode());
-            }
+            log.trace("Port {} was added to container: reinstall interested flows", nc);
+            updateStaticFlowCluster = installFlowsOnNodeConnectorUp(nc);
+
             break;
         case CHANGED:
             break;
         default:
         }
+
+        if (updateStaticFlowCluster) {
+            refreshClusterStaticFlowsStatus(nc.getNode());
+        }
     }
 
     @Override
@@ -2751,6 +2819,30 @@ public class ForwardingRulesManager implements
             return newEntry;
         }
     }
+    private class ContainerFlowChangeEvent extends FRMEvent {
+        private final ContainerFlow previous;
+        private final ContainerFlow current;
+        private final UpdateType type;
+
+        public ContainerFlowChangeEvent(ContainerFlow previous, ContainerFlow current, UpdateType type) {
+            this.previous = previous;
+            this.current = current;
+            this.type = type;
+        }
+
+        public ContainerFlow getPrevious() {
+            return this.previous;
+        }
+
+        public ContainerFlow getCurrent() {
+            return this.current;
+        }
+
+        public UpdateType getType() {
+            return this.type;
+        }
+    }
+
 
     private class WorkStatusCleanup extends FRMEvent {
         private FlowEntryDistributionOrder fe;
@@ -2948,6 +3040,36 @@ public class ForwardingRulesManager implements
         }
     }
 
+    public void _frmProcessErrorEvent(CommandInterpreter ci) throws UnknownHostException {
+        Node node = null;
+        long reqId = 0L;
+        String nodeId = ci.nextArgument();
+        if (nodeId == null) {
+            ci.print("Node id not specified");
+            return;
+        }
+        String requestId = ci.nextArgument();
+        if (requestId == null) {
+            ci.print("Request id not specified");
+            return;
+        }
+        try {
+            node = NodeCreator.createOFNode(Long.valueOf(nodeId));
+        } catch (NumberFormatException e) {
+            ci.print("Node id not a number");
+            return;
+        }
+        try {
+            reqId = Long.parseLong(requestId);
+        } catch (NumberFormatException e) {
+            ci.print("Request id not a number");
+            return;
+        }
+        // null for error object is good enough for now
+        ErrorReportedEvent event = new ErrorReportedEvent(reqId, node, null);
+        this.processErrorEvent(event);
+    }
+
     @Override
     public void flowRemoved(Node node, Flow flow) {
         log.trace("Received flow removed notification on {} for {}", node, flow);
@@ -3000,16 +3122,32 @@ public class ForwardingRulesManager implements
          * mapping will have to be added in future
          */
         FlowEntryInstall target = null;
-        for (FlowEntryInstall index : nodeFlows.get(node)) {
-            FlowEntryInstall entry = installedSwView.get(index);
-            if (entry.getRequestId() == rid) {
-                target = entry;
-                break;
+        List<FlowEntryInstall> flowEntryInstallList = nodeFlows.get(node);
+        // flowEntryInstallList could be null.
+        // so check for it.
+        if(flowEntryInstallList != null) {
+            for (FlowEntryInstall index : flowEntryInstallList) {
+                FlowEntryInstall entry = installedSwView.get(index);
+                if(entry != null) {
+                    if (entry.getRequestId() == rid) {
+                        target = entry;
+                        break;
+                    }
+                }
             }
         }
         if (target != null) {
             // This was a flow install, update database
             this.updateLocalDatabase(target, false);
+            // also update the config
+            if(FlowConfig.STATICFLOWGROUP.equals(target.getGroupName())) {
+                ConcurrentMap.Entry<Integer, FlowConfig> staticFlowEntry = getStaticFlowEntry(target.getFlowName(),target.getNode());
+                // staticFlowEntry should never be null.
+                // the null check is just an extra defensive check.
+                if(staticFlowEntry != null) {
+                    staticFlows.remove(staticFlowEntry.getKey());
+                }
+            }
         }
 
         // Notify listeners
@@ -3078,7 +3216,7 @@ public class ForwardingRulesManager implements
                 return;
             }
             Node n = fei.getNode();
-            if (connectionManager.isLocal(n)) {
+            if (connectionManager.getLocalityStatus(n) == ConnectionLocality.LOCAL) {
                 logsync.trace("workOrder for fe {} processed locally", fe);
                 // I'm the controller in charge for the request, queue it for
                 // processing
@@ -3097,7 +3235,7 @@ public class ForwardingRulesManager implements
              */
             if (fe.getRequestorController()
                     .equals(clusterContainerService.getMyAddress())) {
-                FlowEntryDistributionOrderFutureTask fet = workMonitor.get(fe);
+                FlowEntryDistributionOrderFutureTask fet = workMonitor.remove(fe);
                 if (fet != null) {
                     logsync.trace("workStatus response is for us {}", fe);
                     // Signal we got the status