fix for frm to not act on duplicate mode change notifications
[controller.git] / opendaylight / forwardingrulesmanager / implementation / src / main / java / org / opendaylight / controller / forwardingrulesmanager / internal / ForwardingRulesManager.java
index 665ba7c635781968458dce7b026fc4bf6477d449..9c002206879f1a8cff46a9d1df2bf6c6a9685185 100644 (file)
@@ -11,7 +11,6 @@ package org.opendaylight.controller.forwardingrulesmanager.internal;
 import java.io.FileNotFoundException;
 import java.io.IOException;
 import java.io.ObjectInputStream;
-import java.net.UnknownHostException;
 import java.util.ArrayList;
 import java.util.Collections;
 import java.util.EnumSet;
@@ -29,8 +28,6 @@ import java.util.concurrent.ExecutorService;
 import java.util.concurrent.Executors;
 import java.util.concurrent.LinkedBlockingQueue;
 
-import org.eclipse.osgi.framework.console.CommandInterpreter;
-import org.eclipse.osgi.framework.console.CommandProvider;
 import org.opendaylight.controller.clustering.services.CacheConfigException;
 import org.opendaylight.controller.clustering.services.CacheExistException;
 import org.opendaylight.controller.clustering.services.ICacheUpdateAware;
@@ -69,7 +66,6 @@ import org.opendaylight.controller.sal.match.MatchType;
 import org.opendaylight.controller.sal.utils.EtherTypes;
 import org.opendaylight.controller.sal.utils.GlobalConstants;
 import org.opendaylight.controller.sal.utils.IObjectReader;
-import org.opendaylight.controller.sal.utils.NodeCreator;
 import org.opendaylight.controller.sal.utils.ObjectReader;
 import org.opendaylight.controller.sal.utils.ObjectWriter;
 import org.opendaylight.controller.sal.utils.Status;
@@ -78,8 +74,6 @@ import org.opendaylight.controller.switchmanager.IInventoryListener;
 import org.opendaylight.controller.switchmanager.ISwitchManager;
 import org.opendaylight.controller.switchmanager.ISwitchManagerAware;
 import org.opendaylight.controller.switchmanager.Subnet;
-import org.osgi.framework.BundleContext;
-import org.osgi.framework.FrameworkUtil;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 
@@ -97,7 +91,6 @@ public class ForwardingRulesManager implements
         IInventoryListener,
         IObjectReader,
         ICacheUpdateAware<Object,Object>,
