[ONOS-7285][ONOS-7263] VLAN support by fabric.p4

Change-Id: I9ea460bca2698eb74f0d4988830a1e7cc7bc2768
diff --git a/pipelines/fabric/src/main/java/org/onosproject/pipelines/fabric/FabricConstants.java b/pipelines/fabric/src/main/java/org/onosproject/pipelines/fabric/FabricConstants.java
index d8db593..5bb6a5d 100644
--- a/pipelines/fabric/src/main/java/org/onosproject/pipelines/fabric/FabricConstants.java
+++ b/pipelines/fabric/src/main/java/org/onosproject/pipelines/fabric/FabricConstants.java
@@ -59,6 +59,8 @@
             buildPiMatchField(ETHERNET, "src_addr", true);
     public static final PiMatchFieldId HF_ICMP_ICMP_TYPE_ID =
             buildPiMatchField(ICMP, "icmp_type", true);
+    public static final PiMatchFieldId HF_STANDARD_METADATA_EGRESS_PORT_ID =
+            buildPiMatchField(STANDARD_METADATA, "egress_port", false);
     public static final PiMatchFieldId HF_FABRIC_METADATA_NEXT_ID_ID =
             buildPiMatchField(FABRIC_METADATA, "next_id", false);
     public static final PiMatchFieldId HF_FABRIC_METADATA_L4_DST_PORT_ID =
@@ -91,6 +93,8 @@
             PiTableId.of("FabricIngress.forwarding.acl");
     public static final PiTableId TBL_HASHED_ID =
             PiTableId.of("FabricIngress.next.hashed");
+    public static final PiTableId TBL_EGRESS_VLAN_ID =
+            PiTableId.of("FabricEgress.egress_next.egress_vlan");
     public static final PiTableId TBL_MPLS_ID =
             PiTableId.of("FabricIngress.forwarding.mpls");
     public static final PiTableId TBL_MULTICAST_ID =
@@ -105,12 +109,14 @@
             PiTableId.of("FabricIngress.filtering.fwd_classifier");
     public static final PiTableId TBL_BRIDGING_ID =
             PiTableId.of("FabricIngress.forwarding.bridging");
+    public static final PiTableId TBL_SIMPLE_ID =
+            PiTableId.of("FabricIngress.next.simple");
     public static final PiTableId TBL_INGRESS_PORT_VLAN_ID =
             PiTableId.of("FabricIngress.filtering.ingress_port_vlan");
     public static final PiTableId TBL_UNICAST_V6_ID =
             PiTableId.of("FabricIngress.forwarding.unicast_v6");
-    public static final PiTableId TBL_SIMPLE_ID =
-            PiTableId.of("FabricIngress.next.simple");
+    public static final PiTableId TBL_VLAN_META_ID =
+            PiTableId.of("FabricIngress.next.vlan_meta");
 
     // Indirect Counter IDs
     public static final PiCounterId CNT_EGRESS_PORT_COUNTER_ID =
@@ -123,8 +129,8 @@
             PiCounterId.of("FabricIngress.forwarding.acl_counter");
     public static final PiCounterId CNT_MULTICAST_COUNTER_ID =
             PiCounterId.of("FabricIngress.next.multicast_counter");
-    public static final PiCounterId CNT_SIMPLE_COUNTER_ID =
-            PiCounterId.of("FabricIngress.next.simple_counter");
+    public static final PiCounterId CNT_VLAN_META_COUNTER_ID =
+            PiCounterId.of("FabricIngress.next.vlan_meta_counter");
     public static final PiCounterId CNT_FWD_CLASSIFIER_COUNTER_ID =
             PiCounterId.of("FabricIngress.filtering.fwd_classifier_counter");
     public static final PiCounterId CNT_BRIDGING_COUNTER_ID =
@@ -137,6 +143,8 @@
             PiCounterId.of("FabricIngress.forwarding.unicast_v6_counter");
     public static final PiCounterId CNT_UNICAST_V4_COUNTER_ID =
             PiCounterId.of("FabricIngress.forwarding.unicast_v4_counter");
+    public static final PiCounterId CNT_SIMPLE_COUNTER_ID =
+            PiCounterId.of("FabricIngress.next.simple_counter");
     public static final PiCounterId CNT_INGRESS_PORT_VLAN_COUNTER_ID =
             PiCounterId.of("FabricIngress.filtering.ingress_port_vlan_counter");
     public static final PiCounterId CNT_MPLS_COUNTER_ID =
