From: Jason Ye Date: Thu, 27 Feb 2014 20:25:04 +0000 (+0000) Subject: Merge "FlowConfig to only run syntactic validation" X-Git-Tag: autorelease-tag-v20140601202136_82eb3f9~369 X-Git-Url: https://git.opendaylight.org/gerrit/gitweb?p=controller.git;a=commitdiff_plain;h=986d8ca7ceae77314cc3e63461b960a28a032955;hp=-c Merge "FlowConfig to only run syntactic validation" --- 986d8ca7ceae77314cc3e63461b960a28a032955 diff --combined opendaylight/forwardingrulesmanager/implementation/src/main/java/org/opendaylight/controller/forwardingrulesmanager/internal/ForwardingRulesManager.java index bdbf7acd5f,be95bc98d5..614c39e060 --- 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 @@@ -50,6 -50,9 +50,9 @@@ import org.opendaylight.controller.forw import org.opendaylight.controller.forwardingrulesmanager.implementation.data.FlowEntryDistributionOrder; import org.opendaylight.controller.sal.action.Action; import org.opendaylight.controller.sal.action.ActionType; + import org.opendaylight.controller.sal.action.Enqueue; + import org.opendaylight.controller.sal.action.Flood; + import org.opendaylight.controller.sal.action.FloodAll; import org.opendaylight.controller.sal.action.Output; import org.opendaylight.controller.sal.connection.ConnectionLocality; import org.opendaylight.controller.sal.core.Config; @@@ -243,6 -246,57 +246,57 @@@ public class ForwardingRulesManager imp return null; } + /** + * Checks if the FlowEntry targets are valid for this container + * + * @param flowEntry + * The flow entry to test + * @return a Status object representing the result of the validation + */ + private Status validateEntry(FlowEntry flowEntry) { + // Node presence check + Node node = flowEntry.getNode(); + if (!switchManager.getNodes().contains(node)) { + return new Status(StatusCode.BADREQUEST, String.format("Node %s is not present in this container", node)); + } + + // Ports and actions validation check + Flow flow = flowEntry.getFlow(); + Match match = flow.getMatch(); + if (match.isPresent(MatchType.IN_PORT)) { + NodeConnector inputPort = (NodeConnector)match.getField(MatchType.IN_PORT).getValue(); + if (!switchManager.getNodeConnectors(node).contains(inputPort)) { + String msg = String.format("Ingress port %s is not present on this container", inputPort); + return new Status(StatusCode.BADREQUEST, msg); + } + } + for (Action action : flow.getActions()) { + if (action instanceof Flood && !GlobalConstants.DEFAULT.toString().equals(getContainerName())) { + return new Status(StatusCode.BADREQUEST, String.format("Flood is only allowed in default container")); + } + if (action instanceof FloodAll && !GlobalConstants.DEFAULT.toString().equals(getContainerName())) { + return new Status(StatusCode.BADREQUEST, String.format("FloodAll is only allowed in default container")); + } + if (action instanceof Output) { + Output out = (Output)action; + NodeConnector outputPort = out.getPort(); + if (!switchManager.getNodeConnectors(node).contains(outputPort)) { + String msg = String.format("Output port %s is not present on this container", outputPort); + return new Status(StatusCode.BADREQUEST, msg); + } + } + if (action instanceof Enqueue) { + Enqueue out = (Enqueue)action; + NodeConnector outputPort = out.getPort(); + if (!switchManager.getNodeConnectors(node).contains(outputPort)) { + String msg = String.format("Enqueue port %s is not present on this container", outputPort); + return new Status(StatusCode.BADREQUEST, msg); + } + } + } + return new Status(StatusCode.SUCCESS); + } + /** * Adds a flow entry onto the network node It runs various validity checks * and derive the final container flows merged entries that will be @@@ -264,6 -318,15 +318,15 @@@ return new Status(StatusCode.NOTACCEPTABLE, INVALID_FLOW_ENTRY); } + // Operational check: input, output and queue ports presence check and + // action validation for this container + Status status = validateEntry(flowEntry); + if (!status.isSuccess()) { + String msg = String.format("%s: %s", INVALID_FLOW_ENTRY, status.getDescription()); + log.warn("{}: {}", msg, flowEntry); + return new Status(StatusCode.NOTACCEPTABLE, msg); + } + /* * Redundant Check: Check if the request is a redundant one from the * same application the flowEntry is equal to an existing one. Given we @@@ -422,6 -485,15 +485,15 @@@ return new Status(StatusCode.SUCCESS, msg); } + // Operational check: input, output and queue ports presence check and + // action validation for this container + Status status = validateEntry(newFlowEntry); + if (!status.isSuccess()) { + String msg = String.format("Modify: %s: %s", INVALID_FLOW_ENTRY, status.getDescription()); + log.warn("{}: {}", msg, newFlowEntry); + return new Status(StatusCode.NOTACCEPTABLE, msg); + } + /* * Conflict Check: Verify the new entry would not conflict with an * existing one. This is a loose check on the previous original flow @@@ -1121,10 -1193,7 +1193,10 @@@ List list = new ArrayList(groupFlows.get(groupName)); toBeRemoved = list.size(); for (FlowEntryInstall entry : list) { - Status status = this.removeEntry(entry.getOriginal(), false); + // since this is the entry that was stored in groupFlows + // it is already validated and merged + // so can call removeEntryInternal directly + Status status = this.removeEntryInternal(entry, false); if (status.isSuccess()) { toBeRemoved -= 1; } else { @@@ -1519,7 -1588,7 +1591,7 @@@ @Override public Status addStaticFlow(FlowConfig config) { // Configuration object validation - Status status = config.validate(container); + Status status = config.validate(); if (!status.isSuccess()) { log.warn("Invalid Configuration for flow {}. The failure is {}", config, status.getDescription()); String error = "Invalid Configuration (" + status.getDescription() + ")"; @@@ -1683,6 -1752,7 +1755,7 @@@ config.setStatus(StatusCode.SUCCESS.toString()); break; default: + break; } } } @@@ -1778,7 -1848,7 +1851,7 @@@ } // Validity Check - Status status = newFlowConfig.validate(container); + Status status = newFlowConfig.validate(); if (!status.isSuccess()) { String msg = "Invalid Configuration (" + status.getDescription() + ")"; newFlowConfig.setStatus(msg); @@@ -1858,7 -1928,7 +1931,7 @@@ } } if (target != null) { - Status status = target.validate(container); + Status status = target.validate(); if (!status.isSuccess()) { log.warn(status.getDescription()); return status; @@@ -2760,6 -2830,7 +2833,7 @@@ this.reinstallAllFlowEntries(); break; default: + break; } // Update our configuration DB