CORD-81 Support for editing buckets of ALL groups, both L2Flood and
L3Multicast in the OF-DPA driver. Additionally changed SR app to
react to edge ports going up/down by sending flood group edit calls.

Change-Id: I6d7749f579d3d1d800e6ca8ce2316bd0e3f6136b
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 81fb220..ca41a0f 100644
--- a/apps/segmentrouting/src/main/java/org/onosproject/segmentrouting/SegmentRoutingManager.java
+++ b/apps/segmentrouting/src/main/java/org/onosproject/segmentrouting/SegmentRoutingManager.java
@@ -667,7 +667,8 @@
         public void event(DeviceEvent event) {
             switch (event.type()) {
             case DEVICE_ADDED:
-            case PORT_REMOVED:
+            case PORT_UPDATED:
+            case PORT_ADDED:
             case DEVICE_UPDATED:
             case DEVICE_AVAILABILITY_CHANGED:
                 log.debug("Event {} received from Device Service", event.type());
@@ -740,20 +741,21 @@
                                     event.type(), ((Device) event.subject()).id());
                             processDeviceRemoved((Device) event.subject());
                         }
-                    } else if (event.type() == DeviceEvent.Type.PORT_REMOVED) {
-                        processPortRemoved((Device) event.subject(),
-                                           ((DeviceEvent) event).port());
-                    } else if (event.type() == DeviceEvent.Type.PORT_ADDED ||
-                            event.type() == DeviceEvent.Type.PORT_UPDATED) {
-                        log.info("** PORT ADDED OR UPDATED {}/{} -> {}",
+                    } else if (event.type() == DeviceEvent.Type.PORT_ADDED) {
+                        // XXX typically these calls come when device is added
+                        // so port filtering rules are handled there, and it
+                        // represents all ports on the device, enabled or not.
+                        log.debug("** PORT ADDED {}/{} -> {}",
                                  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 if (event.type() == DeviceEvent.Type.PORT_UPDATED) {
+                        log.info("** PORT UPDATED {}/{} -> {}",
+                                 event.subject(),
+                                 ((DeviceEvent) event).port(),
+                                 event.type());
+                        processPortUpdated(((Device) event.subject()),
+                                           ((DeviceEvent) event).port());
                     } else {
                         log.warn("Unhandled event type: {}", event.type());
                     }
@@ -898,12 +900,51 @@
         xConnectHandler.removeDevice(device.id());
     }
 
-    private void processPortRemoved(Device device, Port port) {
-        log.info("Port {} was removed", port.toString());
+    private void processPortUpdated(Device device, Port port) {
+        if (deviceConfiguration == null || !deviceConfiguration.isConfigured(device.id())) {
+            log.warn("Device configuration uploading. Not handling port event for"
+                    + "dev: {} port: {}", device.id(), port.number());
+            return;
+        }
+        /* XXX create method for single port filtering rules which are needed
+           for both switch-to-switch ports and edge ports
+        if (defaultRoutingHandler != null) {
+            defaultRoutingHandler.populatePortAddressingRules(
+                ((Device) event.subject()).id());
+        }*/
+
+        // portUpdated calls are for ports that have gone down or up. For switch
+        // to switch ports, link-events should take care of any re-routing or
+        // group editing necessary for port up/down. Here we only process edge ports
+        // that are already configured.
+         Ip4Prefix configuredSubnet = deviceConfiguration.getPortSubnet(device.id(),
+                                                                        port.number());
+        if (configuredSubnet == null) {
+            log.debug("Not handling port updated event for unconfigured port "
+                    + "dev/port: {}/{}", device.id(), port.number());
+            return;
+        }
+        processEdgePort(device, port, configuredSubnet);
+    }
+
+    private void processEdgePort(Device device, Port port, Ip4Prefix subnet) {
+        boolean portUp = port.isEnabled();
+        if (portUp) {
+            log.info("Device:EdgePort {}:{} is enabled in subnet: {}", device.id(),
+                     port.number(), subnet);
+        } else {
+            log.info("Device:EdgePort {}:{} is disabled in subnet: {}", device.id(),
+                     port.number(), subnet);
+        }
+
         DefaultGroupHandler groupHandler = groupHandlerMap.get(device.id());
         if (groupHandler != null) {
-            groupHandler.portDown(port.number(),
+            groupHandler.processEdgePort(port.number(), subnet, portUp,
                                   mastershipService.isLocalMaster(device.id()));
+        } else {
+            log.warn("Group handler not found for dev:{}. Not handling edge port"
+                    + " {} event for port:{}", device.id(),
+                    (portUp) ? "UP" : "DOWN", port.number());
         }
     }
 
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 c378541..52606cf 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
@@ -78,13 +78,16 @@
     // 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;
+    // distributed store for (device+subnet-ip-prefix) mapped to next-id
     protected EventuallyConsistentMap<SubnetNextObjectiveStoreKey, Integer>
             subnetNextObjStore = null;
+    // distributed store for (device+port+treatment) mapped to next-id
     protected EventuallyConsistentMap<PortNextObjectiveStoreKey, Integer>
             portNextObjStore = null;
     private SegmentRoutingManager srManager;
@@ -271,7 +274,8 @@
     }
 
     /**
-     * Performs group recovery procedures when a port goes down on this device.
+     * Performs hash group recovery procedures when a switch-to-switch
+     * port goes down on this device.
      *
      * @param port port number that has gone down
      * @param isMaster true if local instance is the master
@@ -350,6 +354,70 @@
     }
 
     /**
+     * Adds or removes a port that has been configured with a subnet to a broadcast group
+     * for bridging. Note that this does not create the broadcast group itself.
+     *
+     * @param port the port on this device that needs to be added/removed to a bcast group
+     * @param subnet the subnet corresponding  to the broadcast group
+     * @param portUp true if port is enabled, false if disabled
+     * @param isMaster true if local instance is the master
+     */
+    public void processEdgePort(PortNumber port, Ip4Prefix subnet,
+                                boolean portUp, boolean isMaster) {
+        if (!isMaster) {
+            return;
+        }
+        //get the next id for the subnet and edit it.
+        Integer nextId = getSubnetNextObjectiveId(subnet);
+        if (nextId == -1) {
+            if (portUp) {
+                log.debug("**Creating flooding group for first port enabled in"
+                        + " subnet {} on dev {} port {}", subnet, deviceId, port);
+                createBcastGroupFromSubnet(subnet, Collections.singletonList(port));
+            } else {
+                log.warn("Could not find flooding group for subnet {} on dev:{} when"
+                        + " removing port:{}", subnet, deviceId, port);
+            }
+            return;
+        }
+
+        log.info("**port{} in device {}: {} Bucket with Port {} to"
+                + " next-id {}", (portUp) ? "UP" : "DOWN", deviceId,
+                                          (portUp) ? "Adding" : "Removing",
+                                          port, nextId);
+        // Create the bucket to be added or removed
+        TrafficTreatment.Builder tBuilder = DefaultTrafficTreatment.builder();
+        tBuilder.popVlan();
+        tBuilder.setOutput(port);
+
+        VlanId assignedVlanId =
+                srManager.getSubnetAssignedVlanId(this.deviceId, subnet);
+        TrafficSelector metadata =
+                DefaultTrafficSelector.builder().matchVlanId(assignedVlanId).build();
+
+        NextObjective.Builder nextObjBuilder = DefaultNextObjective
+                .builder().withId(nextId)
+                .withType(NextObjective.Type.BROADCAST).fromApp(appId)
+                .addTreatment(tBuilder.build())
+                .withMeta(metadata);
+
+        ObjectiveContext context = new DefaultObjectiveContext(
+            (objective) -> log.debug("port {} successfully {} NextObj {} on {}",
+                                     port, (portUp) ? "addedTo" : "removedFrom",
+                                     nextId, deviceId),
+            (objective, error) ->
+            log.warn("port {} failed to {} NextObj {} on {}: {}",
+                     port, (portUp) ? "addTo" : "removeFrom",
+                     nextId, deviceId, error));
+
+        NextObjective nextObj = (portUp) ? nextObjBuilder.addToExisting(context)
+                                         : nextObjBuilder.removeFromExisting(context);
+        log.debug("edgePort processed: Submited next objective {} in device {}",
+                  nextId, deviceId);
+        flowObjectiveService.next(deviceId, nextObj);
+    }
+
+    /**
      * Returns the next objective of type hashed associated with the neighborset.
      * If there is no next objective for this neighborset, this method
      * would create a next objective and return. Optionally metadata can be
@@ -394,7 +462,8 @@
      * Returns the next objective of type broadcast associated with the subnet,
      * or -1 if no such objective exists. Note that this method does NOT create
      * the next objective as a side-effect. It is expected that is objective is
-     * created at startup from network configuration.
+     * created at startup from network configuration. Typically this is used
+     * for L2 flooding within the subnet configured on the switch.
      *
      * @param prefix subnet information
      * @return int if found or -1
@@ -411,7 +480,9 @@
      * 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
-     * the creation of the objective.
+     * 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.
      *
      * @param portNum the port number for the simple next objective
      * @param treatment the actions to apply on the packets (should include outport)
@@ -567,7 +638,7 @@
     }
 
     /**
-     * Creates Groups from a set of NeighborSet given.
+     * Creates hash groups from a set of NeighborSet given.
      *
      * @param nsSet a set of NeighborSet
      * @param meta metadata passed into the creation of a Next Objective
@@ -633,7 +704,8 @@
     }
 
     /**
-     * Creates broadcast groups for all ports in the same configured subnet.
+     * Creates broadcast groups for all ports in the same subnet for
+     * all configured subnets.
      */
     public void createGroupsFromSubnetConfig() {
         Map<Ip4Prefix, List<PortNumber>> subnetPortMap;
@@ -645,43 +717,50 @@
             return;
         }
         // Construct a broadcast group for each subnet
-        subnetPortMap.forEach((subnet, ports) -> {
-            SubnetNextObjectiveStoreKey key =
-                    new SubnetNextObjectiveStoreKey(deviceId, subnet);
+        subnetPortMap.forEach((subnet, ports) -> createBcastGroupFromSubnet(subnet, ports));
+    }
 
-            if (subnetNextObjStore.containsKey(key)) {
-                log.debug("Broadcast group for device {} and subnet {} exists",
-                          deviceId, subnet);
-                return;
-            }
+    /**
+     * Creates a single broadcast group from a given subnet and list of ports.
+     *
+     * @param subnet a configured subnet
+     * @param ports list of ports in the subnet
+     */
+    public void createBcastGroupFromSubnet(Ip4Prefix subnet, List<PortNumber> ports) {
+        SubnetNextObjectiveStoreKey key =
+                new SubnetNextObjectiveStoreKey(deviceId, subnet);
 
-            VlanId assignedVlanId =
-                    srManager.getSubnetAssignedVlanId(this.deviceId, subnet);
-            TrafficSelector metadata =
-                    DefaultTrafficSelector.builder().matchVlanId(assignedVlanId).build();
+        if (subnetNextObjStore.containsKey(key)) {
+            log.debug("Broadcast group for device {} and subnet {} exists",
+                      deviceId, subnet);
+            return;
+        }
 
-            int nextId = flowObjectiveService.allocateNextId();
+        VlanId assignedVlanId =
+                srManager.getSubnetAssignedVlanId(this.deviceId, subnet);
+        TrafficSelector metadata =
+                DefaultTrafficSelector.builder().matchVlanId(assignedVlanId).build();
 
-            NextObjective.Builder nextObjBuilder = DefaultNextObjective
-                    .builder().withId(nextId)
-                    .withType(NextObjective.Type.BROADCAST).fromApp(appId)
-                    .withMeta(metadata);
+        int nextId = flowObjectiveService.allocateNextId();
 
-            ports.forEach(port -> {
-                TrafficTreatment.Builder tBuilder = DefaultTrafficTreatment.builder();
-                tBuilder.popVlan();
-                tBuilder.setOutput(port);
-                nextObjBuilder.addTreatment(tBuilder.build());
-            });
+        NextObjective.Builder nextObjBuilder = DefaultNextObjective
+                .builder().withId(nextId)
+                .withType(NextObjective.Type.BROADCAST).fromApp(appId)
+                .withMeta(metadata);
 
-            NextObjective nextObj = nextObjBuilder.add();
-            flowObjectiveService.next(deviceId, nextObj);
-            log.debug("createGroupFromSubnetConfig: Submited "
-                              + "next objective {} in device {}",
-                      nextId, deviceId);
-
-            subnetNextObjStore.put(key, nextId);
+        ports.forEach(port -> {
+            TrafficTreatment.Builder tBuilder = DefaultTrafficTreatment.builder();
+            tBuilder.popVlan();
+            tBuilder.setOutput(port);
+            nextObjBuilder.addTreatment(tBuilder.build());
         });
+
+        NextObjective nextObj = nextObjBuilder.add();
+        flowObjectiveService.next(deviceId, nextObj);
+        log.debug("createBcastGroupFromSubnet: Submited next objective {} in device {}",
+                  nextId, deviceId);
+
+        subnetNextObjStore.put(key, nextId);
     }
 
     /**
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 acb7a7c..7da0be5 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
@@ -495,7 +495,7 @@
      * As per the OFDPA 2.0 TTP, packets are sent out of ports by using
      * a chain of groups. The broadcast Next Objective passed in by the application
      * has to be broken up into a group chain comprising of an
-     * L2 Flood group whose buckets point to L2 Interface groups.
+     * L2 Flood group or L3 Multicast group, whose buckets point to L2 Interface groups.
      *
      * @param nextObj  the nextObjective of type BROADCAST
      */
@@ -905,13 +905,17 @@
      * @param next the representation of the existing group-chain for this next objective
      */
     protected void addBucketToGroup(NextObjective nextObjective, NextGroup next) {
-        if (nextObjective.type() != NextObjective.Type.HASHED) {
+        if (nextObjective.type() != NextObjective.Type.HASHED &&
+                nextObjective.type() != NextObjective.Type.BROADCAST) {
             log.warn("AddBuckets not applied to nextType:{} in dev:{} for next:{}",
                     nextObjective.type(), deviceId, nextObjective.id());
+            Ofdpa2Pipeline.fail(nextObjective, ObjectiveError.UNSUPPORTED);
             return;
         }
         if (nextObjective.next().size() > 1) {
+            // FIXME - support editing multiple buckets CORD-555
             log.warn("Only one bucket can be added at a time");
+            Ofdpa2Pipeline.fail(nextObjective, ObjectiveError.UNSUPPORTED);
             return;
         }
         // first check to see if bucket being added is not a duplicate of an
@@ -937,36 +941,34 @@
                      newport, deviceId, nextObjective.id());
             return;
         }
+        if (nextObjective.type() == NextObjective.Type.HASHED) {
+            addBucketToHashGroup(nextObjective, allActiveKeys, newport);
+        } else if (nextObjective.type() == NextObjective.Type.BROADCAST) {
+            addBucketToBroadcastGroup(nextObjective, allActiveKeys, newport);
+        }
+    }
 
+    private void addBucketToHashGroup(NextObjective nextObjective,
+                                      List<Deque<GroupKey>> allActiveKeys,
+                                      PortNumber newport) {
         // storage for all group keys in the chain of groups created
         List<Deque<GroupKey>> allGroupKeys = new ArrayList<>();
         List<GroupInfo> unsentGroups = new ArrayList<>();
         createHashBucketChains(nextObjective, allGroupKeys, unsentGroups);
 
-        // now we can create the outermost L3 ECMP group bucket to add
+        // now we can create the bucket to add to the outermost L3 ECMP group
         GroupInfo gi = unsentGroups.get(0); // only one bucket, so only one group-chain
         TrafficTreatment.Builder ttb = DefaultTrafficTreatment.builder();
         ttb.group(new DefaultGroupId(gi.nextGroupDesc.givenGroupId()));
         GroupBucket sbucket = DefaultGroupBucket.createSelectGroupBucket(ttb.build());
 
-        // retrieve the original L3 ECMP group id and description from the first
-        // element in any deque.
-        GroupKey l3ecmpGroupKey = null;
-        if (!allActiveKeys.isEmpty()) {
-            l3ecmpGroupKey = allActiveKeys.get(0).peekFirst();
-        } else {
-            log.warn("Could not determine top level group while trying to"
-                    + "add bucket for port:{} in dev:{} for next:{}",
-                    deviceId, nextObjective.id(), newport);
-            return;
-        }
-        Group l3ecmpGroup = groupService.getGroup(deviceId, l3ecmpGroupKey);
+        // retrieve the original L3 ECMP group
+        Group l3ecmpGroup = retrieveTopLevelGroup(allActiveKeys, nextObjective.id());
         if (l3ecmpGroup == null) {
-            log.warn("Could not find l3 ecmp group while trying to add bucket"
-                    + "for port:{} in dev:{} for next:{}", deviceId,
-                    nextObjective.id(), newport);
+            Ofdpa2Pipeline.fail(nextObjective, ObjectiveError.GROUPMISSING);
             return;
         }
+        GroupKey l3ecmpGroupKey = l3ecmpGroup.appCookie();
         int l3ecmpGroupId = l3ecmpGroup.id().id();
 
         // Although GroupDescriptions are not necessary for adding buckets to
@@ -1001,21 +1003,190 @@
                 Integer.toHexString(gi.innerMostGroupDesc.givenGroupId()), deviceId);
         updatePendingGroups(gi.nextGroupDesc.appCookie(), l3ecmpGce);
         groupService.addGroup(gi.innerMostGroupDesc);
+    }
 
+    private void addBucketToBroadcastGroup(NextObjective nextObj,
+                                        List<Deque<GroupKey>> allActiveKeys,
+                                        PortNumber newport) {
+        VlanId assignedVlan = Ofdpa2Pipeline.readVlanFromSelector(nextObj.meta());
+        if (assignedVlan == null) {
+            log.warn("VLAN ID required by broadcast next obj is missing. "
+                    + "Aborting add bucket to broadcast group for next:{} in dev:{}",
+                    nextObj.id(), deviceId);
+            Ofdpa2Pipeline.fail(nextObj, ObjectiveError.BADPARAMS);
+            return;
+        }
+
+        List<GroupInfo> groupInfos = prepareL2InterfaceGroup(nextObj, assignedVlan);
+
+        IpPrefix ipDst = Ofdpa2Pipeline.readIpDstFromSelector(nextObj.meta());
+        if (ipDst != null) {
+            if (ipDst.isMulticast()) {
+                addBucketToL3MulticastGroup(nextObj, allActiveKeys,
+                                            groupInfos, assignedVlan, newport);
+            } else {
+                log.warn("Broadcast NextObj with non-multicast IP address {}", nextObj);
+                Ofdpa2Pipeline.fail(nextObj, ObjectiveError.BADPARAMS);
+                return;
+            }
+        } else {
+            addBucketToL2FloodGroup(nextObj, allActiveKeys,
+                                    groupInfos, assignedVlan, newport);
+        }
+    }
+
+    private void addBucketToL2FloodGroup(NextObjective nextObj,
+                                         List<Deque<GroupKey>> allActiveKeys,
+                                         List<GroupInfo> groupInfos,
+                                         VlanId assignedVlan,
+                                         PortNumber newport) {
+        // create the bucket to add to the outermost L2 Flood group
+        GroupInfo groupInfo = groupInfos.get(0); // only one bucket to add
+        GroupDescription l2intGrpDesc = groupInfo.nextGroupDesc;
+        TrafficTreatment.Builder ttb = DefaultTrafficTreatment.builder();
+        ttb.group(new DefaultGroupId(l2intGrpDesc.givenGroupId()));
+        GroupBucket abucket = DefaultGroupBucket.createAllGroupBucket(ttb.build());
+        // get the group being edited
+        Group l2floodGroup = retrieveTopLevelGroup(allActiveKeys, nextObj.id());
+        if (l2floodGroup == null) {
+            Ofdpa2Pipeline.fail(nextObj, ObjectiveError.GROUPMISSING);
+            return;
+        }
+        GroupKey l2floodGroupKey = l2floodGroup.appCookie();
+        int l2floodGroupId = l2floodGroup.id().id();
+
+        //ensure assignedVlan applies to the chosen group
+        VlanId expectedVlan = VlanId.vlanId((short) ((l2floodGroupId & 0x0fff0000) >> 16));
+        if (!expectedVlan.equals(assignedVlan)) {
+            log.warn("VLAN ID {} does not match Flood group {} to which bucket is "
+                    + "being added, for next:{} in dev:{}. Abort.", assignedVlan,
+                    Integer.toHexString(l2floodGroupId), nextObj.id(), deviceId);
+            Ofdpa2Pipeline.fail(nextObj, ObjectiveError.BADPARAMS);
+        }
+        GroupDescription l2floodGroupDescription =
+                new DefaultGroupDescription(
+                        deviceId,
+                        GroupDescription.Type.ALL,
+                        new GroupBuckets(Collections.singletonList(abucket)),
+                        l2floodGroupKey,
+                        l2floodGroupId,
+                        nextObj.appId());
+        GroupChainElem l2floodGce = new GroupChainElem(l2floodGroupDescription, 1, true);
+
+        // update original NextGroup with new bucket-chain
+        // If active keys shows only the top-level group without a chain of groups,
+        // then it represents an empty group. Update by replacing empty chain.
+        Deque<GroupKey> newBucketChain = new ArrayDeque<>();
+        newBucketChain.addFirst(groupInfo.nextGroupDesc.appCookie());
+        newBucketChain.addFirst(l2floodGroupKey);
+        if (allActiveKeys.size() == 1 && allActiveKeys.get(0).size() == 1) {
+            allActiveKeys.clear();
+        }
+        allActiveKeys.add(newBucketChain);
+        updatePendingNextObjective(l2floodGroupKey,
+                                   new OfdpaNextGroup(allActiveKeys, nextObj));
+        log.debug("Adding to L2FLOOD: device:{} gid:{} gkey:{} nextId:{}",
+                  deviceId, Integer.toHexString(l2floodGroupId),
+                  l2floodGroupKey, nextObj.id());
+        // send the innermost group
+        log.debug("Sending innermost group {} in group chain on device {} ",
+                Integer.toHexString(groupInfo.innerMostGroupDesc.givenGroupId()),
+                deviceId);
+        updatePendingGroups(groupInfo.nextGroupDesc.appCookie(), l2floodGce);
+        groupService.addGroup(groupInfo.innerMostGroupDesc);
+    }
+
+    private void addBucketToL3MulticastGroup(NextObjective nextObj,
+                                             List<Deque<GroupKey>> allActiveKeys,
+                                             List<GroupInfo> groupInfos,
+                                             VlanId assignedVlan,
+                                             PortNumber newport) {
+        // create the bucket to add to the outermost L3 Multicast group
+        GroupInfo groupInfo = groupInfos.get(0); // only one bucket to add
+        // Points to L3 interface group if there is one.
+        // Otherwise points to L2 interface group directly.
+        GroupDescription nextGroupDesc = (groupInfo.nextGroupDesc != null) ?
+                groupInfo.nextGroupDesc : groupInfo.innerMostGroupDesc;
+        TrafficTreatment.Builder ttb = DefaultTrafficTreatment.builder();
+        ttb.group(new DefaultGroupId(nextGroupDesc.givenGroupId()));
+        GroupBucket abucket = DefaultGroupBucket.createAllGroupBucket(ttb.build());
+
+        // get the group being edited
+        Group l3mcastGroup = retrieveTopLevelGroup(allActiveKeys, nextObj.id());
+        if (l3mcastGroup == null) {
+            Ofdpa2Pipeline.fail(nextObj, ObjectiveError.GROUPMISSING);
+            return;
+        }
+        GroupKey l3mcastGroupKey = l3mcastGroup.appCookie();
+        int l3mcastGroupId = l3mcastGroup.id().id();
+
+        //ensure assignedVlan applies to the chosen group
+        VlanId expectedVlan = VlanId.vlanId((short) ((l3mcastGroupId & 0x0fff0000) >> 16));
+        if (!expectedVlan.equals(assignedVlan)) {
+            log.warn("VLAN ID {} does not match L3 Mcast group {} to which bucket is "
+                    + "being added, for next:{} in dev:{}. Abort.", assignedVlan,
+                    Integer.toHexString(l3mcastGroupId), nextObj.id(), deviceId);
+            Ofdpa2Pipeline.fail(nextObj, ObjectiveError.BADPARAMS);
+        }
+        GroupDescription l3mcastGroupDescription =
+                new DefaultGroupDescription(
+                        deviceId,
+                        GroupDescription.Type.ALL,
+                        new GroupBuckets(Collections.singletonList(abucket)),
+                        l3mcastGroupKey,
+                        l3mcastGroupId,
+                        nextObj.appId());
+        GroupChainElem l3mcastGce = new GroupChainElem(l3mcastGroupDescription,
+                                                       1, true);
+
+        // update original NextGroup with new bucket-chain
+        Deque<GroupKey> newBucketChain = new ArrayDeque<>();
+        newBucketChain.addFirst(groupInfo.innerMostGroupDesc.appCookie());
+        // Add L3 interface group to the chain if there is one.
+        if (!groupInfo.nextGroupDesc.equals(groupInfo.innerMostGroupDesc)) {
+            newBucketChain.addFirst(groupInfo.nextGroupDesc.appCookie());
+        }
+        newBucketChain.addFirst(l3mcastGroupKey);
+        // If active keys shows only the top-level group without a chain of groups,
+        // then it represents an empty group. Update by replacing empty chain.
+        if (allActiveKeys.size() == 1 && allActiveKeys.get(0).size() == 1) {
+            allActiveKeys.clear();
+        }
+        allActiveKeys.add(newBucketChain);
+        updatePendingNextObjective(l3mcastGroupKey,
+                                   new OfdpaNextGroup(allActiveKeys, nextObj));
+
+        updatePendingGroups(groupInfo.nextGroupDesc.appCookie(), l3mcastGce);
+        // Point next group to inner-most group, if any
+        if (!groupInfo.nextGroupDesc.equals(groupInfo.innerMostGroupDesc)) {
+            GroupChainElem innerGce = new GroupChainElem(groupInfo.nextGroupDesc,
+                    1, false);
+            updatePendingGroups(groupInfo.innerMostGroupDesc.appCookie(), innerGce);
+        }
+        log.debug("Adding to L3MCAST: device:{} gid:{} gkey:{} nextId:{}",
+                  deviceId, Integer.toHexString(l3mcastGroupId),
+                  l3mcastGroupKey, nextObj.id());
+        // send the innermost group
+        log.debug("Sending innermost group {} in group chain on device {} ",
+                Integer.toHexString(groupInfo.innerMostGroupDesc.givenGroupId()),
+                deviceId);
+        groupService.addGroup(groupInfo.innerMostGroupDesc);
     }
 
     /**
      * Removes the bucket in the top level group of a possible group-chain. Does
-     * not remove the groups in a group-chain pointed to by this bucket, as they
+     * not remove the groups in the group-chain pointed to by this bucket, as they
      * may be in use (referenced by other groups) elsewhere.
      *
      * @param nextObjective the bucket information for a next group
      * @param next the representation of the existing group-chain for this next objective
      */
     protected void removeBucketFromGroup(NextObjective nextObjective, NextGroup next) {
-        if (nextObjective.type() != NextObjective.Type.HASHED) {
+        if (nextObjective.type() != NextObjective.Type.HASHED &&
+                nextObjective.type() != NextObjective.Type.BROADCAST) {
             log.warn("RemoveBuckets not applied to nextType:{} in dev:{} for next:{}",
                     nextObjective.type(), deviceId, nextObjective.id());
+            Ofdpa2Pipeline.fail(nextObjective, ObjectiveError.UNSUPPORTED);
             return;
         }
         Collection<TrafficTreatment> treatments = nextObjective.next();
@@ -1026,6 +1197,7 @@
         if (portToRemove == null) {
             log.warn("next objective {} has no outport.. cannot remove bucket"
                     + "from group in dev: {}", nextObjective.id(), deviceId);
+            Ofdpa2Pipeline.fail(nextObjective, ObjectiveError.BADPARAMS);
             return;
         }
 
@@ -1049,48 +1221,58 @@
             }
             index++;
         }
-        if (foundChain != null) {
-            //first groupkey is the one we want to modify
-            GroupKey modGroupKey = foundChain.peekFirst();
-            Group modGroup = groupService.getGroup(deviceId, modGroupKey);
-            //second groupkey is the one we wish to remove the reference to
-            GroupKey pointedGroupKey = null;
-            int i = 0;
-            for (GroupKey gk : foundChain) {
-                if (i++ == 1) {
-                    pointedGroupKey = gk;
-                    break;
-                }
-            }
-            Group pointedGroup = groupService.getGroup(deviceId, pointedGroupKey);
-            GroupBucket bucket = DefaultGroupBucket.createSelectGroupBucket(
-                    DefaultTrafficTreatment.builder()
-                            .group(pointedGroup.id())
-                            .build());
-            GroupBuckets removeBuckets = new GroupBuckets(Collections
-                    .singletonList(bucket));
-            log.debug("Removing buckets from group id 0x{} pointing to group id 0x{}"
-                    + "for next id {} in device {}", Integer.toHexString(modGroup.id().id()),
-                    Integer.toHexString(pointedGroup.id().id()), nextObjective.id(), deviceId);
-            groupService.removeBucketsFromGroup(deviceId, modGroupKey,
-                    removeBuckets, modGroupKey,
-                    nextObjective.appId());
-            // update store
-            // If the bucket removed was the last bucket in the group, then
-            // retain an entry for the top level group which still exists.
-            if (allActiveKeys.size() == 1) {
-                ArrayDeque<GroupKey> top = new ArrayDeque<>();
-                top.add(modGroupKey);
-                allActiveKeys.add(top);
-            }
-            allActiveKeys.remove(index);
-            flowObjectiveStore.putNextGroup(nextObjective.id(),
-                                            new OfdpaNextGroup(allActiveKeys,
-                                                               nextObjective));
-        } else {
+        if (foundChain == null) {
             log.warn("Could not find appropriate group-chain for removing bucket"
                     + " for next id {} in dev:{}", nextObjective.id(), deviceId);
+            Ofdpa2Pipeline.fail(nextObjective, ObjectiveError.BADPARAMS);
+            return;
         }
+
+        //first groupkey is the one we want to modify
+        GroupKey modGroupKey = foundChain.peekFirst();
+        Group modGroup = groupService.getGroup(deviceId, modGroupKey);
+        //second groupkey is the one we wish to remove the reference to
+        GroupKey pointedGroupKey = null;
+        int i = 0;
+        for (GroupKey gk : foundChain) {
+            if (i++ == 1) {
+                pointedGroupKey = gk;
+                break;
+            }
+        }
+        Group pointedGroup = groupService.getGroup(deviceId, pointedGroupKey);
+        GroupBucket bucket = null;
+        if (nextObjective.type() == NextObjective.Type.HASHED) {
+            bucket = DefaultGroupBucket.createSelectGroupBucket(
+                                            DefaultTrafficTreatment.builder()
+                                            .group(pointedGroup.id())
+                                            .build());
+        } else {
+            bucket = DefaultGroupBucket.createAllGroupBucket(
+                                            DefaultTrafficTreatment.builder()
+                                            .group(pointedGroup.id())
+                                            .build());
+        }
+        GroupBuckets removeBuckets = new GroupBuckets(Collections
+                                                      .singletonList(bucket));
+        log.debug("Removing buckets from group id 0x{} pointing to group id 0x{} "
+                + "for next id {} in device {}", Integer.toHexString(modGroup.id().id()),
+                Integer.toHexString(pointedGroup.id().id()), nextObjective.id(), deviceId);
+        groupService.removeBucketsFromGroup(deviceId, modGroupKey,
+                                            removeBuckets, modGroupKey,
+                                            nextObjective.appId());
+        // update store
+        // If the bucket removed was the last bucket in the group, then
+        // retain an entry for the top level group which still exists.
+        if (allActiveKeys.size() == 1) {
+            ArrayDeque<GroupKey> top = new ArrayDeque<>();
+            top.add(modGroupKey);
+            allActiveKeys.add(top);
+        }
+        allActiveKeys.remove(index);
+        flowObjectiveStore.putNextGroup(nextObjective.id(),
+                                        new OfdpaNextGroup(allActiveKeys,
+                                                           nextObjective));
     }
 
     /**
@@ -1288,8 +1470,34 @@
         return L2_INTERFACE_TYPE | (TYPE_MASK & hash << 6) | portLowerBits;
     }
 
+    private Group retrieveTopLevelGroup(List<Deque<GroupKey>> allActiveKeys,
+                                        int nextid) {
+        GroupKey topLevelGroupKey = null;
+        if (!allActiveKeys.isEmpty()) {
+            topLevelGroupKey = allActiveKeys.get(0).peekFirst();
+        } else {
+            log.warn("Could not determine top level group while processing"
+                    + "next:{} in dev:{}", nextid, deviceId);
+            return null;
+        }
+        Group topGroup = groupService.getGroup(deviceId, topLevelGroupKey);
+        if (topGroup == null) {
+            log.warn("Could not find top level group while processing "
+                    + "next:{} in dev:{}", nextid, deviceId);
+        }
+        return topGroup;
+    }
+
     /**
      * Utility class for moving group information around.
+     *
+     * Example: Suppose we are trying to create a group-chain A-B-C-D, where
+     * A is the top level group, and D is the inner-most group, typically L2 Interface.
+     * The innerMostGroupDesc is always D. At various stages of the creation
+     * process the nextGroupDesc may be C or B. The nextGroupDesc exists to
+     * inform the referencing group about which group it needs to point to,
+     * and wait for. In some cases the group chain may simply be A-B. In this case,
+     * both innerMostGroupDesc and nextGroupDesc will be B.
      */
     protected class GroupInfo {
         /**
@@ -1301,7 +1509,7 @@
         /**
          * Description of the next group in the group chain.
          * It can be L2 interface, L3 interface, L3 unicast, L3 VPN group.
-         * It is possible that nextGroup is the same as the innerMostGroup.
+         * It is possible that nextGroupDesc is the same as the innerMostGroup.
          */
         private GroupDescription nextGroupDesc;