Dual-homing redirection fix
This commit fixes the bug that prevent initial redirection from happening if the host is added without IP and get updated with an IP later
Change-Id: Ic4e90763e38eff94b1613d90f943f76f5285cf94
diff --git a/apps/segmentrouting/src/main/java/org/onosproject/segmentrouting/HostHandler.java b/apps/segmentrouting/src/main/java/org/onosproject/segmentrouting/HostHandler.java
index 07f6d8d..6ac25f3 100644
--- a/apps/segmentrouting/src/main/java/org/onosproject/segmentrouting/HostHandler.java
+++ b/apps/segmentrouting/src/main/java/org/onosproject/segmentrouting/HostHandler.java
@@ -254,21 +254,44 @@
}
void processHostUpdatedEvent(HostEvent event) {
- MacAddress mac = event.subject().mac();
- VlanId vlanId = event.subject().vlan();
+ MacAddress hostMac = event.subject().mac();
+ VlanId hostVlanId = event.subject().vlan();
Set<HostLocation> locations = event.subject().locations();
Set<IpAddress> prevIps = event.prevSubject().ipAddresses();
Set<IpAddress> newIps = event.subject().ipAddresses();
- log.info("Host {}/{} is updated", mac, vlanId);
+ log.info("Host {}/{} is updated", hostMac, hostVlanId);
locations.stream().filter(srManager::isMasterOf).forEach(location -> {
Sets.difference(prevIps, newIps).forEach(ip ->
- processRoutingRule(location.deviceId(), location.port(), mac, vlanId, ip, true)
+ processRoutingRule(location.deviceId(), location.port(), hostMac, hostVlanId, ip, true)
);
Sets.difference(newIps, prevIps).forEach(ip ->
- processRoutingRule(location.deviceId(), location.port(), mac, vlanId, ip, false)
+ processRoutingRule(location.deviceId(), location.port(), hostMac, hostVlanId, ip, false)
);
});
+
+ // Use the pair link temporarily before the second location of a dual-homed host shows up.
+ // This do not affect single-homed hosts since the flow will be blocked in
+ // processBridgingRule or processRoutingRule due to VLAN or IP mismatch respectively
+ locations.forEach(location -> {
+ srManager.getPairDeviceId(location.deviceId()).ifPresent(pairDeviceId -> {
+ if (srManager.mastershipService.isLocalMaster(pairDeviceId) &&
+ locations.stream().noneMatch(l -> l.deviceId().equals(pairDeviceId))) {
+ srManager.getPairLocalPorts(pairDeviceId).ifPresent(pairRemotePort -> {
+ // NOTE: Since the pairLocalPort is trunk port, use assigned vlan of original port
+ // when the host is untagged
+ VlanId vlanId = Optional.ofNullable(srManager.getInternalVlanId(location)).orElse(hostVlanId);
+
+ Sets.difference(prevIps, newIps).forEach(ip ->
+ processRoutingRule(pairDeviceId, pairRemotePort, hostMac, vlanId, ip, true)
+ );
+ Sets.difference(newIps, prevIps).forEach(ip ->
+ processRoutingRule(pairDeviceId, pairRemotePort, hostMac, vlanId, ip, false)
+ );
+ });
+ }
+ });
+ });
}
/**
diff --git a/apps/segmentrouting/src/test/java/org/onosproject/segmentrouting/HostHandlerTest.java b/apps/segmentrouting/src/test/java/org/onosproject/segmentrouting/HostHandlerTest.java
index 98f6fd0..3b6d00a 100644
--- a/apps/segmentrouting/src/test/java/org/onosproject/segmentrouting/HostHandlerTest.java
+++ b/apps/segmentrouting/src/test/java/org/onosproject/segmentrouting/HostHandlerTest.java
@@ -696,6 +696,80 @@
}
@Test
+ public void testDelayedIpAndLocation() throws Exception {
+ Host host1 = new DefaultHost(PROVIDER_ID, HOST_ID_UNTAGGED, HOST_MAC, HOST_VLAN_UNTAGGED,
+ Sets.newHashSet(HOST_LOC31), Sets.newHashSet(), false);
+ Host host2 = new DefaultHost(PROVIDER_ID, HOST_ID_UNTAGGED, HOST_MAC, HOST_VLAN_UNTAGGED,
+ Sets.newHashSet(HOST_LOC31), Sets.newHashSet(HOST_IP11), false);
+ Host host3 = new DefaultHost(PROVIDER_ID, HOST_ID_UNTAGGED, HOST_MAC, HOST_VLAN_UNTAGGED,
+ Sets.newHashSet(HOST_LOC31, HOST_LOC41), Sets.newHashSet(HOST_IP11), false);
+
+ // Add a dual-home host with only one location and no IP
+ // Expect: only bridging redirection works
+ hostHandler.processHostAddedEvent(new HostEvent(HostEvent.Type.HOST_ADDED, host1));
+ assertEquals(0, ROUTING_TABLE.size());
+ assertEquals(2, BRIDGING_TABLE.size());
+ assertEquals(P1, BRIDGING_TABLE.get(new MockBridgingTableKey(DEV3, HOST_MAC, INTF_VLAN_UNTAGGED)).portNumber);
+ assertEquals(P9, BRIDGING_TABLE.get(new MockBridgingTableKey(DEV4, HOST_MAC, INTF_VLAN_UNTAGGED)).portNumber);
+
+ // Discover IP
+ // Expect: routing redirection should also work
+ hostHandler.processHostAddedEvent(new HostEvent(HostEvent.Type.HOST_UPDATED, host2, host1));
+ assertEquals(2, ROUTING_TABLE.size());
+ assertEquals(P1, ROUTING_TABLE.get(new MockRoutingTableKey(DEV3, HOST_IP11.toIpPrefix())).portNumber);
+ assertEquals(P9, ROUTING_TABLE.get(new MockRoutingTableKey(DEV4, HOST_IP11.toIpPrefix())).portNumber);
+ assertEquals(2, BRIDGING_TABLE.size());
+ assertEquals(P1, BRIDGING_TABLE.get(new MockBridgingTableKey(DEV3, HOST_MAC, INTF_VLAN_UNTAGGED)).portNumber);
+ assertEquals(P9, BRIDGING_TABLE.get(new MockBridgingTableKey(DEV4, HOST_MAC, INTF_VLAN_UNTAGGED)).portNumber);
+
+ // Discover location
+ // Expect: cancel all redirections
+ hostHandler.processHostAddedEvent(new HostEvent(HostEvent.Type.HOST_MOVED, host3, host2));
+ assertEquals(2, ROUTING_TABLE.size());
+ assertEquals(P1, ROUTING_TABLE.get(new MockRoutingTableKey(DEV3, HOST_IP11.toIpPrefix())).portNumber);
+ assertEquals(P1, ROUTING_TABLE.get(new MockRoutingTableKey(DEV4, HOST_IP11.toIpPrefix())).portNumber);
+ assertEquals(2, BRIDGING_TABLE.size());
+ assertEquals(P1, BRIDGING_TABLE.get(new MockBridgingTableKey(DEV3, HOST_MAC, INTF_VLAN_UNTAGGED)).portNumber);
+ assertEquals(P1, BRIDGING_TABLE.get(new MockBridgingTableKey(DEV4, HOST_MAC, INTF_VLAN_UNTAGGED)).portNumber);
+ }
+
+ @Test
+ public void testDelayedIpAndLocation2() throws Exception {
+ Host host1 = new DefaultHost(PROVIDER_ID, HOST_ID_UNTAGGED, HOST_MAC, HOST_VLAN_UNTAGGED,
+ Sets.newHashSet(HOST_LOC31), Sets.newHashSet(), false);
+ Host host2 = new DefaultHost(PROVIDER_ID, HOST_ID_UNTAGGED, HOST_MAC, HOST_VLAN_UNTAGGED,
+ Sets.newHashSet(HOST_LOC31, HOST_LOC41), Sets.newHashSet(), false);
+ Host host3 = new DefaultHost(PROVIDER_ID, HOST_ID_UNTAGGED, HOST_MAC, HOST_VLAN_UNTAGGED,
+ Sets.newHashSet(HOST_LOC31, HOST_LOC41), Sets.newHashSet(HOST_IP11), false);
+
+ // Add a dual-home host with only one location and no IP
+ // Expect: only bridging redirection works
+ hostHandler.processHostAddedEvent(new HostEvent(HostEvent.Type.HOST_ADDED, host1));
+ assertEquals(0, ROUTING_TABLE.size());
+ assertEquals(2, BRIDGING_TABLE.size());
+ assertEquals(P1, BRIDGING_TABLE.get(new MockBridgingTableKey(DEV3, HOST_MAC, INTF_VLAN_UNTAGGED)).portNumber);
+ assertEquals(P9, BRIDGING_TABLE.get(new MockBridgingTableKey(DEV4, HOST_MAC, INTF_VLAN_UNTAGGED)).portNumber);
+
+ // Discover Location
+ // Expect: cancel bridging redirections
+ hostHandler.processHostAddedEvent(new HostEvent(HostEvent.Type.HOST_MOVED, host2, host1));
+ assertEquals(0, ROUTING_TABLE.size());
+ assertEquals(2, BRIDGING_TABLE.size());
+ assertEquals(P1, BRIDGING_TABLE.get(new MockBridgingTableKey(DEV3, HOST_MAC, INTF_VLAN_UNTAGGED)).portNumber);
+ assertEquals(P1, BRIDGING_TABLE.get(new MockBridgingTableKey(DEV4, HOST_MAC, INTF_VLAN_UNTAGGED)).portNumber);
+
+ // Discover IP
+ // Expect: add IP rules to both location
+ hostHandler.processHostAddedEvent(new HostEvent(HostEvent.Type.HOST_UPDATED, host3, host2));
+ assertEquals(2, ROUTING_TABLE.size());
+ assertEquals(P1, ROUTING_TABLE.get(new MockRoutingTableKey(DEV3, HOST_IP11.toIpPrefix())).portNumber);
+ assertEquals(P1, ROUTING_TABLE.get(new MockRoutingTableKey(DEV4, HOST_IP11.toIpPrefix())).portNumber);
+ assertEquals(2, BRIDGING_TABLE.size());
+ assertEquals(P1, BRIDGING_TABLE.get(new MockBridgingTableKey(DEV3, HOST_MAC, INTF_VLAN_UNTAGGED)).portNumber);
+ assertEquals(P1, BRIDGING_TABLE.get(new MockBridgingTableKey(DEV4, HOST_MAC, INTF_VLAN_UNTAGGED)).portNumber);
+ }
+
+ @Test
public void testDualHomedHostUpdated() throws Exception {
Host host1 = new DefaultHost(PROVIDER_ID, HOST_ID_UNTAGGED, HOST_MAC, HOST_VLAN_UNTAGGED,
Sets.newHashSet(HOST_LOC11, HOST_LOC21), Sets.newHashSet(HOST_IP11, HOST_IP12), false);