fix egress packet treatment ONOS-3467

Change-Id: Ia88568d0ed8f1a982479e5212495923d55238d7b

fix egress packet treatment ONOS-3467
Change-Id: Ia88568d0ed8f1a982479e5212495923d55238d7b
Michele Santuari · Gerrit Code Review
Commit 3ea539086a089374ffd66041ef5dad4b89f24b39 3ea53908 1 parent e312fc7c
Showing 2 changed files with 92 additions and 6 deletions
core/net/src/main/java/org/onosproject/net/intent/impl/compiler/PathIntentCompiler.java
core/net/src/test/java/org/onosproject/net/intent/impl/compiler/PathIntentCompilerTest.java
--- a/core/net/src/main/java/org/onosproject/net/intent/impl/compiler/PathIntentCompiler.java
View file @3ea5390
+++ b/core/net/src/main/java/org/onosproject/net/intent/impl/compiler/PathIntentCompiler.java
View file @3ea5390
@@ -38,6 +38,7 @@ import org.onosproject.net.flow.TrafficSelector;
 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 @@ public class PathIntentCompiler implements IntentCompiler<PathIntent> {
                         .matchInPort(prev.port())
                         .matchVlanId(prevVlanId).build();
                 TrafficTreatment.Builder egressTreat = DefaultTrafficTreatment.builder(intent.treatment());
-                if (vlanCriterion.isPresent()) {
+
-                    egressTreat.setVlanId(vlanCriterion.get().vlanId());
+                Optional<L2ModificationInstruction.ModVlanIdInstruction> modVlanIdInstruction = intent.treatment()
-                } else {
+                        .allInstructions().stream().filter(
-                    egressTreat.popVlan();
+                                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;
--- a/core/net/src/test/java/org/onosproject/net/intent/impl/compiler/PathIntentCompilerTest.java
View file @3ea5390
+++ b/core/net/src/test/java/org/onosproject/net/intent/impl/compiler/PathIntentCompilerTest.java
View file @3ea5390
@@ -72,6 +72,13 @@ public class PathIntentCompilerTest {
     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 @@ public class PathIntentCompilerTest {
     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 @@ public class PathIntentCompilerTest {
                 .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 @@ public class PathIntentCompilerTest {
                 .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 @@ public class PathIntentCompilerTest {
     /**
      * 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 @@ public class PathIntentCompilerTest {
         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) {