From 203d065ce3c77d9851f427f9234b7667508cf5d8 Mon Sep 17 00:00:00 2001
From: Ryan Izard <rizard@g.clemson.edu>
Date: Thu, 13 Aug 2015 08:50:43 -0400
Subject: [PATCH] Patched the handshake so that on transition to master, we
 guarantee the order of flow table operations (e.g. clear, adding table-miss
 flows, and IOFSwitchListener module switchAdded/switchActivated listener
 operations). Barriers are used inbetween all such operations.

---
 .../internal/OFSwitchHandshakeHandler.java    | 57 ++++++++++++++-----
 .../internal/OFSwitchHandlerTestBase.java     | 34 +++++++++--
 2 files changed, 70 insertions(+), 21 deletions(-)

diff --git a/src/main/java/net/floodlightcontroller/core/internal/OFSwitchHandshakeHandler.java b/src/main/java/net/floodlightcontroller/core/internal/OFSwitchHandshakeHandler.java
index 0b219e087..6eccbe072 100644
--- a/src/main/java/net/floodlightcontroller/core/internal/OFSwitchHandshakeHandler.java
+++ b/src/main/java/net/floodlightcontroller/core/internal/OFSwitchHandshakeHandler.java
@@ -482,6 +482,14 @@ public class OFSwitchHandshakeHandler implements IOFConnectionListener {
 			.setGroupType(OFGroupType.SELECT)
 			.build();
 			this.sw.write(delgroup);
+			
+			/*
+			 * Make sure we allow these operations to complete before proceeding.
+			 */
+			OFBarrierRequest barrier = factory.buildBarrierRequest()
+					.setXid(handshakeTransactionIds--)
+					.build();
+			sw.write(barrier);
 		}
 	}
 
@@ -639,11 +647,11 @@ public class OFSwitchHandshakeHandler implements IOFConnectionListener {
 												.setMaxLen(0xffFFffFF)
 												.setPort(p)
 												.build()))
-								.build()))
-						.setGroup(OFDPAUtils.GroupIds.createL2Interface(p, VlanVid.ofVlan(100)))
-						.build();
+												.build()))
+												.setGroup(OFDPAUtils.GroupIds.createL2Interface(p, VlanVid.ofVlan(100)))
+												.build();
 				sw.write(ga);