@@ -147,8 +155,12 @@
     // Action IDs
     public static final PiActionId ACT_FABRICINGRESS_FILTERING_DROP_ID =
             PiActionId.of("FabricIngress.filtering.drop");
+    public static final PiActionId ACT_FABRICINGRESS_NEXT_SET_VLAN_ID =
+            PiActionId.of("FabricIngress.next.set_vlan");
     public static final PiActionId ACT_FABRICINGRESS_FORWARDING_POP_MPLS_AND_NEXT_ID =
             PiActionId.of("FabricIngress.forwarding.pop_mpls_and_next");
+    public static final PiActionId ACT_FABRICEGRESS_EGRESS_NEXT_POP_VLAN_ID =
+            PiActionId.of("FabricEgress.egress_next.pop_vlan");
     public static final PiActionId ACT_FABRICINGRESS_FILTERING_SET_FORWARDING_TYPE_ID =
             PiActionId.of("FabricIngress.filtering.set_forwarding_type");
     public static final PiActionId ACT_NOP_ID = PiActionId.of("nop");
@@ -156,6 +168,10 @@
             PiActionId.of("FabricIngress.filtering.set_vlan");
     public static final PiActionId ACT_FABRICINGRESS_NEXT_MPLS_ROUTING_V6_ID =
             PiActionId.of("FabricIngress.next.mpls_routing_v6");
+    public static final PiActionId ACT_FABRICEGRESS_PKT_IO_EGRESS_POP_VLAN_ID =
+            PiActionId.of("FabricEgress.pkt_io_egress.pop_vlan");
+    public static final PiActionId ACT_FABRICINGRESS_NEXT_L3_ROUTING_VLAN_ID =
+            PiActionId.of("FabricIngress.next.l3_routing_vlan");
     public static final PiActionId ACT_NOACTION_ID = PiActionId.of("NoAction");
     public static final PiActionId ACT_FABRICINGRESS_NEXT_SET_MCAST_GROUP_ID =
             PiActionId.of("FabricIngress.next.set_mcast_group");
@@ -203,6 +219,5 @@
             PiControlMetadataId.of("ingress_port");
     public static final PiControlMetadataId CTRL_META_EGRESS_PORT_ID =
             PiControlMetadataId.of("egress_port");
-
     public static final int PORT_BITWIDTH = 9;
 }
diff --git a/pipelines/fabric/src/main/java/org/onosproject/pipelines/fabric/FabricInterpreter.java b/pipelines/fabric/src/main/java/org/onosproject/pipelines/fabric/FabricInterpreter.java
index ef011ee..fb85ff3 100644
--- a/pipelines/fabric/src/main/java/org/onosproject/pipelines/fabric/FabricInterpreter.java
+++ b/pipelines/fabric/src/main/java/org/onosproject/pipelines/fabric/FabricInterpreter.java
@@ -90,7 +90,10 @@
                                                                                FabricConstants.TBL_ACL_ID);
     private static final Set<PiTableId> NEXT_CTRL_TBLS = ImmutableSet.of(FabricConstants.TBL_SIMPLE_ID,
                                                                          FabricConstants.TBL_HASHED_ID,
-                                                                         FabricConstants.TBL_MULTICAST_ID);
+                                                                         FabricConstants.TBL_MULTICAST_ID,
+                                                                         FabricConstants.TBL_VLAN_META_ID);
+
+    private static final Set<PiTableId> E_NEXT_CTRL_TBLS = ImmutableSet.of(FabricConstants.TBL_EGRESS_VLAN_ID);
 
     private static final ImmutableMap<Criterion.Type, PiMatchFieldId> CRITERION_MAP =
             ImmutableMap.<Criterion.Type, PiMatchFieldId>builder()
@@ -161,6 +164,8 @@
             return FabricTreatmentInterpreter.mapForwardingTreatment(treatment);
         } else if (NEXT_CTRL_TBLS.contains(piTableId)) {
             return FabricTreatmentInterpreter.mapNextTreatment(treatment);
+        } else if (E_NEXT_CTRL_TBLS.contains(piTableId)) {
+            return FabricTreatmentInterpreter.mapEgressNextTreatment(treatment);
         } else {
             throw new PiInterpreterException(String.format("Table %s unsupported", piTableId));
         }
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 3dc2f41..3be56bb 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
@@ -50,6 +50,13 @@
 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 PiAction NOP = PiAction.builder()
