CORD-1304 Set of changes for fabric routing to optimize use of ECMP groups

Also removing some old demo code in the SR app
Adding a couple of CLI commands for debugging
Bug fix in the DistributedGroupStore for group_exists error message
Bug fixes for ofdpa driver:
    - synchronized update of flowObjectiveStore when buckets are added to or removed from groups
      to avoid one thread from overwriting an update from another thread doing an update at the same time
    - addBucketToL2FloodGroup now updates flowObjectiveStore after accounting for changes
    - addBucketToHashGroup accounts for all added buckets, not just the first one

Change-Id: I6207c1c3c1b4379986805d73a73bc460fea8fe3f
diff --git a/src/main/java/org/onosproject/segmentrouting/DefaultRoutingHandler.java b/src/main/java/org/onosproject/segmentrouting/DefaultRoutingHandler.java
index 678d8a0..df29c74 100644
--- a/src/main/java/org/onosproject/segmentrouting/DefaultRoutingHandler.java
+++ b/src/main/java/org/onosproject/segmentrouting/DefaultRoutingHandler.java
@@ -16,6 +16,8 @@
 package org.onosproject.segmentrouting;
 
 import com.google.common.base.MoreObjects;
+import com.google.common.collect.ImmutableMap;
+import com.google.common.collect.ImmutableMap.Builder;
 import com.google.common.collect.ImmutableSet;
 import com.google.common.collect.Lists;
 import com.google.common.collect.Maps;
@@ -29,6 +31,7 @@
 import org.onosproject.net.Link;
 import org.onosproject.segmentrouting.config.DeviceConfigNotFoundException;
 import org.onosproject.segmentrouting.config.DeviceConfiguration;
+import org.onosproject.segmentrouting.grouphandler.DefaultGroupHandler;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 
@@ -100,12 +103,26 @@
     }
 
     /**
+     * Returns an immutable copy of the current ECMP shortest-path graph as
+     * computed by this controller instance.
+     *
+     * @return the current ECMP graph
+     */
+    public ImmutableMap<DeviceId, EcmpShortestPathGraph> getCurrentEmcpSpgMap() {
+        Builder<DeviceId, EcmpShortestPathGraph> builder = ImmutableMap.builder();
+        currentEcmpSpgMap.entrySet().forEach(entry -> {
+            if (entry.getValue() != null) {
+                builder.put(entry.getKey(), entry.getValue());
+            }
+        });
+        return builder.build();
+    }
+
+    /**
      * Populates all routing rules to all connected routers, including default
      * routing rules, adjacency rules, and policy rules if any.
-     *
-     * @return true if it succeeds in populating all rules, otherwise false
      */
-    public boolean populateAllRoutingRules() {
+    public void populateAllRoutingRules() {
 
         statusLock.lock();
         try {
@@ -116,8 +133,8 @@
 
             for (Device sw : srManager.deviceService.getDevices()) {
                 if (!srManager.mastershipService.isLocalMaster(sw.id())) {
-                    log.debug("populateAllRoutingRules: skipping device {}...we are not master",
-                              sw.id());
+                    log.debug("populateAllRoutingRules: skipping device {}..."
+                            + "we are not master", sw.id());
                     continue;
                 }
 
@@ -126,9 +143,10 @@
                     log.debug("populateAllRoutingRules: populationStatus is ABORTED");
                     populationStatus = Status.ABORTED;
                     log.debug("Abort routing rule population");
-                    return false;
+                    return;
                 }
                 currentEcmpSpgMap.put(sw.id(), ecmpSpg);
+                log.debug("Updating ECMPspg for sw:{}", sw.id());
 
                 // TODO: Set adjacency routing rule for all switches
             }
@@ -137,31 +155,42 @@
             populationStatus = Status.SUCCEEDED;
             log.info("Completed routing rule population. Total # of rules pushed : {}",
                     rulePopulator.getCounter());
-            return true;
+            return;
         } finally {
             statusLock.unlock();
         }
     }
 
     /**
-     * Populates the routing rules according to the route changes due to the link
-     * failure or link add. It computes the routes changed due to the link changes and
-     * repopulates the rules only for these routes. Note that when a switch goes
-     * away, all of its links fail as well, but this is handled as a single
-     * switch removal event.
+     * Populates the routing rules or makes hash group changes according to the
+     * route-path changes due to link failure, switch failure or link up. This
+     * method should only be called for one of these three possible event-types.
+     * Note that when a switch goes away, all of its links fail as well,
+     * but this is handled as a single switch removal event.
      *
-     * @param failedLink the single failed link, or null for other conditions
-     *                  such as an added link or a removed switch
-     * @return true if it succeeds to populate all rules, false otherwise
+     * @param linkDown the single failed link, or null for other conditions
+     *                  such as link-up or a removed switch
+     * @param linkUp the single link up, or null for other conditions such as
+     *                  link-down or a removed switch
+     * @param switchDown the removed switch, or null for other conditions such as
+     *                  link-down or link-up
      */
