Use pair link before the 2nd location of a dual-homed host is not discovered
In addition,
- Improve host added log message
- Improve false cache eviction log message
Change-Id: Iece05d4a2ba76a3da4ad736c4e072ced43fecacc
(cherry picked from commit 9ff637e3089ae2160f53eab52e9cec2ec6ef4c46)
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 7a205ee..539af62 100644
--- a/apps/segmentrouting/src/main/java/org/onosproject/segmentrouting/HostHandler.java
+++ b/apps/segmentrouting/src/main/java/org/onosproject/segmentrouting/HostHandler.java
@@ -86,20 +86,36 @@
void processHostAddedAtLocation(Host host, HostLocation location) {
checkArgument(host.locations().contains(location), "{} is not a location of {}", location, host);
- if (!srManager.isMasterOf(location)) {
- return;
- }
-
- MacAddress mac = host.mac();
- VlanId vlanId = host.vlan();
+ MacAddress hostMac = host.mac();
+ VlanId hostVlanId = host.vlan();
Set<HostLocation> locations = host.locations();
Set<IpAddress> ips = host.ipAddresses();
- log.info("Host {}/{} is added at {}", mac, vlanId, locations);
+ log.info("Host {}/{} is added at {}", hostMac, hostVlanId, locations);
- processBridgingRule(location.deviceId(), location.port(), mac, vlanId, false);
- ips.forEach(ip ->
- processRoutingRule(location.deviceId(), location.port(), mac, vlanId, ip, false)
- );
+ if (srManager.isMasterOf(location)) {
+ processBridgingRule(location.deviceId(), location.port(), hostMac, hostVlanId, false);
+ ips.forEach(ip ->
+ 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
+ srManager.getPairDeviceId(location.deviceId()).ifPresent(pairDeviceId -> {
+ if (srManager.mastershipService.isLocalMaster(pairDeviceId) &&
+ host.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);
+
+ processBridgingRule(pairDeviceId, pairRemotePort, hostMac, vlanId, false);
+ ips.forEach(ip -> processRoutingRule(pairDeviceId, pairRemotePort, hostMac, vlanId,
+ ip, false));
+ });
+ }
+ });
}
void processHostRemovedEvent(HostEvent event) {
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 3e576b95..98f6fd0 100644
--- a/apps/segmentrouting/src/test/java/org/onosproject/segmentrouting/HostHandlerTest.java
+++ b/apps/segmentrouting/src/test/java/org/onosproject/segmentrouting/HostHandlerTest.java
@@ -77,6 +77,7 @@
private static final IpAddress HOST_IP12 = IpAddress.valueOf("10.0.1.2");
private static final IpAddress HOST_IP13 = IpAddress.valueOf("10.0.1.3");
private static final IpAddress HOST_IP14 = IpAddress.valueOf("10.0.1.4");
+ private static final IpAddress HOST_IP32 = IpAddress.valueOf("10.0.3.2");
// Device
private static final DeviceId DEV1 = DeviceId.deviceId("of:0000000000000001");
private static final DeviceId DEV2 = DeviceId.deviceId("of:0000000000000002");
@@ -103,6 +104,8 @@
// Connect Point for dual-homed host failover
private static final ConnectPoint CP31 = new ConnectPoint(DEV3, P1);
private static final HostLocation HOST_LOC31 = new HostLocation(CP31, 0);
+ private static final ConnectPoint CP32 = new ConnectPoint(DEV3, P2);
+ private static final HostLocation HOST_LOC32 = new HostLocation(CP32, 0);
private static final ConnectPoint CP41 = new ConnectPoint(DEV4, P1);
private static final HostLocation HOST_LOC41 = new HostLocation(CP41, 0);
private static final ConnectPoint CP39 = new ConnectPoint(DEV3, P9);
@@ -118,13 +121,17 @@
private static final VlanId INTF_VLAN_NATIVE = VlanId.vlanId((short) 30);
private static final Set<VlanId> INTF_VLAN_PAIR = Sets.newHashSet(VlanId.vlanId((short) 10),
VlanId.vlanId((short) 20), VlanId.vlanId((short) 30));
+ private static final VlanId INTF_VLAN_OTHER = VlanId.vlanId((short) 40);
// Interface subnet
private static final IpPrefix INTF_PREFIX1 = IpPrefix.valueOf("10.0.1.254/24");
private static final IpPrefix INTF_PREFIX2 = IpPrefix.valueOf("10.0.2.254/24");
+ private static final IpPrefix INTF_PREFIX3 = IpPrefix.valueOf("10.0.3.254/24");
private static final InterfaceIpAddress INTF_IP1 =
new InterfaceIpAddress(INTF_PREFIX1.address(), INTF_PREFIX1);
private static final InterfaceIpAddress INTF_IP2 =
new InterfaceIpAddress(INTF_PREFIX2.address(), INTF_PREFIX2);
+ private static final InterfaceIpAddress INTF_IP3 =
+ new InterfaceIpAddress(INTF_PREFIX3.address(), INTF_PREFIX3);
// Interfaces
private static final Interface INTF11 =
new Interface(null, CP11, Lists.newArrayList(INTF_IP1), MacAddress.NONE, null,
@@ -144,6 +151,9 @@
private static final Interface INTF31 =
new Interface(null, CP31, Lists.newArrayList(INTF_IP1), MacAddress.NONE, null,
INTF_VLAN_UNTAGGED, null, null);
+ private static final Interface INTF32 =
+ new Interface(null, CP32, Lists.newArrayList(INTF_IP3), MacAddress.NONE, null,
+ INTF_VLAN_OTHER, null, null);
private static final Interface INTF39 =
new Interface(null, CP39, Lists.newArrayList(INTF_IP1), MacAddress.NONE, null,
null, INTF_VLAN_PAIR, null);
@@ -164,7 +174,7 @@
private static final Set<DeviceId> LOCAL_DEVICES = Sets.newHashSet(DEV1, DEV2, DEV3, DEV4);
// A set of interfaces
private static final Set<Interface> INTERFACES = Sets.newHashSet(INTF11, INTF12, INTF13, INTF21,
- INTF22, INTF31, INTF39, INTF41, INTF49);
+ INTF22, INTF31, INTF32, INTF39, INTF41, INTF49);
@Before
public void setUp() throws Exception {
@@ -297,6 +307,48 @@
}
@Test
+ public void testSingleHomedHostAddedOnPairLeaf() throws Exception {
+ Host host1 = new DefaultHost(PROVIDER_ID, HOST_ID_UNTAGGED, HOST_MAC, HOST_VLAN_UNTAGGED,
+ Sets.newHashSet(HOST_LOC32), Sets.newHashSet(HOST_IP32), false);
+
+ // Add a single-homed host with one location
+ // Expect: the pair link should not be utilized
+ hostHandler.processHostAddedEvent(new HostEvent(HostEvent.Type.HOST_ADDED, host1));
+ assertEquals(1, ROUTING_TABLE.size());
+ assertEquals(P2, ROUTING_TABLE.get(new MockRoutingTableKey(DEV3, HOST_IP32.toIpPrefix())).portNumber);
+ assertEquals(1, BRIDGING_TABLE.size());
+ assertEquals(P2, BRIDGING_TABLE.get(new MockBridgingTableKey(DEV3, HOST_MAC, INTF_VLAN_OTHER)).portNumber);
+ }
+
+ @Test
+ public void testDualHomedHostAddedOneByOne() throws Exception {
+ Host host1 = new DefaultHost(PROVIDER_ID, HOST_ID_UNTAGGED, HOST_MAC, HOST_VLAN_UNTAGGED,
+ Sets.newHashSet(HOST_LOC31), Sets.newHashSet(HOST_IP11), false);
+ Host host2 = 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-homed host with one location
+ // Expect: the pair link is utilized temporarily before the second location is discovered
+ hostHandler.processHostAddedEvent(new HostEvent(HostEvent.Type.HOST_ADDED, 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);
+
+ // Add the second location of dual-homed host
+ // Expect: no longer use the pair link
+ hostHandler.processHostMovedEvent(new HostEvent(HostEvent.Type.HOST_MOVED, host2, host1));
+ 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 testHostRemoved() throws Exception {
Host subject = new DefaultHost(PROVIDER_ID, HOST_ID_UNTAGGED, HOST_MAC, HOST_VLAN_UNTAGGED,
Sets.newHashSet(HOST_LOC11), Sets.newHashSet(HOST_IP11), false);
diff --git a/core/store/dist/src/main/java/org/onosproject/store/host/impl/DistributedHostStore.java b/core/store/dist/src/main/java/org/onosproject/store/host/impl/DistributedHostStore.java
index b5dcc76..8a913cd 100644
--- a/core/store/dist/src/main/java/org/onosproject/store/host/impl/DistributedHostStore.java
+++ b/core/store/dist/src/main/java/org/onosproject/store/host/impl/DistributedHostStore.java
@@ -117,8 +117,9 @@
case EXPIRED:
PendingHostLocation expired = notification.getValue();
if (expired != null) {
- log.info("Evict timeout probe {} from pendingHostLocations", notification.getValue());
- timeoutPendingHostLocation(notification.getKey());
+ if (timeoutPendingHostLocation(notification.getKey())) {
+ log.info("Evict {} from pendingHosts due to probe timeout", notification.getValue());
+ }
}
break;
case EXPLICIT:
@@ -401,11 +402,12 @@
pendingHosts.remove(probeMac);
}
- private void timeoutPendingHostLocation(MacAddress probeMac) {
- pendingHosts.computeIfPresent(probeMac, (k, v) -> {
+ private boolean timeoutPendingHostLocation(MacAddress probeMac) {
+ PendingHostLocation phl = pendingHosts.computeIfPresent(probeMac, (k, v) -> {
v.setExpired(true);
return v;
});
+ return phl != null;
}
private Set<Host> filter(Collection<DefaultHost> collection, Predicate<DefaultHost> predicate) {