+            .withId(FabricConstants.ACT_NOP_ID)
+            .build();
+
+    private static final PiAction POP_VLAN = PiAction.builder()
+            .withId(FabricConstants.ACT_FABRICEGRESS_EGRESS_NEXT_POP_VLAN_ID)
+            .build();
 
     // Hide default constructor
     protected FabricTreatmentInterpreter() {
@@ -72,9 +79,7 @@
         Instruction noActInst = Instructions.createNoAction();
         if (instructions.isEmpty() || instructions.contains(noActInst)) {
             // nop
-            return PiAction.builder()
-                    .withId(FabricConstants.ACT_NOP_ID)
-                    .build();
+            return NOP;
         }
 
         L2ModificationInstruction.ModVlanHeaderInstruction pushVlanInst = null;
@@ -159,17 +164,16 @@
      * output
      * set_vlan_output
      * l3_routing
+     * l3_routing_vlan
      * mpls_routing_v4
      *
-     * Unsupported, using PiAction directly:
-     * set_next_type
-     *
      * Unsupported, need to find a way to implement it
      * mpls_routing_v6
      */
 
     public static PiAction mapNextTreatment(TrafficTreatment treatment)
             throws PiInterpreterException {
+        // TODO: refactor this method
         List<Instruction> insts = treatment.allInstructions();
         OutputInstruction outInst = null;
         ModEtherInstruction modEthDstInst = null;
@@ -195,8 +199,6 @@
                         case MPLS_LABEL:
                             modMplsInst = (ModMplsLabelInstruction) l2Inst;
                             break;
-                        case VLAN_PUSH:
-                            break;
                         default:
                             log.warn("Unsupported l2 instruction sub type: {}", l2Inst.subtype());
                             break;
@@ -212,8 +214,23 @@
         }
 
         if (outInst == null) {
-            throw new PiInterpreterException(format(INVALID_TREATMENT, "next", treatment));
+            // for vlan_meta table only
+            if (modVlanIdInst != null) {
+                // set_vlan
+                VlanId vlanId = modVlanIdInst.vlanId();
+                PiActionParam newVlanParam =
+                        new PiActionParam(FabricConstants.ACT_PRM_NEW_VLAN_ID_ID,
+                                          ImmutableByteSequence.copyFrom(vlanId.toShort()));
+                // set_vlan_output
+                return PiAction.builder()
+                        .withId(FabricConstants.ACT_FABRICINGRESS_NEXT_SET_VLAN_ID)
+                        .withParameter(newVlanParam)
+                        .build();
+            } else {
+                throw new PiInterpreterException(format(INVALID_TREATMENT, "next", treatment));
+            }
         }
+
         short portNum = (short) outInst.port().toLong();
         PiActionParam portNumParam = new PiActionParam(FabricConstants.ACT_PRM_PORT_NUM_ID,
                                                        ImmutableByteSequence.copyFrom(portNum));
@@ -267,15 +284,39 @@
                 }
             }
 
-            // L3 routing
-            return PiAction.builder()
-                    .withId(FabricConstants.ACT_FABRICINGRESS_NEXT_L3_ROUTING_ID)
-                    .withParameters(ImmutableList.of(portNumParam,
-                                                     srcMacParam,
-                                                     dstMacParam))
-                    .build();
+            if (modVlanIdInst != null) {
+                VlanId vlanId = modVlanIdInst.vlanId();
+                PiActionParam vlanParam =
+                        new PiActionParam(FabricConstants.ACT_PRM_NEW_VLAN_ID_ID,
+                                          ImmutableByteSequence.copyFrom(vlanId.toShort()));
+                // L3 routing and set VLAN
+                return PiAction.builder()
+                        .withId(FabricConstants.ACT_FABRICINGRESS_NEXT_L3_ROUTING_VLAN_ID)
+                        .withParameters(ImmutableList.of(srcMacParam, dstMacParam, portNumParam, vlanParam))
+                        .build();
+            } else {
+                // L3 routing
+                return PiAction.builder()
+                        .withId(FabricConstants.ACT_FABRICINGRESS_NEXT_L3_ROUTING_ID)
+                        .withParameters(ImmutableList.of(portNumParam,
+                                                         srcMacParam,
+                                                         dstMacParam))
+                        .build();
+            }
         }
 
         throw new PiInterpreterException(format(INVALID_TREATMENT, "next", treatment));
     }
+
+    public static PiAction mapEgressNextTreatment(TrafficTreatment treatment) {
+        // 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);
+    }
 }
