In this commit:

   Bug fix when optimized SR re-routing fails, do full re-route
   Bug fix filtering objectives should be called for new device even if there is an existing grouphandler
   Bug fix NPE in ofdpa driver due to null check on the wrong variable
   New cli command for debugging flow-objectives for pending next-objectives
   Flow objective cli commands now start with "obj-"

Change-Id: I819f82d1d67769cb9fbbde60f099d29b8e7f7c9e

@@ -16,7 +16,7 @@
 package org.onosproject.pce.util;
 package org.onosproject.pce.util;
 
 
 import java.util.List;
 import java.util.List;
-
+import com.google.common.collect.ImmutableList;
 import org.onosproject.net.DeviceId;
 import org.onosproject.net.DeviceId;
 import org.onosproject.net.flowobjective.FilteringObjective;
 import org.onosproject.net.flowobjective.FilteringObjective;
 import org.onosproject.net.flowobjective.FlowObjectiveService;
 import org.onosproject.net.flowobjective.FlowObjectiveService;
@@ -60,6 +60,11 @@ public class FlowObjServiceAdapter implements FlowObjectiveService {
 
 
     @Override
     @Override
     public List<String> getNextMappings() {
     public List<String> getNextMappings() {
-        return null;
+        return ImmutableList.of();
+    }
+
+    @Override
+    public List<String> getPendingNexts() {
+        return ImmutableList.of();
     }
     }
 }
 }

