A race condition occurs between ARPHandler and HostTracker if the ARP
[controller.git] / opendaylight / hosttracker / implementation / src / main / java / org / opendaylight / controller / hosttracker / internal / HostTracker.java
index 7c312e1577b927850e3c11839c3b817cb9282238..aa1c8592d9225f997d7906e7dbb3e679b5f0f270 100644 (file)
@@ -33,11 +33,11 @@ import org.opendaylight.controller.clustering.services.CacheConfigException;
 import org.opendaylight.controller.clustering.services.CacheExistException;
 import org.opendaylight.controller.clustering.services.IClusterContainerServices;
 import org.opendaylight.controller.clustering.services.IClusterServices;
-import org.opendaylight.controller.hosttracker.hostAware.HostNodeConnector;
-import org.opendaylight.controller.hosttracker.hostAware.IHostFinder;
 import org.opendaylight.controller.hosttracker.IfHostListener;
 import org.opendaylight.controller.hosttracker.IfIptoHost;
 import org.opendaylight.controller.hosttracker.IfNewHostNotify;
+import org.opendaylight.controller.hosttracker.hostAware.HostNodeConnector;
+import org.opendaylight.controller.hosttracker.hostAware.IHostFinder;
 import org.opendaylight.controller.sal.core.ConstructionException;
 import org.opendaylight.controller.sal.core.Edge;
 import org.opendaylight.controller.sal.core.Host;
@@ -52,6 +52,7 @@ import org.opendaylight.controller.sal.packet.address.EthernetAddress;
 import org.opendaylight.controller.sal.topology.TopoEdgeUpdate;
 import org.opendaylight.controller.sal.utils.GlobalConstants;
 import org.opendaylight.controller.sal.utils.HexEncode;
+import org.opendaylight.controller.sal.utils.NetUtils;
 import org.opendaylight.controller.sal.utils.NodeCreator;
 import org.opendaylight.controller.sal.utils.Status;
 import org.opendaylight.controller.sal.utils.StatusCode;
@@ -77,10 +78,9 @@ import org.slf4j.LoggerFactory;
  *       removed the database
  */
 
-public class HostTracker implements IfIptoHost, IfHostListener,
-        ISwitchManagerAware, IInventoryListener, ITopologyManagerAware {
-    private static final Logger logger = LoggerFactory
-            .getLogger(HostTracker.class);
+public class HostTracker implements IfIptoHost, IfHostListener, ISwitchManagerAware, IInventoryListener,
+        ITopologyManagerAware {
+    private static final Logger logger = LoggerFactory.getLogger(HostTracker.class);
     private IHostFinder hostFinder;
     private ConcurrentMap<InetAddress, HostNodeConnector> hostsDB;
     /*
@@ -89,16 +89,15 @@ public class HostTracker implements IfIptoHost, IfHostListener,
      * added here until both come up
      */
     private ConcurrentMap<NodeConnector, HostNodeConnector> inactiveStaticHosts;
-    private Set<IfNewHostNotify> newHostNotify = Collections
-            .synchronizedSet(new HashSet<IfNewHostNotify>());
+    private final Set<IfNewHostNotify> newHostNotify = Collections.synchronizedSet(new HashSet<IfNewHostNotify>());
 
     private ITopologyManager topologyManager;
     private IClusterContainerServices clusterContainerService = null;
     private ISwitchManager switchManager = null;
     private Timer timer;
-    private Timer arp_refresh_timer;
+    private Timer arpRefreshTimer;
     private String containerName = null;
-
+    private ExecutorService executor;
     private static class ARPPending {
         protected InetAddress hostIP;
         protected short sent_count;
@@ -131,7 +130,7 @@ public class HostTracker implements IfIptoHost, IfHostListener,
 
     // This list contains the hosts for which ARP requests are being sent
     // periodically
-    private List<ARPPending> ARPPendingList = new ArrayList<HostTracker.ARPPending>();
+    private final List<ARPPending> ARPPendingList = new ArrayList<HostTracker.ARPPending>();
     /*
      * This list below contains the hosts which were initially in ARPPendingList
      * above, but ARP response didn't come from there hosts after multiple
@@ -140,15 +139,15 @@ public class HostTracker implements IfIptoHost, IfHostListener,
      * wasn't created for this host in the controller. This would cause
      * arphandler not to know where to send the ARP 2. The host facing port is
      * down 3. The IP host doesn't exist or is not responding to ARP requests
-     * 
+     *
      * Conditions 1 and 2 above can be recovered if ARP is sent when the
      * relevant L3 interface is added or the port facing host comes up. Whenever
      * L3 interface is added or host facing port comes up, ARP will be sent to
      * hosts in this list.
-     * 
+     *
      * We can't recover from condition 3 above
      */
-    private ArrayList<ARPPending> failedARPReqList = new ArrayList<HostTracker.ARPPending>();
+    private final ArrayList<ARPPending> failedARPReqList = new ArrayList<HostTracker.ARPPending>();
 
     public HostTracker() {
     }
@@ -159,10 +158,10 @@ public class HostTracker implements IfIptoHost, IfHostListener,
 
         timer = new Timer();
         timer.schedule(new OutStandingARPHandler(), 4000, 4000);
-
+        executor = Executors.newFixedThreadPool(2);
         /* ARP Refresh Timer to go off every 5 seconds to implement ARP aging */
-        arp_refresh_timer = new Timer();
-        arp_refresh_timer.schedule(new ARPRefreshHandler(), 5000, 5000);
+        arpRefreshTimer = new Timer();
+        arpRefreshTimer.schedule(new ARPRefreshHandler(), 5000, 5000);
         logger.debug("startUp: Caches created, timers started");
     }
 
@@ -213,17 +212,6 @@ public class HostTracker implements IfIptoHost, IfHostListener,
         inactiveStaticHosts = new ConcurrentHashMap<NodeConnector, HostNodeConnector>();
     }
 
-    @SuppressWarnings("deprecation")
-    private void destroyCache() {
-        if (this.clusterContainerService == null) {
-            logger.error("un-initialized clusterMger, can't destroy cache");
-            return;
-        }
-        logger.debug("Destroying Cache for HostTracker");
-        this.clusterContainerService.destroyCache("hostTrackerAH");
-        this.clusterContainerService.destroyCache("hostTrackerIH");
-        nonClusterObjectCreate();
-    }
 
     public void shutDown() {
     }
@@ -267,27 +255,20 @@ public class HostTracker implements IfIptoHost, IfHostListener,
         return hostsDB.get(networkAddress);
     }
 
