Traffic Treatements now support deferred, immediate, table, and clear instructions.

By default, treatments are all immediate. Treatments will be deferred if the builder
predicated by deferred(). Subsequent treatments will be deferred until immediate is called
on the builder again. Multiple calls to deferred and immediate are permitted.

Change-Id: I76b3a44f2219fc1e72a7fb41b72d7bd602be85b7

@@ -34,6 +34,7 @@ import org.onlab.packet.VlanId;
 import org.onosproject.core.ApplicationId;
 import org.onosproject.core.CoreService;
 import org.onosproject.net.DeviceId;
+import org.onosproject.net.PortNumber;
 import org.onosproject.net.flow.DefaultFlowRule;
 import org.onosproject.net.flow.DefaultTrafficSelector;
 import org.onosproject.net.flow.DefaultTrafficTreatment;
@@ -322,7 +323,7 @@ public class BgpRouter {
         private static final int HIGHEST_PRIORITY = 0xffff;
         private Set<InterfaceIpAddress> intfIps = new HashSet<InterfaceIpAddress>();
         private Set<MacAddress> intfMacs = new HashSet<MacAddress>();
-        private Set<VlanId> intfVlans = new HashSet<VlanId>();
+        private Map<PortNumber, VlanId> portVlanPair = Maps.newHashMap();
 
         public void provision(boolean install, Set<Interface> intfs) {
             getIntefaceConfig(intfs);
@@ -341,7 +342,7 @@ public class BgpRouter {
             for (Interface intf : intfs) {
                 intfIps.addAll(intf.ipAddresses());
                 intfMacs.add(intf.mac());
-                intfVlans.add(intf.vlan());
+                portVlanPair.put(intf.connectPoint().port(), intf.vlan());
             }
         }
 
@@ -447,14 +448,15 @@ public class BgpRouter {
             FlowRule rule;
 
             //Interface Vlans
-            for (VlanId vid : intfVlans) {
-                log.debug("adding rule for VLAN: {}", vid);
+            for (Map.Entry<PortNumber, VlanId> portVlan : portVlanPair.entrySet()) {
+                log.debug("adding rule for VLAN: {}", portVlan);
                 selector = DefaultTrafficSelector.builder();
                 treatment = DefaultTrafficTreatment.builder();
 
-                selector.matchVlanId(vid);
-                treatment.stripVlan();
+                selector.matchVlanId(portVlan.getValue());
+                selector.matchInPort(portVlan.getKey());
                 treatment.transition(Type.ETHER);
+                treatment.deferred().popVlan();
 
                 rule = new DefaultFlowRule(deviceId, selector.build(),
                                            treatment.build(), CONTROLLER_PRIORITY, appId,

@@ -195,7 +195,7 @@ public class FlowsListCommand extends AbstractShellCommand {
                       f.bytes(), f.packets(), f.life(), f.priority(), f.type(),
                       coreService.getAppId(f.appId()).name());
                 print(SFMT, f.selector().criteria());
-                print(TFMT, f.treatment().instructions());
+                print(TFMT, f.treatment());
             }
         }
     }

@@ -17,7 +17,7 @@ package org.onosproject.net.flow;
 
 import com.google.common.base.MoreObjects;
 import com.google.common.collect.ImmutableList;
-
+import com.google.common.collect.Lists;
 import org.onlab.packet.IpAddress;
 import org.onlab.packet.MacAddress;
 import org.onlab.packet.MplsLabel;
@@ -27,7 +27,6 @@ import org.onosproject.net.PortNumber;
 import org.onosproject.net.flow.instructions.Instruction;
 import org.onosproject.net.flow.instructions.Instructions;
 
-import java.util.LinkedList;
 import java.util.List;
 import java.util.Objects;
 
@@ -36,7 +35,11 @@ import java.util.Objects;
  */
 public final class DefaultTrafficTreatment implements TrafficTreatment {
 
-    private final List<Instruction> instructions;
+    private final List<Instruction> immediate;
+    private final List<Instruction> deferred;
+    private final Instructions.TableTypeTransition table;
+
+    private final boolean hasClear;
 
     /**
      * Creates a new traffic treatment from the specified list of instructions.
@@ -44,12 +47,46 @@ public final class DefaultTrafficTreatment implements TrafficTreatment {
      * @param instructions treatment instructions
      */
     private DefaultTrafficTreatment(List<Instruction> instructions) {
-        this.instructions = ImmutableList.copyOf(instructions);
+        this.immediate = ImmutableList.copyOf(instructions);
+        this.deferred = ImmutableList.of();
+        this.hasClear = false;
+        this.table = null;
+    }
+
+    private DefaultTrafficTreatment(List<Instruction> deferred,
+                                   List<Instruction> immediate,
+                                   Instructions.TableTypeTransition table,
+                                   boolean clear) {
+        this.immediate = ImmutableList.copyOf(immediate);
+        this.deferred = ImmutableList.copyOf(deferred);
+        this.table = table;
+        this.hasClear = clear;
+
     }
 
     @Override
     public List<Instruction> instructions() {
-        return instructions;
+        return immediate;
+    }
+
+    @Override
+    public List<Instruction> deferred() {
+        return deferred;
+    }
+
+    @Override
+    public List<Instruction> immediate() {
+        return immediate;
+    }
+
+    @Override
+    public Instructions.TableTypeTransition tableTransition() {
+        return table;
+    }
+
+    @Override
+    public Boolean clearedDeferred() {
+        return hasClear;
     }
 
     /**
@@ -75,7 +112,7 @@ public final class DefaultTrafficTreatment implements TrafficTreatment {
     //FIXME: Order of instructions may affect hashcode
     @Override
     public int hashCode() {
-        return Objects.hash(instructions);
+        return Objects.hash(immediate, deferred, table);
     }
 
     @Override
@@ -85,7 +122,9 @@ public final class DefaultTrafficTreatment implements TrafficTreatment {
         }
         if (obj instanceof DefaultTrafficTreatment) {
             DefaultTrafficTreatment that = (DefaultTrafficTreatment) obj;
-            return Objects.equals(instructions, that.instructions);
+            return Objects.equals(immediate, that.immediate) &&
+                    Objects.equals(deferred, that.deferred) &&
+                    Objects.equals(table, that.table);
 
         }
         return false;
@@ -94,7 +133,10 @@ public final class DefaultTrafficTreatment implements TrafficTreatment {
     @Override
     public String toString() {
         return MoreObjects.toStringHelper(getClass())
-                .add("instructions", instructions)
+                .add("immediate", immediate)
+                .add("deferred", deferred)
+                .add("transition", table == null ? "None" : table.toString())
+                .add("cleared", hasClear)
                 .toString();
     }
 
@@ -106,19 +148,22 @@ public final class DefaultTrafficTreatment implements TrafficTreatment {
 
         boolean drop = false;
 
-        List<Instruction> outputs = new LinkedList<>();
+        boolean clear = false;
+
+        Instructions.TableTypeTransition table;
 
-        // TODO: should be a list of instructions based on group objects
-        List<Instruction> groups = new LinkedList<>();
+        List<Instruction> deferred = Lists.newLinkedList();
 
-        // TODO: should be a list of instructions based on modification objects
-        List<Instruction> modifications = new LinkedList<>();
+        List<Instruction> immediate = Lists.newLinkedList();
+
+        List<Instruction> current = immediate;
 
         // Creates a new builder
         private Builder() {
         }
 
         // Creates a new builder based off an existing treatment
+        //FIXME only works for immediate instruction sets.
         private Builder(TrafficTreatment treatment) {
             for (Instruction instruction : treatment.instructions()) {
                 add(instruction);
@@ -127,30 +172,26 @@ public final class DefaultTrafficTreatment implements TrafficTreatment {
 
         @Override
         public Builder add(Instruction instruction) {
-            if (drop) {
-                return this;
-            }
+
             switch (instruction.type()) {
                 case DROP:
-                    drop = true;
-                    break;
-                case TABLE:
                 case OUTPUT:
-                    outputs.add(instruction);
-                    break;
+                case GROUP:
                 case L0MODIFICATION:
                 case L2MODIFICATION:
                 case L3MODIFICATION:
-                    // TODO: enforce modification order if any
-                    modifications.add(instruction);
+                    current.add(instruction);
                     break;
-                case GROUP:
-                    groups.add(instruction);
+                case TABLE:
+                    table = (Instructions.TableTypeTransition) instruction;
                     break;
                 default:
                     throw new IllegalArgumentException("Unknown instruction type: " +
                                                                instruction.type());
+
+
             }
+
             return this;
         }
 
@@ -254,27 +295,40 @@ public final class DefaultTrafficTreatment implements TrafficTreatment {
         }
 
         @Override
-        public TrafficTreatment.Builder transition(FlowRule.Type type) {
+        public Builder popVlan() {
+            return add(Instructions.popVlan());
+        }
+
+        @Override
+        public Builder transition(FlowRule.Type type) {
             return add(Instructions.transition(type));
         }
 
         @Override
-        public Builder popVlan() {
-            return add(Instructions.popVlan());
+        public Builder immediate() {
+            current = immediate;
+            return this;
         }
 
         @Override
-        public TrafficTreatment build() {
+        public Builder deferred() {
+            current = deferred;
+            return this;
+        }
 
-            //If we are dropping should we just return an empty list?
-            List<Instruction> instructions = new LinkedList<Instruction>();
-            instructions.addAll(modifications);
-            instructions.addAll(groups);
-            if (!drop) {
-                instructions.addAll(outputs);
-            }
+        @Override
+        public Builder wipeDeferred() {
+            clear = true;
+            return this;
+        }
 
-            return new DefaultTrafficTreatment(instructions);
+        @Override
+        public TrafficTreatment build() {
+            if (deferred.size() == 0 && immediate.size() == 0
+                    && table == null && !clear) {
+                drop();
+            }
+            return new DefaultTrafficTreatment(deferred, immediate, table, clear);
         }
 
     }

@@ -25,6 +25,7 @@ import org.onlab.packet.IpAddress;
 import org.onlab.packet.MacAddress;
 import org.onlab.packet.MplsLabel;
 import org.onlab.packet.VlanId;
+import org.onosproject.net.flow.instructions.Instructions;
 
 /**
  * Abstraction of network traffic treatment.
@@ -36,9 +37,37 @@ public interface TrafficTreatment {
      *
      * @return list of treatment instructions
      */
+    @Deprecated
     List<Instruction> instructions();
 
     /**
+     * Returns the list of treatment instructions that will be applied
+     * further down the pipeline.
+     * @return list of treatment instructions
+     */
+    List<Instruction> deferred();
+
+    /**
+     * Returns the list of treatment instructions that will be applied
+     * immediately.
+     * @return list of treatment instructions
+     */
+    List<Instruction> immediate();
+
+    /**
+     * Returns the next table in the pipeline.
+     * @return a table transition; may be null.
+     */
+    Instructions.TableTypeTransition tableTransition();
+
+    /**
+     * Whether the deferred treatment instructions will be cleared
+     * by the device.
+     * @return a boolean
+     */
+    Boolean clearedDeferred();
+
+    /**
      * Builder of traffic treatment entities.
      */
     public interface Builder {
@@ -218,6 +247,25 @@ public interface TrafficTreatment {
         public Builder popVlan();
 
         /**
+         * Any instructions preceded by this method call will be deferred.
+         * @return a treatment builder
+         */
+        public Builder deferred();
+
+        /**
+         * Any instructions preceded by this method call will be immediate.
+         * @return a treatment builder
+         */
+        public Builder immediate();
+
+
+        /**
+         * Instructs the device to clear the deferred instructions set.
+         * @return a treatment builder
+         */
+        public Builder wipeDeferred();
+
+        /**
          * Builds an immutable traffic treatment descriptor.
          *
          * @return traffic treatment

@@ -295,7 +295,7 @@ public final class Instructions {
 
         @Override
         public String toString() {
-            return toStringHelper(type()).toString();
+            return toStringHelper(type().toString()).toString();
 
         }
 

@@ -15,17 +15,16 @@
  */
 package org.onosproject.net.flow;
 
-import java.util.List;
-
+import com.google.common.testing.EqualsTester;
 import org.junit.Test;
-import org.onosproject.net.PortNumber;
-import org.onosproject.net.flow.instructions.Instruction;
-import org.onosproject.net.flow.instructions.Instructions;
 import org.onlab.packet.IpAddress;
 import org.onlab.packet.MacAddress;
 import org.onlab.packet.VlanId;
+import org.onosproject.net.PortNumber;
+import org.onosproject.net.flow.instructions.Instruction;
+import org.onosproject.net.flow.instructions.Instructions;
 
-import com.google.common.testing.EqualsTester;
+import java.util.List;
 
 import static org.hamcrest.CoreMatchers.equalTo;
 import static org.hamcrest.MatcherAssert.assertThat;
@@ -83,7 +82,7 @@ public class DefaultTrafficTreatmentTest {
         builder1.add(instruction1);
 
         final List<Instruction> instructions2 = builder1.build().instructions();
-        assertThat(instructions2, hasSize(8));
+        assertThat(instructions2, hasSize(11));
     }
 
     /**

@@ -45,6 +45,7 @@ import org.onosproject.net.flow.TrafficTreatment;
 import org.onosproject.net.flow.criteria.Criterion;
 import org.onosproject.net.flow.criteria.Criterion.Type;
 import org.onosproject.net.flow.instructions.Instruction;
+import org.onosproject.net.flow.instructions.Instructions;
 import org.onosproject.net.resource.Bandwidth;
 import org.onosproject.net.resource.BandwidthResourceRequest;
 import org.onosproject.net.resource.Lambda;
@@ -96,6 +97,26 @@ public class IntentTestsMocks {
         public List<Instruction> instructions() {
             return new ArrayList<>();
         }
+
+        @Override
+        public List<Instruction> deferred() {
+            return null;
+        }
+
+        @Override
+        public List<Instruction> immediate() {
+            return null;
+        }
+
+        @Override
+        public Instructions.TableTypeTransition tableTransition() {
+            return null;
+        }
+
+        @Override
+        public Boolean clearedDeferred() {
+            return null;
+        }
     }
 
     /**

@@ -63,6 +63,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.instructions.Instruction;
+import org.onosproject.net.flow.instructions.Instructions;
 import org.onosproject.net.provider.AbstractProvider;
 import org.onosproject.net.provider.ProviderId;
 import org.onosproject.store.trivial.impl.SimpleFlowRuleStore;
@@ -562,6 +563,26 @@ public class FlowRuleManagerTest {
         }
 
         @Override
+        public List<Instruction> deferred() {
+            return null;
+        }
+
+        @Override
+        public List<Instruction> immediate() {
+            return null;
+        }
+
+        @Override
+        public Instructions.TableTypeTransition tableTransition() {
+            return null;
+        }
+
+        @Override
+        public Boolean clearedDeferred() {
+            return null;
+        }
+
+        @Override
         public int hashCode() {
             return testval;
         }

@@ -16,13 +16,14 @@
 package org.onosproject.store.statistic.impl;
 
 import com.google.common.collect.Sets;
-
+import org.apache.commons.collections.ListUtils;
 import org.apache.felix.scr.annotations.Activate;
 import org.apache.felix.scr.annotations.Component;
 import org.apache.felix.scr.annotations.Deactivate;
 import org.apache.felix.scr.annotations.Reference;
 import org.apache.felix.scr.annotations.ReferenceCardinality;
 import org.apache.felix.scr.annotations.Service;
+import org.onlab.util.KryoNamespace;
 import org.onosproject.cluster.ClusterService;
 import org.onosproject.net.ConnectPoint;
 import org.onosproject.net.DeviceId;
@@ -39,12 +40,12 @@ import org.onosproject.store.flow.ReplicaInfo;
 import org.onosproject.store.flow.ReplicaInfoService;
 import org.onosproject.store.serializers.KryoNamespaces;
 import org.onosproject.store.serializers.KryoSerializer;
-import org.onlab.util.KryoNamespace;
 import org.slf4j.Logger;
 
 import java.io.IOException;
 import java.util.Collections;
 import java.util.HashSet;
+import java.util.List;
 import java.util.Map;
 import java.util.Set;
 import java.util.concurrent.ConcurrentHashMap;
@@ -305,7 +306,9 @@ public class DistributedStatisticStore implements StatisticStore {
     }
 
     private PortNumber getOutput(FlowRule rule) {
-        for (Instruction i : rule.treatment().instructions()) {
+        List<Instruction> all = ListUtils.union(rule.treatment().immediate(),
+                                                rule.treatment().deferred());
+        for (Instruction i : all) {
             if (i.type() == Instruction.Type.OUTPUT) {
                 Instructions.OutputInstruction out = (Instructions.OutputInstruction) i;
                 return out.port();

@@ -205,7 +205,7 @@ public abstract class FlowModBuilder {
                                        OFVlanVidMatch.PRESENT);
                 } else {
                     mBuilder.setExact(MatchField.VLAN_VID,
-                            OFVlanVidMatch.ofVlanVid(VlanVid.ofVlan(vid.vlanId().toShort())));
+                                      OFVlanVidMatch.ofVlanVid(VlanVid.ofVlan(vid.vlanId().toShort())));
                 }
                 break;
             case VLAN_PCP:

@@ -15,6 +15,7 @@
  */
 package org.onosproject.provider.of.flow.impl;
 
+import com.google.common.collect.Lists;
 import org.onlab.packet.Ip4Address;
 import org.onlab.packet.Ip6Address;
 import org.onosproject.net.PortNumber;
@@ -95,18 +96,22 @@ public class FlowModBuilderVer13 extends FlowModBuilder {
     @Override
     public OFFlowAdd buildFlowAdd() {
         Match match = buildMatch();
-        List<OFAction> actions = buildActions();
-        List<OFInstruction> instructions = buildInstructions();
-
-        // FIXME had to revert back to using apply-actions instead of
-        // write-actions because LINC-OE apparently doesn't support
-        // write-actions. I would prefer to change this back in the future
-        // because apply-actions is an optional instruction in OF 1.3.
-
-        if (actions != null) {
-            OFInstruction applyActions =
-                    factory().instructions().applyActions(actions);
-            instructions.add(applyActions);
+        List<OFAction> deferredActions = buildActions(treatment.deferred());
+        List<OFAction> immediateActions = buildActions(treatment.immediate());
+        List<OFInstruction> instructions = Lists.newLinkedList();
+
+
+        if (immediateActions.size() > 0) {
+            instructions.add(factory().instructions().applyActions(immediateActions));
+        }
+        if (treatment.clearedDeferred()) {
+            instructions.add(factory().instructions().clearActions());
+        }
+        if (deferredActions.size() > 0) {
+            instructions.add(factory().instructions().writeActions(deferredActions));
+        }
+        if (treatment.tableTransition() != null) {
+            instructions.add(buildTableGoto(treatment.tableTransition()));
         }
 
         long cookie = flowRule().id().value();
@@ -128,13 +133,22 @@ public class FlowModBuilderVer13 extends FlowModBuilder {
     @Override
     public OFFlowMod buildFlowMod() {
         Match match = buildMatch();
-        List<OFAction> actions = buildActions();
-        List<OFInstruction> instructions = buildInstructions();
+        List<OFAction> deferredActions = buildActions(treatment.deferred());
+        List<OFAction> immediateActions = buildActions(treatment.immediate());
+        List<OFInstruction> instructions = Lists.newLinkedList();
+
 
-        if (actions != null) {
-            OFInstruction applyActions =
-                    factory().instructions().applyActions(actions);
-            instructions.add(applyActions);
+        if (immediateActions.size() > 0) {
+            instructions.add(factory().instructions().applyActions(immediateActions));
+        }
+        if (treatment.clearedDeferred()) {
+            instructions.add(factory().instructions().clearActions());
+        }
+        if (deferredActions.size() > 0) {
+            instructions.add(factory().instructions().writeActions(deferredActions));
+        }
+        if (treatment.tableTransition() != null) {
+            instructions.add(buildTableGoto(treatment.tableTransition()));
         }
 
         long cookie = flowRule().id().value();
@@ -189,13 +203,13 @@ public class FlowModBuilderVer13 extends FlowModBuilder {
         return instructions;
     }
 
-    private List<OFAction> buildActions() {
+    private List<OFAction> buildActions(List<Instruction> treatments) {
         List<OFAction> actions = new LinkedList<>();
         boolean tableFound = false;
         if (treatment == null) {
             return actions;
         }
-        for (Instruction i : treatment.instructions()) {
+        for (Instruction i : treatments) {
             switch (i.type()) {
                 case DROP:
                     log.warn("Saw drop action; assigning drop action");
@@ -320,7 +334,7 @@ public class FlowModBuilderVer13 extends FlowModBuilder {
                 return factory().actions().popMpls(EthType.of(popHeaderInstructions
                                                               .ethernetType()));
             case STRIP_VLAN:
-                return factory().actions().stripVlan();
+                return factory().actions().popVlan();
             case MPLS_LABEL:
                 ModMplsLabelInstruction mplsLabel =
                         (ModMplsLabelInstruction) l2m;

@@ -380,7 +380,7 @@ public final class IntentJsonMatcher extends TypeSafeDiagnosingMatcher<JsonNode>
 
         for (Instruction instruction : instructions) {
             boolean instructionFound = false;
-            for (int instructionIndex = 0; instructionIndex < jsonCriteria.size(); instructionIndex++) {
+            for (int instructionIndex = 0; instructionIndex < jsonInstructions.size(); instructionIndex++) {
                 final InstructionJsonMatcher instructionMatcher =
                         InstructionJsonMatcher.matchesInstruction(instruction);
                 if (instructionMatcher.matches(jsonInstructions.get(instructionIndex))) {