Network Graph Refactoring: Northbound API cleanup
* Renamed/shortened methods:
Link.getSourcePort() -> getSrcPort()
Link.getDestinationPort() -> getDstPort()
Link.getSourceSwitch() -> getSrcSwitch()
Link.getDestinationSwitch() -> getDstSwitch()
* Added Javadoc comments
* Removed deprecated and unused methods:
Link.getSourceSwitchDpid()
Link.getSorcePortNumber()
Link.getDestinationSwitchDpid()
Link.getDestinationPortNumber()
NetworkGraph.getOutgoingLinksFromSwitch()
NetworkGraph.getIncomingLinksFromSwitch()
* Moved method LinkEvent.getLink(NetworkGraph graph)
to NetworkGraph.getLink()
Change-Id: Ibedba53fc6cd04d77f508ee67d41b2cfa515d3fc
diff --git a/src/main/java/net/onrc/onos/intent/ConstrainedBFSTree.java b/src/main/java/net/onrc/onos/intent/ConstrainedBFSTree.java
index a219d97..7f38883 100644
--- a/src/main/java/net/onrc/onos/intent/ConstrainedBFSTree.java
+++ b/src/main/java/net/onrc/onos/intent/ConstrainedBFSTree.java
@@ -44,7 +44,7 @@
while (!switchQueue.isEmpty()) {
Switch sw = switchQueue.poll();
for (Link link: sw.getOutgoingLinks()) {
- Switch reachedSwitch = link.getDestinationPort().getSwitch();
+ Switch reachedSwitch = link.getDstPort().getSwitch();
if (switchSearched.contains(reachedSwitch)) continue;
if (intents != null && intents.getAvailableBandwidth(link) < bandwidth) continue;
switchQueue.add(reachedSwitch);
diff --git a/src/main/java/net/onrc/onos/intent/runtime/PlanCalcRuntime.java b/src/main/java/net/onrc/onos/intent/runtime/PlanCalcRuntime.java
index 55cc475..18e962f 100644
--- a/src/main/java/net/onrc/onos/intent/runtime/PlanCalcRuntime.java
+++ b/src/main/java/net/onrc/onos/intent/runtime/PlanCalcRuntime.java
@@ -72,12 +72,15 @@
}
List<FlowEntry> entries = new ArrayList<>();
for(LinkEvent linkEvent : intent.getPath()) {
- Link link = linkEvent.getLink(graph);
- Switch sw = link.getSourceSwitch();
- dstPort = link.getSourcePort();
+ Link link = graph.getLink(linkEvent.getSrc().getDpid(),
+ linkEvent.getSrc().getNumber(),
+ linkEvent.getDst().getDpid(),
+ linkEvent.getDst().getNumber());
+ Switch sw = link.getSrcSwitch();
+ dstPort = link.getSrcPort();
FlowEntry fe = new FlowEntry(sw, srcPort, dstPort, srcMac, dstMac);
entries.add(fe);
- srcPort = link.getDestinationPort();
+ srcPort = link.getDstPort();
}
if(lastDstPort != null) {
Switch sw = lastDstPort.getSwitch();
diff --git a/src/main/java/net/onrc/onos/ofcontroller/networkgraph/Device.java b/src/main/java/net/onrc/onos/ofcontroller/networkgraph/Device.java
index cd5ddf8..c6e1ec2 100644
--- a/src/main/java/net/onrc/onos/ofcontroller/networkgraph/Device.java
+++ b/src/main/java/net/onrc/onos/ofcontroller/networkgraph/Device.java
@@ -15,12 +15,35 @@
*
*/
public interface Device {
- public MACAddress getMacAddress();
+ /**
+ * Get the device MAC address.
+ *
+ * @return the device MAC address.
+ */
+ public MACAddress getMacAddress();
- public Collection<InetAddress> getIpAddress();
+ /**
+ * Get the device IP addresses.
+ *
+ * @return the device IP addresses.
+ */
+ public Collection<InetAddress> getIpAddress();
- // Add requirement for Iteration order? Latest observed port first.
- public Iterable<Port> getAttachmentPoints();
+ /**
+ * Get the device attachment points.
+ *
+ * Add requirement for Iteration order? Latest observed port first.
+ *
+ * @return the device attachment points.
+ */
+ public Iterable<Port> getAttachmentPoints();
- public long getLastSeenTime();
+ /**
+ * Get the device last seen time.
+ *
+ * TODO: what is the time definition?
+ *
+ * @return the device last seen time.
+ */
+ public long getLastSeenTime();
}
diff --git a/src/main/java/net/onrc/onos/ofcontroller/networkgraph/DeviceImpl.java b/src/main/java/net/onrc/onos/ofcontroller/networkgraph/DeviceImpl.java
index 4a79edd..2b8930b 100644
--- a/src/main/java/net/onrc/onos/ofcontroller/networkgraph/DeviceImpl.java
+++ b/src/main/java/net/onrc/onos/ofcontroller/networkgraph/DeviceImpl.java
@@ -15,7 +15,6 @@
public class DeviceImpl extends NetworkGraphObject implements Device {
private final MACAddress macAddr;
- // These should be ConcurrentCollecton if Graph is going to be
protected LinkedList<Port> attachmentPoints;
protected Set<InetAddress> ipAddresses;
@@ -84,5 +83,4 @@
boolean removeIpAddress(InetAddress addr) {
return this.ipAddresses.remove(addr);
}
-
}
diff --git a/src/main/java/net/onrc/onos/ofcontroller/networkgraph/Link.java b/src/main/java/net/onrc/onos/ofcontroller/networkgraph/Link.java
index 4fd53af..996997a 100644
--- a/src/main/java/net/onrc/onos/ofcontroller/networkgraph/Link.java
+++ b/src/main/java/net/onrc/onos/ofcontroller/networkgraph/Link.java
@@ -8,25 +8,59 @@
*
*/
public interface Link {
- public Port getSourcePort();
- public Port getDestinationPort();
- public Switch getSourceSwitch();
- public Switch getDestinationSwitch();
+ /**
+ * Get the source switch for the link.
+ *
+ * @return the source switch for the link.
+ */
+ public Switch getSrcSwitch();
- public long getLastSeenTime();
+ /**
+ * Get the source port for the link.
+ *
+ * @return the source port for the link.
+ */
+ public Port getSrcPort();
- public int getCost();
- public Double getCapacity();
+ /**
+ * Get the destination switch for the link.
+ *
+ * @return the destination switch for the link.
+ */
+ public Switch getDstSwitch();
- // Not sure if we want to expose these northbound
- // Toshi: I think these are unnecessary because we can get them
- // Toshi: like "this.getSourcePort().getSwitch()" etc.
- @Deprecated
- public Long getSourceSwitchDpid();
- @Deprecated
- public Long getSourcePortNumber();
- @Deprecated
- public Long getDestinationSwitchDpid();
- @Deprecated
- public Long getDestinationPortNumber();
+ /**
+ * Get the destination port for the link.
+ *
+ * @return the destination port for the link.
+ */
+ public Port getDstPort();
+
+ /**
+ * Get the last seen time for the link.
+ *
+ * TODO: Not implemented yet.
+ * TODO: what is the time definition?
+ *
+ * @return the last seen time for the link.
+ */
+ public long getLastSeenTime();
+
+ /**
+ * Get the link cost.
+ *
+ * TODO: What is the unit?
+ *
+ * @param return the link cost.
+ */
+ public int getCost();
+
+ /**
+ * Get the link capacity.
+ *
+ * TODO: What is the unit?
+ *
+ * @return the link capacity.
+ */
+ public Double getCapacity();
}
diff --git a/src/main/java/net/onrc/onos/ofcontroller/networkgraph/LinkEvent.java b/src/main/java/net/onrc/onos/ofcontroller/networkgraph/LinkEvent.java
index 83af786..d12e99c 100644
--- a/src/main/java/net/onrc/onos/ofcontroller/networkgraph/LinkEvent.java
+++ b/src/main/java/net/onrc/onos/ofcontroller/networkgraph/LinkEvent.java
@@ -29,10 +29,10 @@
}
public LinkEvent(Link link) {
- src = new SwitchPort(link.getSourceSwitch().getDpid(),
- link.getSourcePort().getNumber());
- dst = new SwitchPort(link.getDestinationSwitch().getDpid(),
- link.getDestinationPort().getNumber());
+ src = new SwitchPort(link.getSrcSwitch().getDpid(),
+ link.getSrcPort().getNumber());
+ dst = new SwitchPort(link.getDstSwitch().getDpid(),
+ link.getDstPort().getNumber());
}
public SwitchPort getSrc() {
@@ -62,16 +62,6 @@
dst.getDpid(), dst.getNumber());
}
- public Link getLink(NetworkGraph graph) {
- Port srcPort = graph.getPort(getSrc().getDpid(), getSrc().getNumber());
- if (srcPort == null) return null;
- Link link = srcPort.getOutgoingLink();
- if (link == null) return null;
- if (link.getDestinationSwitch().getDpid() != getDst().getDpid()) return null;
- if (link.getDestinationPort().getNumber() != getDst().getNumber()) return null;
- return link;
- }
-
@Override
public int hashCode() {
final int prime = 31;
diff --git a/src/main/java/net/onrc/onos/ofcontroller/networkgraph/LinkImpl.java b/src/main/java/net/onrc/onos/ofcontroller/networkgraph/LinkImpl.java
index 801e780..bb598fe 100644
--- a/src/main/java/net/onrc/onos/ofcontroller/networkgraph/LinkImpl.java
+++ b/src/main/java/net/onrc/onos/ofcontroller/networkgraph/LinkImpl.java
@@ -30,6 +30,26 @@
setToPorts();
}
+ @Override
+ public Switch getSrcSwitch() {
+ return srcPort.getSwitch();
+ }
+
+ @Override
+ public Port getSrcPort() {
+ return srcPort;
+ }
+
+ @Override
+ public Switch getDstSwitch() {
+ return dstPort.getSwitch();
+ }
+
+ @Override
+ public Port getDstPort() {
+ return dstPort;
+ }
+
protected void setToPorts() {
((PortImpl)srcPort).setOutgoingLink(this);
((PortImpl)dstPort).setIncomingLink(this);
@@ -41,26 +61,6 @@
}
@Override
- public Port getSourcePort() {
- return srcPort;
- }
-
- @Override
- public Port getDestinationPort() {
- return dstPort;
- }
-
- @Override
- public Switch getSourceSwitch() {
- return srcPort.getSwitch();
- }
-
- @Override
- public Switch getDestinationSwitch() {
- return dstPort.getSwitch();
- }
-
- @Override
public long getLastSeenTime() {
// TODO Auto-generated method stub
return 0;
@@ -84,35 +84,11 @@
this.capacity = capacity;
}
- @Deprecated
- @Override
- public Long getSourceSwitchDpid() {
- return srcPort.getSwitch().getDpid();
- }
-
- @Deprecated
- @Override
- public Long getSourcePortNumber() {
- return srcPort.getNumber();
- }
-
- @Deprecated
- @Override
- public Long getDestinationSwitchDpid() {
- return dstPort.getSwitch().getDpid();
- }
-
- @Deprecated
- @Override
- public Long getDestinationPortNumber() {
- return dstPort.getNumber();
- }
-
@Override
public String toString() {
return String.format("%s --(cap:%f Mbps)--> %s",
- getSourcePort().toString(),
+ getSrcPort().toString(),
getCapacity(),
- getDestinationPort().toString());
+ getDstPort().toString());
}
}
diff --git a/src/main/java/net/onrc/onos/ofcontroller/networkgraph/NetworkGraph.java b/src/main/java/net/onrc/onos/ofcontroller/networkgraph/NetworkGraph.java
index 58c24ad..904e9a5 100644
--- a/src/main/java/net/onrc/onos/ofcontroller/networkgraph/NetworkGraph.java
+++ b/src/main/java/net/onrc/onos/ofcontroller/networkgraph/NetworkGraph.java
@@ -14,7 +14,7 @@
* Get the switch for a given switch DPID.
*
* @param dpid the switch dpid.
- * @return the switch if found, otherwise, null.
+ * @return the switch if found, otherwise null.
*/
public Switch getSwitch(Long dpid);
@@ -30,11 +30,33 @@
*
* @param dpid the switch DPID.
* @param number the switch port number.
- * @return the switch port if found, otherwise, null.
+ * @return the switch port if found, otherwise null.
*/
public Port getPort(Long dpid, Long number);
/**
+ * Get the outgoing link for a switch and a 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);
+
+ /**
+ * Get the outgoing link from a switch and a port to another switch and
+ * a port.
+ *
+ * @param srcDpid the source switch DPID.
+ * @param srcNumber the source switch port number.
+ * @param dstDpid the destination switch DPID.
+ * @param dstNumber the destination switch port number.
+ * @return the outgoing link if found, otherwise null.
+ */
+ public Link getLink(Long srcDpid, Long srcNumber, Long dstDpid,
+ Long dstNumber);
+
+ /**
* Get all links in the network.
*
* TODO: Not clear if this method is needed. Remove if not used.
@@ -44,28 +66,6 @@
public Iterable<Link> getLinks();
/**
- * Get all outgoing links for a switch.
- *
- * TODO: Not clear if this method is needed. Remove if not used.
- * E.g, getSwitch(dpid).getOutgoingLinks() is equivalent.
- *
- * @param dpid the switch DPID.
- * @return all outgoing links for a switch.
- */
- public Iterable<Link> getOutgoingLinksFromSwitch(Long dpid);
-
- /**
- * Get all incoming links for a switch.
- *
- * TODO: Not clear if this method is needed. Remove if not used.
- * E.g, getSwitch(dpid).getIncomingLinks() is equivalent.
- *
- * @param dpid the switch DPID.
- * @return all incoming links for a switch.
- */
- public Iterable<Link> getIncomingLinksFromSwitch(Long dpid);
-
- /**
* Get the network devices for a given IP address.
*
* @param ipAddress the IP address to use.
@@ -87,10 +87,10 @@
* {@link releaseLock()}. This method will block until a read lock is
* available.
*/
- public void acquireLock();
+ public void acquireLock();
- /**
- * Release the read lock on the topology.
- */
- public void releaseLock();
+ /**
+ * Release the read lock on the topology.
+ */
+ public void releaseLock();
}
diff --git a/src/main/java/net/onrc/onos/ofcontroller/networkgraph/NetworkGraphImpl.java b/src/main/java/net/onrc/onos/ofcontroller/networkgraph/NetworkGraphImpl.java
index d690821..83fc952 100644
--- a/src/main/java/net/onrc/onos/ofcontroller/networkgraph/NetworkGraphImpl.java
+++ b/src/main/java/net/onrc/onos/ofcontroller/networkgraph/NetworkGraphImpl.java
@@ -70,6 +70,27 @@
}
@Override
+ public Link getLink(Long dpid, Long number) {
+ Port srcPort = getPort(dpid, number);
+ if (srcPort == null)
+ return null;
+ return srcPort.getOutgoingLink();
+ }
+
+ @Override
+ public Link getLink(Long srcDpid, Long srcNumber, Long dstDpid,
+ Long dstNumber) {
+ Link link = getLink(srcDpid, srcNumber);
+ if (link == null)
+ return null;
+ if (link.getDstSwitch().getDpid() != dstDpid)
+ return null;
+ if (link.getDstPort().getNumber() != dstNumber)
+ return null;
+ return link;
+ }
+
+ @Override
public Iterable<Link> getLinks() {
List<Link> linklist = new LinkedList<>();
@@ -83,43 +104,6 @@
}
@Override
- public Iterable<Link> getOutgoingLinksFromSwitch(Long dpid) {
- Switch sw = getSwitch(dpid);
- if (sw == null) {
- return Collections.emptyList();
- }
- Iterable<Link> links = sw.getOutgoingLinks();
- if (links instanceof Collection) {
- return Collections.unmodifiableCollection((Collection<Link>) links);
- } else {
- List<Link> linklist = new LinkedList<>();
- for (Link l : links) {
- linklist.add(l);
- }
- return linklist;
- }
- }
-
- @Override
- public Iterable<Link> getIncomingLinksFromSwitch(Long dpid) {
- Switch sw = getSwitch(dpid);
- if (sw == null) {
- return Collections.emptyList();
- }
- Iterable<Link> links = sw.getIncomingLinks();
- if (links instanceof Collection) {
- return Collections.unmodifiableCollection((Collection<Link>) links);
- } else {
- List<Link> linklist = new LinkedList<>();
- for (Link l : links) {
- linklist.add(l);
- }
- return linklist;
- }
- }
-
-
- @Override
public Iterable<Device> getDevicesByIp(InetAddress ipAddress) {
Set<Device> devices = addr2Device.get(ipAddress);
if (devices == null) {
diff --git a/src/main/java/net/onrc/onos/ofcontroller/networkgraph/SwitchImpl.java b/src/main/java/net/onrc/onos/ofcontroller/networkgraph/SwitchImpl.java
index 97e9004..00f1236 100644
--- a/src/main/java/net/onrc/onos/ofcontroller/networkgraph/SwitchImpl.java
+++ b/src/main/java/net/onrc/onos/ofcontroller/networkgraph/SwitchImpl.java
@@ -52,11 +52,11 @@
public Iterable<Switch> getNeighbors() {
Set<Switch> neighbors = new HashSet<>();
for (Link link : getOutgoingLinks()) {
- neighbors.add(link.getDestinationSwitch());
+ neighbors.add(link.getDstSwitch());
}
// XXX should incoming considered neighbor?
for (Link link : getIncomingLinks()) {
- neighbors.add(link.getSourceSwitch());
+ neighbors.add(link.getSrcSwitch());
}
return neighbors;
}
@@ -64,7 +64,7 @@
@Override
public Link getLinkToNeighbor(Long neighborDpid) {
for (Link link : getOutgoingLinks()) {
- if (link.getDestinationSwitch().getDpid().equals(neighborDpid) ) {
+ if (link.getDstSwitch().getDpid().equals(neighborDpid) ) {
return link;
}
}
diff --git a/src/main/java/net/onrc/onos/ofcontroller/networkgraph/TopologyManager.java b/src/main/java/net/onrc/onos/ofcontroller/networkgraph/TopologyManager.java
index 0275575..a01057b 100644
--- a/src/main/java/net/onrc/onos/ofcontroller/networkgraph/TopologyManager.java
+++ b/src/main/java/net/onrc/onos/ofcontroller/networkgraph/TopologyManager.java
@@ -417,10 +417,10 @@
}
log.debug("Removing Link {} on Port {}", link, portEvent);
LinkEvent linkEvent =
- new LinkEvent(link.getSourceSwitchDpid(),
- link.getSourcePortNumber(),
- link.getDestinationSwitchDpid(),
- link.getDestinationPortNumber());
+ new LinkEvent(link.getSrcSwitch().getDpid(),
+ link.getSrcPort().getNumber(),
+ link.getDstSwitch().getDpid(),
+ link.getDstPort().getNumber());
// calling Discovery API to wipe from DB, etc.
// Call internal remove Link, which will check
@@ -480,8 +480,8 @@
// XXX Check if we should reject or just accept these cases.
// it should be harmless to remove the Link on event from DB anyways
if (link == null ||
- !link.getDestinationPortNumber().equals(linkEvent.getDst().number)
- || !link.getDestinationSwitchDpid().equals(linkEvent.getDst().dpid)) {
+ !link.getDstPort().getNumber().equals(linkEvent.getDst().number)
+ || !link.getDstSwitch().getDpid().equals(linkEvent.getDst().dpid)) {
log.warn("Dropping remove link event because link doesn't exist: {}", linkEvent);
return false;
}
@@ -760,10 +760,10 @@
continue;
}
log.debug("Removing Link {} on Port {}", link, portEvent);
- LinkEvent linkEvent = new LinkEvent(link.getSourceSwitchDpid(),
- link.getSourcePortNumber(),
- link.getDestinationSwitchDpid(),
- link.getDestinationPortNumber());
+ LinkEvent linkEvent = new LinkEvent(link.getSrcSwitch().getDpid(),
+ link.getSrcPort().getNumber(),
+ link.getDstSwitch().getDpid(),
+ link.getDstPort().getNumber());
linksToRemove.add(linkEvent);
}
for (LinkEvent linkEvent : linksToRemove) {
diff --git a/src/main/java/net/onrc/onos/ofcontroller/networkgraph/serializers/LinkSerializer.java b/src/main/java/net/onrc/onos/ofcontroller/networkgraph/serializers/LinkSerializer.java
index bb13c75..57bcbe8 100644
--- a/src/main/java/net/onrc/onos/ofcontroller/networkgraph/serializers/LinkSerializer.java
+++ b/src/main/java/net/onrc/onos/ofcontroller/networkgraph/serializers/LinkSerializer.java
@@ -22,13 +22,13 @@
throws IOException, JsonGenerationException {
jsonGenerator.writeStartObject();
jsonGenerator.writeStringField("src-switch",
- HexString.toHexString(link.getSourceSwitch().getDpid()));
+ HexString.toHexString(link.getSrcSwitch().getDpid()));
jsonGenerator.writeNumberField("src-port",
- link.getSourcePort().getNumber());
+ link.getSrcPort().getNumber());
jsonGenerator.writeStringField("dst-switch",
- HexString.toHexString(link.getDestinationSwitch().getDpid()));
+ HexString.toHexString(link.getDstSwitch().getDpid()));
jsonGenerator.writeNumberField("dst-port",
- link.getDestinationPort().getNumber());
+ link.getDstPort().getNumber());
jsonGenerator.writeEndObject();
}
diff --git a/src/test/java/net/onrc/onos/intent/MockNetworkGraph.java b/src/test/java/net/onrc/onos/intent/MockNetworkGraph.java
index 263cbf4..a8f928a 100644
--- a/src/test/java/net/onrc/onos/intent/MockNetworkGraph.java
+++ b/src/test/java/net/onrc/onos/intent/MockNetworkGraph.java
@@ -77,7 +77,7 @@
public void removeLink(Long srcDpid, Long srcPortNo, Long dstDpid, Long dstPortNo) {
DetachableLinkImpl link = (DetachableLinkImpl)getSwitch(srcDpid).getPort(srcPortNo).getOutgoingLink();
- if (link.getDestinationSwitch().getDpid().equals(dstDpid) && link.getDestinationPort().getNumber().equals(dstPortNo)) {
+ if (link.getDstSwitch().getDpid().equals(dstDpid) && link.getDstPort().getNumber().equals(dstPortNo)) {
link.detachFromGraph();
}
}