From: aboch Date: Mon, 1 Apr 2013 20:36:56 +0000 (-0700) Subject: ISSUE X-Git-Tag: releasepom-0.1.0~610^2 X-Git-Url: https://git.opendaylight.org/gerrit/gitweb?p=controller.git;a=commitdiff_plain;h=48c293350f30f1d0104adcb548d233d7d79efd80 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 Change-Id: Id628042b7d656d95ace87a1e72c13f2950e6795c Signed-off-by: aboch --- 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 index 0000000000..2da70fff1e --- /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 f1217b5340..aa7578ba77 100644 --- 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 getFieldsNames() { - List fieldList = new ArrayList(); - 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 bdaae5b28c..6cdc785a19 100644 --- 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, 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 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 toRemove = new ArrayList(); + for (Entry 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 nonDynamicFlows = new ConcurrentHashMap(); 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 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 propMap) { + UpdateType type, Map 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()); 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 550ca69e7d..758a99b8b0 100644 --- 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 b36d8c1283..d8180040e8 100755 --- 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 props) { log.trace("{} added", node); - if (nodeProps == null) + if (nodeProps == null) { return; + } Map 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 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;