Fixing FlowRule priority in intent compilers
Change-Id: I13998e88d2a116017e87c019f4829101db6c6b6b
Showing
7 changed files
with
46 additions
and
18 deletions
| ... | @@ -24,7 +24,6 @@ import org.apache.felix.scr.annotations.Reference; | ... | @@ -24,7 +24,6 @@ import org.apache.felix.scr.annotations.Reference; |
| 24 | import org.apache.felix.scr.annotations.ReferenceCardinality; | 24 | import org.apache.felix.scr.annotations.ReferenceCardinality; |
| 25 | import org.onosproject.core.ApplicationId; | 25 | import org.onosproject.core.ApplicationId; |
| 26 | import org.onosproject.core.CoreService; | 26 | import org.onosproject.core.CoreService; |
| 27 | -import org.onosproject.core.DefaultGroupId; | ||
| 28 | import org.onosproject.net.ConnectPoint; | 27 | import org.onosproject.net.ConnectPoint; |
| 29 | import org.onosproject.net.DeviceId; | 28 | import org.onosproject.net.DeviceId; |
| 30 | import org.onosproject.net.EdgeLink; | 29 | import org.onosproject.net.EdgeLink; |
| ... | @@ -134,9 +133,14 @@ public class LinkCollectionIntentCompiler implements IntentCompiler<LinkCollecti | ... | @@ -134,9 +133,14 @@ public class LinkCollectionIntentCompiler implements IntentCompiler<LinkCollecti |
| 134 | treatment = defaultTreatment; | 133 | treatment = defaultTreatment; |
| 135 | } | 134 | } |
| 136 | 135 | ||
| 137 | - DefaultFlowRule rule = new DefaultFlowRule(deviceId, selector, treatment, 123, appId, | 136 | + FlowRule rule = DefaultFlowRule.builder() |
| 138 | - new DefaultGroupId((short) (intent.id().fingerprint() & 0xffff)), 0, true); | 137 | + .forDevice(deviceId) |
| 139 | - | 138 | + .withSelector(selector) |
| 139 | + .withTreatment(treatment) | ||
| 140 | + .withPriority(intent.priority()) | ||
| 141 | + .fromApp(appId) | ||
| 142 | + .makePermanent() | ||
| 143 | + .build(); | ||
| 140 | rules.add(rule); | 144 | rules.add(rule); |
| 141 | } | 145 | } |
| 142 | 146 | ... | ... |
| ... | @@ -188,13 +188,13 @@ public class OpticalCircuitIntentCompiler implements IntentCompiler<OpticalCircu | ... | @@ -188,13 +188,13 @@ public class OpticalCircuitIntentCompiler implements IntentCompiler<OpticalCircu |
| 188 | 188 | ||
| 189 | // Create optical circuit intent | 189 | // Create optical circuit intent |
| 190 | List<FlowRule> rules = new LinkedList<>(); | 190 | List<FlowRule> rules = new LinkedList<>(); |
| 191 | - rules.add(connectPorts(src, connIntent.getSrc())); | 191 | + rules.add(connectPorts(src, connIntent.getSrc(), intent.priority())); |
| 192 | - rules.add(connectPorts(connIntent.getDst(), dst)); | 192 | + rules.add(connectPorts(connIntent.getDst(), dst, intent.priority())); |
| 193 | 193 | ||
| 194 | // Create flow rules for reverse path | 194 | // Create flow rules for reverse path |
| 195 | if (intent.isBidirectional()) { | 195 | if (intent.isBidirectional()) { |
| 196 | - rules.add(connectPorts(connIntent.getSrc(), src)); | 196 | + rules.add(connectPorts(connIntent.getSrc(), src, intent.priority())); |
| 197 | - rules.add(connectPorts(dst, connIntent.getDst())); | 197 | + rules.add(connectPorts(dst, connIntent.getDst(), intent.priority())); |
| 198 | } | 198 | } |
| 199 | 199 | ||
| 200 | circuitIntent = new FlowRuleIntent(appId, rules, intent.resources()); | 200 | circuitIntent = new FlowRuleIntent(appId, rules, intent.resources()); |
| ... | @@ -348,7 +348,7 @@ public class OpticalCircuitIntentCompiler implements IntentCompiler<OpticalCircu | ... | @@ -348,7 +348,7 @@ public class OpticalCircuitIntentCompiler implements IntentCompiler<OpticalCircu |
| 348 | * @param dst destination port | 348 | * @param dst destination port |
| 349 | * @return flow rules | 349 | * @return flow rules |
| 350 | */ | 350 | */ |
| 351 | - private FlowRule connectPorts(ConnectPoint src, ConnectPoint dst) { | 351 | + private FlowRule connectPorts(ConnectPoint src, ConnectPoint dst, int priority) { |
| 352 | checkArgument(src.deviceId().equals(dst.deviceId())); | 352 | checkArgument(src.deviceId().equals(dst.deviceId())); |
| 353 | 353 | ||
| 354 | TrafficSelector.Builder selectorBuilder = DefaultTrafficSelector.builder(); | 354 | TrafficSelector.Builder selectorBuilder = DefaultTrafficSelector.builder(); |
| ... | @@ -363,7 +363,7 @@ public class OpticalCircuitIntentCompiler implements IntentCompiler<OpticalCircu | ... | @@ -363,7 +363,7 @@ public class OpticalCircuitIntentCompiler implements IntentCompiler<OpticalCircu |
| 363 | .forDevice(src.deviceId()) | 363 | .forDevice(src.deviceId()) |
| 364 | .withSelector(selectorBuilder.build()) | 364 | .withSelector(selectorBuilder.build()) |
| 365 | .withTreatment(treatmentBuilder.build()) | 365 | .withTreatment(treatmentBuilder.build()) |
| 366 | - .withPriority(100) | 366 | + .withPriority(priority) |
| 367 | .fromApp(appId) | 367 | .fromApp(appId) |
| 368 | .makePermanent() | 368 | .makePermanent() |
| 369 | .build(); | 369 | .build(); | ... | ... |
| ... | @@ -111,7 +111,7 @@ public class OpticalPathIntentCompiler implements IntentCompiler<OpticalPathInte | ... | @@ -111,7 +111,7 @@ public class OpticalPathIntentCompiler implements IntentCompiler<OpticalPathInte |
| 111 | .forDevice(current.deviceId()) | 111 | .forDevice(current.deviceId()) |
| 112 | .withSelector(selectorBuilder.build()) | 112 | .withSelector(selectorBuilder.build()) |
| 113 | .withTreatment(treatmentBuilder.build()) | 113 | .withTreatment(treatmentBuilder.build()) |
| 114 | - .withPriority(100) | 114 | + .withPriority(intent.priority()) |
| 115 | .fromApp(appId) | 115 | .fromApp(appId) |
| 116 | .makePermanent() | 116 | .makePermanent() |
| 117 | .build(); | 117 | .build(); |
| ... | @@ -132,7 +132,7 @@ public class OpticalPathIntentCompiler implements IntentCompiler<OpticalPathInte | ... | @@ -132,7 +132,7 @@ public class OpticalPathIntentCompiler implements IntentCompiler<OpticalPathInte |
| 132 | .forDevice(intent.dst().deviceId()) | 132 | .forDevice(intent.dst().deviceId()) |
| 133 | .withSelector(selectorBuilder.build()) | 133 | .withSelector(selectorBuilder.build()) |
| 134 | .withTreatment(treatmentLast.build()) | 134 | .withTreatment(treatmentLast.build()) |
| 135 | - .withPriority(100) | 135 | + .withPriority(intent.priority()) |
| 136 | .fromApp(appId) | 136 | .fromApp(appId) |
| 137 | .makePermanent() | 137 | .makePermanent() |
| 138 | .build(); | 138 | .build(); |
| ... | @@ -163,7 +163,7 @@ public class OpticalPathIntentCompiler implements IntentCompiler<OpticalPathInte | ... | @@ -163,7 +163,7 @@ public class OpticalPathIntentCompiler implements IntentCompiler<OpticalPathInte |
| 163 | .forDevice(current.deviceId()) | 163 | .forDevice(current.deviceId()) |
| 164 | .withSelector(selectorBuilder.build()) | 164 | .withSelector(selectorBuilder.build()) |
| 165 | .withTreatment(treatmentBuilder.build()) | 165 | .withTreatment(treatmentBuilder.build()) |
| 166 | - .withPriority(100) | 166 | + .withPriority(intent.priority()) |
| 167 | .fromApp(appId) | 167 | .fromApp(appId) |
| 168 | .makePermanent() | 168 | .makePermanent() |
| 169 | .build(); | 169 | .build(); |
| ... | @@ -184,7 +184,7 @@ public class OpticalPathIntentCompiler implements IntentCompiler<OpticalPathInte | ... | @@ -184,7 +184,7 @@ public class OpticalPathIntentCompiler implements IntentCompiler<OpticalPathInte |
| 184 | .forDevice(intent.src().deviceId()) | 184 | .forDevice(intent.src().deviceId()) |
| 185 | .withSelector(selectorBuilder.build()) | 185 | .withSelector(selectorBuilder.build()) |
| 186 | .withTreatment(treatmentLast.build()) | 186 | .withTreatment(treatmentLast.build()) |
| 187 | - .withPriority(100) | 187 | + .withPriority(intent.priority()) |
| 188 | .fromApp(appId) | 188 | .fromApp(appId) |
| 189 | .makePermanent() | 189 | .makePermanent() |
| 190 | .build(); | 190 | .build(); | ... | ... |
| ... | @@ -76,7 +76,9 @@ public class PathIntentCompiler implements IntentCompiler<PathIntent> { | ... | @@ -76,7 +76,9 @@ public class PathIntentCompiler implements IntentCompiler<PathIntent> { |
| 76 | for (int i = 0; i < links.size() - 1; i++) { | 76 | for (int i = 0; i < links.size() - 1; i++) { |
| 77 | ConnectPoint ingress = links.get(i).dst(); | 77 | ConnectPoint ingress = links.get(i).dst(); |
| 78 | ConnectPoint egress = links.get(i + 1).src(); | 78 | ConnectPoint egress = links.get(i + 1).src(); |
| 79 | - FlowRule rule = createFlowRule(intent.selector(), intent.treatment(), ingress, egress, isLast(links, i)); | 79 | + FlowRule rule = createFlowRule(intent.selector(), intent.treatment(), |
| 80 | + ingress, egress, intent.priority(), | ||
| 81 | + isLast(links, i)); | ||
| 80 | rules.add(rule); | 82 | rules.add(rule); |
| 81 | } | 83 | } |
| 82 | 84 | ||
| ... | @@ -84,7 +86,8 @@ public class PathIntentCompiler implements IntentCompiler<PathIntent> { | ... | @@ -84,7 +86,8 @@ public class PathIntentCompiler implements IntentCompiler<PathIntent> { |
| 84 | } | 86 | } |
| 85 | 87 | ||
| 86 | private FlowRule createFlowRule(TrafficSelector originalSelector, TrafficTreatment originalTreatment, | 88 | private FlowRule createFlowRule(TrafficSelector originalSelector, TrafficTreatment originalTreatment, |
| 87 | - ConnectPoint ingress, ConnectPoint egress, boolean last) { | 89 | + ConnectPoint ingress, ConnectPoint egress, |
| 90 | + int priority, boolean last) { | ||
| 88 | TrafficSelector selector = DefaultTrafficSelector.builder(originalSelector) | 91 | TrafficSelector selector = DefaultTrafficSelector.builder(originalSelector) |
| 89 | .matchInPort(ingress.port()) | 92 | .matchInPort(ingress.port()) |
| 90 | .build(); | 93 | .build(); |
| ... | @@ -97,7 +100,14 @@ public class PathIntentCompiler implements IntentCompiler<PathIntent> { | ... | @@ -97,7 +100,14 @@ public class PathIntentCompiler implements IntentCompiler<PathIntent> { |
| 97 | } | 100 | } |
| 98 | TrafficTreatment treatment = treatmentBuilder.setOutput(egress.port()).build(); | 101 | TrafficTreatment treatment = treatmentBuilder.setOutput(egress.port()).build(); |
| 99 | 102 | ||
| 100 | - return new DefaultFlowRule(ingress.deviceId(), selector, treatment, 123, appId, 0, true); | 103 | + return DefaultFlowRule.builder() |
| 104 | + .forDevice(ingress.deviceId()) | ||
| 105 | + .withSelector(selector) | ||
| 106 | + .withTreatment(treatment) | ||
| 107 | + .withPriority(priority) | ||
| 108 | + .fromApp(appId) | ||
| 109 | + .makePermanent() | ||
| 110 | + .build(); | ||
| 101 | } | 111 | } |
| 102 | 112 | ||
| 103 | private boolean isLast(List<Link> links, int i) { | 113 | private boolean isLast(List<Link> links, int i) { | ... | ... |
| ... | @@ -81,7 +81,7 @@ public class LinkCollectionIntentCompilerTest { | ... | @@ -81,7 +81,7 @@ public class LinkCollectionIntentCompilerTest { |
| 81 | private LinkCollectionIntentCompiler sut; | 81 | private LinkCollectionIntentCompiler sut; |
| 82 | 82 | ||
| 83 | @Before | 83 | @Before |
| 84 | - public void setUP() { | 84 | + public void setUp() { |
| 85 | sut = new LinkCollectionIntentCompiler(); | 85 | sut = new LinkCollectionIntentCompiler(); |
| 86 | coreService = createMock(CoreService.class); | 86 | coreService = createMock(CoreService.class); |
| 87 | expect(coreService.registerApplication("org.onosproject.net.intent")) | 87 | expect(coreService.registerApplication("org.onosproject.net.intent")) |
| ... | @@ -132,6 +132,7 @@ public class LinkCollectionIntentCompilerTest { | ... | @@ -132,6 +132,7 @@ public class LinkCollectionIntentCompilerTest { |
| 132 | assertThat(rule1.treatment(), is( | 132 | assertThat(rule1.treatment(), is( |
| 133 | DefaultTrafficTreatment.builder(intent.treatment()).setOutput(d1p1.port()).build() | 133 | DefaultTrafficTreatment.builder(intent.treatment()).setOutput(d1p1.port()).build() |
| 134 | )); | 134 | )); |
| 135 | + assertThat(rule1.priority(), is(intent.priority())); | ||
| 135 | 136 | ||
| 136 | FlowRule rule2 = rules.stream() | 137 | FlowRule rule2 = rules.stream() |
| 137 | .filter(rule -> rule.deviceId().equals(d2p0.deviceId())) | 138 | .filter(rule -> rule.deviceId().equals(d2p0.deviceId())) |
| ... | @@ -143,6 +144,7 @@ public class LinkCollectionIntentCompilerTest { | ... | @@ -143,6 +144,7 @@ public class LinkCollectionIntentCompilerTest { |
| 143 | assertThat(rule2.treatment(), is( | 144 | assertThat(rule2.treatment(), is( |
| 144 | DefaultTrafficTreatment.builder().setOutput(d2p1.port()).build() | 145 | DefaultTrafficTreatment.builder().setOutput(d2p1.port()).build() |
| 145 | )); | 146 | )); |
| 147 | + assertThat(rule2.priority(), is(intent.priority())); | ||
| 146 | 148 | ||
| 147 | FlowRule rule3 = rules.stream() | 149 | FlowRule rule3 = rules.stream() |
| 148 | .filter(rule -> rule.deviceId().equals(d3p0.deviceId())) | 150 | .filter(rule -> rule.deviceId().equals(d3p0.deviceId())) |
| ... | @@ -154,6 +156,7 @@ public class LinkCollectionIntentCompilerTest { | ... | @@ -154,6 +156,7 @@ public class LinkCollectionIntentCompilerTest { |
| 154 | assertThat(rule3.treatment(), is( | 156 | assertThat(rule3.treatment(), is( |
| 155 | DefaultTrafficTreatment.builder().setOutput(d3p1.port()).build() | 157 | DefaultTrafficTreatment.builder().setOutput(d3p1.port()).build() |
| 156 | )); | 158 | )); |
| 159 | + assertThat(rule3.priority(), is(intent.priority())); | ||
| 157 | 160 | ||
| 158 | sut.deactivate(); | 161 | sut.deactivate(); |
| 159 | } | 162 | } | ... | ... |
| ... | @@ -50,6 +50,7 @@ import static org.easymock.EasyMock.expect; | ... | @@ -50,6 +50,7 @@ import static org.easymock.EasyMock.expect; |
| 50 | import static org.easymock.EasyMock.replay; | 50 | import static org.easymock.EasyMock.replay; |
| 51 | import static org.hamcrest.MatcherAssert.assertThat; | 51 | import static org.hamcrest.MatcherAssert.assertThat; |
| 52 | import static org.hamcrest.Matchers.hasSize; | 52 | import static org.hamcrest.Matchers.hasSize; |
| 53 | +import static org.junit.Assert.assertEquals; | ||
| 53 | import static org.onosproject.net.Link.Type.DIRECT; | 54 | import static org.onosproject.net.Link.Type.DIRECT; |
| 54 | import static org.onosproject.net.NetTestTools.PID; | 55 | import static org.onosproject.net.NetTestTools.PID; |
| 55 | import static org.onosproject.net.NetTestTools.connectPoint; | 56 | import static org.onosproject.net.NetTestTools.connectPoint; |
| ... | @@ -135,7 +136,12 @@ public class OpticalPathIntentCompilerTest { | ... | @@ -135,7 +136,12 @@ public class OpticalPathIntentCompilerTest { |
| 135 | .findFirst() | 136 | .findFirst() |
| 136 | .get(); | 137 | .get(); |
| 137 | 138 | ||
| 139 | + rules.forEach(rule -> assertEquals("FlowRule priority is incorrect", | ||
| 140 | + intent.priority(), rule.priority())); | ||
| 141 | + | ||
| 138 | sut.deactivate(); | 142 | sut.deactivate(); |
| 139 | } | 143 | } |
| 140 | 144 | ||
| 145 | + //TODO test bidirectional optical paths and verify rules | ||
| 146 | + | ||
| 141 | } | 147 | } | ... | ... |
| ... | @@ -75,6 +75,7 @@ public class PathIntentCompilerTest { | ... | @@ -75,6 +75,7 @@ public class PathIntentCompilerTest { |
| 75 | private final ConnectPoint d3p1 = connectPoint("s3", 1); | 75 | private final ConnectPoint d3p1 = connectPoint("s3", 1); |
| 76 | private final ConnectPoint d3p0 = connectPoint("s3", 10); | 76 | private final ConnectPoint d3p0 = connectPoint("s3", 10); |
| 77 | private final ConnectPoint d1p0 = connectPoint("s1", 10); | 77 | private final ConnectPoint d1p0 = connectPoint("s1", 10); |
| 78 | + private static final int PRIORITY = 555; | ||
| 78 | 79 | ||
| 79 | private final List<Link> links = Arrays.asList( | 80 | private final List<Link> links = Arrays.asList( |
| 80 | createEdgeLink(d1p0, true), | 81 | createEdgeLink(d1p0, true), |
| ... | @@ -102,6 +103,7 @@ public class PathIntentCompilerTest { | ... | @@ -102,6 +103,7 @@ public class PathIntentCompilerTest { |
| 102 | .appId(APP_ID) | 103 | .appId(APP_ID) |
| 103 | .selector(selector) | 104 | .selector(selector) |
| 104 | .treatment(treatment) | 105 | .treatment(treatment) |
| 106 | + .priority(PRIORITY) | ||
| 105 | .path(new DefaultPath(pid, links, hops)) | 107 | .path(new DefaultPath(pid, links, hops)) |
| 106 | .build(); | 108 | .build(); |
| 107 | intentExtensionService = createMock(IntentExtensionService.class); | 109 | intentExtensionService = createMock(IntentExtensionService.class); |
| ... | @@ -141,6 +143,7 @@ public class PathIntentCompilerTest { | ... | @@ -141,6 +143,7 @@ public class PathIntentCompilerTest { |
| 141 | is(DefaultTrafficSelector.builder(selector).matchInPort(d1p0.port()).build())); | 143 | is(DefaultTrafficSelector.builder(selector).matchInPort(d1p0.port()).build())); |
| 142 | assertThat(rule1.treatment(), | 144 | assertThat(rule1.treatment(), |
| 143 | is(DefaultTrafficTreatment.builder().setOutput(d1p1.port()).build())); | 145 | is(DefaultTrafficTreatment.builder().setOutput(d1p1.port()).build())); |
| 146 | + assertThat(rule1.priority(), is(intent.priority())); | ||
| 144 | 147 | ||
| 145 | FlowRule rule2 = rules.stream() | 148 | FlowRule rule2 = rules.stream() |
| 146 | .filter(x -> x.deviceId().equals(d2p0.deviceId())) | 149 | .filter(x -> x.deviceId().equals(d2p0.deviceId())) |
| ... | @@ -151,6 +154,7 @@ public class PathIntentCompilerTest { | ... | @@ -151,6 +154,7 @@ public class PathIntentCompilerTest { |
| 151 | is(DefaultTrafficSelector.builder(selector).matchInPort(d2p0.port()).build())); | 154 | is(DefaultTrafficSelector.builder(selector).matchInPort(d2p0.port()).build())); |
| 152 | assertThat(rule2.treatment(), | 155 | assertThat(rule2.treatment(), |
| 153 | is(DefaultTrafficTreatment.builder().setOutput(d2p1.port()).build())); | 156 | is(DefaultTrafficTreatment.builder().setOutput(d2p1.port()).build())); |
| 157 | + assertThat(rule2.priority(), is(intent.priority())); | ||
| 154 | 158 | ||
| 155 | FlowRule rule3 = rules.stream() | 159 | FlowRule rule3 = rules.stream() |
| 156 | .filter(x -> x.deviceId().equals(d3p0.deviceId())) | 160 | .filter(x -> x.deviceId().equals(d3p0.deviceId())) |
| ... | @@ -161,6 +165,7 @@ public class PathIntentCompilerTest { | ... | @@ -161,6 +165,7 @@ public class PathIntentCompilerTest { |
| 161 | is(DefaultTrafficSelector.builder(selector).matchInPort(d3p1.port()).build())); | 165 | is(DefaultTrafficSelector.builder(selector).matchInPort(d3p1.port()).build())); |
| 162 | assertThat(rule3.treatment(), | 166 | assertThat(rule3.treatment(), |
| 163 | is(DefaultTrafficTreatment.builder(treatment).setOutput(d3p0.port()).build())); | 167 | is(DefaultTrafficTreatment.builder(treatment).setOutput(d3p0.port()).build())); |
| 168 | + assertThat(rule3.priority(), is(intent.priority())); | ||
| 164 | 169 | ||
| 165 | sut.deactivate(); | 170 | sut.deactivate(); |
| 166 | } | 171 | } | ... | ... |
-
Please register or login to post a comment