Improve fabric.p4 to reduce pipeline resources and refactor pipeconf impl

This patch affects both the P4 pipeline implementation and the
Java pipeconf.

P4 PIPELINE
- Less tables and smarter use of metadata to reduce inter-tables
dependencies and favor parallel execution of tables.
- Removed unused actions / renamed existing ones to make forwarding
behavior clearer (e.g. ingress_port_vlan table)
- Remove co-existence of simple and hansed table. Hashed should be the
default one, but implementations that do not support action profiles
might compile fabric.p4 to use the simple one.
- Use @name annotations for match fields to make control plane
independent of table implementation.
- Use @hidden to avoid showing actions and table on the p4info that
cannot be controlled at runtime.
- First attempt to support double VLAN cross-connect (xconnect table).
- New design has been tested with "fabric-refactoring" branch of
fabric-p4test:
github.com/opennetworkinglab/fabric-p4test/tree/fabric-refactoring

JAVA PIPECONF
This patch brings a major refactoring that reflects the experience
gathered in the past months of working on fabric.p4 and reasoning on its
pipeconf implementation. Indeed, the FlowObjective API is
under-specified and sometimes ambiguous which makes the process of
creating and maintaining a pipeliner implementation tedious. This
refactoring brings a simplified implementation by removing unused/
unnecessary functionalities and by recognizing commonality when possible
(e.g. by means of abstract and utility classes). It also makes design
patterns more explicit and consistent. Overall, the goal is to reduce
technical debt and to make it easier to support new features as we
evolve fabric.p4

Changes include:
- Changes in pipeliner/interpreter to reflect new pipeline design.
- By default translate objective treatment to PiAction. This favors
debuggability of flow rules in ONOS.
- Support new NextObjective’s NextTreatment class.
- Remove lots of unused/unnecessary code (e.g. async callback handling
for pending objective install status in pipeliner as current
implementation was always returning success)
- Gather commonality in abstract classes and simplify implementation
for objective translator (filtering, forwarding, next)
- New implementation of ForwardingFunctionTypes (FFT) that looks at
criterion instance values along with their types (to avoid relying on
case-specific if-else conditions to recognize variants of an FFT)
- Adaptive translation of NextObjective based on presence of simple or
hashed table.
- Support DENY FilteringObjective

Also:
- Fix onos-p4-gen-constants to avoid generating conflicting
PiMatchFieldId variable names.
- Install Graphviz tools in p4vm to generate p4c graphs
- Generate p4c graphs by default when compiling fabric.p4
- Use more compact Hex string when printing PI values

Change-Id: Ife79e44054dc5bc48833f95d0551a7370150eac5
diff --git a/pipelines/fabric/src/main/java/org/onosproject/pipelines/fabric/FabricTreatmentInterpreter.java b/pipelines/fabric/src/main/java/org/onosproject/pipelines/fabric/FabricTreatmentInterpreter.java
index 818fb43..4248111 100644
--- a/pipelines/fabric/src/main/java/org/onosproject/pipelines/fabric/FabricTreatmentInterpreter.java
+++ b/pipelines/fabric/src/main/java/org/onosproject/pipelines/fabric/FabricTreatmentInterpreter.java
@@ -16,371 +16,241 @@
 
 package org.onosproject.pipelines.fabric;
 
-import com.google.common.collect.ImmutableList;
-import org.onlab.packet.MacAddress;
-import org.onlab.packet.MplsLabel;
-import org.onlab.packet.VlanId;
-import org.onlab.util.ImmutableByteSequence;
+import com.google.common.collect.ImmutableMap;
 import org.onosproject.net.PortNumber;
 import org.onosproject.net.flow.DefaultTrafficTreatment;
 import org.onosproject.net.flow.TrafficTreatment;
 import org.onosproject.net.flow.instructions.Instruction;
-import org.onosproject.net.flow.instructions.Instructions;
 import org.onosproject.net.flow.instructions.Instructions.OutputInstruction;
 import org.onosproject.net.flow.instructions.L2ModificationInstruction;
 import org.onosproject.net.flow.instructions.L2ModificationInstruction.ModEtherInstruction;
 import org.onosproject.net.flow.instructions.L2ModificationInstruction.ModMplsLabelInstruction;
 import org.onosproject.net.flow.instructions.L2ModificationInstruction.ModVlanIdInstruction;
