Improved LinkInfo handling for ONOS

 * LinkInfo class is now immutable and uses primitive types
 * Removed logic to check STP state of ports. ONOS will not operate in
   networks using STP.
 * Cleaned up LinkInfo port state update logic to be clearer. We still
   maintain the port states in LinkInfo, though they are not used right now.
 * Removed the port updates that were sent by the link discovery module.
   ONOS does not use these, port updates are sent directly from the switch
   manager.


Change-Id: Icd2995de404948fa85b9e6852652bd4389d7cab8
diff --git a/src/main/java/net/onrc/onos/core/linkdiscovery/Link.java b/src/main/java/net/onrc/onos/core/linkdiscovery/Link.java
index 187e8b7..db67cd5 100644
--- a/src/main/java/net/onrc/onos/core/linkdiscovery/Link.java
+++ b/src/main/java/net/onrc/onos/core/linkdiscovery/Link.java
@@ -70,6 +70,14 @@
         return dstPort;
     }
 
+    public boolean isSrcPort(NodePortTuple npt) {
+        return npt.getNodeId() == src && npt.getPortId() == srcPort;
+    }
+
+    public boolean isDstPort(NodePortTuple npt) {
+        return npt.getNodeId() == dst && npt.getPortId() == dstPort;
+    }
+
     @Override
     public int hashCode() {
         final int prime = 31;
diff --git a/src/main/java/net/onrc/onos/core/linkdiscovery/LinkInfo.java b/src/main/java/net/onrc/onos/core/linkdiscovery/LinkInfo.java
index 9d8481a..0c2400c 100644
--- a/src/main/java/net/onrc/onos/core/linkdiscovery/LinkInfo.java
+++ b/src/main/java/net/onrc/onos/core/linkdiscovery/LinkInfo.java
@@ -1,6 +1,7 @@
 /**
- *    Copyright 2011, Big Switch Networks, Inc.*    Originally created by David Erickson, Stanford University
- **    Licensed under the Apache License, Version 2.0 (the "License"); you may
+ *    Copyright 2011, Big Switch Networks, Inc.
+ *    Originally created by David Erickson, Stanford University
+ *    Licensed under the Apache License, Version 2.0 (the "License"); you may
  *    not use this file except in compliance with the License. You may obtain
  *    a copy of the License at
  *
@@ -11,31 +12,18 @@
  *    WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
  *    License for the specific language governing permissions and limitations
  *    under the License.
- **/
+ */
 
 package net.onrc.onos.core.linkdiscovery;
 
 import net.onrc.onos.core.linkdiscovery.ILinkDiscovery.LinkType;
 
-import org.openflow.protocol.OFPhysicalPort.OFPortState;
+import com.google.common.primitives.Longs;
 
-public class LinkInfo {
-
-    public LinkInfo(Long firstSeenTime,
-                    Long lastLldpReceivedTime,
-                    int srcPortState,
-                    int dstPortState) {
-        super();
-        this.srcPortState = srcPortState;
-        this.dstPortState = dstPortState;
-        this.firstSeenTime = firstSeenTime;
-        this.lastLldpReceivedTime = lastLldpReceivedTime;
-    }
-
-    protected Integer srcPortState;
-    protected Integer dstPortState;
-    protected Long firstSeenTime;
-    protected Long lastLldpReceivedTime; /* Standard LLLDP received time */
+/**
+ * Records information about a link.
+ */
+public final class LinkInfo {
 
     /**
      * The port states stored here are topology's last knowledge of
@@ -46,51 +34,78 @@
      * new port state, so topology needs to keep its own copy so that
      * it can determine if the port state has changed and therefore
      * requires the new state to be written to storage.
+     *
+     * Note the port state values are defined in the OF 1.0 spec.
+     * These will change in some way once we move to OF 1.3.
      */
+    private final int srcPortState;
+    private final int dstPortState;
 
+    private final long firstSeenTime;
+    private final long lastLldpReceivedTime;
 
-    public boolean linkStpBlocked() {
-        return ((srcPortState & OFPortState.OFPPS_STP_MASK.getValue()) == OFPortState.OFPPS_STP_BLOCK.getValue()) ||
-                ((dstPortState & OFPortState.OFPPS_STP_MASK.getValue()) == OFPortState.OFPPS_STP_BLOCK.getValue());
+    /**
+     * Constructs a LinkInfo object.
+     *
+     * @param firstSeenTime the timestamp when the link was first seen
+     * @param lastLldpReceivedTime the timestamp when the link was last seen
+     * @param srcPortState the port state of the source port
+     * @param dstPortState the port state of the destination port
+     */
+    public LinkInfo(long firstSeenTime,
+            long lastLldpReceivedTime,
+            int srcPortState,
+            int dstPortState) {
+        this.srcPortState = srcPortState;
+        this.dstPortState = dstPortState;
+        this.firstSeenTime = firstSeenTime;
+        this.lastLldpReceivedTime = lastLldpReceivedTime;
     }
 
-    public Long getFirstSeenTime() {
+    /**
+     * Gets the timestamp when the link was first seen.
+     *
+     * @return the first seen timestamp
+     */
+    public long getFirstSeenTime() {
         return firstSeenTime;
     }
 
-    public void setFirstSeenTime(Long firstSeenTime) {
-        this.firstSeenTime = firstSeenTime;
-    }
-
-    public Long getUnicastValidTime() {
+    /**
+     * Gets the timestamp when the link was last seen.
+     *
+     * @return the last seen timestamp
+     */
+    public long getLastProbeReceivedTime() {
         return lastLldpReceivedTime;
     }
 
-    public void setUnicastValidTime(Long unicastValidTime) {
-        this.lastLldpReceivedTime = unicastValidTime;
-    }
-
-    public Integer getSrcPortState() {
+    /**
+     * Gets the state of the source port.
+     *
+     * @return the source port state, as defined in the OF1.0 spec
+     */
+    public int getSrcPortState() {
         return srcPortState;
     }
 
-    public void setSrcPortState(Integer srcPortState) {
-        this.srcPortState = srcPortState;
-    }
-
-    public Integer getDstPortState() {
+    /**
+     * Gets the state of the destination port.
+     *
+     * @return the destination port state, as defined in the OF1.0 spec
+     */
+    public int getDstPortState() {
         return dstPortState;
     }
 
-    public void setDstPortState(int dstPortState) {
-        this.dstPortState = dstPortState;
-    }
-
+    /**
+     * Gets the link type.
+     *
+     * @return the link type
+     * @see LinkType
+     */
     public LinkType getLinkType() {
-        if (lastLldpReceivedTime != null) {
-            return LinkType.DIRECT_LINK;
-        }
-        return LinkType.INVALID_LINK;
+        return LinkType.DIRECT_LINK;
     }
 
     /* (non-Javadoc)
@@ -100,10 +115,10 @@
     public int hashCode() {
         final int prime = 5557;
         int result = 1;
-        result = prime * result + ((firstSeenTime == null) ? 0 : firstSeenTime.hashCode());
-        result = prime * result + ((lastLldpReceivedTime == null) ? 0 : lastLldpReceivedTime.hashCode());
-        result = prime * result + ((srcPortState == null) ? 0 : srcPortState.hashCode());
-        result = prime * result + ((dstPortState == null) ? 0 : dstPortState.hashCode());
+        result = prime * result + Longs.hashCode(firstSeenTime);
+        result = prime * result + Longs.hashCode(lastLldpReceivedTime);
+        result = prime * result + srcPortState;
+        result = prime * result + dstPortState;
         return result;
     }
 
@@ -115,47 +130,17 @@
         if (this == obj) {
             return true;
         }
-        if (obj == null) {
-            return false;
-        }
+
         if (!(obj instanceof LinkInfo)) {
             return false;
         }
+
         LinkInfo other = (LinkInfo) obj;
 
-        if (firstSeenTime == null) {
-            if (other.firstSeenTime != null) {
-                return false;
-            }
-        } else if (!firstSeenTime.equals(other.firstSeenTime)) {
-            return false;
-        }
-
-        if (lastLldpReceivedTime == null) {
-            if (other.lastLldpReceivedTime != null) {
-                return false;
-            }
-        } else if (!lastLldpReceivedTime.equals(other.lastLldpReceivedTime)) {
-            return false;
-        }
-
-        if (srcPortState == null) {
-            if (other.srcPortState != null) {
-                return false;
-            }
-        } else if (!srcPortState.equals(other.srcPortState)) {
-            return false;
-        }
-
-        if (dstPortState == null) {
-            if (other.dstPortState != null) {
-                return false;
-            }
-        } else if (!dstPortState.equals(other.dstPortState)) {
-            return false;
-        }
-
-        return true;
+        return firstSeenTime == other.firstSeenTime &&
+               lastLldpReceivedTime == other.lastLldpReceivedTime &&
+               srcPortState == other.srcPortState &&
+               dstPortState == other.dstPortState;
     }
 
 
@@ -164,9 +149,9 @@
      */
     @Override
     public String toString() {
-        return "LinkInfo [unicastValidTime=" + ((lastLldpReceivedTime == null) ? "null" : lastLldpReceivedTime)
-                + ", srcPortState=" + ((srcPortState == null) ? "null" : srcPortState)
-                + ", dstPortState=" + ((dstPortState == null) ? "null" : srcPortState)
+        return "LinkInfo [unicastValidTime=" + lastLldpReceivedTime
+                + ", srcPortState=" + srcPortState
+                + ", dstPortState=" + dstPortState
                 + "]";
     }
 }
diff --git a/src/main/java/net/onrc/onos/core/linkdiscovery/internal/LinkDiscoveryManager.java b/src/main/java/net/onrc/onos/core/linkdiscovery/internal/LinkDiscoveryManager.java
index 22baf58..54592fb 100644
--- a/src/main/java/net/onrc/onos/core/linkdiscovery/internal/LinkDiscoveryManager.java
+++ b/src/main/java/net/onrc/onos/core/linkdiscovery/internal/LinkDiscoveryManager.java
@@ -258,10 +258,7 @@
 
     @Override
     public ILinkDiscovery.LinkType getLinkType(Link lt, LinkInfo info) {
-        if (info.getUnicastValidTime() != null) {
-            return ILinkDiscovery.LinkType.DIRECT_LINK;
-        }
-        return ILinkDiscovery.LinkType.INVALID_LINK;
+        return ILinkDiscovery.LinkType.DIRECT_LINK;
     }
 
 
@@ -455,7 +452,7 @@
                 break;
             case PORT_STATUS:
                 if (msg instanceof OFPortStatus) {
-                    return this.handlePortStatus(sw.getId(), (OFPortStatus) msg);
+                    return this.handlePortStatus(sw, (OFPortStatus) msg);
                 }
                 break;
             default:
@@ -525,24 +522,18 @@
         // Store the time of update to this link, and push it out to routingEngine
         Link lt = new Link(remoteDpid, remotePort, iofSwitch.getId(), pi.getInPort());
 
-        Long firstSeenTime = System.currentTimeMillis();
+        LinkInfo linkInfo = new LinkInfo(System.currentTimeMillis(),
+                System.currentTimeMillis(), srcPortState, dstPortState);
 
-        Long lastLldpTime = System.currentTimeMillis();
-
-        LinkInfo newLinkInfo =
-                new LinkInfo(firstSeenTime, lastLldpTime,
-                        srcPortState, dstPortState);
-
-        addOrUpdateLink(lt, newLinkInfo);
+        addOrUpdateLink(lt, linkInfo);
 
         // Check if reverse link exists.
         // If it doesn't exist and if the forward link was seen
         // first seen within a small interval, send probe on the
         // reverse link.
-
         boolean isReverse = OnosLldp.isReverse(lldp);
 
-        newLinkInfo = links.get(lt);
+        LinkInfo newLinkInfo = links.get(lt);
         if (newLinkInfo != null && !isReverse) {
             Link reverseLink = new Link(lt.getDst(), lt.getDstPort(),
                     lt.getSrc(), lt.getSrcPort());
@@ -581,48 +572,23 @@
         return Command.CONTINUE;
     }
 
-    protected UpdateOperation getUpdateOperation(int srcPortState,
-                                                 int dstPortState) {
-        boolean added =
-                (((srcPortState &
-                        OFPortState.OFPPS_STP_MASK.getValue()) !=
-                        OFPortState.OFPPS_STP_BLOCK.getValue()) &&
-                        ((dstPortState &
-                                OFPortState.OFPPS_STP_MASK.getValue()) !=
-                                OFPortState.OFPPS_STP_BLOCK.getValue()));
-
-        if (added) {
-            return UpdateOperation.LINK_UPDATED;
-        }
-        return UpdateOperation.LINK_REMOVED;
-    }
-
-
-    protected UpdateOperation getUpdateOperation(int srcPortState) {
-        boolean portUp = ((srcPortState &
-                OFPortState.OFPPS_STP_MASK.getValue()) !=
-                OFPortState.OFPPS_STP_BLOCK.getValue());
-
-        if (portUp) {
-            return UpdateOperation.PORT_UP;
-        } else {
-            return UpdateOperation.PORT_DOWN;
-        }
-    }
-
-    protected boolean addOrUpdateLink(Link lt, LinkInfo newInfo) {
+    protected boolean addOrUpdateLink(Link lt, LinkInfo detectedLinkInfo) {
 
         NodePortTuple srcNpt, dstNpt;
         boolean linkChanged = false;
 
         lock.writeLock().lock();
         try {
-            // put the new info.  if an old info exists, it will be returned.
-            LinkInfo oldInfo = links.put(lt, newInfo);
-            if (oldInfo != null &&
-                    oldInfo.getFirstSeenTime() < newInfo.getFirstSeenTime()) {
-                newInfo.setFirstSeenTime(oldInfo.getFirstSeenTime());
-            }
+            LinkInfo existingInfo = links.get(lt);
+
+            LinkInfo newLinkInfo = new LinkInfo(
+                    ((existingInfo == null) ? detectedLinkInfo.getFirstSeenTime()
+                            : existingInfo.getFirstSeenTime()),
+                    detectedLinkInfo.getLastProbeReceivedTime(),
+                    detectedLinkInfo.getSrcPortState(),
+                    detectedLinkInfo.getDstPortState());
+            links.put(lt, newLinkInfo);
+
 
             if (log.isTraceEnabled()) {
                 log.trace("addOrUpdateLink: {}", lt);
@@ -634,7 +600,7 @@
             srcNpt = new NodePortTuple(lt.getSrc(), lt.getSrcPort());
             dstNpt = new NodePortTuple(lt.getDst(), lt.getDstPort());
 
-            if (oldInfo == null) {
+            if (existingInfo == null) {
                 // index it by switch source
                 if (!switchLinks.containsKey(lt.getSrc())) {
                     switchLinks.put(lt.getSrc(), new HashSet<Link>());
@@ -662,55 +628,13 @@
                 updateOperation = UpdateOperation.LINK_ADDED;
                 linkChanged = true;
 
-            } else {
-                // Since the link info is already there, we need to
-                // update the right fields.
-                if (newInfo.getUnicastValidTime() == null) {
-                    // This is due to a multicast LLDP, so copy the old unicast
-                    // value.
-                    if (oldInfo.getUnicastValidTime() != null) {
-                        newInfo.setUnicastValidTime(oldInfo.getUnicastValidTime());
-                    }
-                }
-
-                Long oldTime = oldInfo.getUnicastValidTime();
-                Long newTime = newInfo.getUnicastValidTime();
-                // the link has changed its state between openflow and non-openflow
-                // if the unicastValidTimes are null or not null
-                if (oldTime != null & newTime == null) {
-                    // openflow -> non-openflow transition
-                    // we need to add the link tuple to the portNOFLinks
-                    //addLinkToBroadcastDomain(lt);
-                    linkChanged = true;
-                } else if (oldTime == null & newTime != null) {
-                    // non-openflow -> openflow transition
-                    // we need to remove the link from the portNOFLinks
-                    //removeLinkFromBroadcastDomain(lt);
-                    linkChanged = true;
-                }
-
-                // Only update the port states if they've changed
-                if (newInfo.getSrcPortState().intValue() !=
-                        oldInfo.getSrcPortState().intValue() ||
-                        newInfo.getDstPortState().intValue() !=
-                                oldInfo.getDstPortState().intValue()) {
-                    linkChanged = true;
-                }
-
-                if (linkChanged) {
-                    updateOperation = getUpdateOperation(newInfo.getSrcPortState(),
-                            newInfo.getDstPortState());
-                    if (log.isTraceEnabled()) {
-                        log.trace("Updated link {}", lt);
-                    }
-                }
             }
 
             if (linkChanged) {
                 // find out if the link was added or removed here.
                 LinkUpdate update = new LinkUpdate(new LDUpdate(lt.getSrc(), lt.getSrcPort(),
                         lt.getDst(), lt.getDstPort(),
-                        getLinkType(lt, newInfo),
+                        getLinkType(lt, newLinkInfo),
                         updateOperation));
                 controller.publishUpdate(update);
             }
@@ -732,13 +656,11 @@
      * @param linksToDelete The List of @LinkTuple to delete.
      */
     protected void deleteLinks(List<Link> linksToDelete, String reason) {
-        NodePortTuple srcNpt, dstNpt;
-
         lock.writeLock().lock();
         try {
             for (Link lt : linksToDelete) {
-                srcNpt = new NodePortTuple(lt.getSrc(), lt.getSrcPort());
-                dstNpt = new NodePortTuple(lt.getDst(), lt.getDstPort());
+                NodePortTuple srcNpt = new NodePortTuple(lt.getSrc(), lt.getSrcPort());
+                NodePortTuple dstNpt = new NodePortTuple(lt.getDst(), lt.getDstPort());
 
                 switchLinks.get(lt.getSrc()).remove(lt);
                 switchLinks.get(lt.getDst()).remove(lt);
@@ -791,22 +713,17 @@
      * @param ps The OFPortStatus message
      * @return The Command to continue or stop after we process this message
      */
-    protected Command handlePortStatus(long sw, OFPortStatus ps) {
-
-        IOFSwitch iofSwitch = floodlightProvider.getSwitches().get(sw);
-        if (iofSwitch == null) {
-            return Command.CONTINUE;
-        }
+    protected Command handlePortStatus(IOFSwitch sw, OFPortStatus ps) {
 
         // ONOS: If we do not control this switch, then we should not process its port status messages
-        if (!registryService.hasControl(iofSwitch.getId())) {
+        if (!registryService.hasControl(sw.getId())) {
             return Command.CONTINUE;
         }
 
         if (log.isTraceEnabled()) {
             log.trace("handlePortStatus: Switch {} port #{} reason {}; " +
                     "config is {} state is {}",
-                    new Object[]{iofSwitch.getStringId(),
+                    new Object[]{sw.getStringId(),
                             ps.getDesc().getPortNumber(),
                             ps.getReason(),
                             ps.getDesc().getConfig(),
@@ -814,7 +731,7 @@
         }
 
         short port = ps.getDesc().getPortNumber();
-        NodePortTuple npt = new NodePortTuple(sw, port);
+        NodePortTuple npt = new NodePortTuple(sw.getId(), port);
         boolean linkDeleted = false;
         boolean linkInfoChanged = false;
 
@@ -825,9 +742,8 @@
             if ((byte) OFPortReason.OFPPR_DELETE.ordinal() == ps.getReason() ||
                     ((byte) OFPortReason.OFPPR_MODIFY.ordinal() ==
                             ps.getReason() && !portEnabled(ps.getDesc()))) {
+
                 deleteLinksOnPort(npt, "Port Status Changed");
-                LinkUpdate update = new LinkUpdate(new LDUpdate(sw, port, UpdateOperation.PORT_DOWN));
-                controller.publishUpdate(update);
                 linkDeleted = true;
             } else if (ps.getReason() ==
                     (byte) OFPortReason.OFPPR_MODIFY.ordinal()) {
@@ -838,52 +754,42 @@
                     for (Link lt : this.portLinks.get(npt)) {
                         LinkInfo linkInfo = links.get(lt);
                         assert (linkInfo != null);
-                        Integer updatedSrcPortState = null;
-                        Integer updatedDstPortState = null;
-                        if (lt.getSrc() == npt.getNodeId() &&
-                                lt.getSrcPort() == npt.getPortId() &&
-                                (linkInfo.getSrcPortState() !=
-                                        ps.getDesc().getState())) {
-                            updatedSrcPortState = ps.getDesc().getState();
-                            linkInfo.setSrcPortState(updatedSrcPortState);
-                        }
-                        if (lt.getDst() == npt.getNodeId() &&
-                                lt.getDstPort() == npt.getPortId() &&
-                                (linkInfo.getDstPortState() !=
-                                        ps.getDesc().getState())) {
-                            updatedDstPortState = ps.getDesc().getState();
-                            linkInfo.setDstPortState(updatedDstPortState);
-                        }
-                        if ((updatedSrcPortState != null) ||
-                                (updatedDstPortState != null)) {
-                            // The link is already known to link discovery
-                            // manager and the status has changed, therefore
-                            // send an LinkUpdate.
-                            UpdateOperation operation =
-                                    getUpdateOperation(linkInfo.getSrcPortState(),
-                                            linkInfo.getDstPortState());
-                            LinkUpdate update = new LinkUpdate(new LDUpdate(lt.getSrc(), lt.getSrcPort(),
-                                    lt.getDst(), lt.getDstPort(),
-                                    getLinkType(lt, linkInfo),
-                                    operation));
-                            controller.publishUpdate(update);
+                        LinkInfo newLinkInfo = null;
 
+                        if (lt.isSrcPort(npt) &&
+                                linkInfo.getSrcPortState() != ps.getDesc().getState()) {
+                            // If this port status is for the src port and the port
+                            // state has changed, create a new link info with the new state
+
+                            newLinkInfo = new LinkInfo(linkInfo.getFirstSeenTime(),
+                                    linkInfo.getLastProbeReceivedTime(),
+                                    ps.getDesc().getState(),
+                                    linkInfo.getDstPortState());
+                        } else if (lt.isDstPort(npt) &&
+                                linkInfo.getDstPortState() != ps.getDesc().getState()) {
+                            // If this port status is for the dst port and the port
+                            // state has changed, create a new link info with the new state
+
+                            newLinkInfo = new LinkInfo(linkInfo.getFirstSeenTime(),
+                                    linkInfo.getLastProbeReceivedTime(),
+                                    linkInfo.getSrcPortState(),
+                                    ps.getDesc().getState());
+                        }
+
+                        if (newLinkInfo != null) {
                             linkInfoChanged = true;
+                            links.put(lt, newLinkInfo);
                         }
                     }
                 }
-
-                UpdateOperation operation =
-                        getUpdateOperation(ps.getDesc().getState());
-                LinkUpdate update = new LinkUpdate(new LDUpdate(sw, port, operation));
-                controller.publishUpdate(update);
             }
 
+
             if (!linkDeleted && !linkInfoChanged) {
                 if (log.isTraceEnabled()) {
                     log.trace("handlePortStatus: Switch {} port #{} reason {};" +
                             " no links to update/remove",
-                            new Object[]{HexString.toHexString(sw),
+                            new Object[]{HexString.toHexString(sw.getId()),
                                     ps.getDesc().getPortNumber(),
                                     ps.getReason()});
                 }
@@ -1008,7 +914,6 @@
     protected void timeoutLinks() {
         List<Link> eraseList = new ArrayList<Link>();
         Long curTime = System.currentTimeMillis();
-        boolean linkChanged = false;
 
         // reentrant required here because deleteLink also write locks
         lock.writeLock().lock();
@@ -1017,52 +922,15 @@
                     this.links.entrySet().iterator();
             while (it.hasNext()) {
                 Entry<Link, LinkInfo> entry = it.next();
-                Link lt = entry.getKey();
                 LinkInfo info = entry.getValue();
 
-                // Timeout the unicast and multicast LLDP valid times
-                // independently.
-                if ((info.getUnicastValidTime() != null) &&
-                        (info.getUnicastValidTime() + (1000L * LINK_TIMEOUT) < curTime)) {
-                    info.setUnicastValidTime(null);
-
-                    //if (info.getMulticastValidTime() != null) {
-                        //addLinkToBroadcastDomain(lt);
-                    //}
-                    // Note that even if mTime becomes null later on,
-                    // the link would be deleted, which would trigger updateClusters().
-                    linkChanged = true;
-                }
-                /*if ((info.getMulticastValidTime() != null) &&
-                        (info.getMulticastValidTime() + (1000L * LINK_TIMEOUT) < curTime)) {
-                    info.setMulticastValidTime(null);
-                    // if uTime is not null, then link will remain as openflow
-                    // link. If uTime is null, it will be deleted.  So, we
-                    // don't care about linkChanged flag here.
-                    removeLinkFromBroadcastDomain(lt);
-                    linkChanged = true;
-                }*/
-                // Add to the erase list only if the unicast
-                // time is null.
-                if (info.getUnicastValidTime() == null) { //&&
-                        //info.getMulticastValidTime() == null) {
+                if ((info.getLastProbeReceivedTime() + (1000L * LINK_TIMEOUT)
+                        < curTime)) {
                     eraseList.add(entry.getKey());
-                } else if (linkChanged) {
-                    UpdateOperation operation;
-                    operation = getUpdateOperation(info.getSrcPortState(),
-                            info.getDstPortState());
-                    LinkUpdate update = new LinkUpdate(new LDUpdate(lt.getSrc(), lt.getSrcPort(),
-                            lt.getDst(), lt.getDstPort(),
-                            getLinkType(lt, info),
-                            operation));
-                    controller.publishUpdate(update);
                 }
             }
 
-            // if any link was deleted or any link was changed.
-            if ((eraseList.size() > 0) || linkChanged) {
-                deleteLinks(eraseList, "LLDP timeout");
-            }
+            deleteLinks(eraseList, "LLDP timeout");
         } finally {
             lock.writeLock().unlock();
         }
@@ -1078,9 +946,6 @@
         if ((OFPortState.OFPPS_LINK_DOWN.getValue() & port.getState()) > 0) {
             return false;
         }
-        // Port STP state doesn't work with multiple VLANs, so ignore it for now
-        // if ((port.getState() & OFPortState.OFPPS_STP_MASK.getValue()) == OFPortState.OFPPS_STP_BLOCK.getValue())
-        //    return false;
         return true;
     }
 
diff --git a/src/test/java/net/onrc/onos/core/linkdiscovery/internal/LinkDiscoveryManagerTest.java b/src/test/java/net/onrc/onos/core/linkdiscovery/internal/LinkDiscoveryManagerTest.java
index dd748ac..22ec42e 100644
--- a/src/test/java/net/onrc/onos/core/linkdiscovery/internal/LinkDiscoveryManagerTest.java
+++ b/src/test/java/net/onrc/onos/core/linkdiscovery/internal/LinkDiscoveryManagerTest.java
@@ -42,12 +42,15 @@
 import net.onrc.onos.core.linkdiscovery.Link;
 import net.onrc.onos.core.linkdiscovery.LinkInfo;
 import net.onrc.onos.core.linkdiscovery.NodePortTuple;
+import net.onrc.onos.core.registry.IControllerRegistryService;
 
 import org.easymock.EasyMock;
 import org.junit.Before;
 import org.junit.Test;
 import org.openflow.protocol.OFMessage;
 import org.openflow.protocol.OFPhysicalPort;
+import org.openflow.protocol.OFPortStatus;
+import org.openflow.protocol.OFPortStatus.OFPortReason;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 
@@ -95,6 +98,11 @@
         ldm.linkDiscoveryAware = new ArrayList<ILinkDiscoveryListener>();
         MockThreadPoolService tp = new MockThreadPoolService();
         RestApiServer restApi = new RestApiServer();
+        IControllerRegistryService registry =
+                EasyMock.createMock(IControllerRegistryService.class);
+        expect(registry.hasControl(EasyMock.anyLong())).andReturn(true).anyTimes();
+        replay(registry);
+        cntx.addService(IControllerRegistryService.class, registry);
         cntx.addService(IRestApiService.class, restApi);
         cntx.addService(IThreadPoolService.class, tp);
         cntx.addService(ILinkDiscoveryService.class, ldm);
@@ -120,7 +128,8 @@
         LinkDiscoveryManager topology = getTopology();
 
         Link lt = new Link(1L, 2, 2L, 1);
-        LinkInfo info = new LinkInfo(System.currentTimeMillis(),
+        long firstSeenTime = System.currentTimeMillis();
+        LinkInfo info = new LinkInfo(firstSeenTime,
                 System.currentTimeMillis(), 0, 0);
         topology.addOrUpdateLink(lt, info);
 
@@ -136,6 +145,29 @@
         assertNotNull(topology.portLinks.get(dstNpt));
         assertTrue(topology.portLinks.get(dstNpt).contains(lt));
         assertTrue(topology.links.containsKey(lt));
+
+        LinkInfo infoToVerify = topology.links.get(lt);
+        assertEquals(firstSeenTime, infoToVerify.getFirstSeenTime());
+        assertEquals(0, infoToVerify.getSrcPortState());
+        assertEquals(0, infoToVerify.getDstPortState());
+
+        // Arbitrary new port states to verify that the port state is updated
+        final int newSrcPortState = 1;
+        final int newDstPortState = 2;
+
+        // Update the last received probe timestamp and the port states
+        LinkInfo infoWithStateChange = new LinkInfo(System.currentTimeMillis(),
+                System.currentTimeMillis(), newSrcPortState, newDstPortState);
+
+        topology.addOrUpdateLink(lt, infoWithStateChange);
+
+        assertNotNull(topology.links.get(lt));
+        infoToVerify = topology.links.get(lt);
+        // First seen time should be the original time, not the second update time
+        assertEquals(firstSeenTime, infoToVerify.getFirstSeenTime());
+        // Both port states should have been updated
+        assertEquals(newSrcPortState, infoToVerify.getSrcPortState());
+        assertEquals(newDstPortState, infoToVerify.getDstPortState());
     }
 
     @Test
@@ -270,24 +302,14 @@
 
         topology.timeoutLinks();
 
-
-        info = new LinkInfo(System.currentTimeMillis(), /* firstseen */
-                null, /* unicast */0, 0);
-        topology.addOrUpdateLink(lt, info);
-        assertTrue(topology.links.get(lt).getUnicastValidTime() == null);
-
-
         // Add a link info based on info that would be obtained from unicast LLDP
         // Setting the unicast LLDP reception time to be 40 seconds old, so we can use
-        // this to test timeout after this test.  Although the info is initialized
-        // with LT_OPENFLOW_LINK, the link property should be changed to LT_NON_OPENFLOW
-        // by the addOrUpdateLink method.
+        // this to test timeout after this test.
         info = new LinkInfo(System.currentTimeMillis() - 40000,
                 System.currentTimeMillis() - 40000, 0, 0);
         topology.addOrUpdateLink(lt, info);
 
-        // Expect to timeout the unicast Valid Time, but not the multicast Valid time
-        // So the link type should go back to non-openflow link.
+        // Expect to timeout the unicast Valid Time, so the link should disappear
         topology.timeoutLinks();
         assertTrue(topology.links.get(lt) == null);
     }
@@ -331,4 +353,87 @@
 
         verify(swTest);
     }
+
+    @Test
+    public void testHandlePortStatusForNewPort() throws IOException {
+        byte[] macAddress = new byte[] {0x0, 0x0, 0x0, 0x0, 0x0, 0x1};
+
+        LinkDiscoveryManager linkDiscovery = getTopology();
+
+        long dpid = 3L;
+        IOFSwitch sw = createMockSwitch(dpid);
+        getMockFloodlightProvider().getSwitches().put(dpid, sw);
+
+        short portNum = 1;
+        OFPhysicalPort ofpPort = new OFPhysicalPort();
+        ofpPort.setPortNumber(portNum);
+        ofpPort.setHardwareAddress(macAddress);
+
+        OFPortStatus portStatus = new OFPortStatus();
+        portStatus.setDesc(ofpPort);
+        portStatus.setReason((byte) OFPortReason.OFPPR_ADD.ordinal());
+
+        expect(sw.getPort(portNum)).andReturn(ofpPort).anyTimes();
+        sw.write(EasyMock.anyObject(OFMessage.class),
+                EasyMock.anyObject(FloodlightContext.class));
+        sw.flush();
+
+        replay(sw);
+
+        linkDiscovery.handlePortStatus(sw, portStatus);
+
+        verify(sw);
+    }
+
+    @Test
+    public void testHandlePortStatusForExistingPort() {
+        byte[] macAddress = new byte[] {0x0, 0x0, 0x0, 0x0, 0x0, 0x1};
+
+        LinkDiscoveryManager linkDiscovery = getTopology();
+
+        // Add a link that we can update later during the test
+        Link lt = new Link(1L, 1, 2L, 1);
+        LinkInfo info = new LinkInfo(System.currentTimeMillis(),
+                System.currentTimeMillis(), 0, 0);
+        linkDiscovery.addOrUpdateLink(lt, info);
+
+        short portNum = 1;
+        // src port
+        int srcPortState = 2;
+        OFPhysicalPort srcPort = new OFPhysicalPort();
+        srcPort.setPortNumber(portNum);
+        srcPort.setHardwareAddress(macAddress);
+        srcPort.setState(srcPortState);
+
+        // dst port
+        int dstPortState = 4;
+        OFPhysicalPort dstPort = new OFPhysicalPort();
+        dstPort.setPortNumber(portNum);
+        dstPort.setHardwareAddress(macAddress);
+        dstPort.setState(dstPortState);
+
+        OFPortStatus srcPortStatus = new OFPortStatus();
+        srcPortStatus.setDesc(srcPort);
+        srcPortStatus.setReason((byte) OFPortReason.OFPPR_MODIFY.ordinal());
+
+        OFPortStatus dstPortStatus = new OFPortStatus();
+        dstPortStatus.setDesc(dstPort);
+        dstPortStatus.setReason((byte) OFPortReason.OFPPR_MODIFY.ordinal());
+
+        linkDiscovery.handlePortStatus(
+                getMockFloodlightProvider().getSwitches().get(1L), srcPortStatus);
+
+
+        LinkInfo newInfo = linkDiscovery.links.get(lt);
+        assertEquals(srcPortState, newInfo.getSrcPortState());
+        assertEquals(0, newInfo.getDstPortState());
+
+
+        linkDiscovery.handlePortStatus(
+                getMockFloodlightProvider().getSwitches().get(2L), dstPortStatus);
+
+        newInfo = linkDiscovery.links.get(lt);
+        assertEquals(srcPortState, newInfo.getSrcPortState());
+        assertEquals(dstPortState, newInfo.getDstPortState());
+    }
 }