Merge remote-tracking branch 'origin/master'

@@ -372,4 +372,8 @@ public class OpticalConfigProvider extends AbstractProvider implements DevicePro
         // TODO Auto-generated method stub.
     }
 
+    @Override
+    public boolean isReachable(Device device) {
+        return false;
+    }
 }

@@ -15,16 +15,18 @@
  */
 package org.onlab.onos.cli;
 
-import org.onlab.onos.core.ApplicationId;
+import java.util.Comparator;
+
 import org.onlab.onos.cluster.ControllerNode;
+import org.onlab.onos.core.ApplicationId;
+import org.onlab.onos.net.ConnectPoint;
 import org.onlab.onos.net.Element;
 import org.onlab.onos.net.ElementId;
 import org.onlab.onos.net.Port;
 import org.onlab.onos.net.flow.FlowRule;
+import org.onlab.onos.net.host.PortAddresses;
 import org.onlab.onos.net.topology.TopologyCluster;
 
-import java.util.Comparator;
-
 /**
  * Various comparators.
  */
@@ -84,4 +86,21 @@ public final class Comparators {
         }
     };
 
+    public static final Comparator<ConnectPoint> CONNECT_POINT_COMPARATOR = new Comparator<ConnectPoint>() {
+        @Override
+        public int compare(ConnectPoint o1, ConnectPoint o2) {
+            int compareId = ELEMENT_ID_COMPARATOR.compare(o1.elementId(), o2.elementId());
+            return (compareId != 0) ?
+                compareId :
+                Long.signum(o1.port().toLong() - o2.port().toLong());
+        }
+    };
+
+    public static final Comparator<PortAddresses> ADDRESSES_COMPARATOR = new Comparator<PortAddresses>() {
+        @Override
+        public int compare(PortAddresses arg0, PortAddresses arg1) {
+            return CONNECT_POINT_COMPARATOR.compare(arg0.connectPoint(), arg1.connectPoint());
+        }
+    };
+
 }

+package org.onlab.onos.cli.net;
+
+import java.util.Collections;
+import java.util.List;
+import java.util.Set;
+
+import org.apache.karaf.shell.commands.Command;
+import org.onlab.onos.cli.AbstractShellCommand;
+import org.onlab.onos.cli.Comparators;
+import org.onlab.onos.net.host.HostService;
+import org.onlab.onos.net.host.InterfaceIpAddress;
+import org.onlab.onos.net.host.PortAddresses;
+
+import com.google.common.collect.Lists;
+
+/**
+ * Lists all configured address port bindings.
+ */
+@Command(scope = "onos", name = "address-bindings",
+        description = "Lists all configured address port bindings.")
+public class AddressBindingsListCommand extends AbstractShellCommand {
+
+    private static final String FORMAT =
+            "port=%s/%s, ip(s)=%s, mac=%s";
+
+    @Override
+    protected void execute() {
+        HostService hostService = get(HostService.class);
+
+        List<PortAddresses> addresses =
+                Lists.newArrayList(hostService.getAddressBindings());
+
+        Collections.sort(addresses, Comparators.ADDRESSES_COMPARATOR);
+
+        for (PortAddresses pa : addresses) {
+            print(FORMAT, pa.connectPoint().deviceId(), pa.connectPoint().port(),
+                    printIpAddresses(pa.ipAddresses()), pa.mac());
+        }
+    }
+
+    private String printIpAddresses(Set<InterfaceIpAddress> addresses) {
+        StringBuilder output = new StringBuilder("[");
+        for (InterfaceIpAddress address : addresses) {
+            output.append(address.ipAddress().toString());
+            output.append("/");
+            output.append(address.subnetAddress().prefixLength());
+            output.append(", ");
+        }
+        // Remove the last comma
+        output.delete(output.length() - 2 , output.length());
+        output.append("]");
+        return output.toString();
+    }
+
+}

@@ -169,6 +169,9 @@
         <command>
             <action class="org.onlab.onos.cli.net.HostsListCommand"/>
         </command>
+        <command>
+            <action class="org.onlab.onos.cli.net.AddressBindingsListCommand"/>
+        </command>
 
         <command>
             <action class="org.onlab.onos.cli.net.FlowsListCommand"/>

