General link discovery module cleanup.

* Removed all events we don't use from the event system (switch and port
  events, link updates)
* Refactored event interface to two separate methods to match the other event
  interfaces
* Removed the LDUpdate class and moved all the event enums and classes to be
  internal to the LinkDiscoveryManager
* Removed all LinkTypes we no longer used and moved the one remaining type
  to the ILinkDiscoveryService. After this the ILinkDiscovery interface is
  no longer needed.
* Made Link immutable
* Removed the linkdiscovery.internal package as it only contained one class
* Readability improvements to LinkDiscoveryManager

Change-Id: Ifae97879aadc49b70a7b3d2294dcc540538c2cfc
diff --git a/src/main/java/net/floodlightcontroller/core/internal/Controller.java b/src/main/java/net/floodlightcontroller/core/internal/Controller.java
index 7374803..ae362a2 100644
--- a/src/main/java/net/floodlightcontroller/core/internal/Controller.java
+++ b/src/main/java/net/floodlightcontroller/core/internal/Controller.java
@@ -1088,7 +1088,7 @@
             // and attempted to be written to the database before the port is in
             // the database. We now suppress link discovery on ports until we're
             // sure they're in the database.
-            linkDiscovery.addToSuppressLLDPs(sw.getId(), port.getPortNumber());
+            linkDiscovery.disableDiscoveryOnPort(sw.getId(), port.getPortNumber());
 
             sw.setPort(port);
             SwitchUpdate update = new SwitchUpdate(sw, port, SwitchUpdateType.PORTADDED);
@@ -1341,7 +1341,7 @@
         // the database. We now suppress link discovery on ports until we're
         // sure they're in the database.
         for (OFPhysicalPort port : sw.getPorts()) {
-            linkDiscovery.addToSuppressLLDPs(sw.getId(), port.getPortNumber());
+            linkDiscovery.disableDiscoveryOnPort(sw.getId(), port.getPortNumber());
         }
 
         // TODO: is it safe to modify the HashMap without holding 
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 b8d8e87..e130e67 100644
--- a/src/main/java/net/onrc/onos/apps/sdnip/SdnIp.java
+++ b/src/main/java/net/onrc/onos/apps/sdnip/SdnIp.java
@@ -14,8 +14,6 @@
 import java.util.concurrent.ExecutorService;
 import java.util.concurrent.Executors;
 import java.util.concurrent.LinkedBlockingQueue;
-import java.util.concurrent.ScheduledExecutorService;
-import java.util.concurrent.TimeUnit;
 
 import net.floodlightcontroller.core.IFloodlightProviderService;
 import net.floodlightcontroller.core.IOFSwitch;
@@ -24,7 +22,6 @@
 import net.floodlightcontroller.core.module.FloodlightModuleException;
 import net.floodlightcontroller.core.module.IFloodlightModule;
 import net.floodlightcontroller.core.module.IFloodlightService;
-import net.floodlightcontroller.core.util.SingletonTask;
 import net.floodlightcontroller.restserver.IRestApiService;
 import net.floodlightcontroller.util.MACAddress;
 import net.onrc.onos.apps.proxyarp.IArpRequester;
@@ -36,7 +33,6 @@
 import net.onrc.onos.core.intent.IntentOperationList;
 import net.onrc.onos.core.intent.ShortestPathIntent;
 import net.onrc.onos.core.intent.runtime.IPathCalcRuntimeService;
-import net.onrc.onos.core.linkdiscovery.ILinkDiscovery.LDUpdate;
 import net.onrc.onos.core.linkdiscovery.ILinkDiscoveryService;
 import net.onrc.onos.core.main.config.IConfigInfoService;
 import net.onrc.onos.core.packet.Ethernet;
@@ -149,8 +145,7 @@
     // True when we have a full mesh of shortest paths between gateways
     private volatile boolean topologyReady = false;
 
-    private List<LDUpdate> linkUpdates;
-    private SingletonTask topologyChangeDetectorTask;
+    //private SingletonTask topologyChangeDetectorTask;
 
     private SetMultimap<InetAddress, RibUpdate> prefixesWaitingOnArp;
 
@@ -168,12 +163,12 @@
     // TODO: Fix for the new Topology Network Graph
     // private volatile Topology topology = null;
 
+    /*
     private class TopologyChangeDetector implements Runnable {
         @Override
         public void run() {
             log.debug("Running topology change detection task");
             // TODO: Fix the code below after topoLinkService was removed
-            /*
             synchronized (linkUpdates) {
 
                 ITopoLinkService topoLinkService = new TopoLinkServiceImpl();
@@ -191,7 +186,6 @@
                     }
                 }
             }
-            */
 
             if (!topologyReady) {
                 if (linkUpdates.isEmpty()) {
@@ -207,6 +201,7 @@
             }
         }
     }