diff --git a/pipelines/fabric/src/main/java/org/onosproject/pipelines/fabric/pipeliner/FabricNextPipeliner.java b/pipelines/fabric/src/main/java/org/onosproject/pipelines/fabric/pipeliner/FabricNextPipeliner.java
index c7c7647..12602b4 100644
--- a/pipelines/fabric/src/main/java/org/onosproject/pipelines/fabric/pipeliner/FabricNextPipeliner.java
+++ b/pipelines/fabric/src/main/java/org/onosproject/pipelines/fabric/pipeliner/FabricNextPipeliner.java
@@ -16,16 +16,21 @@
 
 package org.onosproject.pipelines.fabric.pipeliner;
 
+import org.onlab.packet.VlanId;
 import org.onosproject.net.DeviceId;
+import org.onosproject.net.PortNumber;
 import org.onosproject.net.driver.Driver;
 import org.onosproject.net.flow.DefaultFlowRule;
 import org.onosproject.net.flow.DefaultTrafficSelector;
 import org.onosproject.net.flow.DefaultTrafficTreatment;
 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.PiCriterion;
+import org.onosproject.net.flow.criteria.VlanIdCriterion;
 import org.onosproject.net.flow.instructions.Instruction;
 import org.onosproject.net.flow.instructions.Instructions.OutputInstruction;
+import org.onosproject.net.flow.instructions.L2ModificationInstruction;
 import org.onosproject.net.flowobjective.DefaultNextObjective;
 import org.onosproject.net.flowobjective.NextObjective;
 import org.onosproject.net.flowobjective.Objective;
@@ -44,8 +49,11 @@
 
 import static org.onosproject.pipelines.fabric.FabricConstants.ACT_PRF_FABRICINGRESS_NEXT_ECMP_SELECTOR_ID;
 import static org.onosproject.pipelines.fabric.FabricConstants.HF_FABRIC_METADATA_NEXT_ID_ID;
+import static org.onosproject.pipelines.fabric.FabricConstants.HF_STANDARD_METADATA_EGRESS_PORT_ID;
+import static org.onosproject.pipelines.fabric.FabricConstants.TBL_EGRESS_VLAN_ID;
 import static org.onosproject.pipelines.fabric.FabricConstants.TBL_HASHED_ID;
 import static org.onosproject.pipelines.fabric.FabricConstants.TBL_SIMPLE_ID;
