Merge branch 'master' of ssh://gerrit.onlab.us:29418/onos-next

+package org.onlab.onos.net;
+
+public final class AnnotationsUtil {
+
+    public static boolean isEqual(Annotations lhs, Annotations rhs) {
+        if (lhs == rhs) {
+            return true;
+        }
+        if (lhs == null || rhs == null) {
+            return false;
+        }
+
+        if (!lhs.keys().equals(rhs.keys())) {
+            return false;
+        }
+
+        for (String key : lhs.keys()) {
+            if (!lhs.value(key).equals(rhs.value(key))) {
+                return false;
+            }
+        }
+        return true;
+    }
+
+    // not to be instantiated
+    private AnnotationsUtil() {}
+}

 package org.onlab.onos.net.link;
 package org.onlab.onos.net.link;
 
 
 import org.onlab.onos.net.ConnectPoint;
 import org.onlab.onos.net.ConnectPoint;
+import org.onlab.onos.net.Description;
 import org.onlab.onos.net.Link;
 import org.onlab.onos.net.Link;
 
 
 /**
 /**
  * Describes an infrastructure link.
  * Describes an infrastructure link.
  */
  */
-public interface LinkDescription {
+public interface LinkDescription extends Description {
 
 
     /**
     /**
      * Returns the link source.
      * Returns the link source.

@@ -76,7 +76,7 @@ public class HostManager
         eventDispatcher.addSink(HostEvent.class, listenerRegistry);
         eventDispatcher.addSink(HostEvent.class, listenerRegistry);
 
 
         monitor = new HostMonitor(deviceService,  packetService, this);
         monitor = new HostMonitor(deviceService,  packetService, this);
-
+        monitor.start();
     }
     }
 
 
     @Deactivate
     @Deactivate

@@ -2,7 +2,7 @@ package org.onlab.onos.net.host.impl;
 
 
 import java.nio.ByteBuffer;
 import java.nio.ByteBuffer;
 import java.util.ArrayList;
 import java.util.ArrayList;
-import java.util.HashSet;
+import java.util.Collections;
 import java.util.List;
 import java.util.List;
 import java.util.Set;
 import java.util.Set;
 import java.util.concurrent.ConcurrentHashMap;
 import java.util.concurrent.ConcurrentHashMap;
@@ -33,8 +33,6 @@ import org.onlab.packet.IpAddress;
 import org.onlab.packet.IpPrefix;
 import org.onlab.packet.IpPrefix;
 import org.onlab.packet.MacAddress;
 import org.onlab.packet.MacAddress;
 import org.onlab.util.Timer;
 import org.onlab.util.Timer;
-import org.slf4j.Logger;
-import org.slf4j.LoggerFactory;
 
 
 /**
 /**
  * Monitors hosts on the dataplane to detect changes in host data.
  * Monitors hosts on the dataplane to detect changes in host data.
@@ -44,15 +42,6 @@ import org.slf4j.LoggerFactory;
  * probe for hosts that have not yet been detected (specified by IP address).
  * probe for hosts that have not yet been detected (specified by IP address).
  */
  */
 public class HostMonitor implements TimerTask {
 public class HostMonitor implements TimerTask {
-    private static final Logger log = LoggerFactory.getLogger(HostMonitor.class);
-
-    private static final byte[] ZERO_MAC_ADDRESS =
-            MacAddress.valueOf("00:00:00:00:00:00").getAddress();
-
-    // TODO put on Ethernet
-    private static final byte[] BROADCAST_MAC =
-            MacAddress.valueOf("ff:ff:ff:ff:ff:ff").getAddress();
-
     private DeviceService deviceService;
     private DeviceService deviceService;
     private PacketService packetService;
     private PacketService packetService;
     private HostManager hostManager;
     private HostManager hostManager;
@@ -64,8 +53,15 @@ public class HostMonitor implements TimerTask {
     private static final long DEFAULT_PROBE_RATE = 30000; // milliseconds
     private static final long DEFAULT_PROBE_RATE = 30000; // milliseconds
     private long probeRate = DEFAULT_PROBE_RATE;
     private long probeRate = DEFAULT_PROBE_RATE;
 
 
-    private final Timeout timeout;
+    private Timeout timeout;
 
 
+    /**
+     * Creates a new host monitor.
+     *
+     * @param deviceService device service used to find edge ports
+     * @param packetService packet service used to send packets on the data plane
+     * @param hostService host service used to look up host information
+     */
     public HostMonitor(DeviceService deviceService, PacketService packetService,
     public HostMonitor(DeviceService deviceService, PacketService packetService,
             HostManager hostService) {
             HostManager hostService) {
 
 
@@ -73,24 +69,59 @@ public class HostMonitor implements TimerTask {
         this.packetService = packetService;
         this.packetService = packetService;
         this.hostManager = hostService;
         this.hostManager = hostService;
 
 
-        monitoredAddresses = new HashSet<>();
+        monitoredAddresses = Collections.newSetFromMap(
+                new ConcurrentHashMap<IpAddress, Boolean>());
         hostProviders = new ConcurrentHashMap<>();
         hostProviders = new ConcurrentHashMap<>();
 
 
         timeout = Timer.getTimer().newTimeout(this, 0, TimeUnit.MILLISECONDS);
         timeout = Timer.getTimer().newTimeout(this, 0, TimeUnit.MILLISECONDS);
     }
     }
 
 
+    /**
+     * Adds an IP address to be monitored by the host monitor. The monitor will
+     * periodically probe the host to detect changes.
+     *
+     * @param ip IP address of the host to monitor
+     */
     void addMonitoringFor(IpAddress ip) {
     void addMonitoringFor(IpAddress ip) {
         monitoredAddresses.add(ip);
         monitoredAddresses.add(ip);
     }
     }
 
 
+    /**
+     * Stops monitoring the given IP address.
+     *
+     * @param ip IP address to stop monitoring on
+     */
     void stopMonitoring(IpAddress ip) {
     void stopMonitoring(IpAddress ip) {
         monitoredAddresses.remove(ip);
         monitoredAddresses.remove(ip);
     }
     }
 
 
+    /**
+     * Starts the host monitor. Does nothing if the monitor is already running.
+     */
+    void start() {
+        synchronized (this) {
+            if (timeout == null) {
+                timeout = Timer.getTimer().newTimeout(this, 0, TimeUnit.MILLISECONDS);
+            }
+        }
+    }
+
+    /**
+     * Stops the host monitor.
+     */
     void shutdown() {
     void shutdown() {
-        timeout.cancel();
+        synchronized (this) {
+            timeout.cancel();
+            timeout = null;
+        }
     }
     }
 
 
+    /**
+     * Registers a host provider with the host monitor. The monitor can use the
+     * provider to probe hosts.
+     *
+     * @param provider the host provider to register
+     */
     void registerHostProvider(HostProvider provider) {
     void registerHostProvider(HostProvider provider) {
         hostProviders.put(provider.id(), provider);
         hostProviders.put(provider.id(), provider);
     }
     }
@@ -117,7 +148,7 @@ public class HostMonitor implements TimerTask {
             }
             }
         }
         }
 
 
-        timeout = Timer.getTimer().newTimeout(this, probeRate, TimeUnit.MILLISECONDS);
+        this.timeout = Timer.getTimer().newTimeout(this, probeRate, TimeUnit.MILLISECONDS);
     }
     }
 
 
     /**
     /**
@@ -173,12 +204,12 @@ public class HostMonitor implements TimerTask {
 
 
         arp.setSenderHardwareAddress(sourceMac.getAddress())
         arp.setSenderHardwareAddress(sourceMac.getAddress())
            .setSenderProtocolAddress(sourceIp.toOctets())
            .setSenderProtocolAddress(sourceIp.toOctets())
-           .setTargetHardwareAddress(ZERO_MAC_ADDRESS)
+           .setTargetHardwareAddress(MacAddress.ZERO_MAC_ADDRESS)
            .setTargetProtocolAddress(targetIp.toOctets());
            .setTargetProtocolAddress(targetIp.toOctets());
 
 
         Ethernet ethernet = new Ethernet();
         Ethernet ethernet = new Ethernet();
         ethernet.setEtherType(Ethernet.TYPE_ARP)
         ethernet.setEtherType(Ethernet.TYPE_ARP)
-                .setDestinationMACAddress(BROADCAST_MAC)
+                .setDestinationMACAddress(MacAddress.BROADCAST_MAC)
                 .setSourceMACAddress(sourceMac.getAddress())
                 .setSourceMACAddress(sourceMac.getAddress())
                 .setPayload(arp);
                 .setPayload(arp);
 
 

@@ -11,7 +11,7 @@ import org.apache.felix.scr.annotations.Deactivate;
 import org.apache.felix.scr.annotations.Reference;
 import org.apache.felix.scr.annotations.Reference;
 import org.apache.felix.scr.annotations.ReferenceCardinality;
 import org.apache.felix.scr.annotations.ReferenceCardinality;
 import org.apache.felix.scr.annotations.Service;
 import org.apache.felix.scr.annotations.Service;
-import org.onlab.onos.net.Annotations;
+import org.onlab.onos.net.AnnotationsUtil;
 import org.onlab.onos.net.DefaultAnnotations;
 import org.onlab.onos.net.DefaultAnnotations;
 import org.onlab.onos.net.DefaultDevice;
 import org.onlab.onos.net.DefaultDevice;
 import org.onlab.onos.net.DefaultPort;
 import org.onlab.onos.net.DefaultPort;
@@ -33,6 +33,7 @@ import org.onlab.onos.store.AbstractStore;
 import org.onlab.onos.store.ClockService;
 import org.onlab.onos.store.ClockService;
 import org.onlab.onos.store.Timestamp;
 import org.onlab.onos.store.Timestamp;
 import org.onlab.onos.store.common.impl.Timestamped;
 import org.onlab.onos.store.common.impl.Timestamped;
+import org.onlab.util.NewConcurrentHashMap;
 import org.slf4j.Logger;
 import org.slf4j.Logger;
 
 
 import java.util.ArrayList;
 import java.util.ArrayList;
@@ -136,8 +137,7 @@ public class GossipDeviceStore
 
 
         // Collection of DeviceDescriptions for a Device
         // Collection of DeviceDescriptions for a Device
         ConcurrentMap<ProviderId, DeviceDescriptions> providerDescs
         ConcurrentMap<ProviderId, DeviceDescriptions> providerDescs
-            = createIfAbsentUnchecked(deviceDescs, deviceId,
-                    new InitConcurrentHashMap<ProviderId, DeviceDescriptions>());
+            = getDeviceDescriptions(deviceId);
 
 
 
 
         DeviceDescriptions descs
         DeviceDescriptions descs
@@ -196,7 +196,7 @@ public class GossipDeviceStore
         // We allow only certain attributes to trigger update
         // We allow only certain attributes to trigger update
         if (!Objects.equals(oldDevice.hwVersion(), newDevice.hwVersion()) ||
         if (!Objects.equals(oldDevice.hwVersion(), newDevice.hwVersion()) ||
             !Objects.equals(oldDevice.swVersion(), newDevice.swVersion()) ||
             !Objects.equals(oldDevice.swVersion(), newDevice.swVersion()) ||
-            !isAnnotationsEqual(oldDevice.annotations(), newDevice.annotations())) {
+            !AnnotationsUtil.isEqual(oldDevice.annotations(), newDevice.annotations())) {
 
 
             boolean replaced = devices.replace(newDevice.id(), oldDevice, newDevice);
             boolean replaced = devices.replace(newDevice.id(), oldDevice, newDevice);
             if (!replaced) {
             if (!replaced) {
@@ -223,8 +223,7 @@ public class GossipDeviceStore
     @Override
     @Override
     public DeviceEvent markOffline(DeviceId deviceId) {
     public DeviceEvent markOffline(DeviceId deviceId) {
         ConcurrentMap<ProviderId, DeviceDescriptions> providerDescs
         ConcurrentMap<ProviderId, DeviceDescriptions> providerDescs
-            = createIfAbsentUnchecked(deviceDescs, deviceId,
-                    new InitConcurrentHashMap<ProviderId, DeviceDescriptions>());
+            = getDeviceDescriptions(deviceId);
 
 
         // locking device
         // locking device
         synchronized (providerDescs) {
         synchronized (providerDescs) {
@@ -327,7 +326,7 @@ public class GossipDeviceStore
                                    Port newPort,
                                    Port newPort,
                                    Map<PortNumber, Port> ports) {
                                    Map<PortNumber, Port> ports) {
         if (oldPort.isEnabled() != newPort.isEnabled() ||
         if (oldPort.isEnabled() != newPort.isEnabled() ||
-            !isAnnotationsEqual(oldPort.annotations(), newPort.annotations())) {
+            !AnnotationsUtil.isEqual(oldPort.annotations(), newPort.annotations())) {
 
 
             ports.put(oldPort.number(), newPort);
             ports.put(oldPort.number(), newPort);
             return new DeviceEvent(PORT_UPDATED, device, newPort);
             return new DeviceEvent(PORT_UPDATED, device, newPort);
@@ -358,7 +357,13 @@ public class GossipDeviceStore
     // exist, it creates and registers a new one.
     // exist, it creates and registers a new one.
     private ConcurrentMap<PortNumber, Port> getPortMap(DeviceId deviceId) {
     private ConcurrentMap<PortNumber, Port> getPortMap(DeviceId deviceId) {
         return createIfAbsentUnchecked(devicePorts, deviceId,
         return createIfAbsentUnchecked(devicePorts, deviceId,
-                new InitConcurrentHashMap<PortNumber, Port>());
+                NewConcurrentHashMap.<PortNumber, Port>ifNeeded());
+    }
+
+    private ConcurrentMap<ProviderId, DeviceDescriptions> getDeviceDescriptions(
+            DeviceId deviceId) {
+        return createIfAbsentUnchecked(deviceDescs, deviceId,
+                NewConcurrentHashMap.<ProviderId, DeviceDescriptions>ifNeeded());
     }
     }
 
 
     @Override
     @Override
@@ -438,33 +443,18 @@ public class GossipDeviceStore
 
 
     @Override
     @Override
     public DeviceEvent removeDevice(DeviceId deviceId) {
     public DeviceEvent removeDevice(DeviceId deviceId) {
-        synchronized (this) {
+        ConcurrentMap<ProviderId, DeviceDescriptions> descs = getDeviceDescriptions(deviceId);
+        synchronized (descs) {
             Device device = devices.remove(deviceId);
             Device device = devices.remove(deviceId);
+            // should DEVICE_REMOVED carry removed ports?
+            devicePorts.get(deviceId).clear();
+            availableDevices.remove(deviceId);
+            descs.clear();
             return device == null ? null :
             return device == null ? null :
-                    new DeviceEvent(DEVICE_REMOVED, device, null);
+                new DeviceEvent(DEVICE_REMOVED, device, null);
         }
         }
     }
     }
 
 
-    private static boolean isAnnotationsEqual(Annotations lhs, Annotations rhs) {
-        if (lhs == rhs) {
-            return true;
-        }
-        if (lhs == null || rhs == null) {
-            return false;
-        }
-
-        if (!lhs.keys().equals(rhs.keys())) {
-            return false;
-        }
-
-        for (String key : lhs.keys()) {
-            if (!lhs.value(key).equals(rhs.value(key))) {
-                return false;
-            }
-        }
-        return true;
-    }
-
     /**
     /**
      * Returns a Device, merging description given from multiple Providers.
      * Returns a Device, merging description given from multiple Providers.
      *
      *
@@ -567,14 +557,6 @@ public class GossipDeviceStore
         return fallBackPrimary;
         return fallBackPrimary;
     }
     }
 
 
-    private static final class InitConcurrentHashMap<K, V> implements
-            ConcurrentInitializer<ConcurrentMap<K, V>> {
-        @Override
-        public ConcurrentMap<K, V> get() throws ConcurrentException {
-            return new ConcurrentHashMap<>();
-        }
-    }
-
     public static final class InitDeviceDescs
     public static final class InitDeviceDescs
         implements ConcurrentInitializer<DeviceDescriptions> {
         implements ConcurrentInitializer<DeviceDescriptions> {
 
 

@@ -116,17 +116,19 @@ public class GossipDeviceStoreTest {
         deviceClockManager.deactivate();
         deviceClockManager.deactivate();
     }
     }
 
 
-    private void putDevice(DeviceId deviceId, String swVersion) {
+    private void putDevice(DeviceId deviceId, String swVersion,
+                           SparseAnnotations... annotations) {
         DeviceDescription description =
         DeviceDescription description =
                 new DefaultDeviceDescription(deviceId.uri(), SWITCH, MFR,
                 new DefaultDeviceDescription(deviceId.uri(), SWITCH, MFR,
-                        HW, swVersion, SN);
+                        HW, swVersion, SN, annotations);
         deviceStore.createOrUpdateDevice(PID, deviceId, description);
         deviceStore.createOrUpdateDevice(PID, deviceId, description);
     }
     }
 
 
-    private void putDeviceAncillary(DeviceId deviceId, String swVersion) {
+    private void putDeviceAncillary(DeviceId deviceId, String swVersion,
+                                    SparseAnnotations... annotations) {
         DeviceDescription description =
         DeviceDescription description =
                 new DefaultDeviceDescription(deviceId.uri(), SWITCH, MFR,
                 new DefaultDeviceDescription(deviceId.uri(), SWITCH, MFR,
-                        HW, swVersion, SN);
+                        HW, swVersion, SN, annotations);
         deviceStore.createOrUpdateDevice(PIDA, deviceId, description);
         deviceStore.createOrUpdateDevice(PIDA, deviceId, description);
     }
     }
 
 
@@ -448,16 +450,37 @@ public class GossipDeviceStoreTest {
 
 
     @Test
     @Test
     public final void testRemoveDevice() {
     public final void testRemoveDevice() {
-        putDevice(DID1, SW1);
+        putDevice(DID1, SW1, A1);
+        List<PortDescription> pds = Arrays.<PortDescription>asList(
+                new DefaultPortDescription(P1, true, A2)
+                );
+        deviceStore.updatePorts(PID, DID1, pds);
         putDevice(DID2, SW1);
         putDevice(DID2, SW1);
 
 
         assertEquals(2, deviceStore.getDeviceCount());
         assertEquals(2, deviceStore.getDeviceCount());
+        assertEquals(1, deviceStore.getPorts(DID1).size());
+        assertAnnotationsEquals(deviceStore.getDevice(DID1).annotations(), A1);
+        assertAnnotationsEquals(deviceStore.getPort(DID1, P1).annotations(), A2);
 
 
         DeviceEvent event = deviceStore.removeDevice(DID1);
         DeviceEvent event = deviceStore.removeDevice(DID1);
         assertEquals(DEVICE_REMOVED, event.type());
         assertEquals(DEVICE_REMOVED, event.type());
         assertDevice(DID1, SW1, event.subject());
         assertDevice(DID1, SW1, event.subject());
 
 
         assertEquals(1, deviceStore.getDeviceCount());
         assertEquals(1, deviceStore.getDeviceCount());
+        assertEquals(0, deviceStore.getPorts(DID1).size());
+
+        // putBack Device, Port w/o annotation
+        putDevice(DID1, SW1);
+        List<PortDescription> pds2 = Arrays.<PortDescription>asList(
+                new DefaultPortDescription(P1, true)
+                );
+        deviceStore.updatePorts(PID, DID1, pds2);
+
+        // annotations should not survive
+        assertEquals(2, deviceStore.getDeviceCount());
+        assertEquals(1, deviceStore.getPorts(DID1).size());
+        assertAnnotationsEquals(deviceStore.getDevice(DID1).annotations());
+        assertAnnotationsEquals(deviceStore.getPort(DID1, P1).annotations());
     }
     }
 
 
     // If Delegates should be called only on remote events,
     // If Delegates should be called only on remote events,

@@ -9,7 +9,7 @@ import org.apache.felix.scr.annotations.Activate;
 import org.apache.felix.scr.annotations.Component;
 import org.apache.felix.scr.annotations.Component;
 import org.apache.felix.scr.annotations.Deactivate;
 import org.apache.felix.scr.annotations.Deactivate;
 import org.apache.felix.scr.annotations.Service;
 import org.apache.felix.scr.annotations.Service;
-import org.onlab.onos.net.Annotations;
+import org.onlab.onos.net.AnnotationsUtil;
 import org.onlab.onos.net.DefaultAnnotations;
 import org.onlab.onos.net.DefaultAnnotations;
 import org.onlab.onos.net.DefaultDevice;
 import org.onlab.onos.net.DefaultDevice;
 import org.onlab.onos.net.DefaultPort;
 import org.onlab.onos.net.DefaultPort;
@@ -28,6 +28,7 @@ import org.onlab.onos.net.device.DeviceStoreDelegate;
 import org.onlab.onos.net.device.PortDescription;
 import org.onlab.onos.net.device.PortDescription;
 import org.onlab.onos.net.provider.ProviderId;
 import org.onlab.onos.net.provider.ProviderId;
 import org.onlab.onos.store.AbstractStore;
 import org.onlab.onos.store.AbstractStore;
+import org.onlab.util.NewConcurrentHashMap;
 import org.slf4j.Logger;
 import org.slf4j.Logger;
 
 
 import java.util.ArrayList;
 import java.util.ArrayList;
@@ -52,7 +53,6 @@ import static org.slf4j.LoggerFactory.getLogger;
 import static org.apache.commons.lang3.concurrent.ConcurrentUtils.createIfAbsentUnchecked;
 import static org.apache.commons.lang3.concurrent.ConcurrentUtils.createIfAbsentUnchecked;
 import static org.onlab.onos.net.DefaultAnnotations.merge;
 import static org.onlab.onos.net.DefaultAnnotations.merge;
 
 
-// TODO: synchronization should be done in more fine-grained manner.
 /**
 /**
  * Manages inventory of infrastructure devices using trivial in-memory
  * Manages inventory of infrastructure devices using trivial in-memory
  * structures implementation.
  * structures implementation.
@@ -109,8 +109,7 @@ public class SimpleDeviceStore
     public synchronized DeviceEvent createOrUpdateDevice(ProviderId providerId, DeviceId deviceId,
     public synchronized DeviceEvent createOrUpdateDevice(ProviderId providerId, DeviceId deviceId,
                                      DeviceDescription deviceDescription) {
                                      DeviceDescription deviceDescription) {
         ConcurrentMap<ProviderId, DeviceDescriptions> providerDescs
         ConcurrentMap<ProviderId, DeviceDescriptions> providerDescs
-            = createIfAbsentUnchecked(deviceDescs, deviceId,
-                    new InitConcurrentHashMap<ProviderId, DeviceDescriptions>());
+            = getDeviceDescriptions(deviceId);
 
 
         Device oldDevice = devices.get(deviceId);
         Device oldDevice = devices.get(deviceId);
 
 
@@ -151,7 +150,7 @@ public class SimpleDeviceStore
         // We allow only certain attributes to trigger update
         // We allow only certain attributes to trigger update
         if (!Objects.equals(oldDevice.hwVersion(), newDevice.hwVersion()) ||
         if (!Objects.equals(oldDevice.hwVersion(), newDevice.hwVersion()) ||
             !Objects.equals(oldDevice.swVersion(), newDevice.swVersion()) ||
             !Objects.equals(oldDevice.swVersion(), newDevice.swVersion()) ||
-            !isAnnotationsEqual(oldDevice.annotations(), newDevice.annotations())) {
+            !AnnotationsUtil.isEqual(oldDevice.annotations(), newDevice.annotations())) {
 
 
             synchronized (this) {
             synchronized (this) {
                 devices.replace(newDevice.id(), oldDevice, newDevice);
                 devices.replace(newDevice.id(), oldDevice, newDevice);
@@ -238,7 +237,7 @@ public class SimpleDeviceStore
                                    Port newPort,
                                    Port newPort,
                                    ConcurrentMap<PortNumber, Port> ports) {
                                    ConcurrentMap<PortNumber, Port> ports) {
         if (oldPort.isEnabled() != newPort.isEnabled() ||
         if (oldPort.isEnabled() != newPort.isEnabled() ||
-            !isAnnotationsEqual(oldPort.annotations(), newPort.annotations())) {
+            !AnnotationsUtil.isEqual(oldPort.annotations(), newPort.annotations())) {
 
 
             ports.put(oldPort.number(), newPort);
             ports.put(oldPort.number(), newPort);
             return new DeviceEvent(PORT_UPDATED, device, newPort);
             return new DeviceEvent(PORT_UPDATED, device, newPort);
@@ -264,11 +263,17 @@ public class SimpleDeviceStore
         return events;
         return events;
     }
     }
 
 
+    private ConcurrentMap<ProviderId, DeviceDescriptions> getDeviceDescriptions(
+            DeviceId deviceId) {
+        return createIfAbsentUnchecked(deviceDescs, deviceId,
+                NewConcurrentHashMap.<ProviderId, DeviceDescriptions>ifNeeded());
+    }
+
     // Gets the map of ports for the specified device; if one does not already
     // Gets the map of ports for the specified device; if one does not already
     // exist, it creates and registers a new one.
     // exist, it creates and registers a new one.
     private ConcurrentMap<PortNumber, Port> getPortMap(DeviceId deviceId) {
     private ConcurrentMap<PortNumber, Port> getPortMap(DeviceId deviceId) {
         return createIfAbsentUnchecked(devicePorts, deviceId,
         return createIfAbsentUnchecked(devicePorts, deviceId,
-                new InitConcurrentHashMap<PortNumber, Port>());
+                NewConcurrentHashMap.<PortNumber, Port>ifNeeded());
     }
     }
 
 
     @Override
     @Override
@@ -323,31 +328,19 @@ public class SimpleDeviceStore
 
 
     @Override
     @Override
     public DeviceEvent removeDevice(DeviceId deviceId) {
     public DeviceEvent removeDevice(DeviceId deviceId) {
-        synchronized (this) {
+        ConcurrentMap<ProviderId, DeviceDescriptions> descs = getDeviceDescriptions(deviceId);
+        synchronized (descs) {
             Device device = devices.remove(deviceId);
             Device device = devices.remove(deviceId);
-            return device == null ? null :
-                    new DeviceEvent(DEVICE_REMOVED, device, null);
-        }
-    }
-
-    private static boolean isAnnotationsEqual(Annotations lhs, Annotations rhs) {
-        if (lhs == rhs) {
-            return true;
-        }
-        if (lhs == null || rhs == null) {
-            return false;
-        }
-
-        if (!lhs.keys().equals(rhs.keys())) {
-            return false;
-        }
-
-        for (String key : lhs.keys()) {
-            if (!lhs.value(key).equals(rhs.value(key))) {
-                return false;
+            // should DEVICE_REMOVED carry removed ports?
+            ConcurrentMap<PortNumber, Port> ports = devicePorts.get(deviceId);
+            if (ports != null) {
+                ports.clear();
             }
             }
+            availableDevices.remove(deviceId);
+            descs.clear();
+            return device == null ? null :
+                new DeviceEvent(DEVICE_REMOVED, device, null);
         }
         }
-        return true;
     }
     }
 
 
     /**
     /**
@@ -445,15 +438,6 @@ public class SimpleDeviceStore
         return fallBackPrimary;
         return fallBackPrimary;
     }
     }
 
 
-    // TODO: can be made generic
-    private static final class InitConcurrentHashMap<K, V> implements
-            ConcurrentInitializer<ConcurrentMap<K, V>> {
-        @Override
-        public ConcurrentMap<K, V> get() throws ConcurrentException {
-            return new ConcurrentHashMap<>();
-        }
-    }
-
     public static final class InitDeviceDescs
     public static final class InitDeviceDescs
         implements ConcurrentInitializer<DeviceDescriptions> {
         implements ConcurrentInitializer<DeviceDescriptions> {
         private final DeviceDescription deviceDesc;
         private final DeviceDescription deviceDesc;

@@ -103,17 +103,19 @@ public class SimpleDeviceStoreTest {
         simpleDeviceStore.deactivate();
         simpleDeviceStore.deactivate();
     }
     }
 
 
-    private void putDevice(DeviceId deviceId, String swVersion) {
+    private void putDevice(DeviceId deviceId, String swVersion,
+                           SparseAnnotations... annotations) {
         DeviceDescription description =
         DeviceDescription description =
                 new DefaultDeviceDescription(deviceId.uri(), SWITCH, MFR,
                 new DefaultDeviceDescription(deviceId.uri(), SWITCH, MFR,
-                        HW, swVersion, SN);
+                        HW, swVersion, SN, annotations);
         deviceStore.createOrUpdateDevice(PID, deviceId, description);
         deviceStore.createOrUpdateDevice(PID, deviceId, description);
     }
     }
 
 
-    private void putDeviceAncillary(DeviceId deviceId, String swVersion) {
+    private void putDeviceAncillary(DeviceId deviceId, String swVersion,
+                                    SparseAnnotations... annotations) {
         DeviceDescription description =
         DeviceDescription description =
                 new DefaultDeviceDescription(deviceId.uri(), SWITCH, MFR,
                 new DefaultDeviceDescription(deviceId.uri(), SWITCH, MFR,
-                        HW, swVersion, SN);
+                        HW, swVersion, SN, annotations);
         deviceStore.createOrUpdateDevice(PIDA, deviceId, description);
         deviceStore.createOrUpdateDevice(PIDA, deviceId, description);
     }
     }
 
 
@@ -126,6 +128,7 @@ public class SimpleDeviceStoreTest {
         assertEquals(SN, device.serialNumber());
         assertEquals(SN, device.serialNumber());
     }
     }
 
 
+    // TODO slice this out somewhere
     /**
     /**
      * Verifies that Annotations created by merging {@code annotations} is
      * Verifies that Annotations created by merging {@code annotations} is
      * equal to actual Annotations.
      * equal to actual Annotations.
@@ -133,7 +136,7 @@ public class SimpleDeviceStoreTest {
      * @param actual Annotations to check
      * @param actual Annotations to check
      * @param annotations
      * @param annotations
      */
      */
-    private static void assertAnnotationsEquals(Annotations actual, SparseAnnotations... annotations) {
+    public static void assertAnnotationsEquals(Annotations actual, SparseAnnotations... annotations) {
         DefaultAnnotations expected = DefaultAnnotations.builder().build();
         DefaultAnnotations expected = DefaultAnnotations.builder().build();
         for (SparseAnnotations a : annotations) {
         for (SparseAnnotations a : annotations) {
             expected = DefaultAnnotations.merge(expected, a);
             expected = DefaultAnnotations.merge(expected, a);
@@ -347,6 +350,7 @@ public class SimpleDeviceStoreTest {
         assertFalse("Port is disabled", event.port().isEnabled());
         assertFalse("Port is disabled", event.port().isEnabled());
 
 
     }
     }
+
     @Test
     @Test
     public final void testUpdatePortStatusAncillary() {
     public final void testUpdatePortStatusAncillary() {
         putDeviceAncillary(DID1, SW1);
         putDeviceAncillary(DID1, SW1);
@@ -435,16 +439,37 @@ public class SimpleDeviceStoreTest {
 
 
     @Test
     @Test
     public final void testRemoveDevice() {
     public final void testRemoveDevice() {
-        putDevice(DID1, SW1);
+        putDevice(DID1, SW1, A1);
+        List<PortDescription> pds = Arrays.<PortDescription>asList(
+                new DefaultPortDescription(P1, true, A2)
+                );
+        deviceStore.updatePorts(PID, DID1, pds);
         putDevice(DID2, SW1);
         putDevice(DID2, SW1);
 
 
         assertEquals(2, deviceStore.getDeviceCount());
         assertEquals(2, deviceStore.getDeviceCount());
+        assertEquals(1, deviceStore.getPorts(DID1).size());
+        assertAnnotationsEquals(deviceStore.getDevice(DID1).annotations(), A1);
+        assertAnnotationsEquals(deviceStore.getPort(DID1, P1).annotations(), A2);
 
 
         DeviceEvent event = deviceStore.removeDevice(DID1);
         DeviceEvent event = deviceStore.removeDevice(DID1);
         assertEquals(DEVICE_REMOVED, event.type());
         assertEquals(DEVICE_REMOVED, event.type());
         assertDevice(DID1, SW1, event.subject());
         assertDevice(DID1, SW1, event.subject());
 
 
         assertEquals(1, deviceStore.getDeviceCount());
         assertEquals(1, deviceStore.getDeviceCount());
+        assertEquals(0, deviceStore.getPorts(DID1).size());
+
+        // putBack Device, Port w/o annotation
+        putDevice(DID1, SW1);
+        List<PortDescription> pds2 = Arrays.<PortDescription>asList(
+                new DefaultPortDescription(P1, true)
+                );
+        deviceStore.updatePorts(PID, DID1, pds2);
+
+        // annotations should not survive
+        assertEquals(2, deviceStore.getDeviceCount());
+        assertEquals(1, deviceStore.getPorts(DID1).size());
+        assertAnnotationsEquals(deviceStore.getDevice(DID1).annotations());
+        assertAnnotationsEquals(deviceStore.getPort(DID1, P1).annotations());
     }
     }
 
 
     // If Delegates should be called only on remote events,
     // If Delegates should be called only on remote events,

@@ -4,7 +4,9 @@ import static org.junit.Assert.*;
 import static org.onlab.onos.net.DeviceId.deviceId;
 import static org.onlab.onos.net.DeviceId.deviceId;
 import static org.onlab.onos.net.Link.Type.*;
 import static org.onlab.onos.net.Link.Type.*;
 import static org.onlab.onos.net.link.LinkEvent.Type.*;
 import static org.onlab.onos.net.link.LinkEvent.Type.*;
+import static org.onlab.onos.store.trivial.impl.SimpleDeviceStoreTest.assertAnnotationsEquals;
 
 
+import java.util.Collections;
 import java.util.HashMap;
 import java.util.HashMap;
 import java.util.Map;
 import java.util.Map;
 import java.util.Set;
 import java.util.Set;
@@ -18,10 +20,12 @@ import org.junit.BeforeClass;
 import org.junit.Ignore;
 import org.junit.Ignore;
 import org.junit.Test;
 import org.junit.Test;
 import org.onlab.onos.net.ConnectPoint;
 import org.onlab.onos.net.ConnectPoint;
+import org.onlab.onos.net.DefaultAnnotations;
 import org.onlab.onos.net.DeviceId;
 import org.onlab.onos.net.DeviceId;
 import org.onlab.onos.net.Link;
 import org.onlab.onos.net.Link;
 import org.onlab.onos.net.LinkKey;
 import org.onlab.onos.net.LinkKey;
 import org.onlab.onos.net.PortNumber;
 import org.onlab.onos.net.PortNumber;
+import org.onlab.onos.net.SparseAnnotations;
 import org.onlab.onos.net.Link.Type;
 import org.onlab.onos.net.Link.Type;
 import org.onlab.onos.net.link.DefaultLinkDescription;
 import org.onlab.onos.net.link.DefaultLinkDescription;
 import org.onlab.onos.net.link.LinkEvent;
 import org.onlab.onos.net.link.LinkEvent;
@@ -37,6 +41,7 @@ import com.google.common.collect.Iterables;
 public class SimpleLinkStoreTest {
 public class SimpleLinkStoreTest {
 
 
     private static final ProviderId PID = new ProviderId("of", "foo");
     private static final ProviderId PID = new ProviderId("of", "foo");
+    private static final ProviderId PIDA = new ProviderId("of", "bar", true);
     private static final DeviceId DID1 = deviceId("of:foo");
     private static final DeviceId DID1 = deviceId("of:foo");
     private static final DeviceId DID2 = deviceId("of:bar");
     private static final DeviceId DID2 = deviceId("of:bar");
 
 
@@ -44,6 +49,23 @@ public class SimpleLinkStoreTest {
     private static final PortNumber P2 = PortNumber.portNumber(2);
     private static final PortNumber P2 = PortNumber.portNumber(2);
     private static final PortNumber P3 = PortNumber.portNumber(3);
     private static final PortNumber P3 = PortNumber.portNumber(3);
 
 
+    private static final SparseAnnotations A1 = DefaultAnnotations.builder()
+            .set("A1", "a1")
+            .set("B1", "b1")
+            .build();
+    private static final SparseAnnotations A1_2 = DefaultAnnotations.builder()
+            .remove("A1")
+            .set("B3", "b3")
+            .build();
+    private static final SparseAnnotations A2 = DefaultAnnotations.builder()
+            .set("A2", "a2")
+            .set("B2", "b2")
+            .build();
+    private static final SparseAnnotations A2_2 = DefaultAnnotations.builder()
+            .remove("A2")
+            .set("B4", "b4")
+            .build();
+
 
 
     private SimpleLinkStore simpleLinkStore;
     private SimpleLinkStore simpleLinkStore;
     private LinkStore linkStore;
     private LinkStore linkStore;
@@ -69,16 +91,17 @@ public class SimpleLinkStoreTest {
     }
     }
 
 
     private void putLink(DeviceId srcId, PortNumber srcNum,
     private void putLink(DeviceId srcId, PortNumber srcNum,
-                         DeviceId dstId, PortNumber dstNum, Type type) {
+                         DeviceId dstId, PortNumber dstNum, Type type,
+                         SparseAnnotations... annotations) {
         ConnectPoint src = new ConnectPoint(srcId, srcNum);
         ConnectPoint src = new ConnectPoint(srcId, srcNum);
         ConnectPoint dst = new ConnectPoint(dstId, dstNum);
         ConnectPoint dst = new ConnectPoint(dstId, dstNum);
-        linkStore.createOrUpdateLink(PID, new DefaultLinkDescription(src, dst, type));
+        linkStore.createOrUpdateLink(PID, new DefaultLinkDescription(src, dst, type, annotations));
     }
     }
 
 
-    private void putLink(LinkKey key, Type type) {
+    private void putLink(LinkKey key, Type type, SparseAnnotations... annotations) {
         putLink(key.src().deviceId(), key.src().port(),
         putLink(key.src().deviceId(), key.src().port(),
                 key.dst().deviceId(), key.dst().port(),
                 key.dst().deviceId(), key.dst().port(),
-                type);
+                type, annotations);
     }
     }
 
 
     private static void assertLink(DeviceId srcId, PortNumber srcNum,
     private static void assertLink(DeviceId srcId, PortNumber srcNum,
@@ -270,14 +293,67 @@ public class SimpleLinkStoreTest {
     }
     }
 
 
     @Test
     @Test
+    public final void testCreateOrUpdateLinkAncillary() {
+        ConnectPoint src = new ConnectPoint(DID1, P1);
+        ConnectPoint dst = new ConnectPoint(DID2, P2);
+
+        // add Ancillary link
+        LinkEvent event = linkStore.createOrUpdateLink(PIDA,
+                    new DefaultLinkDescription(src, dst, INDIRECT, A1));
+
+        assertNull("Ancillary only link is ignored", event);
+
+        // add Primary link
+        LinkEvent event2 = linkStore.createOrUpdateLink(PID,
+                new DefaultLinkDescription(src, dst, INDIRECT, A2));
+
+        assertLink(DID1, P1, DID2, P2, INDIRECT, event2.subject());
+        assertAnnotationsEquals(event2.subject().annotations(), A2, A1);
+        assertEquals(LINK_ADDED, event2.type());
+
+        // update link type
+        LinkEvent event3 = linkStore.createOrUpdateLink(PID,
+                new DefaultLinkDescription(src, dst, DIRECT, A2));
+        assertLink(DID1, P1, DID2, P2, DIRECT, event3.subject());
+        assertAnnotationsEquals(event3.subject().annotations(), A2, A1);
+        assertEquals(LINK_UPDATED, event3.type());
+
+
+        // no change
+        LinkEvent event4 = linkStore.createOrUpdateLink(PID,
+                new DefaultLinkDescription(src, dst, DIRECT));
+        assertNull("No change event expected", event4);
+
+        // update link annotation (Primary)
+        LinkEvent event5 = linkStore.createOrUpdateLink(PID,
+                new DefaultLinkDescription(src, dst, DIRECT, A2_2));
+        assertLink(DID1, P1, DID2, P2, DIRECT, event5.subject());
+        assertAnnotationsEquals(event5.subject().annotations(), A2, A2_2, A1);
+        assertEquals(LINK_UPDATED, event5.type());
+
+        // update link annotation (Ancillary)
+        LinkEvent event6 = linkStore.createOrUpdateLink(PIDA,
+                new DefaultLinkDescription(src, dst, DIRECT, A1_2));
+        assertLink(DID1, P1, DID2, P2, DIRECT, event6.subject());
+        assertAnnotationsEquals(event6.subject().annotations(), A2, A2_2, A1, A1_2);
+        assertEquals(LINK_UPDATED, event6.type());
+
+        // update link type (Ancillary) : ignored
+        LinkEvent event7 = linkStore.createOrUpdateLink(PIDA,
+                new DefaultLinkDescription(src, dst, EDGE));
+        assertNull("Ancillary change other than annotation is ignored", event7);
+    }
+
+
+    @Test
     public final void testRemoveLink() {
     public final void testRemoveLink() {
         final ConnectPoint d1P1 = new ConnectPoint(DID1, P1);
         final ConnectPoint d1P1 = new ConnectPoint(DID1, P1);
         final ConnectPoint d2P2 = new ConnectPoint(DID2, P2);
         final ConnectPoint d2P2 = new ConnectPoint(DID2, P2);
         LinkKey linkId1 = new LinkKey(d1P1, d2P2);
         LinkKey linkId1 = new LinkKey(d1P1, d2P2);
         LinkKey linkId2 = new LinkKey(d2P2, d1P1);
         LinkKey linkId2 = new LinkKey(d2P2, d1P1);
 
 
-        putLink(linkId1, DIRECT);
-        putLink(linkId2, DIRECT);
+        putLink(linkId1, DIRECT, A1);
+        putLink(linkId2, DIRECT, A2);
 
 
         // DID1,P1 => DID2,P2
         // DID1,P1 => DID2,P2
         // DID2,P2 => DID1,P1
         // DID2,P2 => DID1,P1
@@ -285,10 +361,41 @@ public class SimpleLinkStoreTest {
 
 
         LinkEvent event = linkStore.removeLink(d1P1, d2P2);
         LinkEvent event = linkStore.removeLink(d1P1, d2P2);
         assertEquals(LINK_REMOVED, event.type());
         assertEquals(LINK_REMOVED, event.type());
+        assertAnnotationsEquals(event.subject().annotations(), A1);
         LinkEvent event2 = linkStore.removeLink(d1P1, d2P2);
         LinkEvent event2 = linkStore.removeLink(d1P1, d2P2);
         assertNull(event2);
         assertNull(event2);
 
 
         assertLink(linkId2, DIRECT, linkStore.getLink(d2P2, d1P1));
         assertLink(linkId2, DIRECT, linkStore.getLink(d2P2, d1P1));
+        assertAnnotationsEquals(linkStore.getLink(d2P2, d1P1).annotations(), A2);
+
+        // annotations, etc. should not survive remove
+        putLink(linkId1, DIRECT);
+        assertLink(linkId1, DIRECT, linkStore.getLink(d1P1, d2P2));
+        assertAnnotationsEquals(linkStore.getLink(d1P1, d2P2).annotations());
+    }
+
+    @Test
+    public final void testAncillaryOnlyNotVisible() {
+        ConnectPoint src = new ConnectPoint(DID1, P1);
+        ConnectPoint dst = new ConnectPoint(DID2, P2);
+
+        // add Ancillary link
+        linkStore.createOrUpdateLink(PIDA,
+                    new DefaultLinkDescription(src, dst, INDIRECT, A1));
+
+        // Ancillary only link should not be visible
+        assertEquals(0, linkStore.getLinkCount());
+
+        assertTrue(Iterables.isEmpty(linkStore.getLinks()));
+
+        assertNull(linkStore.getLink(src, dst));
+
+        assertEquals(Collections.emptySet(), linkStore.getIngressLinks(dst));
+
+        assertEquals(Collections.emptySet(), linkStore.getEgressLinks(src));
+
+        assertEquals(Collections.emptySet(), linkStore.getDeviceEgressLinks(DID1));
+        assertEquals(Collections.emptySet(), linkStore.getDeviceIngressLinks(DID2));
     }
     }
 
 
     // If Delegates should be called only on remote events,
     // If Delegates should be called only on remote events,

@@ -167,11 +167,11 @@ class OFChannelHandler extends IdleStateAwareChannelHandler {
                 // TODO We could check for the optional bitmap, but for now
                 // TODO We could check for the optional bitmap, but for now
                 // we are just checking the version number.
                 // we are just checking the version number.
                 if (m.getVersion() == OFVersion.OF_13) {
                 if (m.getVersion() == OFVersion.OF_13) {
-                    log.info("Received {} Hello from {}", m.getVersion(),
+                    log.debug("Received {} Hello from {}", m.getVersion(),
                             h.channel.getRemoteAddress());
                             h.channel.getRemoteAddress());
                     h.ofVersion = OFVersion.OF_13;
                     h.ofVersion = OFVersion.OF_13;
                 } else if (m.getVersion() == OFVersion.OF_10) {
                 } else if (m.getVersion() == OFVersion.OF_10) {
-                    log.info("Received {} Hello from {} - switching to OF "
+                    log.debug("Received {} Hello from {} - switching to OF "
                             + "version 1.0", m.getVersion(),
                             + "version 1.0", m.getVersion(),
                             h.channel.getRemoteAddress());
                             h.channel.getRemoteAddress());
                     h.ofVersion = OFVersion.OF_10;
                     h.ofVersion = OFVersion.OF_10;
@@ -222,7 +222,7 @@ class OFChannelHandler extends IdleStateAwareChannelHandler {
             void processOFFeaturesReply(OFChannelHandler h, OFFeaturesReply  m)
             void processOFFeaturesReply(OFChannelHandler h, OFFeaturesReply  m)
                     throws IOException {
                     throws IOException {
                 h.thisdpid = m.getDatapathId().getLong();
                 h.thisdpid = m.getDatapathId().getLong();
-                log.info("Received features reply for switch at {} with dpid {}",
+                log.debug("Received features reply for switch at {} with dpid {}",
                         h.getSwitchInfoString(), h.thisdpid);
                         h.getSwitchInfoString(), h.thisdpid);
 
 
                 h.featuresReply = m; //temp store
                 h.featuresReply = m; //temp store
@@ -409,7 +409,7 @@ class OFChannelHandler extends IdleStateAwareChannelHandler {
 
 
 
 
 
 
-                log.info("Switch {} bound to class {}, description {}",
+                log.debug("Switch {} bound to class {}, description {}",
                         new Object[] {h.sw, h.sw.getClass(), drep });
                         new Object[] {h.sw, h.sw.getClass(), drep });
                 //Put switch in EQUAL mode until we hear back from the global registry
                 //Put switch in EQUAL mode until we hear back from the global registry
                 //log.debug("Setting new switch {} to EQUAL and sending Role request",
                 //log.debug("Setting new switch {} to EQUAL and sending Role request",
@@ -651,7 +651,7 @@ class OFChannelHandler extends IdleStateAwareChannelHandler {
          * @param error The error message
          * @param error The error message
          */
          */
         protected void logError(OFChannelHandler h, OFErrorMsg error) {
         protected void logError(OFChannelHandler h, OFErrorMsg error) {
-            log.info("{} from switch {} in state {}",
+            log.error("{} from switch {} in state {}",
                     new Object[] {
                     new Object[] {
                     error,
                     error,
                     h.getSwitchInfoString(),
                     h.getSwitchInfoString(),
@@ -1050,7 +1050,7 @@ class OFChannelHandler extends IdleStateAwareChannelHandler {
             throws Exception {
             throws Exception {
         OFFactory factory = (ofVersion == OFVersion.OF_13) ? factory13 : factory10;
         OFFactory factory = (ofVersion == OFVersion.OF_13) ? factory13 : factory10;
         OFMessage m = factory.buildEchoRequest().build();
         OFMessage m = factory.buildEchoRequest().build();
-        log.info("Sending Echo Request on idle channel: {}",
+        log.debug("Sending Echo Request on idle channel: {}",
                 e.getChannel().getPipeline().getLast().toString());
                 e.getChannel().getPipeline().getLast().toString());
         e.getChannel().write(Collections.singletonList(m));
         e.getChannel().write(Collections.singletonList(m));
         // XXX S some problems here -- echo request has no transaction id, and
         // XXX S some problems here -- echo request has no transaction id, and

@@ -22,6 +22,12 @@ import java.util.Arrays;
  *
  *
  */
  */
 public class MacAddress {
 public class MacAddress {
+    public static final byte[] ZERO_MAC_ADDRESS =
+            MacAddress.valueOf("00:00:00:00:00:00").getAddress();
+
+    public static final byte[] BROADCAST_MAC =
+            MacAddress.valueOf("ff:ff:ff:ff:ff:ff").getAddress();
+
     public static final int MAC_ADDRESS_LENGTH = 6;
     public static final int MAC_ADDRESS_LENGTH = 6;
     private byte[] address = new byte[MacAddress.MAC_ADDRESS_LENGTH];
     private byte[] address = new byte[MacAddress.MAC_ADDRESS_LENGTH];
 
 

+package org.onlab.util;
+
+import java.util.concurrent.ConcurrentHashMap;
+import java.util.concurrent.ConcurrentMap;
+
+import org.apache.commons.lang3.concurrent.ConcurrentException;
+import org.apache.commons.lang3.concurrent.ConcurrentInitializer;
+
+/**
+ * Creates an instance of new ConcurrentHashMap on each {@link #get()} call.
+ * <p>
+ * To be used with
+ * {@link org.apache.commons.lang3.concurrent.ConcurrentUtils#createIfAbsent()
+ *  ConcurrentUtils#createIfAbsent}
+ *
+ * @param <K> ConcurrentHashMap key type
+ * @param <V> ConcurrentHashMap value type
+ */
+public final class NewConcurrentHashMap<K, V>
+    implements  ConcurrentInitializer<ConcurrentMap<K, V>> {
+
+    public static final NewConcurrentHashMap<?, ?> INSTANCE = new NewConcurrentHashMap<>();
+
+    @SuppressWarnings("unchecked")
+    public static <K, V> NewConcurrentHashMap<K, V> ifNeeded() {
+        return (NewConcurrentHashMap<K, V>) INSTANCE;
+    }
+
+    @Override
+    public ConcurrentMap<K, V> get() throws ConcurrentException {
+        return new ConcurrentHashMap<>();
+    }
+}

@@ -9,7 +9,7 @@ public class EchoHandler implements MessageHandler {
 
 
     @Override
     @Override
     public void handle(Message message) throws IOException {
     public void handle(Message message) throws IOException {
-        System.out.println("Received: " + message.payload() + ". Echoing it back to the sender.");
+        System.out.println("Received message. Echoing it back to the sender.");
         message.respond(message.payload());
         message.respond(message.payload());
     }
     }
 }
 }

@@ -8,6 +8,14 @@ public class Endpoint {
     private final int port;
     private final int port;
     private final String host;
     private final String host;
 
 
+    /**
+     * Used for serialization.
+     */
+    private Endpoint() {
+        port = 0;
+        host = null;
+    }
+
     public Endpoint(String host, int port) {
     public Endpoint(String host, int port) {
         this.host = host;
         this.host = host;
         this.port = port;
         this.port = port;

@@ -35,6 +35,10 @@ public final class InternalMessage implements Message {
         return payload;
         return payload;
     }
     }
 
 
+    protected void setMessagingService(NettyMessagingService messagingService) {
+        this.messagingService = messagingService;
+    }
+
     @Override
     @Override
     public void respond(Object data) throws IOException {
     public void respond(Object data) throws IOException {
         Builder builder = new Builder(messagingService);
         Builder builder = new Builder(messagingService);

 package org.onlab.netty;
 package org.onlab.netty;
 
 
 import org.onlab.util.KryoPool;
 import org.onlab.util.KryoPool;
-import org.slf4j.Logger;
-import org.slf4j.LoggerFactory;
 
 
+import java.nio.ByteBuffer;
 import java.util.ArrayList;
 import java.util.ArrayList;
 import java.util.HashMap;
 import java.util.HashMap;
 
 
@@ -12,8 +11,6 @@ import java.util.HashMap;
  */
  */
 public class KryoSerializer implements Serializer {
 public class KryoSerializer implements Serializer {
 
 
-    private final Logger log = LoggerFactory.getLogger(getClass());
-
     private KryoPool serializerPool;
     private KryoPool serializerPool;
 
 
     public KryoSerializer() {
     public KryoSerializer() {
@@ -28,7 +25,9 @@ public class KryoSerializer implements Serializer {
         serializerPool = KryoPool.newBuilder()
         serializerPool = KryoPool.newBuilder()
                 .register(ArrayList.class,
                 .register(ArrayList.class,
                           HashMap.class,
                           HashMap.class,
-                          ArrayList.class
+                          ArrayList.class,
+                          InternalMessage.class,
+                          Endpoint.class
                 )
                 )
                 .build()
                 .build()
                 .populate(1);
                 .populate(1);
@@ -36,7 +35,7 @@ public class KryoSerializer implements Serializer {
 
 
 
 
     @Override
     @Override
-    public Object decode(byte[] data) {
+    public <T> T decode(byte[] data) {
         return serializerPool.deserialize(data);
         return serializerPool.deserialize(data);
     }
     }
 
 
@@ -44,4 +43,14 @@ public class KryoSerializer implements Serializer {
     public byte[] encode(Object payload) {
     public byte[] encode(Object payload) {
         return serializerPool.serialize(payload);
         return serializerPool.serialize(payload);
     }
     }
+
+    @Override
+    public <T> T deserialize(ByteBuffer buffer) {
+        return serializerPool.deserialize(buffer);
+    }
+
+    @Override
+    public void serialize(Object obj, ByteBuffer buffer) {
+        serializerPool.serialize(obj, buffer);
+    }
 }
 }

 package org.onlab.netty;
 package org.onlab.netty;
 
 
-import java.util.Arrays;
-import java.util.List;
-
 import static com.google.common.base.Preconditions.checkState;
 import static com.google.common.base.Preconditions.checkState;
-
 import io.netty.buffer.ByteBuf;
 import io.netty.buffer.ByteBuf;
 import io.netty.channel.ChannelHandlerContext;
 import io.netty.channel.ChannelHandlerContext;
-import io.netty.handler.codec.ByteToMessageDecoder;
+import io.netty.handler.codec.ReplayingDecoder;
+
+import java.util.Arrays;
+import java.util.List;
 
 
-/**
- * Decode bytes into a InternalMessage.
- */
-public class MessageDecoder extends ByteToMessageDecoder {
+// TODO: Implement performance enchancements such as those described in the javadoc for ReplayingDecoder.
+public class MessageDecoder extends ReplayingDecoder<InternalMessage> {
 
 
     private final NettyMessagingService messagingService;
     private final NettyMessagingService messagingService;
     private final Serializer serializer;
     private final Serializer serializer;
@@ -23,36 +20,21 @@ public class MessageDecoder extends ByteToMessageDecoder {
     }
     }
 
 
     @Override
     @Override
-    protected void decode(ChannelHandlerContext context, ByteBuf in,
-            List<Object> messages) throws Exception {
+    protected void decode(
+            ChannelHandlerContext context,
+            ByteBuf buffer,
+            List<Object> out) throws Exception {
 
 
-        byte[] preamble = in.readBytes(MessageEncoder.PREAMBLE.length).array();
+        byte[] preamble = new byte[MessageEncoder.PREAMBLE.length];
+        buffer.readBytes(preamble);
         checkState(Arrays.equals(MessageEncoder.PREAMBLE, preamble), "Message has wrong preamble");
         checkState(Arrays.equals(MessageEncoder.PREAMBLE, preamble), "Message has wrong preamble");
 
 
-        // read message Id.
-        long id = in.readLong();
-
-        // read message type; first read size and then bytes.
-        String type = new String(in.readBytes(in.readInt()).array());
-
-        // read sender host name; first read size and then bytes.
-        String host = new String(in.readBytes(in.readInt()).array());
-
-        // read sender port.
-        int port = in.readInt();
-
-        Endpoint sender = new Endpoint(host, port);
-
-        // read message payload; first read size and then bytes.
-        Object payload = serializer.decode(in.readBytes(in.readInt()).array());
-
-        InternalMessage message = new InternalMessage.Builder(messagingService)
-                .withId(id)
-                .withSender(sender)
-                .withType(type)
-                .withPayload(payload)
-                .build();
+        int bodySize = buffer.readInt();
+        byte[] body = new byte[bodySize];
+        buffer.readBytes(body);
 
 
-        messages.add(message);
+        InternalMessage message = serializer.decode(body);
+        message.setMessagingService(messagingService);
+        out.add(message);
     }
     }
 }
 }

@@ -19,42 +19,20 @@ public class MessageEncoder extends MessageToByteEncoder<InternalMessage> {
     }
     }
 
 
     @Override
     @Override
-    protected void encode(ChannelHandlerContext context, InternalMessage message,
+    protected void encode(
+            ChannelHandlerContext context,
+            InternalMessage message,
             ByteBuf out) throws Exception {
             ByteBuf out) throws Exception {
 
 
         // write preamble
         // write preamble
         out.writeBytes(PREAMBLE);
         out.writeBytes(PREAMBLE);
 
 
-        // write id
-        out.writeLong(message.id());
+        byte[] payload = serializer.encode(message);
 
 
-        // write type length
-        out.writeInt(message.type().length());
-
-        // write type
-        out.writeBytes(message.type().getBytes());
-
-        // write sender host name size
-        out.writeInt(message.sender().host().length());
-
-        // write sender host name.
-        out.writeBytes(message.sender().host().getBytes());
-
-        // write port
-        out.writeInt(message.sender().port());
-
-        try {
-            serializer.encode(message.payload());
-        } catch (Exception e) {
-            e.printStackTrace();
-        }
-
-        byte[] payload = serializer.encode(message.payload());
-
-        // write payload length.
+        // write payload length
         out.writeInt(payload.length);
         out.writeInt(payload.length);
 
 
-        // write payload bytes
+        // write payload.
         out.writeBytes(payload);
         out.writeBytes(payload);
     }
     }
 }
 }

@@ -22,7 +22,6 @@ import io.netty.channel.socket.nio.NioServerSocketChannel;
 import io.netty.channel.socket.nio.NioSocketChannel;
 import io.netty.channel.socket.nio.NioSocketChannel;
 
 
 import org.apache.commons.lang.math.RandomUtils;
 import org.apache.commons.lang.math.RandomUtils;
-import org.apache.commons.pool.KeyedObjectPool;
 import org.apache.commons.pool.KeyedPoolableObjectFactory;
 import org.apache.commons.pool.KeyedPoolableObjectFactory;
 import org.apache.commons.pool.impl.GenericKeyedObjectPool;
 import org.apache.commons.pool.impl.GenericKeyedObjectPool;
 import org.slf4j.Logger;
 import org.slf4j.Logger;
@@ -38,8 +37,8 @@ public class NettyMessagingService implements MessagingService {
 
 
     private final Logger log = LoggerFactory.getLogger(getClass());
     private final Logger log = LoggerFactory.getLogger(getClass());
 
 
-    private KeyedObjectPool<Endpoint, Channel> channels =
-            new GenericKeyedObjectPool<Endpoint, Channel>(new OnosCommunicationChannelFactory());
+    private GenericKeyedObjectPool<Endpoint, Channel> channels;
+
     private final int port;
     private final int port;
     private final EventLoopGroup bossGroup = new NioEventLoopGroup();
     private final EventLoopGroup bossGroup = new NioEventLoopGroup();
     private final EventLoopGroup workerGroup = new NioEventLoopGroup();
     private final EventLoopGroup workerGroup = new NioEventLoopGroup();
@@ -66,6 +65,9 @@ public class NettyMessagingService implements MessagingService {
     }
     }
 
 
     public void activate() throws Exception {
     public void activate() throws Exception {
+        channels = new GenericKeyedObjectPool<Endpoint, Channel>(new OnosCommunicationChannelFactory());
+        channels.setTestOnBorrow(true);
+        channels.setTestOnReturn(true);
         responseFutures = CacheBuilder.newBuilder()
         responseFutures = CacheBuilder.newBuilder()
                 .maximumSize(100000)
                 .maximumSize(100000)
                 .weakValues()
                 .weakValues()
@@ -95,17 +97,14 @@ public class NettyMessagingService implements MessagingService {
     protected void sendAsync(Endpoint ep, InternalMessage message) throws IOException {
     protected void sendAsync(Endpoint ep, InternalMessage message) throws IOException {
         Channel channel = null;
         Channel channel = null;
         try {
         try {
-            channel = channels.borrowObject(ep);
-            channel.eventLoop().execute(new WriteTask(channel, message));
-        } catch (Exception e) {
-            throw new IOException(e);
-        } finally {
             try {
             try {
+                channel = channels.borrowObject(ep);
+                channel.eventLoop().execute(new WriteTask(channel, message));
+            } finally {
                 channels.returnObject(ep, channel);
                 channels.returnObject(ep, channel);
-            } catch (Exception e) {
-                log.warn("Error returning object back to the pool", e);
-                // ignored.
             }
             }
+        } catch (Exception e) {
+            throw new IOException(e);
         }
         }
     }
     }
 
 
@@ -141,6 +140,8 @@ public class NettyMessagingService implements MessagingService {
 
 
     private void startAcceptingConnections() throws InterruptedException {
     private void startAcceptingConnections() throws InterruptedException {
         ServerBootstrap b = new ServerBootstrap();
         ServerBootstrap b = new ServerBootstrap();
+        b.option(ChannelOption.WRITE_BUFFER_HIGH_WATER_MARK, 32 * 1024);
+        b.option(ChannelOption.WRITE_BUFFER_LOW_WATER_MARK, 32 * 1024);
         b.option(ChannelOption.ALLOCATOR, PooledByteBufAllocator.DEFAULT);
         b.option(ChannelOption.ALLOCATOR, PooledByteBufAllocator.DEFAULT);
         b.group(bossGroup, workerGroup)
         b.group(bossGroup, workerGroup)
             .channel(NioServerSocketChannel.class)
             .channel(NioServerSocketChannel.class)
@@ -169,6 +170,8 @@ public class NettyMessagingService implements MessagingService {
         public Channel makeObject(Endpoint ep) throws Exception {
         public Channel makeObject(Endpoint ep) throws Exception {
             Bootstrap b = new Bootstrap();
             Bootstrap b = new Bootstrap();
             b.option(ChannelOption.ALLOCATOR, PooledByteBufAllocator.DEFAULT);
             b.option(ChannelOption.ALLOCATOR, PooledByteBufAllocator.DEFAULT);
+            b.option(ChannelOption.WRITE_BUFFER_HIGH_WATER_MARK, 32 * 1024);
+            b.option(ChannelOption.WRITE_BUFFER_LOW_WATER_MARK, 32 * 1024);
             b.group(workerGroup);
             b.group(workerGroup);
             // TODO: Make this faster:
             // TODO: Make this faster:
             // http://normanmaurer.me/presentations/2014-facebook-eng-netty/slides.html#37.0
             // http://normanmaurer.me/presentations/2014-facebook-eng-netty/slides.html#37.0
@@ -197,20 +200,20 @@ public class NettyMessagingService implements MessagingService {
         @Override
         @Override
         protected void initChannel(SocketChannel channel) throws Exception {
         protected void initChannel(SocketChannel channel) throws Exception {
             channel.pipeline()
             channel.pipeline()
-                .addLast(new MessageEncoder(serializer))
-                .addLast(new MessageDecoder(NettyMessagingService.this, serializer))
-                .addLast(new NettyMessagingService.InboundMessageDispatcher());
+                .addLast("encoder", new MessageEncoder(serializer))
+                .addLast("decoder", new MessageDecoder(NettyMessagingService.this, serializer))
+                .addLast("handler", new InboundMessageDispatcher());
         }
         }
     }
     }
 
 
     private class WriteTask implements Runnable {
     private class WriteTask implements Runnable {
 
 
-        private final Object message;
+        private final InternalMessage message;
         private final Channel channel;
         private final Channel channel;
 
 
-        public WriteTask(Channel channel, Object message) {
-            this.message = message;
+        public WriteTask(Channel channel, InternalMessage message) {
             this.channel = channel;
             this.channel = channel;
+            this.message = message;
         }
         }
 
 
         @Override
         @Override
@@ -240,5 +243,11 @@ public class NettyMessagingService implements MessagingService {
             MessageHandler handler = NettyMessagingService.this.getMessageHandler(type);
             MessageHandler handler = NettyMessagingService.this.getMessageHandler(type);
             handler.handle(message);
             handler.handle(message);
         }
         }
+
+
+        @Override
+        public void exceptionCaught(ChannelHandlerContext context, Throwable cause) {
+            context.close();
+        }
     }
     }
 }
 }

 package org.onlab.netty;
 package org.onlab.netty;
 
 
+import java.nio.ByteBuffer;
+
 /**
 /**
  * Interface for encoding/decoding message payloads.
  * Interface for encoding/decoding message payloads.
  */
  */
@@ -11,7 +13,7 @@ public interface Serializer {
      * @param data byte array.
      * @param data byte array.
      * @return POJO
      * @return POJO
      */
      */
-    Object decode(byte[] data);
+    public <T> T decode(byte[] data);
 
 
     /**
     /**
      * Encodes the specified POJO into a byte array.
      * Encodes the specified POJO into a byte array.
@@ -19,6 +21,23 @@ public interface Serializer {
      * @param data POJO to be encoded
      * @param data POJO to be encoded
      * @return byte array.
      * @return byte array.
      */
      */
-    byte[] encode(Object message);
+    public byte[] encode(Object data);
+
+    /**
+     * Serializes the specified object into bytes using one of the
+     * pre-registered serializers.
+     *
+     * @param obj object to be serialized
+     * @param buffer to write serialized bytes
+     */
+    public void serialize(final Object obj, ByteBuffer buffer);
 
 
+    /**
+     * Deserializes the specified bytes into an object using one of the
+     * pre-registered serializers.
+     *
+     * @param buffer bytes to be deserialized
+     * @return deserialized object
+     */
+    public <T> T deserialize(final ByteBuffer buffer);
 }
 }