[AETHER-537][AETHER-1031][AETHER-540] Several bugfixes for SR (p2)

This patch includes:
- new method to update the indirect nexthops when the
  internal vlan is changed
- fix update l2 interface scenario (tagged <-> untagged)
- fix a race condition during the removal of fwd objectives
- new method to signal the driver when a port update is happening
- minor fixes in the log messages
- fix two issues in the update of the l3 unicast. One is related
  to the vlan to be used during the nexthop moves. Another issue
  was happening when there was a mismatch between hostVlan and
  vlan configuration on a specific port

Change-Id: I149a03c09daafc015ea4bf131b0968761d02273c
diff --git a/impl/src/main/java/org/onosproject/segmentrouting/DefaultRoutingHandler.java b/impl/src/main/java/org/onosproject/segmentrouting/DefaultRoutingHandler.java
index de3fd73..7a65e95 100644
--- a/impl/src/main/java/org/onosproject/segmentrouting/DefaultRoutingHandler.java
+++ b/impl/src/main/java/org/onosproject/segmentrouting/DefaultRoutingHandler.java
@@ -1405,19 +1405,21 @@
         return CompletableFuture.completedFuture(null);
     }
 
-    void updateBridging(DeviceId deviceId, PortNumber portNum, MacAddress hostMac,
-                        VlanId vlanId, boolean popVlan, boolean install) {
+    CompletableFuture<Objective> updateBridging(DeviceId deviceId, PortNumber portNum, MacAddress hostMac,
+                                                VlanId vlanId, boolean popVlan, boolean install) {
         if (shouldProgram(deviceId)) {
-            srManager.routingRulePopulator.updateBridging(deviceId, portNum, hostMac, vlanId, popVlan, install);
+            return srManager.routingRulePopulator.updateBridging(deviceId, portNum, hostMac, vlanId, popVlan, install);
         }
+        return CompletableFuture.completedFuture(null);
     }
 
-    void updateFwdObj(DeviceId deviceId, PortNumber portNumber, IpPrefix prefix, MacAddress hostMac,
-                      VlanId vlanId, boolean popVlan, boolean install) {
+    CompletableFuture<Objective> updateFwdObj(DeviceId deviceId, PortNumber portNumber, IpPrefix prefix,
+                                              MacAddress hostMac, VlanId vlanId, boolean popVlan, boolean install) {
         if (shouldProgram(deviceId)) {
-            srManager.routingRulePopulator.updateFwdObj(deviceId, portNumber, prefix, hostMac,
+            return srManager.routingRulePopulator.updateFwdObj(deviceId, portNumber, prefix, hostMac,
                     vlanId, popVlan, install);
         }
+        return CompletableFuture.completedFuture(null);
     }
 
     /**
diff --git a/impl/src/main/java/org/onosproject/segmentrouting/HostHandler.java b/impl/src/main/java/org/onosproject/segmentrouting/HostHandler.java
index 46231b0..07e619f 100644
--- a/impl/src/main/java/org/onosproject/segmentrouting/HostHandler.java
+++ b/impl/src/main/java/org/onosproject/segmentrouting/HostHandler.java
@@ -208,12 +208,17 @@
             }
         });
 
+        // NOTE: that we use the nexthop vlanId to retrieve the nextId
+        //       while the vlanId used to program the L3 unicast chain
+        //       is derived from the port configuration. In case of
+        //       a tagged interface we use host vlanId. Host vlan should
+        //       be part of the tags configured for that port. See the
+        //       code in DefaultGroupHandler.updateL3UcastGroupBucket
         int nextId = srManager.getMacVlanNextObjectiveId(location.deviceId(), hostMac, hostVlanId, null, false);
         if (nextId != -1) {
-            VlanId vlanId = Optional.ofNullable(srManager.getInternalVlanId(location)).orElse(hostVlanId);
             log.debug(" Updating next objective for device {}, host {}/{}, port {}, nextid {}",
-                                location.deviceId(), hostMac, vlanId, location.port(), nextId);
-            srManager.updateMacVlanTreatment(location.deviceId(), hostMac, vlanId,
+                                location.deviceId(), hostMac, hostVlanId, location.port(), nextId);
+            srManager.updateMacVlanTreatment(location.deviceId(), hostMac, hostVlanId,
                                 location.port(), nextId);
         }
 
@@ -759,8 +764,21 @@
             return;
         }
 
-        hosts.forEach(host -> hostWorkers.execute(() -> processIntfVlanUpdatedEventInternal(
-                host, deviceId, portNum, vlanId, popVlan, install), host.id().hashCode()));
+        List<CompletableFuture<Void>> hostFutures = Lists.newArrayList();
+        hosts.forEach(host -> hostFutures.add(hostWorkers.submit(() -> processIntfVlanUpdatedEventInternal(
+                host, deviceId, portNum, vlanId, popVlan, install), host.id().hashCode())));
+        // Let's wait for the completion of all hosts
+        try {
+            CompletableFuture.allOf(hostFutures.toArray(new CompletableFuture[0]))
+                    .thenApply(objectives -> hostFutures.stream()
+                            .map(CompletableFuture::join)
+                            .collect(Collectors.toList())
+                    )
+                    .get();
+        } catch (InterruptedException | ExecutionException e) {
+            log.warn("Exception caught when executing processIntfVlanUpdatedEvent futures");
+            hostFutures.forEach(future -> future.cancel(false));
+        }
     }
 
     private void processIntfVlanUpdatedEventInternal(Host host, DeviceId deviceId, PortNumber portNum,
@@ -768,30 +786,43 @@
         MacAddress mac = host.mac();
         VlanId hostVlanId = host.vlan();
 
+        List<CompletableFuture<Objective>> futures = Lists.newArrayList();
         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);
+            futures.add(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)
-            );
+                    futures.add(srManager.defaultRoutingHandler.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);
+                futures.add(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)
-                );
+                    futures.add(srManager.defaultRoutingHandler.updateFwdObj(deviceId, portNum, ipAddress.toIpPrefix(),
+                            mac, vlanId, popVlan, true)));
             }
         }
+        // Let's wait for the completion of update procedure
+        try {
+            CompletableFuture.allOf(futures.toArray(new CompletableFuture[0]))
+                    .thenApply(objectives -> futures.stream()
+                            .map(CompletableFuture::join)
+                            .collect(Collectors.toList())
+                    )
+                    .get();
+        } catch (InterruptedException | ExecutionException e) {
+            log.warn("Exception caught when executing processIntfVlanUpdatedEventInternal futures");
+            futures.forEach(future -> future.cancel(false));
+        }
     }
 
     /**
diff --git a/impl/src/main/java/org/onosproject/segmentrouting/RouteHandler.java b/impl/src/main/java/org/onosproject/segmentrouting/RouteHandler.java
index d9372fe..c0ccfb6 100644
--- a/impl/src/main/java/org/onosproject/segmentrouting/RouteHandler.java
+++ b/impl/src/main/java/org/onosproject/segmentrouting/RouteHandler.java
@@ -22,6 +22,7 @@
 import org.onlab.packet.MacAddress;
 import org.onlab.packet.VlanId;
 import org.onosproject.net.ConnectPoint;
+import org.onosproject.net.Host;
 import org.onosproject.net.HostLocation;
 import org.onosproject.net.PortNumber;
 import org.onosproject.net.host.HostEvent;
@@ -264,20 +265,24 @@
         if (!batchedSubnets.isEmpty()) {
            // For each new location, if NextObj exists for the host, update with new location ..
            Sets.difference(newLocations, prevLocations).forEach(newLocation -> {
-                  int nextId = srManager.getMacVlanNextObjectiveId(newLocation.deviceId(),
-                                                                   hostMac, hostVlanId, null, false);
-                  VlanId vlanId = Optional.ofNullable(srManager.getInternalVlanId(newLocation)).orElse(hostVlanId);
-
-                  if (nextId != -1) {
-                      //Update the nextId group bucket
-                      log.debug("HostMoved. NextId exists, update L3 Ucast Group Bucket {}, {}, {} --> {}",
-                                             newLocation, hostMac, vlanId, nextId);
-                      srManager.updateMacVlanTreatment(newLocation.deviceId(), hostMac, vlanId,
-                                                       newLocation.port(), nextId);
-                  } else {
-                      log.debug("HostMoved. NextId does not exist for this location {}, host {}/{}",
-                                              newLocation, hostMac, vlanId);
-                  }
+                    // NOTE: that we use the nexthop vlanId to retrieve the nextId
+                    //       while the vlanId used to program the L3 unicast chain
+                    //       is derived from the port configuration. In case of
+                    //       a tagged interface we use host vlanId. Host vlan should
+                    //       be part of the tags configured for that port. See the
+                    //       code in DefaultGroupHandler.updateL3UcastGroupBucket
+                    int nextId = srManager.getMacVlanNextObjectiveId(newLocation.deviceId(),
+                            hostMac, hostVlanId, null, false);
+                    if (nextId != -1) {
+                        //Update the nextId group bucket
+                        log.debug("HostMoved. NextId exists, update L3 Ucast Group Bucket {}, {}, {} --> {}",
+                                newLocation, hostMac, hostVlanId, nextId);
+                        srManager.updateMacVlanTreatment(newLocation.deviceId(), hostMac, hostVlanId,
+                                newLocation.port(), nextId);
+                    } else {
+                        log.debug("HostMoved. NextId does not exist for this location {}, host {}/{}",
+                                newLocation, hostMac, hostVlanId);
+                    }
            });
         }
 
@@ -327,6 +332,40 @@
 
     }
 
+    public void processIntfVlanUpdatedEvent(DeviceId deviceId, PortNumber portNum) {
+        log.info("processIntfVlanUpdatedEvent {}/{}", deviceId, portNum);
+
+        // Verify there are hosts attached to the port
+        ConnectPoint connectPoint = new ConnectPoint(deviceId, portNum);
+        Set<Host> hosts = srManager.hostService.getConnectedHosts(connectPoint);
+        if (hosts == null || hosts.size() == 0) {
+            log.debug("processIntfVlanUpdatedEvent: No hosts connected to {}", connectPoint);
+            return;
+        }
+
+        // Iterate over the hosts and update if needed the vlan of the nextobjective
+        hosts.forEach(host -> {
+            // Verify if there is a nexthop and only then update the l3 indirect unicast chain
+            int nextId = srManager.getMacVlanNextObjectiveId(deviceId, host.mac(), host.vlan(),
+                    null, false);
+            if (nextId != -1) {
+                //Update the nextId group bucket
+                log.debug("intfVlanUpdated. NextId exists, update L3 Ucast Group Bucket {}, {}, {} --> {}",
+                        connectPoint, host.mac(), host.vlan(), nextId);
+                // NOTE: that we use the nexthop vlanId to retrieve the nextId
+                //       while the vlanId used to program the L3 unicast chain
+                //       is derived from the port configuration. In case of
+                //       a tagged interface we use host vlanId. Host vlan should
+                //       be part of the tags configured for that port. See the
+                //       code in DefaultGroupHandler.updateL3UcastGroupBucket
+                srManager.updateMacVlanTreatment(deviceId, host.mac(), host.vlan(), portNum, nextId);
+            } else {
+                log.debug("intfVlanUpdated. NextId does not exist for this location {}, host {}/{}",
+                        connectPoint, host.mac(), host.vlan());
+            }
+        });
+    }
+
     private boolean isReady() {
         return Objects.nonNull(srManager.deviceConfiguration) &&
                 Objects.nonNull(srManager.defaultRoutingHandler);
diff --git a/impl/src/main/java/org/onosproject/segmentrouting/RoutingRulePopulator.java b/impl/src/main/java/org/onosproject/segmentrouting/RoutingRulePopulator.java
index c3ecfb9..751bbcc 100644
--- a/impl/src/main/java/org/onosproject/segmentrouting/RoutingRulePopulator.java
+++ b/impl/src/main/java/org/onosproject/segmentrouting/RoutingRulePopulator.java
@@ -67,6 +67,7 @@
 import java.util.Optional;
 import java.util.Set;
 import java.util.concurrent.CompletableFuture;
+import java.util.concurrent.ExecutionException;
 import java.util.concurrent.atomic.AtomicLong;
 import java.util.stream.Collectors;
 
@@ -78,7 +79,6 @@
 import static org.onlab.packet.ICMP6.ROUTER_ADVERTISEMENT;
 import static org.onlab.packet.ICMP6.ROUTER_SOLICITATION;
 import static org.onlab.packet.IPv6.PROTOCOL_ICMP6;
-import static org.onosproject.segmentrouting.SegmentRoutingService.DEFAULT_PRIORITY;
 
 /**
  * Populator of segment routing flow rules.
@@ -95,7 +95,9 @@
 
     // used for signalling the driver to remove vlan table and tmac entry also
     private static final long CLEANUP_DOUBLE_TAGGED_HOST_ENTRIES = 1;
-    private static final long DOUBLE_TAGGED_METADATA_MASK = 0xffffffffffffffffL;
+    // used for signalling the driver when not remove tmac entries
+    private static final long INTERFACE_CONFIG_UPDATE = 2;
+    private static final long METADATA_MASK = 0xffffffffffffffffL;
 
     /**
      * Creates a RoutingRulePopulator object.
@@ -232,7 +234,7 @@
                 mbuilder.matchVlanId(nativeVlan);
                 tbuilder.immediate().popVlan();
             } else {
-                log.warn("Untagged host {}/{} is not allowed on {} without untagged or native" +
+                log.warn("Untagged host {}/{} is not allowed on {} without untagged or native " +
                         "vlan config", mac, hostVlanId, connectPoint);
                 return null;
             }
@@ -272,10 +274,11 @@
      * @param vlanId Vlan ID configured on the switch port
      * @param popVlan true to pop Vlan tag at TrafficTreatment, false otherwise
      * @param install true to populate the objective, false to revoke
+     * @return a completable future that completes when the update of the bridging rule completes
      */
     // TODO Refactor. There are a lot of duplications between this method, populateBridging,
     //      revokeBridging and bridgingFwdObjBuilder.
-    void updateBridging(DeviceId deviceId, PortNumber portNum, MacAddress hostMac,
+    CompletableFuture<Objective> updateBridging(DeviceId deviceId, PortNumber portNum, MacAddress hostMac,
                         VlanId vlanId, boolean popVlan, boolean install) {
         // Create host selector
         TrafficSelector.Builder sbuilder = DefaultTrafficSelector.builder();
@@ -297,6 +300,7 @@
 
         int portNextObjId = srManager.getPortNextObjectiveId(deviceId, portNum,
                 tbuilder.build(), mbuilder.build(), install);
+        CompletableFuture<Objective> future = new CompletableFuture<>();
         if (portNextObjId != -1) {
             ForwardingObjective.Builder fob = DefaultForwardingObjective.builder()
                     .withFlag(ForwardingObjective.Flag.SPECIFIC)
@@ -307,14 +311,21 @@
                     .makePermanent();
 
             ObjectiveContext context = new DefaultObjectiveContext(
-                    (objective) -> log.debug("Brigding rule for {}/{} {}", hostMac, vlanId,
-                            install ? "populated" : "revoked"),
-                    (objective, error) -> log.warn("Failed to {} bridging rule for {}/{}: {}",
-                            install ? "populate" : "revoke", hostMac, vlanId, error));
+                    (objective) -> {
+                        log.debug("Brigding rule for {}/{} {}", hostMac, vlanId, install ? "populated" : "revoked");
+                        future.complete(objective);
+                    },
+                    (objective, error) -> {
+                        log.warn("Failed to {} bridging rule for {}/{}: {}", install ? "populate" : "revoke",
+                                hostMac, vlanId, error);
+                        future.complete(null);
+                    });
             srManager.flowObjectiveService.forward(deviceId, install ? fob.add(context) : fob.remove(context));
         } else {
             log.warn("Failed to retrieve next objective for {}/{}", hostMac, vlanId);
+            return CompletableFuture.completedFuture(null);
         }
+        return future;
     }
 
     /**
@@ -1053,31 +1064,34 @@
         if (taggedVlans.size() != 0) {
             // Filter for tagged vlans
             if (!srManager.interfaceService.getTaggedVlanId(connectPoint).stream().allMatch(taggedVlanId ->
-                    processSinglePortFiltersInternal(deviceId, portnum, false, taggedVlanId, install))) {
+                    processSinglePortFiltersInternal(deviceId, portnum, false, taggedVlanId,
+                            install, false))) {
                 return false;
             }
             if (nativeVlan != null) {
                 // Filter for native vlan
-                if (!processSinglePortFiltersInternal(deviceId, portnum, true, nativeVlan, install)) {
+                if (!processSinglePortFiltersInternal(deviceId, portnum, true, nativeVlan,
+                        install, false)) {
                     return false;
                 }
             }
         } else if (untaggedVlan != null) {
             // Filter for untagged vlan
-            if (!processSinglePortFiltersInternal(deviceId, portnum, true, untaggedVlan, install)) {
+            if (!processSinglePortFiltersInternal(deviceId, portnum, true, untaggedVlan,
+                    install, false)) {
                 return false;
             }
         } else if (!hasIPConfiguration(connectPoint)) {
             // Filter for unconfigured upstream port, using INTERNAL_VLAN
             if (!processSinglePortFiltersInternal(deviceId, portnum, true,
                                                   srManager.getDefaultInternalVlan(),
-                                                  install)) {
+                                                  install, false)) {
                 return false;
             }
             // Filter for receiveing pseudowire traffic
             if (!processSinglePortFiltersInternal(deviceId, portnum, false,
                                                   srManager.getPwTransportVlan(),
-                                                  install)) {
+                                                  install, false)) {
                 return false;
             }
         }
@@ -1095,14 +1109,19 @@
      */
     void updateSinglePortFilters(DeviceId deviceId, PortNumber portNum,
                                  boolean pushVlan, VlanId vlanId, boolean install) {
-        if (!processSinglePortFiltersInternal(deviceId, portNum, pushVlan, vlanId, install)) {
+        // NOTE: update port filters is used only when the vlan configuration
+        //       has been updated. Port configuration is never removed here.
+        //       For pipeline sharing the TMAC entry among different vlans
+        //       there is no need to push or remove the TMAC entry
+        if (!processSinglePortFiltersInternal(deviceId, portNum, pushVlan, vlanId, install, true)) {
             log.warn("Failed to update FilteringObjective for {}/{} with vlan {}",
                      deviceId, portNum, vlanId);
         }
     }
 
     private boolean processSinglePortFiltersInternal(DeviceId deviceId, PortNumber portnum,
-                                                      boolean pushVlan, VlanId vlanId, boolean install) {
+                                                     boolean pushVlan, VlanId vlanId, boolean install,
+                                                     boolean update) {
         boolean doTMAC = true;
 
         if (!pushVlan) {
@@ -1119,7 +1138,7 @@
             }
         }
 
-        FilteringObjective.Builder fob = buildFilteringObjective(deviceId, portnum, pushVlan, vlanId, doTMAC);
+        FilteringObjective.Builder fob = buildFilteringObjective(deviceId, portnum, pushVlan, vlanId, doTMAC, update);
         if (fob == null) {
             // error encountered during build
             return false;
@@ -1140,7 +1159,8 @@
     }
 
     private FilteringObjective.Builder buildFilteringObjective(DeviceId deviceId, PortNumber portnum,
-                                                               boolean pushVlan, VlanId vlanId, boolean doTMAC) {
+                                                               boolean pushVlan, VlanId vlanId,
+                                                               boolean doTMAC, boolean update) {
         MacAddress deviceMac;
         try {
             deviceMac = config.getDeviceMac(deviceId);
@@ -1175,6 +1195,13 @@
             tBuilder.wipeDeferred();
         }
 
+        // NOTE: Some switch hardware share the same tmac flow among different vlans.
+        //       We use this metadata to let the driver know that there is still a vlan
+        //       configuration associated to that port
+        if (update) {
+            tBuilder.writeMetadata(INTERFACE_CONFIG_UPDATE, METADATA_MASK);
+        }
+
         fob.withMeta(tBuilder.build());
 
         fob.permit().fromApp(srManager.appId);
@@ -1257,9 +1284,9 @@
 
         // special metadata for driver
         if (cleanupDoubleTaggedRules) {
-            tBuilder.writeMetadata(CLEANUP_DOUBLE_TAGGED_HOST_ENTRIES, DOUBLE_TAGGED_METADATA_MASK);
+            tBuilder.writeMetadata(CLEANUP_DOUBLE_TAGGED_HOST_ENTRIES, METADATA_MASK);
         } else {
-            tBuilder.writeMetadata(0, DOUBLE_TAGGED_METADATA_MASK);
+            tBuilder.writeMetadata(0, METADATA_MASK);
         }
 
         // NOTE: Some switch hardware share the same filtering flow among different ports.
@@ -1677,9 +1704,11 @@
                 .fromApp(srManager.appId)
                 .makePermanent();
         ObjectiveContext context = new DefaultObjectiveContext(
-                (objective) -> log.debug("Vlan broadcast rule for {} populated", vlanId),
+                (objective) -> log.debug("Vlan broadcast rule for {} {}", vlanId, install ?
+                        "populated" : "removed"),
                 (objective, error) ->
-                        log.warn("Failed to populate vlan broadcast rule for {}: {}", vlanId, error));
+                        log.warn("Failed to {} vlan broadcast rule for {}: {}", install ? "populate" : "remove",
+                                vlanId, error));
 
         if (install) {
             srManager.flowObjectiveService.forward(deviceId, fob.add(context));
@@ -1705,9 +1734,11 @@
      * @param vlanId Vlan ID of the port
      * @param popVlan true to pop vlan tag in TrafficTreatment
      * @param install true to populate the forwarding objective, false to revoke
+     * @return a completable future that completes when the fwdobj completes. In case of removal,
+     * the completable future completes when also the nextobj completes.
      */
-    void updateFwdObj(DeviceId deviceId, PortNumber portNumber, IpPrefix prefix, MacAddress hostMac,
-                      VlanId vlanId, boolean popVlan, boolean install) {
+    CompletableFuture<Objective> updateFwdObj(DeviceId deviceId, PortNumber portNumber, IpPrefix prefix,
+                                              MacAddress hostMac, VlanId vlanId, boolean popVlan, boolean install) {
         ForwardingObjective.Builder fob;
         TrafficSelector.Builder sbuilder = buildIpSelectorFromIpPrefix(prefix);
         MacAddress deviceMac;
@@ -1715,7 +1746,7 @@
             deviceMac = config.getDeviceMac(deviceId);
         } catch (DeviceConfigNotFoundException e) {
             log.warn(e.getMessage() + " Aborting updateFwdObj.");
-            return;
+            return CompletableFuture.completedFuture(null);
         }
 
         TrafficTreatment.Builder tbuilder = DefaultTrafficTreatment.builder();
@@ -1738,9 +1769,10 @@
         // the next-objective does not exist, then a new one should not be created
         int portNextObjId = srManager.getPortNextObjectiveId(deviceId, portNumber,
                                                              tbuilder.build(), mbuilder.build(), install);
+        CompletableFuture<Objective> future = new CompletableFuture<>();
         if (portNextObjId == -1) {
             // Warning log will come from getPortNextObjective method
-            return;
+            return CompletableFuture.completedFuture(null);
         }
 
         fob = DefaultForwardingObjective.builder().withSelector(sbuilder.build())
@@ -1748,10 +1780,15 @@
                 .withPriority(getPriorityFromPrefix(prefix)).withFlag(ForwardingObjective.Flag.SPECIFIC);
 
         ObjectiveContext context = new DefaultObjectiveContext(
-                (objective) -> log.debug("IP rule for route {} {}", prefix, install ? "installed" : "revoked"),
-                (objective, error) ->
-                        log.warn("Failed to {} IP rule for route {}: {}",
-                                 install ? "install" : "revoke", prefix, error));
+                (objective) -> {
+                    log.debug("IP rule for route {} {}", prefix, install ? "installed" : "revoked");
+                    future.complete(objective);
+                },
+                (objective, error) -> {
+                    log.warn("Failed to {} IP rule for route {}: {}", install ? "install" : "revoke",
+                            prefix, error);
+                    future.complete(null);
+                });
         srManager.flowObjectiveService.forward(deviceId, install ? fob.add(context) : fob.remove(context));
 
         if (!install) {
@@ -1761,11 +1798,19 @@
                 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());
+                    // Before moving forward we have to be sure flow has been removed;
+                    try {
+                        future.get();
+                    } catch (InterruptedException | ExecutionException e) {
+                        log.warn("Exception caught when executing IP rule removal for route {}", prefix);
+                    }
+                    // Remove L3UG for the given port and host and return the future
+                    // Flow future has been already consumed (normally or exceptionally)
+                    return grpHandler.removeGroupFromPort(portNumber, tbuilder.build(), mbuilder.build());
                 }
             }
         }
+        return future;
     }
 
     /**
@@ -1793,42 +1838,6 @@
     }
 
     /**
-     * Returns a forwarding objective builder for egress forwarding rules.
-     * <p>
-     * The forwarding objective installs flow rules to egress pipeline to push
-     * two vlan headers with given inner, outer vlan ids and outer tpid.
-     *
-     * @param portNumber port where the next hop attaches to
-     * @param dummyVlanId vlan ID of the packet to match
-     * @param innerVlan inner vlan ID of the next hop
-     * @param outerVlan outer vlan ID of the next hop
-     * @param outerTpid outer TPID of the next hop
-     * @return forwarding objective builder
-     */
-    private ForwardingObjective.Builder egressFwdObjBuilder(PortNumber portNumber, VlanId dummyVlanId,
-                                                            VlanId innerVlan, VlanId outerVlan, EthType outerTpid) {
-        TrafficSelector.Builder sbuilder = DefaultTrafficSelector.builder();
-        sbuilder.matchVlanId(dummyVlanId);
-        TrafficTreatment.Builder tbuilder = DefaultTrafficTreatment.builder();
-        tbuilder.setOutput(portNumber).setVlanId(innerVlan);
-
-        if (outerTpid.equals(EthType.EtherType.QINQ.ethType())) {
-            tbuilder.pushVlan(outerTpid);
-        } else {
-            tbuilder.pushVlan();
-        }
-
-        tbuilder.setVlanId(outerVlan);
-        return DefaultForwardingObjective.builder()
-                .withSelector(sbuilder.build())
-                .withTreatment(tbuilder.build())
-                .fromApp(srManager.appId)
-                .makePermanent()
-                .withPriority(DEFAULT_PRIORITY)
-                .withFlag(ForwardingObjective.Flag.EGRESS);
-    }
-
-    /**
      * Populates IP rules for a route that has double-tagged next hop.
      *
      * @param deviceId device ID of the device that next hop attaches to
@@ -1884,7 +1893,6 @@
         ForwardingObjective.Builder fwdBuilder;
         log.debug("Revoking direct routing entry for double-tagged host route {} at {}:{}",
                 prefix, deviceId, outPort);
-
         try {
             fwdBuilder = routingFwdObjBuilder(deviceId, prefix, hostMac, outerVlan, outPort, innerVlan, outerTpid,
                     true, true);
@@ -1901,7 +1909,6 @@
         int nextId = fwdBuilder.remove().nextId();
         DefaultObjectiveContext context = new DefaultObjectiveContext(objective -> {
             log.debug("Direct routing rule for double-tagged host route {} revoked. nextId={}", prefix, nextId);
-
             // Try to remove next objective as well
             ImmutablePair<TrafficTreatment, TrafficSelector> treatmentAndMeta;
             try {
@@ -1910,12 +1917,10 @@
                 log.error(e.getMessage() + " Aborting revokeDoubleTaggedRoute");
                 return;
             }
-
             if (treatmentAndMeta == null) {
                 // Warning log will come from getTreatmentAndMeta method
                 return;
             }
-
             DefaultGroupHandler groupHandler = srManager.getGroupHandler(deviceId);
             if (groupHandler == null) {
                 log.warn("Failed to revoke direct routing rule for double-tagged host route {}: " +
diff --git a/impl/src/main/java/org/onosproject/segmentrouting/SegmentRoutingManager.java b/impl/src/main/java/org/onosproject/segmentrouting/SegmentRoutingManager.java
index 4e15cad..9e98218 100644
--- a/impl/src/main/java/org/onosproject/segmentrouting/SegmentRoutingManager.java
+++ b/impl/src/main/java/org/onosproject/segmentrouting/SegmentRoutingManager.java
@@ -146,9 +146,11 @@
 import java.util.concurrent.CompletableFuture;
 import java.util.concurrent.ConcurrentHashMap;
 import java.util.concurrent.CopyOnWriteArrayList;
+import java.util.concurrent.ExecutionException;
 import java.util.concurrent.ExecutorService;
 import java.util.concurrent.Executors;
 import java.util.concurrent.ScheduledExecutorService;
+import java.util.concurrent.ScheduledFuture;
 import java.util.concurrent.TimeUnit;
 import java.util.concurrent.atomic.AtomicBoolean;
 import java.util.stream.Collectors;
@@ -1314,7 +1316,7 @@
      * @param nextId of Next Obj which needs to be updated.
      */
     public void updateMacVlanTreatment(DeviceId deviceId, MacAddress hostMac,
-                             VlanId hostVlanId, PortNumber port, int nextId) {
+                                       VlanId hostVlanId, PortNumber port, int nextId) {
         // Check if we are the king of this device
         // just one instance should perform this update
         if (!defaultRoutingHandler.shouldProgram(deviceId)) {
@@ -2104,6 +2106,9 @@
 
             DeviceId deviceId = intf.connectPoint().deviceId();
             PortNumber portNum = intf.connectPoint().port();
+            // We need to do nexthop update al least one time for each
+            // interface config change. There is no difference when it is done;
+            boolean updateNexthop = false;
 
             removeSubnetConfig(prevIntf.connectPoint(),
                                Sets.difference(new HashSet<>(prevIntf.ipAddressesList()),
@@ -2116,8 +2121,12 @@
                     // Update filtering objective and L2IG group bucket
                     updatePortVlanTreatment(deviceId, portNum, prevIntf.vlanNative(), false);
                 } else {
-                    // RemoveVlanNative
+                    // RemoveVlanNative - affected scenarios:
+                    // (T,N)->U; (T*,N)->U; (T,N)->(T,N); (T,N)->T
                     updateVlanConfigInternal(deviceId, portNum, prevIntf.vlanNative(), true, false);
+                    // Update the nexthops of the indirect routes
+                    updateNexthop = true;
+                    routeEventExecutor.execute(() -> routeHandler.processIntfVlanUpdatedEvent(deviceId, portNum));
                 }
             }
 
@@ -2125,16 +2134,27 @@
                     && !prevIntf.vlanUntagged().equals(intf.vlanUntagged())
                     && !prevIntf.vlanUntagged().equals(intf.vlanNative())) {
                 if (intf.vlanTagged().contains(prevIntf.vlanUntagged())) {
-                    // Update filtering objective and L2IG group bucket
+                    // Update filtering objective and L2IG group bucket - affected scenarios:
+                    // U->(T*,N); U->T*
                     updatePortVlanTreatment(deviceId, portNum, prevIntf.vlanUntagged(), false);
+                    if (!updateNexthop) {
+                        updateNexthop = true;
+                        routeEventExecutor.execute(() -> routeHandler.processIntfVlanUpdatedEvent(deviceId, portNum));
+                    }
                 } else {
-                    // RemoveVlanUntagged
+                    // RemoveVlanUntagged - affected scenarios:
+                    // U->U; U->(T,N); U->T
                     updateVlanConfigInternal(deviceId, portNum, prevIntf.vlanUntagged(), true, false);
+                    if (!updateNexthop) {
+                        updateNexthop = true;
+                        routeEventExecutor.execute(() -> routeHandler.processIntfVlanUpdatedEvent(deviceId, portNum));
+                    }
                 }
             }
 
             if (!prevIntf.vlanTagged().isEmpty() && !intf.vlanTagged().equals(prevIntf.vlanTagged())) {
-                // RemoveVlanTagged
+                // RemoveVlanTagged - affected scenarios:
+                // T->U; T->T; (T,N*)->U; (T,N)->(T,N)
                 Sets.difference(prevIntf.vlanTagged(), intf.vlanTagged()).stream()
                         .filter(i -> !intf.vlanUntagged().equals(i))
                         .filter(i -> !intf.vlanNative().equals(i))
@@ -2149,30 +2169,45 @@
                     // Update filtering objective and L2IG group bucket
                     updatePortVlanTreatment(deviceId, portNum, intf.vlanNative(), true);
                 } else {
-                    // AddVlanNative
+                    // AddVlanNative - affected scenarios
+                    // U->(T,N); U->(T*,N); T->(T,N)
                     updateVlanConfigInternal(deviceId, portNum, intf.vlanNative(), true, true);
+                    if (!updateNexthop) {
+                        updateNexthop = true;
+                        routeEventExecutor.execute(() -> routeHandler.processIntfVlanUpdatedEvent(deviceId, portNum));
+                    }
                 }
             }
 
             if (!intf.vlanTagged().isEmpty() && !intf.vlanTagged().equals(prevIntf.vlanTagged())) {
-                // AddVlanTagged
+                // AddVlanTagged - affected scenarios
+                // U->T; U->(T,N*); T->T; (T,N)->(T,N)
                 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().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
+                    // Update filtering objective and L2IG group bucket - affected scenarios
+                    // (T*,N)->U; T*->U
                     updatePortVlanTreatment(deviceId, portNum, intf.vlanUntagged(), true);
+                    if (!updateNexthop) {
+                        routeEventExecutor.execute(() -> routeHandler.processIntfVlanUpdatedEvent(deviceId, portNum));
+                    }
                 } else {
-                    // AddVlanUntagged
+                    // AddVlanUntagged - affected scenarios
+                    // U->U; (T,N)->U; T->U
                     updateVlanConfigInternal(deviceId, portNum, intf.vlanUntagged(), true, true);
+                    if (!updateNexthop) {
+                        routeEventExecutor.execute(() -> routeHandler.processIntfVlanUpdatedEvent(deviceId, portNum));
+                    }
                 }
             }
             addSubnetConfig(prevIntf.connectPoint(),
@@ -2198,11 +2233,6 @@
         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);
         }
