From 85dd95826beb7be8cac1d55439acbdc81a9a6ba0 Mon Sep 17 00:00:00 2001 From: bibhuprasad-hcl <161687009+bibhuprasad-hcl@users.noreply.github.com> Date: Mon, 24 Jun 2024 00:50:35 -0400 Subject: [PATCH] [pdpi][multicast] Add PRE multicast support to IR. (#226) * [pdpi][multicast] Add PRE multicast support to IR. --------- Co-authored-by: smolkaj Co-authored-by: Srikishen Pondicherry Shanmugam --- .bazelversion | 2 +- p4_pdpi/BUILD.bazel | 1 + p4_pdpi/ir.cc | 161 +++++++++++++- p4_pdpi/ir.h | 8 + p4_pdpi/ir.proto | 21 ++ p4_pdpi/testing/table_entry_test_runner.cc | 95 ++++++++ p4_pdpi/testing/testdata/table_entry.expected | 210 ++++++++++++++++++ pins_infra_deps.bzl | 22 +- 8 files changed, 506 insertions(+), 14 deletions(-) diff --git a/.bazelversion b/.bazelversion index 6aba2b24..831446cb 100644 --- a/.bazelversion +++ b/.bazelversion @@ -1 +1 @@ -4.2.0 +5.1.0 diff --git a/p4_pdpi/BUILD.bazel b/p4_pdpi/BUILD.bazel index cc08f743..2502a24c 100644 --- a/p4_pdpi/BUILD.bazel +++ b/p4_pdpi/BUILD.bazel @@ -207,6 +207,7 @@ cc_library( "@com_github_p4lang_p4runtime//:p4runtime_cc_proto", "@com_github_p4lang_p4runtime//:p4types_cc_proto", "@com_google_absl//absl/algorithm:container", + "@com_google_absl//absl/container:flat_hash_map", "@com_google_absl//absl/container:flat_hash_set", "@com_google_absl//absl/status", "@com_google_absl//absl/status:statusor", diff --git a/p4_pdpi/ir.cc b/p4_pdpi/ir.cc index 35975b08..969a5307 100644 --- a/p4_pdpi/ir.cc +++ b/p4_pdpi/ir.cc @@ -22,6 +22,7 @@ #include #include "absl/algorithm/container.h" +#include "absl/container/flat_hash_map.h" #include "absl/container/flat_hash_set.h" #include "absl/status/status.h" #include "absl/status/statusor.h" @@ -1642,6 +1643,81 @@ StatusOr PiTableEntryToIr(const IrP4Info &info, return ir; } +StatusOr PiReplicaToIr(const IrP4Info &info, + const p4::v1::Replica &pi) { + IrReplica ir; + if (pi.port_kind_case() != p4::v1::Replica::kPort) { + return gutil::InvalidArgumentErrorBuilder() + << "expected `port` field to be set in Replica, but found < " + << gutil::PrintShortTextProto(pi) << " >"; + } + ir.set_port(pi.port()); + ir.set_instance(pi.instance()); + return ir; +} + +StatusOr PiMulticastGroupEntryToIr( + const IrP4Info &info, const p4::v1::MulticastGroupEntry &pi, + TranslationOptions options) { + IrMulticastGroupEntry ir; + ir.set_multicast_group_id(pi.multicast_group_id()); + + if (options.key_only) { + return ir; + } + + absl::flat_hash_map> + instances_by_port; + std::vector invalid_reasons; + for (const auto &replica : pi.replicas()) { + absl::StatusOr ir_replica = PiReplicaToIr(info, replica); + if (!ir_replica.ok()) { + invalid_reasons.push_back( + absl::StrCat(kNewBullet, ir_replica.status().message())); + continue; + } + // Check that {port, instance} pair is unique. + bool replica_is_unique = instances_by_port[ir_replica->port()] + .insert(ir_replica->instance()) + .second; + if (!replica_is_unique) { + invalid_reasons.push_back(absl::StrCat( + kNewBullet, + "Each replica must have a unique (port, instance)-pair, but found " + "multiple replicas with pair ('", + ir_replica->port(), "', ", ir_replica->instance(), ").")); + } + *ir.add_replicas() = std::move(*ir_replica); + } + + if (!invalid_reasons.empty()) { + return gutil::InvalidArgumentErrorBuilder() << GenerateFormattedError( + absl::StrCat("MulticastGroupEntry with group id '", + pi.multicast_group_id(), "'"), + absl::StrJoin(invalid_reasons, "\n")); + } + return ir; +} + +StatusOr PiPacketReplicationEngineEntryToIr( + const IrP4Info &info, const p4::v1::PacketReplicationEngineEntry &pi, + TranslationOptions options) { + IrPacketReplicationEngineEntry ir; + switch (pi.type_case()) { + case p4::v1::PacketReplicationEngineEntry::kMulticastGroupEntry: { + ASSIGN_OR_RETURN( + *ir.mutable_multicast_group_entry(), + PiMulticastGroupEntryToIr(info, pi.multicast_group_entry(), options)); + break; + } + default: { + return gutil::UnimplementedErrorBuilder() + << "Only PRE entries of type multicast group entry are supported."; + } + } + return ir; +} + StatusOr PiPacketInToIr(const IrP4Info &info, const p4::v1::PacketIn &packet) { return PiPacketIoToIr(info, "packet-in", @@ -1713,7 +1789,13 @@ StatusOr PiEntityToIr(const IrP4Info &info, const p4::v1::Entity &pi, PiTableEntryToIr(info, pi.table_entry(), options)); break; } - // TODO: Add PacketReplicationEngine support to IR. + case p4::v1::Entity::kPacketReplicationEngineEntry: { + ASSIGN_OR_RETURN( + *ir_entity.mutable_packet_replication_engine_entry(), + PiPacketReplicationEngineEntryToIr( + info, pi.packet_replication_engine_entry(), options)); + break; + } default: { auto entity_name = gutil::GetOneOfFieldName(pi, "entity"); if (!entity_name.ok()) { @@ -2123,6 +2205,75 @@ StatusOr IrTableEntryToPi(const IrP4Info &info, return pi; } +StatusOr IrReplicaToPi(const IrP4Info &info, + const IrReplica &ir) { + p4::v1::Replica pi; + pi.set_port(ir.port()); + pi.set_instance(ir.instance()); + return pi; +} + +StatusOr IrMulticastGroupEntryToPi( + const IrP4Info &info, const IrMulticastGroupEntry &ir, + TranslationOptions options) { + p4::v1::MulticastGroupEntry pi; + pi.set_multicast_group_id(ir.multicast_group_id()); + + if (options.key_only) { + return pi; + } + + absl::flat_hash_map> + instances_by_port; + std::vector invalid_reasons; + for (const auto &replica : ir.replicas()) { + absl::StatusOr pi_replica = IrReplicaToPi(info, replica); + if (!pi_replica.ok()) { + invalid_reasons.push_back( + absl::StrCat(kNewBullet, pi_replica.status().message())); + continue; + } + // Check that {port, instance} pair is unique. + bool replica_is_unique = instances_by_port[pi_replica->port()] + .insert(pi_replica->instance()) + .second; + if (!replica_is_unique) { + invalid_reasons.push_back(absl::StrCat( + kNewBullet, + "Each replica must have a unique (port, instance)-pair, but found " + "multiple replicas with pair ('", + pi_replica->port(), "', ", pi_replica->instance(), ").")); + } + *pi.add_replicas() = std::move(*pi_replica); + } + if (!invalid_reasons.empty()) { + return gutil::InvalidArgumentErrorBuilder() << GenerateFormattedError( + absl::StrCat("MulticastGroupEntry with group id '", + ir.multicast_group_id(), "'"), + absl::StrJoin(invalid_reasons, "\n")); + } + return pi; +} + +StatusOr +IrPacketReplicationEngineEntryToPi(const IrP4Info &info, + const IrPacketReplicationEngineEntry &ir, + TranslationOptions options) { + p4::v1::PacketReplicationEngineEntry pi; + switch (ir.type_case()) { + case IrPacketReplicationEngineEntry::kMulticastGroupEntry: { + ASSIGN_OR_RETURN( + *pi.mutable_multicast_group_entry(), + IrMulticastGroupEntryToPi(info, ir.multicast_group_entry(), options)); + break; + } + default: + return gutil::UnimplementedErrorBuilder() + << "Only PRE entries of type multicast group entry are supported."; + } + return pi; +} + StatusOr IrPacketInToPi(const IrP4Info &info, const IrPacketIn &packet) { return IrPacketIoToPi(info, "packet-in", @@ -2160,7 +2311,13 @@ StatusOr IrEntityToPi(const IrP4Info &info, const IrEntity &ir, IrTableEntryToPi(info, ir.table_entry(), options)); break; } - // TODO: Add PacketReplicationEngine support to IR. + case IrEntity::kPacketReplicationEngineEntry: { + ASSIGN_OR_RETURN( + *pi_entity.mutable_packet_replication_engine_entry(), + IrPacketReplicationEngineEntryToPi( + info, ir.packet_replication_engine_entry(), options)); + break; + } default: { auto entity_name = gutil::GetOneOfFieldName(ir, "entity"); if (!entity_name.ok()) { diff --git a/p4_pdpi/ir.h b/p4_pdpi/ir.h index baee77cd..ca97ddd3 100644 --- a/p4_pdpi/ir.h +++ b/p4_pdpi/ir.h @@ -49,6 +49,10 @@ absl::StatusOr PiTableEntriesToIr( const IrP4Info& info, absl::Span pi, TranslationOptions options = {}); +absl::StatusOr PiMulticastGroupEntryToIr( + const IrP4Info& info, const p4::v1::MulticastGroupEntry& pi, + TranslationOptions options = {}); + absl::StatusOr PiPacketInToIr(const IrP4Info& info, const p4::v1::PacketIn& packet); @@ -93,6 +97,10 @@ absl::StatusOr> IrTableEntriesToPi( const IrP4Info& info, absl::Span ir, TranslationOptions options = {}); +absl::StatusOr IrMulticastGroupEntryToPi( + const IrP4Info& info, const IrMulticastGroupEntry& ir, + TranslationOptions options = {}); + absl::StatusOr IrPacketInToPi(const IrP4Info& info, const IrPacketIn& packet); diff --git a/p4_pdpi/ir.proto b/p4_pdpi/ir.proto index 2db894b7..a9f46e8e 100644 --- a/p4_pdpi/ir.proto +++ b/p4_pdpi/ir.proto @@ -303,6 +303,26 @@ message IrTableEntries { repeated IrTableEntry entries = 1; } +//-- Packet Replication Engine ------------------------------------------------- + +// Describes a PacketReplicationEngine (PRE) entry. Currently, the only +// supported PRE entry is multicast group entry. +message IrPacketReplicationEngineEntry { + oneof type { + IrMulticastGroupEntry multicast_group_entry = 1; + } +} + +message IrReplica { + string port = 1; + uint32 instance = 2; +} + +message IrMulticastGroupEntry { + uint32 multicast_group_id = 1; + repeated IrReplica replicas = 2; +} + // -- Packet IO ---------------------------------------------------------------- // Describes a packet in. @@ -328,6 +348,7 @@ message IrPacketMetadata { message IrEntity { oneof entity { IrTableEntry table_entry = 1; + IrPacketReplicationEngineEntry packet_replication_engine_entry = 2; } } diff --git a/p4_pdpi/testing/table_entry_test_runner.cc b/p4_pdpi/testing/table_entry_test_runner.cc index 863c5a44..98d168fe 100644 --- a/p4_pdpi/testing/table_entry_test_runner.cc +++ b/p4_pdpi/testing/table_entry_test_runner.cc @@ -97,6 +97,51 @@ static void RunPdTableEntryTest(const pdpi::IrP4Info& info, }); } +static void RunPiMulticastTest(const pdpi::IrP4Info& info) { + RunPiEntityTest(info, "multicast group entry with deprecated egress_port set", + gutil::ParseProtoOrDie(R"pb( + packet_replication_engine_entry { + multicast_group_entry { + multicast_group_id: 7 + replicas { egress_port: 3 instance: 1 } + } + } + )pb")); + RunPiEntityTest(info, "multicast group entry with duplicate replica", + gutil::ParseProtoOrDie(R"pb( + packet_replication_engine_entry { + multicast_group_entry { + multicast_group_id: 7 + replicas { port: "some_port" instance: 1 } + replicas { port: "some_port" instance: 1 } + } + } + )pb")); + RunPiEntityTest(info, "valid multicast group entry", + gutil::ParseProtoOrDie(R"pb( + packet_replication_engine_entry { + multicast_group_entry { + multicast_group_id: 7 + replicas { port: "some_port" instance: 1 } + replicas { port: "some_port" instance: 2 } + replicas { port: "some_other_port" instance: 1 } + } + } + )pb"), + /*validity=*/INPUT_IS_VALID); + RunPiEntityTest(info, "valid multicast group entry without explicit instance", + gutil::ParseProtoOrDie(R"pb( + packet_replication_engine_entry { + multicast_group_entry { + multicast_group_id: 7 + replicas { port: "some_port" } + replicas { port: "some_other_port" } + } + } + )pb"), + /*validity=*/INPUT_IS_VALID); +} + static void RunPiTests(const pdpi::IrP4Info info) { RunPiEntityTest(info, "empty PI", gutil::ParseProtoOrDie(R"pb( )pb")); @@ -776,6 +821,7 @@ static void RunPiTests(const pdpi::IrP4Info info) { } )pb"), /*validity=*/INPUT_IS_VALID); + RunPiMulticastTest(info); } // NOLINT(readability/fn_size) static void RunIrNoActionTableTests(const pdpi::IrP4Info& info) { @@ -844,6 +890,54 @@ static void RunIrTernaryTableTests(const pdpi::IrP4Info info) { )pb")); } +static void RunIrMulticastTest(const pdpi::IrP4Info& info) { + RunIrEntityTest(info, "multicast group entry with duplicate replica", + gutil::ParseProtoOrDie(R"pb( + packet_replication_engine_entry { + multicast_group_entry { + multicast_group_id: 7 + replicas { port: "some_port" instance: 1 } + replicas { port: "some_port" instance: 1 } + } + } + )pb"), + IrTestConfig{ + // TODO: Add PRE support to PD. + .test_ir_to_pd = false, + }); + RunIrEntityTest(info, "valid multicast group entry", + gutil::ParseProtoOrDie(R"pb( + packet_replication_engine_entry { + multicast_group_entry { + multicast_group_id: 7 + replicas { port: "some_port" instance: 1 } + replicas { port: "some_port" instance: 2 } + replicas { port: "some_other_port" instance: 1 } + } + } + )pb"), + IrTestConfig{ + .validity = INPUT_IS_VALID, + // TODO: Add PRE support to PD. + .test_ir_to_pd = false, + }); + RunIrEntityTest(info, "valid multicast group entry without explicit instance", + gutil::ParseProtoOrDie(R"pb( + packet_replication_engine_entry { + multicast_group_entry { + multicast_group_id: 7 + replicas { port: "some_port" } + replicas { port: "some_other_port" } + } + } + )pb"), + IrTestConfig{ + .validity = INPUT_IS_VALID, + // TODO: Add PRE support to PD. + .test_ir_to_pd = false, + }); +} + static void RunIrMeterCounterTableEntryTests(const pdpi::IrP4Info& info) { RunIrEntityTest(info, "meter counter data in a table with no counters", gutil::ParseProtoOrDie(R"pb( @@ -1769,6 +1863,7 @@ static void RunIrTests(const pdpi::IrP4Info info) { IrTestConfig{ .validity = INPUT_IS_VALID, }); + RunIrMulticastTest(info); } // NOLINT(readability/fn_size) static void RunPdMeterCounterTableEntryTests(const pdpi::IrP4Info& info) { diff --git a/p4_pdpi/testing/testdata/table_entry.expected b/p4_pdpi/testing/testdata/table_entry.expected index 245cce21..d6f3d9de 100644 --- a/p4_pdpi/testing/testdata/table_entry.expected +++ b/p4_pdpi/testing/testdata/table_entry.expected @@ -1262,6 +1262,120 @@ table_entry { } +========================================================================= +multicast group entry with deprecated egress_port set +========================================================================= + +--- PI (Input): +packet_replication_engine_entry { + multicast_group_entry { + multicast_group_id: 7 + replicas { + egress_port: 3 + instance: 1 + } + } +} + +--- PI is invalid/unsupported: +INVALID_ARGUMENT: MulticastGroupEntry with group id '7' is invalid: expected `port` field to be set in Replica, but found < egress_port: 3 instance: 1 > + +========================================================================= +multicast group entry with duplicate replica +========================================================================= + +--- PI (Input): +packet_replication_engine_entry { + multicast_group_entry { + multicast_group_id: 7 + replicas { + instance: 1 + port: "some_port" + } + replicas { + instance: 1 + port: "some_port" + } + } +} + +--- PI is invalid/unsupported: +INVALID_ARGUMENT: MulticastGroupEntry with group id '7' is invalid: Each replica must have a unique (port, instance)-pair, but found multiple replicas with pair ('some_port', 1). + +========================================================================= +valid multicast group entry +========================================================================= + +--- PI (Input): +packet_replication_engine_entry { + multicast_group_entry { + multicast_group_id: 7 + replicas { + instance: 1 + port: "some_port" + } + replicas { + instance: 2 + port: "some_port" + } + replicas { + instance: 1 + port: "some_other_port" + } + } +} + +--- IR: +packet_replication_engine_entry { + multicast_group_entry { + multicast_group_id: 7 + replicas { + port: "some_port" + instance: 1 + } + replicas { + port: "some_port" + instance: 2 + } + replicas { + port: "some_other_port" + instance: 1 + } + } +} + + +========================================================================= +valid multicast group entry without explicit instance +========================================================================= + +--- PI (Input): +packet_replication_engine_entry { + multicast_group_entry { + multicast_group_id: 7 + replicas { + port: "some_port" + } + replicas { + port: "some_other_port" + } + } +} + +--- IR: +packet_replication_engine_entry { + multicast_group_entry { + multicast_group_id: 7 + replicas { + port: "some_port" + } + replicas { + port: "some_other_port" + } + } +} + + ========================================================================= empty IR (IR -> PI) ========================================================================= @@ -3193,6 +3307,102 @@ id_test_table_entry { } +========================================================================= +multicast group entry with duplicate replica (IR -> PI) +========================================================================= + +--- IR (Input): +packet_replication_engine_entry { + multicast_group_entry { + multicast_group_id: 7 + replicas { + port: "some_port" + instance: 1 + } + replicas { + port: "some_port" + instance: 1 + } + } +} + +--- IR (converting to PI) is invalid/unsupported: +INVALID_ARGUMENT: MulticastGroupEntry with group id '7' is invalid: Each replica must have a unique (port, instance)-pair, but found multiple replicas with pair ('some_port', 1). + +========================================================================= +valid multicast group entry (IR -> PI) +========================================================================= + +--- IR (Input): +packet_replication_engine_entry { + multicast_group_entry { + multicast_group_id: 7 + replicas { + port: "some_port" + instance: 1 + } + replicas { + port: "some_port" + instance: 2 + } + replicas { + port: "some_other_port" + instance: 1 + } + } +} + +--- PI: +packet_replication_engine_entry { + multicast_group_entry { + multicast_group_id: 7 + replicas { + instance: 1 + port: "some_port" + } + replicas { + instance: 2 + port: "some_port" + } + replicas { + instance: 1 + port: "some_other_port" + } + } +} + + +========================================================================= +valid multicast group entry without explicit instance (IR -> PI) +========================================================================= + +--- IR (Input): +packet_replication_engine_entry { + multicast_group_entry { + multicast_group_id: 7 + replicas { + port: "some_port" + } + replicas { + port: "some_other_port" + } + } +} + +--- PI: +packet_replication_engine_entry { + multicast_group_entry { + multicast_group_id: 7 + replicas { + port: "some_port" + } + replicas { + port: "some_other_port" + } + } +} + + ========================================================================= empty PD pdpi::TranslationOptions{.key_only = false, .allow_unsupported = false} diff --git a/pins_infra_deps.bzl b/pins_infra_deps.bzl index cc677e66..d5531915 100644 --- a/pins_infra_deps.bzl +++ b/pins_infra_deps.bzl @@ -82,27 +82,27 @@ def pins_infra_deps(): if not native.existing_rule("com_github_p4lang_p4c"): http_archive( name = "com_github_p4lang_p4c", - # Newest commit on main on 2021-12-07. - url = "https://github.com/p4lang/p4c/archive/a9aa5ff46affe8fd5dde78c2411d1bc58a715b33.zip", - strip_prefix = "p4c-a9aa5ff46affe8fd5dde78c2411d1bc58a715b33", - sha256 = "fa22c3d2b3105a39a73fc3938cbc6cd5d7895113a3e6ed6c5a48fbbd958a28af", + # Newest commit on main on 2022-11-23. + url = "https://github.com/p4lang/p4c/archive/bc1798bb8529c9f71f2794dbc690b29f040549c4.zip", + strip_prefix = "p4c-bc1798bb8529c9f71f2794dbc690b29f040549c4", + sha256 = "21fece70b3fc2d1430ccc3e023b038ce0ca74e1682e6249fb350809d1c61215f", ) if not native.existing_rule("com_github_p4lang_p4runtime"): # We frequently need bleeding-edge, unreleased version of P4Runtime, so we use a commit # rather than a release. http_archive( name = "com_github_p4lang_p4runtime", - # 13f0d0 is the newest commit on main as of 2022-08-02. - urls = ["https://github.com/p4lang/p4runtime/archive/13f0d02a521e38b4252f4fddfc98e2bfa1dbf7e6.zip"], - strip_prefix = "p4runtime-13f0d02a521e38b4252f4fddfc98e2bfa1dbf7e6/proto", - sha256 = "ad18b27341ea1a875919a63d37eaa347d3d85299a1f90c4bbc33789c1d0580b7", + # 90553b9 is the newest commit on main as of 2023-05-05. + urls = ["https://github.com/p4lang/p4runtime/archive/90553b90a12ead5c19700e7fef21164dea5b6d22.zip"], + strip_prefix = "p4runtime-90553b90a12ead5c19700e7fef21164dea5b6d22/proto", + sha256 = "21f060e8cb590174c9be0fbe22665cb9a896f5f0b9399e17794defc9bcda3adc", ) if not native.existing_rule("com_github_p4lang_p4_constraints"): http_archive( name = "com_github_p4lang_p4_constraints", - urls = ["https://github.com/p4lang/p4-constraints/archive/50a7ddc19faf912f5a6cb61cbfa94bf1aaa9bf82.zip"], - strip_prefix = "p4-constraints-50a7ddc19faf912f5a6cb61cbfa94bf1aaa9bf82", - sha256 = "62d810efbadfe6b5428370f2f4f1f40622563344a0ef27a8c3a631115945a795", + urls = ["https://github.com/p4lang/p4-constraints/archive/11a08877ae8e3cf60acfa90c0f020d65b5a91ed5.zip"], + strip_prefix = "p4-constraints-11a08877ae8e3cf60acfa90c0f020d65b5a91ed5", + sha256 = "e85039af2378264f5e980984ab95f28903baebb3f515f1097377e294e681b2b6", ) if not native.existing_rule("com_github_nlohmann_json"): http_archive(