Remove host location when port down or device down
Also refactor Host Location Provider
Change-Id: I57d682ee51e80ddd7e141883521a12da705a336d
diff --git a/apps/dhcp/app/src/test/java/org/onosproject/dhcp/impl/DhcpManagerTest.java b/apps/dhcp/app/src/test/java/org/onosproject/dhcp/impl/DhcpManagerTest.java
index 3f27109..934e601 100644
--- a/apps/dhcp/app/src/test/java/org/onosproject/dhcp/impl/DhcpManagerTest.java
+++ b/apps/dhcp/app/src/test/java/org/onosproject/dhcp/impl/DhcpManagerTest.java
@@ -34,6 +34,7 @@
import org.onosproject.dhcp.IpAssignment;
import org.onosproject.net.Host;
import org.onosproject.net.HostId;
+import org.onosproject.net.HostLocation;
import org.onosproject.net.config.NetworkConfigRegistryAdapter;
import org.onosproject.net.host.HostDescription;
import org.onosproject.net.host.HostProvider;
@@ -357,6 +358,10 @@
}
+ @Override
+ public void removeLocationFromHost(HostId hostId, HostLocation location) {
+
+ }
}
/**
diff --git a/core/api/src/main/java/org/onosproject/net/host/HostProviderService.java b/core/api/src/main/java/org/onosproject/net/host/HostProviderService.java
index dfb0ac2..9125049 100644
--- a/core/api/src/main/java/org/onosproject/net/host/HostProviderService.java
+++ b/core/api/src/main/java/org/onosproject/net/host/HostProviderService.java
@@ -17,6 +17,7 @@
import org.onlab.packet.IpAddress;
import org.onosproject.net.HostId;
+import org.onosproject.net.HostLocation;
import org.onosproject.net.provider.ProviderService;
/**
@@ -49,4 +50,11 @@
*/
void removeIpFromHost(HostId hostId, IpAddress ipAddress);
+ /**
+ * Notifies the core when a location is no longer associated with a host.
+ *
+ * @param hostId id of the host
+ * @param location location of host that vanished
+ */
+ void removeLocationFromHost(HostId hostId, HostLocation location);
}
diff --git a/core/api/src/main/java/org/onosproject/net/host/HostStore.java b/core/api/src/main/java/org/onosproject/net/host/HostStore.java
index e73fbb7..e8a3e56 100644
--- a/core/api/src/main/java/org/onosproject/net/host/HostStore.java
+++ b/core/api/src/main/java/org/onosproject/net/host/HostStore.java
@@ -22,6 +22,7 @@
import org.onosproject.net.DeviceId;
import org.onosproject.net.Host;
import org.onosproject.net.HostId;
+import org.onosproject.net.HostLocation;
import org.onosproject.net.provider.ProviderId;
import org.onosproject.store.Store;
@@ -64,6 +65,14 @@
HostEvent removeIp(HostId hostId, IpAddress ipAddress);
/**
+ * Removes the specified location from the host entry.
+ *
+ * @param hostId host identification
+ * @param location location to be removed
+ */
+ void removeLocation(HostId hostId, HostLocation location);
+
+ /**
* Returns the number of hosts in the store.
*
* @return host count
diff --git a/core/common/src/test/java/org/onosproject/store/trivial/SimpleHostStore.java b/core/common/src/test/java/org/onosproject/store/trivial/SimpleHostStore.java
index 98a9252..89214e4 100644
--- a/core/common/src/test/java/org/onosproject/store/trivial/SimpleHostStore.java
+++ b/core/common/src/test/java/org/onosproject/store/trivial/SimpleHostStore.java
@@ -167,10 +167,16 @@
@Override
public HostEvent removeIp(HostId hostId, IpAddress ipAddress) {
+ // TODO implement this
return null;
}
@Override
+ public void removeLocation(HostId hostId, HostLocation location) {
+ hosts.get(hostId).locations().remove(location);
+ }
+
+ @Override
public int getHostCount() {
return hosts.size();
}
diff --git a/core/net/src/main/java/org/onosproject/net/host/impl/HostManager.java b/core/net/src/main/java/org/onosproject/net/host/impl/HostManager.java
index 06f3120..ee0fc4c 100644
--- a/core/net/src/main/java/org/onosproject/net/host/impl/HostManager.java
+++ b/core/net/src/main/java/org/onosproject/net/host/impl/HostManager.java
@@ -30,16 +30,18 @@
import org.onlab.util.Tools;
import org.onosproject.cfg.ComponentConfigService;
import org.onosproject.incubator.net.intf.InterfaceService;
-import org.onosproject.net.ConnectPoint;
-import org.onosproject.net.DeviceId;
-import org.onosproject.net.Host;
-import org.onosproject.net.HostId;
+import org.onosproject.net.HostLocation;
+import org.onosproject.net.edge.EdgePortService;
+import org.onosproject.net.provider.AbstractListenerProviderRegistry;
import org.onosproject.net.config.NetworkConfigEvent;
import org.onosproject.net.config.NetworkConfigListener;
import org.onosproject.net.config.NetworkConfigService;
import org.onosproject.net.config.basics.BasicHostConfig;
+import org.onosproject.net.ConnectPoint;
+import org.onosproject.net.DeviceId;
+import org.onosproject.net.Host;
+import org.onosproject.net.HostId;
import org.onosproject.net.device.DeviceService;
-import org.onosproject.net.edge.EdgePortService;
import org.onosproject.net.host.HostAdminService;
import org.onosproject.net.host.HostDescription;
import org.onosproject.net.host.HostEvent;
@@ -51,7 +53,6 @@
import org.onosproject.net.host.HostStore;
import org.onosproject.net.host.HostStoreDelegate;
import org.onosproject.net.packet.PacketService;
-import org.onosproject.net.provider.AbstractListenerProviderRegistry;
import org.onosproject.net.provider.AbstractProviderService;
import org.osgi.service.component.ComponentContext;
import org.slf4j.Logger;
@@ -413,8 +414,8 @@
checkValidity();
Host host = store.getHost(hostId);
- // Disallow removing inexistent host or host provided by others
- if (host == null || !host.providerId().equals(provider().id())) {
+ if (!allowedToChange(hostId)) {
+ log.info("Request to remove {} is ignored due to provider mismatch", hostId);
return;
}
@@ -430,8 +431,35 @@
public void removeIpFromHost(HostId hostId, IpAddress ipAddress) {
checkNotNull(hostId, HOST_ID_NULL);
checkValidity();
+
+ if (!allowedToChange(hostId)) {
+ log.info("Request to remove {} from {} is ignored due to provider mismatch",
+ ipAddress, hostId);
+ return;
+ }
+
store.removeIp(hostId, ipAddress);
}
+
+ @Override
+ public void removeLocationFromHost(HostId hostId, HostLocation location) {
+ checkNotNull(hostId, HOST_ID_NULL);
+ checkValidity();
+
+ if (!allowedToChange(hostId)) {
+ log.info("Request to remove {} from {} is ignored due to provider mismatch",
+ location, hostId);
+ return;
+ }
+
+ store.removeLocation(hostId, location);
+ }
+
+ private boolean allowedToChange(HostId hostId) {
+ // Disallow removing inexistent host or host provided by others
+ Host host = store.getHost(hostId);
+ return host != null && host.providerId().equals(provider().id());
+ }
}
// Store delegate to re-post events emitted from the store.
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 64d24a7..76d14e7 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
@@ -266,6 +266,29 @@
}
@Override
+ public void removeLocation(HostId hostId, HostLocation location) {
+ hosts.compute(hostId, (id, existingHost) -> {
+ if (existingHost != null) {
+ checkState(Objects.equals(hostId.mac(), existingHost.mac()),
+ "Existing and new MAC addresses differ.");
+ checkState(Objects.equals(hostId.vlanId(), existingHost.vlan()),
+ "Existing and new VLANs differ.");
+
+ Set<HostLocation> locations = new HashSet<>(existingHost.locations());
+ locations.remove(location);
+
+ // Remove entire host if we are removing the last location
+ return locations.isEmpty() ? null :
+ new DefaultHost(existingHost.providerId(),
+ hostId, existingHost.mac(), existingHost.vlan(),
+ locations, existingHost.ipAddresses(),
+ existingHost.configured(), existingHost.annotations());
+ }
+ return null;
+ });
+ }
+
+ @Override
public int getHostCount() {
return hosts.size();
}
diff --git a/providers/host/src/main/java/org/onosproject/provider/host/impl/HostLocationProvider.java b/providers/host/src/main/java/org/onosproject/provider/host/impl/HostLocationProvider.java
index 65472dd..97b49a4 100644
--- a/providers/host/src/main/java/org/onosproject/provider/host/impl/HostLocationProvider.java
+++ b/providers/host/src/main/java/org/onosproject/provider/host/impl/HostLocationProvider.java
@@ -46,6 +46,7 @@
import org.onosproject.core.CoreService;
import org.onosproject.net.ConnectPoint;
import org.onosproject.net.Device;
+import org.onosproject.net.DeviceId;
import org.onosproject.net.Host;
import org.onosproject.net.HostId;
import org.onosproject.net.HostLocation;
@@ -77,7 +78,6 @@
import java.nio.ByteBuffer;
import java.util.Dictionary;
-import java.util.Set;
import java.util.concurrent.ExecutorService;
import static java.util.concurrent.Executors.newSingleThreadScheduledExecutor;
@@ -365,9 +365,7 @@
packetService.emit(outboundPacket);
}
- /*
- * This method is using source ip as 0.0.0.0 , to receive the reply even from the sub net hosts.
- */
+ // This method is using source ip as 0.0.0.0 , to receive the reply even from the sub net hosts.
private Ethernet buildArpRequest(IpAddress targetIp, Host host) {
ARP arp = new ARP();
@@ -393,36 +391,19 @@
private class InternalHostProvider implements PacketProcessor {
/**
- * Updates host location only.
+ * Create or update host information.
+ * Will not update IP if IP is null, all zero or self-assigned.
*
* @param hid host ID
* @param mac source Mac address
* @param vlan VLAN ID
* @param hloc host location
+ * @param ip source IP address or null if not updating
*/
- private void updateLocation(HostId hid, MacAddress mac,
- VlanId vlan, HostLocation hloc) {
- HostDescription desc = new DefaultHostDescription(mac, vlan, hloc);
- try {
- providerService.hostDetected(hid, desc, false);
- } catch (IllegalStateException e) {
- log.debug("Host {} suppressed", hid);
- }
- }
-
- /**
- * Updates host location and IP address.
- *
- * @param hid host ID
- * @param mac source Mac address
- * @param vlan VLAN ID
- * @param hloc host location
- * @param ip source IP address
- */
- private void updateLocationIP(HostId hid, MacAddress mac,
- VlanId vlan, HostLocation hloc,
- IpAddress ip) {
- HostDescription desc = ip.isZero() || ip.isSelfAssigned() ?
+ private void createOrUpdateHost(HostId hid, MacAddress mac,
+ VlanId vlan, HostLocation hloc,
+ IpAddress ip) {
+ HostDescription desc = ip == null || ip.isZero() || ip.isSelfAssigned() ?
new DefaultHostDescription(mac, vlan, hloc) :
new DefaultHostDescription(mac, vlan, hloc, ip);
try {
@@ -433,20 +414,20 @@
}
/**
- * Updates host IP address for an existing host.
+ * Updates IP address for an existing host.
*
* @param hid host ID
* @param ip IP address
*/
- private void updateIp(HostId hid, IpAddress ip) {
+ private void updateHostIp(HostId hid, IpAddress ip) {
Host host = hostService.getHost(hid);
if (host == null) {
log.debug("Fail to update IP for {}. Host does not exist");
return;
}
- HostDescription desc =
- new DefaultHostDescription(hid.mac(), hid.vlanId(), host.location(), ip);
+ HostDescription desc = new DefaultHostDescription(hid.mac(), hid.vlanId(),
+ host.location(), ip);
try {
providerService.hostDetected(hid, desc, false);
} catch (IllegalStateException e) {
@@ -492,7 +473,7 @@
ARP arp = (ARP) eth.getPayload();
IpAddress ip = IpAddress.valueOf(IpAddress.Version.INET,
arp.getSenderProtocolAddress());
- updateLocationIP(hid, srcMac, vlan, hloc, ip);
+ createOrUpdateHost(hid, srcMac, vlan, hloc, ip);
// IPv4: update location only
// DHCP ACK: additionally update IP of DHCP client
@@ -512,12 +493,12 @@
MacAddress hostMac = MacAddress.valueOf(dhcp.getClientHardwareAddress());
VlanId hostVlan = VlanId.vlanId(eth.getVlanID());
HostId hostId = HostId.hostId(hostMac, hostVlan);
- updateIp(hostId, IpAddress.valueOf(dhcp.getYourIPAddress()));
+ updateHostIp(hostId, IpAddress.valueOf(dhcp.getYourIPAddress()));
}
}
}
}
- updateLocation(hid, srcMac, vlan, hloc);
+ createOrUpdateHost(hid, srcMac, vlan, hloc, null);
//
// NeighborAdvertisement and NeighborSolicitation: possible
@@ -552,7 +533,7 @@
return;
}
// NeighborSolicitation, NeighborAdvertisement
- updateLocationIP(hid, srcMac, vlan, hloc, ip);
+ createOrUpdateHost(hid, srcMac, vlan, hloc, ip);
return;
}
}
@@ -563,7 +544,7 @@
}
// normal IPv6 packets
- updateLocation(hid, srcMac, vlan, hloc);
+ createOrUpdateHost(hid, srcMac, vlan, hloc, null);
}
}
}
@@ -581,9 +562,8 @@
case DEVICE_ADDED:
break;
case DEVICE_AVAILABILITY_CHANGED:
- if (hostRemovalEnabled &&
- !deviceService.isAvailable(device.id())) {
- removeHosts(hostService.getConnectedHosts(device.id()));
+ if (hostRemovalEnabled && !deviceService.isAvailable(device.id())) {
+ processDeviceDown(device.id());
}
break;
case DEVICE_SUSPENDED:
@@ -592,16 +572,14 @@
break;
case DEVICE_REMOVED:
if (hostRemovalEnabled) {
- removeHosts(hostService.getConnectedHosts(device.id()));
+ processDeviceDown(device.id());
}
break;
case PORT_ADDED:
break;
case PORT_UPDATED:
- if (hostRemovalEnabled) {
- ConnectPoint point =
- new ConnectPoint(device.id(), event.port().number());
- removeHosts(hostService.getConnectedHosts(point));
+ if (hostRemovalEnabled && !event.port().isEnabled()) {
+ processPortDown(new ConnectPoint(device.id(), event.port().number()));
}
break;
case PORT_REMOVED:
@@ -613,11 +591,28 @@
}
}
- // Signals host vanish for all specified hosts.
- private void removeHosts(Set<Host> hosts) {
- for (Host host : hosts) {
- providerService.hostVanished(host.id());
- }
+ /**
+ * When a device goes down, update the location of affected hosts.
+ *
+ * @param deviceId the device that goes down
+ */
+ private void processDeviceDown(DeviceId deviceId) {
+ hostService.getConnectedHosts(deviceId).forEach(affectedHost -> affectedHost.locations().stream()
+ .filter(hostLocation -> hostLocation.deviceId().equals(deviceId))
+ .forEach(affectedLocation ->
+ providerService.removeLocationFromHost(affectedHost.id(), affectedLocation))
+ );
+ }
+
+ /**
+ * When a port goes down, update the location of affected hosts.
+ *
+ * @param connectPoint the port that goes down
+ */
+ private void processPortDown(ConnectPoint connectPoint) {
+ hostService.getConnectedHosts(connectPoint).forEach(affectedHost ->
+ providerService.removeLocationFromHost(affectedHost.id(), new HostLocation(connectPoint, 0L))
+ );
}
}
diff --git a/providers/host/src/test/java/org/onosproject/provider/host/impl/HostLocationProviderTest.java b/providers/host/src/test/java/org/onosproject/provider/host/impl/HostLocationProviderTest.java
index b1c661c..b642458 100644
--- a/providers/host/src/test/java/org/onosproject/provider/host/impl/HostLocationProviderTest.java
+++ b/providers/host/src/test/java/org/onosproject/provider/host/impl/HostLocationProviderTest.java
@@ -256,12 +256,12 @@
Device device = new DefaultDevice(ProviderId.NONE, deviceId(DEV1), SWITCH,
"m", "h", "s", "n", new ChassisId(0L));
deviceService.listener.event(new DeviceEvent(DEVICE_REMOVED, device));
- assertEquals("incorrect remove count", 2, providerService.removeCount);
+ assertEquals("incorrect remove count", 2, providerService.locationRemoveCount);
device = new DefaultDevice(ProviderId.NONE, deviceId(DEV4), SWITCH,
"m", "h", "s", "n", new ChassisId(0L));
deviceService.listener.event(new DeviceEvent(DEVICE_REMOVED, device));
- assertEquals("incorrect remove count", 3, providerService.removeCount);
+ assertEquals("incorrect remove count", 3, providerService.locationRemoveCount);
}
@Test
@@ -273,12 +273,12 @@
Device device = new DefaultDevice(ProviderId.NONE, deviceId(DEV1), SWITCH,
"m", "h", "s", "n", new ChassisId(0L));
deviceService.listener.event(new DeviceEvent(DEVICE_AVAILABILITY_CHANGED, device));
- assertEquals("incorrect remove count", 2, providerService.removeCount);
+ assertEquals("incorrect remove count", 2, providerService.locationRemoveCount);
device = new DefaultDevice(ProviderId.NONE, deviceId(DEV4), SWITCH,
"m", "h", "s", "n", new ChassisId(0L));
deviceService.listener.event(new DeviceEvent(DEVICE_AVAILABILITY_CHANGED, device));
- assertEquals("incorrect remove count", 3, providerService.removeCount);
+ assertEquals("incorrect remove count", 3, providerService.locationRemoveCount);
}
@Test
@@ -291,13 +291,13 @@
"m", "h", "s", "n", new ChassisId(0L));
deviceService.listener.event(new DeviceEvent(PORT_UPDATED, device,
new DefaultPort(device, portNumber(INPORT), false)));
- assertEquals("incorrect remove count", 1, providerService.removeCount);
+ assertEquals("incorrect remove count", 1, providerService.locationRemoveCount);
device = new DefaultDevice(ProviderId.NONE, deviceId(DEV4), SWITCH,
"m", "h", "s", "n", new ChassisId(0L));
deviceService.listener.event(new DeviceEvent(PORT_UPDATED, device,
new DefaultPort(device, portNumber(INPORT), false)));
- assertEquals("incorrect remove count", 2, providerService.removeCount);
+ assertEquals("incorrect remove count", 2, providerService.locationRemoveCount);
}
/**
@@ -483,11 +483,15 @@
implements HostProviderService {
List<HostDescription> descriptions = Lists.newArrayList();
- int removeCount;
+ int hostRemoveCount;
+ int ipRemoveCount;
+ int locationRemoveCount;
public void clear() {
descriptions.clear();
- removeCount = 0;
+ hostRemoveCount = 0;
+ ipRemoveCount = 0;
+ locationRemoveCount = 0;
}
protected TestHostProviderService(HostProvider provider) {
@@ -501,11 +505,17 @@
@Override
public void hostVanished(HostId hostId) {
- removeCount++;
+ hostRemoveCount++;
}
@Override
public void removeIpFromHost(HostId hostId, IpAddress ipAddress) {
+ ipRemoveCount++;
+ }
+
+ @Override
+ public void removeLocationFromHost(HostId hostId, HostLocation location) {
+ locationRemoveCount++;
}
}
diff --git a/providers/netcfghost/src/test/java/org/onosproject/provider/netcfghost/NetworkConfigHostProviderTest.java b/providers/netcfghost/src/test/java/org/onosproject/provider/netcfghost/NetworkConfigHostProviderTest.java
index ac47dba..50071db 100644
--- a/providers/netcfghost/src/test/java/org/onosproject/provider/netcfghost/NetworkConfigHostProviderTest.java
+++ b/providers/netcfghost/src/test/java/org/onosproject/provider/netcfghost/NetworkConfigHostProviderTest.java
@@ -121,7 +121,12 @@
@Override
public void removeIpFromHost(HostId hostId, IpAddress ipAddress) {
- // Note: This method is never used.
+
+ }
+
+ @Override
+ public void removeLocationFromHost(HostId hostId, HostLocation location) {
+
}
public void clear() {
diff --git a/providers/ovsdb/host/src/test/java/org/onosproject/ovsdb/provider/host/OvsdbHostProviderTest.java b/providers/ovsdb/host/src/test/java/org/onosproject/ovsdb/provider/host/OvsdbHostProviderTest.java
index c5eeedd..40999c5 100644
--- a/providers/ovsdb/host/src/test/java/org/onosproject/ovsdb/provider/host/OvsdbHostProviderTest.java
+++ b/providers/ovsdb/host/src/test/java/org/onosproject/ovsdb/provider/host/OvsdbHostProviderTest.java
@@ -30,6 +30,7 @@
import org.onlab.packet.TpPort;
import org.onosproject.net.DeviceId;
import org.onosproject.net.HostId;
+import org.onosproject.net.HostLocation;
import org.onosproject.net.host.HostDescription;
import org.onosproject.net.host.HostProvider;
import org.onosproject.net.host.HostProviderRegistry;
@@ -167,6 +168,10 @@
}
+ @Override
+ public void removeLocationFromHost(HostId hostId, HostLocation location) {
+
+ }
}
private class OvsdbControllerTest implements OvsdbController {