Network Graph Refactoring: WIP
- Move method getPort() from TopologyManager to NetworkGraph
- Renamed NetworkGraph.getDeviceByIp() to getDevicesByIp() to reflect
the fact that it might return multiple devices.
- Added Javadoc comments to interface NetworkGraph
Change-Id: Ibc15497c287847716d92a5ee27a79deb9a635dd2
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 eff5952..45e4188 100644
--- a/src/main/java/net/onrc/onos/ofcontroller/networkgraph/NetworkGraph.java
+++ b/src/main/java/net/onrc/onos/ofcontroller/networkgraph/NetworkGraph.java
@@ -8,18 +8,76 @@
* The northbound interface to the topology network graph. This interface
* is presented to the rest of ONOS. It is currently read-only, as we want
* only the discovery modules to be allowed to modify the topology.
- *
*/
public interface NetworkGraph {
- public Switch getSwitch(Long dpid);
- public Iterable<Switch> getSwitches();
+ /**
+ * Get the switch for a given switch DPID.
+ *
+ * @param dpid the switch dpid.
+ * @return the switch if found, otherwise, null.
+ */
+ public Switch getSwitch(Long dpid);
- // TODO Not sure about the use-case of this method? Remove if not used at the end.
- public Iterable<Link> getLinks();
- // XXX next 2 method can be removed. getSwitch(dpid).getOutgoingLinks() is equivalent
- public Iterable<Link> getOutgoingLinksFromSwitch(Long dpid); // Toshi: unnecessary
- public Iterable<Link> getIncomingLinksFromSwitch(Long dpid); // Toshi: unnecessary
+ /**
+ * Get all switches in the network.
+ *
+ * @return all switches in the network.
+ */
+ public Iterable<Switch> getSwitches();
- public Iterable<Device> getDeviceByIp(InetAddress ipAddress);
- public Device getDeviceByMac(MACAddress address);
+ /**
+ * Get the port on a switch.
+ *
+ * @param dpid the switch DPID.
+ * @param number the switch port number.
+ * @return the switch port if found, otherwise, null.
+ */
+ public Port getPort(Long dpid, Long number);
+
+ /**
+ * Get all links in the network.
+ *
+ * TODO: Not clear if this method is needed. Remove if not used.
+ *
+ * @return all links in the network.
+ */
+ 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.
+ * @return the network devices for the IP address.
+ */
+ public Iterable<Device> getDevicesByIp(InetAddress ipAddress);
+
+ /**
+ * Get the network device for a given MAC address.
+ *
+ * @param address the MAC address to use.
+ * @return the network device for the MAC address if found, otherwise null.
+ */
+ public Device getDeviceByMac(MACAddress address);
}
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 4425a7d..628ad7c 100644
--- a/src/main/java/net/onrc/onos/ofcontroller/networkgraph/NetworkGraphImpl.java
+++ b/src/main/java/net/onrc/onos/ofcontroller/networkgraph/NetworkGraphImpl.java
@@ -53,6 +53,15 @@
}
@Override
+ public Port getPort(Long dpid, Long number) {
+ Switch sw = getSwitch(dpid);
+ if (sw != null) {
+ return sw.getPort(number);
+ }
+ return null;
+ }
+
+ @Override
public Iterable<Link> getLinks() {
List<Link> linklist = new LinkedList<>();
@@ -103,7 +112,7 @@
@Override
- public Iterable<Device> getDeviceByIp(InetAddress ipAddress) {
+ public Iterable<Device> getDevicesByIp(InetAddress ipAddress) {
Set<Device> devices = addr2Device.get(ipAddress);
if (devices == null) {
return Collections.emptySet();
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 4d56fed..fc57b3b 100644
--- a/src/main/java/net/onrc/onos/ofcontroller/networkgraph/TopologyManager.java
+++ b/src/main/java/net/onrc/onos/ofcontroller/networkgraph/TopologyManager.java
@@ -413,7 +413,8 @@
}
private boolean prepareForRemovePortEvent(PortEvent portEvt) {
- Port port = getPort(portEvt.getDpid(), portEvt.getNumber());
+ Port port = networkGraph.getPort(portEvt.getDpid(),
+ portEvt.getNumber());
if ( port == null ) {
log.debug("Port already removed? {}", portEvt);
// let it pass
@@ -453,8 +454,10 @@
private boolean prepareForAddLinkEvent(LinkEvent linkEvt) {
// Src/Dst Port must exist
- Port srcPort = getPort(linkEvt.getSrc().dpid, linkEvt.getSrc().number);
- Port dstPort = getPort(linkEvt.getDst().dpid, linkEvt.getDst().number);
+ Port srcPort = networkGraph.getPort(linkEvt.getSrc().dpid,
+ linkEvt.getSrc().number);
+ Port dstPort = networkGraph.getPort(linkEvt.getDst().dpid,
+ linkEvt.getDst().number);
if ( srcPort == null || dstPort == null ) {
log.warn("Dropping add link event because port doesn't exist: {}",
linkEvt);
@@ -483,8 +486,10 @@
private boolean prepareForRemoveLinkEvent(LinkEvent linkEvt) {
// Src/Dst Port must exist
- Port srcPort = getPort(linkEvt.getSrc().dpid, linkEvt.getSrc().number);
- Port dstPort = getPort(linkEvt.getDst().dpid, linkEvt.getDst().number);
+ Port srcPort = networkGraph.getPort(linkEvt.getSrc().dpid,
+ linkEvt.getSrc().number);
+ Port dstPort = networkGraph.getPort(linkEvt.getDst().dpid,
+ linkEvt.getDst().number);
if ( srcPort == null || dstPort == null ) {
log.warn("Dropping remove link event because port doesn't exist {}", linkEvt);
return false;
@@ -515,7 +520,7 @@
ArrayList<PortEvent.SwitchPort> failedSwitchPort = new ArrayList<>();
for ( PortEvent.SwitchPort swp : deviceEvt.getAttachmentPoints() ) {
// Attached Ports must exist
- Port port = getPort(swp.dpid, swp.number);
+ Port port = networkGraph.getPort(swp.dpid, swp.number);
if ( port == null ) {
preconditionBroken = true;
failedSwitchPort.add(swp);
@@ -792,7 +797,8 @@
throw new IllegalArgumentException("Link cannot be null");
}
- Port srcPort = getPort(linkEvt.getSrc().dpid, linkEvt.getSrc().number);
+ Port srcPort = networkGraph.getPort(linkEvt.getSrc().dpid,
+ linkEvt.getSrc().number);
if (srcPort == null) {
throw new BrokenInvariantException(
String.format(
@@ -800,7 +806,8 @@
linkEvt.getSrc() ));
}
- Port dstPort = getPort(linkEvt.getDst().dpid, linkEvt.getDst().number);
+ Port dstPort = networkGraph.getPort(linkEvt.getDst().dpid,
+ linkEvt.getDst().number);
if (dstPort == null) {
throw new BrokenInvariantException(
String.format(
@@ -861,13 +868,15 @@
throw new IllegalArgumentException("Link cannot be null");
}
- Port srcPort = getPort(linkEvt.getSrc().dpid, linkEvt.getSrc().number);
+ Port srcPort = networkGraph.getPort(linkEvt.getSrc().dpid,
+ linkEvt.getSrc().number);
if (srcPort == null) {
log.warn("Src Port for Link {} already removed, ignoring", linkEvt);
return;
}
- Port dstPort = getPort(linkEvt.getDst().dpid, linkEvt.getDst().number);
+ Port dstPort = networkGraph.getPort(linkEvt.getDst().dpid,
+ linkEvt.getDst().number);
if (dstPort == null) {
log.warn("Dst Port for Link {} already removed, ignoring", linkEvt);
return;
@@ -908,7 +917,7 @@
// for each attachment point
for (SwitchPort swp : deviceEvt.getAttachmentPoints() ) {
// Attached Ports must exist
- Port port = getPort(swp.dpid, swp.number);
+ Port port = networkGraph.getPort(swp.dpid, swp.number);
if ( port == null ) {
log.warn("Port for the attachment point {} did not exist. skipping mutation", swp);
continue;
@@ -941,7 +950,7 @@
// for each attachment point
for (SwitchPort swp : deviceEvt.getAttachmentPoints() ) {
// Attached Ports must exist
- Port port = getPort(swp.dpid, swp.number);
+ Port port = networkGraph.getPort(swp.dpid, swp.number);
if ( port == null ) {
log.warn("Port for the attachment point {} did not exist. skipping attachment point mutation", swp);
continue;
@@ -1011,15 +1020,6 @@
}
}
- // we might want to include this in NetworkGraph interface
- private Port getPort(Long dpid, Long number) {
- Switch sw = networkGraph.getSwitch(dpid);
- if (sw != null) {
- return sw.getPort(number);
- }
- return null;
- }
-
private SwitchImpl getSwitchImpl(Switch sw) {
if (sw instanceof SwitchImpl) {
return (SwitchImpl) sw;
@@ -1077,8 +1077,10 @@
for (RCLink l : RCLink.getAllLinks()) {
// check if src/dst switch/port exist before triggering event
- Port srcPort = getPort(l.getSrc().dpid, l.getSrc().number);
- Port dstPort = getPort(l.getDst().dpid, l.getDst().number);
+ Port srcPort = networkGraph.getPort(l.getSrc().dpid,
+ l.getSrc().number);
+ Port dstPort = networkGraph.getPort(l.getDst().dpid,
+ l.getDst().number);
if ( srcPort == null || dstPort == null ) {
continue;
}