RouteHandler misbehaved when next hop moves from single location to dual location
sr-device-subnet was not properly maintained and the subnet was not populated
Also implement the unit test for this scenario
Change-Id: I1507ce2457588992ff8aae216339b6bd80c67acd
diff --git a/apps/segmentrouting/app/src/main/java/org/onosproject/segmentrouting/RouteHandler.java b/apps/segmentrouting/app/src/main/java/org/onosproject/segmentrouting/RouteHandler.java
index d930923..8790d93 100644
--- a/apps/segmentrouting/app/src/main/java/org/onosproject/segmentrouting/RouteHandler.java
+++ b/apps/segmentrouting/app/src/main/java/org/onosproject/segmentrouting/RouteHandler.java
@@ -237,30 +237,22 @@
Set<HostLocation> prevLocations = event.prevSubject().locations();
Set<HostLocation> newLocations = event.subject().locations();
+ Set<ConnectPoint> connectPoints = newLocations.stream()
+ .map(l -> (ConnectPoint) l).collect(Collectors.toSet());
+ log.debug("HostMoved. populateSubnet {}, {}", newLocations, prefix);
+ srManager.defaultRoutingHandler.populateSubnet(connectPoints, Sets.newHashSet(prefix));
+
Set<DeviceId> newDeviceIds = newLocations.stream().map(HostLocation::deviceId)
.collect(Collectors.toSet());
// For each old location
Sets.difference(prevLocations, newLocations).forEach(prevLocation -> {
- // Redirect the flows to pair link if configured
- // Note: Do not continue removing any rule
- Optional<DeviceId> pairDeviceId = srManager.getPairDeviceId(prevLocation.deviceId());
- Optional<PortNumber> pairLocalPort = srManager.getPairLocalPort(prevLocation.deviceId());
- if (pairDeviceId.isPresent() && pairLocalPort.isPresent() && newLocations.stream()
- .anyMatch(location -> location.deviceId().equals(pairDeviceId.get()))) {
- // NOTE: Since the pairLocalPort is trunk port, use assigned vlan of original port
- // when the host is untagged
- VlanId vlanId = Optional.ofNullable(srManager.getInternalVlanId(prevLocation)).orElse(hostVlanId);
- log.debug("HostMoved. populateRoute {}, {}, {}, {}", prevLocation, prefix, hostMac, vlanId);
- srManager.defaultRoutingHandler.populateRoute(prevLocation.deviceId(), prefix,
- hostMac, vlanId, pairLocalPort.get());
- return;
- }
-
- // No pair information supplied.
// Remove flows for unchanged IPs only when the host moves from a switch to another.
// Otherwise, do not remove and let the adding part update the old flow
if (!newDeviceIds.contains(prevLocation.deviceId())) {
+ log.debug("HostMoved. removeSubnet {}, {}", prevLocation, prefix);
+ srManager.deviceConfiguration.removeSubnet(prevLocation, prefix);
+
log.debug("HostMoved. revokeRoute {}, {}, {}, {}", prevLocation, prefix, hostMac, hostVlanId);
srManager.defaultRoutingHandler.revokeRoute(prevLocation.deviceId(), prefix,
hostMac, hostVlanId, prevLocation.port());
@@ -269,6 +261,9 @@
// For each new location, add all new IPs.
Sets.difference(newLocations, prevLocations).forEach(newLocation -> {
+ log.debug("HostMoved. addSubnet {}, {}", newLocation, prefix);
+ srManager.deviceConfiguration.addSubnet(newLocation, prefix);
+
log.debug("HostMoved. populateRoute {}, {}, {}, {}", newLocation, prefix, hostMac, hostVlanId);
srManager.defaultRoutingHandler.populateRoute(newLocation.deviceId(), prefix,
hostMac, hostVlanId, newLocation.port());
diff --git a/apps/segmentrouting/app/src/test/java/org/onosproject/segmentrouting/RouteHandlerTest.java b/apps/segmentrouting/app/src/test/java/org/onosproject/segmentrouting/RouteHandlerTest.java
index a727e67..7804e68 100644
--- a/apps/segmentrouting/app/src/test/java/org/onosproject/segmentrouting/RouteHandlerTest.java
+++ b/apps/segmentrouting/app/src/test/java/org/onosproject/segmentrouting/RouteHandlerTest.java
@@ -406,23 +406,25 @@
ROUTE_STORE.put(P1, Sets.newHashSet(RR3));
+ reset(srManager.deviceConfiguration);
+ srManager.deviceConfiguration.removeSubnet(CP2, P1);
+ expectLastCall().once();
+ replay(srManager.deviceConfiguration);
+
HostEvent he = new HostEvent(HostEvent.Type.HOST_MOVED, H3S, H3D);
routeHandler.processHostMovedEvent(he);
- assertEquals(2, ROUTING_TABLE.size());
+ assertEquals(1, ROUTING_TABLE.size());
MockRoutingTableValue rtv1 = ROUTING_TABLE.get(new MockRoutingTableKey(CP1.deviceId(), P1));
- MockRoutingTableValue rtv2 = ROUTING_TABLE.get(new MockRoutingTableKey(CP2.deviceId(), P1));
assertEquals(M3, rtv1.macAddress);
- assertEquals(M3, rtv2.macAddress);
assertEquals(V3, rtv1.vlanId);
- assertEquals(V3, rtv2.vlanId);
assertEquals(CP1.port(), rtv1.portNumber);
- assertEquals(P9, rtv2.portNumber);
// ECMP route table hasn't changed
- assertEquals(2, SUBNET_TABLE.size());
+ assertEquals(1, SUBNET_TABLE.size());
assertTrue(SUBNET_TABLE.get(CP1).contains(P1));
- assertTrue(SUBNET_TABLE.get(CP2).contains(P1));
+
+ verify(srManager.deviceConfiguration);
}
@Test
@@ -448,6 +450,35 @@
}
@Test
+ public void testSingleHomedToDualHomed() {
+ testDualHomedSingleLocationFail();
+
+ reset(srManager.deviceConfiguration);
+ srManager.deviceConfiguration.addSubnet(CP2, P1);
+ expectLastCall().once();
+ replay(srManager.deviceConfiguration);
+
+ HostEvent he = new HostEvent(HostEvent.Type.HOST_MOVED, H3D, H3S);
+ routeHandler.processHostMovedEvent(he);
+
+ assertEquals(2, ROUTING_TABLE.size());
+ MockRoutingTableValue rtv1 = ROUTING_TABLE.get(new MockRoutingTableKey(CP1.deviceId(), P1));
+ MockRoutingTableValue rtv2 = ROUTING_TABLE.get(new MockRoutingTableKey(CP2.deviceId(), P1));
+ assertEquals(M3, rtv1.macAddress);
+ assertEquals(M3, rtv2.macAddress);
+ assertEquals(V3, rtv1.vlanId);
+ assertEquals(V3, rtv2.vlanId);
+ assertEquals(CP1.port(), rtv1.portNumber);
+ assertEquals(CP2.port(), rtv2.portNumber);
+
+ assertEquals(2, SUBNET_TABLE.size());
+ assertTrue(SUBNET_TABLE.get(CP1).contains(P1));
+ assertTrue(SUBNET_TABLE.get(CP2).contains(P1));
+
+ verify(srManager.deviceConfiguration);
+ }
+
+ @Test
public void testTwoSingleHomedRemoved() {
testTwoSingleHomedAdded();