Bug fix: Do not create groups as a side effect when revoking routes
Also changing pendingGroups to a cache that will automatically purge failed groups
Improving log messagesin several places

Change-Id: I6843a66d58e623259c7fd20ffe64d56a46d963f0
diff --git a/apps/segmentrouting/src/main/java/org/onosproject/segmentrouting/HostHandler.java b/apps/segmentrouting/src/main/java/org/onosproject/segmentrouting/HostHandler.java
index 441597e..94d8805 100644
--- a/apps/segmentrouting/src/main/java/org/onosproject/segmentrouting/HostHandler.java
+++ b/apps/segmentrouting/src/main/java/org/onosproject/segmentrouting/HostHandler.java
@@ -93,7 +93,7 @@
             log.debug("Populating bridging entry for host {}/{} at {}:{}",
                     mac, vlanId, deviceId, port);
             ForwardingObjective.Builder fob =
-                    bridgingFwdObjBuilder(deviceId, mac, vlanId, port);
+                    bridgingFwdObjBuilder(deviceId, mac, vlanId, port, false);
             if (fob == null) {
                 log.warn("Fail to create fwd obj for host {}/{}. Abort.", mac, vlanId);
                 return;
@@ -132,7 +132,7 @@
         if (accepted(host)) {
             // Revoke bridging table entry
             ForwardingObjective.Builder fob =
-                    bridgingFwdObjBuilder(deviceId, mac, vlanId, port);
+                    bridgingFwdObjBuilder(deviceId, mac, vlanId, port, true);
             if (fob == null) {
                 log.warn("Fail to create fwd obj for host {}/{}. Abort.", mac, vlanId);
                 return;
@@ -170,7 +170,7 @@
         if (accepted(event.prevSubject())) {
             // Revoke previous bridging table entry
             ForwardingObjective.Builder prevFob =
-                    bridgingFwdObjBuilder(prevDeviceId, mac, vlanId, prevPort);
+                    bridgingFwdObjBuilder(prevDeviceId, mac, vlanId, prevPort, true);
             if (prevFob == null) {
                 log.warn("Fail to create fwd obj for host {}/{}. Abort.", mac, vlanId);
                 return;
@@ -193,7 +193,7 @@
         if (accepted(event.subject())) {
             // Populate new bridging table entry
             ForwardingObjective.Builder newFob =
-                    bridgingFwdObjBuilder(newDeviceId, mac, vlanId, newPort);
+                    bridgingFwdObjBuilder(newDeviceId, mac, vlanId, newPort, false);
             if (newFob == null) {
                 log.warn("Fail to create fwd obj for host {}/{}. Abort.", mac, vlanId);
                 return;
@@ -260,11 +260,12 @@
      * @param mac MAC address of the host
      * @param hostVlanId VLAN ID of the host
      * @param outport Port that host attaches to
+     * @param revoke true if forwarding objective is meant to revoke forwarding rule
      * @return Forwarding objective builder
      */
     private ForwardingObjective.Builder bridgingFwdObjBuilder(
             DeviceId deviceId, MacAddress mac, VlanId hostVlanId,
-            PortNumber outport) {
+            PortNumber outport, boolean revoke) {
         ConnectPoint connectPoint = new ConnectPoint(deviceId, outport);
         VlanId untaggedVlan = srManager.getUntaggedVlanId(connectPoint);
         Set<VlanId> taggedVlans = srManager.getTaggedVlanId(connectPoint);
@@ -313,9 +314,10 @@
         }
 
         // All forwarding is via Groups. Drivers can re-purpose to flow-actions if needed.
+        // If the objective is to revoke an existing rule, and for some reason
+        // the next-objective does not exist, then a new one should not be created
         int portNextObjId = srManager.getPortNextObjectiveId(deviceId, outport,
-                tbuilder.build(),
-                mbuilder.build());
+                tbuilder.build(), mbuilder.build(), !revoke);
         if (portNextObjId == -1) {
             // Warning log will come from getPortNextObjective method
             return null;
diff --git a/apps/segmentrouting/src/main/java/org/onosproject/segmentrouting/IcmpHandler.java b/apps/segmentrouting/src/main/java/org/onosproject/segmentrouting/IcmpHandler.java
index 38293d0..aaa8d4c 100644
--- a/apps/segmentrouting/src/main/java/org/onosproject/segmentrouting/IcmpHandler.java
+++ b/apps/segmentrouting/src/main/java/org/onosproject/segmentrouting/IcmpHandler.java
@@ -272,7 +272,7 @@
     private void handleNdpRequest(NeighbourMessageContext pkt, HostService hostService) {
         // ND request for the gateway. We have to reply on behalf of the gateway.
         if (isNdpForGateway(pkt)) {
-            log.debug("Sending NDP reply on behalf of the router");
+            log.trace("Sending NDP reply on behalf of gateway IP for pkt: {}", pkt);
             sendResponse(pkt, config.getRouterMacForAGatewayIp(pkt.target()), hostService);
         } else {
             // NOTE: Ignore NDP packets except those target for the router
diff --git a/apps/segmentrouting/src/main/java/org/onosproject/segmentrouting/RoutingRulePopulator.java b/apps/segmentrouting/src/main/java/org/onosproject/segmentrouting/RoutingRulePopulator.java
index 95f07b2..850962a 100644
--- a/apps/segmentrouting/src/main/java/org/onosproject/segmentrouting/RoutingRulePopulator.java
+++ b/apps/segmentrouting/src/main/java/org/onosproject/segmentrouting/RoutingRulePopulator.java
@@ -122,7 +122,8 @@
                 prefix, deviceId, outPort);
         ForwardingObjective.Builder fwdBuilder;
         try {
-            fwdBuilder = routingFwdObjBuilder(deviceId, prefix, hostMac, hostVlanId, outPort);
+            fwdBuilder = routingFwdObjBuilder(deviceId, prefix, hostMac,
+                                              hostVlanId, outPort, false);
         } catch (DeviceConfigNotFoundException e) {
             log.warn(e.getMessage() + " Aborting populateIpRuleForHost.");
             return;
@@ -156,7 +157,8 @@
                 prefix, deviceId, outPort);
         ForwardingObjective.Builder fwdBuilder;
         try {
-            fwdBuilder = routingFwdObjBuilder(deviceId, prefix, hostMac, hostVlanId, outPort);
+            fwdBuilder = routingFwdObjBuilder(deviceId, prefix, hostMac,
+                                              hostVlanId, outPort, true);
         } catch (DeviceConfigNotFoundException e) {
             log.warn(e.getMessage() + " Aborting revokeIpRuleForHost.");
             return;
@@ -184,12 +186,14 @@
      * @param hostMac MAC address of the nexthop
      * @param hostVlanId Vlan ID of the nexthop
      * @param outPort port where the nexthop attaches to
+     * @param revoke true if forwarding objective is meant to revoke forwarding rule
      * @return forwarding objective builder
      * @throws DeviceConfigNotFoundException if given device is not configured
      */
     private ForwardingObjective.Builder routingFwdObjBuilder(
             DeviceId deviceId, IpPrefix prefix,
-            MacAddress hostMac, VlanId hostVlanId, PortNumber outPort)
+            MacAddress hostMac, VlanId hostVlanId, PortNumber outPort,
+            boolean revoke)
             throws DeviceConfigNotFoundException {
         MacAddress deviceMac;
         deviceMac = config.getDeviceMac(deviceId);
@@ -231,13 +235,14 @@
                 mbuilder.matchVlanId(INTERNAL_VLAN);
             }
         } else {
-            log.warn("Tagged nexthop {}/{} is not allowed on {} without VLAN listed in tagged vlan",
-                    hostMac, hostVlanId, connectPoint);
+            log.warn("Tagged nexthop {}/{} is not allowed on {} without VLAN listed"
+                    + " in tagged vlan", hostMac, hostVlanId, connectPoint);
             return null;
         }
-
+        // if the objective is to revoke an existing rule, and for some reason
+        // the next-objective does not exist, then a new one should not be created
         int portNextObjId = srManager.getPortNextObjectiveId(deviceId, outPort,
-                                                             tbuilder.build(), mbuilder.build());
+                                          tbuilder.build(), mbuilder.build(), !revoke);
         if (portNextObjId == -1) {
             // Warning log will come from getPortNextObjective method
             return null;
@@ -437,12 +442,13 @@
             // 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.
-            ForwardingObjective.Builder fwdObjNoBosBuilder = getMplsForwardingObjective(targetSwId,
-                                                                                        nextHops,
-                                                                                        true,
-                                                                                        isMplsBos,
-                                                                                        metabuilder.build(),
-                                                                                        routerIp);
+            ForwardingObjective.Builder fwdObjNoBosBuilder =
+                    getMplsForwardingObjective(targetSwId,
+                                               nextHops,
+                                               true,
+                                               isMplsBos,
+                                               metabuilder.build(),
+                                               routerIp);
             // Error case, we cannot handle, exit.
             if (fwdObjNoBosBuilder == null) {
                 return Collections.emptyList();
@@ -456,12 +462,13 @@
             // Not-bos pop case. If MPLS-ECMP has been configured, the
             // application we will request the installation for an MPLS-ECMP
             // group.
-            ForwardingObjective.Builder fwdObjNoBosBuilder = getMplsForwardingObjective(targetSwId,
-                                                                                        nextHops,
-                                                                                        false,
-                                                                                        isMplsBos,
-                                                                                        metabuilder.build(),
-                                                                                        routerIp);
+            ForwardingObjective.Builder fwdObjNoBosBuilder =
+                    getMplsForwardingObjective(targetSwId,
+                                               nextHops,
+                                               false,
+                                               isMplsBos,
+                                               metabuilder.build(),
+                                               routerIp);
             // Error case, we cannot handle, exit.
             if (fwdObjNoBosBuilder == null) {
                 return Collections.emptyList();
@@ -727,9 +734,6 @@
             log.warn(e.getMessage() + " Processing SinglePortFilters aborted");
             return null;
         }
-        // suppressed ports still have filtering rules pushed by SR using default vlan
-        ConnectPoint connectPoint = new ConnectPoint(deviceId, portnum);
-
         FilteringObjective.Builder fob = DefaultFilteringObjective.builder();
         fob.withKey(Criteria.matchInPort(portnum))
             .addCondition(Criteria.matchEthDst(deviceMac))
diff --git a/apps/segmentrouting/src/main/java/org/onosproject/segmentrouting/SegmentRoutingManager.java b/apps/segmentrouting/src/main/java/org/onosproject/segmentrouting/SegmentRoutingManager.java
index 01a09e4..5b05187 100644
--- a/apps/segmentrouting/src/main/java/org/onosproject/segmentrouting/SegmentRoutingManager.java
+++ b/apps/segmentrouting/src/main/java/org/onosproject/segmentrouting/SegmentRoutingManager.java
@@ -705,21 +705,23 @@
     /**
      * Returns the next objective ID for the given portNumber, given the treatment.
      * There could be multiple different treatments to the same outport, which
-     * would result in different objectives. If the next object
-     * does not exist, a new one is created and its id is returned.
+     * would result in different objectives. If the next object does not exist,
+     * and should be created, a new one is created and its id is returned.
      *
      * @param deviceId Device ID
      * @param portNum port number on device for which NextObjective is queried
      * @param treatment the actions to apply on the packets (should include outport)
      * @param meta metadata passed into the creation of a Next Objective if necessary
+     * @param createIfMissing true if a next object should be created if not found
      * @return next objective ID or -1 if an error occurred during retrieval or creation
      */
     public int getPortNextObjectiveId(DeviceId deviceId, PortNumber portNum,
                                       TrafficTreatment treatment,
-                                      TrafficSelector meta) {
+                                      TrafficSelector meta,
+                                      boolean createIfMissing) {
         DefaultGroupHandler ghdlr = groupHandlerMap.get(deviceId);
         if (ghdlr != null) {
-            return ghdlr.getPortNextObjectiveId(portNum, treatment, meta);
+            return ghdlr.getPortNextObjectiveId(portNum, treatment, meta, createIfMissing);
         } else {
             log.warn("getPortNextObjectiveId query - groupHandler for device {}"
                     + " not found", deviceId);
diff --git a/apps/segmentrouting/src/main/java/org/onosproject/segmentrouting/grouphandler/DefaultGroupHandler.java b/apps/segmentrouting/src/main/java/org/onosproject/segmentrouting/grouphandler/DefaultGroupHandler.java
index e73a0d8..fce4e50 100644
--- a/apps/segmentrouting/src/main/java/org/onosproject/segmentrouting/grouphandler/DefaultGroupHandler.java
+++ b/apps/segmentrouting/src/main/java/org/onosproject/segmentrouting/grouphandler/DefaultGroupHandler.java
@@ -23,7 +23,6 @@
 import org.onlab.packet.VlanId;
 import org.onlab.util.KryoNamespace;
 import org.onosproject.core.ApplicationId;
-import org.onosproject.incubator.net.intf.Interface;
 import org.onosproject.net.ConnectPoint;
 import org.onosproject.net.DeviceId;
 import org.onosproject.net.Link;
@@ -489,7 +488,7 @@
      * Returns the next objective of type simple associated with the port on the
      * device, given the treatment. Different treatments to the same port result
      * in different next objectives. If no such objective exists, this method
-     * creates one and returns the id. Optionally metadata can be passed in for
+     * creates one (if requested) and returns the id. Optionally metadata can be passed in for
      * the creation of the objective. Typically this is used for L2 and L3 forwarding
      * to compute nodes and containers/VMs on the compute nodes directly attached
      * to the switch.
@@ -497,24 +496,31 @@
      * @param portNum the port number for the simple next objective
      * @param treatment the actions to apply on the packets (should include outport)
      * @param meta optional metadata passed into the creation of the next objective
+     * @param createIfMissing true if a next object should be created if not found
      * @return int if found or created, -1 if there are errors during the
      *          creation of the next objective.
      */
     public int getPortNextObjectiveId(PortNumber portNum, TrafficTreatment treatment,
-                                      TrafficSelector meta) {
+                                      TrafficSelector meta, boolean createIfMissing) {
         Integer nextId = portNextObjStore
                 .get(new PortNextObjectiveStoreKey(deviceId, portNum, treatment, meta));
+        if (nextId != null) {
+            return nextId;
+        }
+        log.debug("getPortNextObjectiveId in device {}: Next objective id "
+                + "not found for port: {} .. {}", deviceId, portNum,
+                (createIfMissing) ? "creating" : "aborting");
+        if (!createIfMissing) {
+            return -1;
+        }
+        // create missing next objective
+        createGroupFromPort(portNum, treatment, meta);
+        nextId = portNextObjStore.get(new PortNextObjectiveStoreKey(deviceId, portNum,
+                                                                    treatment, meta));
         if (nextId == null) {
-            log.debug("getPortNextObjectiveId in device{}: Next objective id "
-                    + "not found for {} and {} creating", deviceId, portNum);
-            createGroupFromPort(portNum, treatment, meta);
-            nextId = portNextObjStore.get(
-                         new PortNextObjectiveStoreKey(deviceId, portNum, treatment, meta));
-            if (nextId == null) {
-                log.warn("getPortNextObjectiveId: unable to create next obj"
-                        + "for dev:{} port:{}", deviceId, portNum);
-                return -1;
-            }
+            log.warn("getPortNextObjectiveId: unable to create next obj"
+                    + "for dev:{} port:{}", deviceId, portNum);
+            return -1;
         }
         return nextId;
     }
@@ -722,12 +728,12 @@
 
             ObjectiveContext context = new DefaultObjectiveContext(
                     (objective) ->
-                            log.debug("createGroupsFromNeighborsets installed NextObj {} on {}",
-                            nextId, deviceId),
+                            log.debug("createGroupsFromNeighborsets installed "
+                                    + "NextObj {} on {}", nextId, deviceId),
                     (objective, error) ->
-                            log.warn("createGroupsFromNeighborsets failed to install NextObj {} on {}: {}",
-                                    nextId, deviceId, error)
-            );
+                            log.warn("createGroupsFromNeighborsets failed to install"
+                                    + " NextObj {} on {}: {}", nextId, deviceId, error)
+                    );
             NextObjective nextObj = nextObjBuilder.add(context);
             log.debug("**createGroupsFromNeighborsets: Submited "
                     + "next objective {} in device {}",
@@ -743,8 +749,6 @@
      * all configured subnets.
      */
     public void createGroupsFromVlanConfig() {
-        Set<Interface> interfaces = srManager.interfaceService.getInterfaces();
-
         srManager.getVlanPortMap(deviceId).asMap().forEach((vlanId, ports) -> {
             createBcastGroupFromVlan(vlanId, ports);
         });
@@ -784,7 +788,15 @@
             nextObjBuilder.addTreatment(tBuilder.build());
         });
 
-        NextObjective nextObj = nextObjBuilder.add();
+        ObjectiveContext context = new DefaultObjectiveContext(
+            (objective) ->
+                log.debug("createBroadcastGroupFromVlan installed "
+                        + "NextObj {} on {}", nextId, deviceId),
+            (objective, error) ->
+                log.warn("createBroadcastGroupFromVlan failed to install"
+                        + " NextObj {} on {}: {}", nextId, deviceId, error)
+            );
+        NextObjective nextObj = nextObjBuilder.add(context);
         flowObjectiveService.next(deviceId, nextObj);
         log.debug("createBcastGroupFromVlan: Submited next objective {} in device {}",
                   nextId, deviceId);
@@ -825,7 +837,15 @@
                 .fromApp(appId)
                 .withMeta(meta);
 
-        NextObjective nextObj = nextObjBuilder.add();
+        ObjectiveContext context = new DefaultObjectiveContext(
+            (objective) ->
+                log.debug("createGroupFromPort installed "
+                        + "NextObj {} on {}", nextId, deviceId),
+            (objective, error) ->
+                log.warn("createGroupFromPort failed to install"
+                        + " NextObj {} on {}: {}", nextId, deviceId, error)
+            );
+        NextObjective nextObj = nextObjBuilder.add(context);
         flowObjectiveService.next(deviceId, nextObj);
         log.debug("createGroupFromPort: Submited next objective {} in device {} "
                 + "for port {}", nextId, deviceId, portNum);
diff --git a/drivers/default/src/main/java/org/onosproject/driver/pipeline/Ofdpa2GroupHandler.java b/drivers/default/src/main/java/org/onosproject/driver/pipeline/Ofdpa2GroupHandler.java
index c93ad9b..a069c42 100644
--- a/drivers/default/src/main/java/org/onosproject/driver/pipeline/Ofdpa2GroupHandler.java
+++ b/drivers/default/src/main/java/org/onosproject/driver/pipeline/Ofdpa2GroupHandler.java
@@ -91,7 +91,7 @@
 import static org.slf4j.LoggerFactory.getLogger;
 
 /**
- * Group handler that emulates Broadcom OF-DPA TTP on CpqD.
+ * Group handler that emulates Broadcom OF-DPA TTP.
  */
 public class Ofdpa2GroupHandler {
     /*
@@ -134,7 +134,7 @@
     private FlowObjectiveStore flowObjectiveStore;
     private Cache<GroupKey, List<OfdpaNextGroup>> pendingAddNextObjectives;
     private Cache<NextObjective, List<GroupKey>> pendingRemoveNextObjectives;
-    private ConcurrentHashMap<GroupKey, Set<GroupChainElem>> pendingGroups;
+    private Cache<GroupKey, Set<GroupChainElem>> pendingGroups;
     private ConcurrentHashMap<GroupKey, Set<NextObjective>> pendingUpdateNextObjectives;
     private ScheduledExecutorService groupChecker =
             Executors.newScheduledThreadPool(2, groupedThreads("onos/pipeliner", "ofdpa2-%d", log));
@@ -205,7 +205,15 @@
                                     ObjectiveError.GROUPREMOVALFAILED);
                     }
                 }).build();
-        pendingGroups = new ConcurrentHashMap<>();
+        pendingGroups = CacheBuilder.newBuilder()
+                .expireAfterWrite(20, TimeUnit.SECONDS)
+                .removalListener((
+                        RemovalNotification<GroupKey, Set<GroupChainElem>> notification) -> {
+                    if (notification.getCause() == RemovalCause.EXPIRED) {
+                        log.error("Unable to install group with key {} and pending GCEs: {}",
+                                  notification.getKey(), notification.getValue());
+                    }
+                }).build();
         pendingUpdateNextObjectives = new ConcurrentHashMap<>();
         groupChecker.scheduleAtFixedRate(new GroupChecker(), 0, 500, TimeUnit.MILLISECONDS);
 
@@ -875,7 +883,6 @@
             updatePendingGroups(gi.nextGroupDesc.appCookie(), l3ecmpGce);
             groupService.addGroup(gi.innerMostGroupDesc);
         }
-
     }
 
     /**
@@ -1061,11 +1068,9 @@
 
         nextObjective.next().forEach(trafficTreatment -> {
             PortNumber portNumber = readOutPortFromTreatment(trafficTreatment);
-
             if (portNumber == null) {
                 return;
             }
-
             if (existingPorts.contains(portNumber)) {
                 duplicateBuckets.add(trafficTreatment);
             } else {
@@ -1118,7 +1123,6 @@
                  });
             }
         });
-
         return existingPorts;
     }
 
@@ -1180,7 +1184,6 @@
             updatePendingGroups(groupInfo.nextGroupDesc.appCookie(), l3ecmpGce);
             groupService.addGroup(groupInfo.innerMostGroupDesc);
         });
-
     }
 
     private void addBucketToBroadcastGroup(NextObjective nextObj,
@@ -1418,9 +1421,6 @@
 
         updatePendingNextObjective(l3mcastGroupKey,
                                    new OfdpaNextGroup(allActiveKeys, nextObj));
-
-
-
     }
 
     /**
@@ -1537,8 +1537,6 @@
                 .map(id -> HEX_PREFIX + id)
                 .collect(Collectors.toList());
 
-
-
         log.debug("Removing buckets from group id 0x{} pointing to group id(s) {} "
                 + "for next id {} in device {}", Integer.toHexString(modGroup.id().id()),
                 pointedGroupIds, nextObjective.id(), deviceId);
@@ -1561,8 +1559,7 @@
     }
 
     /**
-     * Removes all groups in multiple possible group-chains that represent the next
-     * objective.
+     * Removes all groups in multiple possible group-chains that represent the next-obj.
      *
      * @param nextObjective the next objective to remove
      * @param next the NextGroup that represents the existing group-chain for
@@ -1584,24 +1581,24 @@
     //  Helper Methods and Classes
     //////////////////////////////////////
 
-    protected void updatePendingNextObjective(GroupKey key, OfdpaNextGroup value) {
-        List<OfdpaNextGroup> nextList = new CopyOnWriteArrayList<OfdpaNextGroup>();
-        nextList.add(value);
-        List<OfdpaNextGroup> ret = pendingAddNextObjectives.asMap()
-                .putIfAbsent(key, nextList);
-        if (ret != null) {
-            ret.add(value);
-        }
+    protected void updatePendingNextObjective(GroupKey gkey, OfdpaNextGroup value) {
+        pendingAddNextObjectives.asMap().compute(gkey, (k, val) -> {
+            if (val == null) {
+                val = new CopyOnWriteArrayList<OfdpaNextGroup>();
+            }
+            val.add(value);
+            return val;
+        });
     }
 
     protected void updatePendingGroups(GroupKey gkey, GroupChainElem gce) {
-        Set<GroupChainElem> gceSet = Collections.newSetFromMap(
-                new ConcurrentHashMap<GroupChainElem, Boolean>());
-        gceSet.add(gce);
-        Set<GroupChainElem> retval = pendingGroups.putIfAbsent(gkey, gceSet);
-        if (retval != null) {
-            retval.add(gce);
-        }
+        pendingGroups.asMap().compute(gkey, (k, val) -> {
+            if (val == null) {
+                val = Sets.newConcurrentHashSet();
+            }
+            val.add(gce);
+            return val;
+        });
     }
 
     private void addPendingUpdateNextObjective(GroupKey groupKey, NextObjective nextObjective) {
@@ -1650,7 +1647,14 @@
     private class GroupChecker implements Runnable {
         @Override
         public void run() {
-            Set<GroupKey> keys = pendingGroups.keySet().stream()
+            if (pendingGroups.size() != 0) {
+                log.debug("pending groups being checked: {}", pendingGroups.asMap().keySet());
+            }
+            if (pendingAddNextObjectives.size() != 0) {
+                log.debug("pending add-next-obj being checked: {}",
+                          pendingAddNextObjectives.asMap().keySet());
+            }
+            Set<GroupKey> keys = pendingGroups.asMap().keySet().stream()
                     .filter(key -> groupService.getGroup(deviceId, key) != null)
                     .collect(Collectors.toSet());
             Set<GroupKey> otherkeys = pendingAddNextObjectives.asMap().keySet().stream()
@@ -1684,7 +1688,6 @@
     }
 
     private void processPendingUpdateNextObjs(GroupKey groupKey) {
-
         pendingUpdateNextObjectives.compute(groupKey, (gKey, nextObjs) -> {
             if (nextObjs != null) {
 
@@ -1695,14 +1698,13 @@
                     Ofdpa2Pipeline.pass(nextObj);
                 });
             }
-
             return Sets.newHashSet();
         });
     }
 
     private void processPendingAddGroupsOrNextObjs(GroupKey key, boolean added) {
         //first check for group chain
-        Set<GroupChainElem> gceSet = pendingGroups.remove(key);
+        Set<GroupChainElem> gceSet = pendingGroups.asMap().remove(key);
         if (gceSet != null) {
             for (GroupChainElem gce : gceSet) {
                 log.debug("Group service {} group key {} in device {}. "
diff --git a/drivers/default/src/main/java/org/onosproject/driver/pipeline/Ofdpa2Pipeline.java b/drivers/default/src/main/java/org/onosproject/driver/pipeline/Ofdpa2Pipeline.java
index b937762..6edebdf 100644
--- a/drivers/default/src/main/java/org/onosproject/driver/pipeline/Ofdpa2Pipeline.java
+++ b/drivers/default/src/main/java/org/onosproject/driver/pipeline/Ofdpa2Pipeline.java
@@ -399,12 +399,12 @@
 
         if (ethCriterion == null || ethCriterion.mac().equals(NONE)) {
             // NOTE: it is possible that a filtering objective only has vidCriterion
-            log.debug("filtering objective missing dstMac, cannot program TMAC table");
+            log.warn("filtering objective missing dstMac, cannot program TMAC table");
         } else {
             for (FlowRule tmacRule : processEthDstFilter(portCriterion, ethCriterion,
                                                          vidCriterion, assignedVlan,
                                                          applicationId)) {
-                log.debug("{} MAC filtering rules in TMAC table: {} for dev: {}",
+                log.trace("{} MAC filtering rules in TMAC table: {} for dev: {}",
                           (install) ? "adding" : "removing", tmacRule, deviceId);
                 ops = install ? ops.add(tmacRule) : ops.remove(tmacRule);
             }
@@ -443,7 +443,7 @@
             });
 
             for (FlowRule filteringRule : filteringRules) {
-                log.debug("{} VLAN filtering rule in VLAN table: {} for dev: {}",
+                log.trace("{} VLAN filtering rule in VLAN table: {} for dev: {}",
                           (install) ? "adding" : "removing", filteringRule, deviceId);
                 ops = install ? ops.add(filteringRule) : ops.remove(filteringRule);
             }
@@ -451,7 +451,7 @@
             ops.newStage();
 
             for (FlowRule assignmentRule : assignmentRules) {
-                log.debug("{} VLAN assignment rule in VLAN table: {} for dev: {}",
+                log.trace("{} VLAN assignment rule in VLAN table: {} for dev: {}",
                         (install) ? "adding" : "removing", assignmentRule, deviceId);
                 ops = install ? ops.add(assignmentRule) : ops.remove(assignmentRule);
             }