From 22c60b611be29d74be8757d1e1c7ca02c357e480 Mon Sep 17 00:00:00 2001
From: Shudong Zhou <shudongzhou@gmail.com>
Date: Sat, 17 Nov 2012 11:27:27 -0800
Subject: [PATCH] Fixes to HA role request handling (Gregor's comments)
---
.../core/annotations/LogMessageDoc.java | 4 +
.../core/internal/Controller.java | 77 +------
.../core/internal/RoleChanger.java | 214 +++++++++++-------
.../core/internal/ControllerTest.java | 21 +-
.../core/internal/RoleChangerTest.java | 38 +++-
5 files changed, 169 insertions(+), 185 deletions(-)
diff --git a/src/main/java/net/floodlightcontroller/core/annotations/LogMessageDoc.java b/src/main/java/net/floodlightcontroller/core/annotations/LogMessageDoc.java
index c703687a1..08b48dc84 100644
--- a/src/main/java/net/floodlightcontroller/core/annotations/LogMessageDoc.java
+++ b/src/main/java/net/floodlightcontroller/core/annotations/LogMessageDoc.java
@@ -35,6 +35,10 @@ public @interface LogMessageDoc {
public static final String CHECK_SWITCH =
"Check the health of the indicated switch. " +
"Test and troubleshoot IP connectivity.";
+ public static final String HA_CHECK_SWITCH =
+ "Check the health of the indicated switch. If the problem " +
+ "persists or occurs repeatedly, it likely indicates a defect " +
+ "in the switch HA implementation.";
public static final String CHECK_CONTROLLER =
"Verify controller system health, CPU usage, and memory. " +
"Rebooting the controller node may help if the controller " +
diff --git a/src/main/java/net/floodlightcontroller/core/internal/Controller.java b/src/main/java/net/floodlightcontroller/core/internal/Controller.java
index dda0432dd..b7b048874 100644
--- a/src/main/java/net/floodlightcontroller/core/internal/Controller.java
+++ b/src/main/java/net/floodlightcontroller/core/internal/Controller.java
@@ -449,6 +449,7 @@ public class Controller implements IFloodlightProviderService,
}
synchronized(roleChanger) {
connectedSwitches.remove(sw);
+ roleChanger.removePendingRequests(sw);
}
sw.setConnected(false);
}
@@ -917,9 +918,6 @@ public class Controller implements IFloodlightProviderService,
sendFeatureReplyConfiguration();
state.featuresReply = (OFFeaturesReply) m;
state.hsState = HandshakeState.FEATURES_REPLY;
- // uncomment to enable "dumb" switches like cbench
- // state.hsState = HandshakeState.READY;
- // addSwitch(sw);
} else {
// return results to rest api caller
sw.setFeaturesReply((OFFeaturesReply) m);
@@ -956,81 +954,14 @@ public class Controller implements IFloodlightProviderService,
// This will probable involve rewriting the way we handle
// request/reply style messages.
OFError error = (OFError) m;
- boolean shouldLogError = true;
- // TODO: should we check that firstRoleReplyReceived is false,
- // i.e., check only whether the first request fails?
if (roleChanger.checkFirstPendingRoleRequestXid(
sw, error.getXid())) {
- boolean isBadVendorError =
- (error.getErrorType() == OFError.OFErrorType.
- OFPET_BAD_REQUEST.getValue());
- // We expect to receive a bad vendor error when
- // we're connected to a switch that doesn't support
- // the Nicira vendor extensions (i.e. not OVS or
- // derived from OVS). By protocol, it should also be
- // BAD_VENDOR, but too many switch implementations
- // get it wrong and we can already check the xid()
- // so we can ignore the type with confidence that this
- // is not a spurious error
- shouldLogError = !isBadVendorError;
- if (isBadVendorError) {
- if (sw.getHARole() != null) {
- log.warn("Received ERROR from sw {} that "
- +"indicates roles are not supported "
- +"but we have received a valid "
- +"role reply earlier", sw);
- }
-
- roleChanger.deliverRoleRequestNotSupported(sw, error.getXid());
- synchronized(roleChanger) {
- if (Controller.this.role==Role.SLAVE) {
- // the switch doesn't understand role request
- // messages and the current controller role is
- // slave. We need to disconnect the switch.
- // @see RoleChanger for rationale
- sw.disconnectOutputStream();
- }
- else {
- // Controller's role is master: add to
- // active
- // TODO: check if clearing flow table is
- // right choice here.
- // Need to clear FlowMods before we add the switch
- // and dispatch updates otherwise we have a race condition.
- // TODO: switch update is async. Won't we still have a potential
- // race condition?
- addSwitch(sw, true);
- }
- }
- }
- else {
- // TODO: Is this the right thing to do if we receive
- // some other error besides a bad vendor error?
- // Presumably that means the switch did actually
- // understand the role request message, but there
- // was some other error from processing the message.
- // OF 1.2 specifies a OFPET_ROLE_REQUEST_FAILED
- // error code, but it doesn't look like the Nicira
- // role request has that. Should check OVS source
- // code to see if it's possible for any other errors
- // to be returned.
- // If we received an error the switch is not
- // in the correct role, so we need to disconnect it.
- // We could also resend the request but then we need to
- // check if there are other pending request in which
- // case we shouldn't resend. If we do resend we need
- // to make sure that the switch eventually accepts one
- // of our requests or disconnect the switch. This feels
- // cumbersome.
- sw.disconnectOutputStream();
- }
+ roleChanger.deliverRoleRequestError(sw, error);
}
- // Once we support OF 1.2, we'd add code to handle it here.
- //if (error.getXid() == state.ofRoleRequestXid) {
- //}
- if (shouldLogError)
+ else {
logError(sw, error);
+ }
break;
case STATS_REPLY:
if (state.hsState.ordinal() <
diff --git a/src/main/java/net/floodlightcontroller/core/internal/RoleChanger.java b/src/main/java/net/floodlightcontroller/core/internal/RoleChanger.java
index 16b302f1c..f9ec793b3 100644
--- a/src/main/java/net/floodlightcontroller/core/internal/RoleChanger.java
+++ b/src/main/java/net/floodlightcontroller/core/internal/RoleChanger.java
@@ -11,10 +11,10 @@ import java.util.concurrent.DelayQueue;
import java.util.concurrent.Delayed;
import java.util.concurrent.TimeUnit;
+import org.openflow.protocol.OFError;
import org.openflow.protocol.OFMessage;
import org.openflow.protocol.OFType;
import org.openflow.protocol.OFVendor;
-import org.openflow.util.HexString;
import org.openflow.vendor.nicira.OFNiciraVendorData;
import org.openflow.vendor.nicira.OFRoleReplyVendorData;
import org.openflow.vendor.nicira.OFRoleRequestVendorData;
@@ -80,22 +80,19 @@ import net.floodlightcontroller.core.annotations.LogMessageDocs;
* OFSwitchImpl's queue of pending requests)
* - We handle requests and timeouts in the same thread. We use a priority queue
* to schedule them so we are guaranteed that they are processed in
- * the same order as they are submitted. If a request times out we drop
- * the connection to this switch.
+ * the same order as they are submitted. If a request times out we assume
+ * the switch doesn't support HA role (the same as receiving an error reply).
* - Since we decouple submission of role change requests and actually sending
* them we cannot check a received role reply against the controller's current
* role because the controller's current role could have changed again.
- * - Receiving Role Reply messages is handled by OFChannelHandler and
- * OFSwitchImpl directly. The OFSwitchImpl checks if the received request
- * is as expected (xid and role match the head of the pending queue in
- * OFSwitchImpl). If so
- * the switch updates its role. Otherwise the connection is dropped. If this
- * is the first reply, the SWITCH_SUPPORTS_NX_ROLE attribute is set.
- * Next, we call addSwitch(), removeSwitch() to update the list of active
- * switches if appropriate.
+ * - Receiving Role Reply messages is received by OFChannelHandler and
+ * delivered here. We call switch's setHARole() to mark the switch role and
+ * indicate that a reply was received. Next, we call addSwitch(),
+ * removeSwitch() to update the list of active switches if appropriate.
* - If we receive an Error indicating that roles are not supported by the
- * switch, we set the SWITCH_SUPPORTS_NX_ROLE to false. We keep the
- * switch connection alive while in MASTER and EQUAL role.
+ * switch, we set the SWITCH_SUPPORTS_NX_ROLE to false. We call switch's
+ * setHARole(), indicating no reply was received. We keep the switch
+ * connection alive while in MASTER and EQUAL role.
* (TODO: is this the right behavior for EQUAL??). If the role changes to
* SLAVE the switch connection is dropped (remember: only if the switch
* doesn't support role requests)
@@ -107,13 +104,10 @@ import net.floodlightcontroller.core.annotations.LogMessageDocs;
* New switch connection:
* - Switch handshake is done without sending any role request messages.
* - After handshake completes, switch is added to the list of connected switches
- * and we send the first role request message if role
- * requests are enabled. If roles are disabled automatically promote switch to
- * active switch list and clear FlowTable.
+ * and we send the first role request message. If role is disabled, we assume
+ * the role is MASTER.
* - When we receive the first reply we proceed as above. In addition, if
- * the role request is for MASTER we wipe the flow table. We do not wipe
- * the flow table if the switch connected while role supported was disabled
- * on the controller.
+ * the role request is for MASTER we wipe the flow table.
*
*/
public class RoleChanger {
@@ -138,10 +132,6 @@ public class RoleChanger {
protected static long DEFAULT_TIMEOUT = 5L*1000*1000*1000L; // 5s
protected static Logger log = LoggerFactory.getLogger(RoleChanger.class);
- private static final String HA_CHECK_SWITCH =
- "Check the health of the indicated switch. If the problem " +
- "persists or occurs repeatedly, it likely indicates a defect " +
- "in the switch HA implementation.";
/**
* A queued task to be handled by the Role changer thread.
@@ -352,12 +342,7 @@ public class RoleChanger {
log.warn("Timeout while waiting for role reply from switch {}"
+ " with datapath {}", sw,
sw.getAttribute(IOFSwitch.SWITCH_DESCRIPTION_DATA));
- sw.setHARole(entry.role, false);
- if (entry.role == Role.SLAVE) {
- sw.disconnectOutputStream();
- } else {
- controller.addSwitch(sw, true);
- }
+ setSwitchHARole(sw, entry.role, false);
}
}
}
@@ -369,7 +354,7 @@ public class RoleChanger {
* our expectations we disconnect the switch.
*
* We must not check the received role against the controller's current
- * role because there's no synchronization but that's fine @see RoleChanger
+ * role because there's no synchronization but that's fine.
*
* Will be called by the OFChannelHandler's receive loop
*
@@ -382,24 +367,24 @@ public class RoleChanger {
"Role {role}" +
" Disconnecting switch",
explanation="The switch sent an unexpected HA role reply",
- recommendation=HA_CHECK_SWITCH),
+ recommendation=LogMessageDoc.HA_CHECK_SWITCH),
@LogMessageDoc(level="ERROR",
message="Switch {switch}: expected role reply with " +
"Xid {xid}, got {xid}. Disconnecting switch",
explanation="The switch sent an unexpected HA role reply",
- recommendation=HA_CHECK_SWITCH),
+ recommendation=LogMessageDoc.HA_CHECK_SWITCH),
@LogMessageDoc(level="ERROR",
message="Switch {switch}: expected role reply with " +
"Role {role}, got {role}. Disconnecting switch",
explanation="The switch sent an unexpected HA role reply",
- recommendation=HA_CHECK_SWITCH)
+ recommendation=LogMessageDoc.HA_CHECK_SWITCH)
})
protected void deliverRoleReply(IOFSwitch sw, int xid, Role role) {
LinkedList<PendingRoleRequestEntry> pendingRoleRequests =
pendingRequestMap.get(sw);
if (pendingRoleRequests == null) {
- log.warn("Switch {}: received unexpected role reply for Role {}" +
+ log.debug("Switch {}: received unexpected role reply for Role {}" +
", ignored", sw, role );
return;
}
@@ -428,7 +413,7 @@ public class RoleChanger {
else {
log.debug("Received role reply message from {}, setting role to {}",
this, role);
- sw.setHARole(role, true);
+ setSwitchHARole(sw, role, true);
}
}
}
@@ -486,7 +471,7 @@ public class RoleChanger {
* Otherwise we ignore it.
* @param xid
*/
- public void deliverRoleRequestNotSupported(IOFSwitch sw, int xid) {
+ private void deliverRoleRequestNotSupported(IOFSwitch sw, int xid) {
LinkedList<PendingRoleRequestEntry> pendingRoleRequests =
pendingRequestMap.get(sw);
if (pendingRoleRequests == null) {
@@ -497,9 +482,7 @@ public class RoleChanger {
synchronized(pendingRoleRequests) {
PendingRoleRequestEntry head = pendingRoleRequests.poll();
if (head != null && head.xid == xid) {
- sw.setHARole(head.role, false);
- } else {
- sw.disconnectOutputStream();
+ setSwitchHARole(sw, head.role, false);
}
}
}
@@ -622,56 +605,119 @@ public class RoleChanger {
new Object[] { role, sw, controller.role}
);
- Role oldRole = sw.getHARole();
deliverRoleReply(sw, vendorMessage.getXid(), role);
+ }
+
+ @LogMessageDocs({
+ @LogMessageDoc(level="ERROR",
+ message="Received ERROR from sw {switch} that indicates roles " +
+ "are not supported but we have received a valid role " +
+ "reply earlier",
+ explanation="Switch is responding to HA role request in an " +
+ "inconsistent manner.",
+ recommendation=LogMessageDoc.CHECK_SWITCH),
+ @LogMessageDoc(level="ERROR",
+ message="Unexpected error {error} from switch {switch} " +
+ "disconnecting",
+ explanation="Swith sent an error reply to request, but the error " +
+ "type is not OPET_BAD_REQUEST as required by the protocol",
+ recommendation=LogMessageDoc.CHECK_SWITCH)
+ })
+ protected void deliverRoleRequestError(IOFSwitch sw, OFError error) {
+ // We expect to receive a bad request error when
+ // we're connected to a switch that doesn't support
+ // the Nicira vendor extensions (i.e. not OVS or
+ // derived from OVS). By protocol, it should also be
+ // BAD_VENDOR, but too many switch implementations
+ // get it wrong and we can already check the xid()
+ // so we can ignore the type with confidence that this
+ // is not a spurious error
+ boolean isBadRequestError =
+ (error.getErrorType() == OFError.OFErrorType.
+ OFPET_BAD_REQUEST.getValue());
+ if (isBadRequestError) {
+ if (sw.getHARole() != null) {
+ log.warn("Received ERROR from sw {} that "
+ +"indicates roles are not supported "
+ +"but we have received a valid "
+ +"role reply earlier", sw);
+ }
+ // First, clean up pending requests
+ deliverRoleRequestNotSupported(sw, error.getXid());
+ } else {
+ // TODO: Is this the right thing to do if we receive
+ // some other error besides a bad request error?
+ // Presumably that means the switch did actually
+ // understand the role request message, but there
+ // was some other error from processing the message.
+ // OF 1.2 specifies a OFPET_ROLE_REQUEST_FAILED
+ // error code, but it doesn't look like the Nicira
+ // role request has that. Should check OVS source
+ // code to see if it's possible for any other errors
+ // to be returned.
+ // If we received an error the switch is not
+ // in the correct role, so we need to disconnect it.
+ // We could also resend the request but then we need to
+ // check if there are other pending request in which
+ // case we shouldn't resend. If we do resend we need
+ // to make sure that the switch eventually accepts one
+ // of our requests or disconnect the switch. This feels
+ // cumbersome.
+ log.error("Unexpected error {} from switch {}, disconnecting",
+ error, sw);
+ sw.disconnectOutputStream();
+ }
+ }
+
+ /**
+ * Set switch's HA role and adjust switch's list membership
+ * based on the new role and switch's HA capability. This is
+ * a synchronized method to keep role handling consistent.
+ * @param sw
+ * @param role
+ * @param replyReceived
+ */
+ private synchronized void setSwitchHARole(IOFSwitch sw, Role role,
+ boolean replyReceived) {
+ // Record previous role and set the current role
+ // We tell switch whether a correct reply was received.
+ // The switch may deduce if HA role request is supported
+ // (if it doesn't already know).
+ Role oldRole = sw.getHARole();
+ sw.setHARole(role, replyReceived);
- if (sw.getHARole() != Role.SLAVE) {
- // Transition from SLAVE to MASTER.
- boolean shouldClearFlowMods =
+ // Adjust the active switch list based on the role
+ if (role != Role.SLAVE) {
+ // SLAVE->MASTER, need to add switch to active list.
+ // If this is the initial connection, clear flodmods.
+ boolean clearFlowMods = (oldRole == null) ||
controller.getAlwaysClearFlowsOnSwAdd();
- if (oldRole == null) {
- // This is the first role-reply message we receive from
- // this switch. Delete all pre-existing flows for new
- // connections to the master.
- //
- // FIXME: Need to think more about what the test should
- // be for when we flush the flow-table? For example,
- // if all the controllers are temporarily in the backup
- // role (e.g. right after a failure of the master
- // controller) at the point the switch connects, then
- // all of the controllers will initially connect as
- // backup controllers and not flush the flow-table.
- // Then when one of them is promoted to master following
- // the master controller election the flow-table
- // will still not be flushed because that's treated as
- // a failover event where we don't want to flush the
- // flow-table. The end result would be that the flow
- // table for a newly connected switch is never
- // flushed. Not sure how to handle that case though...
- shouldClearFlowMods = true;
- log.debug("First role reply from master switch {}, " +
- "clear FlowTable to active switch list",
- HexString.toHexString(sw.getId()));
+ controller.addSwitch(sw, clearFlowMods);
+ } else {
+ // Initial SLAVE setting. Nothing to do if role reply received.
+ // Else disconnect the switch.
+ if (oldRole == null && !replyReceived) {
+ sw.disconnectOutputStream();
+ } else {
+ // MASTER->SLAVE transition. If switch supports HA roles,
+ // remove it from active list, but still leave it connected.
+ // Otherwise, disconnect it. Cleanup happens after channel
+ // handler receives the disconnect notification.
+ if (replyReceived) {
+ controller.removeSwitch(sw);
+ } else {
+ sw.disconnectOutputStream();
+ }
}
-
- // Only add the switch to the active switch list if
- // we're not in the slave role. Note that if the role
- // attribute is null, then that means that the switch
- // doesn't support the role request messages, so in that
- // case we're effectively in the EQUAL role and the
- // switch should be included in the active switch list.
- controller.addSwitch(sw, shouldClearFlowMods);
- log.debug("Added master switch {} to active switch list",
- HexString.toHexString(sw.getId()));
- }
- else {
- // Transition from MASTER to SLAVE: remove switch
- // from active switch list.
- log.debug("Removed slave switch {} from active switch" +
- " list", HexString.toHexString(sw.getId()));
- controller.removeSwitch(sw);
}
}
-
+ /**
+ * Cleanup pending requests associated witch switch. Called when
+ * switch disconnects.
+ * @param sw
+ */
+ public void removePendingRequests(IOFSwitch sw) {
+ pendingRequestMap.remove(sw);
+ }
}
diff --git a/src/test/java/net/floodlightcontroller/core/internal/ControllerTest.java b/src/test/java/net/floodlightcontroller/core/internal/ControllerTest.java
index 5071fff1e..bb9d90ce0 100644
--- a/src/test/java/net/floodlightcontroller/core/internal/ControllerTest.java
+++ b/src/test/java/net/floodlightcontroller/core/internal/ControllerTest.java
@@ -70,7 +70,6 @@ import org.easymock.EasyMock;
import org.jboss.netty.channel.Channel;
import org.junit.Test;
import org.openflow.protocol.OFError;
-import org.openflow.protocol.OFError.OFBadRequestCode;
import org.openflow.protocol.OFError.OFErrorType;
import org.openflow.protocol.OFFeaturesReply;
import org.openflow.protocol.OFPacketIn;
@@ -1029,7 +1028,6 @@ public class ControllerTest extends FloodlightTestCase
msg.setType(OFType.ERROR);
msg.setXid(xid);
msg.setErrorType(OFErrorType.OFPET_BAD_REQUEST);
- 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
@@ -1037,6 +1035,7 @@ public class ControllerTest extends FloodlightTestCase
setupPendingRoleRequest(chdlr.sw, xid, controller.role, 123456);
expect(chdlr.sw.getHARole()).andReturn(null);
chdlr.sw.setHARole(Role.SLAVE, false);
+ expect(chdlr.sw.getHARole()).andReturn(Role.SLAVE);
chdlr.sw.disconnectOutputStream();
replay(ch, chdlr.sw);
@@ -1046,24 +1045,6 @@ public class ControllerTest extends FloodlightTestCase
controller.activeSwitches.isEmpty());
reset(ch, chdlr.sw);
-
- // a different error message - should also reject role request
- msg.setErrorType(OFErrorType.OFPET_BAD_REQUEST);
- msg.setErrorCode(OFBadRequestCode.OFPBRC_EPERM);
- controller.role = Role.SLAVE;
- setupPendingRoleRequest(chdlr.sw, xid, controller.role, 123456);
- expect(chdlr.sw.getHARole()).andReturn(null);
- chdlr.sw.setHARole(Role.SLAVE, false);
- chdlr.sw.disconnectOutputStream();
- replay(ch, chdlr.sw);
-
- chdlr.processOFMessage(msg);
- verify(ch, chdlr.sw);
- assertTrue("activeSwitches must be empty",
- controller.activeSwitches.isEmpty());
- reset(ch, chdlr.sw);
-
-
// We are MASTER, the switch should be added to the list of active
// switches.
controller.role = Role.MASTER;
diff --git a/src/test/java/net/floodlightcontroller/core/internal/RoleChangerTest.java b/src/test/java/net/floodlightcontroller/core/internal/RoleChangerTest.java
index 507768959..cb446b2a9 100644
--- a/src/test/java/net/floodlightcontroller/core/internal/RoleChangerTest.java
+++ b/src/test/java/net/floodlightcontroller/core/internal/RoleChangerTest.java
@@ -27,6 +27,8 @@ import org.easymock.EasyMock;
import org.jboss.netty.channel.Channel;
import org.junit.Before;
import org.junit.Test;
+import org.openflow.protocol.OFError;
+import org.openflow.protocol.OFError.OFErrorType;
import org.openflow.protocol.OFMessage;
import org.openflow.protocol.OFType;
import org.openflow.protocol.OFVendor;
@@ -182,6 +184,7 @@ public class RoleChangerTest {
// Timed out switch should become active
expect(sw2.getAttribute(IOFSwitch.SWITCH_DESCRIPTION_DATA))
.andReturn(null);
+ expect(sw2.getHARole()).andReturn(null);
sw2.setHARole(Role.MASTER, false);
EasyMock.expectLastCall();
switches.add(sw2);
@@ -239,9 +242,11 @@ public class RoleChangerTest {
(FloodlightContext)EasyMock.anyObject());
expect(sw1.getAttribute(IOFSwitch.SWITCH_DESCRIPTION_DATA))
.andReturn(null);
+ expect(sw1.getHARole()).andReturn(null);
sw1.setHARole(Role.MASTER, false);
expect(sw1.getAttribute(IOFSwitch.SWITCH_DESCRIPTION_DATA))
.andReturn(null);
+ expect(sw1.getHARole()).andReturn(Role.MASTER);
sw1.setHARole(Role.SLAVE, false);
// Disconnect on timing out SLAVE request
sw1.disconnectOutputStream();
@@ -402,33 +407,47 @@ public class RoleChangerTest {
}
@Test
- public void testDeliverRoleRequestNotSupported () {
+ public void testDeliverRoleRequestError() {
// normal case. xid is pending
int xid = (int) System.currentTimeMillis();
long cookie = System.nanoTime();
Role role = Role.MASTER;
OFSwitchImpl sw = new OFSwitchImpl();
+ Channel ch = createMock(Channel.class);
+ SocketAddress sa = new InetSocketAddress(42);
+ expect(ch.getRemoteAddress()).andReturn(sa).anyTimes();
+ sw.setChannel(ch);
setupPendingRoleRequest(sw, xid, role, cookie);
- roleChanger.deliverRoleRequestNotSupported(sw, xid);
+ OFError error = new OFError();
+ error.setErrorType(OFErrorType.OFPET_BAD_REQUEST);
+ error.setXid(xid);
+ replay(ch);
+ roleChanger.deliverRoleRequestError(sw, error);
+ verify(ch);
assertEquals(false, sw.getAttribute(IOFSwitch.SWITCH_SUPPORTS_NX_ROLE));
assertEquals(role, sw.getHARole());
assertEquals(0, roleChanger.pendingRequestMap.get(sw).size());
}
@Test
- public void testDeliverRoleRequestNotSupportedNonePending() {
+ public void testDeliverRoleRequestErrorNonePending() {
// nothing pending
OFSwitchImpl sw = new OFSwitchImpl();
Channel ch = createMock(Channel.class);
SocketAddress sa = new InetSocketAddress(42);
expect(ch.getRemoteAddress()).andReturn(sa).anyTimes();
sw.setChannel(ch);
- roleChanger.deliverRoleRequestNotSupported(sw, 1);
+ OFError error = new OFError();
+ error.setErrorType(OFErrorType.OFPET_BAD_REQUEST);
+ error.setXid(1);
+ replay(ch);
+ roleChanger.deliverRoleRequestError(sw, error);
+ verify(ch);
assertEquals(null, sw.getHARole());
}
@Test
- public void testDeliverRoleRequestNotSupportedWrongXid() {
+ public void testDeliverRoleRequestErrorWrongXid() {
// wrong xid received
// wrong xid received
int xid = (int) System.currentTimeMillis();
@@ -439,13 +458,16 @@ public class RoleChangerTest {
Channel ch = createMock(Channel.class);
SocketAddress sa = new InetSocketAddress(42);
expect(ch.getRemoteAddress()).andReturn(sa).anyTimes();
- sw.setChannel(ch);
expect(ch.close()).andReturn(null);
+ sw.setChannel(ch);
replay(ch);
- roleChanger.deliverRoleRequestNotSupported(sw, xid+1);
+ OFError error = new OFError();
+ error.setErrorCode(OFError.OFErrorType.OFPET_BAD_REQUEST.getValue());
+ error.setXid(xid + 1);
+ roleChanger.deliverRoleRequestError(sw, error);
verify(ch);
assertEquals(null, sw.getAttribute(IOFSwitch.SWITCH_SUPPORTS_NX_ROLE));
- assertEquals(0, roleChanger.pendingRequestMap.get(sw).size());
+ assertEquals(1, roleChanger.pendingRequestMap.get(sw).size());
}
public void doSendNxRoleRequest(Role role, int nx_role) throws Exception {
--
GitLab