+import static org.onosproject.pipelines.fabric.FabricConstants.TBL_VLAN_META_ID;
 import static org.slf4j.LoggerFactory.getLogger;
 
 /**
@@ -66,6 +74,8 @@
     public PipelinerTranslationResult next(NextObjective nextObjective) {
         PipelinerTranslationResult.Builder resultBuilder = PipelinerTranslationResult.builder();
 
+        processNextVlanMeta(nextObjective, resultBuilder);
+
         switch (nextObjective.type()) {
             case SIMPLE:
                 processSimpleNext(nextObjective, resultBuilder);
@@ -82,6 +92,38 @@
         return resultBuilder.build();
     }
 
+    private void processNextVlanMeta(NextObjective next,
+                                     PipelinerTranslationResult.Builder resultBuilder) {
+        TrafficSelector meta = next.meta();
+        if (meta == null) {
+            // do nothing if there is no metadata in the next objective.
+            return;
+        }
+        VlanIdCriterion vlanIdCriterion =
+                (VlanIdCriterion) meta.getCriterion(Criterion.Type.VLAN_VID);
+
+        if (vlanIdCriterion == null) {
+            // do nothing if we can't find vlan from next objective metadata.
+            return;
+        }
+
+        VlanId vlanId = vlanIdCriterion.vlanId();
+        TrafficSelector selector = buildNextIdSelector(next.id());
+        TrafficTreatment treatment = DefaultTrafficTreatment.builder()
+                .setVlanId(vlanId)
+                .build();
+
+        resultBuilder.addFlowRule(DefaultFlowRule.builder()
+                                          .withSelector(selector)
+                                          .withTreatment(treatment)
+                                          .forTable(TBL_VLAN_META_ID)
+                                          .makePermanent()
+                                          .withPriority(next.priority())
+                                          .forDevice(deviceId)
+                                          .fromApp(next.appId())
+                                          .build());
+    }
+
     private void processSimpleNext(NextObjective next,
                                    PipelinerTranslationResult.Builder resultBuilder) {
 
@@ -93,18 +135,14 @@
 
         TrafficSelector selector = buildNextIdSelector(next.id());
         TrafficTreatment treatment = next.next().iterator().next();
-        OutputInstruction outputInst = treatment.allInstructions()
-                .stream()
-                .filter(inst -> inst.type() == Instruction.Type.OUTPUT)
-                .map(inst -> (OutputInstruction) inst)
-                .findFirst()
-                .orElse(null);
+        PortNumber outputPort = getOutputPort(treatment);
 
-        if (outputInst == null) {
+        if (outputPort == null) {
             log.warn("At least one output instruction in simple next objective");
             resultBuilder.setError(ObjectiveError.BADPARAMS);
             return;
         }
+
         resultBuilder.addFlowRule(DefaultFlowRule.builder()
                                           .withSelector(selector)
                                           .withTreatment(treatment)
@@ -114,29 +152,81 @@
                                           .forDevice(deviceId)
                                           .fromApp(next.appId())
                                           .build());
+
+        if (includesPopVlanInst(treatment)) {
+            processVlanPopRule(outputPort, next, resultBuilder);
+        }
     }
 
-    private void processHashedNext(NextObjective nextObjective, PipelinerTranslationResult.Builder resultBuilder) {
+    private PortNumber getOutputPort(TrafficTreatment treatment) {
+        return treatment.allInstructions()
+                .stream()
+                .filter(inst -> inst.type() == Instruction.Type.OUTPUT)
+                .map(inst -> (OutputInstruction) inst)
+                .findFirst()
+                .map(OutputInstruction::port)
+                .orElse(null);
+    }
+
+    private boolean includesPopVlanInst(TrafficTreatment treatment) {
+        return treatment.allInstructions()
+                .stream()
+                .filter(inst -> inst.type() == Instruction.Type.L2MODIFICATION)
+                .map(inst -> (L2ModificationInstruction) inst)
+                .anyMatch(inst -> inst.subtype() == L2ModificationInstruction.L2SubType.VLAN_POP);
+    }
+
+    private void processVlanPopRule(PortNumber port, NextObjective next,
+                                    PipelinerTranslationResult.Builder resultBuilder) {
+        TrafficSelector meta = next.meta();
+        VlanIdCriterion vlanIdCriterion =
+                (VlanIdCriterion) meta.getCriterion(Criterion.Type.VLAN_VID);
+        VlanId vlanId = vlanIdCriterion.vlanId();
+
+        PiCriterion egressVlanTableMatch = PiCriterion.builder()
+                .matchExact(HF_STANDARD_METADATA_EGRESS_PORT_ID,
+                            (short) port.toLong())
+                .build();
+        // Add VLAN pop rule to egress pipeline table
+        TrafficSelector selector = DefaultTrafficSelector.builder()
+                .matchPi(egressVlanTableMatch)
+                .matchVlanId(vlanId)
+                .build();
+        TrafficTreatment treatment = DefaultTrafficTreatment.builder()
+                .popVlan()
+                .build();
+        resultBuilder.addFlowRule(DefaultFlowRule.builder()
+                                          .withSelector(selector)
+                                          .withTreatment(treatment)
+                                          .forTable(TBL_EGRESS_VLAN_ID)
+                                          .makePermanent()
+                                          .withPriority(next.priority())
+                                          .forDevice(deviceId)
+                                          .fromApp(next.appId())
+                                          .build());
+    }
+
+    private void processHashedNext(NextObjective next, PipelinerTranslationResult.Builder resultBuilder) {
         boolean noHashedTable = Boolean.parseBoolean(driver.getProperty(NO_HASHED_TABLE));
 
         if (noHashedTable) {
-            if (nextObjective.next().isEmpty()) {
+            if (next.next().isEmpty()) {
                 return;
             }
             // use first action if not support hashed group
-            TrafficTreatment treatment = nextObjective.next().iterator().next();
+            TrafficTreatment treatment = next.next().iterator().next();
 
             NextObjective.Builder simpleNext = DefaultNextObjective.builder()
                     .addTreatment(treatment)
-                    .withId(nextObjective.id())
-                    .fromApp(nextObjective.appId())
+                    .withId(next.id())
+                    .fromApp(next.appId())
                     .makePermanent()
-                    .withMeta(nextObjective.meta())
-                    .withPriority(nextObjective.priority())
+                    .withMeta(next.meta())
+                    .withPriority(next.priority())
                     .withType(NextObjective.Type.SIMPLE);
 
-            if (nextObjective.context().isPresent()) {
-                processSimpleNext(simpleNext.add(nextObjective.context().get()), resultBuilder);
+            if (next.context().isPresent()) {
+                processSimpleNext(simpleNext.add(next.context().get()), resultBuilder);
             } else {
                 processSimpleNext(simpleNext.add(), resultBuilder);
             }
@@ -144,15 +234,23 @@
         }
 
         // create hash groups
-        int groupId = nextObjective.id();
-        List<GroupBucket> bucketList = nextObjective.next().stream()
+        int groupId = next.id();
+        List<GroupBucket> bucketList = next.next().stream()
                 .map(DefaultGroupBucket::createSelectGroupBucket)
                 .collect(Collectors.toList());
 
-        if (bucketList.size() != nextObjective.next().size()) {
+        // Egress VLAN handling
+        next.next().forEach(treatment -> {
+            PortNumber outputPort = getOutputPort(treatment);
+            if (includesPopVlanInst(treatment) && outputPort != null) {
+                processVlanPopRule(outputPort, next, resultBuilder);
+            }
+        });
+
+        if (bucketList.size() != next.next().size()) {
             // some action not converted
             // set error
-            log.warn("Expected bucket size {}, got {}", nextObjective.next().size(), bucketList.size());
+            log.warn("Expected bucket size {}, got {}", next.next().size(), bucketList.size());
             resultBuilder.setError(ObjectiveError.BADPARAMS);
             return;
         }
@@ -167,18 +265,18 @@
                                                            buckets,
                                                            groupKey,
                                                            groupId,
-                                                           nextObjective.appId()));
+                                                           next.appId()));
 
         // flow
         // If operation is ADD_TO_EXIST or REMOVE_FROM_EXIST, means we modify
         // group buckets only, no changes for flow rule
-        if (nextObjective.op() == Objective.Operation.ADD_TO_EXISTING ||
-                nextObjective.op() == Objective.Operation.REMOVE_FROM_EXISTING) {
+        if (next.op() == Objective.Operation.ADD_TO_EXISTING ||
+                next.op() == Objective.Operation.REMOVE_FROM_EXISTING) {
             return;
         }
-        TrafficSelector selector = buildNextIdSelector(nextObjective.id());
+        TrafficSelector selector = buildNextIdSelector(next.id());
         TrafficTreatment treatment = DefaultTrafficTreatment.builder()
-                .piTableAction(PiActionGroupId.of(nextObjective.id()))
+                .piTableAction(PiActionGroupId.of(next.id()))
                 .build();
 
         resultBuilder.addFlowRule(DefaultFlowRule.builder()
@@ -186,9 +284,9 @@
                                           .withTreatment(treatment)
                                           .forTable(TBL_HASHED_ID)
                                           .makePermanent()
-                                          .withPriority(nextObjective.priority())
+                                          .withPriority(next.priority())
                                           .forDevice(deviceId)
-                                          .fromApp(nextObjective.appId())
+                                          .fromApp(next.appId())
                                           .build());
     }
 
diff --git a/pipelines/fabric/src/main/java/org/onosproject/pipelines/fabric/pipeliner/FabricPipeliner.java b/pipelines/fabric/src/main/java/org/onosproject/pipelines/fabric/pipeliner/FabricPipeliner.java
index 5c66474..363a95c 100644
--- a/pipelines/fabric/src/main/java/org/onosproject/pipelines/fabric/pipeliner/FabricPipeliner.java
+++ b/pipelines/fabric/src/main/java/org/onosproject/pipelines/fabric/pipeliner/FabricPipeliner.java
@@ -263,7 +263,12 @@
         };
 
         FlowRuleOperations ops = buildFlowRuleOps(objective, flowRules, ctx);
-        flowRuleService.apply(ops);
+        if (ops != null) {
+            flowRuleService.apply(ops);
+        } else {
+            // remove pendings
+            flowRules.forEach(flowRule -> pendingInstallObjectiveFlows.remove(flowRule.id()));
+        }
     }
 
     private void installGroups(Objective objective, Collection<GroupDescription> groups) {
@@ -317,8 +322,13 @@
             case REMOVE:
                 flowRules.forEach(ops::remove);
                 break;
+            case ADD_TO_EXISTING:
+            case REMOVE_FROM_EXISTING:
+                // Next objective may use ADD_TO_EXIST or REMOVE_FROM_EXIST op
+                // No need to update FlowRuls for vlan_meta table.
+                return null;
             default:
-                log.warn("Unsupported op {} for {}", objective);
+                log.warn("Unsupported op {} for {}", objective.op(), objective);
                 fail(objective, ObjectiveError.BADPARAMS);
                 return null;
         }