Jonathan Hart
Committed by Gerrit Code Review

Revert "Fix for ONOS-5032 and ONOS-5034"

This reverts commit 50ad0806.

Change-Id: Id2f9924a28c18686b4be30200244dbd0c975e90a
......@@ -24,7 +24,6 @@ import org.onosproject.cli.AbstractShellCommand;
import org.onosproject.net.Host;
import org.onosproject.net.host.HostService;
import org.onosproject.utils.Comparators;
import org.onlab.util.Tools;
import java.util.Collections;
import java.util.List;
......@@ -39,7 +38,7 @@ import static com.google.common.collect.Lists.newArrayList;
public class HostsListCommand extends AbstractShellCommand {
private static final String FMT =
"id=%s, mac=%s, location=%s/%s, vlan=%s, ip(s)=%s%s, last seen time=%s";
"id=%s, mac=%s, location=%s/%s, vlan=%s, ip(s)=%s%s";
private static final String FMT_SHORT =
"id=%s, mac=%s, location=%s/%s, vlan=%s, ip(s)=%s";
......@@ -94,9 +93,7 @@ public class HostsListCommand extends AbstractShellCommand {
} else {
print(FMT, host.id(), host.mac(),
host.location().deviceId(), host.location().port(),
host.vlan(), host.ipAddresses(), annotations(host.annotations()),
Tools.timeAgo(host.timestamp().unixTimestamp()));
host.vlan(), host.ipAddresses(), annotations(host.annotations()));
}
}
}
......
......@@ -16,7 +16,6 @@
package org.onosproject.net;
import org.onosproject.net.provider.ProviderId;
import org.onosproject.store.service.WallClockTimestamp;
import org.onlab.packet.IpAddress;
import org.onlab.packet.MacAddress;
import org.onlab.packet.VlanId;
......@@ -37,7 +36,6 @@ public class DefaultHost extends AbstractElement implements Host {
private final VlanId vlan;
private final HostLocation location;
private final Set<IpAddress> ips;
private final WallClockTimestamp timestamp;
/**
* Creates an end-station host using the supplied information.
......@@ -53,29 +51,10 @@ public class DefaultHost extends AbstractElement implements Host {
public DefaultHost(ProviderId providerId, HostId id, MacAddress mac,
VlanId vlan, HostLocation location, Set<IpAddress> ips,
Annotations... annotations) {
this(providerId, id, mac, vlan, location, ips, new WallClockTimestamp(), annotations);
}
/**
* Creates an end-station host using the supplied information.
*
* @param providerId provider identity
* @param id host identifier
* @param mac host MAC address
* @param vlan host VLAN identifier
* @param location host location
* @param ips host IP addresses
* @param timestamp last host updated time
* @param annotations optional key/value annotations
*/
public DefaultHost(ProviderId providerId, HostId id, MacAddress mac,
VlanId vlan, HostLocation location, Set<IpAddress> ips,
WallClockTimestamp timestamp, Annotations... annotations) {
super(providerId, id, annotations);
this.mac = mac;
this.vlan = vlan;
this.location = location;
this.timestamp = timestamp;
this.ips = new HashSet<>(ips);
}
......@@ -105,11 +84,6 @@ public class DefaultHost extends AbstractElement implements Host {
}
@Override
public WallClockTimestamp timestamp() {
return timestamp;
}
@Override
public int hashCode() {
return Objects.hash(id, mac, vlan, location);
}
......@@ -140,7 +114,6 @@ public class DefaultHost extends AbstractElement implements Host {
.add("location", location())
.add("ipAddresses", ipAddresses())
.add("annotations", annotations())
.add("timestamp", timestamp())
.toString();
}
......
......@@ -18,7 +18,6 @@ package org.onosproject.net;
import org.onlab.packet.IpAddress;
import org.onlab.packet.MacAddress;
import org.onlab.packet.VlanId;
import org.onosproject.store.service.WallClockTimestamp;
import java.util.Set;
......@@ -64,14 +63,6 @@ public interface Host extends Element {
*/
HostLocation location();
/**
* Returns the host recent time.
* @return host last updated time
*/
default WallClockTimestamp timestamp() {
return null;
}
// TODO: explore capturing list of recent locations to aid in mobility
}
......
......@@ -21,7 +21,6 @@ import java.util.Set;
import org.onosproject.net.AbstractDescription;
import org.onosproject.net.HostLocation;
import org.onosproject.net.SparseAnnotations;
import org.onosproject.store.service.WallClockTimestamp;
import org.onlab.packet.IpAddress;
import org.onlab.packet.MacAddress;
import org.onlab.packet.VlanId;
......@@ -41,7 +40,6 @@ public class DefaultHostDescription extends AbstractDescription
private final VlanId vlan;
private final HostLocation location;
private final Set<IpAddress> ip;
private final WallClockTimestamp timestamp;
/**
* Creates a host description using the supplied information.
......@@ -85,28 +83,11 @@ public class DefaultHostDescription extends AbstractDescription
public DefaultHostDescription(MacAddress mac, VlanId vlan,
HostLocation location, Set<IpAddress> ip,
SparseAnnotations... annotations) {
this(mac, vlan, location, ip, new WallClockTimestamp(), annotations);
}
/**
* Creates a host description using the supplied information.
*
* @param mac host MAC address
* @param vlan host VLAN identifier
* @param location host location
* @param ip host IP addresses
* @param timestamp host recent updated time
* @param annotations optional key/value annotations map
*/
public DefaultHostDescription(MacAddress mac, VlanId vlan,
HostLocation location, Set<IpAddress> ip,
WallClockTimestamp timestamp, SparseAnnotations... annotations) {
super(annotations);
this.mac = mac;
this.vlan = vlan;
this.location = location;
this.ip = ImmutableSet.copyOf(ip);
this.timestamp = timestamp;
}
@Override
......@@ -130,18 +111,12 @@ public class DefaultHostDescription extends AbstractDescription
}
@Override
public WallClockTimestamp timestamp() {
return timestamp;
}
@Override
public String toString() {
return toStringHelper(this)
.add("mac", mac)
.add("vlan", vlan)
.add("location", location)
.add("ipAddress", ip)
.add("timestamp", timestamp)
.toString();
}
......@@ -164,4 +139,5 @@ public class DefaultHostDescription extends AbstractDescription
}
return false;
}
}
......
......@@ -22,7 +22,6 @@ import org.onosproject.net.HostLocation;
import org.onlab.packet.IpAddress;
import org.onlab.packet.MacAddress;
import org.onlab.packet.VlanId;
import org.onosproject.store.service.WallClockTimestamp;
/**
* Information describing host and its location.
......@@ -56,12 +55,4 @@ public interface HostDescription extends Description {
* @return host IP address
*/
Set<IpAddress> ipAddress();
/**
* Returns the host recent updated Time.
* @return host last updated time.
*/
default WallClockTimestamp timestamp() {
return null;
}
}
......
......@@ -39,8 +39,7 @@ public final class HostCodec extends AnnotatedCodec<Host> {
final ObjectNode result = context.mapper().createObjectNode()
.put("id", host.id().toString())
.put("mac", host.mac().toString())
.put("vlan", host.vlan().toString())
.put("timestamp", host.timestamp().unixTimestamp());
.put("vlan", host.vlan().toString());
final ArrayNode jsonIpAddresses = result.putArray("ipAddresses");
for (final IpAddress ipAddress : host.ipAddresses()) {
......
......@@ -12,7 +12,6 @@ TEST_DEPS = [
'//core/common:onos-core-common',
'//core/store/dist:onos-core-dist',
'//core/store/dist:onos-core-dist-tests',
'//utils/osgi:onlab-osgi-tests',
]
osgi_jar_with_tests (
......
......@@ -125,13 +125,6 @@
<groupId>org.onosproject</groupId>
<artifactId>onos-incubator-net</artifactId>
</dependency>
<dependency>
<groupId>org.onosproject</groupId>
<artifactId>onlab-osgi</artifactId>
<version>${project.version}</version>
<classifier>tests</classifier>
<scope>test</scope>
</dependency>
</dependencies>
</project>
......
......@@ -106,15 +106,6 @@ public class HostManager
label = "Enable removal of duplicate ip address")
private boolean allowDuplicateIps = true;
@Property(name = "monitorHosts", boolValue = false,
label = "Enable/Disable monitoring of hosts")
private boolean monitorHosts = false;
@Property(name = "probeRate", longValue = 30000,
label = "Set the probe Rate in milli seconds")
private long probeRate = 30000;
private HostMonitor monitor;
......@@ -123,71 +114,17 @@ public class HostManager
store.setDelegate(delegate);
eventDispatcher.addSink(HostEvent.class, listenerRegistry);
cfgService.registerProperties(getClass());
modified(context);
networkConfigService.addListener(networkConfigListener);
monitor = new HostMonitor(packetService, this, interfaceService, edgePortService);
monitor.setProbeRate(probeRate);
monitor.start();
modified(context);
cfgService.registerProperties(getClass());
log.info("Started");
}
@Deactivate
public void deactivate() {
store.unsetDelegate(delegate);
eventDispatcher.removeSink(HostEvent.class);
networkConfigService.removeListener(networkConfigListener);
cfgService.unregisterProperties(getClass(), false);
monitor.shutdown();
log.info("Stopped");
}
@Modified
public void modified(ComponentContext context) {
boolean oldValue = monitorHosts;
readComponentConfiguration(context);
if (probeRate > 0) {
monitor.setProbeRate(probeRate);
} else {
log.warn("probeRate cannot be lessthan 0");
}
if (oldValue != monitorHosts) {
if (monitorHosts) {
startMonitoring();
} else {
stopMonitoring();
}
}
}
/**
* Extracts properties from the component configuration context.
*
* @param context the component context
*/
private void readComponentConfiguration(ComponentContext context) {
Dictionary<?, ?> properties = context.getProperties();
Boolean flag;
flag = Tools.isPropertyEnabled(properties, "monitorHosts");
if (flag == null) {
log.info("monitorHosts is not enabled " +
"using current value of {}", monitorHosts);
} else {
monitorHosts = flag;
log.info("Configured. monitorHosts {}",
monitorHosts ? "enabled" : "disabled");
}
Long longValue = Tools.getLongProperty(properties, "probeRate");
if (longValue == null || longValue == 0) {
log.info("probeRate is not set sing default value of {}", probeRate);
} else {
probeRate = longValue;
log.info("Configured. probeRate {}", probeRate);
}
flag = Tools.isPropertyEnabled(properties, "allowDuplicateIps");
if (flag == null) {
log.info("Removal of duplicate ip address is not configured");
......@@ -196,32 +133,14 @@ public class HostManager
log.info("Removal of duplicate ip address is {}",
allowDuplicateIps ? "disabled" : "enabled");
}
}
/**
* Starts monitoring the hosts by IP Address.
*
*/
private void startMonitoring() {
store.getHosts().forEach(host -> {
host.ipAddresses().forEach(ip -> {
monitor.addMonitoringFor(ip);
});
});
}
/**
* Stops monitoring the hosts by IP Address.
*
*/
private void stopMonitoring() {
store.getHosts().forEach(host -> {
host.ipAddresses().forEach(ip -> {
monitor.stopMonitoring(ip);
});
});
@Deactivate
public void deactivate() {
store.unsetDelegate(delegate);
eventDispatcher.removeSink(HostEvent.class);
networkConfigService.removeListener(networkConfigListener);
log.info("Stopped");
}
@Override
......@@ -325,15 +244,8 @@ public class HostManager
}
store.createOrUpdateHost(provider().id(), hostId,
hostDescription, replaceIps);
if (monitorHosts) {
hostDescription.ipAddress().forEach(ip -> {
monitor.addMonitoringFor(ip);
});
}
}
// When a new IP is detected, remove that IP on other hosts if it exists
public void removeDuplicates(HostId hostId, HostDescription desc) {
desc.ipAddress().forEach(ip -> {
......@@ -346,7 +258,9 @@ public class HostManager
}
});
});
}
}
// returns a HostDescription made from the union of the BasicHostConfig
......@@ -362,12 +276,6 @@ public class HostManager
public void hostVanished(HostId hostId) {
checkNotNull(hostId, HOST_ID_NULL);
checkValidity();
Host host = store.getHost(hostId);
if (monitorHosts) {
host.ipAddresses().forEach(ip -> {
monitor.stopMonitoring(ip);
});
}
store.removeHost(hostId);
}
......@@ -415,4 +323,3 @@ public class HostManager
}
}
}
......
......@@ -150,13 +150,6 @@ public class HostMonitor implements TimerTask {
hostProviders.put(provider.id(), provider);
}
/*
* Sets the probe rate.
*/
void setProbeRate(long probeRate) {
this.probeRate = probeRate;
}
@Override
public void run(Timeout timeout) throws Exception {
monitoredAddresses.forEach(this::probe);
......
......@@ -98,14 +98,12 @@ public class HostManagerTest {
protected TestHostProvider provider;
protected HostProviderService providerService;
private static final ComponentContextAdapter REMOVE_DUPS_MONITOR =
private static final ComponentContextAdapter REMOVE_DUPS =
new ComponentContextAdapter() {
@Override
public Dictionary getProperties() {
Hashtable<String, String> props = new Hashtable<>();
props.put("allowDuplicateIps", "true");
props.put("monitorHosts", "true");
props.put("probeRate", "40000");
return props;
}
};
......@@ -118,27 +116,28 @@ public class HostManagerTest {
registry = mgr;
mgr.networkConfigService = new TestNetworkConfigService();
mgr.cfgService = new ComponentConfigAdapter();
mgr.activate(REMOVE_DUPS_MONITOR);
mgr.activate(REMOVE_DUPS);
mgr.addListener(listener);
provider = new TestHostProvider();
providerService = registry.register(provider);
assertTrue("provider should be registered", registry.getProviders().contains(provider.id()));
assertTrue("provider should be registered",
registry.getProviders().contains(provider.id()));
}
@After
public void tearDown() {
registry.unregister(provider);
assertFalse("provider should not be registered", registry.getProviders().contains(provider.id()));
assertFalse("provider should not be registered",
registry.getProviders().contains(provider.id()));
mgr.removeListener(listener);
mgr.deactivate();
injectEventDispatcher(mgr, null);
}
private void detect(HostId hid, MacAddress mac, VlanId vlan, HostLocation loc, IpAddress ip) {
private void detect(HostId hid, MacAddress mac, VlanId vlan,
HostLocation loc, IpAddress ip) {
HostDescription descr = new DefaultHostDescription(mac, vlan, loc, ip);
providerService.hostDetected(hid, descr, false);
assertNotNull("host should be found", mgr.getHost(hid));
......@@ -218,7 +217,8 @@ public class HostManagerTest {
assertNull("host should have been removed", mgr.getHost(HID3));
}
private void validateHosts(String msg, Iterable<Host> hosts, HostId... ids) {
private void validateHosts(
String msg, Iterable<Host> hosts, HostId... ids) {
Set<HostId> hids = Sets.newHashSet(ids);
for (Host h : hosts) {
assertTrue(msg, hids.remove(h.id()));
......@@ -252,7 +252,8 @@ public class HostManagerTest {
assertTrue("incorrect host location", mgr.getConnectedHosts(DID2).isEmpty());
}
private static class TestHostProvider extends AbstractProvider implements HostProvider {
private static class TestHostProvider extends AbstractProvider
implements HostProvider {
protected TestHostProvider() {
super(PID);
......@@ -283,4 +284,3 @@ public class HostManagerTest {
private class TestNetworkConfigService extends NetworkConfigServiceAdapter {
}
}
......
......@@ -48,7 +48,6 @@ import org.onosproject.store.service.MapEvent;
import org.onosproject.store.service.MapEventListener;
import org.onosproject.store.service.Serializer;
import org.onosproject.store.service.StorageService;
import org.onosproject.store.service.WallClockTimestamp;
import org.slf4j.Logger;
import java.util.Collection;
......@@ -88,8 +87,7 @@ public class DistributedHostStore
@Activate
public void activate() {
KryoNamespace.Builder hostSerializer = KryoNamespace.newBuilder()
.register(KryoNamespaces.API)
.register(WallClockTimestamp.class);
.register(KryoNamespaces.API);
hostsConsistentMap = storageService.<HostId, DefaultHost>consistentMapBuilder()
.withName("onos-hosts")
......@@ -124,9 +122,7 @@ public class DistributedHostStore
if (!Objects.equals(existingHost.providerId(), providerId) ||
!Objects.equals(existingHost.mac(), hostDescription.hwAddress()) ||
!Objects.equals(existingHost.vlan(), hostDescription.vlan()) ||
!Objects.equals(existingHost.location(), hostDescription.location()) ||
hostDescription.timestamp() == null ||
hostDescription.timestamp().isNewerThan(existingHost.timestamp())) {
!Objects.equals(existingHost.location(), hostDescription.location())) {
return true;
}
......@@ -177,13 +173,13 @@ public class DistributedHostStore
} else {
annotations = hostDescription.annotations();
}
return new DefaultHost(providerId,
hostId,
hostDescription.hwAddress(),
hostDescription.vlan(),
location,
addresses,
hostDescription.timestamp(),
annotations);
});
return null;
......@@ -306,4 +302,3 @@ public class DistributedHostStore
}
}
}
......
......@@ -44,22 +44,17 @@ import org.onosproject.net.Device;
import org.onosproject.net.Host;
import org.onosproject.net.HostId;
import org.onosproject.net.HostLocation;
import org.onosproject.net.MastershipRole;
import org.onosproject.net.device.DeviceEvent;
import org.onosproject.net.device.DeviceListener;
import org.onosproject.net.device.DeviceService;
import org.onosproject.net.flow.DefaultTrafficSelector;
import org.onosproject.net.flow.DefaultTrafficTreatment;
import org.onosproject.net.flow.TrafficSelector;
import org.onosproject.net.flow.TrafficTreatment;
import org.onosproject.net.host.DefaultHostDescription;
import org.onosproject.net.host.HostDescription;
import org.onosproject.net.host.HostProvider;
import org.onosproject.net.host.HostProviderRegistry;
import org.onosproject.net.host.HostProviderService;
import org.onosproject.net.host.HostService;
import org.onosproject.net.packet.DefaultOutboundPacket;
import org.onosproject.net.packet.OutboundPacket;
import org.onosproject.net.packet.PacketContext;
import org.onosproject.net.packet.PacketPriority;
import org.onosproject.net.packet.PacketProcessor;
......@@ -71,7 +66,6 @@ import org.onosproject.net.topology.TopologyService;
import org.osgi.service.component.ComponentContext;
import org.slf4j.Logger;
import java.nio.ByteBuffer;
import java.util.Dictionary;
import java.util.Set;
import java.util.concurrent.ExecutorService;
......@@ -132,8 +126,6 @@ public class HostLocationProvider extends AbstractProvider implements HostProvid
protected ExecutorService eventHandler;
private static final byte[] SENDER_ADDRESS = IpAddress.valueOf("0.0.0.0").toOctets();
/**
* Creates an OpenFlow host provider.
*/
......@@ -269,60 +261,7 @@ public class HostLocationProvider extends AbstractProvider implements HostProvid
@Override
public void triggerProbe(Host host) {
log.info("Triggering probe on device {} ", host);
MastershipRole role = deviceService.getRole(host.location().deviceId());
if (role.equals(MastershipRole.MASTER)) {
host.ipAddresses().forEach(ip -> {
sendProbe(host, ip);
});
} else {
log.info("not the master, master will probe {}");
}
}
private void sendProbe(Host host, IpAddress targetIp) {
Ethernet probePacket = null;
if (targetIp.isIp4()) {
// IPv4: Use ARP
probePacket = buildArpRequest(targetIp, host);
} else {
// IPv6: Use Neighbor Discovery
//FIX ME need to implement ndp probe
log.info("Triggering probe on device {} ", host);
}
TrafficTreatment treatment = DefaultTrafficTreatment.builder().setOutput(host.location().port()).build();
OutboundPacket outboundPacket = new DefaultOutboundPacket(host.location().deviceId(), treatment,
ByteBuffer.wrap(probePacket.serialize()));
packetService.emit(outboundPacket);
}
/*
* 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();
arp.setHardwareType(ARP.HW_TYPE_ETHERNET)
.setHardwareAddressLength((byte) Ethernet.DATALAYER_ADDRESS_LENGTH)
.setProtocolType(ARP.PROTO_TYPE_IP)
.setProtocolAddressLength((byte) IpAddress.INET_BYTE_LENGTH)
.setOpCode(ARP.OP_REQUEST);
arp.setSenderHardwareAddress(MacAddress.BROADCAST.toBytes())
.setSenderProtocolAddress(SENDER_ADDRESS)
.setTargetHardwareAddress(MacAddress.BROADCAST.toBytes())
.setTargetProtocolAddress(targetIp.toOctets());
Ethernet ethernet = new Ethernet();
ethernet.setEtherType(Ethernet.TYPE_ARP)
.setDestinationMACAddress(MacAddress.BROADCAST)
.setSourceMACAddress(MacAddress.BROADCAST).setPayload(arp);
ethernet.setPad(true);
return ethernet;
log.info("Triggering probe on device {}", host);
}
private class InternalHostProvider implements PacketProcessor {
......@@ -510,4 +449,3 @@ public class HostLocationProvider extends AbstractProvider implements HostProvid
}
}
......
......@@ -348,26 +348,6 @@ public abstract class Tools {
}
/**
* Get Long property from the propertyName
* Return null if propertyName is not found.
*
* @param properties properties to be looked up
* @param propertyName the name of the property to look up
* @return value when the propertyName is defined or return null
*/
public static Long getLongProperty(Dictionary<?, ?> properties,
String propertyName) {
Long value;
try {
String s = get(properties, propertyName);
value = Strings.isNullOrEmpty(s) ? null : Long.valueOf(s);
} catch (NumberFormatException | ClassCastException e) {
value = null;
}
return value;
}
/**
* Suspends the current thread for a specified number of millis.
*
* @param ms number of millis
......
......@@ -198,7 +198,7 @@ public class HostResourceTest extends ResourceTest {
@Override
public boolean matchesSafely(JsonArray json) {
boolean hostFound = false;
final int expectedAttributes = 6;
final int expectedAttributes = 5;
for (int jsonHostIndex = 0; jsonHostIndex < json.size();
jsonHostIndex++) {
......