fix egress packet treatment ONOS-3467
Change-Id: Ia88568d0ed8f1a982479e5212495923d55238d7b
diff --git a/core/net/src/main/java/org/onosproject/net/intent/impl/compiler/PathIntentCompiler.java b/core/net/src/main/java/org/onosproject/net/intent/impl/compiler/PathIntentCompiler.java
index 569f095..a86138a 100644
--- a/core/net/src/main/java/org/onosproject/net/intent/impl/compiler/PathIntentCompiler.java
+++ b/core/net/src/main/java/org/onosproject/net/intent/impl/compiler/PathIntentCompiler.java
@@ -38,6 +38,7 @@
import org.onosproject.net.flow.TrafficTreatment;
import org.onosproject.net.flow.criteria.Criterion;
import org.onosproject.net.flow.criteria.VlanIdCriterion;
+import org.onosproject.net.flow.instructions.L2ModificationInstruction;
import org.onosproject.net.intent.FlowRuleIntent;
import org.onosproject.net.intent.Intent;
import org.onosproject.net.intent.IntentCompiler;
@@ -221,16 +222,28 @@
.matchInPort(prev.port())
.matchVlanId(prevVlanId).build();
TrafficTreatment.Builder egressTreat = DefaultTrafficTreatment.builder(intent.treatment());
- if (vlanCriterion.isPresent()) {
- egressTreat.setVlanId(vlanCriterion.get().vlanId());
- } else {
- egressTreat.popVlan();
+
+ Optional<L2ModificationInstruction.ModVlanIdInstruction> modVlanIdInstruction = intent.treatment()
+ .allInstructions().stream().filter(
+ instruction -> instruction instanceof L2ModificationInstruction.ModVlanIdInstruction)
+ .map(x -> (L2ModificationInstruction.ModVlanIdInstruction) x).findAny();
+
+ Optional<L2ModificationInstruction.PopVlanInstruction> popVlanInstruction = intent.treatment()
+ .allInstructions().stream().filter(
+ instruction -> instruction instanceof L2ModificationInstruction.PopVlanInstruction)
+ .map(x -> (L2ModificationInstruction.PopVlanInstruction) x).findAny();
+
+ if (!modVlanIdInstruction.isPresent() && !popVlanInstruction.isPresent()) {
+ if (vlanCriterion.isPresent()) {
+ egressTreat.setVlanId(vlanCriterion.get().vlanId());
+ } else {
+ egressTreat.popVlan();
+ }
}
rules.add(createFlowRule(egressSelector,
egressTreat.build(), prev, link.src(), intent.priority(), true));
}
-
}
return rules;
diff --git a/core/net/src/test/java/org/onosproject/net/intent/impl/compiler/PathIntentCompilerTest.java b/core/net/src/test/java/org/onosproject/net/intent/impl/compiler/PathIntentCompilerTest.java
index 4af8ea4..007c132 100644
--- a/core/net/src/test/java/org/onosproject/net/intent/impl/compiler/PathIntentCompilerTest.java
+++ b/core/net/src/test/java/org/onosproject/net/intent/impl/compiler/PathIntentCompilerTest.java
@@ -72,6 +72,13 @@
private final TrafficSelector selector = DefaultTrafficSelector.builder().build();
private final TrafficTreatment treatment = DefaultTrafficTreatment.builder().build();
+ private final VlanId ingressVlan = VlanId.vlanId(((short) 101));
+ private final TrafficSelector vlanSelector = DefaultTrafficSelector.builder()
+ .matchVlanId(ingressVlan).build();
+ private final VlanId egressVlan = VlanId.vlanId((short) 100);
+ private final TrafficTreatment vlanTreatment = DefaultTrafficTreatment.builder()
+ .setVlanId(egressVlan).build();
+
private final ApplicationId appId = new TestApplicationId("test");
private final ProviderId pid = new ProviderId("of", "test");
private final ConnectPoint d1p1 = connectPoint("s1", 0);
@@ -91,6 +98,7 @@
private final int hops = links.size() - 1;
private PathIntent intent;
private PathIntent constraintIntent;
+ private PathIntent constrainIngressEgressVlanIntent;
/**
* Configures objects used in all the test cases.
@@ -113,6 +121,7 @@
.priority(PRIORITY)
.path(new DefaultPath(pid, links, hops))
.build();
+ //Intent with VLAN encap without egress VLAN
constraintIntent = PathIntent.builder()
.appId(APP_ID)
.selector(selector)
@@ -121,6 +130,15 @@
.constraints(ImmutableList.of(new EncapsulationConstraint(EncapsulationType.VLAN)))
.path(new DefaultPath(pid, links, hops))
.build();
+ //Intent with VLAN encap with ingress and egress VLAN
+ constrainIngressEgressVlanIntent = PathIntent.builder()
+ .appId(APP_ID)
+ .selector(vlanSelector)
+ .treatment(vlanTreatment)
+ .priority(PRIORITY)
+ .constraints(ImmutableList.of(new EncapsulationConstraint(EncapsulationType.VLAN)))
+ .path(new DefaultPath(pid, links, hops))
+ .build();
intentExtensionService = createMock(IntentExtensionService.class);
intentExtensionService.registerCompiler(PathIntent.class, sut);
intentExtensionService.unregisterCompiler(PathIntent.class);
@@ -187,7 +205,7 @@
/**
* Tests the compilation behavior of the path intent compiler in case of
- * encasulation costraint {@link EncapsulationConstraint}.
+ * VLAN {@link EncapsulationType} encapsulation constraint {@link EncapsulationConstraint}.
*/
@Test
public void testEncapCompile() {
@@ -229,6 +247,61 @@
sut.deactivate();
}
+ /**
+ * Tests the compilation behavior of the path intent compiler in case of
+ * VLAN {@link EncapsulationType} encapsulation constraint {@link EncapsulationConstraint}.
+ * This test includes a selector to match a VLAN at the ingress and a treatment to set VLAN at the egress.
+ */
+ @Test
+ public void testEncapIngressEgressVlansCompile() {
+ sut.activate();
+
+ List<Intent> compiled = sut.compile(constrainIngressEgressVlanIntent,
+ Collections.emptyList(), Collections.emptySet());
+ assertThat(compiled, hasSize(1));
+
+ Collection<FlowRule> rules = ((FlowRuleIntent) compiled.get(0)).flowRules();
+ assertThat(rules, hasSize(3));
+
+ FlowRule rule1 = rules.stream()
+ .filter(x -> x.deviceId().equals(d1p0.deviceId()))
+ .findFirst()
+ .get();
+ assertThat(rule1.deviceId(), is(d1p0.deviceId()));
+ assertThat(rule1.priority(), is(intent.priority()));
+ verifyEncapSelector(rule1.selector(), d1p0, ingressVlan);
+ VlanId vlanToEncap = verifyEncapTreatment(rule1.treatment(), d1p1, true, false);
+
+ FlowRule rule2 = rules.stream()
+ .filter(x -> x.deviceId().equals(d2p0.deviceId()))
+ .findFirst()
+ .get();
+ assertThat(rule2.deviceId(), is(d2p0.deviceId()));
+ assertThat(rule2.priority(), is(intent.priority()));
+ verifyEncapSelector(rule2.selector(), d2p0, vlanToEncap);
+ verifyEncapTreatment(rule2.treatment(), d2p1, false, false);
+
+ FlowRule rule3 = rules.stream()
+ .filter(x -> x.deviceId().equals(d3p0.deviceId()))
+ .findFirst()
+ .get();
+ assertThat(rule3.deviceId(), is(d3p1.deviceId()));
+ assertThat(rule3.priority(), is(intent.priority()));
+ verifyEncapSelector(rule3.selector(), d3p1, vlanToEncap);
+ Set<L2ModificationInstruction.ModVlanIdInstruction> vlanMod = rule3.treatment().allInstructions().stream()
+ .filter(treat -> treat instanceof L2ModificationInstruction.ModVlanIdInstruction)
+ .map(x -> (L2ModificationInstruction.ModVlanIdInstruction) x)
+ .collect(Collectors.toSet());
+ assertThat(rule3.treatment().allInstructions().stream()
+ .filter(treat -> treat instanceof L2ModificationInstruction.ModVlanIdInstruction)
+ .collect(Collectors.toSet()), hasSize(1));
+ assertThat(vlanMod.iterator().next().vlanId(), is(egressVlan));
+ assertThat(rule3.treatment().allInstructions().stream()
+ .filter(treat -> treat instanceof L2ModificationInstruction.PopVlanInstruction)
+ .collect(Collectors.toSet()), hasSize(0));
+
+ sut.deactivate();
+ }
private VlanId verifyEncapTreatment(TrafficTreatment trafficTreatment,
ConnectPoint egress, boolean isIngress, boolean isEgress) {