Use effectiveLocations rather than locations while processing host events

Change-Id: I5918c2ba6297939453dfbecd46a0d49f23c4d2a9

apps/segmentrouting/app/src/main/java/org/onosproject/segmentrouting/HostHandler.java

@@ -82,7 +82,7 @@ public class HostHandler {
     }
 
     private void initHost(Host host, DeviceId deviceId) {
-        host.locations().forEach(location -> {
+        effectiveLocations(host).forEach(location -> {
             if (location.deviceId().equals(deviceId) ||
                     location.deviceId().equals(srManager.getPairDeviceId(deviceId).orElse(null))) {
                 processHostAddedAtLocation(host, location);
@@ -96,10 +96,10 @@ public class HostHandler {
     }
 
     private void processHostAdded(Host host) {
-        host.locations().forEach(location -> processHostAddedAtLocation(host, location));
+        effectiveLocations(host).forEach(location -> processHostAddedAtLocation(host, location));
         // ensure dual-homed host locations have viable uplinks
-        if (host.locations().size() > 1 || srManager.singleHomedDown) {
+        if (effectiveLocations(host).size() > 1 || srManager.singleHomedDown) {
-            host.locations().forEach(loc -> {
+            effectiveLocations(host).forEach(loc -> {
                 if (srManager.mastershipService.isLocalMaster(loc.deviceId())) {
                     srManager.linkHandler.checkUplinksForHost(loc);
                 }
@@ -108,11 +108,11 @@ public class HostHandler {
     }
 
     void processHostAddedAtLocation(Host host, HostLocation location) {
-        checkArgument(host.locations().contains(location), "{} is not a location of {}", location, host);
+        checkArgument(effectiveLocations(host).contains(location), "{} is not a location of {}", location, host);
 
         MacAddress hostMac = host.mac();
         VlanId hostVlanId = host.vlan();
-        Set<HostLocation> locations = host.locations();
+        Set<HostLocation> locations = effectiveLocations(host);
         Set<IpAddress> ips = host.ipAddresses();
         log.info("Host {}/{} is added at {}", hostMac, hostVlanId, locations);
 
@@ -132,7 +132,7 @@ public class HostHandler {
         // This do not affect single-homed hosts since the flow will be blocked in
         // processBridgingRule or processRoutingRule due to VLAN or IP mismatch respectively
         srManager.getPairDeviceId(location.deviceId()).ifPresent(pairDeviceId -> {
-            if (host.locations().stream().noneMatch(l -> l.deviceId().equals(pairDeviceId))) {
+            if (effectiveLocations(host).stream().noneMatch(l -> l.deviceId().equals(pairDeviceId))) {
                 srManager.getPairLocalPort(pairDeviceId).ifPresent(pairRemotePort -> {
                     // NOTE: Since the pairLocalPort is trunk port, use assigned vlan of original port
                     //       when the host is untagged
@@ -170,7 +170,7 @@ public class HostHandler {
     private void processHostRemoved(Host host) {
         MacAddress hostMac = host.mac();
         VlanId hostVlanId = host.vlan();
-        Set<HostLocation> locations = host.locations();
+        Set<HostLocation> locations = effectiveLocations(host);
         Set<IpAddress> ips = host.ipAddresses();
         log.info("Host {}/{} is removed from {}", hostMac, hostVlanId, locations);
 
@@ -223,12 +223,22 @@ public class HostHandler {
     }
 
     private void processHostMovedEventInternal(HostEvent event) {
+        // This method will be called when one of the following value has changed:
+        // (1) locations (2) auxLocations or (3) both locations and auxLocations.
+        // We only need to proceed when effectiveLocation has changed.
+        Set<HostLocation> newLocations = effectiveLocations(event.subject());
+        Set<HostLocation> prevLocations = effectiveLocations(event.prevSubject());
+
+        if (newLocations.equals(prevLocations)) {
+            log.info("effectiveLocations of {} has not changed. Skipping {}", event.subject().id(), event);
+            return;
+        }
+
         Host host = event.subject();
+        Host prevHost = event.prevSubject();
         MacAddress hostMac = host.mac();
         VlanId hostVlanId = host.vlan();
-        Set<HostLocation> prevLocations = event.prevSubject().locations();
+        Set<IpAddress> prevIps = prevHost.ipAddresses();
-        Set<IpAddress> prevIps = event.prevSubject().ipAddresses();
-        Set<HostLocation> newLocations = host.locations();
         Set<IpAddress> newIps = host.ipAddresses();
         EthType hostTpid = host.tpid();
         boolean doubleTaggedHost = isDoubleTaggedHost(host);
@@ -385,7 +395,7 @@ public class HostHandler {
         MacAddress hostMac = host.mac();
         VlanId hostVlanId = host.vlan();
         EthType hostTpid = host.tpid();
-        Set<HostLocation> locations = host.locations();
+        Set<HostLocation> locations = effectiveLocations(host);
         Set<IpAddress> prevIps = event.prevSubject().ipAddresses();
         Set<IpAddress> newIps = host.ipAddresses();
         log.info("Host {}/{} is updated", hostMac, hostVlanId);
@@ -451,6 +461,7 @@ public class HostHandler {
      *
      * @param cp connect point
      */
+    // TODO Current implementation does not handle dual-homed hosts with auxiliary locations.
     void processPortUp(ConnectPoint cp) {
         if (cp.port().equals(srManager.getPairLocalPort(cp.deviceId()).orElse(null))) {
             return;
@@ -463,6 +474,7 @@ public class HostHandler {
         }
     }
 
+    // TODO Current implementation does not handle dual-homed hosts with auxiliary locations.
     private void probingIfNecessary(Host host, DeviceId pairDeviceId, ConnectPoint cp) {
         if (isHostInVlanOfPort(host, pairDeviceId, cp)) {
             srManager.probingService.probeHost(host, cp, ProbeMode.DISCOVER);
@@ -482,7 +494,7 @@ public class HostHandler {
         Set<VlanId> taggedVlan = srManager.interfaceService.getTaggedVlanId(cp);
 
         return taggedVlan.contains(host.vlan()) ||
-                (internalVlan != null && host.locations().stream()
+                (internalVlan != null && effectiveLocations(host).stream()
                         .filter(l -> l.deviceId().equals(deviceId))
                         .map(srManager::getInternalVlanId)
                         .anyMatch(internalVlan::equals));
@@ -496,6 +508,7 @@ public class HostHandler {
      * @param pairDeviceId pair device id
      * @param pairRemotePort pair remote port
      */
+    // TODO Current implementation does not handle dual-homed hosts with auxiliary locations.
     private void probe(Host host, ConnectPoint location, DeviceId pairDeviceId, PortNumber pairRemotePort) {
         //Check if the host still exists in the host store
         if (hostService.getHost(host.id()) == null) {
@@ -621,7 +634,7 @@ public class HostHandler {
     void populateAllDoubleTaggedHost() {
         log.info("Enabling routing for all double tagged hosts");
         Sets.newHashSet(srManager.hostService.getHosts()).stream().filter(this::isDoubleTaggedHost)
-                .forEach(h -> h.locations().forEach(l ->
+                .forEach(h -> effectiveLocations(h).forEach(l ->
                     h.ipAddresses().forEach(i ->
                         processDoubleTaggedRoutingRule(l.deviceId(), l.port(), h.mac(), h.innerVlan(),
                                 h.vlan(), h.tpid(), i, false)
@@ -633,7 +646,7 @@ public class HostHandler {
     void revokeAllDoubleTaggedHost() {
         log.info("Disabling routing for all double tagged hosts");
         Sets.newHashSet(srManager.hostService.getHosts()).stream().filter(this::isDoubleTaggedHost)
-                .forEach(h -> h.locations().forEach(l ->
+                .forEach(h -> effectiveLocations(h).forEach(l ->
                     h.ipAddresses().forEach(i ->
                         processDoubleTaggedRoutingRule(l.deviceId(), l.port(), h.mac(), h.innerVlan(),
                             h.vlan(), h.tpid(), i, true)
@@ -674,6 +687,7 @@ public class HostHandler {
      * @param popVlan true to pop Vlan tag at TrafficTreatment, false otherwise
      * @param install true to populate the objective, false to revoke
      */
+    // TODO Current implementation does not handle dual-homed hosts with auxiliary locations.
     void processIntfVlanUpdatedEvent(DeviceId deviceId, PortNumber portNum, VlanId vlanId,
                                      boolean popVlan, boolean install) {
         ConnectPoint connectPoint = new ConnectPoint(deviceId, portNum);
@@ -712,6 +726,7 @@ public class HostHandler {
      * @param ipPrefixSet IP Prefixes added or removed
      * @param install true if IP Prefixes added, false otherwise
      */
+    // TODO Current implementation does not handle dual-homed hosts with auxiliary locations.
     void processIntfIpUpdatedEvent(ConnectPoint cp, Set<IpPrefix> ipPrefixSet, boolean install) {
         Set<Host> hosts = hostService.getConnectedHosts(cp);
 
@@ -752,8 +767,8 @@ public class HostHandler {
     Set<PortNumber> getDualHomedHostPorts(DeviceId deviceId) {
         Set<PortNumber> dualHomedLocations = new HashSet<>();
         srManager.hostService.getConnectedHosts(deviceId).stream()
-            .filter(host -> host.locations().size() == 2)
+            .filter(host -> effectiveLocations(host).size() == 2)
-            .forEach(host -> host.locations().stream()
+            .forEach(host -> effectiveLocations(host).stream()
                      .filter(loc -> loc.deviceId().equals(deviceId))
                         .forEach(loc -> dualHomedLocations.add(loc.port())));
         return dualHomedLocations;
@@ -769,4 +784,14 @@ public class HostHandler {
         return !host.innerVlan().equals(VlanId.NONE);
     }
 
+    /**
+     * Returns effective locations of given host.
+     *
+     * @param host host to check
+     * @return auxLocations of the host if exists, or locations of the host otherwise.
+     */
+    Set<HostLocation> effectiveLocations(Host host) {
+        return (host.auxLocations() != null) ? host.auxLocations() : host.locations();
+    }
+
 }

apps/segmentrouting/app/src/main/java/org/onosproject/segmentrouting/SegmentRoutingManager.java

@@ -1455,6 +1455,10 @@ public class SegmentRoutingManager implements SegmentRoutingService {
                 } else if (event.type() == HostEvent.Type.HOST_MOVED) {
                     hostHandler.processHostMovedEvent((HostEvent) event);
                     routeHandler.processHostMovedEvent((HostEvent) event);
+                } else if (event.type() == HostEvent.Type.HOST_AUX_MOVED) {
+                    hostHandler.processHostMovedEvent((HostEvent) event);
+                    // TODO RouteHandler also needs to process this event in order to
+                    //      support nexthops that has auxLocations
                 } else if (event.type() == HostEvent.Type.HOST_REMOVED) {
                     hostHandler.processHostRemovedEvent((HostEvent) event);
                 } else if (event.type() == HostEvent.Type.HOST_UPDATED) {

apps/segmentrouting/app/src/test/java/org/onosproject/segmentrouting/HostHandlerTest.java

@@ -24,6 +24,7 @@ import com.google.common.collect.Sets;
 import org.easymock.EasyMock;
 import org.junit.Before;
 import org.junit.Test;
+import org.onlab.packet.EthType;
 import org.onlab.packet.IpAddress;
 import org.onlab.packet.IpPrefix;
 import org.onlab.packet.MacAddress;
@@ -998,4 +999,16 @@ public class HostHandlerTest {
 
         verify(hostHandler.srManager.probingService);
     }
+
+    @Test
+    public void testEffectiveLocations() {
+        Host regularHost = new DefaultHost(PROVIDER_ID, HOST_ID_UNTAGGED, HOST_MAC, HOST_VLAN_TAGGED,
+                Sets.newHashSet(HOST_LOC11, HOST_LOC12), Sets.newHashSet(HOST_IP11), false);
+        Host auxHost = new DefaultHost(PROVIDER_ID, HOST_ID_UNTAGGED, HOST_MAC, HOST_VLAN_TAGGED,
+                Sets.newHashSet(HOST_LOC11, HOST_LOC12), Sets.newHashSet(HOST_LOC21, HOST_LOC22),
+                Sets.newHashSet(HOST_IP11), VlanId.NONE, EthType.EtherType.UNKNOWN.ethType(), false, false);
+
+        assertEquals(Sets.newHashSet(HOST_LOC11, HOST_LOC12), hostHandler.effectiveLocations(regularHost));
+        assertEquals(Sets.newHashSet(HOST_LOC21, HOST_LOC22), hostHandler.effectiveLocations(auxHost));
+    }
 }