diff --git a/build.xml b/build.xml
index c94be94c1475fafe689bd3811b2e9fbfade75047..f5f26f3ebae508894b1f87003e8ccd97e260774f 100644
--- a/build.xml
+++ b/build.xml
@@ -43,7 +43,6 @@
<include name="logback-core-1.0.0.jar"/>
<include name="jackson-core-asl-1.8.6.jar"/>
<include name="jackson-mapper-asl-1.8.6.jar"/>
- <include name="org.easymock_2.5.2.jar"/>
<include name="slf4j-api-1.6.4.jar"/>
<include name="org.restlet-2.1-RC1.jar"/>
<include name="org.restlet.ext.jackson-2.1-RC1.jar"/>
@@ -84,7 +83,9 @@
<patternset id="lib-test">
<include name="junit-4.8.2.jar"/>
- <include name="org.easymock_2.5.2.jar"/>
+ <include name="org.easymock-3.1.jar"/>
+ <include name="objenesis-1.2.jar"/> <!-- required by easymock to mock classes -->
+ <include name="cglib-nodep-2.2.2.jar"/> <!-- required by easymock to mock classes -->
</patternset>
<path id="classpath-test">
<fileset dir="lib">
diff --git a/lib/org.easymock_2.5.2.jar b/lib/org.easymock_2.5.2.jar
deleted file mode 100644
index 97c8bf0acbb56bfc38845a25ca03f5d98037169e..0000000000000000000000000000000000000000
Binary files a/lib/org.easymock_2.5.2.jar and /dev/null differ
diff --git a/src/main/java/net/floodlightcontroller/core/IOFSwitch.java b/src/main/java/net/floodlightcontroller/core/IOFSwitch.java
index 11212ea084e4aefe0cf6f79d581e2809752e3b01..b6188dcd63131bbeb44de0a3d452a83e41bdb27b 100644
--- a/src/main/java/net/floodlightcontroller/core/IOFSwitch.java
+++ b/src/main/java/net/floodlightcontroller/core/IOFSwitch.java
@@ -51,6 +51,7 @@ public interface IOFSwitch {
/**
* Writes to the OFMessage to the output stream.
* The message will be handed to the floodlightProvider for possible filtering
+ * and processing by message listeners
* @param m
* @param bc
* @throws IOException
@@ -60,6 +61,7 @@ public interface IOFSwitch {
/**
* Writes the list of messages to the output stream
* The message will be handed to the floodlightProvider for possible filtering
+ * and processing by message listeners.
* @param msglist
* @param bc
* @throws IOException
@@ -73,7 +75,8 @@ public interface IOFSwitch {
public void disconnectOutputStream();
/**
- *
+ * FIXME: remove getChannel(). All access to the channel should be through
+ * wrapper functions in IOFSwitch
* @return
*/
public Channel getChannel();
@@ -214,12 +217,6 @@ public interface IOFSwitch {
*/
public Role getRole();
- /**
- * Set the role of the controller for the switch
- * @param role controller role
- */
- public void setRole(Role role);
-
/**
* Check if the controller is an active controller for the switch.
* The controller is active if its role is MASTER or EQUAL.
diff --git a/src/main/java/net/floodlightcontroller/core/internal/Controller.java b/src/main/java/net/floodlightcontroller/core/internal/Controller.java
index d65972029dc260bb919b783bbd95bb1a53e17861..ae2b192dc07e8c6e9516e7b7a10bd3b1c3b433e3 100644
--- a/src/main/java/net/floodlightcontroller/core/internal/Controller.java
+++ b/src/main/java/net/floodlightcontroller/core/internal/Controller.java
@@ -24,9 +24,11 @@ import java.net.InetSocketAddress;
import java.net.SocketAddress;
import java.util.ArrayList;
import java.nio.channels.ClosedChannelException;
+import java.util.Collection;
import java.util.Collections;
import java.util.Date;
import java.util.HashMap;
+import java.util.HashSet;
import java.util.Iterator;
import java.util.LinkedHashMap;
import java.util.List;
@@ -145,7 +147,9 @@ public class Controller implements IFloodlightProviderService,
// connectedSwitches contains all connected switches, including ones where
// we're a slave controller. We need to keep track of them so that we can
// send role request messages to switches when our role changes to master
- protected ConcurrentHashMap<Long, IOFSwitch> connectedSwitches;
+ // We add a switch to this set after it successfully completes the
+ // handshake. Access to this Set needs to be synchronized with roleChanger
+ protected HashSet<OFSwitchImpl> connectedSwitches;
// The controllerNodeIPsCache maps Controller IDs to their IP address.
// It's only used by handleControllerNodeIPsChanged
@@ -173,6 +177,8 @@ public class Controller implements IFloodlightProviderService,
// The current role of the controller.
// If the controller isn't configured to support roles, then this is null.
protected Role role;
+ // A helper that handles sending and timeout handling for role requests
+ protected RoleChanger roleChanger;
// Storage table names
protected static final String CONTROLLER_TABLE_NAME = "controller_controller";
@@ -343,128 +349,29 @@ public class Controller implements IFloodlightProviderService,
@Override
public synchronized void setRole(Role role) {
if (role == null) throw new NullPointerException("Role can not be null.");
- Role oldRole = this.role;
- this.role = role;
- // Send role request messages to all of the connected switches.
- // FIXME: Should maybe handle this in an asynchronous task.
- for (IOFSwitch sw: connectedSwitches.values()) {
+ // Need to synchronize to ensure a reliable ordering on role request
+ // messages send and to ensure the list of connected switches is stable
+ // RoleChanger will handle the actual sending of the message and
+ // timeout handling
+ // @see RoleChanger
+ synchronized(roleChanger) {
+ Role oldRole = this.role;
+ this.role = role;
+
+ log.debug("Submitting role change request to role {}", role);
+ roleChanger.submitRequest(connectedSwitches, role);
+
+ // Enqueue an update for our listeners.
try {
- sendRoleRequest(sw, role);
- }
- catch (IOException exc) {
- // FIXME: What's the right thing to do in this case?
- // Should we try to send it again later?
- // Terminate the switch connection?
- log.error("Error sending role request message to switch {}", sw);
+ this.updates.put(new HARoleUpdate(role, oldRole));
+ } catch (InterruptedException e) {
+ log.error("Failure adding update to queue", e);
}
}
-
- // Enqueue an update for our listeners.
- try {
- this.updates.put(new HARoleUpdate(role, oldRole));
- } catch (InterruptedException e) {
- log.error("Failure adding update to queue", e);
- }
}
- /**
- * Send NX role request message to the switch requesting the specified role.
- * @param sw switch to send the role request message to
- * @param role role to request
- * @return transaction id of the role request message that was sent
- */
- protected int sendNxRoleRequest(IOFSwitch sw, Role role)
- throws IOException {
- // Convert the role enum to the appropriate integer constant used
- // in the NX role request message
- int nxRole = 0;
- switch (role) {
- case EQUAL:
- nxRole = OFRoleVendorData.NX_ROLE_OTHER;
- break;
- case MASTER:
- nxRole = OFRoleVendorData.NX_ROLE_MASTER;
- break;
- case SLAVE:
- nxRole = OFRoleVendorData.NX_ROLE_SLAVE;
- break;
- default:
- assert false;
- }
-
- // Construct the role request message
- OFVendor roleRequest = (OFVendor)factory.getMessage(OFType.VENDOR);
- int xid = sw.getNextTransactionId();
- roleRequest.setXid(xid);
- roleRequest.setVendor(OFNiciraVendorData.NX_VENDOR_ID);
- OFRoleRequestVendorData roleRequestData = new OFRoleRequestVendorData();
- roleRequestData.setRole(nxRole);
- roleRequest.setVendorData(roleRequestData);
- roleRequest.setLengthU(OFVendor.MINIMUM_LENGTH +
- roleRequestData.getLength());
-
- // Send it to the switch
- sw.write(roleRequest, null);
-
- return xid;
- }
- /**
- * Send a role request message to the switch. This checks the capabilities
- * of the switch for understanding role request messaging. Currently we only
- * support the OVS-style role request message, but once the controller
- * supports OF 1.2, this function will also handle sending out the
- * OF 1.2-style role request message.
- * @param sw the switch to send the role request to
- * @param role the role to request
- * @throws IOException
- */
- protected void sendRoleRequest(IOFSwitch sw, Role role) throws IOException {
- // There are three cases to consider:
- //
- // 1) If the controller role at the point the switch connected was
- // null/disabled, then we never sent the role request probe to the
- // switch and therefore never set the SWITCH_SUPPORTS_NX_ROLE
- // attribute for the switch, so supportsNxRole is null. In that
- // case since we're now enabling role support for the controller
- // we should send out the role request probe/update to the switch.
- //
- // 2) If supportsNxRole == Boolean.TRUE then that means we've already
- // sent the role request probe to the switch and it replied with
- // a role reply message, so we know it supports role request
- // messages. Now we're changing the role and we want to send
- // it another role request message to inform it of the new role
- // for the controller.
- //
- // 3) If supportsNxRole == Boolean.FALSE, then that means we sent the
- // role request probe to the switch but it responded with an error
- // indicating that it didn't understand the role request message.
- // In that case we don't want to send it another role request that
- // it (still) doesn't understand. But if the new role of the
- // controller is SLAVE, then we don't want the switch to remain
- // connected to this controller. It might support the older serial
- // failover model for HA support, so we want to terminate the
- // connection and get it to initiate a connection with another
- // controller in its list of controllers. Eventually (hopefully, if
- // things are configured correctly) it will walk down its list of
- // controllers and connect to the current master controller.
- Boolean supportsNxRole = (Boolean)
- sw.getAttribute(IOFSwitch.SWITCH_SUPPORTS_NX_ROLE);
- if ((supportsNxRole == null) || supportsNxRole) {
- // Handle cases #1 and #2
- sendNxRoleRequest(sw, role);
- } else {
- // Handle case #3
- if (getRole() == Role.SLAVE) {
- log.error("Disconnecting switch {} that doesn't support " +
- "role request messages from a controller that went to SLAVE mode");
- // Closing the channel should result in a call to
- // channelDisconnect which updates all state
- sw.getChannel().close();
- }
- }
- }
// **********************
// ChannelUpstreamHandler
@@ -514,9 +421,15 @@ public class Controller implements IFloodlightProviderService,
public void channelDisconnected(ChannelHandlerContext ctx,
ChannelStateEvent e) throws Exception {
if (sw != null && state.hsState == HandshakeState.READY) {
- if (sw.getRole() != Role.SLAVE)
+ if (activeSwitches.containsKey(sw.getId())) {
+ // It's safe to call removeSwitch even though the map might
+ // not contain this particular switch but another with the
+ // same DPID
removeSwitch(sw);
- connectedSwitches.remove(sw.getId());
+ }
+ synchronized(roleChanger) {
+ connectedSwitches.remove(sw);
+ }
sw.setConnected(false);
}
log.info("Disconnected switch {}", sw);
@@ -701,88 +614,64 @@ public class Controller implements IFloodlightProviderService,
sw.setAttribute(IOFSwitch.SWITCH_DESCRIPTION_FUTURE,
dfuture);
- // We need to keep track of all of the switches that are connected
- // to the controller, in any role, so that we can later send the
- // role request messages when the controller role changes.
- connectedSwitches.put(sw.getId(), sw);
-
- // Send a role request if role support is enabled for the controller
- // This is a probe that we'll use to determine if the switch
- // actually supports the role request message. If it does we'll
- // get back a role reply message. If it doesn't we'll get back an
- // OFError message.
- if (role != null) {
- log.debug("This controllers role is:{}, sending role req msg",
- role);
- state.nxRoleRequestXid = sendNxRoleRequest(sw, role);
- } else {
- // if role support isn't enabled for the controller, then we're
- // not sending the role request probe to the switch
- log.info("This controllers role is null - not sending role-" +
- "request-msg");
- // The hasNxRole field is just a flag that's checked before
- // advancing the handshake state to READY. In this case,
- // we set the flag, so we don't need to wait for a
- // reply/error before transitioning to the READY state.
- state.hasNxRoleReply = true;
- }
}
protected void checkSwitchReady() {
- if (state.hasDescription && state.hasGetConfigReply &&
- state.hasNxRoleReply) {
+ if (state.hsState == HandshakeState.FEATURES_REPLY &&
+ state.hasDescription && state.hasGetConfigReply) {
state.hsState = HandshakeState.READY;
- if (getRole() == Role.SLAVE && sw.getRole() == null) {
- // When the controller is in the slave role and
- // the switch doesn't understand the role request message -
- // we disconnect the switch! The expected behavior is that
- // the switch will probably try to reconnect repeatedly
- // (with some sort of exponential backoff), but after a
- // while will give-up and move on to the next controller-IP
- // configured on the switch. This is the serial failover
- // mechanism from OpenFlow spec v1.0.
- log.error("Disconnecting switch from SLAVE controller." +
- " Switch {} doesn't support role request messages",
- sw.getId());
- sw.setConnected(false);
- connectedSwitches.remove(sw.getId());
- sw.getChannel().close();
- } else {
- log.info("Switch handshake successful: {}", sw);
+ synchronized(roleChanger) {
+ // We need to keep track of all of the switches that are connected
+ // to the controller, in any role, so that we can later send the
+ // role request messages when the controller role changes.
+ // We need to be synchronized while doing this: we must not
+ // send a another role request to the connectedSwitches until
+ // we were able to add this new switch to connectedSwitches
+ // *and* send the current role to the new switch.
+ connectedSwitches.add(sw);
- if (sw.getRole() != Role.SLAVE) {
- // Only add the switch to the active switch list if
- // we're not in the slave role. Note that if the role
- // attribute is null, then that means that the switch
- // doesn't support the role request messages, so in that
- // case we're effectively in the EQUAL role and the
- // switch should be included in the active switch list.
+ if (role != null) {
+ // Send a role request if role support is enabled for the controller
+ // This is a probe that we'll use to determine if the switch
+ // actually supports the role request message. If it does we'll
+ // get back a role reply message. If it doesn't we'll get back an
+ // OFError message.
+ // If role is MASTER we will promote switch to active
+ // list when we receive the switch's role reply messages
+ log.debug("This controller's role is {}, " +
+ "sending initial role request msg to {}",
+ role, sw);
+ Collection<OFSwitchImpl> swList = new ArrayList<OFSwitchImpl>(1);
+ swList.add(sw);
+ roleChanger.submitRequest(swList, role);
+ }
+ else {
+ // Role supported not enabled on controller (for now)
+ // automatically promote switch to active state.
+ log.debug("This controller's role is null, " +
+ "not sending role request msg to {}",
+ role, sw);
addSwitch(sw);
-
- // Delete all pre-existing flows for new connections to
- // the master
- // FIXME: Need to think more about what the test should
- // be for when we flush the flow-table? For example,
- // if all the controllers are temporarily in the backup
- // role (e.g. right after a failure of the master
- // controller) at the point the switch connects, then
- // all of the controllers will initially connect as
- // backup controllers and not flush the flow-table.
- // Then when one of them is promoted to master following
- // the master controller election the flow-table
- // will still not be flushed because that's treated as
- // a failover event where we don't want to flush the
- // flow-table. The end result would be that the flow
- // table for a newly connected switch is never
- // flushed. Not sure how to handle that case though...
sw.clearAllFlowMods();
+ state.firstRoleReplyReceived = true;
}
}
}
}
-
+
+ /* Handle a role reply message we received from the switch. Since
+ * netty serializes message dispatch we don't need to synchronize
+ * against other receive operations from the same switch, so no need
+ * to synchronize addSwitch(), removeSwitch() operations from the same
+ * connection.
+ * FIXME: However, when a switch with the same DPID connects we do
+ * need some synchronization. However, handling switches with same
+ * DPID needs to be revisited anyways (get rid of r/w-lock and synchronous
+ * removedSwitch notification):1
+ *
+ */
protected void handleRoleReplyMessage(OFVendor vendorMessage,
OFRoleReplyVendorData roleReplyVendorData) {
// Map from the role code in the message to our role enum
@@ -800,38 +689,68 @@ public class Controller implements IFloodlightProviderService,
break;
default:
log.error("Invalid role value in role reply message");
- break;
+ sw.getChannel().close();
+ return;
}
- log.info("Received NX role reply message; setting role of " +
- "controller to {}", role.name());
+ log.debug("Handling role reply for role {} from {}. " +
+ "Controller's role is {} ",
+ new Object[] { role, sw, Controller.this.role}
+ );
- sw.setRole(role);
+ sw.deliverRoleReply(vendorMessage.getXid(), role);
- if (state.hsState == HandshakeState.FEATURES_REPLY) {
- sw.setAttribute(IOFSwitch.SWITCH_SUPPORTS_NX_ROLE, true);
- state.hasNxRoleReply = true;
- checkSwitchReady();
- } else if (state.hsState == HandshakeState.READY) {
- // FIXME: Need to think more about possible synchronization
- // issues here.
- boolean isActive = activeSwitches.containsKey(sw.getId());
- String roleName = (role != null) ? role.name() : "none";
- log.debug("Handling role reply; switch is {}; role = {}",
- new Object[] {isActive ? "active" : "inactive",
- roleName});
- if (role == Role.SLAVE) {
- if (isActive) {
- removeSwitch(sw);
- log.debug("Removed slave switch {} from active switch" +
- " list", HexString.toHexString(sw.getId()));
- }
- } else if (!isActive) {
- addSwitch(sw);
- log.debug("Added master switch {} to active switch list",
+ boolean isActive = activeSwitches.containsKey(sw.getId());
+ if (!isActive && sw.isActive()) {
+ // Transition from SLAVE to MASTER.
+
+ // Only add the switch to the active switch list if
+ // we're not in the slave role. Note that if the role
+ // attribute is null, then that means that the switch
+ // doesn't support the role request messages, so in that
+ // case we're effectively in the EQUAL role and the
+ // switch should be included in the active switch list.
+ addSwitch(sw);
+ log.debug("Added master switch {} to active switch list",
+ HexString.toHexString(sw.getId()));
+
+ if (!state.firstRoleReplyReceived) {
+ // This is the first role-reply message we receive from
+ // this switch or roles were disabled when the switch
+ // connected:
+ // Delete all pre-existing flows for new connections to
+ // the master
+ //
+ // FIXME: Need to think more about what the test should
+ // be for when we flush the flow-table? For example,
+ // if all the controllers are temporarily in the backup
+ // role (e.g. right after a failure of the master
+ // controller) at the point the switch connects, then
+ // all of the controllers will initially connect as
+ // backup controllers and not flush the flow-table.
+ // Then when one of them is promoted to master following
+ // the master controller election the flow-table
+ // will still not be flushed because that's treated as
+ // a failover event where we don't want to flush the
+ // flow-table. The end result would be that the flow
+ // table for a newly connected switch is never
+ // flushed. Not sure how to handle that case though...
+ sw.clearAllFlowMods();
+ log.debug("First role reply from master switch {}, " +
+ "clear FlowTable to active switch list",
HexString.toHexString(sw.getId()));
}
+ }
+ else if (isActive && !sw.isActive()) {
+ // Transition from MASTER to SLAVE: remove switch
+ // from active switch list.
+ log.debug("Removed slave switch {} from active switch" +
+ " list", HexString.toHexString(sw.getId()));
+ removeSwitch(sw);
}
+
+ // Indicate that we have received a role reply message.
+ state.firstRoleReplyReceived = true;
}
protected boolean handleVendorMessage(OFVendor vendorMessage) {
@@ -928,24 +847,57 @@ public class Controller implements IFloodlightProviderService,
shouldHandleMessage = handleVendorMessage((OFVendor)m);
break;
case ERROR:
+ // TODO: we need better error handling. Especially for
+ // request/reply style message (stats, roles) we should have
+ // a unified way to lookup the xid in the error message.
+ // This will probable involve rewriting the way we handle
+ // request/reply style messages.
OFError error = (OFError) m;
boolean shouldLogError = true;
- if (state.hsState == HandshakeState.FEATURES_REPLY) {
- if (error.getXid() == state.nxRoleRequestXid) {
- boolean isBadVendorError =
- (error.getErrorType() == OFError.OFErrorType.
- OFPET_BAD_REQUEST.getValue()) &&
- (error.getErrorCode() == OFError.
- OFBadRequestCode.
- OFPBRC_BAD_VENDOR.ordinal());
- // We expect to receive a bad vendor error when
- // we're connected to a switch that doesn't support
- // the Nicira vendor extensions (i.e. not OVS or
- // derived from OVS). So that's not a real error
- // case and we don't want to log those spurious errors.
- shouldLogError = !isBadVendorError;
-
- // FIXME: Is this the right thing to do if we receive
+ // TODO: should we check that firstRoleReplyReceived is false,
+ // i.e., check only whether the first request fails?
+ if (sw.checkFirstPendingRoleRequestXid(error.getXid())) {
+ boolean isBadVendorError =
+ (error.getErrorType() == OFError.OFErrorType.
+ OFPET_BAD_REQUEST.getValue()) &&
+ (error.getErrorCode() == OFError.
+ OFBadRequestCode.
+ OFPBRC_BAD_VENDOR.ordinal());
+ // We expect to receive a bad vendor error when
+ // we're connected to a switch that doesn't support
+ // the Nicira vendor extensions (i.e. not OVS or
+ // derived from OVS). So that's not a real error
+ // case and we don't want to log those spurious errors.
+ shouldLogError = !isBadVendorError;
+ if (isBadVendorError) {
+ if (!state.firstRoleReplyReceived) {
+ log.warn("Received ERROR from sw {} that "
+ +"indicates roles are not supported "
+ +"but we have received a valid "
+ +"role reply earlier", sw);
+ }
+ state.firstRoleReplyReceived = true;
+ sw.deliverRoleRequestNotSupported(error.getXid());
+ synchronized(roleChanger) {
+ if (sw.role == null && Controller.this.role==Role.SLAVE) {
+ // the switch doesn't understand role request
+ // messages and the current controller role is
+ // slave. We need to disconnect the switch.
+ // @see RoleChanger for rationale
+ sw.getChannel().close();
+ }
+ else if (sw.role == null) {
+ // Controller's role is master: add to
+ // active
+ addSwitch(sw);
+ // TODO: check if clearing flow table is
+ // right choice here.
+ sw.clearAllFlowMods();
+ }
+ }
+ }
+ else {
+ // TODO: Is this the right thing to do if we receive
// some other error besides a bad vendor error?
// Presumably that means the switch did actually
// understand the role request message, but there
@@ -955,15 +907,20 @@ public class Controller implements IFloodlightProviderService,
// role request has that. Should check OVS source
// code to see if it's possible for any other errors
// to be returned.
- sw.setAttribute(IOFSwitch.SWITCH_SUPPORTS_NX_ROLE,
- !isBadVendorError);
- state.hasNxRoleReply = true;
- checkSwitchReady();
+ // If we received an error the switch is not
+ // in the correct role, so we need to disconnect it.
+ // We could also resend the request but then we need to
+ // check if there are other pending request in which
+ // case we shouldn't resend. If we do resend we need
+ // to make sure that the switch eventually accepts one
+ // of our requests or disconnect the switch. This feels
+ // cumbersome.
+ sw.getChannel().close();
}
- // Once we support OF 1.2, we'd add code to handle it here.
- //if (error.getXid() == state.ofRoleRequestXid) {
- //}
}
+ // Once we support OF 1.2, we'd add code to handle it here.
+ //if (error.getXid() == state.ofRoleRequestXid) {
+ //}
if (shouldLogError)
logError(sw, error);
break;
@@ -1006,7 +963,7 @@ public class Controller implements IFloodlightProviderService,
// Check if the controller is in the slave role for the
// switch. If it is, then don't dispatch the message to
// the listeners.
- // FIXME: Should we dispatch messages that we expect to
+ // TODO: Should we dispatch messages that we expect to
// receive when we're in the slave role, e.g. port
// status messages? Since we're "hiding" switches from
// the listeners when we're in the slave role, then it
@@ -1232,80 +1189,6 @@ public class Controller implements IFloodlightProviderService,
}
}
- /**
- * Add a switch that has completed the initial handshake with the
- * controller to the list of connected switches.
- * @param sw the switch to connect
- */
- /*
- protected void connectSwitch(OFSwitchImpl sw) {
- // FIXME: robv: I don't think this code is completely thread-safe. I don't
- // think it will work correctly if multiple switches with the same DPID
- // connect at the same time. I think to handle that we'd need to have
- // a controller-wide lock on the switch lists instead of trying to have
- // per-switch locks. Having a single lock would reduce concurrency but
- OFSwitchImpl oldSw = (OFSwitchImpl) this.connectedSwitches.get(sw.getId());
- if (sw == oldSw) {
- // Note == for object equality, not .equals for value
- log.info("New switch connection for switch {} that's already connected", sw);
- return;
- }
-
- log.info("Switch handshake successful: {}", sw);
-
- if (oldSw != null) {
- if (oldSw.isActive())
- deactivateSwitch(oldSw);
- disconnectSwitch(oldSw);
- // Close the channel to disconnect the controller from the switch.
- // This will eventually trigger a removeSwitch(), which will cause
- // a "Not removing Switch ... already removed debug message.
- // FIXME: Should be some better way to handle this to avoid spurious
- // debug message.
- oldSw.getChannel().close();
- }
-
- sw.getWriteLock().lock();
- try {
- connectedSwitches.put(sw.getId(), sw);
- sw.setConnected(true);
- }
- finally {
- sw.getWriteLock().unlock();
- }
- }
- */
- /**
- * Remove a switch from the list of connected switches
- * @param sw the switch to disconnect
- */
- /*
- protected void disconnectSwitch(IOFSwitch sw) {
- oldSw.getListenerWriteLock().lock();
- try {
- log.error("New switch connection {} for already-connected switch {}",
- sw, oldSw);
- oldSw.setConnected(false);
- if (oldSw)
- updateInactiveSwitchInfo(oldSw);
-
- // we need to clean out old switch state definitively
- // before adding the new switch
- if (switchListeners != null) {
- for (IOFSwitchListener listener : switchListeners) {
- listener.removedSwitch(oldSw);
- }
- }
- // will eventually trigger a removeSwitch(), which will cause
- // a "Not removing Switch ... already removed debug message.
- oldSw.getChannel().close();
- }
- finally {
- oldSw.getWriteLock().unlock();
- }
- }
- */
-
/**
* Add a switch to the active switch list and call the switch listeners.
* This happens either when a switch first connects (and the controller is
@@ -1313,6 +1196,8 @@ public class Controller implements IFloodlightProviderService,
* slave to master.
* @param sw the switch that has been added
*/
+ // TODO: need to rethink locking and the synchronous switch update.
+ // We can / should also handle duplicate DPIDs in connectedSwitches
protected void addSwitch(IOFSwitch sw) {
// TODO: is it safe to modify the HashMap without holding
// the old switch's lock?
@@ -1875,13 +1760,14 @@ public class Controller implements IFloodlightProviderService,
this.switchListeners = new CopyOnWriteArraySet<IOFSwitchListener>();
this.haListeners = new CopyOnWriteArraySet<IHAListener>();
this.activeSwitches = new ConcurrentHashMap<Long, IOFSwitch>();
- this.connectedSwitches = new ConcurrentHashMap<Long, IOFSwitch>();
+ this.connectedSwitches = new HashSet<OFSwitchImpl>();
this.controllerNodeIPsCache = new HashMap<String, String>();
this.updates = new LinkedBlockingQueue<IUpdate>();
this.factory = new BasicFactory();
this.providerMap = new HashMap<String, List<IInfoProvider>>();
setConfigParams(configParams);
this.role = getInitialRole(configParams);
+ this.roleChanger = new RoleChanger();
initVendorMessages();
}
diff --git a/src/main/java/net/floodlightcontroller/core/internal/OFChannelState.java b/src/main/java/net/floodlightcontroller/core/internal/OFChannelState.java
index ac6765b6cfa7c0ec21190c9106b98c7a94999041..ad5a3772fa6e47ae6f1356d93455d597b1a36530 100644
--- a/src/main/java/net/floodlightcontroller/core/internal/OFChannelState.java
+++ b/src/main/java/net/floodlightcontroller/core/internal/OFChannelState.java
@@ -39,6 +39,7 @@ class OFChannelState {
/**
* We've received the features reply
+ * Waiting for Config and Description reply
*/
FEATURES_REPLY,
@@ -53,11 +54,11 @@ class OFChannelState {
protected boolean hasGetConfigReply = false;
protected boolean hasDescription = false;
- // The hasNxRoleReply flag doesn't mean that the switch supports the NX
- // role messages, just that we've received an answer back from the
- // switch (possibly a bad vendor error) in response to our initial
- // role request. It's used as a flag to indicate that we've met one
- // of the conditions necessary to advance the handshake state to READY.
- protected boolean hasNxRoleReply = false;
- protected int nxRoleRequestXid;
+ // The firstRoleReplyRecevied flag indicates if we have received the
+ // first role reply message on this connection (in response to the
+ // role request sent after the handshake). If role support is disabled
+ // on the controller we also set this flag to true.
+ // The flag is used to decide if the flow table should be wiped
+ // @see Controller.handleRoleReplyMessage()
+ protected boolean firstRoleReplyReceived = false;
}
\ No newline at end of file
diff --git a/src/main/java/net/floodlightcontroller/core/internal/OFSwitchImpl.java b/src/main/java/net/floodlightcontroller/core/internal/OFSwitchImpl.java
index e23b2488191a37f95b29e314d7e4a569f7df86ab..a48fddfba30a76901e7f390360764e16e3504968 100644
--- a/src/main/java/net/floodlightcontroller/core/internal/OFSwitchImpl.java
+++ b/src/main/java/net/floodlightcontroller/core/internal/OFSwitchImpl.java
@@ -21,6 +21,7 @@ import java.io.IOException;
import java.util.ArrayList;
import java.util.Date;
import java.util.HashMap;
+import java.util.LinkedList;
import java.util.List;
import java.util.Map;
import java.util.concurrent.ConcurrentHashMap;
@@ -47,6 +48,7 @@ import org.openflow.protocol.OFMessage;
import org.openflow.protocol.OFPhysicalPort;
import org.openflow.protocol.OFPort;
import org.openflow.protocol.OFType;
+import org.openflow.protocol.OFVendor;
import org.openflow.protocol.OFPhysicalPort.OFPortConfig;
import org.openflow.protocol.OFPhysicalPort.OFPortState;
import org.openflow.protocol.OFStatisticsRequest;
@@ -54,6 +56,9 @@ import org.openflow.protocol.statistics.OFDescriptionStatistics;
import org.openflow.protocol.statistics.OFStatistics;
import org.openflow.util.HexString;
import org.openflow.util.U16;
+import org.openflow.vendor.nicira.OFNiciraVendorData;
+import org.openflow.vendor.nicira.OFRoleRequestVendorData;
+import org.openflow.vendor.nicira.OFRoleVendorData;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
@@ -62,6 +67,8 @@ import org.slf4j.LoggerFactory;
* @author David Erickson (daviderickson@cs.stanford.edu)
*/
public class OFSwitchImpl implements IOFSwitch {
+ // TODO: should we really do logging in the class or should we throw
+ // exception that can then be handled by callers?
protected static Logger log = LoggerFactory.getLogger(OFSwitchImpl.class);
protected ConcurrentMap<Object, Object> attributes;
@@ -82,6 +89,17 @@ public class OFSwitchImpl implements IOFSwitch {
protected TimedCache<Long> timedCache;
protected ReentrantReadWriteLock listenerLock;
protected ConcurrentMap<Short, Long> portBroadcastCacheHitMap;
+ /**
+ * When sending a role request message, the role request is added
+ * to this queue. If a role reply is received this queue is checked to
+ * verify that the reply matches the expected reply. We require in order
+ * delivery of replies. That's why we use a Queue.
+ * The RoleChanger uses a timeout to ensure we receive a timely reply.
+ *
+ * Need to synchronize on this instance if a request is sent, received,
+ * checked.
+ */
+ protected LinkedList<PendingRoleRequestEntry> pendingRoleRequests;
public static IOFSwitchFeatures switchFeatures;
protected static final ThreadLocal<Map<OFSwitchImpl,List<OFMessage>>> local_msg_buffer =
@@ -95,6 +113,18 @@ public class OFSwitchImpl implements IOFSwitch {
// for managing our map sizes
protected static final int MAX_MACS_PER_SWITCH = 1000;
+ protected static class PendingRoleRequestEntry {
+ protected int xid;
+ protected Role role;
+ // cookie is used to identify the role "generation". roleChanger uses
+ protected long cookie;
+ public PendingRoleRequestEntry(int xid, Role role, long cookie) {
+ this.xid = xid;
+ this.role = role;
+ this.cookie = cookie;
+ }
+ }
+
public OFSwitchImpl() {
this.attributes = new ConcurrentHashMap<Object, Object>();
this.connectedSince = new Date();
@@ -108,6 +138,7 @@ public class OFSwitchImpl implements IOFSwitch {
this.timedCache = new TimedCache<Long>(100, 5*1000 ); // 5 seconds interval
this.listenerLock = new ReentrantReadWriteLock();
this.portBroadcastCacheHitMap = new ConcurrentHashMap<Short, Long>();
+ this.pendingRoleRequests = new LinkedList<OFSwitchImpl.PendingRoleRequestEntry>();
// Defaults properties for an ideal switch
this.setAttribute(PROP_FASTWILDCARDS, (Integer) OFMatch.OFPFW_ALL);
@@ -148,6 +179,7 @@ public class OFSwitchImpl implements IOFSwitch {
this.channel = channel;
}
+ // TODO: document the difference between the different write functions
public void write(OFMessage m, FloodlightContext bc) throws IOException {
Map<OFSwitchImpl,List<OFMessage>> msg_buffer_map = local_msg_buffer.get();
List<OFMessage> msg_buffer = msg_buffer_map.get(this);
@@ -373,17 +405,12 @@ public class OFSwitchImpl implements IOFSwitch {
}
@Override
- public synchronized Role getRole() {
+ public Role getRole() {
return role;
}
@Override
- public synchronized void setRole(Role role) {
- this.role = role;
- }
-
- @Override
- public synchronized boolean isActive() {
+ public boolean isActive() {
return (role != Role.SLAVE);
}
@@ -473,4 +500,166 @@ public class OFSwitchImpl implements IOFSwitch {
public Lock getListenerWriteLock() {
return listenerLock.writeLock();
}
+
+ /**
+ * Send NX role request message to the switch requesting the specified role.
+ *
+ * This method should ONLY be called by @see RoleChanger.submitRequest().
+ *
+ * After sending the request add it to the queue of pending request. We
+ * use the queue to later verify that we indeed receive the correct reply.
+ * @param sw switch to send the role request message to
+ * @param role role to request
+ * @param cookie an opaque value that will be stored in the pending queue so
+ * RoleChanger can check for timeouts.
+ * @return transaction id of the role request message that was sent
+ */
+ protected int sendNxRoleRequest(Role role, long cookie)
+ throws IOException {
+ synchronized(pendingRoleRequests) {
+ // Convert the role enum to the appropriate integer constant used
+ // in the NX role request message
+ int nxRole = 0;
+ switch (role) {
+ case EQUAL:
+ nxRole = OFRoleVendorData.NX_ROLE_OTHER;
+ break;
+ case MASTER:
+ nxRole = OFRoleVendorData.NX_ROLE_MASTER;
+ break;
+ case SLAVE:
+ nxRole = OFRoleVendorData.NX_ROLE_SLAVE;
+ break;
+ default:
+ log.error("Invalid Role specified for switch {}."
+ + " Disconnecting.", this);
+ // TODO: should throw an error
+ return 0;
+ }
+
+ // Construct the role request message
+ OFVendor roleRequest = (OFVendor)floodlightProvider.
+ getOFMessageFactory().getMessage(OFType.VENDOR);
+ int xid = this.getNextTransactionId();
+ roleRequest.setXid(xid);
+ roleRequest.setVendor(OFNiciraVendorData.NX_VENDOR_ID);
+ OFRoleRequestVendorData roleRequestData = new OFRoleRequestVendorData();
+ roleRequestData.setRole(nxRole);
+ roleRequest.setVendorData(roleRequestData);
+ roleRequest.setLengthU(OFVendor.MINIMUM_LENGTH +
+ roleRequestData.getLength());
+
+ // Send it to the switch
+ List<OFMessage> msglist = new ArrayList<OFMessage>(1);
+ msglist.add(roleRequest);
+ // FIXME: should this use this.write() in order for messages to
+ // be processed by handleOutgoingMessage()
+ this.channel.write(msglist);
+
+ pendingRoleRequests.add(new PendingRoleRequestEntry(xid, role, cookie));
+ return xid;
+ }
+ }
+
+ /**
+ * Deliver a RoleReply message to this switch. Checks if the reply
+ * message matches the expected reply (head of the pending request queue).
+ * We require in-order delivery of replies. If there's any deviation from
+ * our expectations we disconnect the switch.
+ *
+ * We must not check the received role against the controller's current
+ * role because there's no synchronization but that's fine @see RoleChanger
+ *
+ * Will be called by the OFChannelHandler's receive loop
+ *
+ * @param xid Xid of the reply message
+ * @param role The Role in the the reply message
+ */
+ protected void deliverRoleReply(int xid, Role role) {
+ synchronized(pendingRoleRequests) {
+ PendingRoleRequestEntry head = pendingRoleRequests.poll();
+ if (head == null) {
+ // Maybe don't disconnect if the role reply we received is
+ // for the same role we are already in.
+ log.error("Switch {}: received unexpected role reply for Role {}" +
+ " Disconnecting switch", this, role );
+ this.channel.close();
+ }
+ else if (head.xid != xid) {
+ // check xid before role!!
+ log.error("Switch {}: expected role reply with " +
+ "Xid {}, got {}. Disconnecting switch",
+ new Object[] { this, head.xid, xid } );
+ this.channel.close();
+ }
+ else if (head.role != role) {
+ log.error("Switch {}: expected role reply with " +
+ "Role {}, got {}. Disconnecting switch",
+ new Object[] { this, head.role, role } );
+ this.channel.close();
+ }
+ else {
+ log.debug("Received role reply message from {}, setting role to {}",
+ this, role);
+ if (this.role == null && getAttribute(SWITCH_SUPPORTS_NX_ROLE) == null) {
+ // The first role reply we received. Set the attribute
+ // that the switch supports roles
+ setAttribute(SWITCH_SUPPORTS_NX_ROLE, true);
+ }
+ this.role = role;
+ }
+ }
+ }
+
+ /**
+ * Checks whether the given xid matches the xid of the first pending
+ * role request.
+ * @param xid
+ * @return
+ */
+ protected boolean checkFirstPendingRoleRequestXid (int xid) {
+ synchronized(pendingRoleRequests) {
+ PendingRoleRequestEntry head = pendingRoleRequests.peek();
+ if (head == null)
+ return false;
+ else
+ return head.xid == xid;
+ }
+ }
+
+ /**
+ * Checks whether the given request cookie matches the cookie of the first
+ * pending request
+ * @param cookie
+ * @return
+ */
+ protected boolean checkFirstPendingRoleRequestCookie(long cookie) {
+ synchronized(pendingRoleRequests) {
+ PendingRoleRequestEntry head = pendingRoleRequests.peek();
+ if (head == null)
+ return false;
+ else
+ return head.cookie == cookie;
+ }
+ }
+
+ /**
+ * Called if we receive a vendor error message indicating that roles
+ * are not supported by the switch. If the xid matches the first pending
+ * one, we'll mark the switch as not supporting roles and remove the head.
+ * Otherwise we ignore it.
+ * @param xid
+ */
+ protected void deliverRoleRequestNotSupported(int xid) {
+ synchronized(pendingRoleRequests) {
+ PendingRoleRequestEntry head = pendingRoleRequests.poll();
+ this.role = null;
+ if (head!=null && head.xid == xid) {
+ setAttribute(SWITCH_SUPPORTS_NX_ROLE, false);
+ }
+ else {
+ this.channel.close();
+ }
+ }
+ }
}
diff --git a/src/main/java/net/floodlightcontroller/core/internal/RoleChanger.java b/src/main/java/net/floodlightcontroller/core/internal/RoleChanger.java
index 5af9264be63f31bffb138e62c9ca17a706de0ac4..1a79dfce5e732cbe69688822c34fc21201c6a429 100644
--- a/src/main/java/net/floodlightcontroller/core/internal/RoleChanger.java
+++ b/src/main/java/net/floodlightcontroller/core/internal/RoleChanger.java
@@ -293,7 +293,7 @@ public class RoleChanger {
protected void verifyRoleReplyReceived(Collection<OFSwitchImpl> switches,
long cookie) {
for (OFSwitchImpl sw: switches) {
- if (sw.checkFirstPendingRoleReqeustCookie(cookie)) {
+ if (sw.checkFirstPendingRoleRequestCookie(cookie)) {
sw.getChannel().close();
log.warn("Timeout while waiting for role reply from switch {}."
+ " Disconnecting", sw);
diff --git a/src/test/java/net/floodlightcontroller/core/internal/ControllerTest.java b/src/test/java/net/floodlightcontroller/core/internal/ControllerTest.java
index 1b2b7e9a94b0dfb98e30c1b4a1c665999d3f47b2..93a2dcc1213ae433af586665c0a08a6f72e8c01e 100644
--- a/src/test/java/net/floodlightcontroller/core/internal/ControllerTest.java
+++ b/src/test/java/net/floodlightcontroller/core/internal/ControllerTest.java
@@ -19,9 +19,8 @@ package net.floodlightcontroller.core.internal;
import static org.easymock.EasyMock.*;
-import java.net.InetSocketAddress;
-import java.net.SocketAddress;
import java.util.ArrayList;
+import java.util.Collection;
import java.util.Date;
import java.util.HashMap;
import java.util.HashSet;
@@ -42,6 +41,7 @@ import net.floodlightcontroller.core.IOFMessageListener.Command;
import net.floodlightcontroller.core.IOFSwitch;
import net.floodlightcontroller.core.IOFSwitchListener;
import net.floodlightcontroller.core.OFMessageFilterManager;
+import net.floodlightcontroller.core.internal.OFChannelState.HandshakeState;
import net.floodlightcontroller.core.module.FloodlightModuleContext;
import net.floodlightcontroller.core.test.MockFloodlightProvider;
import net.floodlightcontroller.core.test.MockThreadPoolService;
@@ -60,27 +60,30 @@ import net.floodlightcontroller.storage.memory.MemoryStorageSource;
import net.floodlightcontroller.test.FloodlightTestCase;
import net.floodlightcontroller.threadpool.IThreadPoolService;
+import org.easymock.Capture;
import org.jboss.netty.channel.Channel;
-import org.jboss.netty.channel.ChannelStateEvent;
import org.junit.Test;
import org.openflow.protocol.OFError;
+import org.openflow.protocol.OFError.OFBadRequestCode;
+import org.openflow.protocol.OFError.OFErrorType;
import org.openflow.protocol.OFFeaturesReply;
-import org.openflow.protocol.OFMessage;
import org.openflow.protocol.OFPacketIn;
import org.openflow.protocol.OFPacketOut;
import org.openflow.protocol.OFPhysicalPort;
import org.openflow.protocol.OFPort;
import org.openflow.protocol.OFStatisticsReply;
import org.openflow.protocol.OFType;
-import org.openflow.protocol.OFError.OFBadRequestCode;
-import org.openflow.protocol.OFError.OFErrorType;
import org.openflow.protocol.OFPacketIn.OFPacketInReason;
+import org.openflow.protocol.OFVendor;
import org.openflow.protocol.action.OFAction;
import org.openflow.protocol.action.OFActionOutput;
import org.openflow.protocol.factory.BasicFactory;
import org.openflow.protocol.statistics.OFFlowStatisticsReply;
import org.openflow.protocol.statistics.OFStatistics;
import org.openflow.protocol.statistics.OFStatisticsType;
+import org.openflow.util.HexString;
+import org.openflow.vendor.nicira.OFNiciraVendorData;
+import org.openflow.vendor.nicira.OFRoleReplyVendorData;
/**
*
@@ -146,6 +149,25 @@ public class ControllerTest extends FloodlightTestCase {
return sr;
}
+ /* Set the mock expectations for sw when sw is passed to addSwitch */
+ protected void setupSwitchForAddSwitch(IOFSwitch sw, long dpid) {
+ String dpidString = HexString.toHexString(dpid);
+
+ OFFeaturesReply featuresReply = new OFFeaturesReply();
+ featuresReply.setDatapathId(dpid);
+ featuresReply.setPorts(new ArrayList<OFPhysicalPort>());
+
+ expect(sw.getId()).andReturn(dpid).anyTimes();
+ expect(sw.getStringId()).andReturn(dpidString).anyTimes();
+ expect(sw.getConnectedSince()).andReturn(new Date());
+ Channel channel = createMock(Channel.class);
+ expect(sw.getChannel()).andReturn(channel);
+ expect(channel.getRemoteAddress()).andReturn(null);
+
+ expect(sw.getFeaturesReply()).andReturn(featuresReply).anyTimes();
+ expect(sw.getPorts()).andReturn(new HashMap<Short,OFPhysicalPort>());
+ }
+
/**
* Run the controller's main loop so that updates are processed
*/
@@ -675,9 +697,172 @@ public class ControllerTest extends FloodlightTestCase {
expectedCurMap, controller.getControllerNodeIPs());
}
+ @Test
+ public void testSetRoleNull() {
+ try {
+ controller.setRole(null);
+ fail("Should have thrown an Exception");
+ }
+ catch (NullPointerException e) {
+ //exptected
+ }
+ }
+
+ @Test
+ public void testSetRole() {
+ controller.connectedSwitches.add(new OFSwitchImpl());
+ RoleChanger roleChanger = createMock(RoleChanger.class);
+ roleChanger.submitRequest(controller.connectedSwitches, Role.SLAVE);
+ controller.roleChanger = roleChanger;
+
+ assertEquals("Check that update queue is empty", 0,
+ controller.updates.size());
+
+ replay(roleChanger);
+ controller.setRole(Role.SLAVE);
+ verify(roleChanger);
+
+ Controller.IUpdate upd = controller.updates.poll();
+ assertNotNull("Check that update queue has an update", upd);
+ assertTrue("Check that update is HARoleUpdate",
+ upd instanceof Controller.HARoleUpdate);
+ Controller.HARoleUpdate roleUpd = (Controller.HARoleUpdate)upd;
+ assertSame(null, roleUpd.oldRole);
+ assertSame(Role.SLAVE, roleUpd.newRole);
+ }
+
+ @Test
+ public void testCheckSwitchReady() {
+ OFChannelState state = new OFChannelState();
+ Controller.OFChannelHandler chdlr = controller.new OFChannelHandler(state);
+ chdlr.sw = createMock(OFSwitchImpl.class);
+
+ // Wrong current state
+ // Should not go to READY
+ state.hsState = OFChannelState.HandshakeState.HELLO;
+ state.hasDescription = true;
+ state.hasGetConfigReply = true;
+ replay(chdlr.sw); // nothing called on sw
+ chdlr.checkSwitchReady();
+ verify(chdlr.sw);
+ assertSame(OFChannelState.HandshakeState.HELLO, state.hsState);
+ reset(chdlr.sw);
+
+ // Have only config reply
+ state.hsState = OFChannelState.HandshakeState.FEATURES_REPLY;
+ state.hasDescription = false;
+ state.hasGetConfigReply = true;
+ replay(chdlr.sw);
+ chdlr.checkSwitchReady();
+ verify(chdlr.sw);
+ assertSame(OFChannelState.HandshakeState.FEATURES_REPLY, state.hsState);
+ assertTrue(controller.connectedSwitches.isEmpty());
+ assertTrue(controller.activeSwitches.isEmpty());
+ reset(chdlr.sw);
+
+ // Have only desc reply
+ state.hsState = OFChannelState.HandshakeState.FEATURES_REPLY;
+ state.hasDescription = true;
+ state.hasGetConfigReply = false;
+ replay(chdlr.sw);
+ chdlr.checkSwitchReady();
+ verify(chdlr.sw);
+ assertSame(OFChannelState.HandshakeState.FEATURES_REPLY, state.hsState);
+ assertTrue(controller.connectedSwitches.isEmpty());
+ assertTrue(controller.activeSwitches.isEmpty());
+ reset(chdlr.sw);
+
+ //////////////////////////////////////////
+ // Finally, everything is right. Should advance to READY
+ //////////////////////////////////////////
+ controller.roleChanger = createMock(RoleChanger.class);
+ state.hsState = OFChannelState.HandshakeState.FEATURES_REPLY;
+ state.hasDescription = true;
+ state.hasGetConfigReply = true;
+ // Role support disabled. Switch should be promoted to active switch
+ // list.
+ setupSwitchForAddSwitch(chdlr.sw, 0L);
+ chdlr.sw.clearAllFlowMods();
+ replay(controller.roleChanger, chdlr.sw);
+ chdlr.checkSwitchReady();
+ verify(controller.roleChanger, chdlr.sw);
+ assertSame(OFChannelState.HandshakeState.READY, state.hsState);
+ assertSame(chdlr.sw, controller.activeSwitches.get(0L));
+ assertTrue(controller.connectedSwitches.contains(chdlr.sw));
+ assertTrue(state.firstRoleReplyReceived);
+ reset(chdlr.sw);
+ reset(controller.roleChanger);
+ controller.connectedSwitches.clear();
+ controller.activeSwitches.clear();
+
+
+ // Role support enabled.
+ state.hsState = OFChannelState.HandshakeState.FEATURES_REPLY;
+ controller.role = Role.MASTER;
+ Capture<Collection<OFSwitchImpl>> swListCapture =
+ new Capture<Collection<OFSwitchImpl>>();
+ controller.roleChanger.submitRequest(capture(swListCapture),
+ same(Role.MASTER));
+ replay(controller.roleChanger, chdlr.sw);
+ chdlr.checkSwitchReady();
+ verify(controller.roleChanger, chdlr.sw);
+ assertSame(OFChannelState.HandshakeState.READY, state.hsState);
+ assertTrue(controller.activeSwitches.isEmpty());
+ assertTrue(controller.connectedSwitches.contains(chdlr.sw));
+ assertTrue(state.firstRoleReplyReceived);
+ Collection<OFSwitchImpl> swList = swListCapture.getValue();
+ assertEquals(1, swList.size());
+ assertTrue("swList must contain this switch", swList.contains(chdlr.sw));
+ }
+
+
+ @Test
+ public void testChannelDisconnected() throws Exception {
+ OFChannelState state = new OFChannelState();
+ state.hsState = OFChannelState.HandshakeState.READY;
+ Controller.OFChannelHandler chdlr = controller.new OFChannelHandler(state);
+ chdlr.sw = createMock(OFSwitchImpl.class);
+
+ // Switch is active
+ expect(chdlr.sw.getId()).andReturn(0L).anyTimes();
+ expect(chdlr.sw.getStringId()).andReturn("00:00:00:00:00:00:00:00")
+ .anyTimes();
+ chdlr.sw.cancelAllStatisticsReplies();
+ chdlr.sw.setConnected(false);
+ expect(chdlr.sw.isConnected()).andReturn(true);
+
+ controller.connectedSwitches.add(chdlr.sw);
+ controller.activeSwitches.put(0L, chdlr.sw);
+
+ replay(chdlr.sw);
+ chdlr.channelDisconnected(null, null);
+ verify(chdlr.sw);
+
+ // Switch is connected but not active
+ reset(chdlr.sw);
+ expect(chdlr.sw.getId()).andReturn(0L).anyTimes();
+ chdlr.sw.setConnected(false);
+ replay(chdlr.sw);
+ chdlr.channelDisconnected(null, null);
+ verify(chdlr.sw);
+
+ // Not in ready state
+ state.hsState = HandshakeState.START;
+ reset(chdlr.sw);
+ replay(chdlr.sw);
+ chdlr.channelDisconnected(null, null);
+ verify(chdlr.sw);
+
+ // Switch is null
+ state.hsState = HandshakeState.READY;
+ chdlr.sw = null;
+ chdlr.channelDisconnected(null, null);
+ }
+
+ /*
@Test
public void testRoleChangeForSerialFailoverSwitch() throws Exception {
- IOFSwitch newsw = createMock(IOFSwitch.class);
+ OFSwitchImpl newsw = createMock(OFSwitchImpl.class);
expect(newsw.getId()).andReturn(0L).anyTimes();
expect(newsw.getStringId()).andReturn("00:00:00:00:00:00:00").anyTimes();
Channel channel2 = createMock(Channel.class);
@@ -688,7 +873,7 @@ public class ControllerTest extends FloodlightTestCase {
expect(newsw.getAttribute(IOFSwitch.SWITCH_SUPPORTS_NX_ROLE))
.andReturn(false);
// switch is connected
- controller.connectedSwitches.put(0L, newsw);
+ controller.connectedSwitches.add(newsw);
// the switch should get disconnected when role is changed to SLAVE
expect(channel2.close()).andReturn(null);
@@ -697,42 +882,273 @@ public class ControllerTest extends FloodlightTestCase {
controller.setRole(Role.SLAVE);
verify(newsw, channel2);
}
+ */
@Test
- public void testSlaveRoleHandshakeForSerialFailoverSwitch()
- throws Exception {
- controller.role = Role.SLAVE;
- OFFeaturesReply featuresReply = new OFFeaturesReply();
- featuresReply.setDatapathId(0L);
- featuresReply.setPorts(new ArrayList<OFPhysicalPort>());
+ public void testRoleNotSupportedError() throws Exception {
+ int xid = 424242;
OFChannelState state = new OFChannelState();
- state.hsState = OFChannelState.HandshakeState.FEATURES_REPLY;
- state.hasDescription = true;
- state.hasGetConfigReply = true;
+ state.hsState = HandshakeState.READY;
Controller.OFChannelHandler chdlr = controller.new OFChannelHandler(state);
-
+ chdlr.sw = createMock(OFSwitchImpl.class);
Channel ch = createMock(Channel.class);
- ChannelStateEvent e = createMock(ChannelStateEvent.class);
- expect(e.getChannel()).andReturn(ch).anyTimes();
- SocketAddress sa = new InetSocketAddress(45454);
- expect(ch.getRemoteAddress()).andReturn(sa);
- expect(ch.write(anyObject())).andReturn(null);
-
+
// the error returned when role request message is not supported by sw
- OFMessage msg = new OFError();
+ OFError msg = new OFError();
msg.setType(OFType.ERROR);
- ((OFError) msg).setErrorType(OFErrorType.OFPET_BAD_REQUEST);
- ((OFError) msg).setErrorCode(OFBadRequestCode.OFPBRC_BAD_VENDOR);
+ msg.setXid(xid);
+ msg.setErrorType(OFErrorType.OFPET_BAD_REQUEST);
+ msg.setErrorCode(OFBadRequestCode.OFPBRC_BAD_VENDOR);
// the switch connection should get disconnected when the controller is
// in SLAVE mode and the switch does not support role-request messages
+ state.firstRoleReplyReceived = false;
+ controller.role = Role.SLAVE;
+ expect(chdlr.sw.checkFirstPendingRoleRequestXid(xid)).andReturn(true);
+ chdlr.sw.deliverRoleRequestNotSupported(xid);
+ expect(chdlr.sw.getChannel()).andReturn(ch).anyTimes();
expect(ch.close()).andReturn(null);
- replay(ch, e);
- chdlr.channelConnected(null, e);
- chdlr.sw.setFeaturesReply(featuresReply);
+ replay(ch, chdlr.sw);
chdlr.processOFMessage(msg);
- verify(ch, e);
+ verify(ch, chdlr.sw);
+ assertTrue("state.firstRoleReplyReceived must be true",
+ state.firstRoleReplyReceived);
+ assertTrue("activeSwitches must be empty",
+ controller.activeSwitches.isEmpty());
+ reset(ch, chdlr.sw);
+ // We are MASTER, the switch should be added to the list of active
+ // switches.
+ state.firstRoleReplyReceived = false;
+ controller.role = Role.MASTER;
+ expect(chdlr.sw.checkFirstPendingRoleRequestXid(xid)).andReturn(true);
+ chdlr.sw.deliverRoleRequestNotSupported(xid);
+ setupSwitchForAddSwitch(chdlr.sw, 0L);
+ chdlr.sw.clearAllFlowMods();
+ replay(ch, chdlr.sw);
+
+ chdlr.processOFMessage(msg);
+ verify(ch, chdlr.sw);
+ assertTrue("state.firstRoleReplyReceived must be true",
+ state.firstRoleReplyReceived);
+ assertSame("activeSwitches must contain this switch",
+ chdlr.sw, controller.activeSwitches.get(0L));
+ reset(ch, chdlr.sw);
+
+
+ // a different error messge
+ msg.setErrorType(OFErrorType.OFPET_BAD_REQUEST);
+ msg.setErrorCode(OFBadRequestCode.OFPBRC_EPERM);
+ state.firstRoleReplyReceived = false;
+ controller.role = Role.MASTER;
+ controller.activeSwitches.clear();
+ expect(chdlr.sw.checkFirstPendingRoleRequestXid(xid)).andReturn(true);
+ expect(chdlr.sw.getChannel()).andReturn(ch).anyTimes();
+ expect(ch.close()).andReturn(null);
+ replay(ch, chdlr.sw);
+
+ chdlr.processOFMessage(msg);
+ verify(ch, chdlr.sw);
+ assertFalse("state.firstRoleReplyReceived must be false",
+ state.firstRoleReplyReceived);
+ assertTrue("activeSwitches must be empty",
+ controller.activeSwitches.isEmpty());
+ reset(ch, chdlr.sw);
}
+
+
+ @Test
+ public void testVendorMessageUnknown() throws Exception {
+ // Check behavior with an unknown vendor id
+ OFChannelState state = new OFChannelState();
+ state.hsState = HandshakeState.READY;
+ Controller.OFChannelHandler chdlr = controller.new OFChannelHandler(state);
+ OFVendor msg = new OFVendor();
+ msg.setVendor(0);
+ chdlr.processOFMessage(msg);
+ }
+
+
+ // Helper function.
+ protected Controller.OFChannelHandler getChannelHandlerForRoleReplyTest() {
+ OFChannelState state = new OFChannelState();
+ state.hsState = HandshakeState.READY;
+ Controller.OFChannelHandler chdlr = controller.new OFChannelHandler(state);
+ chdlr.sw = createMock(OFSwitchImpl.class);
+ return chdlr;
+ }
+
+ // Helper function
+ protected OFVendor getRoleReplyMsgForRoleReplyTest(int xid, int nicira_role) {
+ OFVendor msg = new OFVendor();
+ msg.setXid(xid);
+ msg.setVendor(OFNiciraVendorData.NX_VENDOR_ID);
+ OFRoleReplyVendorData roleReplyVendorData =
+ new OFRoleReplyVendorData(OFRoleReplyVendorData.NXT_ROLE_REPLY);
+ msg.setVendorData(roleReplyVendorData);
+ roleReplyVendorData.setRole(nicira_role);
+ return msg;
+ }
+
+ /** invalid role in role reply */
+ @Test
+ public void testNiciraRoleReplyInvalidRole()
+ throws Exception {
+ int xid = 424242;
+ Controller.OFChannelHandler chdlr = getChannelHandlerForRoleReplyTest();
+ Channel ch = createMock(Channel.class);
+ expect(chdlr.sw.getChannel()).andReturn(ch);
+ expect(ch.close()).andReturn(null);
+ OFVendor msg = getRoleReplyMsgForRoleReplyTest(xid, 232323);
+ replay(chdlr.sw, ch);
+ chdlr.processOFMessage(msg);
+ verify(chdlr.sw, ch);
+ }
+
+ /** First role reply message received: transition from slave to master */
+ @Test
+ public void testNiciraRoleReplySlave2MasterFristTime()
+ throws Exception {
+ int xid = 424242;
+ Controller.OFChannelHandler chdlr = getChannelHandlerForRoleReplyTest();
+ OFVendor msg = getRoleReplyMsgForRoleReplyTest(xid,
+ OFRoleReplyVendorData.NX_ROLE_MASTER);
+
+ chdlr.sw.deliverRoleReply(xid, Role.MASTER);
+ expect(chdlr.sw.isActive()).andReturn(true);
+ setupSwitchForAddSwitch(chdlr.sw, 1L);
+ chdlr.sw.clearAllFlowMods();
+ chdlr.state.firstRoleReplyReceived = false;
+ replay(chdlr.sw);
+ chdlr.processOFMessage(msg);
+ verify(chdlr.sw);
+ assertTrue("state.firstRoleReplyReceived must be true",
+ chdlr.state.firstRoleReplyReceived);
+ assertSame("activeSwitches must contain this switch",
+ chdlr.sw, controller.activeSwitches.get(1L));
+ }
+
+
+ /** Not first role reply message received: transition from slave to master */
+ @Test
+ public void testNiciraRoleReplySlave2MasterNotFristTime()
+ throws Exception {
+ int xid = 424242;
+ Controller.OFChannelHandler chdlr = getChannelHandlerForRoleReplyTest();
+ OFVendor msg = getRoleReplyMsgForRoleReplyTest(xid,
+ OFRoleReplyVendorData.NX_ROLE_MASTER);
+
+ chdlr.sw.deliverRoleReply(xid, Role.MASTER);
+ expect(chdlr.sw.isActive()).andReturn(true);
+ setupSwitchForAddSwitch(chdlr.sw, 1L);
+ chdlr.state.firstRoleReplyReceived = true;
+ // Flow table shouldn't be wipe
+ replay(chdlr.sw);
+ chdlr.processOFMessage(msg);
+ verify(chdlr.sw);
+ assertTrue("state.firstRoleReplyReceived must be true",
+ chdlr.state.firstRoleReplyReceived);
+ assertSame("activeSwitches must contain this switch",
+ chdlr.sw, controller.activeSwitches.get(1L));
+ }
+
+ /** transition from slave to equal */
+ @Test
+ public void testNiciraRoleReplySlave2Equal()
+ throws Exception {
+ int xid = 424242;
+ Controller.OFChannelHandler chdlr = getChannelHandlerForRoleReplyTest();
+ OFVendor msg = getRoleReplyMsgForRoleReplyTest(xid,
+ OFRoleReplyVendorData.NX_ROLE_OTHER);
+
+ chdlr.sw.deliverRoleReply(xid, Role.EQUAL);
+ expect(chdlr.sw.isActive()).andReturn(true);
+ setupSwitchForAddSwitch(chdlr.sw, 1L);
+ chdlr.sw.clearAllFlowMods();
+ chdlr.state.firstRoleReplyReceived = false;
+ replay(chdlr.sw);
+ chdlr.processOFMessage(msg);
+ verify(chdlr.sw);
+ assertTrue("state.firstRoleReplyReceived must be true",
+ chdlr.state.firstRoleReplyReceived);
+ assertSame("activeSwitches must contain this switch",
+ chdlr.sw, controller.activeSwitches.get(1L));
+ };
+
+ @Test
+ /** Slave2Slave transition ==> no change */
+ public void testNiciraRoleReplySlave2Slave() throws Exception{
+ int xid = 424242;
+ Controller.OFChannelHandler chdlr = getChannelHandlerForRoleReplyTest();
+ OFVendor msg = getRoleReplyMsgForRoleReplyTest(xid,
+ OFRoleReplyVendorData.NX_ROLE_SLAVE);
+
+ chdlr.sw.deliverRoleReply(xid, Role.SLAVE);
+ expect(chdlr.sw.getId()).andReturn(1L).anyTimes();
+ expect(chdlr.sw.getStringId()).andReturn("00:00:00:00:00:00:00:01")
+ .anyTimes();
+ expect(chdlr.sw.isActive()).andReturn(false);
+ // don't add switch to activeSwitches ==> slave2slave
+ chdlr.state.firstRoleReplyReceived = false;
+ replay(chdlr.sw);
+ chdlr.processOFMessage(msg);
+ verify(chdlr.sw);
+ assertTrue("state.firstRoleReplyReceived must be true",
+ chdlr.state.firstRoleReplyReceived);
+ assertTrue("activeSwitches must be empty",
+ controller.activeSwitches.isEmpty());
+ }
+
+ @Test
+ /** Equal2Master transition ==> no change */
+ public void testNiciraRoleReplyEqual2Master() throws Exception{
+ int xid = 424242;
+ Controller.OFChannelHandler chdlr = getChannelHandlerForRoleReplyTest();
+ OFVendor msg = getRoleReplyMsgForRoleReplyTest(xid,
+ OFRoleReplyVendorData.NX_ROLE_MASTER);
+
+ chdlr.sw.deliverRoleReply(xid, Role.MASTER);
+ expect(chdlr.sw.getId()).andReturn(1L).anyTimes();
+ expect(chdlr.sw.getStringId()).andReturn("00:00:00:00:00:00:00:01")
+ .anyTimes();
+ expect(chdlr.sw.isActive()).andReturn(true);
+ controller.activeSwitches.put(1L, chdlr.sw);
+ chdlr.state.firstRoleReplyReceived = false;
+ replay(chdlr.sw);
+ chdlr.processOFMessage(msg);
+ verify(chdlr.sw);
+ assertTrue("state.firstRoleReplyReceived must be true",
+ chdlr.state.firstRoleReplyReceived);
+ assertSame("activeSwitches must contain this switch",
+ chdlr.sw, controller.activeSwitches.get(1L));
+ }
+
+ @Test
+ public void testNiciraRoleReplyMaster2Slave()
+ throws Exception {
+ int xid = 424242;
+ Controller.OFChannelHandler chdlr = getChannelHandlerForRoleReplyTest();
+ OFVendor msg = getRoleReplyMsgForRoleReplyTest(xid,
+ OFRoleReplyVendorData.NX_ROLE_SLAVE);
+
+ chdlr.sw.deliverRoleReply(xid, Role.SLAVE);
+ expect(chdlr.sw.getId()).andReturn(1L).anyTimes();
+ expect(chdlr.sw.getStringId()).andReturn("00:00:00:00:00:00:00:01")
+ .anyTimes();
+ controller.activeSwitches.put(1L, chdlr.sw);
+ expect(chdlr.sw.isActive()).andReturn(false);
+ expect(chdlr.sw.isConnected()).andReturn(true);
+ chdlr.sw.cancelAllStatisticsReplies();
+ chdlr.state.firstRoleReplyReceived = false;
+ replay(chdlr.sw);
+ chdlr.processOFMessage(msg);
+ verify(chdlr.sw);
+ assertTrue("state.firstRoleReplyReceived must be true",
+ chdlr.state.firstRoleReplyReceived);
+ assertTrue("activeSwitches must be empty",
+ controller.activeSwitches.isEmpty());
+ }
+
+
}
diff --git a/src/test/java/net/floodlightcontroller/core/internal/RoleChangerTest.java b/src/test/java/net/floodlightcontroller/core/internal/RoleChangerTest.java
index 221110d1d4cae249bc5a3ff4400d1b6c10f12829..991afffc282aef54b03076582dc9cafcef6b1dc6 100644
--- a/src/test/java/net/floodlightcontroller/core/internal/RoleChangerTest.java
+++ b/src/test/java/net/floodlightcontroller/core/internal/RoleChangerTest.java
@@ -140,13 +140,13 @@ public class RoleChangerTest {
// Add a switch that has received a role reply
OFSwitchImpl sw1 = EasyMock.createMock(OFSwitchImpl.class);
- expect(sw1.checkFirstPendingRoleReqeustCookie(123456))
+ expect(sw1.checkFirstPendingRoleRequestCookie(123456))
.andReturn(false).once();
switches.add(sw1);
// Add a switch that has not yet received a role reply
OFSwitchImpl sw2 = EasyMock.createMock(OFSwitchImpl.class);
- expect(sw2.checkFirstPendingRoleReqeustCookie(123456))
+ expect(sw2.checkFirstPendingRoleRequestCookie(123456))
.andReturn(true).once();
Channel channel2 = createMock(Channel.class);
expect(sw2.getChannel()).andReturn(channel2);
@@ -201,9 +201,9 @@ public class RoleChangerTest {
expect(sw1.sendNxRoleRequest(EasyMock.same(Role.SLAVE), EasyMock.anyLong()))
.andReturn(1);
// The following calls happen for timeout handling:
- expect(sw1.checkFirstPendingRoleReqeustCookie(EasyMock.anyLong()))
+ expect(sw1.checkFirstPendingRoleRequestCookie(EasyMock.anyLong()))
.andReturn(false);
- expect(sw1.checkFirstPendingRoleReqeustCookie(EasyMock.anyLong()))
+ expect(sw1.checkFirstPendingRoleRequestCookie(EasyMock.anyLong()))
.andReturn(false);
switches.add(sw1);