[ONOS-6303] Fix incorrect flow rule from link collection Intent compiler
Change-Id: If39da291c7558cf6a97e742dc0774df0874a9330
diff --git a/core/net/src/main/java/org/onosproject/net/intent/impl/compiler/IntentConfigurableRegistrator.java b/core/net/src/main/java/org/onosproject/net/intent/impl/compiler/IntentConfigurableRegistrator.java
index e14115c..5750486 100644
--- a/core/net/src/main/java/org/onosproject/net/intent/impl/compiler/IntentConfigurableRegistrator.java
+++ b/core/net/src/main/java/org/onosproject/net/intent/impl/compiler/IntentConfigurableRegistrator.java
@@ -70,10 +70,10 @@
private String labelSelection = DEFAULT_LABEL_SELECTION;
private static final boolean DEFAULT_FLOW_OPTIMIZATION = false;
- @Property(name = "useFlowOptimization",
+ @Property(name = "optimizeInstructions",
boolValue = DEFAULT_FLOW_OPTIMIZATION,
label = "Indicates whether or not to optimize the flows in the link collection compiler")
- private boolean useFlowOptimization = DEFAULT_FLOW_OPTIMIZATION;
+ private boolean optimizeInstructions = DEFAULT_FLOW_OPTIMIZATION;
private static final boolean DEFAULT_COPY_TTL = false;
@Property(name = "useCopyTtl",
@@ -115,7 +115,7 @@
if (context == null) {
log.info("Settings: useFlowObjectives={}", useFlowObjectives);
log.info("Settings: labelSelection={}", labelSelection);
- log.info("Settings: useFlowOptimization={}", useFlowOptimization);
+ log.info("Settings: useFlowOptimization={}", optimizeInstructions);
log.info("Settings: useCopyTtl={}", useCopyTtl);
// FIXME: temporary code for switching old compiler to new compiler
@@ -169,15 +169,15 @@
boolean newFlowOptimization;
try {
String s = Tools.get(context.getProperties(), "useFlowOptimization");
- newFlowOptimization = isNullOrEmpty(s) ? useFlowOptimization : Boolean.parseBoolean(s.trim());
+ newFlowOptimization = isNullOrEmpty(s) ? optimizeInstructions : Boolean.parseBoolean(s.trim());
} catch (ClassCastException e) {
- newFlowOptimization = useFlowOptimization;
+ newFlowOptimization = optimizeInstructions;
}
- if (useFlowOptimization != newFlowOptimization) {
- useFlowOptimization = newFlowOptimization;
+ if (optimizeInstructions != newFlowOptimization) {
+ optimizeInstructions = newFlowOptimization;
changeFlowOptimization();
- log.info("Settings: useFlowOptimization={}", useFlowOptimization);
+ log.info("Settings: useFlowOptimization={}", optimizeInstructions);
}
boolean newCopyTtl;
@@ -266,7 +266,7 @@
}
private void changeFlowOptimization() {
- LinkCollectionCompiler.optimize = useFlowOptimization;
+ LinkCollectionCompiler.optimizeInstructions = optimizeInstructions;
}
private void changeCopyTtl() {
diff --git a/core/net/src/main/java/org/onosproject/net/intent/impl/compiler/LinkCollectionCompiler.java b/core/net/src/main/java/org/onosproject/net/intent/impl/compiler/LinkCollectionCompiler.java
index df87844..7d5c4db 100644
--- a/core/net/src/main/java/org/onosproject/net/intent/impl/compiler/LinkCollectionCompiler.java
+++ b/core/net/src/main/java/org/onosproject/net/intent/impl/compiler/LinkCollectionCompiler.java
@@ -77,7 +77,7 @@
/**
* Shared APIs and implementations for Link Collection compilers.
*/
-public class LinkCollectionCompiler<T> {
+public abstract class LinkCollectionCompiler<T> {
/**
* Reference to the label allocator.
@@ -88,7 +88,7 @@
* Influence compiler behavior. If true the compiler
* try to optimize the chain of the actions.
*/
- static boolean optimize;
+ static boolean optimizeInstructions;
/**
* Influence compiler behavior. If true the compiler
@@ -188,6 +188,13 @@
private static final String UNSUPPORTED_INSTRUCTION = "Unknown instruction type";
/**
+ * Influence compiler behavior.
+ *
+ * @return true if we need the compiler optimizeTreatments the chain of the actions.
+ */
+ abstract boolean optimizeTreatments();
+
+ /**
* Creates the flows representations. This default implementation does
* nothing. Subclasses should override this method to create their
* specific flows representations (flow rule, flow objective).
@@ -331,7 +338,7 @@
* The encapsulation modifies the packet. If we are optimizing
* we have to update the state.
*/
- if (optimize) {
+ if (optimizeTreatments()) {
preCondition = encapBuilder;
}
} else {
@@ -345,7 +352,7 @@
* the others.
*/
TrafficSelector prevState = preCondition.build();
- if (optimize) {
+ if (optimizeTreatments()) {
egressPoints = orderedEgressPoints(prevState, egressPoints);
}
/*
@@ -395,7 +402,7 @@
* Finally we set the output action.
*/
treatmentBuilder.setOutput(egressPoint.connectPoint().port());
- if (optimize) {
+ if (optimizeTreatments()) {
/*
* We update the previous state. In this way instead of
* transiting from FIP->FEP we do FEP->FEP and so on.
@@ -558,7 +565,7 @@
* point then the others.
*/
TrafficSelector prevState = filteredIngressPoint.get().trafficSelector();
- if (optimize) {
+ if (optimizeTreatments()) {
egressPoints = orderedEgressPoints(prevState, egressPoints);
}
/*
diff --git a/core/net/src/main/java/org/onosproject/net/intent/impl/compiler/LinkCollectionIntentCompiler.java b/core/net/src/main/java/org/onosproject/net/intent/impl/compiler/LinkCollectionIntentCompiler.java
index 969372d..b828856 100644
--- a/core/net/src/main/java/org/onosproject/net/intent/impl/compiler/LinkCollectionIntentCompiler.java
+++ b/core/net/src/main/java/org/onosproject/net/intent/impl/compiler/LinkCollectionIntentCompiler.java
@@ -127,6 +127,11 @@
}
@Override
+ boolean optimizeTreatments() {
+ return true;
+ }
+
+ @Override
protected List<FlowRule> createRules(LinkCollectionIntent intent,
DeviceId deviceId,
Set<PortNumber> inPorts,
@@ -150,7 +155,7 @@
labels
);
- if (optimize) {
+ if (optimizeInstructions) {
TrafficTreatment compactedTreatment = compactActions(instructions.treatment());
instructions = new ForwardingInstructions(compactedTreatment, instructions.selector());
}
diff --git a/core/net/src/main/java/org/onosproject/net/intent/impl/compiler/LinkCollectionIntentFlowObjectiveCompiler.java b/core/net/src/main/java/org/onosproject/net/intent/impl/compiler/LinkCollectionIntentFlowObjectiveCompiler.java
index 9396ab5..8ea0433 100644
--- a/core/net/src/main/java/org/onosproject/net/intent/impl/compiler/LinkCollectionIntentFlowObjectiveCompiler.java
+++ b/core/net/src/main/java/org/onosproject/net/intent/impl/compiler/LinkCollectionIntentFlowObjectiveCompiler.java
@@ -128,6 +128,11 @@
}
@Override
+ boolean optimizeTreatments() {
+ return false;
+ }
+
+ @Override
protected List<Objective> createRules(LinkCollectionIntent intent,
DeviceId deviceId,
Set<PortNumber> inPorts,
diff --git a/core/net/src/main/java/org/onosproject/net/intent/impl/compiler/LinkCollectionIntentObjectiveCompiler.java b/core/net/src/main/java/org/onosproject/net/intent/impl/compiler/LinkCollectionIntentObjectiveCompiler.java
index 2af0be5..d2bdbbc 100644
--- a/core/net/src/main/java/org/onosproject/net/intent/impl/compiler/LinkCollectionIntentObjectiveCompiler.java
+++ b/core/net/src/main/java/org/onosproject/net/intent/impl/compiler/LinkCollectionIntentObjectiveCompiler.java
@@ -138,6 +138,11 @@
}
@Override
+ boolean optimizeTreatments() {
+ return false;
+ }
+
+ @Override
protected List<Objective> createRules(LinkCollectionIntent intent,
DeviceId deviceId,
Set<PortNumber> inPorts,
diff --git a/core/net/src/test/java/org/onosproject/net/intent/impl/compiler/LinkCollectionEncapIntentCompilerTest.java b/core/net/src/test/java/org/onosproject/net/intent/impl/compiler/LinkCollectionEncapIntentCompilerTest.java
index 9d83e2c..028f8bf 100644
--- a/core/net/src/test/java/org/onosproject/net/intent/impl/compiler/LinkCollectionEncapIntentCompilerTest.java
+++ b/core/net/src/test/java/org/onosproject/net/intent/impl/compiler/LinkCollectionEncapIntentCompilerTest.java
@@ -80,7 +80,7 @@
sut.registrator = registrator;
sut.resourceService = new MockResourceService();
- LinkCollectionCompiler.optimize = false;
+ LinkCollectionCompiler.optimizeInstructions = false;
LinkCollectionCompiler.copyTtl = false;
replay(coreService, intentExtensionService);
@@ -326,7 +326,6 @@
.builder()
.popMpls(IPV4.ethType())
.setOutput(d1p10.port())
- .popMpls(IPV4.ethType())
.setOutput(d1p11.port())
.build()
));
@@ -580,8 +579,6 @@
.pushMpls()
.setMpls(((MplsCriterion) mpls100Selector.getCriterion(MPLS_LABEL)).label())
.setOutput(d1p10.port())
- .popVlan()
- .pushMpls()
.setMpls(((MplsCriterion) mpls200Selector.getCriterion(MPLS_LABEL)).label())
.setOutput(d1p11.port())
.build()
@@ -856,8 +853,6 @@
.stream()
.filter(instruction -> instruction instanceof ModEtherInstruction)
.findFirst().get()).mac())
- .popMpls(IPV4.ethType())
- .pushVlan()
.setVlanId(((VlanIdCriterion) vlan200Selector.getCriterion(VLAN_VID)).vlanId())
.setOutput(d1p11.port())
.build()
@@ -1102,12 +1097,12 @@
assertThat(ruleS1.treatment(), is(
DefaultTrafficTreatment
.builder()
+ .setVlanId(((VlanIdCriterion) vlan100Selector.getCriterion(VLAN_VID)).vlanId())
+ .setOutput(d1p11.port())
.popVlan()
.pushMpls()
.setMpls(((MplsCriterion) mpls100Selector.getCriterion(MPLS_LABEL)).label())
.setOutput(d1p10.port())
- .setVlanId(((VlanIdCriterion) vlan100Selector.getCriterion(VLAN_VID)).vlanId())
- .setOutput(d1p11.port())
.build()
));
@@ -1162,6 +1157,7 @@
.pushVlan()
.setVlanId(VlanId.vlanId(LABEL))
.setOutput(d1p0.port())
+ .popVlan()
.setOutput(d1p11.port())
.build()
));
@@ -1363,6 +1359,8 @@
.pushMpls()
.setMpls(MplsLabel.mplsLabel(LABEL))
.setOutput(d1p0.port())
+ .popMpls(IPV4.ethType())
+ .pushVlan()
.setVlanId(((VlanIdCriterion) vlan200Selector.getCriterion(VLAN_VID)).vlanId())
.setOutput(d1p11.port())
.build()
@@ -1780,8 +1778,6 @@
.stream()
.filter(instruction -> instruction instanceof ModEtherInstruction)
.findFirst().get()).mac())
- .popVlan()
- .pushMpls()
.setMpls(((MplsCriterion) mpls100Selector.getCriterion(MPLS_LABEL)).label())
.setOutput(d1p11.port())
.build()
diff --git a/core/net/src/test/java/org/onosproject/net/intent/impl/compiler/LinkCollectionIntentCompilerP2PTest.java b/core/net/src/test/java/org/onosproject/net/intent/impl/compiler/LinkCollectionIntentCompilerP2PTest.java
index 7df4af3..44754a6 100644
--- a/core/net/src/test/java/org/onosproject/net/intent/impl/compiler/LinkCollectionIntentCompilerP2PTest.java
+++ b/core/net/src/test/java/org/onosproject/net/intent/impl/compiler/LinkCollectionIntentCompilerP2PTest.java
@@ -83,7 +83,7 @@
sut.registrator = registrator;
sut.resourceService = new MockResourceService();
- LinkCollectionCompiler.optimize = false;
+ LinkCollectionCompiler.optimizeInstructions = false;
LinkCollectionCompiler.copyTtl = false;
replay(coreService, intentExtensionService);
diff --git a/core/net/src/test/java/org/onosproject/net/intent/impl/compiler/LinkCollectionIntentCompilerTest.java b/core/net/src/test/java/org/onosproject/net/intent/impl/compiler/LinkCollectionIntentCompilerTest.java
index a905eac..f70c69b 100644
--- a/core/net/src/test/java/org/onosproject/net/intent/impl/compiler/LinkCollectionIntentCompilerTest.java
+++ b/core/net/src/test/java/org/onosproject/net/intent/impl/compiler/LinkCollectionIntentCompilerTest.java
@@ -104,7 +104,7 @@
sut.registrator = registrator;
sut.resourceService = new MockResourceService();
- LinkCollectionCompiler.optimize = false;
+ LinkCollectionCompiler.optimizeInstructions = false;
LinkCollectionCompiler.copyTtl = false;
replay(coreService, intentExtensionService);
diff --git a/core/net/src/test/java/org/onosproject/net/intent/impl/compiler/LinkCollectionIntentObjectiveCompilerTest.java b/core/net/src/test/java/org/onosproject/net/intent/impl/compiler/LinkCollectionIntentObjectiveCompilerTest.java
index 22ecf48..613c3cb 100644
--- a/core/net/src/test/java/org/onosproject/net/intent/impl/compiler/LinkCollectionIntentObjectiveCompilerTest.java
+++ b/core/net/src/test/java/org/onosproject/net/intent/impl/compiler/LinkCollectionIntentObjectiveCompilerTest.java
@@ -96,7 +96,7 @@
compiler.registrator = registrator;
compiler.resourceService = resourceService;
- LinkCollectionCompiler.optimize = false;
+ LinkCollectionCompiler.optimizeInstructions = false;
LinkCollectionCompiler.copyTtl = false;
replay(coreService, intentExtensionService);
diff --git a/core/net/src/test/java/org/onosproject/net/intent/impl/compiler/LinkCollectionOptimizationTest.java b/core/net/src/test/java/org/onosproject/net/intent/impl/compiler/LinkCollectionOptimizationTest.java
index 01b86ba..8038619 100644
--- a/core/net/src/test/java/org/onosproject/net/intent/impl/compiler/LinkCollectionOptimizationTest.java
+++ b/core/net/src/test/java/org/onosproject/net/intent/impl/compiler/LinkCollectionOptimizationTest.java
@@ -83,7 +83,7 @@
/*
* We activate the optimizations.
*/
- LinkCollectionCompiler.optimize = true;
+ LinkCollectionCompiler.optimizeInstructions = true;
LinkCollectionCompiler.copyTtl = true;
replay(coreService, intentExtensionService);