-				
+
 				/*
 				 * Add the port+bucket for creating the FLOOD group below.
 				 * All L2_INTERFACE groups in a VLAN should be within a
@@ -652,14 +660,14 @@ public class OFSwitchHandshakeHandler implements IOFConnectionListener {
 				buckets.add(factory.buildBucket().setActions(
 						Collections.singletonList(
 								(OFAction) factory.actions().buildOutput()
-										.setMaxLen(0xffFFffFF)
-										.setPort(p)
-										.build()
-						)
-				).build());
+								.setMaxLen(0xffFFffFF)
+								.setPort(p)
+								.build()
+								)
+						).build());
 			}
 		}
-		
+
 		OFGroupAdd ga = factory.buildGroupAdd()
 				.setGroupType(OFGroupType.ALL)
 				.setBuckets(buckets)
@@ -667,7 +675,7 @@ public class OFSwitchHandshakeHandler implements IOFConnectionListener {
 				.build();
 		sw.write(ga);
 	}
-	
+
 	/**
 	 * Default implementation for message handlers in any state.
 	 *
@@ -1471,23 +1479,42 @@ public class OFSwitchHandshakeHandler implements IOFConnectionListener {
 	 * SLAVE.
 	 */
 	public class MasterState extends OFSwitchHandshakeState {
-
+		
 		MasterState() {
 			super(true);
 		}
+		
+		private long sendBarrier() {
+			long xid = handshakeTransactionIds--;
+			OFBarrierRequest barrier = factory.buildBarrierRequest()
+					.setXid(xid)
+					.build();
+			sw.write(barrier); /* don't use ListenableFuture here; we receive via barrier reply OR error (barrier unsupported) */
+			return xid;
+		}
 
 		@Override
 		void enterState() {
-			setSwitchStatus(SwitchStatus.MASTER);
 			if (OFSwitchManager.clearTablesOnEachTransitionToMaster) {
-				log.info("Clearing flow tables of {} on recent transition to MASTER.", sw.getId().toString());
+				log.info("Clearing flow tables of {} on upcoming transition to MASTER.", sw.getId().toString());
 				clearAllTables();
 			} else if (OFSwitchManager.clearTablesOnInitialConnectAsMaster && initialRole == null) { /* don't do it if we were slave first */
 				initialRole = OFControllerRole.ROLE_MASTER;
-				log.info("Clearing flow tables of {} on initial role as MASTER.", sw.getId().toString());
+				log.info("Clearing flow tables of {} on upcoming initial role as MASTER.", sw.getId().toString());
 				clearAllTables();
 			}
+			
+			sendBarrier(); /* Need to make sure the tables are clear before adding default flows */
 			addDefaultFlows();
+
+			/*
+			 * We also need a barrier between adding flows and notifying modules of the
+			 * transition to master. Some modules might modify the flow tables and expect 
+			 * the clear/default flow operations above to have completed.
+			 */
+			sendBarrier();
+			
+			setSwitchStatus(SwitchStatus.MASTER);
 		}
 
 		@LogMessageDoc(level="WARN",
diff --git a/src/test/java/net/floodlightcontroller/core/internal/OFSwitchHandlerTestBase.java b/src/test/java/net/floodlightcontroller/core/internal/OFSwitchHandlerTestBase.java
index 12cd0be08..9c67de3b2 100644
--- a/src/test/java/net/floodlightcontroller/core/internal/OFSwitchHandlerTestBase.java
+++ b/src/test/java/net/floodlightcontroller/core/internal/OFSwitchHandlerTestBase.java
@@ -48,6 +48,7 @@ import net.floodlightcontroller.debugcounter.IDebugCounterService;
 
 import org.projectfloodlight.openflow.protocol.OFBadActionCode;
 import org.projectfloodlight.openflow.protocol.OFBadRequestCode;
+import org.projectfloodlight.openflow.protocol.OFBarrierReply;
 import org.projectfloodlight.openflow.protocol.OFControllerRole;
 import org.projectfloodlight.openflow.protocol.OFDescStatsReply;
 import org.projectfloodlight.openflow.protocol.OFErrorMsg;
@@ -547,8 +548,13 @@ public abstract class OFSwitchHandlerTestBase {
 			switchManager.switchStatusChanged(sw, SwitchStatus.HANDSHAKE, newStatus);
 		}
 		replay(sw, switchManager);
-
+		
 		switchHandler.sendRoleRequest(role);
+		
+		/* Now, trigger transition to master */
+		OFBarrierReply br = getFactory().buildBarrierReply()
+				.build();
+		switchHandler.processOFMessage(br);
 
 		verify(sw, switchManager);
 	}
@@ -605,6 +611,7 @@ public abstract class OFSwitchHandlerTestBase {
 		expect(sw.getStatus()).andReturn(SwitchStatus.HANDSHAKE).once();
 		sw.setStatus(SwitchStatus.MASTER);
 		expectLastCall().once();
+
 		if (factory.getVersion().compareTo(OFVersion.OF_13) >= 0) {
 			expect(sw.getMaxTableForTableMissFlow()).andReturn(TableId.ZERO).times(1);
 			expect(sw.getTableFeatures(TableId.ZERO)).andReturn(TableFeatures.of(createTableFeaturesStatsReply().getEntries().get(0))).anyTimes();
@@ -617,8 +624,13 @@ public abstract class OFSwitchHandlerTestBase {
 		replay(switchManager);
 		OFMessage reply = getRoleReply(xid, OFControllerRole.ROLE_MASTER);
 
-		// sendMessageToHandler will verify and rest controller mock
+		/* Go into the MasterState */
 		switchHandler.processOFMessage(reply);
+		
+		/* Now, trigger transition to master */
+		OFBarrierReply br = getFactory().buildBarrierReply()
+				.build();
+		switchHandler.processOFMessage(br);
 
 		assertThat(switchHandler.getStateForTesting(), CoreMatchers.instanceOf(OFSwitchHandshakeHandler.MasterState.class));
 	}
@@ -719,8 +731,14 @@ public abstract class OFSwitchHandlerTestBase {
 		switchManager.switchStatusChanged(sw, SwitchStatus.HANDSHAKE, SwitchStatus.MASTER);
 		expectLastCall().once();
 		replay(switchManager);
-		// sendMessageToHandler will verify and rest controller mock
+		
+		/* Go into the MasterState */
 		switchHandler.processOFMessage(err);
+		
+		/* Now, trigger transition to master */
+		OFBarrierReply br = getFactory().buildBarrierReply()
+				.build();
+		switchHandler.processOFMessage(br);
 
 		assertThat(switchHandler.getStateForTesting(), CoreMatchers.instanceOf(OFSwitchHandshakeHandler.MasterState.class));
 	}
@@ -743,7 +761,7 @@ public abstract class OFSwitchHandlerTestBase {
 		assertThat(switchHandler.getStateForTesting(), CoreMatchers.instanceOf(OFSwitchHandshakeHandler.WaitInitialRoleState.class));
 
 		// Set the role
-		setupSwitchSendRoleRequestAndVerify(null, OFControllerRole.ROLE_MASTER);
+		setupSwitchSendRoleRequestAndVerify(null, OFControllerRole.ROLE_MASTER); /* don't care about the XID, since we assume the reply is lost */
 		assertThat(switchHandler.getStateForTesting(), CoreMatchers.instanceOf(OFSwitchHandshakeHandler.WaitInitialRoleState.class));
 
 		// prepare mocks and inject the role reply message
@@ -767,15 +785,19 @@ public abstract class OFSwitchHandlerTestBase {
 		}
 		replay(sw);
 
-		OFMessage m = factory.buildBarrierReply().build();
+		OFMessage m = factory.barrierReply();
 
-		Thread.sleep(timeout+5);
+		Thread.sleep(timeout + 5);
 
 		reset(switchManager);
 		switchManager.switchStatusChanged(sw, SwitchStatus.HANDSHAKE, SwitchStatus.MASTER);
 		expectLastCall().once();
 		replay(switchManager);
 		switchHandler.processOFMessage(m);
+		
+		/* Send another barrier to trigger becoming master */
+		switchHandler.processOFMessage(m);
+
 
 		assertThat(switchHandler.getStateForTesting(), CoreMatchers.instanceOf(OFSwitchHandshakeHandler.MasterState.class));
 
-- 
GitLab