[PDPI] Add SortEntities to sequencing. (sonic-net#480)

Co-authored-by: jonathan-dilorenzo <[email protected]>

p4_pdpi/BUILD.bazel

@@ -133,6 +133,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 = [
+        ":helpers",
         ":ir_cc_proto",
         ":sequencing_util",
         "//gutil:collections",
@@ -506,3 +507,33 @@ cc_test(
         "@com_google_googletest//:gtest_main",
     ],
 )
+
+cc_library(
+    name = "helpers",
+    srcs = ["helpers.cc"],
+    hdrs = ["helpers.h"],
+    deps = [
+        ":built_ins",
+        ":ir_cc_proto",
+        "//gutil:collections",
+        "//gutil:status",
+        "@com_github_p4lang_p4runtime//:p4info_cc_proto",
+        "@com_github_p4lang_p4runtime//:p4runtime_cc_proto",
+        "@com_google_absl//absl/status",
+        "@com_google_absl//absl/status:statusor",
+    ],
+)
+
+cc_test(
+    name = "helpers_test",
+    srcs = ["helpers_test.cc"],
+    deps = [
+        ":built_ins",
+        ":helpers",
+        ":ir_cc_proto",
+        "//gutil:status_matchers",
+        "//p4_pdpi/testing:test_p4info_cc",
+        "@com_github_p4lang_p4runtime//:p4runtime_cc_proto",
+        "@com_google_googletest//:gtest_main",
+    ],
+)

p4_pdpi/built_ins.cc

@@ -32,8 +32,13 @@ constexpr absl::string_view kMulticastGroupIdString = "multicast_group_id";
 constexpr absl::string_view kReplicaString = "replica";
 constexpr absl::string_view kReplicaPortString = "replica.port";
 constexpr absl::string_view kReplicaInstanceString = "replica.instance";
+
 }  // namespace
 
+std::string GetMulticastGroupTableName() {
+  return absl::StrCat(kBuiltInPrefix, kMulticastGroupTableString);
+}
+
 bool IsBuiltInTable(absl::string_view table_name) {
   return StringToIrBuiltInTable(table_name).ok();
 }
