From cb7622bef7e89cc9481da66255680d91e62f207f Mon Sep 17 00:00:00 2001 From: P4 Infra Team Date: Wed, 23 Jul 2025 13:57:07 -0700 Subject: [PATCH] [DVaaS][Minimization] Enable table-based bisection for minimization. PiperOrigin-RevId: 786414117 --- p4_pdpi/BUILD.bazel | 4 +- p4_pdpi/sequencing.cc | 41 ++++ p4_pdpi/sequencing.expected | 343 ++++++++++++++++++++++++++++++ p4_pdpi/sequencing.h | 9 + p4_pdpi/sequencing_test_runner.cc | 50 +++++ 5 files changed, 446 insertions(+), 1 deletion(-) diff --git a/p4_pdpi/BUILD.bazel b/p4_pdpi/BUILD.bazel index cc4b9d6..01c46fd 100644 --- a/p4_pdpi/BUILD.bazel +++ b/p4_pdpi/BUILD.bazel @@ -153,6 +153,7 @@ cc_library( # Disable default arguments internally. Using them in PDPI itself is very likely a bug. local_defines = ["PDPI_DISABLE_TRANSLATION_OPTIONS_DEFAULT"], deps = [ + ":entity_keys", ":ir_cc_proto", ":names", ":references", @@ -171,6 +172,7 @@ cc_library( "@com_google_absl//absl/status", "@com_google_absl//absl/status:statusor", "@com_google_absl//absl/strings", + "@com_google_absl//absl/types:optional", "@com_google_absl//absl/types:span", "@com_google_gutil//gutil:collections", "@com_google_gutil//gutil:status", @@ -745,7 +747,7 @@ cc_test( cmd_diff_test( name = "sequencing_diff_test", actual_cmd = "$(execpath :sequencing_test_runner) $(location :main-p4info.pb.txt)", - expected = "//p4_pdpi:sequencing.expected", + expected = ":sequencing.expected", tools = [ "main-p4info.pb.txt", ":sequencing_test_runner", diff --git a/p4_pdpi/sequencing.cc b/p4_pdpi/sequencing.cc index debbdb7..8ce5de8 100644 --- a/p4_pdpi/sequencing.cc +++ b/p4_pdpi/sequencing.cc @@ -19,6 +19,7 @@ #include #include #include +#include #include #include @@ -30,6 +31,7 @@ #include "absl/status/status.h" #include "absl/status/statusor.h" #include "absl/strings/str_cat.h" +#include "absl/types/optional.h" #include "absl/types/span.h" #include "boost/graph/adjacency_list.hpp" #include "google/protobuf/repeated_ptr_field.h" @@ -37,6 +39,7 @@ #include "gutil/status.h" #include "p4/config/v1/p4info.pb.h" #include "p4/v1/p4runtime.pb.h" +#include "p4_pdpi/entity_keys.h" #include "p4_pdpi/ir.pb.h" #include "p4_pdpi/names.h" #include "p4_pdpi/references.h" @@ -376,6 +379,44 @@ absl::Status StableSortEntities(const IrP4Info& info, return absl::OkStatus(); } +absl::Status SortEntities(const IrP4Info& info, + std::vector& entities) { + absl::c_sort(entities, [&](const p4::v1::Entity& a, const p4::v1::Entity& b) { + return a.DebugString() < b.DebugString(); + }); + + struct EntityTuple { + p4::v1::Entity entity; + EntityKey key; + int order; + }; + + std::vector augmented_entities; + for (p4::v1::Entity& entity : entities) { + ASSIGN_OR_RETURN(EntityKey key, EntityKey::MakeEntityKey(entity)); + ASSIGN_OR_RETURN(std::string table_name, EntityToTableName(info, entity)); + ASSIGN_OR_RETURN( + int order, + gutil::FindOrStatus(info.dependency_rank_by_table_name(), table_name)); + + augmented_entities.push_back({std::move(entity), key, order}); + } + absl::c_stable_sort(augmented_entities, + [&](const EntityTuple& a, const EntityTuple& b) { + return a.key < b.key; + }); + + absl::c_stable_sort(augmented_entities, + [&](const EntityTuple& a, const EntityTuple& b) { + return a.order > b.order; + }); + + for (int i = 0; i < entities.size(); ++i) { + entities[i] = std::move(augmented_entities[i].entity); + } + return absl::OkStatus(); +} + absl::Status StableSortUpdates( const IrP4Info& info, google::protobuf::RepeatedPtrField& updates, diff --git a/p4_pdpi/sequencing.expected b/p4_pdpi/sequencing.expected index 3a79102..1652296 100644 --- a/p4_pdpi/sequencing.expected +++ b/p4_pdpi/sequencing.expected @@ -1212,6 +1212,9 @@ SortTest: Empty input --- Sorted entities (output): +--- Non-stable sorted entities (output): + + ========================================================================= SortTest: EntryWithOneKey{key-a} -> EntryThatRefersTo{key-a} ========================================================================= @@ -1260,6 +1263,28 @@ referring_by_action_table_entry { } } +--- Non-stable sorted entities (output): +one_match_field_table_entry { + match { + id: "key-a" + } + action { + do_thing_4 { + } + } +} + +referring_by_action_table_entry { + match { + val: "0x001" + } + action { + referring_to_one_match_field_action { + referring_id_1: "key-a" + } + } +} + ========================================================================= SortTest: EntryWithOneKey{key-a} -> EntryThatRefersTo{key-a, 0x002} ========================================================================= @@ -1310,6 +1335,29 @@ referring_by_action_table_entry { } } +--- Non-stable sorted entities (output): +one_match_field_table_entry { + match { + id: "key-a" + } + action { + do_thing_4 { + } + } +} + +referring_by_action_table_entry { + match { + val: "0x001" + } + action { + referring_to_two_match_fields_action { + referring_id_1: "key-a" + referring_id_2: "0x002" + } + } +} + ========================================================================= SortTest: EntryWithTwoKeys{key-a, 0x002} -> EntryThatRefersTo{key-a, 0x002} ========================================================================= @@ -1362,6 +1410,30 @@ referring_by_action_table_entry { } } +--- Non-stable sorted entities (output): +two_match_fields_table_entry { + match { + id_1: "key-a" + id_2: "0x002" + } + action { + do_thing_4 { + } + } +} + +referring_by_action_table_entry { + match { + val: "0x001" + } + action { + referring_to_two_match_fields_action { + referring_id_1: "key-a" + referring_id_2: "0x002" + } + } +} + ========================================================================= SortTest: EntryThatRefersTo{key-a, 0x002} -> EntryWithTwoKeys{key-c, 0x004} ========================================================================= @@ -1414,6 +1486,30 @@ referring_by_action_table_entry { } } +--- Non-stable sorted entities (output): +two_match_fields_table_entry { + match { + id_1: "key-c" + id_2: "0x004" + } + action { + do_thing_4 { + } + } +} + +referring_by_action_table_entry { + match { + val: "0x001" + } + action { + referring_to_two_match_fields_action { + referring_id_1: "key-a" + referring_id_2: "0x002" + } + } +} + ========================================================================= SortTest: Entries at same rank should maintain the original order. ========================================================================= @@ -1470,6 +1566,32 @@ referring_by_action_table_entry { } } +--- Non-stable sorted entities (output): +packet_count_and_meter_table_entry { + match { + ipv4 { + value: "16.36.50.0" + prefix_length: 24 + } + } + action { + packet_count_and_meter { + } + } +} + +referring_by_action_table_entry { + match { + val: "0x001" + } + action { + referring_to_two_match_fields_action { + referring_id_1: "key-a" + referring_id_2: "0x002" + } + } +} + ========================================================================= SortTest: EntryWithOneKey{key-a} -> EntryThatRefersTo{key-a} , another entry ========================================================================= @@ -1544,6 +1666,41 @@ referring_by_action_table_entry { } } +--- Non-stable sorted entities (output): +lpm2_table_entry { + match { + ipv6 { + value: "ffff::abcd:0:0" + prefix_length: 96 + } + } + action { + NoAction { + } + } +} + +one_match_field_table_entry { + match { + id: "key-a" + } + action { + do_thing_4 { + } + } +} + +referring_by_action_table_entry { + match { + val: "0x001" + } + action { + referring_to_one_match_field_action { + referring_id_1: "key-a" + } + } +} + ========================================================================= SortTest: EntryWithOneKey{key-a} -> EntryThatRefersTo{key-a} ,EntryWithOneKey{key-b} -> EntryThatRefersTo{key-b} ========================================================================= @@ -1638,6 +1795,169 @@ referring_by_action_table_entry { } } +--- Non-stable sorted entities (output): +two_match_fields_table_entry { + match { + id_1: "key-c" + id_2: "0x004" + } + action { + do_thing_4 { + } + } +} + +one_match_field_table_entry { + match { + id: "key-a" + } + action { + do_thing_4 { + } + } +} + +referring_by_action_table_entry { + match { + val: "0x001" + } + action { + referring_to_two_match_fields_action { + referring_id_1: "key-a" + referring_id_2: "0x002" + } + } +} + +referring_by_action_table_entry { + match { + val: "0x001" + } + action { + referring_to_one_match_field_action { + referring_id_1: "key-a" + } + } +} + +========================================================================= +SortTest: lpm2_table_entry and two_match_fields_table_entry have the same dependency rank of 0 +========================================================================= + +--- PD entries (input): +lpm2_table_entry { + match { + ipv6 { + value: "ffff::abcd:0:0" + prefix_length: 96 + } + } + action { + NoAction { + } + } +} + +two_match_fields_table_entry { + match { + id_1: "key-c" + id_2: "0x003" + } + action { + do_thing_4 { + } + } +} + +lpm2_table_entry { + match { + ipv6 { + value: "ffff::abcd:0:0" + prefix_length: 96 + } + } + action { + NoAction { + } + } +} + +--- Sorted entities (output): +lpm2_table_entry { + match { + ipv6 { + value: "ffff::abcd:0:0" + prefix_length: 96 + } + } + action { + NoAction { + } + } +} + +two_match_fields_table_entry { + match { + id_1: "key-c" + id_2: "0x003" + } + action { + do_thing_4 { + } + } +} + +lpm2_table_entry { + match { + ipv6 { + value: "ffff::abcd:0:0" + prefix_length: 96 + } + } + action { + NoAction { + } + } +} + +--- Non-stable sorted entities (output): +lpm2_table_entry { + match { + ipv6 { + value: "ffff::abcd:0:0" + prefix_length: 96 + } + } + action { + NoAction { + } + } +} + +lpm2_table_entry { + match { + ipv6 { + value: "ffff::abcd:0:0" + prefix_length: 96 + } + } + action { + NoAction { + } + } +} + +two_match_fields_table_entry { + match { + id_1: "key-c" + id_2: "0x003" + } + action { + do_thing_4 { + } + } +} + ========================================================================= SortTest: two_match_fields_table_entry -> referring_by_match_field_table_entry ========================================================================= @@ -1688,6 +2008,29 @@ referring_by_match_field_table_entry { } } +--- Non-stable sorted entities (output): +two_match_fields_table_entry { + match { + id_1: "key-a" + id_2: "0x001" + } + action { + do_thing_4 { + } + } +} + +referring_by_match_field_table_entry { + match { + referring_id_1: "key-a" + referring_id_2: "0x001" + } + action { + do_thing_4 { + } + } +} + ========================================================================= GetEntriesUnreachableFromRootsTest: Empty input generates no garbage. ========================================================================= diff --git a/p4_pdpi/sequencing.h b/p4_pdpi/sequencing.h index eed4c9d..5e33782 100644 --- a/p4_pdpi/sequencing.h +++ b/p4_pdpi/sequencing.h @@ -70,6 +70,15 @@ absl::StatusOr>> SequencePiUpdatesInPlace( absl::Status StableSortEntities(const IrP4Info& info, std::vector& entities); +// Sorts the entities such that entities that may be depended on come first. +// That is, two entities x and y where x could refer to y will be sorted +// as [y, x]. This is done based on the dependency ranks given in the IrP4Info. +// Any entities with the same dependency rank are sorted by their entity key +// value. +// Imposes a total ordering on entities. +absl::Status SortEntities(const IrP4Info& info, + std::vector& entities); + // Same as StableSortEntities but sorts the repeated `Update` message. absl::Status StableSortUpdates( const IrP4Info& info, diff --git a/p4_pdpi/sequencing_test_runner.cc b/p4_pdpi/sequencing_test_runner.cc index c9cb812..6cec950 100644 --- a/p4_pdpi/sequencing_test_runner.cc +++ b/p4_pdpi/sequencing_test_runner.cc @@ -132,6 +132,8 @@ void SortTest(const pdpi::IrP4Info& info, const std::string& test_name, std::cout << PrintTextProto(entry) << std::endl; } + std::vector entities_to_sort = pi_entities; + // Run sorting. absl::Status status = pdpi::StableSortEntities(info, pi_entities); if (!status.ok()) { @@ -154,6 +156,30 @@ void SortTest(const pdpi::IrP4Info& info, const std::string& test_name, } std::cout << PrintTextProto(pd_entry) << std::endl; } + + // Run non-stable sorting. + status = pdpi::SortEntities(info, entities_to_sort); + if (!status.ok()) { + std::cout << "--- Sorting failed (output):" << std::endl; + std::cout << status << std::endl; + return; + } + + // Output results. + std::cout << "--- Non-stable sorted entities (output):" << std::endl; + if (entities_to_sort.empty()) + std::cout << "" << std::endl << std::endl; + for (const auto& entry : entities_to_sort) { + pdpi::TableEntry pd_entry; + if (absl::Status status = + pdpi::PiEntityToPdTableEntry(info, entry, &pd_entry); + !status.ok()) { + std::cerr << "Unable to convert PI Entity to PD TableEntry." << status + << std::endl; + return; + } + std::cout << PrintTextProto(pd_entry) << std::endl; + } } void GetEntriesUnreachableFromRootsTest( @@ -1057,6 +1083,30 @@ void RunSortTests(const pdpi::IrP4Info& info) { )pb", }); + SortTest(info, + "lpm2_table_entry and two_match_fields_table_entry have the same " + "dependency rank of 0", + { + R"pb( + lpm2_table_entry { + match { ipv6 { value: "ffff::abcd:0:0" prefix_length: 96 } } + action { NoAction {} } + } + )pb", + R"pb( + two_match_fields_table_entry { + match { id_1: "key-c", id_2: "0x003" } + action { do_thing_4 {} } + } + )pb", + R"pb( + lpm2_table_entry { + match { ipv6 { value: "ffff::abcd:0:0" prefix_length: 96 } } + action { NoAction {} } + } + )pb", + }); + SortTest( info, "two_match_fields_table_entry -> referring_by_match_field_table_entry",