diff --git a/src/main/java/net/floodlightcontroller/core/internal/Controller.java b/src/main/java/net/floodlightcontroller/core/internal/Controller.java
index b7b048874a874b508e327bd87998832898f2c93f..6b183ca95d2dcd960e70f4d32b6b12bf812e7916 100644
--- a/src/main/java/net/floodlightcontroller/core/internal/Controller.java
+++ b/src/main/java/net/floodlightcontroller/core/internal/Controller.java
@@ -180,6 +180,13 @@ 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;
+ // This is the role of the controller based on HARoleChange notifications
+ // we have sent. I.e., this field reflects the last role notification
+ // we have sent to the listeners. On a transition to slave we first set
+ // this role and then notify, on a transition to master we first notify
+ // and then set the role. We then use it to make sure we don't forward
+ // OF messages while the modules are in slave role.
+ protected volatile Role notifiedRole;
// A helper that handles sending and timeout handling for role requests
protected RoleChanger roleChanger;
@@ -286,11 +293,19 @@ public class Controller implements IFloodlightProviderService,
log.trace("Dispatching HA Role update newRole = {}, oldRole = {}",
newRole, oldRole);
}
+ // Set notified role to slave before notifying listeners. This
+ // stops OF messages from being sent to listeners
+ if (newRole == Role.SLAVE)
+ Controller.this.notifiedRole = newRole;
if (haListeners != null) {
for (IHAListener listener : haListeners) {
listener.roleChanged(oldRole, newRole);
}
}
+ // Set notified role to master/equal after notifying listeners.
+ // We now forward messages again
+ if (newRole != Role.SLAVE)
+ Controller.this.notifiedRole = newRole;
}
}
@@ -981,7 +996,7 @@ public class Controller implements IFloodlightProviderService,
shouldHandleMessage = true;
} else {
// Queue till we complete driver binding
- state.queuedOFMessages.add((OFPortStatus) m);
+ state.queuedOFMessages.add(m);
}
break;
@@ -1001,9 +1016,9 @@ public class Controller implements IFloodlightProviderService,
try {
if (sw.isConnected()) {
- // Check if the controller is in the slave role for the
- // switch. If it is, then don't dispatch the message to
- // the listeners.
+ // Only dispatch message if the switch is in the
+ // activeSwitch map and if the switches role is
+ // not slave and the modules are not in slave
// 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
@@ -1012,7 +1027,9 @@ public class Controller implements IFloodlightProviderService,
// to them. On the other hand there might be special
// modules that care about all of the connected switches
// and would like to receive port status notifications.
- if (sw.getHARole() == Role.SLAVE) {
+ if (sw.getHARole() == Role.SLAVE ||
+ notifiedRole == Role.SLAVE ||
+ !activeSwitches.containsKey(sw.getId())) {
// Don't log message if it's a port status message
// since we expect to receive those from the switch
// and don't want to emit spurious messages.
@@ -1121,7 +1138,7 @@ public class Controller implements IFloodlightProviderService,
FloodlightContext bContext)
throws IOException {
Ethernet eth = null;
-
+
switch (m.getType()) {
case PACKET_IN:
OFPacketIn pi = (OFPacketIn)m;
@@ -1742,6 +1759,7 @@ public class Controller implements IFloodlightProviderService,
this.providerMap = new HashMap<String, List<IInfoProvider>>();
setConfigParams(configParams);
this.role = getInitialRole(configParams);
+ this.notifiedRole = this.role;
this.roleChanger = new RoleChanger(this);
initVendorMessages();
this.systemStartTime = System.currentTimeMillis();
diff --git a/src/test/java/net/floodlightcontroller/core/internal/ControllerTest.java b/src/test/java/net/floodlightcontroller/core/internal/ControllerTest.java
index 195f1c042cfcf2f90878ae7ced4f773eb1fdb998..6879f3f13a93ced3a2b08e41dba11d6fd84edf11 100644
--- a/src/test/java/net/floodlightcontroller/core/internal/ControllerTest.java
+++ b/src/test/java/net/floodlightcontroller/core/internal/ControllerTest.java
@@ -18,7 +18,6 @@
package net.floodlightcontroller.core.internal;
import static org.easymock.EasyMock.*;
-
import java.util.ArrayList;
import java.util.Collection;
import java.util.HashMap;
@@ -29,15 +28,16 @@ import java.util.Map;
import java.util.concurrent.ConcurrentHashMap;
import java.util.concurrent.Future;
import java.util.concurrent.TimeUnit;
+import java.util.concurrent.locks.Lock;
-import net.floodlightcontroller.core.FloodlightProvider;
import net.floodlightcontroller.core.FloodlightContext;
+import net.floodlightcontroller.core.FloodlightProvider;
import net.floodlightcontroller.core.IFloodlightProviderService;
-import net.floodlightcontroller.core.IHAListener;
import net.floodlightcontroller.core.IFloodlightProviderService.Role;
+import net.floodlightcontroller.core.IHAListener;
+import net.floodlightcontroller.core.IListener.Command;
import net.floodlightcontroller.core.IOFMessageFilterManagerService;
import net.floodlightcontroller.core.IOFMessageListener;
-import net.floodlightcontroller.core.IListener.Command;
import net.floodlightcontroller.core.IOFSwitch;
import net.floodlightcontroller.core.IOFSwitchDriver;
import net.floodlightcontroller.core.IOFSwitchListener;
@@ -72,15 +72,16 @@ import org.junit.Test;
import org.openflow.protocol.OFError;
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.OFPacketIn.OFPacketInReason;
import org.openflow.protocol.OFPacketOut;
import org.openflow.protocol.OFPhysicalPort;
import org.openflow.protocol.OFPort;
import org.openflow.protocol.OFPortStatus;
+import org.openflow.protocol.OFPortStatus.OFPortReason;
import org.openflow.protocol.OFStatisticsReply;
import org.openflow.protocol.OFType;
-import org.openflow.protocol.OFPacketIn.OFPacketInReason;
-import org.openflow.protocol.OFPortStatus.OFPortReason;
import org.openflow.protocol.OFVendor;
import org.openflow.protocol.action.OFAction;
import org.openflow.protocol.action.OFActionOutput;
@@ -172,6 +173,7 @@ public class ControllerTest extends FloodlightTestCase
* Run the controller's main loop so that updates are processed
*/
protected class ControllerRunThread extends Thread {
+ @Override
public void run() {
controller.openFlowPort = 0; // Don't listen
controller.run();
@@ -540,14 +542,17 @@ public class ControllerTest extends FloodlightTestCase
nRemoved = 0;
nPortChanged = 0;
}
+ @Override
public synchronized void addedSwitch(IOFSwitch sw) {
nAdded++;
notifyAll();
}
+ @Override
public synchronized void removedSwitch(IOFSwitch sw) {
nRemoved++;
notifyAll();
}
+ @Override
public String getName() {
return "dummy";
}
@@ -1349,5 +1354,356 @@ public class ControllerTest extends FloodlightTestCase
}
return null;
}
+
+ private void setupSwitchForDispatchTest(IOFSwitch sw,
+ boolean isConnected,
+ Role role) {
+ Lock lock = createNiceMock(Lock.class);
+ expect(sw.getId()).andReturn(1L).anyTimes();
+ expect(sw.getStringId()).andReturn("00:00:00:00:00:01").anyTimes();
+ expect(sw.getListenerReadLock()).andReturn(lock).anyTimes();
+ expect(sw.isConnected()).andReturn(isConnected).anyTimes();
+ expect(sw.getHARole()).andReturn(role).anyTimes();
+ replay(lock);
+
+ }
+
+ @Test
+ public void testMessageDispatch() throws Exception {
+ // Mock a dummy packet in
+ // Build our test packet
+ IPacket testPacket = new Ethernet()
+ .setSourceMACAddress("00:44:33:22:11:00")
+ .setDestinationMACAddress("00:11:22:33:44:55")
+ .setEtherType(Ethernet.TYPE_ARP)
+ .setPayload(
+ new ARP()
+ .setHardwareType(ARP.HW_TYPE_ETHERNET)
+ .setProtocolType(ARP.PROTO_TYPE_IP)
+ .setHardwareAddressLength((byte) 6)
+ .setProtocolAddressLength((byte) 4)
+ .setOpCode(ARP.OP_REPLY)
+ .setSenderHardwareAddress(Ethernet.toMACAddress("00:44:33:22:11:00"))
+ .setSenderProtocolAddress(IPv4.toIPv4AddressBytes("192.168.1.1"))
+ .setTargetHardwareAddress(Ethernet.toMACAddress("00:11:22:33:44:55"))
+ .setTargetProtocolAddress(IPv4.toIPv4AddressBytes("192.168.1.2")));
+ byte[] testPacketSerialized = testPacket.serialize();
+
+ // Build the PacketIn
+ OFPacketIn pi = ((OFPacketIn) new BasicFactory().getMessage(OFType.PACKET_IN))
+ .setBufferId(-1)
+ .setInPort((short) 1)
+ .setPacketData(testPacketSerialized)
+ .setReason(OFPacketInReason.NO_MATCH)
+ .setTotalLength((short) testPacketSerialized.length);
+
+
+ // Mock switch and add to data structures
+ IOFSwitch sw = createMock(IOFSwitch.class);
+
+ controller.connectedSwitches.add(sw);
+
+ // create a channel handler
+ OFChannelState state = new OFChannelState();
+ state.hsState = HandshakeState.READY;
+ Controller.OFChannelHandler chdlr = controller.new OFChannelHandler(state);
+ chdlr.sw = sw;
+
+ // mock role changer
+ RoleChanger roleChanger = createMock(RoleChanger.class);
+ roleChanger.submitRequest(eq(controller.connectedSwitches),
+ anyObject(Role.class));
+ expectLastCall().anyTimes();
+ controller.roleChanger = roleChanger;
+
+
+ // Mock message listener and add
+ IOFMessageListener listener = createNiceMock(IOFMessageListener.class);
+ expect(listener.getName()).andReturn("foobar").anyTimes();
+ replay(listener);
+ controller.addOFMessageListener(OFType.PACKET_IN, listener);
+ resetToStrict(listener);
+
+
+ assertEquals("Check that update queue is empty", 0,
+ controller.updates.size());
+
+
+ replay(roleChanger);
+
+ //-------------------
+ // Test 1: role is master, switch is master and in activeMap
+ // we expect the msg to be dispatched
+ reset(sw);
+ resetToDefault(listener);
+ controller.activeSwitches.put(1L, sw);
+ setupSwitchForDispatchTest(sw, true, Role.MASTER);
+ listener.receive(same(sw), same(pi),
+ anyObject(FloodlightContext.class));
+ expectLastCall().andReturn(Command.STOP).once();
+ replay(sw, listener);
+ chdlr.processOFMessage(pi);
+ verify(sw, listener);
+ assertEquals(0, controller.updates.size());
+
+
+ //-------------------
+ // Test 1b: role is master, switch is master and in activeMap
+ // but switch is not connected
+ // no message dispatched
+ reset(sw);
+ resetToDefault(listener);
+ controller.activeSwitches.put(1L, sw);
+ setupSwitchForDispatchTest(sw, false, Role.MASTER);
+ replay(sw, listener);
+ chdlr.processOFMessage(pi);
+ verify(sw, listener);
+ assertEquals(0, controller.updates.size());
+
+
+ //-------------------
+ // Test 1c: role is master, switch is slave and in activeMap
+ // no message dispatched
+ reset(sw);
+ resetToDefault(listener);
+ controller.activeSwitches.put(1L, sw);
+ setupSwitchForDispatchTest(sw, true, Role.SLAVE);
+ replay(sw, listener);
+ chdlr.processOFMessage(pi);
+ verify(sw, listener);
+ assertEquals(0, controller.updates.size());
+
+
+ //-------------------
+ // Test 1d: role is master, switch is master but not in activeMap
+ // we expect the msg to be dispatched
+ reset(sw);
+ resetToDefault(listener);
+ controller.activeSwitches.remove(1L);
+ setupSwitchForDispatchTest(sw, true, Role.MASTER);
+ replay(sw, listener);
+ chdlr.processOFMessage(pi);
+ verify(sw, listener);
+ assertEquals(0, controller.updates.size());
+
+
+
+ //-------------------
+ // Test 2: check correct dispatch and HA notification behavior
+ // We set the role to slave but do not notify the clients
+ reset(sw);
+ resetToDefault(listener);
+ controller.activeSwitches.put(1L, sw);
+ setupSwitchForDispatchTest(sw, true, Role.MASTER);
+ listener.receive(same(sw), same(pi),
+ anyObject(FloodlightContext.class));
+ expectLastCall().andReturn(Command.STOP).once();
+ replay(sw, listener);
+ controller.setRole(Role.SLAVE);
+ chdlr.processOFMessage(pi);
+ verify(sw, listener);
+ assertEquals(1, controller.updates.size());
+
+ // Now notify listeners
+ Controller.IUpdate upd = controller.updates.poll(1, TimeUnit.NANOSECONDS);
+ assertTrue("Check that update is HARoleUpdate",
+ upd instanceof Controller.HARoleUpdate);
+ upd.dispatch();
+ resetToDefault(listener);
+ replay(listener);
+ chdlr.processOFMessage(pi);
+ verify(listener);
+ assertEquals(0, controller.updates.size());
+
+ // transition back to master but don't notify yet
+ resetToDefault(listener);
+ replay(listener);
+ controller.setRole(Role.MASTER);
+ chdlr.processOFMessage(pi);
+ verify(listener);
+ assertEquals(1, controller.updates.size());
+
+ // now notify listeners
+ upd = controller.updates.poll(1, TimeUnit.NANOSECONDS);
+ assertTrue("Check that update is HARoleUpdate",
+ upd instanceof Controller.HARoleUpdate);
+ upd.dispatch();
+ resetToDefault(listener);
+ listener.receive(same(sw), same(pi),
+ anyObject(FloodlightContext.class));
+ expectLastCall().andReturn(Command.STOP).once();
+ replay(listener);
+ chdlr.processOFMessage(pi);
+ verify(listener);
+ assertEquals(0, controller.updates.size());
+
+ verify(sw);
+ }
+
+
+ /*
+ * Test correct timing behavior between HA Role notification and dispatching
+ * OFMessages to listeners.
+ * When transitioning to SLAVE: stop dispatching message before sending
+ * notifications
+ * When transitioning to MASTER: start dispatching messages after sending
+ * notifications
+ * (This implies that messages should not be dispatched while the
+ * notifications are being sent).
+ *
+ * We encapsulate the logic for this in a class that implements both
+ * IHAListener and IOFMessageListener. Then we inject an OFMessage fom
+ * the IHAListener and check that it gets dropped correctly.
+ */
+ @Test
+ public void testRoleNotifcationAndMessageDispatch() throws Exception {
+ class TestRoleNotificationsAndDispatch implements IHAListener, IOFMessageListener {
+ OFPacketIn pi;
+ Controller.OFChannelHandler chdlr;
+ IOFSwitch sw;
+ private boolean haveReceived;
+ private boolean doInjectMessageFromHAListener;
+
+ public TestRoleNotificationsAndDispatch() {
+ IPacket testPacket = new Ethernet()
+ .setSourceMACAddress("00:44:33:22:11:00")
+ .setDestinationMACAddress("00:11:22:33:44:55")
+ .setEtherType(Ethernet.TYPE_ARP)
+ .setPayload(
+ new ARP()
+ .setHardwareType(ARP.HW_TYPE_ETHERNET)
+ .setProtocolType(ARP.PROTO_TYPE_IP)
+ .setHardwareAddressLength((byte) 6)
+ .setProtocolAddressLength((byte) 4)
+ .setOpCode(ARP.OP_REPLY)
+ .setSenderHardwareAddress(Ethernet.toMACAddress("00:44:33:22:11:00"))
+ .setSenderProtocolAddress(IPv4.toIPv4AddressBytes("192.168.1.1"))
+ .setTargetHardwareAddress(Ethernet.toMACAddress("00:11:22:33:44:55"))
+ .setTargetProtocolAddress(IPv4.toIPv4AddressBytes("192.168.1.2")));
+ byte[] testPacketSerialized = testPacket.serialize();
+
+ // Build the PacketIn
+ pi = ((OFPacketIn) new BasicFactory().getMessage(OFType.PACKET_IN))
+ .setBufferId(-1)
+ .setInPort((short) 1)
+ .setPacketData(testPacketSerialized)
+ .setReason(OFPacketInReason.NO_MATCH)
+ .setTotalLength((short) testPacketSerialized.length);
+
+ // Mock switch and add to data structures
+ sw = createMock(IOFSwitch.class);
+ controller.connectedSwitches.add(sw);
+ controller.activeSwitches.put(1L, sw);
+ setupSwitchForDispatchTest(sw, true, Role.MASTER);
+ replay(sw);
+
+ // create a channel handler
+ OFChannelState state = new OFChannelState();
+ state.hsState = HandshakeState.READY;
+ chdlr = controller.new OFChannelHandler(state);
+ chdlr.sw = this.sw;
+
+ // add ourself as listeners
+ controller.addOFMessageListener(OFType.PACKET_IN, this);
+ controller.addHAListener(this);
+ }
+
+
+ private void injectMessage(boolean shouldReceive) throws Exception {
+ haveReceived = false;
+ chdlr.processOFMessage(pi);
+ assertEquals(shouldReceive, haveReceived);
+ }
+
+ public void transitionToSlave() throws Exception {
+ IUpdate update;
+
+ // Bring controller into well defined state for MASTER
+ doInjectMessageFromHAListener = false;
+ update = controller.new HARoleUpdate(Role.MASTER, Role.SLAVE);
+ update.dispatch();
+ doInjectMessageFromHAListener = true;
+
+
+ // inject message. Listener called
+ injectMessage(true);
+ // Dispatch update
+ update = controller.new HARoleUpdate(Role.SLAVE, Role.MASTER);
+ update.dispatch();
+ // inject message. Listener not called
+ injectMessage(false);
+ }
+
+ public void transitionToMaster() throws Exception {
+ IUpdate update;
+
+ // Bring controller into well defined state for SLAVE
+ doInjectMessageFromHAListener = false;
+ update = controller.new HARoleUpdate(Role.SLAVE, Role.MASTER);
+ update.dispatch();
+ doInjectMessageFromHAListener = true;
+
+
+ // inject message. Listener not called
+ injectMessage(false);
+ // Dispatch update
+ update = controller.new HARoleUpdate(Role.MASTER, Role.SLAVE);
+ update.dispatch();
+ // inject message. Listener called
+ injectMessage(true);
+ }
+
+ //---------------
+ // IHAListener
+ //---------------
+ @Override
+ public void roleChanged(Role oldRole, Role newRole) {
+ try {
+ if (doInjectMessageFromHAListener)
+ injectMessage(false);
+ } catch (Exception e) {
+ // TODO Auto-generated catch block
+ e.printStackTrace();
+ }
+ }
+
+ @Override
+ public
+ void
+ controllerNodeIPsChanged(Map<String, String> curControllerNodeIPs,
+ Map<String, String> addedControllerNodeIPs,
+ Map<String, String> removedControllerNodeIPs) {
+ // TODO Auto-generated method stub
+ }
+
+ //-------------------------
+ // IOFMessageListener
+ //-------------------------
+ @Override
+ public String getName() {
+ return "FooBar";
+ }
+ @Override
+ public boolean isCallbackOrderingPrereq(OFType type, String name) {
+ return false;
+ }
+ @Override
+ public boolean isCallbackOrderingPostreq(OFType type, String name) {
+ return false;
+ }
+ @Override
+ public Command receive(IOFSwitch sw,
+ OFMessage msg,
+ FloodlightContext cntx) {
+ haveReceived = true;
+ return Command.STOP;
+ }
+ }
+
+ TestRoleNotificationsAndDispatch x = new TestRoleNotificationsAndDispatch();
+ x.transitionToSlave();
+ x.transitionToMaster();
+
+ }
}