unsigned PortNumber
PortNumber class update
- internal representation now uses int
- toString() now converts the value as unsigned integer
- renamed value() to shortValue()
- added value() method returning port number as unsigned long
- added constructor from String
- JSON serialization now uses the new value(), so that special handling for
unsigned integer is not required
Update classes using PortNumber
- Replaced value() calls with new value(), when the return value was
converted to long immediately.
- toString() result may change if the object contained PortNumber.
- e.g., FlowEntryAction, FlowEntry, SwitchPort, ...
Change-Id: I0b1f8aa92208e165b6113c8dd6954076c0513167
diff --git a/src/main/java/net/onrc/onos/apps/forwarding/Forwarding.java b/src/main/java/net/onrc/onos/apps/forwarding/Forwarding.java
index 969b2b7..a2a8e48 100644
--- a/src/main/java/net/onrc/onos/apps/forwarding/Forwarding.java
+++ b/src/main/java/net/onrc/onos/apps/forwarding/Forwarding.java
@@ -331,7 +331,7 @@
//This code assumes the host has only one port. It should be problem.
Port destinationPort = ports.next();
- short destinationPortNum = destinationPort.getNumber().value();
+ short destinationPortNum = destinationPort.getNumber().shortValue();
Switch destinationSw = destinationPort.getSwitch();
long destinationDpid = destinationSw.getDpid().value();
@@ -525,7 +525,7 @@
outPort = (short) spfIntent.getDstPortNumber();
log.debug("Path is empty. Maybe hosts on the same switch. outPort {}", outPort);
} else {
- outPort = graphPath.get(0).getSrc().getPortNumber().value();
+ outPort = graphPath.get(0).getSrc().getPortNumber().shortValue();
log.debug("path{}, outPort {}", graphPath, outPort);
}
diff --git a/src/main/java/net/onrc/onos/apps/proxyarp/ProxyArpManager.java b/src/main/java/net/onrc/onos/apps/proxyarp/ProxyArpManager.java
index 53a2b5d..d276691 100644
--- a/src/main/java/net/onrc/onos/apps/proxyarp/ProxyArpManager.java
+++ b/src/main/java/net/onrc/onos/apps/proxyarp/ProxyArpManager.java
@@ -374,7 +374,7 @@
ARP arp = (ARP) eth.getPayload();
learnArp(arp);
if (arp.getOpCode() == ARP.OP_REQUEST) {
- handleArpRequest(sw.getDpid().value(), inPort.getNumber().value(),
+ handleArpRequest(sw.getDpid().value(), inPort.getNumber().shortValue(),
arp, eth);
} else if (arp.getOpCode() == ARP.OP_REPLY) {
// For replies we simply send a notification via Hazelcast
diff --git a/src/main/java/net/onrc/onos/core/datastore/topology/KVLink.java b/src/main/java/net/onrc/onos/core/datastore/topology/KVLink.java
index 73b5986..a6a02a7 100644
--- a/src/main/java/net/onrc/onos/core/datastore/topology/KVLink.java
+++ b/src/main/java/net/onrc/onos/core/datastore/topology/KVLink.java
@@ -69,7 +69,7 @@
*/
public SwitchPort(final Dpid srcDpid, final PortNumber srcPortNo) {
this.dpid = srcDpid.value();
- this.number = (long) srcPortNo.value();
+ this.number = srcPortNo.value();
}
/**
diff --git a/src/main/java/net/onrc/onos/core/datastore/topology/KVPort.java b/src/main/java/net/onrc/onos/core/datastore/topology/KVPort.java
index 9a3014e..2fc6be9 100644
--- a/src/main/java/net/onrc/onos/core/datastore/topology/KVPort.java
+++ b/src/main/java/net/onrc/onos/core/datastore/topology/KVPort.java
@@ -182,7 +182,7 @@
* @param namespace namespace to create this object
*/
public KVPort(final Dpid dpid, final PortNumber number, final String namespace) {
- this(dpid.value(), (long) number.value(), namespace);
+ this(dpid.value(), number.value(), namespace);
}
/**
diff --git a/src/main/java/net/onrc/onos/core/flowprogrammer/FlowPusher.java b/src/main/java/net/onrc/onos/core/flowprogrammer/FlowPusher.java
index 1a7cae8..2c64ec1 100644
--- a/src/main/java/net/onrc/onos/core/flowprogrammer/FlowPusher.java
+++ b/src/main/java/net/onrc/onos/core/flowprogrammer/FlowPusher.java
@@ -667,7 +667,7 @@
// Match the Incoming Port
PortNumber matchInPort = flowEntryMatch.inPort();
if (matchInPort != null) {
- match.setInputPort(matchInPort.value());
+ match.setInputPort(matchInPort.shortValue());
match.setWildcards(match.getWildcards() & ~OFMatch.OFPFW_IN_PORT);
}
@@ -792,10 +792,10 @@
ActionEnqueue actionEnqueue = action.actionEnqueue();
if (actionOutput != null) {
- actionOutputPort.portNumber = actionOutput.port().value();
+ actionOutputPort.portNumber = actionOutput.port().shortValue();
// XXX: The max length is hard-coded for now
OFActionOutput ofa = new OFActionOutput(actionOutput.port()
- .value(), (short) 0xffff);
+ .shortValue(), (short) 0xffff);
openFlowActions.add(ofa);
}
@@ -862,7 +862,7 @@
if (actionEnqueue != null) {
OFActionEnqueue ofa =
- new OFActionEnqueue(actionEnqueue.port().value(), actionEnqueue.queueId());
+ new OFActionEnqueue(actionEnqueue.port().shortValue(), actionEnqueue.queueId());
openFlowActions.add(ofa);
}
}
diff --git a/src/main/java/net/onrc/onos/core/hostmanager/HostManager.java b/src/main/java/net/onrc/onos/core/hostmanager/HostManager.java
index fd34519..ff3ee8c 100644
--- a/src/main/java/net/onrc/onos/core/hostmanager/HostManager.java
+++ b/src/main/java/net/onrc/onos/core/hostmanager/HostManager.java
@@ -283,7 +283,7 @@
deleteHost = new Host(host.getMacAddress(),
null,
switchPort.getDpid().value(),
- (long) switchPort.getNumber().value(),
+ switchPort.getNumber().value(),
new Date(host.getLastSeenTime()));
break;
}
diff --git a/src/main/java/net/onrc/onos/core/linkdiscovery/LinkDiscoveryManager.java b/src/main/java/net/onrc/onos/core/linkdiscovery/LinkDiscoveryManager.java
index c02aff0..a924f00 100644
--- a/src/main/java/net/onrc/onos/core/linkdiscovery/LinkDiscoveryManager.java
+++ b/src/main/java/net/onrc/onos/core/linkdiscovery/LinkDiscoveryManager.java
@@ -443,9 +443,8 @@
SwitchPort switchPort = OnosLldp.extractSwitchPort(packetData);
long remoteDpid = switchPort.dpid().value();
- short remotePort = switchPort.port().value();
- IOFSwitch remoteSwitch = floodlightProvider.getSwitches().get(
- switchPort.dpid().value());
+ short remotePort = switchPort.port().shortValue();
+ IOFSwitch remoteSwitch = floodlightProvider.getSwitches().get(switchPort.dpid().value());
OFPhysicalPort physicalPort = null;
diff --git a/src/main/java/net/onrc/onos/core/packetservice/PacketModule.java b/src/main/java/net/onrc/onos/core/packetservice/PacketModule.java
index 9a03fed..afe2770 100644
--- a/src/main/java/net/onrc/onos/core/packetservice/PacketModule.java
+++ b/src/main/java/net/onrc/onos/core/packetservice/PacketModule.java
@@ -104,7 +104,7 @@
public void sendPacket(Ethernet eth, SwitchPort switchPort) {
SinglePacketOutNotification notification =
new SinglePacketOutNotification(eth.serialize(), 0,
- switchPort.dpid().value(), switchPort.port().value());
+ switchPort.dpid().value(), switchPort.port().shortValue());
// TODO We shouldn't care what the destination MAC is
long dstMac = eth.getDestinationMAC().toLong();
@@ -127,7 +127,7 @@
public void broadcastPacketOutEdge(Ethernet eth, SwitchPort inSwitchPort) {
BroadcastPacketOutNotification notification =
new BroadcastPacketOutNotification(eth.serialize(), 0,
- inSwitchPort.dpid().value(), inSwitchPort.port().value());
+ inSwitchPort.dpid().value(), inSwitchPort.port().shortValue());
long dstMac = eth.getDestinationMAC().toLong();
packetOutEventChannel.addTransientEntry(dstMac, notification);
diff --git a/src/main/java/net/onrc/onos/core/topology/LinkEvent.java b/src/main/java/net/onrc/onos/core/topology/LinkEvent.java
index 831b407..00b4b90 100644
--- a/src/main/java/net/onrc/onos/core/topology/LinkEvent.java
+++ b/src/main/java/net/onrc/onos/core/topology/LinkEvent.java
@@ -146,8 +146,8 @@
public static ByteBuffer getLinkID(Dpid srcDpid, PortNumber srcPortNo,
Dpid dstDpid, PortNumber dstPortNo) {
- return getLinkID(srcDpid.value(), (long) srcPortNo.value(),
- dstDpid.value(), (long) dstPortNo.value());
+ return getLinkID(srcDpid.value(), srcPortNo.value(),
+ dstDpid.value(), dstPortNo.value());
}
public static ByteBuffer getLinkID(Long srcDpid, Long srcPortNo,
diff --git a/src/main/java/net/onrc/onos/core/topology/PortEvent.java b/src/main/java/net/onrc/onos/core/topology/PortEvent.java
index 19b6a80..305f4aa 100644
--- a/src/main/java/net/onrc/onos/core/topology/PortEvent.java
+++ b/src/main/java/net/onrc/onos/core/topology/PortEvent.java
@@ -134,7 +134,7 @@
public static ByteBuffer getPortID(Dpid dpid, PortNumber number) {
Validate.notNull(dpid);
Validate.notNull(number);
- return getPortID(dpid.value(), (long) number.value());
+ return getPortID(dpid.value(), number.value());
}
public static ByteBuffer getPortID(Long dpid, Long number) {
diff --git a/src/main/java/net/onrc/onos/core/topology/web/serializers/PortEventSerializer.java b/src/main/java/net/onrc/onos/core/topology/web/serializers/PortEventSerializer.java
index 4f2d5bf..db22b2d 100644
--- a/src/main/java/net/onrc/onos/core/topology/web/serializers/PortEventSerializer.java
+++ b/src/main/java/net/onrc/onos/core/topology/web/serializers/PortEventSerializer.java
@@ -29,13 +29,8 @@
jsonGenerator.writeStringField(TopologyElement.TYPE, portEvent.getType());
jsonGenerator.writeStringField("state", "ACTIVE");
jsonGenerator.writeStringField("dpid", portEvent.getDpid().toString());
- //
- // FIXME: The solution below to preresent the "short" port number
- // as an unsigned value is a hack. The fix should be elsewhere
- // (e.g., in class PortNumber itself).
- //
jsonGenerator.writeNumberField("portNumber",
- (0xffff & portEvent.getPortNumber().value()));
+ portEvent.getPortNumber().value());
jsonGenerator.writeStringField("desc",
null /* port.getDescription() */);
jsonGenerator.writeEndObject();
diff --git a/src/main/java/net/onrc/onos/core/topology/web/serializers/PortSerializer.java b/src/main/java/net/onrc/onos/core/topology/web/serializers/PortSerializer.java
index 34d411f..628a923 100644
--- a/src/main/java/net/onrc/onos/core/topology/web/serializers/PortSerializer.java
+++ b/src/main/java/net/onrc/onos/core/topology/web/serializers/PortSerializer.java
@@ -30,13 +30,8 @@
jsonGenerator.writeStringField(TopologyElement.TYPE, port.getType());
jsonGenerator.writeStringField("state", "ACTIVE");
jsonGenerator.writeStringField("dpid", port.getDpid().toString());
- //
- // FIXME: The solution below to preresent the "short" port number
- // as an unsigned value is a hack. The fix should be elsewhere
- // (e.g., in class PortNumber itself).
- //
jsonGenerator.writeNumberField("portNumber",
- (0xffff & port.getNumber().value()));
+ port.getNumber().value());
jsonGenerator.writeStringField("desc", port.getDescription());
jsonGenerator.writeObjectFieldStart("stringAttributes");
for (Entry<String, String> entry : port.getAllStringAttributes().entrySet()) {
diff --git a/src/main/java/net/onrc/onos/core/util/FlowEntryAction.java b/src/main/java/net/onrc/onos/core/util/FlowEntryAction.java
index 02a969c..d5ad674 100644
--- a/src/main/java/net/onrc/onos/core/util/FlowEntryAction.java
+++ b/src/main/java/net/onrc/onos/core/util/FlowEntryAction.java
@@ -177,8 +177,7 @@
String[] tokens = decode.split("port=");
if (tokens.length > 1 && tokens[1] != null) {
try {
- Short valueShort = Short.valueOf(tokens[1]);
- port = new PortNumber(valueShort);
+ port = new PortNumber(tokens[1]);
} catch (NumberFormatException e) {
throw new IllegalArgumentException("Invalid action string");
}
diff --git a/src/main/java/net/onrc/onos/core/util/PortNumber.java b/src/main/java/net/onrc/onos/core/util/PortNumber.java
index 7273dea..c4c7eed 100644
--- a/src/main/java/net/onrc/onos/core/util/PortNumber.java
+++ b/src/main/java/net/onrc/onos/core/util/PortNumber.java
@@ -2,6 +2,8 @@
import org.codehaus.jackson.annotate.JsonProperty;
+import com.google.common.primitives.UnsignedInts;
+
/**
* Immutable class representing a port number.
* <p/>
@@ -9,7 +11,7 @@
*/
public final class PortNumber {
- private final short value;
+ private final int value;
/**
* Default constructor.
@@ -24,7 +26,7 @@
* @param other the object to copy from.
*/
public PortNumber(PortNumber other) {
- this.value = other.value();
+ this.value = other.value;
}
/**
@@ -33,27 +35,78 @@
* @param value the value to use.
*/
public PortNumber(short value) {
- this.value = value;
+ this.value = (int) shortToUnsignedLong(value);
}
/**
- * Get the value of the port.
+ * Constructor from an int.
+ *
+ * @param value the value to use. (Value will not be validated in any way.)
+ */
+ PortNumber(int value) {
+ this.value = value;
+ }
+
+ // TODO We may want a factory method version
+ // which does the range validation of parsed value.
+ /**
+ * Constructor from decimal string.
+ *
+ * @param decStr decimal string representation of a port number
+ */
+ public PortNumber(String decStr) {
+ this(decStr, 10);
+ }
+
+ /**
+ * Constructor from string.
+ *
+ * @param s string representation of a port number
+ * @param radix the radix to use while parsing {@code s}
+ */
+ public PortNumber(String s, int radix) {
+ this(UnsignedInts.parseUnsignedInt(s, radix));
+ }
+
+ /**
+ * Convert unsigned short to unsigned long.
+ *
+ * @param portno unsigned integer representing port number
+ * @return port number as unsigned long
+ */
+ public static long shortToUnsignedLong(short portno) {
+ return UnsignedInts.toLong(0xffff & portno);
+ }
+
+ /**
+ * Gets the port number as short.
+ * <p/>
+ * Note: User of this method needs to be careful, handling unsigned value.
+ * @return number as short
+ */
+ public short shortValue() {
+ return (short) value;
+ }
+
+ /**
+ * Gets the value of the port as unsigned integer.
*
* @return the value of the port.
*/
@JsonProperty("value")
- public short value() {
+ public long value() {
+ // TODO Will require masking when we start storing 32bit port number.
return value;
}
/**
- * Convert the port value to a string.
+ * Convert the port value as unsigned integer to a string.
*
* @return the port value as a string.
*/
@Override
public String toString() {
- return Short.toString(this.value);
+ return UnsignedInts.toString(value);
}
@Override
diff --git a/src/main/java/net/onrc/onos/core/util/serializers/SwitchPortSerializer.java b/src/main/java/net/onrc/onos/core/util/serializers/SwitchPortSerializer.java
index 9f0041d..163a43f 100644
--- a/src/main/java/net/onrc/onos/core/util/serializers/SwitchPortSerializer.java
+++ b/src/main/java/net/onrc/onos/core/util/serializers/SwitchPortSerializer.java
@@ -42,7 +42,7 @@
// (e.g., in class PortNumber itself).
//
jsonGenerator.writeNumberField("portNumber",
- (0xffff & switchPort.getPortNumber().value()));
+ switchPort.getPortNumber().value());
jsonGenerator.writeEndObject();
}
diff --git a/src/test/java/net/onrc/onos/core/util/FlowEntryTest.java b/src/test/java/net/onrc/onos/core/util/FlowEntryTest.java
index cca5d02..769198a 100644
--- a/src/test/java/net/onrc/onos/core/util/FlowEntryTest.java
+++ b/src/test/java/net/onrc/onos/core/util/FlowEntryTest.java
@@ -278,7 +278,7 @@
+ " flowEntryActions=["
+ "[type=ACTION_OUTPUT action=[port=9 maxLen=0]];"
// PORT_CONTROLLER((short) 0xfffd) = 65533 (-3)
- + "[type=ACTION_OUTPUT action=[port=-3 maxLen=0]];"
+ + "[type=ACTION_OUTPUT action=[port=65533 maxLen=0]];"
+ "[type=ACTION_SET_VLAN_VID action=[vlanId=3]];"
+ "[type=ACTION_SET_VLAN_PCP action=[vlanPriority=4]];"
+ "[type=ACTION_STRIP_VLAN action=[stripVlan=true]];"
diff --git a/src/test/java/net/onrc/onos/core/util/LinkTupleTest.java b/src/test/java/net/onrc/onos/core/util/LinkTupleTest.java
index 88c2ca9..06cce85 100644
--- a/src/test/java/net/onrc/onos/core/util/LinkTupleTest.java
+++ b/src/test/java/net/onrc/onos/core/util/LinkTupleTest.java
@@ -143,8 +143,7 @@
*/
@Test
public void testToString() {
- // FIXME when we start handling unsigned integer properly
- assertEquals("(00:00:00:00:00:00:00:01/-1=>00:00:00:00:00:00:00:02/-2)",
+ assertEquals("(00:00:00:00:00:00:00:01/65535=>00:00:00:00:00:00:00:02/65534)",
L2.toString());
}
diff --git a/src/test/java/net/onrc/onos/core/util/PortNumberTest.java b/src/test/java/net/onrc/onos/core/util/PortNumberTest.java
new file mode 100644
index 0000000..eb9915a
--- /dev/null
+++ b/src/test/java/net/onrc/onos/core/util/PortNumberTest.java
@@ -0,0 +1,134 @@
+/**
+ *
+ */
+package net.onrc.onos.core.util;
+
+import static org.junit.Assert.*;
+
+import org.junit.Test;
+
+/**
+ * PortNumber test mainly focused on handling unsigned port no correctly.
+ */
+public class PortNumberTest {
+
+ private static final PortNumber PORT_NONE = new PortNumber(0xffff);
+ private static final PortNumber PORT_LOCAL = new PortNumber(0xfffe);
+
+ /**
+ * Test method for {@link PortNumber#PortNumber(String)}.
+ */
+ @Test
+ public void testPortNumberDecimalString() {
+ final PortNumber ref = new PortNumber(PORT_NONE);
+ PortNumber parsed = new PortNumber("65535");
+
+ assertEquals(ref, parsed);
+ }
+
+ /**
+ * Test method for {@link PortNumber#PortNumber(String, int)}.
+ */
+ @Test
+ public void testPortNumberString() {
+ final PortNumber ref = new PortNumber(PORT_NONE);
+ PortNumber parsed = new PortNumber("ffff", 16);
+ // Note: cannot parse "0xffff" now,
+ // which is the same behavior as Integer.parseInt.
+
+ assertEquals(ref, parsed);
+ }
+
+ /**
+ * Test method for {@link PortNumber#PortNumber(PortNumber)}.
+ */
+ @Test
+ public void testPortNumberPortNumber() {
+ PortNumber p = new PortNumber(PORT_LOCAL);
+ PortNumber copy = new PortNumber(p);
+
+ assertEquals(p, copy);
+ }
+
+ /**
+ * Test method for {@link PortNumber#PortNumber(PortNumber.PortValues)}.
+ */
+ @Test
+ public void testPortNumberPortValues() {
+ PortNumber local = new PortNumber(PORT_LOCAL);
+ assertEquals(0xfffeL, local.value());
+ assertEquals((short) 0xfffe, local.shortValue());
+ }
+
+ /**
+ * Test method for {@link PortNumber#shortToUnsignedLong(short)}.
+ */
+ @Test
+ public void testLongValueShort() {
+ assertEquals(0L, PortNumber.shortToUnsignedLong((short) 0));
+
+ assertEquals(1L, PortNumber.shortToUnsignedLong((short) 1));
+
+ // -1 as unsigned short
+ assertEquals(0xffffL, PortNumber.shortToUnsignedLong((short) -1));
+ }
+
+ /**
+ * Test method for {@link PortNumber#shortValue()}.
+ */
+ @Test
+ public void testShortValue() {
+ assertEquals(0L, new PortNumber((short) 0).shortValue());
+
+ assertEquals(1L, new PortNumber((short) 1).shortValue());
+
+ // user of #shortValue() needs to be careful
+ // simply widening them will result in negative value
+ assertEquals(-1L, new PortNumber(PORT_NONE).shortValue());
+
+ // user of #shortValue() needs to be careful
+ // should use PortNumber.shortToUnsignedLong or mask manually
+ assertEquals(0xffffL, PortNumber.shortToUnsignedLong(new PortNumber(PORT_NONE).shortValue()));
+ assertEquals(0xffffL, 0xffff & new PortNumber(PORT_NONE).shortValue());
+}
+
+ /**
+ * Test method for {@link PortNumber#value()}.
+ */
+ @Test
+ public void testLongValue() {
+ assertEquals(0L, new PortNumber((short) 0).value());
+
+ assertEquals(1L, new PortNumber((short) 1).value());
+
+ assertEquals(0xffffL, new PortNumber(PORT_NONE).value());
+ }
+
+ /**
+ * Test method for {@link PortNumber#toString()}.
+ */
+ @Test
+ public void testToString() {
+ assertEquals("0", new PortNumber((short) 0).toString());
+
+ assertEquals("1", new PortNumber((short) 1).toString());
+
+ // 0xffff in decimal
+ assertEquals("65535", new PortNumber(PORT_NONE).toString());
+ }
+
+ /**
+ * Test method for {@link PortNumber#equals(java.lang.Object)}.
+ */
+ @Test
+ public void testEqualsObject() {
+ // Some trivial
+ assertTrue(new PortNumber(PORT_NONE).equals(new PortNumber((short) 0xffff)));
+ assertFalse(new PortNumber((short) 0).equals(new PortNumber((short) 1)));
+
+ // different type
+ assertFalse(new PortNumber((short) 0).equals(Short.valueOf((short) 0)));
+
+ }
+
+}
diff --git a/src/test/java/net/onrc/onos/core/util/SwitchPortTest.java b/src/test/java/net/onrc/onos/core/util/SwitchPortTest.java
index 48c3b65..1c01e8f 100644
--- a/src/test/java/net/onrc/onos/core/util/SwitchPortTest.java
+++ b/src/test/java/net/onrc/onos/core/util/SwitchPortTest.java
@@ -48,7 +48,7 @@
*/
@Test
public void testToString() {
- assertEquals("00:00:00:00:00:00:00:01/-1", SWP2.toString());
+ assertEquals("00:00:00:00:00:00:00:01/65535", SWP2.toString());
}
/**