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.
[Merge from branch onos-1.0 - manually]
Change-Id: Ic378b6e8be033a70b016f4ba5550d91fe08ddd9a
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 852e143..55c0cfb 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,16 +36,19 @@
private final Set<Link> links;
+ private final Set<ConnectPoint> ingressPoints;
private final Set<ConnectPoint> egressPoints;
/**
- * Creates a new actionable intent capable of funneling the selected traffic
- * along the specified convergent tree and out the given egress point.
+ * Creates a new actionable intent capable of funneling the selected
+ * traffic along the specified convergent tree and out the given egress
+ * point.
*
* @param appId application identifier
* @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,19 +56,22 @@
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());
}
/**
* Creates a new actionable intent capable of funneling the selected
- * traffic along the specified convergent tree and out the given egress point
- * satisfying the specified constraints.
+ * traffic along the specified convergent tree and out the given egress
+ * point satisfying the specified constraints.
*
* @param appId application identifier
* @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,22 +80,26 @@
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);
}
/**
- * Creates a new actionable intent capable of funneling the selected traffic
- * along the specified convergent tree and out the given egress point.
+ * Creates a new actionable intent capable of funneling the selected
+ * traffic along the specified convergent tree and out the given egress
+ * point.
*
* @param appId application identifier
* @param selector traffic match
* @param treatment action
* @param links traversed links
- * @param egressPoints Set of egress point
+ * @param ingressPoints Set of ingress points
+ * @param egressPoints Set of egress points
* @param constraints the constraints
* @throws NullPointerException {@code path} is null
*/
@@ -97,11 +107,13 @@
TrafficSelector selector,
TrafficTreatment treatment,
Set<Link> links,
+ Set<ConnectPoint> ingressPoints,
Set<ConnectPoint> egressPoints,
List<Constraint> constraints) {
super(appId, resources(links), selector, treatment, constraints);
this.links = links;
+ this.ingressPoints = ImmutableSet.copyOf(ingressPoints);
this.egressPoints = ImmutableSet.copyOf(egressPoints);
}
@@ -111,6 +123,7 @@
protected LinkCollectionIntent() {
super();
this.links = null;
+ this.ingressPoints = null;
this.egressPoints = null;
}
@@ -125,9 +138,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;
@@ -148,6 +170,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 260bd5b..0d788e5 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
@@ -43,9 +43,9 @@
import org.onosproject.net.intent.LinkCollectionIntent;
import java.util.Collection;
+import java.util.HashSet;
import java.util.List;
import java.util.Set;
-import java.util.stream.Collectors;
/**
* Installer for {@link org.onosproject.net.intent.LinkCollectionIntent} path
@@ -88,35 +88,29 @@
LinkCollectionIntent intent, FlowRuleOperation.Type operation) {
//TODO do we need a set here?
+ SetMultimap<DeviceId, PortNumber> inputPorts = HashMultimap.create();
SetMultimap<DeviceId, PortNumber> outputPorts = HashMultimap.create();
for (Link link : intent.links()) {
+ inputPorts.put(link.dst().deviceId(), link.dst().port());
outputPorts.put(link.src().deviceId(), link.src().port());
}
+ for (ConnectPoint ingressPoint : intent.ingressPoints()) {
+ inputPorts.put(ingressPoint.deviceId(), ingressPoint.port());
+ }
+
for (ConnectPoint egressPoint : intent.egressPoints()) {
outputPorts.put(egressPoint.deviceId(), egressPoint.port());
}
- //FIXME change to new api
- /* Fear of streams */
- /*
- Set<FlowRuleBatchEntry> rules = Sets.newHashSet();
- for (DeviceId deviceId : outputPorts.keys()) {
- rules.add(createBatchEntry(operation,
- intent, deviceId,
- outputPorts.get(deviceId)));
- }
- */
-
- Set<FlowRuleOperation> rules =
- outputPorts
- .keys()
- .stream()
- .map(deviceId -> createBatchEntry(operation,
- intent, deviceId,
- outputPorts.get(deviceId)))
- .collect(Collectors.toSet());
+ List<FlowRuleOperation> rules = Lists.newArrayList();
+ outputPorts.keys().stream()
+ .map(deviceId -> createBatchEntries(operation,
+ intent, deviceId,
+ inputPorts.get(deviceId),
+ outputPorts.get(deviceId)))
+ .forEach(rules::addAll);
return Lists.newArrayList(ImmutableSet.of(rules));
}
@@ -132,35 +126,72 @@
}
/**
- * Creates a FlowRuleBatchEntry based on the provided parameters.
+ * Creates a collection of FlowRuleOperation based on the provided
+ * parameters.
*
- * @param operation the FlowRuleOperation to use
+ * @param operation the FlowRuleOperation type to use
* @param intent the link collection intent
* @param deviceId the device ID for the flow rule
+ * @param inPorts the logical input ports of the flow rule
* @param outPorts the set of output ports for the flow rule
- * @return the new flow rule batch entry
+ * @return a collection with the new flow rule batch entries
*/
- private FlowRuleOperation createBatchEntry(FlowRuleOperation.Type operation,
- LinkCollectionIntent intent,
- DeviceId deviceId,
- Set<PortNumber> outPorts) {
+ private Collection<FlowRuleOperation> createBatchEntries(
+ FlowRuleOperation.Type operation,
+ LinkCollectionIntent intent,
+ DeviceId deviceId,
+ Set<PortNumber> inPorts,
+ Set<PortNumber> outPorts) {
+ Collection<FlowRuleOperation> 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,
new DefaultGroupId((short) (intent.id().fingerprint() & 0xffff)),
0, true);
+ result.add(new FlowRuleOperation(rule, operation));
+ }
- return new FlowRuleOperation(rule, operation);
+ return result;
}
}
diff --git a/core/net/src/main/java/org/onosproject/net/intent/impl/compiler/MultiPointToSinglePointIntentCompiler.java b/core/net/src/main/java/org/onosproject/net/intent/impl/compiler/MultiPointToSinglePointIntentCompiler.java
index c016f41..b4889f4 100644
--- a/core/net/src/main/java/org/onosproject/net/intent/impl/compiler/MultiPointToSinglePointIntentCompiler.java
+++ b/core/net/src/main/java/org/onosproject/net/intent/impl/compiler/MultiPointToSinglePointIntentCompiler.java
@@ -16,6 +16,7 @@
package org.onosproject.net.intent.impl.compiler;
import java.util.Arrays;
+import java.util.Collections;
import java.util.HashMap;
import java.util.List;
import java.util.Map;
@@ -40,6 +41,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;
/**
@@ -76,8 +78,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 different
+ // 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;
}
@@ -87,7 +91,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/compiler/SinglePointToMultiPointIntentCompiler.java b/core/net/src/main/java/org/onosproject/net/intent/impl/compiler/SinglePointToMultiPointIntentCompiler.java
index fd344e2..8360820 100644
--- a/core/net/src/main/java/org/onosproject/net/intent/impl/compiler/SinglePointToMultiPointIntentCompiler.java
+++ b/core/net/src/main/java/org/onosproject/net/intent/impl/compiler/SinglePointToMultiPointIntentCompiler.java
@@ -33,6 +33,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> {
@@ -67,6 +69,7 @@
Intent result = new LinkCollectionIntent(intent.appId(),
intent.selector(),
intent.treatment(), links,
+ ImmutableSet.of(intent.ingressPoint()),
intent.egressPoints(), Collections.emptyList());
return Arrays.asList(result);