Use effectiveLocations rather than locations while processing host events
Change-Id: I5918c2ba6297939453dfbecd46a0d49f23c4d2a9
diff --git a/app/src/main/java/org/onosproject/segmentrouting/HostHandler.java b/app/src/main/java/org/onosproject/segmentrouting/HostHandler.java
index c62e395..dcc77f8 100644
--- a/app/src/main/java/org/onosproject/segmentrouting/HostHandler.java
+++ b/app/src/main/java/org/onosproject/segmentrouting/HostHandler.java
@@ -82,7 +82,7 @@
}
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 @@
}
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) {
- host.locations().forEach(loc -> {
+ if (effectiveLocations(host).size() > 1 || srManager.singleHomedDown) {
+ effectiveLocations(host).forEach(loc -> {
if (srManager.mastershipService.isLocalMaster(loc.deviceId())) {
srManager.linkHandler.checkUplinksForHost(loc);
}
@@ -108,11 +108,11 @@
}
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 @@
// 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 @@
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 @@
}
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 = event.prevSubject().ipAddresses();
- Set<HostLocation> newLocations = host.locations();
+ Set<IpAddress> prevIps = prevHost.ipAddresses();
Set<IpAddress> newIps = host.ipAddresses();
EthType hostTpid = host.tpid();
boolean doubleTaggedHost = isDoubleTaggedHost(host);
@@ -385,7 +395,7 @@
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 @@
*
* @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 @@
}
}
+ // 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 @@
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 @@
* @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 @@
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 @@
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 @@
* @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 @@
* @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 @@
Set<PortNumber> getDualHomedHostPorts(DeviceId deviceId) {
Set<PortNumber> dualHomedLocations = new HashSet<>();
srManager.hostService.getConnectedHosts(deviceId).stream()
- .filter(host -> host.locations().size() == 2)
- .forEach(host -> host.locations().stream()
+ .filter(host -> effectiveLocations(host).size() == 2)
+ .forEach(host -> effectiveLocations(host).stream()
.filter(loc -> loc.deviceId().equals(deviceId))
.forEach(loc -> dualHomedLocations.add(loc.port())));
return dualHomedLocations;
@@ -769,4 +784,14 @@
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();
+ }
+
}
diff --git a/app/src/main/java/org/onosproject/segmentrouting/SegmentRoutingManager.java b/app/src/main/java/org/onosproject/segmentrouting/SegmentRoutingManager.java
index dba6141..97436ab 100644
--- a/app/src/main/java/org/onosproject/segmentrouting/SegmentRoutingManager.java
+++ b/app/src/main/java/org/onosproject/segmentrouting/SegmentRoutingManager.java
@@ -1455,6 +1455,10 @@
} 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) {
diff --git a/app/src/test/java/org/onosproject/segmentrouting/HostHandlerTest.java b/app/src/test/java/org/onosproject/segmentrouting/HostHandlerTest.java
index d97cbc7..0fa9217 100644
--- a/app/src/test/java/org/onosproject/segmentrouting/HostHandlerTest.java
+++ b/app/src/test/java/org/onosproject/segmentrouting/HostHandlerTest.java
@@ -24,6 +24,7 @@
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 @@
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));
+ }
}