@@ -170,6 +170,7 @@ public class DefaultRoutingHandler {
             log.trace("populateRoutingRulesForLinkStatusChange: "
             log.trace("populateRoutingRulesForLinkStatusChange: "
                     + "populationStatus is STARTED");
                     + "populationStatus is STARTED");
             populationStatus = Status.STARTED;
             populationStatus = Status.STARTED;
+            // optimized re-routing
             if (linkFail == null) {
             if (linkFail == null) {
                 // Compare all routes of existing ECMP SPG with the new ones
                 // Compare all routes of existing ECMP SPG with the new ones
                 routeChanges = computeRouteChange();
                 routeChanges = computeRouteChange();
@@ -178,6 +179,11 @@ public class DefaultRoutingHandler {
                 routeChanges = computeDamagedRoutes(linkFail);
                 routeChanges = computeDamagedRoutes(linkFail);
             }
             }
 
 
+            // null routeChanges indicates that full re-routing is required
+            if (routeChanges == null) {
+                return populateAllRoutingRules();
+            }
+
             if (routeChanges.isEmpty()) {
             if (routeChanges.isEmpty()) {
                 log.info("No route changes for the link status change");
                 log.info("No route changes for the link status change");
                 log.debug("populateRoutingRulesForLinkStatusChange: populationStatus is SUCCEEDED");
                 log.debug("populateRoutingRulesForLinkStatusChange: populationStatus is SUCCEEDED");
@@ -276,6 +282,15 @@ public class DefaultRoutingHandler {
         return true;
         return true;
     }
     }
 
 
+    /**
+     * Computes set of affected ECMP routes due to failed link. Assumes
+     * previous ecmp shortest-path graph exists for a switch in order to compute
+     * affected routes. If such a graph does not exist, the method returns null.
+     *
+     * @param linkFail the failed link
+     * @return the set of affected routes which may be empty if no routes were
+     *         affected, or null if no previous ecmp spg was found for comparison
+     */
     private Set<ArrayList<DeviceId>> computeDamagedRoutes(Link linkFail) {
     private Set<ArrayList<DeviceId>> computeDamagedRoutes(Link linkFail) {
 
 
         Set<ArrayList<DeviceId>> routes = new HashSet<>();
         Set<ArrayList<DeviceId>> routes = new HashSet<>();
@@ -284,20 +299,31 @@ public class DefaultRoutingHandler {
             log.debug("Computing the impacted routes for device {} due to link fail",
             log.debug("Computing the impacted routes for device {} due to link fail",
                       sw.id());
                       sw.id());
             if (!srManager.mastershipService.isLocalMaster(sw.id())) {
             if (!srManager.mastershipService.isLocalMaster(sw.id())) {
+                log.debug("No mastership for {} .. skipping route optimization",
+                          sw.id());
                 continue;
                 continue;
             }
             }
             EcmpShortestPathGraph ecmpSpg = currentEcmpSpgMap.get(sw.id());
             EcmpShortestPathGraph ecmpSpg = currentEcmpSpgMap.get(sw.id());
             if (ecmpSpg == null) {
             if (ecmpSpg == null) {
-                log.error("No existing ECMP graph for switch {}", sw.id());
-                continue;
+                log.warn("No existing ECMP graph for switch {}. Aborting optimized"
+                        + " rerouting and opting for full-reroute", sw.id());
+                return null;
             }
             }
             HashMap<Integer, HashMap<DeviceId, ArrayList<ArrayList<DeviceId>>>> switchVia =
             HashMap<Integer, HashMap<DeviceId, ArrayList<ArrayList<DeviceId>>>> switchVia =
                     ecmpSpg.getAllLearnedSwitchesAndVia();
                     ecmpSpg.getAllLearnedSwitchesAndVia();
             for (Integer itrIdx : switchVia.keySet()) {
             for (Integer itrIdx : switchVia.keySet()) {
+                log.trace("Iterindex# {}", itrIdx);
                 HashMap<DeviceId, ArrayList<ArrayList<DeviceId>>> swViaMap =
                 HashMap<DeviceId, ArrayList<ArrayList<DeviceId>>> swViaMap =
                         switchVia.get(itrIdx);
                         switchVia.get(itrIdx);
                 for (DeviceId targetSw : swViaMap.keySet()) {
                 for (DeviceId targetSw : swViaMap.keySet()) {
                     DeviceId destSw = sw.id();
                     DeviceId destSw = sw.id();
+                    if (log.isTraceEnabled()) {
+                        log.trace("TargetSwitch {} --> RootSwitch {}", targetSw, destSw);
+                        for (ArrayList<DeviceId> via : swViaMap.get(targetSw)) {
+                            log.trace(" Via:");
+                            via.forEach(e -> { log.trace("  {}", e); });
+                        }
+                    }
                     Set<ArrayList<DeviceId>> subLinks =
                     Set<ArrayList<DeviceId>> subLinks =
                             computeLinks(targetSw, destSw, swViaMap);
                             computeLinks(targetSw, destSw, swViaMap);
                     for (ArrayList<DeviceId> alink: subLinks) {
                     for (ArrayList<DeviceId> alink: subLinks) {
@@ -327,20 +353,18 @@ public class DefaultRoutingHandler {
         Set<ArrayList<DeviceId>> routes = new HashSet<>();
         Set<ArrayList<DeviceId>> routes = new HashSet<>();
 
 
         for (Device sw : srManager.deviceService.getDevices()) {
         for (Device sw : srManager.deviceService.getDevices()) {
-            log.debug("Computing the impacted routes for device {}",
-                      sw.id());
+            log.debug("Computing the impacted routes for device {}", sw.id());
             if (!srManager.mastershipService.isLocalMaster(sw.id())) {
             if (!srManager.mastershipService.isLocalMaster(sw.id())) {
-                log.debug("No mastership for {} and skip route optimization",
+                log.debug("No mastership for {} ... skipping route optimization",
                           sw.id());
                           sw.id());
                 continue;
                 continue;
             }
             }
-
-            log.trace("link of {} - ", sw.id());
-            for (Link link: srManager.linkService.getDeviceLinks(sw.id())) {
-                log.trace("{} -> {} ", link.src().deviceId(), link.dst().deviceId());
+            if (log.isTraceEnabled()) {
+                log.trace("link of {} - ", sw.id());
+                for (Link link: srManager.linkService.getDeviceLinks(sw.id())) {
+                    log.trace("{} -> {} ", link.src().deviceId(), link.dst().deviceId());
+                }
             }
             }
-
-            log.debug("Checking route change for switch {}", sw.id());
             EcmpShortestPathGraph ecmpSpg = currentEcmpSpgMap.get(sw.id());
             EcmpShortestPathGraph ecmpSpg = currentEcmpSpgMap.get(sw.id());
             if (ecmpSpg == null) {
             if (ecmpSpg == null) {
                 log.debug("No existing ECMP graph for device {}", sw.id());
                 log.debug("No existing ECMP graph for device {}", sw.id());
@@ -363,7 +387,7 @@ public class DefaultRoutingHandler {
                     ArrayList<ArrayList<DeviceId>> viaUpdated = swViaMapUpdated.get(srcSw);
                     ArrayList<ArrayList<DeviceId>> viaUpdated = swViaMapUpdated.get(srcSw);
                     ArrayList<ArrayList<DeviceId>> via = getVia(switchVia, srcSw);
                     ArrayList<ArrayList<DeviceId>> via = getVia(switchVia, srcSw);
                     if ((via == null) || !viaUpdated.equals(via)) {
                     if ((via == null) || !viaUpdated.equals(via)) {
-                        log.debug("Impacted route:{}->{}", srcSw, sw.id());
+                        log.debug("Impacted route:{} -> {}", srcSw, sw.id());
                         ArrayList<DeviceId> route = new ArrayList<>();
                         ArrayList<DeviceId> route = new ArrayList<>();
                         route.add(srcSw);
                         route.add(srcSw);
                         route.add(sw.id());
                         route.add(sw.id());
@@ -373,15 +397,16 @@ public class DefaultRoutingHandler {
             }
             }
         }
         }
 
 
-        for (ArrayList<DeviceId> link: routes) {
-            log.trace("Route changes - ");
-            if (link.size() == 1) {
-                log.trace(" : {} - all", link.get(0));
-            } else {
-                log.trace(" : {} - {}", link.get(0), link.get(1));
+        if (log.isTraceEnabled()) {
+            for (ArrayList<DeviceId> link: routes) {
+                log.trace("Route changes - ");
+                if (link.size() == 1) {
+                    log.trace(" : all -> {}", link.get(0));
+                } else {
+                    log.trace(" : {} -> {}", link.get(0), link.get(1));
+                }
             }
             }
         }
         }
-
         return routes;
         return routes;
     }
     }
 
 

@@ -718,6 +718,17 @@ public class SegmentRoutingManager implements SegmentRoutingService {
                     } else if (event.type() == DeviceEvent.Type.PORT_REMOVED) {
                     } else if (event.type() == DeviceEvent.Type.PORT_REMOVED) {
                         processPortRemoved((Device) event.subject(),
                         processPortRemoved((Device) event.subject(),
                                            ((DeviceEvent) event).port());
                                            ((DeviceEvent) event).port());
+                    } else if (event.type() == DeviceEvent.Type.PORT_ADDED ||
+                            event.type() == DeviceEvent.Type.PORT_UPDATED) {
+                        log.info("** PORT ADDED OR UPDATED {}/{} -> {}",
+                                 (Device) event.subject(),
+                                 ((DeviceEvent) event).port(),
+                                 event.type());
+                        /* XXX create method for single port filtering rules
+                        if (defaultRoutingHandler != null) {
+                            defaultRoutingHandler.populatePortAddressingRules(
+                                ((Device) event.subject()).id());
+                        }*/
                     } else {
                     } else {
                         log.warn("Unhandled event type: {}", event.type());
                         log.warn("Unhandled event type: {}", event.type());
                     }
                     }
@@ -730,7 +741,7 @@ public class SegmentRoutingManager implements SegmentRoutingService {
     }
     }
 
 
     private void processLinkAdded(Link link) {
     private void processLinkAdded(Link link) {
-        log.debug("A new link {} was added", link.toString());
+        log.info("** LINK ADDED {}", link.toString());
         if (!deviceConfiguration.isConfigured(link.src().deviceId())) {
         if (!deviceConfiguration.isConfigured(link.src().deviceId())) {
             log.warn("Source device of this link is not configured.");
             log.warn("Source device of this link is not configured.");
             return;
             return;
@@ -767,7 +778,7 @@ public class SegmentRoutingManager implements SegmentRoutingService {
     }
     }
 
 
     private void processLinkRemoved(Link link) {
     private void processLinkRemoved(Link link) {
-        log.debug("A link {} was removed", link.toString());
+        log.info("** LINK REMOVED {}", link.toString());
         DefaultGroupHandler groupHandler = groupHandlerMap.get(link.src().deviceId());
         DefaultGroupHandler groupHandler = groupHandlerMap.get(link.src().deviceId());
         if (groupHandler != null) {
         if (groupHandler != null) {
             groupHandler.portDown(link.src().port(),
             groupHandler.portDown(link.src().port(),
@@ -782,7 +793,7 @@ public class SegmentRoutingManager implements SegmentRoutingService {
     }
     }
 
 
     private void processDeviceAdded(Device device) {
     private void processDeviceAdded(Device device) {
-        log.debug("A new device with ID {} was added", device.id());
+        log.info("** DEVICE ADDED with ID {}", device.id());
         if (deviceConfiguration == null || !deviceConfiguration.isConfigured(device.id())) {
         if (deviceConfiguration == null || !deviceConfiguration.isConfigured(device.id())) {
             log.warn("Device configuration uploading. Device {} will be "
             log.warn("Device configuration uploading. Device {} will be "
                     + "processed after config completes.", device.id());
                     + "processed after config completes.", device.id());
@@ -816,12 +827,13 @@ public class SegmentRoutingManager implements SegmentRoutingService {
             log.debug("updating groupHandlerMap with new config for device: {}",
             log.debug("updating groupHandlerMap with new config for device: {}",
                     deviceId);
                     deviceId);
             groupHandlerMap.put(deviceId, groupHandler);
             groupHandlerMap.put(deviceId, groupHandler);
-            // Also, in some cases, drivers may need extra
-            // information to process rules (eg. Router IP/MAC); and so, we send
-            // port addressing rules to the driver as well irrespective of whether
-            // this instance is the master or not.
-            defaultRoutingHandler.populatePortAddressingRules(deviceId);
         }
         }
+        // Also, in some cases, drivers may need extra
+        // information to process rules (eg. Router IP/MAC); and so, we send
+        // port addressing rules to the driver as well irrespective of whether
+        // this instance is the master or not.
+        defaultRoutingHandler.populatePortAddressingRules(deviceId);
+
         if (mastershipService.isLocalMaster(deviceId)) {
         if (mastershipService.isLocalMaster(deviceId)) {
             hostHandler.readInitialHosts(deviceId);
             hostHandler.readInitialHosts(deviceId);
             DefaultGroupHandler groupHandler = groupHandlerMap.get(deviceId);
             DefaultGroupHandler groupHandler = groupHandlerMap.get(deviceId);
@@ -866,7 +878,7 @@ public class SegmentRoutingManager implements SegmentRoutingService {
     }
     }
 
 
     private void processPortRemoved(Device device, Port port) {
     private void processPortRemoved(Device device, Port port) {
-        log.debug("Port {} was removed", port.toString());
+        log.info("Port {} was removed", port.toString());
         DefaultGroupHandler groupHandler = groupHandlerMap.get(device.id());
         DefaultGroupHandler groupHandler = groupHandlerMap.get(device.id());
         if (groupHandler != null) {
         if (groupHandler != null) {
             groupHandler.portDown(port.number(),
             groupHandler.portDown(port.number(),

@@ -23,6 +23,8 @@ import org.onosproject.net.flowobjective.FlowObjectiveService;
 import org.onosproject.net.flowobjective.ForwardingObjective;
 import org.onosproject.net.flowobjective.ForwardingObjective;
 import org.onosproject.net.flowobjective.NextObjective;
 import org.onosproject.net.flowobjective.NextObjective;
 
 
+import com.google.common.collect.ImmutableList;
+
 /**
 /**
  * Testing version of implementation on FlowObjectiveService.
  * Testing version of implementation on FlowObjectiveService.
  */
  */
@@ -60,6 +62,11 @@ public class FlowObjectiveAdapter implements FlowObjectiveService {
 
 
     @Override
     @Override
     public List<String> getNextMappings() {
     public List<String> getNextMappings() {
-        return null;
+        return ImmutableList.of();
+    }
+
+    @Override
+    public List<String> getPendingNexts() {
+        return ImmutableList.of();
     }
     }
 }
 }

@@ -16,30 +16,30 @@
 package org.onosproject.cli.net;
 package org.onosproject.cli.net;
 
 
 import java.util.List;
 import java.util.List;
+import org.onlab.osgi.ServiceNotFoundException;
 
 
-//import org.apache.karaf.shell.commands.Argument;
 import org.apache.karaf.shell.commands.Command;
 import org.apache.karaf.shell.commands.Command;
 import org.onosproject.cli.AbstractShellCommand;
 import org.onosproject.cli.AbstractShellCommand;
 import org.onosproject.net.flowobjective.FlowObjectiveService;
 import org.onosproject.net.flowobjective.FlowObjectiveService;
 
 
 /**
 /**
  * Returns a mapping of FlowObjective next-ids to the groups that get created
  * Returns a mapping of FlowObjective next-ids to the groups that get created
- * by a device driver.
+ * by a device driver. These mappings are controller instance specific.
  */
  */
-@Command(scope = "onos", name = "next-ids",
-        description = "flow-objective next-ids to group-ids mapping")
+@Command(scope = "onos", name = "obj-next-ids",
+        description = "flow-objectives next-ids to group-ids mapping")
 public class FlowObjectiveNextListCommand extends AbstractShellCommand {
 public class FlowObjectiveNextListCommand extends AbstractShellCommand {
 
 
-    /*@Argument(index = 1, name = "uri", description = "Device ID",
-            required = false, multiValued = false)
-    String uri = null;
-     */
-    private static final String FORMAT_MAPPING =
-            "  %s";
+    private static final String FORMAT_MAPPING = "  %s";
+
     @Override
     @Override
     protected void execute() {
     protected void execute() {
-        FlowObjectiveService service = get(FlowObjectiveService.class);
-        printNexts(service.getNextMappings());
+        try {
+            FlowObjectiveService service = get(FlowObjectiveService.class);
+            printNexts(service.getNextMappings());
+        } catch (ServiceNotFoundException e) {
+            print(FORMAT_MAPPING, "FlowObjectiveService unavailable");
+        }
     }
     }
 
 
     private void printNexts(List<String> nextGroupMappings) {
     private void printNexts(List<String> nextGroupMappings) {

+/*
+ * 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.cli.net;
+
+import java.util.List;
+
+import org.apache.karaf.shell.commands.Command;
+import org.onlab.osgi.ServiceNotFoundException;
+import org.onosproject.cli.AbstractShellCommand;
+import org.onosproject.net.flowobjective.FlowObjectiveService;
+
+/**
+ * Returns a list of FlowObjective next-ids waiting to get created by device-drivers.
+ * Also returns the forwarding objectives waiting on the pending next-objectives.
+ * These lists are controller instance specific.
+ */
+@Command(scope = "onos", name = "obj-pending-nexts",
+        description = "flow-objectives pending next-objectives")
+public class FlowObjectivePendingNextCommand extends AbstractShellCommand {
+
+    private static final String FORMAT_MAPPING = "  %s";
+
+    @Override
+    protected void execute() {
+        try {
+            FlowObjectiveService service = get(FlowObjectiveService.class);
+            printNexts(service.getPendingNexts());
+        } catch (ServiceNotFoundException e) {
+            print(FORMAT_MAPPING, "FlowObjectiveService unavailable");
+        }
+    }
+
+    private void printNexts(List<String> pendingNexts) {
+        pendingNexts.forEach(str -> print(FORMAT_MAPPING, str));
+    }
+
+}

@@ -31,6 +31,10 @@
         </command>
         </command>
 
 
         <command>
         <command>
+            <action class="org.onosproject.cli.net.FlowObjectivePendingNextCommand"/>
+        </command>
+
+        <command>
             <action class="org.onosproject.cli.net.FlowObjectiveCompositionCommand"/>
             <action class="org.onosproject.cli.net.FlowObjectiveCompositionCommand"/>
         </command>
         </command>
 
 

@@ -95,7 +95,18 @@ public interface FlowObjectiveService {
      *
      *
      * @return a list of strings preformatted by the device-drivers to provide
      * @return a list of strings preformatted by the device-drivers to provide
      *         information on next-id to group-id mapping. Consumed by the
      *         information on next-id to group-id mapping. Consumed by the
-     *         "next-ids" command on the CLI.
+     *         "obj-next-ids" command on the CLI.
      */
      */
     List<String> getNextMappings();
     List<String> getNextMappings();
+
+    /**
+     * Retrieve all nextObjectives that are waiting to hear back from device
+     * drivers, and the forwarding-objectives that are waiting on the
+     * successful completion of the next-objectives. Consumed by the
+     * "obj-pending-nexts" command on the CLI.
+     *
+     * @return a list of strings preformatted to provide information on the
+     *          next-ids awaiting confirmation from the device-drivers.
+     */
+    List<String> getPendingNexts();
 }
 }

@@ -482,4 +482,21 @@ public class FlowObjectiveManager implements FlowObjectiveService {
         }
         }
         return mappings;
         return mappings;
     }
     }
+
+    @Override
+    public List<String> getPendingNexts() {
+        List<String> pendingNexts = new ArrayList<>();
+        for (Integer nextId : pendingForwards.keySet()) {
+            Set<PendingNext> pnext = pendingForwards.get(nextId);
+            StringBuffer pend = new StringBuffer();
+            pend.append("Next Id: ").append(Integer.toString(nextId))
+                .append(" :: ");
+            for (PendingNext pn : pnext) {
+                pend.append(Integer.toString(pn.forwardingObjective().id()))
+                    .append(" ");
+            }
+            pendingNexts.add(pend.toString());
+        }
+        return pendingNexts;
+    }
 }
 }

@@ -15,6 +15,7 @@
  */
  */
 package org.onosproject.net.flowobjective.impl.composition;
 package org.onosproject.net.flowobjective.impl.composition;
 
 
+import com.google.common.collect.ImmutableList;
 import com.google.common.collect.Maps;
 import com.google.common.collect.Maps;
 import com.google.common.collect.Sets;
 import com.google.common.collect.Sets;
 import org.apache.felix.scr.annotations.Activate;
 import org.apache.felix.scr.annotations.Activate;
@@ -440,6 +441,12 @@ public class FlowObjectiveCompositionManager implements FlowObjectiveService {
     @Override
     @Override
     public List<String> getNextMappings() {
     public List<String> getNextMappings() {
         // TODO Implementation deferred as this is an experimental component.
         // TODO Implementation deferred as this is an experimental component.
-        return null;
+        return ImmutableList.of();
+    }
+
+    @Override
+    public List<String> getPendingNexts() {
+        // TODO Implementation deferred as this is an experimental component.
+        return ImmutableList.of();
     }
     }
 }
 }

@@ -1083,7 +1083,7 @@ public class Ofdpa2Pipeline extends AbstractHandlerBehaviour implements Pipeline
             }
             }
             // add port information for last group in group-chain
             // add port information for last group in group-chain
             List<Instruction> lastGroupIns = new ArrayList<Instruction>();
             List<Instruction> lastGroupIns = new ArrayList<Instruction>();
-            if (gchain != null) {
+            if (lastGroup != null) {
                 lastGroupIns = lastGroup.buckets().buckets().get(0)
                 lastGroupIns = lastGroup.buckets().buckets().get(0)
                                     .treatment().allInstructions();
                                     .treatment().allInstructions();
             }
             }