Respect bidirectional link config
- Evict link in both direction, only if the configuration is bidirectional
Change-Id: I64b5d6dcedbbaf6e4e4a146e9dd123c8444c85b3
diff --git a/core/api/src/main/java/org/onosproject/net/config/basics/BasicLinkConfig.java b/core/api/src/main/java/org/onosproject/net/config/basics/BasicLinkConfig.java
index e9fea14..eeed07f 100644
--- a/core/api/src/main/java/org/onosproject/net/config/basics/BasicLinkConfig.java
+++ b/core/api/src/main/java/org/onosproject/net/config/basics/BasicLinkConfig.java
@@ -159,7 +159,7 @@
*
* @return true for bidirectional, false otherwise
*/
- public Boolean isBidirectional() {
+ public boolean isBidirectional() {
JsonNode res = object.path(IS_BIDIRECTIONAL);
if (res.isMissingNode()) {
return true;
diff --git a/core/net/src/main/java/org/onosproject/net/link/impl/LinkManager.java b/core/net/src/main/java/org/onosproject/net/link/impl/LinkManager.java
index 667b94e..0925c35 100644
--- a/core/net/src/main/java/org/onosproject/net/link/impl/LinkManager.java
+++ b/core/net/src/main/java/org/onosproject/net/link/impl/LinkManager.java
@@ -248,22 +248,36 @@
}
}
- // returns a LinkDescription made from the union of the BasicLinkConfig
- // annotations if it exists
+ /**
+ * Validates configuration against link configuration.
+ *
+ * @param linkDescription input
+ * @return description combined with configuration or null if disallowed
+ */
private LinkDescription validateLink(LinkDescription linkDescription) {
- // TODO Investigate whether this can be made more efficient
BasicLinkConfig cfg = networkConfigService.getConfig(linkKey(linkDescription.src(),
linkDescription.dst()),
BasicLinkConfig.class);
- BasicLinkConfig cfgTwo = networkConfigService.getConfig(linkKey(linkDescription.dst(),
- linkDescription.src()),
- BasicLinkConfig.class);
- if (isAllowed(cfg) && isAllowed(cfgTwo)) {
- return BasicLinkOperator.combine(cfg, linkDescription);
- } else {
+ if (!isAllowed(cfg)) {
log.trace("Link {} is not allowed", linkDescription);
return null;
}
+
+ // test if bidirectional reverse configuration exists
+ BasicLinkConfig cfgRev = networkConfigService.getConfig(linkKey(linkDescription.dst(),
+ linkDescription.src()),
+ BasicLinkConfig.class);
+ LinkDescription description = linkDescription;
+ if (cfgRev != null && cfgRev.isBidirectional()) {
+ if (!cfgRev.isAllowed()) {
+ log.trace("Link {} is not allowed (rev)", linkDescription);
+ return null;
+ }
+ description = BasicLinkOperator.combine(cfgRev, description);
+ }
+
+ description = BasicLinkOperator.combine(cfg, description);
+ return description;
}
@Override
@@ -343,7 +357,9 @@
if (!isAllowed(cfg)) {
log.info("Kicking out links between {} and {}", lk.src(), lk.dst());
removeLink(lk.src(), lk.dst());
- removeLink(lk.dst(), lk.src());
+ if (cfg.isBidirectional()) {
+ removeLink(lk.dst(), lk.src());
+ }
return;
}
@@ -358,6 +374,9 @@
LinkDescription desc;
if (link == null) {
+ // TODO Revisit this behaviour.
+ // config alone probably should not be adding a link,
+ // netcfg provider should be the one.
desc = BasicLinkOperator.descriptionOf(src, dst, cfg);
} else {
desc = BasicLinkOperator.combine(cfg,