-    private Entry<NodeConnector, HostNodeConnector> getHostFromInactiveDB(
-            InetAddress networkAddress) {
-        for (Entry<NodeConnector, HostNodeConnector> entry : inactiveStaticHosts
-                .entrySet()) {
+    private Entry<NodeConnector, HostNodeConnector> getHostFromInactiveDB(InetAddress networkAddress) {
+        for (Entry<NodeConnector, HostNodeConnector> entry : inactiveStaticHosts.entrySet()) {
             if (entry.getValue().equalsByIP(networkAddress)) {
-                logger.debug(
-                        "getHostFromInactiveDB(): Inactive Host found for IP:{} ",
-                        networkAddress.getHostAddress());
+                logger.debug("getHostFromInactiveDB(): Inactive Host found for IP:{} ", networkAddress.getHostAddress());
                 return entry;
             }
         }
-        logger.debug(
-                "getHostFromInactiveDB() Inactive Host Not found for IP: {}",
-                networkAddress.getHostAddress());
+        logger.debug("getHostFromInactiveDB() Inactive Host Not found for IP: {}", networkAddress.getHostAddress());
         return null;
     }
 
     private void removeHostFromInactiveDB(InetAddress networkAddress) {
         NodeConnector nodeConnector = null;
-        for (Entry<NodeConnector, HostNodeConnector> entry : inactiveStaticHosts
-                .entrySet()) {
+        for (Entry<NodeConnector, HostNodeConnector> entry : inactiveStaticHosts.entrySet()) {
             if (entry.getValue().equalsByIP(networkAddress)) {
                 nodeConnector = entry.getKey();
                 break;
@@ -295,12 +276,10 @@ public class HostTracker implements IfIptoHost, IfHostListener,
         }
         if (nodeConnector != null) {
             inactiveStaticHosts.remove(nodeConnector);
-            logger.debug("removeHostFromInactiveDB(): Host Removed for IP: {}",
-                    networkAddress.getHostAddress());
+            logger.debug("removeHostFromInactiveDB(): Host Removed for IP: {}", networkAddress.getHostAddress());
             return;
         }
-        logger.debug("removeHostFromInactiveDB(): Host Not found for IP: {}",
-                networkAddress.getHostAddress());
+        logger.debug("removeHostFromInactiveDB(): Host Not found for IP: {}", networkAddress.getHostAddress());
     }
 
     protected boolean hostMoved(HostNodeConnector host) {
@@ -310,22 +289,23 @@ public class HostTracker implements IfIptoHost, IfHostListener,
         return false;
     }
 
+    @Override
     public HostNodeConnector hostQuery(InetAddress networkAddress) {
         return hostsDB.get(networkAddress);
     }
 
+    @Override
     public Future<HostNodeConnector> discoverHost(InetAddress networkAddress) {
-        ExecutorService executor = Executors.newFixedThreadPool(1);
         if (executor == null) {
             logger.error("discoverHost: Null executor");
             return null;
         }
-        Callable<HostNodeConnector> worker = new HostTrackerCallable(this,
-                networkAddress);
+        Callable<HostNodeConnector> worker = new HostTrackerCallable(this, networkAddress);
         Future<HostNodeConnector> submit = executor.submit(worker);
         return submit;
     }
 
+    @Override
     public HostNodeConnector hostFind(InetAddress networkAddress) {
         /*
          * Sometimes at boot with containers configured in the startup we hit
@@ -340,20 +320,23 @@ public class HostTracker implements IfIptoHost, IfHostListener,
 
         HostNodeConnector host = hostQuery(networkAddress);
         if (host != null) {
-            logger.debug("hostFind(): Host found for IP: {}",
-                    networkAddress.getHostAddress());
+            logger.debug("hostFind(): Host found for IP: {}", networkAddress.getHostAddress());
             return host;
         }
-        /* host is not found, initiate a discovery */
-        hostFinder.find(networkAddress);
-        /* Also add this host to ARPPending List for any potential retries */
+
+        /* Add this host to ARPPending List for any potential retries */
+
         AddtoARPPendingList(networkAddress);
-        logger.debug(
-                "hostFind(): Host Not Found for IP: {}, Inititated Host Discovery ...",
+        logger.debug("hostFind(): Host Not Found for IP: {}, Inititated Host Discovery ...",
                 networkAddress.getHostAddress());
+
+        /* host is not found, initiate a discovery */
+
+        hostFinder.find(networkAddress);
         return null;
     }
 
+    @Override
     public Set<HostNodeConnector> getAllHosts() {
         Set<HostNodeConnector> allHosts = new HashSet<HostNodeConnector>();
         for (Entry<InetAddress, HostNodeConnector> entry : hostsDB.entrySet()) {
@@ -380,8 +363,7 @@ public class HostTracker implements IfIptoHost, IfHostListener,
     @Override
     public Set<HostNodeConnector> getInactiveStaticHosts() {
         Set<HostNodeConnector> list = new HashSet<HostNodeConnector>();
-        for (Entry<NodeConnector, HostNodeConnector> entry : inactiveStaticHosts
-                .entrySet()) {
+        for (Entry<NodeConnector, HostNodeConnector> entry : inactiveStaticHosts.entrySet()) {
             list.add(entry.getValue());
         }
         logger.debug("getInactiveStaticHosts(): Found {} Hosts", list.size());
@@ -394,14 +376,12 @@ public class HostTracker implements IfIptoHost, IfHostListener,
         arphost.setHostIP(networkAddr);
         arphost.setSent_count((short) 1);
         ARPPendingList.add(arphost);
-        logger.debug("Host Added to ARPPending List, IP: {}",
-                networkAddr.toString());
+        logger.debug("Host Added to ARPPending List, IP: {}", networkAddr);
     }
 
     private void removePendingARPFromList(int index) {
         if (index >= ARPPendingList.size()) {
-            logger.warn(
-                    "removePendingARPFromList(): index greater than the List. Size:{}, Index:{}",
+            logger.warn("removePendingARPFromList(): index greater than the List. Size:{}, Index:{}",
                     ARPPendingList.size(), index);
             return;
         }
@@ -411,8 +391,7 @@ public class HostTracker implements IfIptoHost, IfHostListener,
             htCallable.wakeup();
     }
 
-    public void setCallableOnPendingARP(InetAddress networkAddr,
-            HostTrackerCallable callable) {
+    public void setCallableOnPendingARP(InetAddress networkAddr, HostTrackerCallable callable) {
         ARPPending arphost;
         for (int i = 0; i < ARPPendingList.size(); i++) {
             arphost = ARPPendingList.get(i);
@@ -433,8 +412,7 @@ public class HostTracker implements IfIptoHost, IfHostListener,
                  * the request
                  */
                 removePendingARPFromList(i);
-                logger.debug("Host Removed from ARPPending List, IP: {}",
-                        networkAddr.toString());
+                logger.debug("Host Removed from ARPPending List, IP: {}", networkAddr);
                 return;
             }
         }
@@ -451,8 +429,7 @@ public class HostTracker implements IfIptoHost, IfHostListener,
                  * the request
                  */
                 failedARPReqList.remove(i);
-                logger.debug("Host Removed from FailedARPReqList List, IP: {}",
-                        networkAddr.toString());
+                logger.debug("Host Removed from FailedARPReqList List, IP: {}", networkAddr);
                 return;
             }
         }
@@ -461,73 +438,100 @@ public class HostTracker implements IfIptoHost, IfHostListener,
     // Learn a new Host
     private void learnNewHost(HostNodeConnector host) {
         host.initArpSendCountDown();
-        hostsDB.put(host.getNetworkAddress(), host);
-        logger.debug("New Host Learned: MAC: {}  IP: {}",
-                HexEncode.bytesToHexString(host.getDataLayerAddressBytes()),
-                host.getNetworkAddress().getHostAddress());
+        HostNodeConnector rHost = hostsDB.putIfAbsent(host.getNetworkAddress(), host);
+        if (rHost != null) {
+            // Another host is already learned for this IP address, replace it
+            replaceHost(host.getNetworkAddress(), rHost, host);
+        } else {
+            logger.debug("New Host Learned: MAC: {}  IP: {}", HexEncode.bytesToHexString(host
+                    .getDataLayerAddressBytes()), host.getNetworkAddress().getHostAddress());
+        }
+    }
+
+    private void replaceHost(InetAddress networkAddr, HostNodeConnector removedHost, HostNodeConnector newHost) {
+        newHost.initArpSendCountDown();
+        if (hostsDB.replace(networkAddr, removedHost, newHost)) {
+            logger.debug("Host move occurred: Old Host IP:{}, New Host IP: {}", removedHost.getNetworkAddress()
+                    .getHostAddress(), newHost.getNetworkAddress().getHostAddress());
+            logger.debug("Old Host MAC: {}, New Host MAC: {}",
+                    HexEncode.bytesToHexString(removedHost.getDataLayerAddressBytes()),
+                    HexEncode.bytesToHexString(newHost.getDataLayerAddressBytes()));
+            // Display the Old and New HostNodeConnectors also
+            logger.debug("Old {}, New {}", removedHost, newHost);
+        } else {
+            /*
+             * Host replacement has failed, do the recovery
+             */
+            hostsDB.put(networkAddr, newHost);
+            logger.error("Host replacement failed. Overwrite the host. Repalced Host: {}, New Host: {}", removedHost,
+                    newHost);
+        }
+        notifyHostLearnedOrRemoved(removedHost, false);
+        notifyHostLearnedOrRemoved(newHost, true);
+        if (!newHost.isStaticHost()) {
+            ProcPendingARPReqs(networkAddr);
+        }
     }
 
     // Remove known Host
     private void removeKnownHost(InetAddress key) {
         HostNodeConnector host = hostsDB.get(key);
         if (host != null) {
-            logger.debug("Removing Host: IP:{}", host.getNetworkAddress()
-                    .getHostAddress());
+            logger.debug("Removing Host: IP:{}", host.getNetworkAddress().getHostAddress());
             hostsDB.remove(key);
         } else {
-            logger.error(
-                    "removeKnownHost(): Host for IP address {} not found in hostsDB",
-                    key.getHostAddress());
+            logger.error("removeKnownHost(): Host for IP address {} not found in hostsDB", key.getHostAddress());
         }
     }
 
     private class NotifyHostThread extends Thread {
 
-        private HostNodeConnector host;
+        private final HostNodeConnector host;
 
         public NotifyHostThread(HostNodeConnector h) {
             this.host = h;
         }
 
+        @Override
         public void run() {
+            HostNodeConnector removedHost = null;
+            InetAddress networkAddr = host.getNetworkAddress();
+
             /* Check for Host Move case */
             if (hostMoved(host)) {
                 /*
                  * Host has been moved from one location (switch,port, MAC, or
-                 * VLAN). Remove the existing host with its previous location
-                 * parameters, inform the applications, and add it as a new Host
+                 * VLAN) to another. Replace the existing host and its previous
+                 * location parameters with new information, and notify the
+                 * applications listening to host move.
                  */
-                HostNodeConnector removedHost = hostsDB.get(host
-                        .getNetworkAddress());
-                removeKnownHost(host.getNetworkAddress());
+                removedHost = hostsDB.get(networkAddr);
                 if (removedHost != null) {
-                    notifyHostLearnedOrRemoved(removedHost, false);
-                    logger.debug(
-                            "Host move occurred. Old Host:{}, New Host: {}",
-                            removedHost, host);
+                    replaceHost(networkAddr, removedHost, host);
+                    return;
                 } else {
-                    logger.error(
-                            "Host to be removed not found in hostsDB. Host {}",
-                            removedHost);
+                    logger.error("Host to be removed not found in hostsDB. Host {}", removedHost);
                 }
             }
 
-            /* check if there is an outstanding request for this host */
-            InetAddress networkAddr = host.getNetworkAddress();
+            if (removedHost == null) {
+                // It is a new host
+                learnNewHost(host);
+            }
 
-            // add and notify
-            learnNewHost(host);
+            /* check if there is an outstanding request for this host */
             ProcPendingARPReqs(networkAddr);
             notifyHostLearnedOrRemoved(host, true);
         }
     }
 
+    @Override
     public void hostListener(HostNodeConnector host) {
 
+        logger.debug("ARP received for Host: IP {}, MAC {}, {}", host.getNetworkAddress().getHostAddress(),
+                HexEncode.bytesToHexString(host.getDataLayerAddressBytes()), host);
         if (hostExists(host)) {
-            logger.debug("ARP received for Host: {}", host);
-            HostNodeConnector existinghost = hostsDB.get(host
-                    .getNetworkAddress());
+            HostNodeConnector existinghost = hostsDB.get(host.getNetworkAddress());
             existinghost.initArpSendCountDown();
             return;
         }
@@ -539,6 +543,8 @@ public class HostTracker implements IfIptoHost, IfHostListener,
     private void notifyHostLearnedOrRemoved(HostNodeConnector host, boolean add) {
         // Update listeners if any
         if (newHostNotify != null) {
+            logger.debug("Notifying Applications for Host {} Being {}", host.getNetworkAddress().getHostAddress(),
+                    add ? "Added" : "Deleted");
             synchronized (this.newHostNotify) {
                 for (IfNewHostNotify ta : newHostNotify) {
                     try {
@@ -562,39 +568,20 @@ public class HostTracker implements IfIptoHost, IfHostListener,
         Host h = null;
         NodeConnector p = host.getnodeConnector();
         try {
-            DataLinkAddress dla = new EthernetAddress(
-                    host.getDataLayerAddressBytes());
-            h = new org.opendaylight.controller.sal.core.Host(dla,
-                    host.getNetworkAddress());
+            DataLinkAddress dla = new EthernetAddress(host.getDataLayerAddressBytes());
+            h = new Host(dla, host.getNetworkAddress());
         } catch (ConstructionException ce) {
             p = null;
             h = null;
         }
 
         if (topologyManager != null && p != null && h != null) {
+            logger.debug("Notifying Topology Manager for Host {} Being {}", h.getNetworkAddress().getHostAddress(),
+                    add ? "Added" : "Deleted");
             if (add == true) {
                 Tier tier = new Tier(1);
                 switchManager.setNodeProp(node, tier);
                 topologyManager.updateHostLink(p, h, UpdateType.ADDED, null);
-                /*
-                 * This is a temporary fix for Cisco Live's Hadoop
-                 * Demonstration. The concept of Tiering must be revisited based
-                 * on other application requirements and the design might
-                 * warrant a separate module (as it involves tracking the
-                 * topology/ host changes & updating the Tiering numbers in an
-                 * effective manner).
-                 */
-                updateSwitchTiers(node, 1);
-
-                /*
-                 * The following 2 lines are added for testing purposes. We can
-                 * remove it once the North-Bound APIs are available for
-                 * testing.
-                 * 
-                 * ArrayList<ArrayList<String>> hierarchies =
-                 * getHostNetworkHierarchy(host.getNetworkAddress());
-                 * logHierarchies(hierarchies);
-                 */
             } else {
                 // No need to reset the tiering if no other hosts are currently
                 // connected
@@ -611,10 +598,10 @@ public class HostTracker implements IfIptoHost, IfHostListener,
      * When a new Host is learnt by the hosttracker module, it places the
      * directly connected Node in Tier-1 & using this function, updates the Tier
      * value for all other Nodes in the network hierarchy.
-     * 
+     *
      * This is a recursive function and it takes care of updating the Tier value
      * for all the connected and eligible Nodes.
-     * 
+     *
      * @param n
      *            Node that represents one of the Vertex in the Topology Graph.
      * @param currentTier
@@ -623,21 +610,17 @@ public class HostTracker implements IfIptoHost, IfHostListener,
     private void updateSwitchTiers(Node n, int currentTier) {
         Map<Node, Set<Edge>> ndlinks = topologyManager.getNodeEdges();
         if (ndlinks == null) {
-            logger.debug(
-                    "updateSwitchTiers(): ndlinks null for Node: {}, Tier:{}",
-                    n, currentTier);
+            logger.debug("updateSwitchTiers(): ndlinks null for Node: {}, Tier:{}", n, currentTier);
             return;
         }
         Set<Edge> links = ndlinks.get(n);
         if (links == null) {
-            logger.debug("updateSwitchTiers(): links null for ndlinks:{}",
-                    ndlinks);
+            logger.debug("updateSwitchTiers(): links null for ndlinks:{}", ndlinks);
             return;
         }
         ArrayList<Node> needsVisiting = new ArrayList<Node>();
         for (Edge lt : links) {
-            if (!lt.getHeadNodeConnector().getType()
-                    .equals(NodeConnector.NodeConnectorIDType.OPENFLOW)) {
+            if (!lt.getHeadNodeConnector().getType().equals(NodeConnector.NodeConnectorIDType.OPENFLOW)) {
                 // We don't want to work on Node that are not openflow
                 // for now
                 continue;
@@ -663,7 +646,7 @@ public class HostTracker implements IfIptoHost, IfHostListener,
      * Internal convenience routine to check the eligibility of a Switch for a
      * Tier update. Any Node with Tier=0 or a Tier value that is greater than
      * the new Tier Value is eligible for the update.
-     * 
+     *
      * @param n
      *            Node for which the Tier update eligibility is checked
      * @param tier
@@ -674,8 +657,7 @@ public class HostTracker implements IfIptoHost, IfHostListener,
 
     private boolean switchNeedsTieringUpdate(Node n, int tier) {
         if (n == null) {
-            logger.error("switchNeedsTieringUpdate(): Null node for tier: {}",
-                    tier);
+            logger.error("switchNeedsTieringUpdate(): Null node for tier: {}", tier);
             return false;
         }
         /*
@@ -727,7 +709,7 @@ public class HostTracker implements IfIptoHost, IfHostListener,
             for (String switchName : hierarchy) {
                 buf.append(switchName + "/");
             }
-            logger.debug("{} -> {}", getContainerName(), buf.toString());
+            logger.debug("{} -> {}", getContainerName(), buf);
             num++;
         }
     }
@@ -737,12 +719,13 @@ public class HostTracker implements IfIptoHost, IfHostListener,
      * that returns the Network Hierarchy for a given Host. This API is
      * typically used by applications like Hadoop for Rack Awareness
      * functionality.
-     * 
+     *
      * @param hostAddress
      *            IP-Address of the host/node.
      * @return Network Hierarchies represented by an Array of Array (of
      *         Switch-Ids as String).
      */
+    @Override
     public List<List<String>> getHostNetworkHierarchy(InetAddress hostAddress) {
         HostNodeConnector host = hostQuery(hostAddress);
         if (host == null)
@@ -762,7 +745,7 @@ public class HostTracker implements IfIptoHost, IfHostListener,
      * is used as the network for Hadoop Demos & in order to give a meaningful
      * rack-awareness switch names, the DPID is organized in ASCII Characters
      * and retrieved as string.
-     * 
+     *
      * @param dpid
      *            Switch DataPath Id
      * @return Ascii String represented by the DPID.
@@ -785,7 +768,7 @@ public class HostTracker implements IfIptoHost, IfHostListener,
 
     /**
      * A convenient recursive routine to obtain the Hierarchy of Switches.
-     * 
+     *
      * @param node
      *            Current Node in the Recursive routine.
      * @param currHierarchy
@@ -795,18 +778,17 @@ public class HostTracker implements IfIptoHost, IfHostListener,
      *            Array of multiple Hierarchies that represent a given host.
      */
     @SuppressWarnings("unchecked")
-    private void updateCurrentHierarchy(Node node,
-            ArrayList<String> currHierarchy, List<List<String>> fullHierarchy) {
+    private void updateCurrentHierarchy(Node node, ArrayList<String> currHierarchy, List<List<String>> fullHierarchy) {
         // currHierarchy.add(String.format("%x", currSw.getId()));
         currHierarchy.add(dpidToHostNameHack((Long) node.getID()));
-        ArrayList<String> currHierarchyClone = (ArrayList<String>) currHierarchy
-                .clone(); // Shallow copy as required
+        ArrayList<String> currHierarchyClone = (ArrayList<String>) currHierarchy.clone(); // Shallow
+                                                                                          // copy
+                                                                                          // as
+                                                                                          // required
 
         Map<Node, Set<Edge>> ndlinks = topologyManager.getNodeEdges();
         if (ndlinks == null) {
-            logger.debug(
-                    "updateCurrentHierarchy(): topologyManager returned null ndlinks for node: {}",
-                    node);
+            logger.debug("updateCurrentHierarchy(): topologyManager returned null ndlinks for node: {}", node);
             return;
         }
         Node n = NodeCreator.createOFNode((Long) node.getID());
@@ -816,23 +798,22 @@ public class HostTracker implements IfIptoHost, IfHostListener,
             return;
         }
         for (Edge lt : links) {
-            if (!lt.getHeadNodeConnector().getType()
-                    .equals(NodeConnector.NodeConnectorIDType.OPENFLOW)) {
+            if (!lt.getHeadNodeConnector().getType().equals(NodeConnector.NodeConnectorIDType.OPENFLOW)) {
                 // We don't want to work on Node that are not openflow
                 // for now
                 continue;
             }
             Node dstNode = lt.getHeadNodeConnector().getNode();
 
-            Tier nodeTier = (Tier) switchManager.getNodeProp(node,
-                    Tier.TierPropName);
-            Tier dstNodeTier = (Tier) switchManager.getNodeProp(dstNode,
-                    Tier.TierPropName);
+            Tier nodeTier = (Tier) switchManager.getNodeProp(node, Tier.TierPropName);
+            Tier dstNodeTier = (Tier) switchManager.getNodeProp(dstNode, Tier.TierPropName);
             if (dstNodeTier.getValue() > nodeTier.getValue()) {
                 ArrayList<String> buildHierarchy = currHierarchy;
                 if (currHierarchy.size() > currHierarchyClone.size()) {
-                    buildHierarchy = (ArrayList<String>) currHierarchyClone
-                            .clone(); // Shallow copy as required
+                    buildHierarchy = (ArrayList<String>) currHierarchyClone.clone(); // Shallow
+                                                                                     // copy
+                                                                                     // as
+                                                                                     // required
                     fullHierarchy.add(buildHierarchy);
                 }
                 updateCurrentHierarchy(dstNode, buildHierarchy, fullHierarchy);
@@ -862,8 +843,7 @@ public class HostTracker implements IfIptoHost, IfHostListener,
             }
 
             if (!srcType.equals(NodeConnector.NodeConnectorIDType.OPENFLOW)) {
-                logger.error("For now we cannot handle updates for "
-                        + "non-openflow nodes");
+                logger.error("For now we cannot handle updates for " + "non-openflow nodes");
                 return;
             }
 
@@ -873,8 +853,7 @@ public class HostTracker implements IfIptoHost, IfHostListener,
             }
 
             if (!dstType.equals(NodeConnector.NodeConnectorIDType.OPENFLOW)) {
-                logger.error("For now we cannot handle updates for "
-                        + "non-openflow nodes");
+                logger.error("For now we cannot handle updates for " + "non-openflow nodes");
                 return;
             }
 
@@ -896,19 +875,8 @@ public class HostTracker implements IfIptoHost, IfHostListener,
             }
         }
 
-        logger.debug(
-                "HostTracker Topology linkUpdate handling src:{}[port {}] dst:{}[port {}] added: {}",
+        logger.debug("HostTracker Topology linkUpdate handling src:{}[port {}] dst:{}[port {}] added: {}",
                 new Object[] { srcNid, srcPort, dstNid, dstPort, added });
-        clearTiers();
-        for (Entry<InetAddress, HostNodeConnector> entry : hostsDB.entrySet()) {
-            HostNodeConnector host = entry.getValue();
-            Node node = host.getnodeconnectorNode();
-            if (node != null) {
-                Tier t = new Tier(1);
-                switchManager.setNodeProp(node, t);
-                updateSwitchTiers(node, 1);
-            }
-        }
     }
 
     @Override
@@ -921,71 +889,73 @@ public class HostTracker implements IfIptoHost, IfHostListener,
         }
     }
 
+    @Override
     public void subnetNotify(Subnet sub, boolean add) {
         logger.debug("Received subnet notification: {}  add={}", sub, add);
         if (add) {
             for (int i = 0; i < failedARPReqList.size(); i++) {
                 ARPPending arphost;
                 arphost = failedARPReqList.get(i);
-                logger.debug(
-                        "Sending the ARP from FailedARPReqList fors IP: {}",
-                        arphost.getHostIP().getHostAddress());
-                hostFinder.find(arphost.getHostIP());
+                if (hostFinder == null) {
+                    logger.warn("ARPHandler Services are not available on subnet addition");
+                    continue;
+                }
+               logger.debug("Sending the ARP from FailedARPReqList fors IP: {}", arphost.getHostIP().getHostAddress());
+               hostFinder.find(arphost.getHostIP());
             }
         }
     }
 
     class OutStandingARPHandler extends TimerTask {
+        @Override
         public void run() {
             ARPPending arphost;
             /* This routine runs every 4 seconds */
-            // logger.info ("ARP Handler called");
             for (int i = 0; i < ARPPendingList.size(); i++) {
                 arphost = ARPPendingList.get(i);
                 if (arphost.getSent_count() < switchManager.getHostRetryCount()) {
                     /*
                      * No reply has been received of first ARP Req, send the
-                     * next one
+                     * next one. Before sending the ARP, check if ARPHandler
+                     * is available or not
                      */
+                    if (hostFinder == null) {
+                        logger.warn("ARPHandler Services are not available for Outstanding ARPs");
+                        continue;
+                    }
                     hostFinder.find(arphost.getHostIP());
                     arphost.sent_count++;
-                    logger.debug("ARP Sent from ARPPending List, IP: {}",
-                            arphost.getHostIP().getHostAddress());
-                } else if (arphost.getSent_count() >= switchManager
-                        .getHostRetryCount()) {
+                    logger.debug("ARP Sent from ARPPending List, IP: {}", arphost.getHostIP().getHostAddress());
+                } else if (arphost.getSent_count() >= switchManager.getHostRetryCount()) {
                     /*
-                     * Two ARP requests have been sent without receiving a
+                     * ARP requests have been sent without receiving a
                      * reply, remove this from the pending list
                      */
                     removePendingARPFromList(i);
-                    logger.debug(
-                            "ARP reply not received after two attempts, removing from Pending List IP: {}",
+                    logger.debug("ARP reply not received after multiple attempts, removing from Pending List IP: {}",
                             arphost.getHostIP().getHostAddress());
                     /*
                      * Add this host to a different list which will be processed
                      * on link up events
                      */
-                    logger.debug("Adding the host to FailedARPReqList IP: {}",
-                            arphost.getHostIP().getHostAddress());
+                    logger.debug("Adding the host to FailedARPReqList IP: {}", arphost.getHostIP().getHostAddress());
                     failedARPReqList.add(arphost);
 
                 } else {
-                    logger.error(
-                            "Inavlid arp_sent count for entery at index: {}", i);
+                    logger.error("Inavlid arp_sent count for entry at index: {}", i);
                 }
             }
         }
     }
 
     private class ARPRefreshHandler extends TimerTask {
+        @Override
         @SuppressWarnings("deprecation")
         public void run() {
-            if ((clusterContainerService != null)
-                    && !clusterContainerService.amICoordinator()) {
+            if ((clusterContainerService != null) && !clusterContainerService.amICoordinator()) {
                 return;
             }
-            if ((switchManager != null)
-                    && !switchManager.isHostRefreshEnabled()) {
+            if ((switchManager != null) && !switchManager.isHostRefreshEnabled()) {
                 /*
                  * The host probe procedure was disabled by CLI
                  */
@@ -996,8 +966,7 @@ public class HostTracker implements IfIptoHost, IfHostListener,
                 logger.error("ARPRefreshHandler(): hostsDB is not allocated yet:");
                 return;
             }
-            for (Entry<InetAddress, HostNodeConnector> entry : hostsDB
-                    .entrySet()) {
+            for (Entry<InetAddress, HostNodeConnector> entry : hostsDB.entrySet()) {
                 HostNodeConnector host = entry.getValue();
                 if (host.isStaticHost()) {
                     /* this host was learned via API3, don't age it out */
@@ -1020,14 +989,22 @@ public class HostTracker implements IfIptoHost, IfHostListener,
                      * Use the services of arphandler to check if host is still
                      * there
                      */
-                    logger.trace(
-                            "ARP Probing ({}) for {}({})",
-                            new Object[] {
-                                    arp_cntdown,
-                                    host.getNetworkAddress().getHostAddress(),
-                                    HexEncode.bytesToHexString(host
-                                            .getDataLayerAddressBytes()) });
+                    if (logger.isTraceEnabled()) {
+                        logger.trace(
+                                "ARP Probing ({}) for {}({})",
+                                new Object[] { arp_cntdown, host.getNetworkAddress().getHostAddress(),
+                                        HexEncode.bytesToHexString(host.getDataLayerAddressBytes()) });
+                    }
                     host.setArpSendCountDown(arp_cntdown);
+                    if (hostFinder == null) {
+                        /*
+                         * If hostfinder is not available, then can't send the
+                         * probe. However, continue the age out the hosts since
+                         * we don't know if the host is indeed out there or not.
+                         */
+                        logger.warn("ARPHandler is not avaialable, can't send the probe");
+                        continue;
+                    }
                     hostFinder.probe(host);
                 }
             }
@@ -1037,7 +1014,7 @@ public class HostTracker implements IfIptoHost, IfHostListener,
     /**
      * Inform the controller IP to MAC binding of a host and its connectivity to
      * an openflow switch in terms of Node, port, and VLAN.
-     * 
+     *
      * @param networkAddr
      *            IP address of the host
      * @param dataLayer
@@ -1048,21 +1025,22 @@ public class HostTracker implements IfIptoHost, IfHostListener,
      *            Port of the switch to which host is connected
      * @param vlan
      *            Vlan of which this host is member of
-     * 
+     *
      * @return Status The status object as described in {@code Status}
      *         indicating the result of this action.
      */
 
-    public Status addStaticHostReq(InetAddress networkAddr,
-            byte[] dataLayerAddress, NodeConnector nc, short vlan) {
-        if (dataLayerAddress.length != 6) {
+    public Status addStaticHostReq(InetAddress networkAddr, byte[] dataLayerAddress, NodeConnector nc, short vlan) {
+        if (dataLayerAddress.length != NetUtils.MACAddrLengthInBytes) {
             return new Status(StatusCode.BADREQUEST, "Invalid MAC address");
         }
 
+        if (nc == null) {
+            return new Status(StatusCode.BADREQUEST, "Invalid NodeConnector");
+        }
         HostNodeConnector host = null;
         try {
-            host = new HostNodeConnector(dataLayerAddress, networkAddr, nc,
-                    vlan);
+            host = new HostNodeConnector(dataLayerAddress, networkAddr, nc, vlan);
             if (hostExists(host)) {
                 // This host is already learned either via ARP or through a
                 // northbound request
@@ -1070,7 +1048,21 @@ public class HostTracker implements IfIptoHost, IfHostListener,
                 transHost.setStaticHost(true);
                 return new Status(StatusCode.SUCCESS, null);
             }
+
+            if (hostsDB.get(networkAddr) != null) {
+                // There is already a host with this IP address (but behind
+                // a different (switch, port, vlan) tuple. Return an error
+                return new Status(StatusCode.CONFLICT, "Existing IP, Use PUT to update");
+            }
             host.setStaticHost(true);
+            /*
+             * Check if the nc is an ISL port
+             */
+            if (topologyManager != null) {
+                if (topologyManager.isInternal(nc)) {
+                    return new Status(StatusCode.BADREQUEST, "Cannot add host on ISL port");
+                }
+            }
             /*
              * Before adding host, Check if the switch and the port have already
              * come up
@@ -1080,14 +1072,13 @@ public class HostTracker implements IfIptoHost, IfHostListener,
                 notifyHostLearnedOrRemoved(host, true);
             } else {
                 inactiveStaticHosts.put(nc, host);
-                logger.debug(
-                        "Switch or switchport is not up, adding host {} to inactive list",
+                logger.debug("Switch or switchport is not up, adding host {} to inactive list",
                         networkAddr.getHostName());
             }
             return new Status(StatusCode.SUCCESS, null);
         } catch (ConstructionException e) {
-            return new Status(StatusCode.INTERNALERROR,
-                    "Host could not be created");
+            logger.error("", e);
+            return new Status(StatusCode.INTERNALERROR, "Host could not be created");
         }
 
     }
@@ -1095,7 +1086,7 @@ public class HostTracker implements IfIptoHost, IfHostListener,
     /**
      * Update the controller IP to MAC binding of a host and its connectivity to
      * an openflow switch in terms of switch id, switch port, and VLAN.
-     * 
+     *
      * @param networkAddr
      *            IP address of the host
      * @param dataLayer
@@ -1106,39 +1097,71 @@ public class HostTracker implements IfIptoHost, IfHostListener,
      *            Port of the switch to which host is connected
      * @param vlan
      *            Vlan of which this host is member of
-     * 
-     * @return boolean true if the host was added successfully, false otherwise
+     *
+     * @return Status The status object as described in {@code Status}
+     *         indicating the result of this action.
      */
-    public boolean updateHostReq(InetAddress networkAddr,
-            byte[] dataLayerAddress, NodeConnector nc, short vlan) {
+    public Status updateHostReq(InetAddress networkAddr, byte[] dataLayerAddress, NodeConnector nc, short vlan) {
+        HostNodeConnector tobeUpdatedHost;
+        HostNodeConnector host = null;
+
+        if (dataLayerAddress.length != NetUtils.MACAddrLengthInBytes) {
+            return new Status(StatusCode.BADREQUEST, "Invalid MAC address");
+        }
+
         if (nc == null) {
-            return false;
+            return new Status(StatusCode.BADREQUEST, "Invalid NodeConnector");
         }
-        HostNodeConnector host = null;
+
         try {
-            host = new HostNodeConnector(dataLayerAddress, networkAddr, nc,
-                    vlan);
-            if (!hostExists(host)) {
-                if ((inactiveStaticHosts.get(nc)) != null) {
-                    inactiveStaticHosts.replace(nc, host);
-                    return true;
+            host = new HostNodeConnector(dataLayerAddress, networkAddr, nc, vlan);
+            if (hostExists(host)) {
+                return new Status(StatusCode.BADREQUEST, "Host already exists");
+            }
+
+            if ((tobeUpdatedHost = hostsDB.get(networkAddr)) != null) {
+                if (hostsDB.replace(networkAddr, tobeUpdatedHost, host)) {
+                    logger.debug("Host replaced from hostsDB. Old host: {} New Host: {}", tobeUpdatedHost, host);
+                    notifyHostLearnedOrRemoved(tobeUpdatedHost, false);
+                    notifyHostLearnedOrRemoved(host, true);
+                    return new Status(StatusCode.SUCCESS);
+                } else {
+                    logger.error("Static host replacement failed from hostsDB, Replaced Host: {}, New Host: {}",
+                            tobeUpdatedHost, host);
+                    return new Status(StatusCode.INTERNALERROR,
+                            "Host Replacement Failed due to presence of another host with same IP");
                 }
-                return false;
             }
-            hostsDB.replace(networkAddr, host);
-            return true;
+
+            // Check if the host exists in inactive hosts database
+            if ((tobeUpdatedHost = inactiveStaticHosts.get(nc)) != null) {
+                if (inactiveStaticHosts.replace(nc, tobeUpdatedHost, host)) {
+                    logger.debug("Host replaced from inactive hostsDB. Old host: {} New Host: {}", tobeUpdatedHost,
+                            host);
+                    return new Status(StatusCode.SUCCESS);
+                } else {
+                    logger.error("Static host replacement failed, Replaced Host: {}, New Host: {}", tobeUpdatedHost,
+                            host);
+                    return new Status(StatusCode.INTERNALERROR,
+                            "Host Replacement Failed due to presence of another host with same IP");
+                }
+            }
+
+            // Host doesn't exist
+            return new Status(StatusCode.BADREQUEST, "Host doesn't exists, can't update");
         } catch (ConstructionException e) {
+            logger.error("", e);
+            return new Status(StatusCode.INTERNALERROR, "host object creation failure");
         }
-        return false;
     }
 
     /**
      * Remove from the controller IP to MAC binding of a host and its
      * connectivity to an openflow switch
-     * 
+     *
      * @param networkAddr
      *            IP address of the host
-     * 
+     *
      * @return boolean true if the host was removed successfully, false
      *         otherwise
      */
@@ -1149,8 +1172,7 @@ public class HostTracker implements IfIptoHost, IfHostListener,
         if (host != null) {
             // Validation check
             if (!host.isStaticHost()) {
-                return new Status(StatusCode.FORBIDDEN, "Host "
-                        + networkAddress.getHostName() + " is not static");
+                return new Status(StatusCode.FORBIDDEN, "Host " + networkAddress.getHostName() + " is not static");
             }
             // Remove and notify
             notifyHostLearnedOrRemoved(host, false);
@@ -1164,8 +1186,7 @@ public class HostTracker implements IfIptoHost, IfHostListener,
             host = entry.getValue();
             // Validation check
             if (!host.isStaticHost()) {
-                return new Status(StatusCode.FORBIDDEN, "Host "
-                        + networkAddress.getHostName() + " is not static");
+                return new Status(StatusCode.FORBIDDEN, "Host " + networkAddress.getHostName() + " is not static");
             }
             this.removeHostFromInactiveDB(networkAddress);
             return new Status(StatusCode.SUCCESS, null);
@@ -1181,22 +1202,17 @@ public class HostTracker implements IfIptoHost, IfHostListener,
     }
 
     @Override
-    public void notifyNode(Node node, UpdateType type,
-            Map<String, Property> propMap) {
+    public void notifyNode(Node node, UpdateType type, Map<String, Property> propMap) {
         if (node == null)
             return;
 
         switch (type) {
         case REMOVED:
-            long sid = (Long) node.getID();
-            logger.debug("Received removedSwitch for sw id {}",
-                    HexEncode.longToHexString(sid));
-            for (Entry<InetAddress, HostNodeConnector> entry : hostsDB
-                    .entrySet()) {
+            logger.debug("Received removed node {}", node);
+            for (Entry<InetAddress, HostNodeConnector> entry : hostsDB.entrySet()) {
                 HostNodeConnector host = entry.getValue();
-                if (host.getnodeconnectornodeId() == sid) {
-                    logger.debug("Switch: {} is down, remove from Hosts_DB",
-                            sid);
+                if (host.getnodeconnectorNode().equals(node)) {
+                    logger.debug("Node: {} is down, remove from Hosts_DB", node);
                     removeKnownHost(entry.getKey());
                     notifyHostLearnedOrRemoved(host, false);
                 }
@@ -1208,8 +1224,7 @@ public class HostTracker implements IfIptoHost, IfHostListener,
     }
 
     @Override
-    public void notifyNodeConnector(NodeConnector nodeConnector,
-            UpdateType type, Map<String, Property> propMap) {
+    public void notifyNodeConnector(NodeConnector nodeConnector, UpdateType type, Map<String, Property> propMap) {
         if (nodeConnector == null)
             return;
 
@@ -1238,16 +1253,13 @@ public class HostTracker implements IfIptoHost, IfHostListener,
     }
 
     @Override
-    public Status addStaticHost(String networkAddress, String dataLayerAddress,
-            NodeConnector nc, String vlan) {
+    public Status addStaticHost(String networkAddress, String dataLayerAddress, NodeConnector nc, String vlan) {
         try {
             InetAddress ip = InetAddress.getByName(networkAddress);
             if (nc == null) {
                 return new Status(StatusCode.BADREQUEST, "Invalid NodeId");
             }
-            return addStaticHostReq(ip,
-                    HexEncode.bytesFromHexString(dataLayerAddress), nc,
-                    Short.valueOf(vlan));
+            return addStaticHostReq(ip, HexEncode.bytesFromHexString(dataLayerAddress), nc, Short.valueOf(vlan));
         } catch (UnknownHostException e) {
             logger.error("", e);
             return new Status(StatusCode.BADREQUEST, "Invalid Address");
@@ -1273,8 +1285,13 @@ public class HostTracker implements IfIptoHost, IfHostListener,
 
         for (int i = 0; i < failedARPReqList.size(); i++) {
             arphost = failedARPReqList.get(i);
-            logger.debug("Sending the ARP from FailedARPReqList fors IP: {}",
-                    arphost.getHostIP().getHostAddress());
+            logger.debug("Sending the ARP from FailedARPReqList fors IP: {}", arphost.getHostIP().getHostAddress());
+            if (hostFinder == null) {
+                logger.warn("ARPHandler is not available at interface  up");
+                logger.warn("Since this event is missed, host(s) connected to interface {} may not be discovered",
+                        nodeConnector);
+                continue;
+            }
             hostFinder.find(arphost.getHostIP());
         }
         HostNodeConnector host = inactiveStaticHosts.get(nodeConnector);
@@ -1286,18 +1303,12 @@ public class HostTracker implements IfIptoHost, IfHostListener,
     }
 
     private void handleNodeConnectorStatusDown(NodeConnector nodeConnector) {
-        long sid = (Long) nodeConnector.getNode().getID();
-        short port = (Short) nodeConnector.getID();
-
         logger.debug("handleNodeConnectorStatusDown {}", nodeConnector);
 
         for (Entry<InetAddress, HostNodeConnector> entry : hostsDB.entrySet()) {
             HostNodeConnector host = entry.getValue();
-            if ((host.getnodeconnectornodeId() == sid)
-                    && (host.getnodeconnectorportId() == port)) {
-                logger.debug(
-                        "Switch: {}, Port: {} is down, remove from Hosts_DB",
-                        sid, port);
+            if (host.getnodeConnector().equals(nodeConnector)) {
+                logger.debug(" NodeConnector: {} is down, remove from Hosts_DB", nodeConnector);
                 removeKnownHost(entry.getKey());
                 notifyHostLearnedOrRemoved(host, false);
             }
@@ -1337,7 +1348,7 @@ public class HostTracker implements IfIptoHost, IfHostListener,
     /**
      * Function called by the dependency manager when all the required
      * dependencies are satisfied
-     * 
+     *
      */
     void init(Component c) {
         Dictionary<?, ?> props = c.getServiceProperties();
@@ -1355,16 +1366,15 @@ public class HostTracker implements IfIptoHost, IfHostListener,
      * Function called by the dependency manager when at least one dependency
      * become unsatisfied or when the component is shutting down because for
      * example bundle is being stopped.
-     * 
+     *
      */
     void destroy() {
-        destroyCache();
     }
 
     /**
      * Function called by dependency manager after "init ()" is called and after
      * the services provided by the class are registered in the service registry
-     * 
+     *
      */
     void start() {
     }
@@ -1373,9 +1383,15 @@ public class HostTracker implements IfIptoHost, IfHostListener,
      * Function called by the dependency manager before the services exported by
      * the component are unregistered, this will be followed by a "destroy ()"
      * calls
-     * 
+     *
      */
-    void stop() {
+    void stop(){
+    }
+
+    void stopping() {
+        arpRefreshTimer.cancel();
+        timer.cancel();
+        executor.shutdown();
     }
 
     @Override