+    */
 
     private void readConfiguration(String configFilename) {
         File gatewaysFile = new File(configFilename);
@@ -295,9 +290,9 @@
 
         controllerRegistryService = context.getServiceImpl(IControllerRegistryService.class);
         pathRuntime = context.getServiceImpl(IPathCalcRuntimeService.class);
-        linkUpdates = new ArrayList<>();
-        ScheduledExecutorService executor = Executors.newScheduledThreadPool(1);
-        topologyChangeDetectorTask = new SingletonTask(executor, new TopologyChangeDetector());
+
+        //ScheduledExecutorService executor = Executors.newScheduledThreadPool(1);
+        //topologyChangeDetectorTask = new SingletonTask(executor, new TopologyChangeDetector());
 
         pathsWaitingOnArp = new HashMap<>();
         prefixesWaitingOnArp = Multimaps.synchronizedSetMultimap(
@@ -843,7 +838,7 @@
         //Suppress link discovery on external-facing router ports
 
         for (Interface intf : interfaces.values()) {
-            linkDiscoveryService.addToSuppressLLDPs(intf.getDpid(), intf.getPort());
+            linkDiscoveryService.disableDiscoveryOnPort(intf.getDpid(), intf.getPort());
         }
 
         bgpUpdatesExecutor.execute(new Runnable() {
@@ -1253,7 +1248,7 @@
 
         //Suppress link discovery on external-facing router ports
         for (Interface intf : interfaces.values()) {
-            linkDiscoveryService.addToSuppressLLDPs(intf.getDpid(), intf.getPort());
+            linkDiscoveryService.disableDiscoveryOnPort(intf.getDpid(), intf.getPort());
         }
 
         bgpUpdatesExecutor.execute(new Runnable() {
diff --git a/src/main/java/net/onrc/onos/core/linkdiscovery/ILinkDiscovery.java b/src/main/java/net/onrc/onos/core/linkdiscovery/ILinkDiscovery.java
deleted file mode 100644
index bfc19d2..0000000
--- a/src/main/java/net/onrc/onos/core/linkdiscovery/ILinkDiscovery.java
+++ /dev/null
@@ -1,174 +0,0 @@
-package net.onrc.onos.core.linkdiscovery;
-
-import net.floodlightcontroller.core.IUpdate;
-
-import org.codehaus.jackson.map.annotate.JsonSerialize;
-import org.codehaus.jackson.map.ser.std.ToStringSerializer;
-import org.openflow.util.HexString;
-
-public interface ILinkDiscovery {
-
-    @JsonSerialize(using = ToStringSerializer.class)
-    public enum UpdateOperation {
-        LINK_ADDED("Link Added"), // Operation Added by ONOS
-        LINK_UPDATED("Link Updated"),
-        LINK_REMOVED("Link Removed"),
-        SWITCH_UPDATED("Switch Updated"),
-        SWITCH_REMOVED("Switch Removed"),
-        PORT_UP("Port Up"),
-        PORT_DOWN("Port Down");
-
-        private String value;
-
-        UpdateOperation(String v) {
-            value = v;
-        }
-
-        @Override
-        public String toString() {
-            return value;
-        }
-    }
-
-    public class LDUpdate implements IUpdate {
-        protected long src;
-        protected short srcPort;
-        protected long dst;
-        protected short dstPort;
-        protected SwitchType srcType;
-        protected LinkType type;
-        protected UpdateOperation operation;
-
-        public LDUpdate(long src, short srcPort,
-                        long dst, short dstPort,
-                        ILinkDiscovery.LinkType type,
-                        UpdateOperation operation) {
-            this.src = src;
-            this.srcPort = srcPort;
-            this.dst = dst;
-            this.dstPort = dstPort;
-            this.type = type;
-            this.operation = operation;
-        }
-
-        public LDUpdate(LDUpdate old) {
-            this.src = old.src;
-            this.srcPort = old.srcPort;
-            this.dst = old.dst;
-            this.dstPort = old.dstPort;
-            this.srcType = old.srcType;
-            this.type = old.type;
-            this.operation = old.operation;
-        }
-
-        // For updtedSwitch(sw)
-        public LDUpdate(long switchId, SwitchType stype, UpdateOperation oper) {
-            this.operation = oper;
-            this.src = switchId;
-            this.srcType = stype;
-        }
-
-        // For port up or port down.
-        public LDUpdate(long sw, short port, UpdateOperation operation) {
-            this.src = sw;
-            this.srcPort = port;
-            this.operation = operation;
-        }
-
-        public long getSrc() {
-            return src;
-        }
-
-        public short getSrcPort() {
-            return srcPort;
-        }
-
-        public long getDst() {
-            return dst;
-        }
-
-        public short getDstPort() {
-            return dstPort;
-        }
-
-        public SwitchType getSrcType() {
-            return srcType;
-        }
-
-        public LinkType getType() {
-            return type;
-        }
-
-        public UpdateOperation getOperation() {
-            return operation;
-        }
-
-        public void setOperation(UpdateOperation operation) {
-            this.operation = operation;
-        }
-
-        @Override
-        public String toString() {
-            switch (operation) {
-                case LINK_ADDED:
-                case LINK_REMOVED:
-                case LINK_UPDATED:
-                    return "LDUpdate [operation=" + operation +
-                            ", src=" + HexString.toHexString(src)
-                            + ", srcPort=" + srcPort
-                            + ", dst=" + HexString.toHexString(dst)
-                            + ", dstPort=" + dstPort
-                            + ", type=" + type + "]";
-                case PORT_DOWN:
-                case PORT_UP:
-                    return "LDUpdate [operation=" + operation +
-                            ", src=" + HexString.toHexString(src)
-                            + ", srcPort=" + srcPort + "]";
-                case SWITCH_REMOVED:
-                case SWITCH_UPDATED:
-                    return "LDUpdate [operation=" + operation +
-                            ", src=" + HexString.toHexString(src) + "]";
-                default:
-                    return "LDUpdate: Unknown update.";
-            }
-        }
-
-        @Override
-        public void dispatch() {
-            // TODO Auto-generated method stub
-
-        }
-    }
-
-    public enum SwitchType {
-        BASIC_SWITCH,
-        CORE_SWITCH
-    }
-
-    public enum LinkType {
-        INVALID_LINK {
-            @Override
-            public String toString() {
-                return "invalid";
-            }
-        },
-        DIRECT_LINK {
-            @Override
-            public String toString() {
-                return "internal";
-            }
-        },
-        MULTIHOP_LINK {
-            @Override
-            public String toString() {
-                return "external";
-            }
-        },
-        TUNNEL {
-            @Override
-            public String toString() {
-                return "tunnel";
-            }
-        }
-    }
-}
diff --git a/src/main/java/net/onrc/onos/core/linkdiscovery/ILinkDiscoveryListener.java b/src/main/java/net/onrc/onos/core/linkdiscovery/ILinkDiscoveryListener.java
index 484616e..88d832b 100644
--- a/src/main/java/net/onrc/onos/core/linkdiscovery/ILinkDiscoveryListener.java
+++ b/src/main/java/net/onrc/onos/core/linkdiscovery/ILinkDiscoveryListener.java
@@ -17,7 +17,28 @@
 
 package net.onrc.onos.core.linkdiscovery;
 
-public interface ILinkDiscoveryListener extends ILinkDiscovery {
+/**
+ * Provides callbacks for link discovery events.
+ */
+public interface ILinkDiscoveryListener {
 
-    public void linkDiscoveryUpdate(LDUpdate update);
+    /**
+     * Called when a new link is detected. A link discovery probe has been
+     * received on a port, and the link was not previously known to the link
+     * discovery manager.
+     *
+     * @param link the new link that was detected
+     */
+    public void linkAdded(Link link);
+
+    /**
+     * Called when a link is removed. The link may have been removed because it
+     * timed out (no probes received on the destination port for an interval),
+     * or because the port is no longer available for link discovery, either
+     * because the switch was removed, the port went down, or link discovery
+     * was disabled on the port.
+     *
+     * @param link the link that was removed
+     */
+    public void linkRemoved(Link link);
 }
diff --git a/src/main/java/net/onrc/onos/core/linkdiscovery/ILinkDiscoveryService.java b/src/main/java/net/onrc/onos/core/linkdiscovery/ILinkDiscoveryService.java
index 8f232b1..0a3fc74 100644
--- a/src/main/java/net/onrc/onos/core/linkdiscovery/ILinkDiscoveryService.java
+++ b/src/main/java/net/onrc/onos/core/linkdiscovery/ILinkDiscoveryService.java
@@ -22,29 +22,36 @@
 
 import net.floodlightcontroller.core.module.IFloodlightService;
 
-
+/**
+ * Interface to the link discovery module.
+ */
 public interface ILinkDiscoveryService extends IFloodlightService {
     /**
+     * Represents the type of a link.
+     * <p/>
+     * This is a placeholder at the moment. Floodlight had defined a number of
+     * different link types which are irrelevant to us now we no longer use
+     * BDDP or support OpenFlow clusters.
+     * Currently we have no differentiation of link types, but in the future we
+     * may want to differentiate between intra-instance links and
+     * inter-instance links.
+     */
+    public enum LinkType {
+        DIRECT_LINK {
+            @Override
+            public String toString() {
+                return "internal";
+            }
+        }
+    }
+
+    /**
      * Retrieves a map of all known link connections between OpenFlow switches
      * and the associated info (valid time, port states) for the link.
      */
     public Map<Link, LinkInfo> getLinks();
 
     /**
-     * Returns link type of a given link.
-     *
-     * @param info
-     * @return the link type
-     */
-    public ILinkDiscovery.LinkType getLinkType(Link lt, LinkInfo info);
-
-    /**
-     * Returns an unmodifiable map from switch id to a set of all links with it
-     * as an endpoint.
-     */
-    public Map<Long, Set<Link>> getSwitchLinks();
-
-    /**
      * Adds a listener to listen for ILinkDiscoveryService messages.
      *
      * @param listener The listener that wants the notifications
@@ -52,17 +59,38 @@
     public void addListener(ILinkDiscoveryListener listener);
 
     /**
-     * Retrieves a set of all switch ports on which lldps are suppressed.
+     * Removes a link discovery listener.
+     *
+     * @param listener the listener to remove
      */
-    public Set<NodePortTuple> getSuppressLLDPsInfo();
+    public void removeListener(ILinkDiscoveryListener listener);
 
     /**
-     * Adds a switch port to suppress lldp set.
+     * Gets the set of switch ports on which link discovery is disabled.
      */
-    public void addToSuppressLLDPs(long sw, short port);
+    public Set<NodePortTuple> getDiscoveryDisabledPorts();
 
     /**
-     * Removes a switch port from suppress lldp set.
+     * Disables link discovery on a switch port. This method suppresses
+     * discovery probes from being sent from the port, and deletes any existing
+     * links that the discovery module has previously detected on the port.
+     *
+     * @param sw the dpid of the switch the port is on
+     * @param port the port number to disable discovery on
      */
-    public void removeFromSuppressLLDPs(long sw, short port);
+    public void disableDiscoveryOnPort(long sw, short port);
+
+    /**
+     * Enables link discovery on a switch port. Discovery probes will now be
+     * sent from the port and any links on the port will be discovered.
+     * <p/>
+     * Note: All ports are enabled for discovery by default, however this
+     * method is provided to re-enable link discovery if it had previously been
+     * disabled on the port by a call to
+     * {@link #disableDiscoveryOnPort(long, short)}.
+     *
+     * @param sw the dpid of the switch the port is on
+     * @param port the port number to enable discovery on
+     */
+    public void enableDiscoveryOnPort(long sw, short port);
 }
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 db67cd5..2d1508f 100644
--- a/src/main/java/net/onrc/onos/core/linkdiscovery/Link.java
+++ b/src/main/java/net/onrc/onos/core/linkdiscovery/Link.java
@@ -24,11 +24,11 @@
 import org.codehaus.jackson.map.annotate.JsonSerialize;
 import org.openflow.util.HexString;
 
-public class Link {
-    private long src;
-    private short srcPort;
-    private long dst;
-    private short dstPort;
+public final class Link {
+    private final long src;
+    private final short srcPort;
+    private final long dst;
+    private final short dstPort;
 
 
     public Link(long srcId, short srcPort, long dstId, short dstPort) {
diff --git a/src/main/java/net/onrc/onos/core/linkdiscovery/internal/LinkDiscoveryManager.java b/src/main/java/net/onrc/onos/core/linkdiscovery/LinkDiscoveryManager.java
similarity index 72%
rename from src/main/java/net/onrc/onos/core/linkdiscovery/internal/LinkDiscoveryManager.java
rename to src/main/java/net/onrc/onos/core/linkdiscovery/LinkDiscoveryManager.java
index 54592fb..db35809 100644
--- a/src/main/java/net/onrc/onos/core/linkdiscovery/internal/LinkDiscoveryManager.java
+++ b/src/main/java/net/onrc/onos/core/linkdiscovery/LinkDiscoveryManager.java
@@ -15,7 +15,7 @@
  *    under the License.
  **/
 
-package net.onrc.onos.core.linkdiscovery.internal;
+package net.onrc.onos.core.linkdiscovery;
 
 import java.io.IOException;
 import java.util.ArrayList;
@@ -29,6 +29,7 @@
 import java.util.Map;
 import java.util.Map.Entry;
 import java.util.Set;
+import java.util.concurrent.CopyOnWriteArrayList;
 import java.util.concurrent.ScheduledExecutorService;
 import java.util.concurrent.TimeUnit;
 import java.util.concurrent.locks.ReentrantReadWriteLock;
@@ -38,6 +39,7 @@
 import net.floodlightcontroller.core.IOFMessageListener;
 import net.floodlightcontroller.core.IOFSwitch;
 import net.floodlightcontroller.core.IOFSwitchListener;
+import net.floodlightcontroller.core.IUpdate;
 import net.floodlightcontroller.core.annotations.LogMessageCategory;
 import net.floodlightcontroller.core.annotations.LogMessageDoc;
 import net.floodlightcontroller.core.annotations.LogMessageDocs;
@@ -48,14 +50,6 @@
 import net.floodlightcontroller.core.util.SingletonTask;
 import net.floodlightcontroller.restserver.IRestApiService;
 import net.floodlightcontroller.threadpool.IThreadPoolService;
-import net.onrc.onos.core.linkdiscovery.ILinkDiscovery;
-import net.onrc.onos.core.linkdiscovery.ILinkDiscovery.LDUpdate;
-import net.onrc.onos.core.linkdiscovery.ILinkDiscovery.UpdateOperation;
-import net.onrc.onos.core.linkdiscovery.ILinkDiscoveryListener;
-import net.onrc.onos.core.linkdiscovery.ILinkDiscoveryService;
-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.linkdiscovery.web.LinkDiscoveryWebRoutable;
 import net.onrc.onos.core.packet.Ethernet;
 import net.onrc.onos.core.packet.LLDP;
@@ -81,39 +75,42 @@
 import org.slf4j.LoggerFactory;
 
 /**
- * This class sends out LLDP messages containing the sending switch's datapath
- * id as well as the outgoing port number.  Received LLDP messages that
- * match a known switch cause a new LinkTuple to be created according to the
- * invariant rules listed below.  This new LinkTuple is also passed to routing
- * if it exists to trigger updates.
+ * Discovers links between OpenFlow switches.
  * <p/>
- * This class also handles removing links that are associated to switch ports
- * that go down, and switches that are disconnected.
+ * Discovery is performed by sending probes (LLDP packets) over the links in
+ * the data plane. The LinkDiscoveryManager sends probes periodically on all
+ * ports on all connected switches. The probes contain the sending switch's
+ * DPID and outgoing port number. LLDP packets that are received (via an
+ * OpenFlow packet-in) indicate there is a link between the receiving port and
+ * the sending port, which was encoded in the LLDP. When the
+ * LinkDiscoveryManager observes a new link, a Link object is created and an
+ * event is fired for any event listeners.
  * <p/>
- * Invariants:
- * -portLinks and switchLinks will not contain empty Sets outside of
- * critical sections.
- * -portLinks contains LinkTuples where one of the src or dst
- * SwitchPortTuple matches the map key.
- * -switchLinks contains LinkTuples where one of the src or dst
- * SwitchPortTuple's id matches the switch id.
- * -Each LinkTuple will be indexed into switchLinks for both
- * src.id and dst.id, and portLinks for each src and dst.
- * -The updates queue is only added to from within a held write lock.
+ * Links are removed for one of three reasons:
+ * <ul>
+ * <li>A probe has not been received on the link for an interval (the timeout
+ * interval)</li>
+ * <li>The port went down or was disabled (as observed by OpenFlow port-status
+ * messages) or the disconnection of the switch</li>
+ * <li>Link discovery was explicitly disabled on a port with the
+ * {@link #disableDiscoveryOnPort(long, short)} method</li>
+ * </ul>
+ * When the LinkDiscoveryManager removes a link it also fires an event for the
+ * listeners.
  */
 @LogMessageCategory("Network Topology")
-public class LinkDiscoveryManager
-        implements IOFMessageListener, IOFSwitchListener,
+public class LinkDiscoveryManager implements IOFMessageListener, IOFSwitchListener,
         ILinkDiscoveryService, IFloodlightModule {
-    protected IFloodlightProviderService controller;
-    private static final Logger log = LoggerFactory.getLogger(LinkDiscoveryManager.class);
 
-    protected IFloodlightProviderService floodlightProvider;
-    protected IThreadPoolService threadPool;
-    protected IRestApiService restApi;
-    // Registry Service for ONOS
-    protected IControllerRegistryService registryService;
+    private static final Logger log =
+            LoggerFactory.getLogger(LinkDiscoveryManager.class);
 
+    private IFloodlightProviderService controller;
+
+    private IFloodlightProviderService floodlightProvider;
+    private IThreadPoolService threadPool;
+    private IRestApiService restApi;
+    private IControllerRegistryService registryService;
 
     // LLDP fields
     private static final byte[] LLDP_STANDARD_DST_MAC_STRING =
@@ -122,18 +119,17 @@
     private static final long LINK_LOCAL_VALUE = 0x0180c2000000L;
 
     // Link discovery task details.
-    protected SingletonTask discoveryTask;
-    protected static final int DISCOVERY_TASK_INTERVAL = 1;
-    protected static final int LINK_TIMEOUT = 35; // original 35 secs, aggressive 5 secs
-    protected static final int LLDP_TO_ALL_INTERVAL = 15; //original 15 seconds, aggressive 2 secs.
-    protected long lldpClock = 0;
+    private SingletonTask discoveryTask;
+    private static final int DISCOVERY_TASK_INTERVAL = 1;
+    private static final int LINK_TIMEOUT = 35; // original 35 secs, aggressive 5 secs
+    private static final int LLDP_TO_ALL_INTERVAL = 15; //original 15 seconds, aggressive 2 secs.
+    private long lldpClock = 0;
     // This value is intentionally kept higher than LLDP_TO_ALL_INTERVAL.
     // If we want to identify link failures faster, we could decrease this
     // value to a small number, say 1 or 2 sec.
-    protected static final int LLDP_TO_KNOWN_INTERVAL = 20; // LLDP frequency for known links
+    private static final int LLDP_TO_KNOWN_INTERVAL = 20; // LLDP frequency for known links
 
-    protected ReentrantReadWriteLock lock;
-    int lldpTimeCount = 0;
+    private ReentrantReadWriteLock lock;
 
     /**
      * Map from link to the most recent time it was verified functioning.
@@ -151,64 +147,54 @@
     protected Map<NodePortTuple, Set<Link>> portLinks;
 
     /**
-     * Topology aware components are called in the order they were added to the
-     * the array.
+     * Listeners are called in the order they were added to the the list.
      */
-    protected ArrayList<ILinkDiscoveryListener> linkDiscoveryAware;
+    private final List<ILinkDiscoveryListener> linkDiscoveryListeners
+            = new CopyOnWriteArrayList<>();
 
-    protected class LinkUpdate extends LDUpdate {
+    /**
+     * List of ports through which LLDPs are not sent.
+     */
+    private Set<NodePortTuple> suppressLinkDiscovery;
 
-        public LinkUpdate(LDUpdate old) {
-            super(old);
+    private enum UpdateType {
+        LINK_ADDED,
+        LINK_REMOVED
+    }
+
+    private class LinkUpdate implements IUpdate {
+        private final Link link;
+        private final UpdateType operation;
+
+        public LinkUpdate(Link link, UpdateType operation) {
+            this.link = link;
+            this.operation = operation;
         }
 
-        @LogMessageDoc(level = "ERROR",
-                message = "Error in link discovery updates loop",
-                explanation = "An unknown error occured while dispatching " +
-                        "link update notifications",
-                recommendation = LogMessageDoc.GENERIC_ACTION)
         @Override
         public void dispatch() {
-            if (linkDiscoveryAware != null) {
-                if (log.isTraceEnabled()) {
-                    log.trace("Dispatching link discovery update {} {} {} {} {} for {}",
-                            new Object[]{this.getOperation(),
-                                    HexString.toHexString(this.getSrc()), this.getSrcPort(),
-                                    HexString.toHexString(this.getDst()), this.getDstPort(),
-                                    linkDiscoveryAware});
-                }
-                try {
-                    for (ILinkDiscoveryListener lda : linkDiscoveryAware) { // order maintained
-                        lda.linkDiscoveryUpdate(this);
-                    }
-                } catch (Exception e) {
-                    log.error("Error in link discovery updates loop", e);
+            if (log.isTraceEnabled()) {
+                log.trace("Dispatching link discovery update {} for {}",
+                        operation, link);
+            }
+            for (ILinkDiscoveryListener listener : linkDiscoveryListeners) {
+                switch (operation) {
+                case LINK_ADDED:
+                    listener.linkAdded(link);
+                    break;
+                case LINK_REMOVED:
+                    listener.linkRemoved(link);
+                    break;
+                default:
+                    log.warn("Unknown link update operation {}", operation);
+                    break;
                 }
             }
         }
     }
 
     /**
-     * List of ports through which LLDP/BDDPs are not sent.
-     */
-    protected Set<NodePortTuple> suppressLinkDiscovery;
-
-    /**
-     * A list of ports that are quarantined for discovering links through
-     * them.  Data traffic from these ports are not allowed until the ports
-     * are released from quarantine.
-     */
-    //protected LinkedBlockingQueue<NodePortTuple> quarantineQueue;
-    //protected LinkedBlockingQueue<NodePortTuple> maintenanceQueue;
-
-    /**
-     * Map of broadcast domain ports and the last time a BDDP was either
-     * sent or received on that port.
-     */
-    protected Map<NodePortTuple, Long> broadcastDomainPortTimeMap;
-
-    /**
-     * Get the LLDP sending period in seconds.
+     * Gets the LLDP sending period in seconds.
      *
      * @return LLDP sending period in seconds.
      */
@@ -217,7 +203,7 @@
     }
 
     /**
-     * Get the LLDP timeout value in seconds.
+     * Gets the LLDP timeout value in seconds.
      *
      * @return LLDP timeout value in seconds
      */
@@ -225,51 +211,33 @@
         return LINK_TIMEOUT;
     }
 
-    public Map<NodePortTuple, Set<Link>> getPortLinks() {
-        return portLinks;
-    }
-
     @Override
-    public Set<NodePortTuple> getSuppressLLDPsInfo() {
+    public Set<NodePortTuple> getDiscoveryDisabledPorts() {
         return suppressLinkDiscovery;
     }
 
-    /**
-     * Add a switch port to the suppressed LLDP list.
-     * Remove any known links on the switch port.
-     */
     @Override
-    public void addToSuppressLLDPs(long sw, short port) {
+    public void disableDiscoveryOnPort(long sw, short port) {
         NodePortTuple npt = new NodePortTuple(sw, port);
         this.suppressLinkDiscovery.add(npt);
-        deleteLinksOnPort(npt, "LLDP suppressed.");
+        deleteLinksOnPort(npt);
     }
 
-    /**
-     * Remove a switch port from the suppressed LLDP list.
-     * Discover links on that switchport.
-     */
     @Override
-    public void removeFromSuppressLLDPs(long sw, short port) {
+    public void enableDiscoveryOnPort(long sw, short port) {
         NodePortTuple npt = new NodePortTuple(sw, port);
         this.suppressLinkDiscovery.remove(npt);
         discover(npt);
     }
 
-    @Override
-    public ILinkDiscovery.LinkType getLinkType(Link lt, LinkInfo info) {
-        return ILinkDiscovery.LinkType.DIRECT_LINK;
-    }
-
-
     private boolean isLinkDiscoverySuppressed(long sw, short portNumber) {
         return this.suppressLinkDiscovery.contains(new NodePortTuple(sw, portNumber));
     }
 
-    protected void discoverLinks() {
+    private void discoverLinks() {
 
-        // timeout known links.
-        timeoutLinks();
+        // time out known links.
+        timeOutLinks();
 
         //increment LLDP clock
         lldpClock = (lldpClock + 1) % LLDP_TO_ALL_INTERVAL;
@@ -294,26 +262,21 @@
         }
     }
 
-    protected void discover(NodePortTuple npt) {
+    private void discover(NodePortTuple npt) {
         discover(npt.getNodeId(), npt.getPortId());
     }
 
-    protected void discover(long sw, short port) {
+    private void discover(long sw, short port) {
         sendDiscoveryMessage(sw, port, false);
     }
 
     /**
      * Send link discovery message out of a given switch port.
-     * The discovery message may be a standard LLDP or a modified
-     * LLDP, where the dst mac address is set to :ff.
-     * <p/>
-     * TODO: The modified LLDP will updated in the future and may
-     * use a different eth-type.
+     * The discovery message is a standard LLDP containing ONOS-specific TLVs.
      *
-     * @param sw
-     * @param port
-     * @param isStandard indicates standard or modified LLDP
-     * @param isReverse  indicates whether the LLDP was sent as a response
+     * @param sw the switch to send on
+     * @param port the port to send out
+     * @param isReverse indicates whether the LLDP was sent as a response
      */
     @LogMessageDoc(level = "ERROR",
             message = "Failure sending LLDP out port {port} on switch {switch}",
@@ -342,9 +305,7 @@
         }
 
         if (isLinkDiscoverySuppressed(sw, port)) {
-            /* Dont send LLDPs out of this port as suppressLLDPs set
-             *
-             */
+            // Don't send LLDPs out of this port as suppressLLDPs set
             return;
         }
 
@@ -359,7 +320,8 @@
             iofSwitch.write(po, null);
             iofSwitch.flush();
         } catch (IOException e) {
-            log.error("Failure sending LLDP out port " + port + " on switch " + iofSwitch.getStringId(), e);
+            log.error("Failure sending LLDP out port " + port + " on switch "
+                    + iofSwitch.getStringId(), e);
         }
 
     }
@@ -416,22 +378,17 @@
         if (log.isTraceEnabled()) {
             log.trace("Sending LLDP packets out of all the enabled ports on switch");
         }
-        Set<Long> switches = floodlightProvider.getSwitches().keySet();
-        // Send standard LLDPs
-        for (long sw : switches) {
-            IOFSwitch iofSwitch = floodlightProvider.getSwitches().get(sw);
-            if (iofSwitch == null) {
+
+        for (IOFSwitch sw : floodlightProvider.getSwitches().values()) {
+            if (sw.getEnabledPorts() == null) {
                 continue;
             }
-            if (iofSwitch.getEnabledPorts() != null) {
-                for (OFPhysicalPort ofp : iofSwitch.getEnabledPorts()) {
-                    if (isLinkDiscoverySuppressed(sw, ofp.getPortNumber())) {
-                        continue;
-                    }
-
-                    // sends forward LLDP only non-fastports.
-                    sendDiscoveryMessage(sw, ofp.getPortNumber(), false);
+            for (OFPhysicalPort ofp : sw.getEnabledPorts()) {
+                if (isLinkDiscoverySuppressed(sw.getId(), ofp.getPortNumber())) {
+                    continue;
                 }
+
+                sendDiscoveryMessage(sw.getId(), ofp.getPortNumber(), false);
             }
         }
     }
@@ -487,7 +444,8 @@
         SwitchPort switchPort = OnosLldp.extractSwitchPort(packetData);
         long remoteDpid = switchPort.dpid().value();
         short remotePort = switchPort.port().value();
-        IOFSwitch remoteSwitch = floodlightProvider.getSwitches().get(switchPort.dpid().value());
+        IOFSwitch remoteSwitch = floodlightProvider.getSwitches().get(
+                switchPort.dpid().value());
 
 
         OFPhysicalPort physicalPort = null;
@@ -495,22 +453,24 @@
             physicalPort = remoteSwitch.getPort(remotePort);
             if (!remoteSwitch.portEnabled(remotePort)) {
                 if (log.isTraceEnabled()) {
-                    log.trace("Ignoring link with disabled source port: switch {} port {}", remoteSwitch, remotePort);
+                    log.trace("Ignoring link with disabled source port: " +
+                            "switch {} port {}", remoteSwitch, remotePort);
                 }
                 return Command.STOP;
             }
-            if (suppressLinkDiscovery.contains(new NodePortTuple(remoteSwitch.getId(),
-                    remotePort))) {
+            if (suppressLinkDiscovery.contains(
+                    new NodePortTuple(remoteSwitch.getId(), remotePort))) {
                 if (log.isTraceEnabled()) {
-                    log.trace("Ignoring link with suppressed src port: switch {} port {}",
-                            remoteSwitch, remotePort);
+                    log.trace("Ignoring link with suppressed src port: " +
+                            "switch {} port {}", remoteSwitch, remotePort);
                 }
                 return Command.STOP;
             }
         }
         if (!iofSwitch.portEnabled(pi.getInPort())) {
             if (log.isTraceEnabled()) {
-                log.trace("Ignoring link with disabled dest port: switch {} port {}", sw, pi.getInPort());
+                log.trace("Ignoring link with disabled dest port: " +
+                        "switch {} port {}", sw, pi.getInPort());
             }
             return Command.STOP;
         }
@@ -540,7 +500,8 @@
             LinkInfo reverseInfo = links.get(reverseLink);
             if (reverseInfo == null) {
                 // the reverse link does not exist.
-                if (newLinkInfo.getFirstSeenTime() > System.currentTimeMillis() - LINK_TIMEOUT) {
+                if (newLinkInfo.getFirstSeenTime() >
+                        System.currentTimeMillis() - LINK_TIMEOUT) {
                     this.sendDiscoveryMessage(lt.getDst(), lt.getDstPort(), true);
                 }
             }
@@ -572,11 +533,7 @@
         return Command.CONTINUE;
     }
 
-    protected boolean addOrUpdateLink(Link lt, LinkInfo detectedLinkInfo) {
-
-        NodePortTuple srcNpt, dstNpt;
-        boolean linkChanged = false;
-
+    protected void addOrUpdateLink(Link lt, LinkInfo detectedLinkInfo) {
         lock.writeLock().lock();
         try {
             LinkInfo existingInfo = links.get(lt);
@@ -587,19 +544,19 @@
                     detectedLinkInfo.getLastProbeReceivedTime(),
                     detectedLinkInfo.getSrcPortState(),
                     detectedLinkInfo.getDstPortState());
-            links.put(lt, newLinkInfo);
 
+            // Add new LinkInfo or update old LinkInfo
+            links.put(lt, newLinkInfo);
 
             if (log.isTraceEnabled()) {
                 log.trace("addOrUpdateLink: {}", lt);
             }
 
-            UpdateOperation updateOperation = null;
-            linkChanged = false;
+            NodePortTuple srcNpt = new NodePortTuple(lt.getSrc(), lt.getSrcPort());
+            NodePortTuple dstNpt = new NodePortTuple(lt.getDst(), lt.getDstPort());
 
-            srcNpt = new NodePortTuple(lt.getSrc(), lt.getSrcPort());
-            dstNpt = new NodePortTuple(lt.getDst(), lt.getDstPort());
-
+            // If this is the first time we've seen the link, add the Link
+            // object to the data structures/indexes as well
             if (existingInfo == null) {
                 // index it by switch source
                 if (!switchLinks.containsKey(lt.getSrc())) {
@@ -624,38 +581,20 @@
                 }
                 portLinks.get(dstNpt).add(lt);
 
-                // ONOS: Distinguish added event separately from updated event
-                updateOperation = UpdateOperation.LINK_ADDED;
-                linkChanged = true;
-
-            }
-
-            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, newLinkInfo),
-                        updateOperation));
-                controller.publishUpdate(update);
+                // Publish LINK_ADDED event
+                controller.publishUpdate(new LinkUpdate(lt, UpdateType.LINK_ADDED));
             }
         } finally {
             lock.writeLock().unlock();
         }
-
-        return linkChanged;
-    }
-
-    @Override
-    public Map<Long, Set<Link>> getSwitchLinks() {
-        return this.switchLinks;
     }
 
     /**
-     * Removes links from memory and storage.
+     * Removes links from the data structures.
      *
-     * @param linksToDelete The List of @LinkTuple to delete.
+     * @param linksToDelete the list of links to delete
      */
-    protected void deleteLinks(List<Link> linksToDelete, String reason) {
+    protected void deleteLinks(List<Link> linksToDelete) {
         lock.writeLock().lock();
         try {
             for (Link lt : linksToDelete) {
@@ -686,15 +625,10 @@
                     }
                 }
 
-                LinkInfo info = this.links.remove(lt);
-                LinkUpdate update = new LinkUpdate(new LDUpdate(lt.getSrc(), lt.getSrcPort(),
-                        lt.getDst(), lt.getDstPort(),
-                        getLinkType(lt, info),
-                        UpdateOperation.LINK_REMOVED));
-                controller.publishUpdate(update);
+                this.links.remove(lt);
 
-                // TODO  Whenever link is removed, it has to checked if
-                // the switchports must be added to quarantine.
+                controller.publishUpdate(new LinkUpdate(lt,
+                        UpdateType.LINK_REMOVED));
 
                 if (log.isTraceEnabled()) {
                     log.trace("Deleted link {}", lt);
@@ -709,13 +643,14 @@
      * Handles an OFPortStatus message from a switch. We will add or
      * delete LinkTupes as well re-compute the topology if needed.
      *
-     * @param sw The IOFSwitch that sent the port status message
+     * @param sw The dpid of the switch that sent the port status message
      * @param ps The OFPortStatus message
      * @return The Command to continue or stop after we process this message
      */
     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 we do not control this switch, then we should not process its
+        // port status messages
         if (!registryService.hasControl(sw.getId())) {
             return Command.CONTINUE;
         }
@@ -743,7 +678,7 @@
                     ((byte) OFPortReason.OFPPR_MODIFY.ordinal() ==
                             ps.getReason() && !portEnabled(ps.getDesc()))) {
 
-                deleteLinksOnPort(npt, "Port Status Changed");
+                deleteLinksOnPort(npt);
                 linkDeleted = true;
             } else if (ps.getReason() ==
                     (byte) OFPortReason.OFPPR_MODIFY.ordinal()) {
@@ -813,12 +748,10 @@
     /**
      * Process a new port.
      * If link discovery is disabled on the port, then do nothing.
-     * If autoportfast feature is enabled and the port is a fast port, then
-     * do nothing.
-     * Otherwise, send LLDP message.  Add the port to quarantine.
+     * Otherwise, send LLDP message.
      *
-     * @param sw
-     * @param p
+     * @param sw the dpid of the switch the port is on
+     * @param p the number of the port
      */
     private void processNewPort(long sw, short p) {
         if (isLinkDiscoverySuppressed(sw, p)) {
@@ -841,14 +774,12 @@
                 processNewPort(sw.getId(), p);
             }
         }
-
-        LinkUpdate update = new LinkUpdate(new LDUpdate(sw.getId(), null,
-                UpdateOperation.SWITCH_UPDATED));
-        controller.publishUpdate(update);
     }
 
     /**
      * When a switch disconnects we remove any links from our map and notify.
+     *
+     * @param iofSwitch the switch that was removed
      */
     @Override
     public void removedSwitch(IOFSwitch iofSwitch) {
@@ -865,18 +796,14 @@
                 }
                 // add all tuples with an endpoint on this switch to erase list
                 eraseList.addAll(switchLinks.get(sw));
-                deleteLinks(eraseList, "Switch Removed");
-
-                // Send a switch removed update
-                LinkUpdate update = new LinkUpdate(new LDUpdate(sw, null, UpdateOperation.SWITCH_REMOVED));
-                controller.publishUpdate(update);
+                deleteLinks(eraseList);
             }
         } finally {
             lock.writeLock().unlock();
         }
     }
 
-    /**
+    /*
      * We don't react the port changed notifications here. we listen for
      * OFPortStatus messages directly. Might consider using this notifier
      * instead
@@ -889,10 +816,9 @@
     /**
      * Delete links incident on a given switch port.
      *
-     * @param npt
-     * @param reason
+     * @param npt the port to delete links on
      */
-    protected void deleteLinksOnPort(NodePortTuple npt, String reason) {
+    protected void deleteLinksOnPort(NodePortTuple npt) {
         List<Link> eraseList = new ArrayList<Link>();
         if (this.portLinks.containsKey(npt)) {
             if (log.isTraceEnabled()) {
@@ -903,7 +829,7 @@
                                 this.portLinks.get(npt)});
             }
             eraseList.addAll(this.portLinks.get(npt));
-            deleteLinks(eraseList, reason);
+            deleteLinks(eraseList);
         }
     }
 
@@ -911,7 +837,7 @@
      * Iterates through the list of links and deletes if the
      * last discovery message reception time exceeds timeout values.
      */
-    protected void timeoutLinks() {
+    protected void timeOutLinks() {
         List<Link> eraseList = new ArrayList<Link>();
         Long curTime = System.currentTimeMillis();
 
@@ -930,7 +856,7 @@
                 }
             }
 
-            deleteLinks(eraseList, "LLDP timeout");
+            deleteLinks(eraseList);
         } finally {
             lock.writeLock().unlock();
         }
@@ -963,27 +889,12 @@
 
     @Override
     public void addListener(ILinkDiscoveryListener listener) {
-        linkDiscoveryAware.add(listener);
+        linkDiscoveryListeners.add(listener);
     }
 
-    /**
-     * Register a link discovery aware component.
-     *
-     * @param linkDiscoveryAwareComponent
-     */
-    public void addLinkDiscoveryAware(ILinkDiscoveryListener linkDiscoveryAwareComponent) {
-        // TODO make this a copy on write set or lock it somehow
-        this.linkDiscoveryAware.add(linkDiscoveryAwareComponent);
-    }
-
-    /**
-     * Deregister a link discovery aware component.
-     *
-     * @param linkDiscoveryAwareComponent
-     */
-    public void removeLinkDiscoveryAware(ILinkDiscoveryListener linkDiscoveryAwareComponent) {
-        // TODO make this a copy on write set or lock it somehow
-        this.linkDiscoveryAware.remove(linkDiscoveryAwareComponent);
+    @Override
+    public void removeListener(ILinkDiscoveryListener listener) {
+        linkDiscoveryListeners.remove(listener);
     }
 
     @Override
@@ -1008,24 +919,19 @@
 
     @Override
     public Map<Class<? extends IFloodlightService>, IFloodlightService>
-    getServiceImpls() {
-        Map<Class<? extends IFloodlightService>,
-                IFloodlightService> m =
-                new HashMap<Class<? extends IFloodlightService>,
-                        IFloodlightService>();
-        // We are the class that implements the service
+            getServiceImpls() {
+        Map<Class<? extends IFloodlightService>, IFloodlightService> m =
+                new HashMap<>();
         m.put(ILinkDiscoveryService.class, this);
         return m;
     }
 
     @Override
     public Collection<Class<? extends IFloodlightService>> getModuleDependencies() {
-        Collection<Class<? extends IFloodlightService>> l =
-                new ArrayList<Class<? extends IFloodlightService>>();
+        Collection<Class<? extends IFloodlightService>> l = new ArrayList<>();
         l.add(IFloodlightProviderService.class);
         l.add(IThreadPoolService.class);
         l.add(IRestApiService.class);
-        // Added by ONOS
         l.add(IControllerRegistryService.class);
         return l;
     }
@@ -1036,11 +942,8 @@
         floodlightProvider = context.getServiceImpl(IFloodlightProviderService.class);
         threadPool = context.getServiceImpl(IThreadPoolService.class);
         restApi = context.getServiceImpl(IRestApiService.class);
-        // Added by ONOS
         registryService = context.getServiceImpl(IControllerRegistryService.class);
 
-        // We create this here because there is no ordering guarantee
-        this.linkDiscoveryAware = new ArrayList<ILinkDiscoveryListener>();
         this.lock = new ReentrantReadWriteLock();
         this.links = new HashMap<Link, LinkInfo>();
         this.portLinks = new HashMap<NodePortTuple, Set<Link>>();
@@ -1075,10 +978,8 @@
     })
     public void startUp(FloodlightModuleContext context) {
         ScheduledExecutorService ses = threadPool.getScheduledExecutor();
-        controller =
-                context.getServiceImpl(IFloodlightProviderService.class);
+        controller = context.getServiceImpl(IFloodlightProviderService.class);
 
-        // To be started by the first switch connection
         discoveryTask = new SingletonTask(ses, new Runnable() {
             @Override
             public void run() {
@@ -1094,7 +995,6 @@
             }
         });
 
-        // Always reschedule link discovery as we are never in SLAVE role now
         discoveryTask.reschedule(DISCOVERY_TASK_INTERVAL, TimeUnit.SECONDS);
 
         // Register for the OpenFlow messages we want to receive
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 0c2400c..fc19951 100644
--- a/src/main/java/net/onrc/onos/core/linkdiscovery/LinkInfo.java
+++ b/src/main/java/net/onrc/onos/core/linkdiscovery/LinkInfo.java
@@ -16,7 +16,7 @@
 
 package net.onrc.onos.core.linkdiscovery;
 
-import net.onrc.onos.core.linkdiscovery.ILinkDiscovery.LinkType;
+import net.onrc.onos.core.linkdiscovery.ILinkDiscoveryService.LinkType;
 
 import com.google.common.primitives.Longs;
 
diff --git a/src/main/java/net/onrc/onos/core/linkdiscovery/README b/src/main/java/net/onrc/onos/core/linkdiscovery/README
deleted file mode 100644
index 3cec58d..0000000
--- a/src/main/java/net/onrc/onos/core/linkdiscovery/README
+++ /dev/null
@@ -1,8 +0,0 @@
-Note about this directory
-=========================
-
-This directory contains link discovery manager and it's related module derived from Flood Light v0.9.0.
-Many of the code is unmodified from it's original state, but they had to be moved due to package visibility etc.
-
-Compare with floodlight's linkdiscovery directory to see what was modified from it's original code.
-
diff --git a/src/main/java/net/onrc/onos/core/linkdiscovery/web/LinkWithType.java b/src/main/java/net/onrc/onos/core/linkdiscovery/web/LinkWithType.java
index 234ff98..01a1539 100644
--- a/src/main/java/net/onrc/onos/core/linkdiscovery/web/LinkWithType.java
+++ b/src/main/java/net/onrc/onos/core/linkdiscovery/web/LinkWithType.java
@@ -2,7 +2,7 @@
 
 import java.io.IOException;
 
-import net.onrc.onos.core.linkdiscovery.ILinkDiscovery.LinkType;
+import net.onrc.onos.core.linkdiscovery.ILinkDiscoveryService.LinkType;
 import net.onrc.onos.core.linkdiscovery.Link;
 
 import org.codehaus.jackson.JsonGenerator;
diff --git a/src/main/java/net/onrc/onos/core/linkdiscovery/web/LinksResource.java b/src/main/java/net/onrc/onos/core/linkdiscovery/web/LinksResource.java
index 81c0b9e..2738041 100644
--- a/src/main/java/net/onrc/onos/core/linkdiscovery/web/LinksResource.java
+++ b/src/main/java/net/onrc/onos/core/linkdiscovery/web/LinksResource.java
@@ -30,7 +30,7 @@
                 LinkWithType lwt = new LinkWithType(link,
                         info.getSrcPortState(),
                         info.getDstPortState(),
-                        ld.getLinkType(link, info));
+                        info.getLinkType());
                 returnLinkSet.add(lwt);
             }
         }
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 6cb2bc1..1d5b8a6 100644
--- a/src/main/java/net/onrc/onos/core/topology/TopologyPublisher.java
+++ b/src/main/java/net/onrc/onos/core/topology/TopologyPublisher.java
@@ -19,6 +19,7 @@
 import net.onrc.onos.core.devicemanager.OnosDevice;
 import net.onrc.onos.core.linkdiscovery.ILinkDiscoveryListener;
 import net.onrc.onos.core.linkdiscovery.ILinkDiscoveryService;
+import net.onrc.onos.core.linkdiscovery.Link;
 import net.onrc.onos.core.main.IOFSwitchPortListener;
 import net.onrc.onos.core.registry.IControllerRegistryService;
 import net.onrc.onos.core.registry.IControllerRegistryService.ControlChangeCallback;
@@ -132,43 +133,45 @@
     }
 
     @Override
-    public void linkDiscoveryUpdate(LDUpdate update) {
-
+    public void linkAdded(Link link) {
         LinkEvent linkEvent = new LinkEvent(
-                        new SwitchPort(update.getSrc(), update.getSrcPort()),
-                        new SwitchPort(update.getDst(), update.getDstPort()));
+                new SwitchPort(link.getSrc(), link.getSrcPort()),
+                new SwitchPort(link.getDst(), link.getDstPort()));
+
         // FIXME should be merging, with existing attrs, etc..
         // TODO define attr name as constant somewhere.
         // TODO populate appropriate attributes.
         linkEvent.freeze();
 
-        switch (update.getOperation()) {
-            case LINK_ADDED:
-                if (!registryService.hasControl(update.getDst())) {
-                    // Don't process or send a link event if we're not master for the
-                    // destination switch
-                    log.debug("Not the master for dst switch {}. Suppressed link add event {}.",
-                            update.getDst(), linkEvent);
-                    return;
-                }
-                topologyDiscoveryInterface.putLinkDiscoveryEvent(linkEvent);
-                break;
-            case LINK_UPDATED:
-                // We don't use the LINK_UPDATED event (unsure what it means)
-                break;
-            case LINK_REMOVED:
-                if (!registryService.hasControl(update.getDst())) {
-                    // Don't process or send a link event if we're not master for the
-                    // destination switch
-                    log.debug("Not the master for dst switch {}. Suppressed link remove event {}.",
-                            update.getDst(), linkEvent);
-                    return;
-                }
-                topologyDiscoveryInterface.removeLinkDiscoveryEvent(linkEvent);
-                break;
-            default:
-                break;
+        if (!registryService.hasControl(link.getDst())) {
+            // Don't process or send a link event if we're not master for the
+            // destination switch
+            log.debug("Not the master for dst switch {}. Suppressed link add event {}.",
+                    link.getDst(), linkEvent);
+            return;
         }
+        topologyDiscoveryInterface.putLinkDiscoveryEvent(linkEvent);
+    }
+
+    @Override
+    public void linkRemoved(Link link) {
+        LinkEvent linkEvent = new LinkEvent(
+                new SwitchPort(link.getSrc(), link.getSrcPort()),
+                new SwitchPort(link.getDst(), link.getDstPort()));
+
+        // FIXME should be merging, with existing attrs, etc..
+        // TODO define attr name as constant somewhere.
+        // TODO populate appropriate attributes.
+        linkEvent.freeze();
+
+        if (!registryService.hasControl(link.getDst())) {
+            // Don't process or send a link event if we're not master for the
+            // destination switch
+            log.debug("Not the master for dst switch {}. Suppressed link remove event {}.",
+                    link.getDst(), linkEvent);
+            return;
+        }
+        topologyDiscoveryInterface.removeLinkDiscoveryEvent(linkEvent);
     }
 
     @Override
@@ -184,7 +187,7 @@
 
         if (registryService.hasControl(switchId)) {
             topologyDiscoveryInterface.putPortDiscoveryEvent(portEvent);
-            linkDiscovery.removeFromSuppressLLDPs(switchId, port.getPortNumber());
+            linkDiscovery.enableDiscoveryOnPort(switchId, port.getPortNumber());
         } else {
             log.debug("Not the master for switch {}. Suppressed port add event {}.",
                     new Dpid(switchId), portEvent);
@@ -240,7 +243,7 @@
         for (OFPhysicalPort port : sw.getPorts()) {
             // Allow links to be discovered on this port now that it's
             // in the database
-            linkDiscovery.removeFromSuppressLLDPs(sw.getId(), port.getPortNumber());
+            linkDiscovery.enableDiscoveryOnPort(sw.getId(), port.getPortNumber());
         }
     }
 
diff --git a/src/main/resources/META-INF/services/net.floodlightcontroller.core.module.IFloodlightModule b/src/main/resources/META-INF/services/net.floodlightcontroller.core.module.IFloodlightModule
index 5e6c88a..37be163 100644
--- a/src/main/resources/META-INF/services/net.floodlightcontroller.core.module.IFloodlightModule
+++ b/src/main/resources/META-INF/services/net.floodlightcontroller.core.module.IFloodlightModule
@@ -1,6 +1,6 @@
 net.floodlightcontroller.core.FloodlightProvider
 net.onrc.onos.core.topology.TopologyPublisher
-net.onrc.onos.core.linkdiscovery.internal.LinkDiscoveryManager
+net.onrc.onos.core.linkdiscovery.LinkDiscoveryManager
 net.floodlightcontroller.restserver.RestApiServer
 net.floodlightcontroller.threadpool.ThreadPool
 net.floodlightcontroller.core.test.MockFloodlightProvider
diff --git a/src/test/java/net/floodlightcontroller/core/internal/ControllerTest.java b/src/test/java/net/floodlightcontroller/core/internal/ControllerTest.java
index 8f69e64..eb63904 100644
--- a/src/test/java/net/floodlightcontroller/core/internal/ControllerTest.java
+++ b/src/test/java/net/floodlightcontroller/core/internal/ControllerTest.java
@@ -56,7 +56,7 @@
 import net.floodlightcontroller.test.FloodlightTestCase;
 import net.floodlightcontroller.threadpool.IThreadPoolService;
 import net.onrc.onos.core.linkdiscovery.ILinkDiscoveryService;
-import net.onrc.onos.core.linkdiscovery.internal.LinkDiscoveryManager;
+import net.onrc.onos.core.linkdiscovery.LinkDiscoveryManager;
 import net.onrc.onos.core.main.IOFSwitchPortListener;
 import net.onrc.onos.core.packet.ARP;
 import net.onrc.onos.core.packet.Ethernet;
diff --git a/src/test/java/net/onrc/onos/core/linkdiscovery/internal/LinkDiscoveryManagerTest.java b/src/test/java/net/onrc/onos/core/linkdiscovery/LinkDiscoveryManagerTest.java
similarity index 95%
rename from src/test/java/net/onrc/onos/core/linkdiscovery/internal/LinkDiscoveryManagerTest.java
rename to src/test/java/net/onrc/onos/core/linkdiscovery/LinkDiscoveryManagerTest.java
index 22ec42e..d6c0043 100644
--- a/src/test/java/net/onrc/onos/core/linkdiscovery/internal/LinkDiscoveryManagerTest.java
+++ b/src/test/java/net/onrc/onos/core/linkdiscovery/LinkDiscoveryManagerTest.java
@@ -15,7 +15,7 @@
  *    under the License.
  **/
 
-package net.onrc.onos.core.linkdiscovery.internal;
+package net.onrc.onos.core.linkdiscovery;
 
 import static org.easymock.EasyMock.createNiceMock;
 import static org.easymock.EasyMock.expect;
@@ -23,7 +23,6 @@
 import static org.easymock.EasyMock.verify;
 
 import java.io.IOException;
-import java.util.ArrayList;
 import java.util.Collections;
 import java.util.HashMap;
 import java.util.Map;
@@ -37,11 +36,6 @@
 import net.floodlightcontroller.restserver.RestApiServer;
 import net.floodlightcontroller.test.FloodlightTestCase;
 import net.floodlightcontroller.threadpool.IThreadPoolService;
-import net.onrc.onos.core.linkdiscovery.ILinkDiscoveryListener;
-import net.onrc.onos.core.linkdiscovery.ILinkDiscoveryService;
-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;
@@ -95,7 +89,7 @@
         super.setUp();
         FloodlightModuleContext cntx = new FloodlightModuleContext();
         ldm = new TestLinkDiscoveryManager();
-        ldm.linkDiscoveryAware = new ArrayList<ILinkDiscoveryListener>();
+        //ldm.linkDiscoveryAware = new ArrayList<ILinkDiscoveryListener>();
         MockThreadPoolService tp = new MockThreadPoolService();
         RestApiServer restApi = new RestApiServer();
         IControllerRegistryService registry =
@@ -178,7 +172,7 @@
         LinkInfo info = new LinkInfo(System.currentTimeMillis(),
                 System.currentTimeMillis(), 0, 0);
         topology.addOrUpdateLink(lt, info);
-        topology.deleteLinks(Collections.singletonList(lt), "Test");
+        topology.deleteLinks(Collections.singletonList(lt));
 
         // check invariants hold
         assertNull(topology.switchLinks.get(lt.getSrc()));
@@ -221,7 +215,7 @@
         LinkInfo info = new LinkInfo(System.currentTimeMillis(),
                 System.currentTimeMillis(), 0, 0);
         topology.addOrUpdateLink(lt, info);
-        topology.deleteLinks(Collections.singletonList(lt), "Test to self");
+        topology.deleteLinks(Collections.singletonList(lt));
 
         // check invariants hold
         assertNull(topology.switchLinks.get(lt.getSrc()));
@@ -300,7 +294,7 @@
         assertTrue(topology.portLinks.get(dstNpt).contains(lt));
         assertTrue(topology.links.containsKey(lt));
 
-        topology.timeoutLinks();
+        topology.timeOutLinks();
 
         // 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
@@ -310,7 +304,7 @@
         topology.addOrUpdateLink(lt, info);
 
         // Expect to timeout the unicast Valid Time, so the link should disappear
-        topology.timeoutLinks();
+        topology.timeOutLinks();
         assertTrue(topology.links.get(lt) == null);
     }