From 75aaf8281de9f03579be7c071cd00c664d696cfc Mon Sep 17 00:00:00 2001 From: Jonathan DiLorenzo Date: Fri, 12 Apr 2024 02:52:10 +0000 Subject: [PATCH 01/10] Add tentative support for new platform property annotations. --- control-plane/p4RuntimeAnnotations.h | 1 + control-plane/p4RuntimeSerializer.cpp | 33 +++++++++++++++++++++++++++ 2 files changed, 34 insertions(+) diff --git a/control-plane/p4RuntimeAnnotations.h b/control-plane/p4RuntimeAnnotations.h index c31a5e93dd3..1a1e223b2f9 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 251a39150f4..19990111a38 100644 --- a/control-plane/p4RuntimeSerializer.cpp +++ b/control-plane/p4RuntimeSerializer.cpp @@ -950,6 +950,39 @@ class P4RuntimeAnalyzer { } } } + + // @platform_property annotation + for (auto *annotation : decl->getAnnotations()->annotations) { + if (annotation->name != "platform_property") continue; + // TODO(dilo): Add once supported by P4Runtime protobuf. + // auto *platform_property = pkginfo->mutable_platform_property(); + for (auto *kv : annotation->kv) { + auto name = kv->name.name; + auto setInt64Field = [kv](cstring fName) { + auto *v = kv->expression->to(); + if (v == nullptr) { + ::error(ErrorType::ERR_UNSUPPORTED, + "Value for '%1%' key in @platform_property annotation is not a integer", kv); + return; + } + // TODO(dilo): Fix below once supported by P4Runtime protobuf. + ::warning(ErrorType::WARN_IGNORE, "Got Int64 field: %1% with value %2%", fName, static_cast(v->value)); + // use Protobuf reflection library to minimize code + // duplication. + // auto *descriptor = platform_property->GetDescriptor(); + // auto *f = descriptor->FindFieldByName(static_cast(fName)); + // platform_property->GetReflection()->SetInt64(platform_property, f, + // static_cast(v->value)); + }; + if (name == "multicast_table_size" || name == "multicast_table_total_replicas" || + name == "multicast_table_max_replicas_per_entry") { + setInt64Field(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. From eccf5a86423ff26a285d8bf68d0abe63c3840f82 Mon Sep 17 00:00:00 2001 From: Jonathan DiLorenzo Date: Fri, 12 Apr 2024 02:53:06 +0000 Subject: [PATCH 02/10] Format fix. --- control-plane/p4RuntimeSerializer.cpp | 15 +++++++++------ 1 file changed, 9 insertions(+), 6 deletions(-) diff --git a/control-plane/p4RuntimeSerializer.cpp b/control-plane/p4RuntimeSerializer.cpp index 19990111a38..4d193f2efa5 100644 --- a/control-plane/p4RuntimeSerializer.cpp +++ b/control-plane/p4RuntimeSerializer.cpp @@ -950,7 +950,7 @@ class P4RuntimeAnalyzer { } } } - + // @platform_property annotation for (auto *annotation : decl->getAnnotations()->annotations) { if (annotation->name != "platform_property") continue; @@ -961,20 +961,23 @@ class P4RuntimeAnalyzer { auto setInt64Field = [kv](cstring fName) { auto *v = kv->expression->to(); if (v == nullptr) { - ::error(ErrorType::ERR_UNSUPPORTED, - "Value for '%1%' key in @platform_property annotation is not a integer", kv); + ::error( + ErrorType::ERR_UNSUPPORTED, + "Value for '%1%' key in @platform_property annotation is not a integer", + kv); return; } // TODO(dilo): Fix below once supported by P4Runtime protobuf. - ::warning(ErrorType::WARN_IGNORE, "Got Int64 field: %1% with value %2%", fName, static_cast(v->value)); + ::warning(ErrorType::WARN_IGNORE, "Got Int64 field: %1% with value %2%", fName, + static_cast(v->value)); // use Protobuf reflection library to minimize code // duplication. // auto *descriptor = platform_property->GetDescriptor(); // auto *f = descriptor->FindFieldByName(static_cast(fName)); // platform_property->GetReflection()->SetInt64(platform_property, f, - // static_cast(v->value)); + // static_cast(v->value)); }; - if (name == "multicast_table_size" || name == "multicast_table_total_replicas" || + if (name == "multicast_table_size" || name == "multicast_table_total_replicas" || name == "multicast_table_max_replicas_per_entry") { setInt64Field(name); } else { From 901d6e02953bd9a2d20230914afb272d1e9fc744 Mon Sep 17 00:00:00 2001 From: Jonathan DiLorenzo Date: Fri, 12 Apr 2024 04:30:28 +0000 Subject: [PATCH 03/10] Add a test for the platform property annotation. --- 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 | 3 + .../pkginfo_annotation.p4.entries.txtpb | 3 + .../pkginfo_annotation.p4.p4info.txtpb | 12 ++++ 8 files changed, 247 insertions(+) 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/testdata/p4_16_samples/pkginfo_annotation.p4 b/testdata/p4_16_samples/pkginfo_annotation.p4 new file mode 100644 index 00000000000..afb93ba7cdd --- /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_table_size = 10, + multicast_table_total_replicas = 20, + multicast_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 00000000000..73695f1d022 --- /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_table_size = 10 , multicast_table_total_replicas = 20 , multicast_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 00000000000..73695f1d022 --- /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_table_size = 10 , multicast_table_total_replicas = 20 , multicast_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 00000000000..73695f1d022 --- /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_table_size = 10 , multicast_table_total_replicas = 20 , multicast_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 00000000000..f84aba0abee --- /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_table_size = 10 , multicast_table_total_replicas = 20 , multicast_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 00000000000..beb96ace49b --- /dev/null +++ b/testdata/p4_16_samples_outputs/pkginfo_annotation.p4-stderr @@ -0,0 +1,3 @@ +[--Wwarn=ignore] warning: Got Int64 field: multicast_table_size with value 10 +[--Wwarn=ignore] warning: Got Int64 field: multicast_table_total_replicas with value 20 +[--Wwarn=ignore] warning: Got Int64 field: multicast_table_max_replicas_per_entry with value 30 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 00000000000..5cb9652623a --- /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 00000000000..d9e81b8e138 --- /dev/null +++ b/testdata/p4_16_samples_outputs/pkginfo_annotation.p4.p4info.txtpb @@ -0,0 +1,12 @@ +# proto-file: p4/config/v1/p4info.proto +# proto-message: p4.config.v1.P4Info + +pkg_info { + name: "some_name" + version: "some_version" + annotations: "@platform_property(multicast_table_size=10, multicast_table_total_replicas=20, multicast_table_max_replicas_per_entry=30)" + arch: "v1model" + organization: "some_org" + contact: "some_contact" + url: "some_url" +} From 57f64d716ce205194852facbff928ec885f5079e Mon Sep 17 00:00:00 2001 From: Jonathan DiLorenzo Date: Fri, 12 Apr 2024 04:38:32 +0000 Subject: [PATCH 04/10] Fix minor bug causing double processing of platform_property annotation. --- control-plane/p4RuntimeSerializer.cpp | 4 +++- .../p4_16_samples_outputs/pkginfo_annotation.p4.p4info.txtpb | 1 - 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/control-plane/p4RuntimeSerializer.cpp b/control-plane/p4RuntimeSerializer.cpp index 4d193f2efa5..bbb8d0bcc2e 100644 --- a/control-plane/p4RuntimeSerializer.cpp +++ b/control-plane/p4RuntimeSerializer.cpp @@ -989,7 +989,9 @@ class P4RuntimeAnalyzer { // 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_outputs/pkginfo_annotation.p4.p4info.txtpb b/testdata/p4_16_samples_outputs/pkginfo_annotation.p4.p4info.txtpb index d9e81b8e138..141f51f50b7 100644 --- a/testdata/p4_16_samples_outputs/pkginfo_annotation.p4.p4info.txtpb +++ b/testdata/p4_16_samples_outputs/pkginfo_annotation.p4.p4info.txtpb @@ -4,7 +4,6 @@ pkg_info { name: "some_name" version: "some_version" - annotations: "@platform_property(multicast_table_size=10, multicast_table_total_replicas=20, multicast_table_max_replicas_per_entry=30)" arch: "v1model" organization: "some_org" contact: "some_contact" From 1424b06bc61d0258a2b930cd2d14ebd2e4386eac Mon Sep 17 00:00:00 2001 From: Jonathan DiLorenzo Date: Thu, 30 May 2024 18:43:58 +0000 Subject: [PATCH 05/10] Update annotations to actually change new proto fields. --- CMakeLists.txt | 2 +- bazel/p4c_deps.bzl | 5 ++-- control-plane/p4RuntimeSerializer.cpp | 23 ++++++++----------- testdata/p4_16_samples/pkginfo_annotation.p4 | 6 ++--- .../pkginfo_annotation-first.p4 | 2 +- .../pkginfo_annotation-frontend.p4 | 2 +- .../pkginfo_annotation-midend.p4 | 2 +- .../pkginfo_annotation.p4 | 2 +- .../pkginfo_annotation.p4-stderr | 3 --- .../pkginfo_annotation.p4.p4info.txtpb | 5 ++++ 10 files changed, 25 insertions(+), 27 deletions(-) diff --git a/CMakeLists.txt b/CMakeLists.txt index d6c6b0d07cd..4f9006d9976 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -416,7 +416,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 5388d0fb81d..42602b5097e 100644 --- a/bazel/p4c_deps.bzl +++ b/bazel/p4c_deps.bzl @@ -47,9 +47,8 @@ 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", - shallow_since = "1680213111 -0700", + # Newest commit on main branch as of May 30, 2024. + commit = "62a9bd60599b87497a15feb6c7893b7ec8ba461f", # strip_prefix is broken; we use patch_cmds as a workaround, # see https://github.com/bazelbuild/bazel/issues/10062. # strip_prefix = "proto", diff --git a/control-plane/p4RuntimeSerializer.cpp b/control-plane/p4RuntimeSerializer.cpp index bbb8d0bcc2e..9a0919a2aab 100644 --- a/control-plane/p4RuntimeSerializer.cpp +++ b/control-plane/p4RuntimeSerializer.cpp @@ -954,11 +954,10 @@ class P4RuntimeAnalyzer { // @platform_property annotation for (auto *annotation : decl->getAnnotations()->annotations) { if (annotation->name != "platform_property") continue; - // TODO(dilo): Add once supported by P4Runtime protobuf. - // auto *platform_property = pkginfo->mutable_platform_property(); + auto *platform_properties = pkginfo->mutable_platform_properties(); for (auto *kv : annotation->kv) { auto name = kv->name.name; - auto setInt64Field = [kv](cstring fName) { + auto setInt32Field = [kv, &platform_properties](cstring fName) { auto *v = kv->expression->to(); if (v == nullptr) { ::error( @@ -967,19 +966,17 @@ class P4RuntimeAnalyzer { kv); return; } - // TODO(dilo): Fix below once supported by P4Runtime protobuf. - ::warning(ErrorType::WARN_IGNORE, "Got Int64 field: %1% with value %2%", fName, - static_cast(v->value)); // use Protobuf reflection library to minimize code // duplication. - // auto *descriptor = platform_property->GetDescriptor(); - // auto *f = descriptor->FindFieldByName(static_cast(fName)); - // platform_property->GetReflection()->SetInt64(platform_property, f, - // static_cast(v->value)); + 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_table_size" || name == "multicast_table_total_replicas" || - name == "multicast_table_max_replicas_per_entry") { - setInt64Field(name); + 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); diff --git a/testdata/p4_16_samples/pkginfo_annotation.p4 b/testdata/p4_16_samples/pkginfo_annotation.p4 index afb93ba7cdd..6430e142302 100644 --- a/testdata/p4_16_samples/pkginfo_annotation.p4 +++ b/testdata/p4_16_samples/pkginfo_annotation.p4 @@ -53,9 +53,9 @@ control ComputeChecksumI(inout H hdr, inout M meta) { url = "some_url" ) @platform_property( - multicast_table_size = 10, - multicast_table_total_replicas = 20, - multicast_table_max_replicas_per_entry = 30 + 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 index 73695f1d022..67af340954b 100644 --- a/testdata/p4_16_samples_outputs/pkginfo_annotation-first.p4 +++ b/testdata/p4_16_samples_outputs/pkginfo_annotation-first.p4 @@ -39,4 +39,4 @@ control ComputeChecksumI(inout H hdr, inout M meta) { } } -@pkginfo(name="some_name", organization="some_org", version="some_version", contact="some_contact", url="some_url") @platform_property(multicast_table_size = 10 , multicast_table_total_replicas = 20 , multicast_table_max_replicas_per_entry = 30) V1Switch(ParserI(), VerifyChecksumI(), IngressI(), EgressI(), ComputeChecksumI(), DeparserI()) main; +@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 index 73695f1d022..67af340954b 100644 --- a/testdata/p4_16_samples_outputs/pkginfo_annotation-frontend.p4 +++ b/testdata/p4_16_samples_outputs/pkginfo_annotation-frontend.p4 @@ -39,4 +39,4 @@ control ComputeChecksumI(inout H hdr, inout M meta) { } } -@pkginfo(name="some_name", organization="some_org", version="some_version", contact="some_contact", url="some_url") @platform_property(multicast_table_size = 10 , multicast_table_total_replicas = 20 , multicast_table_max_replicas_per_entry = 30) V1Switch(ParserI(), VerifyChecksumI(), IngressI(), EgressI(), ComputeChecksumI(), DeparserI()) main; +@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 index 73695f1d022..67af340954b 100644 --- a/testdata/p4_16_samples_outputs/pkginfo_annotation-midend.p4 +++ b/testdata/p4_16_samples_outputs/pkginfo_annotation-midend.p4 @@ -39,4 +39,4 @@ control ComputeChecksumI(inout H hdr, inout M meta) { } } -@pkginfo(name="some_name", organization="some_org", version="some_version", contact="some_contact", url="some_url") @platform_property(multicast_table_size = 10 , multicast_table_total_replicas = 20 , multicast_table_max_replicas_per_entry = 30) V1Switch(ParserI(), VerifyChecksumI(), IngressI(), EgressI(), ComputeChecksumI(), DeparserI()) main; +@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 index f84aba0abee..7cf3fe2b51a 100644 --- a/testdata/p4_16_samples_outputs/pkginfo_annotation.p4 +++ b/testdata/p4_16_samples_outputs/pkginfo_annotation.p4 @@ -39,4 +39,4 @@ control ComputeChecksumI(inout H hdr, inout M meta) { } } -@pkginfo(name="some_name", organization="some_org", version="some_version", contact="some_contact", url="some_url") @platform_property(multicast_table_size = 10 , multicast_table_total_replicas = 20 , multicast_table_max_replicas_per_entry = 30) V1Switch(ParserI(), VerifyChecksumI(), IngressI(), EgressI(), ComputeChecksumI(), DeparserI()) main; +@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 index beb96ace49b..e69de29bb2d 100644 --- a/testdata/p4_16_samples_outputs/pkginfo_annotation.p4-stderr +++ b/testdata/p4_16_samples_outputs/pkginfo_annotation.p4-stderr @@ -1,3 +0,0 @@ -[--Wwarn=ignore] warning: Got Int64 field: multicast_table_size with value 10 -[--Wwarn=ignore] warning: Got Int64 field: multicast_table_total_replicas with value 20 -[--Wwarn=ignore] warning: Got Int64 field: multicast_table_max_replicas_per_entry with value 30 diff --git a/testdata/p4_16_samples_outputs/pkginfo_annotation.p4.p4info.txtpb b/testdata/p4_16_samples_outputs/pkginfo_annotation.p4.p4info.txtpb index 141f51f50b7..2af5fb7f8ba 100644 --- a/testdata/p4_16_samples_outputs/pkginfo_annotation.p4.p4info.txtpb +++ b/testdata/p4_16_samples_outputs/pkginfo_annotation.p4.p4info.txtpb @@ -8,4 +8,9 @@ pkg_info { 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 + } } From 351f7db829da61c5b0d005b2793e134d41d98566 Mon Sep 17 00:00:00 2001 From: Jonathan DiLorenzo Date: Thu, 30 May 2024 18:50:31 +0000 Subject: [PATCH 06/10] Adding back . --- bazel/p4c_deps.bzl | 1 + 1 file changed, 1 insertion(+) diff --git a/bazel/p4c_deps.bzl b/bazel/p4c_deps.bzl index 42602b5097e..3a8f7d2255a 100644 --- a/bazel/p4c_deps.bzl +++ b/bazel/p4c_deps.bzl @@ -49,6 +49,7 @@ filegroup( remote = "https://github.com/p4lang/p4runtime", # 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", From 428f622062eaa8acadb053bc7a5dba5c21bef05e Mon Sep 17 00:00:00 2001 From: Jonathan DiLorenzo Date: Thu, 30 May 2024 18:57:23 +0000 Subject: [PATCH 07/10] Add -f to mv command --- bazel/p4c_deps.bzl | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/bazel/p4c_deps.bzl b/bazel/p4c_deps.bzl index 3a8f7d2255a..273acffc86b 100644 --- a/bazel/p4c_deps.bzl +++ b/bazel/p4c_deps.bzl @@ -53,7 +53,7 @@ filegroup( # 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/* ."], + patch_cmds = ["mv -f proto/* ."], ) if not native.existing_rule("com_google_googletest"): # Cannot currently use local_repository due to Bazel limitation, From 5a9d5727b39e4548921758b7b65463c9837ca652 Mon Sep 17 00:00:00 2001 From: Jonathan DiLorenzo Date: Thu, 30 May 2024 19:04:27 +0000 Subject: [PATCH 08/10] Now trying -n --- bazel/p4c_deps.bzl | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/bazel/p4c_deps.bzl b/bazel/p4c_deps.bzl index 273acffc86b..326d85940ac 100644 --- a/bazel/p4c_deps.bzl +++ b/bazel/p4c_deps.bzl @@ -53,7 +53,7 @@ filegroup( # 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 -f proto/* ."], + patch_cmds = ["mv -n proto/* ."], ) if not native.existing_rule("com_google_googletest"): # Cannot currently use local_repository due to Bazel limitation, From 7a626ac64ccdbeda10971cfe653a110ef935d6b8 Mon Sep 17 00:00:00 2001 From: Jonathan DiLorenzo Date: Thu, 30 May 2024 19:16:25 +0000 Subject: [PATCH 09/10] Strip prefix is now supposed to be fixed, so trying that. --- bazel/p4c_deps.bzl | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/bazel/p4c_deps.bzl b/bazel/p4c_deps.bzl index 326d85940ac..3a479222663 100644 --- a/bazel/p4c_deps.bzl +++ b/bazel/p4c_deps.bzl @@ -50,10 +50,7 @@ filegroup( # 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 -n proto/* ."], + strip_prefix = "proto", ) if not native.existing_rule("com_google_googletest"): # Cannot currently use local_repository due to Bazel limitation, From c7877c9335d2a525b2d47434b1363a4441022fd3 Mon Sep 17 00:00:00 2001 From: Jonathan DiLorenzo Date: Thu, 30 May 2024 22:10:26 +0000 Subject: [PATCH 10/10] Address comments. --- control-plane/p4RuntimeSerializer.cpp | 13 ++++++------- 1 file changed, 6 insertions(+), 7 deletions(-) diff --git a/control-plane/p4RuntimeSerializer.cpp b/control-plane/p4RuntimeSerializer.cpp index 9a0919a2aab..bf6d2645211 100644 --- a/control-plane/p4RuntimeSerializer.cpp +++ b/control-plane/p4RuntimeSerializer.cpp @@ -951,7 +951,7 @@ class P4RuntimeAnalyzer { } } - // @platform_property annotation + // 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(); @@ -960,14 +960,13 @@ class P4RuntimeAnalyzer { 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 a integer", - kv); + ::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. + // 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,