@@ -97,7 +102,7 @@ absl::StatusOr<IrBuiltInAction> GetBuiltInActionFromBuiltInParameter(
 absl::StatusOr<std::string> IrBuiltInTableToString(IrBuiltInTable table) {
   switch (table) {
     case pdpi::BUILT_IN_TABLE_MULTICAST_GROUP_TABLE: {
-      return absl::StrCat(kBuiltInPrefix, kMulticastGroupTableString);
+      return GetMulticastGroupTableName();
     }
     default: {
       return gutil::InvalidArgumentErrorBuilder() << "Unknown built-in table.";
@@ -146,7 +151,7 @@ absl::StatusOr<std::string> IrBuiltInParameterToString(
 }
 
 absl::StatusOr<IrBuiltInTable> StringToIrBuiltInTable(absl::string_view table) {
-  if (table == absl::StrCat(kBuiltInPrefix, kMulticastGroupTableString)) {
+  if (table == GetMulticastGroupTableName()) {
     return pdpi::BUILT_IN_TABLE_MULTICAST_GROUP_TABLE;
   }
   return gutil::InvalidArgumentErrorBuilder()

p4_pdpi/built_ins.h

@@ -28,6 +28,9 @@
 
 namespace pdpi {
 
+// Returns the PDPI name for the multicast group table.
+std::string GetMulticastGroupTableName();
+
 // Returns true if `table_name` is a known built_in table.
 // Useful for branching on table type.
 bool IsBuiltInTable(absl::string_view table_name);

p4_pdpi/helpers.cc

@@ -0,0 +1,42 @@
+#include "p4_pdpi/helpers.h"
+
+#include <string>
+
+#include "absl/status/statusor.h"
+#include "gutil/collections.h"
+#include "gutil/status.h"
+#include "p4/config/v1/p4info.pb.h"
+#include "p4/v1/p4runtime.pb.h"
+#include "p4_pdpi/built_ins.h"
+#include "p4_pdpi/ir.pb.h"
+
+namespace pdpi {
+
+absl::StatusOr<std::string> EntityToTableName(const pdpi::IrP4Info& info,
+                                              const p4::v1::Entity& entity) {
+  switch (entity.entity_case()) {
+    case p4::v1::Entity::kTableEntry: {
+      ASSIGN_OR_RETURN(const IrTableDefinition table,
+                       gutil::FindOrStatus(info.tables_by_id(),
+                                           entity.table_entry().table_id()));
+      return table.preamble().alias();
+    }
+    case p4::v1::Entity::kPacketReplicationEngineEntry: {
+      if (!entity.packet_replication_engine_entry()
+               .has_multicast_group_entry()) {
+        return gutil::InvalidArgumentErrorBuilder()
+               << "Expected a `multicast_group_entry`, but got unexpected "
+                  "packet_replication_engine_entry: "
+               << entity.packet_replication_engine_entry().DebugString();
+      }
+      return GetMulticastGroupTableName();
+    }
+    default:
+      return gutil::InvalidArgumentErrorBuilder()
+             << "Expected a `table_entry` or "
+                "`packet_replication_engine_entry`, but got unexpected entity:"
+             << entity.DebugString();
+  }
+}
+
+}  // namespace pdpi

p4_pdpi/helpers.h

@@ -0,0 +1,17 @@
+#ifndef PINS_P4_PDPI_HELPERS_H_
+#define PINS_P4_PDPI_HELPERS_H_
+
+#include <string>
+
+#include "absl/status/statusor.h"
+#include "p4/v1/p4runtime.pb.h"
+#include "p4_pdpi/ir.pb.h"
+
+namespace pdpi {
+
+// Returns the table name associated with the given entity.
+absl::StatusOr<std::string> EntityToTableName(const IrP4Info& info,
+                                              const p4::v1::Entity& entity);
+}  // namespace pdpi
+
+#endif  // PINS_P4_PDPI_HELPERS_H_

p4_pdpi/helpers_test.cc

@@ -0,0 +1,59 @@
+#include "p4_pdpi/helpers.h"
+
+#include <string>
+
+#include "gmock/gmock.h"
+#include "gtest/gtest.h"
+#include "gutil/status_matchers.h"
+#include "p4/v1/p4runtime.pb.h"
+#include "p4_pdpi/built_ins.h"
+#include "p4_pdpi/ir.pb.h"
+#include "p4_pdpi/testing/test_p4info.h"
+
+namespace pdpi {
+namespace {
+
+using gutil::IsOk;
+using gutil::IsOkAndHolds;
+using testing::Not;
+
+TEST(EntityToTableNameTest, StandardTableSupported) {
+  IrP4Info kInfo = GetTestIrP4Info();
+  IrTableDefinition kTestTable = kInfo.tables_by_id().begin()->second;
+  p4::v1::Entity entity;
+  entity.mutable_table_entry()->set_table_id(kTestTable.preamble().id());
+
+  EXPECT_THAT(EntityToTableName(kInfo, entity),
+              IsOkAndHolds(kTestTable.preamble().alias()));
+}
+
+TEST(EntityToTableNameTest, MulticastTableSupported) {
+  p4::v1::Entity entity;
+  entity.mutable_packet_replication_engine_entry()
+      ->mutable_multicast_group_entry();
+
+  ASSERT_OK_AND_ASSIGN(
+      std::string multicast_group_table_name,
+      IrBuiltInTableToString(BUILT_IN_TABLE_MULTICAST_GROUP_TABLE));
+
+  EXPECT_THAT(EntityToTableName(GetTestIrP4Info(), entity),
+              IsOkAndHolds(multicast_group_table_name));
+}
+
+TEST(EntityToTableNameTest, OtherPacketReplicationEngineUnsupported) {
+  p4::v1::Entity entity;
+  entity.mutable_packet_replication_engine_entry()
+      ->mutable_clone_session_entry();
+
+  EXPECT_THAT(EntityToTableName(GetTestIrP4Info(), entity), Not(IsOk()));
+}
+
+TEST(EntityToTableNameTest, OtherEntitiesUnsupported) {
+  p4::v1::Entity entity;
+  entity.mutable_direct_counter_entry();
+
+  EXPECT_THAT(EntityToTableName(GetTestIrP4Info(), entity), Not(IsOk()));
+}
+
+}  // namespace
+}  // namespace pdpi

p4_pdpi/ir.proto

@@ -358,6 +358,10 @@ message IrP4Info {
   // Maps built-in table name to definitions. Refer to `IrBuiltInTable` for
   // valid built-in table names.
   map<string, IrBuiltInTableDefinition> built_in_tables = 13;
+  // Maps each table alias to a dependency rank such that if an entry in A can
+  // depend on an entry in B (including transitively), then
+  // dependency_rank_by_table_name[B] > dependency_rank_by_table_name[A].
+  map<string, int32> dependency_rank_by_table_name = 14;
 }
 
 // -- Table entries ------------------------------------------------------------

p4_pdpi/sequencing.cc

@@ -26,6 +26,7 @@
 #include "absl/container/flat_hash_map.h"
 #include "absl/container/flat_hash_set.h"
 #include "absl/status/status.h"
+#include "absl/status/statusor.h"
 #include "absl/strings/str_cat.h"
 #include "absl/types/span.h"
 #include "boost/graph/adjacency_list.hpp"
@@ -34,6 +35,7 @@
 #include "gutil/status.h"
 #include "p4/config/v1/p4info.pb.h"
 #include "p4/v1/p4runtime.pb.h"
+#include "p4_pdpi/helpers.h"
 #include "p4_pdpi/ir.pb.h"
 #include "p4_pdpi/sequencing_util.h"
 
@@ -332,6 +334,41 @@ absl::Status SortTableEntries(const IrP4Info& info,
   return absl::OkStatus();
 }
 
+// Returns true if the table of `first` comes before the table of `second` in
+// the dependency order contained in `info`. I.e. if installing `first` before
+// `second` never fails due to dependency issues between them.
+absl::StatusOr<bool> GreaterThanInDependencyOrder(
+    const IrP4Info& info, const p4::v1::Entity& first,
+    const p4::v1::Entity& second) {
+  ASSIGN_OR_RETURN(std::string first_table, EntityToTableName(info, first));
+  ASSIGN_OR_RETURN(std::string second_table, EntityToTableName(info, second));
+  ASSIGN_OR_RETURN(
+      int first_order,
+      gutil::FindOrStatus(info.dependency_rank_by_table_name(), first_table));
+  ASSIGN_OR_RETURN(
+      int second_order,
+      gutil::FindOrStatus(info.dependency_rank_by_table_name(), second_table));
+  return first_order > second_order;
+}
+
+absl::Status StableSortEntities(const IrP4Info& info,
+                          std::vector<p4::v1::Entity>& entities) {
+  absl::c_stable_sort(
+      entities, [&](const p4::v1::Entity& a, const p4::v1::Entity& b) {
+        auto b_may_depend_on_a = GreaterThanInDependencyOrder(info, a, b);
+        if (!b_may_depend_on_a.ok()) {
+          LOG(ERROR) << "Failed to compare entities with error: "
+                     << b_may_depend_on_a.status() << "\nEntities were:\n"
+                     << a.DebugString() << "\n\n   and   \n\n"
+                     << b.DebugString();
+          return false;
+        }
+        return *b_may_depend_on_a;
+      });
+
+  return absl::OkStatus();
+}
+
 absl::StatusOr<std::vector<p4::v1::Entity>> GetEntitiesUnreachableFromRoots(
     absl::Span<const p4::v1::Entity> entities,
     absl::FunctionRef<absl::StatusOr<bool>(const p4::v1::Entity&)>

p4_pdpi/sequencing.h

@@ -57,9 +57,17 @@ absl::StatusOr<std::vector<std::vector<int>>> SequencePiUpdatesInPlace(
 
 // Sorts the table entries such that entries that are depended on come first.
 // That is, two entries x and y where x refers to y will be sorted as [y, x].
+ABSL_DEPRECATED("Prefer StableSortEntities")
 absl::Status SortTableEntries(const IrP4Info& info,
                               std::vector<p4::v1::TableEntry>& entries);
 
+// Stably 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 rank remain in the same relative order.
+// Any entities with the same dependency rank remain in the same relative order.
+absl::Status StableSortEntities(const IrP4Info& info,
+                          std::vector<p4::v1::Entity>& entities);
+
 // Returns the subset of Entities in `entities` that is not reachable from any
 // root entity in `entities`, where a root entity is determined by the
 // `is_root_entity` function. An entity `e` is reachable from a root entity `r`