Controller: HA support for switches that have older v1.0 style serial failover

[#28337615]

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

@@ -335,6 +335,17 @@ public class Controller implements IFloodlightProviderService {
                 sw.getAttribute(IOFSwitch.SWITCH_SUPPORTS_NX_ROLE);
         if ((supportsNxRole != null) && supportsNxRole) {
             sendNxRoleRequest(sw, role);
+        } else if (supportsNxRole != null && !supportsNxRole) {
+            // We know switch does not support role-request (and so sw.role is null)
+            // but we may have just switched roles from MASTER to SLAVE in which
+            // case we should disconnect switch
+            if (getRole() == Role.SLAVE && sw.getRole() == null) {
+                log.error("Disconnecting switch {} that doesn't support " +
+                "role request messages from a controller that went to SLAVE mode");
+                // Closing the channel should result in a call to
+                // channelDisconnect which updates all state 
+                sw.getChannel().close();
+            }
         }
     }
     
@@ -588,14 +599,14 @@ public class Controller implements IFloodlightProviderService {
                          role);
                 state.nxRoleRequestXid = sendNxRoleRequest(sw, role);
             } else {
-                // The hasNxRole field is just a flag that's checked before 
-                // advancing the handshake state to READY. In this case, if role 
-                // support isn't enabled for the controller, then we're not 
-                // sending the role request probe to the switch so we don't need 
-                // to wait for a reply/error before transitioning to the READY 
-                // state.
+                // if role support isn't enabled for the controller, then we're  
+                // not sending the role request probe to the switch
                 log.info("This controllers role is null - not sending role-" +
-                		"request-msg");
+                "request-msg");
+                // The hasNxRole field is just a flag that's checked before 
+                // advancing the handshake state to READY. In this case,  
+                // we set the flag, so we don't need to wait for a 
+                // reply/error before transitioning to the READY state.
                 state.hasNxRoleReply = true;
             }
         }