-    public boolean populateRoutingRulesForLinkStatusChange(Link failedLink) {
+    public void populateRoutingRulesForLinkStatusChange(Link linkDown,
+                                                           Link linkUp,
+                                                           DeviceId switchDown) {
+        if ((linkDown != null && (linkUp != null || switchDown != null)) ||
+                (linkUp != null && (linkDown != null || switchDown != null)) ||
+                (switchDown != null && (linkUp != null || linkDown != null))) {
+            log.warn("Only one event can be handled for link status change .. aborting");
+            return;
+        }
 
         statusLock.lock();
         try {
 
             if (populationStatus == Status.STARTED) {
                 log.warn("Previous rule population is not finished.");
-                return true;
+                return;
             }
 
             // Take the snapshots of the links
@@ -178,60 +207,269 @@
             log.info("Starts rule population from link change");
 
             Set<ArrayList<DeviceId>> routeChanges;
-            log.trace("populateRoutingRulesForLinkStatusChange: "
+            log.debug("populateRoutingRulesForLinkStatusChange: "
                     + "populationStatus is STARTED");
             populationStatus = Status.STARTED;
             // try optimized re-routing
-            if (failedLink == null) {
-                // Compare all routes of existing ECMP SPG to new ECMP SPG
+            if (linkDown == null) {
+                // either a linkUp or a switchDown - compute all route changes by
+                // comparing all routes of existing ECMP SPG to new ECMP SPG
                 routeChanges = computeRouteChange();
+
+                if (routeChanges != null) {
+                    // deal with linkUp of a seen-before link
+                    if (linkUp != null && srManager.isSeenLink(linkUp)) {
+                        if (!isBidirectional(linkUp)) {
+                            log.warn("Not a bidirectional link yet .. not "
+                                    + "processing link {}", linkUp);
+                            srManager.updateSeenLink(linkUp, true);
+                            populationStatus = Status.ABORTED;
+                            return;
+                        }
+                        // link previously seen before
+                        // do hash-bucket changes instead of a re-route
+                        processHashGroupChange(routeChanges, false, null);
+                        // clear out routesChanges so a re-route is not attempted
+                        routeChanges = ImmutableSet.of();
+                    }
+
+                    //deal with switchDown
+                    if (switchDown != null) {
+                        processHashGroupChange(routeChanges, true, switchDown);
+                        // clear out routesChanges so a re-route is not attempted
+                        routeChanges = ImmutableSet.of();
+                    }
+
+                    // for a linkUp of a never-seen-before link
+                    // let it fall through to a reroute of the routeChanges
+
+                }
+
+                // now that we are past the check for a previously seen link
+                // it is safe to update the store for the linkUp
+                if (linkUp != null) {
+                    srManager.updateSeenLink(linkUp, true);
+                }
+
             } else {
-                // Compare existing ECMP SPG only with the link removed
-                routeChanges = computeDamagedRoutes(failedLink);
+                // link has gone down
+                // Compare existing ECMP SPG only with the link that went down
+                routeChanges = computeDamagedRoutes(linkDown);
+                if (routeChanges != null) {
+                    processHashGroupChange(routeChanges, true, null);
+                    // clear out routesChanges so a re-route is not attempted
+                    routeChanges = ImmutableSet.of();
+                }
             }
 
             // do full re-routing if optimized routing returns null routeChanges
             if (routeChanges == null) {
-                return populateAllRoutingRules();
+                log.info("Optimized routing failed... doing full re-route");
+                populateAllRoutingRules();
+                return;
             }
 
             if (routeChanges.isEmpty()) {
-                log.info("No route changes for the link status change");
+                log.info("No re-route attempted for the link status change");
                 log.debug("populateRoutingRulesForLinkStatusChange: populationStatus is SUCCEEDED");
                 populationStatus = Status.SUCCEEDED;
-                return true;
+                return;
             }
 
+            // reroute of routeChanges
             if (repopulateRoutingRulesForRoutes(routeChanges)) {
                 log.debug("populateRoutingRulesForLinkStatusChange: populationStatus is SUCCEEDED");
                 populationStatus = Status.SUCCEEDED;
-                log.info("Complete to repopulate the rules. # of rules populated : {}",
+                log.info("Completed repopulation of rules. # of rules populated : {}",
                         rulePopulator.getCounter());
-                return true;
+                return;
             } else {
                 log.debug("populateRoutingRulesForLinkStatusChange: populationStatus is ABORTED");
                 populationStatus = Status.ABORTED;
                 log.warn("Failed to repopulate the rules.");
-                return false;
+                return;
             }
         } finally {
             statusLock.unlock();
         }
     }
 
-    private boolean repopulateRoutingRulesForRoutes(Set<ArrayList<DeviceId>> routes) {
+    /**
+     * Returns true if the link being queried is a bidirectional link. A bidi
+     * link is defined as a link, whose reverse link - ie. the link in the reverse
+     * direction - has been seen-before and is up.
+     *
+     * @param link the infrastructure link being queried
+     * @return true if another unidirectional link exists in the reverse direction,
+     *              has been seen-before and is up
+     */
+    private boolean isBidirectional(Link link) {
+        Link reverseLink = srManager.linkService.getLink(link.dst(), link.src());
+        if (reverseLink == null) {
+            return false;
+        }
+        Boolean result = srManager.isSeenLinkUp(reverseLink);
+        if (result == null) {
+            return false;
+        }
+        return result.booleanValue();
+    }
+
+    /**
+     * Processes a set a route-path changes by editing hash groups.
+     *
+     * @param routeChanges a set of route-path changes, where each route-path is
+     *                     a list with its first element the src-switch of the path
+     *                     and the second element the dst-switch of the path.
+     * @param linkOrSwitchFailed true if the route changes are for a failed
+     *                           switch or linkDown event
+     * @param failedSwitch the switchId if the route changes are for a failed switch,
+     *                     otherwise null
+     */
+    private void processHashGroupChange(Set<ArrayList<DeviceId>> routeChanges,
+                                        boolean linkOrSwitchFailed,
+                                        DeviceId failedSwitch) {
+        for (ArrayList<DeviceId> route : routeChanges) {
+            DeviceId targetSw = route.get(0);
+            boolean success;
+            DeviceId dstSw = null;
+            if (route.size() > 1) {
+                dstSw = route.get(1);
+            }
+
+            if (linkOrSwitchFailed) {
+                success = fixHashGroupsForRoute(route, true);
+                // it's possible that we cannot fix hash groups for a route
+                // if the target switch has failed. Nevertheless the ecmp graph
+                // for the impacted switch must still be updated.
+                if (failedSwitch != null && targetSw.equals(failedSwitch)
+                        && dstSw != null) {
+                    currentEcmpSpgMap.put(dstSw, updatedEcmpSpgMap.get(dstSw));
+                    currentEcmpSpgMap.remove(targetSw);
+                    log.debug("Updating ECMPspg for dst:{} removing failed "
+                            + "target:{}", dstSw, targetSw);
+                    return;
+                }
+                //linkfailed - update both sides
+                currentEcmpSpgMap.put(targetSw, updatedEcmpSpgMap.get(targetSw));
+                dstSw = route.get(1);
+                currentEcmpSpgMap.put(dstSw, updatedEcmpSpgMap.get(dstSw));
+                log.debug("Updating ECMPspg for dst:{} and target:{}", dstSw, targetSw);
+            } else {
+                success = fixHashGroupsForRoute(route, false);
+                if (success) {
+                    currentEcmpSpgMap.put(targetSw, updatedEcmpSpgMap.get(targetSw));
+                    if (dstSw != null) {
+                        currentEcmpSpgMap.put(dstSw, updatedEcmpSpgMap.get(dstSw));
+                    }
+                    log.debug("Updating ECMPspg for target:{} and dst:{}",
+                              targetSw, dstSw);
+                }
+            }
+        }
+    }
+
+    /**
+     * Edits hash groups in the src-switch (targetSw) of a route-path by
+     * calling the groupHandler to either add or remove buckets in an existing
+     * hash group.
+     *
+     * @param route a single list representing a route-path where the first element
+     *                  is the src-switch (targetSw) of the route-path and the
+     *                  second element is the dst-switch
+     * @param revoke true if buckets in the hash-groups need to be removed;
+     *              false if buckets in the hash-groups need to be added
+     * @return true if the hash group editing is successful
+     */
+    private boolean fixHashGroupsForRoute(ArrayList<DeviceId> route,
+                                          boolean revoke) {
+        DeviceId targetSw = route.get(0);
+        if (route.size() < 2) {
+            log.warn("Cannot fixHashGroupsForRoute - no dstSw in route {}", route);
+            return false;
+        }
+        DeviceId destSw = route.get(1);
+        log.debug("Processing fixHashGroupsForRoute: Target {} -> Dest {}",
+                  targetSw, destSw);
+        boolean targetIsEdge = false;
+        try {
+            targetIsEdge = srManager.deviceConfiguration.isEdgeDevice(targetSw);
+        } catch (DeviceConfigNotFoundException e) {
+            log.warn(e.getMessage() + "Cannot determine if targetIsEdge {}.. "
+                    + "continuing fixHash", targetSw);
+        }
+
+        // figure out the new next hops at the targetSw towards the destSw
+        Set<DeviceId> nextHops = new HashSet<>();
+        EcmpShortestPathGraph ecmpSpg = updatedEcmpSpgMap.get(destSw);
+        HashMap<Integer, HashMap<DeviceId, ArrayList<ArrayList<DeviceId>>>> switchVia =
+                ecmpSpg.getAllLearnedSwitchesAndVia();
+        for (Integer itrIdx : switchVia.keySet()) {
+            HashMap<DeviceId, ArrayList<ArrayList<DeviceId>>> swViaMap =
+                    switchVia.get(itrIdx);
+            for (DeviceId target : swViaMap.keySet()) {
+                if (target.equals(targetSw)) {
+                    // found the iteration where targetSw is reached- get nextHops
+                    if (!targetIsEdge && itrIdx > 1) {
+                        // optimization for spines to not use other leaves to get
+                        // to a leaf to avoid loops
+                        log.debug("Avoiding {} hop path for non-edge targetSw:{}"
+                                + " --> dstSw:{}", itrIdx, targetSw, destSw);
+                        break;
+                    }
+                    for (ArrayList<DeviceId> via : swViaMap.get(target)) {
+                        if (via.isEmpty()) {
+                            nextHops.add(destSw);
+                        } else {
+                            // first elem is next-hop in each ECMP path
+                            nextHops.add(via.get(0));
+                        }
+                    }
+                    break;
+                }
+            }
+        }
+
+        // call group handler to change hash group at targetSw
+        DefaultGroupHandler grpHandler = srManager.getGroupHandler(targetSw);
+        if (grpHandler == null) {
+            log.warn("Cannot find grouphandler for dev:{} .. aborting"
+                    + " {} hash group buckets for route:{} ", targetSw,
+                    (revoke) ? "revoke" : "repopulate", route);
+            return false;
+        }
+        log.debug("{} hash-groups buckets For Route {} -> {} to next-hops {}",
+                  (revoke) ? "revoke" : "repopulating",
+                  targetSw, destSw, nextHops);
+        return (revoke) ? grpHandler.fixHashGroups(targetSw, nextHops,
+                                                       destSw, true)
+                            : grpHandler.fixHashGroups(targetSw, nextHops,
+                                                       destSw, false);
+    }
+
+    /**
+     * Processes a set a route-path changes by reprogramming routing rules and
+     * creating new hash-groups if necessary.
+     *
+     * @param routeChanges a set of route-path changes, where each route-path is
+     *                     a list with its first element the src-switch of the path
+     *                     and the second element the dst-switch of the path.
+     * @return true if successful in repopulating routes
+     */
+    private boolean repopulateRoutingRulesForRoutes(Set<ArrayList<DeviceId>> routeChanges) {
         rulePopulator.resetCounter();
         HashMap<DeviceId, ArrayList<ArrayList<DeviceId>>> routesBydevice =
                 new HashMap<>();
-        for (ArrayList<DeviceId> link: routes) {
+        for (ArrayList<DeviceId> link: routeChanges) {
             // When only the source device is defined, reinstall routes to all other devices
             if (link.size() == 1) {
-                log.trace("repopulateRoutingRulesForRoutes: running ECMP graph for device {}", link.get(0));
+                log.debug("-- repopulateRoutingRulesForRoutes: running ECMP graph for device {}", link.get(0));
                 EcmpShortestPathGraph ecmpSpg = new EcmpShortestPathGraph(link.get(0), srManager);
                 if (populateEcmpRoutingRules(link.get(0), ecmpSpg, ImmutableSet.of())) {
                     log.debug("Populating flow rules from all to dest:{} is successful",
                               link.get(0));
                     currentEcmpSpgMap.put(link.get(0), ecmpSpg);
+                    log.debug("Updating ECMPspg for dest:{}", link.get(0));
                 } else {
                     log.warn("Failed to populate the flow rules from all to dest:{}", link.get(0));
                     return false;
@@ -251,7 +489,7 @@
             ArrayList<ArrayList<DeviceId>> deviceRoutes =
                     routesBydevice.get(impactedDevice);
             for (ArrayList<DeviceId> link: deviceRoutes) {
-                log.debug("repopulate RoutingRules For Routes {} -> {}",
+                log.debug("-- repopulateRoutingRulesForRoutes {} -> {}",
                           link.get(0), link.get(1));
                 DeviceId src = link.get(0);
                 DeviceId dst = link.get(1);
@@ -267,6 +505,7 @@
                         }
                         Set<DeviceId> nextHops = new HashSet<>();
                         for (ArrayList<DeviceId> via : swViaMap.get(targetSw)) {
+                            // in this ECMP path to the targetSw, get the next hop
                             if (via.isEmpty()) {
                                 nextHops.add(dst);
                             } else {
@@ -281,7 +520,6 @@
                                   targetSw, dst);
                     }
                 }
-                //currentEcmpSpgMap.put(dst, ecmpSpg);
             }
             //Only if all the flows for all impacted routes to a
             //specific target are pushed successfully, update the
@@ -292,7 +530,11 @@
             //is updated here, without any flows being pushed.
             currentEcmpSpgMap.put(impactedDevice,
                                   updatedEcmpSpgMap.get(impactedDevice));
+            log.debug("Updating ECMPspg for impacted dev:{}", impactedDevice);
         }
+
+        processHashGroupChange(routeChanges, false, null);
+
         return true;
     }
 
@@ -323,10 +565,15 @@
                         + " rerouting and opting for full-reroute", sw.id());
                 return null;
             }
+            if (log.isDebugEnabled()) {
+                log.debug("Root switch: {}", sw.id());
+                log.debug("  Current/Existing SPG: {}", ecmpSpg);
+                log.debug("       New/Updated SPG: {}", updatedEcmpSpgMap.get(sw.id()));
+            }
             HashMap<Integer, HashMap<DeviceId, ArrayList<ArrayList<DeviceId>>>> switchVia =
                     ecmpSpg.getAllLearnedSwitchesAndVia();
             for (Integer itrIdx : switchVia.keySet()) {
-                log.trace("Iterindex# {}", itrIdx);
+                log.trace("Current/Exiting SPG Iterindex# {}", itrIdx);
                 HashMap<DeviceId, ArrayList<ArrayList<DeviceId>>> swViaMap =
                         switchVia.get(itrIdx);
                 for (DeviceId targetSw : swViaMap.keySet()) {
@@ -348,8 +595,8 @@
                                      alink.get(1).equals(linkFail.src().deviceId()))) {
                             log.debug("Impacted route:{}->{}", targetSw, rootSw);
                             ArrayList<DeviceId> aRoute = new ArrayList<>();
-                            aRoute.add(targetSw);
-                            aRoute.add(rootSw);
+                            aRoute.add(targetSw); // switch with rules to populate
+                            aRoute.add(rootSw); // towards this destination
                             routes.add(aRoute);
                             break;
                         }
@@ -382,22 +629,23 @@
                 continue;
             }
             if (log.isTraceEnabled()) {
-                log.trace("link of {} - ", rootSw);
+                log.trace("Device links for dev: {}", rootSw);
                 for (Link link: srManager.linkService.getDeviceLinks(rootSw)) {
                     log.trace("{} -> {} ", link.src().deviceId(), link.dst().deviceId());
                 }
             }
             EcmpShortestPathGraph currEcmpSpg = currentEcmpSpgMap.get(rootSw);
             if (currEcmpSpg == null) {
-                log.debug("No existing ECMP graph for device {}", rootSw);
+                log.debug("No existing ECMP graph for device {}.. adding self as "
+                        + "changed route", rootSw);
                 changedRoutesBuilder.add(Lists.newArrayList(rootSw));
                 continue;
             }
             EcmpShortestPathGraph newEcmpSpg = updatedEcmpSpgMap.get(rootSw);
-            if (log.isTraceEnabled()) {
-                log.trace("Root switch: {}", rootSw);
-                log.trace("  Current/Existing SPG: {}", currEcmpSpg);
-                log.trace("       New/Updated SPG: {}", newEcmpSpg);
+            if (log.isDebugEnabled()) {
+                log.debug("Root switch: {}", rootSw);
+                log.debug("  Current/Existing SPG: {}", currEcmpSpg);
+                log.debug("       New/Updated SPG: {}", newEcmpSpg);
             }
             // first use the updated/new map to compare to current/existing map
             // as new links may have come up
@@ -445,7 +693,7 @@
                 ArrayList<ArrayList<DeviceId>> basePath = baseViaMap.get(targetSw);
                 ArrayList<ArrayList<DeviceId>> compPath = getVia(compMap, targetSw);
                 if ((compPath == null) || !basePath.equals(compPath)) {
-                    log.debug("Impacted route:{} -> {}", targetSw, rootSw);
+                    log.trace("Impacted route:{} -> {}", targetSw, rootSw);
                     ArrayList<DeviceId> route = new ArrayList<>();
                     route.add(targetSw);
                     route.add(rootSw);
@@ -501,7 +749,7 @@
      * @param destSw Device ID of destination switch
      * @param ecmpSPG ECMP shortest path graph
      * @param subnets Subnets to be populated. If empty, populate all configured subnets.
-     * @return true if succeed
+     * @return true if it succeeds in populating rules
      */
     private boolean populateEcmpRoutingRules(DeviceId destSw,
                                              EcmpShortestPathGraph ecmpSPG,
@@ -538,7 +786,7 @@
      * @param destSw Device ID of final destination switch to which the rules will forward
      * @param nextHops List of next hops via which destSw will be reached
      * @param subnets Subnets to be populated. If empty, populate all configured subnets.
-     * @return true if succeed
+     * @return true if it succees in populating rules
      */
     private boolean populateEcmpRoutingRulePartial(DeviceId targetSw,
                                                    DeviceId destSw,
@@ -567,70 +815,58 @@
         }
 
         if (targetIsEdge && destIsEdge) {
-            subnets = (subnets != null && !subnets.isEmpty()) ? subnets : config.getSubnets(destSw);
-            log.debug("* populateEcmpRoutingRulePartial in device {} towards {} for subnets {}",
-                      targetSw, destSw, subnets);
+            subnets = (subnets != null && !subnets.isEmpty()) ? subnets
+                                                              : config.getSubnets(destSw);
+            log.debug("* populateEcmpRoutingRulePartial in device {} towards {} "
+                    + "for subnets {}", targetSw, destSw, subnets);
             result = rulePopulator.populateIpRuleForSubnet(targetSw, subnets,
                                                            destSw, nextHops);
             if (!result) {
                 return false;
             }
-
             IpPrefix routerIpPrefix = destRouterIpv4.toIpPrefix();
-            log.debug("* populateEcmpRoutingRulePartial in device {} towards {} for router IP {}",
-                      targetSw, destSw, routerIpPrefix);
-            result = rulePopulator.populateIpRuleForRouter(targetSw, routerIpPrefix, destSw, nextHops);
+            log.debug("* populateEcmpRoutingRulePartial in device {} towards {} "
+                    + "for router IP {}", targetSw, destSw, routerIpPrefix);
+            result = rulePopulator.populateIpRuleForRouter(targetSw, routerIpPrefix,
+                                                           destSw, nextHops);
             if (!result) {
                 return false;
             }
-            /*
-             * If present we deal with IPv6 loopback.
-             */
+            // If present we deal with IPv6 loopback.
             if (destRouterIpv6 != null) {
                 routerIpPrefix = destRouterIpv6.toIpPrefix();
-                log.debug("* populateEcmpRoutingRulePartial in device {} towards {} for v6 router IP {}",
-                          targetSw, destSw, routerIpPrefix);
-                result = rulePopulator.populateIpRuleForRouter(targetSw, routerIpPrefix, destSw, nextHops);
-                if (!result) {
-                    return false;
-                }
-            }
-
-        } else if (targetIsEdge) {
-            // If the target switch is an edge router, then set IP rules for the router IP.
-            IpPrefix routerIpPrefix = destRouterIpv4.toIpPrefix();
-            log.debug("* populateEcmpRoutingRulePartial in device {} towards {} for router IP {}",
-                      targetSw, destSw, routerIpPrefix);
-            result = rulePopulator.populateIpRuleForRouter(targetSw, routerIpPrefix, destSw, nextHops);
-            if (!result) {
-                return false;
-            }
-            if (destRouterIpv6 != null) {
-                routerIpPrefix = destRouterIpv6.toIpPrefix();
-                log.debug("* populateEcmpRoutingRulePartial in device {} towards {} for v6 router IP {}",
-                          targetSw, destSw, routerIpPrefix);
-                result = rulePopulator.populateIpRuleForRouter(targetSw, routerIpPrefix, destSw, nextHops);
+                log.debug("* populateEcmpRoutingRulePartial in device {} towards {}"
+                        + " for v6 router IP {}", targetSw, destSw, routerIpPrefix);
+                result = rulePopulator.populateIpRuleForRouter(targetSw, routerIpPrefix,
+                                                               destSw, nextHops);
                 if (!result) {
                     return false;
                 }
             }
         }
-        // Populates MPLS rules to all routers
-        log.debug("* populateEcmpRoutingRulePartial in device{} towards {} for all MPLS rules",
-                targetSw, destSw);
-        result = rulePopulator.populateMplsRule(targetSw, destSw, nextHops, destRouterIpv4);
-        if (!result) {
-            return false;
-        }
-        /*
-         * If present we will populate the MPLS rules for the IPv6 sid.
-         */
-        if (destRouterIpv6 != null) {
-            result = rulePopulator.populateMplsRule(targetSw, destSw, nextHops, destRouterIpv6);
+
+        if (!targetIsEdge && destIsEdge) {
+            // MPLS rules in all non-edge target devices
+            log.debug("* populateEcmpRoutingRulePartial in device{} towards {} for "
+                    + "all MPLS rules", targetSw, destSw);
+            result = rulePopulator.populateMplsRule(targetSw, destSw, nextHops,
+                                                    destRouterIpv4);
             if (!result) {
                 return false;
             }
+            if (destRouterIpv6 != null) {
+                result = rulePopulator.populateMplsRule(targetSw, destSw, nextHops,
+                                                        destRouterIpv6);
+                if (!result) {
+                    return false;
+                }
+            }
         }
+
+        // To save on ECMP groups
+        // avoid MPLS rules in non-edge-devices to non-edge-devices
+        // avoid MPLS transit rules in edge-devices
+        // avoid loopback IP rules in edge-devices to non-edge-devices
         return true;
     }
 
@@ -740,7 +976,6 @@
         if (updatedEcmpSpgMap != null) {
             updatedEcmpSpgMap.remove(deviceId);
         }
-        this.populateRoutingRulesForLinkStatusChange(null);
     }
 
     /**
diff --git a/src/main/java/org/onosproject/segmentrouting/EcmpShortestPathGraph.java b/src/main/java/org/onosproject/segmentrouting/EcmpShortestPathGraph.java
index f5d8ed1..50bab91 100644
--- a/src/main/java/org/onosproject/segmentrouting/EcmpShortestPathGraph.java
+++ b/src/main/java/org/onosproject/segmentrouting/EcmpShortestPathGraph.java
@@ -361,16 +361,19 @@
         StringBuilder sBuilder = new StringBuilder();
         for (Device device: srManager.deviceService.getDevices()) {
             if (device.id() != rootDevice) {
-                sBuilder.append("\r\n Paths from " + rootDevice + " to "
-                                + device.id() + "\r\n");
+                sBuilder.append("\r\n  Paths from " + rootDevice + " to "
+                                + device.id());
                 ArrayList<Path> paths = getECMPPaths(device.id());
                 if (paths != null) {
                     for (Path path : paths) {
-                        sBuilder.append("\r\n == "); // equal cost paths delimiter
-                        for (Link link : path.links()) {
+                        sBuilder.append("\r\n       == "); // equal cost paths delimiter
+                        for (int i = path.links().size() - 1; i >= 0; i--) {
+                            Link link = path.links().get(i);
                             sBuilder.append(" : " + link.src() + " -> " + link.dst());
                         }
                     }
+                } else {
+                    sBuilder.append("\r\n       == no paths");
                 }
             }
         }
diff --git a/src/main/java/org/onosproject/segmentrouting/RoutingRulePopulator.java b/src/main/java/org/onosproject/segmentrouting/RoutingRulePopulator.java
index 850962a..ca24e00 100644
--- a/src/main/java/org/onosproject/segmentrouting/RoutingRulePopulator.java
+++ b/src/main/java/org/onosproject/segmentrouting/RoutingRulePopulator.java
@@ -34,6 +34,7 @@
 import org.onosproject.segmentrouting.DefaultRoutingHandler.PortFilterInfo;
 import org.onosproject.segmentrouting.config.DeviceConfigNotFoundException;
 import org.onosproject.segmentrouting.config.DeviceConfiguration;
+import org.onosproject.segmentrouting.grouphandler.DefaultGroupHandler;
 import org.onosproject.segmentrouting.grouphandler.NeighborSet;
 import org.onosproject.net.DeviceId;
 import org.onosproject.net.Port;
@@ -326,10 +327,10 @@
         // is not set.
         if (nextHops.size() == 1 && nextHops.toArray()[0].equals(destSw)) {
             tbuilder.immediate().decNwTtl();
-            ns = new NeighborSet(nextHops, false);
+            ns = new NeighborSet(nextHops, false, destSw);
             treatment = tbuilder.build();
         } else {
-            ns = new NeighborSet(nextHops, false, segmentId);
+            ns = new NeighborSet(nextHops, false, segmentId, destSw);
             treatment = null;
         }
 
@@ -338,8 +339,14 @@
         // other neighboring routers, there is no subnet assigned on those ports.
         TrafficSelector.Builder metabuilder = DefaultTrafficSelector.builder(selector);
         metabuilder.matchVlanId(SegmentRoutingManager.INTERNAL_VLAN);
+        DefaultGroupHandler grpHandler = srManager.getGroupHandler(deviceId);
+        if (grpHandler == null) {
+            log.warn("populateIPRuleForRouter: groupHandler for device {} "
+                    + "not found", deviceId);
+            return false;
+        }
 
-        int nextId = srManager.getNextObjectiveId(deviceId, ns, metabuilder.build());
+        int nextId = grpHandler.getNextObjectiveId(ns, metabuilder.build(), true);
         if (nextId <= 0) {
             log.warn("No next objective in {} for ns: {}", deviceId, ns);
             return false;
@@ -356,10 +363,8 @@
         if (treatment != null) {
             fwdBuilder.withTreatment(treatment);
         }
-        log.debug("Installing IPv4 forwarding objective "
-                        + "for router IP/subnet {} in switch {}",
-                ipPrefix,
-                deviceId);
+        log.debug("Installing IPv4 forwarding objective for router IP/subnet {} "
+                + "in switch {} with nextId: {}", ipPrefix, deviceId, nextId);
         ObjectiveContext context = new DefaultObjectiveContext(
                 (objective) -> log.debug("IP rule for router {} populated in dev:{}",
                                          ipPrefix, deviceId),
@@ -438,7 +443,7 @@
         if (nextHops.size() == 1 && destSwId.equals(nextHops.toArray()[0])) {
             // If the next hop is the destination router for the segment, do pop
             log.debug("populateMplsRule: Installing MPLS forwarding objective for "
-                              + "label {} in switch {} with pop", segmentId, targetSwId);
+                    + "label {} in switch {} with pop", segmentId, targetSwId);
             // Not-bos pop case (php for the current label). If MPLS-ECMP
             // has been configured, the application we will request the
             // installation for an MPLS-ECMP group.
@@ -448,7 +453,8 @@
                                                true,
                                                isMplsBos,
                                                metabuilder.build(),
-                                               routerIp);
+                                               routerIp,
+                                               destSwId);
             // Error case, we cannot handle, exit.
             if (fwdObjNoBosBuilder == null) {
                 return Collections.emptyList();
@@ -457,8 +463,8 @@
 
         } else {
             // next hop is not destination, SR CONTINUE case (swap with self)
-            log.debug("Installing MPLS forwarding objective for "
-                              + "label {} in switch {} without pop", segmentId, targetSwId);
+            log.debug("Installing MPLS forwarding objective for label {} in "
+                    + "switch {} without pop", segmentId, targetSwId);
             // Not-bos pop case. If MPLS-ECMP has been configured, the
             // application we will request the installation for an MPLS-ECMP
             // group.
@@ -468,7 +474,8 @@
                                                false,
                                                isMplsBos,
                                                metabuilder.build(),
-                                               routerIp);
+                                               routerIp,
+                                               destSwId);
             // Error case, we cannot handle, exit.
             if (fwdObjNoBosBuilder == null) {
                 return Collections.emptyList();
@@ -561,7 +568,8 @@
                                              boolean phpRequired,
                                              boolean isBos,
                                              TrafficSelector meta,
-                                             IpAddress routerIp) {
+                                             IpAddress routerIp,
+                                             DeviceId destSw) {
 
         ForwardingObjective.Builder fwdBuilder = DefaultForwardingObjective
                 .builder().withFlag(ForwardingObjective.Flag.SPECIFIC);
@@ -592,11 +600,11 @@
         fwdBuilder.withTreatment(tbuilder.build());
         // if MPLS-ECMP == True we will build a standard NeighborSet.
         // Otherwise a RandomNeighborSet.
-        NeighborSet ns = NeighborSet.neighborSet(false, nextHops, false);
+        NeighborSet ns = NeighborSet.neighborSet(false, nextHops, false, destSw);
         if (!isBos && this.srManager.getMplsEcmp()) {
-            ns = NeighborSet.neighborSet(false, nextHops, true);
+            ns = NeighborSet.neighborSet(false, nextHops, true, destSw);
         } else if (!isBos && !this.srManager.getMplsEcmp()) {
-            ns = NeighborSet.neighborSet(true, nextHops, true);
+            ns = NeighborSet.neighborSet(true, nextHops, true, destSw);
         }
         log.debug("Trying to get a nextObjId for mpls rule on device:{} to ns:{}",
                   deviceId, ns);
@@ -606,7 +614,13 @@
         // MPLS-ECMP.
         // The metadata informs the driver that the next-Objective will be used
         // by MPLS flows and if Bos == False the driver will use MPLS groups.
-        int nextId = srManager.getNextObjectiveId(deviceId, ns, meta, isBos);
+        DefaultGroupHandler grpHandler = srManager.getGroupHandler(deviceId);
+        if (grpHandler == null) {
+            log.warn("populateIPRuleForRouter: groupHandler for device {} "
+                    + "not found", deviceId);
+            return null;
+        }
+        int nextId = grpHandler.getNextObjectiveId(ns, meta, isBos);
         if (nextId <= 0) {
             log.warn("No next objective in {} for ns: {}", deviceId, ns);
             return null;
@@ -710,13 +724,13 @@
             // error encountered during build
             return false;
         }
-        log.info("{} filtering objectives for dev/port:{}/{}", (install ? "Installing" : "Removing"),
-                deviceId, portnum);
+        log.debug("{} filtering objectives for dev/port:{}/{}",
+                 install ? "Installing" : "Removing", deviceId, portnum);
         ObjectiveContext context = new DefaultObjectiveContext(
                 (objective) -> log.debug("Filter for {}/{} {}", deviceId, portnum,
-                        (install ? "installed" : "removed")),
+                        install ? "installed" : "removed"),
                 (objective, error) -> log.warn("Failed to {} filter for {}/{}: {}",
-                        (install ? "install" : "remove"), deviceId, portnum, error));
+                        install ? "install" : "remove", deviceId, portnum, error));
         if (install) {
             srManager.flowObjectiveService.filter(deviceId, fob.add(context));
         } else {
@@ -894,10 +908,6 @@
      * priority Bridging Table entries to a group that contains all ports of
      * its subnet.
      *
-     * Note: We assume that packets sending from the edge switches to the hosts
-     * have untagged VLAN.
-     * The VLAN tag will be popped later in the flooding group.
-     *
      * @param deviceId switch ID to set the rules
      */
     public void populateSubnetBroadcastRule(DeviceId deviceId) {
diff --git a/src/main/java/org/onosproject/segmentrouting/SegmentRoutingManager.java b/src/main/java/org/onosproject/segmentrouting/SegmentRoutingManager.java
index c783081..3bc56b6 100644
--- a/src/main/java/org/onosproject/segmentrouting/SegmentRoutingManager.java
+++ b/src/main/java/org/onosproject/segmentrouting/SegmentRoutingManager.java
@@ -16,6 +16,7 @@
 package org.onosproject.segmentrouting;
 
 import com.google.common.collect.HashMultimap;
+import com.google.common.collect.ImmutableMap;
 import com.google.common.collect.Maps;
 import com.google.common.collect.Multimap;
 import org.apache.felix.scr.annotations.Activate;
@@ -99,6 +100,7 @@
 
 import java.util.List;
 import java.util.Map;
+import java.util.Map.Entry;
 import java.util.Set;
 import java.util.concurrent.ConcurrentHashMap;
 import java.util.concurrent.ConcurrentLinkedQueue;
@@ -220,6 +222,16 @@
     EventuallyConsistentMap<PortNextObjectiveStoreKey, Integer>
             portNextObjStore = null;
 
+    // Local store for all links seen and their present status, used for
+    // optimized routing. The existence of the link in the keys is enough to know
+    // if the link has been "seen-before" by this instance of the controller.
+    // The boolean value indicates if the link is currently up or not.
+    // XXX Currently the optimized routing logic depends on "forgetting" a link
+    // when a switch goes down, but "remembering" it when only the link goes down.
+    // Consider changing this logic so we can use the Link Service instead of
+    // a local cache.
+    private Map<Link, Boolean> seenLinks = new ConcurrentHashMap<>();
+
     private EventuallyConsistentMap<String, Tunnel> tunnelStore = null;
     private EventuallyConsistentMap<String, Policy> policyStore = null;
 
@@ -488,6 +500,25 @@
         return deviceSubnetMap;
     }
 
+
+    @Override
+    public ImmutableMap<DeviceId, EcmpShortestPathGraph> getCurrentEcmpSpg() {
+        if (defaultRoutingHandler != null) {
+            return defaultRoutingHandler.getCurrentEmcpSpgMap();
+        } else {
+            return null;
+        }
+    }
+
+    @Override
+    public ImmutableMap<NeighborSetNextObjectiveStoreKey, Integer> getNeighborSet() {
+        if (nsNextObjStore != null) {
+            return ImmutableMap.copyOf(nsNextObjStore.entrySet());
+        } else {
+            return ImmutableMap.of();
+        }
+    }
+
     /**
      * Extracts the application ID from the manager.
      *
@@ -630,48 +661,6 @@
     }
 
     /**
-     * Returns the next objective ID for the given NeighborSet.
-     * If the nextObjective does not exist, a new one is created and
-     * its id is returned.
-     *
-     * @param deviceId Device ID
-     * @param ns NegighborSet
-     * @param meta metadata passed into the creation of a Next Objective
-     * @param isBos indicates if it is BoS or not
-     * @return next objective ID or -1 if an error was encountered during the
-     *         creation of the nextObjective
-     */
-    public int getNextObjectiveId(DeviceId deviceId, NeighborSet ns,
-                                  TrafficSelector meta, boolean isBos) {
-        if (groupHandlerMap.get(deviceId) != null) {
-            log.trace("getNextObjectiveId query in device {}", deviceId);
-            return groupHandlerMap
-                    .get(deviceId).getNextObjectiveId(ns, meta, isBos);
-        } else {
-            log.warn("getNextObjectiveId query - groupHandler for device {} "
-                    + "not found", deviceId);
-            return -1;
-        }
-    }
-
-    /**
-     * Returns the next objective ID for the given NeighborSet.
-     * If the nextObjective does not exist, a new one is created and
-     * its id is returned.
-     *
-     * @param deviceId Device ID
-     * @param ns NegighborSet
-     * @param meta metadata passed into the creation of a Next Objective
-     * @return next objective ID or -1 if an error was encountered during the
-     *         creation of the nextObjective
-     */
-    public int getNextObjectiveId(DeviceId deviceId,
-                                  NeighborSet ns,
-                                  TrafficSelector meta) {
-        return this.getNextObjectiveId(deviceId, ns, meta, true);
-    }
-
-    /**
      * Returns the next objective ID for the given subnet prefix. It is expected
      * Returns the next objective ID for the given vlan id. It is expected
      * that the next-objective has been pre-created from configuration.
@@ -718,6 +707,88 @@
         }
     }
 
+    /**
+     * Returns the group handler object for the specified device id.
+     *
+     * @param devId the device identifier
+     * @return the groupHandler object for the device id, or null if not found
+     */
+    public DefaultGroupHandler getGroupHandler(DeviceId devId) {
+        return groupHandlerMap.get(devId);
+    }
+
+    /**
+     * Returns true if this controller instance has seen this link before. The
+     * link may not be currently up, but as long as the link had been seen before
+     * this method will return true. The one exception is when the link was
+     * indeed seen before, but this controller instance was forced to forget it
+     * by a call to purgeSeenLink method.
+     *
+     * @param link the infrastructure link being queried
+     * @return true if this controller instance has seen this link before
+     */
+    public boolean isSeenLink(Link link) {
+        return seenLinks.containsKey(link);
+    }
+
+    /**
+     * Updates the seen link store. Updates can be for links that are currently
+     * available or not.
+     *
+     * @param link the link to update in the seen-link local store
+     * @param up the status of the link, true if up, false if down
+     */
+    public void updateSeenLink(Link link, boolean up) {
+        seenLinks.put(link, up);
+    }
+
+    /**
+     * Returns the status of a seen-link (up or down). If the link has not
+     * been seen-before, a null object is returned.
+     *
+     * @param link the infrastructure link being queried
+     * @return null if the link was not seen-before;
+     *         true if the seen-link is up;
+     *         false if the seen-link is down
+     */
+    public Boolean isSeenLinkUp(Link link) {
+        return seenLinks.get(link);
+    }
+
+    /**
+     * Makes this controller instance forget a previously seen before link.
+     *
+     * @param link the infrastructure link to purge
+     */
+    public void purgeSeenLink(Link link) {
+        seenLinks.remove(link);
+    }
+
+    /**
+     * Returns the status of a link as parallel link. A parallel link
+     * is defined as a link which has common src and dst switches as another
+     * seen-link that is currently enabled. It is not necessary for the link being
+     * queried to be a seen-link.
+     *
+     *  @param link the infrastructure link being queried
+     *  @return true if a seen-link exists that is up, and shares the
+     *          same src and dst switches as the link being queried
+     */
+    public boolean isParallelLink(Link link) {
+        for (Entry<Link, Boolean> seen : seenLinks.entrySet()) {
+            Link seenLink = seen.getKey();
+           if (seenLink.equals(link)) {
+               continue;
+           }
+           if (seenLink.src().deviceId().equals(link.src().deviceId()) &&
+                   seenLink.dst().deviceId().equals(link.dst().deviceId()) &&
+                   seen.getValue()) {
+               return true;
+           }
+        }
+        return false;
+    }
+
     private class InternalPacketProcessor implements PacketProcessor {
         @Override
         public void process(PacketContext context) {
@@ -735,7 +806,8 @@
 
             log.trace("Rcvd pktin: {}", ethernet);
             if (ethernet.getEtherType() == TYPE_ARP) {
-                log.warn("Received unexpected ARP packet on {}", context.inPacket().receivedFrom());
+                log.warn("Received unexpected ARP packet on {}",
+                         context.inPacket().receivedFrom());
                 log.trace("{}", ethernet);
                 return;
             } else if (ethernet.getEtherType() == Ethernet.TYPE_IPV4) {
@@ -758,7 +830,7 @@
                             icmp6Packet.getIcmpType() == ICMP6.ECHO_REPLY) {
                         icmpHandler.processIcmpv6(ethernet, pkt.receivedFrom());
                     } else {
-                        log.debug("Received ICMPv6 0x{} - not handled",
+                        log.trace("Received ICMPv6 0x{} - not handled",
                                 Integer.toHexString(icmp6Packet.getIcmpType() & 0xff));
                     }
                 } else {
@@ -791,7 +863,7 @@
             case PORT_ADDED:
             case DEVICE_UPDATED:
             case DEVICE_AVAILABILITY_CHANGED:
-                log.debug("Event {} received from Device Service", event.type());
+                log.trace("Event {} received from Device Service", event.type());
                 scheduleEventHandlerIfNotScheduled(event);
                 break;
             default:
@@ -837,15 +909,28 @@
                     }
                     if (event.type() == LinkEvent.Type.LINK_ADDED ||
                             event.type() == LinkEvent.Type.LINK_UPDATED) {
+                        // Note: do not update seenLinks here, otherwise every
+                        // link, even one seen for the first time, will be appear
+                        // to be a previously seen link
                         processLinkAdded((Link) event.subject());
                     } else if (event.type() == LinkEvent.Type.LINK_REMOVED) {
                         Link linkRemoved = (Link) event.subject();
+                        if (linkRemoved.type() == Link.Type.DIRECT) {
+                            updateSeenLink(linkRemoved, false);
+                        }
+                        // device availability check helps to ensure that
+                        // multiple link-removed events are actually treated as a
+                        // single switch removed event. purgeSeenLink is necessary
+                        // so we do rerouting (instead of rehashing) when switch
+                        // comes back.
                         if (linkRemoved.src().elementId() instanceof DeviceId &&
                                 !deviceService.isAvailable(linkRemoved.src().deviceId())) {
+                            purgeSeenLink(linkRemoved);
                             continue;
                         }
                         if (linkRemoved.dst().elementId() instanceof DeviceId &&
                                 !deviceService.isAvailable(linkRemoved.dst().deviceId())) {
+                            purgeSeenLink(linkRemoved);
                             continue;
                         }
                         processLinkRemoved((Link) event.subject());
@@ -867,7 +952,7 @@
                         // so port filtering rules are handled at the device_added event.
                         // port added calls represent all ports on the device,
                         // enabled or not.
-                        log.debug("** PORT ADDED {}/{} -> {}",
+                        log.trace("** PORT ADDED {}/{} -> {}",
                                   ((DeviceEvent) event).subject().id(),
                                   ((DeviceEvent) event).port().number(),
                                   event.type());
@@ -897,16 +982,22 @@
             log.warn("Source device of this link is not configured.");
             return;
         }
-        //Irrespective whether the local is a MASTER or not for this device,
-        //create group handler instance and push default TTP flow rules.
-        //Because in a multi-instance setup, instances can initiate
-        //groups for any devices. Also the default TTP rules are needed
-        //to be pushed before inserting any IP table entries for any device
+        if (link.type() != Link.Type.DIRECT) {
+            // NOTE: A DIRECT link might be transiently marked as INDIRECT
+            //       if BDDP is received before LLDP. We can safely ignore that
+            //       until the LLDP is received and the link is marked as DIRECT.
+            log.info("Ignore link {}->{}. Link type is {} instead of DIRECT.",
+                    link.src(), link.dst(), link.type());
+            return;
+        }
+
+        //Irrespective of whether the local is a MASTER or not for this device,
+        //create group handler instance and push default TTP flow rules if needed,
+        //as in a multi-instance setup, instances can initiate groups for any device.
         DefaultGroupHandler groupHandler = groupHandlerMap.get(link.src()
                 .deviceId());
         if (groupHandler != null) {
-            groupHandler.linkUp(link, mastershipService.isLocalMaster(
-                                           link.src().deviceId()));
+            groupHandler.portUpForLink(link);
         } else {
             Device device = deviceService.getDevice(link.src().deviceId());
             if (device != null) {
@@ -916,29 +1007,45 @@
                 processDeviceAdded(device);
                 groupHandler = groupHandlerMap.get(link.src()
                                                    .deviceId());
-                groupHandler.linkUp(link, mastershipService.isLocalMaster(device.id()));
+                groupHandler.portUpForLink(link);
             }
         }
 
         log.trace("Starting optimized route population process");
-        defaultRoutingHandler.populateRoutingRulesForLinkStatusChange(null);
-        //log.trace("processLinkAdded: re-starting route population process");
-        //defaultRoutingHandler.startPopulationProcess();
+        boolean seenBefore = isSeenLink(link);
+        defaultRoutingHandler.populateRoutingRulesForLinkStatusChange(null, link, null);
+        if (mastershipService.isLocalMaster(link.src().deviceId())) {
+            if (!seenBefore) {
+                // if link seen first time, we need to ensure hash-groups have all ports
+                groupHandler.retryHash(link, false, true);
+            } else {
+                //seen before-link
+                if (isParallelLink(link)) {
+                    groupHandler.retryHash(link, false, false);
+                }
+            }
+        }
 
         mcastHandler.init();
     }
 
     private void processLinkRemoved(Link link) {
         log.info("** LINK REMOVED {}", link.toString());
+        defaultRoutingHandler.populateRoutingRulesForLinkStatusChange(link, null, null);
+
+        // update local groupHandler stores
         DefaultGroupHandler groupHandler = groupHandlerMap.get(link.src().deviceId());
         if (groupHandler != null) {
-            groupHandler.portDown(link.src().port(),
-                                  mastershipService.isLocalMaster(link.src().deviceId()));
+            if (mastershipService.isLocalMaster(link.src().deviceId()) &&
+                    isParallelLink(link)) {
+                groupHandler.retryHash(link, true, false);
+            }
+            // ensure local stores are updated
+            groupHandler.portDown(link.src().port());
+        } else {
+            log.warn("group handler not found for dev:{} when removing link: {}",
+                     link.src().deviceId(), link);
         }
-        log.trace("Starting optimized route population process");
-        defaultRoutingHandler.populateRoutingRulesForLinkStatusChange(link);
-        //log.trace("processLinkRemoved: re-starting route population process");
-        //defaultRoutingHandler.startPopulationProcess();
 
         mcastHandler.processLinkDown(link);
     }
@@ -1011,6 +1118,11 @@
                 });
         groupHandlerMap.remove(device.id());
         defaultRoutingHandler.purgeEcmpGraph(device.id());
+        // Note that a switch going down is associated with all of its links
+        // going down as well, but it is treated as a single switch down event
+        // while the link-downs are ignored.
+        defaultRoutingHandler
+            .populateRoutingRulesForLinkStatusChange(null, null, device.id());
         mcastHandler.removeDevice(device.id());
         xConnectHandler.removeDevice(device.id());
     }
@@ -1049,7 +1161,7 @@
         Set<VlanId> taggedVlans = getTaggedVlanId(cp);
 
         if (untaggedVlan == null && nativeVlan == null && taggedVlans.isEmpty()) {
-            log.debug("Not handling port updated event for unconfigured port "
+            log.debug("Not handling port updated event for non-edge port (unconfigured) "
                     + "dev/port: {}/{}", device.id(), port.number());
             return;
         }
@@ -1272,4 +1384,5 @@
             }
         }
     }
+
 }
diff --git a/src/main/java/org/onosproject/segmentrouting/SegmentRoutingService.java b/src/main/java/org/onosproject/segmentrouting/SegmentRoutingService.java
index 6f4b4e6..c0b096a 100644
--- a/src/main/java/org/onosproject/segmentrouting/SegmentRoutingService.java
+++ b/src/main/java/org/onosproject/segmentrouting/SegmentRoutingService.java
@@ -17,6 +17,9 @@
 
 import org.onlab.packet.IpPrefix;
 import org.onosproject.net.DeviceId;
+import org.onosproject.segmentrouting.storekey.NeighborSetNextObjectiveStoreKey;
+
+import com.google.common.collect.ImmutableMap;
 
 import java.util.List;
 import java.util.Map;
@@ -116,4 +119,18 @@
      * @return device-subnet mapping
      */
     Map<DeviceId, Set<IpPrefix>> getDeviceSubnetMap();
+
+    /**
+     * Returns the current ECMP shortest path graph in this controller instance.
+     *
+     * @return ECMP shortest path graph
+     */
+    ImmutableMap<DeviceId, EcmpShortestPathGraph> getCurrentEcmpSpg();
+
+    /**
+     * Returns the neighborSet-NextObjective store contents.
+     *
+     * @return current contents of the neighborSetNextObjectiveStore
+     */
+    ImmutableMap<NeighborSetNextObjectiveStoreKey, Integer> getNeighborSet();
 }
diff --git a/src/main/java/org/onosproject/segmentrouting/TunnelHandler.java b/src/main/java/org/onosproject/segmentrouting/TunnelHandler.java
index cd9467a..ee7076e 100644
--- a/src/main/java/org/onosproject/segmentrouting/TunnelHandler.java
+++ b/src/main/java/org/onosproject/segmentrouting/TunnelHandler.java
@@ -222,7 +222,9 @@
             deviceIds.add(config.getDeviceId(sid));
         }
         // For these NeighborSet isMpls is meaningless.
