Added unit test for writing messages while in MASTER/SLAVE. Changed to a...

Added unit test for writing messages while in MASTER/SLAVE. Changed to a blacklist mechanism instead of a whitelist (to match how the spec describes invalid (write) SLAVE messages). Completed invalid SLAVE messages for each OF version.

src/main/java/net/floodlightcontroller/core/internal/OFSwitch.java

@@ -682,29 +682,39 @@ public class OFSwitch implements IOFSwitchBackend {
 	}
 
 	protected static class SwitchRoleMessageValidator {
-		private static final Map<OFVersion, Set<OFType>> validSlaveMsgsByOFVersion;
+		private static final Map<OFVersion, Set<OFType>> invalidSlaveMsgsByOFVersion;
 		static {
 			Map<OFVersion, Set<OFType>> m = new HashMap<OFVersion, Set<OFType>>();
 			Set<OFType> s = new HashSet<OFType>();
-			s.add(OFType.ROLE_REQUEST);
-			s.add(OFType.SET_ASYNC);
-			s.add(OFType.SET_CONFIG);
-			s.add(OFType.GET_ASYNC_REQUEST);
-			s.add(OFType.ECHO_REQUEST);
-			s.add(OFType.GET_CONFIG_REQUEST);
-			s.add(OFType.STATS_REQUEST);
-			s.add(OFType.FEATURES_REQUEST);
+			s.add(OFType.PACKET_OUT);
+			s.add(OFType.FLOW_MOD);
+			s.add(OFType.PORT_MOD);
+			s.add(OFType.TABLE_MOD);
+			s.add(OFType.BARRIER_REQUEST);
 			m.put(OFVersion.OF_10, Collections.unmodifiableSet(s));
+			
 			s = new HashSet<OFType>();
+			s.addAll(m.get(OFVersion.OF_10));
+			s.add(OFType.GROUP_MOD);
+			s.add(OFType.TABLE_MOD);
 			m.put(OFVersion.OF_11, Collections.unmodifiableSet(s));
+			
 			s = new HashSet<OFType>();
+			s.addAll(m.get(OFVersion.OF_11));
 			m.put(OFVersion.OF_12, Collections.unmodifiableSet(s));
+			
 			s = new HashSet<OFType>();
+			s.addAll(m.get(OFVersion.OF_12));
+			s.add(OFType.METER_MOD);
 			m.put(OFVersion.OF_13, Collections.unmodifiableSet(s));
+			
 			s = new HashSet<OFType>();
+			s.addAll(m.get(OFVersion.OF_13));
+			s.add(OFType.BUNDLE_ADD_MESSAGE);
+			s.add(OFType.BUNDLE_CONTROL);
 			m.put(OFVersion.OF_14, Collections.unmodifiableSet(s));
 
-			validSlaveMsgsByOFVersion = Collections.unmodifiableMap(m);
+			invalidSlaveMsgsByOFVersion = Collections.unmodifiableMap(m);
 		}
 
 		/**
@@ -724,12 +734,12 @@ public class OFSwitch implements IOFSwitchBackend {
 				valid.addAll(IterableUtils.toCollection(msgList));
 				return Collections.emptyList();
 			} else { /* slave */
-				Set<OFType> validSlaveMsgs = validSlaveMsgsByOFVersion.get(swVersion);
+				Set<OFType> invalidSlaveMsgs = invalidSlaveMsgsByOFVersion.get(swVersion);
 				List<OFMessage> invalid = new ArrayList<OFMessage>();
 				Iterator<OFMessage> itr = msgList.iterator();
 				while (itr.hasNext()) {
 					OFMessage m = itr.next();
-					if (!validSlaveMsgs.contains(m.getType())) {
+					if (invalidSlaveMsgs.contains(m.getType())) {
 						invalid.add(m);
 					} else {
 						valid.add(m);

src/test/java/net/floodlightcontroller/core/internal/OFSwitchBaseTest.java

@@ -59,6 +59,8 @@ import net.floodlightcontroller.debugcounter.IDebugCounterService;
 import org.projectfloodlight.openflow.protocol.OFControllerRole;
 import org.projectfloodlight.openflow.protocol.OFFactories;
 import org.projectfloodlight.openflow.protocol.OFFactory;
+import org.projectfloodlight.openflow.protocol.OFFlowAdd;
+import org.projectfloodlight.openflow.protocol.OFFlowStatsRequest;
 import org.projectfloodlight.openflow.protocol.OFMessage;
 import org.projectfloodlight.openflow.protocol.OFPortConfig;
 import org.projectfloodlight.openflow.protocol.OFPortDesc;
@@ -145,7 +147,7 @@ public class OFSwitchBaseTest {
 
         IOFConnectionBackend conn = EasyMock.createNiceMock(IOFConnectionBackend.class);
         capturedMessage = new Capture<Iterable<OFMessage>>();
-        expect(conn.write(EasyMock.capture(capturedMessage))).andReturn(Collections.<OFMessage>emptyList()).once();
+        expect(conn.write(EasyMock.capture(capturedMessage))).andReturn(Collections.<OFMessage>emptyList()).atLeastOnce();
         expect(conn.getOFFactory()).andReturn(factory).anyTimes();
         expect(conn.getAuxId()).andReturn(OFAuxId.MAIN).anyTimes();
         EasyMock.replay(conn);
@@ -1413,4 +1415,34 @@ public class OFSwitchBaseTest {
         verify(switchManager);
     }
 
-}
+
+	@Test
+	public void testMasterSlaveWrites() {
+		OFFactory factory = OFFactories.getFactory(OFVersion.OF_13);
+		OFFlowAdd fa = factory.buildFlowAdd().build();
+		OFFlowStatsRequest fsr = factory.buildFlowStatsRequest().build();
+		List<OFMessage> msgList = new ArrayList<OFMessage>();
+		msgList.add(fa);
+		msgList.add(fsr);
+		
+		reset(switchManager);
+        expect(switchManager.isCategoryRegistered(LogicalOFMessageCategory.MAIN)).andReturn(true).times(6);
+        switchManager.handleOutgoingMessage(sw, fa);
+        expectLastCall().times(2);
+        switchManager.handleOutgoingMessage(sw, fsr);
+        expectLastCall().times(4);
+        replay(switchManager);
+
+		/* test master -- both messages should be written */
+		sw.setControllerRole(OFControllerRole.ROLE_MASTER);
+		assertTrue(sw.write(fa));
+		assertTrue(sw.write(fsr));
+		assertEquals(Collections.<OFMessage>emptyList(), sw.write(msgList));
+		
+		/* test slave -- flow-add (mod op) should fail each time; flow stats (read op) should pass */
+		sw.setControllerRole(OFControllerRole.ROLE_SLAVE);
+		assertFalse(sw.write(fa)); /* flow-add should be stopped (mod op) */
+		assertTrue(sw.write(fsr)); /* stats request makes it (read op) */
+		assertEquals(Collections.<OFMessage>singletonList(fa), sw.write(msgList)); /* return bad flow-add */
+	}
+}
\ No newline at end of file

src/test/java/net/floodlightcontroller/core/internal/OFSwitchTest.java

@@ -49,116 +49,116 @@ import org.projectfloodlight.openflow.types.DatapathId;
 import org.projectfloodlight.openflow.types.OFAuxId;
 
 public class OFSwitchTest {
-    protected OFSwitch sw;
-    protected OFFactory factory = OFFactories.getFactory(OFVersion.OF_13);
-
-    @Before
-    public void setUp() throws Exception {
-        MockOFConnection mockConnection = new MockOFConnection(DatapathId.of(1), OFAuxId.MAIN);
-        sw = new OFSwitch(mockConnection, OFFactories.getFactory(OFVersion.OF_10),
-                EasyMock.createMock(IOFSwitchManager.class), DatapathId.of(1));
-    }
-
-    @Test
-    public void testSetHARoleReply() {
-        sw.setControllerRole(OFControllerRole.ROLE_MASTER);
-        assertEquals(OFControllerRole.ROLE_MASTER, sw.getControllerRole());
-
-        sw.setControllerRole(OFControllerRole.ROLE_EQUAL);
-        assertEquals(OFControllerRole.ROLE_EQUAL, sw.getControllerRole());
-
-        sw.setControllerRole(OFControllerRole.ROLE_SLAVE);
-        assertEquals(OFControllerRole.ROLE_SLAVE, sw.getControllerRole());
-    }
-
-    @Test
-    public void testSubHandshake() {
-        OFFactory factory = OFFactories.getFactory(OFVersion.OF_10);
-        OFMessage m = factory.buildNiciraControllerRoleReply()
-                .setXid(1)
-                .setRole(OFNiciraControllerRole.ROLE_MASTER)
-                .build();
-        // test exceptions before handshake is started
-        try {
-            sw.processDriverHandshakeMessage(m);
-            fail("expected exception not thrown");
-        } catch (SwitchDriverSubHandshakeNotStarted e) { /* expected */ }
-        try {
-            sw.isDriverHandshakeComplete();
-            fail("expected exception not thrown");
-        } catch (SwitchDriverSubHandshakeNotStarted e) { /* expected */ }
-
-        // start the handshake -- it should immediately complete
-        sw.startDriverHandshake();
-        assertTrue("Handshake should be complete",
-                   sw.isDriverHandshakeComplete());
-
-        // test exceptions after handshake is completed
-        try {
-            sw.processDriverHandshakeMessage(m);
-            fail("expected exception not thrown");
-        } catch (SwitchDriverSubHandshakeCompleted e) { /* expected */ }
-        try {
-            sw.startDriverHandshake();
-            fail("Expected exception not thrown");
-        } catch (SwitchDriverSubHandshakeAlreadyStarted e) { /* expected */ }
-    }
-
-    /**
-     * Helper to load controller connection messages into a switch for testing.
-     * @param sw the switch to insert the message on
-     * @param role the role for the controller connection message
-     * @param state the state for the controller connection message
-     * @param uri the URI for the controller connection message
-     */
-    public void updateControllerConnections(IOFSwitchBackend sw, OFControllerRole role1, OFBsnControllerConnectionState state1, String uri1
-                                            ,  OFControllerRole role2, OFBsnControllerConnectionState state2, String uri2) {
-        OFBsnControllerConnection connection1 = factory.buildBsnControllerConnection()
-                .setAuxiliaryId(OFAuxId.MAIN)
-                .setRole(role1)
-                .setState(state1)
-                .setUri(uri1)
-                .build();
-
-        OFBsnControllerConnection connection2 = factory.buildBsnControllerConnection()
-                .setAuxiliaryId(OFAuxId.MAIN)
-                .setRole(role2)
-                .setState(state2)
-                .setUri(uri2)
-                .build();
-
-        List<OFBsnControllerConnection> connections = new ArrayList<OFBsnControllerConnection>();
-        connections.add(connection1);
-        connections.add(connection2);
-
-        OFBsnControllerConnectionsReply reply = factory.buildBsnControllerConnectionsReply()
-                .setConnections(connections)
-                .build();
-
-        sw.updateControllerConnections(reply);
-    }
-
-    /**
-     * This test ensures that the switch accurately determined if another master
-     * exists in the cluster by examining the controller connections it has.
-     */
-    @Test
-    public void testHasAnotherMaster() {
-        URI cokeUri = URIUtil.createURI("1.2.3.4", 6653);
-        InetSocketAddress address = (InetSocketAddress) sw.getConnection(OFAuxId.MAIN).getLocalInetAddress();
-        URI pepsiUri = URIUtil.createURI(address.getHostName(), address.getPort());
-
-        updateControllerConnections(sw, OFControllerRole.ROLE_SLAVE, OFBsnControllerConnectionState.BSN_CONTROLLER_CONNECTION_STATE_CONNECTED, cokeUri.toString(),
-                                    OFControllerRole.ROLE_MASTER, OFBsnControllerConnectionState.BSN_CONTROLLER_CONNECTION_STATE_CONNECTED, pepsiUri.toString());
-
-        // From the perspective of pepsi, the cluster currently does NOT have another master controller
-        assertFalse(sw.hasAnotherMaster());
-
-        // Switch the controller connections so that pepsi is no longer master
-        updateControllerConnections(sw, OFControllerRole.ROLE_MASTER, OFBsnControllerConnectionState.BSN_CONTROLLER_CONNECTION_STATE_CONNECTED, cokeUri.toString(),
-                                    OFControllerRole.ROLE_SLAVE, OFBsnControllerConnectionState.BSN_CONTROLLER_CONNECTION_STATE_CONNECTED, pepsiUri.toString());
-
-        // From the perspective of pepsi, the cluster currently has another master controller
-        assertTrue(sw.hasAnotherMaster());
-    }
+	protected OFSwitch sw;
+	protected OFFactory factory = OFFactories.getFactory(OFVersion.OF_13);
+
+	@Before
+	public void setUp() throws Exception {
+		MockOFConnection mockConnection = new MockOFConnection(DatapathId.of(1), OFAuxId.MAIN);
+		sw = new OFSwitch(mockConnection, OFFactories.getFactory(OFVersion.OF_10),
+				EasyMock.createMock(IOFSwitchManager.class), DatapathId.of(1));
+	}
+
+	@Test
+	public void testSetHARoleReply() {
+		sw.setControllerRole(OFControllerRole.ROLE_MASTER);
+		assertEquals(OFControllerRole.ROLE_MASTER, sw.getControllerRole());
+
+		sw.setControllerRole(OFControllerRole.ROLE_EQUAL);
+		assertEquals(OFControllerRole.ROLE_EQUAL, sw.getControllerRole());
+
+		sw.setControllerRole(OFControllerRole.ROLE_SLAVE);
+		assertEquals(OFControllerRole.ROLE_SLAVE, sw.getControllerRole());
+	}
+
+	@Test
+	public void testSubHandshake() {
+		OFFactory factory = OFFactories.getFactory(OFVersion.OF_10);
+		OFMessage m = factory.buildNiciraControllerRoleReply()
+				.setXid(1)
+				.setRole(OFNiciraControllerRole.ROLE_MASTER)
+				.build();
+		// test exceptions before handshake is started
+		try {
+			sw.processDriverHandshakeMessage(m);
+			fail("expected exception not thrown");
+		} catch (SwitchDriverSubHandshakeNotStarted e) { /* expected */ }
+		try {
+			sw.isDriverHandshakeComplete();
+			fail("expected exception not thrown");
+		} catch (SwitchDriverSubHandshakeNotStarted e) { /* expected */ }
+
+		// start the handshake -- it should immediately complete
+		sw.startDriverHandshake();
+		assertTrue("Handshake should be complete",
+				sw.isDriverHandshakeComplete());
+
+		// test exceptions after handshake is completed
+		try {
+			sw.processDriverHandshakeMessage(m);
+			fail("expected exception not thrown");
+		} catch (SwitchDriverSubHandshakeCompleted e) { /* expected */ }
+		try {
+			sw.startDriverHandshake();
+			fail("Expected exception not thrown");
+		} catch (SwitchDriverSubHandshakeAlreadyStarted e) { /* expected */ }
+	}
+
+	/**
+	 * Helper to load controller connection messages into a switch for testing.
+	 * @param sw the switch to insert the message on
+	 * @param role the role for the controller connection message
+	 * @param state the state for the controller connection message
+	 * @param uri the URI for the controller connection message
+	 */
+	public void updateControllerConnections(IOFSwitchBackend sw, OFControllerRole role1, OFBsnControllerConnectionState state1, String uri1
+			,  OFControllerRole role2, OFBsnControllerConnectionState state2, String uri2) {
+		OFBsnControllerConnection connection1 = factory.buildBsnControllerConnection()
+				.setAuxiliaryId(OFAuxId.MAIN)
+				.setRole(role1)
+				.setState(state1)
+				.setUri(uri1)
+				.build();
+
+		OFBsnControllerConnection connection2 = factory.buildBsnControllerConnection()
+				.setAuxiliaryId(OFAuxId.MAIN)
+				.setRole(role2)
+				.setState(state2)
+				.setUri(uri2)
+				.build();
+
+		List<OFBsnControllerConnection> connections = new ArrayList<OFBsnControllerConnection>();
+		connections.add(connection1);
+		connections.add(connection2);
+
+		OFBsnControllerConnectionsReply reply = factory.buildBsnControllerConnectionsReply()
+				.setConnections(connections)
+				.build();
+
+		sw.updateControllerConnections(reply);
+	}
+
+	/**
+	 * This test ensures that the switch accurately determined if another master
+	 * exists in the cluster by examining the controller connections it has.
+	 */
+	@Test
+	public void testHasAnotherMaster() {
+		URI cokeUri = URIUtil.createURI("1.2.3.4", 6653);
+		InetSocketAddress address = (InetSocketAddress) sw.getConnection(OFAuxId.MAIN).getLocalInetAddress();
+		URI pepsiUri = URIUtil.createURI(address.getHostName(), address.getPort());
+
+		updateControllerConnections(sw, OFControllerRole.ROLE_SLAVE, OFBsnControllerConnectionState.BSN_CONTROLLER_CONNECTION_STATE_CONNECTED, cokeUri.toString(),
+				OFControllerRole.ROLE_MASTER, OFBsnControllerConnectionState.BSN_CONTROLLER_CONNECTION_STATE_CONNECTED, pepsiUri.toString());
+
+		// From the perspective of pepsi, the cluster currently does NOT have another master controller
+		assertFalse(sw.hasAnotherMaster());
+
+		// Switch the controller connections so that pepsi is no longer master
+		updateControllerConnections(sw, OFControllerRole.ROLE_MASTER, OFBsnControllerConnectionState.BSN_CONTROLLER_CONNECTION_STATE_CONNECTED, cokeUri.toString(),
+				OFControllerRole.ROLE_SLAVE, OFBsnControllerConnectionState.BSN_CONTROLLER_CONNECTION_STATE_CONNECTED, pepsiUri.toString());
+
+		// From the perspective of pepsi, the cluster currently has another master controller
+		assertTrue(sw.hasAnotherMaster());
+	}
 }