Fix bugs for dynamic interface configuration

- portNextObjStore is not updated when adding or removing portNextObjective
- Group keys for L2IG in flowObjectiveStore are deleted while modifying L2IG, which in turn causes an exception
- L3UG pointing to L2IG, which is already removed, is not removed
- Empty L2FG, with VLAN ID removed from the configuration, is not removed
- Bridging and unicast routing rules for hosts are not updated when changing port VLAN from untagged to tagged and vice versa

Change-Id: I9454fe553ae53e0fc8839a4ad629c0b5b9039a36
diff --git a/app/src/main/java/org/onosproject/segmentrouting/HostHandler.java b/app/src/main/java/org/onosproject/segmentrouting/HostHandler.java
index dcc77f8..a21e327 100644
--- a/app/src/main/java/org/onosproject/segmentrouting/HostHandler.java
+++ b/app/src/main/java/org/onosproject/segmentrouting/HostHandler.java
@@ -707,15 +707,29 @@
         MacAddress mac = host.mac();
         VlanId hostVlanId = host.vlan();
 
-        // Check whether the host vlan is valid for new interface configuration
-        if ((!popVlan && hostVlanId.equals(vlanId)) ||
-                (popVlan && hostVlanId.equals(VlanId.NONE))) {
-            srManager.defaultRoutingHandler.updateBridging(deviceId, portNum, mac, vlanId, popVlan, install);
-            // Update Forwarding objective and corresponding simple Next objective
-            // for each host and IP address connected to given port
-            host.ipAddresses().forEach(ipAddress -> srManager.defaultRoutingHandler.updateFwdObj(
-                    deviceId, portNum, ipAddress.toIpPrefix(), mac, vlanId, popVlan, install)
+        if (!install) {
+            // Do not check the host validity. Just remove all rules corresponding to the vlan id
+            // Revoke forwarding objective for bridging to the host
+            srManager.defaultRoutingHandler.updateBridging(deviceId, portNum, mac, vlanId, popVlan, false);
+
+            // Revoke forwarding objective and corresponding simple Next objective
+            // for each Host and IP address connected to given port
+            host.ipAddresses().forEach(ipAddress ->
+                srManager.routingRulePopulator.updateFwdObj(deviceId, portNum, ipAddress.toIpPrefix(),
+                                                            mac, vlanId, popVlan, false)
             );
+        } else {
+            // Check whether the host vlan is valid for new interface configuration
+            if ((!popVlan && hostVlanId.equals(vlanId)) ||
+                    (popVlan && hostVlanId.equals(VlanId.NONE))) {
+                srManager.defaultRoutingHandler.updateBridging(deviceId, portNum, mac, vlanId, popVlan, true);
+                // Update Forwarding objective and corresponding simple Next objective
+                // for each Host and IP address connected to given port
+                host.ipAddresses().forEach(ipAddress ->
+                    srManager.routingRulePopulator.updateFwdObj(deviceId, portNum, ipAddress.toIpPrefix(),
+                                                                mac, vlanId, popVlan, true)
+                );
+            }
         }
     }
 
diff --git a/app/src/main/java/org/onosproject/segmentrouting/RoutingRulePopulator.java b/app/src/main/java/org/onosproject/segmentrouting/RoutingRulePopulator.java
index 9313f34..b5fcbcb 100644
--- a/app/src/main/java/org/onosproject/segmentrouting/RoutingRulePopulator.java
+++ b/app/src/main/java/org/onosproject/segmentrouting/RoutingRulePopulator.java
@@ -1685,12 +1685,15 @@
         srManager.flowObjectiveService.forward(deviceId, install ? fob.add(context) : fob.remove(context));
 
         if (!install) {
-            DefaultGroupHandler grpHandler = srManager.getGroupHandler(deviceId);
-            if (grpHandler == null) {
-                log.warn("updateFwdObj: groupHandler for device {} not found", deviceId);
-            } else {
-                // Remove L3UG for the given port and host
-                grpHandler.removeGroupFromPort(portNumber, tbuilder.build(), mbuilder.build());
+            if (!srManager.getVlanPortMap(deviceId).containsKey(vlanId) ||
+                    !srManager.getVlanPortMap(deviceId).get(vlanId).contains(portNumber)) {
+                DefaultGroupHandler grpHandler = srManager.getGroupHandler(deviceId);
+                if (grpHandler == null) {
+                    log.warn("updateFwdObj: groupHandler for device {} not found", deviceId);
+                } else {
+                    // Remove L3UG for the given port and host
+                    grpHandler.removeGroupFromPort(portNumber, tbuilder.build(), mbuilder.build());
+                }
             }
         }
     }
diff --git a/app/src/main/java/org/onosproject/segmentrouting/SegmentRoutingManager.java b/app/src/main/java/org/onosproject/segmentrouting/SegmentRoutingManager.java
index 281a3c1..a948bfd 100644
--- a/app/src/main/java/org/onosproject/segmentrouting/SegmentRoutingManager.java
+++ b/app/src/main/java/org/onosproject/segmentrouting/SegmentRoutingManager.java
@@ -2149,6 +2149,11 @@
         if (getVlanNextObjectiveId(deviceId, vlanId) != -1) {
             // Update L2IG bucket of the port
             grpHandler.updateL2InterfaceGroupBucket(portNum, vlanId, pushVlan);
+            // Update bridging and unicast routing rule for each host
+            hostEventExecutor.execute(() -> hostHandler.processIntfVlanUpdatedEvent(deviceId, portNum,
+                    vlanId, !pushVlan, false));
+            hostEventExecutor.execute(() -> hostHandler.processIntfVlanUpdatedEvent(deviceId, portNum,
+                    vlanId, pushVlan, true));
         } else {
             log.warn("Failed to retrieve next objective for vlan {} in device {}:{}", vlanId, deviceId, portNum);
         }
@@ -2171,9 +2176,6 @@
         int nextId = getVlanNextObjectiveId(deviceId, vlanId);
 
         if (nextId != -1 && !install) {
-            // Update next objective for a single port as an output port
-            // Remove a single port from L2FG
-            grpHandler.updateGroupFromVlanConfiguration(vlanId, portNum, nextId, install);
             // Remove L2 Bridging rule and L3 Unicast rule to the host
             hostEventExecutor.execute(() -> hostHandler.processIntfVlanUpdatedEvent(deviceId, portNum,
                     vlanId, pushVlan, install));
@@ -2185,10 +2187,17 @@
                 // Remove L2FG for VLAN
                 grpHandler.removeBcastGroupFromVlan(deviceId, portNum, vlanId, pushVlan);
             } else {
-                // Remove L2IG of the port
-                grpHandler.removePortNextObjective(deviceId, portNum, vlanId, pushVlan);
+                // Remove a single port from L2FG
+                grpHandler.updateGroupFromVlanConfiguration(vlanId, portNum, nextId, install);
             }
+            // Remove L2IG of the port
+            grpHandler.removePortNextObjective(deviceId, portNum, vlanId, pushVlan);
         } else if (install) {
+            // Create L2IG of the port
+            grpHandler.createPortNextObjective(deviceId, portNum, vlanId, pushVlan);
+            // Create L2 Bridging rule and L3 Unicast rule to the host
+            hostEventExecutor.execute(() -> hostHandler.processIntfVlanUpdatedEvent(deviceId, portNum,
+                    vlanId, pushVlan, install));
             if (nextId != -1) {
                 // Add a single port to L2FG
                 grpHandler.updateGroupFromVlanConfiguration(vlanId, portNum, nextId, install);
@@ -2197,8 +2206,6 @@
                 grpHandler.createBcastGroupFromVlan(vlanId, Collections.singleton(portNum));
                 routingRulePopulator.updateSubnetBroadcastRule(deviceId, vlanId, install);
             }
-            hostEventExecutor.execute(() -> hostHandler.processIntfVlanUpdatedEvent(deviceId, portNum,
-                    vlanId, pushVlan, install));
         } else {
             log.warn("Failed to retrieve next objective for vlan {} in device {}:{}", vlanId, deviceId, 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 15513cf..f8d3053 100644
--- a/app/src/main/java/org/onosproject/segmentrouting/grouphandler/DefaultGroupHandler.java
+++ b/app/src/main/java/org/onosproject/segmentrouting/grouphandler/DefaultGroupHandler.java
@@ -1353,6 +1353,27 @@
     }
 
     /**
+     * Creates simple next objective for a single port.
+     *
+     * @param deviceId device id that has the port to deal with
+     * @param portNum the outgoing port on the device
+     * @param vlanId vlan id associated with the port
+     * @param popVlan true if POP_VLAN action is applied on the packets, false otherwise
+     */
+    public void createPortNextObjective(DeviceId deviceId, PortNumber portNum, VlanId vlanId, boolean popVlan) {
+        TrafficSelector.Builder mbuilder = DefaultTrafficSelector.builder();
+        mbuilder.matchVlanId(vlanId);
+
+        TrafficTreatment.Builder tbuilder = DefaultTrafficTreatment.builder();
+        tbuilder.immediate().setOutput(portNum);
+        if (popVlan) {
+            tbuilder.immediate().popVlan();
+        }
+
+        createGroupFromPort(portNum, tbuilder.build(), mbuilder.build());
+    }
+
+    /**
      * Removes simple next objective for a single port.
      *
      * @param deviceId device id that has the port to deal with
@@ -1513,16 +1534,23 @@
      *                 does not involve pop vlan tag action.
      */
     public void updateL2InterfaceGroupBucket(PortNumber portNumber, VlanId vlanId, boolean pushVlan) {
+        TrafficTreatment.Builder oldTBuilder = DefaultTrafficTreatment.builder();
         TrafficTreatment.Builder tBuilder = DefaultTrafficTreatment.builder();
+        tBuilder.setOutput(portNumber);
+        oldTBuilder.setOutput(portNumber);
         if (pushVlan) {
             tBuilder.popVlan();
+        } else {
+            oldTBuilder.popVlan();
         }
-        tBuilder.setOutput(portNumber);
 
         TrafficSelector metadata =
                 DefaultTrafficSelector.builder().matchVlanId(vlanId).build();
 
-        int nextId = getVlanNextObjectiveId(vlanId);
+        // Update portNextObjStore with new L2IG
+        int nextId = getPortNextObjectiveId(portNumber, oldTBuilder.build(), metadata, false);
+        portNextObjStore.remove(new PortNextObjectiveStoreKey(deviceId, portNumber, oldTBuilder.build(), metadata));
+        portNextObjStore.put(new PortNextObjectiveStoreKey(deviceId, portNumber, tBuilder.build(), metadata), nextId);
 
         NextObjective.Builder nextObjBuilder = DefaultNextObjective
                 .builder().withId(nextId)