-        NeighborSet ns = new NeighborSet(deviceIds, false, tunnel.labelIds().get(2));
+        NeighborSet ns = new NeighborSet(deviceIds, false,
+                                         tunnel.labelIds().get(2),
+                                         DeviceId.NONE);
 
         // If the tunnel reuses any existing groups, then tunnel handler
         // should not remove the group.
diff --git a/src/main/java/org/onosproject/segmentrouting/cli/EcmpGraphCommand.java b/src/main/java/org/onosproject/segmentrouting/cli/EcmpGraphCommand.java
new file mode 100644
index 0000000..de8a62a
--- /dev/null
+++ b/src/main/java/org/onosproject/segmentrouting/cli/EcmpGraphCommand.java
@@ -0,0 +1,57 @@
+/*
+ * Copyright 2016-present Open Networking Laboratory
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.onosproject.segmentrouting.cli;
+
+
+import java.util.Map;
+
+import org.apache.karaf.shell.commands.Command;
+import org.onosproject.cli.AbstractShellCommand;
+import org.onosproject.net.DeviceId;
+import org.onosproject.segmentrouting.EcmpShortestPathGraph;
+import org.onosproject.segmentrouting.SegmentRoutingService;
+
+/**
+ * Command to read the current state of the ECMP shortest-path graph.
+ *
+ */
+@Command(scope = "onos", name = "sr-ecmp-spg",
+        description = "Displays the current ecmp shortest-path-graph in this "
+                + "controller instance")
+public class EcmpGraphCommand extends AbstractShellCommand {
+
+    private static final String FORMAT_MAPPING = "  %s";
+
+    @Override
+    protected void execute() {
+        SegmentRoutingService srService =
+                AbstractShellCommand.get(SegmentRoutingService.class);
+        printEcmpGraph(srService.getCurrentEcmpSpg());
+    }
+
+    private void printEcmpGraph(Map<DeviceId, EcmpShortestPathGraph> currentEcmpSpg) {
+        if (currentEcmpSpg == null) {
+            print(FORMAT_MAPPING, "No ECMP graph found");
+            return;
+        }
+        StringBuilder ecmp = new StringBuilder();
+        currentEcmpSpg.forEach((key, value) -> {
+            ecmp.append("\n\nRoot Device: " + key + " ECMP Paths: " + value);
+        });
+        print(FORMAT_MAPPING, ecmp.toString());
+    }
+}
diff --git a/src/main/java/org/onosproject/segmentrouting/cli/NeighborSetCommand.java b/src/main/java/org/onosproject/segmentrouting/cli/NeighborSetCommand.java
new file mode 100644
index 0000000..d85b4fc
--- /dev/null
+++ b/src/main/java/org/onosproject/segmentrouting/cli/NeighborSetCommand.java
@@ -0,0 +1,53 @@
+/*
+ * Copyright 2016-present Open Networking Laboratory
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.onosproject.segmentrouting.cli;
+
+
+import java.util.Map;
+
+import org.apache.karaf.shell.commands.Command;
+import org.onosproject.cli.AbstractShellCommand;
+import org.onosproject.segmentrouting.SegmentRoutingService;
+import org.onosproject.segmentrouting.storekey.NeighborSetNextObjectiveStoreKey;
+
+/**
+ * Command to read the current state of the neighborSetNextObjectiveStore.
+ *
+ */
+@Command(scope = "onos", name = "sr-ns-objstore",
+        description = "Displays the current neighborSet seen by each switch "
+                + "in the network and the next-id it maps to")
+public class NeighborSetCommand extends AbstractShellCommand {
+
+    private static final String FORMAT_MAPPING = "  %s";
+
+    @Override
+    protected void execute() {
+        SegmentRoutingService srService =
+                AbstractShellCommand.get(SegmentRoutingService.class);
+        printNeighborSet(srService.getNeighborSet());
+    }
+
+    private void printNeighborSet(Map<NeighborSetNextObjectiveStoreKey, Integer> ns) {
+        StringBuilder nsbldr = new StringBuilder();
+        ns.entrySet().forEach(entry -> {
+            nsbldr.append("\n " + entry.getKey());
+            nsbldr.append(" --> NextId: " + entry.getValue());
+        });
+        print(FORMAT_MAPPING, nsbldr.toString());
+    }
+}
diff --git a/src/main/java/org/onosproject/segmentrouting/grouphandler/DefaultEdgeGroupHandler.java b/src/main/java/org/onosproject/segmentrouting/grouphandler/DefaultEdgeGroupHandler.java
deleted file mode 100644
index 7fc9480..0000000
--- a/src/main/java/org/onosproject/segmentrouting/grouphandler/DefaultEdgeGroupHandler.java
+++ /dev/null
@@ -1,181 +0,0 @@
-/*
- * Copyright 2015-present Open Networking Laboratory
- *
- * Licensed under the Apache License, Version 2.0 (the "License");
- * you may not use this file except in compliance with the License.
- * You may obtain a copy of the License at
- *
- *     http://www.apache.org/licenses/LICENSE-2.0
- *
- * Unless required by applicable law or agreed to in writing, software
- * distributed under the License is distributed on an "AS IS" BASIS,
- * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
- * See the License for the specific language governing permissions and
- * limitations under the License.
- */
-package org.onosproject.segmentrouting.grouphandler;
-
-import org.onosproject.core.ApplicationId;
-import org.onosproject.net.DeviceId;
-import org.onosproject.net.Link;
-import org.onosproject.net.flowobjective.FlowObjectiveService;
-import org.onosproject.net.link.LinkService;
-import org.onosproject.segmentrouting.SegmentRoutingManager;
-import org.onosproject.segmentrouting.config.DeviceProperties;
-
-import java.util.HashSet;
-import java.util.List;
-import java.util.Set;
-
-/**
- * Default ECMP group handler creation module for an edge device.
- * This component creates a set of ECMP groups for every neighbor
- * that this device is connected to.
- * For example, consider a network of 4 devices: D0 (Segment ID: 100),
- * D1 (Segment ID: 101), D2 (Segment ID: 102) and D3 (Segment ID: 103),
- * where D0 and D3 are edge devices and D1 and D2 are transit devices.
- * Assume device D0 is connected to 2 neighbors (D1 and D2 ).
- * The following groups will be created in D0:
- * 1) all ports to D1 + with no label push, // direct attach case, seen
- * 2) all ports to D1 + with label 102 pushed, // this is not needed
- * 3) all ports to D1 + with label 103 pushed, // maybe needed, sometimes seen
- * 4) all ports to D2 + with no label push,
- * 5) all ports to D2 + with label 101 pushed,
- * 6) all ports to D2 + with label 103 pushed,
- * 7) all ports to D1 and D2 + with label 103 pushed // ecmp case
- * 8) what about ecmp no label case
- */
-public class DefaultEdgeGroupHandler extends DefaultGroupHandler {
-    protected DefaultEdgeGroupHandler(DeviceId deviceId,
-                                  ApplicationId appId,
-                                  DeviceProperties config,
-                                  LinkService linkService,
-                                  FlowObjectiveService flowObjService,
-                                  SegmentRoutingManager srManager) {
-        super(deviceId, appId, config, linkService, flowObjService, srManager);
-    }
-
-    @Override
-    public void createGroups() {
-        log.debug("Creating default groups "
-                + "for edge device {}", deviceId);
-        Set<DeviceId> neighbors = devicePortMap.keySet();
-        if (neighbors == null || neighbors.isEmpty()) {
-            return;
-        }
-
-        // Create all possible Neighbor sets from this router
-        Set<Set<DeviceId>> powerSet = getPowerSetOfNeighbors(neighbors);
-        log.trace("createGroupsAtEdgeRouter: The size of neighbor powerset "
-                + "for sw {} is {}", deviceId, powerSet.size());
-        Set<NeighborSet> nsSet = new HashSet<>();
-        for (Set<DeviceId> combo : powerSet) {
-            if (combo.isEmpty()) {
-                continue;
-            }
-            List<Integer> groupSegmentIds =
-                    getSegmentIdsTobePairedWithNeighborSet(combo);
-            for (Integer sId : groupSegmentIds) {
-                // For these NeighborSet isMpls is meaningless.
-                NeighborSet ns = new NeighborSet(combo, false, sId);
-                log.trace("createGroupsAtEdgeRouter: sw {} "
-                        + "combo {} sId {} ns {}",
-                        deviceId, combo, sId, ns);
-                nsSet.add(ns);
-            }
-        }
-        log.trace("createGroupsAtEdgeRouter: The neighborset "
-                + "with label for sw {} is {}",
-                deviceId, nsSet);
-
-        //createGroupsFromNeighborsets(nsSet);
-    }
-
-    @Override
-    protected void newNeighbor(Link newNeighborLink) {
-        log.debug("New Neighbor: Updating groups "
-                + "for edge device {}", deviceId);
-        // Recompute neighbor power set
-        addNeighborAtPort(newNeighborLink.dst().deviceId(),
-                          newNeighborLink.src().port());
-        // Compute new neighbor sets due to the addition of new neighbor
-        Set<NeighborSet> nsSet = computeImpactedNeighborsetForPortEvent(
-                                             newNeighborLink.dst().deviceId(),
-                                             devicePortMap.keySet());
-        //createGroupsFromNeighborsets(nsSet);
-    }
-
-    @Override
-    protected void newPortToExistingNeighbor(Link newNeighborLink) {
-        /*log.debug("New port to existing neighbor: Updating "
-                + "groups for edge device {}", deviceId);
-        addNeighborAtPort(newNeighborLink.dst().deviceId(),
-                          newNeighborLink.src().port());
-        Set<NeighborSet> nsSet = computeImpactedNeighborsetForPortEvent(
-                                              newNeighborLink.dst().deviceId(),
-                                              devicePortMap.keySet());
-        for (NeighborSet ns : nsSet) {
-            // Create the new bucket to be updated
-            TrafficTreatment.Builder tBuilder =
-                    DefaultTrafficTreatment.builder();
-            tBuilder.setOutput(newNeighborLink.src().port())
-                    .setEthDst(deviceConfig.getDeviceMac(
-                          newNeighborLink.dst().deviceId()))
-                    .setEthSrc(nodeMacAddr);
-            if (ns.getEdgeLabel() != NeighborSet.NO_EDGE_LABEL) {
-                tBuilder.pushMpls()
-                        .setMpls(MplsLabel.
-                                 mplsLabel(ns.getEdgeLabel()));
-            }
-
-            Integer nextId = deviceNextObjectiveIds.get(ns);
-            if (nextId != null) {
-                NextObjective.Builder nextObjBuilder = DefaultNextObjective
-                        .builder().withId(nextId)
-                        .withType(NextObjective.Type.HASHED).fromApp(appId);
-
-                nextObjBuilder.addTreatment(tBuilder.build());
-
-                NextObjective nextObjective = nextObjBuilder.add();
-                flowObjectiveService.next(deviceId, nextObjective);
-            }
-        }*/
-    }
-
-    @Override
-    protected Set<NeighborSet> computeImpactedNeighborsetForPortEvent(
-                                            DeviceId impactedNeighbor,
-                                            Set<DeviceId> updatedNeighbors) {
-        Set<Set<DeviceId>> powerSet = getPowerSetOfNeighbors(updatedNeighbors);
-
-        Set<DeviceId> tmp = new HashSet<>();
-        tmp.addAll(updatedNeighbors);
-        tmp.remove(impactedNeighbor);
-        Set<Set<DeviceId>> tmpPowerSet = getPowerSetOfNeighbors(tmp);
-
-        // Compute the impacted neighbor sets
-        powerSet.removeAll(tmpPowerSet);
-
-        Set<NeighborSet> nsSet = new HashSet<>();
-        for (Set<DeviceId> combo : powerSet) {
-            if (combo.isEmpty()) {
-                continue;
-            }
-            List<Integer> groupSegmentIds =
-                    getSegmentIdsTobePairedWithNeighborSet(combo);
-            for (Integer sId : groupSegmentIds) {
-                // For these NeighborSet isMpls is meaningless.
-                NeighborSet ns = new NeighborSet(combo, false, sId);
-                log.trace("computeImpactedNeighborsetForPortEvent: sw {} "
-                        + "combo {} sId {} ns {}",
-                        deviceId, combo, sId, ns);
-                nsSet.add(ns);
-            }
-        }
-        log.trace("computeImpactedNeighborsetForPortEvent: The neighborset "
-                + "with label for sw {} is {}",
-                deviceId, nsSet);
-        return nsSet;
-    }
-
-}
diff --git a/src/main/java/org/onosproject/segmentrouting/grouphandler/DefaultGroupHandler.java b/src/main/java/org/onosproject/segmentrouting/grouphandler/DefaultGroupHandler.java
index fec8b39..031c805 100644
--- a/src/main/java/org/onosproject/segmentrouting/grouphandler/DefaultGroupHandler.java
+++ b/src/main/java/org/onosproject/segmentrouting/grouphandler/DefaultGroupHandler.java
@@ -17,6 +17,8 @@
 
 
 import com.google.common.collect.Iterables;
