Merge remote-tracking branch 'origin/main' into build-all-test-few

.jenkins/pr_verify.sh

@@ -23,7 +23,7 @@ docker pull "${SDE_P4C_DOCKER_IMG}"
 make -j8 all
 
 echo "Build and verify Java pipeconf"
-make constants pipeconf MVN_FLAGS="-Pci-verify -Pcoverage"
+make constants pipeconf-ci MVN_FLAGS="-Pci-verify -Pcoverage"
 
 echo "Upload coverage to codecov"
 bash .jenkins/codecov.sh -Z

Makefile

@@ -74,6 +74,12 @@ pipeconf-test: _mvn_package
 	docker run --rm -v $(DIR):mvn-src -w /mvn-src \
 		-v $(MVN_CACHE):/root/.m2 $(MAVEN_DOCKER_IMAGE) mvn test
 
+pipeconf-ci:
+	$(info *** Building ONOS app...)
+	@mkdir -p target
+	docker run --rm -v $(DIR):/mvn-src -w /mvn-src \
+		-v $(MVN_CACHE):/root/.m2 $(MAVEN_DOCKER_IMAGE) mvn $(MVN_FLAGS) clean package verify
+
 _pipeconf-oar-exists:
 	@test -f $(PIPECONF_OAR_FILE) || (echo "pipeconf .oar not found" && exit 1)
 

p4src/fabric_tna.p4

@@ -12,6 +12,7 @@
 #include "include/control/filtering.p4"
 #include "include/control/forwarding.p4"
 #include "include/control/pre_next.p4"
+#include "include/control/lookup_md_init.p4"
 #include "include/control/acl.p4"
 #include "include/control/next.p4"
 #include "include/control/hasher.p4"
