Fix flawed code in up4.p4 and the pins switch models. (#3857)

p4lang · Jan 26, 2023 · 1f34073 · 1f34073
1 parent 122538b
commit 1f34073
Show file tree

Hide file tree

Showing 31 changed files with 4,471 additions and 139 deletions.
diff --git a/backends/bmv2/CMakeLists.txt b/backends/bmv2/CMakeLists.txt
@@ -134,6 +134,7 @@ set (BMV2_V1MODEL_TEST_SUITES
   "${P4C_SOURCE_DIR}/testdata/p4_16_bmv_errors/*-bmv2.p4"
   "${P4C_SOURCE_DIR}/testdata/p4_16_samples/fabric_*/fabric.p4"
   "${P4C_SOURCE_DIR}/testdata/p4_16_samples/pins/*.p4"
+  "${P4C_SOURCE_DIR}/testdata/p4_16_samples/omec/*.p4"
   "${P4C_SOURCE_DIR}/testdata/p4_14_samples/switch_*/switch.p4"
   "${P4C_SOURCE_DIR}/testdata/p4_14_samples/*.p4"
   ${v1tests}

diff --git a/backends/p4test/CMakeLists.txt b/backends/p4test/CMakeLists.txt
@@ -63,6 +63,7 @@ set (P4TEST_SUITES
   "${P4C_SOURCE_DIR}/testdata/p4_16_samples/*.p4"
   "${P4C_SOURCE_DIR}/testdata/p4_16_samples/fabric_*/fabric.p4"
   "${P4C_SOURCE_DIR}/testdata/p4_16_samples/pins/*.p4"
+  "${P4C_SOURCE_DIR}/testdata/p4_16_samples/omec/*.p4"
   )
 
 # Builds a list of tests for which P4Info generation is not supported. It makes

diff --git a/backends/p4tools/benchmarks/test_coverage.py b/backends/p4tools/benchmarks/test_coverage.py
@@ -20,6 +20,7 @@
 # generate random seeds, increase the number for extra sampling
 ITERATIONS = 1
 MAX_TESTS = 0
+TEST_BACKEND = "PROTOBUF"
 
 PARSER = argparse.ArgumentParser()
 PARSER.add_argument(
@@ -67,6 +68,14 @@
     default=TESTGEN_BIN,
     help="Specifies the testgen binary.",
 )
+PARSER.add_argument(
+    "-t",
+    "--test-backend",
+    dest="test_backend",
+    default=TEST_BACKEND,
+    type=str,
+    help="Which test back end to generate tests for.",
+)
 
 
 class Options:
@@ -76,6 +85,7 @@ def __init__(self):
         self.out_dir = None  # The output directory.
         self.seed = None  # Program seed.
         self.max_tests = None  # The max tests parameter.
+        self.test_backend = None  # The test back end to generate tests for.
 
 
 class TestArgs:
@@ -121,7 +131,7 @@ def run_strategies_for_max_tests(data_row, options, test_args):
 
     cmd = (
         f"{options.p4testgen_bin} --target bmv2 --arch v1model --std p4-16"
-        f" -I/p4/p4c/build/p4include --test-backend PROTOBUF --seed {test_args.seed} "
+        f" -I/p4/p4c/build/p4include --test-backend {options.test_backend} --seed {test_args.seed} "
         f"--max-tests {options.max_tests}  --out-dir {test_args.test_dir}"
         f" --exploration-strategy {test_args.strategy} --stop-metric MAX_STATEMENT_COVERAGE "
         f" {test_args.extra_args} {options.p4_program}"
@@ -166,6 +176,7 @@ def main(args):
     options.out_dir = Path(args.out_dir).absolute()
     options.seed = args.seed
     options.p4testgen_bin = Path(testutils.check_if_file(args.p4testgen_bin))
+    options.test_backend = args.test_backend.upper()
 
     # 7189 is an example of a good seed, which gets cov 1 with less than 100 tests
     # in random access stack.
@@ -205,7 +216,7 @@ def main(args):
                 )
                 test_args = TestArgs()
                 test_args.seed = seed
-                test_args.test_dir = Path(tempfile.mkdtemp(dir=test_args.test_dir))
+                test_args.test_dir = Path(tempfile.mkdtemp(dir=test_dir))
                 test_args.strategy = strategy
                 test_args.extra_args = config[strategy]
                 data_row = run_strategies_for_max_tests(data_row, options, test_args)

diff --git a/backends/p4tools/modules/testgen/targets/bmv2/test/BMV2PTFXfail.cmake b/backends/p4tools/modules/testgen/targets/bmv2/test/BMV2PTFXfail.cmake
@@ -261,8 +261,6 @@ p4tools_add_xfail_reason(
   pins_middleblock.p4
   issue2283_1-bmv2.p4
 
-  # At index 0: INVALID_ARGUMENT, 'Unexpected number of action parameters'
-  up4.p4
   # At index 0: INVALID_ARGUMENT, '0 is not a valid session id'
   issue1642-bmv2.p4
   issue1653-bmv2.p4

diff --git a/backends/p4tools/modules/testgen/targets/bmv2/test/BMV2ProtobufXfail.cmake b/backends/p4tools/modules/testgen/targets/bmv2/test/BMV2ProtobufXfail.cmake
@@ -134,5 +134,4 @@ p4tools_add_xfail_reason(
   issue2345-multiple_dependencies.p4
   issue2345-with_nested_if.p4
   issue2345.p4
-  up4.p4
 )
diff --git a/backends/p4tools/modules/testgen/targets/bmv2/test/BMV2Xfail.cmake b/backends/p4tools/modules/testgen/targets/bmv2/test/BMV2Xfail.cmake
@@ -84,6 +84,7 @@ p4tools_add_xfail_reason(
 p4tools_add_xfail_reason(
   "testgen-p4c-bmv2"
   "Match type range not implemented for table keys"
+  up4.p4
 )
 
 p4tools_add_xfail_reason(
@@ -264,5 +265,4 @@ p4tools_add_xfail_reason(
   issue2345-multiple_dependencies.p4
   issue2345-with_nested_if.p4
   issue2345.p4
-  up4.p4
 )
diff --git a/backends/p4tools/modules/testgen/targets/bmv2/test/P4Tests.cmake b/backends/p4tools/modules/testgen/targets/bmv2/test/P4Tests.cmake
@@ -29,6 +29,7 @@ set(
   TESTGEN_BMV2_P416_TESTS
   "${CMAKE_CURRENT_LIST_DIR}/p4-programs/*.p4"
   "${P4C_SOURCE_DIR}/testdata/p4_16_samples/pins/*.p4"
+  "${P4C_SOURCE_DIR}/testdata/p4_16_samples/omec/*.p4"
   "${P4C_SOURCE_DIR}/testdata/p4_16_samples/fabric_*/fabric.p4"
 )
 

diff --git a/testdata/p4_16_samples/omec/README.md b/testdata/p4_16_samples/omec/README.md
@@ -0,0 +1,3 @@
+# OMEC P4 Programs
+The P4 programs contained here model a virtual User Plane Function (UPF) pipeline as part of the SD-Fabric project. up4.p4 is a One-Big-UPF abstraction, a program that doesn't run on switches, but is used as the schema to define the content of the P4Runtime messages that can be exchanged with the UP4 app.
+These P4 programs define . They are sourced from https://github.com/omec-project/up4. For more information, please see https://docs.sd-fabric.org/master/advanced/p4-upf.html.
diff --git a/...tgen/targets/bmv2/test/p4-programs/up4.p4 → testdata/p4_16_samples/omec/up4.p4 b/...tgen/targets/bmv2/test/p4-programs/up4.p4 → testdata/p4_16_samples/omec/up4.p4
@@ -397,7 +397,9 @@ control Acl(inout parsed_headers_t hdr, inout local_metadata_t local_meta, inout
         @name("acls") counters = direct_counter(CounterType.packets_and_bytes);
     }
     apply {
-        acls.apply();
+        if (hdr.ethernet.isValid() && hdr.ipv4.isValid()) {
+            acls.apply();
+        }
     }
 }
 
@@ -425,6 +427,9 @@ control Routing(inout parsed_headers_t hdr, inout local_metadata_t local_meta, i
         size = 1024;
     }
     apply {
+        if (!hdr.ipv4.isValid()) {
+            return;
+        }
         hdr.ipv4.ttl = hdr.ipv4.ttl - 1;
         if (hdr.ipv4.ttl == 0) {
             drop();

diff --git a/testdata/p4_16_samples/pins/pins_fabric.p4 b/testdata/p4_16_samples/pins/pins_fabric.p4
@@ -96,15 +96,15 @@ enum bit<8> PreservedFieldList {
     CLONE_I2E = 8w1
 }
 
-@p4runtime_translation("" , string) type bit<10> nexthop_id_t;
-@p4runtime_translation("" , string) type bit<10> tunnel_id_t;
-@p4runtime_translation("" , string) type bit<12> wcmp_group_id_t;
-@p4runtime_translation("" , string) @p4runtime_translation_mappings({ { "" , 0 } , }) type bit<10> vrf_id_t;
+type bit<10> nexthop_id_t;
+type bit<10> tunnel_id_t;
+type bit<12> wcmp_group_id_t;
+type bit<10> vrf_id_t;
 const vrf_id_t kDefaultVrf = 0;
-@p4runtime_translation("" , string) type bit<10> router_interface_id_t;
-@p4runtime_translation("" , string) type bit<9> port_id_t;
-@p4runtime_translation("" , string) type bit<10> mirror_session_id_t;
-@p4runtime_translation("" , string) type bit<8> qos_queue_t;
+type bit<10> router_interface_id_t;
+type bit<9> port_id_t;
+type bit<10> mirror_session_id_t;
+type bit<8> qos_queue_t;
 typedef bit<6> route_metadata_t;
 enum bit<2> MeterColor_t {
     GREEN = 0,

diff --git a/testdata/p4_16_samples/pins/pins_middleblock.p4 b/testdata/p4_16_samples/pins/pins_middleblock.p4
@@ -96,15 +96,15 @@ enum bit<8> PreservedFieldList {
     CLONE_I2E = 8w1
 }
 
-@p4runtime_translation("" , string) type bit<10> nexthop_id_t;
-@p4runtime_translation("" , string) type bit<10> tunnel_id_t;
-@p4runtime_translation("" , string) type bit<12> wcmp_group_id_t;
-@p4runtime_translation("" , string) @p4runtime_translation_mappings({ { "" , 0 } , }) type bit<10> vrf_id_t;
+type bit<10> nexthop_id_t;
+type bit<10> tunnel_id_t;
+type bit<12> wcmp_group_id_t;
+type bit<10> vrf_id_t;
 const vrf_id_t kDefaultVrf = 0;
-@p4runtime_translation("" , string) type bit<10> router_interface_id_t;
-@p4runtime_translation("" , string) type bit<9> port_id_t;
-@p4runtime_translation("" , string) type bit<10> mirror_session_id_t;
-@p4runtime_translation("" , string) type bit<8> qos_queue_t;
+type bit<10> router_interface_id_t;
+type bit<9> port_id_t;
+type bit<10> mirror_session_id_t;
+type bit<8> qos_queue_t;
 typedef bit<6> route_metadata_t;
 enum bit<2> MeterColor_t {
     GREEN = 0,

diff --git a/testdata/p4_16_samples/pins/pins_wbb.p4 b/testdata/p4_16_samples/pins/pins_wbb.p4
@@ -96,15 +96,15 @@ enum bit<8> PreservedFieldList {
     CLONE_I2E = 8w1
 }
 
-@p4runtime_translation("" , string) type bit<10> nexthop_id_t;
-@p4runtime_translation("" , string) type bit<10> tunnel_id_t;
-@p4runtime_translation("" , string) type bit<12> wcmp_group_id_t;
-@p4runtime_translation("" , string) @p4runtime_translation_mappings({ { "" , 0 } , }) type bit<10> vrf_id_t;
+type bit<10> nexthop_id_t;
+type bit<10> tunnel_id_t;
+type bit<12> wcmp_group_id_t;
+type bit<10> vrf_id_t;
 const vrf_id_t kDefaultVrf = 0;
-@p4runtime_translation("" , string) type bit<10> router_interface_id_t;
-@p4runtime_translation("" , string) type bit<9> port_id_t;
-@p4runtime_translation("" , string) type bit<10> mirror_session_id_t;
-@p4runtime_translation("" , string) type bit<8> qos_queue_t;
+type bit<10> router_interface_id_t;
+type bit<9> port_id_t;
+type bit<10> mirror_session_id_t;
+type bit<8> qos_queue_t;
 typedef bit<6> route_metadata_t;
 enum bit<2> MeterColor_t {
     GREEN = 0,