+import com.google.common.collect.Sets;
+
 import org.apache.commons.lang3.RandomUtils;
 import org.onlab.packet.MacAddress;
 import org.onlab.packet.MplsLabel;
@@ -50,6 +52,7 @@
 import java.util.ArrayList;
 import java.util.Collection;
 import java.util.Collections;
+import java.util.HashMap;
 import java.util.HashSet;
 import java.util.List;
 import java.util.Map;
@@ -83,13 +86,18 @@
     protected MacAddress nodeMacAddr = null;
     protected LinkService linkService;
     protected FlowObjectiveService flowObjectiveService;
-    // local store for neighbor-device-ids and the set of ports on this device
-    // that connect to the same neighbor
+    /**
+     * local store for neighbor-device-ids and the set of ports on this device
+     * that connect to the same neighbor.
+     */
     protected ConcurrentHashMap<DeviceId, Set<PortNumber>> devicePortMap =
             new ConcurrentHashMap<>();
-    // local store for ports on this device connected to neighbor-device-id
+    /**
+     *  local store for ports on this device connected to neighbor-device-id.
+     */
     protected ConcurrentHashMap<PortNumber, DeviceId> portDeviceMap =
             new ConcurrentHashMap<>();
+
     // distributed store for (device+neighborset) mapped to next-id
     protected EventuallyConsistentMap<NeighborSetNextObjectiveStoreKey, Integer>
             nsNextObjStore = null;