@@ -32,7 +33,7 @@ control FabricIngress (
     inout ingress_intrinsic_metadata_for_deparser_t  ig_dprsr_md,
     inout ingress_intrinsic_metadata_for_tm_t        ig_tm_md) {
 
-    AclLookupInit() acl_lkp_init;
+    LookupMdInit() lkp_md_init;
     PacketIoIngress() pkt_io;
     Filtering() filtering;
     Forwarding() forwarding;
@@ -48,7 +49,7 @@ control FabricIngress (
 #endif // WITH_INT
 
     apply {
-        acl_lkp_init.apply(hdr, fabric_md.acl_lkp);
+        lkp_md_init.apply(hdr, fabric_md.lkp);
         pkt_io.apply(hdr, fabric_md, ig_intr_md, ig_tm_md, ig_dprsr_md);
         filtering.apply(hdr, fabric_md, ig_intr_md);
 #ifdef WITH_SPGW

p4src/include/control/acl.p4

@@ -7,44 +7,6 @@
 #include "../define.p4"
 #include "../header.p4"
 
-control AclLookupInit (in parsed_headers_t hdr,
-                       out acl_lookup_t lkp) {
-
-    apply {
-       lkp.is_ipv4 = false;
-       lkp.ipv4_src = 0;
-       lkp.ipv4_dst = 0;
-       lkp.ip_proto = 0;
-       lkp.l4_sport = 0;
-       lkp.l4_dport = 0;
-       if (hdr.inner_ipv4.isValid()) {
-           lkp.is_ipv4 = true;
-           lkp.ipv4_src = hdr.inner_ipv4.src_addr;
-           lkp.ipv4_dst = hdr.inner_ipv4.dst_addr;
-           lkp.ip_proto = hdr.inner_ipv4.protocol;
-           if (hdr.inner_tcp.isValid()) {
-               lkp.l4_sport = hdr.inner_tcp.sport;
-               lkp.l4_dport = hdr.inner_tcp.dport;
-           } else if (hdr.inner_udp.isValid()) {
-               lkp.l4_sport = hdr.inner_udp.sport;
-               lkp.l4_dport = hdr.inner_udp.dport;
-           }
-       } else if (hdr.ipv4.isValid()) {
-           lkp.is_ipv4 = true;
-           lkp.ipv4_src = hdr.ipv4.src_addr;
-           lkp.ipv4_dst = hdr.ipv4.dst_addr;
-           lkp.ip_proto = hdr.ipv4.protocol;
-           if (hdr.tcp.isValid()) {
-               lkp.l4_sport = hdr.tcp.sport;
-               lkp.l4_dport = hdr.tcp.dport;
-           } else if (hdr.udp.isValid()) {
-               lkp.l4_sport = hdr.udp.sport;
-               lkp.l4_dport = hdr.udp.dport;
-           }
-       }
-    }
-}
-
 control Acl (inout parsed_headers_t hdr,
              inout fabric_ingress_metadata_t fabric_md,
              in ingress_intrinsic_metadata_t ig_intr_md,
@@ -100,19 +62,19 @@ control Acl (inout parsed_headers_t hdr,
 
     table acl {
         key = {
-            ig_intr_md.ingress_port     : ternary @name("ig_port");   // 9
-            hdr.ethernet.dst_addr       : ternary @name("eth_dst");   // 48
-            hdr.ethernet.src_addr       : ternary @name("eth_src");   // 48
-            hdr.vlan_tag.vlan_id        : ternary @name("vlan_id");   // 12
-            hdr.eth_type.value          : ternary @name("eth_type");  // 16
-            fabric_md.acl_lkp.ipv4_src  : ternary @name("ipv4_src");  // 32
-            fabric_md.acl_lkp.ipv4_dst  : ternary @name("ipv4_dst");  // 32
-            fabric_md.acl_lkp.ip_proto  : ternary @name("ip_proto");  // 8
-            hdr.icmp.icmp_type          : ternary @name("icmp_type"); // 8
-            hdr.icmp.icmp_code          : ternary @name("icmp_code"); // 8
-            fabric_md.acl_lkp.l4_sport  : ternary @name("l4_sport");  // 16
-            fabric_md.acl_lkp.l4_dport  : ternary @name("l4_dport");  // 16
-            fabric_md.ig_port_type      : ternary @name("ig_port_type"); // 2
+            ig_intr_md.ingress_port          : ternary @name("ig_port");   // 9
+            fabric_md.lkp.eth_dst            : ternary @name("eth_dst");   // 48
+            fabric_md.lkp.eth_src            : ternary @name("eth_src");   // 48
+            fabric_md.lkp.vlan_id            : ternary @name("vlan_id");   // 12
+            fabric_md.lkp.eth_type           : ternary @name("eth_type");  // 16
+            fabric_md.lkp.ipv4_src           : ternary @name("ipv4_src");  // 32
+            fabric_md.lkp.ipv4_dst           : ternary @name("ipv4_dst");  // 32
+            fabric_md.lkp.ip_proto           : ternary @name("ip_proto");  // 8
+            fabric_md.lkp.icmp_type          : ternary @name("icmp_type"); // 8
+            fabric_md.lkp.icmp_code          : ternary @name("icmp_code"); // 8
+            fabric_md.lkp.l4_sport           : ternary @name("l4_sport");  // 16
+            fabric_md.lkp.l4_dport           : ternary @name("l4_dport");  // 16
+            fabric_md.ig_port_type           : ternary @name("ig_port_type"); // 2
         }
 
         actions = {

p4src/include/control/filtering.p4

@@ -86,10 +86,10 @@ control Filtering (inout parsed_headers_t hdr,
 
     table fwd_classifier {
         key = {
-            ig_intr_md.ingress_port        : exact @name("ig_port");
-            hdr.ethernet.dst_addr          : ternary @name("eth_dst");
-            hdr.eth_type.value             : ternary @name("eth_type");
-            fabric_md.bridged.base.ip_eth_type  : exact @name("ip_eth_type");
+            ig_intr_md.ingress_port                 : exact @name("ig_port");
+            fabric_md.lkp.eth_dst                   : ternary @name("eth_dst");
+            fabric_md.lkp.eth_type                  : ternary @name("eth_type");
+            fabric_md.bridged.base.ip_eth_type      : exact @name("ip_eth_type");
         }
         actions = {
             set_forwarding_type;
@@ -101,6 +101,8 @@ control Filtering (inout parsed_headers_t hdr,
 
     apply {
         ingress_port_vlan.apply();
+        // we don't check if Ethernet and eth_type is valid,
+        // because it is always extracted in the Parser.
         fwd_classifier.apply();
 #ifdef WTIH_DEBUG
         fwd_type_counter.count(fabric_md.bridged.base.fwd_type);

p4src/include/control/hasher.p4

@@ -4,54 +4,45 @@
 #include "../define.p4"
 #include "../header.p4"
 
-// Used for ECMP hashing.
-struct gtp_flow_t {
-    bit<32>   ipv4_src;
-    bit<32>   ipv4_dst;
-    teid_t    gtpu_teid;
-}
-
 control Hasher(
     in parsed_headers_t hdr,
     inout fabric_ingress_metadata_t fabric_md) {
 
     Hash<flow_hash_t>(HashAlgorithm_t.CRC32) ip_hasher;
     Hash<flow_hash_t>(HashAlgorithm_t.CRC32) gtp_flow_hasher;
+    Hash<flow_hash_t>(HashAlgorithm_t.CRC32) encap_gtp_flow_hasher;
     Hash<flow_hash_t>(HashAlgorithm_t.CRC32) non_ip_hasher;
 
     apply {
-        if (fabric_md.acl_lkp.is_ipv4) {
-            gtp_flow_t to_hash;
-            bool calc_gtp_hash = false;
-
-            // we always need to calculate hash from the inner IPv4 header for the INT reporter.
-            fabric_md.bridged.base.inner_hash = ip_hasher.get(fabric_md.acl_lkp);
-
-            // use inner hash by default
-            fabric_md.ecmp_hash = fabric_md.bridged.base.inner_hash;
-
-            // if an outer GTP header exists, use it to perform GTP-aware ECMP
-            if (hdr.gtpu.isValid()) {
-                to_hash.ipv4_src = hdr.ipv4.src_addr;
-                to_hash.ipv4_dst = hdr.ipv4.dst_addr;
-                to_hash.gtpu_teid = hdr.gtpu.teid;
-                calc_gtp_hash = true;
-            }
+        // we always need to calculate hash from the inner IPv4 header for the INT reporter.
+        fabric_md.bridged.base.inner_hash = ip_hasher.get({
+            fabric_md.lkp.ipv4_src,
+            fabric_md.lkp.ipv4_dst,
+            fabric_md.lkp.ip_proto,
+            fabric_md.lkp.l4_sport,
+            fabric_md.lkp.l4_dport
+        });
 
 #ifdef WITH_SPGW
-            // enable GTP-aware ECMP for downlink packets.
-            if (fabric_md.bridged.spgw.needs_gtpu_encap) {
-                to_hash.ipv4_src = fabric_md.bridged.spgw.gtpu_tunnel_sip;
-                to_hash.ipv4_dst = fabric_md.bridged.spgw.gtpu_tunnel_dip;
-                to_hash.gtpu_teid = fabric_md.bridged.spgw.gtpu_teid;
-                calc_gtp_hash = true;
-            }
+        // enable GTP-aware ECMP for downlink packets.
+        if (fabric_md.bridged.spgw.needs_gtpu_encap) {
+            fabric_md.ecmp_hash = encap_gtp_flow_hasher.get({
+                fabric_md.bridged.spgw.gtpu_tunnel_sip,
+                fabric_md.bridged.spgw.gtpu_tunnel_dip,
+                fabric_md.bridged.spgw.gtpu_teid
+            });
+        } else
 #endif // WITH_SPGW
-
-            if (calc_gtp_hash) {
-                fabric_md.ecmp_hash = gtp_flow_hasher.get(to_hash);
-            }
-
+        // if an outer GTP header exists, use it to perform GTP-aware ECMP
+        if (hdr.gtpu.isValid()) {
+            fabric_md.ecmp_hash = gtp_flow_hasher.get({
+                hdr.ipv4.src_addr,
+                hdr.ipv4.dst_addr,
+                hdr.gtpu.teid
+            });
+        } else if (fabric_md.lkp.is_ipv4) {
+            // use inner hash by default
+            fabric_md.ecmp_hash = fabric_md.bridged.base.inner_hash;
         }
         // FIXME: remove ipv6 support or test it
         //  https://github.com/stratum/fabric-tna/pull/227
@@ -66,7 +57,10 @@ control Hasher(
         // }
         else {
             // Not an IP packet
-            fabric_md.bridged.base.inner_hash = non_ip_hasher.get({
+            // Set inner hash to zero since we will never process this packet through
+            // the INT pipeline.
+            fabric_md.bridged.base.inner_hash = 0;
+            fabric_md.ecmp_hash = non_ip_hasher.get({
                 hdr.ethernet.dst_addr,
                 hdr.ethernet.src_addr,
                 hdr.eth_type.value

p4src/include/control/int.p4

@@ -163,12 +163,12 @@ control IntIngress (
 
     table watchlist {
         key = {
-            fabric_md.acl_lkp.is_ipv4  : exact   @name("ipv4_valid");
-            fabric_md.acl_lkp.ipv4_src : ternary @name("ipv4_src");
-            fabric_md.acl_lkp.ipv4_dst : ternary @name("ipv4_dst");
-            fabric_md.acl_lkp.ip_proto : ternary @name("ip_proto");
-            fabric_md.acl_lkp.l4_sport : range   @name("l4_sport");
-            fabric_md.acl_lkp.l4_dport : range   @name("l4_dport");
+            fabric_md.lkp.is_ipv4  : exact   @name("ipv4_valid");
+            fabric_md.lkp.ipv4_src : ternary @name("ipv4_src");
+            fabric_md.lkp.ipv4_dst : ternary @name("ipv4_dst");
+            fabric_md.lkp.ip_proto : ternary @name("ip_proto");
+            fabric_md.lkp.l4_sport : range   @name("l4_sport");
+            fabric_md.lkp.l4_dport : range   @name("l4_dport");
         }
         actions = {
             mark_to_report;

p4src/include/control/lookup_md_init.p4

@@ -0,0 +1,66 @@
+// Copyright 2020-present Open Networking Foundation
+// SPDX-License-Identifier: LicenseRef-ONF-Member-Only-1.0
+
+#include <core.p4>
+#include <tna.p4>
+
+#include "../define.p4"
+#include "../header.p4"
+
+control LookupMdInit (in parsed_headers_t hdr,
+                      out lookup_metadata_t lkp_md) {
+
+      apply {
+        // we don't check if Ethernet and eth_type is valid,
+        // because it is always extracted in the Parser.
+        lkp_md.eth_dst = hdr.ethernet.dst_addr;
+        lkp_md.eth_src = hdr.ethernet.src_addr;
+        lkp_md.eth_type = hdr.eth_type.value;
+
+        lkp_md.vlan_id = 0;
+        if (hdr.vlan_tag.isValid()) {
+            lkp_md.vlan_id = hdr.vlan_tag.vlan_id;
+        }
+
+        lkp_md.is_ipv4 = false;
+        lkp_md.ipv4_src = 0;
+        lkp_md.ipv4_dst = 0;
+        lkp_md.ip_proto = 0;
+        lkp_md.l4_sport = 0;
+        lkp_md.l4_dport = 0;
+        lkp_md.icmp_type = 0;
+        lkp_md.icmp_code = 0;
+        if (hdr.inner_ipv4.isValid()) {
+            lkp_md.is_ipv4 = true;
+            lkp_md.ipv4_src = hdr.inner_ipv4.src_addr;
+            lkp_md.ipv4_dst = hdr.inner_ipv4.dst_addr;
+            lkp_md.ip_proto = hdr.inner_ipv4.protocol;
+            if (hdr.inner_tcp.isValid()) {
+                lkp_md.l4_sport = hdr.inner_tcp.sport;
+                lkp_md.l4_dport = hdr.inner_tcp.dport;
+            } else if (hdr.inner_udp.isValid()) {
+                lkp_md.l4_sport = hdr.inner_udp.sport;
+                lkp_md.l4_dport = hdr.inner_udp.dport;
+            } else if (hdr.inner_icmp.isValid()) {
+                lkp_md.icmp_type = hdr.inner_icmp.icmp_type;
+                lkp_md.icmp_code = hdr.inner_icmp.icmp_code;
+            }
+        } else if (hdr.ipv4.isValid()) {
+            lkp_md.is_ipv4 = true;
+            lkp_md.ipv4_src = hdr.ipv4.src_addr;
+            lkp_md.ipv4_dst = hdr.ipv4.dst_addr;
+            lkp_md.ip_proto = hdr.ipv4.protocol;
+            if (hdr.tcp.isValid()) {
+                lkp_md.l4_sport = hdr.tcp.sport;
+                lkp_md.l4_dport = hdr.tcp.dport;
+            } else if (hdr.udp.isValid()) {
+                lkp_md.l4_sport = hdr.udp.sport;
+                lkp_md.l4_dport = hdr.udp.dport;
+            } else if (hdr.icmp.isValid()) {
+                lkp_md.icmp_type = hdr.icmp.icmp_type;
+                lkp_md.icmp_code = hdr.icmp.icmp_code;
+          }
+        }
+      }
+
+}

p4src/include/control/spgw.p4

@@ -104,8 +104,8 @@ control UplinkRecirc(
 
     table rules {
         key = {
-            fabric_md.acl_lkp.ipv4_src : ternary @name("ipv4_src");
-            fabric_md.acl_lkp.ipv4_dst : ternary @name("ipv4_dst");
+            fabric_md.lkp.ipv4_src : ternary @name("ipv4_src");
+            fabric_md.lkp.ipv4_dst : ternary @name("ipv4_dst");
         }
         actions = {
             allow;
@@ -339,7 +339,7 @@ control SpgwIngress(
                     decap_gtpu_from_dbuf.apply(hdr, fabric_md);
                 }
                 // PDRs
-                if (fabric_md.spgw.src_iface == SpgwInterface.ACCESS) {
+                if (fabric_md.spgw.src_iface == SpgwInterface.ACCESS && hdr.gtpu.isValid()) {
                     uplink_pdrs.apply();
                 } else if (fabric_md.spgw.src_iface == SpgwInterface.CORE ||
                             fabric_md.spgw.src_iface == SpgwInterface.FROM_DBUF) {

p4src/include/header.p4

@@ -281,19 +281,25 @@ header bridged_metadata_t {
 #endif // WITH_INT
 }
 
-// Used for ACL lookups, INT watchlist, and stats tables. Initialized with the
-// parsed headers, but never updated by the pipe. When both outer and inner
-// IPv4/TCP/UDP headers are valid, this should always carry the inner ones. The
-// assumption is that we terminate GTP tunnels in the fabric, so we are more
-// interested in observing/blocking the inner flows. We might revisit this
-// decision in the future.
-struct acl_lookup_t {
-    bool      is_ipv4;
-    bit<32>   ipv4_src;
-    bit<32>   ipv4_dst;
-    bit<8>    ip_proto;
-    l4_port_t l4_sport;
-    l4_port_t l4_dport;
+// Used for table lookup. Initialized with the parsed headers, or 0 if invalid
+// to avoid unexpected match behavior due to PHV overlay. Never updated by the
+// pipe. When both outer and inner IPv4 headers are valid, this should always
+// carry the inner ones. The assumption is that we terminate GTP tunnels in the
+// fabric, so we are more interested in observing/blocking the inner flows. We
+// might revisit this decision in the future.
+struct lookup_metadata_t {
+    mac_addr_t              eth_dst;
+    mac_addr_t              eth_src;
+    bit<16>                 eth_type;
+    vlan_id_t               vlan_id;
+    bool                    is_ipv4;
+    bit<32>                 ipv4_src;
+    bit<32>                 ipv4_dst;
+    bit<8>                  ip_proto;
+    l4_port_t               l4_sport;
+    l4_port_t               l4_dport;
+    bit<8>                  icmp_type;
+    bit<8>                  icmp_code;
 }
 
 // Ingress pipeline-only metadata
@@ -302,7 +308,7 @@ struct acl_lookup_t {
 struct fabric_ingress_metadata_t {
     bridged_metadata_t      bridged;
     flow_hash_t             ecmp_hash;
-    acl_lookup_t            acl_lkp;
+    lookup_metadata_t       lkp;
     bit<32>                 routing_ipv4_dst; // Outermost
     bool                    skip_forwarding;
     bool                    skip_next;