Committed by
Ray Milkey
Fixed issue where LinkCollectionIntentInstaller was missing a flow rule
for the last hop switch. Change-Id: I0f3d49de10dc5a6fd7cf65463d0d2e9b6d512346
Showing
4 changed files
with
115 additions
and
38 deletions
... | @@ -4,6 +4,7 @@ import java.util.Collection; | ... | @@ -4,6 +4,7 @@ import java.util.Collection; |
4 | import java.util.Objects; | 4 | import java.util.Objects; |
5 | import java.util.Set; | 5 | import java.util.Set; |
6 | 6 | ||
7 | +import org.onlab.onos.net.ConnectPoint; | ||
7 | import org.onlab.onos.net.Link; | 8 | import org.onlab.onos.net.Link; |
8 | import org.onlab.onos.net.flow.TrafficSelector; | 9 | import org.onlab.onos.net.flow.TrafficSelector; |
9 | import org.onlab.onos.net.flow.TrafficTreatment; | 10 | import org.onlab.onos.net.flow.TrafficTreatment; |
... | @@ -18,6 +19,8 @@ public final class LinkCollectionIntent extends ConnectivityIntent implements In | ... | @@ -18,6 +19,8 @@ public final class LinkCollectionIntent extends ConnectivityIntent implements In |
18 | 19 | ||
19 | private final Set<Link> links; | 20 | private final Set<Link> links; |
20 | 21 | ||
22 | + private final ConnectPoint egressPoint; | ||
23 | + | ||
21 | /** | 24 | /** |
22 | * Creates a new point-to-point intent with the supplied ingress/egress | 25 | * Creates a new point-to-point intent with the supplied ingress/egress |
23 | * ports and using the specified explicit path. | 26 | * ports and using the specified explicit path. |
... | @@ -26,19 +29,23 @@ public final class LinkCollectionIntent extends ConnectivityIntent implements In | ... | @@ -26,19 +29,23 @@ public final class LinkCollectionIntent extends ConnectivityIntent implements In |
26 | * @param selector traffic match | 29 | * @param selector traffic match |
27 | * @param treatment action | 30 | * @param treatment action |
28 | * @param links traversed links | 31 | * @param links traversed links |
32 | + * @param egressPoint egress point | ||
29 | * @throws NullPointerException {@code path} is null | 33 | * @throws NullPointerException {@code path} is null |
30 | */ | 34 | */ |
31 | public LinkCollectionIntent(IntentId id, | 35 | public LinkCollectionIntent(IntentId id, |
32 | TrafficSelector selector, | 36 | TrafficSelector selector, |
33 | TrafficTreatment treatment, | 37 | TrafficTreatment treatment, |
34 | - Set<Link> links) { | 38 | + Set<Link> links, |
39 | + ConnectPoint egressPoint) { | ||
35 | super(id, selector, treatment); | 40 | super(id, selector, treatment); |
36 | this.links = links; | 41 | this.links = links; |
42 | + this.egressPoint = egressPoint; | ||
37 | } | 43 | } |
38 | 44 | ||
39 | protected LinkCollectionIntent() { | 45 | protected LinkCollectionIntent() { |
40 | super(); | 46 | super(); |
41 | this.links = null; | 47 | this.links = null; |
48 | + this.egressPoint = null; | ||
42 | } | 49 | } |
43 | 50 | ||
44 | @Override | 51 | @Override |
... | @@ -56,6 +63,15 @@ public final class LinkCollectionIntent extends ConnectivityIntent implements In | ... | @@ -56,6 +63,15 @@ public final class LinkCollectionIntent extends ConnectivityIntent implements In |
56 | return links; | 63 | return links; |
57 | } | 64 | } |
58 | 65 | ||
66 | + /** | ||
67 | + * Returns the egress point of the intent. | ||
68 | + * | ||
69 | + * @return the egress point | ||
70 | + */ | ||
71 | + public ConnectPoint egressPoint() { | ||
72 | + return egressPoint; | ||
73 | + } | ||
74 | + | ||
59 | @Override | 75 | @Override |
60 | public boolean equals(Object o) { | 76 | public boolean equals(Object o) { |
61 | if (this == o) { | 77 | if (this == o) { |
... | @@ -70,12 +86,13 @@ public final class LinkCollectionIntent extends ConnectivityIntent implements In | ... | @@ -70,12 +86,13 @@ public final class LinkCollectionIntent extends ConnectivityIntent implements In |
70 | 86 | ||
71 | LinkCollectionIntent that = (LinkCollectionIntent) o; | 87 | LinkCollectionIntent that = (LinkCollectionIntent) o; |
72 | 88 | ||
73 | - return Objects.equals(this.links, that.links); | 89 | + return Objects.equals(this.links, that.links) && |
90 | + Objects.equals(this.egressPoint, that.egressPoint); | ||
74 | } | 91 | } |
75 | 92 | ||
76 | @Override | 93 | @Override |
77 | public int hashCode() { | 94 | public int hashCode() { |
78 | - return Objects.hash(super.hashCode(), links); | 95 | + return Objects.hash(super.hashCode(), links, egressPoint); |
79 | } | 96 | } |
80 | 97 | ||
81 | @Override | 98 | @Override |
... | @@ -85,6 +102,7 @@ public final class LinkCollectionIntent extends ConnectivityIntent implements In | ... | @@ -85,6 +102,7 @@ public final class LinkCollectionIntent extends ConnectivityIntent implements In |
85 | .add("match", selector()) | 102 | .add("match", selector()) |
86 | .add("action", treatment()) | 103 | .add("action", treatment()) |
87 | .add("links", links()) | 104 | .add("links", links()) |
105 | + .add("egress", egressPoint()) | ||
88 | .toString(); | 106 | .toString(); |
89 | } | 107 | } |
90 | } | 108 | } | ... | ... |
1 | package org.onlab.onos.net.intent.impl; | 1 | package org.onlab.onos.net.intent.impl; |
2 | 2 | ||
3 | +import static org.onlab.onos.net.flow.DefaultTrafficTreatment.builder; | ||
4 | +import static org.slf4j.LoggerFactory.getLogger; | ||
5 | + | ||
3 | import java.util.List; | 6 | import java.util.List; |
4 | import java.util.concurrent.Future; | 7 | import java.util.concurrent.Future; |
5 | 8 | ||
... | @@ -10,7 +13,9 @@ import org.apache.felix.scr.annotations.Reference; | ... | @@ -10,7 +13,9 @@ import org.apache.felix.scr.annotations.Reference; |
10 | import org.apache.felix.scr.annotations.ReferenceCardinality; | 13 | import org.apache.felix.scr.annotations.ReferenceCardinality; |
11 | import org.onlab.onos.ApplicationId; | 14 | import org.onlab.onos.ApplicationId; |
12 | import org.onlab.onos.CoreService; | 15 | import org.onlab.onos.CoreService; |
16 | +import org.onlab.onos.net.DeviceId; | ||
13 | import org.onlab.onos.net.Link; | 17 | import org.onlab.onos.net.Link; |
18 | +import org.onlab.onos.net.PortNumber; | ||
14 | import org.onlab.onos.net.flow.CompletedBatchOperation; | 19 | import org.onlab.onos.net.flow.CompletedBatchOperation; |
15 | import org.onlab.onos.net.flow.DefaultFlowRule; | 20 | import org.onlab.onos.net.flow.DefaultFlowRule; |
16 | import org.onlab.onos.net.flow.DefaultTrafficSelector; | 21 | import org.onlab.onos.net.flow.DefaultTrafficSelector; |
... | @@ -29,9 +34,6 @@ import org.slf4j.Logger; | ... | @@ -29,9 +34,6 @@ import org.slf4j.Logger; |
29 | 34 | ||
30 | import com.google.common.collect.Lists; | 35 | import com.google.common.collect.Lists; |
31 | 36 | ||
32 | -import static org.onlab.onos.net.flow.DefaultTrafficTreatment.builder; | ||
33 | -import static org.slf4j.LoggerFactory.getLogger; | ||
34 | - | ||
35 | /** | 37 | /** |
36 | * Installer for {@link org.onlab.onos.net.intent.LinkCollectionIntent} | 38 | * Installer for {@link org.onlab.onos.net.intent.LinkCollectionIntent} |
37 | * path segment intents. | 39 | * path segment intents. |
... | @@ -79,15 +81,17 @@ public class LinkCollectionIntentInstaller implements IntentInstaller<LinkCollec | ... | @@ -79,15 +81,17 @@ public class LinkCollectionIntentInstaller implements IntentInstaller<LinkCollec |
79 | DefaultTrafficSelector.builder(intent.selector()); | 81 | DefaultTrafficSelector.builder(intent.selector()); |
80 | List<FlowRuleBatchEntry> rules = Lists.newLinkedList(); | 82 | List<FlowRuleBatchEntry> rules = Lists.newLinkedList(); |
81 | for (Link link : intent.links()) { | 83 | for (Link link : intent.links()) { |
82 | - TrafficTreatment treatment = builder() | 84 | + rules.add(createBatchEntry(FlowRuleOperation.ADD, |
83 | - .setOutput(link.src().port()).build(); | 85 | + builder.build(), |
84 | - | 86 | + link.src().deviceId(), |
85 | - FlowRule rule = new DefaultFlowRule(link.src().deviceId(), | 87 | + link.src().port())); |
86 | - builder.build(), treatment, | ||
87 | - 123, appId, 600); | ||
88 | - rules.add(new FlowRuleBatchEntry(FlowRuleOperation.ADD, rule)); | ||
89 | } | 88 | } |
90 | 89 | ||
90 | + rules.add(createBatchEntry(FlowRuleOperation.ADD, | ||
91 | + builder.build(), | ||
92 | + intent.egressPoint().deviceId(), | ||
93 | + intent.egressPoint().port())); | ||
94 | + | ||
91 | return applyBatch(rules); | 95 | return applyBatch(rules); |
92 | } | 96 | } |
93 | 97 | ||
... | @@ -98,13 +102,39 @@ public class LinkCollectionIntentInstaller implements IntentInstaller<LinkCollec | ... | @@ -98,13 +102,39 @@ public class LinkCollectionIntentInstaller implements IntentInstaller<LinkCollec |
98 | List<FlowRuleBatchEntry> rules = Lists.newLinkedList(); | 102 | List<FlowRuleBatchEntry> rules = Lists.newLinkedList(); |
99 | 103 | ||
100 | for (Link link : intent.links()) { | 104 | for (Link link : intent.links()) { |
101 | - TrafficTreatment treatment = builder() | 105 | + rules.add(createBatchEntry(FlowRuleOperation.REMOVE, |
102 | - .setOutput(link.src().port()).build(); | 106 | + builder.build(), |
103 | - FlowRule rule = new DefaultFlowRule(link.src().deviceId(), | 107 | + link.src().deviceId(), |
104 | - builder.build(), treatment, | 108 | + link.src().port())); |
105 | - 123, appId, 600); | ||
106 | - rules.add(new FlowRuleBatchEntry(FlowRuleOperation.REMOVE, rule)); | ||
107 | } | 109 | } |
110 | + | ||
111 | + rules.add(createBatchEntry(FlowRuleOperation.REMOVE, | ||
112 | + builder.build(), | ||
113 | + intent.egressPoint().deviceId(), | ||
114 | + intent.egressPoint().port())); | ||
115 | + | ||
108 | return applyBatch(rules); | 116 | return applyBatch(rules); |
109 | } | 117 | } |
118 | + | ||
119 | + /** | ||
120 | + * Creates a FlowRuleBatchEntry based on the provided parameters. | ||
121 | + * | ||
122 | + * @param operation the FlowRuleOperation to use | ||
123 | + * @param selector the traffic selector | ||
124 | + * @param deviceId the device ID for the flow rule | ||
125 | + * @param outPort the output port of the flow rule | ||
126 | + * @return the new flow rule batch entry | ||
127 | + */ | ||
128 | + private FlowRuleBatchEntry createBatchEntry(FlowRuleOperation operation, | ||
129 | + TrafficSelector selector, | ||
130 | + DeviceId deviceId, | ||
131 | + PortNumber outPort) { | ||
132 | + | ||
133 | + TrafficTreatment treatment = builder().setOutput(outPort).build(); | ||
134 | + | ||
135 | + FlowRule rule = new DefaultFlowRule(deviceId, | ||
136 | + selector, treatment, 123, appId, 600); | ||
137 | + | ||
138 | + return new FlowRuleBatchEntry(operation, rule); | ||
139 | + } | ||
110 | } | 140 | } | ... | ... |
... | @@ -62,7 +62,7 @@ public class MultiPointToSinglePointIntentCompiler | ... | @@ -62,7 +62,7 @@ public class MultiPointToSinglePointIntentCompiler |
62 | 62 | ||
63 | Intent result = new LinkCollectionIntent(intentIdGenerator.getNewId(), | 63 | Intent result = new LinkCollectionIntent(intentIdGenerator.getNewId(), |
64 | intent.selector(), intent.treatment(), | 64 | intent.selector(), intent.treatment(), |
65 | - links); | 65 | + links, intent.egressPoint()); |
66 | return Arrays.asList(result); | 66 | return Arrays.asList(result); |
67 | } | 67 | } |
68 | 68 | ... | ... |
1 | package org.onlab.onos.net.intent; | 1 | package org.onlab.onos.net.intent; |
2 | 2 | ||
3 | +import static org.hamcrest.CoreMatchers.not; | ||
4 | +import static org.hamcrest.MatcherAssert.assertThat; | ||
5 | +import static org.hamcrest.Matchers.equalTo; | ||
6 | +import static org.hamcrest.Matchers.is; | ||
7 | +import static org.onlab.onos.net.NetTestTools.link; | ||
8 | + | ||
3 | import java.util.HashSet; | 9 | import java.util.HashSet; |
4 | import java.util.Set; | 10 | import java.util.Set; |
5 | 11 | ||
6 | import org.junit.Before; | 12 | import org.junit.Before; |
7 | import org.junit.Test; | 13 | import org.junit.Test; |
14 | +import org.onlab.onos.net.ConnectPoint; | ||
15 | +import org.onlab.onos.net.DeviceId; | ||
8 | import org.onlab.onos.net.Link; | 16 | import org.onlab.onos.net.Link; |
17 | +import org.onlab.onos.net.PortNumber; | ||
9 | import org.onlab.onos.net.flow.TrafficSelector; | 18 | import org.onlab.onos.net.flow.TrafficSelector; |
10 | import org.onlab.onos.net.flow.TrafficTreatment; | 19 | import org.onlab.onos.net.flow.TrafficTreatment; |
11 | 20 | ||
12 | -import static org.hamcrest.CoreMatchers.not; | ||
13 | -import static org.hamcrest.MatcherAssert.assertThat; | ||
14 | -import static org.hamcrest.Matchers.equalTo; | ||
15 | -import static org.hamcrest.Matchers.is; | ||
16 | -import static org.onlab.onos.net.NetTestTools.link; | ||
17 | - | ||
18 | /** | 21 | /** |
19 | * Unit tests for the LinkCollectionIntent class. | 22 | * Unit tests for the LinkCollectionIntent class. |
20 | */ | 23 | */ |
... | @@ -27,12 +30,18 @@ public class TestLinkCollectionIntent { | ... | @@ -27,12 +30,18 @@ public class TestLinkCollectionIntent { |
27 | private Set<Link> links1; | 30 | private Set<Link> links1; |
28 | private Set<Link> links2; | 31 | private Set<Link> links2; |
29 | 32 | ||
33 | + private ConnectPoint egress1 = new ConnectPoint(DeviceId.deviceId("dev1"), | ||
34 | + PortNumber.portNumber(3)); | ||
35 | + private ConnectPoint egress2 = new ConnectPoint(DeviceId.deviceId("dev2"), | ||
36 | + PortNumber.portNumber(3)); | ||
37 | + | ||
30 | private TrafficSelector selector = new IntentTestsMocks.MockSelector(); | 38 | private TrafficSelector selector = new IntentTestsMocks.MockSelector(); |
31 | private TrafficTreatment treatment = new IntentTestsMocks.MockTreatment(); | 39 | private TrafficTreatment treatment = new IntentTestsMocks.MockTreatment(); |
32 | 40 | ||
33 | - private LinkCollectionIntent makeLinkCollection(long id, Set<Link> links) { | 41 | + private LinkCollectionIntent makeLinkCollection(long id, Set<Link> links, |
42 | + ConnectPoint egress) { | ||
34 | return new LinkCollectionIntent(new IntentId(id), | 43 | return new LinkCollectionIntent(new IntentId(id), |
35 | - selector, treatment, links); | 44 | + selector, treatment, links, egress); |
36 | } | 45 | } |
37 | 46 | ||
38 | @Before | 47 | @Before |
... | @@ -55,8 +64,8 @@ public class TestLinkCollectionIntent { | ... | @@ -55,8 +64,8 @@ public class TestLinkCollectionIntent { |
55 | links2.add(link2); | 64 | links2.add(link2); |
56 | links2.add(link1); | 65 | links2.add(link1); |
57 | 66 | ||
58 | - LinkCollectionIntent i1 = makeLinkCollection(12, links1); | 67 | + LinkCollectionIntent i1 = makeLinkCollection(12, links1, egress1); |
59 | - LinkCollectionIntent i2 = makeLinkCollection(12, links2); | 68 | + LinkCollectionIntent i2 = makeLinkCollection(12, links2, egress1); |
60 | 69 | ||
61 | assertThat(i1, is(equalTo(i2))); | 70 | assertThat(i1, is(equalTo(i2))); |
62 | } | 71 | } |
... | @@ -73,8 +82,28 @@ public class TestLinkCollectionIntent { | ... | @@ -73,8 +82,28 @@ public class TestLinkCollectionIntent { |
73 | links2.add(link3); | 82 | links2.add(link3); |
74 | links2.add(link1); | 83 | links2.add(link1); |
75 | 84 | ||
76 | - LinkCollectionIntent i1 = makeLinkCollection(12, links1); | 85 | + LinkCollectionIntent i1 = makeLinkCollection(12, links1, egress1); |
77 | - LinkCollectionIntent i2 = makeLinkCollection(12, links2); | 86 | + LinkCollectionIntent i2 = makeLinkCollection(12, links2, egress1); |
87 | + | ||
88 | + assertThat(i1, is(not(equalTo(i2)))); | ||
89 | + } | ||
90 | + | ||
91 | + /** | ||
92 | + * Tests the equals() method where two LinkCollectionIntents have references | ||
93 | + * to the same Links but different egress points. These should compare not equal. | ||
94 | + */ | ||
95 | + @Test | ||
96 | + public void testEgressDifferentEquals() { | ||
97 | + links1.add(link1); | ||
98 | + links1.add(link2); | ||
99 | + links1.add(link3); | ||
100 | + | ||
101 | + links2.add(link3); | ||
102 | + links2.add(link2); | ||
103 | + links2.add(link1); | ||
104 | + | ||
105 | + LinkCollectionIntent i1 = makeLinkCollection(12, links1, egress1); | ||
106 | + LinkCollectionIntent i2 = makeLinkCollection(12, links2, egress2); | ||
78 | 107 | ||
79 | assertThat(i1, is(not(equalTo(i2)))); | 108 | assertThat(i1, is(not(equalTo(i2)))); |
80 | } | 109 | } |
... | @@ -91,8 +120,8 @@ public class TestLinkCollectionIntent { | ... | @@ -91,8 +120,8 @@ public class TestLinkCollectionIntent { |
91 | links2.add(link2); | 120 | links2.add(link2); |
92 | links2.add(link1); | 121 | links2.add(link1); |
93 | 122 | ||
94 | - LinkCollectionIntent i1 = makeLinkCollection(1, links1); | 123 | + LinkCollectionIntent i1 = makeLinkCollection(1, links1, egress1); |
95 | - LinkCollectionIntent i2 = makeLinkCollection(2, links2); | 124 | + LinkCollectionIntent i2 = makeLinkCollection(2, links2, egress1); |
96 | 125 | ||
97 | assertThat(i1, is(not(equalTo(i2)))); | 126 | assertThat(i1, is(not(equalTo(i2)))); |
98 | } | 127 | } |
... | @@ -111,8 +140,8 @@ public class TestLinkCollectionIntent { | ... | @@ -111,8 +140,8 @@ public class TestLinkCollectionIntent { |
111 | links2.add(link2); | 140 | links2.add(link2); |
112 | links2.add(link1); | 141 | links2.add(link1); |
113 | 142 | ||
114 | - LinkCollectionIntent i1 = makeLinkCollection(1, links1); | 143 | + LinkCollectionIntent i1 = makeLinkCollection(1, links1, egress1); |
115 | - LinkCollectionIntent i2 = makeLinkCollection(1, links2); | 144 | + LinkCollectionIntent i2 = makeLinkCollection(1, links2, egress1); |
116 | 145 | ||
117 | assertThat(i1.hashCode(), is(equalTo(i2.hashCode()))); | 146 | assertThat(i1.hashCode(), is(equalTo(i2.hashCode()))); |
118 | } | 147 | } |
... | @@ -129,8 +158,8 @@ public class TestLinkCollectionIntent { | ... | @@ -129,8 +158,8 @@ public class TestLinkCollectionIntent { |
129 | links2.add(link1); | 158 | links2.add(link1); |
130 | links2.add(link3); | 159 | links2.add(link3); |
131 | 160 | ||
132 | - LinkCollectionIntent i1 = makeLinkCollection(1, links1); | 161 | + LinkCollectionIntent i1 = makeLinkCollection(1, links1, egress1); |
133 | - LinkCollectionIntent i2 = makeLinkCollection(1, links2); | 162 | + LinkCollectionIntent i2 = makeLinkCollection(1, links2, egress2); |
134 | 163 | ||
135 | assertThat(i1.hashCode(), is(not(equalTo(i2.hashCode())))); | 164 | assertThat(i1.hashCode(), is(not(equalTo(i2.hashCode())))); |
136 | } | 165 | } | ... | ... |
-
Please register or login to post a comment