ONOS-2846, ONOS-2812 Reworking link discovery provider to be fully configurable …

…and to prune links based on links rather than based on ports. Change-Id: I0a042403baf163bebd471bffd112b28571dae75b

ONOS-2846, ONOS-2812 Reworking link discovery provider to be fully configurable …
…and to prune links based on links rather than based on ports. Change-Id: I0a042403baf163bebd471bffd112b28571dae75b
Thomas Vachuska · Gerrit Code Review
Commit 05453c9877feed02427c139ee3e210e1d2bef961 05453c98 1 parent e796610b
Showing 4 changed files with 265 additions and 220 deletions
providers/lldp/src/main/java/org/onosproject/provider/lldp/impl/DiscoveryContext.java
providers/lldp/src/main/java/org/onosproject/provider/lldp/impl/LLDPLinkProvider.java
providers/lldp/src/main/java/org/onosproject/provider/lldp/impl/LinkDiscovery.java
providers/lldp/src/test/java/org/onosproject/provider/lldp/impl/LLDPLinkProviderTest.java
--- a/providers/lldp/src/main/java/org/onosproject/provider/lldp/impl/DiscoveryContext.java 0 → 100644
View file @05453c9
+++ b/providers/lldp/src/main/java/org/onosproject/provider/lldp/impl/DiscoveryContext.java 0 → 100644
View file @05453c9
+/*
+ * Copyright 2015 Open Networking Laboratory
+ *
+ * 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
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT 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 org.onosproject.provider.lldp.impl;
+
+import org.onosproject.mastership.MastershipService;
+import org.onosproject.net.link.LinkProviderService;
+import org.onosproject.net.packet.PacketService;
+
+/**
+ * Shared context for use by link discovery.
+ */
+public interface DiscoveryContext {
+
+    /**
+     * Returns the shared mastership service reference.
+     *
+     * @return mastership service
+     */
+    MastershipService mastershipService();
+
+    /**
+     * Returns the shared link provider service reference.
+     *
+     * @return link provider service
+     */
+    LinkProviderService providerService();
+
+    /**
+     * Returns the shared packet service reference.
+     *
+     * @return packet service
+     */
+    PacketService packetService();
+
+    /**
+     * Returns the probe rate in millis.
+     *
+     * @return probe rate
+     */
+    long probeRate();
+
+    /**
+     * Returns the max stale link age in millis.
+     *
+     * @return stale link age
+     */
+    long staleLinkAge();
+
+    /**
+     * Indicates whether to emit BDDP.
+     *
+     * @return true to emit BDDP
+     */
+    boolean useBDDP();
+}
--- a/providers/lldp/src/main/java/org/onosproject/provider/lldp/impl/LLDPLinkProvider.java
View file @05453c9
+++ b/providers/lldp/src/main/java/org/onosproject/provider/lldp/impl/LLDPLinkProvider.java
View file @05453c9
@@ -15,7 +15,6 @@
  */
 package org.onosproject.provider.lldp.impl;
-import com.google.common.base.Strings;
 import com.google.common.collect.ImmutableMap;
 import com.google.common.collect.ImmutableSet;
 import org.apache.felix.scr.annotations.Activate;
@@ -57,9 +56,11 @@ import java.io.IOException;
 import java.util.Dictionary;
 import java.util.EnumSet;
 import java.util.Map;
+import java.util.Properties;
 import java.util.concurrent.ConcurrentHashMap;
 import java.util.concurrent.ScheduledExecutorService;
+import static com.google.common.base.Strings.isNullOrEmpty;
 import static java.util.concurrent.Executors.newSingleThreadScheduledExecutor;
 import static java.util.concurrent.TimeUnit.SECONDS;
 import static org.onlab.util.Tools.get;