@@ -142,9 +150,7 @@
     }
 
     /**
-     * Creates a group handler object based on the type of device. If device is
-     * of edge type it returns edge group handler, else it returns transit group
-     * handler.
+     * Creates a group handler object.
      *
      * @param deviceId device identifier
      * @param appId application identifier
@@ -163,103 +169,105 @@
                                           FlowObjectiveService flowObjService,
                                           SegmentRoutingManager srManager)
                                                   throws DeviceConfigNotFoundException {
-        // handle possible exception in the caller
-        if (config.isEdgeDevice(deviceId)) {
-            return new DefaultEdgeGroupHandler(deviceId, appId, config,
-                                               linkService,
-                                               flowObjService,
-                                               srManager
-                                               );
-        } else {
-            return new DefaultTransitGroupHandler(deviceId, appId, config,
-                                                  linkService,
-                                                  flowObjService,
-                                                  srManager);
-        }
+        return new DefaultGroupHandler(deviceId, appId, config,
+                                       linkService,
+                                       flowObjService,
+                                       srManager);
     }
 
     /**
-     * Creates the auto created groups for this device based on the current
-     * snapshot of the topology.
+     * Updates local stores for link-src device/port to neighbor (link-dst).
+     *
+     * @param link the infrastructure link
      */
-    // Empty implementations to be overridden by derived classes
-    public void createGroups() {
-    }
+    public void portUpForLink(Link link) {
+       if (!link.src().deviceId().equals(deviceId)) {
+           log.warn("linkUp: deviceId{} doesn't match with link src {}",
+                    deviceId, link.src().deviceId());
+           return;
+       }
+
+       log.info("* portUpForLink: Device {} linkUp at local port {} to "
+               + "neighbor {}", deviceId, link.src().port(), link.dst().deviceId());
+       // ensure local state is updated even if linkup is aborted later on
+       addNeighborAtPort(link.dst().deviceId(),
+                         link.src().port());
+   }
+
+   /**
+    * Updates local stores for port that has gone down.
+    *
+    * @param port port number that has gone down
+    */
+   public void portDown(PortNumber port) {
+       if (portDeviceMap.get(port) == null) {
+           log.warn("portDown: unknown port");
+           return;
+       }
+
+       log.debug("Device {} portDown {} to neighbor {}", deviceId, port,
+                 portDeviceMap.get(port));
+       devicePortMap.get(portDeviceMap.get(port)).remove(port);
+       portDeviceMap.remove(port);
+   }
 
     /**
-     * Performs group creation or update procedures when a new link is
-     * discovered on this device.
+     * Checks all groups in the src-device of link for neighbor sets that include
+     * the dst-device of link, and edits the hash groups according to link up
+     * or down. Should only be called by the master instance of the src-switch
+     * of link. Typically used when there are no route-path changes due to the
+     * link up or down, as the ECMPspg does not change.
      *
-     * @param newLink new neighbor link
-     * @param isMaster true if local instance is the master for src-device of link
-     *
+     * @param link the infrastructure link that has gone down or come up
+     * @param linkDown true if link has gone down
+     * @param firstTime true if link has come up for the first time i.e a link
+     *                  not seen-before
      */
