Refactoring to eliminate duplicate DefaultTopology and DefaultTopologyGraph; eliminating a few fixmes.
Change-Id: Ie0e38dbf812bafdb7c94bba5278f0dd9af5be929
diff --git a/apps/fwd/src/main/java/org/onosproject/fwd/ReactiveForwarding.java b/apps/fwd/src/main/java/org/onosproject/fwd/ReactiveForwarding.java
index 078a7b1..f9bb3eb 100644
--- a/apps/fwd/src/main/java/org/onosproject/fwd/ReactiveForwarding.java
+++ b/apps/fwd/src/main/java/org/onosproject/fwd/ReactiveForwarding.java
@@ -474,11 +474,10 @@
// Otherwise, pick a path that does not lead back to where we
// came from; if no such path, flood and bail.
- Path path = pickForwardPath(paths, pkt.receivedFrom().port());
+ Path path = pickForwardPathIfPossible(paths, pkt.receivedFrom().port());
if (path == null) {
- log.warn("Doh... don't know where to go... {} -> {} received on {}",
- ethPkt.getSourceMAC(), ethPkt.getDestinationMAC(),
- pkt.receivedFrom());
+ log.warn("Don't know where to go from here {} for {} -> {}",
+ pkt.receivedFrom(), ethPkt.getSourceMAC(), ethPkt.getDestinationMAC());
flood(context);
return;
}
@@ -501,14 +500,16 @@
}
// Selects a path from the given set that does not lead back to the
- // specified port.
- private Path pickForwardPath(Set<Path> paths, PortNumber notToPort) {
+ // specified port if possible.
+ private Path pickForwardPathIfPossible(Set<Path> paths, PortNumber notToPort) {
+ Path lastPath = null;
for (Path path : paths) {
+ lastPath = path;
if (!path.src().port().equals(notToPort)) {
return path;
}
}
- return null;
+ return lastPath;
}
// Floods the specified packet if permissible.
@@ -734,7 +735,7 @@
Set<Path> pathsFromCurDevice =
topologyService.getPaths(topologyService.currentTopology(),
curDevice, dstId);
- if (pickForwardPath(pathsFromCurDevice, curLink.src().port()) != null) {
+ if (pickForwardPathIfPossible(pathsFromCurDevice, curLink.src().port()) != null) {
break;
} else {
if (i + 1 == pathLinks.size()) {
@@ -792,8 +793,8 @@
return builder.build();
}
- // Returns set of flowEntries which were created by this application and which egress from the
- // specified connection port
+ // Returns set of flow entries which were created by this application and
+ // which egress from the specified connection port
private Set<FlowEntry> getFlowRulesFrom(ConnectPoint egress) {
ImmutableSet.Builder<FlowEntry> builder = ImmutableSet.builder();
flowRuleService.getFlowEntries(egress.deviceId()).forEach(r -> {
diff --git a/core/api/src/main/java/org/onosproject/net/topology/DefaultGraphDescription.java b/core/api/src/main/java/org/onosproject/net/topology/DefaultGraphDescription.java
index d635d74..f1e20da 100644
--- a/core/api/src/main/java/org/onosproject/net/topology/DefaultGraphDescription.java
+++ b/core/api/src/main/java/org/onosproject/net/topology/DefaultGraphDescription.java
@@ -27,6 +27,7 @@
import java.util.Map;
+import static com.google.common.base.Preconditions.checkArgument;
import static org.slf4j.LoggerFactory.getLogger;
/**
@@ -133,9 +134,7 @@
private TopologyVertex vertexOf(ConnectPoint connectPoint) {
DeviceId id = connectPoint.deviceId();
TopologyVertex vertex = vertexesById.get(id);
- if (vertex == null) {
- throw new IllegalArgumentException("Vertex missing for " + id);
- }
+ checkArgument(vertex != null, "Vertex missing for %s", id);
return vertex;
}
diff --git a/core/common/src/main/java/org/onosproject/common/DefaultTopology.java b/core/common/src/main/java/org/onosproject/common/DefaultTopology.java
index 51857c7..579f336 100644
--- a/core/common/src/main/java/org/onosproject/common/DefaultTopology.java
+++ b/core/common/src/main/java/org/onosproject/common/DefaultTopology.java
@@ -50,7 +50,9 @@
import java.util.Set;
import static com.google.common.base.MoreObjects.toStringHelper;
+import static com.google.common.base.Preconditions.checkArgument;
import static org.onlab.graph.GraphPathSearch.ALL_PATHS;
+import static org.onlab.util.Tools.isNullOrEmpty;
import static org.onosproject.core.CoreService.CORE_PROVIDER_ID;
import static org.onosproject.net.Link.State.ACTIVE;
import static org.onosproject.net.Link.State.INACTIVE;
@@ -228,15 +230,12 @@
// Find the cluster to which the device belongs.
TopologyCluster cluster = clustersByDevice().get(connectPoint.deviceId());
- if (cluster == null) {
- throw new IllegalArgumentException("No cluster found for device "
- + connectPoint.deviceId());
- }
+ checkArgument(cluster != null, "No cluster found for device %s", connectPoint.deviceId());
// If the broadcast set is null or empty, or if the point explicitly
- // belongs to it, return true;
+ // belongs to it, return true.
Set<ConnectPoint> points = broadcastSets.get().get(cluster.id());
- return (points == null) || points.isEmpty() || points.contains(connectPoint);
+ return isNullOrEmpty(points) || points.contains(connectPoint);
}
/**
@@ -250,6 +249,16 @@
}
/**
+ * Returns the set of the cluster broadcast points.
+ *
+ * @param clusterId cluster identifier
+ * @return set of cluster broadcast points
+ */
+ public Set<ConnectPoint> broadcastPoints(ClusterId clusterId) {
+ return broadcastSets.get().get(clusterId);
+ }
+
+ /**
* Returns the set of pre-computed shortest paths between source and
* destination devices.
*
diff --git a/core/net/src/main/java/org/onosproject/net/topology/impl/DefaultTopologyProvider.java b/core/net/src/main/java/org/onosproject/net/topology/impl/DefaultTopologyProvider.java
index e9ab196..20a5ad3 100644
--- a/core/net/src/main/java/org/onosproject/net/topology/impl/DefaultTopologyProvider.java
+++ b/core/net/src/main/java/org/onosproject/net/topology/impl/DefaultTopologyProvider.java
@@ -279,8 +279,7 @@
try {
buildTopology(reasons);
} catch (Exception e) {
- log.warn("Unable to compute topology due to: {}", e.getMessage());
- log.debug("Unable to compute topology", e);
+ log.warn("Unable to compute topology", e);
}
}
}
diff --git a/core/store/dist/src/main/java/org/onosproject/store/topology/impl/DistributedTopologyStore.java b/core/store/dist/src/main/java/org/onosproject/store/topology/impl/DistributedTopologyStore.java
index 928603c..3a0f185 100644
--- a/core/store/dist/src/main/java/org/onosproject/store/topology/impl/DistributedTopologyStore.java
+++ b/core/store/dist/src/main/java/org/onosproject/store/topology/impl/DistributedTopologyStore.java
@@ -65,7 +65,7 @@
private volatile DefaultTopology current =
new DefaultTopology(ProviderId.NONE,
- new DefaultGraphDescription(0L, 0L,
+ new DefaultGraphDescription(0L, System.currentTimeMillis(),
Collections.<Device>emptyList(),
Collections.<Link>emptyList()));
@@ -147,8 +147,7 @@
}
// Have the default topology construct self from the description data.
- DefaultTopology newTopology =
- new DefaultTopology(providerId, graphDescription);
+ DefaultTopology newTopology = new DefaultTopology(providerId, graphDescription);
// Promote the new topology to current and return a ready-to-send event.
synchronized (this) {