ISSUE

Currently ForwardingRulesManager installs the proactive mode flows through the
static flow configuration path. This allows these controller generated flows to
be listed in the flow programmer UI. The drawback of this approach is that now
the proactive flows are saved to FRM startup configuration, when user saves the
config. So when FRM boots up, it will program the proactive flows on the switch
whenever the switch connects to the controller, regardless of the actual switch
forwarding mode configured on switch mgr.

This is not correct as the notion of proactive forwarding mode associated to a
switch is controlled by switch manager, through sm UI. The trigger to install the
proactive flows should come from switch manager, FRM should not save these internal
generated flows to its startup config, nor program them on its own.

CHANGES
1) On FRM config save, the internal generated static flows will not be added to the
list that will be actually saved.
2) In Switch Mgr, when the switch connects, while reading the respective switch
configuration (if any) the mode is learnt. If it is set to proactive, the mode change
event is being played.
3) When a switch disconnect, FRM has to remove the internal static flow configurations
for that switch.
4) Switch Mgr isSpecial method to correctly report special node conenctor (method should
be provided by NodeConnector class in future).
5) Adding the startup directory to the distribution to allow write/read of startup
configuration files.
6) Have FlowConfig class to expose a method to retrieve the configured forwarding mode,
so that internal coding is hidden to clients.

Signed-off-by: aboch <aboch@cisco.com>
Change-Id: Id628042b7d656d95ace87a1e72c13f2950e6795c
Signed-off-by: aboch <aboch@cisco.com>

diff --git a/opendaylight/distribution/opendaylight/src/main/resources/configuration/startup/README b/opendaylight/distribution/opendaylight/src/main/resources/configuration/startup/README
new file mode 100644 (file)
index 0000000..2da70ff
--- /dev/null
+++ b/opendaylight/distribution/opendaylight/src/main/resources/configuration/startup/README
@@ -0,0 +1 @@
+Directory where the opendaylight controller modules store their configuration files

diff --git a/opendaylight/forwardingrulesmanager/src/main/java/org/opendaylight/controller/forwardingrulesmanager/FlowConfig.java b/opendaylight/forwardingrulesmanager/src/main/java/org/opendaylight/controller/forwardingrulesmanager/FlowConfig.java
index f1217b534043b3ad64caec04f38fe0a5a57347dd..aa7578ba77119fdd54865cfc8c2a590f29a4cd31 100644 (file)
--- a/opendaylight/forwardingrulesmanager/src/main/java/org/opendaylight/controller/forwardingrulesmanager/FlowConfig.java
+++ b/opendaylight/forwardingrulesmanager/src/main/java/org/opendaylight/controller/forwardingrulesmanager/FlowConfig.java
@@ -10,7 +10,6 @@
 package org.opendaylight.controller.forwardingrulesmanager;
 
 import java.io.Serializable;
-import java.lang.reflect.Field;
 import java.net.Inet6Address;
 import java.net.InetAddress;
 import java.util.ArrayList;
@@ -64,7 +63,7 @@ import org.slf4j.LoggerFactory;
 
 /**
  * Configuration Java Object which represents a flow configuration information
- * for Forwarding Rrules Manager.
+ * for Forwarding Rules Manager.
  */
 
 @XmlRootElement
