Small fixes and improvements on fabric.p4

- setting s_tag and c_tag for BNG as early as possible in the pipeline
- ingress_port_vlan was matching on inner_vlan_tag but that field could be not present

Change-Id: Id4d51159a314d45cec370471ed244a51fd74338b

Author: daniele-moro

pipelines/fabric/impl/src/main/java/org/onosproject/pipelines/fabric/impl/behaviour/pipeliner/FilteringObjectiveTranslator.java

@@ -161,6 +161,12 @@ private void ingressPortVlanRule(
         final boolean innerVlanValid = innerVlanCriterion != null
                 && !innerVlanCriterion.vlanId().equals(VlanId.NONE);
 
+        if (innerVlanValid && !capabilities.supportDoubleVlanTerm()) {
+            throw new FabricPipelinerException(
+                    "Found 2 VLAN IDs, but the pipeline does not support double VLAN termination",
+                    ObjectiveError.UNSUPPORTED);
+        }
+
         final PiCriterion piCriterion = PiCriterion.builder()
                 .matchExact(FabricConstants.HDR_VLAN_IS_VALID, outerVlanValid ? ONE : ZERO)
                 .build();

pipelines/fabric/impl/src/main/resources/include/bng.p4

@@ -310,9 +310,6 @@ control bng_ingress(
         bng_ingress_upstream() upstream;
         bng_ingress_downstream() downstream;
 
-        vlan_id_t s_tag = 0;
-        vlan_id_t c_tag = 0;
-
         // TABLE: t_line_map
         // Map s_tag and c_tag to a line ID to uniquely identify a subscriber
 
@@ -322,8 +319,8 @@ control bng_ingress(
 
         table t_line_map {
             key = {
-                s_tag : exact @name("s_tag");
-                c_tag : exact @name("c_tag");
+                fmeta.bng.s_tag : exact @name("s_tag");
+                fmeta.bng.c_tag : exact @name("c_tag");
             }
              actions = {
                 set_line;
@@ -334,17 +331,7 @@ control bng_ingress(
         }
 
         apply {
-            if(hdr.pppoe.isValid()) {
-                s_tag = hdr.vlan_tag.vlan_id;
-                c_tag = hdr.inner_vlan_tag.vlan_id;
-            } else {
-                // We expect the packet to be downstream,
-                // the tags are set by the next stage in the metadata.
-                s_tag = fmeta.vlan_id;
-                c_tag = fmeta.inner_vlan_id;
-            }
-
-            // First map the double VLAN tags to a line ID
+             // First map the double VLAN tags to a line ID
             // If table miss line ID will be 0.
             t_line_map.apply();
 

pipelines/fabric/impl/src/main/resources/include/control/filtering.p4

@@ -48,14 +48,16 @@ control Filtering (inout parsed_headers_t hdr,
         permit();
     }
 
-    // FIXME: remove the use of ternary match on valid inner VLAN.
+    // FIXME: remove the use of ternary match on inner VLAN.
     // Use multi-table approach to remove ternary matching
     table ingress_port_vlan {
         key = {
             standard_metadata.ingress_port : exact @name("ig_port");
             hdr.vlan_tag.isValid()         : exact @name("vlan_is_valid");
             hdr.vlan_tag.vlan_id           : ternary @name("vlan_id");
+#ifdef WITH_DOUBLE_VLAN_TERMINATION
             hdr.inner_vlan_tag.vlan_id     : ternary @name("inner_vlan_id");
+#endif // WITH_DOUBLE_VLAN_TERMINATION
         }
         actions = {
             deny();

pipelines/fabric/impl/src/main/resources/include/control/next.p4

@@ -76,6 +76,10 @@ control Next (inout parsed_headers_t hdr,
         set_vlan(outer_vlan_id);
         fabric_metadata.push_double_vlan = _TRUE;
         fabric_metadata.inner_vlan_id = inner_vlan_id;
+#ifdef WITH_BNG
+        fabric_metadata.bng.s_tag = outer_vlan_id;
+        fabric_metadata.bng.c_tag = inner_vlan_id;
+#endif // WITH_BNG
     }
 #endif // WITH_DOUBLE_VLAN_TERMINATION
 

pipelines/fabric/impl/src/main/resources/include/header.p4

@@ -162,10 +162,12 @@ const bng_type_t BNG_TYPE_UPSTREAM = 2w0x1;
 const bng_type_t BNG_TYPE_DOWNSTREAM = 2w0x2;;
 
 struct bng_meta_t {
-    bit<2>  type; // upstream or downstream
-    bit<32> line_id; // subscriber line
-    bit<16> pppoe_session_id;
-    bit<32> ds_meter_result; // for downstream metering
+    bit<2>    type; // upstream or downstream
+    bit<32>   line_id; // subscriber line
+    bit<16>   pppoe_session_id;
+    bit<32>   ds_meter_result; // for downstream metering
+    vlan_id_t s_tag;
+    vlan_id_t c_tag;
 }
 #endif // WITH_BNG
 

pipelines/fabric/impl/src/main/resources/include/parser.p4

@@ -51,6 +51,9 @@ parser FabricParser (packet_in packet,
 
     state parse_vlan_tag {
         packet.extract(hdr.vlan_tag);
+#ifdef WITH_BNG
+        fabric_metadata.bng.s_tag = hdr.vlan_tag.vlan_id;
+#endif // WITH_BNG
         transition select(packet.lookahead<bit<16>>()){
 #if defined(WITH_XCONNECT) || defined(WITH_DOUBLE_VLAN_TERMINATION)
             ETHERTYPE_VLAN: parse_inner_vlan_tag;
@@ -62,6 +65,9 @@ parser FabricParser (packet_in packet,
 #if defined(WITH_XCONNECT) || defined(WITH_DOUBLE_VLAN_TERMINATION)
     state parse_inner_vlan_tag {
         packet.extract(hdr.inner_vlan_tag);
+#ifdef WITH_BNG
+        fabric_metadata.bng.c_tag = hdr.inner_vlan_tag.vlan_id;
+#endif // WITH_BNG
         transition parse_eth_type;
     }
 #endif // WITH_XCONNECT || WITH_DOUBLE_VLAN_TERMINATION