@@ -607,7 +618,7 @@ public class Controller implements IFloodlightProviderService {
                 state.hsState = HandshakeState.READY;
                 
                 if (getRole() == Role.SLAVE && sw.getRole() == null) {
-                    // When the controller is currently in the slave role and 
+                    // When the controller is in the slave role and 
                     // the switch doesn't understand the role request message - 
                     // we disconnect the switch! The expected behavior is that 
                     // the switch will probably try to reconnect repeatedly 
@@ -615,8 +626,9 @@ public class Controller implements IFloodlightProviderService {
                     // while will give-up and move on to the next controller-IP 
                     // configured on the switch. This is the serial failover 
                     // mechanism from OpenFlow spec v1.0.
-                    log.error("Disconnecting switch {} that doesn't support " +
-                    		"role request messages from a slave controller");
+                    log.error("Disconnecting switch from SLAVE controller." +
+                    		" Switch {} doesn't support role request messages",
+                    		sw.getId());
                     sw.setConnected(false);
                     connectedSwitches.remove(sw.getId());
                     sw.getChannel().close();

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

@@ -19,6 +19,8 @@ package net.floodlightcontroller.core.internal;
 
 import static org.easymock.EasyMock.*;
 
+import java.net.InetSocketAddress;
+import java.net.SocketAddress;
 import java.util.ArrayList;
 import java.util.Date;
 import java.util.HashMap;
@@ -31,6 +33,7 @@ import java.util.concurrent.TimeUnit;
 import net.floodlightcontroller.core.FloodlightProvider;
 import net.floodlightcontroller.core.FloodlightContext;
 import net.floodlightcontroller.core.IFloodlightProviderService;
+import net.floodlightcontroller.core.IFloodlightProviderService.Role;
 import net.floodlightcontroller.core.IOFMessageFilterManagerService;
 import net.floodlightcontroller.core.IOFMessageListener;
 import net.floodlightcontroller.core.IOFMessageListener.Command;
@@ -45,6 +48,7 @@ import net.floodlightcontroller.packet.ARP;
 import net.floodlightcontroller.packet.Ethernet;
 import net.floodlightcontroller.packet.IPacket;
 import net.floodlightcontroller.packet.IPv4;
+import net.floodlightcontroller.packetstreamer.thrift.OFMessageType;
 import net.floodlightcontroller.perfmon.IPktInProcessingTimeService;
 import net.floodlightcontroller.perfmon.PktInProcessingTime;
 import net.floodlightcontroller.restserver.IRestApiService;
@@ -55,14 +59,21 @@ import net.floodlightcontroller.test.FloodlightTestCase;
 import net.floodlightcontroller.threadpool.IThreadPoolService;
 
 import org.jboss.netty.channel.Channel;
+import org.jboss.netty.channel.ChannelHandler;
+import org.jboss.netty.channel.ChannelStateEvent;
 import org.junit.Test;
+import org.openflow.protocol.OFError;
 import org.openflow.protocol.OFFeaturesReply;
+import org.openflow.protocol.OFMessage;
 import org.openflow.protocol.OFPacketIn;
 import org.openflow.protocol.OFPacketOut;
 import org.openflow.protocol.OFPhysicalPort;
 import org.openflow.protocol.OFPort;
 import org.openflow.protocol.OFStatisticsReply;
 import org.openflow.protocol.OFType;
+import org.openflow.protocol.OFError.OFBadActionCode;
+import org.openflow.protocol.OFError.OFBadRequestCode;
+import org.openflow.protocol.OFError.OFErrorType;
 import org.openflow.protocol.OFPacketIn.OFPacketInReason;
 import org.openflow.protocol.action.OFAction;
 import org.openflow.protocol.action.OFActionOutput;
@@ -464,7 +475,6 @@ public class ControllerTest extends FloodlightTestCase {
         expect(channel2.getRemoteAddress()).andReturn(null);
         expect(newsw.getFeaturesReply()).andReturn(new OFFeaturesReply()).anyTimes();
         expect(newsw.getPorts()).andReturn(new HashMap<Short,OFPhysicalPort>());
-
         controller.activeSwitches.put(0L, oldsw);
         replay(newsw, channel, channel2);
 
@@ -472,4 +482,67 @@ public class ControllerTest extends FloodlightTestCase {
 
         verify(newsw, channel, channel2);
     }
+    
+    
+    @Test
+    public void testRoleChangeForSerialFailoverSwitch() throws Exception {
+        IOFSwitch newsw = createMock(IOFSwitch.class);
+        expect(newsw.getId()).andReturn(0L).anyTimes();
+        expect(newsw.getStringId()).andReturn("00:00:00:00:00:00:00").anyTimes();
+        Channel channel2 = createMock(Channel.class);
+        expect(newsw.getChannel()).andReturn(channel2);
+        
+        // newsw.role is null because the switch does not support
+        // role request messages
+        expect(newsw.getRole()).andReturn(null);
+        expect(newsw.getAttribute(IOFSwitch.SWITCH_SUPPORTS_NX_ROLE))
+                        .andReturn(false);
+        // switch is connected 
+        controller.connectedSwitches.put(0L, newsw);
+
+        // the switch should get disconnected when role is changed to SLAVE
+        expect(channel2.close()).andReturn(null);
+
+        replay(newsw, channel2);
+        controller.setRole(Role.SLAVE);
+        verify(newsw,  channel2);
+    }
+
+    @Test
+    public void testSlaveRoleHandshakeForSerialFailoverSwitch() 
+            throws Exception {
+        controller.role = Role.SLAVE;
+        OFFeaturesReply featuresReply = new OFFeaturesReply();
+        featuresReply.setDatapathId(0L);
+        featuresReply.setPorts(new ArrayList<OFPhysicalPort>());
+        OFChannelState state = new OFChannelState();
+        state.hsState = OFChannelState.HandshakeState.FEATURES_REPLY;
+        state.hasDescription = true;
+        state.hasGetConfigReply = true;
+        Controller.OFChannelHandler chdlr = controller.new OFChannelHandler(state);
+
+        Channel ch = createMock(Channel.class);
+        ChannelStateEvent e = createMock(ChannelStateEvent.class);
+        expect(e.getChannel()).andReturn(ch).anyTimes();
+        SocketAddress sa = new InetSocketAddress(45454);
+        expect(ch.getRemoteAddress()).andReturn(sa);
+        expect(ch.write(anyObject())).andReturn(null);
+
+        // the error returned when role request message is not supported by sw
+        OFMessage msg = new OFError();
+        msg.setType(OFType.ERROR);
+        ((OFError) msg).setErrorType(OFErrorType.OFPET_BAD_REQUEST);
+        ((OFError) msg).setErrorCode(OFBadRequestCode.OFPBRC_BAD_VENDOR);
+        
+        // the switch connection should get disconnected when the controller is
+        // in SLAVE mode and the switch does not support role-request messages
+        expect(ch.close()).andReturn(null);
+        
+        replay(ch, e);
+        chdlr.channelConnected(null, e);
+        chdlr.sw.setFeaturesReply(featuresReply);
+        chdlr.processOFMessage(msg);
+        verify(ch, e);
+        
+    }
 }