Break the references between Port and Link.
This is in preparation for implementing topology versioning. Ports may have
different Links in different versions of the topology, so we don't want the
reference inside the Port object. References are now held in indexes in the
NetworkGraphImpl.
As a result of this, there was a small API change:
NetworkGraph.getLink -> NetworkGraph.getOutgoingLink
NetworkGraph.getIncomingLink was added.
Change-Id: I086d198c8040607253c8729b9d5f94ab6bc738db
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 dde6f55..3177f61 100644
--- a/src/main/java/net/onrc/onos/core/devicemanager/OnosDeviceManager.java
+++ b/src/main/java/net/onrc/onos/core/devicemanager/OnosDeviceManager.java
@@ -166,7 +166,7 @@
}
//If the switch port we try to attach a new device already has a link, then stop adding device
- if (networkGraph.getLink(dpid, (long) portId) != null) {
+ if (networkGraph.getOutgoingLink(dpid, (long) portId) != null) {
if (log.isTraceEnabled()) {
log.trace("Stop adding OnosDevice {} due to there is a link to: dpid {} port {}",
srcDevice.getMacAddress(), dpid, portId);
diff --git a/src/main/java/net/onrc/onos/core/topology/LinkImpl.java b/src/main/java/net/onrc/onos/core/topology/LinkImpl.java
index 4333e38..bf66100 100644
--- a/src/main/java/net/onrc/onos/core/topology/LinkImpl.java
+++ b/src/main/java/net/onrc/onos/core/topology/LinkImpl.java
@@ -1,5 +1,7 @@
package net.onrc.onos.core.topology;
+import net.onrc.onos.core.util.SwitchPort;
+
/**
* Link Object stored in In-memory Topology.
* <p/>
@@ -7,8 +9,8 @@
* but this Object itself will not issue any read/write to the DataStore.
*/
public class LinkImpl extends NetworkGraphObject implements Link {
- protected Port srcPort;
- protected Port dstPort;
+ private SwitchPort srcPort;
+ private SwitchPort dstPort;
protected static final Double DEFAULT_CAPACITY = Double.POSITIVE_INFINITY;
protected Double capacity = DEFAULT_CAPACITY;
@@ -26,39 +28,28 @@
*/
public LinkImpl(NetworkGraph graph, Port srcPort, Port dstPort) {
super(graph);
- this.srcPort = srcPort;
- this.dstPort = dstPort;
- setToPorts();
+ this.srcPort = srcPort.asSwitchPort();
+ this.dstPort = dstPort.asSwitchPort();
}
@Override
public Switch getSrcSwitch() {
- return srcPort.getSwitch();
+ return graph.getSwitch(srcPort.dpid().value());
}
@Override
public Port getSrcPort() {
- return srcPort;
+ return graph.getPort(srcPort.dpid().value(), (long) srcPort.port().value());
}
@Override
public Switch getDstSwitch() {
- return dstPort.getSwitch();
+ return graph.getSwitch(dstPort.dpid().value());
}
@Override
public Port getDstPort() {
- return dstPort;
- }
-
- protected void setToPorts() {
- ((PortImpl) srcPort).setOutgoingLink(this);
- ((PortImpl) dstPort).setIncomingLink(this);
- }
-
- protected void unsetFromPorts() {
- ((PortImpl) srcPort).setOutgoingLink(null);
- ((PortImpl) dstPort).setIncomingLink(null);
+ return graph.getPort(dstPort.dpid().value(), (long) dstPort.port().value());
}
@Override
diff --git a/src/main/java/net/onrc/onos/core/topology/NetworkGraph.java b/src/main/java/net/onrc/onos/core/topology/NetworkGraph.java
index d450784..6133bb6 100644
--- a/src/main/java/net/onrc/onos/core/topology/NetworkGraph.java
+++ b/src/main/java/net/onrc/onos/core/topology/NetworkGraph.java
@@ -33,13 +33,23 @@
public Port getPort(Long dpid, Long number);
/**
- * Get the outgoing link for a switch and a port.
+ * Get the outgoing link from a switch port.
*
* @param dpid the switch DPID.
* @param number the switch port number.
* @return the outgoing link if found, otherwise null.
*/
- public Link getLink(Long dpid, Long number);
+ public Link getOutgoingLink(Long dpid, Long number);
+ // TODO See if we should change <dpid, port_num> pairs to SwitchPort
+
+ /**
+ * Get the incoming link to a switch port.
+ *
+ * @param dpid the switch DPID.
+ * @param number the switch port number.
+ * @return the incoming link if found, otherwise null.
+ */
+ public Link getIncomingLink(Long dpid, Long number);
/**
* Get the outgoing link from a switch and a port to another switch and
diff --git a/src/main/java/net/onrc/onos/core/topology/NetworkGraphImpl.java b/src/main/java/net/onrc/onos/core/topology/NetworkGraphImpl.java
index b20ab6b..395f2af 100644
--- a/src/main/java/net/onrc/onos/core/topology/NetworkGraphImpl.java
+++ b/src/main/java/net/onrc/onos/core/topology/NetworkGraphImpl.java
@@ -1,8 +1,6 @@
package net.onrc.onos.core.topology;
import java.util.Collections;
-import java.util.LinkedList;
-import java.util.List;
import java.util.concurrent.ConcurrentHashMap;
import java.util.concurrent.ConcurrentMap;
import java.util.concurrent.locks.Lock;
@@ -10,6 +8,7 @@
import java.util.concurrent.locks.ReentrantReadWriteLock;
import net.floodlightcontroller.util.MACAddress;
+import net.onrc.onos.core.util.SwitchPort;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
@@ -19,8 +18,11 @@
private static final Logger log = LoggerFactory.getLogger(NetworkGraphImpl.class);
// DPID -> Switch
- private ConcurrentMap<Long, Switch> switches;
- private ConcurrentMap<MACAddress, Device> mac2Device;
+ private final ConcurrentMap<Long, Switch> switches;
+ private final ConcurrentMap<MACAddress, Device> mac2Device;
+
+ private final ConcurrentMap<SwitchPort, Link> outgoingLinks;
+ private final ConcurrentMap<SwitchPort, Link> incomingLinks;
private ReadWriteLock readWriteLock = new ReentrantReadWriteLock();
private Lock readLock = readWriteLock.readLock();
@@ -31,6 +33,8 @@
// TODO: Does these object need to be stored in Concurrent Collection?
switches = new ConcurrentHashMap<>();
mac2Device = new ConcurrentHashMap<>();
+ outgoingLinks = new ConcurrentHashMap<>();
+ incomingLinks = new ConcurrentHashMap<>();
}
@Override
@@ -63,18 +67,19 @@
}
@Override
- public Link getLink(Long dpid, Long number) {
- Port srcPort = getPort(dpid, number);
- if (srcPort == null) {
- return null;
- }
- return srcPort.getOutgoingLink();
+ public Link getOutgoingLink(Long dpid, Long number) {
+ return outgoingLinks.get(new SwitchPort(dpid, number.shortValue()));
+ }
+
+ @Override
+ public Link getIncomingLink(Long dpid, Long number) {
+ return incomingLinks.get(new SwitchPort(dpid, number.shortValue()));
}
@Override
public Link getLink(Long srcDpid, Long srcNumber, Long dstDpid,
Long dstNumber) {
- Link link = getLink(srcDpid, srcNumber);
+ Link link = getOutgoingLink(srcDpid, srcNumber);
if (link == null) {
return null;
}
@@ -89,15 +94,17 @@
@Override
public Iterable<Link> getLinks() {
- List<Link> linklist = new LinkedList<>();
+ return Collections.unmodifiableCollection(outgoingLinks.values());
+ }
- for (Switch sw : switches.values()) {
- Iterable<Link> links = sw.getOutgoingLinks();
- for (Link l : links) {
- linklist.add(l);
- }
- }
- return linklist;
+ protected void putLink(Link link) {
+ outgoingLinks.put(link.getSrcPort().asSwitchPort(), link);
+ incomingLinks.put(link.getDstPort().asSwitchPort(), link);
+ }
+
+ protected void removeLink(Link link) {
+ outgoingLinks.remove(link.getSrcPort().asSwitchPort(), link);
+ incomingLinks.remove(link.getDstPort().asSwitchPort(), link);
}
@Override
diff --git a/src/main/java/net/onrc/onos/core/topology/Port.java b/src/main/java/net/onrc/onos/core/topology/Port.java
index 75aac52..44f4bcf 100644
--- a/src/main/java/net/onrc/onos/core/topology/Port.java
+++ b/src/main/java/net/onrc/onos/core/topology/Port.java
@@ -1,7 +1,10 @@
package net.onrc.onos.core.topology;
-// TODO Everything returned by these interfaces must be either Unmodifiable view,
-// immutable object, or a copy of the original "SB" In-memory Topology.
+import net.onrc.onos.core.util.SwitchPort;
+
+//TODO Everything returned by these interfaces must be either Unmodifiable view,
+//immutable object, or a copy of the original "SB" In-memory Topology.
+
/**
* Interface of Port object in Network Graph topology.
*/
@@ -22,6 +25,14 @@
public Long getNumber();
/**
+ * Gets a {@link SwitchPort} that represents this Port's dpid and port
+ * number.
+ *
+ * @return a SwitchPort representing the Port
+ */
+ public SwitchPort asSwitchPort();
+
+ /**
* Gets the hardware address of this port.
* <p/>
* TODO Not implemented yet.
diff --git a/src/main/java/net/onrc/onos/core/topology/PortImpl.java b/src/main/java/net/onrc/onos/core/topology/PortImpl.java
index 32646d0..1f6946a 100644
--- a/src/main/java/net/onrc/onos/core/topology/PortImpl.java
+++ b/src/main/java/net/onrc/onos/core/topology/PortImpl.java
@@ -4,6 +4,8 @@
import java.util.HashSet;
import java.util.Set;
+import net.onrc.onos.core.util.SwitchPort;
+
/**
* Port Object stored in In-memory Topology.
* <p/>
@@ -17,8 +19,8 @@
private Long number;
private String description;
- protected Link outgoingLink;
- protected Link incomingLink;
+ private final SwitchPort switchPort;
+
// These needs to be ConcurrentCollecton if allowing Graph to be accessed Concurrently
protected Set<Device> devices;
@@ -27,6 +29,8 @@
this.sw = parentSwitch;
this.number = number;
this.devices = new HashSet<>();
+
+ switchPort = new SwitchPort(parentSwitch.getDpid(), number.shortValue());
}
@Override
@@ -40,6 +44,11 @@
}
@Override
+ public SwitchPort asSwitchPort() {
+ return switchPort;
+ }
+
+ @Override
public String getDescription() {
return description;
}
@@ -61,12 +70,14 @@
@Override
public Link getOutgoingLink() {
- return outgoingLink;
+ return graph.getOutgoingLink(switchPort.dpid().value(),
+ (long) switchPort.port().value());
}
@Override
public Link getIncomingLink() {
- return incomingLink;
+ return graph.getIncomingLink(switchPort.dpid().value(),
+ (long) switchPort.port().value());
}
@Override
@@ -74,14 +85,6 @@
return Collections.unmodifiableSet(this.devices);
}
- public void setOutgoingLink(Link link) {
- outgoingLink = link;
- }
-
- public void setIncomingLink(Link link) {
- incomingLink = link;
- }
-
/**
* @param d
* @return true if successfully added
diff --git a/src/main/java/net/onrc/onos/core/topology/SwitchImpl.java b/src/main/java/net/onrc/onos/core/topology/SwitchImpl.java
index 3fe053c..1e5e1e2 100644
--- a/src/main/java/net/onrc/onos/core/topology/SwitchImpl.java
+++ b/src/main/java/net/onrc/onos/core/topology/SwitchImpl.java
@@ -99,7 +99,7 @@
@Override
public Iterable<Link> getOutgoingLinks() {
LinkedList<Link> links = new LinkedList<Link>();
- for (Port port : getPorts()) {
+ for (Port port : ports.values()) {
Link link = port.getOutgoingLink();
if (link != null) {
links.add(link);
@@ -111,7 +111,7 @@
@Override
public Iterable<Link> getIncomingLinks() {
LinkedList<Link> links = new LinkedList<Link>();
- for (Port port : getPorts()) {
+ for (Port port : ports.values()) {
Link link = port.getIncomingLink();
if (link != null) {
links.add(link);
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 bf3874b..67e8e5d 100644
--- a/src/main/java/net/onrc/onos/core/topology/TopologyManager.java
+++ b/src/main/java/net/onrc/onos/core/topology/TopologyManager.java
@@ -951,10 +951,7 @@
assert (link == srcPort.getOutgoingLink());
if (link == null) {
link = new LinkImpl(networkGraph, srcPort, dstPort);
- PortImpl srcPortImpl = getPortImpl(srcPort);
- PortImpl dstPortImpl = getPortImpl(dstPort);
- srcPortImpl.setOutgoingLink(link);
- dstPortImpl.setIncomingLink(link);
+ networkGraph.putLink(link);
// Remove all Devices attached to the Ports
ArrayList<DeviceEvent> devicesToRemove = new ArrayList<>();
@@ -1019,8 +1016,11 @@
if (link == null) {
log.warn("Link {} already removed on src Port", linkEvent);
}
- getPortImpl(dstPort).setIncomingLink(null);
- getPortImpl(srcPort).setOutgoingLink(null);
+
+ // TODO should we check that we get the same link from each port?
+ if (link != null) {
+ networkGraph.removeLink(link);
+ }
apiRemovedLinkEvents.add(linkEvent);
}