-    public void linkUp(Link newLink, boolean isMaster) {
-
-        if (newLink.type() != Link.Type.DIRECT) {
-            // NOTE: A DIRECT link might be transiently marked as INDIRECT
-            //       if BDDP is received before LLDP. We can safely ignore that
-            //       until the LLDP is received and the link is marked as DIRECT.
-            log.info("Ignore link {}->{}. Link type is {} instead of DIRECT.",
-                    newLink.src(), newLink.dst(), newLink.type());
-            return;
-        }
-
-        if (!newLink.src().deviceId().equals(deviceId)) {
-            log.warn("linkUp: deviceId{} doesn't match with link src {}",
-                     deviceId, newLink.src().deviceId());
-            return;
-        }
-
-        log.info("* LinkUP: Device {} linkUp at local port {} to neighbor {}", deviceId,
-                 newLink.src().port(), newLink.dst().deviceId());
-        // ensure local state is updated even if linkup is aborted later on
-        addNeighborAtPort(newLink.dst().deviceId(),
-                          newLink.src().port());
-
+    public void retryHash(Link link, boolean linkDown, boolean firstTime) {
         MacAddress dstMac;
         try {
-            dstMac = deviceConfig.getDeviceMac(newLink.dst().deviceId());
+            dstMac = deviceConfig.getDeviceMac(link.dst().deviceId());
         } catch (DeviceConfigNotFoundException e) {
-            log.warn(e.getMessage() + " Aborting linkUp.");
+            log.warn(e.getMessage() + " Aborting retryHash.");
             return;
         }
-
-        /*if (devicePortMap.get(newLink.dst().deviceId()) == null) {
-            // New Neighbor
-            newNeighbor(newLink);
-        } else {
-            // Old Neighbor
-            newPortToExistingNeighbor(newLink);
-        }*/
+        // find all the neighborSets related to link
         Set<NeighborSet> nsSet = nsNextObjStore.keySet()
                 .stream()
                 .filter((nsStoreEntry) -> (nsStoreEntry.deviceId().equals(deviceId)))
                 .map((nsStoreEntry) -> (nsStoreEntry.neighborSet()))
                 .filter((ns) -> (ns.getDeviceIds()
-                        .contains(newLink.dst().deviceId())))
+                        .contains(link.dst().deviceId())))
                 .collect(Collectors.toSet());