-import org.onosproject.net.flow.instructions.L3ModificationInstruction;
 import org.onosproject.net.pi.model.PiActionId;
 import org.onosproject.net.pi.model.PiPipelineInterpreter.PiInterpreterException;
 import org.onosproject.net.pi.model.PiTableId;
 import org.onosproject.net.pi.runtime.PiAction;
 import org.onosproject.net.pi.runtime.PiActionParam;
-import org.slf4j.Logger;
-
-import java.util.List;
 
 import static java.lang.String.format;
-import static org.onosproject.net.flow.instructions.Instruction.Type.L2MODIFICATION;
+import static org.onosproject.net.flow.instructions.Instruction.Type.OUTPUT;
+import static org.onosproject.net.flow.instructions.L2ModificationInstruction.L2SubType.ETH_DST;
+import static org.onosproject.net.flow.instructions.L2ModificationInstruction.L2SubType.ETH_SRC;
+import static org.onosproject.net.flow.instructions.L2ModificationInstruction.L2SubType.MPLS_LABEL;
+import static org.onosproject.net.flow.instructions.L2ModificationInstruction.L2SubType.MPLS_PUSH;
 import static org.onosproject.net.flow.instructions.L2ModificationInstruction.L2SubType.VLAN_ID;
-import static org.onosproject.net.flow.instructions.L2ModificationInstruction.L2SubType.VLAN_PUSH;
-import static org.onosproject.pipelines.fabric.FabricUtils.getOutputPort;
-import static org.slf4j.LoggerFactory.getLogger;
+import static org.onosproject.net.flow.instructions.L2ModificationInstruction.L2SubType.VLAN_POP;
+import static org.onosproject.pipelines.fabric.FabricUtils.instruction;
+import static org.onosproject.pipelines.fabric.FabricUtils.l2Instruction;
+import static org.onosproject.pipelines.fabric.FabricUtils.outputPort;
 
