diff --git a/src/main/java/net/floodlightcontroller/core/RoleInfo.java b/src/main/java/net/floodlightcontroller/core/RoleInfo.java index 248cd17b694037c12e86a93aaedf7a80556fea0c..26c1a510bd556e5cedb5eb0b91f45e45ebe0e456 100644 --- a/src/main/java/net/floodlightcontroller/core/RoleInfo.java +++ b/src/main/java/net/floodlightcontroller/core/RoleInfo.java @@ -16,7 +16,9 @@ package net.floodlightcontroller.core; +import java.text.SimpleDateFormat; import java.util.Date; +import java.util.TimeZone; import net.floodlightcontroller.core.IFloodlightProviderService.Role; @@ -64,7 +66,11 @@ public class RoleInfo { } @JsonProperty(value="change-date-time") public String getRoleChangeDateTime() { - return roleChangeDateTime == null ? "" : roleChangeDateTime.toString(); + SimpleDateFormat formatter = + new SimpleDateFormat("yyyy-MM-dd'T'HH:mm:ss.SSS'Z'"); + formatter.setTimeZone(TimeZone.getTimeZone("UTC")); + return roleChangeDateTime == null ? + "" : formatter.format(roleChangeDateTime); } } \ No newline at end of file diff --git a/src/main/java/net/floodlightcontroller/core/internal/Controller.java b/src/main/java/net/floodlightcontroller/core/internal/Controller.java index 104b1d26eb3de013d52c5048c285d9f390279cc0..3fa9a16a1c45f834fed4e059e66acdd004817a3f 100644 --- a/src/main/java/net/floodlightcontroller/core/internal/Controller.java +++ b/src/main/java/net/floodlightcontroller/core/internal/Controller.java @@ -184,7 +184,7 @@ public class Controller implements IFloodlightProviderService, // The current role of the controller. // If the controller isn't configured to support roles, then this is null. protected Role role; - protected String lastRoleChangeDescription = "Inital role set during startup."; + protected String lastRoleChangeDescription = "Controller startup."; protected Date roleChangeDateTime = new Date(); // This is the role of the controller based on HARoleChange notifications // we have sent. I.e., this field reflects the last role notification @@ -1000,9 +1000,9 @@ public class Controller implements IFloodlightProviderService, if (roleChanger.checkFirstPendingRoleRequestXid( sw, error.getXid())) { roleChanger.deliverRoleRequestError(sw, error); - } else if (error.getErrorCode() == + } else if (error.getErrorType() == OFErrorType.OFPET_BAD_REQUEST.getValue() && - error.getErrorType() == + error.getErrorCode() == OFBadRequestCode.OFPBRC_EPERM.ordinal() && role.equals(Role.MASTER)) { // We are the master and the switch returned permission diff --git a/src/main/java/net/floodlightcontroller/packet/ICMP.java b/src/main/java/net/floodlightcontroller/packet/ICMP.java index 9fc99256a98ab3bfb27f2f76cf21b766be2d31a4..6b02b5bd502a5869e35a86579152417f39cd0698 100644 --- a/src/main/java/net/floodlightcontroller/packet/ICMP.java +++ b/src/main/java/net/floodlightcontroller/packet/ICMP.java @@ -18,6 +18,8 @@ package net.floodlightcontroller.packet; import java.nio.ByteBuffer; +import java.util.HashMap; +import java.util.Map; /** * Implements ICMP packet format @@ -28,8 +30,23 @@ public class ICMP extends BasePacket { protected byte icmpCode; protected short checksum; + // The value is the number of bytes of padding + public static final Map<Byte, Short> paddingMap; + public static final byte ECHO_REPLY = 0x0; public static final byte ECHO_REQUEST = 0x8; + public static final byte TIME_EXCEEDED = 0xB; + public static final byte DESTINATION_UNREACHABLE = 0x3; + + public static final byte CODE_PORT_UNREACHABLE = 0x3; + + static { + paddingMap = new HashMap<Byte, Short>(); + ICMP.paddingMap.put(ICMP.ECHO_REPLY, (short) 0); + ICMP.paddingMap.put(ICMP.ECHO_REQUEST, (short) 0); + ICMP.paddingMap.put(ICMP.TIME_EXCEEDED, (short) 4); + ICMP.paddingMap.put(ICMP.DESTINATION_UNREACHABLE, (short) 4); + } /** * @return the icmpType @@ -84,7 +101,11 @@ public class ICMP extends BasePacket { */ @Override public byte[] serialize() { - int length = 4; + short padding = 0; + if (paddingMap.containsKey(this.icmpType)) + padding = paddingMap.get(this.icmpType); + + int length = 4 + padding; byte[] payloadData = null; if (payload != null) { payload.setParent(this); @@ -98,6 +119,9 @@ public class ICMP extends BasePacket { bb.put(this.icmpType); bb.put(this.icmpCode); bb.putShort(this.checksum); + for (int i = 0; i < padding; i++) + bb.put((byte) 0); + if (payloadData != null) bb.put(payloadData); @@ -166,6 +190,12 @@ public class ICMP extends BasePacket { this.icmpCode = bb.get(); this.checksum = bb.getShort(); + short padding = 0; + if (paddingMap.containsKey(this.icmpType)) + padding = paddingMap.get(this.icmpType); + + bb.position(bb.position() + padding); + this.payload = new Data(); this.payload = payload.deserialize(data, bb.position(), bb.limit()-bb.position()); this.payload.setParent(this); diff --git a/src/main/java/net/floodlightcontroller/storage/AbstractStorageSource.java b/src/main/java/net/floodlightcontroller/storage/AbstractStorageSource.java index aae3962073e8b0aa6708b69d98e582bbd3cf5ae0..3033e99e4fd44eb799045b38f7b947ceddcf3c7b 100644 --- a/src/main/java/net/floodlightcontroller/storage/AbstractStorageSource.java +++ b/src/main/java/net/floodlightcontroller/storage/AbstractStorageSource.java @@ -46,6 +46,7 @@ import net.floodlightcontroller.storage.web.StorageWebRoutable; import org.slf4j.Logger; import org.slf4j.LoggerFactory; + @LogMessageCategory("System Database") public abstract class AbstractStorageSource implements IStorageSourceService, IFloodlightModule { @@ -460,6 +461,9 @@ public abstract class AbstractStorageSource " storage listeners", recommendation=LogMessageDoc.GENERIC_ACTION) protected synchronized void notifyListeners(StorageSourceNotification notification) { + if (logger.isTraceEnabled()) { + logger.trace("Notifying storage listeneres: {}", notification); + } String tableName = notification.getTableName(); Set<Object> keys = notification.getKeys(); Set<IStorageSourceListener> tableListeners = listeners.get(tableName); diff --git a/src/main/java/net/floodlightcontroller/topology/TopologyInstance.java b/src/main/java/net/floodlightcontroller/topology/TopologyInstance.java index de381b208f1e8de66a7695ab26f0993ab9f6544e..84052dbee2baf1136a15c981a2ca839cad80a958 100644 --- a/src/main/java/net/floodlightcontroller/topology/TopologyInstance.java +++ b/src/main/java/net/floodlightcontroller/topology/TopologyInstance.java @@ -567,8 +567,10 @@ public class TopologyInstance { } } - protected Route buildroute(RouteId id, long srcId, long dstId) { + protected Route buildroute(RouteId id) { NodePortTuple npt; + long srcId = id.getSrc(); + long dstId = id.getDst(); LinkedList<NodePortTuple> switchPorts = new LinkedList<NodePortTuple>(); @@ -668,7 +670,7 @@ public class TopologyInstance { if (pathcache.containsKey(id)) { result = pathcache.get(id); } else { - result = buildroute(id, srcId, dstId); + result = buildroute(id); pathcache.put(id, result); } if (log.isTraceEnabled()) { diff --git a/src/test/java/net/floodlightcontroller/core/internal/ControllerTest.java b/src/test/java/net/floodlightcontroller/core/internal/ControllerTest.java index eaf508592306c6b487ba14fef50ae1e0fc098f04..5daf3461bb72bfdc4119754ccd72efb7b71db79a 100644 --- a/src/test/java/net/floodlightcontroller/core/internal/ControllerTest.java +++ b/src/test/java/net/floodlightcontroller/core/internal/ControllerTest.java @@ -18,6 +18,9 @@ package net.floodlightcontroller.core.internal; import static org.easymock.EasyMock.*; +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertTrue; + import java.util.ArrayList; import java.util.Collection; import java.util.HashMap; @@ -71,6 +74,7 @@ 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.OFMessage; @@ -91,9 +95,11 @@ import org.openflow.protocol.statistics.OFDescriptionStatistics; import org.openflow.protocol.statistics.OFFlowStatisticsReply; import org.openflow.protocol.statistics.OFStatistics; import org.openflow.protocol.statistics.OFStatisticsType; +import org.openflow.protocol.vendor.OFVendorData; import org.openflow.util.HexString; import org.openflow.vendor.nicira.OFNiciraVendorData; import org.openflow.vendor.nicira.OFRoleReplyVendorData; +import org.openflow.vendor.nicira.OFRoleRequestVendorData; /** * @@ -1112,8 +1118,39 @@ public class ControllerTest extends FloodlightTestCase ld.addListener(OFType.VENDOR, ml); chdlr.processOFMessage(msg); } - - + + @Test + public void testErrorEPERM() throws Exception { + // Check behavior with a BAD_REQUEST/EPERM error + // Ensure controller attempts to reset switch role. + OFChannelState state = new OFChannelState(); + state.hsState = HandshakeState.READY; + Controller.OFChannelHandler chdlr = controller.new OFChannelHandler(state); + OFError error = new OFError(); + error.setErrorType(OFErrorType.OFPET_BAD_REQUEST); + error.setErrorCode(OFBadRequestCode.OFPBRC_EPERM); + IOFSwitch sw = createMock(IOFSwitch.class); + chdlr.sw = sw; + controller.activeSwitches.put(1L, sw); + + // prepare the switch and lock expectations + Lock lock = createNiceMock(Lock.class); + expect(sw.getListenerReadLock()).andReturn(lock).anyTimes(); + expect(sw.isConnected()).andReturn(true).anyTimes(); + expect(sw.getHARole()).andReturn(Role.MASTER).anyTimes(); + expect(sw.getId()).andReturn(1L).anyTimes(); + + // Make sure controller attempts to reset switch master + expect(sw.getAttribute("supportsNxRole")).andReturn(true).anyTimes(); + expect(sw.getNextTransactionId()).andReturn(0).anyTimes(); + + // test + replay(sw, lock); + chdlr.processOFMessage(error); + // Verify there is a pending role change request + assertTrue(controller.roleChanger.pendingTasks.poll() != null); + } + // Helper function. protected Controller.OFChannelHandler getChannelHandlerForRoleReplyTest() { OFChannelState state = new OFChannelState();