@@ -224,9 +223,7 @@ public class FlowConfig implements Serializable {
 
     public boolean isInternalFlow() {
         // Controller generated static flows have name starting with "**"
-        if (this.name == null)
-            return false;
-        return this.name.startsWith("**");
+       return (this.name != null && this.name.startsWith("**"));
     }
 
     public String getName() {
@@ -415,18 +412,6 @@ public class FlowConfig implements Serializable {
         this.actions = actions;
     }
 
-    public static List<String> getFieldsNames() {
-        List<String> fieldList = new ArrayList<String>();
-        for (Field fld : FlowConfig.class.getDeclaredFields()) {
-            fieldList.add(fld.getName());
-        }
-        // Remove 4 static fields + 2 internal field
-        for (int i = 0; i < 6; i++)
-            fieldList.remove(0);
-
-        return fieldList;
-    }
-
     public boolean isPortGroupEnabled() {
         return (portGroup != null);
     }

diff --git a/opendaylight/forwardingrulesmanager/src/main/java/org/opendaylight/controller/forwardingrulesmanager/internal/ForwardingRulesManagerImpl.java b/opendaylight/forwardingrulesmanager/src/main/java/org/opendaylight/controller/forwardingrulesmanager/internal/ForwardingRulesManagerImpl.java
index bdaae5b28ce40245a43e040b1d46e13963c11277..6cdc785a196593a20116efdef0976b9d8550355c 100644 (file)
--- a/opendaylight/forwardingrulesmanager/src/main/java/org/opendaylight/controller/forwardingrulesmanager/internal/ForwardingRulesManagerImpl.java
+++ b/opendaylight/forwardingrulesmanager/src/main/java/org/opendaylight/controller/forwardingrulesmanager/internal/ForwardingRulesManagerImpl.java
@@ -93,6 +93,7 @@ public class ForwardingRulesManagerImpl implements IForwardingRulesManager,
         IConfigurationContainerAware, IInventoryListener, IObjectReader,
         ICacheUpdateAware<Long, String>, CommandProvider {
     private static final String SAVE = "Save";
+    private static final String NODEDOWN = "Node is Down";
     private static final Logger log = LoggerFactory
             .getLogger(ForwardingRulesManagerImpl.class);
     private Map<Long, String> flowsSaveEvent;
@@ -1303,14 +1304,28 @@ public class ForwardingRulesManagerImpl implements IForwardingRulesManager,
     private void updateStaticFlowConfigsOnNodeDown(Node node) {
         log.trace("Updating Static Flow configs on node down: " + node);
 
-        for (FlowConfig config : staticFlows.values()) {
+        List<Integer> toRemove = new ArrayList<Integer>();
+        for (Entry<Integer,FlowConfig> entry : staticFlows.entrySet()) {
+
+               FlowConfig config = entry.getValue();
+
             if (config.isPortGroupEnabled()) {
                 continue;
             }
+            
             if (config.installInHw() && config.getNode().equals(node)) {
-                config.setStatus("Node is down");
+               if (config.isInternalFlow()) {
+                       // Take note of this controller generated static flow
+                       toRemove.add(entry.getKey());
+               } else {
+                       config.setStatus(NODEDOWN);
+               }
             }
         }
+        // Remove controller generated static flows for this node 
+        for (Integer index : toRemove) {
+               staticFlows.remove(index);
+        }        
     }
 
     private void updateStaticFlowConfigsOnContainerModeChange(UpdateType update) {
@@ -1657,8 +1672,10 @@ public class ForwardingRulesManagerImpl implements IForwardingRulesManager,
         ConcurrentHashMap<Integer, FlowConfig> nonDynamicFlows = new ConcurrentHashMap<Integer, FlowConfig>();
         for (Integer ordinal : staticFlows.keySet()) {
             FlowConfig config = staticFlows.get(ordinal);
-            if (config.isDynamic())
+            // Do not save dynamic and controller generated static flows
+            if (config.isDynamic() || config.isInternalFlow()) {
                 continue;
+            }
             nonDynamicFlows.put(ordinal, config);
         }
         objWriter.write(nonDynamicFlows, frmFileName);
@@ -1802,10 +1819,10 @@ public class ForwardingRulesManagerImpl implements IForwardingRulesManager,
     public void notifyNode(Node node, UpdateType type,
             Map<String, Property> propMap) {
         switch (type) {
-        case ADDED:
+        case ADDED: 
             addStaticFlowsToSwitch(node);
             break;
-        case REMOVED:
+        case REMOVED:          
             cleanDatabaseForNode(node);
             updateStaticFlowConfigsOnNodeDown(node);
             break;
@@ -1816,7 +1833,7 @@ public class ForwardingRulesManagerImpl implements IForwardingRulesManager,
 
     @Override
     public void notifyNodeConnector(NodeConnector nodeConnector,
-            UpdateType type, Map<String, Property> propMap) {
+               UpdateType type, Map<String, Property> propMap) {
     }
 
     private FlowConfig getDerivedFlowConfig(FlowConfig original,
@@ -2049,7 +2066,6 @@ public class ForwardingRulesManagerImpl implements IForwardingRulesManager,
      *
      */
     void init() {
-        log.info("Init");
         frmAware = Collections
                 .synchronizedSet(new HashSet<IForwardingRulesManagerAware>());
         frmFileName = GlobalConstants.STARTUPHOME.toString() + "frm_staticflows_"
@@ -2085,7 +2101,6 @@ public class ForwardingRulesManagerImpl implements IForwardingRulesManager,
      *
      */
     void destroy() {
-        log.info("Destroy");
         destroyCaches();
     }
 
@@ -2095,7 +2110,6 @@ public class ForwardingRulesManagerImpl implements IForwardingRulesManager,
      *
      */
     void start() {
-        log.info("Start");
         /*
          * Read startup and build database if we have not already gotten the
          * configurations synced from another node
@@ -2112,7 +2126,6 @@ public class ForwardingRulesManagerImpl implements IForwardingRulesManager,
      *
      */
     void stop() {
-        log.info("Stop");
     }
 
     public void setFlowProgrammerService(IFlowProgrammerService service) {
@@ -2330,7 +2343,7 @@ public class ForwardingRulesManagerImpl implements IForwardingRulesManager,
         boolean verbose = false;
         String verboseCheck = ci.nextArgument();
         if (verboseCheck != null) {
-            verbose = verboseCheck.equals("true");
+            verbose = verboseCheck.equalsIgnoreCase("true");
         }
 
         // Dump per node database

diff --git a/opendaylight/switchmanager/src/main/java/org/opendaylight/controller/switchmanager/SwitchConfig.java b/opendaylight/switchmanager/src/main/java/org/opendaylight/controller/switchmanager/SwitchConfig.java
index 550ca69e7dd0fc21465e025c22678a5fea149bb2..758a99b8b083c9ded59ebe7457d3337b81a2f72d 100644 (file)
--- a/opendaylight/switchmanager/src/main/java/org/opendaylight/controller/switchmanager/SwitchConfig.java
+++ b/opendaylight/switchmanager/src/main/java/org/opendaylight/controller/switchmanager/SwitchConfig.java
@@ -49,6 +49,10 @@ public class SwitchConfig implements Serializable {
         return mode;
     }
 
+    public boolean isProactive() {
+       return Integer.parseInt(mode) != 0;
+    }
+    
     public static long getSerialversionuid() {
         return serialVersionUID;
     }

diff --git a/opendaylight/switchmanager/src/main/java/org/opendaylight/controller/switchmanager/internal/SwitchManagerImpl.java b/opendaylight/switchmanager/src/main/java/org/opendaylight/controller/switchmanager/internal/SwitchManagerImpl.java
index b36d8c12839706de4274101e42d6d44e35690bb4..d8180040e8d8a4e975df9cd8173319676bdb11a9 100755 (executable)
--- a/opendaylight/switchmanager/src/main/java/org/opendaylight/controller/switchmanager/internal/SwitchManagerImpl.java
+++ b/opendaylight/switchmanager/src/main/java/org/opendaylight/controller/switchmanager/internal/SwitchManagerImpl.java
@@ -585,8 +585,9 @@ public class SwitchManagerImpl implements ISwitchManager,
         boolean modeChange = false;
 
         SwitchConfig sc = nodeConfigList.get(cfgObject.getNodeId());
-        if ((sc == null) || !cfgObject.getMode().equals(sc.getMode()))
+        if ((sc == null) || !cfgObject.getMode().equals(sc.getMode())) {
             modeChange = true;
+        }
 
         nodeConfigList.put(cfgObject.getNodeId(), cfgObject);
         try {
@@ -610,8 +611,7 @@ public class SwitchManagerImpl implements ISwitchManager,
                         .getMode());
 
                 if (modeChange) {
-                    notifyModeChange(node, (Integer.parseInt(cfgObject
-                            .getMode()) != 0));
+                    notifyModeChange(node, cfgObject.isProactive());
                 }
             }
         } catch (Exception e) {
@@ -714,8 +714,9 @@ public class SwitchManagerImpl implements ISwitchManager,
 
     private void addNode(Node node, Set<Property> props) {
         log.trace("{} added", node);
-        if (nodeProps == null)
+        if (nodeProps == null) {
             return;
+        }
 
         Map<String, Property> propMap;
         if (nodeProps.get(node) != null) {
@@ -732,6 +733,7 @@ public class SwitchManagerImpl implements ISwitchManager,
         }
 
         // copy node properties from config
+        boolean proactiveForwarding = false;
         if (nodeConfigList != null) {
             String nodeId = node.getNodeIDString();
             for (SwitchConfig conf : nodeConfigList.values()) {
@@ -740,6 +742,7 @@ public class SwitchManagerImpl implements ISwitchManager,
                     propMap.put(name.getName(), name);
                     Property tier = new Tier(Integer.parseInt(conf.getTier()));
                     propMap.put(tier.getName(), tier);
+                    proactiveForwarding = conf.isProactive();
                     break;
                 }
             }
@@ -749,8 +752,13 @@ public class SwitchManagerImpl implements ISwitchManager,
         // check if span ports are configed
         addSpanPorts(node);
 
-        /* notify node listeners */
+        // notify node listeners
         notifyNode(node, UpdateType.ADDED, propMap);
+        
+        // notify proactive mode forwarding
+        if (proactiveForwarding) {
+               notifyModeChange(node, true);
+        }
     }
 
     private void removeNode(Node node) {
@@ -816,8 +824,9 @@ public class SwitchManagerImpl implements ISwitchManager,
 
         log.trace("{} {}", nodeConnector, type);
 
-        if (nodeConnectorProps == null)
+        if (nodeConnectorProps == null) {
             return;
+        }
 
         switch (type) {
         case ADDED:
@@ -862,7 +871,7 @@ public class SwitchManagerImpl implements ISwitchManager,
         Set<Node> nodes = getNodes();
         if (nodes != null) {
             for (Node node : nodes) {
-                if (id == (Long) node.getID()) {
+                if (id.equals((Long)node.getID())) {
                     return node;
                 }
             }
@@ -1384,8 +1393,7 @@ public class SwitchManagerImpl implements ISwitchManager,
         for (Node node : getNodes()) {
             SwitchConfig sc = getSwitchConfig(node.toString());
             if ((sc != null) && isDefaultContainer) {
-                service.modeChangeNotify(node,
-                        (Integer.parseInt(sc.getMode()) != 0));
+                service.modeChangeNotify(node, sc.isProactive());
             }
         }
     }
@@ -1647,10 +1655,10 @@ public class SwitchManagerImpl implements ISwitchManager,
 
     @Override
     public boolean isSpecial(NodeConnector p) {
-        if (p.equals(NodeConnectorIDType.CONTROLLER) ||
-            p.equals(NodeConnectorIDType.ALL) ||
-            p.equals(NodeConnectorIDType.SWSTACK) ||
-            p.equals(NodeConnectorIDType.HWPATH)) {
+        if (p.getType().equals(NodeConnectorIDType.CONTROLLER) ||
+            p.getType().equals(NodeConnectorIDType.ALL) ||
+            p.getType().equals(NodeConnectorIDType.SWSTACK) ||
+            p.getType().equals(NodeConnectorIDType.HWPATH)) {
             return true;
         }
         return false;