From 2ab3d47391cd645e105cd9a6fcf25bdf8f608339 Mon Sep 17 00:00:00 2001
From: Ryan Izard <rizard@g.clemson.edu>
Date: Wed, 27 Aug 2014 12:51:48 -0400
Subject: [PATCH] Working on unit tests. Found bugs in TCP and UDP. ByteBuffer
returns a (signed) short for ports, but they are valid 0-65535, not
-32768-32767. Convert possibly negative ports from BB to positive ints using
a bitmask.
---
.../web/serializers/IOFSwitchSerializer.java | 6 +++++-
.../net/floodlightcontroller/packet/IPv4.java | 9 ++++++---
.../net/floodlightcontroller/packet/TCP.java | 10 +++++-----
.../net/floodlightcontroller/packet/UDP.java | 16 ++++++++--------
.../core/internal/ControllerTest.java | 7 -------
.../core/internal/OFSwitchManagerTest.java | 4 ----
.../floodlightcontroller/packet/DHCPTest.java | 4 ++--
.../net/floodlightcontroller/packet/TCPTest.java | 4 ++--
8 files changed, 28 insertions(+), 32 deletions(-)
diff --git a/src/main/java/net/floodlightcontroller/core/web/serializers/IOFSwitchSerializer.java b/src/main/java/net/floodlightcontroller/core/web/serializers/IOFSwitchSerializer.java
index aa8d6936f..753685a35 100644
--- a/src/main/java/net/floodlightcontroller/core/web/serializers/IOFSwitchSerializer.java
+++ b/src/main/java/net/floodlightcontroller/core/web/serializers/IOFSwitchSerializer.java
@@ -22,10 +22,12 @@ import java.util.Set;
import java.util.Map;
import java.util.Collection;
import java.util.EnumSet;
+
import com.fasterxml.jackson.core.JsonGenerator;
import com.fasterxml.jackson.core.JsonProcessingException;
import com.fasterxml.jackson.databind.JsonSerializer;
import com.fasterxml.jackson.databind.SerializerProvider;
+
import org.projectfloodlight.openflow.protocol.OFCapabilities;
import org.projectfloodlight.openflow.protocol.OFPortDesc;
import org.projectfloodlight.openflow.protocol.OFPortConfig;
@@ -35,6 +37,7 @@ import org.projectfloodlight.openflow.protocol.OFActionType;
import org.projectfloodlight.openflow.protocol.OFControllerRole;
import org.projectfloodlight.openflow.protocol.OFFlowWildcards;
import org.projectfloodlight.openflow.protocol.OFVersion;
+
import net.floodlightcontroller.core.HARole;
import net.floodlightcontroller.core.IOFSwitch;
import net.floodlightcontroller.core.SwitchDescription;
@@ -76,7 +79,8 @@ public class IOFSwitchSerializer extends JsonSerializer<IOFSwitch> {
}
}
- public void serializeAttributes(Map<Object, Object> attributes, JsonGenerator jGen)
+ @SuppressWarnings("unchecked")
+ public void serializeAttributes(Map<Object, Object> attributes, JsonGenerator jGen)
throws IOException, JsonProcessingException {
if ( null == attributes)
jGen.writeStringField("attributes","null");
diff --git a/src/main/java/net/floodlightcontroller/packet/IPv4.java b/src/main/java/net/floodlightcontroller/packet/IPv4.java
index 4f8efb07a..54effbc64 100644
--- a/src/main/java/net/floodlightcontroller/packet/IPv4.java
+++ b/src/main/java/net/floodlightcontroller/packet/IPv4.java
@@ -78,6 +78,9 @@ public class IPv4 extends BasePacket {
this.version = 4;
isTruncated = false;
isFragment = false;
+ protocol = IpProtocol.NONE;
+ sourceAddress = IPv4Address.NONE;
+ destinationAddress = IPv4Address.NONE;
}
/**
@@ -348,7 +351,7 @@ public class IPv4 extends BasePacket {
bb.putShort((short)(((this.flags & IPV4_FLAGS_MASK) << IPV4_FLAGS_SHIFT)
| (this.fragmentOffset & IPV4_OFFSET_MASK)));
bb.put(this.ttl);
- bb.putShort(this.protocol.getIpProtocolNumber());
+ bb.put((byte)this.protocol.getIpProtocolNumber());
bb.putShort(this.checksum);
bb.putInt(this.sourceAddress.getInt());
bb.putInt(this.destinationAddress.getInt());
@@ -408,8 +411,8 @@ public class IPv4 extends BasePacket {
isFragment = ((this.flags & IPV4_FLAGS_DONTFRAG) == 0) &&
((this.flags & IPV4_FLAGS_MOREFRAG) != 0 ||
this.fragmentOffset != 0);
- if (!isFragment && IPv4.protocolClassMap.containsKey(this.protocol)) {
- Class<? extends IPacket> clazz = IPv4.protocolClassMap.get(this.protocol);
+ if (!isFragment && IPv4.protocolClassMap.containsKey((byte)this.protocol.getIpProtocolNumber())) {
+ Class<? extends IPacket> clazz = IPv4.protocolClassMap.get((byte)this.protocol.getIpProtocolNumber());
try {
payload = clazz.newInstance();
} catch (Exception e) {
diff --git a/src/main/java/net/floodlightcontroller/packet/TCP.java b/src/main/java/net/floodlightcontroller/packet/TCP.java
index f4b9a5026..3b794a4ba 100644
--- a/src/main/java/net/floodlightcontroller/packet/TCP.java
+++ b/src/main/java/net/floodlightcontroller/packet/TCP.java
@@ -80,7 +80,7 @@ public class TCP extends BasePacket {
/**
* @param destinationPort the destinationPort to set
*/
- public TCP setDestinationPort(short destinationPort) {
+ public TCP setDestinationPort(int destinationPort) {
this.destinationPort = TransportPort.of(destinationPort);
return this;
}
@@ -265,8 +265,8 @@ public class TCP extends BasePacket {
TCP other = (TCP) obj;
// May want to compare fields based on the flags set
return (checksum == other.checksum) &&
- (destinationPort == other.destinationPort) &&
- (sourcePort == other.sourcePort) &&
+ (destinationPort.equals(other.destinationPort)) &&
+ (sourcePort.equals(other.sourcePort)) &&
(sequence == other.sequence) &&
(acknowledge == other.acknowledge) &&
(dataOffset == other.dataOffset) &&
@@ -280,8 +280,8 @@ public class TCP extends BasePacket {
public IPacket deserialize(byte[] data, int offset, int length)
throws PacketParsingException {
ByteBuffer bb = ByteBuffer.wrap(data, offset, length);
- this.sourcePort = TransportPort.of(bb.getShort());
- this.destinationPort = TransportPort.of(bb.getShort());
+ this.sourcePort = TransportPort.of((int) (bb.getShort() & 0xffff)); // short will be signed, pos or neg
+ this.destinationPort = TransportPort.of((int) (bb.getShort() & 0xffff)); // convert range 0 to 65534, not -32768 to 32767
this.sequence = bb.getInt();
this.acknowledge = bb.getInt();
this.flags = bb.getShort();
diff --git a/src/main/java/net/floodlightcontroller/packet/UDP.java b/src/main/java/net/floodlightcontroller/packet/UDP.java
index fad7a128f..8639cb92f 100644
--- a/src/main/java/net/floodlightcontroller/packet/UDP.java
+++ b/src/main/java/net/floodlightcontroller/packet/UDP.java
@@ -211,11 +211,11 @@ public class UDP extends BasePacket {
UDP other = (UDP) obj;
if (checksum != other.checksum)
return false;
- if (destinationPort != other.destinationPort)
+ if (!destinationPort.equals(other.destinationPort))
return false;
if (length != other.length)
return false;
- if (sourcePort != other.sourcePort)
+ if (!sourcePort.equals(other.sourcePort))
return false;
return true;
}
@@ -224,20 +224,20 @@ public class UDP extends BasePacket {
public IPacket deserialize(byte[] data, int offset, int length)
throws PacketParsingException {
ByteBuffer bb = ByteBuffer.wrap(data, offset, length);
- this.sourcePort = TransportPort.of(bb.getShort());
- this.destinationPort = TransportPort.of(bb.getShort());
+ this.sourcePort = TransportPort.of((int) (bb.getShort() & 0xffff)); // short will be signed, pos or neg
+ this.destinationPort = TransportPort.of((int) (bb.getShort() & 0xffff)); // convert range 0 to 65534, not -32768 to 32767
this.length = bb.getShort();
this.checksum = bb.getShort();
- if (UDP.decodeMap.containsKey(this.destinationPort)) {
+ if (UDP.decodeMap.containsKey((short)this.destinationPort.getPort())) {
try {
- this.payload = UDP.decodeMap.get(this.destinationPort).getConstructor().newInstance();
+ this.payload = UDP.decodeMap.get((short)this.destinationPort.getPort()).getConstructor().newInstance();
} catch (Exception e) {
throw new RuntimeException("Failure instantiating class", e);
}
- } else if (UDP.decodeMap.containsKey(this.sourcePort)) {
+ } else if (UDP.decodeMap.containsKey((short)this.sourcePort.getPort())) {
try {
- this.payload = UDP.decodeMap.get(this.sourcePort).getConstructor().newInstance();
+ this.payload = UDP.decodeMap.get((short)this.sourcePort.getPort()).getConstructor().newInstance();
} catch (Exception e) {
throw new RuntimeException("Failure instantiating class", e);
}
diff --git a/src/test/java/net/floodlightcontroller/core/internal/ControllerTest.java b/src/test/java/net/floodlightcontroller/core/internal/ControllerTest.java
index 9da10335e..47d414b9e 100644
--- a/src/test/java/net/floodlightcontroller/core/internal/ControllerTest.java
+++ b/src/test/java/net/floodlightcontroller/core/internal/ControllerTest.java
@@ -271,7 +271,6 @@ public class ControllerTest extends FloodlightTestCase {
public void testHandleMessagesNoListeners() throws Exception {
IOFSwitch sw = createMock(IOFSwitch.class);
expect(sw.getId()).andReturn(DatapathId.NONE).anyTimes();
- expect(sw.getId().toString()).andReturn(DATAPATH_ID_0.toString()).anyTimes();
replay(sw);
controller.handleMessage(sw, pi, null);
verify(sw);
@@ -292,7 +291,6 @@ public class ControllerTest extends FloodlightTestCase {
IOFSwitch sw = createMock(IOFSwitch.class);
expect(sw.getId()).andReturn(DatapathId.NONE).anyTimes();
- expect(sw.getId().toString()).andReturn("00:00:00:00:00:00:00").anyTimes();
// Setup listener orderings
IOFMessageListener test1 = createMock(IOFMessageListener.class);
@@ -422,7 +420,6 @@ public class ControllerTest extends FloodlightTestCase {
doSetUp(HARole.STANDBY);
IOFSwitch sw = createMock(IOFSwitch.class);
expect(sw.getId()).andReturn(DatapathId.NONE).anyTimes();
- expect(sw.getId().toString()).andReturn(DatapathId.NONE.toString()).anyTimes();
IOFMessageListener test1 = createMock(IOFMessageListener.class);
expect(test1.getName()).andReturn("test1").atLeastOnce();
@@ -468,7 +465,6 @@ public class ControllerTest extends FloodlightTestCase {
public void testHandleMessageWithContext() throws Exception {
IOFSwitch sw = createMock(IOFSwitch.class);
expect(sw.getId()).andReturn(DatapathId.NONE).anyTimes();
- expect(sw.getId().toString()).andReturn(DatapathId.NONE.toString()).anyTimes();
IOFMessageListener test1 = createMock(IOFMessageListener.class);
expect(test1.getName()).andReturn("test1").anyTimes();
@@ -512,7 +508,6 @@ public class ControllerTest extends FloodlightTestCase {
OFMessage m = factory.buildEchoRequest().build();
IOFSwitchBackend sw = createMock(IOFSwitchBackend.class);
expect(sw.getId()).andReturn(DATAPATH_ID_0).anyTimes();
- expect(sw.getId().toString()).andReturn(DATAPATH_ID_0.toString()).anyTimes();
// Add listeners
IOFMessageListener test1 = createMock(IOFMessageListener.class);
@@ -564,7 +559,6 @@ public class ControllerTest extends FloodlightTestCase {
// Test the handleOutgoingMessage
reset(test1, test2, test3, sw);
expect(sw.getId()).andReturn(DATAPATH_ID_0).anyTimes();
- expect(sw.getId().toString()).andReturn(DATAPATH_ID_0.toString()).anyTimes();
expect(test2.receive(same(sw), same(m) , isA(FloodlightContext.class)))
.andReturn(Command.STOP);
expect(test3.receive(same(sw), same(m) , isA(FloodlightContext.class)))
@@ -580,7 +574,6 @@ public class ControllerTest extends FloodlightTestCase {
// Test the handleOutgoingMessage with null context
reset(test1, test2, test3, sw);
expect(sw.getId()).andReturn(DATAPATH_ID_0).anyTimes();
- expect(sw.getId().toString()).andReturn(DATAPATH_ID_0.toString()).anyTimes();
expect(test2.receive(same(sw), same(m) , isA(FloodlightContext.class)))
.andReturn(Command.STOP);
expect(test3.receive(same(sw), same(m) , isA(FloodlightContext.class)))
diff --git a/src/test/java/net/floodlightcontroller/core/internal/OFSwitchManagerTest.java b/src/test/java/net/floodlightcontroller/core/internal/OFSwitchManagerTest.java
index 44c757b45..c61d90dd6 100644
--- a/src/test/java/net/floodlightcontroller/core/internal/OFSwitchManagerTest.java
+++ b/src/test/java/net/floodlightcontroller/core/internal/OFSwitchManagerTest.java
@@ -182,7 +182,6 @@ public class OFSwitchManagerTest{
* is created from the given mocked switch */
protected void setupSwitchForAddSwitch(IOFSwitch sw, DatapathId datapathId,
SwitchDescription description, OFFeaturesReply featuresReply) {
- String dpidString = datapathId.toString();
if (description == null) {
description = createSwitchDescription();
}
@@ -194,7 +193,6 @@ public class OFSwitchManagerTest{
expect(sw.getOFFactory()).andReturn(OFFactories.getFactory(OFVersion.OF_10)).anyTimes();
expect(sw.getStatus()).andReturn(SwitchStatus.MASTER).anyTimes();
expect(sw.getId()).andReturn(datapathId).anyTimes();
- expect(sw.getId().toString()).andReturn(dpidString).anyTimes();
expect(sw.getSwitchDescription()).andReturn(description).anyTimes();
expect(sw.getBuffers())
.andReturn(featuresReply.getNBuffers()).anyTimes();
@@ -317,7 +315,6 @@ public class OFSwitchManagerTest{
public void testNonexistingSwitchDisconnected() throws Exception {
IOFSwitchBackend sw = createMock(IOFSwitchBackend.class);
expect(sw.getId()).andReturn(DATAPATH_ID_1).anyTimes();
- expect(sw.getId().toString()).andReturn(DATAPATH_ID_1.toString()).anyTimes();
IOFSwitchListener listener = createMock(IOFSwitchListener.class);
switchManager.addOFSwitchListener(listener);
replay(sw, listener);
@@ -338,7 +335,6 @@ public class OFSwitchManagerTest{
// create a new mock switch
IOFSwitchBackend sw = createMock(IOFSwitchBackend.class);
expect(sw.getId()).andReturn(DATAPATH_ID_1).anyTimes();
- expect(sw.getId().toString()).andReturn(DATAPATH_ID_1.toString()).anyTimes();
IOFSwitchListener listener = createMock(IOFSwitchListener.class);
switchManager.addOFSwitchListener(listener);
replay(sw, listener);
diff --git a/src/test/java/net/floodlightcontroller/packet/DHCPTest.java b/src/test/java/net/floodlightcontroller/packet/DHCPTest.java
index b83ffa816..cecb7b1e9 100644
--- a/src/test/java/net/floodlightcontroller/packet/DHCPTest.java
+++ b/src/test/java/net/floodlightcontroller/packet/DHCPTest.java
@@ -586,8 +586,8 @@ public class DHCPTest extends TestCase {
assertTrue(udp.getPayload() instanceof DHCP);
DHCP dhcp = (DHCP) udp.getPayload();
- assertEquals(UDP.DHCP_CLIENT_PORT, udp.getSourcePort());
- assertEquals(UDP.DHCP_SERVER_PORT, udp.getDestinationPort());
+ assertEquals(UDP.DHCP_CLIENT_PORT, udp.getSourcePort().getPort());
+ assertEquals(UDP.DHCP_SERVER_PORT, udp.getDestinationPort().getPort());
// should get invalid opCode of 0
assertEquals(0, dhcp.getOpCode());
diff --git a/src/test/java/net/floodlightcontroller/packet/TCPTest.java b/src/test/java/net/floodlightcontroller/packet/TCPTest.java
index b453f972f..e49fb9045 100644
--- a/src/test/java/net/floodlightcontroller/packet/TCPTest.java
+++ b/src/test/java/net/floodlightcontroller/packet/TCPTest.java
@@ -49,8 +49,8 @@ public class TCPTest {
.setSourceAddress("74.125.45.109")
.setDestinationAddress("192.168.1.111")
.setPayload(new TCP()
- .setSourcePort((short) 993)
- .setDestinationPort((short) 49202)
+ .setSourcePort(993)
+ .setDestinationPort(49202)
.setSequence(0xe3adee88)
.setAcknowledge(0xb7dad824)
.setDataOffset((byte) 8)
--
GitLab