Fixed issue where LinkCollectionIntentInstaller was missing a flow rule
for the last hop switch.
Change-Id: I0f3d49de10dc5a6fd7cf65463d0d2e9b6d512346
diff --git a/core/net/src/main/java/org/onlab/onos/net/intent/impl/LinkCollectionIntentInstaller.java b/core/net/src/main/java/org/onlab/onos/net/intent/impl/LinkCollectionIntentInstaller.java
index 51e0d2e..ec668dc 100644
--- a/core/net/src/main/java/org/onlab/onos/net/intent/impl/LinkCollectionIntentInstaller.java
+++ b/core/net/src/main/java/org/onlab/onos/net/intent/impl/LinkCollectionIntentInstaller.java
@@ -1,5 +1,8 @@
package org.onlab.onos.net.intent.impl;
+import static org.onlab.onos.net.flow.DefaultTrafficTreatment.builder;
+import static org.slf4j.LoggerFactory.getLogger;
+
import java.util.List;
import java.util.concurrent.Future;
@@ -10,7 +13,9 @@
import org.apache.felix.scr.annotations.ReferenceCardinality;
import org.onlab.onos.ApplicationId;
import org.onlab.onos.CoreService;
+import org.onlab.onos.net.DeviceId;
import org.onlab.onos.net.Link;
+import org.onlab.onos.net.PortNumber;
import org.onlab.onos.net.flow.CompletedBatchOperation;
import org.onlab.onos.net.flow.DefaultFlowRule;
import org.onlab.onos.net.flow.DefaultTrafficSelector;
@@ -29,9 +34,6 @@
import com.google.common.collect.Lists;
-import static org.onlab.onos.net.flow.DefaultTrafficTreatment.builder;
-import static org.slf4j.LoggerFactory.getLogger;
-
/**
* Installer for {@link org.onlab.onos.net.intent.LinkCollectionIntent}
* path segment intents.
@@ -79,15 +81,17 @@
DefaultTrafficSelector.builder(intent.selector());
List<FlowRuleBatchEntry> rules = Lists.newLinkedList();
for (Link link : intent.links()) {
- TrafficTreatment treatment = builder()
- .setOutput(link.src().port()).build();
-
- FlowRule rule = new DefaultFlowRule(link.src().deviceId(),
- builder.build(), treatment,
- 123, appId, 600);
- rules.add(new FlowRuleBatchEntry(FlowRuleOperation.ADD, rule));
+ rules.add(createBatchEntry(FlowRuleOperation.ADD,
+ builder.build(),
+ link.src().deviceId(),
+ link.src().port()));
}
+ rules.add(createBatchEntry(FlowRuleOperation.ADD,
+ builder.build(),
+ intent.egressPoint().deviceId(),
+ intent.egressPoint().port()));
+
return applyBatch(rules);
}
@@ -98,13 +102,39 @@
List<FlowRuleBatchEntry> rules = Lists.newLinkedList();
for (Link link : intent.links()) {
- TrafficTreatment treatment = builder()
- .setOutput(link.src().port()).build();
- FlowRule rule = new DefaultFlowRule(link.src().deviceId(),
- builder.build(), treatment,
- 123, appId, 600);
- rules.add(new FlowRuleBatchEntry(FlowRuleOperation.REMOVE, rule));
+ rules.add(createBatchEntry(FlowRuleOperation.REMOVE,
+ builder.build(),
+ link.src().deviceId(),
+ link.src().port()));
}
+
+ rules.add(createBatchEntry(FlowRuleOperation.REMOVE,
+ builder.build(),
+ intent.egressPoint().deviceId(),
+ intent.egressPoint().port()));
+
return applyBatch(rules);
}
+
+ /**
+ * Creates a FlowRuleBatchEntry based on the provided parameters.
+ *
+ * @param operation the FlowRuleOperation to use
+ * @param selector the traffic selector
+ * @param deviceId the device ID for the flow rule
+ * @param outPort the output port of the flow rule
+ * @return the new flow rule batch entry
+ */
+ private FlowRuleBatchEntry createBatchEntry(FlowRuleOperation operation,
+ TrafficSelector selector,
+ DeviceId deviceId,
+ PortNumber outPort) {
+
+ TrafficTreatment treatment = builder().setOutput(outPort).build();
+
+ FlowRule rule = new DefaultFlowRule(deviceId,
+ selector, treatment, 123, appId, 600);
+
+ return new FlowRuleBatchEntry(operation, rule);
+ }
}
diff --git a/core/net/src/main/java/org/onlab/onos/net/intent/impl/MultiPointToSinglePointIntentCompiler.java b/core/net/src/main/java/org/onlab/onos/net/intent/impl/MultiPointToSinglePointIntentCompiler.java
index aecd3ba..6ce12d6 100644
--- a/core/net/src/main/java/org/onlab/onos/net/intent/impl/MultiPointToSinglePointIntentCompiler.java
+++ b/core/net/src/main/java/org/onlab/onos/net/intent/impl/MultiPointToSinglePointIntentCompiler.java
@@ -62,7 +62,7 @@
Intent result = new LinkCollectionIntent(intentIdGenerator.getNewId(),
intent.selector(), intent.treatment(),
- links);
+ links, intent.egressPoint());
return Arrays.asList(result);
}
diff --git a/core/net/src/test/java/org/onlab/onos/net/intent/TestLinkCollectionIntent.java b/core/net/src/test/java/org/onlab/onos/net/intent/TestLinkCollectionIntent.java
index f781cf3..a7082b4 100644
--- a/core/net/src/test/java/org/onlab/onos/net/intent/TestLinkCollectionIntent.java
+++ b/core/net/src/test/java/org/onlab/onos/net/intent/TestLinkCollectionIntent.java
@@ -1,20 +1,23 @@
package org.onlab.onos.net.intent;
-import java.util.HashSet;
-import java.util.Set;
-
-import org.junit.Before;
-import org.junit.Test;
-import org.onlab.onos.net.Link;
-import org.onlab.onos.net.flow.TrafficSelector;
-import org.onlab.onos.net.flow.TrafficTreatment;
-
import static org.hamcrest.CoreMatchers.not;
import static org.hamcrest.MatcherAssert.assertThat;
import static org.hamcrest.Matchers.equalTo;
import static org.hamcrest.Matchers.is;
import static org.onlab.onos.net.NetTestTools.link;
+import java.util.HashSet;
+import java.util.Set;
+
+import org.junit.Before;
+import org.junit.Test;
+import org.onlab.onos.net.ConnectPoint;
+import org.onlab.onos.net.DeviceId;
+import org.onlab.onos.net.Link;
+import org.onlab.onos.net.PortNumber;
+import org.onlab.onos.net.flow.TrafficSelector;
+import org.onlab.onos.net.flow.TrafficTreatment;
+
/**
* Unit tests for the LinkCollectionIntent class.
*/
@@ -27,12 +30,18 @@
private Set<Link> links1;
private Set<Link> links2;
+ private ConnectPoint egress1 = new ConnectPoint(DeviceId.deviceId("dev1"),
+ PortNumber.portNumber(3));
+ private ConnectPoint egress2 = new ConnectPoint(DeviceId.deviceId("dev2"),
+ PortNumber.portNumber(3));
+
private TrafficSelector selector = new IntentTestsMocks.MockSelector();
private TrafficTreatment treatment = new IntentTestsMocks.MockTreatment();
- private LinkCollectionIntent makeLinkCollection(long id, Set<Link> links) {
+ private LinkCollectionIntent makeLinkCollection(long id, Set<Link> links,
+ ConnectPoint egress) {
return new LinkCollectionIntent(new IntentId(id),
- selector, treatment, links);
+ selector, treatment, links, egress);
}
@Before
@@ -55,8 +64,8 @@
links2.add(link2);
links2.add(link1);
- LinkCollectionIntent i1 = makeLinkCollection(12, links1);
- LinkCollectionIntent i2 = makeLinkCollection(12, links2);
+ LinkCollectionIntent i1 = makeLinkCollection(12, links1, egress1);
+ LinkCollectionIntent i2 = makeLinkCollection(12, links2, egress1);
assertThat(i1, is(equalTo(i2)));
}
@@ -73,8 +82,28 @@
links2.add(link3);
links2.add(link1);
- LinkCollectionIntent i1 = makeLinkCollection(12, links1);
- LinkCollectionIntent i2 = makeLinkCollection(12, links2);
+ LinkCollectionIntent i1 = makeLinkCollection(12, links1, egress1);
+ LinkCollectionIntent i2 = makeLinkCollection(12, links2, egress1);
+
+ assertThat(i1, is(not(equalTo(i2))));
+ }
+
+ /**
+ * Tests the equals() method where two LinkCollectionIntents have references
+ * to the same Links but different egress points. These should compare not equal.
+ */
+ @Test
+ public void testEgressDifferentEquals() {
+ links1.add(link1);
+ links1.add(link2);
+ links1.add(link3);
+
+ links2.add(link3);
+ links2.add(link2);
+ links2.add(link1);
+
+ LinkCollectionIntent i1 = makeLinkCollection(12, links1, egress1);
+ LinkCollectionIntent i2 = makeLinkCollection(12, links2, egress2);
assertThat(i1, is(not(equalTo(i2))));
}
@@ -91,8 +120,8 @@
links2.add(link2);
links2.add(link1);
- LinkCollectionIntent i1 = makeLinkCollection(1, links1);
- LinkCollectionIntent i2 = makeLinkCollection(2, links2);
+ LinkCollectionIntent i1 = makeLinkCollection(1, links1, egress1);
+ LinkCollectionIntent i2 = makeLinkCollection(2, links2, egress1);
assertThat(i1, is(not(equalTo(i2))));
}
@@ -111,8 +140,8 @@
links2.add(link2);
links2.add(link1);
- LinkCollectionIntent i1 = makeLinkCollection(1, links1);
- LinkCollectionIntent i2 = makeLinkCollection(1, links2);
+ LinkCollectionIntent i1 = makeLinkCollection(1, links1, egress1);
+ LinkCollectionIntent i2 = makeLinkCollection(1, links2, egress1);
assertThat(i1.hashCode(), is(equalTo(i2.hashCode())));
}
@@ -129,8 +158,8 @@
links2.add(link1);
links2.add(link3);
- LinkCollectionIntent i1 = makeLinkCollection(1, links1);
- LinkCollectionIntent i2 = makeLinkCollection(1, links2);
+ LinkCollectionIntent i1 = makeLinkCollection(1, links1, egress1);
+ LinkCollectionIntent i2 = makeLinkCollection(1, links2, egress2);
assertThat(i1.hashCode(), is(not(equalTo(i2.hashCode()))));
}