PortNumber fixes

- hide new PortNumber(short) and use PortNumber.uint16 instead
- added utility to get PortNumber

Change-Id: Ia39fbe6e7126a3d9465d3035a06850c1d54a7f25
diff --git a/src/main/java/net/floodlightcontroller/core/internal/Controller.java b/src/main/java/net/floodlightcontroller/core/internal/Controller.java
index 23d61bb..dd063a6 100644
--- a/src/main/java/net/floodlightcontroller/core/internal/Controller.java
+++ b/src/main/java/net/floodlightcontroller/core/internal/Controller.java
@@ -614,6 +614,7 @@
             FloodlightContext bContext)
             throws IOException {
         Ethernet eth = null;
+        // FIXME losing port number precision here
         short inport = -1;
 
         switch (m.getType()) {
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 7c229b6..127b2c0 100644
--- a/src/main/java/net/onrc/onos/apps/forwarding/Forwarding.java
+++ b/src/main/java/net/onrc/onos/apps/forwarding/Forwarding.java
@@ -397,6 +397,7 @@
 
                     boolean isflowEntryForThisSwitch = false;
                     net.onrc.onos.core.intent.Path path = pathIntent.getPath();
+                    // FIXME should switch to PortNumber, etc.
                     long outPort = -1;
 
                     if (spfIntent.getDstSwitchDpid() == sw.getDpid().value()) {
@@ -430,7 +431,7 @@
 
                         log.debug("Sending packet out from sw {}, outport{}", sw.getDpid(), outPort);
                         packetService.sendPacket(eth, new SwitchPort(
-                                sw.getDpid(), new PortNumber((short) outPort)));
+                                sw.getDpid(), PortNumber.uint16((short) outPort)));
                     }
                 } else {
                     // Flow path has not yet been installed to switches so save the
diff --git a/src/main/java/net/onrc/onos/apps/sdnip/Interface.java b/src/main/java/net/onrc/onos/apps/sdnip/Interface.java
index 463cac8..4052e81 100644
--- a/src/main/java/net/onrc/onos/apps/sdnip/Interface.java
+++ b/src/main/java/net/onrc/onos/apps/sdnip/Interface.java
@@ -73,7 +73,7 @@
     public SwitchPort getSwitchPort() {
         //TODO SwitchPort, Dpid and Port are mutable, but they could probably
         //be made immutable which would prevent the need to copy
-        return new SwitchPort(new Dpid(dpid), new PortNumber(port));
+        return new SwitchPort(new Dpid(dpid), PortNumber.uint16(port));
     }
 
     /**
diff --git a/src/main/java/net/onrc/onos/apps/sdnip/SdnIp.java b/src/main/java/net/onrc/onos/apps/sdnip/SdnIp.java
index e39f404..d6713d5 100644
--- a/src/main/java/net/onrc/onos/apps/sdnip/SdnIp.java
+++ b/src/main/java/net/onrc/onos/apps/sdnip/SdnIp.java
@@ -203,7 +203,7 @@
 
             bgpdAttachmentPoint = new SwitchPort(
                     new Dpid(config.getBgpdAttachmentDpid()),
-                    new PortNumber(config.getBgpdAttachmentPort()));
+                    PortNumber.uint16(config.getBgpdAttachmentPort()));
 
             bgpdMacAddress = config.getBgpdMacAddress();
             vlan = config.getVlan();
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 9f6373d..a5e3bd9 100644
--- a/src/main/java/net/onrc/onos/core/hostmanager/HostManager.java
+++ b/src/main/java/net/onrc/onos/core/hostmanager/HostManager.java
@@ -29,6 +29,7 @@
 import net.onrc.onos.core.topology.Topology;
 import net.onrc.onos.core.util.Dpid;
 import net.onrc.onos.core.util.PortNumber;
+import net.onrc.onos.core.util.PortNumberUtils;
 
 import org.projectfloodlight.openflow.protocol.OFMessage;
 import org.projectfloodlight.openflow.protocol.OFPacketIn;
@@ -121,11 +122,13 @@
     protected Command processPacketIn(IOFSwitch sw, OFPacketIn pi, Ethernet eth,
             short inport) {
         if (log.isTraceEnabled()) {
-            log.trace("Receive PACKET_IN swId {}, portId {}", sw.getId(), pi.getInPort());
+            log.trace("Receive PACKET_IN swId {}, portId {}", sw.getId(), inport);
         }
 
         final Dpid dpid = new Dpid(sw.getId());
-        final PortNumber portNum = new PortNumber(inport);
+        // FIXME method signature needs to be fixed. losing port number precision
+        final PortNumber portNum = PortNumberUtils
+                            .openFlow(sw.getOFVersion(), inport);
 
         Host srcHost =
                 getSourceHostFromPacket(eth, dpid.value(), portNum.value());
diff --git a/src/main/java/net/onrc/onos/core/packetservice/BroadcastPacketOutNotification.java b/src/main/java/net/onrc/onos/core/packetservice/BroadcastPacketOutNotification.java
index 8a9c065..4f96302 100644
--- a/src/main/java/net/onrc/onos/core/packetservice/BroadcastPacketOutNotification.java
+++ b/src/main/java/net/onrc/onos/core/packetservice/BroadcastPacketOutNotification.java
@@ -92,7 +92,7 @@
             topology.acquireReadLock();
             try {
                 globalPort = topology.getPort(new Dpid(entry.getKey()),
-                    new PortNumber(entry.getValue()));
+                    PortNumber.uint16(entry.getValue()));
             } finally {
                 topology.releaseReadLock();
             }
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 35aca3a..61fa735 100644
--- a/src/main/java/net/onrc/onos/core/packetservice/PacketModule.java
+++ b/src/main/java/net/onrc/onos/core/packetservice/PacketModule.java
@@ -157,6 +157,7 @@
 
         Ethernet eth = IFloodlightProviderService.bcStore.
                 get(cntx, IFloodlightProviderService.CONTEXT_PI_PAYLOAD);
+        // FIXME losing port number precision
         short inport = (short) cntx.getStorage()
                 .get(IFloodlightProviderService.CONTEXT_PI_INPORT);
 
@@ -165,7 +166,7 @@
         try {
             topology.acquireReadLock();
             Dpid dpid = new Dpid(sw.getId());
-            PortNumber p = new PortNumber(inport);
+            PortNumber p = PortNumber.uint16(inport);
             topologySwitch = topology.getSwitch(dpid);
             inPort = topology.getPort(dpid, p);
         } finally {
diff --git a/src/main/java/net/onrc/onos/core/topology/TopologyPublisher.java b/src/main/java/net/onrc/onos/core/topology/TopologyPublisher.java
index 2863523..7675aae 100644
--- a/src/main/java/net/onrc/onos/core/topology/TopologyPublisher.java
+++ b/src/main/java/net/onrc/onos/core/topology/TopologyPublisher.java
@@ -21,7 +21,6 @@
 import net.floodlightcontroller.core.module.IFloodlightService;
 import net.floodlightcontroller.core.util.SingletonTask;
 import net.floodlightcontroller.threadpool.IThreadPoolService;
-
 import net.onrc.onos.core.datagrid.IDatagridService;
 import net.onrc.onos.core.datagrid.IEventChannel;
 import net.onrc.onos.core.hostmanager.Host;
@@ -35,7 +34,7 @@
 import net.onrc.onos.core.registry.RegistryException;
 import net.onrc.onos.core.util.Dpid;
 import net.onrc.onos.core.util.OnosInstanceId;
-import net.onrc.onos.core.util.PortNumber;
+import net.onrc.onos.core.util.PortNumberUtils;
 import net.onrc.onos.core.util.SwitchPort;
 
 import org.apache.commons.lang3.concurrent.ConcurrentUtils;
@@ -260,7 +259,7 @@
         List<PortEvent> portEvents = new ArrayList<PortEvent>();
         for (OFPortDesc port : sw.getPorts()) {
             PortEvent portEvent = new PortEvent(dpid,
-                    new PortNumber(port.getPortNo().getShortPortNumber()));
+                                                PortNumberUtils.openFlow(port));
             // FIXME should be merging, with existing attrs, etc..
             // TODO define attr name as constant somewhere.
             // TODO populate appropriate attributes.
@@ -349,8 +348,8 @@
      */
     private void switchPortAdded(long switchId, OFPortDesc port) {
         final Dpid dpid = new Dpid(switchId);
-        PortEvent portEvent = new PortEvent(dpid,
-                new PortNumber(port.getPortNo().getShortPortNumber()));
+        final PortEvent portEvent = new PortEvent(dpid,
+                                        PortNumberUtils.openFlow(port));
         // FIXME should be merging, with existing attrs, etc..
         // TODO define attr name as constant somewhere.
         // TODO populate appropriate attributes.
@@ -371,8 +370,8 @@
     private void switchPortRemoved(long switchId, OFPortDesc port) {
         final Dpid dpid = new Dpid(switchId);
 
-        PortEvent portEvent = new PortEvent(dpid, new PortNumber(
-                port.getPortNo().getShortPortNumber()));
+        final PortEvent portEvent = new PortEvent(dpid,
+                                        PortNumberUtils.openFlow(port));
         // FIXME should be merging, with existing attrs, etc..
         // TODO define attr name as constant somewhere.
         // TODO populate appropriate attributes.
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 0320a42..a9b42b0 100644
--- a/src/main/java/net/onrc/onos/core/util/PortNumber.java
+++ b/src/main/java/net/onrc/onos/core/util/PortNumber.java
@@ -35,7 +35,7 @@
      *
      * @param value the value to use.
      */
-    public PortNumber(short value) {
+    protected PortNumber(short value) {
         this.value = (int) shortToUnsignedLong(value);
     }
 
diff --git a/src/main/java/net/onrc/onos/core/util/PortNumberUtils.java b/src/main/java/net/onrc/onos/core/util/PortNumberUtils.java
new file mode 100644
index 0000000..38051ff
--- /dev/null
+++ b/src/main/java/net/onrc/onos/core/util/PortNumberUtils.java
@@ -0,0 +1,74 @@
+package net.onrc.onos.core.util;
+
+import static com.google.common.base.Preconditions.checkNotNull;
+
+import org.projectfloodlight.openflow.protocol.OFPortDesc;
+import org.projectfloodlight.openflow.protocol.OFVersion;
+import org.projectfloodlight.openflow.types.OFPort;
+
+/**
+ * {@link PortNumber} related utilities.
+ */
+public final class PortNumberUtils {
+
+    /**
+     * Gets {@link PortNumber} for specified port.
+     *
+     * @param ofVersion OF version to use translating {@code portNumber}
+     * @param portNumber treated as uint16 if {@link OFVersion#OF_10}
+     *                  uint32 otherwise
+     * @return {@link PortNumber}
+     */
+    public static PortNumber openFlow(OFVersion ofVersion, int portNumber) {
+        if (ofVersion == OFVersion.OF_10) {
+            return PortNumber.uint16(toOF10(portNumber));
+        } else {
+            return PortNumber.uint32(portNumber);
+        }
+    }
+
+    /**
+     * Gets {@link PortNumber} for specified port.
+     *
+     * @param desc {@link OFPortDesc}
+     * @return {@link PortNumber}
+     */
+    public static PortNumber openFlow(final OFPortDesc desc) {
+        if (checkNotNull(desc).getVersion() == OFVersion.OF_10) {
+            return PortNumber.uint16(desc.getPortNo().getShortPortNumber());
+        } else {
+            return PortNumber.uint32(desc.getPortNo().getPortNumber());
+        }
+    }
+
+    /**
+     * Validate OF1.0 port number.
+     *
+     * @param of10PortNumber 0F1.0 port number or {@link OFPort#getPortNumber()}
+     * @return OF1.0 port number
+     * @throws IllegalArgumentException if out of valid OF1.0 range
+     */
+    public static short toOF10(final int of10PortNumber) {
+        try {
+            // if the input was from OFPort#getPortNumber()
+            // some named/reserved port number to OF1.0 constants.
+            return OFPort.ofInt(of10PortNumber).getShortPortNumber();
+        } catch (IllegalArgumentException e) {
+            // OFPort#getShortPortNumber will rejects OF1.0 special ports
+            if (0xFFf8 <= of10PortNumber && of10PortNumber <= 0xFFff) {
+                // allow OF1.0 Fake output "ports" to pass
+                return (short) (0xFFFF & of10PortNumber);
+            } else if (0xFF00 == of10PortNumber) {
+                // allow OFPP_MAX
+                return (short) (0xFFFF & of10PortNumber);
+            }
+            throw e;
+        }
+    }
+
+    /**
+     * Avoid instantiation.
+     */
+    private PortNumberUtils() {}
+
+}