From cc3a4a0a9a1d04463088edf4b54bbb1c0cffadfe Mon Sep 17 00:00:00 2001 From: Jonathan DiLorenzo Date: Fri, 31 May 2024 12:48:18 -0400 Subject: [PATCH] Add support for new platform property annotations for P4Runtime. (#4611) * Add tentative support for new platform property annotations. * Format fix. * Add a test for the platform property annotation. * Fix minor bug causing double processing of platform_property annotation. * Update annotations to actually change new proto fields. * Adding back . * Add -f to mv command * Now trying -n * Strip prefix is now supposed to be fixed, so trying that. * Address comments. --- CMakeLists.txt | 2 +- bazel/p4c_deps.bzl | 9 +-- control-plane/p4RuntimeAnnotations.h | 1 + control-plane/p4RuntimeSerializer.cpp | 36 ++++++++++- testdata/p4_16_samples/pkginfo_annotation.p4 | 61 +++++++++++++++++++ .../pkginfo_annotation-first.p4 | 42 +++++++++++++ .../pkginfo_annotation-frontend.p4 | 42 +++++++++++++ .../pkginfo_annotation-midend.p4 | 42 +++++++++++++ .../pkginfo_annotation.p4 | 42 +++++++++++++ .../pkginfo_annotation.p4-stderr | 0 .../pkginfo_annotation.p4.entries.txtpb | 3 + .../pkginfo_annotation.p4.p4info.txtpb | 16 +++++ 12 files changed, 288 insertions(+), 8 deletions(-) create mode 100644 testdata/p4_16_samples/pkginfo_annotation.p4 create mode 100644 testdata/p4_16_samples_outputs/pkginfo_annotation-first.p4 create mode 100644 testdata/p4_16_samples_outputs/pkginfo_annotation-frontend.p4 create mode 100644 testdata/p4_16_samples_outputs/pkginfo_annotation-midend.p4 create mode 100644 testdata/p4_16_samples_outputs/pkginfo_annotation.p4 create mode 100644 testdata/p4_16_samples_outputs/pkginfo_annotation.p4-stderr create mode 100644 testdata/p4_16_samples_outputs/pkginfo_annotation.p4.entries.txtpb create mode 100644 testdata/p4_16_samples_outputs/pkginfo_annotation.p4.p4info.txtpb diff --git a/CMakeLists.txt b/CMakeLists.txt index b97c1f1101..02edc3d13a 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -419,7 +419,7 @@ set(FETCHCONTENT_QUIET OFF) FetchContent_Declare( p4runtime GIT_REPOSITORY https://github.com/p4lang/p4runtime.git - GIT_TAG 1e771c4e05c4e7e250df00212b3ca02ee3202d71 + GIT_TAG 62a9bd60599b87497a15feb6c7893b7ec8ba461f GIT_PROGRESS TRUE ) FetchContent_MakeAvailable(p4runtime) diff --git a/bazel/p4c_deps.bzl b/bazel/p4c_deps.bzl index 5388d0fb81..3a47922266 100644 --- a/bazel/p4c_deps.bzl +++ b/bazel/p4c_deps.bzl @@ -47,13 +47,10 @@ filegroup( git_repository( name = "com_github_p4lang_p4runtime", remote = "https://github.com/p4lang/p4runtime", - # Newest commit on main branch as of August 11, 2023. - commit = "1e771c4e05c4e7e250df00212b3ca02ee3202d71", + # Newest commit on main branch as of May 30, 2024. + commit = "62a9bd60599b87497a15feb6c7893b7ec8ba461f", shallow_since = "1680213111 -0700", - # strip_prefix is broken; we use patch_cmds as a workaround, - # see https://github.com/bazelbuild/bazel/issues/10062. - # strip_prefix = "proto", - patch_cmds = ["mv proto/* ."], + strip_prefix = "proto", ) if not native.existing_rule("com_google_googletest"): # Cannot currently use local_repository due to Bazel limitation, diff --git a/control-plane/p4RuntimeAnnotations.h b/control-plane/p4RuntimeAnnotations.h index c31a5e93dd..1a1e223b2f 100644 --- a/control-plane/p4RuntimeAnnotations.h +++ b/control-plane/p4RuntimeAnnotations.h @@ -36,6 +36,7 @@ class ParseP4RuntimeAnnotations : public ParseAnnotations { PARSE("id", Constant), PARSE("brief", StringLiteral), PARSE("description", StringLiteral), + PARSE_KV_LIST("platform_property"), // These annotations are architecture-specific in theory, but // given that they are "reserved" by the P4Runtime // specification, I don't really have any qualms about adding diff --git a/control-plane/p4RuntimeSerializer.cpp b/control-plane/p4RuntimeSerializer.cpp index 251a39150f..bf6d264521 100644 --- a/control-plane/p4RuntimeSerializer.cpp +++ b/control-plane/p4RuntimeSerializer.cpp @@ -951,9 +951,43 @@ class P4RuntimeAnalyzer { } } + // Parse `@platform_property` annotation into the PkgInfo. + for (auto *annotation : decl->getAnnotations()->annotations) { + if (annotation->name != "platform_property") continue; + auto *platform_properties = pkginfo->mutable_platform_properties(); + for (auto *kv : annotation->kv) { + auto name = kv->name.name; + auto setInt32Field = [kv, &platform_properties](cstring fName) { + auto *v = kv->expression->to(); + if (v == nullptr) { + ::error(ErrorType::ERR_UNSUPPORTED, + "Value for '%1%' key in @platform_property annotation is not an " + "integer", + kv); + return; + } + // use Protobuf reflection library to minimize code duplication. + auto *descriptor = platform_properties->GetDescriptor(); + auto *f = descriptor->FindFieldByName(static_cast(fName)); + platform_properties->GetReflection()->SetInt32(platform_properties, f, + static_cast(v->value)); + }; + if (name == "multicast_group_table_size" || + name == "multicast_group_table_total_replicas" || + name == "multicast_group_table_max_replicas_per_entry") { + setInt32Field(name); + } else { + ::warning(ErrorType::WARN_UNKNOWN, + "Unknown key name '%1%' in @platform_property annotation", name); + } + } + } + // add other annotations on the P4 package to the message. @pkginfo is // ignored using the unary predicate argument to addAnnotations. - addAnnotations(pkginfo, decl, [](cstring name) { return name == "pkginfo"; }); + addAnnotations(pkginfo, decl, [](cstring name) { + return name == "pkginfo" || name == "platform_property"; + }); addDocumentation(pkginfo, decl); } diff --git a/testdata/p4_16_samples/pkginfo_annotation.p4 b/testdata/p4_16_samples/pkginfo_annotation.p4 new file mode 100644 index 0000000000..6430e14230 --- /dev/null +++ b/testdata/p4_16_samples/pkginfo_annotation.p4 @@ -0,0 +1,61 @@ +/* +Copyright 2019-present Barefoot Networks, Inc. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +#include +#include + +struct H { }; +struct M { }; + +parser ParserI(packet_in pk, out H hdr, inout M meta, inout standard_metadata_t smeta) { + state start { transition accept; } +} + +control IngressI(inout H hdr, inout M meta, inout standard_metadata_t smeta) { + apply { } +}; + +control EgressI(inout H hdr, inout M meta, inout standard_metadata_t smeta) { + apply { } +}; + +control DeparserI(packet_out pk, in H hdr) { + apply { } +} + +control VerifyChecksumI(inout H hdr, inout M meta) { + apply { } +} + +control ComputeChecksumI(inout H hdr, inout M meta) { + apply { } +} + + +@pkginfo( + name = "some_name", + organization = "some_org", + version = "some_version", + contact = "some_contact", + url = "some_url" +) +@platform_property( + multicast_group_table_size = 10, + multicast_group_table_total_replicas = 20, + multicast_group_table_max_replicas_per_entry = 30 +) +V1Switch(ParserI(), VerifyChecksumI(), IngressI(), EgressI(), + ComputeChecksumI(), DeparserI()) main; diff --git a/testdata/p4_16_samples_outputs/pkginfo_annotation-first.p4 b/testdata/p4_16_samples_outputs/pkginfo_annotation-first.p4 new file mode 100644 index 0000000000..67af340954 --- /dev/null +++ b/testdata/p4_16_samples_outputs/pkginfo_annotation-first.p4 @@ -0,0 +1,42 @@ +#include +#define V1MODEL_VERSION 20180101 +#include + +struct H { +} + +struct M { +} + +parser ParserI(packet_in pk, out H hdr, inout M meta, inout standard_metadata_t smeta) { + state start { + transition accept; + } +} + +control IngressI(inout H hdr, inout M meta, inout standard_metadata_t smeta) { + apply { + } +} + +control EgressI(inout H hdr, inout M meta, inout standard_metadata_t smeta) { + apply { + } +} + +control DeparserI(packet_out pk, in H hdr) { + apply { + } +} + +control VerifyChecksumI(inout H hdr, inout M meta) { + apply { + } +} + +control ComputeChecksumI(inout H hdr, inout M meta) { + apply { + } +} + +@pkginfo(name="some_name", organization="some_org", version="some_version", contact="some_contact", url="some_url") @platform_property(multicast_group_table_size = 10 , multicast_group_table_total_replicas = 20 , multicast_group_table_max_replicas_per_entry = 30) V1Switch(ParserI(), VerifyChecksumI(), IngressI(), EgressI(), ComputeChecksumI(), DeparserI()) main; diff --git a/testdata/p4_16_samples_outputs/pkginfo_annotation-frontend.p4 b/testdata/p4_16_samples_outputs/pkginfo_annotation-frontend.p4 new file mode 100644 index 0000000000..67af340954 --- /dev/null +++ b/testdata/p4_16_samples_outputs/pkginfo_annotation-frontend.p4 @@ -0,0 +1,42 @@ +#include +#define V1MODEL_VERSION 20180101 +#include + +struct H { +} + +struct M { +} + +parser ParserI(packet_in pk, out H hdr, inout M meta, inout standard_metadata_t smeta) { + state start { + transition accept; + } +} + +control IngressI(inout H hdr, inout M meta, inout standard_metadata_t smeta) { + apply { + } +} + +control EgressI(inout H hdr, inout M meta, inout standard_metadata_t smeta) { + apply { + } +} + +control DeparserI(packet_out pk, in H hdr) { + apply { + } +} + +control VerifyChecksumI(inout H hdr, inout M meta) { + apply { + } +} + +control ComputeChecksumI(inout H hdr, inout M meta) { + apply { + } +} + +@pkginfo(name="some_name", organization="some_org", version="some_version", contact="some_contact", url="some_url") @platform_property(multicast_group_table_size = 10 , multicast_group_table_total_replicas = 20 , multicast_group_table_max_replicas_per_entry = 30) V1Switch(ParserI(), VerifyChecksumI(), IngressI(), EgressI(), ComputeChecksumI(), DeparserI()) main; diff --git a/testdata/p4_16_samples_outputs/pkginfo_annotation-midend.p4 b/testdata/p4_16_samples_outputs/pkginfo_annotation-midend.p4 new file mode 100644 index 0000000000..67af340954 --- /dev/null +++ b/testdata/p4_16_samples_outputs/pkginfo_annotation-midend.p4 @@ -0,0 +1,42 @@ +#include +#define V1MODEL_VERSION 20180101 +#include + +struct H { +} + +struct M { +} + +parser ParserI(packet_in pk, out H hdr, inout M meta, inout standard_metadata_t smeta) { + state start { + transition accept; + } +} + +control IngressI(inout H hdr, inout M meta, inout standard_metadata_t smeta) { + apply { + } +} + +control EgressI(inout H hdr, inout M meta, inout standard_metadata_t smeta) { + apply { + } +} + +control DeparserI(packet_out pk, in H hdr) { + apply { + } +} + +control VerifyChecksumI(inout H hdr, inout M meta) { + apply { + } +} + +control ComputeChecksumI(inout H hdr, inout M meta) { + apply { + } +} + +@pkginfo(name="some_name", organization="some_org", version="some_version", contact="some_contact", url="some_url") @platform_property(multicast_group_table_size = 10 , multicast_group_table_total_replicas = 20 , multicast_group_table_max_replicas_per_entry = 30) V1Switch(ParserI(), VerifyChecksumI(), IngressI(), EgressI(), ComputeChecksumI(), DeparserI()) main; diff --git a/testdata/p4_16_samples_outputs/pkginfo_annotation.p4 b/testdata/p4_16_samples_outputs/pkginfo_annotation.p4 new file mode 100644 index 0000000000..7cf3fe2b51 --- /dev/null +++ b/testdata/p4_16_samples_outputs/pkginfo_annotation.p4 @@ -0,0 +1,42 @@ +#include +#define V1MODEL_VERSION 20180101 +#include + +struct H { +} + +struct M { +} + +parser ParserI(packet_in pk, out H hdr, inout M meta, inout standard_metadata_t smeta) { + state start { + transition accept; + } +} + +control IngressI(inout H hdr, inout M meta, inout standard_metadata_t smeta) { + apply { + } +} + +control EgressI(inout H hdr, inout M meta, inout standard_metadata_t smeta) { + apply { + } +} + +control DeparserI(packet_out pk, in H hdr) { + apply { + } +} + +control VerifyChecksumI(inout H hdr, inout M meta) { + apply { + } +} + +control ComputeChecksumI(inout H hdr, inout M meta) { + apply { + } +} + +@pkginfo(name="some_name", organization="some_org", version="some_version", contact="some_contact", url="some_url") @platform_property(multicast_group_table_size = 10 , multicast_group_table_total_replicas = 20 , multicast_group_table_max_replicas_per_entry = 30) V1Switch(ParserI(), VerifyChecksumI(), IngressI(), EgressI(), ComputeChecksumI(), DeparserI()) main; diff --git a/testdata/p4_16_samples_outputs/pkginfo_annotation.p4-stderr b/testdata/p4_16_samples_outputs/pkginfo_annotation.p4-stderr new file mode 100644 index 0000000000..e69de29bb2 diff --git a/testdata/p4_16_samples_outputs/pkginfo_annotation.p4.entries.txtpb b/testdata/p4_16_samples_outputs/pkginfo_annotation.p4.entries.txtpb new file mode 100644 index 0000000000..5cb9652623 --- /dev/null +++ b/testdata/p4_16_samples_outputs/pkginfo_annotation.p4.entries.txtpb @@ -0,0 +1,3 @@ +# proto-file: p4/v1/p4runtime.proto +# proto-message: p4.v1.WriteRequest + diff --git a/testdata/p4_16_samples_outputs/pkginfo_annotation.p4.p4info.txtpb b/testdata/p4_16_samples_outputs/pkginfo_annotation.p4.p4info.txtpb new file mode 100644 index 0000000000..2af5fb7f8b --- /dev/null +++ b/testdata/p4_16_samples_outputs/pkginfo_annotation.p4.p4info.txtpb @@ -0,0 +1,16 @@ +# proto-file: p4/config/v1/p4info.proto +# proto-message: p4.config.v1.P4Info + +pkg_info { + name: "some_name" + version: "some_version" + arch: "v1model" + organization: "some_org" + contact: "some_contact" + url: "some_url" + platform_properties { + multicast_group_table_size: 10 + multicast_group_table_total_replicas: 20 + multicast_group_table_max_replicas_per_entry: 30 + } +}