diff --git a/src/main/java/net/floodlightcontroller/core/OFSwitchBase.java b/src/main/java/net/floodlightcontroller/core/OFSwitchBase.java index 6661cb10699c41cb36f6ff5a26c468e2b447ba90..5397e4633077f09168c94eed5c325f36127bc90f 100644 --- a/src/main/java/net/floodlightcontroller/core/OFSwitchBase.java +++ b/src/main/java/net/floodlightcontroller/core/OFSwitchBase.java @@ -1103,11 +1103,16 @@ public abstract class OFSwitchBase implements IOFSwitch { this.role = role; } + @LogMessageDoc(level="INFO", + message="Switch {switch} flow cleared", + explanation="The switch flow table has been cleared, " + + "this normally happens on switch connection") @Override public void clearAllFlowMods() { if (channel == null || !isConnected()) return; // Delete all pre-existing flows + log.info("Clearing all flows on switch {}", this); OFMatch match = new OFMatch().setWildcards(OFMatch.OFPFW_ALL); OFMessage fm = ((OFFlowMod) floodlightProvider.getOFMessageFactory() .getMessage(OFType.FLOW_MOD)) diff --git a/src/main/java/net/floodlightcontroller/core/internal/Controller.java b/src/main/java/net/floodlightcontroller/core/internal/Controller.java index 6bfd413d38665ac9397375c9c6db05929e8bad2e..fd6c35865ebb9b24127509b6375d367aeae0f71b 100644 --- a/src/main/java/net/floodlightcontroller/core/internal/Controller.java +++ b/src/main/java/net/floodlightcontroller/core/internal/Controller.java @@ -95,6 +95,8 @@ import net.floodlightcontroller.threadpool.IThreadPoolService; import net.floodlightcontroller.util.EventHistory; import net.floodlightcontroller.util.LoadMonitor; import net.floodlightcontroller.util.EventHistory.EvAction; +import net.floodlightcontroller.util.TimedCache; + import org.jboss.netty.bootstrap.ServerBootstrap; import org.jboss.netty.channel.ChannelPipelineFactory; import org.jboss.netty.channel.group.ChannelGroup; @@ -191,6 +193,7 @@ public class Controller implements IFloodlightProviderService, // Flag to always flush flow table on switch reconnect (HA or otherwise) private boolean alwaysClearFlowsOnSwActivate = false; + private TimedCache<Long> swConnectCache; // Storage table names protected static final String CONTROLLER_TABLE_NAME = "controller_controller"; @@ -1063,9 +1066,8 @@ public class Controller implements IFloodlightProviderService, // The switch isn't known to the controller cluster. We // need to send a switchAdded notification and clear all // flows. - // TODO: if we switch was recently (seconds) connected we - // might decide to not wipe the flow table. - sw.clearAllFlowMods(); + if (!swConnectCache.update(sw.getId())) + sw.clearAllFlowMods(); addUpdateToQueue(new SwitchUpdate(dpid, SwitchUpdateType.ADDED)); addUpdateToQueue(new SwitchUpdate(dpid, @@ -1074,8 +1076,9 @@ public class Controller implements IFloodlightProviderService, } else { // FIXME: switch was in store. check if ports or anything else // has changed and send update. - if (alwaysClearFlowsOnSwActivate) + if (alwaysClearFlowsOnSwActivate) { sw.clearAllFlowMods(); + } if (sw.attributeEquals(IOFSwitch.SWITCH_SUPPORTS_NX_ROLE, true)) { // We have a stored switch and the newly activated switch // supports roles. This indicates that the switch was @@ -1208,6 +1211,7 @@ public class Controller implements IFloodlightProviderService, return; } log.debug("removeSwitch {}", sw); + swConnectCache.update(sw.getId()); this.activeSwitches.remove(sw.getId()); removeSwitchFromStore(sw.getId()); // We cancel all outstanding statistics replies if the switch transition @@ -2308,6 +2312,8 @@ public class Controller implements IFloodlightProviderService, INITIAL_ROLE_CHANGE_DESCRIPTION); this.switchManager = new SwitchManager(this.notifiedRole); this.counters = new Counters(); + this.swConnectCache = + new TimedCache<Long>(100, 5*1000 ); // 5 seconds interval } /** diff --git a/src/test/java/net/floodlightcontroller/core/internal/ControllerTest.java b/src/test/java/net/floodlightcontroller/core/internal/ControllerTest.java index cfb828d37933a4c6c61acdc28dc255558f40bca1..14567fdb0561778317c0a1b02a8664ba04fdf429 100644 --- a/src/test/java/net/floodlightcontroller/core/internal/ControllerTest.java +++ b/src/test/java/net/floodlightcontroller/core/internal/ControllerTest.java @@ -1074,15 +1074,14 @@ public class ControllerTest extends FloodlightTestCase { /** - * Create and activate a new switch with the given dpid, features reply - * and description. If description and/or features reply are null we'll - * allocate the default one + * Create and activate a switch, either completely new or reconnected * The mocked switch instance will be returned. It wil be reset. */ - public IOFSwitch doActivateNewSwitch(long dpid, - OFDescriptionStatistics desc, - OFFeaturesReply featuresReply) - throws Exception { + private IOFSwitch doActivateSwitchInt(long dpid, + OFDescriptionStatistics desc, + OFFeaturesReply featuresReply, + boolean clearFlows) + throws Exception { controller.setAlwaysClearFlowsOnSwActivate(true); IOFSwitch sw = createMock(IOFSwitch.class); @@ -1094,8 +1093,10 @@ public class ControllerTest extends FloodlightTestCase { desc = createOFDescriptionStatistics(); } setupSwitchForAddSwitch(sw, dpid, desc, featuresReply); - sw.clearAllFlowMods(); - expectLastCall().once(); + if (clearFlows) { + sw.clearAllFlowMods(); + expectLastCall().once(); + } replay(sw); controller.switchActivated(sw); @@ -1111,6 +1112,30 @@ public class ControllerTest extends FloodlightTestCase { return sw; } + /** + * Create and activate a new switch with the given dpid, features reply + * and description. If description and/or features reply are null we'll + * allocate the default one + * The mocked switch instance will be returned. It wil be reset. + */ + private IOFSwitch doActivateNewSwitch(long dpid, + OFDescriptionStatistics desc, + OFFeaturesReply featuresReply) + throws Exception { + return doActivateSwitchInt(dpid, desc, featuresReply, true); + } + + /** + * Create and activate a switch that's just been disconnected. + * The mocked switch instance will be returned. It wil be reset. + */ + private IOFSwitch doActivateOldSwitch(long dpid, + OFDescriptionStatistics desc, + OFFeaturesReply featuresReply) + throws Exception { + return doActivateSwitchInt(dpid, desc, featuresReply, false); + } + /** * Create a switch sync representation and add it to the store and @@ -1802,7 +1827,8 @@ public class ControllerTest extends FloodlightTestCase { * Disconnect a switch. normal program flow */ @Test - public void testSwitchDisconnected() throws Exception { + private void doTestSwitchConnectReconnect(boolean reconnect) + throws Exception { IOFSwitch sw = doActivateNewSwitch(1L, null, null); expect(sw.getId()).andReturn(1L).anyTimes(); expect(sw.getStringId()).andReturn(HexString.toHexString(1L)).anyTimes(); @@ -1820,6 +1846,23 @@ public class ControllerTest extends FloodlightTestCase { assertNull(controller.getSwitch(1L)); assertNull(storeClient.getValue(1L)); + if (reconnect) { + controller.removeOFSwitchListener(listener); + sw = doActivateOldSwitch(1L, null, null); + } + } + + @Test + public void testSwitchDisconnected() throws Exception { + doTestSwitchConnectReconnect(false); + } + + /** + * Disconnect a switch and reconnect, verify no clearAllFlowmods() + */ + @Test + public void testSwitchReconnect() throws Exception { + doTestSwitchConnectReconnect(true); } /**