Deleted device manager's inside global cache and changed to use the topo cache.
The reason is it caused some unmatched condition with the topo cache when link and port status was changed.
The DeviceEvent timing is changed.
The old one sends it when it is appeared, or attachment point or IP is changed, but now it sends every packet-in to update last-time-seen.
Note:
Looks like there is some vulnerability for changing the mastership of switches.
During mastership changes, the intent reroute is occured, and for a short period of time links or ports haven't found,
so the intent cann't calcurate its path.
Change-Id: I311fa5ad368ca833915999ad112274f1a7979387
diff --git a/src/main/java/net/onrc/onos/core/devicemanager/OnosDevice.java b/src/main/java/net/onrc/onos/core/devicemanager/OnosDevice.java
index d0a5ee4..1a2bdeb 100644
--- a/src/main/java/net/onrc/onos/core/devicemanager/OnosDevice.java
+++ b/src/main/java/net/onrc/onos/core/devicemanager/OnosDevice.java
@@ -19,9 +19,12 @@
import java.io.Serializable;
import java.util.Date;
+import java.util.Objects;
import net.floodlightcontroller.util.MACAddress;
+
+
/**
* An entity on the network is a visible trace of a device that corresponds
* to a packet received from a particular interface on the edge of a network,
@@ -53,13 +56,13 @@
* The DPID of the switch for the ingress point for this entity,
* or null if not present.
*/
- private long switchDPID;
+ private Long switchDPID;
/**
* The port number of the switch for the ingress point for this entity,
* or null if not present.
*/
- private short switchPort;
+ private Long switchPort;
/**
* The last time we observed this entity on the network.
@@ -75,16 +78,16 @@
}
/**
- * Create a new entity.
+ * Create a new device and its information.
*
- * @param macAddress
- * @param vlan
- * @param switchDPID
- * @param switchPort
- * @param lastSeenTimestamp
+ * @param macAddress mac address of this device
+ * @param vlan vlan ID of this device
+ * @param switchDPID switch DPID where the device is attached
+ * @param switchPort port number where the device is attached
+ * @param lastSeenTimestamp last packet-in time of this device
*/
public OnosDevice(MACAddress macAddress, Short vlan, Long switchDPID,
- short switchPort, Date lastSeenTimestamp) {
+ Long switchPort, Date lastSeenTimestamp) {
this.macAddress = macAddress;
this.vlan = vlan;
this.switchDPID = switchDPID;
@@ -112,18 +115,10 @@
return switchDPID;
}
- public void setSwitchDPID(long dpid) {
- this.switchDPID = dpid;
- }
-
- public short getSwitchPort() {
+ public Long getSwitchPort() {
return switchPort;
}
- public void setSwitchPort(short port) {
- this.switchPort = port;
- }
-
public Date getLastSeenTimestamp() {
if (this.lastSeenTimestamp == null) {
return null;
@@ -137,14 +132,11 @@
@Override
public int hashCode() {
- if (hashCode != 0) {
- return hashCode;
- }
final int prime = 31;
hashCode = 1;
hashCode = prime * hashCode + (int) (macAddress.toLong() ^ (macAddress.toLong() >>> 32));
- hashCode = prime * hashCode + (int) switchDPID;
- hashCode = prime * hashCode + (int) switchPort;
+ hashCode = prime * hashCode + ((switchDPID == null) ? 0 : switchDPID.hashCode());
+ hashCode = prime * hashCode + ((switchPort == null) ? 0 : switchPort.hashCode());
hashCode = prime * hashCode + ((vlan == null) ? 0 : vlan.hashCode());
return hashCode;
}
@@ -164,24 +156,16 @@
if (hashCode() != other.hashCode()) {
return false;
}
- if (macAddress == null) {
- if (other.macAddress != null) {
- return false;
- }
- } else if (!macAddress.equals(other.macAddress)) {
+ if (!Objects.equals(macAddress, other.macAddress)) {
return false;
}
- if (switchDPID != other.switchDPID) {
+ if (!Objects.equals(switchDPID, other.switchDPID)) {
return false;
}
- if (switchPort != other.switchPort) {
+ if (!Objects.equals(switchPort, other.switchPort)) {
return false;
}
- if (vlan == null) {
- if (other.vlan != null) {
- return false;
- }
- } else if (!vlan.equals(other.vlan)) {
+ if (!Objects.equals(vlan, other.vlan)) {
return false;
}
return true;
diff --git a/src/main/java/net/onrc/onos/core/devicemanager/OnosDeviceManager.java b/src/main/java/net/onrc/onos/core/devicemanager/OnosDeviceManager.java
index 23944a0..0c7a75e 100644
--- a/src/main/java/net/onrc/onos/core/devicemanager/OnosDeviceManager.java
+++ b/src/main/java/net/onrc/onos/core/devicemanager/OnosDeviceManager.java
@@ -8,7 +8,6 @@
import java.util.List;
import java.util.Map;
import java.util.Set;
-import java.util.concurrent.ConcurrentHashMap;
import java.util.concurrent.CopyOnWriteArrayList;
import java.util.concurrent.Executors;
import java.util.concurrent.ScheduledExecutorService;
@@ -24,11 +23,10 @@
import net.floodlightcontroller.core.module.IFloodlightModule;
import net.floodlightcontroller.core.module.IFloodlightService;
import net.floodlightcontroller.util.MACAddress;
-import net.onrc.onos.core.datagrid.IDatagridService;
-import net.onrc.onos.core.datagrid.IEventChannel;
-import net.onrc.onos.core.datagrid.IEventChannelListener;
import net.onrc.onos.core.packet.Ethernet;
+import net.onrc.onos.core.topology.Device;
import net.onrc.onos.core.topology.ITopologyService;
+import net.onrc.onos.core.topology.Port;
import net.onrc.onos.core.topology.Topology;
import org.openflow.protocol.OFMessage;
@@ -39,8 +37,7 @@
public class OnosDeviceManager implements IFloodlightModule,
IOFMessageListener,
- IOnosDeviceService,
- IEventChannelListener<Long, OnosDevice> {
+ IOnosDeviceService {
private static final Logger log = LoggerFactory.getLogger(OnosDeviceManager.class);
private static final long DEVICE_CLEANING_INITIAL_DELAY = 30;
@@ -52,16 +49,6 @@
private static final ScheduledExecutorService EXECUTOR_SERVICE =
Executors.newSingleThreadScheduledExecutor();
- // TODO This infrastructure maintains a global device cache in the
- // OnosDeviceManager module on each instance (in mapDevice). We want to
- // remove this eventually - the global cache should be maintained by the
- // topology layer (which it currently is as well).
- private IDatagridService datagrid;
- private IEventChannel<Long, OnosDevice> eventChannel;
- private static final String DEVICE_CHANNEL_NAME = "onos.device";
- private final Map<Long, OnosDevice> mapDevice =
- new ConcurrentHashMap<Long, OnosDevice>();
-
private ITopologyService topologyService;
private Topology topology;
@@ -129,6 +116,10 @@
// The above "receive" method couldn't be tested
// because of IFloodlightProviderService static final field.
protected Command processPacketIn(IOFSwitch sw, OFPacketIn pi, Ethernet eth) {
+ if (log.isTraceEnabled()) {
+ log.trace("Receive PACKET_IN swId {}, portId {}", sw.getId(), pi.getInPort());
+ }
+
long dpid = sw.getId();
short portId = pi.getInPort();
Long mac = eth.getSourceMAC().toLong();
@@ -140,31 +131,21 @@
return Command.STOP;
}
- // We check if it is the same device in datagrid to suppress the device update
- OnosDevice exDev = mapDevice.get(mac);
- if (exDev != null && exDev.equals(srcDevice)) {
- // There is the same existing device. Update only ActiveSince time.
- // TODO This doesn't update the timestamp in the Topology module,
- // only in the local cache in this local driver module.
- exDev.setLastSeenTimestamp(new Date());
- if (log.isTraceEnabled()) {
- log.trace("In the local cache, there is the same device."
- + " Only update last seen time: {}", exDev);
- }
- return Command.CONTINUE;
- }
-
// If the switch port we try to attach a new device already has a link,
// then don't add the device
// TODO We probably don't need to check this here, it should be done in
// the Topology module.
- if (topology.getOutgoingLink(dpid, (long) portId) != null) {
- if (log.isTraceEnabled()) {
- log.trace("Stop adding OnosDevice {} as " +
- "there is a link on the port: dpid {} port {}",
- srcDevice.getMacAddress(), dpid, portId);
+ topology.acquireReadLock();
+ try {
+ if (topology.getOutgoingLink(dpid, (long) portId) != null ||
+ topology.getIncomingLink(dpid, (long) portId) != null) {
+ log.debug("Stop adding OnosDevice {} as " +
+ "there is a link on the port: dpid {} port {}",
+ srcDevice.getMacAddress(), dpid, portId);
+ return Command.CONTINUE;
}
- return Command.CONTINUE;
+ } finally {
+ topology.releaseReadLock();
}
addOnosDevice(mac, srcDevice);
@@ -187,28 +168,30 @@
@Override
public void run() {
log.debug("called CleanDevice");
+ topology.acquireReadLock();
try {
- Set<OnosDevice> deleteSet = new HashSet<OnosDevice>();
- for (OnosDevice dev : mapDevice.values()) {
- long now = new Date().getTime();
- if ((now - dev.getLastSeenTimestamp().getTime()
- > agingMillisecConfig)) {
+ Set<Device> deleteSet = new HashSet<Device>();
+ for (Device dev : topology.getDevices()) {
+ long now = System.currentTimeMillis();
+ if ((now - dev.getLastSeenTime() > agingMillisecConfig)) {
if (log.isTraceEnabled()) {
- log.debug("Removing device info from the datagrid: {}, diff {}",
- dev, now - dev.getLastSeenTimestamp().getTime());
+ log.trace("Removing device info: mac {}, now {}, lastSeenTime {}, diff {}",
+ dev.getMacAddress(), now, dev.getLastSeenTime(), now - dev.getLastSeenTime());
}
deleteSet.add(dev);
}
}
- for (OnosDevice dev : deleteSet) {
- deleteOnosDevice(dev);
+ for (Device dev : deleteSet) {
+ deleteOnosDeviceByMac(dev.getMacAddress());
}
} catch (Exception e) {
// Any exception thrown by the task will prevent the Executor
// from running the next iteration, so we need to catch and log
// all exceptions here.
log.error("Exception in device cleanup thread:", e);
+ } finally {
+ topology.releaseReadLock();
}
}
}
@@ -223,7 +206,7 @@
*/
protected OnosDevice getSourceDeviceFromPacket(Ethernet eth,
long swdpid,
- short port) {
+ long port) {
MACAddress sourceMac = eth.getSourceMAC();
// Ignore broadcast/multicast source
@@ -261,7 +244,6 @@
new ArrayList<Class<? extends IFloodlightService>>();
dependencies.add(IFloodlightProviderService.class);
dependencies.add(ITopologyService.class);
- dependencies.add(IDatagridService.class);
return dependencies;
}
@@ -270,7 +252,6 @@
throws FloodlightModuleException {
floodlightProvider = context.getServiceImpl(IFloodlightProviderService.class);
deviceListeners = new CopyOnWriteArrayList<IOnosDeviceListener>();
- datagrid = context.getServiceImpl(IDatagridService.class);
topologyService = context.getServiceImpl(ITopologyService.class);
topology = topologyService.getTopology();
@@ -280,56 +261,48 @@
@Override
public void startUp(FloodlightModuleContext context) {
floodlightProvider.addOFMessageListener(OFType.PACKET_IN, this);
- eventChannel = datagrid.addListener(DEVICE_CHANNEL_NAME, this,
- Long.class,
- OnosDevice.class);
EXECUTOR_SERVICE.scheduleAtFixedRate(new CleanDevice(),
DEVICE_CLEANING_INITIAL_DELAY, cleanupSecondConfig, TimeUnit.SECONDS);
}
@Override
public void deleteOnosDevice(OnosDevice dev) {
- Long mac = dev.getMacAddress().toLong();
- eventChannel.removeEntry(mac);
floodlightProvider.publishUpdate(
new OnosDeviceUpdate(dev, OnosDeviceUpdateType.DELETE));
}
@Override
public void deleteOnosDeviceByMac(MACAddress mac) {
- OnosDevice deleteDevice = mapDevice.get(mac.toLong());
- deleteOnosDevice(deleteDevice);
+ OnosDevice deleteDevice = null;
+ topology.acquireReadLock();
+ try {
+ Device dev = topology.getDeviceByMac(mac);
+
+ for (Port switchPort : dev.getAttachmentPoints()) {
+ // We don't handle vlan now and multiple attachment points.
+ deleteDevice = new OnosDevice(dev.getMacAddress(),
+ null,
+ switchPort.getDpid(),
+ switchPort.getNumber(),
+ new Date(dev.getLastSeenTime()));
+ break;
+ }
+ } finally {
+ topology.releaseReadLock();
+ }
+
+ if (deleteDevice != null) {
+ deleteOnosDevice(deleteDevice);
+ }
}
@Override
public void addOnosDevice(Long mac, OnosDevice dev) {
- eventChannel.addEntry(mac, dev);
floodlightProvider.publishUpdate(
new OnosDeviceUpdate(dev, OnosDeviceUpdateType.ADD));
}
@Override
- public void entryAdded(OnosDevice dev) {
- Long mac = dev.getMacAddress().toLong();
- mapDevice.put(mac, dev);
- log.debug("Device added into local Cache: device mac {}", mac);
- }
-
- @Override
- public void entryRemoved(OnosDevice dev) {
- Long mac = dev.getMacAddress().toLong();
- mapDevice.remove(mac);
- log.debug("Device removed into local Cache: device mac {}", mac);
- }
-
- @Override
- public void entryUpdated(OnosDevice dev) {
- Long mac = dev.getMacAddress().toLong();
- mapDevice.put(mac, dev);
- log.debug("Device updated into local Cache: device mac {}", mac);
- }
-
- @Override
public void addOnosDeviceListener(IOnosDeviceListener listener) {
deviceListeners.add(listener);
}
diff --git a/src/main/java/net/onrc/onos/core/intent/runtime/PlanInstallModule.java b/src/main/java/net/onrc/onos/core/intent/runtime/PlanInstallModule.java
index b74514c..025b6d7 100644
--- a/src/main/java/net/onrc/onos/core/intent/runtime/PlanInstallModule.java
+++ b/src/main/java/net/onrc/onos/core/intent/runtime/PlanInstallModule.java
@@ -340,7 +340,8 @@
states.domainSwitchDpids.addAll(domainSwitchDpids);
}
parentIntentMap.remove(intentParentId);
- log.debug("addEntry to intentStateChannel intentId {}, states {}", flowRemovedMsg.getCookie(), states);
+ log.debug("addEntry to intentStateChannel intentId {}, states {}",
+ pathIntentId, newState);
intentStateChannel.addTransientEntry(flowRemovedMsg.getCookie(), states);
}
diff --git a/src/main/java/net/onrc/onos/core/topology/DeviceImpl.java b/src/main/java/net/onrc/onos/core/topology/DeviceImpl.java
index fc1628e..0ba23dd 100644
--- a/src/main/java/net/onrc/onos/core/topology/DeviceImpl.java
+++ b/src/main/java/net/onrc/onos/core/topology/DeviceImpl.java
@@ -47,19 +47,19 @@
/**
* Only {@link TopologyManager} should use this method.
*
- * @param p
+ * @param port the port that the device is attached to
*/
- void addAttachmentPoint(Port p) {
- this.attachmentPoints.remove(p);
- this.attachmentPoints.addFirst(p);
+ void addAttachmentPoint(Port port) {
+ this.attachmentPoints.remove(port);
+ this.attachmentPoints.addFirst(port);
}
/**
* Only {@link TopologyManager} should use this method.
*
- * @param p
+ * @param port the port that the device is attached to
*/
- boolean removeAttachmentPoint(Port p) {
- return this.attachmentPoints.remove(p);
+ boolean removeAttachmentPoint(Port port) {
+ return this.attachmentPoints.remove(port);
}
}
diff --git a/src/main/java/net/onrc/onos/core/topology/TopologyManager.java b/src/main/java/net/onrc/onos/core/topology/TopologyManager.java
index 8e0a4e6..343102b 100644
--- a/src/main/java/net/onrc/onos/core/topology/TopologyManager.java
+++ b/src/main/java/net/onrc/onos/core/topology/TopologyManager.java
@@ -1085,6 +1085,8 @@
attachmentFound = true;
}
+ deviceImpl.setLastSeenTime(deviceEvent.getLastSeenTime());
+
// Update the device in the topology
if (attachmentFound) {
log.debug("Storing the device info into the Topology: mac {}", deviceEvent.getMac());
diff --git a/src/main/java/net/onrc/onos/core/topology/TopologyPublisher.java b/src/main/java/net/onrc/onos/core/topology/TopologyPublisher.java
index 36399bc..eb76c1e 100644
--- a/src/main/java/net/onrc/onos/core/topology/TopologyPublisher.java
+++ b/src/main/java/net/onrc/onos/core/topology/TopologyPublisher.java
@@ -307,8 +307,7 @@
public void onosDeviceAdded(OnosDevice device) {
log.debug("Called onosDeviceAdded mac {}", device.getMacAddress());
- SwitchPort sp = new SwitchPort(device.getSwitchDPID(),
- (long) device.getSwitchPort());
+ SwitchPort sp = new SwitchPort(device.getSwitchDPID(), device.getSwitchPort());
List<SwitchPort> spLists = new ArrayList<SwitchPort>();
spLists.add(sp);
DeviceEvent event = new DeviceEvent(device.getMacAddress());