X-Git-Url: https://git.opendaylight.org/gerrit/gitweb?a=blobdiff_plain;ds=sidebyside;f=opendaylight%2Fforwardingrulesmanager%2Fimplementation%2Fsrc%2Fmain%2Fjava%2Forg%2Fopendaylight%2Fcontroller%2Fforwardingrulesmanager%2Finternal%2FForwardingRulesManagerImpl.java;h=5fbe12bd1ca44218a9629e50489540bd85d83302;hb=d04b927ef78082525c3b0738126b6eb12e4a7a74;hp=8e80f83d80fbcee10dfd5aa02dbab5634f0200a1;hpb=30c334b11097201c8f8a68263bc01efb869c778b;p=controller.git diff --git a/opendaylight/forwardingrulesmanager/implementation/src/main/java/org/opendaylight/controller/forwardingrulesmanager/internal/ForwardingRulesManagerImpl.java b/opendaylight/forwardingrulesmanager/implementation/src/main/java/org/opendaylight/controller/forwardingrulesmanager/internal/ForwardingRulesManagerImpl.java index 8e80f83d80..5fbe12bd1c 100644 --- a/opendaylight/forwardingrulesmanager/implementation/src/main/java/org/opendaylight/controller/forwardingrulesmanager/internal/ForwardingRulesManagerImpl.java +++ b/opendaylight/forwardingrulesmanager/implementation/src/main/java/org/opendaylight/controller/forwardingrulesmanager/internal/ForwardingRulesManagerImpl.java @@ -59,10 +59,10 @@ import org.opendaylight.controller.sal.core.NodeConnector; import org.opendaylight.controller.sal.core.Property; import org.opendaylight.controller.sal.core.UpdateType; import org.opendaylight.controller.sal.flowprogrammer.Flow; +import org.opendaylight.controller.sal.flowprogrammer.IFlowProgrammerListener; import org.opendaylight.controller.sal.flowprogrammer.IFlowProgrammerService; import org.opendaylight.controller.sal.match.Match; import org.opendaylight.controller.sal.match.MatchType; -import org.opendaylight.controller.sal.utils.StatusCode; import org.opendaylight.controller.sal.utils.EtherTypes; import org.opendaylight.controller.sal.utils.GlobalConstants; import org.opendaylight.controller.sal.utils.HexEncode; @@ -73,6 +73,7 @@ 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; +import org.opendaylight.controller.sal.utils.StatusCode; import org.opendaylight.controller.switchmanager.IInventoryListener; import org.opendaylight.controller.switchmanager.ISwitchManager; import org.opendaylight.controller.switchmanager.ISwitchManagerAware; @@ -90,7 +91,8 @@ import org.slf4j.LoggerFactory; public class ForwardingRulesManagerImpl implements IForwardingRulesManager, PortGroupChangeListener, IContainerListener, ISwitchManagerAware, IConfigurationContainerAware, IInventoryListener, IObjectReader, - ICacheUpdateAware, CommandProvider { + ICacheUpdateAware, CommandProvider, + IFlowProgrammerListener { private static final String SAVE = "Save"; private static final String NODEDOWN = "Node is Down"; private static final Logger log = LoggerFactory @@ -136,13 +138,18 @@ public class ForwardingRulesManagerImpl implements IForwardingRulesManager, * * @param flowEntry * the original flow entry application requested to add - * @return + * @param async + * the flag indicating if this is a asynchronous request + * @return the status of this request. In case of asynchronous call, it + * will contain the unique id assigned to this request */ - private Status addEntry(FlowEntry flowEntry) { + private Status addEntry(FlowEntry flowEntry, boolean async) { + // Sanity Check if (flowEntry == null || flowEntry.getNode() == null) { String msg = "Invalid FlowEntry"; - log.warn(msg + ": " + flowEntry); + String logMsg = msg + ": {}"; + log.warn(logMsg, flowEntry); return new Status(StatusCode.NOTACCEPTABLE, msg); } @@ -156,7 +163,8 @@ public class ForwardingRulesManagerImpl implements IForwardingRulesManager, // Container Flow conflict Check if (toInstallList.isEmpty()) { String msg = "Flow Entry conflicts with all Container Flows"; - log.warn(msg); + String logMsg = msg + ": {}"; + log.warn(logMsg, flowEntry); return new Status(StatusCode.CONFLICT, msg); } @@ -168,7 +176,7 @@ public class ForwardingRulesManagerImpl implements IForwardingRulesManager, if (findMatch(entry.getInstall(), false) != null) { log.warn("Operation Rejected: A flow with same match " + "and priority exists on the target node"); - log.trace("Aborting to install " + entry); + log.trace("Aborting to install {}", entry); continue; } toInstallSafe.add(entry); @@ -179,27 +187,37 @@ public class ForwardingRulesManagerImpl implements IForwardingRulesManager, if (toInstallSafe.size() == 0) { String msg = "A flow with same match and priority exists " + "on the target node"; - log.warn(msg); + String logMsg = msg + ": {}"; + log.warn(logMsg, flowEntry); return new Status(StatusCode.CONFLICT, msg); } // Try to install an entry at the time Status error = new Status(null, null); + Status succeded = null; boolean oneSucceded = false; - for (FlowEntryInstall installEntry : toInstallList) { + for (FlowEntryInstall installEntry : toInstallSafe) { // Install and update database - Status ret = addEntriesInternal(installEntry); + Status ret = addEntriesInternal(installEntry, async); if (ret.isSuccess()) { oneSucceded = true; + /* + * The first successful status response will be returned + * For the asynchronous call, we can discard the container flow + * complication for now and assume we will always deal with + * one flow only per request + */ + succeded = ret; } else { error = ret; - log.warn("Failed to install the entry: " + ret.getDescription()); + log.warn("Failed to install the entry: {}. The failure is: {}", + installEntry, ret.getDescription()); } } - return (oneSucceded) ? new Status(StatusCode.SUCCESS, null) : error; + return (oneSucceded) ? succeded : error; } /** @@ -245,31 +263,38 @@ public class ForwardingRulesManagerImpl implements IForwardingRulesManager, * * @param currentFlowEntry * @param newFlowEntry - * @return Success or error string + * @param async + * the flag indicating if this is a asynchronous request + * @return the status of this request. In case of asynchronous call, it + * will contain the unique id assigned to this request */ private Status modifyEntry(FlowEntry currentFlowEntry, - FlowEntry newFlowEntry) { + FlowEntry newFlowEntry, boolean async) { + Status retExt; // Sanity checks if (currentFlowEntry == null || currentFlowEntry.getNode() == null || newFlowEntry == null || newFlowEntry.getNode() == null) { String msg = "Modify: Invalid FlowEntry"; - log.warn(msg + ": {} or {} ", currentFlowEntry, newFlowEntry); + String logMsg = msg + ": {} or {}"; + log.warn(logMsg, currentFlowEntry, newFlowEntry); return new Status(StatusCode.NOTACCEPTABLE, msg); } if (!currentFlowEntry.getNode().equals(newFlowEntry.getNode()) || !currentFlowEntry.getFlowName().equals( newFlowEntry.getFlowName())) { String msg = "Modify: Incompatible Flow Entries"; - log.warn(msg + ": {} and {}", currentFlowEntry, newFlowEntry); + String logMsg = msg + ": {} and {}"; + log.warn(logMsg, currentFlowEntry, newFlowEntry); return new Status(StatusCode.NOTACCEPTABLE, msg); } // Equality Check if (currentFlowEntry.equals(newFlowEntry)) { String msg = "Modify skipped as flows are the same"; - log.debug(msg + ": " + currentFlowEntry + " and " + newFlowEntry); + String logMsg = msg + ": {} and {}"; + log.debug(logMsg, currentFlowEntry, newFlowEntry); return new Status(StatusCode.SUCCESS, msg); } @@ -284,7 +309,8 @@ public class ForwardingRulesManagerImpl implements IForwardingRulesManager, currentFlowEntry)) { String msg = "Operation Rejected: Another flow with same match " + "and priority exists on the target node"; - log.warn(msg); + String logMsg = msg + ": {}"; + log.warn(logMsg, currentFlowEntry); return new Status(StatusCode.CONFLICT, msg); } @@ -297,6 +323,8 @@ public class ForwardingRulesManagerImpl implements IForwardingRulesManager, if (toInstallList.isEmpty()) { String msg = "Modify Operation Rejected: The new entry " + "conflicts with all the container flows"; + String logMsg = msg + ": {}"; + log.warn(logMsg, newFlowEntry); log.warn(msg); return new Status(StatusCode.CONFLICT, msg); } @@ -317,6 +345,7 @@ public class ForwardingRulesManagerImpl implements IForwardingRulesManager, * So, for the above two cases, to simplify, let's decouple the modify * in: 1) remove current entries 2) install new entries */ + Status succeeded = null; boolean decouple = false; if (installedList.size() != toInstallList.size()) { log.info("Modify: New flow entry does not satisfy the same " @@ -342,11 +371,11 @@ public class ForwardingRulesManagerImpl implements IForwardingRulesManager, if (decouple) { // Remove current entries for (FlowEntryInstall currEntry : installedList) { - this.removeEntryInternal(currEntry); + this.removeEntryInternal(currEntry, async); } // Install new entries for (FlowEntryInstall newEntry : toInstallSafe) { - this.addEntriesInternal(newEntry); + succeeded = this.addEntriesInternal(newEntry, async); } } else { /* @@ -360,13 +389,13 @@ public class ForwardingRulesManagerImpl implements IForwardingRulesManager, * fails, we need to stop, restore the already modified entries, and * declare failure. */ - Status retModify; + Status retModify = null; int i = 0; int size = toInstallList.size(); while (i < size) { // Modify and update database retModify = modifyEntryInternal(installedList.get(i), - toInstallList.get(i)); + toInstallList.get(i), async); if (retModify.isSuccess()) { i++; } else { @@ -382,7 +411,7 @@ public class ForwardingRulesManagerImpl implements IForwardingRulesManager, while (j < i) { log.info("Attempting to restore initial entries"); retExt = modifyEntryInternal(toInstallList.get(i), - installedList.get(i)); + installedList.get(i), async); if (retExt.isSuccess()) { j++; } else { @@ -396,8 +425,15 @@ public class ForwardingRulesManagerImpl implements IForwardingRulesManager, return new Status(StatusCode.INTERNALERROR, msg); } } + succeeded = retModify; } - return new Status(StatusCode.SUCCESS, null); + /* + * The first successful status response will be returned. + * For the asynchronous call, we can discard the container flow + * complication for now and assume we will always deal with + * one flow only per request + */ + return succeeded; } /** @@ -407,18 +443,27 @@ public class ForwardingRulesManagerImpl implements IForwardingRulesManager, * * @param currentEntries * @param newEntries - * @return + * @param async + * the flag indicating if this is a asynchronous request + * @return the status of this request. In case of asynchronous call, it + * will contain the unique id assigned to this request */ private Status modifyEntryInternal(FlowEntryInstall currentEntries, - FlowEntryInstall newEntries) { + FlowEntryInstall newEntries, boolean async) { // Modify the flow on the network node - Status status = programmer.modifyFlow(currentEntries.getNode(), - currentEntries.getInstall().getFlow(), newEntries.getInstall() - .getFlow()); + Status status = (async)? + programmer.modifyFlowAsync(currentEntries.getNode(), + currentEntries.getInstall().getFlow(), newEntries.getInstall() + .getFlow()) : + programmer.modifyFlow(currentEntries.getNode(), + currentEntries.getInstall().getFlow(), newEntries.getInstall() + .getFlow()); + if (!status.isSuccess()) { - log.warn("SDN Plugin failed to program the flow: " - + status.getDescription()); + log.warn( + "SDN Plugin failed to program the flow: {}. The failure is: {}", + newEntries.getInstall(), status.getDescription()); return status; } @@ -426,6 +471,7 @@ public class ForwardingRulesManagerImpl implements IForwardingRulesManager, newEntries.getInstall()); // Update DB + newEntries.setRequestId(status.getRequestId()); updateLocalDatabase(currentEntries, false); updateLocalDatabase(newEntries, true); @@ -437,15 +483,20 @@ public class ForwardingRulesManagerImpl implements IForwardingRulesManager, * (entry or node not present), it return successfully * * @param flowEntry - * @return + * the flow entry to remove + * @param async + * the flag indicating if this is a asynchronous request + * @return the status of this request. In case of asynchronous call, it + * will contain the unique id assigned to this request */ - private Status removeEntry(FlowEntry flowEntry) { + private synchronized Status removeEntry(FlowEntry flowEntry, boolean async) { Status error = new Status(null, null); // Sanity Check if (flowEntry == null || flowEntry.getNode() == null) { String msg = "Invalid FlowEntry"; - log.warn(msg + ": " + flowEntry); + String logMsg = msg + ": {}"; + log.warn(logMsg, flowEntry); return new Status(StatusCode.NOTACCEPTABLE, msg); } @@ -454,36 +505,39 @@ public class ForwardingRulesManagerImpl implements IForwardingRulesManager, flowEntry.clone(), container.getContainerFlows()); Set flowsOnNode = nodeFlows.get(flowEntry.getNode()); + Status succeeded = null; boolean atLeastOneRemoved = false; for (FlowEntryInstall entry : installedList) { if (flowsOnNode == null) { - String msg = "Removal skipped (Node down)"; - log.debug(msg + " for flow entry " + flowEntry); + String msg = "Removal skipped (Node down) for flow entry"; + String logMsg = msg + ": {}"; + log.debug(logMsg, flowEntry); return new Status(StatusCode.SUCCESS, msg); } if (!flowsOnNode.contains(entry)) { - log.debug("Removal skipped (not present in software view) " - + "for flow entry " + flowEntry); - + String logMsg = "Removal skipped (not present in software view) for flow entry: {}"; + log.debug(logMsg, flowEntry); if (installedList.size() == 1) { // If we had only one entry to remove, we are done - return new Status(StatusCode.SUCCESS, null); + return new Status(StatusCode.SUCCESS); } else { continue; } } // Remove and update DB - Status ret = removeEntryInternal(entry); + Status ret = removeEntryInternal(entry, async); if (!ret.isSuccess()) { error = ret; - log.warn("Failed to remove the entry: " + ret.getDescription()); + log.warn("Failed to remove the entry: {}. The failure is: {}", + entry.getInstall(), ret.getDescription()); if (installedList.size() == 1) { // If we had only one entry to remove, this is fatal failure return error; } } else { + succeeded = ret; atLeastOneRemoved = true; } } @@ -493,8 +547,7 @@ public class ForwardingRulesManagerImpl implements IForwardingRulesManager, * of removing the stale entries later, or adjusting the software * database if not in sync with hardware */ - return (atLeastOneRemoved) ? new Status(StatusCode.SUCCESS, null) - : error; + return (atLeastOneRemoved) ? succeeded : error; } /** @@ -503,20 +556,28 @@ public class ForwardingRulesManagerImpl implements IForwardingRulesManager, * validity checks are passed * * @param entry - * the FlowEntryInstall - * @return "Success" or error string + * the flow entry to remove + * @param async + * the flag indicating if this is a asynchronous request + * @return the status of this request. In case of asynchronous call, it + * will contain the unique id assigned to this request */ - private Status removeEntryInternal(FlowEntryInstall entry) { + private Status removeEntryInternal(FlowEntryInstall entry, boolean async) { // Mark the entry to be deleted (for CC just in case we fail) entry.toBeDeleted(); // Remove from node - Status status = programmer.removeFlow(entry.getNode(), entry - .getInstall().getFlow()); + Status status = (async)? + programmer.removeFlowAsync(entry.getNode(), entry + .getInstall().getFlow()) : + programmer.removeFlow(entry.getNode(), entry + .getInstall().getFlow()); + if (!status.isSuccess()) { - log.warn("SDN Plugin failed to remove the flow: " - + status.getDescription()); + log.warn( + "SDN Plugin failed to program the flow: {}. The failure is: {}", + entry.getInstall(), status.getDescription()); return status; } log.trace("Removed {}", entry.getInstall()); @@ -534,23 +595,32 @@ public class ForwardingRulesManagerImpl implements IForwardingRulesManager, * whether this flow would conflict or overwrite an existing one. * * @param entry - * the FlowEntryInstall - * @return "Success" or error string + * the flow entry to install + * @param async + * the flag indicating if this is a asynchronous request + * @return the status of this request. In case of asynchronous call, it + * will contain the unique id assigned to this request */ - private Status addEntriesInternal(FlowEntryInstall entry) { + private Status addEntriesInternal(FlowEntryInstall entry, boolean async) { // Install the flow on the network node - Status status = programmer.addFlow(entry.getNode(), entry.getInstall() - .getFlow()); + Status status = (async)? + programmer.addFlowAsync(entry.getNode(), entry.getInstall() + .getFlow()) : + programmer.addFlow(entry.getNode(), entry.getInstall() + .getFlow()); + if (!status.isSuccess()) { - log.warn("SDN Plugin failed to program the flow: " - + status.getDescription()); + log.warn( + "SDN Plugin failed to program the flow: {}. The failure is: {}", + entry.getInstall(), status.getDescription()); return status; } log.trace("Added {}", entry.getInstall()); // Update DB + entry.setRequestId(status.getRequestId()); updateLocalDatabase(entry, true); return status; @@ -597,7 +667,7 @@ public class ForwardingRulesManagerImpl implements IForwardingRulesManager, /* * Update the node mapped flows database */ - private void updateNodeFlowsDB(FlowEntryInstall flowEntries, boolean add) { + private synchronized void updateNodeFlowsDB(FlowEntryInstall flowEntries, boolean add) { Node node = flowEntries.getNode(); Set flowEntrylist = this.nodeFlows.get(node); @@ -681,7 +751,7 @@ public class ForwardingRulesManagerImpl implements IForwardingRulesManager, * entry is effectively present in the local database */ @SuppressWarnings("unused") - private Status removeEntry(Node node, String flowName) { + private synchronized Status removeEntry(Node node, String flowName) { FlowEntryInstall target = null; // Find in database @@ -704,6 +774,11 @@ public class ForwardingRulesManagerImpl implements IForwardingRulesManager, // Update DB if (status.isSuccess()) { updateLocalDatabase(target, false); + } else { + // log the error + log.warn( + "SDN Plugin failed to remove the flow: {}. The failure is: {}", + target.getInstall(), status.getDescription()); } return status; @@ -711,13 +786,27 @@ public class ForwardingRulesManagerImpl implements IForwardingRulesManager, @Override public Status installFlowEntry(FlowEntry flowEntry) { + Status status; + if (inContainerMode) { + String msg = "Controller in container mode: Install Refused"; + String logMsg = msg + ": {}"; + status = new Status(StatusCode.NOTACCEPTABLE, msg); + log.warn(logMsg, flowEntry); + } else { + status = addEntry(flowEntry, false); + } + return status; + } + + @Override + public Status installFlowEntryAsync(FlowEntry flowEntry) { Status status; if (inContainerMode) { String msg = "Controller in container mode: Install Refused"; status = new Status(StatusCode.NOTACCEPTABLE, msg); log.warn(msg); } else { - status = addEntry(flowEntry); + status = addEntry(flowEntry, true); } return status; } @@ -727,24 +816,52 @@ public class ForwardingRulesManagerImpl implements IForwardingRulesManager, Status status; if (inContainerMode) { String msg = "Controller in container mode: Uninstall Refused"; + String logMsg = msg + ": {}"; status = new Status(StatusCode.NOTACCEPTABLE, msg); - log.warn(msg); + log.warn(logMsg, entry); } else { - status = removeEntry(entry); + status = removeEntry(entry, false); } return status; } + @Override + public Status uninstallFlowEntryAsync(FlowEntry flowEntry) { + Status status; + if (inContainerMode) { + String msg = "Controller in container mode: Uninstall Refused"; + status = new Status(StatusCode.NOTACCEPTABLE, msg); + log.warn(msg); + } else { + status = removeEntry(flowEntry, true); + } + return status; + } + @Override public Status modifyFlowEntry(FlowEntry currentFlowEntry, FlowEntry newFlowEntry) { Status status = null; + if (inContainerMode) { + String msg = "Controller in container mode: Modify Refused"; + String logMsg = msg + ": {}"; + status = new Status(StatusCode.NOTACCEPTABLE, msg); + log.warn(logMsg, newFlowEntry); + } else { + status = modifyEntry(currentFlowEntry, newFlowEntry, false); + } + return status; + } + + @Override + public Status modifyFlowEntryAsync(FlowEntry current, FlowEntry newone) { + Status status = null; if (inContainerMode) { String msg = "Controller in container mode: Modify Refused"; status = new Status(StatusCode.NOTACCEPTABLE, msg); log.warn(msg); } else { - status = modifyEntry(currentFlowEntry, newFlowEntry); + status = modifyEntry(current, newone, true); } return status; } @@ -770,6 +887,28 @@ public class ForwardingRulesManagerImpl implements IForwardingRulesManager, } } + @Override + public Status modifyOrAddFlowEntryAsync(FlowEntry newone) { + /* + * Run a loose check on the installed entries to decide whether to go + * with a add or modify method. A loose check means only check against + * the original flow entry requests and not against the installed flow + * entries which are the result of the original entry merged with the + * container flow(s) (if any). The modifyFlowEntry method in presence of + * conflicts with the Container flows (if any) would revert back to a + * delete + add pattern + */ + FlowEntryInstall currentFlowEntries = findMatch(newone, true); + + if (currentFlowEntries != null) { + return modifyFlowEntryAsync(currentFlowEntries.getOriginal(), + newone); + } else { + return installFlowEntryAsync(newone); + } + } + + /** * Try to find in the database if a Flow with the same Match and priority of * the passed one already exists for the specified network node. Flow, @@ -784,7 +923,7 @@ public class ForwardingRulesManagerImpl implements IForwardingRulesManager, * @return null if not found, otherwise the FlowEntryInstall which contains * the existing flow entry */ - private FlowEntryInstall findMatch(FlowEntry flowEntry, boolean looseCheck) { + private synchronized FlowEntryInstall findMatch(FlowEntry flowEntry, boolean looseCheck) { Flow flow = flowEntry.getFlow(); Match match = flow.getMatch(); short priority = flow.getPriority(); @@ -814,7 +953,7 @@ public class ForwardingRulesManagerImpl implements IForwardingRulesManager, * merged flow may conflict with an existing old container flows merged flow * on the network node */ - private void updateFlowsContainerFlow() { + private synchronized void updateFlowsContainerFlow() { List oldCouples = new ArrayList(); List toReinstall = new ArrayList(); for (Entry> entry : this.nodeFlows @@ -833,7 +972,7 @@ public class ForwardingRulesManagerImpl implements IForwardingRulesManager, // Remove the old couples. No validity checks to be run, use the // internal remove for (FlowEntryInstall oldCouple : oldCouples) { - this.removeEntryInternal(oldCouple); + this.removeEntryInternal(oldCouple, false); } // Reinstall the original flow entries, via the regular path: new // cFlow merge + validations @@ -879,7 +1018,7 @@ public class ForwardingRulesManagerImpl implements IForwardingRulesManager, try { frma.policyUpdate(policyname, add); } catch (Exception e) { - log.error("Exception on callback", e); + log.warn("Exception on callback", e); } } } @@ -902,19 +1041,17 @@ public class ForwardingRulesManagerImpl implements IForwardingRulesManager, @Override public List getFlowEntriesForGroup(String policyName) { - List list = null; + List list = new ArrayList(); if (this.groupFlows != null && this.groupFlows.containsKey(policyName)) { - list = new ArrayList(); for (FlowEntryInstall entries : groupFlows.get(policyName)) { list.add(entries.getOriginal()); } - return new ArrayList(); } return list; } @Override - public void addOutputPort(Node node, String flowName, + public synchronized void addOutputPort(Node node, String flowName, List portList) { Set flowEntryList = this.nodeFlows.get(node); @@ -926,14 +1063,15 @@ public class ForwardingRulesManagerImpl implements IForwardingRulesManager, for (NodeConnector dstPort : portList) { newFlowEntry.getFlow().addAction(new Output(dstPort)); } - Status error = modifyEntry(currentFlowEntry, newFlowEntry); + Status error = modifyEntry(currentFlowEntry, newFlowEntry, false); if (error.isSuccess()) { log.info("Ports {} added to FlowEntry {}", portList, flowName); } else { - log.warn("Failed to add ports {} to Flow entry {}: " - + error.getDescription(), portList, - currentFlowEntry.toString()); + log.warn( + "Failed to add ports {} to Flow entry {}. The failure is: {}", + portList, currentFlowEntry.toString(), + error.getDescription()); } return; } @@ -943,7 +1081,7 @@ public class ForwardingRulesManagerImpl implements IForwardingRulesManager, } @Override - public void removeOutputPort(Node node, String flowName, + public synchronized void removeOutputPort(Node node, String flowName, List portList) { Set flowEntryList = this.nodeFlows.get(node); @@ -956,14 +1094,15 @@ public class ForwardingRulesManagerImpl implements IForwardingRulesManager, Action action = new Output(dstPort); newFlowEntry.getFlow().removeAction(action); } - Status status = modifyEntry(currentFlowEntry, newFlowEntry); + Status status = modifyEntry(currentFlowEntry, newFlowEntry, false); if (status.isSuccess()) { log.info("Ports {} removed from FlowEntry {}", portList, flowName); } else { - log.warn("Failed to remove ports {} from Flow entry {}: " - + status.getDescription(), portList, - currentFlowEntry.toString()); + log.warn( + "Failed to remove ports {} from Flow entry {}. The failure is: {}", + portList, currentFlowEntry.toString(), + status.getDescription()); } return; } @@ -977,7 +1116,7 @@ public class ForwardingRulesManagerImpl implements IForwardingRulesManager, * This function assumes the target flow has only one output port */ @Override - public void replaceOutputPort(Node node, String flowName, + public synchronized void replaceOutputPort(Node node, String flowName, NodeConnector outPort) { FlowEntry currentFlowEntry = null; FlowEntry newFlowEntry = null; @@ -1010,20 +1149,21 @@ public class ForwardingRulesManagerImpl implements IForwardingRulesManager, newFlowEntry.getFlow().addAction(new Output(outPort)); // Modify on network node - Status status = modifyEntry(currentFlowEntry, newFlowEntry); + Status status = modifyEntry(currentFlowEntry, newFlowEntry, false); if (status.isSuccess()) { - log.info("Output port replaced with " + outPort - + " for flow {} on node {}", flowName, node); + log.info("Output port replaced with {} for flow {} on node {}", + outPort, flowName, node); } else { - log.warn("Failed to replace output port for flow {} on node {}: ", - status.getDescription(), flowName, node); + log.warn( + "Failed to replace output port for flow {} on node {}. The failure is: {}", + flowName, node, status.getDescription()); } return; } @Override - public NodeConnector getOutputPort(Node node, String flowName) { + public synchronized NodeConnector getOutputPort(Node node, String flowName) { Set flowEntryList = this.nodeFlows.get(node); for (FlowEntryInstall flow : flowEntryList) { @@ -1079,9 +1219,9 @@ public class ForwardingRulesManagerImpl implements IForwardingRulesManager, EnumSet.of(IClusterServices.cacheMode.NON_TRANSACTIONAL)); } catch (CacheConfigException cce) { - log.error("FRM CacheConfigException"); + log.error("FRM CacheConfigException", cce); } catch (CacheExistException cce) { - log.error("FRM CacheExistException"); + log.error("FRM CacheExistException", cce); } } @@ -1211,13 +1351,17 @@ public class ForwardingRulesManagerImpl implements IForwardingRulesManager, // Presence check if (flowConfigExists(config)) { error = "Entry with this name on specified switch already exists"; + log.warn( + "Entry with this name on specified switch already exists: {}", + config); config.setStatus(error); return new Status(StatusCode.CONFLICT, error); } // Skip validation check if we are trying to restore a saved config if (!restore && !config.isValid(container, resultStr)) { - log.debug(resultStr.toString()); + log.warn("Invalid Configuration for flow {}. The failure is {}", + config, resultStr.toString()); error = "Invalid Configuration (" + resultStr.toString() + ")"; config.setStatus(error); return new Status(StatusCode.BADREQUEST, error); @@ -1231,7 +1375,9 @@ public class ForwardingRulesManagerImpl implements IForwardingRulesManager, } } if (!multipleFlowPush) { - log.debug(resultStr.toString()); + log.warn( + "Invalid Configuration(Invalid PortGroup Name) for flow {}", + config); error = "Invalid Configuration (Invalid PortGroup Name)"; config.setStatus(error); return new Status(StatusCode.BADREQUEST, error); @@ -1246,7 +1392,7 @@ public class ForwardingRulesManagerImpl implements IForwardingRulesManager, // Program hw if (config.installInHw()) { FlowEntry entry = config.getFlowEntry(); - status = this.addEntry(entry); + status = this.addEntry(entry, false); if (!status.isSuccess()) { config.setStatus(status.getDescription()); if (!restore) { @@ -1282,7 +1428,7 @@ public class ForwardingRulesManagerImpl implements IForwardingRulesManager, portGroupChanged(pgconfig, existingData, true); } } - return new Status(StatusCode.SUCCESS, null); + return new Status(StatusCode.SUCCESS); } private void addStaticFlowsToSwitch(Node node) { @@ -1294,7 +1440,7 @@ public class ForwardingRulesManagerImpl implements IForwardingRulesManager, if (config.installInHw() && !config.getStatus().equals( StatusCode.SUCCESS.toString())) { - Status status = this.addEntry(config.getFlowEntry()); + Status status = this.addEntry(config.getFlowEntry(), false); config.setStatus(status.getDescription()); } } @@ -1302,7 +1448,7 @@ public class ForwardingRulesManagerImpl implements IForwardingRulesManager, } private void updateStaticFlowConfigsOnNodeDown(Node node) { - log.trace("Updating Static Flow configs on node down: " + node); + log.trace("Updating Static Flow configs on node down: {}", node); List toRemove = new ArrayList(); for (Entry entry : staticFlows.entrySet()) { @@ -1329,8 +1475,8 @@ public class ForwardingRulesManagerImpl implements IForwardingRulesManager, } private void updateStaticFlowConfigsOnContainerModeChange(UpdateType update) { - log.trace("Updating Static Flow configs on container mode change: " - + update); + log.trace("Updating Static Flow configs on container mode change: {}", + update); for (FlowConfig config : staticFlows.values()) { if (config.isPortGroupEnabled()) { @@ -1361,7 +1507,7 @@ public class ForwardingRulesManagerImpl implements IForwardingRulesManager, for (Map.Entry entry : staticFlows.entrySet()) { if (entry.getValue().isByNameAndNodeIdEqual(config)) { // Program the network node - Status status = this.removeEntry(config.getFlowEntry()); + Status status = this.removeEntry(config.getFlowEntry(), false); // Update configuration database if programming was successful if (status.isSuccess()) { staticFlows.remove(entry.getKey()); @@ -1385,12 +1531,13 @@ public class ForwardingRulesManagerImpl implements IForwardingRulesManager, if (entry.isInternalFlow()) { String msg = "Invalid operation: Controller generated " + "flow cannot be deleted"; - log.warn(msg); + String logMsg = msg + ": {}"; + log.warn(logMsg, name); return new Status(StatusCode.NOTACCEPTABLE, msg); } if (!entry.isPortGroupEnabled()) { // Program the network node - status = this.removeEntry(entry.getFlowEntry()); + status = this.removeEntry(entry.getFlowEntry(), false); } // Update configuration database if programming was successful if (status.isSuccess()) { @@ -1410,7 +1557,8 @@ public class ForwardingRulesManagerImpl implements IForwardingRulesManager, if (newFlowConfig.isInternalFlow()) { String msg = "Invalid operation: Controller generated flow " + "cannot be modified"; - log.warn(msg); + String logMsg = msg + ": {}"; + log.warn(logMsg, newFlowConfig); return new Status(StatusCode.NOTACCEPTABLE, msg); } @@ -1419,7 +1567,8 @@ public class ForwardingRulesManagerImpl implements IForwardingRulesManager, if (!newFlowConfig.isValid(container, resultStr)) { String msg = "Invalid Configuration (" + resultStr.toString() + ")"; newFlowConfig.setStatus(msg); - log.warn(msg); + log.warn("Invalid Configuration for flow {}. The failure is {}", + newFlowConfig, resultStr.toString()); return new Status(StatusCode.BADREQUEST, msg); } @@ -1437,14 +1586,17 @@ public class ForwardingRulesManagerImpl implements IForwardingRulesManager, if (oldFlowConfig == null) { String msg = "Attempt to modify a non existing static flow"; - log.warn(msg); + String logMsg = msg + ": {}"; + log.warn(logMsg, newFlowConfig); return new Status(StatusCode.NOTFOUND, msg); } // Do not attempt to reinstall the flow, warn user if (newFlowConfig.equals(oldFlowConfig)) { String msg = "No modification detected"; - log.info("Static flow modification skipped: " + msg); + log.info( + "Static flow modification skipped. New flow and old flow are the same: {}", + newFlowConfig); return new Status(StatusCode.SUCCESS, msg); } @@ -1452,7 +1604,7 @@ public class ForwardingRulesManagerImpl implements IForwardingRulesManager, Status status = new Status(StatusCode.SUCCESS, "Saved in config"); if (oldFlowConfig.installInHw()) { status = this.modifyEntry(oldFlowConfig.getFlowEntry(), - newFlowConfig.getFlowEntry()); + newFlowConfig.getFlowEntry(), false); } // Update configuration database if programming was successful @@ -1480,7 +1632,8 @@ public class ForwardingRulesManagerImpl implements IForwardingRulesManager, if (config.isInternalFlow()) { String msg = "Invalid operation: Controller generated flow " + "cannot be modified"; - log.warn(msg); + String logMsg = msg + ": {}"; + log.warn(logMsg, config); return new Status(StatusCode.NOTACCEPTABLE, msg); } @@ -1488,11 +1641,11 @@ public class ForwardingRulesManagerImpl implements IForwardingRulesManager, FlowConfig conf = entry.getValue(); if (conf.isByNameAndNodeIdEqual(config)) { // Program the network node - Status status = new Status(StatusCode.SUCCESS, null); + Status status = new Status(StatusCode.SUCCESS); if (conf.installInHw()) { - status = this.removeEntry(conf.getFlowEntry()); + status = this.removeEntry(conf.getFlowEntry(), false); } else { - status = this.addEntry(conf.getFlowEntry()); + status = this.addEntry(conf.getFlowEntry(), false); } if (!status.isSuccess()) { conf.setStatus(status.getDescription()); @@ -1528,11 +1681,10 @@ public class ForwardingRulesManagerImpl implements IForwardingRulesManager, // Now remove the entries for (FlowEntry flowEntry : inactiveFlows) { - Status status = this.removeEntry(flowEntry); + Status status = this.removeEntry(flowEntry, false); if (!status.isSuccess()) { - log.warn( - "Failed to remove entry: {}: " - + status.getDescription(), flowEntry); + log.warn("Failed to remove entry: {}. The failure is: {}", + flowEntry, status.getDescription()); } } } @@ -1546,11 +1698,10 @@ public class ForwardingRulesManagerImpl implements IForwardingRulesManager, log.info("Reinstalling all inactive flows"); for (FlowEntry flowEntry : this.inactiveFlows) { - Status status = this.addEntry(flowEntry); + Status status = this.addEntry(flowEntry, false); if (!status.isSuccess()) { - log.warn( - "Failed to install entry: {}: " - + status.getDescription(), flowEntry); + log.warn("Failed to install entry: {}. The failure is: {}", + flowEntry, status.getDescription()); } } @@ -1777,7 +1928,7 @@ public class ForwardingRulesManagerImpl implements IForwardingRulesManager, * @param node */ private synchronized void cleanDatabaseForNode(Node node) { - log.info("Cleaning Flow database for Node " + node.toString()); + log.info("Cleaning Flow database for Node {}", node.toString()); // Find out which groups the node's flows are part of Set affectedGroups = new HashSet(); @@ -1884,7 +2035,7 @@ public class ForwardingRulesManagerImpl implements IForwardingRulesManager, @Override public void portGroupChanged(PortGroupConfig config, Map data, boolean add) { - log.info("PortGroup Changed for :" + config + " Data: " + portGroupData); + log.info("PortGroup Changed for: {} Data: {}", config, portGroupData); Map existingData = portGroupData.get(config); if (existingData != null) { for (Map.Entry entry : data.entrySet()) { @@ -2242,7 +2393,8 @@ public class ForwardingRulesManagerImpl implements IForwardingRulesManager, try { node = NodeCreator.createOFNode(Long.valueOf(nodeId)); } catch (NumberFormatException e) { - e.printStackTrace(); + ci.print("Node id not a number"); + return; } ci.println(this.programmer.addFlow(node, getSampleFlow(node))); } @@ -2258,7 +2410,8 @@ public class ForwardingRulesManagerImpl implements IForwardingRulesManager, try { node = NodeCreator.createOFNode(Long.valueOf(nodeId)); } catch (NumberFormatException e) { - e.printStackTrace(); + ci.print("Node id not a number"); + return; } ci.println(this.programmer.removeFlow(node, getSampleFlow(node))); } @@ -2357,4 +2510,82 @@ public class ForwardingRulesManagerImpl implements IForwardingRulesManager, } } + @Override + public void flowRemoved(Node node, Flow flow) { + log.trace("Received flow removed notification on {} for {}", node, flow); + // For flow entry identification, only match and priority matter + FlowEntry toFind = new FlowEntry("any", "any", flow, node); + FlowEntryInstall installedEntry = this.findMatch(toFind, false); + if (installedEntry == null) { + log.trace("Entry is not know to us"); + return; + } + + // Update Static flow status + for (Map.Entry entry : staticFlows.entrySet()) { + FlowConfig conf = entry.getValue(); + if (conf.isByNameAndNodeIdEqual(installedEntry.getFlowName(), node)) { + // Update Configuration database + conf.toggleStatus(); + break; + } + } + // Update software views + this.updateLocalDatabase(installedEntry, false); + } + + @Override + public synchronized void flowErrorReported(Node node, long rid, Object err) { + log.trace("Got error {} for message rid {} from node {}", + new Object[] {err, rid, node }); + /* + * If this was for a flow install, remove the corresponding entry + * from the software view. If it was a Looking for the rid going through the + * software database. + * TODO: A more efficient rid <-> FlowEntryInstall mapping will + * have to be added in future + */ + Set entries = nodeFlows.get(node); + if (entries != null) { + FlowEntryInstall target = null; + for (FlowEntryInstall entry : entries) { + if (entry.getRequestId() == rid) { + target = entry; + break; + } + } + if (target != null) { + // This was a flow install, update database + this.updateLocalDatabase(target, false); + } + } + + // Notify listeners + if (frmAware != null) { + synchronized (frmAware) { + for (IForwardingRulesManagerAware frma : frmAware) { + try { + frma.requestFailed(rid, err.toString()); + } catch (Exception e) { + log.warn("Failed to notify {}", frma); + } + } + } + } + } + + @Override + public Status solicitStatusResponse(Node node, boolean blocking) { + Status rv = new Status(StatusCode.INTERNALERROR); + + if (this.programmer != null) { + if (blocking) { + rv = programmer.syncSendBarrierMessage(node); + } else { + rv = programmer.asyncSendBarrierMessage(node); + } + } + + return rv; + } }