-        CommandProvider,
         IFlowProgrammerListener {
 
     private static final Logger log = LoggerFactory.getLogger(ForwardingRulesManager.class);
@@ -474,7 +467,7 @@ public class ForwardingRulesManager implements
         Status succeeded = null;
         boolean decouple = false;
         if (installedList.size() != toInstallList.size()) {
-            log.info("Modify: New flow entry does not satisfy the same "
+            log.trace("Modify: New flow entry does not satisfy the same "
                     + "number of container flows as the original entry does");
             decouple = true;
         }
@@ -486,7 +479,7 @@ public class ForwardingRulesManager implements
              */
             FlowEntryInstall sameMatchEntry = installedSwView.get(installEntry);
             if (sameMatchEntry != null && !sameMatchEntry.getOriginal().equals(currentFlowEntry)) {
-                log.info("Modify: new container flow merged flow entry clashes with existing flow");
+                log.trace("Modify: new container flow merged flow entry clashes with existing flow");
                 decouple = true;
             } else {
                 toInstallSafe.add(installEntry);
@@ -854,6 +847,17 @@ public class ForwardingRulesManager implements
         }
 
         if (add) {
+            // there may be an already existing entry.
+            // remove it before adding the new one.
+            // This is necessary since we have observed that in some cases
+            // Infinispan does aggregation for operations (eg:- remove and then put a different value)
+            // related to the same key within the same transaction.
+            // Need this defensive code as the new FlowEntryInstall may be different
+            // than the old one even though the equals method returns true. This is because
+            // the equals method does not take into account the action list.
+            if(nodeIndeces.contains(flowEntries)) {
+                nodeIndeces.remove(flowEntries);
+            }
             nodeIndeces.add(flowEntries);
         } else {
             nodeIndeces.remove(flowEntries);
@@ -888,6 +892,11 @@ public class ForwardingRulesManager implements
         }
 
         if (add) {
+            // same comments in the similar code section in
+            // updateNodeFlowsDB method apply here too
+            if(indices.contains(flowEntries)) {
+                indices.remove(flowEntries);
+            }
             indices.add(flowEntries);
         } else {
             indices.remove(flowEntries);
@@ -1178,11 +1187,6 @@ public class ForwardingRulesManager implements
         inactiveFlows = new ConcurrentHashMap<FlowEntry, FlowEntry>();
     }
 
-    private void registerWithOSGIConsole() {
-        BundleContext bundleContext = FrameworkUtil.getBundle(this.getClass()).getBundleContext();
-        bundleContext.registerService(CommandProvider.class.getName(), this, null);
-    }
-
     @Override
     public void setTSPolicyData(String policyname, Object o, boolean add) {
 
@@ -1227,7 +1231,7 @@ public class ForwardingRulesManager implements
         if (policyName != null && !policyName.trim().isEmpty()) {
             for (Map.Entry<FlowEntry, FlowEntry> entry : this.originalSwView.entrySet()) {
                 if (policyName.equals(entry.getKey().getGroupName())) {
-                    list.add(entry.getKey().clone());
+                    list.add(entry.getValue().clone());
                 }
             }
         }
@@ -1240,7 +1244,7 @@ public class ForwardingRulesManager implements
         if (policyName != null && !policyName.trim().isEmpty()) {
             for (Map.Entry<FlowEntryInstall, FlowEntryInstall> entry : this.installedSwView.entrySet()) {
                 if (policyName.equals(entry.getKey().getGroupName())) {
-                    list.add(entry.getKey().getInstall().clone());
+                    list.add(entry.getValue().getInstall().clone());
                 }
             }
         }
@@ -1259,7 +1263,7 @@ public class ForwardingRulesManager implements
                 }
                 Status error = modifyEntry(currentFlowEntry, newFlowEntry, false);
                 if (error.isSuccess()) {
-                    log.info("Ports {} added to FlowEntry {}", portList, flowName);
+                    log.trace("Ports {} added to FlowEntry {}", portList, flowName);
                 } else {
                     log.warn("Failed to add ports {} to Flow entry {}. The failure is: {}", portList,
                             currentFlowEntry.toString(), error.getDescription());
@@ -1283,7 +1287,7 @@ public class ForwardingRulesManager implements
                 }
                 Status status = modifyEntry(currentFlowEntry, newFlowEntry, false);
                 if (status.isSuccess()) {
-                    log.info("Ports {} removed from FlowEntry {}", portList, flowName);
+                    log.trace("Ports {} removed from FlowEntry {}", portList, flowName);
                 } else {
                     log.warn("Failed to remove ports {} from Flow entry {}. The failure is: {}", portList,
                             currentFlowEntry.toString(), status.getDescription());
@@ -1331,7 +1335,7 @@ public class ForwardingRulesManager implements
         Status status = modifyEntry(currentFlowEntry, newFlowEntry, false);
 
         if (status.isSuccess()) {
-            log.info("Output port replaced with {} for flow {} on node {}", outPort, flowName, node);
+            log.trace("Output port replaced with {} for flow {} on node {}", outPort, flowName, node);
         } else {
             log.warn("Failed to replace output port for flow {} on node {}. The failure is: {}", flowName, node,
                     status.getDescription());
@@ -1789,7 +1793,7 @@ public class ForwardingRulesManager implements
         // Do not attempt to reinstall the flow, warn user
         if (newFlowConfig.equals(oldFlowConfig)) {
             String msg = "No modification detected";
-            log.info("Static flow modification skipped. New flow and old flow are the same: {}", newFlowConfig);
+            log.trace("Static flow modification skipped. New flow and old flow are the same: {}", newFlowConfig);
             return new Status(StatusCode.SUCCESS, msg);
         }
 
@@ -1891,7 +1895,7 @@ public class ForwardingRulesManager implements
      *            inactive list
      */
     private void uninstallAllFlowEntries(boolean preserveFlowEntries) {
-        log.info("Uninstalling all non-internal flows");
+        log.trace("Uninstalling all non-internal flows");
 
         List<FlowEntryInstall> toRemove = new ArrayList<FlowEntryInstall>();
 
@@ -1929,7 +1933,7 @@ public class ForwardingRulesManager implements
      * default container instance of FRM only when the last container is deleted
      */
     private void reinstallAllFlowEntries() {
-        log.info("Reinstalling all inactive flows");
+        log.trace("Reinstalling all inactive flows");
 
         for (FlowEntry flowEntry : this.inactiveFlows.keySet()) {
             this.addEntry(flowEntry, false);
@@ -2060,6 +2064,22 @@ public class ForwardingRulesManager implements
     public void subnetNotify(Subnet sub, boolean add) {
     }
 
+    private boolean programInternalFlow(boolean proactive, FlowConfig fc) {
+        boolean retVal = true; // program flows unless determined otherwise
+        if(proactive) {
+            // if the flow already exists do not program
+            if(flowConfigExists(fc)) {
+                retVal = false;
+            }
+        } else {
+            // if the flow does not exist do not program
+            if(!flowConfigExists(fc)) {
+                retVal = false;
+            }
+        }
+        return retVal;
+    }
+
     /**
      * (non-Javadoc)
      *
@@ -2114,14 +2134,20 @@ public class ForwardingRulesManager implements
                 dropAllConfig.setActions(dropAction);
                 defaultConfigs.add(dropAllConfig);
 
-                log.info("Forwarding mode for node {} set to {}", node, (proactive ? "proactive" : "reactive"));
+                log.trace("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());
+                    // check if the frm really needs to act on the notification.
+                    // this is to check against duplicate notifications
+                    if(programInternalFlow(proactive, fc)) {
+                        Status status = (proactive) ? addStaticFlowInternal(fc, false) : removeStaticFlow(fc);
+                        if (status.isSuccess()) {
+                            log.trace("{} Proactive Static flow: {}", (proactive ? "Installed" : "Removed"), fc.getName());
+                        } else {
+                            log.warn("Failed to {} Proactive Static flow: {}", (proactive ? "install" : "remove"),
+                                    fc.getName());
+                        }
                     } else {
-                        log.warn("Failed to {} Proactive Static flow: {}", (proactive ? "install" : "remove"),
-                                fc.getName());
+                        log.debug("Got redundant install request for internal flow: {} on node: {}. Request not sent to FRM.", fc.getName(), node);
                     }
                 }
                 return new Status(StatusCode.SUCCESS);
@@ -2141,7 +2167,7 @@ public class ForwardingRulesManager implements
      * @param node
      */
     private void cleanDatabaseForNode(Node node) {
-        log.info("Cleaning Flow database for Node {}", node);
+        log.trace("Cleaning Flow database for Node {}", node);
         if (nodeFlows.containsKey(node)) {
             List<FlowEntryInstall> toRemove = new ArrayList<FlowEntryInstall>(nodeFlows.get(node));
 
@@ -2318,7 +2344,7 @@ public class ForwardingRulesManager implements
 
     @Override
     public void portGroupChanged(PortGroupConfig config, Map<Node, PortGroup> data, boolean add) {
-        log.info("PortGroup Changed for: {} Data: {}", config, portGroupData);
+        log.trace("PortGroup Changed for: {} Data: {}", config, portGroupData);
         Map<Node, PortGroup> existingData = portGroupData.get(config);
         if (existingData != null) {
             for (Map.Entry<Node, PortGroup> entry : data.entrySet()) {
@@ -2470,8 +2496,6 @@ public class ForwardingRulesManager implements
 
         cacheStartup();
 
-        registerWithOSGIConsole();
-
         /*
          * If we are not the first cluster node to come up, do not initialize
          * the static flow entries ordinal
@@ -2622,15 +2646,27 @@ public class ForwardingRulesManager implements
         // Start event handler thread
         frmEventHandler.start();
 
+        // replay the installedSwView data structure to populate
+        // node flows and group flows
+        for (FlowEntryInstall fei : installedSwView.values()) {
+            pendingEvents.offer(new UpdateIndexDBs(fei, true));
+        }
+
         /*
-         * Read startup and build database if we have not already gotten the
-         * configurations synced from another node
+         * Read startup and build database if we are the coordinator
          */
-        if (staticFlows.isEmpty()) {
+        if ((clusterContainerService != null) && (clusterContainerService.amICoordinator())) {
             loadFlowConfiguration();
         }
     }
 
+    /**
+     * Function called by the dependency manager before Container is Stopped and Destroyed.
+     */
+    public void containerStop() {
+        uninstallAllFlowEntries(false);
+    }
+
     /**
      * Function called by the dependency manager before the services exported by
      * the component are unregistered, this will be followed by a "destroy ()"
@@ -2638,7 +2674,6 @@ public class ForwardingRulesManager implements
      */
     void stop() {
         stopping = true;
-        uninstallAllFlowEntries(false);
         // Shutdown executor
         this.executor.shutdownNow();
         // Now walk all the workMonitor and wake up the one sleeping because
@@ -2892,102 +2927,11 @@ public class ForwardingRulesManager implements
         }
     }
 
-    /*
-     * OSGI COMMANDS
-     */
-    @Override
-    public String getHelp() {
-        StringBuffer help = new StringBuffer();
-        return help.toString();
-    }
-
     @Override
     public Status saveConfiguration() {
         return saveConfig();
     }
 
-    public void _frmNodeFlows(CommandInterpreter ci) {
-        String nodeId = ci.nextArgument();
-        Node node = Node.fromString(nodeId);
-        if (node == null) {
-            ci.println("frmNodeFlows <node> [verbose]");
-            return;
-        }
-        boolean verbose = false;
-        String verboseCheck = ci.nextArgument();
-        if (verboseCheck != null) {
-            verbose = verboseCheck.equals("true");
-        }
-
-        if (!nodeFlows.containsKey(node)) {
-            return;
-        }
-        // Dump per node database
-        for (FlowEntryInstall entry : nodeFlows.get(node)) {
-            if (!verbose) {
-                ci.println(node + " " + installedSwView.get(entry).getFlowName());
-            } else {
-                ci.println(node + " " + installedSwView.get(entry).toString());
-            }
-        }
-    }
-
-    public void _frmGroupFlows(CommandInterpreter ci) {
-        String group = ci.nextArgument();
-        if (group == null) {
-            ci.println("frmGroupFlows <group> [verbose]");
-            return;
-        }
-        boolean verbose = false;
-        String verboseCheck = ci.nextArgument();
-        if (verboseCheck != null) {
-            verbose = verboseCheck.equalsIgnoreCase("true");
-        }
-
-        if (!groupFlows.containsKey(group)) {
-            return;
-        }
-        // Dump per node database
-        ci.println("Group " + group + ":\n");
-        for (FlowEntryInstall flowEntry : groupFlows.get(group)) {
-            if (!verbose) {
-                ci.println(flowEntry.getNode() + " " + flowEntry.getFlowName());
-            } else {
-                ci.println(flowEntry.getNode() + " " + flowEntry.toString());
-            }
-        }
-    }
-
-    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);
@@ -3013,8 +2957,16 @@ public class ForwardingRulesManager implements
         }
         if (target != null) {
             // Update Configuration database
-            target.toggleInstallation();
-            target.setStatus(StatusCode.SUCCESS.toString());
+            if (target.getHardTimeout() != null || target.getIdleTimeout() != null) {
+                /*
+                 * No need for checking if actual values: these strings were
+                 * validated at configuration creation. Also, after a switch
+                 * down scenario, no use to reinstall a timed flow. Mark it as
+                 * "do not install". User can manually toggle it.
+                 */
+                target.toggleInstallation();
+            }
+            target.setStatus(StatusCode.GONE.toString());
             staticFlows.put(key, target);
         }
 
@@ -3063,7 +3015,11 @@ public class ForwardingRulesManager implements
                 // staticFlowEntry should never be null.
                 // the null check is just an extra defensive check.
                 if(staticFlowEntry != null) {
-                    staticFlows.remove(staticFlowEntry.getKey());
+                    // Modify status and update cluster cache
+                    log.debug("Updating static flow configuration on async error event");
+                    String status = String.format("Cannot be installed on node. reason: %s", errorString);
+                    staticFlowEntry.getValue().setStatus(status);
+                    refreshClusterStaticFlowsStatus(node);
                 }
             }
         }
@@ -3130,7 +3086,7 @@ public class ForwardingRulesManager implements
          * Streamline the updates for the per node and per group index databases
          */
         if (cacheName.equals(INSTALLED_SW_VIEW_CACHE)) {
-            pendingEvents.offer(new UpdateIndexDBs((FlowEntryInstall)key, true));
+            pendingEvents.offer(new UpdateIndexDBs((FlowEntryInstall)new_value, true));
         }
 
         if (originLocal) {
@@ -3200,7 +3156,7 @@ public class ForwardingRulesManager implements
         if (node != null) {
             for (Map.Entry<FlowEntry, FlowEntry> entry : this.originalSwView.entrySet()) {
                 if (node.equals(entry.getKey().getNode())) {
-                    list.add(entry.getKey().clone());
+                    list.add(entry.getValue().clone());
                 }
             }
         }