From 93e57d4825b5b6f3ae3a7c1fe0dba68c8f1da9ce Mon Sep 17 00:00:00 2001
From: Ryan Izard <rizard@g.clemson.edu>
Date: Tue, 22 Dec 2015 16:30:28 -0500
Subject: [PATCH] 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.
---
.../core/internal/OFSwitch.java | 34 ++-
.../core/internal/OFSwitchBaseTest.java | 36 ++-
.../core/internal/OFSwitchTest.java | 224 +++++++++---------
3 files changed, 168 insertions(+), 126 deletions(-)
diff --git a/src/main/java/net/floodlightcontroller/core/internal/OFSwitch.java b/src/main/java/net/floodlightcontroller/core/internal/OFSwitch.java
index afb7eed40..b31c37966 100644
--- a/src/main/java/net/floodlightcontroller/core/internal/OFSwitch.java
+++ b/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);
diff --git a/src/test/java/net/floodlightcontroller/core/internal/OFSwitchBaseTest.java b/src/test/java/net/floodlightcontroller/core/internal/OFSwitchBaseTest.java
index 5b73331d7..5c067f060 100644
--- a/src/test/java/net/floodlightcontroller/core/internal/OFSwitchBaseTest.java
+++ b/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
diff --git a/src/test/java/net/floodlightcontroller/core/internal/OFSwitchTest.java b/src/test/java/net/floodlightcontroller/core/internal/OFSwitchTest.java
index 4ddef9978..483aab250 100644
--- a/src/test/java/net/floodlightcontroller/core/internal/OFSwitchTest.java
+++ b/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());
+ }
}
--
GitLab