@@ -2226,8 +2256,9 @@
 
         if (nextId != -1 && !install) {
             // Remove L2 Bridging rule and L3 Unicast rule to the host
-            hostEventExecutor.execute(() -> hostHandler.processIntfVlanUpdatedEvent(deviceId, portNum,
-                    vlanId, pushVlan, install));
+            ScheduledFuture<?> future = hostEventExecutor.schedule(
+                    () -> hostHandler.processIntfVlanUpdatedEvent(deviceId, portNum, vlanId, pushVlan, install),
+                    0, TimeUnit.SECONDS);
             // Remove broadcast forwarding rule and corresponding L2FG for VLAN
             // only if there is no port configured on that VLAN ID
             if (!getVlanPortMap(deviceId).containsKey(vlanId)) {
@@ -2239,6 +2270,12 @@
                 // Remove a single port from L2FG
                 grpHandler.updateGroupFromVlanConfiguration(vlanId, portNum, nextId, install);
             }
+            // Before moving forward we have to be sure that processIntfVlanUpdatedEvent completed;
+            try {
+                future.get();
+            } catch (InterruptedException | ExecutionException e) {
+                log.warn("Exception caught when executing updateVlanConfigInternal future");
+            }
             // Remove L2IG of the port
             grpHandler.removePortNextObjective(deviceId, portNum, vlanId, pushVlan);
         } else if (install) {
diff --git a/impl/src/main/java/org/onosproject/segmentrouting/grouphandler/DefaultGroupHandler.java b/impl/src/main/java/org/onosproject/segmentrouting/grouphandler/DefaultGroupHandler.java
index 9c3b713..76ca8b7 100644
--- a/impl/src/main/java/org/onosproject/segmentrouting/grouphandler/DefaultGroupHandler.java
+++ b/impl/src/main/java/org/onosproject/segmentrouting/grouphandler/DefaultGroupHandler.java
@@ -38,6 +38,7 @@
 import org.onosproject.net.flowobjective.DefaultObjectiveContext;
 import org.onosproject.net.flowobjective.FlowObjectiveService;
 import org.onosproject.net.flowobjective.NextObjective;
+import org.onosproject.net.flowobjective.Objective;
 import org.onosproject.net.flowobjective.ObjectiveContext;
 import org.onosproject.net.link.LinkService;
 import org.onosproject.segmentrouting.DefaultRoutingHandler;
@@ -60,6 +61,7 @@
 import java.util.List;
 import java.util.Map;
 import java.util.Set;
+import java.util.concurrent.CompletableFuture;
 import java.util.concurrent.ConcurrentHashMap;
 import java.util.concurrent.ScheduledExecutorService;
 import java.util.concurrent.TimeUnit;
@@ -1475,12 +1477,14 @@
      * @param portNum  the outgoing port on the device
      * @param treatment the actions applied on the packets (should include outport)
      * @param meta optional data to pass to the driver
+     * @return a completable future that completes when the port has been removed
      */
-    public void removeGroupFromPort(PortNumber portNum, TrafficTreatment treatment,
-                                    TrafficSelector meta) {
+    public CompletableFuture<Objective> removeGroupFromPort(PortNumber portNum, TrafficTreatment treatment,
+                                                            TrafficSelector meta) {
         PortNextObjectiveStoreKey key = new PortNextObjectiveStoreKey(
                 deviceId, portNum, treatment, meta);
         Integer nextId = portNextObjStore.get(key);
+        CompletableFuture<Objective> future = new CompletableFuture<>();
 
         NextObjective.Builder nextObjBuilder = DefaultNextObjective
                 .builder().withId(nextId)
@@ -1490,12 +1494,14 @@
                 .withMeta(meta);
 
         ObjectiveContext context = new DefaultObjectiveContext(
-                (objective) ->
-                        log.info("removeGroupFromPort installed "
-                                          + "NextObj {} on {}", nextId, deviceId),
+                (objective) -> {
+                    log.info("removeGroupFromPort done " + "NextObj {} on {}", nextId, deviceId);
+                    future.complete(objective);
+                },
                 (objective, error) -> {
                     log.warn("removeGroupFromPort failed to install NextObj {} on {}: {}", nextId, deviceId, error);
                     srManager.invalidateNextObj(objective.id());
+                    future.complete(null);
                 }
         );
         NextObjective nextObj = nextObjBuilder.remove(context);
@@ -1504,6 +1510,7 @@
                           + "for port {}", nextId, deviceId, portNum);
 
         portNextObjStore.remove(key);
+        return future;
     }
 
     /**
@@ -1627,6 +1634,10 @@
                         hostMac, hostVlanId, connectPoint);
                 return;
             }
+        } else {
+            log.warn("Tagged nexthop {}/{} is not allowed on {} without VLAN listed"
+                    + " in tagged vlan", hostMac, hostVlanId, connectPoint);
+            return;
         }
 
         log.debug(" update L3Ucast : deviceMac {}, port {}, host {}/{}, nextid {}, Treatment {} Meta {}",