-        log.debug("linkUp: nsNextObjStore contents for device {}: {}",
-                  deviceId, nsSet);
+        log.debug("retryHash: nsNextObjStore contents for linkSrc {} -> linkDst {}: {}",
+                  deviceId, link.dst().deviceId(), nsSet);
+
         for (NeighborSet ns : nsSet) {
             Integer nextId = nsNextObjStore.
                     get(new NeighborSetNextObjectiveStoreKey(deviceId, ns));
-            if (nextId != null && isMaster) {
-                addToHashedNextObjective(newLink.src().port(), dstMac, ns,
-                                         nextId, false);
-                // some links may have come up before the next-objective was created
-                // we take this opportunity to ensure other ports to same next-hop-dst
-                // are part of the hash group (see CORD-1180). Duplicate additions
-                // to the same hash group are avoided by the driver.
-                for (PortNumber p : devicePortMap.get(newLink.dst().deviceId())) {
-                    if (p.equals(newLink.src().port())) {
-                        continue;
-                    }
-                    addToHashedNextObjective(p, dstMac, ns, nextId, false);
-                }
-            } else if (isMaster) {
-                log.warn("linkUp in device {}, but global store has no record "
+            if (nextId == null) {
+                log.warn("retryHash in device {}, but global store has no record "
                         + "for neighbor-set {}", deviceId, ns);
+                continue;
+            }
+            if (!linkDown) {
+                addToHashedNextObjective(link.src().port(), dstMac, ns,
+                                         nextId, false);
+                if (firstTime) {
+                    // some links may have come up before the next-objective was created
+                    // we take this opportunity to ensure other ports to same next-hop-dst
+                    // are part of the hash group (see CORD-1180). Duplicate additions
+                    // to the same hash group are avoided by the driver.
+                    for (PortNumber p : devicePortMap.get(link.dst().deviceId())) {
+                        if (p.equals(link.src().port())) {
+                            continue;
+                        }
+                        addToHashedNextObjective(p, dstMac, ns, nextId, false);
+                    }
+                }
+            } else {
+                removeFromHashedNextObjective(link.src().port(), dstMac, ns,
+                                              nextId);
             }
         }
 
@@ -269,12 +277,23 @@
         // not synced up with this instance yet. Thus we perform this check again
         // after a delay (see CORD-1180). Duplicate additions to the same hash group
         // are avoided by the driver.
-        if (isMaster) {
-            executorService.schedule(new RetryHashBkts(newLink, dstMac),
+        if (!linkDown && firstTime) {
+            executorService.schedule(new RetryHashBkts(link, dstMac),
                                      RETRY_INTERVAL_SEC, TimeUnit.SECONDS);
         }
     }
 
+    /**
+     * Makes a call to the FlowObjective service to add a single bucket to
+     * a hashed group.
+     *
+     * @param outport port to add to hash group
+     * @param dstMac destination mac address of next-hop
+     * @param ns neighbor set representing next-hops and destination switch
+     * @param nextId id for next-objective to which the bucket will be added
+     * @param retry indicates if this method is being called on a retry attempt
+     *              at adding a bucket to the group
+     */
     private void addToHashedNextObjective(PortNumber outport, MacAddress dstMac,
             NeighborSet ns, Integer nextId, boolean retry) {
         // Create the new bucket to be updated
@@ -301,97 +320,178 @@
                 .withMeta(metabuilder.build())
                 .fromApp(appId);
         log.info("{} in device {}: Adding Bucket with Port {} to next object id {}",
-                 (retry) ? "**retry" : "**linkup",
+                 (retry) ? "retry-addToHash" : "addToHash",
                          deviceId, outport, nextId);
 
         ObjectiveContext context = new DefaultObjectiveContext(
-                (objective) -> log.debug("LinkUp addedTo NextObj {} on {}",
-                        nextId, deviceId),
+                (objective) -> log.debug("{} addedTo NextObj {} on {}",
+                                         (retry) ? "retry-addToHash" : "addToHash",
+                                         nextId, deviceId),
                 (objective, error) ->
-                        log.warn("LinkUp failed to addTo NextObj {} on {}: {}",
-                                nextId, deviceId, error));
+                        log.warn("{} failed to addTo NextObj {} on {}: {}",
+                                 (retry) ? "retry-addToHash" : "addToHash",
+                                 nextId, deviceId, error));
         NextObjective nextObjective = nextObjBuilder.addToExisting(context);
         flowObjectiveService.next(deviceId, nextObjective);
     }
 
     /**
-     * Performs hash group recovery procedures when a switch-to-switch
-     * port goes down on this device.
+    * Makes a call to the FlowObjective service to remove a single bucket from
+    * a hashed group.
+    *
+    * @param port port to remove from hash group
+    * @param dstMac destination mac address of next-hop
+    * @param ns neighbor set representing next-hops and destination switch
+    * @param nextId id for next-objective from which the bucket will be removed
+    */
+   private void removeFromHashedNextObjective(PortNumber port, MacAddress dstMac,
+                                              NeighborSet ns, Integer nextId) {
+       // Create the bucket to be removed
+       TrafficTreatment.Builder tBuilder = DefaultTrafficTreatment
+               .builder();
+       tBuilder.setOutput(port)
+           .setEthDst(dstMac)
+           .setEthSrc(nodeMacAddr);
+       if (ns.getEdgeLabel() != NeighborSet.NO_EDGE_LABEL) {
+           tBuilder.pushMpls()
+               .copyTtlOut()
+               .setMpls(MplsLabel.mplsLabel(ns.getEdgeLabel()));
+       }
+       log.info("{} in device {}: Removing Bucket with Port {} to next object id {}",
+                "removeFromHash", deviceId, port, nextId);
+       NextObjective.Builder nextObjBuilder = DefaultNextObjective
+               .builder()
+               .withType(NextObjective.Type.HASHED) //same as original
+               .withId(nextId)
+               .fromApp(appId)
+               .addTreatment(tBuilder.build());
+       ObjectiveContext context = new DefaultObjectiveContext(
+           (objective) -> log.debug("port {} removedFrom NextObj {} on {}",
+                                    port, nextId, deviceId),
+           (objective, error) ->
+           log.warn("port {} failed to removeFrom NextObj {} on {}: {}",
+                    port, nextId, deviceId, error));
+       NextObjective nextObjective = nextObjBuilder.
+               removeFromExisting(context);
+
+       flowObjectiveService.next(deviceId, nextObjective);
+   }
+
+    /**
+     * Checks all the hash-groups in the target-switch meant for the destination
+     * switch, and either adds or removes buckets to make the neighbor-set
+     * match the given next-hops. Typically called by the master instance of the
+     * destination switch, which may be different from the master instance of the
+     * target switch where hash-group changes are made.
      *
-     * @param port port number that has gone down
-     * @param isMaster true if local instance is the master
+     * @param targetSw the switch in which the hash groups will be edited
+     * @param nextHops the current next hops for the target switch to reach
+     *                  the dest sw
+     * @param destSw  the destination switch
+     * @param revoke true if hash groups need to remove buckets from the
+     *                          the groups to match the current next hops
+     * @return true if calls are made to edit buckets, or if no edits are required
      */
-    public void portDown(PortNumber port, boolean isMaster) {
-        if (portDeviceMap.get(port) == null) {
-            log.warn("portDown: unknown port");
-            return;
-        }
+    public boolean fixHashGroups(DeviceId targetSw, Set<DeviceId> nextHops,
+                                 DeviceId destSw, boolean revoke) {
+        // temporary storage of keys to be updated
+        Map<NeighborSetNextObjectiveStoreKey, Set<DeviceId>> tempStore =
+                new HashMap<>();
+        boolean foundNextObjective = false;
 
-        MacAddress dstMac;
-        try {
-            dstMac = deviceConfig.getDeviceMac(portDeviceMap.get(port));
-        } catch (DeviceConfigNotFoundException e) {
-            log.warn(e.getMessage() + " Aborting portDown.");
-            return;
-        }
+        // retrieve hash-groups meant for destSw, which have neighborSets
+        // with different neighbors than the given next-hops
+        for (NeighborSetNextObjectiveStoreKey nskey : nsNextObjStore.keySet()) {
+            if (!nskey.deviceId().equals(targetSw) ||
+                    !nskey.neighborSet().getDestinationSw().equals(destSw)) {
+                continue;
+            }
+            foundNextObjective = true;
+            Set<DeviceId> currNeighbors = nskey.neighborSet().getDeviceIds();
+            Integer nextId = nsNextObjStore.get(nskey);
 
-        log.debug("Device {} portDown {} to neighbor {}", deviceId, port,
-                  portDeviceMap.get(port));
-        /*Set<NeighborSet> nsSet = computeImpactedNeighborsetForPortEvent(portDeviceMap
-                                                                                .get(port),
-                                                                        devicePortMap
-                                                                                .keySet());*/
-        Set<NeighborSet> nsSet = nsNextObjStore.keySet()
-                .stream()
-                .filter((nsStoreEntry) -> (nsStoreEntry.deviceId().equals(deviceId)))
-                .map((nsStoreEntry) -> (nsStoreEntry.neighborSet()))
-                .filter((ns) -> (ns.getDeviceIds()
-                        .contains(portDeviceMap.get(port))))
-                .collect(Collectors.toSet());
-        log.debug("portDown: nsNextObjStore contents for device {}:{}",
-                  deviceId, nsSet);
-        for (NeighborSet ns : nsSet) {
-            NeighborSetNextObjectiveStoreKey nsStoreKey =
-                    new NeighborSetNextObjectiveStoreKey(deviceId, ns);
-            Integer nextId = nsNextObjStore.get(nsStoreKey);
-            if (nextId != null && isMaster) {
-                log.info("**portDown in device {}: Removing Bucket "
-                        + "with Port {} to next object id {}",
-                        deviceId,
-                        port,
-                        nextId);
-                // Create the bucket to be removed
-                TrafficTreatment.Builder tBuilder = DefaultTrafficTreatment
-                        .builder();
-                tBuilder.setOutput(port)
-                    .setEthDst(dstMac)
-                    .setEthSrc(nodeMacAddr);
-                if (ns.getEdgeLabel() != NeighborSet.NO_EDGE_LABEL) {
-                    tBuilder.pushMpls()
-                        .copyTtlOut()
-                        .setMpls(MplsLabel.mplsLabel(ns.getEdgeLabel()));
+            Set<DeviceId> diff;
+            if (revoke) {
+                diff = Sets.difference(currNeighbors, nextHops);
+                log.debug("targetSw:{} -> dstSw:{} in nextId:{} has current next "
+                        + "hops:{} ..removing {}", targetSw, destSw, nextId,
+                        currNeighbors, diff);
+            } else {
+                diff = Sets.difference(nextHops, currNeighbors);
+                log.debug("targetSw:{} -> dstSw:{} in nextId:{} has current next "
+                        + "hops:{} ..adding {}", targetSw, destSw, nextId,
+                        currNeighbors, diff);
+            }
+            for (DeviceId neighbor : diff) {
+                MacAddress dstMac;
+                try {
+                    dstMac = deviceConfig.getDeviceMac(neighbor);
+                } catch (DeviceConfigNotFoundException e) {
+                    log.warn(e.getMessage() + " Aborting fixHashGroup for nextId:"
+                            + nskey);
+                    return false;
                 }
-                NextObjective.Builder nextObjBuilder = DefaultNextObjective
-                        .builder()
-                        .withType(NextObjective.Type.HASHED) //same as original
-                        .withId(nextId)
-                        .fromApp(appId)
-                        .addTreatment(tBuilder.build());
-                ObjectiveContext context = new DefaultObjectiveContext(
-                    (objective) -> log.debug("portDown removedFrom NextObj {} on {}",
-                                             nextId, deviceId),
-                    (objective, error) ->
-                    log.warn("portDown failed to removeFrom NextObj {} on {}: {}",
-                             nextId, deviceId, error));
-                NextObjective nextObjective = nextObjBuilder.
-                        removeFromExisting(context);
-
-                flowObjectiveService.next(deviceId, nextObjective);
+                if (devicePortMap.get(neighbor) == null ||
+                        devicePortMap.get(neighbor).isEmpty()) {
+                    log.warn("No ports found in dev:{} for neighbor:{} .. cannot "
+                            + "fix hash group for nextId: {}",
+                             deviceId, neighbor, nextId);
+                    return false;
+                }
+                if (revoke) {
+                    for (PortNumber port : devicePortMap.get(neighbor)) {
+                        log.info("fixHashGroup in device {}: Removing Bucket "
+                                + "with Port {} to next object id {}",
+                                deviceId, port, nextId);
+                        removeFromHashedNextObjective(port, dstMac,
+                                                      nskey.neighborSet(),
+                                                      nextId);
+                    }
+                    // to update neighbor set with changes made
+                    tempStore.put(nskey, Sets.difference(currNeighbors, diff));
+                } else {
+                    for (PortNumber port : devicePortMap.get(neighbor)) {
+                        log.info("fixHashGroup in device {}: Adding Bucket "
+                                + "with Port {} to next object id {}",
+                                deviceId, port, nextId);
+                        addToHashedNextObjective(port, dstMac,
+                                                 nskey.neighborSet(),
+                                                 nextId, false);
+                    }
+                    // to update neighbor set with changes made
+                    tempStore.put(nskey, Sets.union(currNeighbors, diff));
+                }
             }
         }
 
-        devicePortMap.get(portDeviceMap.get(port)).remove(port);
-        portDeviceMap.remove(port);
+        if (!foundNextObjective) {
+            log.debug("Cannot find any nextObjectives for route targetSw:{} "
+                    + "-> dstSw:{}", targetSw, destSw);
+            return true; // nothing to do, return true so ECMPspg is updated
+        }
+
+        // update the nsNextObjectiveStore with new neighborSets to nextId mappings
+        for (NeighborSetNextObjectiveStoreKey oldkey : tempStore.keySet()) {
+            Integer nextId = nsNextObjStore.get(oldkey);
+            if (nextId == null) {
+                continue;
+            }
+            Set<DeviceId> newNeighbors = tempStore.get(oldkey);
+            NeighborSet newNs = new NeighborSet(newNeighbors,
+                                                oldkey.neighborSet().mplsSet(),
+                                                oldkey.neighborSet().getEdgeLabel(),
+                                                oldkey.neighborSet().getDestinationSw());
+            NeighborSetNextObjectiveStoreKey newkey =
+                    new NeighborSetNextObjectiveStoreKey(deviceId, newNs);
+            log.debug("Updating nsNextObjStore: oldKey:{} -> newKey:{} :: nextId:{}",
+                      oldkey, newkey, nextId);
+            synchronized (nsNextObjStore) {
+                nsNextObjStore.remove(oldkey);
+                nsNextObjStore.put(newkey, nextId);
+            }
+        }
+
+        return true;
     }
 
     /**
@@ -573,21 +673,6 @@
         return true;
     }
 
-    // Empty implementation
-    protected void newNeighbor(Link newLink) {
-    }
-
-    // Empty implementation
-    protected void newPortToExistingNeighbor(Link newLink) {
-    }
-
-    // Empty implementation
-    protected Set<NeighborSet>
-        computeImpactedNeighborsetForPortEvent(DeviceId impactedNeighbor,
-                                               Set<DeviceId> updatedNeighbors) {
-        return null;
-    }
-
     private void populateNeighborMaps() {
         Set<Link> outgoingLinks = linkService.getDeviceEgressLinks(deviceId);
         for (Link link : outgoingLinks) {
@@ -614,8 +699,8 @@
         // Update portToDevice database
         DeviceId prev = portDeviceMap.putIfAbsent(portToNeighbor, neighborId);
         if (prev != null) {
-            log.warn("Device: {} port: {} has neighbor: {}. NOT updating "
-                    + "to neighbor: {}", deviceId, portToNeighbor, prev, neighborId);
+            log.debug("Device: {} port: {} already has neighbor: {} ",
+                      deviceId, portToNeighbor, prev, neighborId);
         }
     }
 
diff --git a/src/main/java/org/onosproject/segmentrouting/grouphandler/DefaultTransitGroupHandler.java b/src/main/java/org/onosproject/segmentrouting/grouphandler/DefaultTransitGroupHandler.java
deleted file mode 100644
index cf6975c..0000000
--- a/src/main/java/org/onosproject/segmentrouting/grouphandler/DefaultTransitGroupHandler.java
+++ /dev/null
@@ -1,194 +0,0 @@
-/*
- * Copyright 2015-present Open Networking Laboratory
- *
- * Licensed under the Apache License, Version 2.0 (the "License");
- * you may not use this file except in compliance with the License.
- * You may obtain a copy of the License at
- *
- *     http://www.apache.org/licenses/LICENSE-2.0
- *
- * Unless required by applicable law or agreed to in writing, software
- * distributed under the License is distributed on an "AS IS" BASIS,
- * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
- * See the License for the specific language governing permissions and
- * limitations under the License.
- */
-package org.onosproject.segmentrouting.grouphandler;
-
-import org.onosproject.core.ApplicationId;
-import org.onosproject.net.DeviceId;
-import org.onosproject.net.Link;
-import org.onosproject.net.flowobjective.FlowObjectiveService;
-import org.onosproject.net.link.LinkService;
-import org.onosproject.segmentrouting.SegmentRoutingManager;
-import org.onosproject.segmentrouting.config.DeviceConfigNotFoundException;
-import org.onosproject.segmentrouting.config.DeviceProperties;
-
-import java.util.HashSet;
-import java.util.Set;
-
-/**
- * Default ECMP group handler creation module for a transit device.
- * This component creates a set of ECMP groups for every neighbor
- * that this device is connected to.
- * For example, consider a network of 4 devices: D0 (Segment ID: 100),
- * D1 (Segment ID: 101), D2 (Segment ID: 102) and D3 (Segment ID: 103),
- * where D0 and D3 are edge devices and D1 and D2 are transit devices.
- * Assume transit device D1 is connected to 2 neighbors (D0 and D3 ).
- * The following groups will be created in D1:
- * 1) all ports to D0 + with no label push,
- * 2) all ports to D3 + with no label push,
- */
-public class DefaultTransitGroupHandler extends DefaultGroupHandler {
-    protected DefaultTransitGroupHandler(DeviceId deviceId,
-                                  ApplicationId appId,
-                                  DeviceProperties config,
-                                  LinkService linkService,
-                                  FlowObjectiveService flowObjService,
-                                  SegmentRoutingManager srManager) {
-        super(deviceId, appId, config, linkService, flowObjService, srManager);
-    }
-
-    @Override
-    public void createGroups() {
-        Set<DeviceId> neighbors = devicePortMap.keySet();
-        if (neighbors == null || neighbors.isEmpty()) {
-            return;
-        }
-
-        // Create all possible Neighbor sets from this router
-        // NOTE: Avoid any pairings of edge routers only
-        Set<Set<DeviceId>> sets = getPowerSetOfNeighbors(neighbors);
-        sets = filterEdgeRouterOnlyPairings(sets);
-        log.debug("createGroupsAtTransitRouter: The size of neighbor powerset "
-                + "for sw {} is {}", deviceId, sets.size());
-        Set<NeighborSet> nsSet = new HashSet<>();
-        for (Set<DeviceId> combo : sets) {
-            if (combo.isEmpty()) {
-                continue;
-            }
-             // For these NeighborSet isMpls is meaningless.
-            NeighborSet ns = new NeighborSet(combo, false);
-            log.debug("createGroupsAtTransitRouter: sw {} combo {} ns {}",
-                      deviceId, combo, ns);
-            nsSet.add(ns);
-        }
-        log.debug("createGroupsAtTransitRouter: The neighborset with label "
-                + "for sw {} is {}", deviceId, nsSet);
-
-        //createGroupsFromNeighborsets(nsSet);
-    }
-
-    @Override
-    protected void newNeighbor(Link newNeighborLink) {
-        log.debug("New Neighbor: Updating groups for "
-                + "transit device {}", deviceId);
-        // Recompute neighbor power set
-        addNeighborAtPort(newNeighborLink.dst().deviceId(),
-                          newNeighborLink.src().port());
-        // Compute new neighbor sets due to the addition of new neighbor
-        Set<NeighborSet> nsSet = computeImpactedNeighborsetForPortEvent(
-                                             newNeighborLink.dst().deviceId(),
-                                             devicePortMap.keySet());
-        //createGroupsFromNeighborsets(nsSet);
-    }
-
-    @Override
-    protected void newPortToExistingNeighbor(Link newNeighborLink) {
-        /*log.debug("New port to existing neighbor: Updating "
-                + "groups for transit device {}", deviceId);
-        addNeighborAtPort(newNeighborLink.dst().deviceId(),
-                          newNeighborLink.src().port());
-        Set<NeighborSet> nsSet = computeImpactedNeighborsetForPortEvent(
-                                              newNeighborLink.dst().deviceId(),
-                                              devicePortMap.keySet());
-        for (NeighborSet ns : nsSet) {
-            // Create the new bucket to be updated
-            TrafficTreatment.Builder tBuilder =
-                    DefaultTrafficTreatment.builder();
-            tBuilder.setOutput(newNeighborLink.src().port())
-                    .setEthDst(deviceConfig.getDeviceMac(
-                          newNeighborLink.dst().deviceId()))
-                    .setEthSrc(nodeMacAddr);
-            if (ns.getEdgeLabel() != NeighborSet.NO_EDGE_LABEL) {
-                tBuilder.pushMpls()
-                        .setMpls(MplsLabel.
-                                 mplsLabel(ns.getEdgeLabel()));
-            }
-
-
-            Integer nextId = deviceNextObjectiveIds.get(ns);
-            if (nextId != null) {
-                NextObjective.Builder nextObjBuilder = DefaultNextObjective
-                        .builder().withId(nextId)
-                        .withType(NextObjective.Type.HASHED).fromApp(appId);
-
-                nextObjBuilder.addTreatment(tBuilder.build());
-
-                NextObjective nextObjective = nextObjBuilder.add();
-                flowObjectiveService.next(deviceId, nextObjective);
-            }
-        }*/
-    }
-
-    @Override
-    protected Set<NeighborSet> computeImpactedNeighborsetForPortEvent(
-                                            DeviceId impactedNeighbor,
-                                            Set<DeviceId> updatedNeighbors) {
-        Set<Set<DeviceId>> powerSet = getPowerSetOfNeighbors(updatedNeighbors);
-
-        Set<DeviceId> tmp = new HashSet<>();
-        tmp.addAll(updatedNeighbors);
-        tmp.remove(impactedNeighbor);
-        Set<Set<DeviceId>> tmpPowerSet = getPowerSetOfNeighbors(tmp);
-
-        // Compute the impacted neighbor sets
-        powerSet.removeAll(tmpPowerSet);
-
-        powerSet = filterEdgeRouterOnlyPairings(powerSet);
-        Set<NeighborSet> nsSet = new HashSet<>();
-        for (Set<DeviceId> combo : powerSet) {
-            if (combo.isEmpty()) {
-                continue;
-            }
-            // For these NeighborSet isMpls is meaningless.
-            NeighborSet ns = new NeighborSet(combo, false);
-            log.debug("createGroupsAtTransitRouter: sw {} combo {} ns {}",
-                      deviceId, combo, ns);
-            nsSet.add(ns);
-        }
-        log.debug("computeImpactedNeighborsetForPortEvent: The neighborset with label "
-                + "for sw {} is {}", deviceId, nsSet);
-
-        return nsSet;
-    }
-
-    private Set<Set<DeviceId>> filterEdgeRouterOnlyPairings(Set<Set<DeviceId>> sets) {
-        Set<Set<DeviceId>> fiteredSets = new HashSet<>();
-        for (Set<DeviceId> deviceSubSet : sets) {
-            if (deviceSubSet.size() > 1) {
-                boolean avoidEdgeRouterPairing = true;
-                for (DeviceId device : deviceSubSet) {
-                    boolean isEdge;
-                    try {
-                        isEdge = deviceConfig.isEdgeDevice(device);
-                    } catch (DeviceConfigNotFoundException e) {
-                        log.warn(e.getMessage() + " Skipping filterEdgeRouterOnlyPairings on this device.");
-                        continue;
-                    }
-
-                    if (!isEdge) {
-                        avoidEdgeRouterPairing = false;
-                        break;
-                    }
-                }
-                if (!avoidEdgeRouterPairing) {
-                    fiteredSets.add(deviceSubSet);
-                }
-            } else {
-                fiteredSets.add(deviceSubSet);
-            }
-        }
-        return fiteredSets;
-    }
-}
diff --git a/src/main/java/org/onosproject/segmentrouting/grouphandler/NeighborSet.java b/src/main/java/org/onosproject/segmentrouting/grouphandler/NeighborSet.java
index 06f7861..b2f4fd1 100644
--- a/src/main/java/org/onosproject/segmentrouting/grouphandler/NeighborSet.java
+++ b/src/main/java/org/onosproject/segmentrouting/grouphandler/NeighborSet.java
@@ -17,6 +17,7 @@
 package org.onosproject.segmentrouting.grouphandler;
 
 import org.onosproject.net.DeviceId;
+import org.slf4j.Logger;
 
 import java.util.HashSet;
 import java.util.Objects;
@@ -24,47 +25,56 @@
 
 import static com.google.common.base.MoreObjects.toStringHelper;
 import static com.google.common.base.Preconditions.checkNotNull;
+import static org.slf4j.LoggerFactory.getLogger;
 
 /**
  * Representation of a set of neighbor switch dpids along with edge node
- * label. Meant to be used as a lookup-key in a hash-map to retrieve an
- * ECMP-group that hashes packets to a set of ports connecting to the
- * neighbors in this set.
+ * label and a destination switch. Meant to be used as a lookup-key in a hash-map
+ * to retrieve an ECMP-group that hashes packets to a set of ports connecting to
+ * the neighbors in this set towards a specific destination switch.
  */
 public class NeighborSet {
     private final Set<DeviceId> neighbors;
     private final int edgeLabel;
     public static final int NO_EDGE_LABEL = -1;
     private boolean mplsSet;
+    // the destination switch towards which the neighbors are the next-hops.
+    private final DeviceId dstSw;
+    protected static final Logger log = getLogger(NeighborSet.class);
 
     /**
      * Constructor with set of neighbors. Edge label is
      * default to -1.
      *
-     * @param neighbors set of neighbors to be part of neighbor set
+     * @param neighbors set of neighbors representing the next-hops
      * @param isMplsSet indicates if it is a mpls neighbor set
+     * @param dstSw the destination switch
      */
-    public NeighborSet(Set<DeviceId> neighbors, boolean isMplsSet) {
+    public NeighborSet(Set<DeviceId> neighbors, boolean isMplsSet, DeviceId dstSw) {
         checkNotNull(neighbors);
         this.edgeLabel = NO_EDGE_LABEL;
         this.neighbors = new HashSet<>();
         this.neighbors.addAll(neighbors);
         this.mplsSet = isMplsSet;
+        this.dstSw = dstSw;
     }
 
     /**
      * Constructor with set of neighbors and edge label.
      *
-     * @param neighbors set of neighbors to be part of neighbor set
+     * @param neighbors set of neighbors representing the next-hops
      * @param isMplsSet indicates if it is a mpls neighbor set
      * @param edgeLabel label to be pushed as part of group operation
+     * @param dstSw the destination switch
      */
-    public NeighborSet(Set<DeviceId> neighbors, boolean isMplsSet, int edgeLabel) {
+    public NeighborSet(Set<DeviceId> neighbors, boolean isMplsSet,
+                       int edgeLabel, DeviceId dstSw) {
         checkNotNull(neighbors);
         this.edgeLabel = edgeLabel;
         this.neighbors = new HashSet<>();
         this.neighbors.addAll(neighbors);
         this.mplsSet = isMplsSet;
+        this.dstSw = dstSw;
     }
 
     /**
@@ -74,35 +84,39 @@
         this.edgeLabel = NO_EDGE_LABEL;
         this.neighbors = new HashSet<>();
         this.mplsSet = true;
+        this.dstSw = DeviceId.NONE;
     }
 
     /**
      * Factory method for NeighborSet hierarchy.
      *
      * @param random the expected behavior.
-     * @param neighbors the set of neighbors to be part of neighbor set
+     * @param neighbors the set of neighbors representing the next-hops
      * @param isMplsSet indicates if it is a mpls neighbor set
+     * @param dstSw the destination switch
      * @return the neighbor set object.
      */
-    public static NeighborSet neighborSet(boolean random, Set<DeviceId> neighbors, boolean isMplsSet) {
-        return random ?
-                new RandomNeighborSet(neighbors) :
-                new NeighborSet(neighbors, isMplsSet);
+    public static NeighborSet neighborSet(boolean random, Set<DeviceId> neighbors,
+                                          boolean isMplsSet, DeviceId dstSw) {
+        return random ? new RandomNeighborSet(neighbors, dstSw)
+                      : new NeighborSet(neighbors, isMplsSet, dstSw);
     }
 
     /**
      * Factory method for NeighborSet hierarchy.
      *
      * @param random the expected behavior.
-     * @param neighbors the set of neighbors to be part of neighbor set
+     * @param neighbors the set of neighbors representing the next-hops
      * @param isMplsSet indicates if it is a mpls neighbor set
      * @param edgeLabel label to be pushed as part of group operation
+     * @param dstSw the destination switch
      * @return the neighbor set object
      */
-    public static NeighborSet neighborSet(boolean random, Set<DeviceId> neighbors, boolean isMplsSet, int edgeLabel) {
-        return random ?
-                new RandomNeighborSet(neighbors, edgeLabel) :
-                new NeighborSet(neighbors, isMplsSet, edgeLabel);
+    public static NeighborSet neighborSet(boolean random, Set<DeviceId> neighbors,
+                                          boolean isMplsSet, int edgeLabel,
+                                          DeviceId dstSw) {
+        return random ? new RandomNeighborSet(neighbors, edgeLabel, dstSw)
+                      : new NeighborSet(neighbors, isMplsSet, edgeLabel, dstSw);
     }
 
     /**
@@ -134,6 +148,15 @@
     }
 
     /**
+     * Gets the destination switch for this neighbor set.
+     *
+     * @return the destination switch id
+     */
+    public DeviceId getDestinationSw() {
+        return dstSw;
+    }
+
+    /**
      * Gets the first neighbor of the set. The default
      * implementation assure the first neighbor is the
      * first of the set. Subclasses can modify this.
@@ -165,22 +188,24 @@
         NeighborSet that = (NeighborSet) o;
         return (this.neighbors.containsAll(that.neighbors) &&
                 that.neighbors.containsAll(this.neighbors) &&
-                (this.edgeLabel == that.edgeLabel) &&
-                (this.mplsSet == that.mplsSet));
+                this.edgeLabel == that.edgeLabel &&
+                this.mplsSet == that.mplsSet &&
+                this.dstSw.equals(that.dstSw));
     }
 
     // The list of neighbor ids and label are used for comparison.
     @Override
     public int hashCode() {
-        return Objects.hash(neighbors, edgeLabel, mplsSet);
+        return Objects.hash(neighbors, edgeLabel, mplsSet, dstSw);
     }
 
     @Override
     public String toString() {
         return toStringHelper(this)
-                .add("Neighborset Sw", neighbors)
+                .add("Neighbors", neighbors)
                 .add("Label", edgeLabel)
                 .add("MplsSet", mplsSet)
+                .add("DstSw", dstSw)
                 .toString();
     }
 }
diff --git a/src/main/java/org/onosproject/segmentrouting/grouphandler/RandomNeighborSet.java b/src/main/java/org/onosproject/segmentrouting/grouphandler/RandomNeighborSet.java
index a56859f..594ef20 100644
--- a/src/main/java/org/onosproject/segmentrouting/grouphandler/RandomNeighborSet.java
+++ b/src/main/java/org/onosproject/segmentrouting/grouphandler/RandomNeighborSet.java
@@ -23,17 +23,18 @@
 import java.util.Set;
 
 /**
- * Extends its super classe modifying its internal behavior.
+ * Extends its super class modifying its internal behavior.
  * Pick a neighbor will pick a random neighbor.
  */
 public class RandomNeighborSet extends NeighborSet {
 
-    public RandomNeighborSet(Set<DeviceId> neighbors) {
-        super(neighbors, true);
+    public RandomNeighborSet(Set<DeviceId> neighbors, DeviceId dstSw) {
+        super(neighbors, true, dstSw);
     }
 
-    public RandomNeighborSet(Set<DeviceId> neighbors, int edgeLabel) {
-        super(neighbors, true, edgeLabel);
+    public RandomNeighborSet(Set<DeviceId> neighbors, int edgeLabel,
+                             DeviceId dstSw) {
+        super(neighbors, true, edgeLabel, dstSw);
     }
 
     public RandomNeighborSet() {
@@ -53,6 +54,7 @@
     @Override
     public String toString() {
         return " RandomNeighborset Sw: " + getDeviceIds()
-                + " and Label: " + getEdgeLabel();
+                + " and Label: " + getEdgeLabel()
+                + " for destination: " + getDestinationSw();
     }
 }
diff --git a/src/main/java/org/onosproject/segmentrouting/storekey/NeighborSetNextObjectiveStoreKey.java b/src/main/java/org/onosproject/segmentrouting/storekey/NeighborSetNextObjectiveStoreKey.java
index aa751d1..7695433 100644
--- a/src/main/java/org/onosproject/segmentrouting/storekey/NeighborSetNextObjectiveStoreKey.java
+++ b/src/main/java/org/onosproject/segmentrouting/storekey/NeighborSetNextObjectiveStoreKey.java
@@ -84,6 +84,6 @@
 
     @Override
     public String toString() {
-        return "Device: " + deviceId + " Neighborset: " + ns;
+        return "Device: " + deviceId + " " + ns;
     }
 }
diff --git a/src/main/resources/OSGI-INF/blueprint/shell-config.xml b/src/main/resources/OSGI-INF/blueprint/shell-config.xml
index 65b8608..b99bbcc 100644
--- a/src/main/resources/OSGI-INF/blueprint/shell-config.xml
+++ b/src/main/resources/OSGI-INF/blueprint/shell-config.xml
@@ -16,6 +16,7 @@
 <blueprint xmlns="http://www.osgi.org/xmlns/blueprint/v1.0.0">
 
     <command-bundle xmlns="http://karaf.apache.org/xmlns/shell/v1.1.0">
+        <!-- XXX revisit when we formally add policies
         <command>
             <action class="org.onosproject.segmentrouting.cli.TunnelListCommand"/>
         </command>
@@ -34,12 +35,19 @@
         <command>
             <action class="org.onosproject.segmentrouting.cli.TunnelRemoveCommand"/>
         </command>
+        -->
         <command>
             <action class="org.onosproject.segmentrouting.cli.RerouteNetworkCommand"/>
         </command>
         <command>
             <action class="org.onosproject.segmentrouting.cli.DeviceSubnetListCommand"/>
         </command>
+        <command>
+            <action class="org.onosproject.segmentrouting.cli.EcmpGraphCommand"/>
+        </command>
+        <command>
+            <action class="org.onosproject.segmentrouting.cli.NeighborSetCommand"/>
+        </command>
     </command-bundle>
 </blueprint>