@@ -67,19 +68,16 @@ import static org.onlab.util.Tools.groupedThreads;
 import static org.slf4j.LoggerFactory.getLogger;
 /**
- * Provider which uses an OpenFlow controller to detect network
+ * Provider which uses LLDP and BDDP packets to detect network infrastructure links.
- * infrastructure links.
  */
 @Component(immediate = true)
 public class LLDPLinkProvider extends AbstractProvider implements LinkProvider {
     private static final String PROVIDER_NAME = "org.onosproject.provider.lldp";
-    private static final String PROP_USE_BDDP = "useBDDP";
+    private static final String FORMAT =
-    private static final String PROP_DISABLE_LD = "disableLinkDiscovery";
+            "Settings: enabled={}, useBDDP={}, probeRate={}, " +
-    private static final String PROP_LLDP_SUPPRESSION = "lldpSuppression";
+                    "staleLinkAge={}, lldpSuppression={}";
-
-    private static final String DEFAULT_LLDP_SUPPRESSION_CONFIG = "../config/lldp_suppression.json";
     private final Logger log = getLogger(getClass());
@@ -105,24 +103,41 @@ public class LLDPLinkProvider extends AbstractProvider implements LinkProvider {
     private ScheduledExecutorService executor;
+    private static final long INIT_DELAY = 5;
+    private static final long DELAY = 5;
+
+    private static final String PROP_ENABLED = "enabled";
+    @Property(name = PROP_ENABLED, boolValue = true,
+            label = "If false, link discovery is disabled")
+    private boolean enabled = false;
+
+    private static final String PROP_USE_BDDP = "useBDDP";
     @Property(name = PROP_USE_BDDP, boolValue = true,
             label = "Use BDDP for link discovery")
     private boolean useBDDP = true;
-    @Property(name = PROP_DISABLE_LD, boolValue = false,
+    private static final String PROP_PROBE_RATE = "probeRate";
-            label = "Permanently disable link discovery")
+    private static final int DEFAULT_PROBE_RATE = 3_000;
-    private boolean disableLinkDiscovery = false;
+    @Property(name = PROP_PROBE_RATE, intValue = DEFAULT_PROBE_RATE,
+            label = "LLDP and BDDP probe rate specified in millis")
+    private int probeRate = DEFAULT_PROBE_RATE;
-    private static final long INIT_DELAY = 5;
+    private static final String PROP_STALE_LINK_AGE = "staleLinkAge";
-    private static final long DELAY = 5;
+    private static final int DEFAULT_STALE_LINK_AGE = 10_000;
+    @Property(name = PROP_STALE_LINK_AGE, intValue = DEFAULT_STALE_LINK_AGE,
+            label = "Number of millis beyond which links will be considered stale")
+    private int staleLinkAge = DEFAULT_STALE_LINK_AGE;
+    // FIXME: convert to use network config subsystem instead
+    private static final String PROP_LLDP_SUPPRESSION = "lldpSuppression";
+    private static final String DEFAULT_LLDP_SUPPRESSION_CONFIG = "../config/lldp_suppression.json";
     @Property(name = PROP_LLDP_SUPPRESSION, value = DEFAULT_LLDP_SUPPRESSION_CONFIG,
             label = "Path to LLDP suppression configuration file")
     private String lldpSuppression = DEFAULT_LLDP_SUPPRESSION_CONFIG;
+    private final DiscoveryContext context = new InternalDiscoveryContext();
     private final InternalLinkProvider listener = new InternalLinkProvider();
-
     private final InternalRoleListener roleListener = new InternalRoleListener();
     protected final Map<DeviceId, LinkDiscovery> discoverers = new ConcurrentHashMap<>();
@@ -141,57 +156,82 @@ public class LLDPLinkProvider extends AbstractProvider implements LinkProvider {
     public void activate(ComponentContext context) {
         cfgService.registerProperties(getClass());
         appId = coreService.registerApplication(PROVIDER_NAME);
-
-        // to load configuration at startup
         modified(context);
-        if (disableLinkDiscovery) {
+        log.info("Started");
-            log.info("LinkDiscovery has been permanently disabled by configuration");
+    }
-            return;
+
+    @Deactivate
+    public void deactivate() {
+        cfgService.unregisterProperties(getClass(), false);
+        disable();
+        log.info("Stopped");
+    }
+
+    @Modified
+    public void modified(ComponentContext context) {
+        Dictionary<?, ?> properties = context != null ? context.getProperties() : new Properties();
+
+        boolean newEnabled, newUseBddp;
+        int newProbeRate, newStaleLinkAge;
+        String newLldpSuppression;
+        try {
+            String s = get(properties, PROP_ENABLED);
+            newEnabled = isNullOrEmpty(s) || Boolean.parseBoolean(s.trim());
+
+            s = get(properties, PROP_USE_BDDP);
+            newUseBddp = isNullOrEmpty(s) || Boolean.parseBoolean(s.trim());
+
+            s = get(properties, PROP_PROBE_RATE);
+            newProbeRate = isNullOrEmpty(s) ? probeRate : Integer.parseInt(s.trim());
+
+            s = get(properties, PROP_STALE_LINK_AGE);
+            newStaleLinkAge = isNullOrEmpty(s) ? staleLinkAge : Integer.parseInt(s.trim());
+
+            s = get(properties, PROP_LLDP_SUPPRESSION);
+            newLldpSuppression = isNullOrEmpty(s) ? DEFAULT_LLDP_SUPPRESSION_CONFIG : s;
+
+        } catch (NumberFormatException e) {
+            log.warn(e.getMessage());
+            newEnabled = enabled;
+            newUseBddp = useBDDP;
+            newProbeRate = probeRate;
+            newStaleLinkAge = staleLinkAge;
+            newLldpSuppression = lldpSuppression;
+        }
+
+        boolean wasEnabled = enabled;
+
+        enabled = newEnabled;
+        useBDDP = newUseBddp;
+        probeRate = newProbeRate;
+        staleLinkAge = newStaleLinkAge;
+        lldpSuppression = newLldpSuppression;
+
+        if (!wasEnabled && enabled) {
+            enable();
+        } else if (wasEnabled && !enabled) {
+            disable();
         }
+        log.info(FORMAT, enabled, useBDDP, probeRate, staleLinkAge, lldpSuppression);
+    }
+
+    private void enable() {
         providerService = providerRegistry.register(this);
         deviceService.addListener(listener);
         packetService.addProcessor(listener, PacketProcessor.advisor(0));
         masterService.addListener(roleListener);
-        LinkDiscovery ld;
+        processDevices();
-        for (Device device : deviceService.getAvailableDevices()) {
-            if (rules.isSuppressed(device)) {
-                log.debug("LinkDiscovery from {} disabled by configuration", device.id());
-                continue;
-            }
-            ld = new LinkDiscovery(device, packetService, masterService,
-                                   providerService, useBDDP);
-            discoverers.put(device.id(), ld);
-            addPorts(ld, device.id());
-        }
         executor = newSingleThreadScheduledExecutor(groupedThreads("onos/device", "sync-%d"));
         executor.scheduleAtFixedRate(new SyncDeviceInfoTask(), INIT_DELAY, DELAY, SECONDS);
+        loadSuppressionRules();
         requestIntercepts();
-
-        log.info("Started");
-    }
-
-    private void addPorts(LinkDiscovery discoverer, DeviceId deviceId) {
-        for (Port p : deviceService.getPorts(deviceId)) {
-            if (rules.isSuppressed(p)) {
-                continue;
-            }
-            if (!p.number().isLogical()) {
-                discoverer.addPort(p);
-            }
-        }
-    }
-
-    @Deactivate
-    public void deactivate() {
-        cfgService.unregisterProperties(getClass(), false);
-        if (disableLinkDiscovery) {
-            return;
     }
+    private void disable() {
         withdrawIntercepts();
         providerRegistry.unregister(this);
@@ -199,37 +239,36 @@ public class LLDPLinkProvider extends AbstractProvider implements LinkProvider {
         packetService.removeProcessor(listener);
         masterService.removeListener(roleListener);
+        if (executor != null) {
             executor.shutdownNow();
+        }
         discoverers.values().forEach(LinkDiscovery::stop);
         discoverers.clear();
-        providerService = null;
-        log.info("Stopped");
+        providerService = null;
     }
-    @Modified
+    private void processDevices() {
-    public void modified(ComponentContext context) {
+        for (Device device : deviceService.getAvailableDevices()) {
-        if (context == null) {
+            if (rules.isSuppressed(device)) {
-            loadSuppressionRules();
+                log.debug("LinkDiscovery from {} disabled by configuration", device.id());
-            return;
+                continue;
+            }
+            LinkDiscovery ld = new LinkDiscovery(device, context);
+            discoverers.put(device.id(), ld);
+            addPorts(ld, device.id());
+        }
     }
-        @SuppressWarnings("rawtypes")
-        Dictionary properties = context.getProperties();
-        String s = get(properties, PROP_DISABLE_LD);
+    private void addPorts(LinkDiscovery discoverer, DeviceId deviceId) {
-        if (!Strings.isNullOrEmpty(s)) {
+        for (Port p : deviceService.getPorts(deviceId)) {
-            disableLinkDiscovery = Boolean.valueOf(s);
+            if (rules.isSuppressed(p)) {
+                continue;
             }
-        s = get(properties, PROP_USE_BDDP);
+            if (!p.number().isLogical()) {
-        if (!Strings.isNullOrEmpty(s)) {
+                discoverer.addPort(p);
-            useBDDP = Boolean.valueOf(s);
             }
-        s = get(properties, PROP_LLDP_SUPPRESSION);
-        if (!Strings.isNullOrEmpty(s)) {
-            lldpSuppression = s;
         }
-        requestIntercepts();
-        loadSuppressionRules();
     }
     private void loadSuppressionRules() {
@@ -275,13 +314,7 @@ public class LLDPLinkProvider extends AbstractProvider implements LinkProvider {
         packetService.cancelPackets(selector.build(), PacketPriority.CONTROL, appId);
     }
-    private LinkDiscovery createLinkDiscovery(Device device) {
-        return new LinkDiscovery(device, packetService, masterService,
-                                 providerService, useBDDP);
-    }
-
     private class InternalRoleListener implements MastershipListener {
-
         @Override
         public void event(MastershipEvent event) {
             if (MastershipEvent.Type.BACKUPS_CHANGED.equals(event.type())) {
@@ -298,22 +331,15 @@ public class LLDPLinkProvider extends AbstractProvider implements LinkProvider {
             if (rules.isSuppressed(device)) {
                 return;
             }
-            synchronized (discoverers) {
+            discoverers.computeIfAbsent(deviceId, k -> new LinkDiscovery(device, context));
-                if (!discoverers.containsKey(deviceId)) {
-                    // ideally, should never reach here
-                    log.debug("Device mastership changed ({}) {}", event.type(), deviceId);
-                    discoverers.put(deviceId, createLinkDiscovery(device));
-                }
-            }
         }
     }
     private class InternalLinkProvider implements PacketProcessor, DeviceListener {
-
         @Override
         public void event(DeviceEvent event) {
-            LinkDiscovery ld = null;
+            LinkDiscovery ld;
             Device device = event.subject();
             Port port = event.port();
             if (device == null) {
@@ -333,7 +359,7 @@ public class LLDPLinkProvider extends AbstractProvider implements LinkProvider {
                                 return;
                             }
                             log.debug("Device added ({}) {}", event.type(), deviceId);
-                            discoverers.put(deviceId, createLinkDiscovery(device));
+                            discoverers.put(deviceId, new LinkDiscovery(device, context));
                         } else {
                             if (ld.isStopped()) {
                                 log.debug("Device restarted ({}) {}", event.type(), deviceId);
@@ -418,7 +444,6 @@ public class LLDPLinkProvider extends AbstractProvider implements LinkProvider {
     }
     private final class SyncDeviceInfoTask implements Runnable {
-
         @Override
         public void run() {
             if (Thread.currentThread().isInterrupted()) {
@@ -433,13 +458,9 @@ public class LLDPLinkProvider extends AbstractProvider implements LinkProvider {
                     }
                     DeviceId did = dev.id();
                     synchronized (discoverers) {
-                        LinkDiscovery discoverer = discoverers.get(did);
+                        LinkDiscovery ld = discoverers
-                        if (discoverer == null) {
+                                .computeIfAbsent(did, k -> new LinkDiscovery(dev, context));
-                            discoverer = createLinkDiscovery(dev);
+                        addPorts(ld, did);
-                            discoverers.put(did, discoverer);
-                        }
-
-                        addPorts(discoverer, did);
                     }
                 }
             } catch (Exception e) {
@@ -449,4 +470,35 @@ public class LLDPLinkProvider extends AbstractProvider implements LinkProvider {
         }
     }
+    private class InternalDiscoveryContext implements DiscoveryContext {
+        @Override
+        public MastershipService mastershipService() {
+            return masterService;
+        }
+
+        @Override
+        public LinkProviderService providerService() {
+            return providerService;
+        }
+
+        @Override
+        public PacketService packetService() {
+            return packetService;
+        }
+
+        @Override
+        public long probeRate() {
+            return probeRate;
+        }
+
+        @Override
+        public long staleLinkAge() {
+            return staleLinkAge;
+        }
+
+        @Override
+        public boolean useBDDP() {
+            return useBDDP;
+        }
+    }
 }
--- a/providers/lldp/src/main/java/org/onosproject/provider/lldp/impl/LinkDiscovery.java
View file @05453c9
+++ b/providers/lldp/src/main/java/org/onosproject/provider/lldp/impl/LinkDiscovery.java
View file @05453c9
@@ -22,37 +22,30 @@ import org.jboss.netty.util.TimerTask;
 import org.onlab.packet.Ethernet;
 import org.onlab.packet.ONOSLLDP;
 import org.onlab.util.Timer;
-import org.onosproject.mastership.MastershipService;
 import org.onosproject.net.ConnectPoint;
 import org.onosproject.net.Device;
 import org.onosproject.net.DeviceId;
 import org.onosproject.net.Link.Type;
+import org.onosproject.net.LinkKey;
 import org.onosproject.net.Port;
 import org.onosproject.net.PortNumber;
 import org.onosproject.net.link.DefaultLinkDescription;
 import org.onosproject.net.link.LinkDescription;
-import org.onosproject.net.link.LinkProviderService;
 import org.onosproject.net.packet.DefaultOutboundPacket;
 import org.onosproject.net.packet.OutboundPacket;
 import org.onosproject.net.packet.PacketContext;
-import org.onosproject.net.packet.PacketService;
 import org.slf4j.Logger;
 import java.nio.ByteBuffer;
-import java.util.Iterator;
 import java.util.Map;
 import java.util.Set;
-import java.util.concurrent.atomic.AtomicInteger;
+import java.util.stream.Collectors;
-import static com.google.common.base.Preconditions.checkNotNull;
 import static java.util.concurrent.TimeUnit.MILLISECONDS;
 import static org.onosproject.net.PortNumber.portNumber;
 import static org.onosproject.net.flow.DefaultTrafficTreatment.builder;
 import static org.slf4j.LoggerFactory.getLogger;
-// TODO: add 'fast discovery' mode: drop LLDPs in destination switch but listen for flow_removed messages
-// FIXME: add ability to track links using port pairs or the link inventory
-
 /**
  * Run discovery process from a physical switch. Ports are initially labeled as
  * slow ports. When an LLDP is successfully received, label the remote port as
@@ -64,34 +57,23 @@ public class LinkDiscovery implements TimerTask {
     private final Logger log = getLogger(getClass());
-    private static final short MAX_PROBE_COUNT = 3; // probes to send before link is removed
-    private static final short DEFAULT_PROBE_RATE = 3000; // millis
     private static final String SRC_MAC = "DE:AD:BE:EF:BA:11";
-    private static final String SERVICE_NULL = "Service cannot be null";
     private final Device device;
-
+    private final DiscoveryContext context;
-    // send 1 probe every probeRate milliseconds
-    private final long probeRate = DEFAULT_PROBE_RATE;
-
-    private final Set<Long> slowPorts = Sets.newConcurrentHashSet();
-    // ports, known to have incoming links
-    private final Set<Long> fastPorts = Sets.newConcurrentHashSet();
-
-    // number of unacknowledged probes per port
-    private final Map<Long, AtomicInteger> portProbeCount = Maps.newHashMap();
     private final ONOSLLDP lldpPacket;
     private final Ethernet ethPacket;
     private Ethernet bddpEth;
-    private final boolean useBDDP;
     private Timeout timeout;
     private volatile boolean isStopped;
-    private final LinkProviderService linkProvider;
+    // Set of ports to be probed
-    private final PacketService pktService;
+    private final Set<Long> ports = Sets.newConcurrentHashSet();
-    private final MastershipService mastershipService;
+
+    // Most recent time a link was seen
+    private final Map<LinkKey, Long> linkTimes = Maps.newConcurrentMap();
     /**
      * Instantiates discovery manager for the given physical switch. Creates a
@@ -99,18 +81,11 @@ public class LinkDiscovery implements TimerTask {
      * Starts the the timer for the discovery process.
      *
      * @param device  the physical switch
-     * @param pktService      packet service
+     * @param context discovery context
-     * @param masterService   mastership service
-     * @param providerService link provider service
-     * @param useBDDP         flag to also use BDDP for discovery
      */
-    public LinkDiscovery(Device device, PacketService pktService,
+    public LinkDiscovery(Device device, DiscoveryContext context) {
-                         MastershipService masterService,
-                         LinkProviderService providerService, Boolean... useBDDP) {
         this.device = device;
-        this.linkProvider = checkNotNull(providerService, SERVICE_NULL);
+        this.context = context;
-        this.pktService = checkNotNull(pktService, SERVICE_NULL);
-        this.mastershipService = checkNotNull(masterService, SERVICE_NULL);
         lldpPacket = new ONOSLLDP();
         lldpPacket.setChassisId(device.chassisId());
@@ -122,15 +97,12 @@ public class LinkDiscovery implements TimerTask {
         ethPacket.setPayload(this.lldpPacket);
         ethPacket.setPad(true);
-        this.useBDDP = useBDDP.length > 0 ? useBDDP[0] : false;
-        if (this.useBDDP) {
         bddpEth = new Ethernet();
         bddpEth.setPayload(lldpPacket);
         bddpEth.setEtherType(Ethernet.TYPE_BSN);
         bddpEth.setDestinationMACAddress(ONOSLLDP.BDDP_MULTICAST);
         bddpEth.setPad(true);
         log.info("Using BDDP to discover network");
-        }
         isStopped = true;
         start();
@@ -145,15 +117,8 @@ public class LinkDiscovery implements TimerTask {
      * @param port the port
      */
     public void addPort(Port port) {
-        boolean newPort = false;
+        boolean newPort = ports.add(port.number().toLong());
-        synchronized (this) {
+        boolean isMaster = context.mastershipService().isLocalMaster(device.id());
-            if (!containsPort(port.number().toLong())) {
-                newPort = true;
-                slowPorts.add(port.number().toLong());
-            }
-        }
-
-        boolean isMaster = mastershipService.isLocalMaster(device.id());
         if (newPort && isMaster) {
             log.debug("Sending init probe to port {}@{}", port.number().toLong(), device.id());
             sendProbes(port.number().toLong());
@@ -161,59 +126,31 @@ public class LinkDiscovery implements TimerTask {
     }
     /**
-     * Removes physical port from discovery process.
-     *
-     * @param port the port
-     */
-    public void removePort(Port port) {
-        // Ignore ports that are not on this switch
-        long portnum = port.number().toLong();
-        synchronized (this) {
-            if (slowPorts.contains(portnum)) {
-                slowPorts.remove(portnum);
-
-            } else if (fastPorts.contains(portnum)) {
-                fastPorts.remove(portnum);
-                portProbeCount.remove(portnum);
-                // no iterator to update
-            } else {
-                log.warn("Tried to dynamically remove non-existing port {}", portnum);
-            }
-        }
-    }
-
-    /**
      * Method called by remote port to acknowledge receipt of LLDP sent by
      * this port. If slow port, updates label to fast. If fast port, decrements
      * number of unacknowledged probes.
      *
-     * @param portNumber the port
+     * @param key link key
      */
-    public void ackProbe(Long portNumber) {
+    private void ackProbe(LinkKey key) {
-        synchronized (this) {
+        long portNumber = key.src().port().toLong();
-            if (slowPorts.contains(portNumber)) {
+        if (ports.contains(portNumber)) {
-                log.debug("Setting slow port to fast: {}:{}", device.id(), portNumber);
+            linkTimes.put(key, System.currentTimeMillis());
-                slowPorts.remove(portNumber);
-                fastPorts.add(portNumber);
-                portProbeCount.put(portNumber, new AtomicInteger(0));
-            } else if (fastPorts.contains(portNumber)) {
-                portProbeCount.get(portNumber).set(0);
         } else {
             log.debug("Got ackProbe for non-existing port: {}", portNumber);
         }
     }
-    }
     /**
      * Handles an incoming LLDP packet. Creates link in topology and sends ACK
      * to port where LLDP originated.
      *
-     * @param context packet context
+     * @param packetContext packet context
      * @return true if handled
      */
-    public boolean handleLLDP(PacketContext context) {
+    public boolean handleLLDP(PacketContext packetContext) {
-        Ethernet eth = context.inPacket().parsed();
+        Ethernet eth = packetContext.inPacket().parsed();
         if (eth == null) {
             return false;
         }
@@ -221,20 +158,21 @@ public class LinkDiscovery implements TimerTask {
         ONOSLLDP onoslldp = ONOSLLDP.parseONOSLLDP(eth);
         if (onoslldp != null) {
             PortNumber srcPort = portNumber(onoslldp.getPort());
-            PortNumber dstPort = context.inPacket().receivedFrom().port();
+            PortNumber dstPort = packetContext.inPacket().receivedFrom().port();
             DeviceId srcDeviceId = DeviceId.deviceId(onoslldp.getDeviceString());
-            DeviceId dstDeviceId = context.inPacket().receivedFrom().deviceId();
+            DeviceId dstDeviceId = packetContext.inPacket().receivedFrom().deviceId();
-            ackProbe(dstPort.toLong());
             ConnectPoint src = new ConnectPoint(srcDeviceId, srcPort);
             ConnectPoint dst = new ConnectPoint(dstDeviceId, dstPort);
+            ackProbe(LinkKey.linkKey(src, dst));
+
             LinkDescription ld = eth.getEtherType() == Ethernet.TYPE_LLDP ?
                     new DefaultLinkDescription(src, dst, Type.DIRECT) :
                     new DefaultLinkDescription(src, dst, Type.INDIRECT);
             try {
-                linkProvider.linkDetected(ld);
+                context.providerService().linkDetected(ld);
             } catch (IllegalStateException e) {
                 return true;
             }
@@ -256,48 +194,37 @@ public class LinkDiscovery implements TimerTask {
         if (isStopped()) {
             return;
         }
-        if (!mastershipService.isLocalMaster(device.id())) {
+
+        if (!context.mastershipService().isLocalMaster(device.id())) {
             if (!isStopped()) {
-                // reschedule timer
+                timeout = Timer.getTimer().newTimeout(this, context.probeRate(), MILLISECONDS);
-                timeout = Timer.getTimer().newTimeout(this, probeRate, MILLISECONDS);
             }
             return;
         }
-        log.trace("Sending probes from {}", device.id());
+        // Prune stale links
-        synchronized (this) {
+        linkTimes.entrySet().stream()
-            Iterator<Long> fastIterator = fastPorts.iterator();
+                .filter(e -> isStale(e.getKey(), e.getValue()))
-            while (fastIterator.hasNext()) {
+                .map(Map.Entry::getKey).collect(Collectors.toSet())
-                long portNumber = fastIterator.next();
+                .forEach(this::pruneLink);
-                int probeCount = portProbeCount.get(portNumber).getAndIncrement();
-                if (probeCount < LinkDiscovery.MAX_PROBE_COUNT) {
+        // Probe ports
-                    log.trace("Sending fast probe to port {}", portNumber);
+        log.trace("Sending probes from {}", device.id());
-                    sendProbes(portNumber);
+        ports.forEach(this::sendProbes);
-                } else {
+        if (!isStopped()) {
-                    // Link down, demote to slowPorts; update fast and slow ports
+            timeout = Timer.getTimer().newTimeout(this, context.probeRate(), MILLISECONDS);
-                    fastIterator.remove();
-                    slowPorts.add(portNumber);
-                    portProbeCount.remove(portNumber);
-
-                    ConnectPoint cp = new ConnectPoint(device.id(), portNumber(portNumber));
-                    log.debug("Link down -> {}", cp);
-                    linkProvider.linksVanished(cp);
         }
     }
-            // send a probe for the next slow port
+    private void pruneLink(LinkKey key) {
-            for (long portNumber : slowPorts) {
+        linkTimes.remove(key);
-                log.trace("Sending slow probe to port {}", portNumber);
+        LinkDescription desc = new DefaultLinkDescription(key.src(), key.dst(), Type.DIRECT);
-                sendProbes(portNumber);
+        context.providerService().linkVanished(desc);
-            }
     }
-        if (!isStopped()) {
+    private boolean isStale(LinkKey key, long lastSeen) {
-            // reschedule timer
+        return lastSeen < (System.currentTimeMillis() - context.staleLinkAge());
-            timeout = Timer.getTimer().newTimeout(this, probeRate, MILLISECONDS);
-        }
     }
     public synchronized void stop() {
@@ -351,19 +278,18 @@ public class LinkDiscovery implements TimerTask {
     private void sendProbes(Long portNumber) {
         log.trace("Sending probes out to {}@{}", portNumber, device.id());
         OutboundPacket pkt = createOutBoundLLDP(portNumber);
-        pktService.emit(pkt);
+        context.packetService().emit(pkt);
-        if (useBDDP) {
+        if (context.useBDDP()) {
             OutboundPacket bpkt = createOutBoundBDDP(portNumber);
-            pktService.emit(bpkt);
+            context.packetService().emit(bpkt);
         }
     }
-    public boolean containsPort(Long portNumber) {
-        return slowPorts.contains(portNumber) || fastPorts.contains(portNumber);
-    }
-
     public synchronized boolean isStopped() {
         return isStopped || timeout.isCancelled();
     }
+    boolean containsPort(long portNumber) {
+        return ports.contains(portNumber);
+    }
 }
--- a/providers/lldp/src/test/java/org/onosproject/provider/lldp/impl/LLDPLinkProviderTest.java
View file @05453c9
+++ b/providers/lldp/src/test/java/org/onosproject/provider/lldp/impl/LLDPLinkProviderTest.java
View file @05453c9
@@ -108,7 +108,6 @@ public class LLDPLinkProviderTest {
         provider.providerRegistry = linkService;
         provider.masterService = masterService;
-
         provider.activate(null);
     }