From c94e54cc1f58a5c313e424e9c5cde5af6c821185 Mon Sep 17 00:00:00 2001 From: Alessandro Boch Date: Thu, 25 Apr 2013 16:11:48 -0700 Subject: [PATCH] ForwardingRulesManager to expose async flow api Signed-off-by: Alessandro Boch --- .../FlowEntryInstall.java | 14 +- .../IForwardingRulesManager.java | 80 +++++- .../IForwardingRulesManagerAware.java | 14 + .../internal/ForwardingRulesManagerImpl.java | 268 ++++++++++++++---- .../openflow/core/internal/Controller.java | 5 +- .../openflow/core/internal/SwitchHandler.java | 2 +- .../internal/FlowProgrammerService.java | 38 +-- .../openflow/internal/Utils.java | 15 + .../IFlowProgrammerService.java | 38 ++- .../controller/sal/utils/Status.java | 40 ++- .../internal/FlowProgrammerService.java | 4 +- 11 files changed, 422 insertions(+), 96 deletions(-) diff --git a/opendaylight/forwardingrulesmanager/api/src/main/java/org/opendaylight/controller/forwardingrulesmanager/FlowEntryInstall.java b/opendaylight/forwardingrulesmanager/api/src/main/java/org/opendaylight/controller/forwardingrulesmanager/FlowEntryInstall.java index dfe331ad5f..96b6819e57 100644 --- a/opendaylight/forwardingrulesmanager/api/src/main/java/org/opendaylight/controller/forwardingrulesmanager/FlowEntryInstall.java +++ b/opendaylight/forwardingrulesmanager/api/src/main/java/org/opendaylight/controller/forwardingrulesmanager/FlowEntryInstall.java @@ -28,6 +28,7 @@ public class FlowEntryInstall { private FlowEntry original; private ContainerFlow cFlow; private FlowEntry install; + transient private long requestId; // async request id transient private boolean deletePending; public FlowEntryInstall(FlowEntry original, ContainerFlow cFlow) { @@ -36,6 +37,7 @@ public class FlowEntryInstall { this.install = (cFlow == null) ? original.clone() : original .mergeWith(cFlow); deletePending = false; + requestId = 0; } @Override @@ -84,9 +86,17 @@ public class FlowEntryInstall { this.deletePending = true; } + public void setRequestId(long rid) { + this.requestId = rid; + } + + public long getRequestId() { + return requestId; + } + @Override public String toString() { - return "[Install = " + install + " Original: " + original + " cFlow: " - + cFlow + "]"; + return "[Install = " + install + " Original = " + original + " cFlow = " + + cFlow + " rid = " + requestId + "]"; } } diff --git a/opendaylight/forwardingrulesmanager/api/src/main/java/org/opendaylight/controller/forwardingrulesmanager/IForwardingRulesManager.java b/opendaylight/forwardingrulesmanager/api/src/main/java/org/opendaylight/controller/forwardingrulesmanager/IForwardingRulesManager.java index d0efe4a83f..78917c9c80 100644 --- a/opendaylight/forwardingrulesmanager/api/src/main/java/org/opendaylight/controller/forwardingrulesmanager/IForwardingRulesManager.java +++ b/opendaylight/forwardingrulesmanager/api/src/main/java/org/opendaylight/controller/forwardingrulesmanager/IForwardingRulesManager.java @@ -73,12 +73,90 @@ public interface IForwardingRulesManager { * not valid an error code is returned. If the existing flow is equal to the * passed one it will be a no op and success code is returned. * + * * @param newone * the new flow entry to install * @return the {@code Status} object indicating the result of this action */ public Status modifyOrAddFlowEntry(FlowEntry newone); + /** + * It requests FRM to install the passed Flow Entry through an asynchronous + * call. A unique request id is returned to the caller. FRM will request the + * SDN protocol plugin to install the flow on the network node. As immediate + * result of this asynchronous call, FRM will update its flow database as if + * the flow was successfully installed. + * + * @param flow + * the flow entry to install + * @return the status of this request containing the request id associated + * to this asynchronous request + */ + public Status installFlowEntryAsync(FlowEntry flow); + + /** + * It requests FRM to remove the passed Flow Entry through an asynchronous + * call. A unique request id is returned to the caller. FRM will request the + * SDN protocol plugin to uninstall the flow from the network node. As + * immediate result of this asynchronous call, FRM will update its flow + * database as if the flow was successfully installed. + * + * @param flow + * the flow entry to uninstall + * @return the status of this request containing the unique id associated to + * this asynchronous request + */ + public Status uninstallFlowEntryAsync(FlowEntry flow); + + /** + * It requests FRM to replace the currently installed Flow Entry with the + * new one through an asynchronous call. A unique request id is returned to + * the caller. It is up to the SDN protocol plugin to decide how to convey + * this message to the network node. It could be a delete + add or a single + * modify message depending on the SDN protocol specifications. If the + * current flow is equal to the new one it will be a no op. + * + * @param current + * the current flow entry to modify + * @param newone + * the new flow entry which will replace the current one + * @return the status of this request containing the request id associated + * to this asynchronous request + */ + public Status modifyFlowEntryAsync(FlowEntry current, FlowEntry newone); + + /** + * It requests the FRM to replace the currently installed Flow Entry with + * the new one through an asynchronous call. A unique request id is returned + * to the caller. The currently installed entry is derived by the Match + * portion of the passed Flow. FRM looks in its database for a previously + * installed FlowEntry which Match equals the Match of the passed Flow. If + * it finds it, it will request the SDN protocol plugin to replace the + * existing flow with the new one on the network node. If it does not find + * it, it will request plugin to add the new flow. If the passed entry is + * not valid a zero request id is returned. If the existing flow is equal to + * the passed one it will be a no op. + * + * @param newone + * the new flow entry to install + * @return the unique id associated to this request. In case of not + * acceptable request -1 will be returned. + */ + public Status modifyOrAddFlowEntryAsync(FlowEntry newone); + + /** + * Requests ForwardingRulesManager to solicit the network node to inform + * us about the status of his execution on the asynchronous requests that + * were sent to it so far. It is a way for an application to poke the + * network node in order to get a feedback asap on the asynchronous + * requests generated by the application. It is a non-blocking call + * and does not guarantee the node will respond in any given time. + * + * @param node + * The network node to solicit a response + */ + public void solicitStatusResponse(Node node); + /** * Check whether the passed flow entry conflicts with the Container flows * @@ -209,7 +287,7 @@ public interface IForwardingRulesManager { * @param name * the flow name * @param n - * the netwrok node identifier + * the network node identifier * @return the {@code FlowConfig} object */ public FlowConfig getStaticFlow(String name, Node n); diff --git a/opendaylight/forwardingrulesmanager/api/src/main/java/org/opendaylight/controller/forwardingrulesmanager/IForwardingRulesManagerAware.java b/opendaylight/forwardingrulesmanager/api/src/main/java/org/opendaylight/controller/forwardingrulesmanager/IForwardingRulesManagerAware.java index e2ccdd60c3..43d1b29a75 100644 --- a/opendaylight/forwardingrulesmanager/api/src/main/java/org/opendaylight/controller/forwardingrulesmanager/IForwardingRulesManagerAware.java +++ b/opendaylight/forwardingrulesmanager/api/src/main/java/org/opendaylight/controller/forwardingrulesmanager/IForwardingRulesManagerAware.java @@ -26,4 +26,18 @@ public interface IForwardingRulesManagerAware { * otherwise */ public void policyUpdate(String policyName, boolean add); + + /** + * Inform listeners that the network node has notified us about a failure in + * executing the controller generated asynchronous request identified by the + * passed unique id. + * + * @param requestId + * the unique id associated with the request which failed to be + * executed on the network node + * @param error + * the string describing the error reported by the network node + */ + public void requestFailed(long requestId, String error); + } 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 d5a4d96701..fe2cdc4738 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 @@ -138,9 +138,13 @@ 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"; @@ -190,14 +194,22 @@ public class ForwardingRulesManagerImpl implements IForwardingRulesManager, // 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: {}. The failure is: {}", @@ -205,7 +217,7 @@ public class ForwardingRulesManagerImpl implements IForwardingRulesManager, } } - return (oneSucceded) ? new Status(StatusCode.SUCCESS, null) : error; + return (oneSucceded) ? succeded : error; } /** @@ -251,10 +263,14 @@ 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 @@ -329,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 " @@ -354,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 { /* @@ -372,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 { @@ -394,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 { @@ -408,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; } /** @@ -419,14 +443,22 @@ 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( @@ -439,6 +471,7 @@ public class ForwardingRulesManagerImpl implements IForwardingRulesManager, newEntries.getInstall()); // Update DB + newEntries.setRequestId(status.getRequestId()); updateLocalDatabase(currentEntries, false); updateLocalDatabase(newEntries, true); @@ -450,9 +483,13 @@ 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 Status removeEntry(FlowEntry flowEntry, boolean async) { Status error = new Status(null, null); // Sanity Check @@ -468,6 +505,7 @@ 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) { @@ -481,14 +519,14 @@ public class ForwardingRulesManagerImpl implements IForwardingRulesManager, 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; @@ -499,6 +537,7 @@ public class ForwardingRulesManagerImpl implements IForwardingRulesManager, return error; } } else { + succeeded = ret; atLeastOneRemoved = true; } } @@ -508,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; } /** @@ -518,16 +556,23 @@ 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( @@ -535,7 +580,7 @@ public class ForwardingRulesManagerImpl implements IForwardingRulesManager, entry.getInstall(), status.getDescription()); return status; } - log.info("Removed {}", entry.getInstall()); + log.trace("Removed {}", entry.getInstall()); // Update DB updateLocalDatabase(entry, false); @@ -550,13 +595,20 @@ 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.addFlow(entry.getNode(), entry.getInstall() + .getFlow()) : + programmer.addFlowAsync(entry.getNode(), entry.getInstall() + .getFlow()); + if (!status.isSuccess()) { log.warn( @@ -565,9 +617,10 @@ public class ForwardingRulesManagerImpl implements IForwardingRulesManager, return status; } - log.info("Added {}", entry.getInstall()); + log.trace("Added {}", entry.getInstall()); // Update DB + entry.setRequestId(status.getRequestId()); updateLocalDatabase(entry, true); return status; @@ -740,7 +793,20 @@ public class ForwardingRulesManagerImpl implements IForwardingRulesManager, status = new Status(StatusCode.NOTACCEPTABLE, msg); log.warn(logMsg, flowEntry); } else { - status = addEntry(flowEntry); + 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, true); } return status; } @@ -754,11 +820,24 @@ public class ForwardingRulesManagerImpl implements IForwardingRulesManager, status = new Status(StatusCode.NOTACCEPTABLE, 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) { @@ -769,7 +848,20 @@ public class ForwardingRulesManagerImpl implements IForwardingRulesManager, status = new Status(StatusCode.NOTACCEPTABLE, msg); log.warn(logMsg, newFlowEntry); } else { - status = modifyEntry(currentFlowEntry, newFlowEntry); + 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(current, newone, true); } return status; } @@ -795,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 installFlowEntry(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, @@ -858,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 @@ -951,7 +1065,7 @@ 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); @@ -982,7 +1096,7 @@ 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); @@ -1037,7 +1151,7 @@ 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 {} for flow {} on node {}", @@ -1280,7 +1394,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) { @@ -1316,7 +1430,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) { @@ -1328,7 +1442,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()); } } @@ -1395,7 +1509,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()); @@ -1425,7 +1539,7 @@ public class ForwardingRulesManagerImpl implements IForwardingRulesManager, } 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()) { @@ -1492,7 +1606,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 @@ -1529,11 +1643,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()); @@ -1569,7 +1683,7 @@ 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: {}. The failure is: {}", flowEntry, status.getDescription()); @@ -1586,7 +1700,7 @@ 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: {}. The failure is: {}", flowEntry, status.getDescription()); @@ -2424,7 +2538,49 @@ public class ForwardingRulesManagerImpl implements IForwardingRulesManager, @Override public void flowErrorReported(Node node, long rid, Object err) { - log.error("Got error {} for message rid {} from node {}", new Object[] { - err, rid, node }); + 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 void solicitStatusResponse(Node node) { + if (this.programmer != null) { + programmer.sendBarrierMessage(node); + } } + } diff --git a/opendaylight/protocol_plugins/openflow/src/main/java/org/opendaylight/controller/protocol_plugin/openflow/core/internal/Controller.java b/opendaylight/protocol_plugins/openflow/src/main/java/org/opendaylight/controller/protocol_plugin/openflow/core/internal/Controller.java index 8e6a30fcee..bfa6f0ba0f 100644 --- a/opendaylight/protocol_plugins/openflow/src/main/java/org/opendaylight/controller/protocol_plugin/openflow/core/internal/Controller.java +++ b/opendaylight/protocol_plugins/openflow/src/main/java/org/opendaylight/controller/protocol_plugin/openflow/core/internal/Controller.java @@ -228,8 +228,9 @@ public class Controller implements IController, CommandProvider { instanceName); switchHandler.start(); if (sc.isConnected()) { - logger.info("Switch:{} is connected to the Controller", sc - .getRemoteAddress().toString().split("/")[1]); + logger.info("Switch:{} is connected to the Controller", + sc.socket().getRemoteSocketAddress() + .toString().split("/")[1]); } } catch (IOException e) { diff --git a/opendaylight/protocol_plugins/openflow/src/main/java/org/opendaylight/controller/protocol_plugin/openflow/core/internal/SwitchHandler.java b/opendaylight/protocol_plugins/openflow/src/main/java/org/opendaylight/controller/protocol_plugin/openflow/core/internal/SwitchHandler.java index ea659c82e1..57098ae3c6 100644 --- a/opendaylight/protocol_plugins/openflow/src/main/java/org/opendaylight/controller/protocol_plugin/openflow/core/internal/SwitchHandler.java +++ b/opendaylight/protocol_plugins/openflow/src/main/java/org/opendaylight/controller/protocol_plugin/openflow/core/internal/SwitchHandler.java @@ -573,7 +573,7 @@ public class SwitchHandler implements ISwitch { public String toString() { try { return ("Switch:" - + socket.getRemoteAddress().toString().split("/")[1] + + socket.socket().getRemoteSocketAddress().toString().split("/")[1] + " SWID:" + (isOperational() ? HexString .toHexString(this.sid) : "unknown")); } catch (Exception e) { diff --git a/opendaylight/protocol_plugins/openflow/src/main/java/org/opendaylight/controller/protocol_plugin/openflow/internal/FlowProgrammerService.java b/opendaylight/protocol_plugins/openflow/src/main/java/org/opendaylight/controller/protocol_plugin/openflow/internal/FlowProgrammerService.java index d473a462c4..1d19c8b45c 100644 --- a/opendaylight/protocol_plugins/openflow/src/main/java/org/opendaylight/controller/protocol_plugin/openflow/internal/FlowProgrammerService.java +++ b/opendaylight/protocol_plugins/openflow/src/main/java/org/opendaylight/controller/protocol_plugin/openflow/internal/FlowProgrammerService.java @@ -8,7 +8,6 @@ package org.opendaylight.controller.protocol_plugin.openflow.internal; -import java.nio.ByteBuffer; import java.util.ArrayList; import java.util.HashMap; import java.util.HashSet; @@ -24,15 +23,6 @@ import org.opendaylight.controller.protocol_plugin.openflow.IInventoryShimExtern import org.opendaylight.controller.protocol_plugin.openflow.core.IController; import org.opendaylight.controller.protocol_plugin.openflow.core.IMessageListener; import org.opendaylight.controller.protocol_plugin.openflow.core.ISwitch; -import org.opendaylight.controller.protocol_plugin.openflow.vendorextension.v6extension.V6Error; -import org.openflow.protocol.OFError; -import org.openflow.protocol.OFFlowMod; -import org.openflow.protocol.OFFlowRemoved; -import org.openflow.protocol.OFMessage; -import org.openflow.protocol.OFPort; -import org.openflow.protocol.OFType; -import org.openflow.protocol.action.OFAction; - import org.opendaylight.controller.sal.core.ContainerFlow; import org.opendaylight.controller.sal.core.IContainerListener; import org.opendaylight.controller.sal.core.Node; @@ -47,8 +37,15 @@ import org.opendaylight.controller.sal.match.MatchType; import org.opendaylight.controller.sal.utils.GlobalConstants; import org.opendaylight.controller.sal.utils.HexEncode; import org.opendaylight.controller.sal.utils.NodeCreator; -import org.opendaylight.controller.sal.utils.StatusCode; import org.opendaylight.controller.sal.utils.Status; +import org.opendaylight.controller.sal.utils.StatusCode; +import org.openflow.protocol.OFError; +import org.openflow.protocol.OFFlowMod; +import org.openflow.protocol.OFFlowRemoved; +import org.openflow.protocol.OFMessage; +import org.openflow.protocol.OFPort; +import org.openflow.protocol.OFType; +import org.openflow.protocol.action.OFAction; import org.osgi.framework.BundleContext; import org.osgi.framework.FrameworkUtil; import org.slf4j.Logger; @@ -208,22 +205,11 @@ public class FlowProgrammerService implements IPluginInFlowProgrammerService, } if (result instanceof Boolean) { return ((Boolean) result == Boolean.TRUE) ? new Status( - StatusCode.SUCCESS, null) : new Status( + StatusCode.SUCCESS, rid) : new Status( StatusCode.TIMEOUT, errorString(null, action, "Request Timed Out")); } else if (result instanceof OFError) { OFError res = (OFError) result; - if (res.getErrorType() == V6Error.NICIRA_VENDOR_ERRORTYPE) { - V6Error er = new V6Error(res); - byte[] b = res.getError(); - ByteBuffer bb = ByteBuffer.allocate(b.length); - bb.put(b); - bb.rewind(); - er.readFrom(bb); - return new Status(StatusCode.INTERNALERROR, - errorString("program", action, - "Vendor Extension Internal Error")); - } return new Status(StatusCode.INTERNALERROR, errorString( "program", action, Utils.getOFErrorString(res))); } else { @@ -286,7 +272,7 @@ public class FlowProgrammerService implements IPluginInFlowProgrammerService, return new Status(StatusCode.TIMEOUT, errorString(null, action, "Request Timed Out")); } else if (msg2 == null) { - return new Status(StatusCode.SUCCESS, null); + return new Status(StatusCode.SUCCESS, rid); } } else if (result instanceof OFError) { return new Status(StatusCode.INTERNALERROR, errorString( @@ -315,7 +301,7 @@ public class FlowProgrammerService implements IPluginInFlowProgrammerService, } if (result instanceof Boolean) { return ((Boolean) result == Boolean.TRUE) ? new Status( - StatusCode.SUCCESS, null) : new Status( + StatusCode.SUCCESS, rid) : new Status( StatusCode.TIMEOUT, errorString(null, action, "Request Timed Out")); } else if (result instanceof OFError) { @@ -364,7 +350,7 @@ public class FlowProgrammerService implements IPluginInFlowProgrammerService, } if (result instanceof Boolean) { return ((Boolean) result == Boolean.TRUE) ? new Status( - StatusCode.SUCCESS, null) : new Status( + StatusCode.SUCCESS, rid) : new Status( StatusCode.TIMEOUT, errorString(null, action, "Request Timed Out")); } else if (result instanceof OFError) { diff --git a/opendaylight/protocol_plugins/openflow/src/main/java/org/opendaylight/controller/protocol_plugin/openflow/internal/Utils.java b/opendaylight/protocol_plugins/openflow/src/main/java/org/opendaylight/controller/protocol_plugin/openflow/internal/Utils.java index 77a39e904a..f1ce03af28 100644 --- a/opendaylight/protocol_plugins/openflow/src/main/java/org/opendaylight/controller/protocol_plugin/openflow/internal/Utils.java +++ b/opendaylight/protocol_plugins/openflow/src/main/java/org/opendaylight/controller/protocol_plugin/openflow/internal/Utils.java @@ -9,6 +9,9 @@ package org.opendaylight.controller.protocol_plugin.openflow.internal; +import java.nio.ByteBuffer; + +import org.opendaylight.controller.protocol_plugin.openflow.vendorextension.v6extension.V6Error; import org.openflow.protocol.OFError; import org.openflow.protocol.OFError.OFBadActionCode; import org.openflow.protocol.OFError.OFBadRequestCode; @@ -20,6 +23,18 @@ import org.openflow.protocol.OFError.OFQueueOpFailedCode; public abstract class Utils { public static String getOFErrorString(OFError error) { + // Handle VENDOR extension errors here + if (error.getErrorType() == V6Error.NICIRA_VENDOR_ERRORTYPE) { + V6Error er = new V6Error(error); + byte[] b = error.getError(); + ByteBuffer bb = ByteBuffer.allocate(b.length); + bb.put(b); + bb.rewind(); + er.readFrom(bb); + return er.toString(); + } + + // Handle OF1.0 errors here OFErrorType et = OFErrorType.values()[0xffff & error.getErrorType()]; String errorStr = "Error : " + et.toString(); switch (et) { diff --git a/opendaylight/sal/api/src/main/java/org/opendaylight/controller/sal/flowprogrammer/IFlowProgrammerService.java b/opendaylight/sal/api/src/main/java/org/opendaylight/controller/sal/flowprogrammer/IFlowProgrammerService.java index 5cdf9d8368..dbd7eafa36 100644 --- a/opendaylight/sal/api/src/main/java/org/opendaylight/controller/sal/flowprogrammer/IFlowProgrammerService.java +++ b/opendaylight/sal/api/src/main/java/org/opendaylight/controller/sal/flowprogrammer/IFlowProgrammerService.java @@ -20,7 +20,10 @@ public interface IFlowProgrammerService { * Synchronously add a flow to the network node * * @param node + * The target network node * @param flow + * The flow to install + * @return The status of this request */ Status addFlow(Node node, Flow flow); @@ -28,7 +31,12 @@ public interface IFlowProgrammerService { * Synchronously modify existing flow on the switch * * @param node - * @param flow + * The target network node + * @param oldFlow + * The existing flow to modify + * @param newFlow + * The new flow to install + * @return The status of this request */ Status modifyFlow(Node node, Flow oldFlow, Flow newFlow); @@ -36,7 +44,10 @@ public interface IFlowProgrammerService { * Synchronously remove the flow from the network node * * @param node + * The target network node * @param flow + * The flow to remove + * @return The status of this request */ Status removeFlow(Node node, Flow flow); @@ -44,7 +55,10 @@ public interface IFlowProgrammerService { * Asynchronously add a flow to the network node * * @param node + * The target network node * @param flow + * The flow to install + * @return The status of this request containing the unique request id */ Status addFlowAsync(Node node, Flow flow); @@ -52,7 +66,12 @@ public interface IFlowProgrammerService { * Asynchronously modify existing flow on the switch * * @param node - * @param flow + * The target network node + * @param oldFlow + * The existing flow to modify + * @param newFlow + * The new flow to install + * @return The status of this request containing the unique request id */ Status modifyFlowAsync(Node node, Flow oldFlow, Flow newFlow); @@ -60,7 +79,10 @@ public interface IFlowProgrammerService { * Asynchronously remove the flow from the network node * * @param node + * The target network node * @param flow + * The flow to remove + * @return The status of this request containing the unique request id */ Status removeFlowAsync(Node node, Flow flow); @@ -68,13 +90,23 @@ public interface IFlowProgrammerService { * Remove all flows present on the network node * * @param node + * The target network node + * @return The status of this request containing the unique request id */ Status removeAllFlows(Node node); /** - * Send synchronous Barrier message + * Send synchronous Barrier message + * + * Solicit the network node to report whether all the requests sent so far + * are completed. When this call is done, caller knows that all past flow + * operations requested to the node in asynchronous fashion were satisfied + * by the network node and that in case of any failure, a message was sent + * to the controller. * * @param node + * The network node to solicit + * @return The status of this request containing the unique request id */ Status sendBarrierMessage(Node node); } diff --git a/opendaylight/sal/api/src/main/java/org/opendaylight/controller/sal/utils/Status.java b/opendaylight/sal/api/src/main/java/org/opendaylight/controller/sal/utils/Status.java index 503e08e714..2fbb3e55f8 100644 --- a/opendaylight/sal/api/src/main/java/org/opendaylight/controller/sal/utils/Status.java +++ b/opendaylight/sal/api/src/main/java/org/opendaylight/controller/sal/utils/Status.java @@ -14,8 +14,9 @@ package org.opendaylight.controller.sal.utils; * a string which describes a failure reason (if any) in human readable form. */ public class Status { - StatusCode code; - String description; + private StatusCode code; + private String description; + private long requestId; /** * Generates an instance of the Status class. This is used as return code @@ -34,6 +35,7 @@ public class Status { this.code = (errorCode != null) ? errorCode : StatusCode.UNDEFINED; this.description = (description != null) ? description : this.code .toString(); + this.requestId = 0; } /** @@ -49,6 +51,27 @@ public class Status { this.code = (errorCode != null) ? errorCode : StatusCode.UNDEFINED; this.description = (description != null) ? description : this.code .toString(); + this.requestId = 0; + } + + /** + * Generates an instance of the Status class to be used in case of + * asynchronous call. It is supposed to be created by the underlying + * infrastructure only when it was successful in allocating the asynchronous + * request id, hence caller should expect StatusCode to be successful. + * + * @param errorCode + * The status code. If passed as null, code will be stored as + * {@code StatusCode.UNDEFINED} + * @param requestId + * The request id set by underlying infrastructure for this + * request + */ + public Status(StatusCode errorCode, long requestId) { + this.code = (errorCode != null) ? errorCode : StatusCode.UNDEFINED; + this.description = (description != null) ? description : this.code + .toString(); + this.requestId = requestId; } /** @@ -78,9 +101,20 @@ public class Status { return code == StatusCode.SUCCESS; } + /** + * Return the request id assigned by underlying infrastructure in case of + * asynchronous request. In case of synchronous requests, the returned id + * is expected to be 0 + * + * @return The request id assigned for this asynchronous request + */ + public long getRequestId() { + return requestId; + } + @Override public String toString() { - return code + ": " + description; + return code + ": " + description + " (" + requestId + ")"; } @Override diff --git a/opendaylight/sal/implementation/src/main/java/org/opendaylight/controller/sal/implementation/internal/FlowProgrammerService.java b/opendaylight/sal/implementation/src/main/java/org/opendaylight/controller/sal/implementation/internal/FlowProgrammerService.java index 1c63e4e200..0abebf8352 100644 --- a/opendaylight/sal/implementation/src/main/java/org/opendaylight/controller/sal/implementation/internal/FlowProgrammerService.java +++ b/opendaylight/sal/implementation/src/main/java/org/opendaylight/controller/sal/implementation/internal/FlowProgrammerService.java @@ -28,8 +28,8 @@ import org.opendaylight.controller.sal.action.PopVlan; import org.opendaylight.controller.sal.action.SetNwDst; import org.opendaylight.controller.sal.core.ConstructionException; import org.opendaylight.controller.sal.core.Node; -import org.opendaylight.controller.sal.core.NodeConnector; import org.opendaylight.controller.sal.core.Node.NodeIDType; +import org.opendaylight.controller.sal.core.NodeConnector; import org.opendaylight.controller.sal.flowprogrammer.Flow; import org.opendaylight.controller.sal.flowprogrammer.IFlowProgrammerListener; import org.opendaylight.controller.sal.flowprogrammer.IFlowProgrammerService; @@ -37,11 +37,11 @@ import org.opendaylight.controller.sal.flowprogrammer.IPluginInFlowProgrammerSer import org.opendaylight.controller.sal.flowprogrammer.IPluginOutFlowProgrammerService; 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.IPProtocols; import org.opendaylight.controller.sal.utils.NodeConnectorCreator; import org.opendaylight.controller.sal.utils.Status; +import org.opendaylight.controller.sal.utils.StatusCode; import org.osgi.framework.BundleContext; import org.osgi.framework.FrameworkUtil; import org.slf4j.Logger; -- 2.36.6