Pavlin Radoslavov
Committed by Ray Milkey

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
......@@ -36,16 +36,19 @@ public final class LinkCollectionIntent extends ConnectivityIntent {
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 @@ public final class LinkCollectionIntent extends ConnectivityIntent {
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 @@ public final class LinkCollectionIntent extends ConnectivityIntent {
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 @@ public final class LinkCollectionIntent extends ConnectivityIntent {
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 @@ public final class LinkCollectionIntent extends ConnectivityIntent {
protected LinkCollectionIntent() {
super();
this.links = null;
this.ingressPoints = null;
this.egressPoints = null;
}
......@@ -125,9 +138,18 @@ public final class LinkCollectionIntent extends ConnectivityIntent {
}
/**
* Returns the egress point of the intent.
* Returns the ingress points of the intent.
*
* @return the ingress points
*/
public Set<ConnectPoint> ingressPoints() {
return ingressPoints;
}
/**
* Returns the egress points of the intent.
*
* @return the egress point
* @return the egress points
*/
public Set<ConnectPoint> egressPoints() {
return egressPoints;
......@@ -148,6 +170,7 @@ public final class LinkCollectionIntent extends ConnectivityIntent {
.add("selector", selector())
.add("treatment", treatment())
.add("links", links())
.add("ingress", ingressPoints())
.add("egress", egressPoints())
.toString();
}
......
......@@ -45,6 +45,7 @@ import static org.onosproject.net.NetTestTools.link;
*/
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 @@ public class LinkCollectionIntentTest extends IntentTest {
selector,
treatment,
links1,
ingress,
egress);
final HashSet<Link> links2 = new HashSet<>();
......@@ -79,6 +81,7 @@ public class LinkCollectionIntentTest extends IntentTest {
selector,
treatment,
links2,
ingress,
egress);
new EqualsTester()
......@@ -99,6 +102,7 @@ public class LinkCollectionIntentTest extends IntentTest {
selector,
treatment,
links1,
ingress,
egress);
final Set<Link> createdLinks = collectionIntent.links();
......@@ -106,6 +110,7 @@ public class LinkCollectionIntentTest extends IntentTest {
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 @@ public class LinkCollectionIntentTest extends IntentTest {
selector,
treatment,
links1,
ingress,
egress,
constraints);
......@@ -135,6 +141,7 @@ public class LinkCollectionIntentTest extends IntentTest {
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 @@ public class LinkCollectionIntentTest extends IntentTest {
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 @@ public class LinkCollectionIntentTest extends IntentTest {
selector,
treatment,
links1,
ingress,
egress);
}
......@@ -181,6 +190,7 @@ public class LinkCollectionIntentTest extends IntentTest {
selector,
treatment,
links2,
ingress,
egress);
}
}
......
......@@ -43,9 +43,9 @@ import org.onosproject.net.intent.IntentInstaller;
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 @@ public class LinkCollectionIntentInstaller
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 egressPoint : intent.egressPoints()) {
outputPorts.put(egressPoint.deviceId(), egressPoint.port());
for (ConnectPoint ingressPoint : intent.ingressPoints()) {
inputPorts.put(ingressPoint.deviceId(), ingressPoint.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)));
for (ConnectPoint egressPoint : intent.egressPoints()) {
outputPorts.put(egressPoint.deviceId(), egressPoint.port());
}
*/
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 @@ public class LinkCollectionIntentInstaller
}
/**
* 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) {
TrafficTreatment.Builder treatmentBuilder = DefaultTrafficTreatment
.builder(intent.treatment());
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>();
//
// 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());
}
}
//
// 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) {
treatmentBuilder.setOutput(outPort);
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();
}
TrafficTreatment treatment = treatmentBuilder.build();
TrafficSelector selector = DefaultTrafficSelector
.builder(intent.selector()).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;
}
}
......
......@@ -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.intent.impl.PathNotFoundException;
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 @@ public class MultiPointToSinglePointIntentCompiler
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 @@ public class MultiPointToSinglePointIntentCompiler
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);
}
......
......@@ -33,6 +33,8 @@ import org.onosproject.net.intent.SinglePointToMultiPointIntent;
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 @@ public class SinglePointToMultiPointIntentCompiler
Intent result = new LinkCollectionIntent(intent.appId(),
intent.selector(),
intent.treatment(), links,
ImmutableSet.of(intent.ingressPoint()),
intent.egressPoints(), Collections.emptyList());
return Arrays.asList(result);
......