Changes related to the "LinkCollectionIntent" type of intents
(e.g., Multipoint-to-singlepoint and Singlepoint-to-multipoint)
* Apply the Intent-defined traffic treatment only on the flowmods
on the ingress switch with ingress inport for a flowmod.
Previously, the traffic treatments were applied on each switch,
and semantically it is not the correct (default) behavior.
* Express the flowmods by explicitly specifying the expected inport
in the matching conditions for each flowmod.
Previously, the inport was not included in the matching conditions.
Change-Id: Iefe3e88f7b6257c18fd6f99c2740feabbe8eedc5
diff --git a/core/api/src/main/java/org/onosproject/net/intent/LinkCollectionIntent.java b/core/api/src/main/java/org/onosproject/net/intent/LinkCollectionIntent.java
index 4fedcec..c404c65 100644
--- a/core/api/src/main/java/org/onosproject/net/intent/LinkCollectionIntent.java
+++ b/core/api/src/main/java/org/onosproject/net/intent/LinkCollectionIntent.java
@@ -36,6 +36,7 @@
private final Set<Link> links;
+ private final Set<ConnectPoint> ingressPoints;
private final Set<ConnectPoint> egressPoints;
/**
@@ -46,6 +47,7 @@
* @param selector traffic match
* @param treatment action
* @param links traversed links
+ * @param ingressPoint ingress point
* @param egressPoint egress point
* @throws NullPointerException {@code path} is null
*/
@@ -53,8 +55,10 @@
TrafficSelector selector,
TrafficTreatment treatment,
Set<Link> links,
+ ConnectPoint ingressPoint,
ConnectPoint egressPoint) {
- this(appId, selector , treatment, links, egressPoint, Collections.emptyList());
+ this(appId, selector , treatment, links, ingressPoint, egressPoint,
+ Collections.emptyList());
}
/**
@@ -66,6 +70,7 @@
* @param selector traffic match
* @param treatment action
* @param links traversed links
+ * @param ingressPoint ingress point
* @param egressPoint egress point
* @param constraints optional list of constraints
* @throws NullPointerException {@code path} is null
@@ -74,10 +79,12 @@
TrafficSelector selector,
TrafficTreatment treatment,
Set<Link> links,
+ ConnectPoint ingressPoint,
ConnectPoint egressPoint,
List<Constraint> constraints) {
super(appId, resources(links), selector, treatment, constraints);
this.links = links;
+ this.ingressPoints = ImmutableSet.of(ingressPoint);
this.egressPoints = ImmutableSet.of(egressPoint);
}
@@ -89,6 +96,7 @@
* @param selector traffic match
* @param treatment action
* @param links traversed links
+ * @param ingressPoints Set of ingress point
* @param egressPoints Set of egress point
* @param constraints the constraints
* @throws NullPointerException {@code path} is null
@@ -97,11 +105,13 @@
TrafficSelector selector,
TrafficTreatment treatment,
Set<Link> links,
+ Set<ConnectPoint> ingressPoints,
Set<ConnectPoint> egressPoints,
List<Constraint> constraints) {
super(appId, resources(links), selector, treatment);
this.links = links;
+ this.ingressPoints = ImmutableSet.copyOf(ingressPoints);
this.egressPoints = ImmutableSet.copyOf(egressPoints);
}
@@ -111,6 +121,7 @@
protected LinkCollectionIntent() {
super();
this.links = null;
+ this.ingressPoints = null;
this.egressPoints = null;
}
@@ -125,9 +136,18 @@
}
/**
- * Returns the egress point of the intent.
+ * Returns the ingress points of the intent.
*
- * @return the egress point
+ * @return the ingress points
+ */
+ public Set<ConnectPoint> ingressPoints() {
+ return ingressPoints;
+ }
+
+ /**
+ * Returns the egress points of the intent.
+ *
+ * @return the egress points
*/
public Set<ConnectPoint> egressPoints() {
return egressPoints;
@@ -146,6 +166,7 @@
.add("selector", selector())
.add("treatment", treatment())
.add("links", links())
+ .add("ingress", ingressPoints())
.add("egress", egressPoints())
.toString();
}
diff --git a/core/api/src/test/java/org/onosproject/net/intent/LinkCollectionIntentTest.java b/core/api/src/test/java/org/onosproject/net/intent/LinkCollectionIntentTest.java
index 9282f0f..0327390 100644
--- a/core/api/src/test/java/org/onosproject/net/intent/LinkCollectionIntentTest.java
+++ b/core/api/src/test/java/org/onosproject/net/intent/LinkCollectionIntentTest.java
@@ -45,6 +45,7 @@
*/
public class LinkCollectionIntentTest extends IntentTest {
+ final ConnectPoint ingress = NetTestTools.connectPoint("ingress", 2);
final ConnectPoint egress = NetTestTools.connectPoint("egress", 3);
final TrafficSelector selector = new IntentTestsMocks.MockSelector();
final IntentTestsMocks.MockTreatment treatment = new IntentTestsMocks.MockTreatment();
@@ -70,6 +71,7 @@
selector,
treatment,
links1,
+ ingress,
egress);
final HashSet<Link> links2 = new HashSet<>();
@@ -79,6 +81,7 @@
selector,
treatment,
links2,
+ ingress,
egress);
new EqualsTester()
@@ -99,6 +102,7 @@
selector,
treatment,
links1,
+ ingress,
egress);
final Set<Link> createdLinks = collectionIntent.links();
@@ -106,6 +110,7 @@
assertThat(collectionIntent.isInstallable(), is(true));
assertThat(collectionIntent.treatment(), is(treatment));
assertThat(collectionIntent.selector(), is(selector));
+ assertThat(collectionIntent.ingressPoints(), is(ImmutableSet.of(ingress)));
assertThat(collectionIntent.egressPoints(), is(ImmutableSet.of(egress)));
assertThat(collectionIntent.resources(), hasSize(1));
final List<Constraint> createdConstraints = collectionIntent.constraints();
@@ -127,6 +132,7 @@
selector,
treatment,
links1,
+ ingress,
egress,
constraints);
@@ -135,6 +141,7 @@
assertThat(collectionIntent.isInstallable(), is(true));
assertThat(collectionIntent.treatment(), is(treatment));
assertThat(collectionIntent.selector(), is(selector));
+ assertThat(collectionIntent.ingressPoints(), is(ImmutableSet.of(ingress)));
assertThat(collectionIntent.egressPoints(), is(ImmutableSet.of(egress)));
final List<Constraint> createdConstraints = collectionIntent.constraints();
@@ -156,6 +163,7 @@
assertThat(collectionIntent.isInstallable(), is(true));
assertThat(collectionIntent.treatment(), nullValue());
assertThat(collectionIntent.selector(), nullValue());
+ assertThat(collectionIntent.ingressPoints(), nullValue());
assertThat(collectionIntent.egressPoints(), nullValue());
final List<Constraint> createdConstraints = collectionIntent.constraints();
@@ -170,6 +178,7 @@
selector,
treatment,
links1,
+ ingress,
egress);
}
@@ -181,6 +190,7 @@
selector,
treatment,
links2,
+ ingress,
egress);
}
}
diff --git a/core/net/src/main/java/org/onosproject/net/intent/impl/LinkCollectionIntentInstaller.java b/core/net/src/main/java/org/onosproject/net/intent/impl/LinkCollectionIntentInstaller.java
index 1ddf985..8607119 100644
--- a/core/net/src/main/java/org/onosproject/net/intent/impl/LinkCollectionIntentInstaller.java
+++ b/core/net/src/main/java/org/onosproject/net/intent/impl/LinkCollectionIntentInstaller.java
@@ -15,6 +15,7 @@
*/
package org.onosproject.net.intent.impl;
+import java.util.Collection;
import java.util.HashMap;
import java.util.HashSet;
import java.util.List;
@@ -78,10 +79,16 @@
@Override
public List<FlowRuleBatchOperation> install(LinkCollectionIntent intent) {
+ Map<DeviceId, Set<PortNumber>> inputMap = new HashMap<DeviceId, Set<PortNumber>>();
Map<DeviceId, Set<PortNumber>> outputMap = new HashMap<DeviceId, Set<PortNumber>>();
List<FlowRuleBatchEntry> rules = Lists.newLinkedList();
for (Link link : intent.links()) {
+ if (inputMap.get(link.dst().deviceId()) == null) {
+ inputMap.put(link.dst().deviceId(), new HashSet<PortNumber>());
+ }
+ inputMap.get(link.dst().deviceId()).add(link.dst().port());
+
if (outputMap.get(link.src().deviceId()) == null) {
outputMap.put(link.src().deviceId(), new HashSet<PortNumber>());
}
@@ -89,6 +96,15 @@
}
+ for (ConnectPoint ingressPoint : intent.ingressPoints()) {
+ if (inputMap.get(ingressPoint.deviceId()) == null) {
+ inputMap
+ .put(ingressPoint.deviceId(), new HashSet<PortNumber>());
+ }
+ inputMap.get(ingressPoint.deviceId()).add(ingressPoint.port());
+
+ }
+
for (ConnectPoint egressPoint : intent.egressPoints()) {
if (outputMap.get(egressPoint.deviceId()) == null) {
outputMap
@@ -99,8 +115,11 @@
}
for (Entry<DeviceId, Set<PortNumber>> entry : outputMap.entrySet()) {
- rules.add(createBatchEntry(FlowRuleOperation.ADD, intent,
- entry.getKey(), entry.getValue()));
+ Set<PortNumber> ingressPoints = inputMap.get(entry.getKey());
+ rules.addAll(createBatchEntries(FlowRuleOperation.ADD, intent,
+ entry.getKey(),
+ ingressPoints,
+ entry.getValue()));
}
return Lists.newArrayList(new FlowRuleBatchOperation(rules));
@@ -108,16 +127,31 @@
@Override
public List<FlowRuleBatchOperation> uninstall(LinkCollectionIntent intent) {
+ Map<DeviceId, Set<PortNumber>> inputMap = new HashMap<DeviceId, Set<PortNumber>>();
Map<DeviceId, Set<PortNumber>> outputMap = new HashMap<DeviceId, Set<PortNumber>>();
List<FlowRuleBatchEntry> rules = Lists.newLinkedList();
for (Link link : intent.links()) {
+ if (inputMap.get(link.dst().deviceId()) == null) {
+ inputMap.put(link.dst().deviceId(), new HashSet<PortNumber>());
+ }
+ inputMap.get(link.dst().deviceId()).add(link.dst().port());
+
if (outputMap.get(link.src().deviceId()) == null) {
outputMap.put(link.src().deviceId(), new HashSet<PortNumber>());
}
outputMap.get(link.src().deviceId()).add(link.src().port());
}
+ for (ConnectPoint ingressPoint : intent.ingressPoints()) {
+ if (inputMap.get(ingressPoint.deviceId()) == null) {
+ inputMap
+ .put(ingressPoint.deviceId(), new HashSet<PortNumber>());
+ }
+ inputMap.get(ingressPoint.deviceId()).add(ingressPoint.port());
+
+ }
+
for (ConnectPoint egressPoint : intent.egressPoints()) {
if (outputMap.get(egressPoint.deviceId()) == null) {
outputMap
@@ -127,8 +161,11 @@
}
for (Entry<DeviceId, Set<PortNumber>> entry : outputMap.entrySet()) {
- rules.add(createBatchEntry(FlowRuleOperation.REMOVE, intent,
- entry.getKey(), entry.getValue()));
+ Set<PortNumber> ingressPoints = inputMap.get(entry.getKey());
+ rules.addAll(createBatchEntries(FlowRuleOperation.REMOVE, intent,
+ entry.getKey(),
+ ingressPoints,
+ entry.getValue()));
}
return Lists.newArrayList(new FlowRuleBatchOperation(rules));
@@ -144,34 +181,71 @@
}
/**
- * Creates a FlowRuleBatchEntry based on the provided parameters.
+ * Creates a collection of FlowRuleBatchEntry based on the provided
+ * parameters.
*
* @param operation the FlowRuleOperation to use
* @param intent the link collection intent
* @param deviceId the device ID for the flow rule
- * @param outPorts the output port of the flow rule
- * @return the new flow rule batch entry
+ * @param inPorts the logical input ports of the flow rule
+ * @param outPorts the output ports of the flow rule
+ * @return a collection with the new flow rule batch entries
*/
- private FlowRuleBatchEntry createBatchEntry(FlowRuleOperation operation,
- LinkCollectionIntent intent,
- DeviceId deviceId,
- Set<PortNumber> outPorts) {
+ private Collection<FlowRuleBatchEntry> createBatchEntries(
+ FlowRuleOperation operation,
+ LinkCollectionIntent intent,
+ DeviceId deviceId,
+ Set<PortNumber> inPorts,
+ Set<PortNumber> outPorts) {
+ Collection<FlowRuleBatchEntry> result = Lists.newLinkedList();
+ Set<PortNumber> ingressPorts = new HashSet<PortNumber>();
- TrafficTreatment.Builder treatmentBuilder = DefaultTrafficTreatment
- .builder(intent.treatment());
-
- for (PortNumber outPort : outPorts) {
- treatmentBuilder.setOutput(outPort);
+ //
+ // Collect all ingress ports for this device.
+ // The intent treatment is applied only on those ports.
+ //
+ for (ConnectPoint cp : intent.ingressPoints()) {
+ if (cp.deviceId().equals(deviceId)) {
+ ingressPorts.add(cp.port());
+ }
}
- TrafficTreatment treatment = treatmentBuilder.build();
- TrafficSelector selector = DefaultTrafficSelector
- .builder(intent.selector()).build();
+ //
+ // Create two treatments: one for setting the output ports,
+ // and a second one that applies the intent treatment and sets the
+ // output ports.
+ // NOTE: The second one is created only if there are ingress ports.
+ //
+ TrafficTreatment.Builder defaultTreatmentBuilder =
+ DefaultTrafficTreatment.builder();
+ for (PortNumber outPort : outPorts) {
+ defaultTreatmentBuilder.setOutput(outPort);
+ }
+ TrafficTreatment defaultTreatment = defaultTreatmentBuilder.build();
+ TrafficTreatment intentTreatment = null;
+ if (!ingressPorts.isEmpty()) {
+ TrafficTreatment.Builder intentTreatmentBuilder =
+ DefaultTrafficTreatment.builder(intent.treatment());
+ for (PortNumber outPort : outPorts) {
+ intentTreatmentBuilder.setOutput(outPort);
+ }
+ intentTreatment = intentTreatmentBuilder.build();
+ }
- FlowRule rule = new DefaultFlowRule(deviceId,
+ for (PortNumber inPort : inPorts) {
+ TrafficSelector selector = DefaultTrafficSelector
+ .builder(intent.selector()).matchInport(inPort).build();
+ TrafficTreatment treatment = defaultTreatment;
+ if (ingressPorts.contains(inPort)) {
+ // Use the intent treatment if this is ingress port
+ treatment = intentTreatment;
+ }
+ FlowRule rule = new DefaultFlowRule(deviceId,
selector, treatment, 123,
appId, (short) (intent.id().fingerprint() & 0xffff), 0, true);
+ result.add(new FlowRuleBatchEntry(operation, rule));
+ }
- return new FlowRuleBatchEntry(operation, rule);
+ return result;
}
}
diff --git a/core/net/src/main/java/org/onosproject/net/intent/impl/MultiPointToSinglePointIntentCompiler.java b/core/net/src/main/java/org/onosproject/net/intent/impl/MultiPointToSinglePointIntentCompiler.java
index 6fb94c9..825dc57 100644
--- a/core/net/src/main/java/org/onosproject/net/intent/impl/MultiPointToSinglePointIntentCompiler.java
+++ b/core/net/src/main/java/org/onosproject/net/intent/impl/MultiPointToSinglePointIntentCompiler.java
@@ -16,6 +16,7 @@
package org.onosproject.net.intent.impl;
import java.util.Arrays;
+import java.util.Collections;
import java.util.HashMap;
import java.util.List;
import java.util.Map;
@@ -39,6 +40,7 @@
import org.onosproject.net.resource.LinkResourceAllocations;
import org.onosproject.net.topology.PathService;
+import com.google.common.collect.ImmutableSet;
import com.google.common.collect.Sets;
/**
@@ -75,8 +77,10 @@
for (Link link : path.links()) {
if (links.containsKey(link.src().deviceId())) {
// We've already reached the existing tree with the first
- // part of this path. Don't add the remainder of the path
+ // part of this path. Add the merging point with differen
+ // incoming port, but don't add the remainder of the path
// in case it differs from the path we already have.
+ links.put(link.src().deviceId(), link);
break;
}
@@ -86,7 +90,10 @@
Intent result = new LinkCollectionIntent(intent.appId(),
intent.selector(), intent.treatment(),
- Sets.newHashSet(links.values()), intent.egressPoint());
+ Sets.newHashSet(links.values()),
+ intent.ingressPoints(),
+ ImmutableSet.of(intent.egressPoint()),
+ Collections.emptyList());
return Arrays.asList(result);
}
diff --git a/core/net/src/main/java/org/onosproject/net/intent/impl/SinglePointToMultiPointIntentCompiler.java b/core/net/src/main/java/org/onosproject/net/intent/impl/SinglePointToMultiPointIntentCompiler.java
index f6e330c..91600ac 100644
--- a/core/net/src/main/java/org/onosproject/net/intent/impl/SinglePointToMultiPointIntentCompiler.java
+++ b/core/net/src/main/java/org/onosproject/net/intent/impl/SinglePointToMultiPointIntentCompiler.java
@@ -32,6 +32,8 @@
import org.onosproject.net.provider.ProviderId;
import org.onosproject.net.resource.LinkResourceAllocations;
+import com.google.common.collect.ImmutableSet;
+
@Component(immediate = true)
public class SinglePointToMultiPointIntentCompiler
extends ConnectivityIntentCompiler<SinglePointToMultiPointIntent> {
@@ -66,6 +68,7 @@
Intent result = new LinkCollectionIntent(intent.appId(),
intent.selector(),
intent.treatment(), links,
+ ImmutableSet.of(intent.ingressPoint()),
intent.egressPoints(), null);
return Arrays.asList(result);