-
+/**
+ * Treatment translation logic.
+ */
 final class FabricTreatmentInterpreter {
-    private static final Logger log = getLogger(FabricTreatmentInterpreter.class);
-    private static final String INVALID_TREATMENT = "Invalid treatment for %s block [%s]";
-    private static final String INVALID_TREATMENT_WITH_EXP = "Invalid treatment for %s block: %s [%s]";
-    private static final PiAction NOP = PiAction.builder().withId(FabricConstants.NOP).build();
-    private static final PiAction NOP_INGRESS_PORT_VLAN = PiAction.builder()
-            .withId(FabricConstants.FABRIC_INGRESS_FILTERING_NOP_INGRESS_PORT_VLAN).build();
-    private static final PiAction NOP_ACL = PiAction.builder()
-            .withId(FabricConstants.FABRIC_INGRESS_FORWARDING_NOP_ACL).build();
-    private static final PiAction NOP_ROUTING_V4 = PiAction.builder()
-            .withId(FabricConstants.FABRIC_INGRESS_FORWARDING_NOP_ROUTING_V4).build();
 
-    private static final PiAction POP_VLAN = PiAction.builder()
-            .withId(FabricConstants.FABRIC_EGRESS_EGRESS_NEXT_POP_VLAN)
-            .build();
+    private static final ImmutableMap<PiTableId, PiActionId> NOP_ACTIONS =
+            ImmutableMap.<PiTableId, PiActionId>builder()
+                    .put(FabricConstants.FABRIC_INGRESS_FILTERING_INGRESS_PORT_VLAN,
+                         FabricConstants.FABRIC_INGRESS_FILTERING_PERMIT)
+                    .put(FabricConstants.FABRIC_INGRESS_FORWARDING_ROUTING_V4,
+                         FabricConstants.FABRIC_INGRESS_FORWARDING_NOP_ROUTING_V4)
+                    .put(FabricConstants.FABRIC_INGRESS_ACL_ACL,
+                         FabricConstants.FABRIC_INGRESS_ACL_NOP_ACL)
+                    .put(FabricConstants.FABRIC_EGRESS_EGRESS_NEXT_EGRESS_VLAN,
+                         FabricConstants.FABRIC_EGRESS_EGRESS_NEXT_POP_VLAN)
+                    .build();
 
-    // Hide default constructor
-    protected FabricTreatmentInterpreter() {
+    private FabricTreatmentInterpreter() {
+        // Hide default constructor
     }
 
-    /*
-     * In Filtering block, we need to implement these actions:
-     * push_internal_vlan
-     * set_vlan
-     * nop
-     *
-     * Unsupported, using PiAction directly:
-     * set_forwarding_type
-     * drop
-     */
-
     static PiAction mapFilteringTreatment(TrafficTreatment treatment, PiTableId tableId)
             throws PiInterpreterException {
-        List<Instruction> instructions = treatment.allInstructions();
-        Instruction noActInst = Instructions.createNoAction();
-        if (instructions.isEmpty() || instructions.contains(noActInst)) {
-            // nop
-            return NOP_INGRESS_PORT_VLAN;
+
+        if (!tableId.equals(FabricConstants.FABRIC_INGRESS_FILTERING_INGRESS_PORT_VLAN)) {
+            // Mapping for other tables of the filtering block must be handled
+            // in the pipeliner.
+            tableException(tableId);
         }
 
-        L2ModificationInstruction.ModVlanHeaderInstruction pushVlanInst = null;
-        ModVlanIdInstruction setVlanInst = null;
-
-        for (Instruction inst : instructions) {
-            if (inst.type() == L2MODIFICATION) {
-                L2ModificationInstruction l2Inst = (L2ModificationInstruction) inst;
-
-                if (l2Inst.subtype() == VLAN_PUSH) {
-                    pushVlanInst = (L2ModificationInstruction.ModVlanHeaderInstruction) l2Inst;
-
-                } else if (l2Inst.subtype() == VLAN_ID) {
-                    setVlanInst = (ModVlanIdInstruction) l2Inst;
-                }
-            }
+        if (isNoAction(treatment)) {
+            // Permit action if table is ingress_port_vlan;
+            return nop(tableId);
         }
 
-        if (setVlanInst == null) {
-            throw new PiInterpreterException(format(INVALID_TREATMENT, "filtering", treatment));
-        }
-
-        VlanId vlanId = setVlanInst.vlanId();
-        PiActionParam param = new PiActionParam(FabricConstants.NEW_VLAN_ID,
-                                                ImmutableByteSequence.copyFrom(vlanId.toShort()));
-        PiActionId actionId;
-        if (pushVlanInst != null) {
-            // push_internal_vlan
-            actionId = FabricConstants.FABRIC_INGRESS_FILTERING_PUSH_INTERNAL_VLAN;
-        } else {
-            actionId = FabricConstants.FABRIC_INGRESS_FILTERING_SET_VLAN;
-        }
-
-        // set_vlan
+        final ModVlanIdInstruction setVlanInst = (ModVlanIdInstruction) l2InstructionOrFail(
+                treatment, VLAN_ID, tableId);
         return PiAction.builder()
-                .withId(actionId)
-                .withParameter(param)
+                .withId(FabricConstants.FABRIC_INGRESS_FILTERING_PERMIT_WITH_INTERNAL_VLAN)
+                .withParameter(new PiActionParam(
+                        FabricConstants.VLAN_ID, setVlanInst.vlanId().toShort()))
                 .build();
     }
 
-    /*
-     * In forwarding block, we need to implement these actions:
-     * send_to_controller
-     *
-     * Unsupported, using PiAction directly:
-     * set_next_id_bridging
-     * pop_mpls_and_next
-     * set_next_id_unicast_v4
-     * set_next_id_multicast_v4
-     * set_next_id_acl
-     * drop
-     * set_next_id_unicast_v6
-     * set_next_id_multicast_v6
-     */
 
-    public static PiAction mapForwardingTreatment(TrafficTreatment treatment, PiTableId tableId)
+    static PiAction mapForwardingTreatment(TrafficTreatment treatment, PiTableId tableId)
             throws PiInterpreterException {
-        // Empty treatment, generate table entry with no action
-        if (treatment.equals(DefaultTrafficTreatment.emptyTreatment()) ||
-                treatment.allInstructions().isEmpty()) {
-            if (tableId.equals(FabricConstants.FABRIC_INGRESS_FORWARDING_ACL)) {
-                return NOP_ACL;
-            } else if (tableId.equals(FabricConstants.FABRIC_INGRESS_FORWARDING_ROUTING_V4)) {
-                return NOP_ROUTING_V4;
-            } else {
-                return NOP;
-            }
+        if (isNoAction(treatment)) {
+            return nop(tableId);
         }
-        PortNumber outPort = getOutputPort(treatment);
+        treatmentException(
+                tableId, treatment,
+                "supports mapping only for empty/no-action treatments");
+        return null;
+    }
+
+    static PiAction mapNextTreatment(TrafficTreatment treatment, PiTableId tableId)
+            throws PiInterpreterException {
+        if (tableId == FabricConstants.FABRIC_INGRESS_NEXT_NEXT_VLAN) {
+            return mapNextVlanTreatment(treatment, tableId);
+        } else if (tableId == FabricConstants.FABRIC_INGRESS_NEXT_HASHED) {
+            return mapNextHashedOrSimpleTreatment(treatment, tableId, false);
+        } else if (tableId == FabricConstants.FABRIC_INGRESS_NEXT_SIMPLE) {
+            return mapNextHashedOrSimpleTreatment(treatment, tableId, true);
+        }
+        throw new PiInterpreterException(format(
+                "Treatment mapping not supported for table '%s'", tableId));
+    }
+
+    private static PiAction mapNextVlanTreatment(TrafficTreatment treatment, PiTableId tableId)
+            throws PiInterpreterException {
+        final ModVlanIdInstruction modVlanIdInst = (ModVlanIdInstruction)
+                l2InstructionOrFail(treatment, VLAN_ID, tableId);
+        return PiAction.builder()
+                .withId(FabricConstants.FABRIC_INGRESS_NEXT_SET_VLAN)
+                .withParameter(new PiActionParam(
+                        FabricConstants.VLAN_ID,
+                        modVlanIdInst.vlanId().toShort()))
+                .build();
+    }
+
+    private static PiAction mapNextHashedOrSimpleTreatment(
+            TrafficTreatment treatment, PiTableId tableId, boolean simple)
+            throws PiInterpreterException {
+        // Provide mapping for output_hashed, routing_hashed, and
+        // mpls_routing_hashed. multicast_hashed can only be invoked with
+        // PiAction, hence no mapping. outPort required for all actions. Presence
+        // of other instructions will determine which action to map to.
+        final PortNumber outPort = ((OutputInstruction) instructionOrFail(
+                treatment, OUTPUT, tableId)).port();
+        final ModEtherInstruction ethDst = (ModEtherInstruction) l2Instruction(
+                treatment, ETH_DST);
+        final ModEtherInstruction ethSrc = (ModEtherInstruction) l2Instruction(
+                treatment, ETH_SRC);
+        final Instruction mplsPush = l2Instruction(
+                treatment, MPLS_PUSH);
+        final ModMplsLabelInstruction mplsLabel = (ModMplsLabelInstruction) l2Instruction(
+                treatment, MPLS_LABEL);
+
+        final PiAction.Builder actionBuilder = PiAction.builder()
+                .withParameter(new PiActionParam(FabricConstants.PORT_NUM, outPort.toLong()));
+
+        if (ethDst != null && ethSrc != null) {
+            actionBuilder.withParameter(new PiActionParam(
+                    FabricConstants.SMAC, ethSrc.mac().toBytes()));
+            actionBuilder.withParameter(new PiActionParam(
+                    FabricConstants.DMAC, ethDst.mac().toBytes()));
+            if (mplsLabel != null) {
+                // mpls_routing_hashed
+                return actionBuilder
+                        .withParameter(new PiActionParam(FabricConstants.LABEL, mplsLabel.label().toInt()))
+                        .withId(simple ? FabricConstants.FABRIC_INGRESS_NEXT_MPLS_ROUTING_SIMPLE
+                                        : FabricConstants.FABRIC_INGRESS_NEXT_MPLS_ROUTING_HASHED)
+                        .build();
+            } else {
+                // routing_hashed
+                return actionBuilder
+                        .withId(simple ? FabricConstants.FABRIC_INGRESS_NEXT_ROUTING_SIMPLE
+                                        : FabricConstants.FABRIC_INGRESS_NEXT_ROUTING_HASHED)
+                        .build();
+            }
+        } else {
+            // output_hashed
+            return actionBuilder
+                    .withId(simple ? FabricConstants.FABRIC_INGRESS_NEXT_OUTPUT_SIMPLE
+                                    : FabricConstants.FABRIC_INGRESS_NEXT_OUTPUT_HASHED)
+                    .build();
+        }
+    }
+
+    static PiAction mapAclTreatment(TrafficTreatment treatment, PiTableId tableId)
+            throws PiInterpreterException {
+        if (isNoAction(treatment)) {
+            return nop(tableId);
+        }
+
+        final PortNumber outPort = outputPort(treatment);
         if (outPort == null
                 || !outPort.equals(PortNumber.CONTROLLER)
                 || treatment.allInstructions().size() > 1) {
-            throw new PiInterpreterException(
-                    format(INVALID_TREATMENT_WITH_EXP,
-                           "forwarding", "supports only punt/clone to CPU actions",
-                           treatment));
+            treatmentException(
+                    tableId, treatment,
+                    "supports only punt/clone to CPU actions");
         }
 
         final PiActionId actionId = treatment.clearedDeferred()
-                ? FabricConstants.FABRIC_INGRESS_FORWARDING_PUNT_TO_CPU
-                : FabricConstants.FABRIC_INGRESS_FORWARDING_CLONE_TO_CPU;
+                ? FabricConstants.FABRIC_INGRESS_ACL_PUNT_TO_CPU
+                : FabricConstants.FABRIC_INGRESS_ACL_CLONE_TO_CPU;
 
         return PiAction.builder()
                 .withId(actionId)
                 .build();
     }
 
-    /*
-     * In Next block, we need to implement these actions:
-     * set_vlan
-     * set_vlan_output
-     * output_simple
-     * output_hashed
-     * l3_routing_simple
-     * l3_routing_vlan
-     * l3_routing_hashed
-     * mpls_routing_v4_simple
-     * mpls_routing_v6_simple
-     * mpls_routing_v4_hashed
-     * mpls_routing_v6_hashed
-     *
-     * Unsupported, need to find a way to implement it
-     *
-     * set_mcast_group
-     *
-     */
 
-    public static PiAction mapNextTreatment(TrafficTreatment treatment, PiTableId tableId)
+    static PiAction mapEgressNextTreatment(
+            TrafficTreatment treatment, PiTableId tableId)
             throws PiInterpreterException {
-        // TODO: refactor this method
-        List<Instruction> insts = treatment.allInstructions();
-        OutputInstruction outInst = null;
-        ModEtherInstruction modEthDstInst = null;
-        ModEtherInstruction modEthSrcInst = null;
-        ModVlanIdInstruction modVlanIdInst = null;
-        ModMplsLabelInstruction modMplsInst = null;
+        l2InstructionOrFail(treatment, VLAN_POP, tableId);
+        return PiAction.builder()
+                .withId(FabricConstants.FABRIC_EGRESS_EGRESS_NEXT_POP_VLAN)
+                .build();
 
-        // TODO: add NextFunctionType (like ForwardingFunctionType)
-        for (Instruction inst : insts) {
-            switch (inst.type()) {
-                case L2MODIFICATION:
-                    L2ModificationInstruction l2Inst = (L2ModificationInstruction) inst;
-                    switch (l2Inst.subtype()) {
-                        case ETH_SRC:
-                            modEthSrcInst = (ModEtherInstruction) l2Inst;
-                            break;
-                        case ETH_DST:
-                            modEthDstInst = (ModEtherInstruction) l2Inst;
-                            break;
-                        case VLAN_ID:
-                            modVlanIdInst = (ModVlanIdInstruction) l2Inst;
-                            break;
-                        case MPLS_LABEL:
-                            modMplsInst = (ModMplsLabelInstruction) l2Inst;
-                            break;
-                        case VLAN_POP:
-                            // VLAN_POP will be handled by mapEgressNextTreatment()
-                            break;
-                        case MPLS_PUSH:
-                            // Ignore. fabric.p4 only needs MPLS_LABEL to push a label
-                            break;
-                        default:
-                            log.warn("Unsupported l2 instruction sub type {} [table={}, {}]",
-                                     l2Inst.subtype(), tableId, treatment);
-                            break;
-                    }
-                    break;
-                case L3MODIFICATION:
-                    L3ModificationInstruction l3Inst = (L3ModificationInstruction) inst;
-                    switch (l3Inst.subtype()) {
-                        case TTL_OUT:
-                            // Ignore TTL_OUT
-                            break;
-                        default:
-                            log.warn("Unsupported l3 instruction sub type {} [table={}, {}]",
-                                    l3Inst.subtype(), tableId, treatment);
-                            break;
-                    }
-                    break;
-                case OUTPUT:
-                    outInst = (OutputInstruction) inst;
-                    break;
-                default:
-                    log.warn("Unsupported instruction sub type {} [table={}, {}]",
-                             inst.type(), tableId, treatment);
-                    break;
-            }
-        }
-
-        if (tableId.equals(FabricConstants.FABRIC_INGRESS_NEXT_VLAN_META) &&
-                modVlanIdInst != null) {
-            // set_vlan
-            VlanId vlanId = modVlanIdInst.vlanId();
-            PiActionParam newVlanParam =
-                    new PiActionParam(FabricConstants.NEW_VLAN_ID,
-                                      ImmutableByteSequence.copyFrom(vlanId.toShort()));
-            return PiAction.builder()
-                    .withId(FabricConstants.FABRIC_INGRESS_NEXT_SET_VLAN)
-                    .withParameter(newVlanParam)
-                    .build();
-        }
-
-        if (outInst == null) {
-            throw new PiInterpreterException(format(INVALID_TREATMENT, "next", treatment));
-        }
-
-        short portNum = (short) outInst.port().toLong();
-        PiActionParam portNumParam = new PiActionParam(FabricConstants.PORT_NUM,
-                                                       ImmutableByteSequence.copyFrom(portNum));
-        if (modEthDstInst == null && modEthSrcInst == null) {
-            if (modVlanIdInst != null) {
-                VlanId vlanId = modVlanIdInst.vlanId();
-                PiActionParam vlanParam =
-                        new PiActionParam(FabricConstants.NEW_VLAN_ID,
-                                          ImmutableByteSequence.copyFrom(vlanId.toShort()));
-                // set_vlan_output (simple table)
-                return PiAction.builder()
-                        .withId(FabricConstants.FABRIC_INGRESS_NEXT_SET_VLAN_OUTPUT)
-                        .withParameters(ImmutableList.of(portNumParam, vlanParam))
-                        .build();
-            } else {
-                // output (simple or hashed table)
-                return PiAction.builder()
-                        .withId(FabricConstants.FABRIC_INGRESS_NEXT_OUTPUT_SIMPLE)
-                        .withParameter(portNumParam)
-                        .build();
-            }
-        }
-
-        if (modEthDstInst != null && modEthSrcInst != null) {
-            MacAddress srcMac = modEthSrcInst.mac();
-            MacAddress dstMac = modEthDstInst.mac();
-            PiActionParam srcMacParam = new PiActionParam(FabricConstants.SMAC,
-                                                          ImmutableByteSequence.copyFrom(srcMac.toBytes()));
-            PiActionParam dstMacParam = new PiActionParam(FabricConstants.DMAC,
-                                                          ImmutableByteSequence.copyFrom(dstMac.toBytes()));
-
-            if (modMplsInst != null) {
-                // MPLS routing
-                MplsLabel mplsLabel = modMplsInst.label();
-                try {
-                    ImmutableByteSequence mplsValue =
-                            ImmutableByteSequence.copyFrom(mplsLabel.toInt()).fit(20);
-                    PiActionParam mplsParam = new PiActionParam(FabricConstants.LABEL, mplsValue);
-
-                    PiActionId actionId;
-                    // FIXME: finds a way to determine v4 or v6
-                    if (tableId.equals(FabricConstants.FABRIC_INGRESS_NEXT_SIMPLE)) {
-                        actionId = FabricConstants.FABRIC_INGRESS_NEXT_MPLS_ROUTING_V4_SIMPLE;
-                    } else if (tableId.equals(FabricConstants.FABRIC_INGRESS_NEXT_HASHED)) {
-                        actionId = FabricConstants.FABRIC_INGRESS_NEXT_MPLS_ROUTING_V4_HASHED;
-                    } else {
-                        throw new PiInterpreterException(format(INVALID_TREATMENT, "next", treatment));
-                    }
-
-                    return PiAction.builder()
-                            .withId(actionId)
-                            .withParameters(ImmutableList.of(portNumParam,
-                                                             srcMacParam,
-                                                             dstMacParam,
-                                                             mplsParam))
-                            .build();
-                } catch (ImmutableByteSequence.ByteSequenceTrimException e) {
-                    // Basically this won't happened because we already limited
-                    // size of mpls value to 20 bits (0xFFFFF)
-                    throw new PiInterpreterException(format(INVALID_TREATMENT, "next", treatment));
-                }
-            }
-
-            if (modVlanIdInst != null) {
-                VlanId vlanId = modVlanIdInst.vlanId();
-                PiActionParam vlanParam =
-                        new PiActionParam(FabricConstants.NEW_VLAN_ID,
-                                          ImmutableByteSequence.copyFrom(vlanId.toShort()));
-                // L3 routing and set VLAN
-                return PiAction.builder()
-                        .withId(FabricConstants.FABRIC_INGRESS_NEXT_L3_ROUTING_VLAN)
-                        .withParameters(ImmutableList.of(srcMacParam, dstMacParam, portNumParam, vlanParam))
-                        .build();
-            } else {
-                PiActionId actionId;
-                if (tableId.equals(FabricConstants.FABRIC_INGRESS_NEXT_SIMPLE)) {
-                    actionId = FabricConstants.FABRIC_INGRESS_NEXT_L3_ROUTING_SIMPLE;
-                } else if (tableId.equals(FabricConstants.FABRIC_INGRESS_NEXT_HASHED)) {
-                    actionId = FabricConstants.FABRIC_INGRESS_NEXT_L3_ROUTING_HASHED;
-                } else {
-                    throw new PiInterpreterException(format(INVALID_TREATMENT, "next", treatment));
-                }
-
-                // L3 routing
-                return PiAction.builder()
-                        .withId(actionId)
-                        .withParameters(ImmutableList.of(portNumParam,
-                                                         srcMacParam,
-                                                         dstMacParam))
-                        .build();
-            }
-        }
-
-        throw new PiInterpreterException(format(INVALID_TREATMENT, "next", treatment));
     }
 
-    /*
-     * pop_vlan
-     */
-    public static PiAction mapEgressNextTreatment(TrafficTreatment treatment, PiTableId tableId) {
-        // Pop VLAN action for now, may add new action to this control block in the future.
-        return treatment.allInstructions()
-                .stream()
-                .filter(inst -> inst.type() == Instruction.Type.L2MODIFICATION)
-                .map(inst -> (L2ModificationInstruction) inst)
-                .filter(inst -> inst.subtype() == L2ModificationInstruction.L2SubType.VLAN_POP)
-                .findFirst()
-                .map(inst -> POP_VLAN)
-                .orElse(NOP);
+    private static PiAction nop(PiTableId tableId) throws PiInterpreterException {
+        if (!NOP_ACTIONS.containsKey(tableId)) {
+            throw new PiInterpreterException(format("table '%s' doe not specify a nop action", tableId));
+        }
+        return PiAction.builder().withId(NOP_ACTIONS.get(tableId)).build();
+    }
+
+    private static boolean isNoAction(TrafficTreatment treatment) {
+        return treatment.equals(DefaultTrafficTreatment.emptyTreatment()) ||
+                treatment.allInstructions().isEmpty();
+    }
+
+    private static Instruction l2InstructionOrFail(
+            TrafficTreatment treatment,
+            L2ModificationInstruction.L2SubType subType, PiTableId tableId)
+            throws PiInterpreterException {
+        final Instruction inst = l2Instruction(treatment, subType);
+        if (inst == null) {
+            treatmentException(tableId, treatment, format("missing %s instruction", subType));
+        }
+        return inst;
+    }
+
+    private static Instruction instructionOrFail(
+            TrafficTreatment treatment, Instruction.Type type, PiTableId tableId)
+            throws PiInterpreterException {
+        final Instruction inst = instruction(treatment, type);
+        if (inst == null) {
+            treatmentException(tableId, treatment, format("missing %s instruction", type));
+        }
+        return inst;
+    }
+
+    private static void tableException(PiTableId tableId)
+            throws PiInterpreterException {
+        throw new PiInterpreterException(format("Table '%s' not supported", tableId));
+    }
+
+    private static void treatmentException(
+            PiTableId tableId, TrafficTreatment treatment, String explanation)
+            throws PiInterpreterException {
+        throw new PiInterpreterException(format(
+                "Invalid treatment for table '%s', %s: %s", tableId, explanation, treatment));
     }
 }