Sketching out what link-state addition would look like; quite easy until we get to the distributed store.
Added unit tests to provide durable-nondurable transitions.
FIxed issue where link could be accidentally activated.
Renamed parameter.
Change-Id: I8aa19a6583ec50dbf28769995f0a8ea9be9a4daa
diff --git a/core/store/trivial/src/main/java/org/onlab/onos/store/trivial/impl/DefaultTopology.java b/core/store/trivial/src/main/java/org/onlab/onos/store/trivial/impl/DefaultTopology.java
index 2cae2c3..5478806 100644
--- a/core/store/trivial/src/main/java/org/onlab/onos/store/trivial/impl/DefaultTopology.java
+++ b/core/store/trivial/src/main/java/org/onlab/onos/store/trivial/impl/DefaultTopology.java
@@ -49,6 +49,8 @@
import static org.onlab.graph.GraphPathSearch.Result;
import static org.onlab.graph.TarjanGraphSearch.SCCResult;
import static org.onlab.onos.core.CoreService.CORE_PROVIDER_ID;
+import static org.onlab.onos.net.Link.State.ACTIVE;
+import static org.onlab.onos.net.Link.State.INACTIVE;
import static org.onlab.onos.net.Link.Type.INDIRECT;
/**
@@ -434,7 +436,8 @@
public double weight(TopologyEdge edge) {
// To force preference to use direct paths first, make indirect
// links as expensive as the linear vertex traversal.
- return edge.link().type() == INDIRECT ? indirectLinkCost : 1;
+ return edge.link().state() == ACTIVE ?
+ (edge.link().type() == INDIRECT ? indirectLinkCost : 1) : -1;
}
}
@@ -442,7 +445,7 @@
private static class NoIndirectLinksWeight implements LinkWeight {
@Override
public double weight(TopologyEdge edge) {
- return edge.link().type() == INDIRECT ? -1 : 1;
+ return edge.link().state() == INACTIVE || edge.link().type() == INDIRECT ? -1 : 1;
}
}
diff --git a/core/store/trivial/src/main/java/org/onlab/onos/store/trivial/impl/SimpleLinkStore.java b/core/store/trivial/src/main/java/org/onlab/onos/store/trivial/impl/SimpleLinkStore.java
index 50f4e06..a383167 100644
--- a/core/store/trivial/src/main/java/org/onlab/onos/store/trivial/impl/SimpleLinkStore.java
+++ b/core/store/trivial/src/main/java/org/onlab/onos/store/trivial/impl/SimpleLinkStore.java
@@ -19,20 +19,20 @@
import com.google.common.collect.FluentIterable;
import com.google.common.collect.HashMultimap;
import com.google.common.collect.SetMultimap;
-
import org.apache.felix.scr.annotations.Activate;
import org.apache.felix.scr.annotations.Component;
import org.apache.felix.scr.annotations.Deactivate;
import org.apache.felix.scr.annotations.Service;
+import org.onlab.onos.net.AnnotationKeys;
import org.onlab.onos.net.AnnotationsUtil;
import org.onlab.onos.net.ConnectPoint;
import org.onlab.onos.net.DefaultAnnotations;
import org.onlab.onos.net.DefaultLink;
import org.onlab.onos.net.DeviceId;
import org.onlab.onos.net.Link;
-import org.onlab.onos.net.SparseAnnotations;
import org.onlab.onos.net.Link.Type;
import org.onlab.onos.net.LinkKey;
+import org.onlab.onos.net.SparseAnnotations;
import org.onlab.onos.net.link.DefaultLinkDescription;
import org.onlab.onos.net.link.LinkDescription;
import org.onlab.onos.net.link.LinkEvent;
@@ -46,22 +46,24 @@
import java.util.HashMap;
import java.util.HashSet;
import java.util.Map;
+import java.util.Map.Entry;
import java.util.Objects;
import java.util.Set;
-import java.util.Map.Entry;
import java.util.concurrent.ConcurrentHashMap;
import java.util.concurrent.ConcurrentMap;
-import static org.onlab.onos.net.DefaultAnnotations.union;
+import static com.google.common.base.Predicates.notNull;
+import static com.google.common.base.Verify.verifyNotNull;
+import static com.google.common.collect.Multimaps.synchronizedSetMultimap;
import static org.onlab.onos.net.DefaultAnnotations.merge;
+import static org.onlab.onos.net.DefaultAnnotations.union;
+import static org.onlab.onos.net.Link.State.ACTIVE;
+import static org.onlab.onos.net.Link.State.INACTIVE;
import static org.onlab.onos.net.Link.Type.DIRECT;
import static org.onlab.onos.net.Link.Type.INDIRECT;
import static org.onlab.onos.net.LinkKey.linkKey;
import static org.onlab.onos.net.link.LinkEvent.Type.*;
import static org.slf4j.LoggerFactory.getLogger;
-import static com.google.common.collect.Multimaps.synchronizedSetMultimap;
-import static com.google.common.base.Predicates.notNull;
-import static com.google.common.base.Verify.verifyNotNull;
/**
* Manages inventory of infrastructure links using trivial in-memory structures
@@ -77,7 +79,7 @@
// Link inventory
private final ConcurrentMap<LinkKey, Map<ProviderId, LinkDescription>>
- linkDescs = new ConcurrentHashMap<>();
+ linkDescs = new ConcurrentHashMap<>();
// Link instance cache
private final ConcurrentMap<LinkKey, Link> links = new ConcurrentHashMap<>();
@@ -116,9 +118,9 @@
// lock for iteration
synchronized (srcLinks) {
return FluentIterable.from(srcLinks.get(deviceId))
- .transform(lookupLink())
- .filter(notNull())
- .toSet();
+ .transform(lookupLink())
+ .filter(notNull())
+ .toSet();
}
}
@@ -127,9 +129,9 @@
// lock for iteration
synchronized (dstLinks) {
return FluentIterable.from(dstLinks.get(deviceId))
- .transform(lookupLink())
- .filter(notNull())
- .toSet();
+ .transform(lookupLink())
+ .filter(notNull())
+ .toSet();
}
}
@@ -178,11 +180,30 @@
}
}
+ @Override
+ public LinkEvent removeOrDownLink(ConnectPoint src, ConnectPoint dst) {
+ Link link = getLink(src, dst);
+ if (link == null) {
+ return null;
+ }
+
+ if (link.isDurable()) {
+ return link.state() == INACTIVE ? null :
+ updateLink(linkKey(link.src(), link.dst()), link,
+ new DefaultLink(link.providerId(),
+ link.src(), link.dst(),
+ link.type(), INACTIVE,
+ link.isDurable(),
+ link.annotations()));
+ }
+ return removeLink(src, dst);
+ }
+
// Guarded by linkDescs value (=locking each Link)
private LinkDescription createOrUpdateLinkDescription(
- Map<ProviderId, LinkDescription> descs,
- ProviderId providerId,
- LinkDescription linkDescription) {
+ Map<ProviderId, LinkDescription> descs,
+ ProviderId providerId,
+ LinkDescription linkDescription) {
// merge existing attributes and merge
LinkDescription oldDesc = descs.get(providerId);
@@ -196,11 +217,10 @@
newType = linkDescription.type();
}
SparseAnnotations merged = union(oldDesc.annotations(),
- linkDescription.annotations());
- newDesc = new DefaultLinkDescription(
- linkDescription.src(),
- linkDescription.dst(),
- newType, merged);
+ linkDescription.annotations());
+ newDesc = new DefaultLinkDescription(linkDescription.src(),
+ linkDescription.dst(),
+ newType, merged);
}
return descs.put(providerId, newDesc);
}
@@ -217,8 +237,9 @@
// Updates, if necessary the specified link and returns the appropriate event.
// Guarded by linkDescs value (=locking each Link)
private LinkEvent updateLink(LinkKey key, Link oldLink, Link newLink) {
- if ((oldLink.type() == INDIRECT && newLink.type() == DIRECT) ||
- !AnnotationsUtil.isEqual(oldLink.annotations(), newLink.annotations())) {
+ if (oldLink.state() != newLink.state() ||
+ (oldLink.type() == INDIRECT && newLink.type() == DIRECT) ||
+ !AnnotationsUtil.isEqual(oldLink.annotations(), newLink.annotations())) {
links.put(key, newLink);
// strictly speaking following can be ommitted
@@ -234,11 +255,7 @@
final LinkKey key = linkKey(src, dst);
Map<ProviderId, LinkDescription> descs = getOrCreateLinkDescriptions(key);
synchronized (descs) {
- Link link = links.get(key);
- if (isDurable(link)) {
- return null;
- }
- links.remove(key);
+ Link link = links.remove(key);
descs.clear();
if (link != null) {
srcLinks.remove(link.src().deviceId(), key);
@@ -249,11 +266,6 @@
}
}
- // Indicates if the link has been marked as durable via annotations.
- private boolean isDurable(Link link) {
- return link != null && Objects.equals(link.annotations().value("durable"), "true");
- }
-
private static <K, V> SetMultimap<K, V> createSynchronizedHashMultiMap() {
return synchronizedSetMultimap(HashMultimap.<K, V>create());
}
@@ -301,7 +313,10 @@
annotations = merge(annotations, e.getValue().annotations());
}
- return new DefaultLink(primary , src, dst, type, annotations);
+ boolean isDurable = Objects.equals(annotations.value(AnnotationKeys.DURABLE), "true");
+ boolean isActive = !Objects.equals(annotations.value(AnnotationKeys.INACTIVE), "true");
+ return new DefaultLink(primary, src, dst, type,
+ isActive ? ACTIVE : INACTIVE, isDurable, annotations);
}
private Map<ProviderId, LinkDescription> getOrCreateLinkDescriptions(LinkKey key) {
@@ -321,6 +336,7 @@
}
private final Function<LinkKey, Link> lookupLink = new LookupLink();
+
private Function<LinkKey, Link> lookupLink() {
return lookupLink;
}
diff --git a/core/store/trivial/src/test/java/org/onlab/onos/store/trivial/impl/SimpleLinkStoreTest.java b/core/store/trivial/src/test/java/org/onlab/onos/store/trivial/impl/SimpleLinkStoreTest.java
index 735f99c..cfd9d24 100644
--- a/core/store/trivial/src/test/java/org/onlab/onos/store/trivial/impl/SimpleLinkStoreTest.java
+++ b/core/store/trivial/src/test/java/org/onlab/onos/store/trivial/impl/SimpleLinkStoreTest.java
@@ -23,6 +23,7 @@
import org.junit.BeforeClass;
import org.junit.Ignore;
import org.junit.Test;
+import org.onlab.onos.net.AnnotationKeys;
import org.onlab.onos.net.ConnectPoint;
import org.onlab.onos.net.DefaultAnnotations;
import org.onlab.onos.net.DeviceId;
@@ -80,6 +81,23 @@
.set("B4", "b4")
.build();
+ private static final SparseAnnotations DA1 = DefaultAnnotations.builder()
+ .set("A1", "a1")
+ .set("B1", "b1")
+ .set(AnnotationKeys.DURABLE, "true")
+ .build();
+ private static final SparseAnnotations DA2 = DefaultAnnotations.builder()
+ .set("A2", "a2")
+ .set("B2", "b2")
+ .set(AnnotationKeys.DURABLE, "true")
+ .build();
+ private static final SparseAnnotations NDA1 = DefaultAnnotations.builder()
+ .set("A1", "a1")
+ .set("B1", "b1")
+ .remove(AnnotationKeys.DURABLE)
+ .build();
+
+
private SimpleLinkStore simpleLinkStore;
private LinkStore linkStore;
@@ -105,17 +123,19 @@
}
private void putLink(DeviceId srcId, PortNumber srcNum,
- DeviceId dstId, PortNumber dstNum, Type type,
+ DeviceId dstId, PortNumber dstNum,
+ Type type, boolean isDurable,
SparseAnnotations... annotations) {
ConnectPoint src = new ConnectPoint(srcId, srcNum);
ConnectPoint dst = new ConnectPoint(dstId, dstNum);
- linkStore.createOrUpdateLink(PID, new DefaultLinkDescription(src, dst, type, annotations));
+ linkStore.createOrUpdateLink(PID, new DefaultLinkDescription(src, dst, type,
+ annotations));
}
private void putLink(LinkKey key, Type type, SparseAnnotations... annotations) {
putLink(key.src().deviceId(), key.src().port(),
key.dst().deviceId(), key.dst().port(),
- type, annotations);
+ type, false, annotations);
}
private static void assertLink(DeviceId srcId, PortNumber srcNum,
@@ -138,9 +158,9 @@
public final void testGetLinkCount() {
assertEquals("initialy empty", 0, linkStore.getLinkCount());
- putLink(DID1, P1, DID2, P2, DIRECT);
- putLink(DID2, P2, DID1, P1, DIRECT);
- putLink(DID1, P1, DID2, P2, DIRECT);
+ putLink(DID1, P1, DID2, P2, DIRECT, false);
+ putLink(DID2, P2, DID1, P1, DIRECT, false);
+ putLink(DID1, P1, DID2, P2, DIRECT, false);
assertEquals("expecting 2 unique link", 2, linkStore.getLinkCount());
}
@@ -360,6 +380,47 @@
@Test
+ public final void testRemoveOrDownLink() {
+ removeOrDownLink(false);
+ }
+
+ @Test
+ public final void testRemoveOrDownLinkDurable() {
+ removeOrDownLink(true);
+ }
+
+ private void removeOrDownLink(boolean isDurable) {
+ final ConnectPoint d1P1 = new ConnectPoint(DID1, P1);
+ final ConnectPoint d2P2 = new ConnectPoint(DID2, P2);
+ LinkKey linkId1 = LinkKey.linkKey(d1P1, d2P2);
+ LinkKey linkId2 = LinkKey.linkKey(d2P2, d1P1);
+
+ putLink(linkId1, DIRECT, isDurable ? DA1 : A1);
+ putLink(linkId2, DIRECT, isDurable ? DA2 : A2);
+
+ // DID1,P1 => DID2,P2
+ // DID2,P2 => DID1,P1
+ // DID1,P2 => DID2,P3
+
+ LinkEvent event = linkStore.removeOrDownLink(d1P1, d2P2);
+ assertEquals(isDurable ? LINK_UPDATED : LINK_REMOVED, event.type());
+ assertAnnotationsEquals(event.subject().annotations(), isDurable ? DA1 : A1);
+ LinkEvent event2 = linkStore.removeOrDownLink(d1P1, d2P2);
+ assertNull(event2);
+
+ assertLink(linkId2, DIRECT, linkStore.getLink(d2P2, d1P1));
+ assertAnnotationsEquals(linkStore.getLink(d2P2, d1P1).annotations(),
+ isDurable ? DA2 : A2);
+
+ // annotations, etc. should not survive remove
+ if (!isDurable) {
+ putLink(linkId1, DIRECT);
+ assertLink(linkId1, DIRECT, linkStore.getLink(d1P1, d2P2));
+ assertAnnotationsEquals(linkStore.getLink(d1P1, d2P2).annotations());
+ }
+ }
+
+ @Test
public final void testRemoveLink() {
final ConnectPoint d1P1 = new ConnectPoint(DID1, P1);
final ConnectPoint d2P2 = new ConnectPoint(DID2, P2);
@@ -402,6 +463,30 @@
assertNotNull(linkStore.getLink(src, dst));
}
+ @Test
+ public void testDurableToNonDurable() {
+ final ConnectPoint d1P1 = new ConnectPoint(DID1, P1);
+ final ConnectPoint d2P2 = new ConnectPoint(DID2, P2);
+ LinkKey linkId1 = LinkKey.linkKey(d1P1, d2P2);
+
+ putLink(linkId1, DIRECT, DA1);
+ assertTrue("should be be durable", linkStore.getLink(d1P1, d2P2).isDurable());
+ putLink(linkId1, DIRECT, NDA1);
+ assertFalse("should not be durable", linkStore.getLink(d1P1, d2P2).isDurable());
+ }
+
+ @Test
+ public void testNonDurableToDurable() {
+ final ConnectPoint d1P1 = new ConnectPoint(DID1, P1);
+ final ConnectPoint d2P2 = new ConnectPoint(DID2, P2);
+ LinkKey linkId1 = LinkKey.linkKey(d1P1, d2P2);
+
+ putLink(linkId1, DIRECT, A1);
+ assertFalse("should not be durable", linkStore.getLink(d1P1, d2P2).isDurable());
+ putLink(linkId1, DIRECT, DA1);
+ assertTrue("should be durable", linkStore.getLink(d1P1, d2P2).isDurable());
+ }
+
// If Delegates should be called only on remote events,
// then Simple* should never call them, thus not test required.
@Ignore("Ignore until Delegate spec. is clear.")
@@ -451,7 +536,7 @@
linkStore.unsetDelegate(checkUpdate);
linkStore.setDelegate(checkRemove);
- linkStore.removeLink(d1P1, d2P2);
+ linkStore.removeOrDownLink(d1P1, d2P2);
assertTrue("Remove event fired", removeLatch.await(1, TimeUnit.SECONDS));
}
}