[CORD-2721] Implement group bucket modification

Change-Id: I0f637ec4ff2b0c12db53d70fed195ea28e542535
diff --git a/app/src/main/java/org/onosproject/segmentrouting/SegmentRoutingManager.java b/app/src/main/java/org/onosproject/segmentrouting/SegmentRoutingManager.java
index b9912ac..f72d11b 100644
--- a/app/src/main/java/org/onosproject/segmentrouting/SegmentRoutingManager.java
+++ b/app/src/main/java/org/onosproject/segmentrouting/SegmentRoutingManager.java
@@ -1568,38 +1568,71 @@
                                Sets.difference(new HashSet<>(prevIntf.ipAddressesList()),
                                                new HashSet<>(intf.ipAddressesList())));
 
-            if (prevIntf.vlanNative() != VlanId.NONE && !intf.vlanNative().equals(prevIntf.vlanNative())) {
-                // RemoveVlanNative
-                updateVlanConfigInternal(deviceId, portNum, prevIntf.vlanNative(), true, false);
+            if (!prevIntf.vlanNative().equals(VlanId.NONE)
+                    && !prevIntf.vlanNative().equals(intf.vlanUntagged())
+                    && !prevIntf.vlanNative().equals(intf.vlanNative())) {
+                if (intf.vlanTagged().contains(prevIntf.vlanNative())) {
+                    // Update filtering objective and L2IG group bucket
+                    updatePortVlanTreatment(deviceId, portNum, prevIntf.vlanNative(), false);
+                } else {
+                    // RemoveVlanNative
+                    updateVlanConfigInternal(deviceId, portNum, prevIntf.vlanNative(), true, false);
+                }
+            }
+
+            if (!prevIntf.vlanUntagged().equals(VlanId.NONE)
+                    && !prevIntf.vlanUntagged().equals(intf.vlanUntagged())
+                    && !prevIntf.vlanUntagged().equals(intf.vlanNative())) {
+                if (intf.vlanTagged().contains(prevIntf.vlanUntagged())) {
+                    // Update filtering objective and L2IG group bucket
+                    updatePortVlanTreatment(deviceId, portNum, prevIntf.vlanUntagged(), false);
+                } else {
+                    // RemoveVlanUntagged
+                    updateVlanConfigInternal(deviceId, portNum, prevIntf.vlanUntagged(), true, false);
+                }
             }
 
             if (!prevIntf.vlanTagged().isEmpty() && !intf.vlanTagged().equals(prevIntf.vlanTagged())) {
                 // RemoveVlanTagged
-                prevIntf.vlanTagged().stream().filter(i -> !intf.vlanTagged().contains(i)).forEach(
-                        vlanId -> updateVlanConfigInternal(deviceId, portNum, vlanId, false, false)
-                );
+                Sets.difference(prevIntf.vlanTagged(), intf.vlanTagged()).stream()
+                        .filter(i -> !intf.vlanUntagged().equals(i))
+                        .filter(i -> !intf.vlanNative().equals(i))
+                        .forEach(vlanId -> updateVlanConfigInternal(
+                                deviceId, portNum, vlanId, false, false));
             }
 
-            if (prevIntf.vlanUntagged() != VlanId.NONE && !intf.vlanUntagged().equals(prevIntf.vlanUntagged())) {
-                // RemoveVlanUntagged
-                updateVlanConfigInternal(deviceId, portNum, prevIntf.vlanUntagged(), true, false);
-            }
-
-            if (intf.vlanNative() != VlanId.NONE && !prevIntf.vlanNative().equals(intf.vlanNative())) {
-                // AddVlanNative
-                updateVlanConfigInternal(deviceId, portNum, intf.vlanNative(), true, true);
+            if (!intf.vlanNative().equals(VlanId.NONE)
+                    && !prevIntf.vlanNative().equals(intf.vlanNative())
+                    && !prevIntf.vlanUntagged().equals(intf.vlanNative())) {
+                if (prevIntf.vlanTagged().contains(intf.vlanNative())) {
+                    // Update filtering objective and L2IG group bucket
+                    updatePortVlanTreatment(deviceId, portNum, intf.vlanNative(), true);
+                } else {
+                    // AddVlanNative
+                    updateVlanConfigInternal(deviceId, portNum, intf.vlanNative(), true, true);
+                }
             }
 
             if (!intf.vlanTagged().isEmpty() && !intf.vlanTagged().equals(prevIntf.vlanTagged())) {
                 // AddVlanTagged
-                intf.vlanTagged().stream().filter(i -> !prevIntf.vlanTagged().contains(i)).forEach(
-                        vlanId -> updateVlanConfigInternal(deviceId, portNum, vlanId, false, true)
+                Sets.difference(intf.vlanTagged(), prevIntf.vlanTagged()).stream()
+                        .filter(i -> !prevIntf.vlanUntagged().equals(i))
+                        .filter(i -> !prevIntf.vlanNative().equals(i))
+                        .forEach(vlanId -> updateVlanConfigInternal(
+                                deviceId, portNum, vlanId, false, true)
                 );
             }
 
-            if (intf.vlanUntagged() != VlanId.NONE && !prevIntf.vlanUntagged().equals(intf.vlanUntagged())) {
-                // AddVlanUntagged
-                updateVlanConfigInternal(deviceId, portNum, intf.vlanUntagged(), true, true);
+            if (!intf.vlanUntagged().equals(VlanId.NONE)
+                    && !prevIntf.vlanUntagged().equals(intf.vlanUntagged())
+                    && !prevIntf.vlanNative().equals(intf.vlanUntagged())) {
+                if (prevIntf.vlanTagged().contains(intf.vlanUntagged())) {
+                    // Update filtering objective and L2IG group bucket
+                    updatePortVlanTreatment(deviceId, portNum, intf.vlanUntagged(), true);
+                } else {
+                    // AddVlanUntagged
+                    updateVlanConfigInternal(deviceId, portNum, intf.vlanUntagged(), true, true);
+                }
             }
             addSubnetConfig(prevIntf.connectPoint(),
                             Sets.difference(new HashSet<>(intf.ipAddressesList()),
@@ -1609,6 +1642,26 @@
         }
     }
 
+    private void updatePortVlanTreatment(DeviceId deviceId, PortNumber portNum,
+                                         VlanId vlanId, boolean pushVlan) {
+        DefaultGroupHandler grpHandler = getGroupHandler(deviceId);
+        if (grpHandler == null) {
+            log.warn("Failed to retrieve group handler for device {}", deviceId);
+            return;
+        }
+
+        // Update filtering objective for a single port
+        routingRulePopulator.updateSinglePortFilters(deviceId, portNum, !pushVlan, vlanId, false);
+        routingRulePopulator.updateSinglePortFilters(deviceId, portNum, pushVlan, vlanId, true);
+
+        if (getVlanNextObjectiveId(deviceId, vlanId) != -1) {
+            // Update L2IG bucket of the port
+            grpHandler.updateL2InterfaceGroupBucket(portNum, vlanId, pushVlan);
+        } else {
+            log.warn("Failed to retrieve next objective for vlan {} in device {}:{}", vlanId, deviceId, portNum);
+        }
+    }
+
     private void updateVlanConfigInternal(DeviceId deviceId, PortNumber portNum,
                                           VlanId vlanId, boolean pushVlan, boolean install) {
         DefaultGroupHandler grpHandler = getGroupHandler(deviceId);
@@ -1628,7 +1681,7 @@
         if (nextId != -1 && !install) {
             // Update next objective for a single port as an output port
             // Remove a single port from L2FG
-            grpHandler.updateGroupFromVlanConfiguration(portNum, Collections.singleton(vlanId), nextId, install);
+            grpHandler.updateGroupFromVlanConfiguration(vlanId, portNum, nextId, install);
             // Remove L2 Bridging rule and L3 Unicast rule to the host
             hostHandler.processIntfVlanUpdatedEvent(deviceId, portNum, vlanId, pushVlan, install);
             // Remove broadcast forwarding rule and corresponding L2FG for VLAN
@@ -1645,7 +1698,7 @@
         } else if (install) {
             if (nextId != -1) {
                 // Add a single port to L2FG
-                grpHandler.updateGroupFromVlanConfiguration(portNum, Collections.singleton(vlanId), nextId, install);
+                grpHandler.updateGroupFromVlanConfiguration(vlanId, portNum, nextId, install);
             } else {
                 // Create L2FG for VLAN
                 grpHandler.createBcastGroupFromVlan(vlanId, Collections.singleton(portNum));
diff --git a/app/src/main/java/org/onosproject/segmentrouting/grouphandler/DefaultGroupHandler.java b/app/src/main/java/org/onosproject/segmentrouting/grouphandler/DefaultGroupHandler.java
index 75cab32..3cf1331 100644
--- a/app/src/main/java/org/onosproject/segmentrouting/grouphandler/DefaultGroupHandler.java
+++ b/app/src/main/java/org/onosproject/segmentrouting/grouphandler/DefaultGroupHandler.java
@@ -1277,12 +1277,56 @@
         bc.run();
     }
 
-    public void updateGroupFromVlanConfiguration(PortNumber portNumber, Collection<VlanId> vlanIds,
-                                                 int nextId, boolean install) {
-        vlanIds.forEach(vlanId -> updateGroupFromVlanInternal(vlanId, portNumber, nextId, install));
+    /**
+     * Modifies L2IG bucket when the interface configuration is updated, especially
+     * when the interface has same VLAN ID but the VLAN type is changed (e.g., from
+     * vlan-tagged [10] to vlan-untagged 10), which requires changes on
+     * TrafficTreatment in turn.
+     *
+     * @param portNumber the port on this device that needs to be updated
+     * @param vlanId the vlan id corresponding to this port
+     * @param pushVlan indicates if packets should be sent out untagged or not out
+     *                 from the port. If true, updated TrafficTreatment involves
+     *                 pop vlan tag action. If false, updated TrafficTreatment
+     *                 does not involve pop vlan tag action.
+     */
+    public void updateL2InterfaceGroupBucket(PortNumber portNumber, VlanId vlanId, boolean pushVlan) {
+        TrafficTreatment.Builder tBuilder = DefaultTrafficTreatment.builder();
+        if (pushVlan) {
+            tBuilder.popVlan();
+        }
+        tBuilder.setOutput(portNumber);
+
+        TrafficSelector metadata =
+                DefaultTrafficSelector.builder().matchVlanId(vlanId).build();
+
+        int nextId = getVlanNextObjectiveId(vlanId);
+
+        NextObjective.Builder nextObjBuilder = DefaultNextObjective
+                .builder().withId(nextId)
+                .withType(NextObjective.Type.SIMPLE).fromApp(appId)
+                .addTreatment(tBuilder.build())
+                .withMeta(metadata);
+
+        ObjectiveContext context = new DefaultObjectiveContext(
+                (objective) -> log.debug("port {} successfully updated NextObj {} on {}",
+                                         portNumber, nextId, deviceId),
+                (objective, error) ->
+                        log.warn("port {} failed to updated NextObj {} on {}: {}",
+                                 portNumber, nextId, deviceId, error));
+
+        flowObjectiveService.next(deviceId, nextObjBuilder.modify(context));
     }
 
-    private void updateGroupFromVlanInternal(VlanId vlanId, PortNumber portNum, int nextId, boolean install) {
+    /**
+     * Adds a single port to the L2FG or removes it from the L2FG.
+     *
+     * @param vlanId the vlan id corresponding to this port
+     * @param portNum the port on this device to be updated
+     * @param nextId the next objective ID for the given vlan id
+     * @param install if true, adds the port to L2FG. If false, removes it from L2FG.
+     */
+    public void updateGroupFromVlanConfiguration(VlanId vlanId, PortNumber portNum, int nextId, boolean install) {
         TrafficTreatment.Builder tBuilder = DefaultTrafficTreatment.builder();
         if (toPopVlan(portNum, vlanId)) {
             tBuilder.popVlan();