@@ -47,4 +47,11 @@ public interface DeviceProvider extends Provider {
      */
     void roleChanged(Device device, MastershipRole newRole);
 
+    /**
+     * Checks the reachability (connectivity) of a device from this provider.
+     *
+     * @param device device to check
+     * @return true if reachable, false otherwise
+     */
+    boolean isReachable(Device device);
 }

@@ -97,10 +97,20 @@ implements MastershipService, MastershipAdminService {
         checkNotNull(role, ROLE_NULL);
 
         MastershipEvent event = null;
-        if (role.equals(MastershipRole.MASTER)) {
-            event = store.setMaster(nodeId, deviceId);
-        } else {
-            event = store.setStandby(nodeId, deviceId);
+
+        switch (role) {
+            case MASTER:
+                event = store.setMaster(nodeId, deviceId);
+                break;
+            case STANDBY:
+                event = store.setStandby(nodeId, deviceId);
+                break;
+            case NONE:
+                event = store.relinquishRole(nodeId, deviceId);
+                break;
+            default:
+                log.info("Unknown role; ignoring");
+                return;
         }
 
         if (event != null) {
@@ -259,6 +269,10 @@ implements MastershipService, MastershipAdminService {
 
         @Override
         public void notify(MastershipEvent event) {
+            if (clusterService.getLocalNode().id().equals(event.roleInfo().master())) {
+                log.info("ignoring locally-generated event {}", event);
+               // return;
+            }
             log.info("dispatching mastership event {}", event);
             eventDispatcher.post(event);
         }

@@ -29,6 +29,7 @@ import org.apache.felix.scr.annotations.ReferenceCardinality;
 import org.apache.felix.scr.annotations.Service;
 import org.onlab.onos.cluster.ClusterService;
 import org.onlab.onos.cluster.NodeId;
+import org.onlab.onos.cluster.RoleInfo;
 import org.onlab.onos.event.AbstractListenerRegistry;
 import org.onlab.onos.event.EventDeliveryService;
 import org.onlab.onos.mastership.MastershipEvent;
@@ -58,6 +59,8 @@ import org.onlab.onos.net.provider.AbstractProviderRegistry;
 import org.onlab.onos.net.provider.AbstractProviderService;
 import org.slf4j.Logger;
 
+import com.google.common.collect.HashMultimap;
+
 /**
  * Provides implementation of the device SB & NB APIs.
  */
@@ -159,32 +162,37 @@ public class DeviceManager
 
     // Applies the specified role to the device; ignores NONE
     private void applyRole(DeviceId deviceId, MastershipRole newRole) {
-        if (!newRole.equals(MastershipRole.NONE)) {
-            Device device = store.getDevice(deviceId);
-            // FIXME: Device might not be there yet. (eventual consistent)
-            if (device == null) {
-                return;
-            }
-            DeviceProvider provider = getProvider(device.providerId());
-            if (provider != null) {
-                provider.roleChanged(device, newRole);
+        if (newRole.equals(MastershipRole.NONE)) {
+            return;
+        }
 
-                // only trigger event when request was sent to provider
-                // TODO: consider removing this from Device event type?
-                post(new DeviceEvent(DEVICE_MASTERSHIP_CHANGED, device));
+        Device device = store.getDevice(deviceId);
+        // FIXME: Device might not be there yet. (eventual consistent)
+        if (device == null) {
+            return;
+        }
+
+        DeviceProvider provider = getProvider(device.providerId());
+        if (provider != null) {
+            provider.roleChanged(device, newRole);
+            // only trigger event when request was sent to provider
+            // TODO: consider removing this from Device event type?
+            post(new DeviceEvent(DEVICE_MASTERSHIP_CHANGED, device));
+
+            if (newRole.equals(MastershipRole.MASTER)) {
+                provider.triggerProbe(device);
             }
         }
     }
 
-    // Queries a device for port information.
-    private void queryPortInfo(DeviceId deviceId) {
-        Device device = store.getDevice(deviceId);
+    // Check a device for control channel connectivity.
+    private boolean isReachable(Device device) {
         // FIXME: Device might not be there yet. (eventual consistent)
         if (device == null) {
-            return;
+            return false;
         }
         DeviceProvider provider = getProvider(device.providerId());
-        provider.triggerProbe(device);
+        return provider.isReachable(device);
     }
 
     @Override
@@ -236,7 +244,6 @@ public class DeviceManager
             log.info("Device {} connected", deviceId);
             // check my Role
             MastershipRole role = mastershipService.requestRoleFor(deviceId);
-            log.info("requestedRole, became {} for {}", role, deviceId);
             if (role != MastershipRole.MASTER) {
                 // TODO: Do we need to explicitly tell the Provider that
                 // this instance is no longer the MASTER? probably not
@@ -383,6 +390,12 @@ public class DeviceManager
     // Intercepts mastership events
     private class InternalMastershipListener implements MastershipListener {
 
+        // random cache size
+        private final int cacheSize = 5;
+        // temporarily stores term number + events to check for duplicates. A hack.
+        private HashMultimap<Integer, RoleInfo>  eventCache =
+                HashMultimap.create();
+
         @Override
         public void event(MastershipEvent event) {
             final DeviceId did = event.subject();
@@ -391,6 +404,13 @@ public class DeviceManager
             if (myNodeId.equals(event.roleInfo().master())) {
                 MastershipTerm term = termService.getMastershipTerm(did);
 
+                // TODO duplicate suppression should probably occur in the MastershipManager
+                // itself, so listeners that can't deal with duplicates don't have to
+                // so this check themselves.
+                if (checkDuplicate(event.roleInfo(), term.termNumber())) {
+                    return;
+                }
+
                 if (!myNodeId.equals(term.master())) {
                     // something went wrong in consistency, let go
                     log.warn("Mastership has changed after this event."
@@ -405,14 +425,16 @@ public class DeviceManager
                 // only set the new term if I am the master
                 deviceClockProviderService.setMastershipTerm(did, term);
 
-                // FIXME: we should check that the device is connected on our end.
-                // currently, this is not straight forward as the actual switch
-                // implementation is hidden from the registry. Maybe we can ask the
-                // provider.
                 // if the device is null here, we are the first master to claim the
                 // device. No worries, the DeviceManager will create one soon.
                 Device device = getDevice(did);
                 if ((device != null) && !isAvailable(did)) {
+                    if (!isReachable(device)) {
+                        log.warn("Device {} has disconnected after this event", did);
+                        mastershipService.relinquishMastership(did);
+                        applyRole(did, MastershipRole.STANDBY);
+                        return;
+                    }
                     //flag the device as online. Is there a better way to do this?
                     DeviceEvent devEvent = store.createOrUpdateDevice(device.providerId(), did,
                             new DefaultDeviceDescription(
@@ -422,12 +444,32 @@ public class DeviceManager
                     post(devEvent);
                 }
                 applyRole(did, MastershipRole.MASTER);
-                // re-collect device information to fix potential staleness
-                queryPortInfo(did);
             } else if (event.roleInfo().backups().contains(myNodeId)) {
+                if (!isReachable(getDevice(did))) {
+                    log.warn("Device {} has disconnected after this event", did);
+                    mastershipService.relinquishMastership(did);
+                }
                 applyRole(did, MastershipRole.STANDBY);
             }
         }
+
+        // checks for duplicate event, returning true if one is found.
+        private boolean checkDuplicate(RoleInfo roleInfo, int term) {
+            synchronized (eventCache) {
+                if (eventCache.get(term).contains(roleInfo)) {
+                    log.info("duplicate event detected; ignoring");
+                    return true;
+                } else {
+                    eventCache.put(term, roleInfo);
+                    // purge by-term oldest entries to keep the cache size under limit
+                    if (eventCache.size() > cacheSize) {
+                        eventCache.removeAll(term - cacheSize);
+                    }
+                    return false;
+                }
+            }
+        }
+
     }
 
     // Store delegate to re-post events emitted from the store.

@@ -278,6 +278,11 @@ public class DeviceManagerTest {
             deviceReceived = device;
             roleReceived = newRole;
         }
+
+        @Override
+        public boolean isReachable(Device device) {
+            return false;
+        }
     }
 
     private static class TestListener implements DeviceListener {

@@ -272,6 +272,10 @@ implements MastershipStore {
             switch (role) {
                 case MASTER:
                     event = reelect(nodeId, deviceId, rv);
+                    if (event != null) {
+                        Integer term = terms.get(deviceId);
+                        terms.put(deviceId, ++term);
+                    }
                     //fall through to reinforce relinquishment
                 case STANDBY:
                     //fall through to reinforce relinquishment
@@ -304,15 +308,11 @@ implements MastershipStore {
         if (backup == null) {
             log.info("{} giving up and going to NONE for {}", current, deviceId);
             rv.remove(MASTER, current);
-            roleMap.put(deviceId, rv);
             return null;
         } else {
             log.info("{} trying to pass mastership for {} to {}", current, deviceId, backup);
             rv.replace(current, backup, MASTER);
             rv.reassign(backup, STANDBY, NONE);
-            roleMap.put(deviceId, rv);
-            Integer term = terms.get(deviceId);
-            terms.put(deviceId, ++term);
             return new MastershipEvent(MASTER_CHANGED, deviceId, rv.roleInfo());
         }
     }
@@ -366,7 +366,7 @@ implements MastershipStore {
 
         @Override
         public void entryUpdated(EntryEvent<DeviceId, RoleValue> event) {
-
+            // this subsumes entryAdded event
             notifyDelegate(new MastershipEvent(
                     MASTER_CHANGED, event.getKey(), event.getValue().roleInfo()));
         }

@@ -103,22 +103,31 @@ public class OpenFlowDeviceProvider extends AbstractProvider implements DevicePr
         LOG.info("Stopped");
     }
 
-    @Override
-    public void triggerProbe(Device device) {
-        LOG.info("Triggering probe on device {}", device.id());
 
-        // 1. check device liveness
+    @Override
+    public boolean isReachable(Device device) {
         // FIXME if possible, we might want this to be part of
         // OpenFlowSwitch interface so the driver interface isn't misused.
         OpenFlowSwitch sw = controller.getSwitch(dpid(device.id().uri()));
-        if (sw == null ||
-            !((OpenFlowSwitchDriver) sw).isConnected()) {
-            LOG.error("Failed to probe device {} on sw={}", device, sw);
-            providerService.deviceDisconnected(device.id());
-            return;
+        if (sw == null || !((OpenFlowSwitchDriver) sw).isConnected()) {
+            return false;
         }
+        return true;
+        //return checkChannel(device, sw);
+    }
+
+    @Override
+    public void triggerProbe(Device device) {
+        LOG.info("Triggering probe on device {}", device.id());
+
+        OpenFlowSwitch sw = controller.getSwitch(dpid(device.id().uri()));
+        //if (!checkChannel(device, sw)) {
+          //  LOG.error("Failed to probe device {} on sw={}", device, sw);
+        //  providerService.deviceDisconnected(device.id());
+            //return;
+        //}
 
-        // 2. Prompt an update of port information. Do we have an XID for this?
+        // Prompt an update of port information. We can use any XID for this.
         OFFactory fact = sw.factory();
         switch (fact.getVersion()) {
             case OF_10:
@@ -132,6 +141,16 @@ public class OpenFlowDeviceProvider extends AbstractProvider implements DevicePr
         }
     }
 
+    // Checks if the OF channel is connected.
+    //private boolean checkChannel(Device device, OpenFlowSwitch sw) {
+        // FIXME if possible, we might want this to be part of
+        // OpenFlowSwitch interface so the driver interface isn't misused.
+    //    if (sw == null || !((OpenFlowSwitchDriver) sw).isConnected()) {
+      //      return false;
+  //      }
+    //    return true;
+   // }
+
     @Override
     public void roleChanged(Device device, MastershipRole newRole) {
         switch (newRole) {

@@ -226,4 +226,9 @@ class ConfigProvider implements DeviceProvider, LinkProvider, HostProvider {
     public ProviderId id() {
         return PID;
     }
+
+    @Override
+    public boolean isReachable(Device device) {
+        return false;
+    }
 }